diff --git a/.openzeppelin/mainnet.json b/.openzeppelin/mainnet.json index c28af436..cc7fdcec 100644 --- a/.openzeppelin/mainnet.json +++ b/.openzeppelin/mainnet.json @@ -17043,6 +17043,277 @@ }, "namespaces": {} } + }, + "9478f7e8951b88b640cba0f00a08f84c68202692afe06542a423f300ccb05eac": { + "address": "0x2FE5a394F24B3deC644F982A9BB0Bafe57308597", + "txHash": "0x15f5190397cbcb8f44a893b32e453d553da26d86808348aa37df90c752558cfd", + "layout": { + "solcVersion": "0.8.22", + "storage": [ + { + "label": "_initialized", + "offset": 0, + "slot": "0", + "type": "t_uint8", + "contract": "Initializable", + "src": "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol:63", + "retypedFrom": "bool" + }, + { + "label": "_initializing", + "offset": 1, + "slot": "0", + "type": "t_bool", + "contract": "Initializable", + "src": "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol:68" + }, + { + "label": "__gap", + "offset": 0, + "slot": "1", + "type": "t_array(t_uint256)50_storage", + "contract": "ContextUpgradeable", + "src": "@openzeppelin/contracts-upgradeable/utils/ContextUpgradeable.sol:40" + }, + { + "label": "_balances", + "offset": 0, + "slot": "51", + "type": "t_mapping(t_address,t_uint256)", + "contract": "ERC20Upgradeable", + "src": "@openzeppelin/contracts-upgradeable/token/ERC20/ERC20Upgradeable.sol:40" + }, + { + "label": "_allowances", + "offset": 0, + "slot": "52", + "type": "t_mapping(t_address,t_mapping(t_address,t_uint256))", + "contract": "ERC20Upgradeable", + "src": "@openzeppelin/contracts-upgradeable/token/ERC20/ERC20Upgradeable.sol:42" + }, + { + "label": "_totalSupply", + "offset": 0, + "slot": "53", + "type": "t_uint256", + "contract": "ERC20Upgradeable", + "src": "@openzeppelin/contracts-upgradeable/token/ERC20/ERC20Upgradeable.sol:44" + }, + { + "label": "_name", + "offset": 0, + "slot": "54", + "type": "t_string_storage", + "contract": "ERC20Upgradeable", + "src": "@openzeppelin/contracts-upgradeable/token/ERC20/ERC20Upgradeable.sol:46" + }, + { + "label": "_symbol", + "offset": 0, + "slot": "55", + "type": "t_string_storage", + "contract": "ERC20Upgradeable", + "src": "@openzeppelin/contracts-upgradeable/token/ERC20/ERC20Upgradeable.sol:47" + }, + { + "label": "__gap", + "offset": 0, + "slot": "56", + "type": "t_array(t_uint256)45_storage", + "contract": "ERC20Upgradeable", + "src": "@openzeppelin/contracts-upgradeable/token/ERC20/ERC20Upgradeable.sol:376" + }, + { + "label": "__gap", + "offset": 0, + "slot": "101", + "type": "t_array(t_uint256)50_storage", + "contract": "ERC1967UpgradeUpgradeable", + "src": "@openzeppelin/contracts-upgradeable/proxy/ERC1967/ERC1967UpgradeUpgradeable.sol:169" + }, + { + "label": "__gap", + "offset": 0, + "slot": "151", + "type": "t_array(t_uint256)50_storage", + "contract": "UUPSUpgradeable", + "src": "@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol:111" + }, + { + "label": "_owner", + "offset": 0, + "slot": "201", + "type": "t_address", + "contract": "OwnableUpgradeable", + "src": "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol:22" + }, + { + "label": "__gap", + "offset": 0, + "slot": "202", + "type": "t_array(t_uint256)49_storage", + "contract": "OwnableUpgradeable", + "src": "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol:94" + }, + { + "label": "token", + "offset": 0, + "slot": "251", + "type": "t_contract(IERC20Upgradeable)7682", + "contract": "StakingRewardsPool", + "src": "contracts/core/base/StakingRewardsPool.sol:19" + }, + { + "label": "shares", + "offset": 0, + "slot": "252", + "type": "t_mapping(t_address,t_uint256)", + "contract": "StakingRewardsPool", + "src": "contracts/core/base/StakingRewardsPool.sol:22" + }, + { + "label": "totalShares", + "offset": 0, + "slot": "253", + "type": "t_uint256", + "contract": "StakingRewardsPool", + "src": "contracts/core/base/StakingRewardsPool.sol:24" + }, + { + "label": "strategies", + "offset": 0, + "slot": "254", + "type": "t_array(t_address)dyn_storage", + "contract": "StakingPool", + "src": "contracts/core/StakingPool.sol:25" + }, + { + "label": "totalStaked", + "offset": 0, + "slot": "255", + "type": "t_uint256", + "contract": "StakingPool", + "src": "contracts/core/StakingPool.sol:27" + }, + { + "label": "unusedDepositLimit", + "offset": 0, + "slot": "256", + "type": "t_uint256", + "contract": "StakingPool", + "src": "contracts/core/StakingPool.sol:29" + }, + { + "label": "fees", + "offset": 0, + "slot": "257", + "type": "t_array(t_struct(Fee)19198_storage)dyn_storage", + "contract": "StakingPool", + "src": "contracts/core/StakingPool.sol:32" + }, + { + "label": "priorityPool", + "offset": 0, + "slot": "258", + "type": "t_address", + "contract": "StakingPool", + "src": "contracts/core/StakingPool.sol:35" + }, + { + "label": "rebaseController", + "offset": 0, + "slot": "259", + "type": "t_address", + "contract": "StakingPool", + "src": "contracts/core/StakingPool.sol:37" + }, + { + "label": "poolIndex", + "offset": 20, + "slot": "259", + "type": "t_uint16", + "contract": "StakingPool", + "src": "contracts/core/StakingPool.sol:38" + } + ], + "types": { + "t_address": { + "label": "address", + "numberOfBytes": "20" + }, + "t_array(t_address)dyn_storage": { + "label": "address[]", + "numberOfBytes": "32" + }, + "t_array(t_struct(Fee)19198_storage)dyn_storage": { + "label": "struct StakingPool.Fee[]", + "numberOfBytes": "32" + }, + "t_array(t_uint256)45_storage": { + "label": "uint256[45]", + "numberOfBytes": "1440" + }, + "t_array(t_uint256)49_storage": { + "label": "uint256[49]", + "numberOfBytes": "1568" + }, + "t_array(t_uint256)50_storage": { + "label": "uint256[50]", + "numberOfBytes": "1600" + }, + "t_bool": { + "label": "bool", + "numberOfBytes": "1" + }, + "t_contract(IERC20Upgradeable)7682": { + "label": "contract IERC20Upgradeable", + "numberOfBytes": "20" + }, + "t_mapping(t_address,t_mapping(t_address,t_uint256))": { + "label": "mapping(address => mapping(address => uint256))", + "numberOfBytes": "32" + }, + "t_mapping(t_address,t_uint256)": { + "label": "mapping(address => uint256)", + "numberOfBytes": "32" + }, + "t_string_storage": { + "label": "string", + "numberOfBytes": "32" + }, + "t_struct(Fee)19198_storage": { + "label": "struct StakingPool.Fee", + "members": [ + { + "label": "receiver", + "type": "t_address", + "offset": 0, + "slot": "0" + }, + { + "label": "basisPoints", + "type": "t_uint256", + "offset": 0, + "slot": "1" + } + ], + "numberOfBytes": "64" + }, + "t_uint16": { + "label": "uint16", + "numberOfBytes": "2" + }, + "t_uint256": { + "label": "uint256", + "numberOfBytes": "32" + }, + "t_uint8": { + "label": "uint8", + "numberOfBytes": "1" + } + }, + "namespaces": {} + } } } } diff --git a/contracts/core/base/StakingRewardsPool.sol b/contracts/core/base/StakingRewardsPool.sol index b7be0520..8ec53ad3 100644 --- a/contracts/core/base/StakingRewardsPool.sol +++ b/contracts/core/base/StakingRewardsPool.sol @@ -142,6 +142,7 @@ abstract contract StakingRewardsPool is ERC677Upgradeable, UUPSUpgradeable, Owna require(_sender != address(0), "Transfer from the zero address"); require(_recipient != address(0), "Transfer to the zero address"); + require(sharesToTransfer != 0, "Transfer amount too small"); require(shares[_sender] >= sharesToTransfer, "Transfer amount exceeds balance"); shares[_sender] -= sharesToTransfer; @@ -208,6 +209,7 @@ abstract contract StakingRewardsPool is ERC677Upgradeable, UUPSUpgradeable, Owna uint256 sharesToBurn = getSharesByStake(_amount); require(_account != address(0), "Burn from the zero address"); + require(sharesToBurn != 0, "Burn amount too small"); require(shares[_account] >= sharesToBurn, "Burn amount exceeds balance"); totalShares -= sharesToBurn; diff --git a/test/core/priorityPool/priority-pool.test.ts b/test/core/priorityPool/priority-pool.test.ts index 243c2a87..94d2ebb6 100644 --- a/test/core/priorityPool/priority-pool.test.ts +++ b/test/core/priorityPool/priority-pool.test.ts @@ -855,4 +855,47 @@ describe('PriorityPool', () => { assert.equal(fromEther(await stakingPool.balanceOf(accounts[2])), 3000) assert.equal(fromEther(await stakingPool.totalStaked()), 3000) }) + + it('withdraw should revert when transfer amount rounds to zero shares', async () => { + const { signers, accounts, adrs, pp, token, stakingPool, strategy } = await loadFixture( + deployFixture + ) + + await stakingPool.connect(signers[1]).approve(adrs.pp, ethers.MaxUint256) + + // deposit and accrue rewards so totalStaked > totalShares + await pp.connect(signers[1]).deposit(toEther(1000), false, ['0x']) + await token.transfer(adrs.strategy, toEther(100)) + await stakingPool.updateStrategyRewards([0], '0x') + + assert.equal(await stakingPool.getSharesByStake(1), 0n) + + // queue tokens so totalQueued > 0 + await strategy.setMaxDeposits(toEther(1100)) + await pp.connect(signers[2]).deposit(toEther(500), true, ['0x']) + assert.notEqual(await pp.totalQueued(), 0n) + + const totalQueuedBefore = await pp.totalQueued() + const attackerSharesBefore = await stakingPool.sharesOf(accounts[1]) + const attackerLINKBefore = await token.balanceOf(accounts[1]) + + // attempt to withdraw 1 wei (rounds to 0 shares) - should revert + await expect( + pp.connect(signers[1]).withdraw(1, 0, 0, [], false, false, ['0x']) + ).to.be.revertedWith('Transfer amount too small') + + // verify nothing changed + assert.equal(await pp.totalQueued(), totalQueuedBefore) + assert.equal(await stakingPool.sharesOf(accounts[1]), attackerSharesBefore) + assert.equal(await token.balanceOf(accounts[1]), attackerLINKBefore) + + // attempt ERC677 withdraw path with 1 wei - should also revert + await expect( + stakingPool.transferAndCall( + adrs.pp, + 1, + ethers.AbiCoder.defaultAbiCoder().encode(['bool', 'bytes[]'], [false, ['0x']]) + ) + ).to.be.revertedWith('Transfer amount too small') + }) }) diff --git a/test/core/staking-pool.test.ts b/test/core/staking-pool.test.ts index 184ceeba..d2ceaeb6 100644 --- a/test/core/staking-pool.test.ts +++ b/test/core/staking-pool.test.ts @@ -802,4 +802,45 @@ describe('StakingPool', () => { assert.equal(fromEther(await stakingPool.balanceOf(accounts[2])), 3375) assert.equal(fromEther(await stakingPool.totalStaked()), 4500) }) + + it('should revert transfer and burn when amount rounds to zero shares', async () => { + const { signers, accounts, adrs, stakingPool, token, stake } = await loadFixture(deployFixture) + + await stake(1, 1000) + + // accrue rewards so totalStaked > totalShares + await token.transfer(adrs.strategy1, toEther(500)) + await stakingPool.updateStrategyRewards([0], '0x') + + assert.equal(await stakingPool.getSharesByStake(1), 0n) + + await expect(stakingPool.connect(signers[1]).transfer(accounts[2], 1)).to.be.revertedWith( + 'Transfer amount too small' + ) + await expect(stakingPool.connect(signers[1]).burn(1)).to.be.revertedWith( + 'Burn amount too small' + ) + + // find the max amount that still rounds to zero shares + const totalStaked = await stakingPool.totalStaked() + const totalShares = await stakingPool.totalShares() + const maxZeroShareAmount = totalStaked / totalShares // integer division + + // every amount from 1 to maxZeroShareAmount should revert on transfer + for (let i = 1n; i <= maxZeroShareAmount && i <= 5n; i++) { + assert.equal(await stakingPool.getSharesByStake(i), 0n) + await expect(stakingPool.connect(signers[1]).transfer(accounts[2], i)).to.be.revertedWith( + 'Transfer amount too small' + ) + } + + // amount just above the threshold should succeed and move shares + const firstValidAmount = maxZeroShareAmount + 1n + assert.notEqual(await stakingPool.getSharesByStake(firstValidAmount), 0n) + + const sharesBefore = await stakingPool.sharesOf(accounts[1]) + await stakingPool.connect(signers[1]).transfer(accounts[2], firstValidAmount) + const sharesAfter = await stakingPool.sharesOf(accounts[1]) + assert.notEqual(sharesBefore, sharesAfter, 'shares must change for valid transfer') + }) })