From 8ba88b8ea9b3449dff378005a4f8f0385885269f Mon Sep 17 00:00:00 2001 From: DrZoltanFazekas Date: Mon, 3 Mar 2025 11:42:38 +0100 Subject: [PATCH] Reduce the local impact of rounding errors in _rewards() --- e2e_non-liquid.sh | 11 +++++++ src/BaseDelegation.sol | 2 +- src/NonLiquidDelegation.sol | 64 +++++++++++++++++++++++++++++++++---- 3 files changed, 70 insertions(+), 7 deletions(-) diff --git a/e2e_non-liquid.sh b/e2e_non-liquid.sh index ff18116..f06adda 100755 --- a/e2e_non-liquid.sh +++ b/e2e_non-liquid.sh @@ -502,23 +502,34 @@ report() { join_all # all validators join the pool stake_all # all users stake, withdraw rewards, unstake and claim part of it +echo -n "🟪" && cast call $CONTRACT_ADDRESS "totalRoundingErrors()(uint256)" leave_all # all validators leave and withdraw rewards +echo -n "🟪🟪" && cast call $CONTRACT_ADDRESS "totalRoundingErrors()(uint256)" unstake_all # all users unstake everything and withdraw rewards +echo -n "🟪🟪🟪" && cast call $CONTRACT_ADDRESS "totalRoundingErrors()(uint256)" report # print the status echo "1️⃣ 1️⃣ 1️⃣ 1️⃣ 1️⃣ 1️⃣ 1️⃣ 1️⃣ 1️⃣ 1️⃣ 1️⃣ 1️⃣ 1️⃣ 1️⃣ 1️⃣ 1️⃣" +echo -n "🟪🟪🟪🟪" && cast call $CONTRACT_ADDRESS "totalRoundingErrors()(uint256)" sleep 5s stake_all +echo -n "🟪" && cast call $CONTRACT_ADDRESS "totalRoundingErrors()(uint256)" unstake_all +echo -n "🟪🟪🟪" && cast call $CONTRACT_ADDRESS "totalRoundingErrors()(uint256)" report echo "2️⃣ 2️⃣ 2️⃣ 2️⃣ 2️⃣ 2️⃣ 2️⃣ 2️⃣ 2️⃣ 2️⃣ 2️⃣ 2️⃣ 2️⃣ 2️⃣ 2️⃣ 2️⃣" +echo -n "🟪🟪🟪🟪" && cast call $CONTRACT_ADDRESS "totalRoundingErrors()(uint256)" sleep 5s join_all stake_all +echo -n "🟪" && cast call $CONTRACT_ADDRESS "totalRoundingErrors()(uint256)" leave_all +echo -n "🟪🟪" && cast call $CONTRACT_ADDRESS "totalRoundingErrors()(uint256)" unstake_all +echo -n "🟪🟪🟪" && cast call $CONTRACT_ADDRESS "totalRoundingErrors()(uint256)" report echo "3️⃣ 3️⃣ 3️⃣ 3️⃣ 3️⃣ 3️⃣ 3️⃣ 3️⃣ 3️⃣ 3️⃣ 3️⃣ 3️⃣ 3️⃣ 3️⃣ 3️⃣ 3️⃣" +echo -n "🟪🟪🟪🟪" && cast call $CONTRACT_ADDRESS "totalRoundingErrors()(uint256)" diff --git a/src/BaseDelegation.sol b/src/BaseDelegation.sol index df688ff..df4aafe 100644 --- a/src/BaseDelegation.sol +++ b/src/BaseDelegation.sol @@ -211,7 +211,7 @@ abstract contract BaseDelegation is IDelegation, PausableUpgradeable, Ownable2St /** * @dev The current version of all upgradeable contracts in the repository. */ - uint64 internal immutable VERSION = encodeVersion(0, 5, 0); + uint64 internal immutable VERSION = encodeVersion(0, 5, 1); /** * @dev Return the contracts' version. diff --git a/src/NonLiquidDelegation.sol b/src/NonLiquidDelegation.sol index b6d9de2..8931b0a 100644 --- a/src/NonLiquidDelegation.sol +++ b/src/NonLiquidDelegation.sol @@ -80,6 +80,8 @@ contract NonLiquidDelegation is IDelegation, BaseDelegation, INonLiquidDelegatio mapping(address => uint256) taxedSinceLastStaking; int256 immutableRewards; mapping(address => address) newAddress; +mapping(address => uint256) roundingErrors; +uint256 totalRoundingErrors; } // keccak256(abi.encode(uint256(keccak256("zilliqa.storage.NonLiquidDelegation")) - 1)) & ~bytes32(uint256(0xff)) @@ -314,7 +316,9 @@ contract NonLiquidDelegation is IDelegation, BaseDelegation, INonLiquidDelegatio */ function rewards(uint64 additionalSteps) public view returns(uint256) { NonLiquidDelegationStorage storage $ = _getNonLiquidDelegationStorage(); - (uint256 resultInTotal, , , ) = _rewards(additionalSteps); +//TODO: replace +// (uint256 resultInTotal, , , ) = _rewards(additionalSteps); + (uint256 resultInTotal, , , , , ) = _rewards(additionalSteps); resultInTotal -= $.taxedSinceLastStaking[_msgSender()]; return resultInTotal - resultInTotal * getCommissionNumerator() / DENOMINATOR + $.availableTaxedRewards[_msgSender()]; } @@ -324,7 +328,9 @@ contract NonLiquidDelegation is IDelegation, BaseDelegation, INonLiquidDelegatio */ function rewards() public view returns(uint256) { NonLiquidDelegationStorage storage $ = _getNonLiquidDelegationStorage(); - (uint256 resultInTotal, , , ) = _rewards(); +//TODO: replace +// (uint256 resultInTotal, , , ) = _rewards(); + (uint256 resultInTotal, , , , , ) = _rewards(); resultInTotal -= $.taxedSinceLastStaking[_msgSender()]; return resultInTotal - resultInTotal * getCommissionNumerator() / DENOMINATOR + $.availableTaxedRewards[_msgSender()]; } @@ -393,6 +399,14 @@ contract NonLiquidDelegation is IDelegation, BaseDelegation, INonLiquidDelegatio emit Staked(_msgSender(), amount, ""); } +//TODO: natspec comment +function totalRoundingErrors() public view returns(uint256) { + NonLiquidDelegationStorage storage $ = _getNonLiquidDelegationStorage(); + return $.totalRoundingErrors; +} + +//TODO: remove +event Test(string,uint); /** * @dev Make the requested `amount` of taxed rewards available to the caller for staking or * withdrawing by traversing the {Staking} history in `1 + additionalSteps`. @@ -402,14 +416,22 @@ contract NonLiquidDelegation is IDelegation, BaseDelegation, INonLiquidDelegatio */ function _useRewards(uint256 amount, uint64 additionalSteps) internal whenNotPaused returns(uint256, uint256) { NonLiquidDelegationStorage storage $ = _getNonLiquidDelegationStorage(); +// # emit Test("$.totalRoundingErrors", $.totalRoundingErrors); +// # emit Test("$.roundingErrors[_msgSender()]", $.roundingErrors[_msgSender()]); +// # $.totalRoundingErrors -= $.roundingErrors[_msgSender()]; ( uint256 resultInTotal, uint256 resultAfterLastStaking, uint64 posInStakingIndices, - uint64 nextStakingIndex + uint64 nextStakingIndex, +uint256 roundingError, +uint256 totalRoundingErrors ) = additionalSteps == type(uint64).max ? _rewards() : _rewards(additionalSteps); +// # $.roundingErrors[_msgSender()] += roundingError; +// # $.totalRoundingErrors += roundingError; +// # //$.totalRoundingErrors = totalRoundingErrors; // the caller has not delegated any stake yet if (nextStakingIndex == 0) return (0, 0); @@ -442,7 +464,9 @@ contract NonLiquidDelegation is IDelegation, BaseDelegation, INonLiquidDelegatio uint256 resultInTotal, uint256 resultAfterLastStaking, uint64 posInStakingIndices, - uint64 nextStakingIndex + uint64 nextStakingIndex, +uint256 roundingError, +uint256 totalRoundingErrors ) { return _rewards(type(uint64).max); } @@ -455,12 +479,17 @@ contract NonLiquidDelegation is IDelegation, BaseDelegation, INonLiquidDelegatio uint256 resultInTotal, uint256 resultAfterLastStaking, uint64 posInStakingIndices, - uint64 nextStakingIndex + uint64 nextStakingIndex, +uint256 roundingError, +uint256 totalRoundingErrors ) { NonLiquidDelegationStorage storage $ = _getNonLiquidDelegationStorage(); uint64 firstStakingIndex; uint256 amount; uint256 total; +// # totalRoundingErrors = $.totalRoundingErrors; +// # //TODO: enable the below line +// # //roundingError = $.roundingErrors[_msgSender()]; for ( posInStakingIndices = $.firstStakingIndex[_msgSender()]; posInStakingIndices < $.stakingIndices[_msgSender()].length; @@ -481,10 +510,24 @@ contract NonLiquidDelegation is IDelegation, BaseDelegation, INonLiquidDelegatio ) { if (total > 0) resultInTotal += $.stakings[nextStakingIndex].rewards * amount / total; +if (total > 0) { +roundingError += + 1 ether * $.stakings[nextStakingIndex].rewards * amount / total - + 1 ether * ($.stakings[nextStakingIndex].rewards * amount / total); +// # // totalRoundingErrors = $.totalRoundingErrors + roundingError - $.roundingErrors[_msgSender()]; +} total = $.stakings[nextStakingIndex].total; nextStakingIndex++; if (nextStakingIndex - firstStakingIndex > additionalSteps) - return (resultInTotal, resultAfterLastStaking, posInStakingIndices, nextStakingIndex); +// # //TODO: remove return (resultInTotal, resultAfterLastStaking, posInStakingIndices, nextStakingIndex); + return ( + resultInTotal + roundingError / 1 ether, + resultAfterLastStaking, + posInStakingIndices, + nextStakingIndex, + roundingError - 1 ether * (roundingError / 1 ether), + totalRoundingErrors + ); } } @@ -493,6 +536,12 @@ contract NonLiquidDelegation is IDelegation, BaseDelegation, INonLiquidDelegatio // the last step is to add the rewards accrued since the last staking if (total > 0) { resultAfterLastStaking = (int256(getRewards()) - $.immutableRewards).toUint256() * amount / total; +// # //TODO: replace the above line with +// # // resultAfterLastStaking = (int256(getRewards()) - totalRoundingErrors / 1 ether - $.immutableRewards).toUint256() * amount / total; +roundingError += + 1 ether * (int256(getRewards()) - $.immutableRewards).toUint256() * amount / total - + 1 ether * ((int256(getRewards()) - $.immutableRewards).toUint256() * amount / total); +// # // totalRoundingErrors = $.totalRoundingErrors + roundingError - $.roundingErrors[_msgSender()]; resultInTotal += resultAfterLastStaking; } } @@ -502,6 +551,9 @@ contract NonLiquidDelegation is IDelegation, BaseDelegation, INonLiquidDelegatio // existed during the current call of the function so that we can continue from there if (posInStakingIndices > 0) posInStakingIndices--; +resultInTotal += roundingError / 1 ether; +roundingError -= 1 ether * (roundingError / 1 ether); +// # // totalRoundingErrors = $.totalRoundingErrors + roundingError - $.roundingErrors[_msgSender()]; } /**