From a4aa27701fd252c511d9a0c8360f9a9fcae1584e Mon Sep 17 00:00:00 2001 From: Ubuntu Date: Fri, 7 Mar 2025 12:46:19 +0000 Subject: [PATCH 1/3] SOV-4800: suggested fix to LM contract and test --- contracts/farm/LiquidityMining.sol | 16 +++++++++ tests/farm/LiquidityMining.js | 56 ++++++++++++++++++++++++++++++ 2 files changed, 72 insertions(+) diff --git a/contracts/farm/LiquidityMining.sol b/contracts/farm/LiquidityMining.sol index 53ef9b311..7296f72aa 100644 --- a/contracts/farm/LiquidityMining.sol +++ b/contracts/farm/LiquidityMining.sol @@ -646,6 +646,22 @@ contract LiquidityMining is ILiquidityMining, LiquidityMiningStorage { _updatePool(poolId); _updateReward(pool, user); + // Calculate the user's share of the pool + uint256 totalPoolTokens = pool.poolToken.balanceOf(address(this)); + uint256 userShare = user.amount.mul(PRECISION).div(totalPoolTokens); + + // Calculate the undistributed rewards + uint256 undistributedRewards = user.accumulatedReward.mul(PRECISION.sub(userShare)).div( + PRECISION + ); + + // Update the pool's accumulatedRewardPerShare to redistribute the undistributed rewards + if (undistributedRewards > 0) { + pool.accumulatedRewardPerShare = pool.accumulatedRewardPerShare.add( + undistributedRewards.mul(PRECISION).div(totalPoolTokens.sub(user.amount)) + ); + } + totalUsersBalance = totalUsersBalance.sub(user.accumulatedReward); uint256 userAmount = user.amount; uint256 userAccumulatedReward = user.accumulatedReward; diff --git a/tests/farm/LiquidityMining.js b/tests/farm/LiquidityMining.js index c1a7b5438..626515fcf 100644 --- a/tests/farm/LiquidityMining.js +++ b/tests/farm/LiquidityMining.js @@ -2592,6 +2592,62 @@ contract("LiquidityMining", (accounts) => { expect(reward1).bignumber.equal("15"); expect(reward2).bignumber.equal("5"); }); + + it.only("check calculation for two users, same token (shares taken into account)", async () => { + const token = token1; + const amount = amount1; + await token.mint(account2, amount); + await token.approve(liquidityMining.address, amount, { from: account2 }); + + await liquidityMining.deposit(token.address, amount, ZERO_ADDRESS, { from: account1 }); + // because automining is on, the following will advance a block + await liquidityMining.deposit(token.address, amount, ZERO_ADDRESS, { from: account2 }); + // sanity checks + expect( + await liquidityMining.getUserAccumulatedReward(token.address, account1) + ).bignumber.equal("10"); + expect( + await liquidityMining.getUserAccumulatedReward(token.address, account2) + ).bignumber.equal("0"); + await mineBlock(); + + const reward1 = await liquidityMining.getUserAccumulatedReward( + token.address, + account1 + ); + const reward2 = await liquidityMining.getUserAccumulatedReward( + token.address, + account2 + ); + + // for the first block, user 1 will receive the reward of 10 (reward given per block for 100% of shares) + // for the second block: + // - user 1 owns 1/2 of the shares => expected reward = 5 (total 10 + 5 = 15) + // - user 2 owns 1/2 of the shares => expected reward = 5 + expect(reward1).bignumber.equal("15"); + expect(reward2).bignumber.equal("5"); + + // 1 block pass here due to automining, 5 reward to each account + await liquidityMining.emergencyWithdraw(token1.address, { from: account1 }); + + // 1 block pass here + await mineBlock(); + + const reward11 = await liquidityMining.getUserAccumulatedReward( + token.address, + account1 + ); + expect(reward11).bignumber.equal("0"); + const reward22 = await liquidityMining.getUserAccumulatedReward( + token.address, + account2 + ); + + //Since account 1 has made emergencyWithdraw so account2 should get all unfinalized rewards + // Fails since update pool and user reward reduces reward to 20 + // expect(reward22).bignumber.equal("40"); + expect(reward22).bignumber.equal("30"); + }); }); describe("getEstimatedReward", () => { From ddd503c20b882937483fd9c68bd4b2d9c953a154 Mon Sep 17 00:00:00 2001 From: Ubuntu Date: Fri, 7 Mar 2025 15:16:46 +0000 Subject: [PATCH 2/3] SOV-4800 fixing arithmetic balance [skip ci] --- contracts/farm/LiquidityMining.sol | 6 +-- tests/farm/LiquidityMining.js | 80 ++++++++++++++++++++++++++++-- 2 files changed, 79 insertions(+), 7 deletions(-) diff --git a/contracts/farm/LiquidityMining.sol b/contracts/farm/LiquidityMining.sol index 7296f72aa..fd48a7fe9 100644 --- a/contracts/farm/LiquidityMining.sol +++ b/contracts/farm/LiquidityMining.sol @@ -651,9 +651,9 @@ contract LiquidityMining is ILiquidityMining, LiquidityMiningStorage { uint256 userShare = user.amount.mul(PRECISION).div(totalPoolTokens); // Calculate the undistributed rewards - uint256 undistributedRewards = user.accumulatedReward.mul(PRECISION.sub(userShare)).div( - PRECISION - ); + uint256 undistributedRewards = // user.accumulatedReward.mul(PRECISION.sub(userShare)).div(PRECISION); // 1st try + // user.accumulatedReward.mul(PRECISION.add(userShare)).div(PRECISION); // 2nd try + user.accumulatedReward; // 3rd try // Update the pool's accumulatedRewardPerShare to redistribute the undistributed rewards if (undistributedRewards > 0) { diff --git a/tests/farm/LiquidityMining.js b/tests/farm/LiquidityMining.js index 626515fcf..5705399be 100644 --- a/tests/farm/LiquidityMining.js +++ b/tests/farm/LiquidityMining.js @@ -2599,9 +2599,31 @@ contract("LiquidityMining", (accounts) => { await token.mint(account2, amount); await token.approve(liquidityMining.address, amount, { from: account2 }); + const initialBlockNumber = await web3.eth.getBlockNumber(); + console.log("initialBlockNumber", initialBlockNumber); + const initialBalance = await token1.balanceOf(account1); + console.log("initialBalance", initialBalance.toString()); + await liquidityMining.deposit(token.address, amount, ZERO_ADDRESS, { from: account1 }); + const blockNumberAfterFirstDeposit = await web3.eth.getBlockNumber(); + console.log("blockNumberAfterFirstDeposit", blockNumberAfterFirstDeposit); + const balanceAfterFirstDeposit = await token1.balanceOf(account1); + console.log("balanceAfterFirstDeposit", balanceAfterFirstDeposit.toString()); + const reward001 = await liquidityMining.getUserAccumulatedReward( + token.address, + account1 + ); + console.log("reward user 1 after first deposit", reward001.toString()); + const reward002 = await liquidityMining.getUserAccumulatedReward( + token.address, + account2 + ); + console.log("reward user 2 after first deposit", reward002.toString()); + // because automining is on, the following will advance a block await liquidityMining.deposit(token.address, amount, ZERO_ADDRESS, { from: account2 }); + const blockNumberAfterSecondDeposit = await web3.eth.getBlockNumber(); + console.log("blockNumberAfterSecondDeposit", blockNumberAfterSecondDeposit); // sanity checks expect( await liquidityMining.getUserAccumulatedReward(token.address, account1) @@ -2609,16 +2631,33 @@ contract("LiquidityMining", (accounts) => { expect( await liquidityMining.getUserAccumulatedReward(token.address, account2) ).bignumber.equal("0"); - await mineBlock(); + const reward101 = await liquidityMining.getUserAccumulatedReward( + token.address, + account1 + ); + console.log("reward user 1 after second deposit", reward101.toString()); + const reward102 = await liquidityMining.getUserAccumulatedReward( + token.address, + account2 + ); + console.log("reward user 2 after second deposit", reward102.toString()); + await mineBlock(); + const blockNumberAfterFirstJump = await web3.eth.getBlockNumber(); + console.log("blockNumberAfterFirstJump", blockNumberAfterFirstJump); + expect( + await liquidityMining.getUserAccumulatedReward(token.address, account2) + ).bignumber.equal("5"); const reward1 = await liquidityMining.getUserAccumulatedReward( token.address, account1 ); + console.log("reward user 1 after first jump", reward1.toString()); const reward2 = await liquidityMining.getUserAccumulatedReward( token.address, account2 ); + console.log("reward user 2 after first jump", reward2.toString()); // for the first block, user 1 will receive the reward of 10 (reward given per block for 100% of shares) // for the second block: @@ -2629,24 +2668,57 @@ contract("LiquidityMining", (accounts) => { // 1 block pass here due to automining, 5 reward to each account await liquidityMining.emergencyWithdraw(token1.address, { from: account1 }); + const blockNumberAfterEmergencyWithdraw = await web3.eth.getBlockNumber(); + console.log("blockNumberAfterEmergencyWithdraw", blockNumberAfterEmergencyWithdraw); + const balanceAfterEmergencyWithdraw = await token1.balanceOf(account1); + console.log("balanceAfterEmergencyWithdraw", balanceAfterEmergencyWithdraw.toString()); + + const reward01 = await liquidityMining.getUserAccumulatedReward( + token.address, + account1 + ); + console.log("reward01", reward01.toString()); + const reward02 = await liquidityMining.getUserAccumulatedReward( + token.address, + account2 + ); + console.log("reward02", reward02.toString()); // 1 block pass here await mineBlock(); - + const blockNumberAfterSecondJump = await web3.eth.getBlockNumber(); + console.log("blockNumberAfterSecondJump", blockNumberAfterSecondJump); const reward11 = await liquidityMining.getUserAccumulatedReward( token.address, account1 ); expect(reward11).bignumber.equal("0"); + console.log("reward user 1 after jump after emergency withdraw", reward11.toString()); const reward22 = await liquidityMining.getUserAccumulatedReward( token.address, account2 ); - + console.log("reward user 2 after jump after emergency withdraw", reward22.toString()); //Since account 1 has made emergencyWithdraw so account2 should get all unfinalized rewards // Fails since update pool and user reward reduces reward to 20 // expect(reward22).bignumber.equal("40"); - expect(reward22).bignumber.equal("30"); + expect(reward22).bignumber.equal("40"); + + await mineBlock(); + const blockNumberAfterThirdJump = await web3.eth.getBlockNumber(); + console.log("blockNumberAfterThirdJump", blockNumberAfterThirdJump); + const reward31 = await liquidityMining.getUserAccumulatedReward( + token.address, + account1 + ); + expect(reward31).bignumber.equal("0"); + console.log("reward user 1 after third jump", reward31.toString()); + const reward32 = await liquidityMining.getUserAccumulatedReward( + token.address, + account2 + ); + console.log("reward user 2 after third jump", reward32.toString()); + expect(reward32).bignumber.equal("50"); }); }); From e51625c5c61bfac43b7971a491a1f6f9d9eaaa8f Mon Sep 17 00:00:00 2001 From: Ubuntu Date: Fri, 7 Mar 2025 16:26:54 +0000 Subject: [PATCH 3/3] SOV-4800 - case for a single user solved --- contracts/farm/LiquidityMining.sol | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/contracts/farm/LiquidityMining.sol b/contracts/farm/LiquidityMining.sol index fd48a7fe9..64b5b519a 100644 --- a/contracts/farm/LiquidityMining.sol +++ b/contracts/farm/LiquidityMining.sol @@ -651,12 +651,10 @@ contract LiquidityMining is ILiquidityMining, LiquidityMiningStorage { uint256 userShare = user.amount.mul(PRECISION).div(totalPoolTokens); // Calculate the undistributed rewards - uint256 undistributedRewards = // user.accumulatedReward.mul(PRECISION.sub(userShare)).div(PRECISION); // 1st try - // user.accumulatedReward.mul(PRECISION.add(userShare)).div(PRECISION); // 2nd try - user.accumulatedReward; // 3rd try + uint256 undistributedRewards = user.accumulatedReward; // user.accumulatedReward.mul(PRECISION.add(userShare)).div(PRECISION); // 2nd try // user.accumulatedReward.mul(PRECISION.sub(userShare)).div(PRECISION); // 1st try // 3rd try // Update the pool's accumulatedRewardPerShare to redistribute the undistributed rewards - if (undistributedRewards > 0) { + if (undistributedRewards > 0 && totalPoolTokens > user.amount) { pool.accumulatedRewardPerShare = pool.accumulatedRewardPerShare.add( undistributedRewards.mul(PRECISION).div(totalPoolTokens.sub(user.amount)) );