feat: validate withdrawal fee response

lidofinance · Feb 4, 2025 · 1af1d3a · 1af1d3a
1 parent 6da1d6f
commit 1af1d3a
Show file tree

Hide file tree

Showing 4 changed files with 67 additions and 5 deletions.
diff --git a/contracts/common/lib/TriggerableWithdrawals.sol b/contracts/common/lib/TriggerableWithdrawals.sol
@@ -17,6 +17,7 @@ library TriggerableWithdrawals {
     uint256 internal constant WITHDRAWAL_REQUEST_CALLDATA_LENGTH = 56;
 
     error WithdrawalFeeReadFailed();
+    error WithdrawalFeeInvalidData();
     error WithdrawalRequestAdditionFailed(bytes callData);
 
     error InsufficientWithdrawalFee(uint256 feePerRequest, uint256 minFeePerRequest);
@@ -156,6 +157,10 @@ library TriggerableWithdrawals {
             revert WithdrawalFeeReadFailed();
         }
 
+        if (feeData.length != 32) {
+            revert WithdrawalFeeInvalidData();
+        }
+
         return abi.decode(feeData, (uint256));
     }
 

diff --git a/test/0.8.9/withdrawalVault.test.ts b/test/0.8.9/withdrawalVault.test.ts
@@ -284,6 +284,14 @@ describe("WithdrawalVault.sol", () => {
       await withdrawalsPredeployed.setFailOnGetFee(true);
       await expect(vault.getWithdrawalRequestFee()).to.be.revertedWithCustomError(vault, "WithdrawalFeeReadFailed");
     });
+
+    ["0x", "0x01", "0x" + "0".repeat(61) + "1", "0x" + "0".repeat(65) + "1"].forEach((unexpectedFee) => {
+      it(`Shoud revert if unexpected fee value ${unexpectedFee} is returned`, async function () {
+        await withdrawalsPredeployed.setFeeRaw(unexpectedFee);
+
+        await expect(vault.getWithdrawalRequestFee()).to.be.revertedWithCustomError(vault, "WithdrawalFeeInvalidData");
+      });
+    });
   });
 
   async function getFee(): Promise<bigint> {
@@ -387,6 +395,19 @@ describe("WithdrawalVault.sol", () => {
       ).to.be.revertedWithCustomError(vault, "WithdrawalFeeReadFailed");
     });
 
+    ["0x", "0x01", "0x" + "0".repeat(61) + "1", "0x" + "0".repeat(65) + "1"].forEach((unexpectedFee) => {
+      it(`Shoud revert if unexpected fee value ${unexpectedFee} is returned`, async function () {
+        await withdrawalsPredeployed.setFeeRaw(unexpectedFee);
+
+        const { pubkeysHexString } = generateWithdrawalRequestPayload(2);
+        const fee = 10n;
+
+        await expect(
+          vault.connect(validatorsExitBus).addFullWithdrawalRequests(pubkeysHexString, { value: fee }),
+        ).to.be.revertedWithCustomError(vault, "WithdrawalFeeInvalidData");
+      });
+    });
+
     it("should revert if refund failed", async function () {
       const refundFailureTester: RefundFailureTester = await ethers.deployContract("RefundFailureTester", [
         vaultAddress,

diff --git a/test/common/contracts/EIP7002WithdrawalRequest_Mock.sol b/test/common/contracts/EIP7002WithdrawalRequest_Mock.sol
@@ -7,7 +7,7 @@ pragma solidity 0.8.9;
  * @notice This is a mock of EIP-7002's pre-deploy contract.
  */
 contract EIP7002WithdrawalRequest_Mock {
-    uint256 public fee;
+    bytes public fee;
     bool public failOnAddRequest;
     bool public failOnGetFee;
 
@@ -23,15 +23,18 @@ contract EIP7002WithdrawalRequest_Mock {
 
     function setFee(uint256 _fee) external {
         require(_fee > 0, "fee must be greater than 0");
-        fee = _fee;
+        fee = abi.encode(_fee);
     }
 
-    fallback(bytes calldata input) external payable returns (bytes memory output) {
+    function setFeeRaw(bytes calldata _rawFeeBytes) external {
+        fee = _rawFeeBytes;
+    }
+
+    fallback(bytes calldata input) external payable returns (bytes memory) {
         if (input.length == 0) {
             require(!failOnGetFee, "fail on get fee");
 
-            output = abi.encode(fee);
-            return output;
+            return fee;
         }
 
         require(!failOnAddRequest, "fail on add request");

diff --git a/test/common/lib/triggerableWithdrawals/triggerableWithdrawals.test.ts b/test/common/lib/triggerableWithdrawals/triggerableWithdrawals.test.ts
@@ -81,6 +81,17 @@ describe("TriggerableWithdrawals.sol", () => {
         "WithdrawalFeeReadFailed",
       );
     });
+
+    ["0x", "0x01", "0x" + "0".repeat(61) + "1", "0x" + "0".repeat(65) + "1"].forEach((unexpectedFee) => {
+      it(`Shoud revert if unexpected fee value ${unexpectedFee} is returned`, async function () {
+        await withdrawalsPredeployed.setFeeRaw(unexpectedFee);
+
+        await expect(triggerableWithdrawals.getWithdrawalRequestFee()).to.be.revertedWithCustomError(
+          triggerableWithdrawals,
+          "WithdrawalFeeInvalidData",
+        );
+      });
+    });
   });
 
   context("add triggerable withdrawal requests", () => {
@@ -265,6 +276,28 @@ describe("TriggerableWithdrawals.sol", () => {
       ).to.be.revertedWithCustomError(triggerableWithdrawals, "WithdrawalFeeReadFailed");
     });
 
+    ["0x", "0x01", "0x" + "0".repeat(61) + "1", "0x" + "0".repeat(65) + "1"].forEach((unexpectedFee) => {
+      it(`Shoud revert if unexpected fee value ${unexpectedFee} is returned`, async function () {
+        await withdrawalsPredeployed.setFeeRaw(unexpectedFee);
+
+        const { pubkeysHexString, partialWithdrawalAmounts, mixedWithdrawalAmounts } =
+          generateWithdrawalRequestPayload(2);
+        const fee = 10n;
+
+        await expect(
+          triggerableWithdrawals.addFullWithdrawalRequests(pubkeysHexString, fee),
+        ).to.be.revertedWithCustomError(triggerableWithdrawals, "WithdrawalFeeInvalidData");
+
+        await expect(
+          triggerableWithdrawals.addPartialWithdrawalRequests(pubkeysHexString, partialWithdrawalAmounts, fee),
+        ).to.be.revertedWithCustomError(triggerableWithdrawals, "WithdrawalFeeInvalidData");
+
+        await expect(
+          triggerableWithdrawals.addWithdrawalRequests(pubkeysHexString, mixedWithdrawalAmounts, fee),
+        ).to.be.revertedWithCustomError(triggerableWithdrawals, "WithdrawalFeeInvalidData");
+      });
+    });
+
     it("Should accept withdrawal requests with minimal possible fee when fee not provided", async function () {
       const requestCount = 3;
       const { pubkeysHexString, pubkeys, fullWithdrawalAmounts, partialWithdrawalAmounts, mixedWithdrawalAmounts } =