Skip to content

Commit

Permalink
fix(ct): Prevent forge build --sizes from failing due to `SphinxUti…
Browse files Browse the repository at this point in the history
…ls.sol`
  • Loading branch information
sam-goldman committed Apr 4, 2024
1 parent dba5277 commit dd0cfcc
Show file tree
Hide file tree
Showing 7 changed files with 163 additions and 59 deletions.
7 changes: 7 additions & 0 deletions .changeset/hot-pugs-grin.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'@sphinx-labs/contracts': patch
'@sphinx-labs/plugins': patch
'@sphinx-labs/demo': patch
---

Prevent `forge build --sizes` from failing due to `SphinxUtils.sol`
38 changes: 1 addition & 37 deletions packages/contracts/contracts/foundry/Sphinx.sol
Original file line number Diff line number Diff line change
Expand Up @@ -50,17 +50,6 @@ import { SphinxForkCheck } from "./SphinxForkCheck.sol";
abstract contract Sphinx {
Vm private constant vm = Vm(address(uint160(uint256(keccak256("hevm cheat code")))));

// These are constants thare are used when signing an EIP-712 meta transaction.
bytes32 private constant DOMAIN_SEPARATOR =
keccak256(
abi.encode(
keccak256("EIP712Domain(string name,string version)"),
keccak256(bytes("Sphinx")),
keccak256(bytes("1.0.0"))
)
);
bytes32 private constant TYPE_HASH = keccak256("MerkleRoot(bytes32 root)");

/**
* @dev The configuration options for the user's project. This variable must have `internal`
* visibility so that the user can set fields on it.
Expand Down Expand Up @@ -104,7 +93,7 @@ abstract contract Sphinx {
require(utilsAddr != address(0), "Sphinx: SphinxUtils deployment failed");
require(constantsAddr != address(0), "Sphinx: SphinxConstants deployment failed");
sphinxUtils = SphinxUtils(utilsAddr);
constants = SphinxUtils(constantsAddr);
constants = SphinxConstants(constantsAddr);

// This ensures that these contracts stay deployed in a multi-fork environment (e.g. when
// calling `vm.createSelectFork`).
Expand Down Expand Up @@ -280,31 +269,6 @@ abstract contract Sphinx {
sphinxUtils.finalizeDeploymentInfo(deploymentInfo, accesses, _callDepth, address(this));
}

/**
* @notice Executes a single transaction that deploys a Gnosis Safe, deploys a Sphinx Module,
* and enables the Sphinx Module in the Gnosis Safe
*
* @dev We refer to this function in Sphinx's documentation. Make sure to update the
* documentation if you change the name of this function or change its file
* location.
*/
function _sphinxDeployModuleAndGnosisSafe() private {
IGnosisSafeProxyFactory safeProxyFactory = IGnosisSafeProxyFactory(
constants.safeFactoryAddress()
);
address singletonAddress = constants.safeSingletonAddress();

bytes memory safeInitializerData = sphinxUtils.getGnosisSafeInitializerData(address(this));

// This is the transaction that deploys the Gnosis Safe, deploys the Sphinx Module,
// and enables the Sphinx Module in the Gnosis Safe.
safeProxyFactory.createProxyWithNonce(
singletonAddress,
safeInitializerData,
sphinxConfig.saltNonce
);
}

/**
* @notice A modifier that the user must include on their entry point function when using Sphinx.
* This modifier mainly performs validation on the user's configuration and environment.
Expand Down
6 changes: 6 additions & 0 deletions packages/contracts/contracts/foundry/SphinxUtils.sol
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,12 @@ interface ISphinxScript {
}

contract SphinxUtils is SphinxConstants, StdUtils {
// Ensures that this contract doesn't cause `forge build --sizes` to fail if this command is
// executed by the user. For context, see: https://github.com/foundry-rs/foundry/issues/4615
// Resolves:
// https://linear.app/chugsplash/issue/CHU-891/prevent-the-users-forge-build-sizes-call-from-failing-due-to
bool public IS_SCRIPT = true;

Vm private constant vm = Vm(address(uint160(uint256(keccak256("hevm cheat code")))));

// Source: https://github.com/Arachnid/deterministic-deployment-proxy
Expand Down
80 changes: 61 additions & 19 deletions packages/contracts/test/SphinxInitCode.sol

Large diffs are not rendered by default.

1 change: 0 additions & 1 deletion packages/demo/foundry.toml
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
[profile.default]
script = 'script'
test = 'test'
build_info = true
extra_output = ['storageLayout']
fs_permissions=[{access="read", path="./out"}, {access="read-write", path="./cache"}]
allow_paths = ["../.."]
Expand Down
87 changes: 86 additions & 1 deletion packages/demo/test/Solc.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,80 @@ describe('Solidity Compiler', () => {
deleteForgeProject(contractPath, scriptPath, testPath)
})

// Test that we can compile a sample contract without the optimizer and without the IR compilation
// setting. This is particularly useful for checking that there aren't any contracts in our
// Foundry plugin that are included in the user's compilation process and exceed the contract size
// limit. If this occurs, the user's `forge build --sizes` call will throw an error. See:
// https://linear.app/chugsplash/issue/CHU-891/prevent-the-users-forge-build-sizes-call-from-failing-due-to
//
// This test case is significantly faster than those compiled with IR or the optimizer enabled.
it('Compiles with optimizer disabled and without IR', async () => {
const versions = generateSemverRange(new SemVer('0.8.0'), latestSolcVersion)

// Generate output directory names. We use separate output directories for each compilation to
// prevent race conditions.
const outputDirs = versions.map((version) => `out-${version}`)
// Generate separate cache directory names to prevent race conditions.
const cacheDirs = versions.map((version) => `cache-${version}`)

const buildPromises = versions.map((version, index) => {
return limit(async () => {
return spawnAsync(
`forge`,
[
'build',
'--use',
version,
'--force',
'--out',
outputDirs[index],
'--cache-path',
cacheDirs[index],
'--sizes', // Throw an error if there are any contracts above the size limit.
],
{
FOUNDRY_PROFILE: 'no_optimizer',
}
).then(({ stdout, stderr, code }) => {
return { version, stdout, stderr, code }
})
})
})

const results = await Promise.all(buildPromises)

const errorMessages: Array<string> = []
results.forEach(({ version, stdout, stderr, code }) => {
if (code !== 0) {
if (stderr.includes('Unknown version provided')) {
console.log(
`Detected solc version not currently supported by foundry: ${version}`
)
} else {
errorMessages.push(
`Build failed for ${version} with optimizer and via IR disabled.\nSTDOUT: ${stdout}\nSTDERR: ${stderr}`
)
}
}
})

// Delete the output directories
outputDirs.forEach((dir) => rmSync(dir, { recursive: true, force: true }))

if (errorMessages.length > 0) {
console.error(errorMessages.join('\n\n'))
}

// If no errors, the test passes
expect(errorMessages).to.be.empty

if (errorMessages.length > 0) {
for (const error of errorMessages) {
console.error(error)
}
}
})

// Test that we can compile the Sphinx plugin contracts for solc versions ^0.8.1 using `viaIR` and
// the optimizer. We don't test v0.8.0 because the sample Forge project can't compile with this
// version and settings.
Expand Down Expand Up @@ -113,6 +187,11 @@ describe('Solidity Compiler', () => {
outputDirs[index],
'--cache-path',
cacheDirs[index],
'--sizes', // Throw an error if there are any contracts above the size limit. It may not be necessary
// to check this here since we have a separate test case that checks this while disabling
// IR and the optimizer, which should always produce larger contracts. We check it here
// anyways out of an abundance of caution. Resolves:
// https://linear.app/chugsplash/issue/CHU-891/prevent-the-users-forge-build-sizes-call-from-failing-due-to
]).then(({ stdout, stderr, code }) => {
return { version, stdout, stderr, code }
})
Expand Down Expand Up @@ -180,6 +259,12 @@ describe('Solidity Compiler', () => {
outputDirs[index],
'--cache-path',
cacheDirs[index],
'--sizes', // Throw an error if there are any contracts above the size limit. It may not be necessary
// to check this here since we have a separate test case that checks this
// while disabling IR and the optimizer, which should always produce larger
// contracts. We check it here anyways out of an abundance of caution.
// Resolves:
// https://linear.app/chugsplash/issue/CHU-891/prevent-the-users-forge-build-sizes-call-from-failing-due-to
],
{
FOUNDRY_PROFILE: 'no_optimizer',
Expand All @@ -201,7 +286,7 @@ describe('Solidity Compiler', () => {
)
} else {
errorMessages.push(
`Build failed for ${version} with optimizer enabled.\nSTDOUT: ${stdout}\nSTDERR: ${stderr}`
`Build failed for ${version} with optimizer disabled.\nSTDOUT: ${stdout}\nSTDERR: ${stderr}`
)
}
}
Expand Down
3 changes: 2 additions & 1 deletion packages/plugins/src/sample-project/sample-contracts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,11 @@ export const getSampleScriptFile = (
return `// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;
import { Script } from "forge-std/Script.sol";
import { HelloSphinx } from "${relativeSrcPath}/HelloSphinx.sol";
import "@sphinx-labs/contracts/SphinxPlugin.sol";
contract HelloSphinxScript is Sphinx {
contract HelloSphinxScript is Sphinx, Script {
HelloSphinx helloSphinx;
function configureSphinx() public override {
Expand Down

0 comments on commit dd0cfcc

Please sign in to comment.