Skip to content

test: use generated ABI#4740

Merged
maru-ava merged 3 commits intomasterfrom
JonathanOppenheimer/use-new-abi
Dec 19, 2025
Merged

test: use generated ABI#4740
maru-ava merged 3 commits intomasterfrom
JonathanOppenheimer/use-new-abi

Conversation

@JonathanOppenheimer
Copy link
Member

@JonathanOppenheimer JonathanOppenheimer commented Dec 15, 2025

Why this should be merged

Currently, the committed ABI files are 2 years old, and there is no clear path of how they were generated. This PR deletes those files, and commits the files generated by compile.go instead, establishing a source of truth for the generated .abi files. Additionally the freshness of these files is added to and checked by check-generate-bindings which run as part of CI.

How this was tested

CI

Need to be documented?

No

Need to update RELEASES.md?

No

@JonathanOppenheimer JonathanOppenheimer self-assigned this Dec 15, 2025
@JonathanOppenheimer JonathanOppenheimer requested a review from a team as a code owner December 15, 2025 21:57
@JonathanOppenheimer JonathanOppenheimer added evm Related to EVM functionality subnet-evm Related to the former subnet-evm standalone repository testing This primarily focuses on testing labels Dec 15, 2025
@JonathanOppenheimer
Copy link
Member Author

See this PR for further context: ava-labs/subnet-evm#1912

@maru-ava
Copy link
Contributor

Would it make sense to add a task that generates these files and another for validating that the files are up-to-date, as per the example of other generated files? It's not clear to me what the value of doing a one-time update would be that is not validated by CI. What am I missing?

@JonathanOppenheimer
Copy link
Member Author

JonathanOppenheimer commented Dec 19, 2025

Would it make sense to add a task that generates these files and another for validating that the files are up-to-date, as per the example of other generated files? It's not clear to me what the value of doing a one-time update would be that is not validated by CI. What am I missing?

This is actually already checked but admittedly not clear through just the text of this PR. The check-generate-bindings job does this:

  1. go generate ./precompile/... runs all go:generate directives, including the solc ones that produce .abi files
  2. check-clean-branch checks if any files changed in git (which includes the .abi files)

The only difference is that .abi files aren't deleted before regeneration like the gen_*binding.go files are, as they're directly overwritten with solc --overwrite

@JonathanOppenheimer
Copy link
Member Author

I will add explicit deletion to catch orphans.

Copy link
Contributor

@maru-ava maru-ava left a comment

Choose a reason for hiding this comment

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

Thank you for the generation cleanup. I guess this begs the question of why all of our go generate tasks aren't deleting before generating?

@maru-ava maru-ava added this pull request to the merge queue Dec 19, 2025
Merged via the queue into master with commit dee39dd Dec 19, 2025
52 checks passed
@maru-ava maru-ava deleted the JonathanOppenheimer/use-new-abi branch December 19, 2025 17:30
@JonathanOppenheimer
Copy link
Member Author

Thank you for the generation cleanup. I guess this begs the question of why all of our go generate tasks aren't deleting before generating?

They absolutely should be -- if they were, this issue would have been caught a long time ago, as these .abi files have been orphaned for 2+ years. I will file an issue for this.

@JonathanOppenheimer
Copy link
Member Author

Issue: #4778

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

evm Related to EVM functionality subnet-evm Related to the former subnet-evm standalone repository testing This primarily focuses on testing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants