From 130a75d7f35a58c06906da95e11ba2c2243d7c10 Mon Sep 17 00:00:00 2001 From: Goran Vladika Date: Tue, 23 Jul 2024 14:26:56 +0200 Subject: [PATCH 1/6] Add script to check for unused errors --- test/unused-errors/exceptions.txt | 0 test/unused-errors/find_unused_errors.sh | 68 ++++++++++++++++++++++++ 2 files changed, 68 insertions(+) create mode 100644 test/unused-errors/exceptions.txt create mode 100755 test/unused-errors/find_unused_errors.sh diff --git a/test/unused-errors/exceptions.txt b/test/unused-errors/exceptions.txt new file mode 100644 index 0000000000..e69de29bb2 diff --git a/test/unused-errors/find_unused_errors.sh b/test/unused-errors/find_unused_errors.sh new file mode 100755 index 0000000000..98b764975d --- /dev/null +++ b/test/unused-errors/find_unused_errors.sh @@ -0,0 +1,68 @@ +#!/bin/bash + +# Directory containing the Solidity files +DIR="./contracts" +EXCEPTIONS_FILE="./test/unused-errors/exceptions.txt" + +# Temporary file to store errors and their corresponding files +ERRORS_FILE=$(mktemp) +UNUSED_ERRORS_FILE=$(mktemp) +exit_status=0 + +# Load exceptions into an array +exceptions=() +if [ -f "$EXCEPTIONS_FILE" ]; then + while IFS= read -r line || [ -n "$line" ]; do + exceptions+=("$line") + done < "$EXCEPTIONS_FILE" +fi + +# Function to check if an error is in the exceptions list +is_exception() { + local error="$1" + for exception in "${exceptions[@]}"; do + if [ "$exception" == "$error" ]; then + return 0 + fi + done + return 1 +} + +# Find all error definitions and store them in a temporary file along with filenames +grep -rHo "error\s\+\w\+\s*(" $DIR | awk -F':' '{print $1 ": " $2}' | awk '{gsub(/error /, ""); gsub(/\(/, ""); print}' | sort -u > $ERRORS_FILE + +# Loop through each error to check if it is used +while IFS=: read -r file error; do + # Normalize file and error output + error=$(echo $error | xargs) + + # Skip errors in the exception list + if is_exception "$error"; then + echo "Skipping: $error" + continue + fi + + # Count occurrences of each error in 'revert' statements + count=$(grep -roh "revert\s\+$error" $DIR | wc -l) + + # If count is 0, the error is defined but never used + if [ "$count" -eq 0 ]; then + echo "$error" >> $UNUSED_ERRORS_FILE + exit_status=1 + fi +done < $ERRORS_FILE + +# Print the list of unused errors +if [ -s $UNUSED_ERRORS_FILE ]; then + echo "These errors are defined, but never used:" + cat $UNUSED_ERRORS_FILE +else + echo "All defined errors are used." +fi + +# Remove the temporary files +rm $ERRORS_FILE +rm $UNUSED_ERRORS_FILE + +# Exit with the appropriate status +exit $exit_status From 712f97069dc615885585fd6286764f1d581a8cc9 Mon Sep 17 00:00:00 2001 From: Goran Vladika Date: Tue, 23 Jul 2024 14:28:02 +0200 Subject: [PATCH 2/6] Add check to CI --- .github/workflows/build-test.yml | 3 +++ package.json | 1 + 2 files changed, 4 insertions(+) diff --git a/.github/workflows/build-test.yml b/.github/workflows/build-test.yml index 25ab6b3886..a844e8bfd0 100644 --- a/.github/workflows/build-test.yml +++ b/.github/workflows/build-test.yml @@ -98,6 +98,9 @@ jobs: - name: Test function signatures run: yarn run test:signatures + - name: Test unused errors + run: yarn run test:unused:errors + test-e2e: name: Test e2e runs-on: ubuntu-latest diff --git a/package.json b/package.json index 42067a24ec..2714ab098d 100644 --- a/package.json +++ b/package.json @@ -22,6 +22,7 @@ "test:storage": "./scripts/storage_layout_test.bash", "test:signatures": "./scripts/signatures_test.bash", "test:mutation": "ts-node test-mutation/gambitTester.ts", + "test:unused:errors": "./test/unused-errors/find_unused_errors.sh", "deploy:local:token-bridge": "ts-node ./scripts/local-deployment/deployCreatorAndCreateTokenBridge.ts", "deploy:token-bridge-creator": "ts-node ./scripts/deployment/deployTokenBridgeCreator.ts", "create:token-bridge": "ts-node ./scripts/deployment/createTokenBridge.ts", From 363340573a93a05404903c1dc5c0b88f103c395b Mon Sep 17 00:00:00 2001 From: Goran Vladika Date: Wed, 24 Jul 2024 14:43:11 +0200 Subject: [PATCH 3/6] Use gh action to check for errors --- .github/workflows/build-test.yml | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/.github/workflows/build-test.yml b/.github/workflows/build-test.yml index a844e8bfd0..9d77ff9e11 100644 --- a/.github/workflows/build-test.yml +++ b/.github/workflows/build-test.yml @@ -98,8 +98,11 @@ jobs: - name: Test function signatures run: yarn run test:signatures - - name: Test unused errors - run: yarn run test:unused:errors + - name: Run unused Solidity errors checker + uses: OffchainLabs/actions/check-unused-errors@unused-errors + with: + directory: './contracts' + exceptions_file: './test/unused-errors/exceptions.txt' test-e2e: name: Test e2e From c6a94af258c56eed55357e23bf769d86c0098866 Mon Sep 17 00:00:00 2001 From: Goran Vladika Date: Wed, 24 Jul 2024 15:04:01 +0200 Subject: [PATCH 4/6] Test CI --- .../ethereum/L1AtomicTokenBridgeCreator.sol | 1 + test/unused-errors/find_unused_errors.sh | 68 ------------------- 2 files changed, 1 insertion(+), 68 deletions(-) delete mode 100755 test/unused-errors/find_unused_errors.sh diff --git a/contracts/tokenbridge/ethereum/L1AtomicTokenBridgeCreator.sol b/contracts/tokenbridge/ethereum/L1AtomicTokenBridgeCreator.sol index 571568cc55..049ea91de5 100644 --- a/contracts/tokenbridge/ethereum/L1AtomicTokenBridgeCreator.sol +++ b/contracts/tokenbridge/ethereum/L1AtomicTokenBridgeCreator.sol @@ -57,6 +57,7 @@ contract L1AtomicTokenBridgeCreator is Initializable, OwnableUpgradeable { error L1AtomicTokenBridgeCreator_RollupOwnershipMisconfig(); error L1AtomicTokenBridgeCreator_ProxyAdminNotFound(); error L1AtomicTokenBridgeCreator_L2FactoryCannotBeChanged(); + error TestError(); event OrbitTokenBridgeCreated( address indexed inbox, diff --git a/test/unused-errors/find_unused_errors.sh b/test/unused-errors/find_unused_errors.sh deleted file mode 100755 index 98b764975d..0000000000 --- a/test/unused-errors/find_unused_errors.sh +++ /dev/null @@ -1,68 +0,0 @@ -#!/bin/bash - -# Directory containing the Solidity files -DIR="./contracts" -EXCEPTIONS_FILE="./test/unused-errors/exceptions.txt" - -# Temporary file to store errors and their corresponding files -ERRORS_FILE=$(mktemp) -UNUSED_ERRORS_FILE=$(mktemp) -exit_status=0 - -# Load exceptions into an array -exceptions=() -if [ -f "$EXCEPTIONS_FILE" ]; then - while IFS= read -r line || [ -n "$line" ]; do - exceptions+=("$line") - done < "$EXCEPTIONS_FILE" -fi - -# Function to check if an error is in the exceptions list -is_exception() { - local error="$1" - for exception in "${exceptions[@]}"; do - if [ "$exception" == "$error" ]; then - return 0 - fi - done - return 1 -} - -# Find all error definitions and store them in a temporary file along with filenames -grep -rHo "error\s\+\w\+\s*(" $DIR | awk -F':' '{print $1 ": " $2}' | awk '{gsub(/error /, ""); gsub(/\(/, ""); print}' | sort -u > $ERRORS_FILE - -# Loop through each error to check if it is used -while IFS=: read -r file error; do - # Normalize file and error output - error=$(echo $error | xargs) - - # Skip errors in the exception list - if is_exception "$error"; then - echo "Skipping: $error" - continue - fi - - # Count occurrences of each error in 'revert' statements - count=$(grep -roh "revert\s\+$error" $DIR | wc -l) - - # If count is 0, the error is defined but never used - if [ "$count" -eq 0 ]; then - echo "$error" >> $UNUSED_ERRORS_FILE - exit_status=1 - fi -done < $ERRORS_FILE - -# Print the list of unused errors -if [ -s $UNUSED_ERRORS_FILE ]; then - echo "These errors are defined, but never used:" - cat $UNUSED_ERRORS_FILE -else - echo "All defined errors are used." -fi - -# Remove the temporary files -rm $ERRORS_FILE -rm $UNUSED_ERRORS_FILE - -# Exit with the appropriate status -exit $exit_status From a9d61ad51ad88be4a8804b0ce315160423958e80 Mon Sep 17 00:00:00 2001 From: Goran Vladika Date: Wed, 24 Jul 2024 15:07:23 +0200 Subject: [PATCH 5/6] Remove test --- .github/workflows/build-test.yml | 2 +- contracts/tokenbridge/ethereum/L1AtomicTokenBridgeCreator.sol | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/.github/workflows/build-test.yml b/.github/workflows/build-test.yml index 9d77ff9e11..01a96ad901 100644 --- a/.github/workflows/build-test.yml +++ b/.github/workflows/build-test.yml @@ -67,7 +67,7 @@ jobs: run: yarn test test-contracts: - name: Test storage layout and signatures + name: Test storage layout, signatures and look for unused errors runs-on: ubuntu-latest steps: - uses: actions/checkout@v3 diff --git a/contracts/tokenbridge/ethereum/L1AtomicTokenBridgeCreator.sol b/contracts/tokenbridge/ethereum/L1AtomicTokenBridgeCreator.sol index 049ea91de5..571568cc55 100644 --- a/contracts/tokenbridge/ethereum/L1AtomicTokenBridgeCreator.sol +++ b/contracts/tokenbridge/ethereum/L1AtomicTokenBridgeCreator.sol @@ -57,7 +57,6 @@ contract L1AtomicTokenBridgeCreator is Initializable, OwnableUpgradeable { error L1AtomicTokenBridgeCreator_RollupOwnershipMisconfig(); error L1AtomicTokenBridgeCreator_ProxyAdminNotFound(); error L1AtomicTokenBridgeCreator_L2FactoryCannotBeChanged(); - error TestError(); event OrbitTokenBridgeCreated( address indexed inbox, From 8e4e4ae5dc0878040775a66bd436bb9dc7233339 Mon Sep 17 00:00:00 2001 From: Goran Vladika Date: Fri, 26 Jul 2024 10:20:12 +0200 Subject: [PATCH 6/6] Use main --- .github/workflows/build-test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/build-test.yml b/.github/workflows/build-test.yml index 01a96ad901..28d9241603 100644 --- a/.github/workflows/build-test.yml +++ b/.github/workflows/build-test.yml @@ -99,7 +99,7 @@ jobs: run: yarn run test:signatures - name: Run unused Solidity errors checker - uses: OffchainLabs/actions/check-unused-errors@unused-errors + uses: OffchainLabs/actions/check-unused-errors@main with: directory: './contracts' exceptions_file: './test/unused-errors/exceptions.txt'