Skip to content

Commit

Permalink
Merge pull request #132 from cryptoalgebra/fix/bailsec-audit-integral…
Browse files Browse the repository at this point in the history 
…v1.2

Fixes after second review
  • Loading branch information
@IliaAzhel
IliaAzhel authored Sep 18, 2024
2 parents 7cae571 + 99f3191 commit 86cf458
Show file tree
Hide file tree
Showing 5 changed files with 81 additions and 41 deletions.
2 changes: 1 addition & 1 deletion src/core/contracts/AlgebraFactory.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ contract AlgebraFactory is IAlgebraFactory, Ownable2Step, AccessControlEnumerabl

/// @inheritdoc IAlgebraFactory
/// @dev keccak256 of AlgebraPool init bytecode. Used to compute pool address deterministically
bytes32 public constant POOL_INIT_CODE_HASH = 0x3093a65c28d1e6def42997fdea76d033dfd42b432c107e19412cf91a1eac0f91;
bytes32 public constant POOL_INIT_CODE_HASH = 0xc5ee3168bec63cbec40c1187858f314e9c2a303e86e16005213d885cc87d4305;

constructor(address _poolDeployer) {
require(_poolDeployer != address(0));
Expand Down
12 changes: 8 additions & 4 deletions src/core/contracts/base/ReservesManager.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,8 @@ abstract contract ReservesManager is AlgebraPoolBase {
uint256 fee1,
uint256 lastTimestamp,
bytes32 receiverSlot,
bytes32 feePendingSlot
bytes32 feePendingSlot,
bool dontCheck
) internal returns (uint104, uint104, uint256, uint256) {
if (fee0 | fee1 != 0) {
uint256 feePending0;
Expand All @@ -94,6 +95,7 @@ abstract contract ReservesManager is AlgebraPoolBase {
feePending1 += fee1;

if (
dontCheck ||
_blockTimestamp() - lastTimestamp >= Constants.FEE_TRANSFER_FREQUENCY || feePending0 > type(uint104).max || feePending1 > type(uint104).max
) {
// use sload from slot (like pointer dereference) to avoid gas
Expand All @@ -115,7 +117,7 @@ abstract contract ReservesManager is AlgebraPoolBase {
return (uint104(feePending0), uint104(feePending1), 0, 0);
}
} else {
if (_blockTimestamp() - lastTimestamp >= Constants.FEE_TRANSFER_FREQUENCY) {
if (dontCheck || _blockTimestamp() - lastTimestamp >= Constants.FEE_TRANSFER_FREQUENCY) {
uint256 feePending0;
uint256 feePending1;
assembly {
Expand Down Expand Up @@ -193,7 +195,8 @@ abstract contract ReservesManager is AlgebraPoolBase {
communityFee1,
lastTimestamp,
feeRecipientSlot,
feePendingSlot
feePendingSlot,
false
);
if (feeSent0 | feeSent1 != 0) {
// sent fees so decrease deltas
Expand All @@ -214,7 +217,8 @@ abstract contract ReservesManager is AlgebraPoolBase {
pluginFee1,
lastTimestamp,
feeRecipientSlot,
feePendingSlot
feePendingSlot,
feeSent
);
if (feeSent0 | feeSent1 != 0) {
// sent fees so decrease deltas
Expand Down
36 changes: 36 additions & 0 deletions src/core/test/AlgebraPool.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2528,6 +2528,42 @@ describe('AlgebraPool', () => {
expect(pluginFees[0]).to.be.eq(4n * 10n**15n);
})

it.only('sends pluginFee when communityFee overflows', async () => {
await poolPlugin.setPluginFees(100000, 1);
await pool.setCommunityFee(800);
await swapExact0For1(expandTo18Decimals(1), wallet.address);
// after such swap community fee is going to overflow uint104, but pluginFee is not
await swapExact0For1(10n**35n, wallet.address);
const communityFees = await pool.getCommunityFeePending();
const pluginFees = await pool.getPluginFeePending();

// expected to transfer both fees anyway
expect(communityFees[0]).to.be.eq(0);
expect(pluginFees[0]).to.be.eq(0);
})

it.only('sends pluginFee when communityFee overflows but plugin fee is zero', async () => {
await poolPlugin.setPluginFees(100000, 100);
await pool.setCommunityFee(800);
await swapExact0For1(expandTo18Decimals(1), wallet.address);
await swapExact0For1(expandTo18Decimals(1), wallet.address);

let communityFees = await pool.getCommunityFeePending();
let pluginFees = await pool.getPluginFeePending();
expect(communityFees[0]).to.be.gt(0);
expect(pluginFees[0]).to.be.gt(0);

await poolPlugin.setPluginFees(100000, 0);
// after such swap community fee is going to overflow uint104, but pluginFee is not
await swapExact0For1(10n**35n, wallet.address);

communityFees = await pool.getCommunityFeePending();
pluginFees = await pool.getPluginFeePending();
// expected to transfer both fees anyway
expect(communityFees[0]).to.be.eq(0);
expect(pluginFees[0]).to.be.eq(0);
})

})

describe('PermissionedActions', async () => {
Expand Down
70 changes: 35 additions & 35 deletions src/core/test/__snapshots__/AlgebraPool.gas.spec.ts.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,11 +92,11 @@ exports[`AlgebraPool gas tests [ @skip-on-coverage ] fee is off #swapExact1For0

exports[`AlgebraPool gas tests [ @skip-on-coverage ] fee is off #swapExact1For0 second swap in block with no tick movement 1`] = `103369`;

exports[`AlgebraPool gas tests [ @skip-on-coverage ] fee is off #swapExact1For0 with plugin fee on first swap in block moves tick, no initialized crossings, with transfer 1`] = `157375`;
exports[`AlgebraPool gas tests [ @skip-on-coverage ] fee is off #swapExact1For0 with plugin fee on first swap in block moves tick, no initialized crossings, with transfer 1`] = `157435`;

exports[`AlgebraPool gas tests [ @skip-on-coverage ] fee is off #swapExact1For0 with plugin fee on first swap in block with no tick movement, with transfer 1`] = `157323`;
exports[`AlgebraPool gas tests [ @skip-on-coverage ] fee is off #swapExact1For0 with plugin fee on first swap in block with no tick movement, with transfer 1`] = `157383`;

exports[`AlgebraPool gas tests [ @skip-on-coverage ] fee is off #swapExact1For0 with plugin fee on first swap in block with no tick movement, without transfer 1`] = `131885`;
exports[`AlgebraPool gas tests [ @skip-on-coverage ] fee is off #swapExact1For0 with plugin fee on first swap in block with no tick movement, without transfer 1`] = `131945`;

exports[`AlgebraPool gas tests [ @skip-on-coverage ] fee is on #burn above current price burn entire position after some time passes 1`] = `117084`;

Expand Down Expand Up @@ -126,68 +126,68 @@ exports[`AlgebraPool gas tests [ @skip-on-coverage ] fee is on #collect close to

exports[`AlgebraPool gas tests [ @skip-on-coverage ] fee is on #collect close to worst case, two tokens 1`] = `70408`;

exports[`AlgebraPool gas tests [ @skip-on-coverage ] fee is on #mint above current price add to position after some time passes 1`] = `128577`;
exports[`AlgebraPool gas tests [ @skip-on-coverage ] fee is on #mint above current price add to position after some time passes 1`] = `128565`;

exports[`AlgebraPool gas tests [ @skip-on-coverage ] fee is on #mint above current price add to position existing 1`] = `128577`;
exports[`AlgebraPool gas tests [ @skip-on-coverage ] fee is on #mint above current price add to position existing 1`] = `128565`;

exports[`AlgebraPool gas tests [ @skip-on-coverage ] fee is on #mint above current price new position mint first in range 1`] = `282440`;
exports[`AlgebraPool gas tests [ @skip-on-coverage ] fee is on #mint above current price new position mint first in range 1`] = `282428`;

exports[`AlgebraPool gas tests [ @skip-on-coverage ] fee is on #mint above current price second position in same range 1`] = `145677`;
exports[`AlgebraPool gas tests [ @skip-on-coverage ] fee is on #mint above current price second position in same range 1`] = `145665`;

exports[`AlgebraPool gas tests [ @skip-on-coverage ] fee is on #mint around current price add to position after some time passes 1`] = `153730`;
exports[`AlgebraPool gas tests [ @skip-on-coverage ] fee is on #mint around current price add to position after some time passes 1`] = `153718`;

exports[`AlgebraPool gas tests [ @skip-on-coverage ] fee is on #mint around current price add to position existing 1`] = `153730`;
exports[`AlgebraPool gas tests [ @skip-on-coverage ] fee is on #mint around current price add to position existing 1`] = `153718`;

exports[`AlgebraPool gas tests [ @skip-on-coverage ] fee is on #mint around current price new position mint first in range 1`] = `360638`;
exports[`AlgebraPool gas tests [ @skip-on-coverage ] fee is on #mint around current price new position mint first in range 1`] = `360626`;

exports[`AlgebraPool gas tests [ @skip-on-coverage ] fee is on #mint around current price second position in same range 1`] = `170830`;
exports[`AlgebraPool gas tests [ @skip-on-coverage ] fee is on #mint around current price second position in same range 1`] = `170818`;

exports[`AlgebraPool gas tests [ @skip-on-coverage ] fee is on #mint below current price add to position after some time passes 1`] = `129145`;
exports[`AlgebraPool gas tests [ @skip-on-coverage ] fee is on #mint below current price add to position after some time passes 1`] = `129133`;

exports[`AlgebraPool gas tests [ @skip-on-coverage ] fee is on #mint below current price add to position existing 1`] = `129145`;
exports[`AlgebraPool gas tests [ @skip-on-coverage ] fee is on #mint below current price add to position existing 1`] = `129133`;

exports[`AlgebraPool gas tests [ @skip-on-coverage ] fee is on #mint below current price new position mint first in range 1`] = `356785`;
exports[`AlgebraPool gas tests [ @skip-on-coverage ] fee is on #mint below current price new position mint first in range 1`] = `356773`;

exports[`AlgebraPool gas tests [ @skip-on-coverage ] fee is on #mint below current price second position in same range 1`] = `146245`;
exports[`AlgebraPool gas tests [ @skip-on-coverage ] fee is on #mint below current price second position in same range 1`] = `146233`;

exports[`AlgebraPool gas tests [ @skip-on-coverage ] fee is on #poke best case 1`] = `63735`;

exports[`AlgebraPool gas tests [ @skip-on-coverage ] fee is on #swapExact0For1 first swap in block moves tick, no initialized crossings 1`] = `112143`;
exports[`AlgebraPool gas tests [ @skip-on-coverage ] fee is on #swapExact0For1 first swap in block moves tick, no initialized crossings 1`] = `112191`;

exports[`AlgebraPool gas tests [ @skip-on-coverage ] fee is on #swapExact0For1 first swap in block with no tick movement 1`] = `112112`;
exports[`AlgebraPool gas tests [ @skip-on-coverage ] fee is on #swapExact0For1 first swap in block with no tick movement 1`] = `112160`;

exports[`AlgebraPool gas tests [ @skip-on-coverage ] fee is on #swapExact0For1 first swap in block, large swap crossing a single initialized tick 1`] = `128125`;
exports[`AlgebraPool gas tests [ @skip-on-coverage ] fee is on #swapExact0For1 first swap in block, large swap crossing a single initialized tick 1`] = `128173`;

exports[`AlgebraPool gas tests [ @skip-on-coverage ] fee is on #swapExact0For1 first swap in block, large swap crossing several initialized ticks 1`] = `162350`;
exports[`AlgebraPool gas tests [ @skip-on-coverage ] fee is on #swapExact0For1 first swap in block, large swap crossing several initialized ticks 1`] = `162398`;

exports[`AlgebraPool gas tests [ @skip-on-coverage ] fee is on #swapExact0For1 first swap in block, large swap, no initialized crossings 1`] = `112281`;
exports[`AlgebraPool gas tests [ @skip-on-coverage ] fee is on #swapExact0For1 first swap in block, large swap, no initialized crossings 1`] = `112329`;

exports[`AlgebraPool gas tests [ @skip-on-coverage ] fee is on #swapExact0For1 large swap crossing several initialized ticks after some time passes 1`] = `162350`;
exports[`AlgebraPool gas tests [ @skip-on-coverage ] fee is on #swapExact0For1 large swap crossing several initialized ticks after some time passes 1`] = `162398`;

exports[`AlgebraPool gas tests [ @skip-on-coverage ] fee is on #swapExact0For1 large swap crossing several initialized ticks second time after some time passes 1`] = `181550`;
exports[`AlgebraPool gas tests [ @skip-on-coverage ] fee is on #swapExact0For1 large swap crossing several initialized ticks second time after some time passes 1`] = `181598`;

exports[`AlgebraPool gas tests [ @skip-on-coverage ] fee is on #swapExact0For1 second swap in block moves tick, no initialized crossings 1`] = `112143`;
exports[`AlgebraPool gas tests [ @skip-on-coverage ] fee is on #swapExact0For1 second swap in block moves tick, no initialized crossings 1`] = `112191`;

exports[`AlgebraPool gas tests [ @skip-on-coverage ] fee is on #swapExact0For1 second swap in block with no tick movement 1`] = `103545`;
exports[`AlgebraPool gas tests [ @skip-on-coverage ] fee is on #swapExact0For1 second swap in block with no tick movement 1`] = `103533`;

exports[`AlgebraPool gas tests [ @skip-on-coverage ] fee is on #swapExact0For1 second swap in block, large swap crossing a single initialized tick 1`] = `128949`;
exports[`AlgebraPool gas tests [ @skip-on-coverage ] fee is on #swapExact0For1 second swap in block, large swap crossing a single initialized tick 1`] = `128997`;

exports[`AlgebraPool gas tests [ @skip-on-coverage ] fee is on #swapExact0For1 second swap in block, large swap crossing several initialized ticks 1`] = `163201`;
exports[`AlgebraPool gas tests [ @skip-on-coverage ] fee is on #swapExact0For1 second swap in block, large swap crossing several initialized ticks 1`] = `163249`;

exports[`AlgebraPool gas tests [ @skip-on-coverage ] fee is on #swapExact0For1 several large swaps with pauses 1`] = `181550`;
exports[`AlgebraPool gas tests [ @skip-on-coverage ] fee is on #swapExact0For1 several large swaps with pauses 1`] = `181598`;

exports[`AlgebraPool gas tests [ @skip-on-coverage ] fee is on #swapExact0For1 small swap after several large swaps with pauses 1`] = `103419`;
exports[`AlgebraPool gas tests [ @skip-on-coverage ] fee is on #swapExact0For1 small swap after several large swaps with pauses 1`] = `103407`;

exports[`AlgebraPool gas tests [ @skip-on-coverage ] fee is on #swapExact0For1 small swap with filled dataStorage 1`] = `103415`;
exports[`AlgebraPool gas tests [ @skip-on-coverage ] fee is on #swapExact0For1 small swap with filled dataStorage 1`] = `103403`;

exports[`AlgebraPool gas tests [ @skip-on-coverage ] fee is on #swapExact1For0 first swap in block moves tick, no initialized crossings 1`] = `112215`;
exports[`AlgebraPool gas tests [ @skip-on-coverage ] fee is on #swapExact1For0 first swap in block moves tick, no initialized crossings 1`] = `112263`;

exports[`AlgebraPool gas tests [ @skip-on-coverage ] fee is on #swapExact1For0 first swap in block with no tick movement 1`] = `112163`;
exports[`AlgebraPool gas tests [ @skip-on-coverage ] fee is on #swapExact1For0 first swap in block with no tick movement 1`] = `112211`;

exports[`AlgebraPool gas tests [ @skip-on-coverage ] fee is on #swapExact1For0 second swap in block with no tick movement 1`] = `103606`;
exports[`AlgebraPool gas tests [ @skip-on-coverage ] fee is on #swapExact1For0 second swap in block with no tick movement 1`] = `103594`;

exports[`AlgebraPool gas tests [ @skip-on-coverage ] fee is on #swapExact1For0 with plugin fee on first swap in block moves tick, no initialized crossings, with transfer 1`] = `168690`;
exports[`AlgebraPool gas tests [ @skip-on-coverage ] fee is on #swapExact1For0 with plugin fee on first swap in block moves tick, no initialized crossings, with transfer 1`] = `168519`;

exports[`AlgebraPool gas tests [ @skip-on-coverage ] fee is on #swapExact1For0 with plugin fee on first swap in block with no tick movement, with transfer 1`] = `168638`;
exports[`AlgebraPool gas tests [ @skip-on-coverage ] fee is on #swapExact1For0 with plugin fee on first swap in block with no tick movement, with transfer 1`] = `168467`;

exports[`AlgebraPool gas tests [ @skip-on-coverage ] fee is on #swapExact1For0 with plugin fee on first swap in block with no tick movement, without transfer 1`] = `135590`;
exports[`AlgebraPool gas tests [ @skip-on-coverage ] fee is on #swapExact1For0 with plugin fee on first swap in block with no tick movement, without transfer 1`] = `135632`;
2 changes: 1 addition & 1 deletion src/periphery/contracts/libraries/PoolAddress.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ pragma solidity >=0.5.0;
/// @dev Credit to Uniswap Labs under GPL-2.0-or-later license:
/// https://github.com/Uniswap/v3-periphery
library PoolAddress {
bytes32 internal constant POOL_INIT_CODE_HASH = 0x3093a65c28d1e6def42997fdea76d033dfd42b432c107e19412cf91a1eac0f91;
bytes32 internal constant POOL_INIT_CODE_HASH = 0xc5ee3168bec63cbec40c1187858f314e9c2a303e86e16005213d885cc87d4305;

/// @notice The identifying key of the pool
struct PoolKey {
Expand Down

0 comments on commit 86cf458

Please sign in to comment.