From a576a8949ca20e310f2fbb4ec0bd05a57ac3045f Mon Sep 17 00:00:00 2001 From: Aaryamann Challani <43716372+rymnc@users.noreply.github.com> Date: Mon, 3 Jun 2024 21:03:36 +0530 Subject: [PATCH] chore: emit rate commitment instead of idcommitment (#9) chore: emit rate commitment instead of idcommitment chore chore forge install: openzeppelin-contracts v4.9.6 forge install: openzeppelin-contracts-upgradeable v4.9.6 fix: use oz v4.9.6 --- .gas-snapshot | 28 +++++++++++++------------- .gitmodules | 7 ++++--- .vscode/settings.json | 3 ++- foundry.toml | 2 +- lib/openzeppelin-contracts | 1 + lib/openzeppelin-contracts-upgradeable | 2 +- lib/openzeppelin-foundry-upgrades | 1 - remappings.txt | 4 ++-- script/Deploy.s.sol | 2 +- src/WakuRlnV2.sol | 23 ++++++++++----------- test/WakuRlnV2.t.sol | 26 +++++++++++++++--------- 11 files changed, 53 insertions(+), 46 deletions(-) create mode 160000 lib/openzeppelin-contracts delete mode 160000 lib/openzeppelin-foundry-upgrades diff --git a/.gas-snapshot b/.gas-snapshot index c39b737..4d6ad85 100644 --- a/.gas-snapshot +++ b/.gas-snapshot @@ -1,14 +1,14 @@ -WakuRlnV2Test:test__IdCommitmentToMetadata__DoesntExist() (gas: 16723) -WakuRlnV2Test:test__InvalidPaginationQuery__EndIndexGTIdCommitmentIndex() (gas: 18260) -WakuRlnV2Test:test__InvalidPaginationQuery__StartIndexGTEndIndex() (gas: 16083) -WakuRlnV2Test:test__InvalidRegistration__DuplicateIdCommitment() (gas: 99824) -WakuRlnV2Test:test__InvalidRegistration__FullTree() (gas: 14343) -WakuRlnV2Test:test__InvalidRegistration__InvalidIdCommitment__LargerThanField() (gas: 15258) -WakuRlnV2Test:test__InvalidRegistration__InvalidIdCommitment__Zero() (gas: 14046) -WakuRlnV2Test:test__InvalidRegistration__InvalidUserMessageLimit__LargerThanMax() (gas: 17639) -WakuRlnV2Test:test__InvalidRegistration__InvalidUserMessageLimit__Zero() (gas: 14126) -WakuRlnV2Test:test__Upgrade() (gas: 3668443) -WakuRlnV2Test:test__ValidPaginationQuery(uint32) (runs: 1000, μ: 446587, ~: 159861) -WakuRlnV2Test:test__ValidPaginationQuery__OneElement() (gas: 120064) -WakuRlnV2Test:test__ValidRegistration(uint256,uint32) (runs: 1000, μ: 124674, ~: 124674) -WakuRlnV2Test:test__ValidRegistration__kats() (gas: 96917) \ No newline at end of file +WakuRlnV2Test:test__IdCommitmentToMetadata__DoesntExist() (gas: 16726) +WakuRlnV2Test:test__InvalidPaginationQuery__EndIndexGTcommitmentIndex() (gas: 18249) +WakuRlnV2Test:test__InvalidPaginationQuery__StartIndexGTEndIndex() (gas: 16130) +WakuRlnV2Test:test__InvalidRegistration__DuplicateIdCommitment() (gas: 99502) +WakuRlnV2Test:test__InvalidRegistration__FullTree() (gas: 14328) +WakuRlnV2Test:test__InvalidRegistration__InvalidIdCommitment__LargerThanField() (gas: 15229) +WakuRlnV2Test:test__InvalidRegistration__InvalidIdCommitment__Zero() (gas: 14004) +WakuRlnV2Test:test__InvalidRegistration__InvalidUserMessageLimit__LargerThanMax() (gas: 17703) +WakuRlnV2Test:test__InvalidRegistration__InvalidUserMessageLimit__Zero() (gas: 14085) +WakuRlnV2Test:test__Upgrade() (gas: 3791109) +WakuRlnV2Test:test__ValidPaginationQuery(uint32) (runs: 1000, μ: 445155, ~: 159972) +WakuRlnV2Test:test__ValidPaginationQuery__OneElement() (gas: 119773) +WakuRlnV2Test:test__ValidRegistration(uint256,uint32) (runs: 1000, μ: 124408, ~: 124408) +WakuRlnV2Test:test__ValidRegistration__kats() (gas: 96616) \ No newline at end of file diff --git a/.gitmodules b/.gitmodules index 485d439..81f929e 100644 --- a/.gitmodules +++ b/.gitmodules @@ -2,9 +2,10 @@ branch = "v1" path = lib/forge-std url = https://github.com/foundry-rs/forge-std -[submodule "lib/openzeppelin-foundry-upgrades"] - path = lib/openzeppelin-foundry-upgrades - url = https://github.com/OpenZeppelin/openzeppelin-foundry-upgrades + ignore = dirty +[submodule "lib/openzeppelin-contracts"] + path = lib/openzeppelin-contracts + url = https://github.com/OpenZeppelin/openzeppelin-contracts [submodule "lib/openzeppelin-contracts-upgradeable"] path = lib/openzeppelin-contracts-upgradeable url = https://github.com/OpenZeppelin/openzeppelin-contracts-upgradeable diff --git a/.vscode/settings.json b/.vscode/settings.json index c6fd597..b6a7777 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -6,5 +6,6 @@ "editor.defaultFormatter": "tamasfe.even-better-toml" }, "npm.exclude": "**/lib/**", - "solidity.formatter": "forge" + "solidity.formatter": "forge", + "solidity.compileUsingRemoteVersion": "v0.8.24+commit.e11b9ed9" } diff --git a/foundry.toml b/foundry.toml index 3e2d7fc..b08b955 100644 --- a/foundry.toml +++ b/foundry.toml @@ -5,7 +5,7 @@ block_timestamp = 1_680_220_800 # March 31, 2023 at 00:00 GMT bytecode_hash = "none" cbor_metadata = false - evm_version = "cancun" + evm_version = "paris" fuzz = { runs = 1_000 } gas_reports = ["*"] libs = ["lib"] diff --git a/lib/openzeppelin-contracts b/lib/openzeppelin-contracts new file mode 160000 index 0000000..dc44c9f --- /dev/null +++ b/lib/openzeppelin-contracts @@ -0,0 +1 @@ +Subproject commit dc44c9f1a4c3b10af99492eed84f83ed244203f6 diff --git a/lib/openzeppelin-contracts-upgradeable b/lib/openzeppelin-contracts-upgradeable index 0a71a5e..2d081f2 160000 --- a/lib/openzeppelin-contracts-upgradeable +++ b/lib/openzeppelin-contracts-upgradeable @@ -1 +1 @@ -Subproject commit 0a71a5ebfbf4136cae3176e5cc9bcc5efc23f76b +Subproject commit 2d081f24cac1a867f6f73d512f2022e1fa987854 diff --git a/lib/openzeppelin-foundry-upgrades b/lib/openzeppelin-foundry-upgrades deleted file mode 160000 index 4cd15fc..0000000 --- a/lib/openzeppelin-foundry-upgrades +++ /dev/null @@ -1 +0,0 @@ -Subproject commit 4cd15fc50b141c77d8cc9ff8efb44d00e841a299 diff --git a/remappings.txt b/remappings.txt index 28510ac..6973726 100644 --- a/remappings.txt +++ b/remappings.txt @@ -1,5 +1,5 @@ forge-std/=lib/forge-std/src/ @zk-kit/imt.sol/=node_modules/@zk-kit/imt.sol/contracts poseidon-solidity/=node_modules/poseidon-solidity/ -@openzeppelin/contracts/=lib/openzeppelin-contracts-upgradeable/lib/openzeppelin-contracts/contracts/ -@openzeppelin/contracts-upgradeable/=lib/openzeppelin-contracts-upgradeable/contracts/ +@openzeppelin/contracts/=lib/openzeppelin-contracts/contracts/ +@openzeppelin/contracts-upgradeable/=lib/openzeppelin-contracts-upgradeable/contracts/ \ No newline at end of file diff --git a/script/Deploy.s.sol b/script/Deploy.s.sol index 3c1f37f..a0d8b9a 100644 --- a/script/Deploy.s.sol +++ b/script/Deploy.s.sol @@ -13,7 +13,7 @@ import { DeploymentConfig } from "./DeploymentConfig.s.sol"; contract Deploy is BaseScript { function run() public broadcast returns (WakuRlnV2 w, address impl) { impl = address(new WakuRlnV2()); - bytes memory data = abi.encodeCall(WakuRlnV2.initialize, (msg.sender, 20)); + bytes memory data = abi.encodeCall(WakuRlnV2.initialize, 20); address proxy = address(new ERC1967Proxy(impl, data)); w = WakuRlnV2(proxy); } diff --git a/src/WakuRlnV2.sol b/src/WakuRlnV2.sol index 640e193..dcc567b 100644 --- a/src/WakuRlnV2.sol +++ b/src/WakuRlnV2.sol @@ -38,7 +38,7 @@ contract WakuRlnV2 is Initializable, OwnableUpgradeable, UUPSUpgradeable { uint32 public SET_SIZE; /// @notice The index of the next member to be registered - uint32 public idCommitmentIndex; + uint32 public commitmentIndex; /// @notice the membership metadata of the member struct MembershipInfo { @@ -58,10 +58,9 @@ contract WakuRlnV2 is Initializable, OwnableUpgradeable, UUPSUpgradeable { LazyIMTData public imtData; /// Emitted when a new member is added to the set - /// @param idCommitment The idCommitment of the member - /// @param userMessageLimit the user message limit of the member + /// @param rateCommitment the rateCommitment of the member /// @param index The index of the member in the set - event MemberRegistered(uint256 idCommitment, uint32 userMessageLimit, uint32 index); + event MemberRegistered(uint256 rateCommitment, uint32 index); /// @notice the modifier to check if the idCommitment is valid /// @param idCommitment The idCommitment of the member @@ -81,14 +80,14 @@ contract WakuRlnV2 is Initializable, OwnableUpgradeable, UUPSUpgradeable { _disableInitializers(); } - function initialize(address initialOwner, uint32 maxMessageLimit) public initializer { - __Ownable_init(initialOwner); + function initialize(uint32 maxMessageLimit) public initializer { + __Ownable_init(); __UUPSUpgradeable_init(); MAX_MESSAGE_LIMIT = maxMessageLimit; SET_SIZE = uint32(1 << DEPTH); deployedBlockNumber = uint32(block.number); LazyIMT.init(imtData, DEPTH); - idCommitmentIndex = 0; + commitmentIndex = 0; } function _authorizeUpgrade(address newImplementation) internal override onlyOwner { } // solhint-disable-line @@ -153,15 +152,15 @@ contract WakuRlnV2 is Initializable, OwnableUpgradeable, UUPSUpgradeable { /// @param userMessageLimit The message limit of the member function _register(uint256 idCommitment, uint32 userMessageLimit) internal { if (memberExists(idCommitment)) revert DuplicateIdCommitment(); - if (idCommitmentIndex >= SET_SIZE) revert FullTree(); + if (commitmentIndex >= SET_SIZE) revert FullTree(); uint256 rateCommitment = PoseidonT3.hash([idCommitment, userMessageLimit]); - MembershipInfo memory member = MembershipInfo({ userMessageLimit: userMessageLimit, index: idCommitmentIndex }); + MembershipInfo memory member = MembershipInfo({ userMessageLimit: userMessageLimit, index: commitmentIndex }); LazyIMT.insert(imtData, rateCommitment); memberInfo[idCommitment] = member; - emit MemberRegistered(idCommitment, userMessageLimit, idCommitmentIndex); - idCommitmentIndex += 1; + emit MemberRegistered(rateCommitment, commitmentIndex); + commitmentIndex += 1; } /// @notice Returns the commitments of a range of members @@ -170,7 +169,7 @@ contract WakuRlnV2 is Initializable, OwnableUpgradeable, UUPSUpgradeable { /// @return The commitments of the members function getCommitments(uint32 startIndex, uint32 endIndex) public view returns (uint256[] memory) { if (startIndex > endIndex) revert InvalidPaginationQuery(startIndex, endIndex); - if (endIndex > idCommitmentIndex) revert InvalidPaginationQuery(startIndex, endIndex); + if (endIndex > commitmentIndex) revert InvalidPaginationQuery(startIndex, endIndex); uint256[] memory commitments = new uint256[](endIndex - startIndex + 1); for (uint32 i = startIndex; i <= endIndex; i++) { diff --git a/test/WakuRlnV2.t.sol b/test/WakuRlnV2.t.sol index bb67431..56147fb 100644 --- a/test/WakuRlnV2.t.sol +++ b/test/WakuRlnV2.t.sol @@ -7,6 +7,7 @@ import { DeploymentConfig } from "../script/DeploymentConfig.s.sol"; import "../src/WakuRlnV2.sol"; // solhint-disable-line import { PoseidonT3 } from "poseidon-solidity/PoseidonT3.sol"; import { UUPSUpgradeable } from "@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol"; +import { ERC1967Proxy } from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol"; contract WakuRlnV2Test is Test { WakuRlnV2 internal w; @@ -27,7 +28,7 @@ contract WakuRlnV2Test is Test { vm.resumeGasMetering(); w.register(idCommitment, userMessageLimit); vm.pauseGasMetering(); - assertEq(w.idCommitmentIndex(), 1); + assertEq(w.commitmentIndex(), 1); assertEq(w.memberExists(idCommitment), true); (uint32 fetchedUserMessageLimit, uint32 index) = w.memberInfo(idCommitment); assertEq(fetchedUserMessageLimit, userMessageLimit); @@ -138,14 +139,14 @@ contract WakuRlnV2Test is Test { |---------------------|-----------------------------------------------------|------|--------|-------| | MAX_MESSAGE_LIMIT | uint32 | 0 | 0 | 4 | | SET_SIZE | uint32 | 0 | 4 | 4 | - | idCommitmentIndex | uint32 | 0 | 8 | 4 | + | commitmentIndex | uint32 | 0 | 8 | 4 | | memberInfo | mapping(uint256 => struct WakuRlnV2.MembershipInfo) | 1 | 0 | 32 | | deployedBlockNumber | uint32 | 2 | 0 | 4 | | imtData | struct LazyIMTData | 3 | 0 | 64 |*/ // we set MAX_MESSAGE_LIMIT to 20 (unaltered) // we set SET_SIZE to 4294967295 (1 << 20) (unaltered) - // we set idCommitmentIndex to 4294967295 (1 << 20) (altered) - vm.store(address(w), bytes32(0), 0x0000000000000000000000000000000000000000ffffffffffffffff00000014); + // we set commitmentIndex to 4294967295 (1 << 20) (altered) + vm.store(address(w), bytes32(uint256(201)), 0x0000000000000000000000000000000000000000ffffffffffffffff00000014); vm.expectRevert(FullTree.selector); w.register(1, userMessageLimit); } @@ -155,7 +156,7 @@ contract WakuRlnV2Test is Test { w.getCommitments(1, 0); } - function test__InvalidPaginationQuery__EndIndexGTIdCommitmentIndex() external { + function test__InvalidPaginationQuery__EndIndexGTcommitmentIndex() external { vm.expectRevert(abi.encodeWithSelector(InvalidPaginationQuery.selector, 0, 2)); w.getCommitments(0, 2); } @@ -189,16 +190,21 @@ contract WakuRlnV2Test is Test { } function test__Upgrade() external { - address newImplementation = address(new WakuRlnV2()); - bytes memory data; - UUPSUpgradeable(address(w)).upgradeToAndCall(newImplementation, data); + address testImpl = address(new WakuRlnV2()); + bytes memory data = abi.encodeCall(WakuRlnV2.initialize, 20); + address proxy = address(new ERC1967Proxy(testImpl, data)); + + address newImpl = address(new WakuRlnV2()); + UUPSUpgradeable(proxy).upgradeTo(newImpl); // ensure that the implementation is set correctly // ref: // solhint-disable-next-line // https://github.com/OpenZeppelin/openzeppelin-foundry-upgrades/blob/4cd15fc50b141c77d8cc9ff8efb44d00e841a299/src/internal/Core.sol#L289 address fetchedImpl = address( - uint160(uint256(vm.load(address(w), 0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc))) + uint160( + uint256(vm.load(address(proxy), 0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc)) + ) ); - assertEq(fetchedImpl, newImplementation); + assertEq(fetchedImpl, newImpl); } }