diff --git a/solidity/contracts/bridge/WalletCoordinator.sol b/solidity/contracts/bridge/WalletCoordinator.sol index 3f56171b7..b5311db7d 100644 --- a/solidity/contracts/bridge/WalletCoordinator.sol +++ b/solidity/contracts/bridge/WalletCoordinator.sol @@ -25,6 +25,7 @@ import "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol"; import "./BitcoinTx.sol"; import "./Bridge.sol"; import "./Deposit.sol"; +import "./Redemption.sol"; import "./Wallets.sol"; /// @title Wallet coordinator. @@ -120,6 +121,19 @@ contract WalletCoordinator is OwnableUpgradeable, Reimbursable { bytes4 refundLocktime; } + /// @notice Helper structure representing a redemption proposal. + struct RedemptionProposal { + // 20-byte public key hash of the target wallet. + bytes20 walletPubKeyHash; + // Array of the redeemers' output scripts that should be part of + // the redemption. Each output script MUST BE prefixed by its byte + // length, i.e. passed in the exactly same format as during the + // `Bridge.requestRedemption` transaction. + bytes[] redeemersOutputScripts; + // Proposed BTC fee for the entire transaction. + uint256 redemptionTxFee; + } + /// @notice Mapping that holds addresses allowed to submit proposals and /// request heartbeats. mapping(address => bool) public isCoordinator; @@ -195,6 +209,55 @@ contract WalletCoordinator is OwnableUpgradeable, Reimbursable { /// the current conditions. uint32 public depositSweepProposalSubmissionGasOffset; + /// @notice Determines the redemption proposal validity time. In other + /// words, this is the worst-case time for a redemption during + /// which the wallet is busy and cannot take another actions. This + /// is also the duration of the time lock applied to the wallet + /// once a new redemption proposal is submitted. + /// + /// For example, if a redemption proposal was submitted at + /// 2 pm and redemptionProposalValidity is 2 hours, the next + /// proposal (of any type) can be submitted after 4 pm. + uint32 public redemptionProposalValidity; + + /// @notice The minimum time that must elapse since the redemption request + /// creation before a request becomes eligible for a processing. + /// + /// For example, if a request was created at 9 am and + /// redemptionRequestMinAge is 2 hours, the request is eligible for + /// processing after 11 am. + /// + /// @dev Forcing request minimum age ensures block finality for Ethereum. + uint32 public redemptionRequestMinAge; + + /// @notice Each redemption request can be technically handled until it + /// reaches its timeout timestamp after which it can be reported + /// as timed out. However, allowing the wallet to handle requests + /// that are close to their timeout timestamp may cause a race + /// between the wallet and the redeemer. In result, the wallet may + /// redeem the requested funds even though the redeemer already + /// received back their tBTC (locked during redemption request) upon + /// reporting the request timeout. In effect, the redeemer may end + /// out with both tBTC and redeemed BTC in their hands which has + /// a negative impact on the tBTC <-> BTC peg. In order to mitigate + /// that problem, this parameter determines a safety margin that + /// puts the latest moment a request can be handled far before the + /// point after which the request can be reported as timed out. + /// + /// For example, if a request times out after 8 pm and + /// redemptionRequestTimeoutSafetyMargin is 2 hours, the request is + /// valid for processing only before 6 pm. + uint32 public redemptionRequestTimeoutSafetyMargin; + + /// @notice The maximum count of redemption requests that can be processed + /// within a single redemption. + uint16 public redemptionMaxSize; + + /// @notice Gas that is meant to balance the redemption proposal + /// submission overall cost. Can be updated by the owner based on + /// the current conditions. + uint32 public redemptionProposalSubmissionGasOffset; + event CoordinatorAdded(address indexed coordinator); event CoordinatorRemoved(address indexed coordinator); @@ -225,6 +288,19 @@ contract WalletCoordinator is OwnableUpgradeable, Reimbursable { address indexed coordinator ); + event RedemptionProposalParametersUpdated( + uint32 redemptionProposalValidity, + uint32 redemptionRequestMinAge, + uint32 redemptionRequestTimeoutSafetyMargin, + uint16 redemptionMaxSize, + uint32 redemptionProposalSubmissionGasOffset + ); + + event RedemptionProposalSubmitted( + RedemptionProposal proposal, + address indexed coordinator + ); + modifier onlyCoordinator() { require(isCoordinator[msg.sender], "Caller is not a coordinator"); _; @@ -259,6 +335,12 @@ contract WalletCoordinator is OwnableUpgradeable, Reimbursable { depositRefundSafetyMargin = 24 hours; depositSweepMaxSize = 5; depositSweepProposalSubmissionGasOffset = 20_000; // optimized for 10 inputs + + redemptionProposalValidity = 2 hours; + redemptionRequestMinAge = 600; // 10 minutes or ~50 blocks. + redemptionRequestTimeoutSafetyMargin = 2 hours; + redemptionMaxSize = 20; + redemptionProposalSubmissionGasOffset = 20_000; } /// @notice Adds the given address to the set of coordinator addresses. @@ -467,7 +549,8 @@ contract WalletCoordinator is OwnableUpgradeable, Reimbursable { /// - Each deposit must have valid extra data (see `validateDepositExtraInfo`), /// - Each deposit must have the refund safety margin preserved, /// - Each deposit must be controlled by the same wallet, - /// - Each deposit must target the same vault. + /// - Each deposit must target the same vault, + /// - Each deposit must be unique. /// /// The following off-chain validation must be performed as a bare minimum: /// - Inputs used for the sweep transaction have enough Bitcoin confirmations, @@ -498,22 +581,28 @@ contract WalletCoordinator is OwnableUpgradeable, Reimbursable { address proposalVault = address(0); + uint256[] memory processedDepositKeys = new uint256[]( + proposal.depositsKeys.length + ); + for (uint256 i = 0; i < proposal.depositsKeys.length; i++) { DepositKey memory depositKey = proposal.depositsKeys[i]; DepositExtraInfo memory depositExtraInfo = depositsExtraInfo[i]; - // slither-disable-next-line calls-loop - Deposit.DepositRequest memory depositRequest = bridge.deposits( - uint256( - keccak256( - abi.encodePacked( - depositKey.fundingTxHash, - depositKey.fundingOutputIndex - ) + uint256 depositKeyUint = uint256( + keccak256( + abi.encodePacked( + depositKey.fundingTxHash, + depositKey.fundingOutputIndex ) ) ); + // slither-disable-next-line calls-loop + Deposit.DepositRequest memory depositRequest = bridge.deposits( + depositKeyUint + ); + require(depositRequest.revealedAt != 0, "Deposit not revealed"); require( @@ -554,6 +643,16 @@ contract WalletCoordinator is OwnableUpgradeable, Reimbursable { depositRequest.vault == proposalVault, "Deposit targets different vault" ); + + // Make sure there are no duplicates in the deposits list. + for (uint256 j = 0; j < i; j++) { + require( + processedDepositKeys[j] != depositKeyUint, + "Duplicated deposit" + ); + } + + processedDepositKeys[i] = depositKeyUint; } return true; @@ -687,4 +786,218 @@ contract WalletCoordinator is OwnableUpgradeable, Reimbursable { revert("Extra info funding output script does not match"); } + + /// @notice Updates parameters related to redemption proposal. + /// @param _redemptionProposalValidity The new value of `redemptionProposalValidity`. + /// @param _redemptionRequestMinAge The new value of `redemptionRequestMinAge`. + /// @param _redemptionRequestTimeoutSafetyMargin The new value of + /// `redemptionRequestTimeoutSafetyMargin`. + /// @param _redemptionMaxSize The new value of `redemptionMaxSize`. + /// @param _redemptionProposalSubmissionGasOffset The new value of + /// `redemptionProposalSubmissionGasOffset`. + /// @dev Requirements: + /// - The caller must be the owner. + function updateRedemptionProposalParameters( + uint32 _redemptionProposalValidity, + uint32 _redemptionRequestMinAge, + uint32 _redemptionRequestTimeoutSafetyMargin, + uint16 _redemptionMaxSize, + uint32 _redemptionProposalSubmissionGasOffset + ) external onlyOwner { + redemptionProposalValidity = _redemptionProposalValidity; + redemptionRequestMinAge = _redemptionRequestMinAge; + redemptionRequestTimeoutSafetyMargin = _redemptionRequestTimeoutSafetyMargin; + redemptionMaxSize = _redemptionMaxSize; + redemptionProposalSubmissionGasOffset = _redemptionProposalSubmissionGasOffset; + + emit RedemptionProposalParametersUpdated( + _redemptionProposalValidity, + _redemptionRequestMinAge, + _redemptionRequestTimeoutSafetyMargin, + _redemptionMaxSize, + _redemptionProposalSubmissionGasOffset + ); + } + + /// @notice Submits a redemption proposal. Locks the target wallet + /// for a specific time, equal to the proposal validity period. + /// This function does not store the proposal in the state but + /// just emits an event that serves as a guiding light for wallet + /// off-chain members. Wallet members are supposed to validate + /// the proposal on their own, before taking any action. + /// @param proposal The redemption proposal + /// @dev Requirements: + /// - The caller is a coordinator, + /// - The wallet is not time-locked. + function submitRedemptionProposal(RedemptionProposal calldata proposal) + public + onlyCoordinator + onlyAfterWalletLock(proposal.walletPubKeyHash) + { + walletLock[proposal.walletPubKeyHash] = WalletLock( + /* solhint-disable-next-line not-rely-on-time */ + uint32(block.timestamp) + redemptionProposalValidity, + WalletAction.Redemption + ); + + emit RedemptionProposalSubmitted(proposal, msg.sender); + } + + /// @notice Wraps `submitRedemptionProposal` call and reimburses the + /// caller's transaction cost. + /// @dev See `submitRedemptionProposal` function documentation. + function submitRedemptionProposalWithReimbursement( + RedemptionProposal calldata proposal + ) external { + uint256 gasStart = gasleft(); + + submitRedemptionProposal(proposal); + + reimbursementPool.refund( + (gasStart - gasleft()) + redemptionProposalSubmissionGasOffset, + msg.sender + ); + } + + /// @notice View function encapsulating the main rules of a valid redemption + /// 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 redemption proposal to validate. + /// @return True if the proposal is valid. Reverts otherwise. + /// @dev Requirements: + /// - The target wallet must be in the Live state, + /// - The number of redemption requests included in the redemption + /// proposal must be in the range [1, `redemptionMaxSize`], + /// - The proposed redemption tx fee must be grater than zero, + /// - The proposed redemption tx fee must be lesser than or equal to + /// the maximum total fee allowed by the Bridge + /// (`Bridge.redemptionTxMaxTotalFee`), + /// - The proposed maximum per-request redemption tx fee share must be + /// lesser than or equal to the maximum fee share allowed by the + /// given request (`RedemptionRequest.txMaxFee`), + /// - Each request must be a pending request registered in the Bridge, + /// - Each request must be old enough, i.e. at least `redemptionRequestMinAge` + /// elapsed since their creation time, + /// - Each request must have the timeout safety margin preserved, + /// - Each request must be unique. + function validateRedemptionProposal(RedemptionProposal calldata proposal) + external + view + returns (bool) + { + require( + bridge.wallets(proposal.walletPubKeyHash).state == + Wallets.WalletState.Live, + "Wallet is not in Live state" + ); + + uint256 requestsCount = proposal.redeemersOutputScripts.length; + + require(requestsCount > 0, "Redemption below the min size"); + + require( + requestsCount <= redemptionMaxSize, + "Redemption exceeds the max size" + ); + + ( + , + , + , + uint64 redemptionTxMaxTotalFee, + uint32 redemptionTimeout, + , + + ) = bridge.redemptionParameters(); + + require( + proposal.redemptionTxFee > 0, + "Proposed transaction fee cannot be zero" + ); + + // Make sure the proposed fee does not exceed the total fee limit. + require( + proposal.redemptionTxFee <= redemptionTxMaxTotalFee, + "Proposed transaction fee is too high" + ); + + // Compute the indivisible remainder that remains after dividing the + // redemption transaction fee over all requests evenly. + uint256 redemptionTxFeeRemainder = proposal.redemptionTxFee % + requestsCount; + // Compute the transaction fee per request by dividing the redemption + // transaction fee (reduced by the remainder) by the number of requests. + uint256 redemptionTxFeePerRequest = (proposal.redemptionTxFee - + redemptionTxFeeRemainder) / requestsCount; + + uint256[] memory processedRedemptionKeys = new uint256[](requestsCount); + + for (uint256 i = 0; i < requestsCount; i++) { + bytes memory script = proposal.redeemersOutputScripts[i]; + + // As the wallet public key hash is part of the redemption key, + // we have an implicit guarantee that all requests being part + // of the proposal target the same wallet. + uint256 redemptionKey = uint256( + keccak256( + abi.encodePacked( + keccak256(script), + proposal.walletPubKeyHash + ) + ) + ); + + // slither-disable-next-line calls-loop + Redemption.RedemptionRequest memory redemptionRequest = bridge + .pendingRedemptions(redemptionKey); + + require( + redemptionRequest.requestedAt != 0, + "Not a pending redemption request" + ); + + require( + /* solhint-disable-next-line not-rely-on-time */ + block.timestamp > + redemptionRequest.requestedAt + redemptionRequestMinAge, + "Redemption request min age not achieved yet" + ); + + // Calculate the timeout the given request times out at. + uint32 requestTimeout = redemptionRequest.requestedAt + + redemptionTimeout; + // Make sure we are far enough from the moment the request times out. + require( + /* solhint-disable-next-line not-rely-on-time */ + block.timestamp < + requestTimeout - redemptionRequestTimeoutSafetyMargin, + "Redemption request timeout safety margin is not preserved" + ); + + uint256 feePerRequest = redemptionTxFeePerRequest; + // The last request incurs the fee remainder. + if (i == requestsCount - 1) { + feePerRequest += redemptionTxFeeRemainder; + } + // Make sure the redemption transaction fee share incurred by + // the given request fits in the limit for that request. + require( + feePerRequest <= redemptionRequest.txMaxFee, + "Proposed transaction per-request fee share is too high" + ); + + // Make sure there are no duplicates in the requests list. + for (uint256 j = 0; j < i; j++) { + require( + processedRedemptionKeys[j] != redemptionKey, + "Duplicated request" + ); + } + + processedRedemptionKeys[i] = redemptionKey; + } + + return true; + } } diff --git a/solidity/test/bridge/WalletCoordinator.test.ts b/solidity/test/bridge/WalletCoordinator.test.ts index 4b6f6a7bf..3e4bf96a6 100644 --- a/solidity/test/bridge/WalletCoordinator.test.ts +++ b/solidity/test/bridge/WalletCoordinator.test.ts @@ -1956,83 +1956,1196 @@ describe("WalletCoordinator", () => { context( "when all deposits targets the same vault", () => { - let depositOne - let depositTwo + context( + "when there are duplicated deposits", + () => { + let depositOne + let depositTwo + let depositThree + + before(async () => { + await createSnapshot() + + depositOne = createTestDeposit( + walletPubKeyHash, + vault, + true + ) + + depositTwo = createTestDeposit( + walletPubKeyHash, + vault, + false + ) + + depositThree = createTestDeposit( + walletPubKeyHash, + vault, + false + ) + + bridge.deposits + .whenCalledWith( + depositKey( + depositOne.key.fundingTxHash, + depositOne.key.fundingOutputIndex + ) + ) + .returns(depositOne.request) + + bridge.deposits + .whenCalledWith( + depositKey( + depositTwo.key.fundingTxHash, + depositTwo.key.fundingOutputIndex + ) + ) + .returns(depositTwo.request) + + bridge.deposits + .whenCalledWith( + depositKey( + depositThree.key.fundingTxHash, + depositThree.key + .fundingOutputIndex + ) + ) + .returns(depositThree.request) + }) + + after(async () => { + bridge.deposits.reset() + + await restoreSnapshot() + }) + + it("should revert", async () => { + const proposal = { + walletPubKeyHash, + depositsKeys: [ + depositOne.key, + depositTwo.key, + depositThree.key, + depositTwo.key, // duplicate + ], + sweepTxFee, + depositsRevealBlocks: [], // Not relevant in this scenario. + } + + const depositsExtraInfo = [ + depositOne.extraInfo, + depositTwo.extraInfo, + depositThree.extraInfo, + depositTwo.extraInfo, // duplicate + ] + + await expect( + walletCoordinator.validateDepositSweepProposal( + proposal, + depositsExtraInfo + ) + ).to.be.revertedWith( + "Duplicated deposit" + ) + }) + } + ) + + context( + "when all deposits are unique", + () => { + let depositOne + let depositTwo + + before(async () => { + await createSnapshot() + + depositOne = createTestDeposit( + walletPubKeyHash, + vault, + true + ) + + depositTwo = createTestDeposit( + walletPubKeyHash, + vault, + false + ) + + bridge.deposits + .whenCalledWith( + depositKey( + depositOne.key.fundingTxHash, + depositOne.key.fundingOutputIndex + ) + ) + .returns(depositOne.request) + + bridge.deposits + .whenCalledWith( + depositKey( + depositTwo.key.fundingTxHash, + depositTwo.key.fundingOutputIndex + ) + ) + .returns(depositTwo.request) + }) + + after(async () => { + bridge.deposits.reset() + + await restoreSnapshot() + }) + + it("should succeed", async () => { + const proposal = { + walletPubKeyHash, + depositsKeys: [ + depositOne.key, + depositTwo.key, + ], + sweepTxFee, + depositsRevealBlocks: [], // Not relevant in this scenario. + } + + const depositsExtraInfo = [ + depositOne.extraInfo, + depositTwo.extraInfo, + ] + + const result = + await walletCoordinator.validateDepositSweepProposal( + proposal, + depositsExtraInfo + ) + + // eslint-disable-next-line @typescript-eslint/no-unused-expressions + expect(result).to.be.true + }) + } + ) + } + ) + } + ) + } + ) + }) + }) + }) + }) + }) + }) + }) + }) + }) + }) + + describe("updateRedemptionProposalParameters", () => { + before(async () => { + await createSnapshot() + }) + + after(async () => { + await restoreSnapshot() + }) + + context("when called by a third party", () => { + it("should revert", async () => { + await expect( + walletCoordinator + .connect(thirdParty) + .updateRedemptionProposalParameters(101, 102, 103, 104, 105) + ).to.be.revertedWith("Ownable: caller is not the owner") + }) + }) + + context("when called by the owner", () => { + let tx: ContractTransaction + + before(async () => { + await createSnapshot() + + tx = await walletCoordinator + .connect(owner) + .updateRedemptionProposalParameters(101, 102, 103, 104, 105) + }) + + after(async () => { + await restoreSnapshot() + }) + + it("should update redemption proposal parameters", async () => { + expect( + await walletCoordinator.redemptionProposalValidity() + ).to.be.equal(101) + expect(await walletCoordinator.redemptionRequestMinAge()).to.be.equal( + 102 + ) + expect( + await walletCoordinator.redemptionRequestTimeoutSafetyMargin() + ).to.be.equal(103) + expect(await walletCoordinator.redemptionMaxSize()).to.be.equal(104) + expect( + await walletCoordinator.redemptionProposalSubmissionGasOffset() + ).to.be.equal(105) + }) + + it("should emit the RedemptionProposalParametersUpdated event", async () => { + await expect(tx) + .to.emit(walletCoordinator, "RedemptionProposalParametersUpdated") + .withArgs(101, 102, 103, 104, 105) + }) + }) + }) + + describe("submitRedemptionProposal", () => { + const walletPubKeyHash = "0x7ac2d9378a1c47e589dfb8095ca95ed2140d2726" + + before(async () => { + await createSnapshot() + }) + + after(async () => { + await restoreSnapshot() + }) + + context("when the caller is not a coordinator", () => { + before(async () => { + await createSnapshot() + }) + + after(async () => { + await restoreSnapshot() + }) + + it("should revert", async () => { + const tx = walletCoordinator + .connect(thirdParty) + .submitRedemptionProposal({ + walletPubKeyHash, + redeemersOutputScripts: [ + "0x1976a9142cd680318747b720d67bf4246eb7403b476adb3488ac", + "0x160014e6f9d74726b19b75f16fe1e9feaec048aa4fa1d0", + ], + redemptionTxFee: 5000, + }) + + await expect(tx).to.be.revertedWith("Caller is not a coordinator") + }) + }) + + context("when the caller is a coordinator", () => { + before(async () => { + await createSnapshot() + + await walletCoordinator + .connect(owner) + .addCoordinator(thirdParty.address) + }) + + after(async () => { + await restoreSnapshot() + }) + + context("when wallet is time-locked", () => { + before(async () => { + await createSnapshot() + + // Submit a proposal to set a wallet time lock. + await walletCoordinator.connect(thirdParty).submitRedemptionProposal({ + walletPubKeyHash, + redeemersOutputScripts: [ + "0x1976a9142cd680318747b720d67bf4246eb7403b476adb3488ac", + "0x160014e6f9d74726b19b75f16fe1e9feaec048aa4fa1d0", + ], + redemptionTxFee: 5000, + }) + + // Jump to the end of the lock period but not beyond it. + await increaseTime( + (await walletCoordinator.redemptionProposalValidity()) - 1 + ) + }) + + after(async () => { + await restoreSnapshot() + }) + + it("should revert", async () => { + await expect( + walletCoordinator.connect(thirdParty).submitRedemptionProposal({ + walletPubKeyHash, + redeemersOutputScripts: [ + "0x1976a9142cd680318747b720d67bf4246eb7403b476adb3488ac", + "0x160014e6f9d74726b19b75f16fe1e9feaec048aa4fa1d0", + ], + redemptionTxFee: 5000, + }) + ).to.be.revertedWith("Wallet locked") + }) + }) + + context("when wallet is not time-locked", () => { + let tx: ContractTransaction + + before(async () => { + await createSnapshot() + + // Submit a proposal to set a wallet time lock. + await walletCoordinator.connect(thirdParty).submitRedemptionProposal({ + walletPubKeyHash, + redeemersOutputScripts: [ + "0x1976a9142cd680318747b720d67bf4246eb7403b476adb3488ac", + "0x160014e6f9d74726b19b75f16fe1e9feaec048aa4fa1d0", + ], + redemptionTxFee: 5000, + }) + + // Jump beyond the lock period. + await increaseTime( + await walletCoordinator.redemptionProposalValidity() + ) + + tx = await walletCoordinator + .connect(thirdParty) + .submitRedemptionProposal({ + walletPubKeyHash, + redeemersOutputScripts: [ + "0x1976a9142cd680318747b720d67bf4246eb7403b476adb3488ac", + "0x160014e6f9d74726b19b75f16fe1e9feaec048aa4fa1d0", + ], + redemptionTxFee: 6000, + }) + }) + + after(async () => { + await restoreSnapshot() + }) + + it("should time-lock the wallet", async () => { + const lockedUntil = + (await lastBlockTime()) + + (await walletCoordinator.redemptionProposalValidity()) + + const walletLock = await walletCoordinator.walletLock( + walletPubKeyHash + ) + + expect(walletLock.expiresAt).to.be.equal(lockedUntil) + expect(walletLock.cause).to.be.equal(walletAction.Redemption) + }) + + it("should emit the RedemptionProposalSubmitted event", async () => { + await expect(tx).to.emit( + walletCoordinator, + "RedemptionProposalSubmitted" + ) + + // The `expect.to.emit.withArgs` assertion has troubles with + // matching complex event arguments as it uses strict equality + // underneath. To overcome that problem, we manually get event's + // arguments and check it against the expected ones using deep + // equality assertion (eql). + const receipt = await ethers.provider.getTransactionReceipt(tx.hash) + expect(receipt.logs.length).to.be.equal(1) + expect( + walletCoordinator.interface.parseLog(receipt.logs[0]).args + ).to.be.eql([ + [ + walletPubKeyHash, + [ + "0x1976a9142cd680318747b720d67bf4246eb7403b476adb3488ac", + "0x160014e6f9d74726b19b75f16fe1e9feaec048aa4fa1d0", + ], + BigNumber.from(6000), + ], + thirdParty.address, + ]) + }) + }) + }) + }) + + describe("submitRedemptionProposalWithReimbursement", () => { + const walletPubKeyHash = "0x7ac2d9378a1c47e589dfb8095ca95ed2140d2726" + + before(async () => { + await createSnapshot() + }) + + after(async () => { + await restoreSnapshot() + }) + + // Just double check that `submitRedemptionProposalWithReimbursement` has + // the same ACL as `submitRedemptionProposal`. + context("when the caller is not a coordinator", () => { + before(async () => { + await createSnapshot() + }) + + after(async () => { + await restoreSnapshot() + }) + + it("should revert", async () => { + const tx = walletCoordinator + .connect(thirdParty) + .submitRedemptionProposalWithReimbursement({ + walletPubKeyHash, + redeemersOutputScripts: [ + "0x1976a9142cd680318747b720d67bf4246eb7403b476adb3488ac", + "0x160014e6f9d74726b19b75f16fe1e9feaec048aa4fa1d0", + ], + redemptionTxFee: 5000, + }) + + await expect(tx).to.be.revertedWith("Caller is not a coordinator") + }) + }) + + // Here we just check that the reimbursement works. Detailed + // assertions are already done within the scenario stressing the + // `submitRedemptionProposal` function. + context("when the caller is a coordinator", () => { + let coordinatorBalanceBefore: BigNumber + let coordinatorBalanceAfter: BigNumber + + before(async () => { + await createSnapshot() + + await walletCoordinator + .connect(owner) + .addCoordinator(thirdParty.address) + + // The first-ever proposal will be more expensive given it has to set + // fields to non-zero values. We shouldn't adjust gas offset based on it. + await walletCoordinator + .connect(thirdParty) + .submitRedemptionProposalWithReimbursement({ + walletPubKeyHash, + redeemersOutputScripts: [ + "0x1976a9142cd680318747b720d67bf4246eb7403b476adb3488ac", + "0x160014e6f9d74726b19b75f16fe1e9feaec048aa4fa1d0", + ], + redemptionTxFee: 5000, + }) + + // Jump beyond the lock period. + await increaseTime(await walletCoordinator.redemptionProposalValidity()) + + coordinatorBalanceBefore = await provider.getBalance(thirdParty.address) + + await walletCoordinator + .connect(thirdParty) + .submitRedemptionProposalWithReimbursement({ + walletPubKeyHash, + redeemersOutputScripts: [ + "0x1976a9142cd680318747b720d67bf4246eb7403b476adb3488ac", + "0x160014e6f9d74726b19b75f16fe1e9feaec048aa4fa1d0", + ], + redemptionTxFee: 5000, + }) + + coordinatorBalanceAfter = await provider.getBalance(thirdParty.address) + }) + + after(async () => { + await restoreSnapshot() + }) + + it("should do the refund", async () => { + const diff = coordinatorBalanceAfter.sub(coordinatorBalanceBefore) + expect(diff).to.be.gt(0) + expect(diff).to.be.lt(ethers.utils.parseUnits("4000000", "gwei")) // 0,004 ETH + }) + }) + }) + + describe("validateRedemptionProposal", () => { + const walletPubKeyHash = "0x7ac2d9378a1c47e589dfb8095ca95ed2140d2726" + const ecdsaWalletID = + "0x4ad6b3ccbca81645865d8d0d575797a15528e98ced22f29a6f906d3259569863" + + const bridgeRedemptionTxMaxTotalFee = 10000 + const bridgeRedemptionTimeout = 5 * 86400 // 5 days + + before(async () => { + await createSnapshot() + + bridge.redemptionParameters.returns([ + 0, + 0, + 0, + bridgeRedemptionTxMaxTotalFee, + bridgeRedemptionTimeout, + 0, + 0, + ]) + }) + + after(async () => { + bridge.redemptionParameters.reset() + + await restoreSnapshot() + }) + + context("when wallet is not Live", () => { + const testData = [ + { + testName: "when wallet state is Unknown", + walletState: walletState.Unknown, + }, + { + testName: "when wallet state is MovingFunds", + walletState: walletState.MovingFunds, + }, + { + 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, + 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( + // Only walletPubKeyHash argument is relevant in this scenario. + walletCoordinator.validateRedemptionProposal({ + walletPubKeyHash, + redeemersOutputScripts: [], + redemptionTxFee: 0, + }) + ).to.be.revertedWith("Wallet is not in Live state") + }) + }) + }) + }) + + context("when wallet is Live", () => { + before(async () => { + await createSnapshot() + + bridge.wallets.whenCalledWith(walletPubKeyHash).returns({ + ecdsaWalletID, + mainUtxoHash: HashZero, + pendingRedemptionsValue: 0, + createdAt: 0, + movingFundsRequestedAt: 0, + closingStartedAt: 0, + pendingMovedFundsSweepRequestsCount: 0, + state: walletState.Live, + movingFundsTargetWalletsCommitmentHash: HashZero, + }) + }) + + after(async () => { + bridge.wallets.reset() + + await restoreSnapshot() + }) + + context("when redemption is below the min size", () => { + it("should revert", async () => { + await expect( + walletCoordinator.validateRedemptionProposal({ + walletPubKeyHash, + redeemersOutputScripts: [], // Set size to 0. + redemptionTxFee: 0, // Not relevant in this scenario. + }) + ).to.be.revertedWith("Redemption below the min size") + }) + }) + + context("when redemption is above the min size", () => { + context("when redemption exceeds the max size", () => { + it("should revert", async () => { + const maxSize = await walletCoordinator.redemptionMaxSize() + + // Pick more redemption requests than allowed. + const redeemersOutputScripts = new Array(maxSize + 1).fill( + createTestRedemptionRequest(walletPubKeyHash).key + .redeemerOutputScript + ) + + await expect( + walletCoordinator.validateRedemptionProposal({ + walletPubKeyHash, + redeemersOutputScripts, + redemptionTxFee: 0, // Not relevant in this scenario. + }) + ).to.be.revertedWith("Redemption exceeds the max size") + }) + }) + + context("when redemption does not exceed the max size", () => { + context("when proposed redemption tx fee is invalid", () => { + context("when proposed redemption tx fee is zero", () => { + it("should revert", async () => { + await expect( + walletCoordinator.validateRedemptionProposal({ + walletPubKeyHash, + redeemersOutputScripts: [ + createTestRedemptionRequest(walletPubKeyHash).key + .redeemerOutputScript, + ], + redemptionTxFee: 0, + }) + ).to.be.revertedWith("Proposed transaction fee cannot be zero") + }) + }) + + context( + "when proposed redemption tx fee is greater than the allowed total fee", + () => { + it("should revert", async () => { + await expect( + walletCoordinator.validateRedemptionProposal({ + walletPubKeyHash, + redeemersOutputScripts: [ + createTestRedemptionRequest(walletPubKeyHash).key + .redeemerOutputScript, + ], + // Exceed the max per-request fee by one. + redemptionTxFee: bridgeRedemptionTxMaxTotalFee + 1, + }) + ).to.be.revertedWith("Proposed transaction fee is too high") + }) + } + ) + + // The context block covering the per-redemption fee checks is + // declared at the end of the `validateRedemptionProposal` test suite + // due to the actual order of checks performed by this function. + // See: "when there is a request that incurs an unacceptable tx fee share" + }) + + context("when proposed redemption tx fee is valid", () => { + const redemptionTxFee = 9000 + + context("when there is a non-pending request", () => { + let requestOne + let requestTwo + + before(async () => { + await createSnapshot() + + requestOne = createTestRedemptionRequest( + walletPubKeyHash, + 5000 // necessary to pass the fee share validation + ) + requestTwo = createTestRedemptionRequest(walletPubKeyHash) + + // Request one is a proper one. + bridge.pendingRedemptions + .whenCalledWith( + redemptionKey( + requestOne.key.walletPubKeyHash, + requestOne.key.redeemerOutputScript + ) + ) + .returns(requestOne.content) + + // Simulate the request two is non-pending. + bridge.pendingRedemptions + .whenCalledWith( + redemptionKey( + requestTwo.key.walletPubKeyHash, + requestTwo.key.redeemerOutputScript + ) + ) + .returns({ + ...requestTwo.content, + requestedAt: 0, + }) + }) + + after(async () => { + bridge.pendingRedemptions.reset() + + await restoreSnapshot() + }) + + it("should revert", async () => { + const proposal = { + walletPubKeyHash, + redeemersOutputScripts: [ + requestOne.key.redeemerOutputScript, + requestTwo.key.redeemerOutputScript, + ], + redemptionTxFee, + } + + await expect( + walletCoordinator.validateRedemptionProposal(proposal) + ).to.be.revertedWith("Not a pending redemption request") + }) + }) + + context("when all requests are pending", () => { + context("when there is an immature request", () => { + let requestOne + let requestTwo + + before(async () => { + await createSnapshot() + + requestOne = createTestRedemptionRequest( + walletPubKeyHash, + 5000 // necessary to pass the fee share validation + ) + requestTwo = createTestRedemptionRequest(walletPubKeyHash) + + // Request one is a proper one. + bridge.pendingRedemptions + .whenCalledWith( + redemptionKey( + requestOne.key.walletPubKeyHash, + requestOne.key.redeemerOutputScript + ) + ) + .returns(requestOne.content) + + // Simulate the request two has just been created thus not + // achieved the min age yet. + bridge.pendingRedemptions + .whenCalledWith( + redemptionKey( + requestTwo.key.walletPubKeyHash, + requestTwo.key.redeemerOutputScript + ) + ) + .returns({ + ...requestTwo.content, + requestedAt: await lastBlockTime(), + }) + }) + + after(async () => { + bridge.pendingRedemptions.reset() + + await restoreSnapshot() + }) + + it("should revert", async () => { + const proposal = { + walletPubKeyHash, + redeemersOutputScripts: [ + requestOne.key.redeemerOutputScript, + requestTwo.key.redeemerOutputScript, + ], + redemptionTxFee, + } + + await expect( + walletCoordinator.validateRedemptionProposal(proposal) + ).to.be.revertedWith( + "Redemption request min age not achieved yet" + ) + }) + }) + + context("when all requests achieved the min age", () => { + context( + "when there is a request that violates the timeout safety margin", + () => { + let requestOne + let requestTwo + + before(async () => { + await createSnapshot() - before(async () => { - await createSnapshot() + // Request one is a proper one. + requestOne = createTestRedemptionRequest( + walletPubKeyHash, + 5000 // necessary to pass the fee share validation + ) - depositOne = createTestDeposit( - walletPubKeyHash, - vault, - true - ) + // Simulate that request two violates the timeout safety margin. + // In order to do so, we need to use `createTestRedemptionRequest` + // with a custom request creation time that will produce + // a timeout timestamp being closer to the current + // moment than allowed by the refund safety margin. + const safetyMarginViolatedAt = await lastBlockTime() + const requestTimedOutAt = + safetyMarginViolatedAt + + (await walletCoordinator.redemptionRequestTimeoutSafetyMargin()) + const requestCreatedAt = + requestTimedOutAt - bridgeRedemptionTimeout + + requestTwo = createTestRedemptionRequest( + walletPubKeyHash, + 0, + requestCreatedAt + ) - depositTwo = createTestDeposit( - walletPubKeyHash, - vault, - false - ) + bridge.pendingRedemptions + .whenCalledWith( + redemptionKey( + requestOne.key.walletPubKeyHash, + requestOne.key.redeemerOutputScript + ) + ) + .returns(requestOne.content) - bridge.deposits - .whenCalledWith( - depositKey( - depositOne.key.fundingTxHash, - depositOne.key.fundingOutputIndex - ) - ) - .returns(depositOne.request) + bridge.pendingRedemptions + .whenCalledWith( + redemptionKey( + requestTwo.key.walletPubKeyHash, + requestTwo.key.redeemerOutputScript + ) + ) + .returns(requestTwo.content) + }) - bridge.deposits - .whenCalledWith( - depositKey( - depositTwo.key.fundingTxHash, - depositTwo.key.fundingOutputIndex - ) - ) - .returns(depositTwo.request) - }) + after(async () => { + bridge.pendingRedemptions.reset() - after(async () => { - bridge.deposits.reset() + await restoreSnapshot() + }) - await restoreSnapshot() - }) + it("should revert", async () => { + const proposal = { + walletPubKeyHash, + redeemersOutputScripts: [ + requestOne.key.redeemerOutputScript, + requestTwo.key.redeemerOutputScript, + ], + redemptionTxFee, + } - it("should succeed", async () => { - const proposal = { - walletPubKeyHash, - depositsKeys: [ - depositOne.key, - depositTwo.key, - ], - sweepTxFee, - depositsRevealBlocks: [], // Not relevant in this scenario. - } + await expect( + walletCoordinator.validateRedemptionProposal(proposal) + ).to.be.revertedWith( + "Redemption request timeout safety margin is not preserved" + ) + }) + } + ) - const depositsExtraInfo = [ - depositOne.extraInfo, - depositTwo.extraInfo, - ] + context( + "when all requests preserve the timeout safety margin", + () => { + context( + "when there is a request that incurs an unacceptable tx fee share", + () => { + context("when there is no fee remainder", () => { + let requestOne + let requestTwo - const result = - await walletCoordinator.validateDepositSweepProposal( - proposal, - depositsExtraInfo - ) + before(async () => { + await createSnapshot() - // eslint-disable-next-line @typescript-eslint/no-unused-expressions - expect(result).to.be.true - }) - } + // Request one is a proper one. + requestOne = createTestRedemptionRequest( + walletPubKeyHash, + 4500 // necessary to pass the fee share validation + ) + + // Simulate that request two takes an unacceptable + // tx fee share. Because redemptionTxFee used + // in the proposal is 9000, the actual fee share + // per-request is 4500. In order to test this case + // the second request must allow for 4499 as allowed + // fee share at maximum. + requestTwo = createTestRedemptionRequest( + walletPubKeyHash, + 4499 + ) + + bridge.pendingRedemptions + .whenCalledWith( + redemptionKey( + requestOne.key.walletPubKeyHash, + requestOne.key.redeemerOutputScript + ) ) + .returns(requestOne.content) + + bridge.pendingRedemptions + .whenCalledWith( + redemptionKey( + requestTwo.key.walletPubKeyHash, + requestTwo.key.redeemerOutputScript + ) + ) + .returns(requestTwo.content) + }) + + after(async () => { + bridge.pendingRedemptions.reset() + + await restoreSnapshot() + }) + + it("should revert", async () => { + const proposal = { + walletPubKeyHash, + redeemersOutputScripts: [ + requestOne.key.redeemerOutputScript, + requestTwo.key.redeemerOutputScript, + ], + redemptionTxFee, } - ) - } - ) - }) - }) - }) + + await expect( + walletCoordinator.validateRedemptionProposal( + proposal + ) + ).to.be.revertedWith( + "Proposed transaction per-request fee share is too high" + ) + }) + }) + + context("when there is a fee remainder", () => { + let requestOne + let requestTwo + + before(async () => { + await createSnapshot() + + // Request one is a proper one. + requestOne = createTestRedemptionRequest( + walletPubKeyHash, + 4500 // necessary to pass the fee share validation + ) + + // Simulate that request two takes an unacceptable + // tx fee share. Because redemptionTxFee used + // in the proposal is 9001, the actual fee share + // per-request is 4500 and 4501 for the last request + // which takes the remainder. In order to test this + // case the second (last) request must allow for + // 4500 as allowed fee share at maximum. + requestTwo = createTestRedemptionRequest( + walletPubKeyHash, + 4500 + ) + + bridge.pendingRedemptions + .whenCalledWith( + redemptionKey( + requestOne.key.walletPubKeyHash, + requestOne.key.redeemerOutputScript + ) + ) + .returns(requestOne.content) + + bridge.pendingRedemptions + .whenCalledWith( + redemptionKey( + requestTwo.key.walletPubKeyHash, + requestTwo.key.redeemerOutputScript + ) + ) + .returns(requestTwo.content) + }) + + after(async () => { + bridge.pendingRedemptions.reset() + + await restoreSnapshot() + }) + + it("should revert", async () => { + const proposal = { + walletPubKeyHash, + redeemersOutputScripts: [ + requestOne.key.redeemerOutputScript, + requestTwo.key.redeemerOutputScript, + ], + redemptionTxFee: 9001, + } + + await expect( + walletCoordinator.validateRedemptionProposal( + proposal + ) + ).to.be.revertedWith( + "Proposed transaction per-request fee share is too high" + ) + }) + }) + } + ) + + context( + "when all requests incur an acceptable tx fee share", + () => { + context("when there are duplicated requests", () => { + let requestOne + let requestTwo + let requestThree + + before(async () => { + await createSnapshot() + + requestOne = createTestRedemptionRequest( + walletPubKeyHash, + 2500 // necessary to pass the fee share validation + ) + + requestTwo = createTestRedemptionRequest( + walletPubKeyHash, + 2500 // necessary to pass the fee share validation + ) + + requestThree = createTestRedemptionRequest( + walletPubKeyHash, + 2500 // necessary to pass the fee share validation + ) + + bridge.pendingRedemptions + .whenCalledWith( + redemptionKey( + requestOne.key.walletPubKeyHash, + requestOne.key.redeemerOutputScript + ) + ) + .returns(requestOne.content) + + bridge.pendingRedemptions + .whenCalledWith( + redemptionKey( + requestTwo.key.walletPubKeyHash, + requestTwo.key.redeemerOutputScript + ) + ) + .returns(requestTwo.content) + + bridge.pendingRedemptions + .whenCalledWith( + redemptionKey( + requestThree.key.walletPubKeyHash, + requestThree.key.redeemerOutputScript + ) + ) + .returns(requestThree.content) + }) + + after(async () => { + bridge.pendingRedemptions.reset() + + await restoreSnapshot() + }) + + it("should revert", async () => { + const proposal = { + walletPubKeyHash, + redeemersOutputScripts: [ + requestOne.key.redeemerOutputScript, + requestTwo.key.redeemerOutputScript, + requestThree.key.redeemerOutputScript, + requestTwo.key.redeemerOutputScript, // duplicate + ], + redemptionTxFee, + } + + await expect( + walletCoordinator.validateRedemptionProposal( + proposal + ) + ).to.be.revertedWith("Duplicated request") + }) + }) + + context("when all requests are unique", () => { + let requestOne + let requestTwo + + before(async () => { + await createSnapshot() + + requestOne = createTestRedemptionRequest( + walletPubKeyHash, + 5000 // necessary to pass the fee share validation + ) + + requestTwo = createTestRedemptionRequest( + walletPubKeyHash, + 5000 // necessary to pass the fee share validation + ) + + bridge.pendingRedemptions + .whenCalledWith( + redemptionKey( + requestOne.key.walletPubKeyHash, + requestOne.key.redeemerOutputScript + ) + ) + .returns(requestOne.content) + + bridge.pendingRedemptions + .whenCalledWith( + redemptionKey( + requestTwo.key.walletPubKeyHash, + requestTwo.key.redeemerOutputScript + ) + ) + .returns(requestTwo.content) + }) + + after(async () => { + bridge.pendingRedemptions.reset() + + await restoreSnapshot() + }) + + it("should succeed", async () => { + const proposal = { + walletPubKeyHash, + redeemersOutputScripts: [ + requestOne.key.redeemerOutputScript, + requestTwo.key.redeemerOutputScript, + ], + redemptionTxFee, + } + + const result = + await walletCoordinator.validateRedemptionProposal( + proposal + ) + + // eslint-disable-next-line @typescript-eslint/no-unused-expressions + expect(result).to.be.true + }) + }) + } + ) + } + ) }) }) }) @@ -2142,3 +3255,51 @@ const createTestDeposit = ( }, } } + +const redemptionKey = ( + walletPubKeyHash: BytesLike, + redeemerOutputScript: BytesLike +) => { + const scriptHash = ethers.utils.solidityKeccak256( + ["bytes"], + [redeemerOutputScript] + ) + + return ethers.utils.solidityKeccak256( + ["bytes32", "bytes20"], + [scriptHash, walletPubKeyHash] + ) +} + +const createTestRedemptionRequest = ( + walletPubKeyHash: string, + txMaxFee?: BigNumberish, + requestedAt?: number +) => { + let resolvedRequestedAt = requestedAt + + if (!resolvedRequestedAt) { + // If the request creation time is not explicitly set, use `now - 1 day` to + // ensure request minimum age is achieved by default. + const now = Math.floor(Date.now() / 1000) + resolvedRequestedAt = now - day + } + + const redeemer = `0x${crypto.randomBytes(20).toString("hex")}` + + const redeemerOutputScript = `0x${crypto.randomBytes(32).toString("hex")}` + + return { + key: { + walletPubKeyHash, + redeemerOutputScript, + }, + content: { + redeemer, + requestedAmount: 0, // not relevant + treasuryFee: 0, // not relevant + txMaxFee: txMaxFee ?? 0, + requestedAt: resolvedRequestedAt, + }, + } +}