Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat simplify onAfterAllocateLQTY hook #12

Closed
wants to merge 4 commits into from
Closed

Feat simplify onAfterAllocateLQTY hook #12

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
bdf4832
chore: notes for V4 fixes
GalloDaSballo Oct 7, 2024
83d998c
feat: enforce assert to keep `onAfterAllocateLQTY` up to spec
GalloDaSballo Oct 7, 2024
11b3f8a
fix: tests
GalloDaSballo Oct 7, 2024
de952eb
feat: refactor bribe initiative to be simpler to read
GalloDaSballo Oct 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 30 additions & 60 deletions src/BribeInitiative.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,69 +153,39 @@ contract BribeInitiative is IInitiative, IBribeInitiative {
virtual
onlyGovernance
{
/// @audit TODO: WHY?
if (_currentEpoch == 0) return;

/// @audit Key invariant: Either Voting or Vetoing for a specific initiative
assert(_voteLQTY == 0 || _vetoLQTY == 0);

// User Epoch
uint16 mostRecentUserEpoch = lqtyAllocationByUserAtEpoch[_user].getHead();

if (_currentEpoch == 0) return;
// User total
uint88 prevVoteLQTY = lqtyAllocationByUserAtEpoch[_user].getItem(mostRecentUserEpoch).value;

// Recent Epoch
uint16 mostRecentTotalEpoch = totalLQTYAllocationByEpoch.getHead();

// New total is always equal to last known + new votes - prev votes
uint88 newTotal = totalLQTYAllocationByEpoch.items[mostRecentTotalEpoch].value + _voteLQTY - prevVoteLQTY;


_setTotalLQTYAllocationByEpoch(
_currentEpoch,
newTotal, // We update total with relative change
mostRecentTotalEpoch != _currentEpoch // And insert if it's the first in the epoch
);


_setLQTYAllocationByUserAtEpoch(
_user,
_currentEpoch,
_voteLQTY, // We update user with new absolute value
mostRecentUserEpoch != _currentEpoch // And insert if it's the first user change in the epoch
);

// if this is the first user allocation in the epoch, then insert a new item into the user allocation DLL
if (mostRecentUserEpoch != _currentEpoch) {
uint88 prevVoteLQTY = lqtyAllocationByUserAtEpoch[_user].items[mostRecentUserEpoch].value;
uint88 newVoteLQTY = (_vetoLQTY == 0) ? _voteLQTY : 0;
uint16 mostRecentTotalEpoch = totalLQTYAllocationByEpoch.getHead();
// if this is the first allocation in the epoch, then insert a new item into the total allocation DLL
if (mostRecentTotalEpoch != _currentEpoch) {
uint88 prevTotalLQTYAllocation = totalLQTYAllocationByEpoch.items[mostRecentTotalEpoch].value;
if (_vetoLQTY == 0) {
// no veto to no veto
_setTotalLQTYAllocationByEpoch(
_currentEpoch, prevTotalLQTYAllocation + newVoteLQTY - prevVoteLQTY, true
);
} else {
if (prevVoteLQTY != 0) {
// if the prev user allocation was counted in, then remove the prev user allocation from the
// total allocation (no veto to veto)
_setTotalLQTYAllocationByEpoch(_currentEpoch, prevTotalLQTYAllocation - prevVoteLQTY, true);
} else {
// veto to veto
_setTotalLQTYAllocationByEpoch(_currentEpoch, prevTotalLQTYAllocation, true);
}
}
} else {
if (_vetoLQTY == 0) {
// no veto to no veto
_setTotalLQTYAllocationByEpoch(
_currentEpoch,
totalLQTYAllocationByEpoch.items[_currentEpoch].value + newVoteLQTY - prevVoteLQTY,
false
);
} else if (prevVoteLQTY != 0) {
// no veto to veto
_setTotalLQTYAllocationByEpoch(
_currentEpoch, totalLQTYAllocationByEpoch.items[_currentEpoch].value - prevVoteLQTY, false
);
}
}
// insert a new item into the user allocation DLL
_setLQTYAllocationByUserAtEpoch(_user, _currentEpoch, newVoteLQTY, true);
} else {
uint88 prevVoteLQTY = lqtyAllocationByUserAtEpoch[_user].getItem(_currentEpoch).value;
if (_vetoLQTY == 0) {
// update the allocation for the current epoch by adding the new allocation and subtracting
// the previous one (no veto to no veto)
_setTotalLQTYAllocationByEpoch(
_currentEpoch,
totalLQTYAllocationByEpoch.items[_currentEpoch].value + _voteLQTY - prevVoteLQTY,
false
);
_setLQTYAllocationByUserAtEpoch(_user, _currentEpoch, _voteLQTY, false);
} else {
// if the user vetoed the initiative, subtract the allocation from the DLLs (no veto to veto)
_setTotalLQTYAllocationByEpoch(
_currentEpoch, totalLQTYAllocationByEpoch.items[_currentEpoch].value - prevVoteLQTY, false
);
_setLQTYAllocationByUserAtEpoch(_user, _currentEpoch, 0, false);
}
}
}

/// @inheritdoc IInitiative
Expand Down
2 changes: 2 additions & 0 deletions src/UniV4Donations.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,8 @@ contract UniV4Donations is BribeInitiative, BaseHook {
}

function _donateToPool() internal returns (uint256) {
/// @audit TODO: Need to use storage value here I think
/// TODO: Test and fix release speed, which looks off
Vesting memory _vesting = _restartVesting(uint240(governance.claimForInitiative(address(this))));
uint256 amount =
(_vesting.amount * (block.timestamp - vestingEpochStart()) / VESTING_EPOCH_DURATION) - _vesting.released;
Expand Down
20 changes: 10 additions & 10 deletions test/BribeInitiativeAllocate.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ contract BribeInitiativeAllocateTest is Test {
vm.startPrank(address(governance));

bribeInitiative.onAfterAllocateLQTY(governance.epoch(), user2, 1e18, 0);
assertEq(bribeInitiative.totalLQTYAllocatedByEpoch(governance.epoch()), 1e18);
assertEq(bribeInitiative.totalLQTYAllocatedByEpoch(governance.epoch()), 1e18, "initial LQTY");
assertEq(bribeInitiative.lqtyAllocatedByUserAtEpoch(user2, governance.epoch()), 1e18);

bribeInitiative.onAfterAllocateLQTY(governance.epoch(), user, 1000e18, 0);
Expand All @@ -120,11 +120,11 @@ contract BribeInitiativeAllocateTest is Test {

vm.startPrank(address(governance));

bribeInitiative.onAfterAllocateLQTY(governance.epoch(), user, 2000e18, 1);
bribeInitiative.onAfterAllocateLQTY(governance.epoch(), user, 0, 1);
assertEq(bribeInitiative.totalLQTYAllocatedByEpoch(governance.epoch()), 1e18);
assertEq(bribeInitiative.lqtyAllocatedByUserAtEpoch(user, governance.epoch()), 0);

bribeInitiative.onAfterAllocateLQTY(governance.epoch(), user2, 1e18, 1);
bribeInitiative.onAfterAllocateLQTY(governance.epoch(), user2, 0, 1);
assertEq(bribeInitiative.totalLQTYAllocatedByEpoch(governance.epoch()), 0);
assertEq(bribeInitiative.lqtyAllocatedByUserAtEpoch(user2, governance.epoch()), 0);

Expand Down Expand Up @@ -154,7 +154,7 @@ contract BribeInitiativeAllocateTest is Test {
assertEq(bribeInitiative.lqtyAllocatedByUserAtEpoch(user, governance.epoch()), 1000e18);

governance.setEpoch(2);
bribeInitiative.onAfterAllocateLQTY(governance.epoch(), user, 2000e18, 1);
bribeInitiative.onAfterAllocateLQTY(governance.epoch(), user, 0, 1);
assertEq(bribeInitiative.totalLQTYAllocatedByEpoch(governance.epoch()), 1e18);
assertEq(bribeInitiative.lqtyAllocatedByUserAtEpoch(user, governance.epoch()), 0);

Expand Down Expand Up @@ -196,12 +196,12 @@ contract BribeInitiativeAllocateTest is Test {
assertEq(bribeInitiative.lqtyAllocatedByUserAtEpoch(user, governance.epoch()), 1000e18);

governance.setEpoch(2);
bribeInitiative.onAfterAllocateLQTY(governance.epoch(), user, 2000e18, 1);
bribeInitiative.onAfterAllocateLQTY(governance.epoch(), user, 0, 1);
assertEq(bribeInitiative.totalLQTYAllocatedByEpoch(governance.epoch()), 1e18);
assertEq(bribeInitiative.lqtyAllocatedByUserAtEpoch(user, governance.epoch()), 0);

governance.setEpoch(3);
bribeInitiative.onAfterAllocateLQTY(governance.epoch(), user, 2000e18, 1);
bribeInitiative.onAfterAllocateLQTY(governance.epoch(), user, 0, 1);
assertEq(bribeInitiative.totalLQTYAllocatedByEpoch(governance.epoch()), 1e18);
assertEq(bribeInitiative.lqtyAllocatedByUserAtEpoch(user, governance.epoch()), 0);
}
Expand Down Expand Up @@ -259,7 +259,7 @@ contract BribeInitiativeAllocateTest is Test {
assertEq(bribeInitiative.totalLQTYAllocatedByEpoch(governance.epoch()), 1001e18);
assertEq(bribeInitiative.lqtyAllocatedByUserAtEpoch(user, governance.epoch()), 1000e18);

bribeInitiative.onAfterAllocateLQTY(governance.epoch(), user, 2000e18, 1);
bribeInitiative.onAfterAllocateLQTY(governance.epoch(), user, 0, 1);
assertEq(bribeInitiative.totalLQTYAllocatedByEpoch(governance.epoch()), 1e18);
assertEq(bribeInitiative.lqtyAllocatedByUserAtEpoch(user, governance.epoch()), 0);

Expand Down Expand Up @@ -294,7 +294,7 @@ contract BribeInitiativeAllocateTest is Test {
assertEq(bribeInitiative.totalLQTYAllocatedByEpoch(governance.epoch()), 1001e18);
assertEq(bribeInitiative.lqtyAllocatedByUserAtEpoch(user, governance.epoch()), 1000e18);

bribeInitiative.onAfterAllocateLQTY(governance.epoch(), user, 2000e18, 1);
bribeInitiative.onAfterAllocateLQTY(governance.epoch(), user, 0, 1);
assertEq(bribeInitiative.totalLQTYAllocatedByEpoch(governance.epoch()), 1e18);
assertEq(bribeInitiative.lqtyAllocatedByUserAtEpoch(user, governance.epoch()), 0);

Expand Down Expand Up @@ -326,11 +326,11 @@ contract BribeInitiativeAllocateTest is Test {
assertEq(bribeInitiative.totalLQTYAllocatedByEpoch(governance.epoch()), 1001e18);
assertEq(bribeInitiative.lqtyAllocatedByUserAtEpoch(user, governance.epoch()), 1000e18);

bribeInitiative.onAfterAllocateLQTY(governance.epoch(), user, 2000e18, 1);
bribeInitiative.onAfterAllocateLQTY(governance.epoch(), user, 0, 1);
assertEq(bribeInitiative.totalLQTYAllocatedByEpoch(governance.epoch()), 1e18);
assertEq(bribeInitiative.lqtyAllocatedByUserAtEpoch(user, governance.epoch()), 0);

bribeInitiative.onAfterAllocateLQTY(governance.epoch(), user, 1000e18, 1);
bribeInitiative.onAfterAllocateLQTY(governance.epoch(), user, 0, 1);
assertEq(bribeInitiative.totalLQTYAllocatedByEpoch(governance.epoch()), 1e18);
assertEq(bribeInitiative.lqtyAllocatedByUserAtEpoch(user, governance.epoch()), 0);
}
Expand Down
Loading