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

Feerate should be returned from Transaction construction #524

Open
LLFourn opened this issue Jan 11, 2022 · 9 comments
Open

Feerate should be returned from Transaction construction #524

LLFourn opened this issue Jan 11, 2022 · 9 comments
Labels
good first issue Good for newcomers new feature New feature or request

Comments

@LLFourn
Copy link
Contributor

LLFourn commented Jan 11, 2022

When you get TransactionDetails from tx_builder.finish() it has the fee for the overall transaction but does not include the feerate of the resulting transaction. There is no simple way for the caller to replicate this information since the transaction does not have witness data at this point. This is annoying because after constructing the psbt the user should know the feerate before signing it.

This can either be done by returning another value from builder.finish() or by adding a (perhaps optional) field to TransactionDetails.

@LLFourn LLFourn added the bug Something isn't working label Jan 11, 2022
@notmandatory notmandatory added new feature New feature or request and removed bug Something isn't working labels Jan 25, 2022
@notmandatory notmandatory added good first issue Good for newcomers and removed good first issue Good for newcomers labels Feb 15, 2022
@csralvall
Copy link
Contributor

csralvall commented Mar 3, 2022

Would make sense to save the overall weight or size of the transaction and the fee rate instead of the fee amount? I think that the weight would be useful in the TransactionDetails, and we can implement a method to compute the fee amount. WDYT? I forget to mention that I was talking about a modification to CoinSelectionResult that would allow to pass the fee_rate to Transaction Details inside wallet.create_tx. However, now I think that it is an overkiller, and we can add the field just inside TransactionDetails without touching the CoinSelectionResult struct.

@thunderbiscuit
Copy link
Member

Just want to comment to say that this is also a feature I'd use if implemented.

@i5hi
Copy link

i5hi commented Apr 29, 2022

I am giving this issue a try and have a few thoughts:

Transaction size can be useful since you can calculate the fee_rate with it.

When using create_tx does it make sense to have Some(Transaction) always returned so its easy to calculate size?

Or we can just have size as a field in TransactionDetail?

fee_rate helps too but feels repetitive since there is already a fee field.

Edit: (weight + max_satisfaction_weight)* instead of size*

@LLFourn
Copy link
Contributor Author

LLFourn commented May 4, 2022

When using create_tx does it make sense to have Some(Transaction) always returned so its easy to calculate size?

Even if you did that tx doesn't have the correct weight since it is missing signatures.

@i5hi
Copy link

i5hi commented May 9, 2022

When using create_tx does it make sense to have Some(Transaction) always returned so its easy to calculate size?

Even if you did that tx doesn't have the correct weight since it is missing signatures.

The max satisfaction weight from the descriptor would give you that, right?

@LLFourn
Copy link
Contributor Author

LLFourn commented May 11, 2022

The max satisfaction weight from the descriptor would give you that, right?

If all the inputs are yours and you know which input had which descriptor. But in general no.

@danielabrozzoni danielabrozzoni added the good first issue Good for newcomers label Jul 20, 2022
@thunderbiscuit
Copy link
Member

thunderbiscuit commented Jul 25, 2022

I think we're bumping into a related issue when we attempted to RBF bump a transaction on bdk-ffi/Kotlin.

BIP 125 says that nodes will relay transactions for which the fees are bumped by at least the amount of the minimum transaction relay rate, i.e. if the minimum is 1 sat/vbyte your "currently paying" 3.0 sat/vbyte transaction will need to have a replacement transaction with at least 4.0 sat/vbyte to be related by nodes.

But since we don't know the feerate, calculating how much fees should be added to the RBF replacement transaction is not straightforward (I assume it's doable by looking at nominal fee paid and raw transaction size but having it returned explicitly as part of the transaction would make it much easier).

@thunderbiscuit
Copy link
Member

@LLFourn we've been looking at ways to provide this fee rate to the user in the bindings. Is #728 helpful for your use case? It's not providing the fee rate before the user signs the tx, but it's making it easy to retrieve it from the signed PSBT (as opposed to opening up the Transaction struct and calculating the feerate manually). Let us know if you have any feedback on the approach.

@LLFourn
Copy link
Contributor Author

LLFourn commented Sep 5, 2022

@thunderbiscuit I think you wan to know the feerate before you sign it so you know what you are actually signing. Having those methods on PSBTs seems useful but doesn't quite solve this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers new feature New feature or request
Projects
Status: Todo
Development

No branches or pull requests

6 participants