Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: reduce build time when running foundry migrations #11003

Merged

Conversation

arthurgousset
Copy link
Contributor

@arthurgousset arthurgousset commented May 21, 2024

Description

Before, all contracts were compiled twice during the Foundry migrations when only selected libraries had to be compiled and deployed.

Now, only libraries are compiled and deployed first, then all contracts are compiled and deployed second. The Foundry migrations take 4.8min vs 8.5min before, or a 44% improvement.

image

Source: Google Sheet > Compile times

Changes

  1. adds a new script (deploy_libraries.sh) that exclusively compiles and deploys libraries needed by subsequent contract deployments.
  2. moves any code related to libraries from create_and_migrate_anvil_devchain.sh to deploy_libraries.sh

Other changes

  1. fixes @celo-contracts remapping (although I'm not sure this was strictly necessary)
  2. track start and finish time to measure how long it takes to run the Foundry migrations entirely.

Tested

Yes, CI runs the foundry migrations. Although we don't have any integration or e2e tests using the Foundry devchain.

Related issues

Backwards compatibility

Yes, optimises migration script run time, but doesn't change migration script.

Documentation

Documented with code comments in the migration script.

To measure how long it takes to run the entire script from start to finish.
@arthurgousset arthurgousset changed the title Reduce migration script runtime Reduce time it takes to run migrations May 22, 2024
@arthurgousset
Copy link
Contributor Author

arthurgousset commented May 22, 2024

Notes-to-self:

image

Source: Google Sheet > Compile times

Currently, the create_and_migrate_anvil_devchain.sh script takes ~8.5min to run from start to finish, of which 6.6min or 77% is taken up by compilations. Only 2min or 23% is taken up by migration transactions.

It takes about ~6.6min to compile the contracts, because there are 2 compilation runs, where a single compilation takes ~3.3 minutes.

We can perhaps reduce the time it takes to run the migrations, by reducing the number of compilations.

A summary of the logs and times are below.

Summarised logs
./migrations_sol/create_and_migrate_anvil_devchain.sh
[⠰] Compiling...
[⠔] Compiling 272 files with 0.5.17
[⠒] Compiling 73 files with 0.8.19
[⠊] Solc 0.8.19 finished in 12.03s
[⠃] Solc 0.5.17 finished in 184.48s
Compiler run successful with warnings:

# ... Various solidity warning messages

    |     ^^^^^^^^^^^^^^^^^^^^

Waiting Anvil to launch on 8546...
                             _   _
                            (_) | |
      __ _   _ __   __   __  _  | |
     / _` | | '_ \  \ \ / / | | | |
    | (_| | | | | |  \ V /  | | | |
     \__,_| |_| |_|   \_/   |_| |_|

    0.2.0 (1a2e2e0 2023-11-14T00:27:18.096233000Z)
    https://github.com/foundry-rs/foundry

Available Accounts
==================

(0) "0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266" (60000.000000000000000000 ETH)

# ... List of Anvil accounts

Listening on 127.0.0.1:8546
Connection to localhost port 8546 [tcp/*] succeeded!
anvil_setLoggingEnabled
null
Anvil launched
Deploying precompiles:
anvil_setCode
null
anvil_setCode
null
Setting Registry Proxy
anvil_setCode
null

# ... various logs like above

Library flags are:  --libraries contracts/common/linkedlists/AddressSortedLinkedListWithMedian.sol:AddressSortedLinkedListWithMedian:0x5FbDB2315678afecb367f032d93F642f64180aa3 --libraries contracts/common/Signatures.sol:Signatures:0xe7f1725E7734CE288F8367e1Bb143E90bb3F0512 --libraries contracts/common/linkedlists/AddressLinkedList.sol:AddressLinkedList:0x9fE46736679d2D9a65F0992F2272dE9f3c7fa6e0 --libraries contracts/common/linkedlists/AddressSortedLinkedList.sol:AddressSortedLinkedList:0xCf7Ed3AccA5a467e9e704C703E8D87F634fB0Fc9 --libraries contracts/common/linkedlists/IntegerSortedLinkedList.sol:IntegerSortedLinkedList:0xDc64a140Aa3E981100a9becA4E685f962f0cF6C9 --libraries contracts/governance/Proposals.sol:Proposals:0x5FC8d32690cc91D4c39d9d3abcBD16989F875707

Backing up libraries

Compiling with libraries...
[⠒] Compiling...
[⠢] Compiling 272 files with 0.5.17
[⠃] Compiling 73 files with 0.8.19
[⠢] Solc 0.8.19 finished in 12.09s
[⠊] Solc 0.5.17 finished in 182.05s
Compiler run successful with warnings:

# ... Various solidity warning messages

real	3m19.793s
user	3m17.983s
sys	0m10.315s

eth_getTransactionCount
[⠃] Compiling...
[⠑] Compiling 28 files with 0.8.19
[⠃] Solc 0.8.19 finished in 1.83s
Compiler run successful with warnings:

# ... Various solidity warning messages

eth_blockNumber
eth_gasPrice
eth_getBlockByNumber
eth_chainId
eth_blockNumber
eth_gasPrice
eth_chainId
eth_getBlockByNumber
eth_getTransactionCount
eth_getBalance

# ... various logs like above

Script ran successfully.

== Logs ==
   Implementation deployed to: 0xF09E8DCA7ca021a11CC5927fC8fDD3b701Acf87e
   Calling initialize(..)
  Owner of the Registry Proxy is 0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266
   Setting on the registry contract: Registry
  Done migration registry
  Deploying:  Freezer
   Proxy deployed to: 0x91dfD4C1B1262fad0f75A38D955B42b4bC586Bc1
   Implementation deployed to: 0x871840D455493BA55F04056D557a92AB92c5901a
   Calling initialize(..)
   Setting on the registry contract: Freezer
   Done deploying: Freezer
  ------------------------------
  Deploying:  FeeCurrencyWhitelist

# ... various logs like above 

  Done registering validatora
  Adding to group...
  Making group vote for itself
`Unknown14` is above the contract size limit (40197 > 24576).
`Unknown15` is above the contract size limit (29542 > 24576).
`Unknown29` is above the contract size limit (39461 > 24576).
eth_chainId

EIP-3855 is not supported in one or more of the RPCs used.
Unsupported Chain IDs: 31337.
Contracts deployed with a Solidity version equal or higher than 0.8.20 might not work properly.
For more information, please see https://eips.ethereum.org/EIPS/eip-3855

## Setting up 1 EVM.
eth_blockNumber
eth_gasPrice
eth_chainId
eth_getBlockByNumber

# .. various logs like above

##### anvil-hardhat
✅  [Success]Hash: 0x80e80439a4c645a276c60c4d57079df0c8f90e117ded09bbe41d341e3a11a598
Block: 7
Paid: 0.00197549176170726 ETH (570599 gas * 3.46213674 gwei)

eth_getTransactionCount
eth_chainId
eth_sendTransaction
⠉ [00:00:00] [>------------------------------------] 2/256 txes (42.2s)⠁ [00:00:00] [------------------------------------] 0/1 receipts (0.0s)eth_getTransactionReceipt

    Transaction: 0xc46107b01d9d32eb4bcb6e105c487206f394d9168be3183d800b9d42b8fdbef3
    Gas used: 632352

    Block Number: 8
    Block Hash: 0x87c2f15b2c1644ee33496297fbccb2874efe3dff3c6334d0b9b333f0dcd34dea
    Block Time: "Mon, 20 May 2024 13:49:35 +0000"

eth_getTransactionByHash
eth_getTransactionReceipt
⠉ [00:00:00] [####################################] 1/1 receipts (0.0s)

# ... more logs like the above

✅  [Success]Hash: 0xa015e5318c01899f631c9fef713b4e68f44e936ced0a08a0a84f441bd78ee244
Block: 262
Paid: 0.000999918002666448 ETH (333306 gas * 3.000000008 gwei)


Transactions saved to: /Users/arthur/Documents/celo-org/celo-monorepo/packages/protocol/broadcast/Migration.s.sol/31337/run-latest.json

Sensitive values saved to: /Users/arthur/Documents/celo-org/celo-monorepo/packages/protocol/cache/Migration.s.sol/31337/run-latest.json

==========================

ONCHAIN EXECUTION COMPLETE & SUCCESSFUL.
Total Paid: 0.355899798844949214 ETH (118176092 gas * avg 3.014656079 gwei)

Transactions saved to: /Users/arthur/Documents/celo-org/celo-monorepo/packages/protocol/broadcast/Migration.s.sol/31337/run-latest.json

Sensitive values saved to: /Users/arthur/Documents/celo-org/celo-monorepo/packages/protocol/cache/Migration.s.sol/31337/run-latest.json

real	1m15.406s
user	0m33.265s
sys	0m5.051s

Total elapsed time: 512 second

Worked as expected, but stopped working because of what I think is a cache issue.

Latest best time to run script (with improvement): 338 sec or	5.6	min
Improvement margin: -34% or	2.9	min

Committing this WIP to do a clean checkout and see if it's a cache issue.
@arthurgousset
Copy link
Contributor Author

As of 545efbc, I managed to cut the time it takes to run the script by 34% or 2.9min.

It currently takes 5.6min vs 8.5min before.

I used a temporary directory to only build and deploy the libraries. This avoids building all contracts when only a few library contracts need to be built and deployed.

Next, I'll clean my branch and will mark it as ready for review.

This uses remappings instead of relative import `./`
Currently running `create_and_migrate_anvil_devchain.sh` successfully builds the libraries (fast) as expected.

It then fails with:

```sh
Deploying libraries...
Deploying library: AddressSortedLinkedListWithMedian
Error:
Found incompatible Solidity versions:
test-sol/governance/validators/IntegerSortedLinkedListMock-8.sol (>=0.8.0 <0.8.20) imports:
    contracts-0.8/common/linkedlists/IntegerSortedLinkedList.sol (>=0.8.0 <0.8.20)
    lib/openzeppelin-contracts8/contracts/utils/math/SafeMath.sol (^0.8.0)
    contracts/common/linkedlists/SortedLinkedList.sol (^0.5.13)
    lib/openzeppelin-contracts/contracts/math/SafeMath.sol (^0.5.0)
    contracts/common/linkedlists/LinkedList.sol (^0.5.13)
    lib/openzeppelin-contracts/contracts/math/SafeMath.sol (^0.5.0)
```
Previously I mistakenly updated to `@celo-contracts` which is on the wrong solidity version.
Previously the script tried to deploy contracts in the `protocol/` but the build output in the temp directory.
@arthurgousset
Copy link
Contributor Author

As of 200b9f2, the script takes 4.8min vs 8.5min before. That's a 44% improvement.

image

Source: Google Sheet > Compile times

Using `@celo-contracts` and `@celo-contracts-8` syntax with remappings broke the truffle build process.

Truffle only allows imports (with or without remapping) that are resolved within `node_modules`.

That means I don't think I can't use a directory that is not in `node_modules` (with or without remapping).

> Truffle supports dependencies installed via NPM. To import contracts from a dependency, use the following syntax
> `import "somepackage/SomeContract.sol";`
> Here, `somepackage` represents a package installed via NPM, and `SomeContract.sol` represents a Solidity source file provided by that package.

Source: https://archive.trufflesuite.com/docs/truffle/how-to/compile-contracts/#importing-dependencies-via-file-name

> Since the path didn't start with `./`, Truffle knows to look in your project's `node_modules` directory for the `example-truffle-library` folder. From there, it resolves the path to provide you the contract you requested.

Source: https://archive.trufflesuite.com/docs/truffle/how-to/package-management-via-npm/#within-your-contracts

I decided to revert the changes to the import and the need for remappings, and instead try to make the library compilation work with relative paths.

At this point
1. the truffle compilation works with

```sh
# within protocol directory
yarn build:sol
```

2. the Foundry migrations work

```sh
# within protocol directory
./migrations_sol/create_and_migrate_anvil_devchain.sh
```
@arthurgousset arthurgousset changed the title Reduce time it takes to run migrations Reduce build time to run migrations May 24, 2024
The script was breaking on CI:

```sh
mkdir: cannot create directory ‘/home/runner/work/celo-monorepo/celo-monorepo/packages/protocol/.tmp/libraries’: No such file or directory
```

Source: https://github.com/celo-org/celo-monorepo/actions/runs/9223759766/job/25377654339?pr=11003#step:18:78

That's expected since the temp directory is only created after the "deploy libraries" script is called.
@arthurgousset arthurgousset marked this pull request as ready for review May 24, 2024 15:28
@arthurgousset arthurgousset requested a review from a team as a code owner May 24, 2024 15:28
@arthurgousset
Copy link
Contributor Author

arthurgousset commented May 24, 2024

As of c18aa36:

  1. the Ganache build works ✅
  2. the Foundry migrations work ✅
  3. all tests pass ✅
  4. the Foundry migrations take 4.8min vs 8.5min, or a 44% improvement

I marked this PR as ready for review.

Copy link
Contributor

@soloseng soloseng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks Good! 🚀

I tested this, and these extra commands are not used or needed.
Also removes `TODO` comment.
@arthurgousset arthurgousset changed the title Reduce build time to run migrations Reduce build time to run Foundry migrations May 24, 2024
@arthurgousset arthurgousset changed the title Reduce build time to run Foundry migrations Reduce build time when running foundry migrations May 24, 2024
@arthurgousset arthurgousset merged commit 86c3ffb into master May 24, 2024
22 checks passed
@arthurgousset arthurgousset deleted the arthurgousset/chore/optimise-migration-script-runtime branch May 24, 2024 19:12
@arthurgousset arthurgousset changed the title Reduce build time when running foundry migrations chore: reduce build time when running foundry migrations Jun 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Reduce Foundry compile time during Foundry migrations
2 participants