Fix max_satisfaction_weight calculations for taproot.
#473
Conversation
|
This work is based on my understanding of BIP-0141 and BIP-0341. I'm not sure what is the best way to test this, any ideas will be appreciated. |
| let tree = match self.taptree() { | ||
| // key spend path: | ||
| // scriptSigLen(4) + stackLen(1) + stack[Sig]Len(1) + stack[Sig](65) | ||
| None => return Ok(4 + 1 + 1 + 65), |
There was a problem hiding this comment.
nit: the 65 here is implicitly taking into account an optional sighash flag (because we're doing max I guess this strictly correct). Maybe worth doing a 64 + 1 to indicate this.
There was a problem hiding this comment.
@LLFourn Thank you for the review.
However, at the same time, the sig+sigHash is a single stack item? Maybe a comment would be more appropriate?
|
I think you need to update some tests: for me locally. |
|
Sorry, but could you squash these commitst so the tests pass on every commit? |
* Key spend path should take into consideration the scriptSigLen, stackLen and itemLen (for the one and only stack item). * Script spend path should consider `control_block` and `script` to be items on the stack. I also restructured the function to improve readability.
evanlinjin
force-pushed
the
fix_max_satisfaction_weight_calculations
branch
from
edc583c to
8834b7f
Compare
@apoelstra All done :) |
There was a problem hiding this comment.
utACK 8834b7f. Thanks for the significant code improvement.
|
I'd like to cut a release today. Should we revert this for now? |
@apoelstra I don't think this fix conflicts with the conversation we were having in regards to what would be the definition of "max satisfaction weight". I think we should continue discussing whether we should merge #474 or go with #476 instead. Let me know your thoughts on those PRs. |
…022_10 22ac742 Merge rust-bitcoin/rust-miniscript#473: Fix `max_satisfaction_weight` calculations for taproot.
control_blockandscriptto be items on the stack.I also restructured the function to improve readability.