From 4217bc3f092dfed2d76a0e00060e5d210fb1c4d8 Mon Sep 17 00:00:00 2001 From: Michael Reynolds <103044686+mireynolds@users.noreply.github.com> Date: Sat, 15 Jul 2023 19:16:16 +0100 Subject: [PATCH] Open zep to v4.9, delete slither report --- .github/workflows/slither-report.yml | 2 +- .gitignore | 2 - .gitmodules | 3 + .vscode/settings.json | 10 +++ lib/forge-std | 2 +- lib/openzeppelin-contracts | 2 +- remappings.txt | 6 ++ slitherReport.md | 123 --------------------------- src/examples/Airdrop.sol | 13 ++- test/examples/Airdrop.t.sol | 7 +- 10 files changed, 30 insertions(+), 140 deletions(-) create mode 100644 .vscode/settings.json create mode 100644 remappings.txt delete mode 100644 slitherReport.md diff --git a/.github/workflows/slither-report.yml b/.github/workflows/slither-report.yml index 91cb6ac..6e4ea30 100644 --- a/.github/workflows/slither-report.yml +++ b/.github/workflows/slither-report.yml @@ -29,7 +29,7 @@ jobs: run: forge build --build-info --skip test --skip script - name: Generate slither report - run: slither . --compile-force-framework "foundry" --foundry-out-directory "out" --ignore-compile --fail-medium --skip-clean --filter-paths "lib" --checklist --markdown-root ${{ github.server_url }}/${{ github.repository }}/blob/${{ github.sha }}/ > slitherReport.md + run: slither . --compile-force-framework "foundry" --foundry-out-directory "out" --ignore-compile --skip-clean --filter-paths "lib" --checklist --markdown-root ${{ github.server_url }}/${{ github.repository }}/blob/${{ github.sha }}/ > slitherReport.md - name: Check slither report run: | diff --git a/.gitignore b/.gitignore index 94103bc..a56dff0 100644 --- a/.gitignore +++ b/.gitignore @@ -2,6 +2,4 @@ cache/ out/ broadcast/ Makefile -.vscode -remappings.txt abi.json \ No newline at end of file diff --git a/.gitmodules b/.gitmodules index dd714c8..7b363c2 100644 --- a/.gitmodules +++ b/.gitmodules @@ -1,9 +1,12 @@ [submodule "lib/forge-std"] path = lib/forge-std url = https://github.com/foundry-rs/forge-std + branch = master [submodule "lib/openzeppelin-contracts"] path = lib/openzeppelin-contracts url = https://github.com/openzeppelin/openzeppelin-contracts + branch = release-v4.9 # Master branch is not safe to use [submodule "lib/murky"] path = lib/murky url = https://github.com/dmfxyz/murky + branch = main \ No newline at end of file diff --git a/.vscode/settings.json b/.vscode/settings.json new file mode 100644 index 0000000..86153fc --- /dev/null +++ b/.vscode/settings.json @@ -0,0 +1,10 @@ +{ + "editor.formatOnSave": true, + "[solidity]": { + "editor.defaultFormatter": "NomicFoundation.hardhat-solidity" + }, + "[toml]": { + "editor.defaultFormatter": "tamasfe.even-better-toml" + }, + "solidity.formatter": "forge" +} diff --git a/lib/forge-std b/lib/forge-std index 20872c5..74cfb77 160000 --- a/lib/forge-std +++ b/lib/forge-std @@ -1 +1 @@ -Subproject commit 20872c5b1900526579159bdc6967f6b48c22e50e +Subproject commit 74cfb77e308dd188d2f58864aaf44963ae6b88b1 diff --git a/lib/openzeppelin-contracts b/lib/openzeppelin-contracts index dfef6a6..d26025b 160000 --- a/lib/openzeppelin-contracts +++ b/lib/openzeppelin-contracts @@ -1 +1 @@ -Subproject commit dfef6a68ee18dbd2e1f5a099061a3b8a0e404485 +Subproject commit d26025b4103b8d053684a75129a16852d1ae95c0 diff --git a/remappings.txt b/remappings.txt new file mode 100644 index 0000000..a530ee7 --- /dev/null +++ b/remappings.txt @@ -0,0 +1,6 @@ +ds-test/=lib/forge-std/lib/ds-test/src/ +erc4626-tests/=lib/openzeppelin-contracts/lib/erc4626-tests/ +forge-std/=lib/forge-std/src/ +murky/=lib/murky/src/ +openzeppelin-contracts/=lib/openzeppelin-contracts/ +openzeppelin/=lib/openzeppelin-contracts/contracts/ diff --git a/slitherReport.md b/slitherReport.md deleted file mode 100644 index b70c263..0000000 --- a/slitherReport.md +++ /dev/null @@ -1,123 +0,0 @@ -**THIS CHECKLIST IS NOT COMPLETE**. Use `--show-ignored-findings` to show all the results. -Summary - - [assembly](#assembly) (9 results) (Informational) - - [solc-version](#solc-version) (7 results) (Informational) - - [low-level-calls](#low-level-calls) (1 results) (Informational) -## assembly -Impact: Informational -Confidence: High - - [ ] ID-0 -[DelegateRegistry._writeDelegationAddresses(bytes32,RegistryStorage.Positions,RegistryStorage.Positions,address,address,address)](src/DelegateRegistry.sol#L295-L303) uses assembly - - [INLINE ASM](src/DelegateRegistry.sol#L299-L302) - -src/DelegateRegistry.sol#L295-L303 - - - - [ ] ID-1 -[DelegateRegistry._writeDelegation(bytes32,RegistryStorage.Positions,bytes32)](src/DelegateRegistry.sol#L281-L285) uses assembly - - [INLINE ASM](src/DelegateRegistry.sol#L282-L284) - -src/DelegateRegistry.sol#L281-L285 - - - - [ ] ID-2 -[DelegateRegistry.readSlots(bytes32[])](src/DelegateRegistry.sol#L251-L257) uses assembly - - [INLINE ASM](src/DelegateRegistry.sol#L254-L256) - -src/DelegateRegistry.sol#L251-L257 - - - - [ ] ID-3 -[DelegateRegistry._loadDelegationUint(bytes32,RegistryStorage.Positions)](src/DelegateRegistry.sol#L361-L365) uses assembly - - [INLINE ASM](src/DelegateRegistry.sol#L362-L364) - -src/DelegateRegistry.sol#L361-L365 - - - - [ ] ID-4 -[DelegateRegistry._writeDelegation(bytes32,RegistryStorage.Positions,uint256)](src/DelegateRegistry.sol#L288-L292) uses assembly - - [INLINE ASM](src/DelegateRegistry.sol#L289-L291) - -src/DelegateRegistry.sol#L288-L292 - - - - [ ] ID-5 -[DelegateRegistry._loadDelegationBytes32(bytes32,RegistryStorage.Positions)](src/DelegateRegistry.sol#L354-L358) uses assembly - - [INLINE ASM](src/DelegateRegistry.sol#L355-L357) - -src/DelegateRegistry.sol#L354-L358 - - - - [ ] ID-6 -[DelegateRegistry._loadFrom(bytes32,RegistryStorage.Positions)](src/DelegateRegistry.sol#L368-L374) uses assembly - - [INLINE ASM](src/DelegateRegistry.sol#L370-L372) - -src/DelegateRegistry.sol#L368-L374 - - - - [ ] ID-7 -[DelegateRegistry._loadDelegationAddresses(bytes32,RegistryStorage.Positions,RegistryStorage.Positions)](src/DelegateRegistry.sol#L377-L389) uses assembly - - [INLINE ASM](src/DelegateRegistry.sol#L384-L387) - -src/DelegateRegistry.sol#L377-L389 - - - - [ ] ID-8 -[DelegateRegistry.readSlot(bytes32)](src/DelegateRegistry.sol#L245-L249) uses assembly - - [INLINE ASM](src/DelegateRegistry.sol#L246-L248) - -src/DelegateRegistry.sol#L245-L249 - - -## solc-version -Impact: Informational -Confidence: High - - [ ] ID-9 -Pragma version[>=0.8.13](src/IDelegateRegistry.sol#L2) allows old versions - -src/IDelegateRegistry.sol#L2 - - - - [ ] ID-10 -Pragma version[^0.8.20](src/examples/Airdrop.sol#L2) necessitates a version too recent to be trusted. Consider deploying with 0.8.18. - -src/examples/Airdrop.sol#L2 - - - - [ ] ID-11 -Pragma version[^0.8.20](test/tools/RegistryHarness.sol#L2) necessitates a version too recent to be trusted. Consider deploying with 0.8.18. - -test/tools/RegistryHarness.sol#L2 - - - - [ ] ID-12 -solc-0.8.20 is not recommended for deployment - - - [ ] ID-13 -Pragma version[^0.8.20](src/examples/IPLicenseCheck.sol#L2) necessitates a version too recent to be trusted. Consider deploying with 0.8.18. - -src/examples/IPLicenseCheck.sol#L2 - - - - [ ] ID-14 -Pragma version[^0.8.20](src/DelegateRegistry.sol#L2) necessitates a version too recent to be trusted. Consider deploying with 0.8.18. - -src/DelegateRegistry.sol#L2 - - - - [ ] ID-15 -Pragma version[^0.8.20](src/examples/DelegateClaim.sol#L2) necessitates a version too recent to be trusted. Consider deploying with 0.8.18. - -src/examples/DelegateClaim.sol#L2 - - -## low-level-calls -Impact: Informational -Confidence: High - - [ ] ID-16 -Low level call in [DelegateRegistry.multicall(bytes[])](src/DelegateRegistry.sol#L34-L44): - - [(success,results[i]) = address(this).delegatecall(data[i])](src/DelegateRegistry.sol#L40) - -src/DelegateRegistry.sol#L34-L44 - - diff --git a/src/examples/Airdrop.sol b/src/examples/Airdrop.sol index 7a334c3..8e52fea 100644 --- a/src/examples/Airdrop.sol +++ b/src/examples/Airdrop.sol @@ -1,9 +1,9 @@ // SPDX-License-Identifier: CC0-1.0 pragma solidity ^0.8.20; -import {Merkle} from "murky/Merkle.sol"; -import {ERC20} from "openzeppelin-contracts/contracts/token/ERC20/ERC20.sol"; -import {Math} from "openzeppelin-contracts/contracts/utils/math/Math.sol"; +import {MerkleProof} from "openzeppelin/utils/cryptography/MerkleProof.sol"; +import {ERC20} from "openzeppelin/token/ERC20/ERC20.sol"; +import {Math} from "openzeppelin/utils/math/Math.sol"; import {DelegateClaim} from "src/examples/DelegateClaim.sol"; /** @@ -12,7 +12,6 @@ import {DelegateClaim} from "src/examples/DelegateClaim.sol"; * @dev Inherits the DelegateClaim contract to allow delegates to claim on behalf of vaults. */ contract Airdrop is ERC20, DelegateClaim { - Merkle public immutable merkle; bytes32 public immutable merkleRoot; mapping(address vault => uint256 claimed) public claimed; @@ -22,15 +21,13 @@ contract Airdrop is ERC20, DelegateClaim { * @param referenceToken_ The address of the reference token used by delegateClaimable inherited from DelegateClaim. * @param totalSupply_ The total supply of the airdrop token. * @param merkleRoot_ The root hash of the merkle tree representing the airdrop. - * @param merkle_ The address of the murky Merkle contract used for proof verification. */ - constructor(address registry_, address referenceToken_, bytes32 airdropRight, uint256 totalSupply_, bytes32 merkleRoot_, address merkle_) + constructor(address registry_, address referenceToken_, bytes32 airdropRight, uint256 totalSupply_, bytes32 merkleRoot_) ERC20("Airdrop", "Air") DelegateClaim(registry_, referenceToken_, airdropRight) { _mint(address(this), totalSupply_); merkleRoot = merkleRoot_; - merkle = Merkle(merkle_); } /** @@ -43,7 +40,7 @@ contract Airdrop is ERC20, DelegateClaim { */ function claim(address vault, uint256 claimAmount, uint256 airdropSize, bytes32[] calldata merkleProof) external { // First verify that airdrop for vault of amount airdropSize exists - require(merkle.verifyProof(merkleRoot, merkleProof, keccak256(abi.encodePacked(vault, airdropSize))), "Invalid Proof"); + require(MerkleProof.verifyCalldata(merkleProof, merkleRoot, keccak256(abi.encodePacked(vault, airdropSize))), "Invalid Proof"); // Set claimable to the minimum of claimAmount and the maximum remaining airdrop tokens that can be claimed by // the vault uint256 claimable = Math.min(claimAmount, airdropSize - claimed[vault]); diff --git a/test/examples/Airdrop.t.sol b/test/examples/Airdrop.t.sol index 42ce30c..6a454c2 100644 --- a/test/examples/Airdrop.t.sol +++ b/test/examples/Airdrop.t.sol @@ -7,7 +7,7 @@ import {console2} from "forge-std/console2.sol"; import {Merkle} from "murky/Merkle.sol"; import {Airdrop} from "src/examples/Airdrop.sol"; import {DelegateRegistry} from "src/DelegateRegistry.sol"; -import {Math} from "openzeppelin-contracts/contracts/utils/math/Math.sol"; +import {Math} from "openzeppelin/utils/math/Math.sol"; contract AirdropTest is Test { Merkle public merkle; @@ -97,9 +97,8 @@ contract AirdropTest is Test { totalSupply_ += airdropData[i].amount; } // Create airdrop token - airdrop = new Airdrop(address(registry), referenceToken, acceptableRight, totalSupply_, merkleRoot, address(merkle)); + airdrop = new Airdrop(address(registry), referenceToken, acceptableRight, totalSupply_, merkleRoot); // Check data is stored correctly in token - assertEq(address(merkle), address(airdrop.merkle())); assertEq(address(registry), address(airdrop.delegateRegistry())); assertEq(merkleRoot, airdrop.merkleRoot()); assertEq(referenceToken, airdrop.referenceToken()); @@ -155,7 +154,7 @@ contract AirdropTest is Test { totalSupply_ += airdropData[i].amount; } // Create airdrop token - airdrop = new Airdrop(address(registry), referenceToken, acceptableRight, totalSupply_, merkleRoot, address(merkle)); + airdrop = new Airdrop(address(registry), referenceToken, acceptableRight, totalSupply_, merkleRoot); // Create delegates _createDelegates(delegateSeed, allowanceSeed, n); // Try to claim with delegate