fix(typecheck): correct d: and hash-fragment witness counting - #1015
Open
guggero wants to merge 2 commits into
Open
fix(typecheck): correct d: and hash-fragment witness counting#1015guggero wants to merge 2 commits into
guggero wants to merge 2 commits into
Conversation
Two of the values that rust-bitcoin#859 (f57cac0 "typeck: pull satisfaction/ dissatisfaction limits into struct") moved into `SatData` ended up in the wrong field, so `ExtData` no longer agrees with the witness the satisfier produces: * `cast_dupif` used to add `(w + 2, s + 1)` to the (witness size, scriptsig size) pair and 1 to the element count. It now adds 1 to the witness size and 2 to the element count: the witness size took the scriptsig's increment and the count took the witness size's. A `d:` satisfaction pushes a single element holding `0x01`, which is two bytes in a witness (its length prefix and the byte itself) and one byte in a scriptsig (`OP_1`), one element either way. `Placeholder::PushOne` sizes that element as two, and `or_i`, whose `<1>` selector is the very same element, still adds `(2, 1)`. * The four hash fragments used to have a `stack_elem_count_dissat` of 1 and now have a `dissat_data.max_witness_stack_count` of 2, which is the value of the `exec_stack_elem_count_dissat` that sat next to it. Dissatisfying a hash fragment pushes a single 32-byte element, as the 33-byte `max_witness_stack_size` beside it already says. The witness size of `d:` is the consequential one, because it is an under-estimate and `max_satisfaction_size` feeds `max_weight_to_satisfy` for every descriptor type: a bare, sh, wsh or tr descriptor whose miniscript contains a `d:` wrapper under-states the weight of its input by one weight unit per wrapper, so a fee computed from it lands below the intended feerate. The two element counts are over-estimates, which only cost their users precision. Both are in 13.0.0, 13.0.1 and 13.1.0. This area of the same refactor has needed one follow-up correction already (f3ea32c, the `thresh` execution stack count). Found while differential-testing an independent port of this crate, which disagreed on the witness size of the expressions whose satisfaction goes through a `d:` wrapper (242 of ~16k) and on the element count of the hash dissatisfactions.
The satisfaction sizes in `ExtData` and the witness template the satisfier builds are two independent accountings of the same witness, so they can be held against each other: `dv:older(144)` pins the size of the `<1>` element that a `d:` pushes, and an `andor` whose hash branch has to be dissatisfied pins the element count of a hash dissatisfaction. Both assertions fail without the previous commit.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two of the values that #859 (f57cac0 "typeck: pull satisfaction/
dissatisfaction limits into struct") moved into
SatDataended up in thewrong field, so
ExtDatano longer agrees with the witness the satisfierproduces:
cast_dupifused to add(w + 2, s + 1)to the (witness size,scriptsig size) pair and 1 to the element count. It now adds 1 to the
witness size and 2 to the element count: the witness size took the
scriptsig's increment and the count took the witness size's. A
d:satisfaction pushes a single element holding
0x01, which is two bytesin a witness (its length prefix and the byte itself) and one byte in a
scriptsig (
OP_1), one element either way.Placeholder::PushOnesizes that element as two, and
or_i, whose<1>selector is the verysame element, still adds
(2, 1).The four hash fragments used to have a
stack_elem_count_dissatof 1and now have a
dissat_data.max_witness_stack_countof 2, which is thevalue of the
exec_stack_elem_count_dissatthat sat next to it.Dissatisfying a hash fragment pushes a single 32-byte element, as the
33-byte
max_witness_stack_sizebeside it already says.The witness size of
d:is the consequential one, because it is anunder-estimate and
max_satisfaction_sizefeedsmax_weight_to_satisfyfor every descriptor type: a bare, sh, wsh or tr descriptor whose
miniscript contains a
d:wrapper under-states the weight of its input byone weight unit per wrapper, so a fee computed from it lands below the
intended feerate. The two element counts are over-estimates, which only
cost their users precision.
Both are in 13.0.0, 13.0.1 and 13.1.0. This area of the same refactor has
needed one follow-up correction already (f3ea32c, the
threshexecutionstack count).
Found while differential-testing an independent port of this crate, which
disagreed on the witness size of the expressions whose satisfaction goes
through a
d:wrapper (242 of ~16k) and on the element count of the hashdissatisfactions.