Skip to content

Conversation

@kaze-cow
Copy link
Contributor

Description

On my other PR #3937 , I was having difficulty generating the solidity JSONs, and when I did generate them, the format was not correct. I realized that part of the problem is that the CI should be natively checking for this, but it currently does not, so I added a job.

Additionally, my computer is ARM64, but the ethereum solidity image is only compatible with x86_64, so I had to add a flag to fix this.

Finally, the format followed by the other files in the artifacts JSON directory uses a pretty-printed JSON format, but the existing Makefile uses compact format with jq -c. So I removed the -c flag to ensure this consistent format.

It turns out taht, likely due to a newer solidity version, many of the artifacts have changed slightly since they were originally built. So that is included as part of this PR.

One other issue that can occur is if the artifact file already exists on your computer, it may fail to write the new version since make only builds files that don't exist (or something like that). To resolve this, the -B option is used to force rebuilding of all targets.

Changes

  • modify the Makefile as described above
  • add CI job
  • Ensure the CI job can run properly on ARM64
  • run cd crates/contracts/solidity && make -B artifacts

How to test

Check the new CI job solidity-artifacts

Run make -B artifacts from crates/contracts/solidity to see how it behaves on your own machine.

@kaze-cow kaze-cow self-assigned this Dec 18, 2025
@kaze-cow kaze-cow requested a review from a team as a code owner December 18, 2025 06:39
Copy link
Contributor

@MartinquaXD MartinquaXD left a comment

Choose a reason for hiding this comment

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

Just wanted to flag that this has the potential to cause issues with old forked node tests in the future.
Let's say we have a forked node test that relies on the state before a hardfork introducing a new opcode and a test that either relies on the post-hardfork state or the new opcode. Then this action that will compile all artifacts to the same EVM target version will ensure that one of those tests will always fail.
I believe there was actually one instance of this in the past but it was resolved by updating the forked node test to work on a later point in time.

That being said I think this issue is rare enough that we can merge this PR anyway.

@kaze-cow kaze-cow added this pull request to the merge queue Dec 18, 2025
Merged via the queue into main with commit 869d3ea Dec 18, 2025
19 checks passed
@kaze-cow kaze-cow deleted the feat/verify-artifacts branch December 18, 2025 11:52
@github-actions github-actions bot locked and limited conversation to collaborators Dec 18, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants