Wishlist: Bytecode Size Measuring

[transmissions11]

Component

Forge

Describe the feature you would like

forge build --sizes is cool, would be nice to build on it a bit to make it easier to work with:

• filter out test contracts
• see the delta between size limit and current
• have a flag in CI to fail the build if any of the non test contracts exceed the size limit
• some separate command to find the minimum optimizer runs that can fit your contracts into the limit

Additional context

one of the major pains when writing smart contracts is hitting the bytecode size limit. allowing ppl to catch when their contracts go over the limit as early as possible would be a major relief

[onbjerg]

From #1133 (@mds1):

Component

Forge

Describe the feature you would like

We have forge build --sizes to print a table of contract sizes. forge build --verify-sizes would error if any src contract (not test contracts) are above the size limit, with the difference being that this version is much simpler to use in CI as it does not require users to manually parse the text output

Additional context

No response

[mds1]

I want this pretty soon so might end up implementing myself if there's no objections. @onbjerg @mattsse any guidance you have on how to do this would save me a lot of time! (mainly around how to filter out the test contracts, I think the rest should be easy to figure out)

Here's my proposed implementation:

• Refactor output of forge build --sizes to display a table similar to gas report so it's easier to read
• Changes to the table:
  • No test contracts shown
  • Add a column that shows the delta between current size and max size
  • Color red if over limit, yellow if over 18kb, green otherwise (18 kB feels arbitrary, open to changing the thresholds, but over 75% seems like a good range for yellow)
• Modify forge build --sizes to always error if there's a contract over the size limit, similar to how forge test always errors if a test fails
• "some separate command to find the minimum optimizer runs that can fit your contracts into the limit" — this feels like it can be pulled into a separate issue, and probably out of scope for my current implementation plan 😅

[onbjerg]

Our definition of a test contract is pretty vague right now. I'd say you can filter out any contract that has a function that starts with test... and that's about all we can do right now 😅

The relevant snippet starts here:

foundry/cli/src/compile.rs

Line 113 in ad25304

let compiled_contracts = output.compiled_contracts_by_compiler_version();

compiled_contracts is of the type BTreeMap<Version, Vec<(String, Contract)>> where Version is the compiler version, string is the contract name (I presume, it might be the file path?) and Contract is this struct here, which contains an ABI:

https://github.com/gakonst/ethers-rs/blob/919a570237b0d92cfa34737d8803514caaa62612/ethers-solc/src/artifacts/contract.rs#L16

[transmissions11]

what about IS_TEST? https://github.com/dapphub/ds-test/blob/2c7dbcc8586b33f358e3307a443e524490c17666/src/test.sol#L38

[onbjerg]

It's not a guarantee that there is an IS_TEST, since tests don't even need to inherit from DSTest. A test can revert to signify failure, or set failed as DSTest does

[transmissions11]

imo we should change that and make some canonical test root but 🤷

[onbjerg]

I agree we should find a more concrete definition of what a test contract is, but it requires changes in a lot of places in the codebase, so I don't think this should be blocking @mds1 - also, whatever we decide on, it's likely going to be a breaking change, so we should probably bundle it with other breaking changes (e.g. breaking DT compatibility)

[transmissions11]

oh yeah def a separate issue dont wanna block this

[mds1]

Got it, all sounds good—thanks @onbjerg!