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

Add an extra byte to TX size when calculating Bitcoin TX fees #3358

Closed
wants to merge 1 commit into from

Conversation

wiz
Copy link
Member

@wiz wiz commented Oct 3, 2019

After investigating, it seems Bisq will sometimes create Bitcoin transactions with less than 1 sat/byte fee for TX, caused by:

  1. User has set 1 sat/byte fee in settings
  2. TX size gets rounded down when converting from bytes to vbytes (25% chance)

For example:
2080/4 = 520 (good)
2081/4 = 520.25 (bad, gets rounded down to 520)
2082/4 = 520.5 (good, gets rounded up to 521)
2083/4 = 520.75 (good, gets rounded up to 521)

In the case TX is 2081 bytes, 520 sats of fee gets used, for what should be considered a 521 vbyte TX. This results in a net fee of 0.998 sats/vbyte, which won't get relayed through Bitcoin network with the latest Bitcoin Core default settings.

This PR fixes the above issue by adding a workaround to add 1 byte to the TX size when calculating the fee, so it always "rounds up"

@wiz wiz force-pushed the fee-estimation-add-one-satoshi branch 3 times, most recently from 9c3039c to 531db83 Compare October 10, 2019 09:10
@wiz wiz changed the title Add an extra sat to fee calculation to prevent rounding down bug Add an extra byte to TX size when calculating Bitcoin TX fees Oct 10, 2019
@wiz wiz force-pushed the fee-estimation-add-one-satoshi branch from 531db83 to 385c1dd Compare October 10, 2019 09:26
@wiz wiz marked this pull request as ready for review October 10, 2019 09:33
@wiz wiz requested a review from ripcurlx as a code owner October 10, 2019 09:33
@wiz
Copy link
Member Author

wiz commented Oct 10, 2019

This PR was replaced by #3387

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.

None yet

1 participant