[BUG]: investigate overflow safety of donate and feeGrowthGlobal

[moodysalem]

it seems like it's possible that donate could allow overflowing fees earned to type(uint256).max, but needs investigation

[hensha256]

Okay i think this is possible in very extreme circumstances. I decided to start experimenting using the numbers on a super low liquidity v3 pool (because low liquidity => tiny denominator in the code). The one I played around with is JEJUDOGE ETH

Assuming liquidity 1 for simplicity of a low liquidity pool

JEJUDOGE has the following:

• price: $0.000000000300651
• feeGrowthGlobalX128: 2430461034183898847299003516607034091943925
• decimals: 18

Lets say instead of decimals 18 we have 24, which seems a reasonable number of decimals.

With $1million, someone can:

• Buy 1000000*(10^24)/0.000000000300651 JEJUDOGE tokens, and call donate
• The maths would be:

amount * (2^128) / liquidity
= (1000000*(10^24)/0.000000000300651) * (2^128) / 1
= 1.13182 E78

Given that type(uint256).max is 1.15792 E77, this is an overflow.

This is of course a slightly extreme circumstance (very cheap token/high decimals, and very low liquidity), but its definitely within the realms of possibility.

[moodysalem]

fee growth global is meant to overflow, i.e. overflow of fee growth global is not itself broken

in the example given, you're buying ~2^131 tokens, which violates the assumption that any token has <2^128 total supply (see https://www.wolframalpha.com/input?i=log+base+2+of+%281000000*%2810%5E24%29%2F0.000000000300651%29)

the issue is, given the total supply constraint of type(uint128).max, is it possible to overflow the fees earned computation for any position? or more importantly underflow the fees earned, so that the position can collect more fees than it has earned

[NoahZinsmeister]

one more note: because donate only increments in-memory deltas, it might be possible to trigger undesired behavior like what moody described by donating an amount that isn't actually possible, and manipulating overflows s.t. they don't have to actually pay by the end of the lock.

another type of mitigation would be to only accept smaller uints in donate (uint128?) so that it'd be prohibitively gas-expensive to exploit an overflow