Run CodeJar Code Constructors (#164)

* Run CodeJar Code Constructors Okay okay okay, after long last, we do the Solidity thing to feed the Solidity monster. Specifically, here we run CodeJar's constructor code as initCode, instead of returning it via a newly assembled script. This has advantages and drawbacks, specifically: 1. Pro: scripts can be easily verified 2. Pro: scripts can use const immutables 3. Con: scripts are less deterministic 4. Con: scripts can't be raw evm opcodes 5. Con: scripts can be more malicious (e.g. `owner = tx.origin`) Overall it looks like the pros outweigh the cons, esp. when you consider people are only running scripts they truly trust and thus (4) and (5) become pretty nullified. Weirdly aside of a few tests that were pretty degenerative (testing specific nuances of CodeJar itself), every other test case is passing. We'll need to rethink the deeper core CodeJar tests, but the vast majority of things are good with this change. Weird. Patches: * Add a constructor with const immutable check * Check creation code > 0, and add a few more test cases * Lint * Add a variety of test cases for metamorphic contracts and the wackiness they can present * refactor: scriptSource***s***, and rough test fixes--will go back over * chore: forge fmt, but on nightly this time * bugfix+test+hack: try to fix array encoding and add a test case for structhash, but this isn't working very well * chore: forge fmt * test: address FIXMEs and update outdated test cases * Fix Struct Hash Test This fixes struct hash test. As we build the struct hash otherwise in JavaScript, we need to fix a few values, which isn't a problem per se since it's testing the same thing we'd expect. Patches: * Run format * Add true external Eip-712 signature test * Move `stub` to `YulHelper` * refactor/minor: address style comments in QuarkWallet.sol * test: bring back the empty code EIP-1271 signer test case * refactor: remove InvalidScriptAddress and just fall on EmptyCode * minor: fix compiler warning about unused variable * doc: add a TODO for refactoring QuarkOperationHelper * chore: forge fmt * chore: commit new gas snapshot --------- Co-authored-by: fluffywaffles <[email protected]>
compound-finance · Feb 23, 2024 · 3a4c3f6 · 3a4c3f6
1 parent 75d28ee
commit 3a4c3f6
Show file tree

Hide file tree

Showing 44 changed files with 888 additions and 534 deletions.
diff --git a/.gas-snapshot b/.gas-snapshot
diff --git a/script/DeployQuarkWalletFactory.s.sol b/script/DeployQuarkWalletFactory.s.sol
@@ -55,11 +55,10 @@ contract DeployQuarkWalletFactory is Script {
 
         CodeJar codeJar = QuarkWallet(payable(quarkFactory.quarkWalletProxyFactory().walletImplementation())).codeJar();
 
-        ethcall = Ethcall(codeJar.saveCode(vm.getDeployedCode(string.concat("out/", "Ethcall.sol/Ethcall.json"))));
+        ethcall = Ethcall(codeJar.saveCode(vm.getCode(string.concat("out/", "Ethcall.sol/Ethcall.json"))));
         console.log("Ethcall Deployed:", address(ethcall));
 
-        multicall =
-            Multicall(codeJar.saveCode(vm.getDeployedCode(string.concat("out/", "Multicall.sol/Multicall.json"))));
+        multicall = Multicall(codeJar.saveCode(vm.getCode(string.concat("out/", "Multicall.sol/Multicall.json"))));
         console.log("Multicall Deployed:", address(multicall));
 
         console.log("=============================================================");

diff --git a/script/DeployTerminalScripts.s.sol b/script/DeployTerminalScripts.s.sol
@@ -52,40 +52,40 @@ contract DeployTerminalScripts is Script {
         console.log("Deploying Terminal Scripts");
 
         cometSupplyActions = CometSupplyActions(
-            codeJar.saveCode(vm.getDeployedCode(string.concat("out/", "LegendScript.sol/CometSupplyActions.json")))
+            codeJar.saveCode(vm.getCode(string.concat("out/", "LegendScript.sol/CometSupplyActions.json")))
         );
         console.log("CometSupplyActions Deployed:", address(cometSupplyActions));
 
         cometWithdrawActions = CometWithdrawActions(
-            codeJar.saveCode(vm.getDeployedCode(string.concat("out/", "LegendScript.sol/CometWithdrawActions.json")))
+            codeJar.saveCode(vm.getCode(string.concat("out/", "LegendScript.sol/CometWithdrawActions.json")))
         );
         console.log("CometWithdrawActions Deployed:", address(cometWithdrawActions));
 
         uniswapSwapActions = UniswapSwapActions(
-            codeJar.saveCode(vm.getDeployedCode(string.concat("out/", "LegendScript.sol/UniswapSwapActions.json")))
+            codeJar.saveCode(vm.getCode(string.concat("out/", "LegendScript.sol/UniswapSwapActions.json")))
         );
         console.log("UniswapSwapActions Deployed:", address(uniswapSwapActions));
 
         transferActions = TransferActions(
-            codeJar.saveCode(vm.getDeployedCode(string.concat("out/", "LegendScript.sol/TransferActions.json")))
+            codeJar.saveCode(vm.getCode(string.concat("out/", "LegendScript.sol/TransferActions.json")))
         );
         console.log("TransferActions Deployed:", address(transferActions));
 
         cometClaimRewards = CometClaimRewards(
-            codeJar.saveCode(vm.getDeployedCode(string.concat("out/", "LegendScript.sol/CometClaimRewards.json")))
+            codeJar.saveCode(vm.getCode(string.concat("out/", "LegendScript.sol/CometClaimRewards.json")))
         );
         console.log("CometClaimRewards Deployed:", address(cometClaimRewards));
 
         cometSupplyMultipleAssetsAndBorrow = CometSupplyMultipleAssetsAndBorrow(
             codeJar.saveCode(
-                vm.getDeployedCode(string.concat("out/", "LegendScript.sol/CometSupplyMultipleAssetsAndBorrow.json"))
+                vm.getCode(string.concat("out/", "LegendScript.sol/CometSupplyMultipleAssetsAndBorrow.json"))
             )
         );
         console.log("CometSupplyMultipleAssetsAndBorrow Deployed:", address(cometSupplyMultipleAssetsAndBorrow));
 
         cometRepayAndWithdrawMultipleAssets = CometRepayAndWithdrawMultipleAssets(
             codeJar.saveCode(
-                vm.getDeployedCode(string.concat("out/", "LegendScript.sol/CometRepayAndWithdrawMultipleAssets.json"))
+                vm.getCode(string.concat("out/", "LegendScript.sol/CometRepayAndWithdrawMultipleAssets.json"))
             )
         );
         console.log("CometRepayAndWithdrawMultipleAssets Deployed:", address(cometRepayAndWithdrawMultipleAssets));

diff --git a/src/codejar/src/CodeJar.sol b/src/codejar/src/CodeJar.sol
@@ -1,8 +1,6 @@
 // SPDX-License-Identifier: BSD-3-Clause
 pragma solidity 0.8.23;
 
-import {CodeJarStub} from "./CodeJarStub.sol";
-
 /**
  * @title Code Jar
  * @notice Deploys contract code to deterministic addresses
@@ -12,26 +10,33 @@ contract CodeJar {
     /**
      * @notice Deploys the code via Code Jar, no-op if it already exists
      * @dev This call is meant to be idemponent and fairly inexpensive on a second call
-     * @param code The runtime bytecode of the code to save
-     * @return The address of the contract that matches the input code
+     * @param code The creation bytecode of the code to save
+     * @return The address of the contract that matches the input code's contructor output
      */
-    function saveCode(bytes calldata code) external returns (address) {
+    function saveCode(bytes memory code) external returns (address) {
         address codeAddress = getCodeAddress(code);
-        bytes32 codeAddressHash = codeAddress.codehash;
 
-        if (codeAddressHash == keccak256(code)) {
-            // Code is already deployed and matches expected code
+        if (codeAddress.code.length > 0) {
+            // Code is already deployed
             return codeAddress;
         } else {
             // The code has not been deployed here (or it was deployed and destructed).
-            CodeJarStub script;
-            bytes memory initCode = abi.encodePacked(type(CodeJarStub).creationCode, code);
+            address script;
             assembly {
-                script := create2(0, add(initCode, 0x20), mload(initCode), 0)
+                script := create2(0, add(code, 0x20), mload(code), 0)
             }
 
             // Posit: these cannot fail and are purely defense-in-depth
-            require(address(script) == codeAddress);
+            require(script == codeAddress);
+
+            uint256 scriptSz;
+            assembly {
+                scriptSz := extcodesize(script)
+            }
+
+            // Disallow the empty code
+            // Note: script can still selfdestruct
+            require(scriptSz > 0);
 
             return codeAddress;
         }
@@ -45,27 +50,16 @@ contract CodeJar {
     function codeExists(bytes calldata code) external view returns (bool) {
         address codeAddress = getCodeAddress(code);
 
-        return codeAddress.code.length != 0 && codeAddress.codehash == keccak256(code);
+        return codeAddress.code.length > 0;
     }
 
     /**
-     * @dev Returns the create2 address based on CodeJarStub
-     * @return The create2 address to deploy this code (via CodeJarStub)
+     * @dev Returns the create2 address based on the creation code
+     * @return The create2 address to deploy this code (via init code)
      */
-    function getCodeAddress(bytes memory code) internal view returns (address) {
+    function getCodeAddress(bytes memory code) public view returns (address) {
         return address(
-            uint160(
-                uint256(
-                    keccak256(
-                        abi.encodePacked(
-                            bytes1(0xff),
-                            address(this),
-                            uint256(0),
-                            keccak256(abi.encodePacked(type(CodeJarStub).creationCode, code))
-                        )
-                    )
-                )
-            )
+            uint160(uint256(keccak256(abi.encodePacked(bytes1(0xff), address(this), uint256(0), keccak256(code)))))
         );
     }
 }
diff --git a/src/codejar/src/CodeJarStub.sol b/src/codejar/src/CodeJarStub.sol
diff --git a/src/quark-core/src/QuarkWallet.sol b/src/quark-core/src/QuarkWallet.sol
@@ -22,7 +22,7 @@ library QuarkWalletMetadata {
 
     /// @notice The EIP-712 typehash for authorizing an operation for this version of QuarkWallet
     bytes32 internal constant QUARK_OPERATION_TYPEHASH = keccak256(
-        "QuarkOperation(uint96 nonce,address scriptAddress,bytes scriptSource,bytes scriptCalldata,uint256 expiry)"
+        "QuarkOperation(uint96 nonce,address scriptAddress,bytes[] scriptSources,bytes scriptCalldata,uint256 expiry)"
     );
 
     /// @notice The EIP-712 typehash for authorizing an EIP-1271 signature for this version of QuarkWallet
@@ -50,7 +50,6 @@ interface HasSignerExecutor {
  * @author Compound Labs, Inc.
  */
 contract QuarkWallet is IERC1271 {
-    error AmbiguousScript();
     error BadSignatory();
     error EmptyCode();
     error InvalidEIP1271Signature();
@@ -101,16 +100,10 @@ contract QuarkWallet is IERC1271 {
     struct QuarkOperation {
         /// @notice Nonce identifier for the operation
         uint96 nonce;
-        /**
-         * @notice The address of the transaction script to run
-         * @dev Should be set as address(0) when `scriptSource` is non-empty
-         */
+        /// @notice The address of the transaction script to run
         address scriptAddress;
-        /**
-         * @notice The runtime bytecode of the transaction script to run
-         * @dev Should be set to empty bytes when `scriptAddress` is non-zero
-         */
-        bytes scriptSource;
+        /// @notice Creation codes Quark must ensure are deployed before executing this operation
+        bytes[] scriptSources;
         /// @notice Encoded function selector + arguments to invoke on the script contract
         bytes scriptCalldata;
         /// @notice Expiration time for the signature corresponding to this operation
@@ -144,27 +137,20 @@ contract QuarkWallet is IERC1271 {
             revert SignatureExpired();
         }
 
-        /*
-         * At most one of scriptAddress or scriptSource may be provided;
-         * specifying both adds cost (ie. wasted bytecode) for no benefit.
-         */
-        if ((op.scriptAddress != address(0)) && (op.scriptSource.length > 0)) {
-            revert AmbiguousScript();
-        }
-
-        /*
-         * If scriptAddress is not given, scriptSource must be non-empty
-         */
-        if (op.scriptAddress == address(0) && op.scriptSource.length == 0) {
-            revert EmptyCode();
+        bytes memory encodedArray;
+        for (uint256 i = 0; i < op.scriptSources.length;) {
+            encodedArray = abi.encodePacked(encodedArray, keccak256(op.scriptSources[i]));
+            unchecked {
+                ++i;
+            }
         }
 
         bytes32 structHash = keccak256(
             abi.encode(
                 QUARK_OPERATION_TYPEHASH,
                 op.nonce,
                 op.scriptAddress,
-                keccak256(op.scriptSource),
+                keccak256(encodedArray),
                 keccak256(op.scriptCalldata),
                 op.expiry
             )
@@ -174,15 +160,17 @@ contract QuarkWallet is IERC1271 {
         // if the signature check does not revert, the signature is valid
         checkValidSignatureInternal(HasSignerExecutor(address(this)).signer(), digest, v, r, s);
 
-        // if scriptAddress not given, derive deterministic address from bytecode
-        address scriptAddress = op.scriptAddress;
-        if (scriptAddress == address(0)) {
-            scriptAddress = codeJar.saveCode(op.scriptSource);
+        // guarantee every script in scriptSources is deployed
+        for (uint256 i = 0; i < op.scriptSources.length;) {
+            codeJar.saveCode(op.scriptSources[i]);
+            unchecked {
+                ++i;
+            }
         }
 
-        emit ExecuteQuarkScript(msg.sender, scriptAddress, op.nonce, ExecutionType.Signature);
+        emit ExecuteQuarkScript(msg.sender, op.scriptAddress, op.nonce, ExecutionType.Signature);
 
-        return stateManager.setActiveNonceAndCallback(op.nonce, scriptAddress, op.scriptCalldata);
+        return stateManager.setActiveNonceAndCallback(op.nonce, op.scriptAddress, op.scriptCalldata);
     }
 
     /**

diff --git a/test/ReplayableTransactions.t.sol b/test/ReplayableTransactions.t.sol
@@ -30,7 +30,7 @@ contract ReplayableTransactionsTest is Test {
     address aliceAccount = vm.addr(alicePrivateKey);
     QuarkWallet aliceWallet; // see constructor()
 
-    bytes recurringPurchase = new YulHelper().getDeployed("RecurringPurchase.sol/RecurringPurchase.json");
+    bytes recurringPurchase = new YulHelper().getCode("RecurringPurchase.sol/RecurringPurchase.json");
 
     // Contracts address on mainnet
     address constant USDC = 0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48;