Skip to content

Weight prediction#1636

Merged
apoelstra merged 2 commits intorust-bitcoin:masterfrom
Kixunil:weight-prediction
Feb 13, 2023
Merged

Weight prediction#1636
apoelstra merged 2 commits intorust-bitcoin:masterfrom
Kixunil:weight-prediction

Conversation

@Kixunil
Copy link
Copy Markdown
Collaborator

@Kixunil Kixunil commented Feb 9, 2023

When creating a transaction one must know the the fee beforehand to set
appropriate amounts for outputs and to know the fee, weight is required.
So far we only had a method on an already-constructed transaction. This
method clearly wasn't helpful when constructing the transaction except
for hacks like temporarily adding an all-zeroes signature.

This change adds a function that can compute the transaction weight
without knowing individual bytes of the scripts, witnesses and other
elements. It only needs to know their sizes.

To make the API less error-prone a special, trivial, type is also added
for computing the lengths of witnesses.

Based on #1627

@apoelstra
Copy link
Copy Markdown
Member

Cool! I believe this is correct.

I'd like to play with this in rust-miniscript but happy to slip it into a release before then. We still have some breaking changes over the next few releases so it's not the end of the world if it turns out we want to iterate on this. (One thing I'm waffling on is whether to use usize here or u64, which is not a big deal at all to change ... and I do think usize is probably the right choice.)

utACK 62229eeddba69ed5df02a72d35b2e92c8c512c50 though let's get #1627 in first.

Copy link
Copy Markdown
Member

@tcharding tcharding left a comment

Choose a reason for hiding this comment

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

Few minor suggestions.

@Kixunil Kixunil force-pushed the weight-prediction branch 2 times, most recently from 0c4f097 to 8c7a073 Compare February 10, 2023 10:39
@Kixunil
Copy link
Copy Markdown
Collaborator Author

Kixunil commented Feb 10, 2023

Rebased and fixed typos/grammar. Should be ready for review.

@Kixunil Kixunil added this to the 0.30.0 milestone Feb 10, 2023
apoelstra
apoelstra previously approved these changes Feb 10, 2023
Copy link
Copy Markdown
Member

@apoelstra apoelstra left a comment

Choose a reason for hiding this comment

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

ACK 8c7a0736713ebae7369c53d801766623d3e7b00e

Copy link
Copy Markdown
Member

@sanket1729 sanket1729 left a comment

Choose a reason for hiding this comment

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

Reviewed the fee calculation carefully, just some comments on the docs

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Brainstorming if we can make this API a method in Transaction. Almost all users would be creating a tx with an empty witness. And then calling output.map(|o| o.script_pubkey.len()) for the output_scripts_len. Then, setting the fee output accordingly.

I guess it will be really ugly to use for inputs because some of them might have witnesses filled in.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I also thought about this a bit. I think in a later PR we can maybe make some constructors for these iterators which can be created off of a (potentially incomplete) transaction.

Agree that in the majority of cases users will be starting with some sort of transaction and trying to estimate its final form.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I had the same idea and was about to make the method for output iterator but realized that I actually don't have this flow because it would require to add an output with bogus amount (presumably 0) and then change it... Which is not really that nice. I prefer to calculate everything first and only then construct the transaction. But I can add the method back if you want, it's one line of code.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think it'd be worthwhile, yeah. I think rust-miniscript is going to use this method, and in Miniscript we assume that all inputs and outputs are present, just (potentially) not witnesses.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

OK, added.

When creating a transaction one must know the the fee beforehand to set
appropriate amounts for outputs and to know the fee, weight is required.
So far we only had a method on an already-constructed transaction. This
method clearly wasn't helpful when constructing the transaction except
for hacks like temporarily adding an all-zeroes signature.

This change adds a function that can compute the transaction weight
without knowing individual bytes of the scripts, witnesses and other
elements. It only needs to know their sizes.

To make the API less error-prone a special, trivial, type is also added
for computing the lengths of witnesses.
@Kixunil
Copy link
Copy Markdown
Collaborator Author

Kixunil commented Feb 11, 2023

Addressed documentation issues.

In some cases people construct the transaction with a dummy fee output
value before calculating the weight. A method to create the iterator
over `script_pubkey` lengths is useful in such cases.
@Kixunil
Copy link
Copy Markdown
Collaborator Author

Kixunil commented Feb 13, 2023

Seems pretty clearly latest nightly introduced some compiler bug, I've filed ICE bug.

Copy link
Copy Markdown
Member

@apoelstra apoelstra left a comment

Choose a reason for hiding this comment

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

ACK ae2aaaa

@Kixunil
Copy link
Copy Markdown
Collaborator Author

Kixunil commented Feb 13, 2023

Nightly bug confirmed and should be fixed tomorrow. I don't mind waiting those two days for merge since it looks like there's no conflicting PR but I also wouldn't mind just merging it.

Copy link
Copy Markdown
Member

@tcharding tcharding left a comment

Choose a reason for hiding this comment

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

ACK ae2aaaa

@apoelstra apoelstra merged commit f00b636 into rust-bitcoin:master Feb 13, 2023
@Kixunil Kixunil deleted the weight-prediction branch February 14, 2023 08:13
Comment on lines +1195 to +1199
/// When signing a transaction one doesn't know the signature before knowing the transaction fee and
/// the transaction fee is not known before knowing the transaction size which is not known before
/// knowing the signature. This apparent dependency cycle can be broken by knowing the length of the
/// signature without knowing the contents of the signature e.g., we know all Schnorr signatures
/// are 64 bytes long.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hey, I just saw this PR mentioned in the Bitcoin Optech Newsletter, and wanted to point out that there is a trick to avoiding the catch-22 between transaction weight, input count, and fees. When you build a transaction, you know the recipient outputs you wish to create. You also know the size of the metadata (version, input counter, output counter, locktime, optionally witness marker and witness flag). Before building a transaction, you usually also know the feerate that you wish to achieve with the transaction.

This allows you to calculate the transaction weight of the “fixed part” of the transaction, and per the feerate, the necessary fees for that fixed part. For the inputs, you calculate their effective value. For each input you calculate the weight it will take to add the specific input to a transaction, multiply that weight with the feerate, and deduct this “input fee” from that input’s value. After this preprocessing step, when you pick UTXOs to fund the transaction, you count only their effective value towards the target (i.e. recipient amounts plus “fixed parts fees”)—this makes the selection of inputs fee-neutral because the effective value accounts for the input paying for itself. The target can additionally be adjusted for whether you expect to create a change output, how valuable the change should be at least, and what output type the change is intended to have.

I described an efficient transaction building process using UTXO’s effective value here on Stack Exchange.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Where is the PR mentioned? I don't see it.

Thanks for the idea, we should definitely add support for this. Note that the approach implemented in this PR is still useful when the coin selection is literally "select all". (I have a reasonable use case for this; no, it won't destroy privacy - "all" is normally exactly one UTXO of one address, in rare cases multiple UTXOs of the same address.)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copy link
Copy Markdown

@murchandamus murchandamus Feb 22, 2023

Choose a reason for hiding this comment

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

@Kixunil: Sorry, I was looking at the open Newsletter-PR before the Newsletter got published this morning. :)

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.

6 participants