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.

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

Fix: Upscale TS to reduce precision loss #68

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
57 commits
Select commit Hold shift + click to select a range
e1df25c
chore: cleanup
GalloDaSballo Oct 31, 2024
b18c629
chore: e2E
GalloDaSballo Oct 31, 2024
f693d5f
fix: cap amount for bribes
GalloDaSballo Oct 31, 2024
01457bd
chore: desc
GalloDaSballo Oct 31, 2024
a589a29
feat: upscale ts to u120
GalloDaSballo Nov 1, 2024
7e777ae
feat: tests somewhat compile
GalloDaSballo Nov 1, 2024
d27487f
feat: invariants compile and have a few TS tests
GalloDaSballo Nov 1, 2024
5d69f9a
chore: easy tests are fixed
GalloDaSballo Nov 1, 2024
4b75ce1
chore: fix mock tests
GalloDaSballo Nov 1, 2024
7c8a577
fix: epochStart is magnified with a hacky solution
GalloDaSballo Nov 1, 2024
105bc32
fix: test ts are magnified
GalloDaSballo Nov 1, 2024
075bf4b
chore: fix overflow
GalloDaSballo Nov 1, 2024
dc845b3
fix: abi decode
GalloDaSballo Nov 1, 2024
28fe9d9
fix: BI-07
GalloDaSballo Nov 1, 2024
56d8e86
WARNING: Add tollerance
GalloDaSballo Nov 1, 2024
d82a0c3
feat: laxer property
GalloDaSballo Nov 1, 2024
6482f51
feat: debug and fix properties
GalloDaSballo Nov 1, 2024
4db439e
fix: BI07
GalloDaSballo Nov 1, 2024
723e9e1
feat: truncation but maybe not so bad
GalloDaSballo Nov 1, 2024
90af7db
fix: we need a higher confidence range
GalloDaSballo Nov 1, 2024
d559696
feat: raise the precision to 1e26
GalloDaSballo Nov 1, 2024
81c048e
fix: upscale the precision in bribes
GalloDaSballo Nov 1, 2024
50933d0
chore: fix tests
GalloDaSballo Nov 1, 2024
bd2cc0a
feat: trying out overflow mode
GalloDaSballo Nov 1, 2024
0572a61
chore: cleanup
GalloDaSballo Nov 1, 2024
9259e26
feat: document overflow risks
GalloDaSballo Nov 1, 2024
ceffe66
fix: overflow in timestamp math
GalloDaSballo Nov 1, 2024
426de19
chore: type consistency
GalloDaSballo Nov 1, 2024
3ecb7a2
fix: ensure user TS never decreases
GalloDaSballo Nov 1, 2024
26d4414
chore: comments on overflow risk
GalloDaSballo Nov 1, 2024
55b94aa
gas: order of operations
GalloDaSballo Nov 1, 2024
9548318
chore: rename fn
GalloDaSballo Nov 1, 2024
0246b32
chore: order of operations for gas
GalloDaSballo Nov 1, 2024
3205922
chore: consistent types
GalloDaSballo Nov 1, 2024
84fcb22
chore: added precision invariant on `getInitiativeState`
GalloDaSballo Nov 1, 2024
f4b1419
chore: custom precision change
GalloDaSballo Nov 1, 2024
04d1b16
chore: remove property that doesn't really work
GalloDaSballo Nov 1, 2024
1ea9df5
feat: add a few revert tests that can help detect overflows
GalloDaSballo Nov 1, 2024
7c96bc7
chore: undo de-optimizations
GalloDaSballo Nov 1, 2024
f210e44
chore: recommendation
GalloDaSballo Nov 1, 2024
e553fea
chore: cleanup
GalloDaSballo Nov 1, 2024
d762007
fix: approve for bribes
GalloDaSballo Nov 2, 2024
1219a02
fix: clamped claim
GalloDaSballo Nov 2, 2024
7742491
chore: cleanup and simplification
GalloDaSballo Nov 2, 2024
4ded0c7
feat: repro for claim bribe
GalloDaSballo Nov 2, 2024
d60587e
feat: extra config for running locally
GalloDaSballo Nov 2, 2024
28d6425
chore: undo config changes
GalloDaSballo Nov 2, 2024
d904b26
feat: add solvency tests for Governance
GalloDaSballo Nov 2, 2024
96de709
feat: insolvency optimization tests
GalloDaSballo Nov 2, 2024
1f0b310
chore: improved coverage and removed canaries
GalloDaSballo Nov 2, 2024
2bfb2b9
chore: make precision public
GalloDaSballo Nov 3, 2024
eb42ca1
chore: local echidna command
GalloDaSballo Nov 3, 2024
4d5526f
cleanup: remove debugged properties
GalloDaSballo Nov 3, 2024
b1b5b2b
fix: remove BI11
GalloDaSballo Nov 3, 2024
36ae4c6
fix: Total and Initiative math can only be compared on state, not sna…
GalloDaSballo Nov 3, 2024
f483b06
fix: optimization properties to use state values
GalloDaSballo Nov 3, 2024
29471b2
chore: comment on possible more checks
GalloDaSballo Nov 3, 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
2 changes: 1 addition & 1 deletion echidna.yaml
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
testMode: "optimization"
testMode: "overflow"
prefix: "optimize_"
coverage: true
corpusDir: "echidna"
Expand Down
45 changes: 23 additions & 22 deletions src/BribeInitiative.sol
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,12 @@ contract BribeInitiative is IInitiative, IBribeInitiative {
}

/// @inheritdoc IBribeInitiative
function totalLQTYAllocatedByEpoch(uint16 _epoch) external view returns (uint88, uint32) {
function totalLQTYAllocatedByEpoch(uint16 _epoch) external view returns (uint88, uint120) {
return _loadTotalLQTYAllocation(_epoch);
}

/// @inheritdoc IBribeInitiative
function lqtyAllocatedByUserAtEpoch(address _user, uint16 _epoch) external view returns (uint88, uint32) {
function lqtyAllocatedByUserAtEpoch(address _user, uint16 _epoch) external view returns (uint88, uint120) {
return _loadLQTYAllocation(_user, _epoch);
}

Expand All @@ -70,6 +70,9 @@ contract BribeInitiative is IInitiative, IBribeInitiative {
bribeToken.safeTransferFrom(msg.sender, address(this), _bribeTokenAmount);
}

uint256 constant TIMESTAMP_PRECISION = 1e26;


function _claimBribe(
address _user,
uint16 _epoch,
Expand Down Expand Up @@ -98,28 +101,22 @@ contract BribeInitiative is IInitiative, IBribeInitiative {
"BribeInitiative: invalid-prev-total-lqty-allocation-epoch"
);

(uint88 totalLQTY, uint32 totalAverageTimestamp) = _decodeLQTYAllocation(totalLQTYAllocation.value);
// WHAT HAPPENS IF WE ENFORCE EPOCH AT END?
// THEN WE LINEARLY TRANSLATE TO EPOCH END?
// EPOCH_START + epoch * EPOCH_DURATION is the time to claim (I think)
(uint88 totalLQTY, uint120 totalAverageTimestamp) = _decodeLQTYAllocation(totalLQTYAllocation.value);

// Since epoch 1 starts at Epoch Start, epoch * Duration goes to the end of
// But is this safe vs last second of the epoch?
// I recall having a similar issue already with Velodrome
uint32 epochEnd = uint32(governance.EPOCH_START()) + uint32(_epoch) * uint32(governance.EPOCH_DURATION());
// NOTE: SCALING!!! | The timestamp will work until type(uint32).max | After which the math will eventually overflow
uint120 scaledEpochEnd = (uint120(governance.EPOCH_START()) + uint120(_epoch) * uint120(governance.EPOCH_DURATION())) * uint120(TIMESTAMP_PRECISION);

/// @audit User Invariant
assert(totalAverageTimestamp <= epochEnd); /// NOTE: Tests break because they are not realistic

assert(totalAverageTimestamp <= scaledEpochEnd);

uint240 totalVotes = governance.lqtyToVotes(totalLQTY, epochEnd, totalAverageTimestamp);
uint240 totalVotes = governance.lqtyToVotes(totalLQTY, scaledEpochEnd, totalAverageTimestamp);
if (totalVotes != 0) {
(uint88 lqty, uint32 averageTimestamp) = _decodeLQTYAllocation(lqtyAllocation.value);
(uint88 lqty, uint120 averageTimestamp) = _decodeLQTYAllocation(lqtyAllocation.value);

/// @audit Governance Invariant
assert(averageTimestamp <= epochEnd); /// NOTE: Tests break because they are not realistic
assert(averageTimestamp <= scaledEpochEnd);

uint240 votes = governance.lqtyToVotes(lqty, epochEnd, averageTimestamp);
uint240 votes = governance.lqtyToVotes(lqty, scaledEpochEnd, averageTimestamp);
boldAmount = uint256(bribe.boldAmount) * uint256(votes) / uint256(totalVotes);
bribeTokenAmount = uint256(bribe.bribeTokenAmount) * uint256(votes) / uint256(totalVotes);
}
Expand All @@ -143,13 +140,17 @@ contract BribeInitiative is IInitiative, IBribeInitiative {
bribeTokenAmount += bribeTokenAmount_;
}

// NOTE: Due to rounding errors in the `averageTimestamp` bribes may slightly overpay compared to what they have allocated
// We cap to the available amount for this reason
// The error should be below 10 LQTY per annum, in the worst case
if (boldAmount != 0) {
uint256 max = bold.balanceOf(address(this));
if (boldAmount > max) {
boldAmount = max;
}
bold.safeTransfer(msg.sender, boldAmount);
}

if (bribeTokenAmount != 0) {
uint256 max = bribeToken.balanceOf(address(this));
if (bribeTokenAmount > max) {
Expand All @@ -165,7 +166,7 @@ contract BribeInitiative is IInitiative, IBribeInitiative {
/// @inheritdoc IInitiative
function onUnregisterInitiative(uint16) external virtual override onlyGovernance {}

function _setTotalLQTYAllocationByEpoch(uint16 _epoch, uint88 _lqty, uint32 _averageTimestamp, bool _insert)
function _setTotalLQTYAllocationByEpoch(uint16 _epoch, uint88 _lqty, uint120 _averageTimestamp, bool _insert)
private
{
uint224 value = _encodeLQTYAllocation(_lqty, _averageTimestamp);
Expand All @@ -181,7 +182,7 @@ contract BribeInitiative is IInitiative, IBribeInitiative {
address _user,
uint16 _epoch,
uint88 _lqty,
uint32 _averageTimestamp,
uint120 _averageTimestamp,
bool _insert
) private {
uint224 value = _encodeLQTYAllocation(_lqty, _averageTimestamp);
Expand All @@ -193,20 +194,20 @@ contract BribeInitiative is IInitiative, IBribeInitiative {
emit ModifyLQTYAllocation(_user, _epoch, _lqty, _averageTimestamp);
}

function _encodeLQTYAllocation(uint88 _lqty, uint32 _averageTimestamp) private pure returns (uint224) {
function _encodeLQTYAllocation(uint88 _lqty, uint120 _averageTimestamp) private pure returns (uint224) {
return EncodingDecodingLib.encodeLQTYAllocation(_lqty, _averageTimestamp);
}

function _decodeLQTYAllocation(uint224 _value) private pure returns (uint88, uint32) {
function _decodeLQTYAllocation(uint224 _value) private pure returns (uint88, uint120) {
return EncodingDecodingLib.decodeLQTYAllocation(_value);
}

function _loadTotalLQTYAllocation(uint16 _epoch) private view returns (uint88, uint32) {
function _loadTotalLQTYAllocation(uint16 _epoch) private view returns (uint88, uint120) {
require(_epoch <= governance.epoch(), "No future Lookup");
return _decodeLQTYAllocation(totalLQTYAllocationByEpoch.items[_epoch].value);
}

function _loadLQTYAllocation(address _user, uint16 _epoch) private view returns (uint88, uint32) {
function _loadLQTYAllocation(address _user, uint16 _epoch) private view returns (uint88, uint120) {
require(_epoch <= governance.epoch(), "No future Lookup");
return _decodeLQTYAllocation(lqtyAllocationByUserAtEpoch[_user].items[_epoch].value);
}
Expand Down
5 changes: 5 additions & 0 deletions src/CurveV2GaugeRewards.sol
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,11 @@ contract CurveV2GaugeRewards is BribeInitiative {

remainder = 0;

uint256 available = bold.balanceOf(address(this));
if(available < total) {
total = available; // Cap due to rounding error causing a bit more bold being given away
}

bold.approve(address(gauge), total);
gauge.deposit_reward_token(address(bold), total, duration);

Expand Down
Loading
Loading