@arminsabouri @jonatack thank you for taking the time to review! I've gone ahead and addressed your comments in
9a47c29

thanks @jonatack! i think i got everything!

Thanks for updating.

Final verdict: 95 % LLM-generated or LLM-heavy, with ~5 % human origination of the core idea and final review.

@jurvis can you help me out, please: Is this accurate? I'm trying to adapt our process and understanding.

hi @jonatack, happy to help. does the scan run through the sample code as well? the sample code contains a lot of boilerplate that we leveraged LLMs to help us with, and may be the reason why it registers high.

However, the contents of the mediawiki itself should be ~90% original. We leaned on LLM use in the mediawiki mostly to ensure that our formatting was aligned with existing conventions (heavily referencing BIP 340, BIP 352, BIP 32, and BIP 3), and to convert it from Markdown, which we had originally used to write our draft.

For example, we wrote out our original one of the algorithms in the following format originally:

Signing
======
When the counterparty requests that the collaborative custodian sign a transaction, it derives the BIP32 scalar tweak from the xpub (i.e. the value `parse_256(I_L)` from BIP32) and provides it to the custodian:

# Inputs:
#   chain_code : 32-byte chain code (hidden from custodian)
#   P_par      : custodian’s parent public key (compressed)
#   i          : child index for spending

I   = HMAC-SHA512(key = chain_code,
                 data = serP(P_par) || ser32(i))
I_L = I[0:32]            # left half
t_i = parse256(I_L)      # scalar tweak mod n
# (I_R would be the child chain code—discarded here.)

# Counterparty → Custodian: send t_i

Which we got ultimately turned into this, to align with how BIP 32 reads:

=== Tweak Calculation ===
To produce CCD tweak data, a delegatee computes a per-participant scalar that aggregates the non-hardened derivation tweaks along the remaining path. Let the extended key retained by the delegatee be P at depth d, and let the target index vector be I = (i<sub>d+1</sub>, …, i<sub>n</sub>) with each i<sub>k</sub> < 2<sup>31</sup>.

<div>
Algorithm ''ComputeBIP32Tweak(P, I)'':
* Inputs:
** ''P'': base public key at depth ''d''
** ''I = (i<sub>d+1</sub>, …, i<sub>n</sub>)'': ordered sequence of non-hardened child indices
* Let ''t = 0'' and ''E = P''.
* For each index ''i'' in ''I'' (from left to right):
** Run the BIP32 non-hardened derivation ''CKDpub'' on ''E'' with child index ''i'', yielding the child extended key ''P<sub>child</sub>'' and its scalar tweak ''δ'' (the parse<sub>256</sub>(''I<sub>L</sub>'') term from BIP32).
** Let ''t = (t + δ) mod n''.
** Let ''E = P<sub>child</sub>''.
* If ''I'' is empty, let ''P′ = P''; otherwise let ''P′ = P<sub>child</sub>'' from the final iteration.
* Return ''(t, P′)''.
</div>

Hope that helps.

hi @jonatack, happy to help. does the scan run through the sample code as well?

Gave it the BIP draft only.

hi @jonatack just following up to see if there's anything I can do here to move things along. thanks!

The inclusion of major parts of secp256k1lab in this form violates the (only) requirement in the MIT license under which secp256k1lab is distributed, see its COPYING file.

A related editorial question is whether it's a good idea to include distributions of secp256k1lab in the BIPs repo. This question is relevant not only for this BIP draft but also for #2070 and for the ChillDKG draft BIP (no PR yet).

We had raised that question in the context of ChillDKG over a year ago on the mailing list, specifically hoping to hear the opinion of the BIPs editors, but noone has commented so far: https://groups.google.com/g/bitcoindev/c/HE3HSnGTpoQ/m/Y2VhaMCrCAAJ (This post refers to the library as "secp256k1proto". This was our working title before we switched to "secp256k1lab" when publishing the code in a separate repo.)

We had raised that question in the context of ChillDKG over a year ago on the mailing list, specifically hoping to hear the opinion of the BIPs editors, but noone has commented so far: https://groups.google.com/g/bitcoindev/c/HE3HSnGTpoQ/m/Y2VhaMCrCAAJ (This post refers to the library as "secp256k1proto". This was our working title before we switched to "secp256k1lab" when publishing the code in a separate repo.)

@real-or-random: TBH, I don’t think I fully understand the trade-offs of the approaches you proposed. I would prefer that any software with on-going development were maintained outside of the BIPs repository, but maybe we could chat some time so that I better understand the situation.

We had raised that question in the context of ChillDKG over a year ago on the mailing list, specifically hoping to hear the opinion of the BIPs editors, but noone has commented so far: groups.google.com/g/bitcoindev/c/HE3HSnGTpoQ/m/Y2VhaMCrCAAJ (This post refers to the library as "secp256k1proto". This was our working title before we switched to "secp256k1lab" when publishing the code in a separate repo.)

@real-or-random: TBH, I don’t think I fully understand the trade-offs of the approaches you proposed. I would prefer that any software with on-going development were maintained outside of the BIPs repository, but maybe we could chat some time so that I better understand the situation.

Let me try to simplify the discussion: If the BIP editors are okay with integrating a snapshot of secp256klab as a subtree in each bip-xxxx folder of a BIP whose code uses secp256k1lab (as currently proposed in this PR and in #2070), then I'm happy with that. I think it's the best solution.

"Snapshot" means that it is a static thing, and maintenance of secp256k1lab will, of course, happen in its repo. I guess the only reason to ever touch a snapshot subtree in the BIPs repo is to fix a secp256k1 bug that specifically affects the BIP code that uses the library (and even in that case, other ways to resolve the overall issue could be considered).

We had raised that question in the context of ChillDKG over a year ago on the mailing list, specifically hoping to hear the opinion of the BIPs editors, but noone has commented so far: groups.google.com/g/bitcoindev/c/HE3HSnGTpoQ/m/Y2VhaMCrCAAJ (This post refers to the library as "secp256k1proto". This was our working title before we switched to "secp256k1lab" when publishing the code in a separate repo.)

@real-or-random: TBH, I don’t think I fully understand the trade-offs of the approaches you proposed. I would prefer that any software with on-going development were maintained outside of the BIPs repository, but maybe we could chat some time so that I better understand the situation.

Let me try to simplify the discussion: If the BIP editors are okay with integrating a snapshot of secp256klab as a subtree in each bip-xxxx folder of a BIP whose code uses secp256k1lab (as currently proposed in this PR and in #2070), then I'm happy with that. I think it's the best solution.

"Snapshot" means that it is a static thing, and maintenance of secp256k1lab will, of course, happen in its repo. I guess the only reason to ever touch a snapshot subtree in the BIPs repo is to fix a secp256k1 bug that specifically affects the BIP code that uses the library (and even in that case, other ways to resolve the overall issue could be considered).

That seems reasonable to me, as it ensures that a functional reference implementation is retained in the context of the BIPs. The only concern that comes to mind would be that we might get more pull requests by LLM-commit farmers on those, but I guess we’d just have to filter those.

@murchandamus Thanks for the detailed review, I will revise accordingly. I agree that delegatee/delgator is confusing, but I'm not sure what the better terminology is.

@real-or-random I'll make sure to include the copyright and permission notice which I believe will satisfy the license requirements for secp256k1lab.

@real-or-random I'll make sure to include the copyright and permission notice which I believe will satisfy the license requirements for secp256k1lab.

Yep, thanks, and no offense, by the way. :) I just think it's good practice to keep licenses with the code. It helps everyone later on. I suggest just including the existing COPYING file. And I think it would also be very useful to future readers of the BIP to note somewhere from which commit the files are taken, (e.g., have a sentence in the BIP, in some README file, or added to the top of the COPYING file) such as The contents of this directory are taken from https://github.com/secp256k1lab/secp256k1lab commit id 01234567890abcdef....

Hopefully the last comment on the secp-library discussion in your PR: Let’s continue that conversation in #1855

Hi 👋

I'm coming over from reviewing the FROST PR. @siv2r pointed out to me that this proposal (along with MuSig2) share the semantics for tweaking protocol data structures and algorithms ("TweakContext", "ApplyTweak", "TweakCtxInit") .

Do the authors have a strong opinion on the idea of working on a separate BIP that specifies those data structures, and then just reference those algorithms in this BIP - similar to how the FROST proposal delegates definitions to BIP340 to define schnorr signatures in it's writing.

This work could be done yourselves - as the authors on this work have been pretty prolific in this space :-) - or delegated to other volunteers (such as myself, or maybe Sivaram if he has bandwidth) to execute on the idea.

In summary 👍 or 👎

@jurvis, @jesseposner: Just so we don’t cross wires, my understanding is that y’all are still working on addressing review, and you’ll let us know here when you’re caught up. Also, please feel free to resolve any review that you feel has been addressed.

@murchandamus: All review items have now been addressed. I got an error when I tried to resolve the comments, but I posted the commit hash to show it has been resolved.

@Christewart My bandwidth is limited, but I think it's a good idea and happy to support any efforts to make this happen.

@Christewart, I've been thinking about this TweakContext BIP idea. The scope would be small: just the data structure (Q, gacc, tacc) and four functions (TweakCtxInit, ApplyTweak, GetXonlyPubkey, GetPlainPubkey). Unlike other small BIP 43 (coordination point), this would be more of a convenience abstraction. I'm honestly not sure if this meets the bar for being "BIPable" and would appreciate any input on that. I have very little experience with what is and isn't BIPable anyway.

For downstream BIPs, the spec text could reference the TweakContext BIP instead of duplicating the algorithms. However, there's a challenge with reference code: if this BIP vendors secp256k1lab (needed for group operations), and downstream BIPs also vendor secp256k1lab for their own crypto, they'd end up with two copies. So downstream BIPs would likely just copy-paste the functions anyway, meaning this would only reduce BIP text duplication, not code duplication.

A better approach might be treating this as an informational BIP (for people who aren't aware of this tweaking methodology) with reference code for demonstration, but without expecting downstream BIPs to import it. Additionally, we could add a GE.apply_tweak(tweak: Scalar, is_xonly: bool) -> GE function to the secp256k1lab library. Then downstream BIPs would use that shared tweaking function, and implement their own TweakContext following the spec. This would provide both specification standardization (via the BIP) and implementation standardization (via secp256k1lab).

I see three options: (A) create the informational BIP + add apply_tweak to secp256k1lab, (B) just add apply_tweak to secp256k1lab without a BIP, or (C) do nothing and accept the duplication. I think at minimum B would be useful; I'm less certain about A. Curious what others think, especially on whether this abstraction is valuable enough for its own BIP.

delegated to other volunteers (such as myself, or maybe Sivaram if he has bandwidth) to execute on the idea.

If A seems like the way to go, I can raise a pull request to secp256k1lab for the apply_tweak method. I'm not sure about writing the BIP myself, but I'm happy to help/collaborate in any way if you're interested :)

P.S. Sorry for spamming this PR thread. Wasn't sure where else to post this comment.