c4-submissions commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-10-canto/blob/40edbe0c9558b478c84336aaad9b9626e5d99f34/canto_ambient/contracts/mixins/LiquidityMining.sol#L69

Vulnerability details

Impact

If a user provides liquidity on ticks which are entered and exited a large number of times, the gas required to call the accrueConcentratedPositionTimeWeightedLiquidity can exceed the block gas limit.

Proof of Concept

The accrueConcentratedPositionTimeWeightedLiquidity function loops over the unbounded TickTracking array inside tickTracking_ mapping.

                // Loop through all in-range time spans for the tick or up to the current time (if it is still in range)
                while (time < block.timestamp && tickTrackingIndex < numTickTracking) {
                    TickTracking memory tickTracking = tickTracking_[poolIdx][i][tickTrackingIndex];

https://github.com/code-423n4/2023-10-canto/blob/40edbe0c9558b478c84336aaad9b9626e5d99f34/canto_ambient/contracts/mixins/LiquidityMining.sol#L88-L95

The TickTracking array is pushed with a new value everytime the corresponding tick is entered.

Hence a possible combination of too many tick movements and user's not collecting rewards/updating position for a long time can result in the amount of gas required to iterate the loop exceeding the block gas limit.

Demo:

With 4000 cross tick movements across two ticks before the user claim rewards, the gas used to call the claimConcentratedRewards function is 34M which is above the block gas limit of 30M.

To execute the demo the following changes has to be made to the current repo:

Update ProtocolCmd.sol: Done to add a new command which will allow to cross the tick


diff --git a/canto_ambient/contracts/libraries/ProtocolCmd.sol b/canto_ambient/contracts/libraries/ProtocolCmd.sol
index 21a408d..7641c69 100644
--- a/canto_ambient/contracts/libraries/ProtocolCmd.sol
+++ b/canto_ambient/contracts/libraries/ProtocolCmd.sol
@@ -119,4 +119,5 @@ library UserCmd {
 ////////////////////////////////////////////////////////////////////////////
 uint8 constant CLAIM_CONC_REWARDS_CODE = 101;
 uint8 constant CLAIM_AMB_REWARDS_CODE = 102;
+    uint8 constant CROSS_TICKS = 103;
}


2. Update LiquidityMiningPath.sol: Done to add the command to cross the ticks
```diff
diff --git a/canto_ambient/contracts/callpaths/LiquidityMiningPath.sol b/canto_ambient/contracts/callpaths/LiquidityMiningPath.sol
index e6c63f7..bb511c9 100644
--- a/canto_ambient/contracts/callpaths/LiquidityMiningPath.sol
+++ b/canto_ambient/contracts/callpaths/LiquidityMiningPath.sol
@@ -46,11 +46,15 @@ contract LiquidityMiningPath is LiquidityMining {
             claimConcentratedRewards(poolHash, lowerTick, upperTick, weeksToClaim);
         } else if (code == UserCmd.CLAIM_AMB_REWARDS_CODE) {
             claimAmbientRewards(poolHash, weeksToClaim);
+        } else if (code == UserCmd.CROSS_TICKS){
+            crossTicks(poolHash,lowerTick,upperTick);
         } else {
             revert("Invalid user command");
         }
     }

+    
+
     function claimConcentratedRewards(bytes32 poolIdx, int24 lowerTick, int24 upperTick, uint32[] memory weeksToClaim)
         public
         payable

Update TestLiquidityMining.js: Commented out the swap and added loop to continously cross the tick. Test passes if the gas used is more than 30M


diff --git a/canto_ambient/test_canto/TestLiquidityMining.js b/canto_ambient/test_canto/TestLiquidityMining.js
index bd21a32..0e9c0eb 100644
--- a/canto_ambient/test_canto/TestLiquidityMining.js
+++ b/canto_ambient/test_canto/TestLiquidityMining.js
@@ -31,6 +31,8 @@ function toSqrtPrice(price) {
chai.use(solidity);

describe("Liquidity Mining Tests", function () {
+
+   this.timeout(3000000);
it("deploy contracts and init pool", async function () {
    const [owner] = await ethers.getSigners();

@@ -225,23 +227,23 @@ describe("Liquidity Mining Tests", function () { //////////////////////////////////////////////// // SAMPLE SWAP TEST (swaps 2 USDC for cNOTE) ////////////////////////////////////////////////

swapTx = await dex.swap(
cNOTE.address, // base
USDC.address, // quote
36000, // poolIdx
false, // isBuy
false, // inBaseQty
BigNumber.from("2000000"), // qty
0, // tip
BigNumber.from("16602069666338596454400000"), // limit price
BigNumber.from("1900000000000000000"), // min out
0 // reserveFlag (to use surplus or not)
);
await swapTx.wait();
expect(await USDC.balanceOf(owner.address)).to.equal(
BigNumber.from("999898351768")
);
// swapTx = await dex.swap(
// cNOTE.address, // base
// USDC.address, // quote
// 36000, // poolIdx
// false, // isBuy
// false, // inBaseQty
// BigNumber.from("2000000"), // qty
// 0, // tip
// BigNumber.from("16602069666338596454400000"), // limit price
// BigNumber.from("1900000000000000000"), // min out
// 0 // reserveFlag (to use surplus or not)
// );
// await swapTx.wait();
// expect(await USDC.balanceOf(owner.address)).to.equal(
// BigNumber.from("999898351768")
// );
```
//////////////////////////////////////////////////
// SET LIQUIDITY MINING REWARDS FOR CONCENTRATED LIQUIDITY
```
@@ -270,7 +272,46 @@ describe("Liquidity Mining Tests", function () { tx = await dex.protocolCmd(8, setRewards, true); await tx.wait();
await time.increase(604800 * 5); // fast forward 1000 seconds so that rewards accrue
// make the ticks exit and reenter
for(let i=1;i<=4000;i++){
let tickMover = abi.encode(
["uint8", "bytes32", "int24", "int24", "uint32[]"],
[
103,
keccak256(poolHash),
currentTick ,
currentTick +1,
[
Math.floor(timestampBefore / 604800) * 604800 + 604800,
Math.floor(timestampBefore / 604800) 604800 + 604800 2,
],
]
);
tx = await dex.userCmd(8, tickMover);
await tx.wait();
await time.increase(100);
tickMover = abi.encode(
["uint8", "bytes32", "int24", "int24", "uint32[]"],
[
103,
keccak256(poolHash),
currentTick +1,
currentTick ,
[
Math.floor(timestampBefore / 604800) * 604800 + 604800,
Math.floor(timestampBefore / 604800) 604800 + 604800 2,
],
]
);
tx = await dex.userCmd(8, tickMover);
await tx.wait();
await time.increase(100);
}
await time.increase(604800 * 5);
```
//////////////////////////////////////////////////
// CLAIM REWARDS ACCRUED FROM CONCENTRATED REWARDS
```
@@ -294,7 +335,9 @@ describe("Liquidity Mining Tests", function () { ] ); tx = await dex.userCmd(8, claim);
await tx.wait();
const receipt = await tx.wait();
console.log(receipt.gasUsed);

expect(receipt.gasUsed > 30000000).to.be.eq(true);

// get eth balanace of dex after claim
const dexBalAfter = await ethers.provider.getBalance(dex.address);



## Tools Used
Hardhat

## Recommendations
If possible find an alternative method to calculate the user's concentrated liquidity or warn the user's about this issue.

## Assessed type

Loop

c4-pre-sort commented 1 year ago

141345 marked the issue as duplicate of #114

c4-pre-sort commented 1 year ago

141345 marked the issue as sufficient quality report

c4-judge commented 1 year ago

dmvt changed the severity to 3 (High Risk)

c4-judge commented 1 year ago

dmvt marked the issue as satisfactory

code-423n4 / 2023-10-canto-findings

Users may be unable to claim their rewards and add/remove liquidity due exceeding gas limit #276

Lines of code

Vulnerability details

Impact

Proof of Concept

Demo: