Trade payout address was reused from prior trade & no funds received

[ghost]

Buy trade, maker: czcgg. Upon clicking "Payment started", the trade immediately transitioned to complete, although the seller had not yet acknowledged payment receipt.

Investigation showed that the trade obtained the same payout address as a prior trade done an hour earlier. Therefore the payout address listener found funds (from the prior trade) - this indicated to Bisq that the trade was complete.

The code in Bisq explicitly requests an unused address, ultimately relying on BitcoinJ for that.

An unused payout address is requested at trade initiation:

bisq/core/src/main/java/bisq/core/trade/protocol/tasks/maker/MakerSendsInputsForDepositTxResponse.java

Line 62 in 4849988

AddressEntry makerPayoutAddressEntry = walletService.getOrCreateAddressEntry(id, AddressEntry.Context.TRADE_PAYOUT);

bisq/core/src/main/java/bisq/core/btc/wallet/BtcWalletService.java

Lines 668 to 672 in 4849988

	public AddressEntry getFreshAddressEntry(boolean segwit) {
	AddressEntry.Context context = AddressEntry.Context.AVAILABLE;
	Optional<AddressEntry> addressEntry = getAddressEntryListAsImmutableList().stream()
	.filter(e -> context == e.getContext())
	.filter(e -> isAddressUnused(e.getAddress()))

bisq/core/src/main/java/bisq/core/btc/wallet/WalletService.java

Lines 545 to 546 in 4849988

	public boolean isAddressUnused(Address address) {
	return getNumTxOutputsForAddress(address) == 0;

bisq/core/src/main/java/bisq/core/btc/wallet/WalletService.java

Lines 529 to 542 in 4849988

	public int getNumTxOutputsForAddress(Address address) {
	return getTxOutputAddressMultiset().count(address);
	}
	private Multiset<Address> getTxOutputAddressMultiset() {
	return txOutputAddressCache.updateAndGet(set -> set != null ? set : computeTxOutputAddressMultiset());
	}
	private Multiset<Address> computeTxOutputAddressMultiset() {
	return wallet.getTransactions(false).stream()
	.flatMap(t -> t.getOutputs().stream())
	.map(WalletService::getAddressFromOutput)
	.filter(Objects::nonNull)
	.collect(ImmutableMultiset.toImmutableMultiset());

https://github.com/bisq-network/bitcoinj/blob/3186b200fff690fa51f3ebbf578f427d78242bc2/core/src/main/java/org/bitcoinj/wallet/Wallet.java#L3000-L3009

[ghost]

Is it problem with BitcoinJ getTransactions method, or rather with cache (txOutputAddressCache) ?

[ghost]

I don't see anything wrong with the few lines of code used to create txOutputAddressCache. Unfortunately I don't know a way to reproduce the problem, it just seems to occasionally happen to random users. Similar example

[ghost]

This problem has been reported again. A prior trade completed normally then its payout address was reused by trade t9jejy even though the prior payout had been made, seen and confirmed in the blockchain. So exactly the same circumstances, even the duration of time between payout and re-used payout (approx 1 hour) is the same.

[ghost]

Received a third reported case mpcfvoz buy, taker for 0.01 BTC. The trade closed soon after taker hit "payment initiated". Bisq was looking for a payout on an address, but that address had been used as payout for a previous trade, 11 hours prior. So Bisq saw the earlier payout and closed this trade too soon. This is because Bisq expects a payout address to only be used one time.

A "band-aid" fix might possibly be to check for the payout amount to match the trade details, and that might prevent the issue from a user perspective, but the underlying problem is that Bisq chose to reuse an old address which is bad practice and quite wrong.

Opinions sought from @bisq-network/bisq-maintainers @chimp1984

[chimp1984]

Yes your suggestion sounds feasible. The address should not be reused but there might be edge cases/bug situations where that happens.

[ghost]

Another instance of this issue was reported by @pazza83.

[ghost]

Checking for payout amount would not work if the trade was settled using Manual Payout or with a Mediation penalty. But checking that block height of the payout > deposit block height would work.