Skip to content

Conversation

@JonathanOppenheimer
Copy link
Member

@JonathanOppenheimer JonathanOppenheimer commented Nov 21, 2025

Why this should be merged

This PR accomplishes three things:

  1. Prevent files outside of graft/ directory from importing old, upstream geth files
    • Doing so violates the terms of the license and should absolutely be avoided.
  2. Prevent files inside of the vms/evm directory from importing packages within the graft/ directory.
    • This is important because with the removal of coreth, there is not longer a natural boundary between the uplifted and non-uplifted code. We need to enforce such a boundary.
  3. Add the EVM "libevm allowed import testing"
    • This is not quite as important because the test will exist within graft/* and be enforced, but moving it here will prevent the testing from being duplicated between coreth and subnet-evm
    • I also, frankly, added it because the import inspection code is practically identical, and I would have found it somewhat backwards to copy over the import inspection to code to what would be a 4th location.

How this works

The boundaries are as follows:

  • graft/coreth can be imported everywhere in AvalancheGo except vms/evm.
    • However, vms/evm/emulate is an exception to this rule
  • graft/subnet-evm can not be imported anywhere in AvalancheGo
    • However, vms/evm/emulate is an exception to this rule

How this was tested

Existing CI. I also added a sample "bad" commit (404acd5) to show how CI would behave.
Screenshot 2025-11-21 at 6 34 17 PM

edit: this is no longer a good example -- pretend the file is in vms/evm/sample_bad.go.

Need to be documented in RELEASES.md?

No

@JonathanOppenheimer JonathanOppenheimer self-assigned this Nov 21, 2025
@JonathanOppenheimer JonathanOppenheimer added the testing This primarily focuses on testing label Nov 21, 2025
@JonathanOppenheimer JonathanOppenheimer marked this pull request as ready for review November 21, 2025 23:37

// TestLibevmImportsAreAllowed ensures that all libevm imports in the graft directory
// are explicitly allowed via the libevm-allowed-packages.txt file.
func TestLibevmImportsAreAllowed(t *testing.T) {
Copy link
Member Author

Choose a reason for hiding this comment

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

See here for the source of the code below.

@@ -0,0 +1,57 @@
# Forbidden geth upstream files that cannot be imported outside the graft directory.
Copy link
Member Author

@JonathanOppenheimer JonathanOppenheimer Nov 22, 2025

Choose a reason for hiding this comment

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

I considered translating both of these files to .json to make the loading and parsing simpler in loadPatternFile().

However, the text files are very human readable and easy to maintain. Would like any opinions about the tradeoffs here!

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

Labels

testing This primarily focuses on testing

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

2 participants