BIP-352: generate input_hash after summing up keys (simplification) #1622

1. 
   theStack commented at 10:31 pm on June 19, 2024: contributor

   While working on the sender side equivalent of #1620 I noticed that the current flow of input hash generation and key summing is a bit redundant and potentially confusing.

   For both sender and receiver, generating the input hash is currently listed as the first step: https://github.com/bitcoin/bips/blob/85cda4e225b4d5fd7aff403f69d827f23f6afbbc/bip-0352.mediawiki?plain=1#L302-L304 https://github.com/bitcoin/bips/blob/85cda4e225b4d5fd7aff403f69d827f23f6afbbc/bip-0352.mediawiki?plain=1#L336-L338

   This step already involves summing up the public keys, even though summing up key material (private keys for sender, public keys of inputs for receiver) is then again listed explicitly in later steps. Especially for the sender side, this means that the summing up happens twice – once for the pubkeys to generate the input hash, then again for the private keys to create the shared secret.

   It seems to be more obvious and less redundant (and also hopefully less confusing for readers) to reorder the instructions to calculate the input_hash after the key aggregation is done to reuse the result. In case of the sender, the private key sum has to be multiplicated with G in order to the get to the corresponding input pubkey sum.

   This also corresponds to the current BIP352 implementation in the secp256k1 library (https://github.com/bitcoin-core/secp256k1/pull/1519). The reference implementation in Python here is adapted for the sender side, the receiver side has already generated the input_hash after summing up the pubkeys.

2. BIP-352: generate `input_hash` after summing up keys (simplification)

   For both sender and receiver, generating the input hash is currently
   listed as the first step. This already involves summing up the public
   keys, even though summing up key material (private keys for sender,
   public keys of inputs for receiver) is then again listed explicitly
   in later steps.
   
   It seems to be more obvious and less redundant (and also hopefully less
   confusing for readers) to reorder the instructions to calculate the
   input_hash _after_ the key aggregation is done to reuse the result. In
   case of the sender, the private key sum has to be multiplicated with G
   in order to the get to the corresponding input pubkey sum.
   
   This also corresponds to the current BIP352 implementation in the
   secp256k1 library (https://github.com/bitcoin-core/secp256k1/pull/1519).
   The reference implementation in Python here is adapted for the sender
   side, the receiver side has already generated the input_hash after
   summing up the pubkeys.

   fe0f83531e
3. theStack force-pushed on Jun 19, 2024
4. theStack renamed this:
   BIP-352: generate `input_hash` after summing up keys (simplifcation)
   BIP-352: generate `input_hash` after summing up keys (simplification)
   on Jun 19, 2024
5. jonatack added the label Proposed BIP modification on Jun 19, 2024
6. jonatack added the label Pending acceptance on Jun 19, 2024
7. 
   jonatack commented at 10:54 pm on June 19, 2024: contributor

   Pinging BIP authors @josibake @RubenSomsen for review/sign-off.
8. josibake approved
9. 
   josibake commented at 1:23 pm on June 21, 2024: member

   ACK https://github.com/bitcoin/bips/pull/1622/commits/fe0f83531e35f76d3582813da4cbb518d9bb1d12

   Good catch, thanks for the PR! For context, in previous iterations of the BIP the input hash did not commit to A, but looks like we missed reordering these steps after changing the input_hash to also commit to A.

10. 
    jonatack commented at 2:03 pm on June 21, 2024: contributor

    Sanity checked that ./reference.py send_and_receive_test_vectors.json runs correctly with these changes.
11. jonatack merged this on Jun 21, 2024
12. jonatack closed this on Jun 21, 2024

    ---

13. theStack deleted the branch on Jun 21, 2024
14. in bip-0352.mediawiki:287 in fe0f83531e

    282@@ -284,7 +283,6 @@ The receiver obtains the public key from the ''scriptSig''. The receiver MUST pa
    283 
    284 The sender and receiver MUST calculate an input hash for the transaction in the following manner:
    285 
    286-* Let ''A = A<sub>1</sub> + A<sub>2</sub> + ... + A<sub>n</sub>'', where each ''A<sub>i</sub>'' is the public key of an input from the ''[[#inputs-for-shared-secret-derivation|Inputs For Shared Secret Derivation]]'' list<ref name="why_include_A"></ref>
    

    ---

    ---

    

    andrewtoth commented at 2:00 pm on June 28, 2024:

    One thing about removing this line is that now when reading the spec from top to bottom this section references A without any context. Only after reading the below sections does the spec tell the reader to return here with the value for A. Perhaps we can move this section below sending and receiving and in those sections tell the reader to see below, or we can add a note here saying A is specified below depending on being sender or receiver.

    ---

    

    jonatack commented at 5:06 pm on June 28, 2024:

    Thanks for reviewing, Andrew – @josibake @theStack WDYT?

    ---

    

    josibake commented at 12:34 pm on June 29, 2024:

    Good catch, @andrewtoth . Reading through this again, I think it makes more sense to remove the Input Hash section entirely and just define it inline. I opened #1629 for this
15. josibake referenced this in commit 8ac84bd344 on Jun 29, 2024
16. jonatack removed the label Pending acceptance on Jul 1, 2024

Contributors
theStack jonatack josibake andrewtoth

Labels
Proposed BIP modification