From cd7e9d40414741ef27ce785a6c305a89ab035e4a Mon Sep 17 00:00:00 2001 From: Matthew Slipper Date: Sat, 28 Sep 2024 19:50:51 -0600 Subject: [PATCH] feat: Rewrite natspec checker in Go (#12191) * feat: Rewrite natspec checker in Go Rewrites the `semver-natspec-check-no-build` Just command in Go to reduce runtime. This PR reduces runtime for this check from ~1m30s to about 3 seconds post-compilation. * remove old script * add unit tests * rename test * review updates --- .circleci/config.yml | 3 + packages/contracts-bedrock/justfile | 2 +- .../checks/check-semver-natspec-match.sh | 74 ------ .../scripts/checks/semver-natspec/main.go | 215 ++++++++++++++++++ .../checks/semver-natspec/main_test.go | 124 ++++++++++ 5 files changed, 343 insertions(+), 75 deletions(-) delete mode 100755 packages/contracts-bedrock/scripts/checks/check-semver-natspec-match.sh create mode 100644 packages/contracts-bedrock/scripts/checks/semver-natspec/main.go create mode 100644 packages/contracts-bedrock/scripts/checks/semver-natspec/main_test.go diff --git a/.circleci/config.yml b/.circleci/config.yml index 6930b4ddd1e5..73ac7d81b3fd 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -1458,6 +1458,9 @@ workflows: - op-program - op-service - op-supervisor + - go-test: + name: semver-natspec-tests + module: packages/contracts-bedrock/scripts/checks/semver-natspec - go-test-kurtosis: name: op-chain-ops-integration module: op-chain-ops diff --git a/packages/contracts-bedrock/justfile b/packages/contracts-bedrock/justfile index b5bce1e55b7a..a9c621cf240c 100644 --- a/packages/contracts-bedrock/justfile +++ b/packages/contracts-bedrock/justfile @@ -163,7 +163,7 @@ semver-diff-check: build semver-diff-check-no-build # Checks that semver natspec is equal to the actual semver version. # Does not build contracts. semver-natspec-check-no-build: - ./scripts/checks/check-semver-natspec-match.sh + go run ./scripts/checks/semver-natspec # Checks that semver natspec is equal to the actual semver version. semver-natspec-check: build semver-natspec-check-no-build diff --git a/packages/contracts-bedrock/scripts/checks/check-semver-natspec-match.sh b/packages/contracts-bedrock/scripts/checks/check-semver-natspec-match.sh deleted file mode 100755 index de4de3f8497a..000000000000 --- a/packages/contracts-bedrock/scripts/checks/check-semver-natspec-match.sh +++ /dev/null @@ -1,74 +0,0 @@ -#!/usr/bin/env bash -set -euo pipefail - -# Grab the directory of the contracts-bedrock package -SCRIPT_DIR=$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd ) -CONTRACTS_BASE=$(dirname "$(dirname "$SCRIPT_DIR")") -ARTIFACTS_DIR="$CONTRACTS_BASE/forge-artifacts" -CONTRACTS_DIR="$CONTRACTS_BASE/src" - -# Load semver-utils -# shellcheck source=/dev/null -source "$SCRIPT_DIR/utils/semver-utils.sh" - -# Flag to track if any errors are detected -has_errors=false - -# Iterate through each artifact file -for artifact_file in "$ARTIFACTS_DIR"/**/*.json; do - # Get the contract name and find the corresponding source file - contract_name=$(basename "$artifact_file" .json) - contract_file=$(find "$CONTRACTS_DIR" -name "$contract_name.sol") - - # Try to extract version as a constant - raw_metadata=$(jq -r '.rawMetadata' "$artifact_file") - artifact_version=$(echo "$raw_metadata" | jq -r '.output.devdoc.stateVariables.version."custom:semver"') - - is_constant=true - if [ "$artifact_version" = "null" ]; then - # If not found as a constant, try to extract as a function - artifact_version=$(echo "$raw_metadata" | jq -r '.output.devdoc.methods."version()"."custom:semver"') - is_constant=false - fi - - # If @custom:semver is not found in either location, skip this file - if [ "$artifact_version" = "null" ]; then - continue - fi - - # If source file is not found, report an error - if [ -z "$contract_file" ]; then - echo "❌ $contract_name: Source file not found" - continue - fi - - # Extract version from source based on whether it's a constant or function - if [ "$is_constant" = true ]; then - source_version=$(extract_constant_version "$contract_file") - else - source_version=$(extract_function_version "$contract_file") - fi - - # If source version is not found, report an error - if [ "$source_version" = "" ]; then - echo "❌ Error: failed to find version string for $contract_name" - echo " this is probably a bug in check-contract-semver.sh" - echo " please report or fix the issue if possible" - has_errors=true - fi - - # Compare versions - if [ "$source_version" != "$artifact_version" ]; then - echo "❌ Error: $contract_name has different semver in code and devdoc" - echo " Code: $source_version" - echo " Devdoc: $artifact_version" - has_errors=true - else - echo "✅ $contract_name: code: $source_version, devdoc: $artifact_version" - fi -done - -# If any errors were detected, exit with a non-zero status -if [ "$has_errors" = true ]; then - exit 1 -fi diff --git a/packages/contracts-bedrock/scripts/checks/semver-natspec/main.go b/packages/contracts-bedrock/scripts/checks/semver-natspec/main.go new file mode 100644 index 000000000000..d1e2153c02ef --- /dev/null +++ b/packages/contracts-bedrock/scripts/checks/semver-natspec/main.go @@ -0,0 +1,215 @@ +package main + +import ( + "bufio" + "bytes" + "encoding/json" + "fmt" + "io" + "os" + "path/filepath" + "regexp" + "runtime" + "strings" + "sync" + "sync/atomic" +) + +type ArtifactsWrapper struct { + RawMetadata string `json:"rawMetadata"` +} + +type Artifacts struct { + Output struct { + Devdoc struct { + StateVariables struct { + Version struct { + Semver string `json:"custom:semver"` + } `json:"version"` + } `json:"stateVariables,omitempty"` + Methods struct { + Version struct { + Semver string `json:"custom:semver"` + } `json:"version()"` + } `json:"methods,omitempty"` + } `json:"devdoc"` + } `json:"output"` +} + +var ConstantVersionPattern = regexp.MustCompile(`string.*constant.*version\s+=\s+"([^"]+)";`) + +var FunctionVersionPattern = regexp.MustCompile(`^\s+return\s+"((?P0|[1-9]\d*)\.(?P0|[1-9]\d*)\.(?P0|[1-9]\d*)(?:-(?P(?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*)(?:\.(?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*))*))?(?:\+(?P[0-9a-zA-Z-]+(?:\.[0-9a-zA-Z-]+)*))?)";$`) + +var InteropVersionPattern = regexp.MustCompile(`^\s+return\s+string\.concat\(super\.version\(\), "((.*)\+interop(.*)?)"\);`) + +func main() { + if err := run(); err != nil { + writeStderr("an error occurred: %v", err) + os.Exit(1) + } +} + +func writeStderr(msg string, args ...any) { + _, _ = fmt.Fprintf(os.Stderr, msg+"\n", args...) +} + +func run() error { + cwd, err := os.Getwd() + if err != nil { + return fmt.Errorf("failed to get current working directory: %w", err) + } + + writeStderr("working directory: %s", cwd) + + artifactsDir := filepath.Join(cwd, "forge-artifacts") + srcDir := filepath.Join(cwd, "src") + + artifactFiles, err := glob(artifactsDir, ".json") + if err != nil { + return fmt.Errorf("failed to get artifact files: %w", err) + } + contractFiles, err := glob(srcDir, ".sol") + if err != nil { + return fmt.Errorf("failed to get contract files: %w", err) + } + + var hasErr int32 + var outMtx sync.Mutex + fail := func(msg string, args ...any) { + outMtx.Lock() + writeStderr("❌ "+msg, args...) + outMtx.Unlock() + atomic.StoreInt32(&hasErr, 1) + } + + sem := make(chan struct{}, runtime.NumCPU()) + for contractName, artifactPath := range artifactFiles { + contractName := contractName + artifactPath := artifactPath + + sem <- struct{}{} + + go func() { + defer func() { + <-sem + }() + + af, err := os.Open(artifactPath) + if err != nil { + fail("%s: failed to open contract artifact: %v", contractName, err) + return + } + defer af.Close() + + var wrapper ArtifactsWrapper + if err := json.NewDecoder(af).Decode(&wrapper); err != nil { + fail("%s: failed to parse artifact file: %v", contractName, err) + return + } + + if wrapper.RawMetadata == "" { + return + } + + var artifactData Artifacts + if err := json.Unmarshal([]byte(wrapper.RawMetadata), &artifactData); err != nil { + fail("%s: failed to unwrap artifact metadata: %v", contractName, err) + return + } + + artifactVersion := artifactData.Output.Devdoc.StateVariables.Version.Semver + + isConstant := true + if artifactData.Output.Devdoc.StateVariables.Version.Semver == "" { + artifactVersion = artifactData.Output.Devdoc.Methods.Version.Semver + isConstant = false + } + + if artifactVersion == "" { + return + } + + contractPath := contractFiles[contractName] + if contractPath == "" { + fail("%s: Source file not found", contractName) + return + } + + cf, err := os.Open(contractPath) + if err != nil { + fail("%s: failed to open contract source: %v", contractName, err) + return + } + defer cf.Close() + + sourceData, err := io.ReadAll(cf) + if err != nil { + fail("%s: failed to read contract source: %v", contractName, err) + return + } + + var sourceVersion string + + if isConstant { + sourceVersion = findLine(sourceData, ConstantVersionPattern) + } else { + sourceVersion = findLine(sourceData, FunctionVersionPattern) + } + + // Need to define a special case for interop contracts since they technically + // use an invalid semver format. Checking for sourceVersion == "" allows the + // team to update the format to a valid semver format in the future without + // needing to change this program. + if sourceVersion == "" && strings.HasSuffix(contractName, "Interop") { + sourceVersion = findLine(sourceData, InteropVersionPattern) + } + + if sourceVersion == "" { + fail("%s: version not found in source", contractName) + return + } + + if sourceVersion != artifactVersion { + fail("%s: version mismatch: source=%s, artifact=%s", contractName, sourceVersion, artifactVersion) + return + } + + _, _ = fmt.Fprintf(os.Stderr, "✅ %s: code: %s, artifact: %s\n", contractName, sourceVersion, artifactVersion) + }() + } + + for i := 0; i < cap(sem); i++ { + sem <- struct{}{} + } + + if atomic.LoadInt32(&hasErr) == 1 { + return fmt.Errorf("semver check failed, see logs above") + } + + return nil +} + +func glob(dir string, ext string) (map[string]string, error) { + out := make(map[string]string) + err := filepath.Walk(dir, func(path string, info os.FileInfo, err error) error { + if !info.IsDir() && filepath.Ext(path) == ext { + out[strings.TrimSuffix(filepath.Base(path), ext)] = path + } + return nil + }) + if err != nil { + return nil, fmt.Errorf("failed to walk directory: %w", err) + } + return out, nil +} + +func findLine(in []byte, pattern *regexp.Regexp) string { + scanner := bufio.NewScanner(bytes.NewReader(in)) + for scanner.Scan() { + match := pattern.FindStringSubmatch(scanner.Text()) + if len(match) > 0 { + return match[1] + } + } + return "" +} diff --git a/packages/contracts-bedrock/scripts/checks/semver-natspec/main_test.go b/packages/contracts-bedrock/scripts/checks/semver-natspec/main_test.go new file mode 100644 index 000000000000..7a8872d76d78 --- /dev/null +++ b/packages/contracts-bedrock/scripts/checks/semver-natspec/main_test.go @@ -0,0 +1,124 @@ +package main + +import ( + "regexp" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestRegexes(t *testing.T) { + t.Run("ConstantVersionPattern", func(t *testing.T) { + testRegex(t, ConstantVersionPattern, []regexTest{ + { + name: "constant version", + input: `string constant version = "1.2.3";`, + capture: "1.2.3", + }, + { + name: "constant version with weird spaces", + input: ` string constant version = "1.2.3";`, + capture: "1.2.3", + }, + { + name: "constant version with visibility", + input: `string public constant version = "1.2.3";`, + capture: "1.2.3", + }, + { + name: "different variable name", + input: `string constant VERSION = "1.2.3";`, + capture: "", + }, + { + name: "different type", + input: `uint constant version = 1;`, + capture: "", + }, + { + name: "not constant", + input: `string version = "1.2.3";`, + capture: "", + }, + { + name: "unterminated", + input: `string constant version = "1.2.3"`, + capture: "", + }, + }) + }) + + t.Run("FunctionVersionPattern", func(t *testing.T) { + testRegex(t, FunctionVersionPattern, []regexTest{ + { + name: "function version", + input: ` return "1.2.3";`, + capture: "1.2.3", + }, + { + name: "function version with weird spaces", + input: ` return "1.2.3";`, + capture: "1.2.3", + }, + { + name: "function version with prerelease", + input: ` return "1.2.3-alpha.1";`, + capture: "1.2.3-alpha.1", + }, + { + name: "invalid semver", + input: ` return "1.2.cabdab";`, + capture: "", + }, + { + name: "not a return statement", + input: `function foo()`, + capture: "", + }, + }) + }) + + t.Run("InteropVersionPattern", func(t *testing.T) { + testRegex(t, InteropVersionPattern, []regexTest{ + { + name: "interop version", + input: ` return string.concat(super.version(), "+interop");`, + capture: "+interop", + }, + { + name: "interop version but as a valid semver", + input: ` return string.concat(super.version(), "0.0.0+interop");`, + capture: "0.0.0+interop", + }, + { + name: "not an interop version", + input: ` return string.concat(super.version(), "hello!");`, + capture: "", + }, + { + name: "invalid syntax", + input: ` return string.concat(super.version(), "0.0.0+interop`, + capture: "", + }, + { + name: "something else is concatted", + input: ` return string.concat("superduper", "mart");`, + capture: "", + }, + }) + }) +} + +type regexTest struct { + name string + input string + capture string +} + +func testRegex(t *testing.T, re *regexp.Regexp, tests []regexTest) { + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + require.Equal(t, test.capture, findLine([]byte(test.input), re)) + }) + } +}