diff --git a/solidity/contracts/bridge/Bridge.sol b/solidity/contracts/bridge/Bridge.sol index f4506e83f..f1395e04e 100644 --- a/solidity/contracts/bridge/Bridge.sol +++ b/solidity/contracts/bridge/Bridge.sol @@ -376,6 +376,35 @@ contract Bridge is self.revealDeposit(fundingTx, reveal); } + /// @notice Sibling of the `revealDeposit` function. This function allows + /// to reveal a P2(W)SH Bitcoin deposit with 32-byte extra data + /// embedded in the deposit script. The extra data allows to + /// attach additional context to the deposit. For example, + /// it allows a third-party smart contract to reveal the + /// deposit on behalf of the original depositor and provide + /// additional services once the deposit is handled. In this + /// case, the address of the original depositor can be encoded + /// as extra data. + /// @param fundingTx Bitcoin funding transaction data, see `BitcoinTx.Info`. + /// @param reveal Deposit reveal data, see `RevealInfo struct. + /// @param extraData 32-byte deposit extra data. + /// @dev Requirements: + /// - All requirements from `revealDeposit` function must be met, + /// - `extraData` must not be bytes32(0), + /// - `extraData` must be the actual extra data used in the P2(W)SH + /// BTC deposit transaction. + /// + /// If any of these requirements is not met, the wallet _must_ refuse + /// to sweep the deposit and the depositor has to wait until the + /// deposit script unlocks to receive their BTC back. + function revealDepositWithExtraData( + BitcoinTx.Info calldata fundingTx, + Deposit.DepositRevealInfo calldata reveal, + bytes32 extraData + ) external { + self.revealDepositWithExtraData(fundingTx, reveal, extraData); + } + /// @notice Used by the wallet to prove the BTC deposit sweep transaction /// and to update Bank balances accordingly. Sweep is only accepted /// if it satisfies SPV proof. diff --git a/solidity/contracts/bridge/Deposit.sol b/solidity/contracts/bridge/Deposit.sol index 0fb51d785..a0b2d9d3e 100644 --- a/solidity/contracts/bridge/Deposit.sol +++ b/solidity/contracts/bridge/Deposit.sol @@ -44,6 +44,25 @@ import "./Wallets.sol"; /// Since each depositor has their own Ethereum address and their own /// blinding factor, each depositor’s script is unique, and the hash /// of each depositor’s script is unique. +/// +/// This library also supports another variant of the deposit script +/// allowing to embed 32-byte extra data. The extra data allows to attach +/// additional context to the deposit. The script with 32-byte extra data +/// looks like this: +/// +/// ``` +/// DROP +/// DROP +/// DROP +/// DUP HASH160 EQUAL +/// IF +/// CHECKSIG +/// ELSE +/// DUP HASH160 EQUALVERIFY +/// CHECKLOCKTIMEVERIFY DROP +/// CHECKSIG +/// ENDIF +/// ``` library Deposit { using BTCUtils for bytes; using BytesLib for bytes; @@ -96,6 +115,8 @@ library Deposit { // the time when the sweep proof was delivered to the Ethereum chain. // XXX: Unsigned 32-bit int unix seconds, will break February 7th 2106. uint32 sweptAt; + // The 32-byte deposit extra data. Optional, can be bytes32(0). + bytes32 extraData; // This struct doesn't contain `__gap` property as the structure is stored // in a mapping, mappings store values in different slots and they are // not contiguous with other values. @@ -152,6 +173,28 @@ library Deposit { BitcoinTx.Info calldata fundingTx, DepositRevealInfo calldata reveal ) external { + _revealDeposit(self, fundingTx, reveal, bytes32(0)); + } + + /// @notice Internal function encapsulating the core logic of the deposit + /// reveal process. Handles both regular deposits without extra data + /// as well as deposits with 32-byte extra data embedded in the + /// deposit script. The behavior is controlled by the `extraData` + /// parameter. If `extraData` is bytes32(0), the function triggers + /// the flow for regular deposits. If `extraData` is not bytes32(0), + /// the function triggers the flow for deposits with 32-byte + /// extra data. + /// @param fundingTx Bitcoin funding transaction data, see `BitcoinTx.Info`. + /// @param reveal Deposit reveal data, see `RevealInfo struct. + /// @param extraData 32-byte deposit extra data. Can be bytes32(0). + /// @dev Requirements are described in the docstrings of `revealDeposit` and + /// `revealDepositWithExtraData` external functions. + function _revealDeposit( + BridgeState.Storage storage self, + BitcoinTx.Info calldata fundingTx, + DepositRevealInfo calldata reveal, + bytes32 extraData + ) internal { require( self.registeredWallets[reveal.walletPubKeyHash].state == Wallets.WalletState.Live, @@ -167,33 +210,70 @@ library Deposit { validateDepositRefundLocktime(self, reveal.refundLocktime); } - bytes memory expectedScript = abi.encodePacked( - hex"14", // Byte length of depositor Ethereum address. - msg.sender, - hex"75", // OP_DROP - hex"08", // Byte length of blinding factor value. - reveal.blindingFactor, - hex"75", // OP_DROP - hex"76", // OP_DUP - hex"a9", // OP_HASH160 - hex"14", // Byte length of a compressed Bitcoin public key hash. - reveal.walletPubKeyHash, - hex"87", // OP_EQUAL - hex"63", // OP_IF - hex"ac", // OP_CHECKSIG - hex"67", // OP_ELSE - hex"76", // OP_DUP - hex"a9", // OP_HASH160 - hex"14", // Byte length of a compressed Bitcoin public key hash. - reveal.refundPubKeyHash, - hex"88", // OP_EQUALVERIFY - hex"04", // Byte length of refund locktime value. - reveal.refundLocktime, - hex"b1", // OP_CHECKLOCKTIMEVERIFY - hex"75", // OP_DROP - hex"ac", // OP_CHECKSIG - hex"68" // OP_ENDIF - ); + bytes memory expectedScript; + + if (extraData == bytes32(0)) { + // Regular deposit without 32-byte extra data. + expectedScript = abi.encodePacked( + hex"14", // Byte length of depositor Ethereum address. + msg.sender, + hex"75", // OP_DROP + hex"08", // Byte length of blinding factor value. + reveal.blindingFactor, + hex"75", // OP_DROP + hex"76", // OP_DUP + hex"a9", // OP_HASH160 + hex"14", // Byte length of a compressed Bitcoin public key hash. + reveal.walletPubKeyHash, + hex"87", // OP_EQUAL + hex"63", // OP_IF + hex"ac", // OP_CHECKSIG + hex"67", // OP_ELSE + hex"76", // OP_DUP + hex"a9", // OP_HASH160 + hex"14", // Byte length of a compressed Bitcoin public key hash. + reveal.refundPubKeyHash, + hex"88", // OP_EQUALVERIFY + hex"04", // Byte length of refund locktime value. + reveal.refundLocktime, + hex"b1", // OP_CHECKLOCKTIMEVERIFY + hex"75", // OP_DROP + hex"ac", // OP_CHECKSIG + hex"68" // OP_ENDIF + ); + } else { + // Deposit with 32-byte extra data. + expectedScript = abi.encodePacked( + hex"14", // Byte length of depositor Ethereum address. + msg.sender, + hex"75", // OP_DROP + hex"20", // Byte length of extra data. + extraData, + hex"75", // OP_DROP + hex"08", // Byte length of blinding factor value. + reveal.blindingFactor, + hex"75", // OP_DROP + hex"76", // OP_DUP + hex"a9", // OP_HASH160 + hex"14", // Byte length of a compressed Bitcoin public key hash. + reveal.walletPubKeyHash, + hex"87", // OP_EQUAL + hex"63", // OP_IF + hex"ac", // OP_CHECKSIG + hex"67", // OP_ELSE + hex"76", // OP_DUP + hex"a9", // OP_HASH160 + hex"14", // Byte length of a compressed Bitcoin public key hash. + reveal.refundPubKeyHash, + hex"88", // OP_EQUALVERIFY + hex"04", // Byte length of refund locktime value. + reveal.refundLocktime, + hex"b1", // OP_CHECKLOCKTIMEVERIFY + hex"75", // OP_DROP + hex"ac", // OP_CHECKSIG + hex"68" // OP_ENDIF + ); + } bytes memory fundingOutput = fundingTx .outputVector @@ -258,6 +338,21 @@ library Deposit { deposit.treasuryFee = self.depositTreasuryFeeDivisor > 0 ? fundingOutputAmount / self.depositTreasuryFeeDivisor : 0; + deposit.extraData = extraData; + + _emitDepositRevealedEvent(fundingTxHash, fundingOutputAmount, reveal); + } + + /// @notice Emits the `DepositRevealed` event. + /// @param fundingTxHash The funding transaction hash. + /// @param fundingOutputAmount The funding output amount in satoshi. + /// @param reveal Deposit reveal data, see `RevealInfo struct. + /// @dev This function is extracted to overcome the stack too deep error. + function _emitDepositRevealedEvent( + bytes32 fundingTxHash, + uint64 fundingOutputAmount, + DepositRevealInfo calldata reveal + ) internal { // slither-disable-next-line reentrancy-events emit DepositRevealed( fundingTxHash, @@ -272,6 +367,40 @@ library Deposit { ); } + /// @notice Sibling of the `revealDeposit` function. This function allows + /// to reveal a P2(W)SH Bitcoin deposit with 32-byte extra data + /// embedded in the deposit script. The extra data allows to + /// attach additional context to the deposit. For example, + /// it allows a third-party smart contract to reveal the + /// deposit on behalf of the original depositor and provide + /// additional services once the deposit is handled. In this + /// case, the address of the original depositor can be encoded + /// as extra data. + /// @param fundingTx Bitcoin funding transaction data, see `BitcoinTx.Info`. + /// @param reveal Deposit reveal data, see `RevealInfo struct. + /// @param extraData 32-byte deposit extra data. + /// @dev Requirements: + /// - All requirements from `revealDeposit` function must be met, + /// - `extraData` must not be bytes32(0), + /// - `extraData` must be the actual extra data used in the P2(W)SH + /// BTC deposit transaction. + /// + /// If any of these requirements is not met, the wallet _must_ refuse + /// to sweep the deposit and the depositor has to wait until the + /// deposit script unlocks to receive their BTC back. + function revealDepositWithExtraData( + BridgeState.Storage storage self, + BitcoinTx.Info calldata fundingTx, + DepositRevealInfo calldata reveal, + bytes32 extraData + ) external { + // Strong requirement in order to differentiate from the regular + // reveal flow and reduce potential attack surface. + require(extraData != bytes32(0), "Extra data must not be empty"); + + _revealDeposit(self, fundingTx, reveal, extraData); + } + /// @notice Validates the deposit refund locktime. The validation passes /// successfully only if the deposit reveal is done respectively /// earlier than the moment when the deposit refund locktime is diff --git a/solidity/contracts/bridge/WalletProposalValidator.sol b/solidity/contracts/bridge/WalletProposalValidator.sol index d84866155..ab1d400e1 100644 --- a/solidity/contracts/bridge/WalletProposalValidator.sol +++ b/solidity/contracts/bridge/WalletProposalValidator.sol @@ -66,7 +66,7 @@ contract WalletProposalValidator { uint32 fundingOutputIndex; } - /// @notice Helper structure holding deposit extra data required during + /// @notice Helper structure holding deposit extra info required during /// deposit sweep proposal validation. Basically, this structure /// is a combination of BitcoinTx.Info and relevant parts of /// Deposit.DepositRevealInfo. @@ -213,7 +213,7 @@ contract WalletProposalValidator { /// complexity. Instead of that, each off-chain wallet member is /// supposed to do that check on their own. /// @param proposal The sweeping proposal to validate. - /// @param depositsExtraInfo Deposits extra data required to perform the validation. + /// @param depositsExtraInfo Deposits extra info required to perform the validation. /// @return True if the proposal is valid. Reverts otherwise. /// @dev Requirements: /// - The target wallet must be in the Live state, @@ -221,7 +221,7 @@ contract WalletProposalValidator { /// the range [1, `DEPOSIT_SWEEP_MAX_SIZE`], /// - The length of `depositsExtraInfo` array must be equal to the /// length of `proposal.depositsKeys`, i.e. each deposit must - /// have exactly one set of corresponding extra data, + /// have exactly one set of corresponding extra info, /// - The proposed sweep tx fee must be grater than zero, /// - The proposed maximum per-deposit sweep tx fee must be lesser than /// or equal the maximum fee allowed by the Bridge (`Bridge.depositTxMaxFee`), @@ -229,7 +229,7 @@ contract WalletProposalValidator { /// - Each deposit must be old enough, i.e. at least `DEPOSIT_MIN_AGE /// elapsed since their reveal time, /// - Each deposit must not be swept yet, - /// - Each deposit must have valid extra data (see `validateDepositExtraInfo`), + /// - Each deposit must have valid extra info (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, @@ -257,7 +257,7 @@ contract WalletProposalValidator { require( proposal.depositsKeys.length == depositsExtraInfo.length, - "Each deposit key must have matching extra data" + "Each deposit key must have matching extra info" ); validateSweepTxFee(proposal.sweepTxFee, proposal.depositsKeys.length); @@ -299,6 +299,7 @@ contract WalletProposalValidator { validateDepositExtraInfo( depositKey, depositRequest.depositor, + depositRequest.extraData, depositExtraInfo ); @@ -378,11 +379,12 @@ contract WalletProposalValidator { ); } - /// @notice Validates the extra data for the given deposit. This function + /// @notice Validates the extra info for the given deposit. This function /// is heavily based on `Deposit.revealDeposit` function. /// @param depositKey Key of the given deposit. /// @param depositor Depositor that revealed the deposit. - /// @param depositExtraInfo Extra data being subject of the validation. + /// @param extraData 32-byte deposit extra data. Optional, can be bytes32(0). + /// @param depositExtraInfo Extra info being subject of the validation. /// @dev Requirements: /// - The transaction hash computed using `depositExtraInfo.fundingTx` /// must match the `depositKey.fundingTxHash`. This requirement @@ -392,11 +394,12 @@ contract WalletProposalValidator { /// - The P2(W)SH script inferred from `depositExtraInfo` is actually /// used to lock funds by the `depositKey.fundingOutputIndex` output /// of the `depositExtraInfo.fundingTx` transaction. This requirement - /// ensures the reveal data provided in the extra data container + /// ensures the reveal data provided in the extra info container /// actually matches the given deposit. function validateDepositExtraInfo( DepositKey memory depositKey, address depositor, + bytes32 extraData, DepositExtraInfo memory depositExtraInfo ) internal view { bytes32 depositExtraFundingTxHash = abi @@ -408,39 +411,76 @@ contract WalletProposalValidator { ) .hash256View(); - // Make sure the funding tx provided as part of deposit extra data + // Make sure the funding tx provided as part of deposit extra info // actually matches the deposit referred by the given deposit key. if (depositKey.fundingTxHash != depositExtraFundingTxHash) { revert("Extra info funding tx hash does not match"); } - bytes memory expectedScript = abi.encodePacked( - hex"14", // Byte length of depositor Ethereum address. - depositor, - hex"75", // OP_DROP - hex"08", // Byte length of blinding factor value. - depositExtraInfo.blindingFactor, - hex"75", // OP_DROP - hex"76", // OP_DUP - hex"a9", // OP_HASH160 - hex"14", // Byte length of a compressed Bitcoin public key hash. - depositExtraInfo.walletPubKeyHash, - hex"87", // OP_EQUAL - hex"63", // OP_IF - hex"ac", // OP_CHECKSIG - hex"67", // OP_ELSE - hex"76", // OP_DUP - hex"a9", // OP_HASH160 - hex"14", // Byte length of a compressed Bitcoin public key hash. - depositExtraInfo.refundPubKeyHash, - hex"88", // OP_EQUALVERIFY - hex"04", // Byte length of refund locktime value. - depositExtraInfo.refundLocktime, - hex"b1", // OP_CHECKLOCKTIMEVERIFY - hex"75", // OP_DROP - hex"ac", // OP_CHECKSIG - hex"68" // OP_ENDIF - ); + bytes memory expectedScript; + + if (extraData == bytes32(0)) { + // Regular deposit without 32-byte extra data. + expectedScript = abi.encodePacked( + hex"14", // Byte length of depositor Ethereum address. + depositor, + hex"75", // OP_DROP + hex"08", // Byte length of blinding factor value. + depositExtraInfo.blindingFactor, + hex"75", // OP_DROP + hex"76", // OP_DUP + hex"a9", // OP_HASH160 + hex"14", // Byte length of a compressed Bitcoin public key hash. + depositExtraInfo.walletPubKeyHash, + hex"87", // OP_EQUAL + hex"63", // OP_IF + hex"ac", // OP_CHECKSIG + hex"67", // OP_ELSE + hex"76", // OP_DUP + hex"a9", // OP_HASH160 + hex"14", // Byte length of a compressed Bitcoin public key hash. + depositExtraInfo.refundPubKeyHash, + hex"88", // OP_EQUALVERIFY + hex"04", // Byte length of refund locktime value. + depositExtraInfo.refundLocktime, + hex"b1", // OP_CHECKLOCKTIMEVERIFY + hex"75", // OP_DROP + hex"ac", // OP_CHECKSIG + hex"68" // OP_ENDIF + ); + } else { + // Deposit with 32-byte extra data. + expectedScript = abi.encodePacked( + hex"14", // Byte length of depositor Ethereum address. + depositor, + hex"75", // OP_DROP + hex"20", // Byte length of extra data. + extraData, + hex"75", // OP_DROP + hex"08", // Byte length of blinding factor value. + depositExtraInfo.blindingFactor, + hex"75", // OP_DROP + hex"76", // OP_DUP + hex"a9", // OP_HASH160 + hex"14", // Byte length of a compressed Bitcoin public key hash. + depositExtraInfo.walletPubKeyHash, + hex"87", // OP_EQUAL + hex"63", // OP_IF + hex"ac", // OP_CHECKSIG + hex"67", // OP_ELSE + hex"76", // OP_DUP + hex"a9", // OP_HASH160 + hex"14", // Byte length of a compressed Bitcoin public key hash. + depositExtraInfo.refundPubKeyHash, + hex"88", // OP_EQUALVERIFY + hex"04", // Byte length of refund locktime value. + depositExtraInfo.refundLocktime, + hex"b1", // OP_CHECKLOCKTIMEVERIFY + hex"75", // OP_DROP + hex"ac", // OP_CHECKSIG + hex"68" // OP_ENDIF + ); + } bytes memory fundingOutput = depositExtraInfo .fundingTx @@ -448,7 +488,7 @@ contract WalletProposalValidator { .extractOutputAtIndex(depositKey.fundingOutputIndex); bytes memory fundingOutputHash = fundingOutput.extractHash(); - // Path that checks the deposit extra data validity in case the + // Path that checks the deposit extra info validity in case the // referred deposit is a P2SH. if ( // slither-disable-next-line calls-loop @@ -458,7 +498,7 @@ contract WalletProposalValidator { return; } - // Path that checks the deposit extra data validity in case the + // Path that checks the deposit extra info validity in case the // referred deposit is a P2WSH. if ( fundingOutputHash.length == 32 && diff --git a/solidity/test/bridge/Bridge.Deposit.test.ts b/solidity/test/bridge/Bridge.Deposit.test.ts index 218f0b126..a15799eed 100644 --- a/solidity/test/bridge/Bridge.Deposit.test.ts +++ b/solidity/test/bridge/Bridge.Deposit.test.ts @@ -16,7 +16,10 @@ import type { IVault, BridgeGovernance, } from "../../typechain" -import type { DepositRevealInfoStruct } from "../../typechain/Bridge" +import type { + DepositRevealInfoStruct, + InfoStruct as BitcoinTxInfoStruct, +} from "../../typechain/Bridge" import bridgeFixture from "../fixtures/bridge" import { constants, walletState } from "../fixtures" import { @@ -74,12 +77,19 @@ describe("Bridge - Deposit", () => { await bridge.setDepositRevealAheadPeriod(0) }) - describe("revealDeposit", () => { + type RevealDepositFixture = { + P2SHFundingTx: BitcoinTxInfoStruct + P2WSHFundingTx: BitcoinTxInfoStruct + depositorAddress: string + reveal: DepositRevealInfoStruct + extraData?: string + } + + // Fixture used for revealDeposit test scenario. + const revealDepositFixture: RevealDepositFixture = { // Data of a proper P2SH deposit funding transaction. Little-endian hash is: - // 0x17350f81cdb61cd8d7014ad1507d4af8d032b75812cf88d2c636c1c022991af2 and - // this is the same as `expectedP2SHDeposit.transaction` mentioned in - // tbtc-ts/test/deposit.test.ts file. - const P2SHFundingTx = { + // 0x17350f81cdb61cd8d7014ad1507d4af8d032b75812cf88d2c636c1c022991af2 + P2SHFundingTx: { version: "0x01000000", inputVector: "0x018348cdeb551134fe1f19d378a8adec9b146671cb67b945b71bf56b20d" + @@ -89,13 +99,10 @@ describe("Bridge - Deposit", () => { "da455877ed73b00000000001600147ac2d9378a1c47e589dfb8095ca95ed2" + "140d2726", locktime: "0x00000000", - } - + }, // Data of a proper P2WSH deposit funding transaction. Little-endian hash is: - // 0x6a81de17ce3da1eadc833c5fd9d85dac307d3b78235f57afbcd9f068fc01b99e and - // this is the same as `expectedP2WSHDeposit.transaction` mentioned in - // tbtc-ts/test/deposit.test.ts file. - const P2WSHFundingTx = { + // 0x6a81de17ce3da1eadc833c5fd9d85dac307d3b78235f57afbcd9f068fc01b99e. + P2WSHFundingTx: { version: "0x01000000", inputVector: "0x018348cdeb551134fe1f19d378a8adec9b146671cb67b945b71bf56b20d" + @@ -105,12 +112,56 @@ describe("Bridge - Deposit", () => { "b87d2b6a37d6c3b64722be83c636f10d73b00000000001600147ac2d9378a" + "1c47e589dfb8095ca95ed2140d2726", locktime: "0x00000000", - } + }, + // Data matching the redeem script locking the funding output of + // P2SHFundingTx and P2WSHFundingTx. + depositorAddress: "0x934B98637cA318a4D6E7CA6ffd1690b8e77df637", + reveal: { + fundingOutputIndex: 0, + blindingFactor: "0xf9f0c90d00039523", + // HASH160 of 03989d253b17a6a0f41838b84ff0d20e8898f9d7b1a98f2564da4cc29dcf8581d9. + walletPubKeyHash: "0x8db50eb52063ea9d98b3eac91489a90f738986f6", + // HASH160 of 0300d6f28a2f6bf9836f57fcda5d284c9a8f849316119779f0d6090830d97763a9. + refundPubKeyHash: "0x28e081f285138ccbe389c1eb8985716230129f89", + refundLocktime: "0x60bcea61", + vault: "0x594cfd89700040163727828AE20B52099C58F02C", + }, + } + // Fixture used for revealDepositWithExtraData test scenario. + const revealDepositWithExtraDataFixture: RevealDepositFixture = { + // Data of a proper P2SH deposit funding transaction embedding some + // extra data. Little-endian hash is: + // 0x6383cd1829260b6034cd12bad36171748e8c3c6a8d57fcb6463c62f96116dfbc. + P2SHFundingTx: { + version: "0x01000000", + inputVector: + "0x018348cdeb551134fe1f19d378a8adec9b146671cb67b945b71bf56b20d" + + "c2b952f0100000000ffffffff", + outputVector: + "0x02102700000000000017a9149fe6615a307aa1d7eee668c1227802b2fbc" + + "aa919877ed73b00000000001600147ac2d9378a1c47e589dfb8095ca95ed2" + + "140d2726", + locktime: "0x00000000", + }, + // Data of a proper P2WSH deposit funding transaction embedding some + // extra data. Little-endian hash is: + // 0xc9312103d0d8d55344ef2d51acc409e004fbaaba7893b1725fa505ff73795732. + P2WSHFundingTx: { + version: "0x01000000", + inputVector: + "0x018348cdeb551134fe1f19d378a8adec9b146671cb67b945b71bf56b20d" + + "c2b952f0100000000ffffffff", + outputVector: + "0x021027000000000000220020bfaeddba12b0de6feeb649af76376876bc1" + + "feb6c2248fbfef9293ba3ac51bb4a10d73b00000000001600147ac2d9378a" + + "1c47e589dfb8095ca95ed2140d2726", + locktime: "0x00000000", + }, // Data matching the redeem script locking the funding output of // P2SHFundingTx and P2WSHFundingTx. - const depositorAddress = "0x934B98637cA318a4D6E7CA6ffd1690b8e77df637" - const reveal = { + depositorAddress: "0x934B98637cA318a4D6E7CA6ffd1690b8e77df637", + reveal: { fundingOutputIndex: 0, blindingFactor: "0xf9f0c90d00039523", // HASH160 of 03989d253b17a6a0f41838b84ff0d20e8898f9d7b1a98f2564da4cc29dcf8581d9. @@ -119,7 +170,15 @@ describe("Bridge - Deposit", () => { refundPubKeyHash: "0x28e081f285138ccbe389c1eb8985716230129f89", refundLocktime: "0x60bcea61", vault: "0x594cfd89700040163727828AE20B52099C58F02C", - } + }, + // sha256("fancy extra data") + extraData: + "0xa9b38ea6435c8941d6eda6a46b68e3e2117196995bd154ab55196396b03d9bda", + } + + describe("revealDeposit", () => { + const { P2SHFundingTx, P2WSHFundingTx, depositorAddress, reveal } = + revealDepositFixture let depositor: SignerWithAddress @@ -208,6 +267,10 @@ describe("Bridge - Deposit", () => { expect(deposit.treasuryFee).to.be.equal(5) // Swept time should be unset. expect(deposit.sweptAt).to.be.equal(0) + // Extra data must not be set. + expect(deposit.extraData).to.be.equal( + ethers.constants.HashZero + ) }) it("should emit DepositRevealed event", async () => { @@ -306,6 +369,11 @@ describe("Bridge - Deposit", () => { // value of the `depositTreasuryFeeDivisor`. // The divisor is 0 so the treasury fee is 0 as well. expect(deposit.treasuryFee).to.be.equal(0) + + // Extra data must not be set. + expect(deposit.extraData).to.be.equal( + ethers.constants.HashZero + ) }) it("should accept the deposit", async () => { @@ -425,6 +493,18 @@ describe("Bridge - Deposit", () => { }) } ) + + context("when funding transaction embeds extra data", () => { + it("should revert", async () => { + await expect( + bridge.connect(depositor).revealDeposit( + // Use a transaction that embeds extra data in the deposit script. + revealDepositWithExtraDataFixture.P2SHFundingTx, + reveal + ) + ).to.be.revertedWith("Wrong 20-byte script hash") + }) + }) }) context("when funding transaction is P2WSH", () => { @@ -476,6 +556,10 @@ describe("Bridge - Deposit", () => { expect(deposit.treasuryFee).to.be.equal(5) // Swept time should be unset. expect(deposit.sweptAt).to.be.equal(0) + // Extra data must not be set. + expect(deposit.extraData).to.be.equal( + ethers.constants.HashZero + ) }) it("should emit DepositRevealed event", async () => { @@ -607,6 +691,18 @@ describe("Bridge - Deposit", () => { }) } ) + + context("when funding transaction embeds extra data", () => { + it("should revert", async () => { + await expect( + bridge.connect(depositor).revealDeposit( + // Use a transaction that embeds extra data in the deposit script. + revealDepositWithExtraDataFixture.P2WSHFundingTx, + reveal + ) + ).to.be.revertedWith("Wrong 32-byte script hash") + }) + }) }) context("when funding transaction is neither P2SH nor P2WSH", () => { @@ -781,6 +877,856 @@ describe("Bridge - Deposit", () => { }) }) + describe("revealDepositWithExtraData", () => { + const { + P2SHFundingTx, + P2WSHFundingTx, + depositorAddress, + reveal, + extraData, + } = revealDepositWithExtraDataFixture + + let depositor: SignerWithAddress + + before(async () => { + depositor = await impersonateAccount(depositorAddress, { + from: governance, + value: 10, + }) + }) + + context("when extra data is non-zero", () => { + context("when wallet is in Live state", () => { + before(async () => { + await createSnapshot() + + await bridgeGovernance + .connect(governance) + .setVaultStatus(reveal.vault, true) + + // Simulate the wallet is a Live one and is known in the system. + await bridge.setWallet(reveal.walletPubKeyHash, { + ecdsaWalletID: ethers.constants.HashZero, + mainUtxoHash: ethers.constants.HashZero, + pendingRedemptionsValue: 0, + createdAt: await lastBlockTime(), + movingFundsRequestedAt: 0, + closingStartedAt: 0, + pendingMovedFundsSweepRequestsCount: 0, + state: walletState.Live, + movingFundsTargetWalletsCommitmentHash: ethers.constants.HashZero, + }) + }) + + after(async () => { + await restoreSnapshot() + }) + + context("when reveal ahead period validation is disabled", () => { + context("when funding transaction is P2SH", () => { + context("when funding output script hash is correct", () => { + context("when deposit was not revealed yet", () => { + context("when amount is not below the dust threshold", () => { + context("when deposit is routed to a trusted vault", () => { + let tx: ContractTransaction + + before(async () => { + await createSnapshot() + tx = await bridge + .connect(depositor) + .revealDepositWithExtraData( + P2SHFundingTx, + reveal, + extraData + ) + }) + + after(async () => { + await restoreSnapshot() + }) + + it("should store proper deposit data", async () => { + // Deposit key is keccak256(fundingTxHash | fundingOutputIndex). + const depositKey = ethers.utils.solidityKeccak256( + ["bytes32", "uint32"], + [ + "0x6383cd1829260b6034cd12bad36171748e8c3c6a8d57fcb6463c62f96116dfbc", + reveal.fundingOutputIndex, + ] + ) + + const deposit = await bridge.deposits(depositKey) + + // Depositor address, same as in `reveal.depositor`. + expect(deposit.depositor).to.be.equal( + "0x934B98637cA318a4D6E7CA6ffd1690b8e77df637" + ) + // Deposit amount in satoshi. In this case it's 10000 satoshi + // because the P2SH deposit transaction set this value for the + // funding output. + expect(deposit.amount).to.be.equal(10000) + // Revealed time should be set. + expect(deposit.revealedAt).to.be.equal( + await lastBlockTime() + ) + // Deposit vault, same as in `reveal.vault`. + expect(deposit.vault).to.be.equal( + "0x594cfd89700040163727828AE20B52099C58F02C" + ) + // Treasury fee should be computed according to the current + // value of the `depositTreasuryFeeDivisor`. + expect(deposit.treasuryFee).to.be.equal(5) + // Swept time should be unset. + expect(deposit.sweptAt).to.be.equal(0) + // Extra data must be set. + expect(deposit.extraData).to.be.equal(extraData) + }) + + it("should emit DepositRevealed event", async () => { + await expect(tx) + .to.emit(bridge, "DepositRevealed") + .withArgs( + "0x6383cd1829260b6034cd12bad36171748e8c3c6a8d57fcb6463c62f96116dfbc", + reveal.fundingOutputIndex, + "0x934B98637cA318a4D6E7CA6ffd1690b8e77df637", + 10000, + "0xf9f0c90d00039523", + "0x8db50eb52063ea9d98b3eac91489a90f738986f6", + "0x28e081f285138ccbe389c1eb8985716230129f89", + "0x60bcea61", + reveal.vault + ) + }) + }) + + context("when deposit is not routed to a vault", () => { + let tx: ContractTransaction + let nonRoutedReveal: DepositRevealInfoStruct + + before(async () => { + await createSnapshot() + + nonRoutedReveal = { ...reveal } + nonRoutedReveal.vault = ZERO_ADDRESS + tx = await bridge + .connect(depositor) + .revealDepositWithExtraData( + P2SHFundingTx, + nonRoutedReveal, + extraData + ) + }) + + after(async () => { + await restoreSnapshot() + }) + + it("should accept the deposit", async () => { + await expect(tx) + .to.emit(bridge, "DepositRevealed") + .withArgs( + "0x6383cd1829260b6034cd12bad36171748e8c3c6a8d57fcb6463c62f96116dfbc", + reveal.fundingOutputIndex, + "0x934B98637cA318a4D6E7CA6ffd1690b8e77df637", + 10000, + "0xf9f0c90d00039523", + "0x8db50eb52063ea9d98b3eac91489a90f738986f6", + "0x28e081f285138ccbe389c1eb8985716230129f89", + "0x60bcea61", + ZERO_ADDRESS + ) + }) + }) + + context("when deposit treasury fee is zero", () => { + let tx: ContractTransaction + + before(async () => { + await createSnapshot() + + await bridgeGovernance + .connect(governance) + .beginDepositTreasuryFeeDivisorUpdate(0) + await helpers.time.increaseTime(constants.governanceDelay) + await bridgeGovernance + .connect(governance) + .finalizeDepositTreasuryFeeDivisorUpdate() + + tx = await bridge + .connect(depositor) + .revealDepositWithExtraData( + P2SHFundingTx, + reveal, + extraData + ) + }) + + after(async () => { + await restoreSnapshot() + }) + + it("should store proper deposit data", async () => { + // Deposit key is keccak256(fundingTxHash | fundingOutputIndex). + const depositKey = ethers.utils.solidityKeccak256( + ["bytes32", "uint32"], + [ + "0x6383cd1829260b6034cd12bad36171748e8c3c6a8d57fcb6463c62f96116dfbc", + reveal.fundingOutputIndex, + ] + ) + + const deposit = await bridge.deposits(depositKey) + + // Deposit amount in satoshi. In this case it's 10000 satoshi + // because the P2SH deposit transaction set this value for the + // funding output. + expect(deposit.amount).to.be.equal(10000) + + // Treasury fee should be computed according to the current + // value of the `depositTreasuryFeeDivisor`. + // The divisor is 0 so the treasury fee is 0 as well. + expect(deposit.treasuryFee).to.be.equal(0) + // Extra data must be set. + expect(deposit.extraData).to.be.equal(extraData) + }) + + it("should accept the deposit", async () => { + await expect(tx) + .to.emit(bridge, "DepositRevealed") + .withArgs( + "0x6383cd1829260b6034cd12bad36171748e8c3c6a8d57fcb6463c62f96116dfbc", + reveal.fundingOutputIndex, + "0x934B98637cA318a4D6E7CA6ffd1690b8e77df637", + 10000, + "0xf9f0c90d00039523", + "0x8db50eb52063ea9d98b3eac91489a90f738986f6", + "0x28e081f285138ccbe389c1eb8985716230129f89", + "0x60bcea61", + reveal.vault + ) + }) + }) + + context( + "when deposit is routed to a non-trusted vault", + () => { + let nonTrustedVaultReveal + + before(async () => { + await createSnapshot() + + nonTrustedVaultReveal = { ...reveal } + nonTrustedVaultReveal.vault = + "0x92499afEAD6c41f757Ec3558D0f84bf7ec5aD967" + }) + + after(async () => { + await restoreSnapshot() + }) + + it("should revert", async () => { + await expect( + bridge + .connect(depositor) + .revealDepositWithExtraData( + P2SHFundingTx, + nonTrustedVaultReveal, + extraData + ) + ).to.be.revertedWith("Vault is not trusted") + }) + } + ) + }) + + context("when amount is below the dust threshold", () => { + before(async () => { + await createSnapshot() + + // The `P2SHFundingTx` used within this scenario has an output + // whose value is 10000 satoshi. To make the scenario happen, it + // is enough that the contract's deposit dust threshold is + // bigger by 1 satoshi. + await bridge.setDepositDustThreshold(10001) + }) + + after(async () => { + await restoreSnapshot() + }) + + it("should revert", async () => { + await expect( + bridge + .connect(depositor) + .revealDepositWithExtraData( + P2SHFundingTx, + reveal, + extraData + ) + ).to.be.revertedWith("Deposit amount too small") + }) + }) + }) + + context("when deposit was already revealed", () => { + before(async () => { + await createSnapshot() + + await bridge + .connect(depositor) + .revealDepositWithExtraData( + P2SHFundingTx, + reveal, + extraData + ) + }) + + after(async () => { + await restoreSnapshot() + }) + + it("should revert", async () => { + await expect( + bridge + .connect(depositor) + .revealDepositWithExtraData( + P2SHFundingTx, + reveal, + extraData + ) + ).to.be.revertedWith("Deposit already revealed") + }) + }) + }) + + context("when funding output script hash is wrong", () => { + it("should revert", async () => { + // Corrupt reveal data by setting a wrong blinding factor + const corruptedReveal = { ...reveal } + corruptedReveal.blindingFactor = "0xf9f0c90d00039524" + + await expect( + bridge + .connect(depositor) + .revealDepositWithExtraData( + P2SHFundingTx, + corruptedReveal, + extraData + ) + ).to.be.revertedWith("Wrong 20-byte script hash") + }) + }) + + context( + "when the caller address does not match the funding output script", + () => { + it("should revert", async () => { + const accounts = await getUnnamedAccounts() + const thirdParty = await ethers.getSigner(accounts[0]) + + await expect( + bridge + .connect(thirdParty) + .revealDepositWithExtraData( + P2SHFundingTx, + reveal, + extraData + ) + ).to.be.revertedWith("Wrong 20-byte script hash") + }) + } + ) + + context("when the revealed extra data do not match", () => { + it("should revert", async () => { + // Corrupt the extra data. + const corruptedExtraData = ethers.utils.keccak256(extraData) + + await expect( + bridge + .connect(depositor) + .revealDepositWithExtraData( + P2SHFundingTx, + reveal, + corruptedExtraData + ) + ).to.be.revertedWith("Wrong 20-byte script hash") + }) + }) + + context( + "when funding transaction does not embed extra data", + () => { + it("should revert", async () => { + await expect( + bridge.connect(depositor).revealDepositWithExtraData( + // Use a transaction that doesn't embed extra data in the deposit script. + revealDepositFixture.P2SHFundingTx, + reveal, + extraData + ) + ).to.be.revertedWith("Wrong 20-byte script hash") + }) + } + ) + }) + + context("when funding transaction is P2WSH", () => { + context("when funding output script hash is correct", () => { + context("when deposit was not revealed yet", () => { + context("when deposit is routed to a trusted vault", () => { + let tx: ContractTransaction + + before(async () => { + await createSnapshot() + + tx = await bridge + .connect(depositor) + .revealDepositWithExtraData( + P2WSHFundingTx, + reveal, + extraData + ) + }) + + after(async () => { + await restoreSnapshot() + }) + + it("should store proper deposit data", async () => { + // Deposit key is keccak256(fundingTxHash | fundingOutputIndex). + const depositKey = ethers.utils.solidityKeccak256( + ["bytes32", "uint32"], + [ + "0xc9312103d0d8d55344ef2d51acc409e004fbaaba7893b1725fa505ff73795732", + reveal.fundingOutputIndex, + ] + ) + + const deposit = await bridge.deposits(depositKey) + + // Depositor address, same as in `reveal.depositor`. + expect(deposit.depositor).to.be.equal( + "0x934B98637cA318a4D6E7CA6ffd1690b8e77df637" + ) + // Deposit amount in satoshi. In this case it's 10000 satoshi + // because the P2SH deposit transaction set this value for the + // funding output. + expect(deposit.amount).to.be.equal(10000) + // Revealed time should be set. + expect(deposit.revealedAt).to.be.equal( + await lastBlockTime() + ) + // Deposit vault, same as in `reveal.vault`. + expect(deposit.vault).to.be.equal( + "0x594cfd89700040163727828AE20B52099C58F02C" + ) + // Treasury fee should be computed according to the current + // value of the `depositTreasuryFeeDivisor`. + expect(deposit.treasuryFee).to.be.equal(5) + // Swept time should be unset. + expect(deposit.sweptAt).to.be.equal(0) + // Extra data must be set. + expect(deposit.extraData).to.be.equal(extraData) + }) + + it("should emit DepositRevealed event", async () => { + await expect(tx) + .to.emit(bridge, "DepositRevealed") + .withArgs( + "0xc9312103d0d8d55344ef2d51acc409e004fbaaba7893b1725fa505ff73795732", + reveal.fundingOutputIndex, + "0x934B98637cA318a4D6E7CA6ffd1690b8e77df637", + 10000, + "0xf9f0c90d00039523", + "0x8db50eb52063ea9d98b3eac91489a90f738986f6", + "0x28e081f285138ccbe389c1eb8985716230129f89", + "0x60bcea61", + reveal.vault + ) + }) + }) + + context("when deposit is not routed to a vault", () => { + let tx: ContractTransaction + let nonRoutedReveal: DepositRevealInfoStruct + + before(async () => { + await createSnapshot() + + nonRoutedReveal = { ...reveal } + nonRoutedReveal.vault = ZERO_ADDRESS + tx = await bridge + .connect(depositor) + .revealDepositWithExtraData( + P2WSHFundingTx, + nonRoutedReveal, + extraData + ) + }) + + after(async () => { + await restoreSnapshot() + }) + + it("should accept the deposit", async () => { + await expect(tx) + .to.emit(bridge, "DepositRevealed") + .withArgs( + "0xc9312103d0d8d55344ef2d51acc409e004fbaaba7893b1725fa505ff73795732", + reveal.fundingOutputIndex, + "0x934B98637cA318a4D6E7CA6ffd1690b8e77df637", + 10000, + "0xf9f0c90d00039523", + "0x8db50eb52063ea9d98b3eac91489a90f738986f6", + "0x28e081f285138ccbe389c1eb8985716230129f89", + "0x60bcea61", + ZERO_ADDRESS + ) + }) + }) + + context("when deposit is routed to a non-trusted vault", () => { + let nonTrustedVaultReveal + + before(async () => { + await createSnapshot() + + nonTrustedVaultReveal = { ...reveal } + nonTrustedVaultReveal.vault = + "0x92499afEAD6c41f757Ec3558D0f84bf7ec5aD967" + }) + + after(async () => { + await restoreSnapshot() + }) + + it("should revert", async () => { + await expect( + bridge + .connect(depositor) + .revealDepositWithExtraData( + P2WSHFundingTx, + nonTrustedVaultReveal, + extraData + ) + ).to.be.revertedWith("Vault is not trusted") + }) + }) + }) + + context("when deposit was already revealed", () => { + before(async () => { + await createSnapshot() + + await bridge + .connect(depositor) + .revealDepositWithExtraData( + P2WSHFundingTx, + reveal, + extraData + ) + }) + + after(async () => { + await restoreSnapshot() + }) + + it("should revert", async () => { + await expect( + bridge + .connect(depositor) + .revealDepositWithExtraData( + P2WSHFundingTx, + reveal, + extraData + ) + ).to.be.revertedWith("Deposit already revealed") + }) + }) + }) + + context("when funding output script hash is wrong", () => { + it("should revert", async () => { + // Corrupt reveal data by setting a wrong blinding factor + const corruptedReveal = { ...reveal } + corruptedReveal.blindingFactor = "0xf9f0c90d00039524" + + await expect( + bridge + .connect(depositor) + .revealDepositWithExtraData( + P2WSHFundingTx, + corruptedReveal, + extraData + ) + ).to.be.revertedWith("Wrong 32-byte script hash") + }) + }) + + context( + "when the caller address does not match the funding output script", + () => { + it("should revert", async () => { + const accounts = await getUnnamedAccounts() + const thirdParty = await ethers.getSigner(accounts[0]) + + await expect( + bridge + .connect(thirdParty) + .revealDepositWithExtraData( + P2WSHFundingTx, + reveal, + extraData + ) + ).to.be.revertedWith("Wrong 32-byte script hash") + }) + } + ) + + context("when the revealed extra data do not match", () => { + it("should revert", async () => { + // Corrupt the extra data. + const corruptedExtraData = ethers.utils.keccak256(extraData) + + await expect( + bridge + .connect(depositor) + .revealDepositWithExtraData( + P2WSHFundingTx, + reveal, + corruptedExtraData + ) + ).to.be.revertedWith("Wrong 32-byte script hash") + }) + }) + + context( + "when funding transaction does not embed extra data", + () => { + it("should revert", async () => { + await expect( + bridge.connect(depositor).revealDepositWithExtraData( + // Use a transaction that doesn't embed extra data in the deposit script. + revealDepositFixture.P2WSHFundingTx, + reveal, + extraData + ) + ).to.be.revertedWith("Wrong 32-byte script hash") + }) + } + ) + }) + + context("when funding transaction is neither P2SH nor P2WSH", () => { + it("should revert", async () => { + // Corrupt transaction output data by making a 21-byte script hash. + const corruptedP2SHFundingTx = { ...P2SHFundingTx } + corruptedP2SHFundingTx.outputVector = + "0x02102700000000000017a9156a6ade1c799a3e5a59678e776f21be14d66dc" + + "15ed8877ed73b00000000001600147ac2d9378a1c47e589dfb8095ca95ed2" + + "140d2726" + + await expect( + bridge + .connect(depositor) + .revealDepositWithExtraData( + corruptedP2SHFundingTx, + reveal, + extraData + ) + ).to.be.revertedWith("Wrong script hash length") + }) + }) + }) + + context("when reveal ahead period validation is enabled", () => { + const encodeRefundLocktime = (refundLocktimeTimestamp: number) => { + const refundLocktimeTimestampHex = BigNumber.from( + refundLocktimeTimestamp + ) + .toHexString() + .substring(2) + const refundLocktimeBuffer = Buffer.from( + refundLocktimeTimestampHex, + "hex" + ) + return `0x${refundLocktimeBuffer.reverse().toString("hex")}` + } + + before(async () => { + await createSnapshot() + + // Reveal ahead period is disabled by default in this test suite + // (see root before clause). We need to enable it manually. + await bridge.setDepositRevealAheadPeriod( + constants.depositRevealAheadPeriod + ) + }) + + after(async () => { + await restoreSnapshot() + }) + + context("when reveal ahead period is preserved", () => { + it("should pass the refund locktime validation", async () => { + const now = Math.floor(Date.now() / 1000) + const refundLocktimeDuration = 2592000 // 30 days + const refundLocktimeTimestamp = now + refundLocktimeDuration + const latestPossibleRevealTimestamp = + refundLocktimeTimestamp - constants.depositRevealAheadPeriod + + const alteredReveal = { + ...reveal, + refundLocktime: encodeRefundLocktime(refundLocktimeTimestamp), + } + + await ethers.provider.send("evm_setNextBlockTimestamp", [ + BigNumber.from(latestPossibleRevealTimestamp).toHexString(), + ]) + + // We cannot assert that the reveal transaction succeeded since + // we modified the revealed refund locktime which differs from + // the one embedded in the transaction P2SH. We just make sure + // the execution does not revert on the refund locktime validation. + await expect( + bridge + .connect(depositor) + .revealDepositWithExtraData( + P2WSHFundingTx, + alteredReveal, + extraData + ) + ).to.be.not.revertedWith("Deposit refund locktime is too close") + }) + }) + + context("when reveal ahead period is not preserved", () => { + it("should revert", async () => { + const now = Math.floor(Date.now() / 1000) + const refundLocktimeDuration = 2592000 // 30 days + const refundLocktimeTimestamp = now + refundLocktimeDuration + const latestPossibleRevealTimestamp = + refundLocktimeTimestamp - constants.depositRevealAheadPeriod + + const alteredReveal = { + ...reveal, + refundLocktime: encodeRefundLocktime(refundLocktimeTimestamp), + } + + await ethers.provider.send("evm_setNextBlockTimestamp", [ + BigNumber.from(latestPossibleRevealTimestamp + 1).toHexString(), + ]) + + await expect( + bridge + .connect(depositor) + .revealDepositWithExtraData( + P2WSHFundingTx, + alteredReveal, + extraData + ) + ).to.be.revertedWith("Deposit refund locktime is too close") + }) + }) + + context( + "when refund locktime integer value is less than 500M", + () => { + it("should revert", async () => { + const alteredReveal = { + ...reveal, + refundLocktime: encodeRefundLocktime(499999999), + } + + await expect( + bridge + .connect(depositor) + .revealDepositWithExtraData( + P2WSHFundingTx, + alteredReveal, + extraData + ) + ).to.be.revertedWith("Refund locktime must be a value >= 500M") + }) + } + ) + }) + }) + + context("when wallet is not in Live state", () => { + const testData = [ + { + testName: "when wallet state is Unknown", + walletState: walletState.Unknown, + }, + { + testName: "when wallet state is MovingFunds", + walletState: walletState.MovingFunds, + }, + { + testName: "when the source wallet is in the Closing state", + 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() + await bridge.setWallet(reveal.walletPubKeyHash, { + ecdsaWalletID: ethers.constants.HashZero, + mainUtxoHash: ethers.constants.HashZero, + pendingRedemptionsValue: 0, + createdAt: await lastBlockTime(), + movingFundsRequestedAt: 0, + closingStartedAt: 0, + pendingMovedFundsSweepRequestsCount: 0, + state: test.walletState, + movingFundsTargetWalletsCommitmentHash: + ethers.constants.HashZero, + }) + }) + + after(async () => { + await restoreSnapshot() + }) + + it("should revert", async () => { + await expect( + bridge + .connect(depositor) + .revealDepositWithExtraData(P2SHFundingTx, reveal, extraData) + ).to.be.revertedWith("Wallet must be in Live state") + }) + }) + }) + }) + }) + + context("when extra data is zero", () => { + it("should revert", async () => { + await expect( + bridge + .connect(depositor) + .revealDepositWithExtraData( + P2SHFundingTx, + reveal, + ethers.constants.HashZero + ) + ).to.be.revertedWith("Extra data must not be empty") + }) + }) + }) + describe("submitDepositSweepProof", () => { const walletDraft = { ecdsaWalletID: ethers.constants.HashZero, diff --git a/solidity/test/bridge/WalletProposalValidator.test.ts b/solidity/test/bridge/WalletProposalValidator.test.ts index da0875b40..fa3e7ab5d 100644 --- a/solidity/test/bridge/WalletProposalValidator.test.ts +++ b/solidity/test/bridge/WalletProposalValidator.test.ts @@ -197,7 +197,7 @@ describe("WalletProposalValidator", () => { }) context("when sweep does not exceed the max size", () => { - context("when deposit extra data length does not match", () => { + context("when deposit extra info length does not match", () => { it("should revert", async () => { // The proposal contains one deposit. const proposal = { @@ -207,7 +207,7 @@ describe("WalletProposalValidator", () => { depositsRevealBlocks: [], // Not relevant in this scenario. } - // The extra data array contains two items. + // The extra info array contains two items. const depositsExtraInfo = [ emptyDepositExtraInfo, emptyDepositExtraInfo, @@ -219,12 +219,12 @@ describe("WalletProposalValidator", () => { depositsExtraInfo ) ).to.be.revertedWith( - "Each deposit key must have matching extra data" + "Each deposit key must have matching extra info" ) }) }) - context("when deposit extra data length matches", () => { + context("when deposit extra info length matches", () => { context("when proposed sweep tx fee is invalid", () => { context("when proposed sweep tx fee is zero", () => { let depositOne @@ -565,7 +565,7 @@ describe("WalletProposalValidator", () => { context("when all deposits are not swept yet", () => { context( - "when there is a deposit with invalid extra data", + "when there is a deposit with invalid extra info", () => { context("when funding tx hashes don't match", () => { let deposit @@ -603,7 +603,7 @@ describe("WalletProposalValidator", () => { depositsRevealBlocks: [], // Not relevant in this scenario. } - // Corrupt the extra data by setting a different + // Corrupt the extra info by setting a different // version than 0x01000000 used to produce the hash. const depositsExtraInfo = [ { @@ -664,7 +664,7 @@ describe("WalletProposalValidator", () => { depositsRevealBlocks: [], // Not relevant in this scenario. } - // Corrupt the extra data by reversing the proper + // Corrupt the extra info by reversing the proper // blinding factor used to produce the script. const depositsExtraInfo = [ { @@ -730,7 +730,7 @@ describe("WalletProposalValidator", () => { depositsRevealBlocks: [], // Not relevant in this scenario. } - // Corrupt the extra data by reversing the proper + // Corrupt the extra info by reversing the proper // blinding factor used to produce the script. const depositsExtraInfo = [ { @@ -760,7 +760,7 @@ describe("WalletProposalValidator", () => { } ) - context("when all deposits extra data are valid", () => { + context("when all deposits extra info are valid", () => { context( "when there is a deposit that violates the refund safety margin", () => { @@ -1117,6 +1117,7 @@ describe("WalletProposalValidator", () => { () => { let depositOne let depositTwo + let depositThree before(async () => { await createSnapshot() @@ -1133,6 +1134,16 @@ describe("WalletProposalValidator", () => { false ) + // Use a deposit with embedded 32-byte extra data + // to make sure validation handles them correctly. + depositThree = createTestDeposit( + walletPubKeyHash, + vault, + true, + undefined, + "0xa9b38ea6435c8941d6eda6a46b68e3e2117196995bd154ab55196396b03d9bda" + ) + bridge.deposits .whenCalledWith( depositKey( @@ -1150,6 +1161,16 @@ describe("WalletProposalValidator", () => { ) ) .returns(depositTwo.request) + + bridge.deposits + .whenCalledWith( + depositKey( + depositThree.key.fundingTxHash, + depositThree.key + .fundingOutputIndex + ) + ) + .returns(depositThree.request) }) after(async () => { @@ -1164,6 +1185,7 @@ describe("WalletProposalValidator", () => { depositsKeys: [ depositOne.key, depositTwo.key, + depositThree.key, ], sweepTxFee, depositsRevealBlocks: [], // Not relevant in this scenario. @@ -1172,6 +1194,7 @@ describe("WalletProposalValidator", () => { const depositsExtraInfo = [ depositOne.extraInfo, depositTwo.extraInfo, + depositThree.extraInfo, ] const result = @@ -2609,7 +2632,8 @@ const createTestDeposit = ( walletPubKeyHash: string, vault: string, witness = true, - revealedAt?: number + revealedAt?: number, + extraData?: string ) => { let resolvedRevealedAt = revealedAt @@ -2639,10 +2663,28 @@ const createTestDeposit = ( const blindingFactor = `0x${crypto.randomBytes(8).toString("hex")}` const refundPubKeyHash = `0x${crypto.randomBytes(20).toString("hex")}` - const depositScript = - `0x14${depositor.substring(2)}7508${blindingFactor.substring(2)}7576a914` + - `${walletPubKeyHash.substring(2)}8763ac6776a914` + - `${refundPubKeyHash.substring(2)}8804${refundLocktime.substring(2)}b175ac68` + let depositScript + + if (extraData) { + depositScript = + `0x14${depositor.substring(2)}75` + + `20${extraData.substring(2)}75` + + `08${blindingFactor.substring(2)}75` + + `76a914${walletPubKeyHash.substring(2)}87` + + "63ac67" + + `76a914${refundPubKeyHash.substring(2)}88` + + `04${refundLocktime.substring(2)}b175` + + "ac68" + } else { + depositScript = + `0x14${depositor.substring(2)}75` + + `08${blindingFactor.substring(2)}75` + + `76a914${walletPubKeyHash.substring(2)}87` + + "63ac67" + + `76a914${refundPubKeyHash.substring(2)}88` + + `04${refundLocktime.substring(2)}b175` + + "ac68" + } let depositScriptHash if (witness) { @@ -2686,6 +2728,7 @@ const createTestDeposit = ( vault, treasuryFee: 0, // not relevant sweptAt: 0, // important to pass the validation + extraData: extraData ?? ethers.constants.HashZero, }, extraInfo: { fundingTx, diff --git a/solidity/test/vault/TBTCVault.OptimisticMinting.test.ts b/solidity/test/vault/TBTCVault.OptimisticMinting.test.ts index d09356777..717f1b8ef 100644 --- a/solidity/test/vault/TBTCVault.OptimisticMinting.test.ts +++ b/solidity/test/vault/TBTCVault.OptimisticMinting.test.ts @@ -1789,6 +1789,7 @@ describe("TBTCVault - OptimisticMinting", () => { vault: f.tbtcVault.address, treasuryFee: 10, sweptAt: 0, + extraData: ethers.constants.HashZero, }) f.mockBridge.deposits.whenCalledWith(secondDepositID).returns({ depositor: depositorAddress, @@ -1797,6 +1798,7 @@ describe("TBTCVault - OptimisticMinting", () => { vault: f.tbtcVault.address, treasuryFee: 15, sweptAt: 0, + extraData: ethers.constants.HashZero, }) await f.tbtcVault @@ -1895,6 +1897,7 @@ describe("TBTCVault - OptimisticMinting", () => { vault: f.tbtcVault.address, treasuryFee: 10, sweptAt: 0, + extraData: ethers.constants.HashZero, }) await f.tbtcVault