search

bitcoinops / taproot-workshop

Taproot & Schnorr Python Library & Documentation.
MIT License
369 stars 111 forks source link

Chapter 2.2 update: Added commitment unsealing to 2.2.3/2.24 & Mini case-study #132

Closed jachiang closed 4 years ago

jachiang commented 4 years ago

This update adds the following updates to taptweak chapter 2.2 :

jachiang commented 4 years ago

Builds on #131

jnewbery commented 4 years ago

I've had a quick skim and this looks good. I'll do a detailed review soon. Thanks!

jachiang commented 4 years ago

I've had a quick skim and this looks good. I'll do a detailed review soon. Thanks!

Cheers @jnewbery

jachiang commented 4 years ago

@bitschmidty - Many thanks for your review! I will apply your fixes and suggestions as soon as I can today.

jachiang commented 4 years ago

@bitschmidty Many thanks for your review. All your nits have been addressed.

This PR now builds on the following:

bitschmidty commented 4 years ago

@jachiang Tested again based on the latest. I resolved the conversion for my items. Thanks for addressing those. Left a couple more comments and also left the unresolved items as well.

jachiang commented 4 years ago

@jachiang Tested again based on the latest. I resolved the conversion for my items. Thanks for addressing those. Left a couple more comments and also left the unresolved items as well.

Thanks @bitschmidty for your review. Sorry, what are you referring to as unresolved items?

bitschmidty commented 4 years ago
jachiang commented 4 years ago 
* [#132 (comment)](https://github.com/bitcoinops/taproot-workshop/pull/132#discussion_r341773228)

* [#132 (comment)](https://github.com/bitcoinops/taproot-workshop/pull/132#discussion_r341222960)
jnewbery commented 4 years ago

@jachiang - are you still working on this? What are your thoughts about moving the OP_RETURN construction into the notebook so it's more visible to the student?

jachiang commented 4 years ago

@jnewbery - apologies for the delay. I agree with constructing OP_RETURN out in the open (it's a simple script too), and will prioritize this for an update this week.

jachiang commented 4 years ago

@jnewbery - I have included the OP_RETURN construction in part 3) with createrawtransaction. Alternatively, it could be constructed with CTransaction, by setting vouts individually, resulting in more (boilerplate) code. What do you think is better?

Edit: I have added commit "Rewrite 2.2.11 and 2.2.12 with CTransaction" so the reviewer can compare the alternative OP_RETURN construction with CTransaction.

Other updates: Commit review fixups broke example 2.2.5. at "Alice tweaks her key pair". This has now been fixed.

bitschmidty commented 4 years ago

@jachiang let me know if/when youd like me to test through the applicable notebooks on this PR

jnewbery commented 4 years ago

Rebased on master and added a commit with a few small changes.

@bitschmidty - Chapter 2.2 should be ready for final review.