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 type data to TxOut #2609

Open
yancyribbens opened this issue Mar 18, 2024 · 15 comments
Open

Add type data to TxOut #2609

yancyribbens opened this issue Mar 18, 2024 · 15 comments

Comments

@yancyribbens
Copy link
Contributor

Is it possible to add a type field to TxOut which describes the type of output (P2WPKH, P2PKH..? That way, we can know the predicted weight if the TxOut is used in a new transaction as TxIn by using InputWeightPrediction. InputWeightPrediction can't be used if the type isn't known AFAIK. Is this not implemented because it's not possible for some reason or breaks some design pattern? I know there are other things more pressing things rn but mostly just inquiring if this is a possibility in a future revision?

@apoelstra
Copy link
Member

There are methods on TxOut::script_pubkey that will compute this for you.

@yancyribbens
Copy link
Contributor Author

There are methods on TxOut::script_pubkey that will compute this for you.

@apoelstra thanks, that would make things simpler. It looks like TxOut::script_pubkey is type ScriptBuf. I see on type Script there is a number of bools, for example pub fn is_p2sh(&self) -> bool {. I'm not sure if I'm missing a more straigtforward method to compute this, but one option I guess is to loop through the provided bools until one returns true for the type, then use IWP (Input Weight Prediction) once the type is known? Also I'm guessing this needs to be cast from ScriptBuf to Script since the methods are on Script. Surely I'm missing a simpler way to do this..

@sanket1729
Copy link
Member

If we add additional fields to TxOut, we will need to make the existing fields private to make that the invariants are respected with the new field. Will be a big breaking change for all users.

@apoelstra
Copy link
Member

We would also likely want to populate those fields on construction and deserialization, which could be a perf cost that all users would need to pay regardless of whether they cared about this new data.

@yancyribbens
Copy link
Contributor Author

If we add additional fields to TxOut, we will need to make the existing fields private to make that the invariants are respected with the new field. Will be a big breaking change for all users.

We would also likely want to populate those fields on construction and deserialization, which could be a perf cost that all users would need to pay regardless of whether they cared about this new data.

Instead of a new field then, maybe just a net new method on TxOut or Script which shouldn't be a breaking change. That method would examine Script to determine the type and then either return Weight or InputWeightPrediction for the predicated weight of the corresponding TxIn (The TxIn that consumes this TxOut)?

@yancyribbens
Copy link
Contributor Author

Or a free function that takes TxOut as an argument and returns the predicted weight of the corresponding TxIn perhaps..

@yancyribbens
Copy link
Contributor Author

Actually, a free function that takes Script as an argument and returns the predicated weight of spending/redeeming the script would be great. Maybe I'll try to hack something like that together I guess since it doesn't exists AFAIK

@yancyribbens
Copy link
Contributor Author

s/exists/exist

@apoelstra
Copy link
Member

We could create a non-exhaustive enum that covers all the output template types that we support. But several give us no useful input weight prediction (notably p2sh and p2tr). I'm not entirely opposed to it, but it's overlapping with miniscript functionality.

@sanket1729
Copy link
Member

You can create an Address from Txout via Address::from_script and then get the AddressType . Somewhat indirect, but works.

@apoelstra
Copy link
Member

Oh, lol, if we already have that enum we ought to stick a method onto Script that returns an Option<AddressType>.

cc @TomBriar who was also asking for something like this recently

@yancyribbens
Copy link
Contributor Author

You can create an Address from Txout via Address::from_script and then get the AddressType . Somewhat indirect, but works.

Thanks @sanket1729 , although rust-bitcoin predict_weight doesn't seem to glue together very well currently with the type returned from AddressType AFAICT.

I'm not entirely opposed to it, but it's overlapping with miniscript functionality

@apoelstra using mini script sounds good, I forgot there was the desc.max_weight_to_satisfy(). I see it's possible to create a descriptor from a string, although how about just passing parsing a Bitcoin Script to miniscript::Descriptor? It seems like that would be ideal..

@apoelstra
Copy link
Member

apoelstra commented Mar 22, 2024

You can parse a bitcoin script to miniscript::Miniscript and then embed this in a Descriptor. I think in some circumstances this won't work.

We have something similar in the interpreter module where you need a bunch of extra transaction context beyond just the script, so you can see what extra data might be needed.

But intuitively yes, we should have a method that does this. Even if it sometimes fails if it's missing information.

@yancyribbens
Copy link
Contributor Author

Hmm ok. I'm reading through interpreter and it looks like a descriptor can be constructed from inferred_descriptor()? And then that can be used to find the satisfaction_weight. Seems a bit round about to create a descriptor just to find the satisfaction_weight. In a perfect world, there would just be a method on TxIn to get the satisfaction weight instead of jumping through the hoops of creating a descriptor, especially if such inference can fail.

@apoelstra
Copy link
Member

If you don't have enough information to create a descriptor, you don't have enough information to infer the satisfaction weight.

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

No branches or pull requests

3 participants