diff --git a/graft/coreth/plugin/evm/imports_test.go b/graft/coreth/plugin/evm/imports_test.go deleted file mode 100644 index 05c73e69d76c..000000000000 --- a/graft/coreth/plugin/evm/imports_test.go +++ /dev/null @@ -1,146 +0,0 @@ -// Copyright (C) 2019-2025, Ava Labs, Inc. All rights reserved. -// See the file LICENSE for licensing terms. - -package evm - -import ( - "bufio" - "fmt" - "go/parser" - "go/token" - "os" - "path/filepath" - "regexp" - "slices" - "strings" - "testing" - - "github.com/stretchr/testify/require" - - "github.com/ava-labs/avalanchego/utils/set" -) - -// TestLibevmImportsAreAllowed ensures that all libevm imports in the codebase -// are explicitly allowed via the eth-allowed-packages.txt file. -func TestLibevmImportsAreAllowed(t *testing.T) { - allowedPackages, err := loadAllowedPackages("../../scripts/eth-allowed-packages.txt") - require.NoError(t, err, "Failed to load allowed packages") - - // Find all libevm imports in source files with proper filtering - foundImports, err := findFilteredLibevmImportsWithFiles("../..") - require.NoError(t, err, "Failed to find libevm imports") - - var disallowedImports set.Set[string] - for importPath := range foundImports { - if !allowedPackages.Contains(importPath) { - disallowedImports.Add(importPath) - } - } - - if len(disallowedImports) == 0 { - return - } - - // After this point, there are disallowed imports, and the test will fail. - // The remaining code is just necessary to pretty-print the error message, - // to make it easier to find and fix the disallowed imports. - sortedDisallowed := disallowedImports.List() - slices.Sort(sortedDisallowed) - - var errorMsg strings.Builder - errorMsg.WriteString("New libevm imports should be added to ./scripts/eth-allowed-packages.txt to prevent accidental imports:\n\n") - for _, importPath := range sortedDisallowed { - files := foundImports[importPath] - fileList := files.List() - slices.Sort(fileList) - - errorMsg.WriteString(fmt.Sprintf("- %s\n", importPath)) - errorMsg.WriteString(fmt.Sprintf(" Used in %d file(s):\n", len(fileList))) - for _, file := range fileList { - errorMsg.WriteString(fmt.Sprintf(" • %s\n", file)) - } - errorMsg.WriteString("\n") - } - require.Fail(t, errorMsg.String()) -} - -// loadAllowedPackages reads the allowed packages from the specified file -func loadAllowedPackages(filename string) (set.Set[string], error) { - file, err := os.Open(filename) - if err != nil { - return nil, fmt.Errorf("failed to open allowed packages file: %w", err) - } - defer file.Close() - - allowed := set.Set[string]{} - scanner := bufio.NewScanner(file) - for scanner.Scan() { - line := strings.TrimSpace(scanner.Text()) - if line == "" || strings.HasPrefix(line, "#") { - continue - } - - line = strings.Trim(line, `"`) - allowed.Add(line) - } - - if err := scanner.Err(); err != nil { - return nil, fmt.Errorf("failed to read allowed packages file: %w", err) - } - - return allowed, nil -} - -// findFilteredLibevmImportsWithFiles finds all libevm imports in the codebase, -// excluding underscore imports and "eth*" named imports. -// Returns a map of import paths to the set of files that contain them -func findFilteredLibevmImportsWithFiles(rootDir string) (map[string]set.Set[string], error) { - imports := make(map[string]set.Set[string]) - libevmRegex := regexp.MustCompile(`^github\.com/ava-labs/libevm/`) - - err := filepath.Walk(rootDir, func(path string, _ os.FileInfo, err error) error { - if err != nil || !strings.HasSuffix(path, ".go") { - return err - } - - // Skip generated files, main_test.go, and tempextrastest directory - filename := filepath.Base(path) - if strings.HasPrefix(filename, "gen_") || strings.Contains(path, "core/main_test.go") || strings.Contains(path, "tempextrastest/") { - return nil - } - - node, err := parser.ParseFile(token.NewFileSet(), path, nil, parser.ParseComments) - if err != nil { - return fmt.Errorf("failed to parse %s: %w", path, err) - } - - for _, imp := range node.Imports { - if imp.Path == nil { - continue - } - - importPath := strings.Trim(imp.Path.Value, `"`) - if !libevmRegex.MatchString(importPath) { - continue - } - - // Skip underscore and "eth*" named imports - if imp.Name != nil && (imp.Name.Name == "_" || strings.HasPrefix(imp.Name.Name, "eth")) { - continue - } - - if _, exists := imports[importPath]; !exists { - imports[importPath] = set.Set[string]{} - } - fileSet := imports[importPath] - fileSet.Add(path) - imports[importPath] = fileSet - } - return nil - }) - if err != nil { - return nil, err - } - - return imports, nil -} diff --git a/graft/coreth/scripts/eth-allowed-packages.txt b/graft/coreth/scripts/eth-allowed-packages.txt deleted file mode 100644 index 866a9eaa3c9e..000000000000 --- a/graft/coreth/scripts/eth-allowed-packages.txt +++ /dev/null @@ -1,46 +0,0 @@ -"github.com/ava-labs/libevm/accounts" -"github.com/ava-labs/libevm/accounts/external" -"github.com/ava-labs/libevm/accounts/keystore" -"github.com/ava-labs/libevm/accounts/scwallet" -"github.com/ava-labs/libevm/common" -"github.com/ava-labs/libevm/common/bitutil" -"github.com/ava-labs/libevm/common/compiler" -"github.com/ava-labs/libevm/common/hexutil" -"github.com/ava-labs/libevm/common/lru" -"github.com/ava-labs/libevm/common/math" -"github.com/ava-labs/libevm/common/prque" -"github.com/ava-labs/libevm/consensus/misc/eip4844" -"github.com/ava-labs/libevm/core/asm" -"github.com/ava-labs/libevm/core/bloombits" -"github.com/ava-labs/libevm/core/rawdb" -"github.com/ava-labs/libevm/core/state" -"github.com/ava-labs/libevm/core/types" -"github.com/ava-labs/libevm/core/vm" -"github.com/ava-labs/libevm/crypto" -"github.com/ava-labs/libevm/crypto/blake2b" -"github.com/ava-labs/libevm/crypto/bls12381" -"github.com/ava-labs/libevm/crypto/bn256" -"github.com/ava-labs/libevm/crypto/kzg4844" -"github.com/ava-labs/libevm/eth/tracers/js" -"github.com/ava-labs/libevm/eth/tracers/logger" -"github.com/ava-labs/libevm/eth/tracers/native" -"github.com/ava-labs/libevm/ethdb" -"github.com/ava-labs/libevm/ethdb/leveldb" -"github.com/ava-labs/libevm/ethdb/memorydb" -"github.com/ava-labs/libevm/ethdb/pebble" -"github.com/ava-labs/libevm/event" -"github.com/ava-labs/libevm/libevm" -"github.com/ava-labs/libevm/libevm/ethtest" -"github.com/ava-labs/libevm/libevm/legacy" -"github.com/ava-labs/libevm/libevm/options" -"github.com/ava-labs/libevm/libevm/stateconf" -"github.com/ava-labs/libevm/log" -"github.com/ava-labs/libevm/metrics" -"github.com/ava-labs/libevm/rlp" -"github.com/ava-labs/libevm/trie" -"github.com/ava-labs/libevm/trie/testutil" -"github.com/ava-labs/libevm/trie/trienode" -"github.com/ava-labs/libevm/trie/triestate" -"github.com/ava-labs/libevm/trie/utils" -"github.com/ava-labs/libevm/triedb" -"github.com/ava-labs/libevm/triedb/database" \ No newline at end of file diff --git a/vms/evm/imports_test.go b/vms/evm/imports_test.go new file mode 100644 index 000000000000..2e58c40c1244 --- /dev/null +++ b/vms/evm/imports_test.go @@ -0,0 +1,120 @@ +// Copyright (C) 2019-2025, Ava Labs, Inc. All rights reserved. +// See the file LICENSE for licensing terms. + +package evm + +import ( + "fmt" + "go/parser" + "go/token" + "io/fs" + "path/filepath" + "strings" + "testing" + + "github.com/stretchr/testify/require" +) + +const ( + corethImport = "github.com/ava-labs/avalanchego/graft/coreth" + subnetEVMImport = "github.com/ava-labs/avalanchego/graft/subnet-evm" +) + +// TestImportViolations ensures proper import rules: +// - graft/coreth can be imported anywhere EXCEPT vms/evm (but vms/evm/emulate is an exception) +// - graft/subnet-evm can only be imported within graft/subnet-evm itself and vms/evm/emulate +// - graft/evm can NOT import from graft/coreth or graft/subnet-evm +// +// The rationale for these rules are as follows: +// +// coreth can be imported in AvalancheGo, because it was already imported by Avalanche prior to +// grafting +// +// coreth can NOT be imported in the vms/evm package, because the goal is that vms/evm should +// only contain the 'clean' properly uplifted code, that meets AvalancheGo quality standards. +// +// subnet-evm can NOT be imported anywhere in AvalancheGo besides graft/subnet-evm itself, +// because it must not become a direct dependency of AvalancheGo or mix directly with coreth. +// +// graft/evm is the shared code that both coreth and subnet-evm will depend on, so it must not +// import from either of them to avoid a circular dependency. +// +// both coreth and subnet-evm can be imported in the vms/evm/emulate package, because it +// allows consumers to use both coreth and subnet-evm registration at the same time. +// +// TODO(jonathanoppenheimer): remove the graft functionality once the graft package is removed. +func TestImportViolations(t *testing.T) { + const root = "../.." + repoRoot, err := filepath.Abs(root) + require.NoError(t, err) + + graftDir := filepath.Join(repoRoot, "graft") + graftEVMDir := filepath.Join(graftDir, "evm") + graftSubnetEVMDir := filepath.Join(graftDir, "subnet-evm") + vmsEVMDir := filepath.Join(repoRoot, "vms", "evm") + emulateDir := filepath.Join(vmsEVMDir, "emulate") + + var violations []string + + err = filepath.Walk(root, func(file string, _ fs.FileInfo, err error) error { + if err != nil { + return err + } + if strings.ToLower(filepath.Ext(file)) != ".go" { + return nil + } + + absFile, err := filepath.Abs(file) + if err != nil { + return err + } + + node, err := parser.ParseFile(token.NewFileSet(), file, nil, parser.ImportsOnly) + if err != nil { + return fmt.Errorf("parser.ParseFile(..., %q, ...): %w", file, err) + } + + inGraftEVM := strings.HasPrefix(absFile, graftEVMDir) + inGraftSubnetEVM := strings.HasPrefix(absFile, graftSubnetEVMDir) + inEmulate := strings.HasPrefix(absFile, emulateDir) + inVMsEVM := strings.HasPrefix(absFile, vmsEVMDir) + + for _, spec := range node.Imports { + if spec.Path == nil { + continue + } + importPath := strings.Trim(spec.Path.Value, `"`) + + importsCoreth := isImportIn(importPath, corethImport) + importsSubnetEVM := isImportIn(importPath, subnetEVMImport) + + hasViolation := (importsCoreth && inVMsEVM && !inEmulate || // vm/evm can't import coreth + importsSubnetEVM && !inGraftSubnetEVM && !inEmulate || // vm/evm can't import subnet-evm + (importsCoreth || importsSubnetEVM) && inGraftEVM) // graft/evm can't import coreth or subnet-evm + if hasViolation { + violations = append(violations, fmt.Sprintf("File %q imports %q", file, importPath)) + } + } + return nil + }) + + require.NoErrorf(t, err, "filepath.Walk(%q)", root) + require.Empty(t, violations, "import violations found") +} + +func isImportIn(importPath, targetedImport string) bool { + if importPath == targetedImport { + return true + } + + // importPath must be at least len(targetedImport) + 1 to be a subpackage + // (e.g., "github.com/foo/x" is len("github.com/foo") + 2, minimum subpackage length) + if len(importPath) < len(targetedImport)+1 { + return false + } + + // Check if importPath is a subpackage by ensuring it has the targetedImport prefix + // AND the next character is '/'. This is to prevent false positives where one + // package name is a prefix of another like "github.com/foo" vs "github.com/foobar". + return strings.HasPrefix(importPath, targetedImport) && importPath[len(targetedImport)] == '/' +}