Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: properly check all interfaces #11734

Merged
merged 1 commit into from
Sep 4, 2024
Merged

fix: properly check all interfaces #11734

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
165 changes: 92 additions & 73 deletions packages/contracts-bedrock/scripts/checks/check-interfaces.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand All @@ -52,11 +43,6 @@ EXCLUDE_FILES=(

# Kontrol
"KontrolCheatsBase"
"KontrolCheats"

# Error definition files
"CommonErrors"
"Errors"

# TODO: Interfaces that need to be fixed
"IPreimageOracle"
Expand All @@ -72,105 +58,138 @@ 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

# 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"
}

# 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
Expand Down