Skip to content

Commit

Permalink
refactor: triggerable withdrawals lib rename errors for clarity
Browse files Browse the repository at this point in the history
  • Loading branch information
mkurayan committed Jan 28, 2025
1 parent cfadfb4 commit 9f268cf
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 34 deletions.
24 changes: 13 additions & 11 deletions contracts/common/lib/TriggerableWithdrawals.sol
Original file line number Diff line number Diff line change
Expand Up @@ -11,19 +11,21 @@ pragma solidity >=0.8.9 <0.9.0;
*/
library TriggerableWithdrawals {
address constant WITHDRAWAL_REQUEST = 0x0c15F14308530b7CDB8460094BbB9cC28b9AaaAA;
uint256 internal constant WITHDRAWAL_REQUEST_CALLDATA_LENGTH = 56;

uint256 internal constant PUBLIC_KEY_LENGTH = 48;
uint256 internal constant WITHDRAWAL_AMOUNT_LENGTH = 8;
uint256 internal constant WITHDRAWAL_REQUEST_CALLDATA_LENGTH = 56;

error MismatchedArrayLengths(uint256 keysCount, uint256 amountsCount);
error InsufficientBalance(uint256 balance, uint256 totalWithdrawalFee);
error InsufficientRequestFee(uint256 feePerRequest, uint256 minFeePerRequest);

error WithdrawalRequestFeeReadFailed();
error WithdrawalFeeReadFailed();
error WithdrawalRequestAdditionFailed(bytes callData);

error InsufficientWithdrawalFee(uint256 feePerRequest, uint256 minFeePerRequest);
error TotalWithdrawalFeeExceededBalance(uint256 balance, uint256 totalWithdrawalFee);

error NoWithdrawalRequests();
error MalformedPubkeysArray();
error PartialWithdrawalRequired(uint256 index);
error InvalidPublicKeyLength();
error MismatchedArrayLengths(uint256 keysCount, uint256 amountsCount);

/**
* @dev Send EIP-7002 full withdrawal requests for the specified public keys.
Expand Down Expand Up @@ -151,7 +153,7 @@ library TriggerableWithdrawals {
(bool success, bytes memory feeData) = WITHDRAWAL_REQUEST.staticcall("");

if (!success) {
revert WithdrawalRequestFeeReadFailed();
revert WithdrawalFeeReadFailed();
}

return abi.decode(feeData, (uint256));
Expand All @@ -171,7 +173,7 @@ library TriggerableWithdrawals {

function _validateAndCountPubkeys(bytes calldata pubkeys) private pure returns (uint256) {
if (pubkeys.length % PUBLIC_KEY_LENGTH != 0) {
revert InvalidPublicKeyLength();
revert MalformedPubkeysArray();
}

uint256 keysCount = pubkeys.length / PUBLIC_KEY_LENGTH;
Expand All @@ -190,11 +192,11 @@ library TriggerableWithdrawals {
}

if (feePerRequest < minFeePerRequest) {
revert InsufficientRequestFee(feePerRequest, minFeePerRequest);
revert InsufficientWithdrawalFee(feePerRequest, minFeePerRequest);
}

if (address(this).balance < feePerRequest * keysCount) {
revert InsufficientBalance(address(this).balance, feePerRequest * keysCount);
revert TotalWithdrawalFeeExceededBalance(address(this).balance, feePerRequest * keysCount);
}

return feePerRequest;
Expand Down
11 changes: 4 additions & 7 deletions test/0.8.9/withdrawalVault.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -282,10 +282,7 @@ describe("WithdrawalVault.sol", () => {

it("Should revert if fee read fails", async function () {
await withdrawalsPredeployed.setFailOnGetFee(true);
await expect(vault.getWithdrawalRequestFee()).to.be.revertedWithCustomError(
vault,
"WithdrawalRequestFeeReadFailed",
);
await expect(vault.getWithdrawalRequestFee()).to.be.revertedWithCustomError(vault, "WithdrawalFeeReadFailed");
});
});

Expand Down Expand Up @@ -351,7 +348,7 @@ describe("WithdrawalVault.sol", () => {

await expect(
vault.connect(validatorsExitBus).addFullWithdrawalRequests(invalidPubkeyHexString, { value: fee }),
).to.be.revertedWithCustomError(vault, "InvalidPublicKeyLength");
).to.be.revertedWithCustomError(vault, "MalformedPubkeysArray");
});

it("Should revert if last pubkey not 48 bytes", async function () {
Expand All @@ -364,7 +361,7 @@ describe("WithdrawalVault.sol", () => {

await expect(
vault.connect(validatorsExitBus).addFullWithdrawalRequests(pubkeysHexString, { value: fee }),
).to.be.revertedWithCustomError(vault, "InvalidPublicKeyLength");
).to.be.revertedWithCustomError(vault, "MalformedPubkeysArray");
});

it("Should revert if addition fails at the withdrawal request contract", async function () {
Expand All @@ -387,7 +384,7 @@ describe("WithdrawalVault.sol", () => {

await expect(
vault.connect(validatorsExitBus).addFullWithdrawalRequests(pubkeysHexString, { value: fee }),
).to.be.revertedWithCustomError(vault, "WithdrawalRequestFeeReadFailed");
).to.be.revertedWithCustomError(vault, "WithdrawalFeeReadFailed");
});

it("should revert if refund failed", async function () {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ describe("TriggerableWithdrawals.sol", () => {
await withdrawalsPredeployed.setFailOnGetFee(true);
await expect(triggerableWithdrawals.getWithdrawalRequestFee()).to.be.revertedWithCustomError(
triggerableWithdrawals,
"WithdrawalRequestFeeReadFailed",
"WithdrawalFeeReadFailed",
);
});
});
Expand Down Expand Up @@ -133,15 +133,15 @@ describe("TriggerableWithdrawals.sol", () => {
// 2. Should revert if fee is less than required
const insufficientFee = 2n;
await expect(triggerableWithdrawals.addFullWithdrawalRequests(pubkeysHexString, insufficientFee))
.to.be.revertedWithCustomError(triggerableWithdrawals, "InsufficientRequestFee")
.to.be.revertedWithCustomError(triggerableWithdrawals, "InsufficientWithdrawalFee")
.withArgs(2n, 3n);

await expect(triggerableWithdrawals.addPartialWithdrawalRequests(pubkeysHexString, amounts, insufficientFee))
.to.be.revertedWithCustomError(triggerableWithdrawals, "InsufficientRequestFee")
.to.be.revertedWithCustomError(triggerableWithdrawals, "InsufficientWithdrawalFee")
.withArgs(2n, 3n);

await expect(triggerableWithdrawals.addWithdrawalRequests(pubkeysHexString, amounts, insufficientFee))
.to.be.revertedWithCustomError(triggerableWithdrawals, "InsufficientRequestFee")
.to.be.revertedWithCustomError(triggerableWithdrawals, "InsufficientWithdrawalFee")
.withArgs(2n, 3n);
});

Expand All @@ -154,15 +154,15 @@ describe("TriggerableWithdrawals.sol", () => {

await expect(
triggerableWithdrawals.addFullWithdrawalRequests(invalidPubkeyHexString, fee),
).to.be.revertedWithCustomError(triggerableWithdrawals, "InvalidPublicKeyLength");
).to.be.revertedWithCustomError(triggerableWithdrawals, "MalformedPubkeysArray");

await expect(
triggerableWithdrawals.addPartialWithdrawalRequests(invalidPubkeyHexString, amounts, fee),
).to.be.revertedWithCustomError(triggerableWithdrawals, "InvalidPublicKeyLength");
).to.be.revertedWithCustomError(triggerableWithdrawals, "MalformedPubkeysArray");

await expect(
triggerableWithdrawals.addWithdrawalRequests(invalidPubkeyHexString, amounts, fee),
).to.be.revertedWithCustomError(triggerableWithdrawals, "InvalidPublicKeyLength");
).to.be.revertedWithCustomError(triggerableWithdrawals, "MalformedPubkeysArray");
});

it("Should revert if last pubkey not 48 bytes", async function () {
Expand All @@ -177,15 +177,15 @@ describe("TriggerableWithdrawals.sol", () => {

await expect(
triggerableWithdrawals.addFullWithdrawalRequests(pubkeysHexString, fee),
).to.be.revertedWithCustomError(triggerableWithdrawals, "InvalidPublicKeyLength");
).to.be.revertedWithCustomError(triggerableWithdrawals, "MalformedPubkeysArray");

await expect(
triggerableWithdrawals.addPartialWithdrawalRequests(pubkeysHexString, amounts, fee),
).to.be.revertedWithCustomError(triggerableWithdrawals, "InvalidPublicKeyLength");
).to.be.revertedWithCustomError(triggerableWithdrawals, "MalformedPubkeysArray");

await expect(
triggerableWithdrawals.addWithdrawalRequests(pubkeysHexString, amounts, fee),
).to.be.revertedWithCustomError(triggerableWithdrawals, "InvalidPublicKeyLength");
).to.be.revertedWithCustomError(triggerableWithdrawals, "MalformedPubkeysArray");
});

it("Should revert if addition fails at the withdrawal request contract", async function () {
Expand Down Expand Up @@ -233,15 +233,15 @@ describe("TriggerableWithdrawals.sol", () => {
await setBalance(await triggerableWithdrawals.getAddress(), balance);

await expect(triggerableWithdrawals.addFullWithdrawalRequests(pubkeysHexString, fee))
.to.be.revertedWithCustomError(triggerableWithdrawals, "InsufficientBalance")
.to.be.revertedWithCustomError(triggerableWithdrawals, "TotalWithdrawalFeeExceededBalance")
.withArgs(balance, expectedMinimalBalance);

await expect(triggerableWithdrawals.addPartialWithdrawalRequests(pubkeysHexString, partialWithdrawalAmounts, fee))
.to.be.revertedWithCustomError(triggerableWithdrawals, "InsufficientBalance")
.to.be.revertedWithCustomError(triggerableWithdrawals, "TotalWithdrawalFeeExceededBalance")
.withArgs(balance, expectedMinimalBalance);

await expect(triggerableWithdrawals.addWithdrawalRequests(pubkeysHexString, mixedWithdrawalAmounts, fee))
.to.be.revertedWithCustomError(triggerableWithdrawals, "InsufficientBalance")
.to.be.revertedWithCustomError(triggerableWithdrawals, "TotalWithdrawalFeeExceededBalance")
.withArgs(balance, expectedMinimalBalance);
});

Expand All @@ -254,15 +254,15 @@ describe("TriggerableWithdrawals.sol", () => {

await expect(
triggerableWithdrawals.addFullWithdrawalRequests(pubkeysHexString, fee),
).to.be.revertedWithCustomError(triggerableWithdrawals, "WithdrawalRequestFeeReadFailed");
).to.be.revertedWithCustomError(triggerableWithdrawals, "WithdrawalFeeReadFailed");

await expect(
triggerableWithdrawals.addPartialWithdrawalRequests(pubkeysHexString, partialWithdrawalAmounts, fee),
).to.be.revertedWithCustomError(triggerableWithdrawals, "WithdrawalRequestFeeReadFailed");
).to.be.revertedWithCustomError(triggerableWithdrawals, "WithdrawalFeeReadFailed");

await expect(
triggerableWithdrawals.addWithdrawalRequests(pubkeysHexString, mixedWithdrawalAmounts, fee),
).to.be.revertedWithCustomError(triggerableWithdrawals, "WithdrawalRequestFeeReadFailed");
).to.be.revertedWithCustomError(triggerableWithdrawals, "WithdrawalFeeReadFailed");
});

it("Should accept withdrawal requests with minimal possible fee when fee not provided", async function () {
Expand Down

0 comments on commit 9f268cf

Please sign in to comment.