Merge pull request #53 from flashbots/remove-xfam-attrs-bitmasks-and-add-upgrade-script

remove xfam + tdAttributes bitmasking logic, and add upgrade script +…

Author: Melvillian

README.md

@@ -259,3 +259,23 @@ Then, to execute, run:
 ```
 forge script --chain 1301 script/Interactions.s.sol:AddWorkloadToPolicyScript --rpc-url $RPC_URL --broadcast --verify --interactives 1 -vvvv
 ```
+
+## Upgrade
+
+### UpgradeBlockBuilderFromV1
+
+#### Reason For Upgrade
+
+This is nearly identical to the latest version of the policy contract located at src/BlockBuilderPolicy contract, except in the latest has had the logic around the xfam and tdattributes bit masking removed. This was done because there was a bug in the bit masking logic, and we want to fix the bug and simplify the contract by removing the bit masking logic.
+
+#### Deploy Command
+
+Run the command below, then paste in the private key of the address you want to use to pay for gas and execute the deployment:
+
+```
+forge script script/UpgradeBlockBuilderFromV1.s.sol:UpgradeBlockBuilderPolicyV1 \
+  --sig "run(address)" <POLICY_PROXY_ADDRESS> \
+  --rpc-url <RPC_URL> \
+  -vvvvv --verify --broadcast --interactives 1
+```
+

script/UpgradeBlockBuilderFromV1.s.sol

@@ -0,0 +1,54 @@
+// SPDX-License-Identifier: MIT
+pragma solidity ^0.8.20;
+
+import {Script, console} from "forge-std/Script.sol";
+import {Upgrades, Options} from "openzeppelin-foundry-upgrades/Upgrades.sol";
+import {BlockBuilderPolicy} from "../src/BlockBuilderPolicy.sol";
+
+/**
+ * @title UpgradeBlockBuilderFromV1
+ * @notice Upgrade script for BlockBuilderPolicy contract from V1 (the original version of the contract)
+ * @notice This is nearly identical to the latest version of the policy contract located at
+ * src/BlockBuilderPolicy contract, except in the latest has had the logic around the xfam and tdattributes bit
+ * masking removed. This was done because there was a bug in the bit masking logic, and we want to fix the bug
+ * and simplify the contract by removing the bit masking logic
+ * @dev This script does not require any reinitialization of the contract, as the the only changes to
+ * the contract are removing constant variables and changing the workloadIdForTDRegistration function logic
+ * @dev This script:
+ *      1. Deploys a new BlockBuilderPolicy implementation contract
+ *      2. Upgrades the existing UUPS proxy to point to the new implementation
+ */
+contract UpgradeBlockBuilderPolicyV1 is Script {
+    /**
+     * @notice uses environment variables to get the proxy address of the BlockBuilderPolicy contract
+     * @dev the BLOCK_BUILDER_POLICY_PROXY_ADDRESS env var is the address of the proxy contract for the BlockBuilderPolicy contract
+     */
+    function run() external {
+        address proxyAddress = vm.envAddress("BLOCK_BUILDER_POLICY_PROXY_ADDRESS");
+        run(proxyAddress);
+    }
+
+    function run(address proxyAddress) public {
+        console.log("=== UpgradeBlockBuilderFromV1 Configuration ===");
+        console.log("Proxy address:", proxyAddress);
+        console.log("");
+
+        // Spot check the proxy contract by calling the registry function
+        // This is a safety check to ensure the contract at the proxy address
+        // implements IBlockBuilderPolicy as expected
+        address proxyRegistry = BlockBuilderPolicy(proxyAddress).registry();
+        require(proxyRegistry != address(0), "proxyAddress is not a BlockBuilderPolicy contract");
+
+        vm.startBroadcast();
+
+        // Upgrade the proxy to the new implementation
+        Options memory opts;
+        opts.referenceContract = "V1BlockBuilderPolicy.sol:V1BlockBuilderPolicy";
+        Upgrades.upgradeProxy(proxyAddress, "BlockBuilderPolicy.sol", bytes(""), opts);
+
+        vm.stopBroadcast();
+
+        console.log("=== Upgrade Complete ===");
+        console.log("");
+    }
+}

src/BlockBuilderPolicy.sol

@@ -47,22 +47,6 @@ contract BlockBuilderPolicy is
     bytes32 public constant VERIFY_BLOCK_BUILDER_PROOF_TYPEHASH =
         keccak256("VerifyBlockBuilderProof(uint8 version,bytes32 blockContentHash,uint256 nonce)");
 
-    // ============ TDX workload constants ============
-
-    /// @dev See section 11.5.3 in TDX Module v1.5 Base Architecture Specification https://www.intel.com/content/www/us/en/content-details/733575/intel-tdx-module-v1-5-base-architecture-specification.html
-    /// @notice Enabled FPU (always enabled)
-    bytes8 constant TD_XFAM_FPU = 0x0000000000000001;
-    /// @notice Enabled SSE (always enabled)
-    bytes8 constant TD_XFAM_SSE = 0x0000000000000002;
-
-    /// @dev See section 3.4.1 in TDX Module ABI specification https://cdrdv2.intel.com/v1/dl/getContent/733579
-    /// @notice Allows disabling of EPT violation conversion to #VE on access of PENDING pages. Needed for Linux
-    bytes8 constant TD_TDATTRS_VE_DISABLED = 0x0000000010000000;
-    /// @notice Enabled Supervisor Protection Keys (PKS)
-    bytes8 constant TD_TDATTRS_PKS = 0x0000000040000000;
-    /// @notice Enabled Key Locker (KL)
-    bytes8 constant TD_TDATTRS_KL = 0x0000000080000000;
-
     // ============ Storage Variables ============
 
     /// @notice Mapping from workloadId to its metadata (commit hash and source locators)
@@ -227,12 +211,6 @@ contract BlockBuilderPolicy is
         override
         returns (WorkloadId)
     {
-        // We expect FPU and SSE xfam bits to be set, and anything else should be handled by explicitly allowing the workloadid
-        bytes8 expectedXfamBits = TD_XFAM_FPU | TD_XFAM_SSE;
-
-        // We don't mind VE_DISABLED, PKS, and KL tdattributes bits being set either way, anything else requires explicitly allowing the workloadid
-        bytes8 ignoredTdAttributesBitmask = TD_TDATTRS_VE_DISABLED | TD_TDATTRS_PKS | TD_TDATTRS_KL;
-
         return WorkloadId.wrap(
             keccak256(
                 bytes.concat(
@@ -243,8 +221,8 @@ contract BlockBuilderPolicy is
                     registration.parsedReportBody.rtMr3,
                     // VMM configuration
                     registration.parsedReportBody.mrConfigId,
-                    registration.parsedReportBody.xFAM ^ expectedXfamBits,
-                    registration.parsedReportBody.tdAttributes & ~ignoredTdAttributesBitmask
+                    registration.parsedReportBody.xFAM,
+                    registration.parsedReportBody.tdAttributes
                 )
             )
         );