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

Manual payout tool: prevent absurdly high fee payout #5099

Merged
merged 1 commit into from Jan 22, 2021
Merged

Manual payout tool: prevent absurdly high fee payout #5099

merged 1 commit into from Jan 22, 2021

Conversation

ghost
Copy link

@ghost ghost commented Jan 21, 2021

This PR adds display of the Tx fee as a percentage, and includes validation to disallow the payout processing if tx fee is > 10% of the multisig amount. The Tx cannot be signed or built until validation passes.

Fixes a bug whereby whitespace in an amount caused it to be parsed as 0. It now trims whitespace from numeric input fields before parsing.


Screenshot showing the new Tx fee % field:

image

- trims whitespace from numeric input fields before parsing
- adds percentage display of the tx fee
- validates that the tx fee percentage is not higher than 10%
@@ -544,6 +553,8 @@ private void calculateTxFee() {
.subtract(getInputFieldAsCoin(buyerPayoutAmount))
.subtract(getInputFieldAsCoin(sellerPayoutAmount));
txFee.setText(txFeeValue.toPlainString());
double feePercent = (double) txFeeValue.value / getInputFieldAsCoin(amountInMultisig).value;
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe safer to wrap it into a try catch? E.g. if getInputFieldAsCoin(amountInMultisig).value can be 0 it would throw an exception.

Copy link
Author

Choose a reason for hiding this comment

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

No exception, it returns -Infinity. So I don't see any issue.

image

Copy link
Contributor

@chimp1984 chimp1984 left a comment

Choose a reason for hiding this comment

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

utACK

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 5b0716e into bisq-network:master Jan 22, 2021
@ripcurlx
Copy link
Contributor

I just cherry-picked it into the release branch

@ghost ghost deleted the bugfix_emerg_payout_tool branch January 24, 2021 16:23
@ghost ghost mentioned this pull request Jan 24, 2021
@ripcurlx ripcurlx added this to the v1.6.0 milestone Feb 12, 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.

2 participants