-
Notifications
You must be signed in to change notification settings - Fork 141
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
Another attempt at max_satisfaction_weight
fixes
#476
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -72,6 +72,34 @@ impl<Pk: MiniscriptKey> Wsh<Pk> { | |||||
Ok(()) | ||||||
} | ||||||
|
||||||
/// Computes an upper bound on the difference between a non-satisfied | ||||||
/// `TxIn`'s `segwit_weight` and a satisfied `TxIn`'s `segwit_weight` | ||||||
/// | ||||||
/// Assumes all ECDSA signatures are 73 bytes, including push opcode and | ||||||
/// sighash suffix. | ||||||
/// | ||||||
/// # Errors | ||||||
/// When the descriptor is impossible to safisfy (ex: sh(OP_FALSE)). | ||||||
pub fn max_weight_to_satisfy(&self) -> Result<usize, Error> { | ||||||
let (redeem_script_size, max_sat_elems, max_sat_size) = match self.inner { | ||||||
WshInner::SortedMulti(ref smv) => ( | ||||||
smv.script_size(), | ||||||
smv.max_satisfaction_witness_elements(), | ||||||
smv.max_satisfaction_size(), | ||||||
), | ||||||
WshInner::Ms(ref ms) => ( | ||||||
ms.script_size(), | ||||||
ms.max_satisfaction_witness_elements()?, | ||||||
ms.max_satisfaction_size()?, | ||||||
), | ||||||
}; | ||||||
// stack size varint difference between non-satisfied (0) and satisfied | ||||||
// `max_sat_elems` is inclusive of the "witness script" (redeem script) | ||||||
let stack_varint_diff = varint_len(max_sat_elems) - varint_len(0); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Seems like |
||||||
|
||||||
Ok(stack_varint_diff + varint_len(redeem_script_size) + redeem_script_size + max_sat_size) | ||||||
} | ||||||
|
||||||
/// Computes an upper bound on the weight of a satisfying witness to the | ||||||
/// transaction. | ||||||
/// | ||||||
|
@@ -81,6 +109,7 @@ impl<Pk: MiniscriptKey> Wsh<Pk> { | |||||
/// | ||||||
/// # Errors | ||||||
/// When the descriptor is impossible to safisfy (ex: sh(OP_FALSE)). | ||||||
#[deprecated(note = "use max_weight_to_satisfy instead")] | ||||||
pub fn max_satisfaction_weight(&self) -> Result<usize, Error> { | ||||||
let (script_size, max_sat_elems, max_sat_size) = match self.inner { | ||||||
WshInner::SortedMulti(ref smv) => ( | ||||||
|
@@ -315,6 +344,19 @@ impl<Pk: MiniscriptKey> Wpkh<Pk> { | |||||
} | ||||||
} | ||||||
|
||||||
/// Computes an upper bound on the difference between a non-satisfied | ||||||
/// `TxIn`'s `segwit_weight` and a satisfied `TxIn`'s `segwit_weight` | ||||||
/// | ||||||
/// Assumes all ec-signatures are 73 bytes, including push opcode and | ||||||
/// sighash suffix. | ||||||
pub fn max_weight_to_satisfy(&self) -> usize { | ||||||
// stack items: <varint(sig+sigHash)> <sig(71)+sigHash(1)> <varint(pubkey)> <pubkey> | ||||||
let stack_items_size = 73 + Segwitv0::pk_len(&self.pk); | ||||||
// stackLen varint difference between non-satisfied (0) and satisfied | ||||||
let stack_varint_diff = varint_len(2) - varint_len(0); | ||||||
stack_varint_diff + stack_items_size | ||||||
} | ||||||
|
||||||
/// Computes an upper bound on the weight of a satisfying witness to the | ||||||
/// transaction. | ||||||
/// | ||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you need specify something like this:
It would be nice to have some tests against random descriptors that checks this does actually hold. Do we have a method of doing proptests like this already?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, updated!
I agree that it would be nice to have some tests, but unfortunately we have to wait for the newest rust-bitcoin to be released and for rust-miniscript to be updated before being able to write any.
I'm not entirely sure, I guess I could expand
tests/test_desc.rs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah right. Tests for this equality will have to wait I guess.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Re proptests, we have a foray into Kani in rust-bitcoin rust-bitcoin/rust-bitcoin#1415 but I doubt Kani (which does model-checking using a SMT solver) could handle stuff here, unfortunately.