Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inconsistent spent tx / depositTx is null when taking offer #3721

Closed
julianknutsen opened this issue Dec 1, 2019 · 6 comments · Fixed by #3725
Closed

Inconsistent spent tx / depositTx is null when taking offer #3721

julianknutsen opened this issue Dec 1, 2019 · 6 comments · Fixed by #3725

Comments

@julianknutsen
Copy link
Contributor

julianknutsen commented Dec 1, 2019

Description

When taking an offer an error is presented and error logs display "Inconsistent spent tx:" and "depositTx is null".

Offer appears in "Open Trades" with no available actions.

After restarting Alice & Bob the option is available to move to "Failed Trades".

It looks like the multisig payout transaction from mediation on Bob (mediation payed out all to Alice) is the issue. I have 0% experience with that part of the codebase, but that is the common bad txn across runs.

Version

548a217

Steps to reproduce

100% repro over three runs with the following steps:

Alice - fullDaoNode | Bob - (non-fullDaoNode || fullDaoNode)

  1. Start localnet cluster
  2. Run TestPad test case Arbitration/Support Process: 002
  3. Run TestPad test case Arbitration/Support Process: 009
  4. See failure when trying to take offer from Bob

Expected behaviour

It works, or a more useful error message is presented.

Actual behaviour

Error popup, log errors, and failed trade.

Screenshots

Error Logs
inconsistent_spent_tx

Transactions
inconsistent_spent_bob_txns
inconsistent_spent_alice_txns

Funds Tab
inconsistent_spent_bob_funds
inconsistent_spent_alice_funds

Bob Trade Info
inconsistent_spent_bob_trade_info

After Restart:
inconsistent_spent_bob_after_restart

After SPV Resync
inconsistent_spent_bob_after_spv_resync

Device or machine

Ubuntu 19.10

Additional info

alice-bisq.log
bob-bisq.log

@chimp1984
Copy link
Contributor

@julianknutsen Thanks for finding that! I could reproduce it. It seems BitcoinJ has problems with a tx where you do not receive anything. Still investigating what exactly is the reason and how we can fix that but for now the mediators should not make suggested payouts with one peer getting all and the other nothing to avoid that scenario until we have fixed it. They got informed to follow that until its solved.

@chimp1984
Copy link
Contributor

chimp1984 commented Dec 1, 2019

Ah we do not filter out the 0 BTC output!

mediated payoutTx:

     in   0[] PUSHDATA(72)[3045022100a9511f6e116da5e9dc086961e44fbe4d91e94654be14f0d98de0201c5e9c298b02207eef6f70b95b8042d8b4dc8263539660373510f6f03445eb0430b8a07f44daa701] PUSHDATA(72)[304502210091ae588103038937c558431da206988710eaa2b4675df8b1585aa42756f8358a022060f17384866a271396d1057e2affb116d13b578ee60f2462660de44aa1c232d401] PUSHDATA(71)[522103dee02e13b2484a2af6727371f8edc98fbe03e6727d844b15b45041a8b6db4a172102d6f58a3e89d1f6060e65b0a6c30daa21d54c90ba5c9f5b5a420af992b6a5995b52ae] 0.007038 BTC (703800)
          outpoint:65f3507e8a5b947c4566d18b49f25aac1e5cec04ff8af4a21e93b4d250adbfdc:0 hash160:b7221590dd29a428a70961e863b735bb06e2722f
     out  DUP HASH160 PUSHDATA(20)[390733579084df4539de897de4963325c45bcb7a] EQUALVERIFY CHECKSIG 0.007 BTC (700000) ScriptPubKey: 76a914390733579084df4539de897de4963325c45bcb7a88ac Address:mkiVSHsThWMzQzKhJdyWeGtusfrqpm8q8c 
     out  DUP HASH160 PUSHDATA(20)[809a6b272b4f0e741bcaefb90fc64e7f62ebcfd3] EQUALVERIFY CHECKSIG 0.00 BTC (0) ScriptPubKey: 76a914809a6b272b4f0e741bcaefb90fc64e7f62ebcfd388ac Address:msEwmoG66sv1MMLHpM4WXGc3r4V2VNwZ2x 
     fee  0.000038 BTC for 338 bytes (11 Satoshi/Byte)
     prps UNKNOWN```

chimp1984 added a commit to chimp1984/bisq that referenced this issue Dec 1, 2019
Fixes bisq-network#3721 and
bisq-network#3722

There are still more issues as such a payout tx will cause that the
trade ends up in failed trades. This commit only fixes the invalid
tx issue.
chimp1984 added a commit to chimp1984/bisq that referenced this issue Dec 2, 2019
Fixes bisq-network#3721
(part of the problem was that the trade ended up in failed trade)

Refactor method and add comments.
We did not handle the case of a mediated payout. isPayoutPublished() is
only reflecting non-disputed trade payouts.
@chimp1984
Copy link
Contributor

Fixed the moving to failed trade bug: #3726

@julianknutsen
Copy link
Contributor Author

I'm having a hard time keeping the bugs separate for each fix. Any other bug references I can look at for user-submitted ones?

Are there two separate issues here?

  1. not filtering 0 BTC output causes inconsistent spent tx error and popup/failure
  2. trades that fail w/ inconsistent tx are incorrectly marked as failed trades?

@chimp1984
Copy link
Contributor

Added one: #3727
Just re tested and the bug was independent of the 0 BTC output issue.

ripcurlx added a commit that referenced this issue Dec 2, 2019
* Do not add an output if value is 0 BTC

Fixes #3721 and
#3722

There are still more issues as such a payout tx will cause that the
trade ends up in failed trades. This commit only fixes the invalid
tx issue.

* Refactoring: Replace isGreaterThan(Coin.ZERO) with isPositive()

* Do not display confirmation icon for 0 BTC tx

If we do not get any BTC from a mediated payout tx we do not know about
the confirmation state so it would stay always in the unconfirmed state.
To avoid that confusion we prefer to hide the icon. This is a known
issue from BitcoinJ but we have not found a solution for that yet.
ripcurlx pushed a commit that referenced this issue Dec 2, 2019
Fixes #3721 and
#3722

There are still more issues as such a payout tx will cause that the
trade ends up in failed trades. This commit only fixes the invalid
tx issue.
ripcurlx pushed a commit that referenced this issue Dec 2, 2019
Fixes #3721
(part of the problem was that the trade ended up in failed trade)

Refactor method and add comments.
We did not handle the case of a mediated payout. isPayoutPublished() is
only reflecting non-disputed trade payouts.
@julianknutsen
Copy link
Contributor Author

Confirmed that the TestPad test case now passes on c2e8806

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants