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

Consistent output between forge builds or selective output (e.g. just ABI) #1272

Closed
holic opened this issue Apr 12, 2022 · 20 comments
Closed
Labels
C-forge Command: forge Cmd-forge-build Command: forge build T-feature Type: feature

Comments

@holic
Copy link

holic commented Apr 12, 2022

Component

Forge

Describe the feature you would like

This might be a duplicate/extension of #705, so feel free to close/merge issues if so.

I'd like to be able to commit my build files into git, run forge build in CI, and detect if the output has updated (i.e. code has changed). This is to help ensure that if I make a change in solidity, CI will catch that I need to rebuild the ABI, for example. The downstream effect is that I'll use this ABI to build a typescript SDK (with TypeChain) and use that in my frontend, so I am trying to keep everything in sync/up-to-date.

I noticed that the build in CI is different from what the build outputs locally. I'm not sure if this is an OS difference or something else at play (I'm using the same foundry/forge version in CI as local), but the JSON fields I see changing are bytecode.object, deployedBytecode.object, and ast.absolutePath.

Is it possible to create consistency in each of these outputs between runs of forge build? (e.g. absolutePath becomes relative to the foundry.toml instead of absolute. Unsure what to do about bytecode.object)

Alternatively, is there a way to run forge build just outputting the e.g. ABI? I can output this to a separate directory and use that to run my "outdated build files" check, generate types, etc.

Additional context

No response

@holic holic added the T-feature Type: feature label Apr 12, 2022
@holic
Copy link
Author

holic commented Apr 12, 2022

In the meantime, I wrote a little script to extract just the ABIs to pass through TypeChain but realizing that TypeChain provides some additional helpers if the bytecode is included too. So it'd be useful to have consistent bytecode between local and CI builds.

@onbjerg
Copy link
Member

onbjerg commented Apr 12, 2022

Is the compiler used in CI and locally the same?

@holic
Copy link
Author

holic commented Apr 12, 2022

Is the compiler used in CI and locally the same?

Good question! I'm using your foundry-toolchain action (thank you for that!) in CI and used the curl install command from the readme locally, so I'm honestly not sure which compiler each is using. Any way for me to check/confirm? Any particular one I should be using?

@onbjerg
Copy link
Member

onbjerg commented Apr 12, 2022

No specific you should be using, but the GHA will install the latest matching version every time, whereas locally you probably have an older version installed (that still matches the version requirements).

Try running forge build --force - it should tell you what compiler version it is using. The same output should be present in CI as well

If you absolutely need 100% determinism you can lock down the solc version used by configuring foundry.toml:

[default]
solc_version = "0.8.13"

@holic
Copy link
Author

holic commented Apr 12, 2022

Thank you! I'll give that a try and report back.

@holic
Copy link
Author

holic commented Apr 12, 2022

Both are (and have been) reporting:

Compiling 24 files with 0.8.13

Let me know if there's anything else I can check on!

@onbjerg
Copy link
Member

onbjerg commented Apr 12, 2022

That's pretty strange - the bytecode should be consistent. Are you:

  • Using libraries?
  • On the latest Foundry?

@holic
Copy link
Author

holic commented Apr 12, 2022

  • Using libraries?

Yes: ds-test, erc721a, openzeppelin-contracts

(all via submodules, so commit hash should be the same between both environments?)

  • On the latest Foundry?

I believe both on forge 0.2.0 but may have a different timestamp? I'll double check tomorrow.

@onbjerg
Copy link
Member

onbjerg commented Apr 13, 2022

Can you get a diff of the artifacts somehow? Curious to see what's changing (especially in the bytecode). Remember to update to the latest nightly, I think there were some changes that might affect the output :)

@onbjerg onbjerg added this to Foundry Apr 17, 2022
@onbjerg onbjerg moved this to Todo in Foundry Apr 17, 2022
@onbjerg onbjerg added C-forge Command: forge Cmd-forge-build Command: forge build labels Apr 18, 2022
@xenide
Copy link

xenide commented Apr 21, 2022

I'm getting the same issue as well. My local (ARM64 MBP) build and the GH CI build (Ubuntu 20) outputs slightly different bytecodes. The difference is only in the last few bytes. For the bulk of it, there is no difference.

I have ensured that both my local version and the GitHub CI's version are the same (time slightly diff but commit hash is the same). The GH runner installs foundry via onbjerg/foundry-toolchain@v1.

@mattsse
Copy link
Member

mattsse commented Apr 21, 2022

this could be metadata hash, can you set bytecode_hash = "none" in the config and try again to confirm this is metadata hash related?

@xenide
Copy link

xenide commented Apr 21, 2022

this could be metadata hash, can you set bytecode_hash = "none" in the config and try again to confirm this is metadata hash related?

@mattsse I tried it and that was indeed the case. Somehow my local config defaulted to none (while the CI runner defaulted to ipfs) although I didn't explicitly set it.

Thanks for solving this for me, much appreciated 🙏🏾

@onbjerg
Copy link
Member

onbjerg commented Apr 21, 2022

this could be metadata hash, can you set bytecode_hash = "none" in the config and try again to confirm this is metadata hash related?

@mattsse I tried it and that was indeed the case. Somehow my local config defaulted to none (while the CI runner defaulted to ipfs) although I didn't explicitly set it.

Thanks for solving this for me, much appreciated 🙏🏾

If the defaults differ, make sure you're on the latest Foundry version 😄

Also ping @holic did you figure this out?

@onbjerg onbjerg moved this from Todo to May be solved in Foundry Apr 21, 2022
@holic
Copy link
Author

holic commented Apr 21, 2022

I tried the bytecode_hash = 'none' approach but still seeing similar things. Haven't had a chance to dig deep into why.

The absolute path differing per run is a blocker too, so I've worked around all of this by ignoring the output and just extracting the ABIs:

rm -rf abis
cp -r out abis

FILES="abis/**/*.json"
for file in $FILES; do
  echo "$(cat $file | jq -r '{abi}')" > $file
done

@onbjerg
Copy link
Member

onbjerg commented Apr 21, 2022

And you were running the same version of Forge as your CI? If possible, could you share a repository where this is reproducible?

@holic
Copy link
Author

holic commented Apr 21, 2022

Just updated my local foundry version to the latest nightly (to match with CI) and using bytecode_hash = 'none' does take care of the differing bytecode and deployedBytecode between local and CI. Everything lines up as expected now! Happy to add you to the repo if you still want to see what I am fiddling with.

What's left now is the differing ast.absolutePath. Any thoughts on ABI extraction approaches? My bash script above feels a bit brittle and feels weird to check in files that show path names in my local filesystem.

@onbjerg
Copy link
Member

onbjerg commented Apr 21, 2022

What do you need the AST for? cc @mattsse as he might have some thoughts on why the path in the AST is an absolute path (and if we can fix that)

For ABI you should be able to use --extra-output-files abi should create separate ABI files in the out folder, however it seems like that is not working. I'm creating a new issue for that so we can track that better.

@holic
Copy link
Author

holic commented Apr 21, 2022

I don't necessarily need the AST, just the ABI, so that I can generate Typescript types with TypeChain. I just couldn't find an option to generate just the ABI, so I was extracting it from the full output files (which has that ast.absolutePath).

If generating just the ABI is an option, I would certainly use that! I'll subscribe to #1380 :)

@onbjerg
Copy link
Member

onbjerg commented Apr 21, 2022

Would #1380 be sufficient to close this issue? If so, I will close this issue in favor of #1380.

@holic
Copy link
Author

holic commented Apr 21, 2022

Yep! Thanks for your help!

@holic holic closed this as completed Apr 21, 2022
Repository owner moved this from May be solved to Done in Foundry Apr 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-forge Command: forge Cmd-forge-build Command: forge build T-feature Type: feature
Projects
Archived in project
Development

No branches or pull requests

4 participants