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

Fix emergency payout #4859

Merged
merged 3 commits into from
Dec 1, 2020

Conversation

oscarguindzberg
Copy link
Contributor

Fixes #4854

@ghubstan
Copy link
Contributor

utACK

@ghost
Copy link

ghost commented Nov 30, 2020

NACK? Test failed for segwit although passed for legacy. Perhaps @huey735 could double check the segwit scenario. If necessary I can add more information tomorrow.
p.s. I was testing this due to working on #4061.

@oscarguindzberg
Copy link
Contributor Author

@jmacxx thanks for testing this.
I tested with segwit on regtest and it worked.
Do you have a list of steps I could follow to reproduce the bug?

@ghost
Copy link

ghost commented Nov 30, 2020

Segwit test

  • Run Alice & Bob as clean new v1.50+patch.4859 Bisq regtest instances - let them sync up and fund them with BTC
  • Bob creates an offer to sell 0.01 BTC
  • Alice takes the offer
  • 1 confirmation
  • Alice initiates fiat payment
  • Mediator initiates Emergency Multisig payout tool
  • Fills in the fields: [depositTxid], 0.016, 0.006, 0.00001234, [buyer payout address], [seller payout address ], [buyer prvkey], [seller prvkey], [buyer pubkey], [seller pubkey], [legacy NOT CHECKED]
  • click Sign and publish transaction
  • Transaction DOES NOT appear in Funds, Transactions screen neither Alice nor Bob
  • Transactions screen does not update when subsequent blocks come in.
  • Trade never sees published payout.

Legacy test

  • Run Alice & Bob as clean new v1.39 Bisq regtest instances - let them sync up and fund them with BTC
  • Bob creates an offer to sell 0.01 BTC
  • Alice takes the offer
  • 1 confirmation
  • Alice initiates fiat payment
  • Mediator initiates Emergency Multisig payout tool
  • Fills in the fields: [depositTxid], 0.016, 0.006, 0.00001234, [buyer payout address], [seller payout address], [buyer prvkey], [seller prvkey], [buyer pubkey], [seller pubkey], [legacy CHECKED]
  • click Sign and publish transaction
  • Transaction appears in Funds, Transactions screen on both Alice and Bob
  • 1 confirmation, transactions screen shows the appropriate confirmation progress icons

@oscarguindzberg
Copy link
Contributor Author

@jmacxx do you have the logs of the mediator at the time you click "Sign and publish transaction" for the segwit use case? do you have bitcoin regtest logs?

@oscarguindzberg
Copy link
Contributor Author

@jmacxx are you using default security deposits? If you do, the expected payouts are 0.0115 and 0.0015

@ghost
Copy link

ghost commented Nov 30, 2020

Yes default security deposit is 0.006 for a 0.01 trade. Its the minimum security deposit. Payouts are 0.006 for seller and 0.016 for buyer. Here's what appears in the mediator's log (regtest). The transaction does not show up in the blockchain.

    Nov-30 13:09:58.017 [JavaFX Application Thread] INFO  b.c.btc.wallet.WalletService: 
    payoutTx:
    Transaction{95e2218d447731be5a9da8e6cb8baf19a357c6dd009c0fd99989a67a6bd1c22f, wtxid 9adae4e46eacd21e698dbe34a53b468acbb76d57a7f4b15579d424951c11f580
    weight: 673 wu, 169 virtual bytes, 334 bytes
    purpose: UNKNOWN
       in   <empty>  0.02201234 BTC (2201234)
            witness:EMPTY 30440220641962b4ba8140c8eec8340e6df93f754211dcec64811aa9945c38fc980d22fe02207f0588d13272b8afc17b110e1f13bb5ff7850e5c1890085774da47c516e3e5ef01 3045022100ffe944ff7562cdb369f1b044dfabf4bca84ad5b2be90dcb64eafd1decc2828d5022047c3b2ce5b664ee3cf293d7e50db70f0e39dce462672e8e86868956bcd0ce2c601 522102f77470b7d2cb72796d5d1d4b74bf05720e6227a44aa209d765cd61d0a5fb1838210242a4bc03819c2bb8cdc2d8ed31ef34cb8ae64a1348e3eeb8f7e998acb6a44f7352ae
            unconnected  outpoint:6f306ee571b8feeb7ed2b96b8affb27a393083a4ae3d1d311b543acfbe1e3dc5:0
       out  0[] PUSHDATA(20)[5226d2b975a4e87b4d1bbb9a43bcb3a01c8380ba] (00145226d2b975a4e87b4d1bbb9a43bcb3a01c8380ba)  0.016 BTC (1600000)
            P2WPKH addr:bcrt1q2gnd9wt45n58kngmhwdy809n5qwg8q96enen2q
       out  0[] PUSHDATA(20)[1b3e28d844ce3e7b495abc56609be4c199683fe9] (00141b3e28d844ce3e7b495abc56609be4c199683fe9)  0.006 BTC (600000)
            P2WPKH addr:bcrt1qrvlz3kzyecl8kj26h3txpxlycxvks0lfre7eq2
       fee  0.00001833 BTC/wu, 0.00007301 BTC/vkB, 0.00003694 BTC/kB  0.00001234 BTC
    } 
    Nov-30 13:09:58.018 [JavaFX Application Thread] INFO  o.b.core.TransactionBroadcast: Waiting for 1 peers required for broadcast, we have 1 ... 
    Nov-30 13:09:58.019 [JavaFX Application Thread] INFO  o.b.core.TransactionBroadcast: broadcastTransaction: We have 1 peers, adding 95e2218d447731be5a9da8e6cb8baf19a357c6dd009c0fd99989a67a6bd1c22f to the memory pool 
    Nov-30 13:09:58.019 [JavaFX Application Thread] INFO  o.b.core.TransactionBroadcast: Sending to 1 peers, will wait for 0, sending to: Peer{[127.0.0.1]:18444, version=70015, subVer=/Satoshi:0.19.0.1/, services=1037 (NETWORK, BLOOM, WITNESS, NETWORK_LIMITED), time=2020-11-30 13:08:44, height=4535} 
    Nov-30 13:09:58.022 [JavaFX Application Thread] ERROR b.d.m.o.w.ManualPayoutTxWindow: onSuccess 

@ghost
Copy link

ghost commented Nov 30, 2020

Ok got it to work... had to make the Tx fee equal to the amount left over in the UTXO it is spending... in this example it had to be 0.00003388. Strange that it still worked in legacy tx though. Anyway, sorry for bothering you.

@oscarguindzberg
Copy link
Contributor Author

Yes default security deposit is 0.006 for a 0.01 trade

you are right about that.

Copy link
Contributor

@ripcurlx ripcurlx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK

@ripcurlx ripcurlx merged commit 740071d into bisq-network:master Dec 1, 2020
@ripcurlx ripcurlx added this to the v1.5.1 milestone Dec 1, 2020
@ghost ghost mentioned this pull request Dec 2, 2020
@ghost ghost mentioned this pull request Dec 27, 2020
@ghost ghost mentioned this pull request Jan 24, 2021
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 this pull request may close these issues.

Emergency Multisig payout tool - java.lang.NullPointerException
3 participants