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

Reduce tx broadcast timeout #2657

Merged
merged 1 commit into from
Apr 7, 2019

Conversation

ManfredKarrer
Copy link
Contributor

Currently there is a bug in BitcoinJ causing the timeout at all BSQ
transactions.
It is because BitcoinJ does not handle confidence object correctly in
case as tx got altered after the
Wallet.complete() method is called which is the case for all BSQ txs.
We will work on a fix for that but that
will take more time. In the meantime we reduce the timeout to 5
seconds to avoid that the trade protocol runs
into a timeout when using BSQ for trade fee.

Currently there is a bug in BitcoinJ causing the timeout at all BSQ
transactions.
It is because BitcoinJ does not handle confidence object correctly in
case as tx got altered after the
Wallet.complete() method is called which is the case for all BSQ txs.
We will work on a fix for that but that
will take more time. In the meantime we reduce the timeout to 5
seconds to avoid that the trade protocol runs
into a timeout when using BSQ for trade fee.
@ManfredKarrer
Copy link
Contributor Author

ManfredKarrer commented Apr 5, 2019

This follows after a discussion with @oscarguindzberg about his finding of the tx broadcast bug.
It will require more time to get the proper bugfix for the underlying problem (tx confidence handling in btcj). The issues only happens at BSQ txs as there the complete methods is called before the final tx is created. We discussed different intermediate fixes but decided to not add any risk and just reduce the timeout so that it does not cause issues on the trade protocol as well to not force a too long delay at proposal or blind vote publishings.
This happen only if you are connected to more then 1 btc peer, not on local regtest or dao_regtest....

The tx broadcast itself has no bug, it is just that we don't hear back rom peers because the confidence object has changed and the success handler depends on that. So changing to a rather short time of 5 seconds is justified. The trade protocol has 1 minute timeout and to lose with the trade fee tx publishing too much time would risk that the trade protocol gets into a timeout error. This would be only the case when BSQ is used as trade fee.

@ManfredKarrer ManfredKarrer modified the milestones: v1.0.0, v0.9.8 Apr 5, 2019
Copy link
Member

@sqrrm sqrrm left a comment

Choose a reason for hiding this comment

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

utACK

@ManfredKarrer ManfredKarrer merged commit 6e9edc7 into bisq-network:master Apr 7, 2019
@ManfredKarrer ManfredKarrer deleted the reduce-timeout branch April 7, 2019 19:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants