Skip to content

Commit

Permalink
fix(pg): Avoid recompiling every single time we invoke the users script
Browse files Browse the repository at this point in the history
  • Loading branch information
RPate97 committed Mar 16, 2024
1 parent 7919415 commit 24e1dcc
Show file tree
Hide file tree
Showing 7 changed files with 63 additions and 15 deletions.
5 changes: 5 additions & 0 deletions .changeset/moody-trees-sing.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@sphinx-labs/plugins': patch
---

Avoid recompiling every single time we invoke the users script
1 change: 0 additions & 1 deletion docs/cli-existing-project.md
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,6 @@ ALCHEMY_API_KEY=<your_api_key>
Update your `foundry.toml` file to include a few settings required by Sphinx. We recommend putting them in `[profile.default]`.

```
build_info = true
extra_output = ['storageLayout']
fs_permissions = [{ access = "read-write", path = "./"}]
```
Expand Down
21 changes: 19 additions & 2 deletions packages/plugins/src/cli/deploy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,10 +104,23 @@ export const deploy = async (
)
}

// Run the compiler. It's necessary to do this before we read any contract interfaces.
/**
* Run the compiler. It's necessary to do this before we read any contract interfaces.
* We force recompile and request the build info here. We need to build info to generate
* the compiler config.
*
* The current behavior of Foundry is to disregard the cache when compiling with the build info.
* This causes the users entire project to be rebuilt when the build info file is requested. So
* since we have to rebuild the entire project anyway, we also use force which will remove any
* previous build info files making our later logic for reading the build info files more performant.
*
* If/when Foundry fixes this so that the cache is used when the build info file is requested, we should
* remove force here for the best performance.
*/
compile(
silent,
false // Do not force re-compile.
true, // Force recompile
true // Generate build info
)

const scriptFunctionCalldata = await parseScriptFunctionCalldata(sig)
Expand Down Expand Up @@ -233,6 +246,10 @@ export const deploy = async (
// priority than `DAPP_BLOCK_GAS_LIMIT`.
FOUNDRY_BLOCK_GAS_LIMIT: MAX_UINT64.toString(),
ETH_FROM: safeAddress,
// We specify build info to be false so that calling the script does not cause the users entire
// project to be rebuilt if they have `build_info=true` defined in their foundry.toml file.
// We do need the build info, but that is generated when we compile at the beginning of the script.
FOUNDRY_BUILD_INFO: 'false',
})

if (spawnOutput.code !== 0) {
Expand Down
21 changes: 19 additions & 2 deletions packages/plugins/src/cli/propose/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,10 @@ export const buildNetworkConfigArray: BuildNetworkConfigArray = async (
// priority than `DAPP_BLOCK_GAS_LIMIT`.
FOUNDRY_BLOCK_GAS_LIMIT: MAX_UINT64.toString(),
ETH_FROM: safeAddress,
// We specify build info to be false so that calling the script does not cause the users entire
// project to be rebuilt if they have `build_info=true` defined in their foundry.toml file.
// We do need the build info, but that is generated when we compile at the beginning of the script.
FOUNDRY_BUILD_INFO: 'false',
})

if (spawnOutput.code !== 0) {
Expand Down Expand Up @@ -246,10 +250,23 @@ export const propose = async (
process.exit(1)
}

// Run the compiler. It's necessary to do this before we read any contract interfaces.
/**
* Run the compiler. It's necessary to do this before we read any contract interfaces.
* We force recompile and request the build info here. We need to build info to generate
* the compiler config.
*
* The current behavior of Foundry is to disregard the cache when compiling with the build info.
* This causes the users entire project to be rebuilt when the build info file is requested. So
* since we have to rebuild the entire project anyway, we also use force which will remove any
* previous build info files making our later logic for reading the build info files more performant.
*
* If/when Foundry fixes this so that the cache is used when the build info file is requested, we should
* remove force here for the best performance.
*/
compile(
silent,
false // Do not force re-compile.
true, // Force recompile
true // Generate build info
)

const scriptFunctionCalldata = await parseScriptFunctionCalldata(sig)
Expand Down
6 changes: 0 additions & 6 deletions packages/plugins/src/foundry/options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,6 @@ export const resolvePaths = (outPath: string, buildInfoPath: string) => {
}

export const checkRequiredTomlOptions = (toml: FoundryToml) => {
if (toml.buildInfo !== true) {
throw new Error(
'Missing required option in foundry.toml file:\nbuild_info = true\nPlease update your foundry.toml file and try again.'
)
}

// Check if the user included the `storageLayout` option. Since foundry force recompiles after
// changing the foundry.toml file, we can assume that the contract artifacts will contain the
// necessary info as long as the config includes the expected options
Expand Down
23 changes: 20 additions & 3 deletions packages/plugins/src/foundry/utils/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,11 @@ export const getInitCodeWithArgsArray = (
* It's fine for recompilation to occur after running the user's Forge script because Foundry
* automatically compiles the necessary contracts before executing it.
*/
export const compile = (silent: boolean, force: boolean): void => {
export const compile = (
silent: boolean,
force: boolean,
buildInfo: boolean
): void => {
const forgeBuildArgs = ['build']

if (silent) {
Expand All @@ -309,6 +313,9 @@ export const compile = (silent: boolean, force: boolean): void => {
if (force) {
forgeBuildArgs.push('--force')
}
if (buildInfo) {
forgeBuildArgs.push('--build-info')
}

// We use `spawnSync` to display the compilation process to the user as it occurs. Compiler errors
// will be displayed to the user even if the `silent` flag is included.
Expand Down Expand Up @@ -833,7 +840,12 @@ export const callForgeScriptFunction = async <T>(
code: testCode,
stdout: testOut,
stderr: testErr,
} = await spawnAsync('forge', testScriptArgs)
} = await spawnAsync('forge', testScriptArgs, {
// We specify build info to be false so that calling the script does not cause the users entire
// project to be rebuilt if they have `build_info=true` defined in their foundry.toml file.
// We do need the build info, but that is generated when we compile at the beginning of the script.
FOUNDRY_BUILD_INFO: 'false',
})

if (testCode !== 0) {
spinner?.stop()
Expand All @@ -855,7 +867,12 @@ export const callForgeScriptFunction = async <T>(
true
)

const { code, stdout, stderr } = await spawnAsync('forge', forgeScriptArgs)
const { code, stdout, stderr } = await spawnAsync('forge', forgeScriptArgs, {
// We specify build info to be false so that calling the script does not cause the users entire
// project to be rebuilt if they have `build_info=true` defined in their foundry.toml file.
// We do need the build info, but that is generated when we compile at the beginning of the script.
FOUNDRY_BUILD_INFO: 'false',
})

// For good measure, we still read the code and error if necessary but this is unlikely to be triggered
if (code !== 0) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ export const fetchForgeConfig = (
): string => `[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

0 comments on commit 24e1dcc

Please sign in to comment.