bip-0360: added python reference example; test vector fixes#2202
bip-0360: added python reference example; test vector fixes#2202notmike-5 wants to merge 17 commits into
Conversation
…d to match tree structure of test vectors (bitcoin#45)
…ng construction of P2MR output (bitcoin#46)
This commit adds a python reference example for construction of BIP-360 outputs and control blocks. This commit also updates the test vectors.
murchandamus
left a comment
murchandamus
left a comment
There was a problem hiding this comment.
Looks reasonable at first glance.
cc: @cryptoquick, @EthanHeilman, @Isabelfoxenduke for owner review/sign-off.
murchandamus
added
Proposed BIP modification
This refactors `compute_control_block` to improve performance, and make the function simpler to read (to me at least). The previous version walked the entire script tree searching for the first matching instance of the leaf node. The new version expects the caller to pass in an explicit `path` parameter, which tells us exactly where the leaf node lives, with left/right steps encoded as bits in an integer. We walk down the tree straight to that leaf node, and build the control block as we go. This might not be the best DX for a real-world API or library, but this is just reference code, so we can accept poor usage ergonomics if it makes the code clearer and more explicit.
| # Bech32 encoding code is taken from sipa (BIP-0350), and has been tested against the test vectors therein: | ||
| # https://github.com/sipa/bech32/blob/master/ref/python/tests.py |
There was a problem hiding this comment.
Most of this code already exists in the BIPs repository, under the reference code of BIP352 (silent payments).
https://github.com/bitcoin/bips/blob/master/bip-0352/bech32m.py
@murchandamus What are your thoughts on the bech32m code duplication here? is it better to duplicate for the sake of compartmentalization, or should we update the existing code and reference it here?
There was a problem hiding this comment.
I’d prefer it to be compartmentalized. This is not a software project, so I would prefer if we don’t slip into starting to maintain the reference implementations of all the BIPs here.
There was a problem hiding this comment.
Would it be more convenient if we move the BIP360 reference implementations into an external repository?
There was a problem hiding this comment.
OK, i'll see about closing this PR out and moving the code to a new repository. Please give us some time to get the new repo set up and the code moved over
bip360: simplify computing control blocks using explicit traversal paths
Implements @conduitions improvement on tapbranch_hash() Co-authored-by: conduition <conduition@proton.me>
Standardizes all P2MR-specific functions to use bytes uniformly for input/output. Hex conversions are now confined to two boundaries: reading `script` field out of ScriptTree input, and comparing against hex-encoded test vector data in `run_single_test`. bech32 functions and s2w are left unchanged.
|
Thanks for this work. I'm a little confused as to where it's going to land though. Is there a decision being made regarding reference implementations? Will they all be stripped from the BIPs repository? Will they be fragmented or live in a separate repository? Where is this policy and decision making documented? Who is being consulted in this decision and who are the stakeholders? |
6 participants
This PR adds a python reference example for construction of BIP-360 outputs and control blocks.
It updates the test vectors to remove "internalPubkey" key from vectors 7 and 8, and adds a new test for duplicate script leaves.
This PR also brings in fixes to #2102 and #2103 from @jbride