QuantAMM
QuantAMM
49,600 OP
View results
Previous Next
Submission Details
Severity: low
Valid

Inconsistencies may occur due to the use `block.number` instead `block.timestamp`

HashNode Labs

Vulnerability Details

When updating the FeeData in UpliftOnlyExample::UpliftaddLiquidityProportional(),
we update the blockTimestampDeposit variable from block.timestamp

function addLiquidityProportional(
address pool,
uint256[] memory maxAmountsIn,
uint256 exactBptAmountOut,
bool wethIsEth,
bytes memory userData
) external payable saveSender(msg.sender) returns (uint256[] memory amountsIn) {
if (poolsFeeData[pool][msg.sender].length > 100) {
revert TooManyDeposits(pool, msg.sender);
}
// Do addLiquidity operation - BPT is minted to this contract.
amountsIn = _addLiquidityProportional(
pool,
msg.sender,
address(this),
maxAmountsIn,
exactBptAmountOut,
wethIsEth,
userData
);
​
uint256 tokenID = lpNFT.mint(msg.sender);
​
//this requires the pool to be registered with the QuantAMM update weight runner
//as well as approved with oracles that provide the prices
uint256 depositValue = getPoolLPTokenValue(
IUpdateWeightRunner(_updateWeightRunner).getData(pool),
pool,
MULDIRECTION.MULDOWN
);
​
poolsFeeData[pool][msg.sender].push(
FeeData({
tokenID: tokenID,
amount: exactBptAmountOut,
//this rounding favours the LP
lpTokenDepositValue: depositValue,
//known use of timestamp, caveats are known.
@> blockTimestampDeposit: uint40(block.timestamp),
upliftFeeBps: upliftFeeBps
})
);
​
nftPool[tokenID] = pool;
}
​

But in the function UpliftOnlyExample::afterUpdate(), we are using block.number instead of block.timestamp for same FeeData update.

function afterUpdate(address _from, address _to, uint256 _tokenID) public {
...
...
​
if (tokenIdIndexFound) {
if (_to != address(0)) {
// Update the deposit value to the current value of the pool in base currency (e.g. USD) and the block index to the current block number
//vault.transferLPTokens(_from, _to, feeDataArray[i].amount);
​
feeDataArray[tokenIdIndex].lpTokenDepositValue = lpTokenDepositValueNow;
@> feeDataArray[tokenIdIndex].
blockTimestampDeposit = uint32(block.number);
feeDataArray[tokenIdIndex].upliftFeeBps = upliftFeeBps;
​
//actual transfer not a afterTokenTransfer caused by a burn
poolsFeeData[poolAddress][_to].push(feeDataArray[tokenIdIndex]);
​
...
...
}
}
}

Impact

Using block.number instead of block.timestamp can lead to inconsistencies in the FeeData records. This discrepancy may cause issues while using the blockTimestampDeposit of FeeData in integrations.

Tools Used

Manual Review

Recommendations

We recommend to use block.timestamp in tracking blockTimestampDeposit in afterUpdate() function.

Updates

Lead Judging Commences

n0kto Lead Judge 10 months ago
Submission Judgement Published
Validated
Assigned finding tags:

finding_afterUpdate_blockNumber_instead_of_timestamp

Likelihood: Medium/High, any NFT transfer will change this variable. Impact: Informational/Very Low. This variable is unused and won’t impact anything, but the array is public and its getter will return a variable with inconsistencies.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.