From 863d0c8206e9b71ef182850fda03bcef70db6dd5 Mon Sep 17 00:00:00 2001 From: julien Date: Sat, 28 Oct 2023 20:13:43 +0200 Subject: [PATCH 01/14] refactor(urd): `revokeRoot` => `revokePendingRoot` --- src/UniversalRewardsDistributor.sol | 4 ++-- src/interfaces/IUniversalRewardsDistributor.sol | 2 +- src/libraries/EventsLib.sol | 2 +- test/UniversalRewardsDistributorTest.sol | 14 +++++++------- 4 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/UniversalRewardsDistributor.sol b/src/UniversalRewardsDistributor.sol index 7256127..c016c17 100644 --- a/src/UniversalRewardsDistributor.sol +++ b/src/UniversalRewardsDistributor.sol @@ -170,12 +170,12 @@ contract UniversalRewardsDistributor is IUniversalRewardsDistributor { /// @dev This function can only be called by the owner of the distribution at any time. /// @dev Can be frontrunned by triggering the `acceptRoot` function in case the timelock has passed. This if the /// `owner` responsibility to trigger this function before the end of the timelock. - function revokeRoot() external onlyOwner { + function revokePendingRoot() external onlyOwner { require(pendingRoot.submittedAt != 0, ErrorsLib.NO_PENDING_ROOT); delete pendingRoot; - emit EventsLib.RootRevoked(); + emit EventsLib.PendingRootRevoked(); } /// @notice Sets the `owner` of the distribution to `newOwner`. diff --git a/src/interfaces/IUniversalRewardsDistributor.sol b/src/interfaces/IUniversalRewardsDistributor.sol index 5950085..701bc8a 100644 --- a/src/interfaces/IUniversalRewardsDistributor.sol +++ b/src/interfaces/IUniversalRewardsDistributor.sol @@ -28,7 +28,7 @@ interface IUniversalRewardsDistributor { function setRoot(bytes32 newRoot, bytes32 newIpfsHash) external; function setTimelock(uint256 newTimelock) external; function setRootUpdater(address updater, bool active) external; - function revokeRoot() external; + function revokePendingRoot() external; function setOwner(address newOwner) external; function submitRoot(bytes32 newRoot, bytes32 ipfsHash) external; diff --git a/src/libraries/EventsLib.sol b/src/libraries/EventsLib.sol index 3da3a8a..5938b43 100644 --- a/src/libraries/EventsLib.sol +++ b/src/libraries/EventsLib.sol @@ -26,7 +26,7 @@ library EventsLib { event RootUpdaterSet(address indexed rootUpdater, bool active); /// @notice Emitted when a merkle pending root is revoked. - event RootRevoked(); + event PendingRootRevoked(); /// @notice Emitted when rewards are claimed. /// @param account The address for which rewards are claimd rewards for. diff --git a/test/UniversalRewardsDistributorTest.sol b/test/UniversalRewardsDistributorTest.sol index 79f59f7..45f775b 100644 --- a/test/UniversalRewardsDistributorTest.sol +++ b/test/UniversalRewardsDistributorTest.sol @@ -368,21 +368,21 @@ contract UniversalRewardsDistributorTest is Test { distributionWithoutTimeLock.setRootUpdater(_addrFromHashedString("RANDOM_UPDATER"), active); } - function testRevokeRootShouldRevokeWhenCalledWithOwner() public { + function testRevokePendingRootShouldRevokeWhenCalledWithOwner() public { vm.prank(owner); distributionWithTimeLock.submitRoot(DEFAULT_ROOT, DEFAULT_IPFS_HASH); vm.prank(owner); vm.expectEmit(address(distributionWithTimeLock)); - emit EventsLib.RootRevoked(); - distributionWithTimeLock.revokeRoot(); + emit EventsLib.PendingRootRevoked(); + distributionWithTimeLock.revokePendingRoot(); PendingRoot memory pendingRoot = _getPendingRoot(distributionWithTimeLock); assertEq(pendingRoot.root, bytes32(0)); assertEq(pendingRoot.submittedAt, 0); } - function testRevokeRootShouldRevertIfNotOwner(bytes32 proposedRoot, address caller) public { + function testRevokePendingRootShouldRevertIfNotOwner(bytes32 proposedRoot, address caller) public { vm.assume(proposedRoot != bytes32(0) && caller != owner); vm.prank(owner); @@ -390,13 +390,13 @@ contract UniversalRewardsDistributorTest is Test { vm.prank(caller); vm.expectRevert(bytes(ErrorsLib.CALLER_NOT_OWNER)); - distributionWithTimeLock.revokeRoot(); + distributionWithTimeLock.revokePendingRoot(); } - function testRevokeRootShouldRevertWhenNoPendingRoot() public { + function testRevokePendingRootShouldRevertWhenNoPendingRoot() public { vm.prank(owner); vm.expectRevert(bytes(ErrorsLib.NO_PENDING_ROOT)); - distributionWithTimeLock.revokeRoot(); + distributionWithTimeLock.revokePendingRoot(); } function testSetOwner(address newOwner) public { From b06d6c75260f3632b0274b75401424eac21a4e42 Mon Sep 17 00:00:00 2001 From: julien Date: Tue, 31 Oct 2023 15:48:10 +0100 Subject: [PATCH 02/14] feat(URD): use _setPendingRoot for all pending root updates Signed-off-by: julien --- src/UniversalRewardsDistributor.sol | 18 +++++++++++------- src/libraries/EventsLib.sol | 5 +---- test/UniversalRewardsDistributorTest.sol | 6 +++--- 3 files changed, 15 insertions(+), 14 deletions(-) diff --git a/src/UniversalRewardsDistributor.sol b/src/UniversalRewardsDistributor.sol index c016c17..1f15847 100644 --- a/src/UniversalRewardsDistributor.sol +++ b/src/UniversalRewardsDistributor.sol @@ -83,8 +83,7 @@ contract UniversalRewardsDistributor is IUniversalRewardsDistributor { if (timelock == 0) { _setRoot(newRoot, newIpfsHash); } else { - pendingRoot = PendingRoot(block.timestamp, newRoot, newIpfsHash); - emit EventsLib.RootProposed(newRoot, newIpfsHash); + _setPendingRoot(PendingRoot(block.timestamp, newRoot, newIpfsHash)); } } @@ -100,7 +99,7 @@ contract UniversalRewardsDistributor is IUniversalRewardsDistributor { emit EventsLib.RootSet(pendingRoot.root, pendingRoot.ipfsHash); - delete pendingRoot; + _setPendingRoot(PendingRoot(0, 0, 0)); } /// @notice Claims rewards. @@ -173,9 +172,7 @@ contract UniversalRewardsDistributor is IUniversalRewardsDistributor { function revokePendingRoot() external onlyOwner { require(pendingRoot.submittedAt != 0, ErrorsLib.NO_PENDING_ROOT); - delete pendingRoot; - - emit EventsLib.PendingRootRevoked(); + _setPendingRoot(PendingRoot(0, 0, 0)); } /// @notice Sets the `owner` of the distribution to `newOwner`. @@ -192,7 +189,9 @@ contract UniversalRewardsDistributor is IUniversalRewardsDistributor { root = newRoot; ipfsHash = newIpfsHash; - delete pendingRoot; + if (pendingRoot.submittedAt != 0) { + _setPendingRoot(PendingRoot(0, 0, 0)); + } emit EventsLib.RootSet(newRoot, newIpfsHash); } @@ -210,4 +209,9 @@ contract UniversalRewardsDistributor is IUniversalRewardsDistributor { emit EventsLib.TimelockSet(newTimelock); } + + function _setPendingRoot(PendingRoot memory _pendingRoot) internal { + pendingRoot = _pendingRoot; + emit EventsLib.RootProposed(_pendingRoot.root, _pendingRoot.ipfsHash, _pendingRoot.submittedAt); + } } diff --git a/src/libraries/EventsLib.sol b/src/libraries/EventsLib.sol index 5938b43..c5c18e5 100644 --- a/src/libraries/EventsLib.sol +++ b/src/libraries/EventsLib.sol @@ -14,7 +14,7 @@ library EventsLib { /// @notice Emitted when a new merkle root is proposed. /// @param newRoot The new merkle root. /// @param newIpfsHash The optional ipfs hash containing metadata about the root (e.g. the merkle tree itself). - event RootProposed(bytes32 indexed newRoot, bytes32 indexed newIpfsHash); + event RootProposed(bytes32 indexed newRoot, bytes32 indexed newIpfsHash, uint256 timestamp); /// @notice Emitted when a merkle tree distribution timelock is set. /// @param timelock The new merkle timelock. @@ -25,9 +25,6 @@ library EventsLib { /// @param active The merkle tree updater's active state. event RootUpdaterSet(address indexed rootUpdater, bool active); - /// @notice Emitted when a merkle pending root is revoked. - event PendingRootRevoked(); - /// @notice Emitted when rewards are claimed. /// @param account The address for which rewards are claimd rewards for. /// @param reward The address of the reward token. diff --git a/test/UniversalRewardsDistributorTest.sol b/test/UniversalRewardsDistributorTest.sol index 45f775b..13fd9b1 100644 --- a/test/UniversalRewardsDistributorTest.sol +++ b/test/UniversalRewardsDistributorTest.sol @@ -156,7 +156,7 @@ contract UniversalRewardsDistributorTest is Test { function testSubmitRootWithTimelockAsOwner() public { vm.prank(owner); vm.expectEmit(address(distributionWithTimeLock)); - emit EventsLib.RootProposed(DEFAULT_ROOT, DEFAULT_IPFS_HASH); + emit EventsLib.RootProposed(DEFAULT_ROOT, DEFAULT_IPFS_HASH, block.timestamp); distributionWithTimeLock.submitRoot(DEFAULT_ROOT, DEFAULT_IPFS_HASH); assert(distributionWithTimeLock.root() != DEFAULT_ROOT); @@ -170,7 +170,7 @@ contract UniversalRewardsDistributorTest is Test { function testSubmitRootWithTimelockAsUpdater() public { vm.prank(updater); vm.expectEmit(address(distributionWithTimeLock)); - emit EventsLib.RootProposed(DEFAULT_ROOT, DEFAULT_IPFS_HASH); + emit EventsLib.RootProposed(DEFAULT_ROOT, DEFAULT_IPFS_HASH, block.timestamp); distributionWithTimeLock.submitRoot(DEFAULT_ROOT, DEFAULT_IPFS_HASH); assert(distributionWithTimeLock.root() != DEFAULT_ROOT); @@ -374,7 +374,7 @@ contract UniversalRewardsDistributorTest is Test { vm.prank(owner); vm.expectEmit(address(distributionWithTimeLock)); - emit EventsLib.PendingRootRevoked(); + emit EventsLib.RootProposed(0, 0, 0); distributionWithTimeLock.revokePendingRoot(); PendingRoot memory pendingRoot = _getPendingRoot(distributionWithTimeLock); From df4b6608fe7e4c3fc86e0131d9118d0e7d5abee3 Mon Sep 17 00:00:00 2001 From: julien Date: Tue, 31 Oct 2023 15:49:33 +0100 Subject: [PATCH 03/14] refactor(acceptRoot): emit RootSet after RootProposed Signed-off-by: julien --- src/UniversalRewardsDistributor.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/UniversalRewardsDistributor.sol b/src/UniversalRewardsDistributor.sol index 1f15847..6994635 100644 --- a/src/UniversalRewardsDistributor.sol +++ b/src/UniversalRewardsDistributor.sol @@ -97,9 +97,9 @@ contract UniversalRewardsDistributor is IUniversalRewardsDistributor { root = pendingRoot.root; ipfsHash = pendingRoot.ipfsHash; - emit EventsLib.RootSet(pendingRoot.root, pendingRoot.ipfsHash); - _setPendingRoot(PendingRoot(0, 0, 0)); + + emit EventsLib.RootSet(pendingRoot.root, pendingRoot.ipfsHash); } /// @notice Claims rewards. From 7fb0fa4e2933887cf3dd7e3eba7143db6cdce693 Mon Sep 17 00:00:00 2001 From: julien Date: Tue, 31 Oct 2023 16:06:53 +0100 Subject: [PATCH 04/14] test(pendingRoot): test event emission of pending values Signed-off-by: julien --- src/UniversalRewardsDistributor.sol | 3 ++- test/UniversalRewardsDistributorTest.sol | 22 ++++++++++++++-------- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/src/UniversalRewardsDistributor.sol b/src/UniversalRewardsDistributor.sol index 6994635..60ae274 100644 --- a/src/UniversalRewardsDistributor.sol +++ b/src/UniversalRewardsDistributor.sol @@ -99,7 +99,7 @@ contract UniversalRewardsDistributor is IUniversalRewardsDistributor { _setPendingRoot(PendingRoot(0, 0, 0)); - emit EventsLib.RootSet(pendingRoot.root, pendingRoot.ipfsHash); + emit EventsLib.RootSet(root, ipfsHash); } /// @notice Claims rewards. @@ -212,6 +212,7 @@ contract UniversalRewardsDistributor is IUniversalRewardsDistributor { function _setPendingRoot(PendingRoot memory _pendingRoot) internal { pendingRoot = _pendingRoot; + emit EventsLib.RootProposed(_pendingRoot.root, _pendingRoot.ipfsHash, _pendingRoot.submittedAt); } } diff --git a/test/UniversalRewardsDistributorTest.sol b/test/UniversalRewardsDistributorTest.sol index 13fd9b1..7740dfb 100644 --- a/test/UniversalRewardsDistributorTest.sol +++ b/test/UniversalRewardsDistributorTest.sol @@ -198,6 +198,8 @@ contract UniversalRewardsDistributorTest is Test { vm.prank(randomCaller); vm.expectEmit(address(distributionWithTimeLock)); + emit EventsLib.RootProposed(0, 0, 0); + vm.expectEmit(address(distributionWithTimeLock)); emit EventsLib.RootSet(DEFAULT_ROOT, DEFAULT_IPFS_HASH); distributionWithTimeLock.acceptRoot(); @@ -254,18 +256,20 @@ contract UniversalRewardsDistributorTest is Test { assertEq(pendingRoot.submittedAt, 0); } - function testSetRootShouldRemovePendingRoot(bytes32 newRoot, address randomCaller) public { - vm.assume(newRoot != DEFAULT_ROOT && randomCaller != owner); - - vm.startPrank(owner); + function testSetRootShouldUpdateTheCurrentPendingRoot(bytes32 newRoot, bytes32 newIpfsHash) public { + vm.prank(updater); distributionWithTimeLock.submitRoot(DEFAULT_ROOT, DEFAULT_IPFS_HASH); - assertEq(_getPendingRoot(distributionWithTimeLock).root, DEFAULT_ROOT); + vm.prank(owner); + vm.expectEmit(address(distributionWithTimeLock)); + emit EventsLib.RootProposed(0, 0, 0); + distributionWithTimeLock.setRoot(newRoot, newIpfsHash); - distributionWithTimeLock.setRoot(newRoot, DEFAULT_IPFS_HASH); - vm.stopPrank(); + PendingRoot memory pendingRoot = _getPendingRoot(distributionWithTimeLock); - assertEq(_getPendingRoot(distributionWithTimeLock).root, bytes32(0)); + assertEq(pendingRoot.submittedAt, 0); + assertEq(pendingRoot.root, 0); + assertEq(pendingRoot.ipfsHash, 0); } function testSetTimelockShouldChangeTheDistributionTimelock(uint256 newTimelock) public { @@ -306,6 +310,8 @@ contract UniversalRewardsDistributorTest is Test { vm.warp(block.timestamp + afterEndOfTimelock); vm.expectEmit(address(distributionWithTimeLock)); + emit EventsLib.RootProposed(0, 0, 0); + vm.expectEmit(address(distributionWithTimeLock)); emit EventsLib.RootSet(DEFAULT_ROOT, DEFAULT_IPFS_HASH); distributionWithTimeLock.acceptRoot(); } From d9f45ed4915403910cd3d642cb434fea7438c574 Mon Sep 17 00:00:00 2001 From: julien Date: Tue, 31 Oct 2023 16:15:04 +0100 Subject: [PATCH 05/14] refactor(pendingRoot): change event names RootProposed => PendingRootSet Signed-off-by: julien --- src/UniversalRewardsDistributor.sol | 2 +- src/libraries/EventsLib.sol | 2 +- test/UniversalRewardsDistributorTest.sol | 12 ++++++------ 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/UniversalRewardsDistributor.sol b/src/UniversalRewardsDistributor.sol index 60ae274..1731d4c 100644 --- a/src/UniversalRewardsDistributor.sol +++ b/src/UniversalRewardsDistributor.sol @@ -213,6 +213,6 @@ contract UniversalRewardsDistributor is IUniversalRewardsDistributor { function _setPendingRoot(PendingRoot memory _pendingRoot) internal { pendingRoot = _pendingRoot; - emit EventsLib.RootProposed(_pendingRoot.root, _pendingRoot.ipfsHash, _pendingRoot.submittedAt); + emit EventsLib.PendingRootSet(_pendingRoot.root, _pendingRoot.ipfsHash, _pendingRoot.submittedAt); } } diff --git a/src/libraries/EventsLib.sol b/src/libraries/EventsLib.sol index c5c18e5..5116766 100644 --- a/src/libraries/EventsLib.sol +++ b/src/libraries/EventsLib.sol @@ -14,7 +14,7 @@ library EventsLib { /// @notice Emitted when a new merkle root is proposed. /// @param newRoot The new merkle root. /// @param newIpfsHash The optional ipfs hash containing metadata about the root (e.g. the merkle tree itself). - event RootProposed(bytes32 indexed newRoot, bytes32 indexed newIpfsHash, uint256 timestamp); + event PendingRootSet(bytes32 indexed newRoot, bytes32 indexed newIpfsHash, uint256 timestamp); /// @notice Emitted when a merkle tree distribution timelock is set. /// @param timelock The new merkle timelock. diff --git a/test/UniversalRewardsDistributorTest.sol b/test/UniversalRewardsDistributorTest.sol index 7740dfb..d629378 100644 --- a/test/UniversalRewardsDistributorTest.sol +++ b/test/UniversalRewardsDistributorTest.sol @@ -156,7 +156,7 @@ contract UniversalRewardsDistributorTest is Test { function testSubmitRootWithTimelockAsOwner() public { vm.prank(owner); vm.expectEmit(address(distributionWithTimeLock)); - emit EventsLib.RootProposed(DEFAULT_ROOT, DEFAULT_IPFS_HASH, block.timestamp); + emit EventsLib.PendingRootSet(DEFAULT_ROOT, DEFAULT_IPFS_HASH, block.timestamp); distributionWithTimeLock.submitRoot(DEFAULT_ROOT, DEFAULT_IPFS_HASH); assert(distributionWithTimeLock.root() != DEFAULT_ROOT); @@ -170,7 +170,7 @@ contract UniversalRewardsDistributorTest is Test { function testSubmitRootWithTimelockAsUpdater() public { vm.prank(updater); vm.expectEmit(address(distributionWithTimeLock)); - emit EventsLib.RootProposed(DEFAULT_ROOT, DEFAULT_IPFS_HASH, block.timestamp); + emit EventsLib.PendingRootSet(DEFAULT_ROOT, DEFAULT_IPFS_HASH, block.timestamp); distributionWithTimeLock.submitRoot(DEFAULT_ROOT, DEFAULT_IPFS_HASH); assert(distributionWithTimeLock.root() != DEFAULT_ROOT); @@ -198,7 +198,7 @@ contract UniversalRewardsDistributorTest is Test { vm.prank(randomCaller); vm.expectEmit(address(distributionWithTimeLock)); - emit EventsLib.RootProposed(0, 0, 0); + emit EventsLib.PendingRootSet(0, 0, 0); vm.expectEmit(address(distributionWithTimeLock)); emit EventsLib.RootSet(DEFAULT_ROOT, DEFAULT_IPFS_HASH); distributionWithTimeLock.acceptRoot(); @@ -262,7 +262,7 @@ contract UniversalRewardsDistributorTest is Test { vm.prank(owner); vm.expectEmit(address(distributionWithTimeLock)); - emit EventsLib.RootProposed(0, 0, 0); + emit EventsLib.PendingRootSet(0, 0, 0); distributionWithTimeLock.setRoot(newRoot, newIpfsHash); PendingRoot memory pendingRoot = _getPendingRoot(distributionWithTimeLock); @@ -310,7 +310,7 @@ contract UniversalRewardsDistributorTest is Test { vm.warp(block.timestamp + afterEndOfTimelock); vm.expectEmit(address(distributionWithTimeLock)); - emit EventsLib.RootProposed(0, 0, 0); + emit EventsLib.PendingRootSet(0, 0, 0); vm.expectEmit(address(distributionWithTimeLock)); emit EventsLib.RootSet(DEFAULT_ROOT, DEFAULT_IPFS_HASH); distributionWithTimeLock.acceptRoot(); @@ -380,7 +380,7 @@ contract UniversalRewardsDistributorTest is Test { vm.prank(owner); vm.expectEmit(address(distributionWithTimeLock)); - emit EventsLib.RootProposed(0, 0, 0); + emit EventsLib.PendingRootSet(0, 0, 0); distributionWithTimeLock.revokePendingRoot(); PendingRoot memory pendingRoot = _getPendingRoot(distributionWithTimeLock); From 655f9dc818dd75e6e5f7e1945d0cbc76294767ae Mon Sep 17 00:00:00 2001 From: julien Date: Thu, 2 Nov 2023 11:29:34 +0100 Subject: [PATCH 06/14] fix(events): do not emit `PendingRootSet` if `RootSet` is emitted --- src/UniversalRewardsDistributor.sol | 6 ++---- test/UniversalRewardsDistributorTest.sol | 6 ------ 2 files changed, 2 insertions(+), 10 deletions(-) diff --git a/src/UniversalRewardsDistributor.sol b/src/UniversalRewardsDistributor.sol index 1731d4c..3574010 100644 --- a/src/UniversalRewardsDistributor.sol +++ b/src/UniversalRewardsDistributor.sol @@ -97,7 +97,7 @@ contract UniversalRewardsDistributor is IUniversalRewardsDistributor { root = pendingRoot.root; ipfsHash = pendingRoot.ipfsHash; - _setPendingRoot(PendingRoot(0, 0, 0)); + delete pendingRoot; emit EventsLib.RootSet(root, ipfsHash); } @@ -189,9 +189,7 @@ contract UniversalRewardsDistributor is IUniversalRewardsDistributor { root = newRoot; ipfsHash = newIpfsHash; - if (pendingRoot.submittedAt != 0) { - _setPendingRoot(PendingRoot(0, 0, 0)); - } + delete pendingRoot; emit EventsLib.RootSet(newRoot, newIpfsHash); } diff --git a/test/UniversalRewardsDistributorTest.sol b/test/UniversalRewardsDistributorTest.sol index d629378..0a9035d 100644 --- a/test/UniversalRewardsDistributorTest.sol +++ b/test/UniversalRewardsDistributorTest.sol @@ -198,8 +198,6 @@ contract UniversalRewardsDistributorTest is Test { vm.prank(randomCaller); vm.expectEmit(address(distributionWithTimeLock)); - emit EventsLib.PendingRootSet(0, 0, 0); - vm.expectEmit(address(distributionWithTimeLock)); emit EventsLib.RootSet(DEFAULT_ROOT, DEFAULT_IPFS_HASH); distributionWithTimeLock.acceptRoot(); @@ -261,8 +259,6 @@ contract UniversalRewardsDistributorTest is Test { distributionWithTimeLock.submitRoot(DEFAULT_ROOT, DEFAULT_IPFS_HASH); vm.prank(owner); - vm.expectEmit(address(distributionWithTimeLock)); - emit EventsLib.PendingRootSet(0, 0, 0); distributionWithTimeLock.setRoot(newRoot, newIpfsHash); PendingRoot memory pendingRoot = _getPendingRoot(distributionWithTimeLock); @@ -310,8 +306,6 @@ contract UniversalRewardsDistributorTest is Test { vm.warp(block.timestamp + afterEndOfTimelock); vm.expectEmit(address(distributionWithTimeLock)); - emit EventsLib.PendingRootSet(0, 0, 0); - vm.expectEmit(address(distributionWithTimeLock)); emit EventsLib.RootSet(DEFAULT_ROOT, DEFAULT_IPFS_HASH); distributionWithTimeLock.acceptRoot(); } From 60b96009c3377932e42915795802aaa84771a907 Mon Sep 17 00:00:00 2001 From: julien Date: Thu, 2 Nov 2023 16:02:54 +0100 Subject: [PATCH 07/14] feat: allow updaters to revoke pending root --- src/UniversalRewardsDistributor.sol | 33 ++++++++---------------- src/libraries/EventsLib.sol | 5 +++- test/UniversalRewardsDistributorTest.sol | 26 ++++++++++++++----- 3 files changed, 35 insertions(+), 29 deletions(-) diff --git a/src/UniversalRewardsDistributor.sol b/src/UniversalRewardsDistributor.sol index 3574010..b2f46a5 100644 --- a/src/UniversalRewardsDistributor.sol +++ b/src/UniversalRewardsDistributor.sol @@ -83,7 +83,9 @@ contract UniversalRewardsDistributor is IUniversalRewardsDistributor { if (timelock == 0) { _setRoot(newRoot, newIpfsHash); } else { - _setPendingRoot(PendingRoot(block.timestamp, newRoot, newIpfsHash)); + pendingRoot = PendingRoot(block.timestamp, newRoot, newIpfsHash); + + emit EventsLib.PendingRootSet(newRoot, newIpfsHash); } } @@ -94,12 +96,16 @@ contract UniversalRewardsDistributor is IUniversalRewardsDistributor { require(pendingRoot.submittedAt > 0, ErrorsLib.NO_PENDING_ROOT); require(block.timestamp >= pendingRoot.submittedAt + timelock, ErrorsLib.TIMELOCK_NOT_EXPIRED); - root = pendingRoot.root; - ipfsHash = pendingRoot.ipfsHash; + _setRoot(pendingRoot.root, pendingRoot.ipfsHash); + } - delete pendingRoot; + /// @notice Revokes the pending root. + /// @dev Can be frontrunned by triggering the `acceptRoot` function in case the timelock has passed. + function revokePendingRoot() external onlyUpdater { + require(pendingRoot.submittedAt != 0, ErrorsLib.NO_PENDING_ROOT); - emit EventsLib.RootSet(root, ipfsHash); + delete pendingRoot; + emit EventsLib.PendingRootRevoked(); } /// @notice Claims rewards. @@ -152,7 +158,6 @@ contract UniversalRewardsDistributor is IUniversalRewardsDistributor { ErrorsLib.TIMELOCK_NOT_EXPIRED ); } - _setTimelock(newTimelock); } @@ -165,16 +170,6 @@ contract UniversalRewardsDistributor is IUniversalRewardsDistributor { emit EventsLib.RootUpdaterSet(updater, active); } - /// @notice Revokes the pending root of a given distribution. - /// @dev This function can only be called by the owner of the distribution at any time. - /// @dev Can be frontrunned by triggering the `acceptRoot` function in case the timelock has passed. This if the - /// `owner` responsibility to trigger this function before the end of the timelock. - function revokePendingRoot() external onlyOwner { - require(pendingRoot.submittedAt != 0, ErrorsLib.NO_PENDING_ROOT); - - _setPendingRoot(PendingRoot(0, 0, 0)); - } - /// @notice Sets the `owner` of the distribution to `newOwner`. function setOwner(address newOwner) external onlyOwner { _setOwner(newOwner); @@ -207,10 +202,4 @@ contract UniversalRewardsDistributor is IUniversalRewardsDistributor { emit EventsLib.TimelockSet(newTimelock); } - - function _setPendingRoot(PendingRoot memory _pendingRoot) internal { - pendingRoot = _pendingRoot; - - emit EventsLib.PendingRootSet(_pendingRoot.root, _pendingRoot.ipfsHash, _pendingRoot.submittedAt); - } } diff --git a/src/libraries/EventsLib.sol b/src/libraries/EventsLib.sol index 5116766..6808ee5 100644 --- a/src/libraries/EventsLib.sol +++ b/src/libraries/EventsLib.sol @@ -14,7 +14,10 @@ library EventsLib { /// @notice Emitted when a new merkle root is proposed. /// @param newRoot The new merkle root. /// @param newIpfsHash The optional ipfs hash containing metadata about the root (e.g. the merkle tree itself). - event PendingRootSet(bytes32 indexed newRoot, bytes32 indexed newIpfsHash, uint256 timestamp); + event PendingRootSet(bytes32 indexed newRoot, bytes32 indexed newIpfsHash); + + /// @notice Emitted when the pending root is revoked by an updater. + event PendingRootRevoked(); /// @notice Emitted when a merkle tree distribution timelock is set. /// @param timelock The new merkle timelock. diff --git a/test/UniversalRewardsDistributorTest.sol b/test/UniversalRewardsDistributorTest.sol index 0a9035d..999a0ca 100644 --- a/test/UniversalRewardsDistributorTest.sol +++ b/test/UniversalRewardsDistributorTest.sol @@ -156,7 +156,7 @@ contract UniversalRewardsDistributorTest is Test { function testSubmitRootWithTimelockAsOwner() public { vm.prank(owner); vm.expectEmit(address(distributionWithTimeLock)); - emit EventsLib.PendingRootSet(DEFAULT_ROOT, DEFAULT_IPFS_HASH, block.timestamp); + emit EventsLib.PendingRootSet(DEFAULT_ROOT, DEFAULT_IPFS_HASH); distributionWithTimeLock.submitRoot(DEFAULT_ROOT, DEFAULT_IPFS_HASH); assert(distributionWithTimeLock.root() != DEFAULT_ROOT); @@ -170,7 +170,7 @@ contract UniversalRewardsDistributorTest is Test { function testSubmitRootWithTimelockAsUpdater() public { vm.prank(updater); vm.expectEmit(address(distributionWithTimeLock)); - emit EventsLib.PendingRootSet(DEFAULT_ROOT, DEFAULT_IPFS_HASH, block.timestamp); + emit EventsLib.PendingRootSet(DEFAULT_ROOT, DEFAULT_IPFS_HASH); distributionWithTimeLock.submitRoot(DEFAULT_ROOT, DEFAULT_IPFS_HASH); assert(distributionWithTimeLock.root() != DEFAULT_ROOT); @@ -374,7 +374,7 @@ contract UniversalRewardsDistributorTest is Test { vm.prank(owner); vm.expectEmit(address(distributionWithTimeLock)); - emit EventsLib.PendingRootSet(0, 0, 0); + emit EventsLib.PendingRootRevoked(); distributionWithTimeLock.revokePendingRoot(); PendingRoot memory pendingRoot = _getPendingRoot(distributionWithTimeLock); @@ -382,14 +382,28 @@ contract UniversalRewardsDistributorTest is Test { assertEq(pendingRoot.submittedAt, 0); } - function testRevokePendingRootShouldRevertIfNotOwner(bytes32 proposedRoot, address caller) public { - vm.assume(proposedRoot != bytes32(0) && caller != owner); + function testRevokePendingRootShouldRevokeWhenCalledWithUpdater() public { + vm.prank(owner); + distributionWithTimeLock.submitRoot(DEFAULT_ROOT, DEFAULT_IPFS_HASH); + + vm.prank(updater); + vm.expectEmit(address(distributionWithTimeLock)); + emit EventsLib.PendingRootRevoked(); + distributionWithTimeLock.revokePendingRoot(); + + PendingRoot memory pendingRoot = _getPendingRoot(distributionWithTimeLock); + assertEq(pendingRoot.root, bytes32(0)); + assertEq(pendingRoot.submittedAt, 0); + } + + function testRevokePendingRootShouldRevertIfNotUpdater(bytes32 proposedRoot, address caller) public { + vm.assume(!distributionWithTimeLock.isUpdater(caller)); vm.prank(owner); distributionWithTimeLock.submitRoot(proposedRoot, DEFAULT_IPFS_HASH); vm.prank(caller); - vm.expectRevert(bytes(ErrorsLib.CALLER_NOT_OWNER)); + vm.expectRevert(bytes(ErrorsLib.CALLER_NOT_OWNER_OR_UPDATER)); distributionWithTimeLock.revokePendingRoot(); } From 70fffb419a25a7a27f73448eab3bbe1f0d80320c Mon Sep 17 00:00:00 2001 From: julien Date: Thu, 2 Nov 2023 16:07:35 +0100 Subject: [PATCH 08/14] refactor: use `{}` to set `PendingRoot` struct and reorder fields --- src/UniversalRewardsDistributor.sol | 2 +- src/interfaces/IUniversalRewardsDistributor.sol | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/UniversalRewardsDistributor.sol b/src/UniversalRewardsDistributor.sol index b2f46a5..39a574b 100644 --- a/src/UniversalRewardsDistributor.sol +++ b/src/UniversalRewardsDistributor.sol @@ -83,7 +83,7 @@ contract UniversalRewardsDistributor is IUniversalRewardsDistributor { if (timelock == 0) { _setRoot(newRoot, newIpfsHash); } else { - pendingRoot = PendingRoot(block.timestamp, newRoot, newIpfsHash); + pendingRoot = PendingRoot({root: newRoot, ipfsHash: newIpfsHash, submittedAt: block.timestamp}); emit EventsLib.PendingRootSet(newRoot, newIpfsHash); } diff --git a/src/interfaces/IUniversalRewardsDistributor.sol b/src/interfaces/IUniversalRewardsDistributor.sol index 701bc8a..161ed27 100644 --- a/src/interfaces/IUniversalRewardsDistributor.sol +++ b/src/interfaces/IUniversalRewardsDistributor.sol @@ -3,12 +3,12 @@ pragma solidity >=0.7.4; /// @notice The pending root struct for a merkle tree distribution during the timelock. struct PendingRoot { - /// @dev The timestamp of the block in which the pending root was submitted. - uint256 submittedAt; /// @dev The submitted pending root. bytes32 root; /// @dev The optional ipfs hash containing metadata about the root (e.g. the merkle tree itself). bytes32 ipfsHash; + /// @dev The timestamp of the block in which the pending root was submitted. + uint256 submittedAt; } /// @title IUniversalRewardsDistributor @@ -21,7 +21,7 @@ interface IUniversalRewardsDistributor { function timelock() external view returns (uint256); function ipfsHash() external view returns (bytes32); function isUpdater(address) external view returns (bool); - function pendingRoot() external view returns (uint256 submittedAt, bytes32 root, bytes32 ipfsHash); + function pendingRoot() external view returns (bytes32 root, bytes32 ipfsHash, uint256 submittedAt); function claimed(address, address) external view returns (uint256); function acceptRoot() external; From c4bed81f2e8de6fbccbef34b8beec2881cba61c6 Mon Sep 17 00:00:00 2001 From: julien Date: Thu, 2 Nov 2023 16:11:14 +0100 Subject: [PATCH 09/14] chore: format code --- src/UniversalRewardsDistributor.sol | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/UniversalRewardsDistributor.sol b/src/UniversalRewardsDistributor.sol index 39a574b..4772e68 100644 --- a/src/UniversalRewardsDistributor.sol +++ b/src/UniversalRewardsDistributor.sol @@ -100,11 +100,12 @@ contract UniversalRewardsDistributor is IUniversalRewardsDistributor { } /// @notice Revokes the pending root. - /// @dev Can be frontrunned by triggering the `acceptRoot` function in case the timelock has passed. + /// @dev Can be frontrunned with `acceptRoot` in case the timelock has passed. function revokePendingRoot() external onlyUpdater { require(pendingRoot.submittedAt != 0, ErrorsLib.NO_PENDING_ROOT); delete pendingRoot; + emit EventsLib.PendingRootRevoked(); } From a74092ae0b7dc7b1e726e99f3327903f629ff5c0 Mon Sep 17 00:00:00 2001 From: julien Date: Mon, 6 Nov 2023 09:59:58 +0100 Subject: [PATCH 10/14] Merge branch 'cantina-review' into fix/more-explicit-function-name-80 --- src/UniversalRewardsDistributor.sol | 2 +- src/interfaces/IUniversalRewardsDistributor.sol | 2 +- test/UniversalRewardsDistributorTest.sol | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/UniversalRewardsDistributor.sol b/src/UniversalRewardsDistributor.sol index 01be707..98441d8 100644 --- a/src/UniversalRewardsDistributor.sol +++ b/src/UniversalRewardsDistributor.sol @@ -99,7 +99,7 @@ contract UniversalRewardsDistributor is IUniversalRewardsDistributorStaticTyping /// @notice Revokes the pending root. /// @dev Can be frontrunned with `acceptRoot` in case the timelock has passed. - function revokePendingRoot() external onlyUpdater { + function revokePendingRoot() external onlyUpdaterRole { require(pendingRoot.validAt != 0, ErrorsLib.NO_PENDING_ROOT); delete pendingRoot; diff --git a/src/interfaces/IUniversalRewardsDistributor.sol b/src/interfaces/IUniversalRewardsDistributor.sol index f782522..9557182 100644 --- a/src/interfaces/IUniversalRewardsDistributor.sol +++ b/src/interfaces/IUniversalRewardsDistributor.sol @@ -40,7 +40,7 @@ interface IUniversalRewardsDistributorBase { /// compiler. /// @dev Consider using the IUniversalRewardsDistributor interface instead of this one. interface IUniversalRewardsDistributorStaticTyping is IUniversalRewardsDistributorBase { - function pendingRoot() external view returns (uint256 submittedAt, bytes32 root, bytes32 ipfsHash); + function pendingRoot() external view returns (bytes32 root, bytes32 ipfsHash, uint256 validAt); } /// @title IUniversalRewardsDistributor diff --git a/test/UniversalRewardsDistributorTest.sol b/test/UniversalRewardsDistributorTest.sol index cb9ffe2..b63d31a 100644 --- a/test/UniversalRewardsDistributorTest.sol +++ b/test/UniversalRewardsDistributorTest.sol @@ -385,7 +385,7 @@ contract UniversalRewardsDistributorTest is Test { PendingRoot memory pendingRoot = distributionWithTimeLock.pendingRoot(); assertEq(pendingRoot.root, bytes32(0)); - assertEq(pendingRoot.submittedAt, 0); + assertEq(pendingRoot.validAt, 0); } function testRevokePendingRootShouldRevokeWhenCalledWithUpdater() public { @@ -409,7 +409,7 @@ contract UniversalRewardsDistributorTest is Test { distributionWithTimeLock.submitRoot(proposedRoot, DEFAULT_IPFS_HASH); vm.prank(caller); - vm.expectRevert(bytes(ErrorsLib.CALLER_NOT_OWNER_OR_UPDATER)); + vm.expectRevert(bytes(ErrorsLib.NOT_UPDATER_ROLE)); distributionWithTimeLock.revokePendingRoot(); } From 5ed38f19d49ad3ca3dd2d978232ea3dda458e823 Mon Sep 17 00:00:00 2001 From: julien Date: Mon, 6 Nov 2023 10:02:33 +0100 Subject: [PATCH 11/14] docs(natspecs): update updater as owner or updater --- src/libraries/EventsLib.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/EventsLib.sol b/src/libraries/EventsLib.sol index 6808ee5..1bf564c 100644 --- a/src/libraries/EventsLib.sol +++ b/src/libraries/EventsLib.sol @@ -16,7 +16,7 @@ library EventsLib { /// @param newIpfsHash The optional ipfs hash containing metadata about the root (e.g. the merkle tree itself). event PendingRootSet(bytes32 indexed newRoot, bytes32 indexed newIpfsHash); - /// @notice Emitted when the pending root is revoked by an updater. + /// @notice Emitted when the pending root is revoked by the owner or an updater. event PendingRootRevoked(); /// @notice Emitted when a merkle tree distribution timelock is set. From 7594fc3cb97093c53205b40e73b42adae6fc7728 Mon Sep 17 00:00:00 2001 From: julien Date: Mon, 6 Nov 2023 10:06:58 +0100 Subject: [PATCH 12/14] fix(tests): owner = updater --- test/UniversalRewardsDistributorTest.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/UniversalRewardsDistributorTest.sol b/test/UniversalRewardsDistributorTest.sol index b63d31a..c5926ed 100644 --- a/test/UniversalRewardsDistributorTest.sol +++ b/test/UniversalRewardsDistributorTest.sol @@ -403,7 +403,7 @@ contract UniversalRewardsDistributorTest is Test { } function testRevokePendingRootShouldRevertIfNotUpdater(bytes32 proposedRoot, address caller) public { - vm.assume(!distributionWithTimeLock.isUpdater(caller)); + vm.assume(!distributionWithTimeLock.isUpdater(caller) && caller != owner); vm.prank(owner); distributionWithTimeLock.submitRoot(proposedRoot, DEFAULT_IPFS_HASH); From cc9686067869112bc0c7cc694f555e5dd208f3cc Mon Sep 17 00:00:00 2001 From: julien Date: Mon, 6 Nov 2023 13:29:14 +0100 Subject: [PATCH 13/14] feat(events): add caller to pending root events --- src/UniversalRewardsDistributor.sol | 4 ++-- src/libraries/EventsLib.sol | 4 ++-- test/UniversalRewardsDistributorTest.sol | 8 ++++---- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/UniversalRewardsDistributor.sol b/src/UniversalRewardsDistributor.sol index 98441d8..1418c1f 100644 --- a/src/UniversalRewardsDistributor.sol +++ b/src/UniversalRewardsDistributor.sol @@ -83,7 +83,7 @@ contract UniversalRewardsDistributor is IUniversalRewardsDistributorStaticTyping } else { pendingRoot = PendingRoot({root: newRoot, ipfsHash: newIpfsHash, validAt: block.timestamp + timelock}); - emit EventsLib.PendingRootSet(newRoot, newIpfsHash); + emit EventsLib.PendingRootSet(msg.sender, newRoot, newIpfsHash); } } @@ -104,7 +104,7 @@ contract UniversalRewardsDistributor is IUniversalRewardsDistributorStaticTyping delete pendingRoot; - emit EventsLib.PendingRootRevoked(); + emit EventsLib.PendingRootRevoked(msg.sender); } /// @notice Claims rewards. diff --git a/src/libraries/EventsLib.sol b/src/libraries/EventsLib.sol index 1bf564c..de0f3d3 100644 --- a/src/libraries/EventsLib.sol +++ b/src/libraries/EventsLib.sol @@ -14,10 +14,10 @@ library EventsLib { /// @notice Emitted when a new merkle root is proposed. /// @param newRoot The new merkle root. /// @param newIpfsHash The optional ipfs hash containing metadata about the root (e.g. the merkle tree itself). - event PendingRootSet(bytes32 indexed newRoot, bytes32 indexed newIpfsHash); + event PendingRootSet(address indexed caller, bytes32 indexed newRoot, bytes32 indexed newIpfsHash); /// @notice Emitted when the pending root is revoked by the owner or an updater. - event PendingRootRevoked(); + event PendingRootRevoked(address indexed caller); /// @notice Emitted when a merkle tree distribution timelock is set. /// @param timelock The new merkle timelock. diff --git a/test/UniversalRewardsDistributorTest.sol b/test/UniversalRewardsDistributorTest.sol index c5926ed..ae21710 100644 --- a/test/UniversalRewardsDistributorTest.sol +++ b/test/UniversalRewardsDistributorTest.sol @@ -165,7 +165,7 @@ contract UniversalRewardsDistributorTest is Test { function testSubmitRootWithTimelockAsOwner() public { vm.prank(owner); vm.expectEmit(address(distributionWithTimeLock)); - emit EventsLib.PendingRootSet(DEFAULT_ROOT, DEFAULT_IPFS_HASH); + emit EventsLib.PendingRootSet(owner, DEFAULT_ROOT, DEFAULT_IPFS_HASH); distributionWithTimeLock.submitRoot(DEFAULT_ROOT, DEFAULT_IPFS_HASH); assert(distributionWithTimeLock.root() != DEFAULT_ROOT); @@ -179,7 +179,7 @@ contract UniversalRewardsDistributorTest is Test { function testSubmitRootWithTimelockAsUpdater() public { vm.prank(updater); vm.expectEmit(address(distributionWithTimeLock)); - emit EventsLib.PendingRootSet(DEFAULT_ROOT, DEFAULT_IPFS_HASH); + emit EventsLib.PendingRootSet(updater, DEFAULT_ROOT, DEFAULT_IPFS_HASH); distributionWithTimeLock.submitRoot(DEFAULT_ROOT, DEFAULT_IPFS_HASH); assert(distributionWithTimeLock.root() != DEFAULT_ROOT); @@ -380,7 +380,7 @@ contract UniversalRewardsDistributorTest is Test { vm.prank(owner); vm.expectEmit(address(distributionWithTimeLock)); - emit EventsLib.PendingRootRevoked(); + emit EventsLib.PendingRootRevoked(owner); distributionWithTimeLock.revokePendingRoot(); PendingRoot memory pendingRoot = distributionWithTimeLock.pendingRoot(); @@ -394,7 +394,7 @@ contract UniversalRewardsDistributorTest is Test { vm.prank(updater); vm.expectEmit(address(distributionWithTimeLock)); - emit EventsLib.PendingRootRevoked(); + emit EventsLib.PendingRootRevoked(updater); distributionWithTimeLock.revokePendingRoot(); PendingRoot memory pendingRoot = distributionWithTimeLock.pendingRoot(); From c6b6ce2d32bf4b9e586af74148cbc9b0b0a30e71 Mon Sep 17 00:00:00 2001 From: MerlinEgalite Date: Mon, 6 Nov 2023 19:47:34 +0300 Subject: [PATCH 14/14] docs: add caller to natspecs --- src/libraries/EventsLib.sol | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libraries/EventsLib.sol b/src/libraries/EventsLib.sol index de0f3d3..8243021 100644 --- a/src/libraries/EventsLib.sol +++ b/src/libraries/EventsLib.sol @@ -12,6 +12,7 @@ library EventsLib { event RootSet(bytes32 indexed newRoot, bytes32 indexed newIpfsHash); /// @notice Emitted when a new merkle root is proposed. + /// @param caller The address of the caller. /// @param newRoot The new merkle root. /// @param newIpfsHash The optional ipfs hash containing metadata about the root (e.g. the merkle tree itself). event PendingRootSet(address indexed caller, bytes32 indexed newRoot, bytes32 indexed newIpfsHash);