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.

Sign up for GitHub

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

Jump to bottom

Add missing check for mandatory bsq output #2614

Merged
merged 3 commits into from
Apr 2, 2019
Merged

Add missing check for mandatory bsq output #2614

merged 3 commits into from
Apr 2, 2019

Conversation

ManfredKarrer
Copy link
Contributor

@ManfredKarrer ManfredKarrer commented Apr 2, 2019 •
edited
Loading

@ripcurlx This is an important fix which should be included in the release!

@sqrrm If you have time for a critical review would be great!

Fixes #2613

@ManfredKarrer
Refactoring: rename local variable
6387cfa
@ManfredKarrer
Add check for mandatory BSQ output for compensation tx
69a2a04 
Fixes bisq-network#2613

If the user has the exact fee in his wallet no BSQ change output would
be created but that violates our requirement for compensation or
reimbursement txs.
Beside that the error message for dust outputs was not clear.
@ManfredKarrer ManfredKarrer added this to the v0.9.6 milestone Apr 2, 2019
ripcurlx
ripcurlx approved these changes Apr 2, 2019
View reviewed changes
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

devinbileck
devinbileck approved these changes Apr 2, 2019
View reviewed changes
Copy link
Member

@devinbileck devinbileck left a comment

Choose a reason for hiding this comment

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

ACK
Tested with no change output (2 BSQ available), and below minimum dust output (<7.46 BSQ available).

core/src/main/java/bisq/core/btc/wallet/BsqWalletService.java Show resolved Hide resolved
@ripcurlx ripcurlx merged commit 6e2aaec into bisq-network:master Apr 2, 2019
sqrrm
sqrrm reviewed Apr 2, 2019
View reviewed changes
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

core/src/main/java/bisq/core/btc/wallet/BsqWalletService.java Show resolved Hide resolved
core/src/main/java/bisq/core/dao/node/parser/TxParser.java Show resolved Hide resolved
@devinbileck devinbileck mentioned this pull request Apr 2, 2019
Closed
ManfredKarrer added a commit that referenced this pull request Apr 3, 2019
@ManfredKarrer
Improve error message
8d22d7b 
See #2620
#2614 (comment)
@ManfredKarrer ManfredKarrer deleted the add-missing-check-for-mandatory-bsq-output branch April 4, 2019 17:56
@ripcurlx ripcurlx mentioned this pull request May 3, 2019
Closed
@devinbileck devinbileck mentioned this pull request May 4, 2019
Closed
@sqrrm sqrrm mentioned this pull request May 7, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sqrrm sqrrm sqrrm left review comments

@ripcurlx ripcurlx ripcurlx approved these changes

@devinbileck devinbileck devinbileck approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v0.9.6
Development

Successfully merging this pull request may close these issues.

Issues submitting proposal with less than 7.46 BSQ available
4 participants
@ManfredKarrer @ripcurlx @devinbileck @sqrrm