Skip to content

N
nebula
Commit 1f0d8a8b authored by smartcontracts's avatar smartcontracts Committed by GitHub
Browse files
Options

fix: properly check all interfaces (#11734) 

Interfaces check script had a bug that would cause it to use the
wrong ABI when checking an interface. Each compiler artifact only
has a single ABI in it but this same ABI was being used for all
contract definitions found within that file. Since each separate
contract definition gets its own file anyway, this script now
properly verifies by finding the specific definition that matches
the filename.
parent 2f10fb7a
Hide whitespace changes
Inline Side-by-side
Showing with 92 additions and 73 deletions
packages/contracts-bedrock/scripts/checks/check-interfaces.sh
View file @ 1f0d8a8b
......@@ -17,31 +17,22 @@ fi
SCRIPT_DIR=$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )
CONTRACTS_BASE=$(dirname "$(dirname "$SCRIPT_DIR")")
# Define the files to exclude (glob patterns can be used)
EXCLUDE_FILES=(
# Define contracts that should be excluded from the check
EXCLUDE_CONTRACTS=(
# External dependencies
"IMulticall3"
"IERC20"
"IERC721"
"IERC721Enumerable"
"IERC721Metadata"
"IERC721Upgradeable"
"IERC721Receiver"
"IERC1271"
"IERC165"
"IVotes"
"IBeacon"
"IProxyCreationCallback"
"IAutomate"
"IGelato1Balance"
"IERC165Upgradeable"
"ERC721TokenReceiver"
"ERC1155TokenReceiver"
"ERC777TokensRecipient"
"Guard"
"GnosisSafeProxy"
"IProxy"
# Foundry
"Common"
"Vm"
"VmSafe"
......@@ -52,11 +43,6 @@ EXCLUDE_FILES=(
# Kontrol
"KontrolCheatsBase"
"KontrolCheats"
# Error definition files
"CommonErrors"
"Errors"
# TODO: Interfaces that need to be fixed
"IPreimageOracle"
......@@ -72,15 +58,16 @@ EXCLUDE_FILES=(
"IDelayedWETH"
"IAnchorStateRegistry"
"ICrossL2Inbox"
"IL1CrossDomainMessenger"
"IL2ToL2CrossDomainMessenger"
"KontrolInterfaces"
"IL1ERC721Bridge"
"IL1StandardBridge"
"ISuperchainConfig"
"IOptimismPortal"
)
# Convert the exclude files array to a pipe-separated string
EXCLUDE_PATTERN=$( (IFS="|"; echo "${EXCLUDE_FILES[*]}") )
# Find all JSON files in the forge-artifacts folder
JSON_FILES=$(find "$CONTRACTS_BASE/forge-artifacts" -type f -name "*.json" | grep -Ev "$EXCLUDE_PATTERN")
JSON_FILES=$(find "$CONTRACTS_BASE/forge-artifacts" -type f -name "*.json")
# Initialize a flag to track if any issues are detected
issues_detected=false
......@@ -88,7 +75,7 @@ issues_detected=false
# Create a temporary file to store files that have already been reported
REPORTED_INTERFACES_FILE=$(mktemp)
# Define a cleanup function
# Clean up the temporary file on exit
cleanup() {
rm -f "$REPORTED_INTERFACES_FILE"
}
......@@ -96,81 +83,113 @@ cleanup() {
# Trap exit and error signals and call cleanup function
trap cleanup EXIT ERR
# Check if a contract is excluded
is_excluded() {
for exclude in "${EXCLUDE_CONTRACTS[@]}"; do
if [[ "$exclude" == "$1" ]]; then
return 0
fi
done
return 1
}
# Iterate over all JSON files
for interface_file in $JSON_FILES; do
# Extract contract kind and name in a single pass
# Grab the contract name from the file name
contract_name=$(basename "$interface_file" .json | cut -d '.' -f 1)
# Extract all contract definitions in a single pass
contract_definitions=$(jq -r '.ast.nodes[] | select(.nodeType == "ContractDefinition") | "\(.contractKind),\(.name)"' "$interface_file")
# Warn and continue if no contract definitions are found
# Continue if no contract definitions are found
# Can happen in Solidity files that don't declare any contracts/interfaces
if [ -z "$contract_definitions" ]; then
echo "Warning: Could not extract contract definitions from $interface_file."
echo "Add this file to the EXCLUDE_FILES list if it can be ignored."
continue
fi
while IFS=',' read -r contract_kind contract_name; do
# If contract kind is not "interface", skip the file
if [ "$contract_kind" != "interface" ]; then
continue
# Iterate over the found contract definitions and figure out which one
# matches the file name. We do this so that we can figure out if this is an
# interface or a contract based on the contract kind.
found=false
contract_temp=""
contract_kind=""
for definition in $contract_definitions; do
IFS=',' read -r contract_kind contract_temp <<< "$definition"
if [[ "$contract_name" == "$contract_temp" ]]; then
found=true
break
fi
done
# If contract name is in the exclude list, skip the file
# Exclude list functions double duty as a list of files to exclude (glob patterns allowed)
# and a list of interface names that shouldn't be checked. Simplifies the script a bit and
# means we can ignore specific interfaces without ignoring the entire file if desired.
exclude=false
for exclude_item in "${EXCLUDE_FILES[@]}"; do
if [[ "$exclude_item" == "$contract_name" ]]; then
exclude=true
break
fi
done
if [[ "$exclude" == true ]]; then
continue
fi
# Continue if no matching contract name is found. Can happen in Solidity
# files where no contracts/interfaces are defined with the same name as the
# file. Still OK because a separate artifact *will* be generated for the
# specific contract/interface.
if [ "$found" = false ]; then
continue
fi
# If contract name does not start with an "I", throw an error
if [[ "$contract_name" != I* ]]; then
if ! grep -q "^$contract_name$" "$REPORTED_INTERFACES_FILE"; then
# If contract kind is not "interface", skip the file
if [ "$contract_kind" != "interface" ]; then
continue
fi
# If contract name does not start with an "I", throw an error
if [[ "$contract_name" != I* ]]; then
if ! grep -q "^$contract_name$" "$REPORTED_INTERFACES_FILE"; then
echo "$contract_name" >> "$REPORTED_INTERFACES_FILE"
if ! is_excluded "$contract_name"; then
echo "Issue found in ABI for interface $contract_name from file $interface_file."
echo "Interface $contract_name does not start with 'I'."
echo "$contract_name" >> "$REPORTED_INTERFACES_FILE"
issues_detected=true
fi
continue
fi
continue
fi
# Construct the corresponding contract name by removing the leading "I"
contract_basename=${contract_name:1}
corresponding_contract_file="$CONTRACTS_BASE/forge-artifacts/$contract_basename.sol/$contract_basename.json"
# Construct the corresponding contract name by removing the leading "I"
contract_basename=${contract_name:1}
corresponding_contract_file="$CONTRACTS_BASE/forge-artifacts/$contract_basename.sol/$contract_basename.json"
# Check if the corresponding contract file exists
if [ -f "$corresponding_contract_file" ]; then
# Extract and compare ABIs excluding constructors
interface_abi=$(jq '[.abi[] | select(.type != "constructor")]' < "$interface_file")
contract_abi=$(jq '[.abi[] | select(.type != "constructor")]' < "$corresponding_contract_file")
# Use jq to compare the ABIs
if ! diff_result=$(diff -u <(echo "$interface_abi" | jq -S .) <(echo "$contract_abi" | jq -S .)); then
if ! grep -q "^$contract_name$" "$REPORTED_INTERFACES_FILE"; then
echo "Issue found in ABI for interface $contract_name from file $interface_file."
echo "Differences found in ABI between interface $contract_name and actual contract $contract_basename."
if [ "$no_diff" = false ]; then
echo "$diff_result"
fi
echo "$contract_name" >> "$REPORTED_INTERFACES_FILE"
issues_detected=true
# Skip the file if the corresponding contract file does not exist
if [ ! -f "$corresponding_contract_file" ]; then
continue
fi
# Extract and compare ABIs excluding constructors
interface_abi=$(jq '[.abi[] | select(.type != "constructor")]' < "$interface_file")
contract_abi=$(jq '[.abi[] | select(.type != "constructor")]' < "$corresponding_contract_file")
# Use jq to compare the ABIs
if ! diff_result=$(diff -u <(echo "$interface_abi" | jq -S .) <(echo "$contract_abi" | jq -S .)); then
if ! grep -q "^$contract_name$" "$REPORTED_INTERFACES_FILE"; then
echo "$contract_name" >> "$REPORTED_INTERFACES_FILE"
if ! is_excluded "$contract_name"; then
echo "Issue found in ABI for interface $contract_name from file $interface_file."
echo "Differences found in ABI between interface $contract_name and actual contract $contract_basename."
if [ "$no_diff" = false ]; then
echo "$diff_result"
fi
issues_detected=true
fi
fi
done <<< "$contract_definitions"
continue
fi
done
# Check for unnecessary exclusions
for exclude_item in "${EXCLUDE_CONTRACTS[@]}"; do
if ! grep -q "^$exclude_item$" "$REPORTED_INTERFACES_FILE"; then
echo "Warning: $exclude_item is in the exclude list but was not reported as an issue."
echo "Consider removing it from the EXCLUDE_CONTRACTS list in the script."
fi
done
# Fail the script if any issues were detected
if [ "$issues_detected" = true ]; then
echo "Issues were detected while validating interface files."
echo "If the interface is an external dependency or should otherwise be excluded from this"
echo "check, add the interface name to the EXCLUDE_FILES list in the script. This will prevent"
echo "check, add the interface name to the EXCLUDE_CONTRACTS list in the script. This will prevent"
echo "the script from comparing it against a corresponding contract."
exit 1
else
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment