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

Segwit fee estimation #4710

Merged
merged 18 commits into from
Nov 5, 2020

Conversation

oscarguindzberg
Copy link
Contributor

@oscarguindzberg oscarguindzberg commented Oct 26, 2020

Users reduce btc miner fees for segwit txs.

Pre-segwit miners used to evaluate tx fee and tx size. Txs were selected by their fee/byte.
Segwit brought the concept of "virtual size".
virtual size of a tx = tx serialized without witness data size + witness size (i.e. pub keys and signatures) / 4.
The virtual size of a legacy tx is the same as its size.
Segwit txs have similar sizes compared to legacy txs, but their virtual size is smaller.
New terms are now used such as vsize, vbyte, fee/vbyte, etc.
Miners now compare txs by its fee/vbyte (This is not entirely true, since they use tx weight, but we can do our math using fee/vbyte which is roughly equivalent)
See https://github.com/bitcoin/bips/blob/master/bip-0141.mediawiki#additional-definitions for more info.

This PR does not include renaming size to vsize, feePerByte to feePerVbyte, etc.
I expect this PR won't be merged to master for quite some time.
The renames will introduce hundreds of changes all over the code, leading to merging hell.
I plan to do the renames either when merging date is closer or as an independent PR.

I suggest to use this link oscarguindzberg/bisq@segwit...oscarguindzberg:fee-estimation to understand the content of this PR until #4612 is merged. Otherwise you will find here the content for both PRs making understanding this PR more difficult

@oscarguindzberg
Copy link
Contributor Author

btw, the intention of this PR is not to refactor/improve the way fee calculation is done in bisq. It just pretends to take advantage of segwit fee reduction.

chimp1984
chimp1984 previously approved these changes Oct 28, 2020
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

@oscarguindzberg oscarguindzberg changed the title [WIP] Fee estimation [WIP] Segwit fee estimation Oct 28, 2020
@chimp1984
Copy link
Contributor

btw, the intention of this PR is not to refactor/improve the way fee calculation is done in bisq. It just pretends to take advantage of segwit fee reduction.

Is it planned to move to a bitcoinj native estimation model in a future PR?

@oscarguindzberg
Copy link
Contributor Author

oscarguindzberg commented Oct 28, 2020

btw, the intention of this PR is not to refactor/improve the way fee calculation is done in bisq. It just pretends to take advantage of segwit fee reduction.

Is it planned to move to a bitcoinj native estimation model in a future PR?

I have no plans to add any further code regarding fee estimation. What do you mean by "bitcoinj native estimation model"? Bisq uses bitcoinj fee estimation code and builds on top of it. Maybe you mean to use bicoinj 0.15's SendRequest.recipientsPayFees feature?. I think it would be great to rewrite bisq's fee estimation code, but I think it is out of scope for the segwit integration project.

@chimp1984
Copy link
Contributor

I was referring to the fee estimation you implemented for BitcoinJ. I don't know any details about it, but I thought it was planned to use that in future.

@oscarguindzberg oscarguindzberg changed the base branch from master to release/v1.5.0 November 5, 2020 14:53
@oscarguindzberg oscarguindzberg changed the base branch from master to release/v1.5.0 November 5, 2020 14:53
@oscarguindzberg oscarguindzberg changed the title [WIP] Segwit fee estimation Segwit fee estimation Nov 5, 2020
@oscarguindzberg
Copy link
Contributor Author

  • WIP tag removed
  • Pointed PR to release/v1.5.0

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

@ripcurlx ripcurlx merged commit 93806e9 into bisq-network:release/v1.5.0 Nov 5, 2020
@ripcurlx ripcurlx added this to the v1.5.0 milestone Nov 9, 2020
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.

3 participants