diff --git a/protocol-contracts/staking/contracts/OperatorStaking.sol b/protocol-contracts/staking/contracts/OperatorStaking.sol index 2f047261d3..762b66d77a 100644 --- a/protocol-contracts/staking/contracts/OperatorStaking.sol +++ b/protocol-contracts/staking/contracts/OperatorStaking.sol @@ -444,8 +444,12 @@ contract OperatorStaking is ERC1363Upgradeable, ReentrancyGuardTransient, UUPSUp ); } + /** + * @dev Returns a decimal offset of 2 to mitigate the ERC4626 inflation attack. + * This creates 100 virtual shares per asset unit, making the attack economically unfeasible. + */ function _decimalsOffset() internal view virtual returns (uint8) { - return 0; + return 2; } function _getOperatorStakingStorage() private pure returns (OperatorStakingStorage storage $) { diff --git a/protocol-contracts/staking/test/OperatorRewarder.test.ts b/protocol-contracts/staking/test/OperatorRewarder.test.ts index 0f398f1cc0..9b70489002 100644 --- a/protocol-contracts/staking/test/OperatorRewarder.test.ts +++ b/protocol-contracts/staking/test/OperatorRewarder.test.ts @@ -7,6 +7,11 @@ import hre from 'hardhat'; const timeIncreaseNoMine = (duration: number) => time.latest().then(clock => time.setNextBlockTimestamp(clock + duration)); +// DECIMAL_OFFSET is used in OperatorStaking to mitigate inflation attacks. +// This creates 10^DECIMAL_OFFSET virtual shares per asset unit. +const DECIMAL_OFFSET = 2n; +const VIRTUAL_SHARES = 10n ** DECIMAL_OFFSET; + describe('OperatorRewarder', function () { beforeEach(async function () { const [delegator1, delegator2, claimer, admin, beneficiary, anyone, ...accounts] = await ethers.getSigners(); @@ -156,7 +161,8 @@ describe('OperatorRewarder', function () { await timeIncreaseNoMine(10); await this.protocolStaking.connect(this.admin).setRewardRate(0); await this.mock.connect(this.delegator1).claimRewards(this.delegator1); // claims past rewards before not being able to - await this.operatorStaking.connect(this.delegator1).transfer(this.delegator2, ethers.parseEther('1')); + const sharesToTransfer = ethers.parseEther('1') * VIRTUAL_SHARES; + await this.operatorStaking.connect(this.delegator1).transfer(this.delegator2, sharesToTransfer); // delegator1 will be able deposit and claim reward again await expect(this.mock.earned(this.delegator1)).to.eventually.eq(0); // delegator2 cannot claim any reward @@ -214,20 +220,17 @@ describe('OperatorRewarder', function () { await this.mock.connect(this.delegator1).claimRewards(this.delegator1); await this.mock.connect(this.delegator2).claimRewards(this.delegator2); + const sharesToRedeem = ethers.parseEther('1') * VIRTUAL_SHARES; await this.operatorStaking .connect(this.delegator1) - .requestRedeem(ethers.parseEther('1'), this.delegator1, this.delegator1); + .requestRedeem(sharesToRedeem, this.delegator1, this.delegator1); await this.operatorStaking .connect(this.delegator2) - .requestRedeem(ethers.parseEther('1'), this.delegator2, this.delegator2); + .requestRedeem(sharesToRedeem, this.delegator2, this.delegator2); await timeIncreaseNoMine(60); - await this.operatorStaking - .connect(this.delegator1) - .redeem(ethers.parseEther('1'), this.delegator1, this.delegator1); - await this.operatorStaking - .connect(this.delegator2) - .redeem(ethers.parseEther('1'), this.delegator2, this.delegator2); + await this.operatorStaking.connect(this.delegator1).redeem(sharesToRedeem, this.delegator1, this.delegator1); + await this.operatorStaking.connect(this.delegator2).redeem(sharesToRedeem, this.delegator2, this.delegator2); await this.operatorStaking.connect(this.delegator1).deposit(ethers.parseEther('1'), this.delegator1); await expect(this.mock.earned(this.delegator1)).to.eventually.eq(0); @@ -252,9 +255,10 @@ describe('OperatorRewarder', function () { await timeIncreaseNoMine(10); + const sharesToRedeem = ethers.parseEther('2') * VIRTUAL_SHARES; await this.operatorStaking .connect(this.delegator1) - .requestRedeem(ethers.parseEther('2'), this.delegator1, this.delegator1); + .requestRedeem(sharesToRedeem, this.delegator1, this.delegator1); await time.increase(10); @@ -576,7 +580,8 @@ describe('OperatorRewarder', function () { await this.operatorStaking.connect(this.delegator1).deposit(ethers.parseEther('1'), this.delegator1); await timeIncreaseNoMine(10); - await this.operatorStaking.connect(this.delegator1).transfer(this.delegator2, ethers.parseEther('1')); + const sharesToTransfer = ethers.parseEther('1') * VIRTUAL_SHARES; + await this.operatorStaking.connect(this.delegator1).transfer(this.delegator2, sharesToTransfer); await time.increase(10); await expect(this.mock.earned(this.delegator1)).to.eventually.eq(ethers.parseEther('5')); @@ -587,7 +592,8 @@ describe('OperatorRewarder', function () { await this.operatorStaking.connect(this.delegator1).deposit(ethers.parseEther('1'), this.delegator1); await timeIncreaseNoMine(10); - await this.operatorStaking.connect(this.delegator1).transfer(this.delegator2, ethers.parseEther('0.5')); + const sharesToTransfer = ethers.parseEther('0.5') * VIRTUAL_SHARES; + await this.operatorStaking.connect(this.delegator1).transfer(this.delegator2, sharesToTransfer); await time.increase(10); await expect(this.mock.earned(this.delegator1)).to.eventually.eq(ethers.parseEther('7.5')); diff --git a/protocol-contracts/staking/test/OperatorStaking.test.ts b/protocol-contracts/staking/test/OperatorStaking.test.ts index 086f48f0d7..d8a39719e5 100644 --- a/protocol-contracts/staking/test/OperatorStaking.test.ts +++ b/protocol-contracts/staking/test/OperatorStaking.test.ts @@ -6,6 +6,11 @@ import { ethers, upgrades } from 'hardhat'; const timeIncreaseNoMine = (duration: number) => time.latest().then(clock => time.setNextBlockTimestamp(clock + duration)); +// DECIMAL_OFFSET is used in OperatorStaking to mitigate inflation attacks. +// This creates 10^DECIMAL_OFFSET virtual shares per asset unit. +const DECIMAL_OFFSET = 2n; +const VIRTUAL_SHARES = 10n ** DECIMAL_OFFSET; + describe('OperatorStaking', function () { beforeEach(async function () { const [delegator1, delegator2, admin, beneficiary, anyone, ...accounts] = await ethers.getSigners(); @@ -88,9 +93,10 @@ describe('OperatorStaking', function () { }); it('should mint shares', async function () { + // First deposit gets assets * VIRTUAL_SHARES shares due to decimal offset await expect(this.mock.connect(this.delegator1).deposit(ethers.parseEther('1'), this.delegator1)) .to.emit(this.mock, 'Transfer') - .withArgs(ethers.ZeroAddress, this.delegator1, ethers.parseEther('1')); + .withArgs(ethers.ZeroAddress, this.delegator1, ethers.parseEther('1') * VIRTUAL_SHARES); }); it('should pull tokens', async function () { @@ -179,9 +185,10 @@ describe('OperatorStaking', function () { }); it('should mint shares with permit', async function () { + // First deposit gets assets * VIRTUAL_SHARES shares due to decimal offset await expect(this.depositWithPermitTx) .to.emit(this.mock, 'Transfer') - .withArgs(ethers.ZeroAddress, this.delegatorNoApproval, this.permitValue); + .withArgs(ethers.ZeroAddress, this.delegatorNoApproval, this.permitValue * VIRTUAL_SHARES); }); it('should pull tokens with permit', async function () { @@ -210,19 +217,18 @@ describe('OperatorStaking', function () { describe('redeem', async function () { it('simple redemption', async function () { await this.mock.connect(this.delegator1).deposit(ethers.parseEther('1'), this.delegator1); - await this.mock - .connect(this.delegator1) - .requestRedeem(await this.mock.balanceOf(this.delegator1), this.delegator1, this.delegator1); + const shares = await this.mock.balanceOf(this.delegator1); + await this.mock.connect(this.delegator1).requestRedeem(shares, this.delegator1, this.delegator1); - await expect(this.mock.pendingRedeemRequest(0, this.delegator1)).to.eventually.eq(ethers.parseEther('1')); + await expect(this.mock.pendingRedeemRequest(0, this.delegator1)).to.eventually.eq(shares); await expect(this.mock.claimableRedeemRequest(0, this.delegator1)).to.eventually.eq(0); await time.increase(60); await expect(this.mock.pendingRedeemRequest(0, this.delegator1)).to.eventually.eq(0); - await expect(this.mock.claimableRedeemRequest(0, this.delegator1)).to.eventually.eq(ethers.parseEther('1')); + await expect(this.mock.claimableRedeemRequest(0, this.delegator1)).to.eventually.eq(shares); - await expect(this.mock.connect(this.delegator1).redeem(ethers.parseEther('1'), this.delegator1, this.delegator1)) + await expect(this.mock.connect(this.delegator1).redeem(shares, this.delegator1, this.delegator1)) .to.emit(this.token, 'Transfer') .withArgs(this.mock, this.delegator1, ethers.parseEther('1')); await expect(this.token.balanceOf(this.mock)).to.eventually.be.eq(0); @@ -238,8 +244,10 @@ describe('OperatorStaking', function () { it('should not redeem twice', async function () { await this.mock.connect(this.delegator2).deposit(ethers.parseEther('5'), this.delegator2); await this.mock.connect(this.delegator1).deposit(ethers.parseEther('10'), this.delegator1); - await this.mock.connect(this.delegator1).requestRedeem(ethers.parseEther('1'), this.delegator1, this.delegator1); - await this.mock.connect(this.delegator2).requestRedeem(ethers.parseEther('1'), this.delegator2, this.delegator2); + // Request redeem of shares worth 1 ETH of assets + const sharesToRedeem = ethers.parseEther('1') * VIRTUAL_SHARES; + await this.mock.connect(this.delegator1).requestRedeem(sharesToRedeem, this.delegator1, this.delegator1); + await this.mock.connect(this.delegator2).requestRedeem(sharesToRedeem, this.delegator2, this.delegator2); await timeIncreaseNoMine(60); @@ -253,17 +261,19 @@ describe('OperatorStaking', function () { it('should revert on redeem more than available', async function () { await this.mock.connect(this.delegator1).deposit(ethers.parseEther('10'), this.delegator1); - await this.mock.connect(this.delegator1).requestRedeem(ethers.parseEther('1'), this.delegator1, this.delegator1); + const sharesToRedeem = ethers.parseEther('1') * VIRTUAL_SHARES; + await this.mock.connect(this.delegator1).requestRedeem(sharesToRedeem, this.delegator1, this.delegator1); await timeIncreaseNoMine(10); - await expect(this.mock.connect(this.delegator1).redeem(ethers.parseEther('1'), this.delegator1, this.delegator1)) + await expect(this.mock.connect(this.delegator1).redeem(sharesToRedeem, this.delegator1, this.delegator1)) .to.be.revertedWithCustomError(this.mock, 'ERC4626ExceededMaxRedeem') - .withArgs(this.delegator1, ethers.parseEther('1'), 0); + .withArgs(this.delegator1, sharesToRedeem, 0); }); it('should be able to redeem a second time', async function () { await this.mock.connect(this.delegator1).deposit(ethers.parseEther('10'), this.delegator1); - await this.mock.connect(this.delegator1).requestRedeem(ethers.parseEther('1'), this.delegator1, this.delegator1); + const sharesToRedeem1 = ethers.parseEther('1') * VIRTUAL_SHARES; + await this.mock.connect(this.delegator1).requestRedeem(sharesToRedeem1, this.delegator1, this.delegator1); await timeIncreaseNoMine(60); @@ -271,7 +281,8 @@ describe('OperatorStaking', function () { .to.emit(this.token, 'Transfer') .withArgs(this.mock, this.delegator1, ethers.parseEther('1')); - await this.mock.connect(this.delegator1).requestRedeem(ethers.parseEther('2'), this.delegator1, this.delegator1); + const sharesToRedeem2 = ethers.parseEther('2') * VIRTUAL_SHARES; + await this.mock.connect(this.delegator1).requestRedeem(sharesToRedeem2, this.delegator1, this.delegator1); await timeIncreaseNoMine(60); @@ -283,7 +294,8 @@ describe('OperatorStaking', function () { it('via separate controller', async function () { const controller = this.accounts[0]; await this.mock.connect(this.delegator1).deposit(ethers.parseEther('10'), this.delegator1); - await this.mock.connect(this.delegator1).requestRedeem(ethers.parseEther('1'), controller, this.delegator1); + const sharesToRedeem = ethers.parseEther('1') * VIRTUAL_SHARES; + await this.mock.connect(this.delegator1).requestRedeem(sharesToRedeem, controller, this.delegator1); await timeIncreaseNoMine(60); @@ -300,9 +312,10 @@ describe('OperatorStaking', function () { it('should fail if controller is zero address', async function () { await this.mock.connect(this.delegator1).deposit(ethers.parseEther('1'), this.delegator1); + const sharesToRedeem = ethers.parseEther('1') * VIRTUAL_SHARES; await expect( - this.mock.connect(this.delegator1).requestRedeem(ethers.parseEther('1'), ethers.ZeroAddress, this.delegator1), + this.mock.connect(this.delegator1).requestRedeem(sharesToRedeem, ethers.ZeroAddress, this.delegator1), ).to.be.revertedWithCustomError(this.mock, 'InvalidController'); }); @@ -310,17 +323,18 @@ describe('OperatorStaking', function () { const approvedActor = this.accounts[0]; await this.mock.connect(this.delegator1).deposit(ethers.parseEther('1'), this.delegator1); - await this.mock.connect(this.delegator1).approve(approvedActor, ethers.parseEther('1')); + const sharesToRedeem = ethers.parseEther('1') * VIRTUAL_SHARES; + await this.mock.connect(this.delegator1).approve(approvedActor, sharesToRedeem); - await this.mock.connect(approvedActor).requestRedeem(ethers.parseEther('1'), this.delegator1, this.delegator1); + await this.mock.connect(approvedActor).requestRedeem(sharesToRedeem, this.delegator1, this.delegator1); }); it('should fail via unapproved actor', async function () { await this.mock.connect(this.delegator1).deposit(ethers.parseEther('1'), this.delegator1); + const sharesToRedeem = ethers.parseEther('1') * VIRTUAL_SHARES; - await expect( - this.mock.connect(this.accounts[0]).requestRedeem(ethers.parseEther('1'), this.delegator1, this.delegator1), - ).to.be.reverted; + await expect(this.mock.connect(this.accounts[0]).requestRedeem(sharesToRedeem, this.delegator1, this.delegator1)) + .to.be.reverted; }); it('should handle reduction in cooldown period correctly', async function () { @@ -332,13 +346,14 @@ describe('OperatorStaking', function () { await this.mock.connect(this.delegator2).deposit(ethers.parseEther('1'), this.delegator2); await this.mock.connect(delegator3).deposit(ethers.parseEther('1'), delegator3); - await this.mock.connect(this.delegator1).requestRedeem(ethers.parseEther('1'), this.delegator1, this.delegator1); + const sharesToRedeem = ethers.parseEther('1') * VIRTUAL_SHARES; + await this.mock.connect(this.delegator1).requestRedeem(sharesToRedeem, this.delegator1, this.delegator1); await timeIncreaseNoMine(30); - await this.mock.connect(this.delegator2).requestRedeem(ethers.parseEther('1'), this.delegator2, this.delegator2); + await this.mock.connect(this.delegator2).requestRedeem(sharesToRedeem, this.delegator2, this.delegator2); await this.protocolStaking.connect(this.admin).setUnstakeCooldownPeriod(30); - await this.mock.connect(delegator3).requestRedeem(ethers.parseEther('1'), delegator3, delegator3); + await this.mock.connect(delegator3).requestRedeem(sharesToRedeem, delegator3, delegator3); // delegator 3 will need to wait 59 seconds @@ -365,9 +380,8 @@ describe('OperatorStaking', function () { }); it('should be allowed to redeem on behalf of authorized controller', async function () { - await this.mock - .connect(this.delegator1) - .requestRedeem(ethers.parseEther('1'), this.delegator1, this.delegator1); + const sharesToRedeem = ethers.parseEther('1') * VIRTUAL_SHARES; + await this.mock.connect(this.delegator1).requestRedeem(sharesToRedeem, this.delegator1, this.delegator1); await timeIncreaseNoMine(60); @@ -377,9 +391,8 @@ describe('OperatorStaking', function () { }); it('should not be allowed to redeem on behalf of other controller', async function () { - await this.mock - .connect(this.delegator1) - .requestRedeem(ethers.parseEther('1'), this.delegator2, this.delegator1); + const sharesToRedeem = ethers.parseEther('1') * VIRTUAL_SHARES; + await this.mock.connect(this.delegator1).requestRedeem(sharesToRedeem, this.delegator2, this.delegator1); await timeIncreaseNoMine(60); @@ -401,7 +414,8 @@ describe('OperatorStaking', function () { it('should not transfer required tokens', async function () { await this.mock.connect(this.delegator1).deposit(ethers.parseEther('10'), this.delegator1); await this.mock.connect(this.delegator2).deposit(ethers.parseEther('1'), this.delegator2); - await this.mock.connect(this.delegator2).requestRedeem(ethers.parseEther('1'), this.delegator2, this.delegator2); + const sharesToRedeem = ethers.parseEther('1') * VIRTUAL_SHARES; + await this.mock.connect(this.delegator2).requestRedeem(sharesToRedeem, this.delegator2, this.delegator2); // Increase the value of each share by 10% await this.token.connect(this.delegator1).transfer(this.mock, ethers.parseEther('1.1')); @@ -445,9 +459,8 @@ describe('OperatorStaking', function () { await this.mock.connect(this.delegator1).deposit(ethers.parseEther('1'), this.delegator1); await this.mock.connect(this.delegator2).deposit(ethers.parseEther('2'), this.delegator2); - await this.mock - .connect(this.delegator1) - .requestRedeem(ethers.parseEther('0.5'), this.delegator1, this.delegator1); + const sharesToRedeem = ethers.parseEther('0.5') * VIRTUAL_SHARES; + await this.mock.connect(this.delegator1).requestRedeem(sharesToRedeem, this.delegator1, this.delegator1); // 50% slashing await this.protocolStaking.slash(this.mock, ethers.parseEther('1.5')); @@ -462,12 +475,12 @@ describe('OperatorStaking', function () { await this.mock.connect(this.delegator1).deposit(ethers.parseEther('1'), this.delegator1); await this.mock.connect(this.delegator2).deposit(ethers.parseEther('2'), this.delegator2); - await this.mock.connect(this.delegator1).requestRedeem(ethers.parseEther('1'), this.delegator1, this.delegator1); + const sharesToRedeem1 = ethers.parseEther('1') * VIRTUAL_SHARES; + await this.mock.connect(this.delegator1).requestRedeem(sharesToRedeem1, this.delegator1, this.delegator1); await this.protocolStaking.slash(this.mock, ethers.parseEther('1.5')); - await expect( - this.mock.connect(this.delegator2).requestRedeem(ethers.parseEther('2'), this.delegator2, this.delegator2), - ) + const sharesToRedeem2 = ethers.parseEther('2') * VIRTUAL_SHARES; + await expect(this.mock.connect(this.delegator2).requestRedeem(sharesToRedeem2, this.delegator2, this.delegator2)) .to.emit(this.protocolStaking, 'TokensUnstaked') .withArgs(this.mock, ethers.parseEther('0.5'), anyValue); }); @@ -476,31 +489,32 @@ describe('OperatorStaking', function () { await this.mock.connect(this.delegator1).deposit(ethers.parseEther('1'), this.delegator1); await this.mock.connect(this.delegator2).deposit(ethers.parseEther('1'), this.delegator2); - await this.mock.connect(this.delegator1).requestRedeem(ethers.parseEther('1'), this.delegator1, this.delegator1); + const sharesToRedeem = ethers.parseEther('1') * VIRTUAL_SHARES; + await this.mock.connect(this.delegator1).requestRedeem(sharesToRedeem, this.delegator1, this.delegator1); await this.protocolStaking.slash(this.mock, ethers.parseEther('1')); await timeIncreaseNoMine(30); - await expect( - this.mock.connect(this.delegator2).requestRedeem(ethers.parseEther('1'), this.delegator2, this.delegator2), - ) + await expect(this.mock.connect(this.delegator2).requestRedeem(sharesToRedeem, this.delegator2, this.delegator2)) .to.emit(this.protocolStaking, 'TokensUnstaked') .withArgs(this.mock, 0, anyValue); await time.increase(30); await expect(this.mock.maxRedeem(this.delegator2)).to.eventually.eq(0); - await expect(this.mock.maxRedeem(this.delegator1)).to.eventually.eq(ethers.parseEther('1')); + await expect(this.mock.maxRedeem(this.delegator1)).to.eventually.eq(sharesToRedeem); await time.increase(30); - await expect(this.mock.maxRedeem(this.delegator2)).to.eventually.eq(ethers.parseEther('1')); + await expect(this.mock.maxRedeem(this.delegator2)).to.eventually.eq(sharesToRedeem); }); it('symmetrically passes on losses from withdrawal balance', async function () { await this.mock.connect(this.delegator1).deposit(ethers.parseEther('1'), this.delegator1); await this.mock.connect(this.delegator2).deposit(ethers.parseEther('2'), this.delegator2); - await this.mock.connect(this.delegator1).requestRedeem(ethers.parseEther('1'), this.delegator1, this.delegator1); - await this.mock.connect(this.delegator2).requestRedeem(ethers.parseEther('2'), this.delegator2, this.delegator2); + const sharesToRedeem1 = ethers.parseEther('1') * VIRTUAL_SHARES; + const sharesToRedeem2 = ethers.parseEther('2') * VIRTUAL_SHARES; + await this.mock.connect(this.delegator1).requestRedeem(sharesToRedeem1, this.delegator1, this.delegator1); + await this.mock.connect(this.delegator2).requestRedeem(sharesToRedeem2, this.delegator2, this.delegator2); await this.protocolStaking.slashWithdrawal(this.mock, ethers.parseEther('1.5'));