Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Jun 21, 2024

Conversation

theStack
Copy link
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:

bips/bip-0352.mediawiki

Lines 302 to 304 in 85cda4e

After the inputs have been selected, the sender can create one or more outputs for one or more silent payment addresses in the following manner:
* Generate the ''input_hash'' with the smallest outpoint lexicographically, using the method described above

bips/bip-0352.mediawiki

Lines 336 to 338 in 85cda4e

If each of the checks in ''[[#scanning-silent-payment-eligible-transactions|Scanning silent payment eligible transactions]]'' passes, the receiving wallet must:
* Generate the ''input_hash'' with the smallest outpoint lexicographically, using the method described above

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 (bitcoin-core/secp256k1#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.

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 (bitcoin-core/secp256k1#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.
@theStack theStack force-pushed the bip352-simplify_input-hash_flow branch from 3623b60 to fe0f835 Compare June 19, 2024 22:33
@theStack theStack changed the title BIP-352: generate input_hash after summing up keys (simplifcation) BIP-352: generate input_hash after summing up keys (simplification) Jun 19, 2024
@jonatack jonatack added Proposed BIP modification Pending acceptance This BIP modification requires sign-off by the champion of the BIP being modified labels Jun 19, 2024
@jonatack
Copy link
Contributor

Pinging BIP authors @josibake @RubenSomsen for review/sign-off.

Copy link
Member

@josibake josibake left a comment

Choose a reason for hiding this comment

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

ACK fe0f835

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.

Copy link
Contributor

@jonatack jonatack left a comment

Choose a reason for hiding this comment

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

Sanity checked that ./reference.py send_and_receive_test_vectors.json runs correctly with these changes.

@jonatack jonatack merged commit 70a7143 into bitcoin:master Jun 21, 2024
3 checks passed
@theStack theStack deleted the bip352-simplify_input-hash_flow branch June 21, 2024 14:05
josibake added a commit to josibake/bips that referenced this pull request Jun 29, 2024
Since bitcoin#1622, it makes more sense
to define input_hash inline, vs having its own section.
@jonatack jonatack removed the Pending acceptance This BIP modification requires sign-off by the champion of the BIP being modified label Jul 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants