diff --git a/packages/contracts-bedrock/scripts/checks/test-validation/exclusions.toml b/packages/contracts-bedrock/scripts/checks/test-validation/exclusions.toml index 8adb91a6091be..dd174c68b582e 100644 --- a/packages/contracts-bedrock/scripts/checks/test-validation/exclusions.toml +++ b/packages/contracts-bedrock/scripts/checks/test-validation/exclusions.toml @@ -23,6 +23,7 @@ src_validation = [ "test/cannon/MIPS64Memory.t.sol", # Tests external MIPS implementation "test/dispute/lib/LibClock.t.sol", # Tests library utilities "test/dispute/lib/LibGameId.t.sol", # Tests library utilities + "test/libraries/DeployUtils.t.sol", # Tests library utilities - no direct src counterpart "test/setup/DeployVariations.t.sol", # Tests deployment variations "test/universal/BenchmarkTest.t.sol", # Performance benchmarking tests "test/universal/ExtendedPause.t.sol", # Tests extended functionality @@ -56,6 +57,8 @@ contract_name_validation = [ "test/L2/GasPriceOracle.t.sol", # Contains contracts not matching GasPriceOracle base name "test/universal/StandardBridge.t.sol", # Contains contracts not matching StandardBridge base name "test/L1/OPContractsManagerContractsContainer.t.sol", # Contains contracts not matching OPContractsManagerContractsContainer base name + "test/libraries/Blueprint.t.sol", # Contains helper contracts (BlueprintHarness, ConstructorArgMock) + "test/libraries/SafeCall.t.sol", # Contains helper contracts (SimpleSafeCaller) ] # PATHS EXCLUDED FROM FUNCTION NAME VALIDATION: @@ -64,23 +67,21 @@ contract_name_validation = [ # contract's ABI. # # Common reasons for exclusion: -# - Libraries: Have different artifact structures that the validation system -# doesn't currently support, making function name lookup impossible # - Internal/Private functions: Some contracts test internal functions that # aren't exposed in the public ABI, so they can't be validated # - Misspelled/Incorrect function names: Test contracts may have typos or # incorrect function names that don't match the actual source contract # # Resolving these issues requires either: -# - Enhancing the validation system to support libraries and complex structures +# - Enhancing the validation system to support complex structures # - Fixing misspelled function names in test contracts # - Restructuring tests to match actual function signatures function_name_validation = [ - "test/libraries", # Libraries have different artifact structure, unsupported - "test/dispute/lib/LibPosition.t.sol", # Library testing - artifact structure issues - "test/L1/ProxyAdminOwnedBase.t.sol", # Tests internal functions not in ABI - "test/L1/SystemConfig.t.sol", # Tests internal functions not in ABI - "test/safe/SafeSigners.t.sol", # Function name validation issues + "test/L1/ProxyAdminOwnedBase.t.sol", # Tests internal functions not in ABI + "test/L1/SystemConfig.t.sol", # Tests internal functions not in ABI + "test/safe/SafeSigners.t.sol", # Function name validation issues + "test/libraries/Predeploys.t.sol", # Function 'uncategorizedInterop' doesn't exist in library + "test/libraries/TransientContext.t.sol", # Function 'reentrantAware' doesn't exist in library ] [excluded_tests] @@ -91,4 +92,5 @@ contracts = [ "OptimismPortal2_MigrateLiquidity_Test", # Interop tests hosted in the OptimismPortal2 test file "OptimismPortal2_MigrateToSuperRoots_Test", # Interop tests hosted in the OptimismPortal2 test file "OptimismPortal2_UpgradeInterop_Test", # Interop tests hosted in the OptimismPortal2 test file + "Constants_Test", # Invalid naming pattern - doesn't specify function or Uncategorized ] diff --git a/packages/contracts-bedrock/scripts/checks/test-validation/main.go b/packages/contracts-bedrock/scripts/checks/test-validation/main.go index d0bda5f33618b..d752e2566243d 100644 --- a/packages/contracts-bedrock/scripts/checks/test-validation/main.go +++ b/packages/contracts-bedrock/scripts/checks/test-validation/main.go @@ -37,6 +37,8 @@ func main() { fmt.Printf("error: %v\n", err) os.Exit(1) } + + fmt.Println("✅ All contract test validations passed") } // Processes a single test artifact file and runs all validations @@ -278,6 +280,36 @@ func findArtifactPath(contractFileName, contractName string) (string, error) { return files[0], nil } +// Checks if the artifact represents a library +func isLibrary(artifact *solc.ForgeArtifact) bool { + // Check the AST for ContractKind == "library" + for _, node := range artifact.Ast.Nodes { + if node.NodeType == "ContractDefinition" && node.ContractKind == "library" { + return true + } + } + return false +} + +// Extracts function names from the AST (for libraries with internal functions) +func extractFunctionsFromAST(artifact *solc.ForgeArtifact) []string { + var functions []string + + // Navigate through AST to find function definitions + for _, node := range artifact.Ast.Nodes { + if node.NodeType == "ContractDefinition" { + // Iterate through contract nodes to find functions + for _, childNode := range node.Nodes { + if childNode.NodeType == "FunctionDefinition" && childNode.Name != "" { + functions = append(functions, childNode.Name) + } + } + } + } + + return functions +} + // Validates that a function exists in the source contract func checkFunctionExists(artifact *solc.ForgeArtifact, functionName string) bool { // Special functions always exist @@ -309,7 +341,18 @@ func checkFunctionExists(artifact *solc.ForgeArtifact, functionName string) bool return false } - // Check if function exists in the ABI + // Check if source is a library - use AST for internal functions + if isLibrary(srcArtifact) { + functions := extractFunctionsFromAST(srcArtifact) + for _, fn := range functions { + if strings.EqualFold(fn, functionName) { + return true + } + } + return false + } + + // For contracts, check if function exists in the ABI for _, method := range srcArtifact.Abi.Parsed.Methods { if strings.EqualFold(method.Name, functionName) { return true diff --git a/packages/contracts-bedrock/scripts/checks/test-validation/main_test.go b/packages/contracts-bedrock/scripts/checks/test-validation/main_test.go index b2e6677f1f95b..96f2159e331af 100644 --- a/packages/contracts-bedrock/scripts/checks/test-validation/main_test.go +++ b/packages/contracts-bedrock/scripts/checks/test-validation/main_test.go @@ -369,6 +369,53 @@ func TestFindArtifactPath(t *testing.T) { } } +func TestIsLibrary(t *testing.T) { + library := &solc.ForgeArtifact{ + Ast: solc.Ast{ + Nodes: []solc.AstNode{ + {NodeType: "ContractDefinition", ContractKind: "library"}, + }, + }, + } + contract := &solc.ForgeArtifact{ + Ast: solc.Ast{ + Nodes: []solc.AstNode{ + {NodeType: "ContractDefinition", ContractKind: "contract"}, + }, + }, + } + if !isLibrary(library) { + t.Error("library artifact should be detected as library") + } + if isLibrary(contract) { + t.Error("contract artifact should not be detected as library") + } +} + +func TestExtractFunctionsFromAST(t *testing.T) { + artifact := &solc.ForgeArtifact{ + Ast: solc.Ast{ + Nodes: []solc.AstNode{ + { + NodeType: "ContractDefinition", + Nodes: []solc.AstNode{ + {NodeType: "FunctionDefinition", Name: "add"}, + {NodeType: "FunctionDefinition", Name: "subtract"}, + {NodeType: "VariableDeclaration", Name: "ignored"}, + }, + }, + }, + }, + } + functions := extractFunctionsFromAST(artifact) + if len(functions) != 2 { + t.Errorf("expected 2 functions, got %d", len(functions)) + } + if functions[0] != "add" || functions[1] != "subtract" { + t.Errorf("unexpected function names: %v", functions) + } +} + func TestCheckFunctionExists(t *testing.T) { artifact := &solc.ForgeArtifact{Metadata: solc.ForgeCompilerMetadata{Settings: solc.CompilerSettings{CompilationTarget: map[string]string{"test/Contract.t.sol": "Contract_Test"}}}} if !checkFunctionExists(artifact, "constructor") { diff --git a/packages/contracts-bedrock/test/libraries/DevFeatures.t.sol b/packages/contracts-bedrock/test/libraries/DevFeatures.t.sol index 076e4f04056cb..cd679dd250475 100644 --- a/packages/contracts-bedrock/test/libraries/DevFeatures.t.sol +++ b/packages/contracts-bedrock/test/libraries/DevFeatures.t.sol @@ -7,7 +7,7 @@ import { Test } from "forge-std/Test.sol"; // Target contract import { DevFeatures } from "src/libraries/DevFeatures.sol"; -contract DevFeatures_isEnabled_Test is Test { +contract DevFeatures_isDevFeatureEnabled_Test is Test { bytes32 internal constant FEATURE_A = bytes32(0x0000000000000000000000000000000000000000000000000000000000000001); bytes32 internal constant FEATURE_B = bytes32(0x0000000000000000000000000000000000000000000000000000000000000100); bytes32 internal constant FEATURE_C = bytes32(0x1000000000000000000000000000000000000000000000000000000000000000);