From 2fa865d00bf2281771e4da229eaf116213eeadb2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=9Fingen?= Date: Tue, 19 Nov 2024 16:26:28 +0000 Subject: [PATCH 1/2] fix: Move lqty allocation checks As value now encodes timestamp, it would never be zero, we need to decode and then check. --- src/BribeInitiative.sol | 7 ++++--- test/BribeInitiative.t.sol | 4 ++-- test/BribeInitiativeAllocate.t.sol | 2 ++ 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/BribeInitiative.sol b/src/BribeInitiative.sol index e71f26f..90af197 100644 --- a/src/BribeInitiative.sol +++ b/src/BribeInitiative.sol @@ -88,19 +88,19 @@ contract BribeInitiative is IInitiative, IBribeInitiative { lqtyAllocationByUserAtEpoch[_user].getItem(_prevLQTYAllocationEpoch); require( - lqtyAllocation.value != 0 && _prevLQTYAllocationEpoch <= _epoch - && (lqtyAllocation.next > _epoch || lqtyAllocation.next == 0), + _prevLQTYAllocationEpoch <= _epoch && (lqtyAllocation.next > _epoch || lqtyAllocation.next == 0), "BribeInitiative: invalid-prev-lqty-allocation-epoch" ); DoubleLinkedList.Item memory totalLQTYAllocation = totalLQTYAllocationByEpoch.getItem(_prevTotalLQTYAllocationEpoch); require( - totalLQTYAllocation.value != 0 && _prevTotalLQTYAllocationEpoch <= _epoch + _prevTotalLQTYAllocationEpoch <= _epoch && (totalLQTYAllocation.next > _epoch || totalLQTYAllocation.next == 0), "BribeInitiative: invalid-prev-total-lqty-allocation-epoch" ); (uint88 totalLQTY, uint120 totalAverageTimestamp) = _decodeLQTYAllocation(totalLQTYAllocation.value); + require(totalLQTY > 0, "BribeInitiative: invalid-prev-total-lqty"); // NOTE: SCALING!!! | The timestamp will work until type(uint32).max | After which the math will eventually overflow uint120 scaledEpochEnd = ( @@ -113,6 +113,7 @@ contract BribeInitiative is IInitiative, IBribeInitiative { uint240 totalVotes = governance.lqtyToVotes(totalLQTY, scaledEpochEnd, totalAverageTimestamp); if (totalVotes != 0) { (uint88 lqty, uint120 averageTimestamp) = _decodeLQTYAllocation(lqtyAllocation.value); + require(lqty > 0, "BribeInitiative: invalid-prev-lqty"); /// @audit Governance Invariant assert(averageTimestamp <= scaledEpochEnd); diff --git a/test/BribeInitiative.t.sol b/test/BribeInitiative.t.sol index 6d9e301..ccdcc70 100644 --- a/test/BribeInitiative.t.sol +++ b/test/BribeInitiative.t.sol @@ -570,7 +570,7 @@ contract BribeInitiativeTest is Test { // user2 should receive no bribes if they try to claim claimEpoch = governance.epoch() - 1; // claim for epoch 3 prevAllocationEpoch = governance.epoch() - 1; // epoch 3 - (boldAmount, bribeTokenAmount) = _claimBribe(user2, claimEpoch, prevAllocationEpoch, prevAllocationEpoch); + (boldAmount, bribeTokenAmount) = _claimBribe(user2, claimEpoch, prevAllocationEpoch, prevAllocationEpoch, true); assertEq(boldAmount, 0, "vetoer receives bold bribe amount"); assertEq(bribeTokenAmount, 0, "vetoer receives bribe amount"); } @@ -857,7 +857,7 @@ contract BribeInitiativeTest is Test { epochs[0].epoch = governance.epoch() - 1; epochs[0].prevLQTYAllocationEpoch = governance.epoch() - 2; epochs[0].prevTotalLQTYAllocationEpoch = governance.epoch() - 2; - vm.expectRevert("BribeInitiative: invalid-prev-total-lqty-allocation-epoch"); + vm.expectRevert("BribeInitiative: invalid-prev-total-lqty"); (uint256 boldAmount, uint256 bribeTokenAmount) = bribeInitiative.claimBribes(epochs); vm.stopPrank(); diff --git a/test/BribeInitiativeAllocate.t.sol b/test/BribeInitiativeAllocate.t.sol index 3653a68..a688ce5 100644 --- a/test/BribeInitiativeAllocate.t.sol +++ b/test/BribeInitiativeAllocate.t.sol @@ -281,6 +281,7 @@ contract BribeInitiativeAllocateTest is Test { claimData[0].epoch = 2; claimData[0].prevLQTYAllocationEpoch = 2; claimData[0].prevTotalLQTYAllocationEpoch = 2; + vm.expectRevert("BribeInitiative: invalid-prev-total-lqty"); (uint256 boldAmount, uint256 bribeTokenAmount) = bribeInitiative.claimBribes(claimData); assertEq(boldAmount, 0, "boldAmount nonzero"); assertEq(bribeTokenAmount, 0, "bribeTokenAmount nonzero"); @@ -707,6 +708,7 @@ contract BribeInitiativeAllocateTest is Test { claimData[0].epoch = 1; claimData[0].prevLQTYAllocationEpoch = 1; claimData[0].prevTotalLQTYAllocationEpoch = 1; + vm.expectRevert("BribeInitiative: invalid-prev-lqty"); (uint256 boldAmount, uint256 bribeTokenAmount) = bribeInitiative.claimBribes(claimData); assertEq(boldAmount, 0); assertEq(bribeTokenAmount, 0); From 0a7eefab75985486132a341ea30473c2da75ec6a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=9Fingen?= Date: Thu, 21 Nov 2024 18:25:04 +0000 Subject: [PATCH 2/2] fix: Change revert strings for BribeInitiative --- src/BribeInitiative.sol | 4 ++-- test/BribeInitiative.t.sol | 2 +- test/BribeInitiativeAllocate.t.sol | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/BribeInitiative.sol b/src/BribeInitiative.sol index 90af197..6c13040 100644 --- a/src/BribeInitiative.sol +++ b/src/BribeInitiative.sol @@ -100,7 +100,7 @@ contract BribeInitiative is IInitiative, IBribeInitiative { ); (uint88 totalLQTY, uint120 totalAverageTimestamp) = _decodeLQTYAllocation(totalLQTYAllocation.value); - require(totalLQTY > 0, "BribeInitiative: invalid-prev-total-lqty"); + require(totalLQTY > 0, "BribeInitiative: total-lqty-allocation-zero"); // NOTE: SCALING!!! | The timestamp will work until type(uint32).max | After which the math will eventually overflow uint120 scaledEpochEnd = ( @@ -113,7 +113,7 @@ contract BribeInitiative is IInitiative, IBribeInitiative { uint240 totalVotes = governance.lqtyToVotes(totalLQTY, scaledEpochEnd, totalAverageTimestamp); if (totalVotes != 0) { (uint88 lqty, uint120 averageTimestamp) = _decodeLQTYAllocation(lqtyAllocation.value); - require(lqty > 0, "BribeInitiative: invalid-prev-lqty"); + require(lqty > 0, "BribeInitiative: lqty-allocation-zero"); /// @audit Governance Invariant assert(averageTimestamp <= scaledEpochEnd); diff --git a/test/BribeInitiative.t.sol b/test/BribeInitiative.t.sol index ccdcc70..990bab8 100644 --- a/test/BribeInitiative.t.sol +++ b/test/BribeInitiative.t.sol @@ -857,7 +857,7 @@ contract BribeInitiativeTest is Test { epochs[0].epoch = governance.epoch() - 1; epochs[0].prevLQTYAllocationEpoch = governance.epoch() - 2; epochs[0].prevTotalLQTYAllocationEpoch = governance.epoch() - 2; - vm.expectRevert("BribeInitiative: invalid-prev-total-lqty"); + vm.expectRevert("BribeInitiative: total-lqty-allocation-zero"); (uint256 boldAmount, uint256 bribeTokenAmount) = bribeInitiative.claimBribes(epochs); vm.stopPrank(); diff --git a/test/BribeInitiativeAllocate.t.sol b/test/BribeInitiativeAllocate.t.sol index a688ce5..05dd8fe 100644 --- a/test/BribeInitiativeAllocate.t.sol +++ b/test/BribeInitiativeAllocate.t.sol @@ -281,7 +281,7 @@ contract BribeInitiativeAllocateTest is Test { claimData[0].epoch = 2; claimData[0].prevLQTYAllocationEpoch = 2; claimData[0].prevTotalLQTYAllocationEpoch = 2; - vm.expectRevert("BribeInitiative: invalid-prev-total-lqty"); + vm.expectRevert("BribeInitiative: total-lqty-allocation-zero"); (uint256 boldAmount, uint256 bribeTokenAmount) = bribeInitiative.claimBribes(claimData); assertEq(boldAmount, 0, "boldAmount nonzero"); assertEq(bribeTokenAmount, 0, "bribeTokenAmount nonzero"); @@ -708,7 +708,7 @@ contract BribeInitiativeAllocateTest is Test { claimData[0].epoch = 1; claimData[0].prevLQTYAllocationEpoch = 1; claimData[0].prevTotalLQTYAllocationEpoch = 1; - vm.expectRevert("BribeInitiative: invalid-prev-lqty"); + vm.expectRevert("BribeInitiative: lqty-allocation-zero"); (uint256 boldAmount, uint256 bribeTokenAmount) = bribeInitiative.claimBribes(claimData); assertEq(boldAmount, 0); assertEq(bribeTokenAmount, 0);