Takers deposit transaction rejected for 'too-long-mempool-chain' causing issues

[devinbileck]

Description

If a taker attempts to take too many offers that have not been confirmed, the deposit transaction will be rejected by the Bitcoin network for 'too-long-mempool-chain'. The affected trade will then be moved into his failed trades tab but remains open for the maker without any indication of an error.

The maker then likely waits for the trade timer to run out, with any chat attempts to contact the taker going unanswered (since chat messages are not shown to them for failed trades). Then the maker finally opens a support ticket and the mediator is able to communicate with both Alice and Bob (it will show up under the takers support tickets).

When the mediator suggests a payout, only the maker sees the mediation result in her open trades. The taker doesn't since the trade is in Failed trades. The maker is able to accept the result, but nothing happens since the taker is unable to accept as well. At this point, the maker is unable to contact the mediator (since the support ticket was closed), and is unable to contact the taker.

If the maker then tries to reject the result, she will encounter an error indicating the deposit transaction is not confirmed and unable to open an arbitration dispute [3]. And after doing an SPV resync, encounters an error that the trade has no deposit transaction set and to move it to failed trades [4].

Version

release/v.1.2.4 (commit hash 548a217)

Steps to reproduce

Using a regtest setup, perform the following steps without generating any blocks:

1. Alice makes about 15 offers to buy BTC
2. Bob tries to take all the offers to sell BTC

Actual behaviour

When attempting to take the 13th offer, Bob will encounter a transaction rejection error [1] and [2], and the trade will move into his Failed Trades tab. Meanwhile, the trade will show in Alice's open trades without any indication that an error occurred on the takers side. And even after generating several blocks, remains waiting for blockchain confirmation (note, I did see on a few attempts to reproduce this issue that it did eventually get confirmed after several blocks).

Note, the multisig deposit appears in both wallets, but remains unconfirmed.

Expected behaviour

At a minimum, after Bob encounters the transaction rejection error, Alice's trade should update and indicate an error was encountered. Perhaps it needs to move to Failed Trades as well. Though ideally we should prevent this type of rejected transaction in the first place and perhaps block creating/taking too many offers without confirmation.

Additionally, do we need to allow chat for failed trades and potentially view mediator result for failed trades?

Screenshots

[1]

[2]

[3]

[4]

Environment

• Windows 10
• Regtest environment with Bitcoin core v0.18.1

Additional information

In Bitcoin Core if a transaction in the mempool has 25 descendants, or it and all of its descendants are over 101,000 vbytes, any newly-received transaction that is also a descendant will be rejected. So once you have 25+ UTXOs in a long unspent chain of unconfirmed transactions, you will encounter this issue. And since each take offer has 2 transactions (taker/tx fee and multisig deposit), that is why the issue was encountered when attempting to take the 13th offer.

[devinbileck]

This may be related to the wallet corruption issues encountered lately. If you spend an invalid UTXO then your wallet is corrupted and you can even spread the corruption to other users this way. I have not confirmed this though.

[wiz]

@chimp1984 I think this issue is what initially caused our recent wallet corruption issues - a very big market maker on Bisq quickly created 5 ~ 10 trades each on 5 ~ 10 different markets, and bypassed the Bitcoin rule for 25 unconfirmed transactions chained to each other. Then the maker corrupted every taker's wallets when they got corrupted, and it kept spreading like a virus...

[devinbileck]

Note, if we decide to limit the number of offers placed/taken without confirmation, should also limit the number of BTC/BSQ transactions without confirmation, since that can cause the same type of rejected transaction. In fact after sending too many BSQ transactions and restarting the client, I encountered the following error and was unable to launch the app:

Nov-27 23:50:34.262 [JavaFX Application Thread] ERROR b.core.btc.setup.WalletsSetup: Service failure from state: STARTING; failure={} java.lang.IllegalStateException: Expected PENDING or IN_CONFLICT, was BUILDING.
	at com.google.common.base.Preconditions.checkState(Preconditions.java:469)
	at org.bitcoinj.wallet.Wallet.setTransactionBroadcaster(Wallet.java:5072)
	at org.bitcoinj.core.PeerGroup.addWallet(PeerGroup.java:1309)
	at bisq.core.btc.setup.WalletConfig.startUp(WalletConfig.java:455)
	at com.google.common.util.concurrent.AbstractIdleService$DelegateService$1.run(AbstractIdleService.java:62)
	at com.google.common.util.concurrent.Callables$4.run(Callables.java:122)
	at java.base/java.lang.Thread.run(Thread.java:844)

java.lang.IllegalStateException: Expected PENDING or IN_CONFLICT, was BUILDING.
	at com.google.common.base.Preconditions.checkState(Preconditions.java:469)
	at org.bitcoinj.wallet.Wallet.setTransactionBroadcaster(Wallet.java:5072)
	at org.bitcoinj.core.PeerGroup.addWallet(PeerGroup.java:1309)
	at bisq.core.btc.setup.WalletConfig.startUp(WalletConfig.java:455)
	at com.google.common.util.concurrent.AbstractIdleService$DelegateService$1.run(AbstractIdleService.java:62)
	at com.google.common.util.concurrent.Callables$4.run(Callables.java:122)
	at java.base/java.lang.Thread.run(Thread.java:844)
Nov-27 23:50:34.266 [JavaFX Application Thread] ERROR bisq.core.app.WalletAppSetup: java.lang.IllegalStateException: Expected PENDING or IN_CONFLICT, was BUILDING. 

[wiz]

In case it's not clear, this is a high priority issue that causes user wallet corruption, that still reproduces in the current version of Bisq. A user reported their wallet was corrupted to me today after creating 25 offers within a short period of time.

[ripcurlx]

In case it's not clear, this is a high priority issue that causes user wallet corruption, that still reproduces in the current version of Bisq. A user reported their wallet was corrupted to me today after creating 25 offers within a short period of time.

I removed the milestone label not because I think it is not important, just plainly because no dev committed right now to implement it in-time for the v1.2.5 release. I do plan to have a look at it next, but I'm not sure how quickly it can be fixed in a proper way.

[wiz]

Cool. Actually it's not as critical as it was before because the new "failed trades" feature is preventing it from spreading like a virus, but market makers who create a lot of offers quickly are still reporting their wallet getting corrupted. Of course I told them about the issue now that we know what's causing it, and they will create offers more slowly and wait for confirmations, but I just want to make sure this isn't forgotten 😅

[wiz]

@ripcurlx this critical wallet corrupting bug has still not been addressed - could we target it for v1.2.6 ? I think all we need to do is handle the exception and prevent the user from making any more transactions after it passes a "soft limit" of 15 or so, I think the hard bitcoin limit is 25 unconfirmed transactions

[ripcurlx]

@ripcurlx this critical wallet corrupting bug has still not been addressed - could we target it for v1.2.6 ? I think all we need to do is handle the exception and prevent the user from making any more transactions after it passes a "soft limit" of 15 or so, I think the hard bitcoin limit is 25 unconfirmed transactions

Just a matter of resources and priority. I'll have a look how much dev time I've left for the v1.2.6 release after my current work.

[freimair]

onto it

[wiz]

Call discussion nodes with @freimair about how to tackle this:

1. fix mempool fee estimation Migrate Bisq pricenodes from earn.com fee estimation API to our own self-hosted mempool fee estimation API service projects#27
2. mitigate causes of the too-long-mempool-chain issue from reproducing by implementing a "soft limit" and "hard limit" in many various places on both maker and taker sides
3. since we can't prevent all causes, we still need to handle the currently-unhandled exception state of when you get a broadcast fail error (due to too-long-mempool-chain error), because it could actually get confirmed later, and
4. we need to add a "refund" by doublespending funding TX back to a fresh unused address
5. this bug blocks the GRPC API from being possible because a trading bot creating many offers quickly would immediately trigger this bug and corrupt the wallet (cc @cbeams )
6. as an additional verification step, to check if a deposit TX is valid, since Bisq cannot know (due to limitations of SPV), we could possibly query a REST API backend running on a federated btcnode, if the TXID exists in that full node's mempool - if it does, then it's probably valid
7. the final answer (end of the flow chart) must either be that the deposit TX got confirmed, or a refund TX got confirmed (refund of funding TX)
8. don't need all of the above ideas implemented, can add one at a time, as they all help to mitigate the severity of the issue

[wiz]

9. currently 8 ~ 10 cases per month of this issue, the goal should be to reduce it to less than 1 case per month

[cbeams]

@devinbileck wrote in #3705 (comment)

[...]

[...]

java.lang.IllegalStateException: Expected PENDING or IN_CONFLICT, was BUILDING.
	at com.google.common.base.Preconditions.checkState(Preconditions.java:469)
	at org.bitcoinj.wallet.Wallet.setTransactionBroadcaster(Wallet.java:5072)
	at org.bitcoinj.core.PeerGroup.addWallet(PeerGroup.java:1309)
	at bisq.core.btc.setup.WalletConfig.startUp(WalletConfig.java:455)
	at com.google.common.util.concurrent.AbstractIdleService$DelegateService$1.run(AbstractIdleService.java:62)
	at com.google.common.util.concurrent.Callables$4.run(Callables.java:122)
	at java.base/java.lang.Thread.run(Thread.java:844)
Nov-27 23:50:34.266 [JavaFX Application Thread] ERROR bisq.core.app.WalletAppSetup: java.lang.IllegalStateException: Expected PENDING or IN_CONFLICT, was BUILDING. 

I just ran into this error and was able to get around it by shutting down Bisq, deleting my SPV file from the filesystem and restarting.

[eigentsmis]

@devinbileck wrote in #3705 (comment)

[...]

[...]

java.lang.IllegalStateException: Expected PENDING or IN_CONFLICT, was BUILDING.
	at com.google.common.base.Preconditions.checkState(Preconditions.java:469)
	at org.bitcoinj.wallet.Wallet.setTransactionBroadcaster(Wallet.java:5072)
	at org.bitcoinj.core.PeerGroup.addWallet(PeerGroup.java:1309)
	at bisq.core.btc.setup.WalletConfig.startUp(WalletConfig.java:455)
	at com.google.common.util.concurrent.AbstractIdleService$DelegateService$1.run(AbstractIdleService.java:62)
	at com.google.common.util.concurrent.Callables$4.run(Callables.java:122)
	at java.base/java.lang.Thread.run(Thread.java:844)
Nov-27 23:50:34.266 [JavaFX Application Thread] ERROR bisq.core.app.WalletAppSetup: java.lang.IllegalStateException: Expected PENDING or IN_CONFLICT, was BUILDING. 

I just ran into this error and was able to get around it by shutting down Bisq, deleting my SPV file from the filesystem and restarting.

Had the same issue recently and fixed it the same way as @cbeams

[coinzdude]

I have this same error.

If I manually delete the SPV file, will my offers still be valid?

---

I did delete the SPV file and I still see my offers in the market, so this looks good.