Skip to content

Commit

Permalink
Merge pull request #159 from lidofinance/fix/InvalidClaimableAmount-p…
Browse files Browse the repository at this point in the history
…arams-order

Audit fix: InvalidClaimableAmount params order
  • Loading branch information
bulbozaur authored Nov 11, 2024
2 parents 277ce97 + 3ed157c commit cb4312d
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 20 deletions.
10 changes: 5 additions & 5 deletions contracts/libraries/AssetsAccounting.sol
Original file line number Diff line number Diff line change
Expand Up @@ -127,9 +127,9 @@ library AssetsAccounting {

error InvalidSharesValue(SharesValue value);
error InvalidUnstETHStatus(uint256 unstETHId, UnstETHRecordStatus status);
error InvalidUnstETHHolder(uint256 unstETHId, address actual, address expected);
error InvalidUnstETHHolder(uint256 unstETHId, address holder);
error MinAssetsLockDurationNotPassed(Timestamp lockDurationExpiresAt);
error InvalidClaimableAmount(uint256 unstETHId, ETHValue expected, ETHValue actual);
error InvalidClaimableAmount(uint256 unstETHId, ETHValue claimableAmount);

// ---
// stETH Operations Accounting
Expand Down Expand Up @@ -411,7 +411,7 @@ library AssetsAccounting {
UnstETHRecord storage unstETHRecord = self.unstETHRecords[unstETHId];

if (unstETHRecord.lockedBy != holder) {
revert InvalidUnstETHHolder(unstETHId, holder, unstETHRecord.lockedBy);
revert InvalidUnstETHHolder(unstETHId, holder);
}

if (unstETHRecord.status == UnstETHRecordStatus.NotLocked) {
Expand Down Expand Up @@ -461,7 +461,7 @@ library AssetsAccounting {
if (unstETHRecord.status == UnstETHRecordStatus.Finalized) {
// If the unstETH was marked as finalized earlier, its claimable amount must remain unchanged.
if (unstETHRecord.claimableAmount != claimableAmount) {
revert InvalidClaimableAmount(unstETHId, claimableAmount, unstETHRecord.claimableAmount);
revert InvalidClaimableAmount(unstETHId, claimableAmount);
}
} else {
unstETHRecord.claimableAmount = claimableAmount;
Expand All @@ -480,7 +480,7 @@ library AssetsAccounting {
revert InvalidUnstETHStatus(unstETHId, unstETHRecord.status);
}
if (unstETHRecord.lockedBy != holder) {
revert InvalidUnstETHHolder(unstETHId, holder, unstETHRecord.lockedBy);
revert InvalidUnstETHHolder(unstETHId, holder);
}
unstETHRecord.status = UnstETHRecordStatus.Withdrawn;
amountWithdrawn = unstETHRecord.claimableAmount;
Expand Down
19 changes: 4 additions & 15 deletions test/unit/libraries/AssetsAccounting.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -781,9 +781,7 @@ contract AssetsAccountingUnitTests is UnitTest {
uint256[] memory unstETHIds = new uint256[](1);
unstETHIds[0] = genRandomUnstEthId(1234);

vm.expectRevert(
abi.encodeWithSelector(AssetsAccounting.InvalidUnstETHHolder.selector, unstETHIds[0], holder, address(0x0))
);
vm.expectRevert(abi.encodeWithSelector(AssetsAccounting.InvalidUnstETHHolder.selector, unstETHIds[0], holder));

AssetsAccounting.accountUnstETHUnlock(_accountingContext, holder, unstETHIds);
}
Expand All @@ -798,9 +796,7 @@ contract AssetsAccountingUnitTests is UnitTest {
unstETHIds[0] = genRandomUnstEthId(1234);
_accountingContext.unstETHRecords[unstETHIds[0]].lockedBy = holder;

vm.expectRevert(
abi.encodeWithSelector(AssetsAccounting.InvalidUnstETHHolder.selector, unstETHIds[0], current, holder)
);
vm.expectRevert(abi.encodeWithSelector(AssetsAccounting.InvalidUnstETHHolder.selector, unstETHIds[0], current));

AssetsAccounting.accountUnstETHUnlock(_accountingContext, current, unstETHIds);
}
Expand Down Expand Up @@ -1251,12 +1247,7 @@ contract AssetsAccountingUnitTests is UnitTest {
}

vm.expectRevert(
abi.encodeWithSelector(
AssetsAccounting.InvalidClaimableAmount.selector,
unstETHIds[0],
claimableAmounts[0],
uint256(claimableAmounts[0]) + 1
)
abi.encodeWithSelector(AssetsAccounting.InvalidClaimableAmount.selector, unstETHIds[0], claimableAmounts[0])
);

AssetsAccounting.accountUnstETHClaimed(_accountingContext, unstETHIds, claimableAmountsPrepared);
Expand Down Expand Up @@ -1392,9 +1383,7 @@ contract AssetsAccountingUnitTests is UnitTest {
_accountingContext.unstETHRecords[unstETHIds[0]].lockedBy = holder;
_accountingContext.unstETHRecords[unstETHIds[0]].claimableAmount = ETHValues.from(123);

vm.expectRevert(
abi.encodeWithSelector(AssetsAccounting.InvalidUnstETHHolder.selector, unstETHIds[0], current, holder)
);
vm.expectRevert(abi.encodeWithSelector(AssetsAccounting.InvalidUnstETHHolder.selector, unstETHIds[0], current));

AssetsAccounting.accountUnstETHWithdraw(_accountingContext, current, unstETHIds);
}
Expand Down

0 comments on commit cb4312d

Please sign in to comment.