From 518fd94dc6d034780a273694472bbfc2908f4aa4 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Thu, 10 Oct 2024 20:24:33 +0200 Subject: [PATCH] Fix account abstraction upgradeable testing (#5248) --- .github/workflows/checks.yml | 2 +- contracts/abstraction/utils/ERC4337Utils.sol | 4 ++-- contracts/abstraction/utils/ERC7579Utils.sol | 11 ++++++----- contracts/mocks/Stateless.sol | 2 ++ contracts/package.json | 3 ++- package.json | 3 ++- scripts/upgradeable/patch-apply.sh | 2 +- scripts/upgradeable/patch-save.sh | 2 +- scripts/upgradeable/transpile.sh | 10 +++++++++- scripts/upgradeable/upgradeable.excluded.patch | 17 +++++++++++++++++ slither.config.json | 2 +- 11 files changed, 44 insertions(+), 14 deletions(-) create mode 100644 scripts/upgradeable/upgradeable.excluded.patch diff --git a/.github/workflows/checks.yml b/.github/workflows/checks.yml index 2f1d5aa687e..6a78eda4d9f 100644 --- a/.github/workflows/checks.yml +++ b/.github/workflows/checks.yml @@ -130,4 +130,4 @@ jobs: with: check_hidden: true check_filenames: true - skip: package-lock.json,*.pdf + skip: package-lock.json,*.pdf,vendor diff --git a/contracts/abstraction/utils/ERC4337Utils.sol b/contracts/abstraction/utils/ERC4337Utils.sol index 3d95db10815..f3f049d14cc 100644 --- a/contracts/abstraction/utils/ERC4337Utils.sol +++ b/contracts/abstraction/utils/ERC4337Utils.sol @@ -62,8 +62,8 @@ library ERC4337Utils { if (validationData == 0) { return (address(0), false); } else { - (address agregator, uint48 validAfter, uint48 validUntil) = parseValidationData(validationData); - return (agregator, block.timestamp > validUntil || block.timestamp < validAfter); + (address aggregator_, uint48 validAfter, uint48 validUntil) = parseValidationData(validationData); + return (aggregator_, block.timestamp > validUntil || block.timestamp < validAfter); } } diff --git a/contracts/abstraction/utils/ERC7579Utils.sol b/contracts/abstraction/utils/ERC7579Utils.sol index b35e157101b..4a014d3e3e5 100644 --- a/contracts/abstraction/utils/ERC7579Utils.sol +++ b/contracts/abstraction/utils/ERC7579Utils.sol @@ -11,14 +11,15 @@ type ExecType is bytes1; type ModeSelector is bytes4; type ModePayload is bytes22; +// slither-disable-next-line unused-state library ERC7579Utils { using Packing for *; - CallType constant CALLTYPE_SINGLE = CallType.wrap(0x00); - CallType constant CALLTYPE_BATCH = CallType.wrap(0x01); - CallType constant CALLTYPE_DELEGATECALL = CallType.wrap(0xFF); - ExecType constant EXECTYPE_DEFAULT = ExecType.wrap(0x00); - ExecType constant EXECTYPE_TRY = ExecType.wrap(0x01); + CallType internal constant CALLTYPE_SINGLE = CallType.wrap(0x00); + CallType internal constant CALLTYPE_BATCH = CallType.wrap(0x01); + CallType internal constant CALLTYPE_DELEGATECALL = CallType.wrap(0xFF); + ExecType internal constant EXECTYPE_DEFAULT = ExecType.wrap(0x00); + ExecType internal constant EXECTYPE_TRY = ExecType.wrap(0x01); function encodeMode( CallType callType, diff --git a/contracts/mocks/Stateless.sol b/contracts/mocks/Stateless.sol index 846c77d98e8..e0193a91442 100644 --- a/contracts/mocks/Stateless.sol +++ b/contracts/mocks/Stateless.sol @@ -22,6 +22,8 @@ import {ERC165} from "../utils/introspection/ERC165.sol"; import {ERC165Checker} from "../utils/introspection/ERC165Checker.sol"; import {ERC1967Utils} from "../proxy/ERC1967/ERC1967Utils.sol"; import {ERC721Holder} from "../token/ERC721/utils/ERC721Holder.sol"; +import {ERC4337Utils} from "../abstraction/utils/ERC4337Utils.sol"; +import {ERC7579Utils} from "../abstraction/utils/ERC7579Utils.sol"; import {Heap} from "../utils/structs/Heap.sol"; import {Math} from "../utils/math/Math.sol"; import {MerkleProof} from "../utils/cryptography/MerkleProof.sol"; diff --git a/contracts/package.json b/contracts/package.json index 845e8c4035d..2232d67ca52 100644 --- a/contracts/package.json +++ b/contracts/package.json @@ -5,7 +5,8 @@ "files": [ "**/*.sol", "/build/contracts/*.json", - "!/mocks/**/*" + "!/mocks/**/*", + "!/vendor/erc4337-entrypoint/**/*" ], "scripts": { "prepack": "bash ../scripts/prepack.sh", diff --git a/package.json b/package.json index a82ea47dd08..9075a50ad22 100644 --- a/package.json +++ b/package.json @@ -5,7 +5,8 @@ "private": true, "files": [ "/contracts/**/*.sol", - "!/contracts/mocks/**/*" + "!/contracts/mocks/**/*", + "!/contracts/vendor/erc4337-entrypoint" ], "scripts": { "compile": "hardhat compile", diff --git a/scripts/upgradeable/patch-apply.sh b/scripts/upgradeable/patch-apply.sh index d9e17589b05..e16cf76a4c2 100755 --- a/scripts/upgradeable/patch-apply.sh +++ b/scripts/upgradeable/patch-apply.sh @@ -3,7 +3,7 @@ set -euo pipefail DIRNAME="$(dirname -- "${BASH_SOURCE[0]}")" -PATCH="$DIRNAME/upgradeable.patch" +PATCH="$DIRNAME/${1:-upgradeable.patch}" error() { echo Error: "$*" >&2 diff --git a/scripts/upgradeable/patch-save.sh b/scripts/upgradeable/patch-save.sh index 111e6f1572a..de0cec6a5ef 100755 --- a/scripts/upgradeable/patch-save.sh +++ b/scripts/upgradeable/patch-save.sh @@ -3,7 +3,7 @@ set -euo pipefail DIRNAME="$(dirname -- "${BASH_SOURCE[0]}")" -PATCH="$DIRNAME/upgradeable.patch" +PATCH="$DIRNAME/${1:-upgradeable.patch}" error() { echo Error: "$*" >&2 diff --git a/scripts/upgradeable/transpile.sh b/scripts/upgradeable/transpile.sh index f7c848c1320..d31172ddf4e 100644 --- a/scripts/upgradeable/transpile.sh +++ b/scripts/upgradeable/transpile.sh @@ -5,13 +5,16 @@ set -euo pipefail -x VERSION="$(jq -r .version contracts/package.json)" DIRNAME="$(dirname -- "${BASH_SOURCE[0]}")" +# Apply patch to contracts that are transpiled bash "$DIRNAME/patch-apply.sh" sed -i'' -e "s//$VERSION/g" "contracts/package.json" git add contracts/package.json +# Build artifacts npm run clean npm run compile +# Check artifacts are correctly built build_info=($(jq -r '.input.sources | keys | if any(test("^contracts/mocks/.*\\bunreachable\\b")) then empty else input_filename end' artifacts/build-info/*)) build_info_num=${#build_info[@]} @@ -20,10 +23,13 @@ if [ $build_info_num -ne 1 ]; then exit 1 fi +# Apply changes to the excluded contracts (these don't need to in the artifact and may prevent compilation) +git apply -3 "$DIRNAME/upgradeable.excluded.patch" + # -D: delete original and excluded files # -b: use this build info file # -i: use included Initializable -# -x: exclude proxy-related contracts with a few exceptions +# -x: exclude vendored and proxy-related contracts with a few exceptions # -p: emit public initializer # -n: use namespaces # -N: exclude from namespaces transformation @@ -38,6 +44,8 @@ npx @openzeppelin/upgrade-safe-transpiler -D \ -x '!contracts/proxy/ERC1967/ERC1967Utils.sol' \ -x '!contracts/proxy/utils/UUPSUpgradeable.sol' \ -x '!contracts/proxy/beacon/IBeacon.sol' \ + -x 'contracts/vendor/**/*' \ + -x '!contracts/vendor/compound/ICompoundTimelock.sol' \ -p 'contracts/access/manager/AccessManager.sol' \ -p 'contracts/finance/VestingWallet.sol' \ -p 'contracts/governance/TimelockController.sol' \ diff --git a/scripts/upgradeable/upgradeable.excluded.patch b/scripts/upgradeable/upgradeable.excluded.patch new file mode 100644 index 00000000000..7c49e21e123 --- /dev/null +++ b/scripts/upgradeable/upgradeable.excluded.patch @@ -0,0 +1,17 @@ +diff --git a/contracts/vendor/erc4337-entrypoint/core/EntryPoint.sol b/contracts/vendor/erc4337-entrypoint/core/EntryPoint.sol +index 778115b1..44501524 100644 +--- a/contracts/vendor/erc4337-entrypoint/core/EntryPoint.sol ++++ b/contracts/vendor/erc4337-entrypoint/core/EntryPoint.sol +@@ -15,10 +15,8 @@ import "./Helpers.sol"; + import "./NonceManager.sol"; + import "./UserOperationLib.sol"; + +-// import "@openzeppelin/contracts/utils/introspection/ERC165.sol"; +-// import "@openzeppelin/contracts/utils/ReentrancyGuard.sol"; +-import "../../../utils/introspection/ERC165.sol"; // OZ edit +-import "../../../utils/ReentrancyGuard.sol"; // OZ edit ++import "@openzeppelin/contracts/utils/introspection/ERC165.sol"; ++import "@openzeppelin/contracts/utils/ReentrancyGuard.sol"; + + /* + * Account-Abstraction (EIP-4337) singleton EntryPoint implementation. diff --git a/slither.config.json b/slither.config.json index 069da1f3a21..fa52f4dd1dd 100644 --- a/slither.config.json +++ b/slither.config.json @@ -1,5 +1,5 @@ { "detectors_to_run": "arbitrary-send-erc20,array-by-reference,incorrect-shift,name-reused,rtlo,suicidal,uninitialized-state,uninitialized-storage,arbitrary-send-erc20-permit,controlled-array-length,controlled-delegatecall,delegatecall-loop,msg-value-loop,reentrancy-eth,unchecked-transfer,weak-prng,domain-separator-collision,erc20-interface,erc721-interface,locked-ether,mapping-deletion,shadowing-abstract,tautology,write-after-write,boolean-cst,reentrancy-no-eth,reused-constructor,tx-origin,unchecked-lowlevel,unchecked-send,variable-scope,void-cst,events-access,events-maths,incorrect-unary,boolean-equal,cyclomatic-complexity,deprecated-standards,erc20-indexed,function-init-state,pragma,unused-state,reentrancy-unlimited-gas,constable-states,immutable-states,var-read-using-this", - "filter_paths": "contracts/mocks,contracts-exposed", + "filter_paths": "contracts/mocks,contracts/vendor,contracts-exposed", "compile_force_framework": "hardhat" }