fix(ct): remove the address field from the raw Sphinx actions

.changeset/tasty-cougars-melt.md

@@ -0,0 +1,8 @@
+---
+'@sphinx-labs/contracts': patch
+'@sphinx-labs/executor': patch
+'@sphinx-labs/plugins': patch
+'@sphinx-labs/core': patch
+---
+
+Remove the address field from the raw Sphinx actions

packages/contracts/contracts/SphinxDataTypes.sol

@@ -45,13 +45,11 @@ struct DeploymentState {
  * @custom:field index The unique index of the action in the deployment. Actions must be executed in
    ascending order according to their index.
  * @custom:field data The ABI-encoded data associated with the action.
- * @custom:field addr The address to which the action applies.
  */
 struct RawSphinxAction {
     SphinxActionType actionType;
     uint256 index;
     bytes data;
-    address payable addr;
 }
 
 /**

packages/contracts/contracts/SphinxManager.sol

@@ -556,7 +556,7 @@ contract SphinxManager is
             if (
                 !MerkleTree.verify(
                     deployment.actionRoot,
-                    keccak256(abi.encode(action.addr, action.actionType, action.data)),
+                    keccak256(abi.encode(action.actionType, action.data)),
                     action.index,
                     proof,
                     numTotalActions
@@ -568,14 +568,17 @@ contract SphinxManager is
             deployment.actionsExecuted++;
 
             if (action.actionType == SphinxActionType.CALL) {
-                (uint256 nonce, bytes memory data) = abi.decode(action.data, (uint256, bytes));
-                bytes32 callHash = keccak256(abi.encode(action.addr, data));
+                (uint256 nonce, address to, bytes memory data) = abi.decode(
+                    action.data,
+                    (uint256, address, bytes)
+                );
+                bytes32 callHash = keccak256(abi.encode(to, data));
                 uint256 currentNonce = callNonces[callHash];
                 if (nonce != currentNonce) {
                     emit CallSkipped(activeDeploymentId, action.index);
                     registry.announce("CallSkipped");
                 } else {
-                    (bool success, ) = action.addr.call(data);
+                    (bool success, ) = to.call(data);
                     if (success) {
                         callNonces[callHash] = currentNonce + 1;
                         emit CallExecuted(activeDeploymentId, action.index);
@@ -592,7 +595,10 @@ contract SphinxManager is
                     (bytes32, bytes)
                 );
 
-                address expectedAddress = action.addr;
+                address expectedAddress = ICreate3(create3).getAddressFromDeployer(
+                    salt,
+                    address(this)
+                );
 
                 // Check if the contract has already been deployed.
                 if (expectedAddress.code.length > 0) {
@@ -611,16 +617,13 @@ contract SphinxManager is
                     // Create3 contract instead of delegatecalling it, it'd be possible for an
                     // attacker to deploy a malicious contract at the expected address by calling
                     // the `deploy` function on the Create3 contract directly.
-                    (bool deploySuccess, bytes memory actualAddressBytes) = create3.delegatecall(
+                    (bool deploySuccess, ) = create3.delegatecall(
                         abi.encodeCall(ICreate3.deploy, (salt, creationCodeWithConstructorArgs, 0))
                     );
 
                     if (deploySuccess) {
-                        // Contract was deployed successfully.
-                        address actualAddress = abi.decode(actualAddressBytes, (address));
-
                         emit ContractDeployed(
-                            actualAddress,
+                            expectedAddress,
                             activeDeploymentId,
                             keccak256(creationCodeWithConstructorArgs)
                         );
@@ -771,7 +774,7 @@ contract SphinxManager is
             if (
                 !MerkleTree.verify(
                     deployment.actionRoot,
-                    keccak256(abi.encode(action.addr, action.actionType, action.data)),
+                    keccak256(abi.encode(action.actionType, action.data)),
                     action.index,
                     proof,
                     numTotalActions
@@ -782,10 +785,13 @@ contract SphinxManager is
 
             deployment.actionsExecuted++;
 
-            (bytes32 contractKindHash, bytes32 key, uint8 offset, bytes memory val) = abi.decode(
-                action.data,
-                (bytes32, bytes32, uint8, bytes)
-            );
+            (
+                bytes32 contractKindHash,
+                address to,
+                bytes32 key,
+                uint8 offset,
+                bytes memory val
+            ) = abi.decode(action.data, (bytes32, address, bytes32, uint8, bytes));
 
             if (
                 contractKindHash == IMMUTABLE_CONTRACT_KIND_HASH ||
@@ -800,13 +806,13 @@ contract SphinxManager is
             // Delegatecall the adapter to call `setStorage` on the proxy.
             // slither-disable-next-line controlled-delegatecall
             (bool success, ) = adapter.delegatecall(
-                abi.encodeCall(IProxyAdapter.setStorage, (action.addr, key, offset, val))
+                abi.encodeCall(IProxyAdapter.setStorage, (payable(to), key, offset, val))
             );
             if (!success) {
                 revert SetStorageFailed();
             }
 
-            emit SetProxyStorage(activeDeploymentId, action.addr, msg.sender, action.index);
+            emit SetProxyStorage(activeDeploymentId, to, msg.sender, action.index);
             registry.announce("SetProxyStorage");
         }
 

packages/core/src/actions/bundle.ts

@@ -51,7 +51,6 @@ import {
   HumanReadableActions,
 } from './types'
 import { getStorageLayout } from './artifacts'
-import { getCreate3Address } from '../config/utils'
 import { getProjectBundleInfo } from '../tasks'
 import { getDeployContractCosts, getEstDeployContractCost } from '../estimate'
 import { SupportedChainId } from '../networks'
@@ -67,6 +66,8 @@ export const isSetStorageAction = (
   action: SphinxAction
 ): action is SetStorageAction => {
   return (
+    (action as SetStorageAction).contractKindHash !== undefined &&
+    (action as SetStorageAction).to !== undefined &&
     (action as SetStorageAction).key !== undefined &&
     (action as SetStorageAction).value !== undefined &&
     (action as SetStorageAction).offset !== undefined
@@ -82,11 +83,15 @@ export const isSetStorageAction = (
 export const isDeployContractAction = (
   action: SphinxAction
 ): action is DeployContractAction => {
-  return (action as DeployContractAction).code !== undefined
+  return (
+    (action as DeployContractAction).creationCodeWithConstructorArgs !==
+      undefined && (action as DeployContractAction).salt !== undefined
+  )
 }
 
 export const isCallAction = (action: SphinxAction): action is CallAction => {
   return (
+    (action as CallAction).to !== undefined &&
     (action as CallAction).data !== undefined &&
     (action as CallAction).nonce !== undefined
   )
@@ -130,26 +135,35 @@ export const toRawSphinxAction = (action: SphinxAction): RawSphinxAction => {
   if (isSetStorageAction(action)) {
     return {
       actionType: SphinxActionType.SET_STORAGE,
-      addr: action.addr,
       index: action.index,
       data: coder.encode(
-        ['bytes32', 'bytes32', 'uint8', 'bytes'],
-        [action.contractKindHash, action.key, action.offset, action.value]
+        ['bytes32', 'address', 'bytes32', 'uint8', 'bytes'],
+        [
+          action.contractKindHash,
+          action.to,
+          action.key,
+          action.offset,
+          action.value,
+        ]
       ),
     }
   } else if (isDeployContractAction(action)) {
     return {
       actionType: SphinxActionType.DEPLOY_CONTRACT,
-      addr: action.addr,
       index: action.index,
-      data: coder.encode(['bytes32', 'bytes'], [action.salt, action.code]),
+      data: coder.encode(
+        ['bytes32', 'bytes'],
+        [action.salt, action.creationCodeWithConstructorArgs]
+      ),
     }
   } else if (isCallAction(action)) {
     return {
       actionType: SphinxActionType.CALL,
-      addr: action.addr,
       index: action.index,
-      data: coder.encode(['uint256', 'bytes'], [action.nonce, action.data]),
+      data: coder.encode(
+        ['uint256', 'address', 'bytes'],
+        [action.nonce, action.to, action.data]
+      ),
     }
   } else {
     throw new Error(`unknown action type`)
@@ -167,30 +181,35 @@ export const fromRawSphinxAction = (
 ): SphinxAction => {
   const coder = ethers.AbiCoder.defaultAbiCoder()
   if (rawAction.actionType === SphinxActionType.SET_STORAGE) {
-    const [contractKindHash, key, offset, value] = coder.decode(
-      ['bytes32', 'bytes32', 'uint8', 'bytes'],
+    const [contractKindHash, to, key, offset, value] = coder.decode(
+      ['bytes32', 'address', 'bytes32', 'uint8', 'bytes'],
       rawAction.data
     )
     return {
-      addr: rawAction.addr,
+      to,
       contractKindHash,
       index: rawAction.index,
       key,
       offset,
       value,
     }
   } else if (rawAction.actionType === SphinxActionType.DEPLOY_CONTRACT) {
-    const [salt, code] = coder.decode(['bytes32', 'bytes'], rawAction.data)
+    const [salt, creationCodeWithConstructorArgs] = coder.decode(
+      ['bytes32', 'bytes'],
+      rawAction.data
+    )
     return {
-      addr: rawAction.addr,
       index: rawAction.index,
       salt,
-      code,
+      creationCodeWithConstructorArgs,
     }
   } else if (rawAction.actionType === SphinxActionType.CALL) {
-    const [nonce, data] = coder.decode(['uint256', 'bytes'], rawAction.data)
+    const [nonce, to, data] = coder.decode(
+      ['uint256', 'address', 'bytes'],
+      rawAction.data
+    )
     return {
-      addr: rawAction.addr,
+      to,
       index: rawAction.index,
       data,
       nonce,
@@ -209,10 +228,7 @@ export const fromRawSphinxAction = (
 export const getActionHash = (action: RawSphinxAction): string => {
   const coder = ethers.AbiCoder.defaultAbiCoder()
   return ethers.keccak256(
-    coder.encode(
-      ['address', 'uint8', 'bytes'],
-      [action.addr, action.actionType, action.data]
-    )
+    coder.encode(['uint8', 'bytes'], [action.actionType, action.data])
   )
 }
 
@@ -517,7 +533,7 @@ export const makeActionBundleFromConfig = (
     const { artifact, buildInfo } = configArtifacts[referenceName]
     const { abi, bytecode, sourceName, contractName } = artifact
     const { isTargetDeployed } = configCache.contractConfigCache[referenceName]
-    const { kind, address, salt, constructorArgs } = contractConfig
+    const { kind, salt, constructorArgs } = contractConfig
 
     const deployContractCost = getEstDeployContractCost(
       buildInfo.output.contracts[sourceName][contractName].evm.gasEstimates
@@ -527,10 +543,9 @@ export const makeActionBundleFromConfig = (
       if (kind === 'immutable') {
         // Add a DEPLOY_CONTRACT action for the unproxied contract.
         actions.push({
-          addr: address,
           index: actionIndex,
           salt,
-          code: getCreationCodeWithConstructorArgs(
+          creationCodeWithConstructorArgs: getCreationCodeWithConstructorArgs(
             bytecode,
             constructorArgs[configCache.chainId],
             abi
@@ -541,10 +556,10 @@ export const makeActionBundleFromConfig = (
       } else if (kind === 'proxy') {
         // Add a DEPLOY_CONTRACT action for the default proxy.
         actions.push({
-          addr: address,
           index: actionIndex,
           salt,
-          code: getDefaultProxyInitCode(managerAddress),
+          creationCodeWithConstructorArgs:
+            getDefaultProxyInitCode(managerAddress),
         })
         costs.push(deployContractCost)
       } else {
@@ -579,13 +594,11 @@ export const makeActionBundleFromConfig = (
       // has a one-to-one mapping with its init code. This allows us to skip deploying implementation
       // contracts that have already been deployed.
       const implSalt = ethers.keccak256(implInitCode)
-      const implAddress = getCreate3Address(managerAddress, implSalt)
 
       actions.push({
-        addr: implAddress,
         index: actionIndex,
         salt: implSalt,
-        code: implInitCode,
+        creationCodeWithConstructorArgs: implInitCode,
       })
       costs.push(deployContractCost)
 
@@ -609,7 +622,7 @@ export const makeActionBundleFromConfig = (
       const currentNonce = configCache.callNonces[callHash]
       if (nonce >= currentNonce) {
         actions.push({
-          addr: to,
+          to,
           index: actionIndex,
           data,
           nonce,
@@ -652,7 +665,7 @@ export const makeActionBundleFromConfig = (
     // Add SET_STORAGE actions for each storage slot that we want to modify.
     for (const segment of segments) {
       actions.push({
-        addr: address,
+        to: address,
         contractKindHash: contractKindHashes[kind],
         index: actionIndex,
         key: segment.key,

packages/core/src/actions/types.ts

@@ -31,7 +31,6 @@ export interface RawSphinxAction {
   actionType: SphinxActionType
   index: number
   data: string
-  addr: string
 }
 
 export interface SphinxTarget {
@@ -44,9 +43,9 @@ export interface SphinxTarget {
  * SetStorage action data.
  */
 export interface SetStorageAction {
-  addr: string
-  contractKindHash: string
   index: number
+  to: string
+  contractKindHash: string
   key: string
   offset: number
   value: string
@@ -56,15 +55,14 @@ export interface SetStorageAction {
  * DeployContract action data.
  */
 export interface DeployContractAction {
-  addr: string
   index: number
   salt: string
-  code: string
+  creationCodeWithConstructorArgs: string
 }
 
 export interface CallAction {
-  addr: string
   index: number
+  to: string
   data: string
   nonce: number
 }