Skip to content

Conversation

@antonilol
Copy link
Contributor

  • Save a byte in some scripts
  • Fix a script
  • Set sequence to 1 in second stage HTLC
  • Remove trailing whitespace

see the review comments at the specific lines

"tapleaf_2": "
# funds go to the remote node via a second-stage Claim-PTLC-success transaction by completing an adaptor sig, revealing the payment secret
# NB: we don't use musig2 here because it would force local and remote signatures to use the same sighash flags
<local_ptlcpubkey> OP_CHECKSIGVERIFY <remote_ptlcpubkey> OP_CHECKSIG
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why are there two tapleaves when the only difference between the two is the order? one can be removed to save 32 vbytes

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is only for readability, right?

- Save bytes in some scripts
- Fix a script
- Set sequence to 1 in second stage HTLC
- Remove trailing whitespace
@t-bast
Copy link
Owner

t-bast commented May 11, 2022

Thanks for the feedback!

I have one main comment though: I don't want the scripts here to be efficient, I want them to be as readable as possible. They can indeed be optimized (as you noticed), but this document should stay high-level(ish). Optimizing the scripts will be done with miniscript once we integrate Taproot in the bolts, that's where your proposal will make more sense.

Can you please remove the script optimizations and keep the other changes? Thanks 🙏

@antonilol
Copy link
Contributor Author

done

Copy link
Owner

@t-bast t-bast left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@t-bast t-bast merged commit d6d5dca into t-bast:master May 12, 2022
@antonilol antonilol deleted the taproot-updates branch May 12, 2022 09:51
@antonilol antonilol restored the taproot-updates branch May 12, 2022 12:36
@antonilol antonilol deleted the taproot-updates branch May 12, 2022 12:37
@antonilol antonilol restored the taproot-updates branch May 12, 2022 12:37
@antonilol antonilol deleted the taproot-updates branch May 22, 2022 20:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants