fix: include scriptPubKey size in hash that is checked

bob-collective · Nov 17, 2023 · 63db7b3 · 63db7b3
1 parent 636b0ee
commit 63db7b3
Show file tree

Hide file tree

Showing 2 changed files with 34 additions and 20 deletions.
diff --git a/src/swap/Btc_Marketplace.sol b/src/swap/Btc_Marketplace.sol
@@ -2,6 +2,7 @@ pragma solidity ^0.8.13;
 
 import "@openzeppelin/contracts/token/ERC20/IERC20.sol";
 import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
+import {BTCUtils} from "@bob-collective/bitcoin-spv/BTCUtils.sol";
 import {BitcoinTx} from "../bridge/BitcoinTx.sol";
 // import {LightRelay} from "../relay/LightRelay.sol";
 import {IRelay} from "../bridge/IRelay.sol";
@@ -156,10 +157,7 @@ contract BtcMarketPlace {
 
         relay.validateProof(transaction, proof);
 
-        // Check output script pubkey (recipient address) and amount
-        uint256 txOutputValue =
-            BitcoinTx.getTxOutputValue(keccak256(accept.bitcoinAddress.scriptPubKey), transaction.outputVector);
-        assert(txOutputValue >= accept.amountBtc);
+        _checkBitcoinTxOutput(accept.amountBtc, accept.bitcoinAddress, transaction);
 
         IERC20(accept.ercToken).safeTransfer(accept.requester, accept.ercAmount);
 
@@ -256,10 +254,7 @@ contract BtcMarketPlace {
         relay.validateProof(transaction, proof);
 
         BtcBuyOrder storage order = btcBuyOrders[accept.orderId];
-        // Check output script pubkey (recipient address) and amount
-        uint256 txOutputValue =
-            BitcoinTx.getTxOutputValue(keccak256(order.bitcoinAddress.scriptPubKey), transaction.outputVector);
-        assert(txOutputValue >= order.amountBtc);
+        _checkBitcoinTxOutput(order.amountBtc, order.bitcoinAddress, transaction);
 
         IERC20(accept.ercToken).safeTransfer(accept.accepter, accept.ercAmount);
 
@@ -386,4 +381,25 @@ contract BtcMarketPlace {
         }
         return (ret, identifiers);
     }
+
+    /**
+     * Checks output script pubkey (recipient address) and amount.
+     * Reverts if transaction amount is lower or bitcoin address is not found.
+     *
+     * @param expectedBtcAmount BTC amount requested in order.
+     * @param bitcoinAddress Recipient's bitcoin address.
+     * @param transaction Transaction fulfilling the order.
+     */
+    function _checkBitcoinTxOutput(
+        uint256 expectedBtcAmount,
+        BitcoinAddress storage bitcoinAddress,
+        BitcoinTx.Info calldata transaction
+    ) private {
+        // Prefixes scriptpubkey with its size to match script output data.
+        bytes32 b = keccak256(abi.encodePacked(uint8(bitcoinAddress.scriptPubKey.length), bitcoinAddress.scriptPubKey));
+
+        uint256 txOutputValue = BitcoinTx.getTxOutputValue(b, transaction.outputVector);
+
+        require(txOutputValue >= expectedBtcAmount, "Bitcoin transaction amount is lower than in accepted order.");
+    }
 }
diff --git a/test/swap/Btc_Marketplace.t.sol b/test/swap/Btc_Marketplace.t.sol
@@ -61,6 +61,10 @@ contract MarketPlaceTest is BtcMarketPlace, Test {
         });
     }
 
+    function dummyBitcoinAddress() public returns (BitcoinAddress memory) {
+        return BitcoinAddress({scriptPubKey: hex"76a914fd7e6999cd7e7114383e014b7e612a88ab6be68f88ac"});
+    }
+
     function testSellBtc() public {
         token1.sudoMint(bob, 100);
 
@@ -69,7 +73,7 @@ contract MarketPlaceTest is BtcMarketPlace, Test {
 
         vm.startPrank(bob);
         token1.approve(address(this), 40);
-        this.acceptBtcSellOrder(0, BitcoinAddress({scriptPubKey: "a91476fe4e664d67c35931096d90aabbdbd8e0e11bad87"}), 40);
+        this.acceptBtcSellOrder(0, dummyBitcoinAddress(), 40);
 
         vm.startPrank(alice);
         this.proofBtcSellOrder(1, dummyTransaction(), dummyProof());
@@ -83,7 +87,7 @@ contract MarketPlaceTest is BtcMarketPlace, Test {
 
         vm.startPrank(bob);
         token1.approve(address(this), 40);
-        this.acceptBtcSellOrder(0, BitcoinAddress({scriptPubKey: "a91476fe4e664d67c35931096d90aabbdbd8e0e11bad87"}), 40);
+        this.acceptBtcSellOrder(0, dummyBitcoinAddress(), 40);
 
         vm.startPrank(alice);
         this.withdrawBtcSellOrder(0);
@@ -99,7 +103,7 @@ contract MarketPlaceTest is BtcMarketPlace, Test {
 
         vm.startPrank(bob);
         token1.approve(address(this), 40);
-        this.acceptBtcSellOrder(0, BitcoinAddress({scriptPubKey: "a91476fe4e664d67c35931096d90aabbdbd8e0e11bad87"}), 40);
+        this.acceptBtcSellOrder(0, dummyBitcoinAddress(), 40);
 
         vm.warp(block.timestamp + REQUEST_EXPIRATION_SECONDS + 1);
         assertEq(token1.balanceOf(address(this)), 4);
@@ -114,9 +118,7 @@ contract MarketPlaceTest is BtcMarketPlace, Test {
 
         vm.startPrank(alice);
         token1.approve(address(this), 100);
-        this.placeBtcBuyOrder(
-            1000, BitcoinAddress({scriptPubKey: "a91476fe4e664d67c35931096d90aabbdbd8e0e11bad87"}), address(token1), 100
-        );
+        this.placeBtcBuyOrder(1000, dummyBitcoinAddress(), address(token1), 100);
 
         vm.startPrank(bob);
         this.acceptBtcBuyOrder(0, 40);
@@ -132,9 +134,7 @@ contract MarketPlaceTest is BtcMarketPlace, Test {
 
         vm.startPrank(alice);
         token1.approve(address(this), 100);
-        this.placeBtcBuyOrder(
-            1000, BitcoinAddress({scriptPubKey: "a91476fe4e664d67c35931096d90aabbdbd8e0e11bad87"}), address(token1), 100
-        );
+        this.placeBtcBuyOrder(1000, dummyBitcoinAddress(), address(token1), 100);
 
         vm.startPrank(bob);
         this.acceptBtcBuyOrder(0, 40);
@@ -152,9 +152,7 @@ contract MarketPlaceTest is BtcMarketPlace, Test {
 
         vm.startPrank(alice);
         token1.approve(address(this), 100);
-        this.placeBtcBuyOrder(
-            1000, BitcoinAddress({scriptPubKey: "a91476fe4e664d67c35931096d90aabbdbd8e0e11bad87"}), address(token1), 100
-        );
+        this.placeBtcBuyOrder(1000, dummyBitcoinAddress(), address(token1), 100);
 
         vm.startPrank(bob);
         this.acceptBtcBuyOrder(0, 40);