[bisq-network/bisq] Improve Validation of Deposit Inputs (#5618)

jimmyneutront notifications at github.com
Tue Jul 13 04:40:08 CEST 2021


@jimmyneutront commented on this pull request.



> @@ -356,23 +356,46 @@ public static void validatePayoutTxInput(Transaction depositTx,
     }
 
     public static void validateDepositInputs(Trade trade) throws InvalidTxException {
-        // assumption: deposit tx always has 2 inputs, the maker and taker
-        if (trade == null || trade.getDepositTx() == null || trade.getDepositTx().getInputs().size() != 2) {
-            throw new InvalidTxException("Deposit transaction is null or has unexpected input count");
+        /*
+         * deposit tx usually has 2 inputs, the maker and taker

My apologies for not being more clear.

Could you elaborate a bit more on your suspicions about issues with maker fee payments? It appears that the taker never thoroughly validates the maker fee transaction by ensuring that it sent the correct amount to BTC to a FeeReceiver address. As long as the String in the OfferPayload's offerFeePaymentTxId field matches the id of the transaction that the maker used to fund the deposit transaction, the taker's Bisq instance raises no errors. And even if these don't match, validateDepositInputs() only raises an error AFTER the trade has been taken and funds are locked in multisig. I tested this by completing a trade for which the seller/maker's Bisq instance was built with line 159 of [TradeWalletService.java](https://github.com/bisq-network/bisq/blob/030a76522cb1d9325f45656c7814043b809be68e/core/src/main/java/bisq/core/btc/wallet/TradeWalletService.java) commented out, so the seller/maker paid no maker fee. The buyer/taker was able to take the offer and complete the entire trade protocol with no errors.

How could my proposed change cause any new problems? Additionally, I apologize that my  comment was not as clear and open as it should have been. I'd be happy to amend the commit and clarify the comment, if you would like me to; what information would you like me to include?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/bisq-network/bisq/pull/5618#discussion_r668384759
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.bisq.network/pipermail/bisq-github/attachments/20210712/d075e1b9/attachment.htm>


More information about the bisq-github mailing list