Added validation of moving funds proposal (#757)

#Refs: keep-network/keep-core#3733.

This PR adds moving funds proposal validation to the wallet proposal
validator smart contract.
The validation performs several checks:
- the source wallet's state must be MovingFunds,
- the source wallet's commitment hash must be submitted,
- the target wallets from the proposal must match the submitted
commitment hash,
- the proposed fee must be greater than zero,
- the transaction proposed fee must not exceed the maximum fee.

solidity/contracts/bridge/WalletProposalValidator.sol

@@ -94,6 +94,16 @@ contract WalletProposalValidator {
         uint256 redemptionTxFee;
     }
 
+    /// @notice Helper structure representing a moving funds proposal.
+    struct MovingFundsProposal {
+        // 20-byte public key hash of the source wallet.
+        bytes20 walletPubKeyHash;
+        // List of 20-byte public key hashes of target wallets.
+        bytes20[] targetWallets;
+        // Proposed BTC fee for the entire transaction.
+        uint256 movingFundsTxFee;
+    }
+
     /// @notice Helper structure representing a heartbeat proposal.
     struct HeartbeatProposal {
         // 20-byte public key hash of the target wallet.
@@ -587,6 +597,71 @@ contract WalletProposalValidator {
         return true;
     }
 
+    /// @notice View function encapsulating the main rules of a valid moving
+    ///         funds proposal. This function is meant to facilitate the
+    ///         off-chain validation of the incoming proposals. Thanks to it,
+    ///         most of the work can be done using a single readonly contract
+    ///         call.
+    /// @param proposal The moving funds proposal to validate.
+    /// @return True if the proposal is valid. Reverts otherwise.
+    /// @dev Notice that this function is meant to be invoked after the moving
+    ///      funds commitment has already been submitted. This function skips
+    ///      some checks related to the moving funds procedure as they were
+    ///      already checked on the commitment submission.
+    ///      Requirements:
+    ///      - The source wallet must be in the MovingFunds state,
+    ///      - The target wallets commitment must be submitted,
+    ///      - The target wallets commitment hash must match the target wallets
+    ///        from the proposal,
+    ///      - The proposed moving funds transaction fee must be greater than
+    ///        zero,
+    ///      - The proposed moving funds transaction fee must not exceed the
+    ///        maximum total fee allowed for moving funds.
+    function validateMovingFundsProposal(MovingFundsProposal calldata proposal)
+        external
+        view
+        returns (bool)
+    {
+        Wallets.Wallet memory sourceWallet = bridge.wallets(
+            proposal.walletPubKeyHash
+        );
+
+        // Make sure the source wallet is in MovingFunds state.
+        require(
+            sourceWallet.state == Wallets.WalletState.MovingFunds,
+            "Source wallet is not in MovingFunds state"
+        );
+
+        // Make sure the moving funds commitment has been submitted and
+        // the commitment hash matches the target wallets from the proposal.
+        require(
+            sourceWallet.movingFundsTargetWalletsCommitmentHash != bytes32(0),
+            "Target wallets commitment is not submitted"
+        );
+
+        require(
+            sourceWallet.movingFundsTargetWalletsCommitmentHash ==
+                keccak256(abi.encodePacked(proposal.targetWallets)),
+            "Target wallets do not match target wallets commitment hash"
+        );
+
+        // Make sure the proposed fee is valid.
+        (uint64 movingFundsTxMaxTotalFee, , , , , , , , , , ) = bridge
+            .movingFundsParameters();
+
+        require(
+            proposal.movingFundsTxFee > 0,
+            "Proposed transaction fee cannot be zero"
+        );
+
+        require(
+            proposal.movingFundsTxFee <= movingFundsTxMaxTotalFee,
+            "Proposed transaction fee is too high"
+        );
+
+        return true;
+    }
+
     /// @notice View function encapsulating the main rules of a valid heartbeat
     ///         proposal. This function is meant to facilitate the off-chain
     ///         validation of the incoming proposals. Thanks to it, most

solidity/test/bridge/WalletProposalValidator.test.ts

@@ -5,6 +5,7 @@ import { FakeContract, smock } from "@defi-wonderland/smock"
 import { BigNumber, BigNumberish, BytesLike } from "ethers"
 import type { Bridge, WalletProposalValidator } from "../../typechain"
 import { walletState } from "../fixtures"
+import { NO_MAIN_UTXO } from "../data/deposit-sweep"
 
 chai.use(smock.matchers)
 
@@ -1894,6 +1895,243 @@ describe("WalletProposalValidator", () => {
     })
   })
 
+  describe("validateMovingFundsProposal", () => {
+    const walletPubKeyHash = "0x7ac2d9378a1c47e589dfb8095ca95ed2140d2726"
+    const targetWallets = [
+      "0x84a70187011e156686788e0a2bc50944a4721e83",
+      "0xf64a45c07e3778b8ce58cb0058477c821c543aad",
+      "0xcaea95433d9bfa80bb8dc8819a48e2a9aa96147c",
+    ]
+    // Hash calculated from the above target wallets.
+    const targetWalletsHash =
+      "0x16311d424d513a1743fbc9c0e4fea5b70eddefd15f54613503e5cdfab24f8877"
+    const movingFundsTxMaxTotalFee = 20000
+
+    before(async () => {
+      await createSnapshot()
+
+      bridge.movingFundsParameters.returns([
+        movingFundsTxMaxTotalFee,
+        0,
+        0,
+        0,
+        0,
+        0,
+        0,
+        0,
+        0,
+        0,
+        0,
+      ])
+    })
+
+    after(async () => {
+      bridge.movingFundsParameters.reset()
+
+      await restoreSnapshot()
+    })
+
+    context("when wallet's state is not MovingFunds", () => {
+      const testData = [
+        {
+          testName: "when wallet state is Unknown",
+          walletState: walletState.Unknown,
+        },
+        {
+          testName: "when wallet state is Live",
+          walletState: walletState.Live,
+        },
+        {
+          testName: "when wallet state is Closing",
+          walletState: walletState.Closing,
+        },
+        {
+          testName: "when wallet state is Closed",
+          walletState: walletState.Closed,
+        },
+        {
+          testName: "when wallet state is Terminated",
+          walletState: walletState.Terminated,
+        },
+      ]
+
+      testData.forEach((test) => {
+        context(test.testName, () => {
+          before(async () => {
+            await createSnapshot()
+
+            bridge.wallets.whenCalledWith(walletPubKeyHash).returns({
+              ecdsaWalletID: HashZero,
+              mainUtxoHash: HashZero,
+              pendingRedemptionsValue: 0,
+              createdAt: 0,
+              movingFundsRequestedAt: 0,
+              closingStartedAt: 0,
+              pendingMovedFundsSweepRequestsCount: 0,
+              state: test.walletState,
+              movingFundsTargetWalletsCommitmentHash: HashZero,
+            })
+          })
+
+          after(async () => {
+            bridge.wallets.reset()
+
+            await restoreSnapshot()
+          })
+
+          it("should revert", async () => {
+            await expect(
+              walletProposalValidator.validateMovingFundsProposal({
+                walletPubKeyHash,
+                movingFundsTxFee: 0,
+                targetWallets: [],
+              })
+            ).to.be.revertedWith("Source wallet is not in MovingFunds state")
+          })
+        })
+      })
+    })
+
+    context("when wallet's state is MovingFunds", () => {
+      context("when moving funds commitment has not been submitted", () => {
+        before(async () => {
+          await createSnapshot()
+
+          bridge.wallets.whenCalledWith(walletPubKeyHash).returns({
+            ecdsaWalletID: HashZero,
+            mainUtxoHash: HashZero,
+            pendingRedemptionsValue: 0,
+            createdAt: 0,
+            movingFundsRequestedAt: 0,
+            closingStartedAt: 0,
+            pendingMovedFundsSweepRequestsCount: 0,
+            state: walletState.MovingFunds,
+            // Indicate the commitment has not been submitted.
+            movingFundsTargetWalletsCommitmentHash: HashZero,
+          })
+        })
+
+        after(async () => {
+          bridge.wallets.reset()
+
+          await restoreSnapshot()
+        })
+
+        it("should revert", async () => {
+          await expect(
+            walletProposalValidator.validateMovingFundsProposal({
+              walletPubKeyHash,
+              movingFundsTxFee: 0,
+              targetWallets: [],
+            })
+          ).to.be.revertedWith("Target wallets commitment is not submitted")
+        })
+      })
+
+      context("when moving funds commitment has been submitted", () => {
+        context("when commitment hash does not match target wallets", () => {
+          before(async () => {
+            await createSnapshot()
+
+            bridge.wallets.whenCalledWith(walletPubKeyHash).returns({
+              ecdsaWalletID: HashZero,
+              mainUtxoHash: HashZero,
+              pendingRedemptionsValue: 0,
+              createdAt: 0,
+              movingFundsRequestedAt: 0,
+              closingStartedAt: 0,
+              pendingMovedFundsSweepRequestsCount: 0,
+              state: walletState.MovingFunds,
+              // Set a hash that does not match the target wallets.
+              movingFundsTargetWalletsCommitmentHash:
+                "0xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
+            })
+          })
+
+          after(async () => {
+            bridge.wallets.reset()
+
+            await restoreSnapshot()
+          })
+
+          it("should revert", async () => {
+            await expect(
+              walletProposalValidator.validateMovingFundsProposal({
+                walletPubKeyHash,
+                movingFundsTxFee: 0,
+                targetWallets,
+              })
+            ).to.be.revertedWith(
+              "Target wallets do not match target wallets commitment hash"
+            )
+          })
+        })
+
+        context("when commitment hash matches target wallets", () => {
+          before(async () => {
+            await createSnapshot()
+
+            bridge.wallets.whenCalledWith(walletPubKeyHash).returns({
+              ecdsaWalletID: HashZero,
+              mainUtxoHash: HashZero,
+              pendingRedemptionsValue: 0,
+              createdAt: 0,
+              movingFundsRequestedAt: 0,
+              closingStartedAt: 0,
+              pendingMovedFundsSweepRequestsCount: 0,
+              state: walletState.MovingFunds,
+              // Set the correct hash.
+              movingFundsTargetWalletsCommitmentHash: targetWalletsHash,
+            })
+          })
+
+          after(async () => {
+            bridge.wallets.reset()
+
+            await restoreSnapshot()
+          })
+
+          context("when transaction fee is zero", () => {
+            it("should revert", async () => {
+              await expect(
+                walletProposalValidator.validateMovingFundsProposal({
+                  walletPubKeyHash,
+                  movingFundsTxFee: 0,
+                  targetWallets,
+                })
+              ).to.be.revertedWith("Proposed transaction fee cannot be zero")
+            })
+          })
+
+          context("when transaction fee is too high", () => {
+            it("should revert", async () => {
+              await expect(
+                walletProposalValidator.validateMovingFundsProposal({
+                  walletPubKeyHash,
+                  movingFundsTxFee: movingFundsTxMaxTotalFee + 1,
+                  targetWallets,
+                })
+              ).to.be.revertedWith("Proposed transaction fee is too high")
+            })
+          })
+
+          context("when transaction fee is valid", () => {
+            it("should pass validation", async () => {
+              const result =
+                await walletProposalValidator.validateMovingFundsProposal({
+                  walletPubKeyHash,
+                  movingFundsTxFee: movingFundsTxMaxTotalFee,
+                  targetWallets,
+                })
+              // eslint-disable-next-line @typescript-eslint/no-unused-expressions
+              expect(result).to.be.true
+            })
+          })
+        })
+      })
+    })
+  })
+
   describe("validateHeartbeatProposal", () => {
     context("when message is not valid", () => {
       it("should revert", async () => {