fix(pg): throw an error if contracts are above the size limit

.changeset/curly-rules-boil.md

@@ -0,0 +1,6 @@
+---
+'@sphinx-labs/plugins': patch
+'@sphinx-labs/core': patch
+---
+
+Throw an error if contracts are above the size limit

packages/contracts/contracts/foundry/Sphinx.sol

@@ -250,6 +250,13 @@ abstract contract Sphinx {
         // the delegatecall in our error message.
         require(success, "Sphinx: Deployment script failed.");
         Vm.AccountAccess[] memory accesses = vm.stopAndReturnStateDiff();
+        // Set the `deployedCode` field for each AccountAccess of kind `Create`. There may be a bug
+        // in Foundry that causes this field to not always be populated.
+        for (uint256 i = 0; i < accesses.length; i++) {
+            if (accesses[i].kind == VmSafe.AccountAccessKind.Create) {
+                accesses[i].deployedCode = accesses[i].account.code;
+            }
+        }
 
         // We have to copy fundsRequestedForSafe and safeStartingBalance into deploymentInfo before
         // calling vm.revertTo because that cheatcode will clear the state variables.

packages/contracts/src/constants.ts

@@ -42,3 +42,5 @@ export const EXECUTION_LOCK_TIME = 15 * 60
 
 export const RECOMMENDED_REMAPPING =
   '@sphinx-labs/contracts/=lib/sphinx/packages/contracts/contracts/foundry'
+
+export const MAX_CONTRACT_SIZE_LIMIT = 24576

packages/core/src/utils.ts

@@ -35,6 +35,7 @@ import {
   recursivelyConvertResult,
   DETERMINISTIC_DEPLOYMENT_PROXY_ADDRESS,
   CreateCallArtifact,
+  MAX_CONTRACT_SIZE_LIMIT,
 } from '@sphinx-labs/contracts'
 
 import {
@@ -1709,4 +1710,13 @@ export const isPublicAsyncMethod = (
   return false
 }
 
+/**
+ * Returns true if the given bytecode exceeds the contract size size limit as defined by:
+ * https://github.com/ethereum/EIPs/blob/master/EIPS/eip-170.md
+ */
+export const exceedsContractSizeLimit = (deployedBytecode: string): boolean => {
+  const bytesLength = remove0x(deployedBytecode).length / 2
+  return bytesLength > MAX_CONTRACT_SIZE_LIMIT
+}
+
 export const sphinxCoreUtils = { sleep, callWithTimeout, isPublicAsyncMethod }

packages/plugins/contracts/test/MyContracts.sol

@@ -5,6 +5,7 @@ import { Ownable } from "@openzeppelin/contracts/access/Ownable.sol";
 import { AccessControl } from "@openzeppelin/contracts/access/AccessControl.sol";
 import { ERC721 } from "@openzeppelin/contracts/token/ERC721/ERC721.sol";
 import { Governor } from "@openzeppelin/contracts/governance/Governor.sol";
+import { SphinxUtils } from "@sphinx-labs/contracts/contracts/foundry/SphinxUtils.sol";
 
 struct TopLevelStruct {
     int256 a;
@@ -200,3 +201,5 @@ contract MyImmutableContract {
         return myFirstImmutable + uint256(mySecondImmutable);
     }
 }
+
+contract ExceedsContractMaxSizeLimit is SphinxUtils {}

packages/plugins/src/cli/deploy.ts

@@ -52,6 +52,7 @@ import {
   getSphinxConfigFromScript,
   parseScriptFunctionCalldata,
   readInterface,
+  assertContractSizeLimitNotExceeded,
   writeSystemContracts,
 } from '../foundry/utils'
 import { getFoundryToml } from '../foundry/options'
@@ -304,6 +305,11 @@ export const deploy = async (
     initCodeWithArgsArray
   )
 
+  assertContractSizeLimitNotExceeded(
+    deploymentInfo.accountAccesses,
+    configArtifacts
+  )
+
   await sphinxContext.assertNoLinkedLibraries(
     scriptPath,
     foundryToml.cachePath,

packages/plugins/src/cli/propose/index.ts

@@ -43,6 +43,7 @@ import {
   assertValidVersions,
   validateProposalNetworks,
   parseScriptFunctionCalldata,
+  assertContractSizeLimitNotExceeded,
 } from '../../foundry/utils'
 import { SphinxContext } from '../context'
 import { FoundryToml } from '../../foundry/types'
@@ -177,6 +178,13 @@ export const buildNetworkConfigArray: BuildNetworkConfigArray = async (
     initCodeWithArgsArray
   )
 
+  collected.forEach(({ deploymentInfo }) =>
+    assertContractSizeLimitNotExceeded(
+      deploymentInfo.accountAccesses,
+      configArtifacts
+    )
+  )
+
   await sphinxContext.assertNoLinkedLibraries(
     scriptPath,
     foundryToml.cachePath,

packages/plugins/src/error-messages.ts

@@ -0,0 +1,22 @@
+export const contractsExceedSizeLimitErrorMessage = (
+  contracts: Array<{ address: string; fullyQualifiedName?: string }>
+): string => {
+  const contractLines = contracts
+    .map((contract) => {
+      let line = `- `
+      if (contract.fullyQualifiedName) {
+        line += `${contract.fullyQualifiedName} at `
+      }
+      line += `${contract.address}`
+      if (!contract.fullyQualifiedName) {
+        line += ' (unlabeled)'
+      }
+      return line
+    })
+    .join('\n')
+
+  return (
+    `The following contracts are over the contract size limit (24,576 bytes), which means they\n` +
+    `cannot be deployed on live networks:\n${contractLines}`
+  )
+}

packages/plugins/src/foundry/utils/index.ts

@@ -19,6 +19,7 @@ import {
 } from '@sphinx-labs/core/dist/languages/solidity/types'
 import {
   decodeDeterministicDeploymentProxyData,
+  exceedsContractSizeLimit,
   execAsync,
   fetchNetworkConfigFromDeploymentConfig,
   formatSolcLongVersion,
@@ -101,6 +102,7 @@ import {
   getMixedNetworkTypeErrorMessage,
   getUnsupportedNetworkErrorMessage,
 } from '../error-messages'
+import { contractsExceedSizeLimitErrorMessage } from '../../error-messages'
 
 const readFileAsync = promisify(readFile)
 
@@ -1772,3 +1774,36 @@ export const validateProposalNetworks = async (
   const rpcUrls = valid.map(({ rpcUrl }) => rpcUrl)
   return { rpcUrls, isTestnet }
 }
+
+export const assertContractSizeLimitNotExceeded = (
+  accountAccesses: Array<ParsedAccountAccess>,
+  configArtifacts: ConfigArtifacts
+): void => {
+  const flat = accountAccesses.flatMap((access) => [
+    access.root,
+    ...access.nested,
+  ])
+
+  const tooLarge: Array<{ address: string; fullyQualifiedName?: string }> = []
+
+  for (const access of flat) {
+    if (
+      access.kind === AccountAccessKind.Create &&
+      exceedsContractSizeLimit(access.deployedCode)
+    ) {
+      const initCodeWithArgs = access.data
+      // Find the fully qualified name if it exists. This call returns `undefined` if the contract
+      // is unlabeled.
+      const fullyQualifiedName = findFullyQualifiedNameForInitCode(
+        initCodeWithArgs,
+        configArtifacts
+      )
+
+      tooLarge.push({ address: access.account, fullyQualifiedName })
+    }
+  }
+
+  if (tooLarge.length > 0) {
+    throw new Error(contractsExceedSizeLimitErrorMessage(tooLarge))
+  }
+}

packages/plugins/test/mocha/dummy.ts

@@ -1,4 +1,9 @@
-import { Operation, SphinxMerkleTree } from '@sphinx-labs/contracts'
+import {
+  AccountAccess,
+  AccountAccessKind,
+  Operation,
+  SphinxMerkleTree,
+} from '@sphinx-labs/contracts'
 import {
   ActionInputType,
   BuildInfo,
@@ -340,3 +345,23 @@ export const getDummyDeployment = (): Deployment => {
     treeSigners: [],
   }
 }
+
+export const getDummyAccountAccess = (): AccountAccess => {
+  return {
+    chainInfo: {
+      forkId: '0x1',
+      chainId: '0x3',
+    },
+    kind: AccountAccessKind.Balance,
+    account: '0xAccount',
+    accessor: '0xAccessor',
+    initialized: true,
+    oldBalance: '1000',
+    newBalance: '1500',
+    deployedCode: '0xCode',
+    value: '500',
+    data: '0xData',
+    reverted: false,
+    storageAccesses: [],
+  }
+}

packages/plugins/test/mocha/error-messages.spec.ts

@@ -0,0 +1,25 @@
+import { expect } from 'chai'
+
+import { contractsExceedSizeLimitErrorMessage } from '../../src/error-messages'
+
+describe('Error Messages', () => {
+  it('contractsExceedSizeLimitErrorMessage', () => {
+    const contracts = [
+      {
+        address: '0xABCDEF1234567890ABCDEF1234567890ABCDEF12',
+        fullyQualifiedName: 'contracts/ExampleContract.sol:ExampleContract',
+      },
+      { address: '0x1234567890ABCDEF1234567890ABCDEF12345678' }, // Unlabeled
+    ]
+    // eslint-disable-next-line prettier/prettier
+    const expectedMessage =
+`The following contracts are over the contract size limit (24,576 bytes), which means they
+cannot be deployed on live networks:
+- contracts/ExampleContract.sol:ExampleContract at 0xABCDEF1234567890ABCDEF1234567890ABCDEF12
+- 0x1234567890ABCDEF1234567890ABCDEF12345678 (unlabeled)`
+
+    const actualMessage = contractsExceedSizeLimitErrorMessage(contracts)
+
+    expect(actualMessage).to.equal(expectedMessage)
+  })
+})

packages/plugins/test/mocha/fake.ts

@@ -6,15 +6,18 @@ import {
   SphinxTransactionReceipt,
 } from '@sphinx-labs/core'
 import {
+  AccountAccessKind,
   ContractArtifact,
   Operation,
+  ParsedAccountAccess,
   SphinxModuleABI,
 } from '@sphinx-labs/contracts'
 import { EventFragment, ethers } from 'ethers'
 
 import {
   dummyBuildInfoId,
   dummyModuleAddress,
+  getDummyAccountAccess,
   getDummyBuildInfos,
   getDummyCompilerInput,
   getDummyEventLog,
@@ -145,3 +148,22 @@ export const getFakeDeploymentConfig = async (
     version: '0',
   }
 }
+
+export const getFakeParsedAccountAccess = (fields: {
+  kind: AccountAccessKind
+  data: string
+  account: string
+  deployedCode: string
+}): ParsedAccountAccess => {
+  const { kind, data, account, deployedCode } = fields
+  const root = getDummyAccountAccess()
+  root.kind = kind
+  root.data = data
+  root.account = account
+  root.deployedCode = deployedCode
+
+  return {
+    root,
+    nested: [],
+  }
+}

packages/plugins/test/mocha/foundry/utils.spec.ts

@@ -6,8 +6,10 @@ import chai from 'chai'
 import chaiAsPromised from 'chai-as-promised'
 import { ConstructorFragment, ethers } from 'ethers'
 import {
+  AccountAccessKind,
   ContractArtifact,
   LinkReferences,
+  MAX_CONTRACT_SIZE_LIMIT,
   parseFoundryContractArtifact,
   remove0x,
 } from '@sphinx-labs/contracts'
@@ -33,6 +35,7 @@ import * as MyContract2Artifact from '../../../out/artifacts/MyContracts.sol/MyC
 import * as MyContractWithLibrariesArtifact from '../../../out/artifacts/MyContracts.sol/MyContractWithLibraries.json'
 import * as MyImmutableContractArtifact from '../../../out/artifacts/MyContracts.sol/MyImmutableContract.json'
 import * as MyLargeContractArtifact from '../../../out/artifacts/MyContracts.sol/MyLargeContract.json'
+import * as ExceedsContractMaxSizeLimitArtifact from '../../../out/artifacts/MyContracts.sol/ExceedsContractMaxSizeLimit.json'
 import {
   encodeFunctionCalldata,
   getAnvilRpcUrl,
@@ -43,6 +46,7 @@ import {
 } from '../common'
 import { FoundryToml } from '../../../src/foundry/types'
 import {
+  assertContractSizeLimitNotExceeded,
   assertNoLinkedLibraries,
   makeGetConfigArtifacts,
   parseScriptFunctionCalldata,
@@ -59,6 +63,9 @@ import {
   getMixedNetworkTypeErrorMessage,
   getUnsupportedNetworkErrorMessage,
 } from '../../../src/foundry/error-messages'
+import { contractsExceedSizeLimitErrorMessage } from '../../../src/error-messages'
+import { dummyUnlabeledAddress } from '../dummy'
+import { getFakeConfigArtifacts, getFakeParsedAccountAccess } from '../fake'
 
 describe('Utils', async () => {
   let foundryToml: FoundryToml
@@ -1188,4 +1195,73 @@ describe('Utils', async () => {
       expect(actualCalldata).to.equal(calldata)
     })
   })
+
+  describe('assertContractSizeLimitNotExceeded', () => {
+    it('does not throw an error when contract size is less than the size limit', async () => {
+      const artifact = parseFoundryContractArtifact(MyContract2Artifact)
+      const address = makeAddress(1)
+      const fullyQualifiedName = `${artifact.sourceName}:${artifact.contractName}`
+
+      const accesses = [
+        getFakeParsedAccountAccess({
+          kind: AccountAccessKind.Create,
+          data: artifact.bytecode,
+          account: address,
+          deployedCode: artifact.deployedBytecode,
+        }),
+      ]
+      const configArtifacts = await getFakeConfigArtifacts(
+        [fullyQualifiedName],
+        foundryToml.artifactFolder
+      )
+
+      expect(() =>
+        assertContractSizeLimitNotExceeded(accesses, configArtifacts)
+      ).to.not.throw
+    })
+
+    it('throws an error when a labeled and unlabeled contract exceed the size limit', async () => {
+      const exceedsMaxSizeAddress = '0x' + '11'.repeat(20)
+      const exceedsMaxSizeArtifact = parseFoundryContractArtifact(
+        ExceedsContractMaxSizeLimitArtifact
+      )
+      const exceedsMaxSizeFullyQualifiedName = `${exceedsMaxSizeArtifact.sourceName}:${exceedsMaxSizeArtifact.contractName}`
+
+      const expectedErrorMessageInput = [
+        {
+          address: dummyUnlabeledAddress,
+        },
+        {
+          address: exceedsMaxSizeAddress,
+          fullyQualifiedName: exceedsMaxSizeFullyQualifiedName,
+        },
+      ]
+
+      const accesses = [
+        getFakeParsedAccountAccess({
+          kind: AccountAccessKind.Create,
+          data: '', // Empty so that the initCodeWithArgs doesn't match a labeled contract
+          account: dummyUnlabeledAddress,
+          deployedCode: '0x' + '00'.repeat(MAX_CONTRACT_SIZE_LIMIT + 1),
+        }),
+        getFakeParsedAccountAccess({
+          kind: AccountAccessKind.Create,
+          data: ExceedsContractMaxSizeLimitArtifact.bytecode.object,
+          account: exceedsMaxSizeAddress,
+          deployedCode:
+            ExceedsContractMaxSizeLimitArtifact.deployedBytecode.object,
+        }),
+      ]
+      const configArtifacts = await getFakeConfigArtifacts(
+        [exceedsMaxSizeFullyQualifiedName],
+        foundryToml.artifactFolder
+      )
+
+      expect(() =>
+        assertContractSizeLimitNotExceeded(accesses, configArtifacts)
+      ).to.throw(
+        contractsExceedSizeLimitErrorMessage(expectedErrorMessageInput)
+      )
+    })
+  })
 })