search

LNP-BP / LNPBPs

LNP/BP standards for bitcoin layer 2 & 3 protocols
https://standards.lnp-bp.org
202 stars 39 forks source link

Typos and clarification #106

Closed Sosthene00 closed 3 years ago

Sosthene00 commented 3 years ago

Most proposed modifications are typos and small errors, except for the following points that might be a misunderstanding from my part and probably need further clarification:

  1. Compatibility with Taproot: it seems that the commitment will use the taproot internal key (which is already a sum of n public keys) as S. This seems reasonable, but then in the Compatibility section we read it requires exposure of the complete script with all it branches, allowing verification that all public keys participating in the script were the part of the commitment procedure, which is not very clear but may imply that we need all the keys used in all the scripts commited in the taproot output keys. My understanding is that disclosing the original taproot internal key and the taptweak (as indicated in the extra transaction proof) is enough to compute the taproot output key and verify the commitment, and that we don't need all the scripts commited in the taptweak, let alone all the pubkeys involved in all those scripts.
  2. it seems there is a contradiction between Deterministic public key extraction from Bitcoin Script and the same subtitle in the Rationale part:
    • The first says if a public key hash is met (pk_hMiniscript command) and it can't be resolved against known public keys or other public keys extracted from the script, fail the procedure
    • the other The procedure fails on any public key hash present in the script I spontaneously corrected the second by adding the same mention than the first, but thinking again about it it's probably the other way around, since as it is said somewhere else you can't be sure that a hash refers to a public key or something else, and that P2SH's RedeemScript always contains public keys, not their hash. As for custom P2SH, it would be a strange corner case that the hashed script contains itself a hashed pubkey, as the pubkey is commited itself in the script's hash anyway. Or maybe there's a subtlety of Miniscript in this that I'm not aware of.
dr-orlovsky commented 3 years ago

I also found that I did some of the changes with a PR to LNPBP-2, which I forgot to merge: https://github.com/LNP-BP/LNPBPs/pull/68/files

Since this PR rewrites the text in much more extended fashion, can you pls take the changes from there (most of them are formatting, but GitHub clearly shows non-formatting changes in this mode in "display rich text" mode https://github.com/LNP-BP/LNPBPs/pull/68/files?short_path=b76b834#diff-b76b834af9ed7fed1fd831c4402120cbac3716b3ad7f1d02cd67c72e40ad0030) and put them into this PR as well? If you do not have time for that, I will do that as a separate new PR after merging this one.

Sosthene00 commented 3 years ago

Hi, sorry I've been afk recently for personal reasons. Thanks for the feedback I'll try to tackle it this week.

dr-orlovsky commented 3 years ago

@Sosthene00 thank you!

Sosthene00 commented 3 years ago

I'm resolving conflicts between my PR and #68, I noticed there's a slight difference in commitment and verifying procedures:

I think I get it, since in the case we have multiple public keys they are all provided in the ETP, it makes more sense to provide PO to avoid having to iterate over the whole set to guess which one is PO. S can always be computed from the redeem/witnessScript.

I'll keep the #68 version then I think.

Sosthene00 commented 3 years ago

Anyway I'm done merging it, I tried to make sense when both versions were diverging but besides the change I mentioned above I think it was pretty straightforward. I'll get back to it later with a clearer mind to read it once again and make sure I've got everything right.