From f90e0e0731808e07293f601a222986b775f5de02 Mon Sep 17 00:00:00 2001 From: Taylor Brent Date: Tue, 10 Sep 2024 09:49:41 -0400 Subject: [PATCH] make future withdrawals unimpacted by previous cancellations (#1194) --- contracts/p0/StRSR.sol | 3 +-- contracts/p1/StRSR.sol | 4 +++- test/ZZStRSR.test.ts | 43 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 47 insertions(+), 3 deletions(-) diff --git a/contracts/p0/StRSR.sol b/contracts/p0/StRSR.sol index 814a4a94e7..4a3c0a1da9 100644 --- a/contracts/p0/StRSR.sol +++ b/contracts/p0/StRSR.sol @@ -272,8 +272,7 @@ contract StRSRP0 is IStRSR, ComponentP0, EIP712Upgradeable { uint256 i = start; for (; i < endId; i++) { total += queue[i].rsrAmount; - queue[i].rsrAmount = 0; - queue[i].stakeAmount = 0; + delete queue[i]; } // Execute accumulated withdrawals diff --git a/contracts/p1/StRSR.sol b/contracts/p1/StRSR.sol index 5a6c001229..6e7b1e6f55 100644 --- a/contracts/p1/StRSR.sol +++ b/contracts/p1/StRSR.sol @@ -654,7 +654,9 @@ abstract contract StRSRP1 is Initializable, ComponentP1, IStRSR, EIP712Upgradeab index = queue.length; uint192 oldDrafts = index != 0 ? queue[index - 1].drafts : 0; - uint64 lastAvailableAt = index != 0 ? queue[index - 1].availableAt : 0; + uint64 lastAvailableAt = index != 0 && firstRemainingDraft[draftEra][account] < index + ? queue[index - 1].availableAt + : 0; availableAt = uint64(block.timestamp) + unstakingDelay; if (lastAvailableAt > availableAt) { availableAt = lastAvailableAt; diff --git a/test/ZZStRSR.test.ts b/test/ZZStRSR.test.ts index 6fbcd2b3d4..06fba12cc2 100644 --- a/test/ZZStRSR.test.ts +++ b/test/ZZStRSR.test.ts @@ -552,6 +552,49 @@ describe(`StRSRP${IMPLEMENTATION} contract`, () => { .withArgs(0, 2, addr1.address, amount.div(2), amount, anyValue) }) + it('Should not impact future withdrawals after cancellation -- regression test 9/04/2024', async () => { + // Context: https://github.com/code-423n4/2024-07-reserve-findings/issues/18 + + // old unstakingDelay is 1 day + const oldUnstakingDelay = 3600 * 24 + await stRSR.connect(owner).setUnstakingDelay(oldUnstakingDelay) + const amount: BigNumber = bn('100e18') + await rsr.connect(addr1).approve(stRSR.address, amount) + await stRSR.connect(addr1).stake(amount) + + const draftEra = 1 + const availableAtOfFirst = (await getLatestBlockTimestamp()) + oldUnstakingDelay + 1 + /** + * Unstaking request enter a queue, and withdrawal become available 1 day later + */ + await expect(stRSR.connect(addr1).unstake(amount)) + .emit(stRSR, 'UnstakingStarted') + .withArgs(0, draftEra, addr1.address, amount, amount, availableAtOfFirst) + + /** + * Cancel the unstaking to eliminate any pending withdrawals + */ + await stRSR.connect(addr1).cancelUnstake(1) + + // new unstakingDelay is 1 hour + const newUnstakingDelay = 3600 + await stRSR.connect(owner).setUnstakingDelay(newUnstakingDelay) + + await rsr.connect(addr2).approve(stRSR.address, amount) + await stRSR.connect(addr2).stake(amount) + + const availableAtOfFirstOfUser2 = (await getLatestBlockTimestamp()) + newUnstakingDelay + 1 + /** + * Unstaking request enter a queue. Withdrawal should become available 1 hour later for both users + */ + await expect(stRSR.connect(addr2).unstake(amount)) + .emit(stRSR, 'UnstakingStarted') + .withArgs(0, draftEra, addr2.address, amount, amount, availableAtOfFirstOfUser2) + await expect(stRSR.connect(addr1).unstake(amount)) + .emit(stRSR, 'UnstakingStarted') + .withArgs(1, draftEra, addr1.address, amount, amount, availableAtOfFirstOfUser2 + 1) + }) + it('Should create Pending withdrawal when unstaking', async () => { const amount: BigNumber = bn('1000e18')