Add BIP352 `silentpayments` module #1519

josibake · 2024-04-19T14:21:15Z

This PR adds a new Silent Payments (BIP352) module to secp256k1. It is a continuation of the work started in #1471.

The module implements the full protocol, except for transaction input filtering and silent payment address encoding / decoding as those will be the responsibility of the wallet software. It is organized with functions for sending (prefixed with _sender) and receiving (prefixed by _recipient).

For sending

Collect private keys into two lists: taproot_seckeys and plain_seckeys
Two lists are used since the taproot_seckeys may need negation. taproot_seckeys are passed as keypairs to avoid the function needing to compute the public key to determine parity. plain_seckeys are passed as just secret keys
Create the _silentpayment_recipient objects
These structs hold the scan and spend public key and an index for remembering the original ordering. It is expected that a caller will start with a list of silent payment addresses (with the desired amounts), convert these into an array of recipients and then match the generated outputs back to the original silent payment addresses. The index is used to return the generated outputs in the original order
Call silentpayments_sender_create_outputs to generate the xonly public keys for the recipients
This function can be called with one or more recipients. The same recipient may be repeated to generate multiple outputs for the same recipient

For scanning

Collect the public keys into two lists taproot_pubkeys and plain_pubeys
This avoids the caller needing to convert taproot public keys into compressed public keys (and vice versa)
Compute the input data needed, i.e. sum the public keys and compute the input_hash
This is done as a separate step to allow the caller to reuse this output if scanning for multiple scan keys. It also allows a caller to use this function for aggregating the transaction inputs and storing them in an index to vend to light clients later (or for faster rescans when recovering a wallet)
Call silentpayments_recipient_scan_outputs to scan the transaction outputs and return the tweak data (and optionally label information) needed for spending later

In addition, a few utility functions for labels are provided for the recipient for creating a label tweak and tweaked spend public key for their address. Finally, two functions are exposed in the API for supporting light clients, _recipient_created_shared_secret and _recipient_create_output_pubkey. These functions enable incremental scanning for scenarios where the caller does not have access to the transaction outputs:

Calculating a shared secret
This is done as a separate step to allow the caller to reuse the shared secret result when creating outputs and avoid needing to do a costly ECDH every time they need to check for an additional output
Generate an output (with k = 0)
Check if the output exists in the UTXO set (using their preferred light client protocol)
If the output exists, proceed by generating a new output from the shared secret with k++

See examples/silentpayments.c for a demonstration of how the API is expected to be used.

Note for reviewers

My immediate goal is to get feedback on the API so that I can pull this module into bitcoin/bitcoin#28122 (silent payments in the bitcoin core wallet). That unblocks from finishing the bitcoin core PRs while work continues on this module.

Notable differences between this PR and the previous version

See #1427 and #1471 for discussions on the API design. This iteration of the module attempts to be much more high level and incorporate the feedback from #1471. I also added a secp256k1_silentpayments_public_data opaque data type, which contains the summed public key and the input_hash. My motivation here was:

I caught myself mixing up the order of arguments between A_sum and recipient_spend_key, which was impossible to catch with ARG_CHECKS and would result in the scanning process finishing without errors, but not finding any outputs
Combining public key and input_hash into the same data type allows for completely hiding input_hash from the caller, which makes for an overall simpler API IMO

I also removed the need for the recipient to generate a shared secret before using the secp256k1_silentpayments_recipient_scan_outputs function and instead create the shared secret inside the function.

Outstanding work

clean up the testing code
improve test coverage (currently only using the BIP352 test vectors)
optimize the implementation, where possible

theStack

Concept ACK

Left some initial feedback, especially around the scanning routine, will do an in-depth review round soon. Didn't look closer at the public_data type routines and the examples yet.

src/modules/silentpayments/main_impl.h

include/secp256k1_silentpayments.h

src/modules/silentpayments/main_impl.h

src/modules/silentpayments/tests_impl.h

src/modules/silentpayments/main_impl.h

include/secp256k1_silentpayments.h

src/modules/silentpayments/main_impl.h

josibake · 2024-04-24T08:45:36Z

Rebased on #1518 (3d08027 -> 8b48bf1, compare)

josibake · 2024-04-24T09:39:20Z

Updated 8b48bf1 -> f5585d4 (bip352-silentpayments-module-rebase -> bip352-silentpayments-module-02, compare):

Fix function documentation for _recipient_scan_outputs
Replace VERIFY_CHECK with return 0; in _sender_create_outputs
Remove unneeded declassify code from _sender_create_outputs
Change _gej_add_ge to _gej_add_var in _recipient_public_data_create
Fix label scanning in _recipient_scan_outputs
Remove unneeded prints from the tests

For the label scanning, I looked for an example of using an invalid public key but didn't see anything except for the invalid_pubkey_bytes in the tests. For now, if the output is found without a label, I'm setting found_with_label = 0 and saving the found output in both the output and label field. Happy to change this if there is a better suggestion for communicating an invalid public key.

I also used secp256k1_pubkey_save instead of output = *tx_outputs, as I think this makes the code more clear.

theStack

Second review round through, looks good so far! Left a bunch of nits, mostly about naming and missing ARG_CHECKS etc.

theStack · 2024-04-28T01:18:12Z

src/modules/silentpayments/main_impl.h

+        &(*(const secp256k1_silentpayments_recipient **)pk1)->scan_pubkey,
+        &(*(const secp256k1_silentpayments_recipient **)pk2)->scan_pubkey


pk1/pk2 are addresses for _silentpayments_recipient instances, so involving double-pointers here doesn't seem to be necessary:

Suggested change

&(*(const secp256k1_silentpayments_recipient **)pk1)->scan_pubkey,

&(*(const secp256k1_silentpayments_recipient **)pk2)->scan_pubkey

&((const secp256k1_silentpayments_recipient *)pk1)->scan_pubkey,

&((const secp256k1_silentpayments_recipient *)pk2)->scan_pubkey

I'll admit, I don't fully understand why the double pointers are necessary, but they are: when I removed them, my tests would pass locally but the branch would fail the CI. I noticed the double pointers are also used for the secp256k1_pubkey_sort function, so leaving for now.

src/modules/silentpayments/main_impl.h

theStack · 2024-04-28T01:21:29Z

src/modules/silentpayments/main_impl.h

+
+    /* Sanity check inputs. */
+    VERIFY_CHECK(ctx != NULL);
+    ARG_CHECK(recipients != NULL);


could also ARG_CHECK for n_recipients here:

Suggested change

ARG_CHECK(recipients != NULL);

ARG_CHECK(recipients != NULL);

ARG_CHECK(n_recipients >= 1);

Added. Out of curiosity, I noticed we have VERIFY_CHECK and ARG_CHECK, do you know when one is preferred over the other? In most cases, I see VERIFY_CHECK being used to check the ctx, but there were a few places where VERIFY_CHECK was also being used to check the arguments.

include/secp256k1_silentpayments.h

src/modules/silentpayments/main_impl.h

examples/silentpayments.c

josibake · 2024-05-03T08:25:39Z

Thanks for the thorough review, @theStack ! I've addressed your feedback, along with some other changes.

Update f5585d4 -> 1a3a00b (bip352-silentpayments-module-02 -> bip352-silentpayments-module-03, compare)

Spelling and wording cleanups, notably:
- s/receiver/recipient/, s/labeled/labelled/
- s/scan_seckey/scan_key/
Reduce duplicate code in scan_outputs
Add ARG_CHECKs
Update tests
Add benchmark for scan_outputs

The sending tests now check that the generated outputs match exactly one of the possible expected output sets. Previously, the sending tests were checking that the generated outputs exist in the array of all possible outputs, but this wouldn't catch a bug where k is not being set correctly e.g. [A_k=0, B_k=0] would (incorrectly) pass [A_k=0, B_k=1, A_k=1, B_k=0] but will now (correctly) fail [[A_k=0, B_k=1], [A_k=1, B_k=0]]

josibake · 2024-05-03T11:17:30Z

Rebased on #1518 1a3a00b -> 92f5920 (bip352-silentpayments-module-03 -> bip352-silentpayments-module-03-rebase, compare)

Add function for calculating either a*B or A*b. This function is used by both the sender and the recipient (i.e. a*B == A*b). Add functions for generating the tagged input hash, as this is used when creating the shared secret. In a later commit, `create_shared_secret` will be exposed in the API for the receiver to use when scanning as a light client.

Add methods for creating an xonly output from a shared secret. This involves adding a tagged hash for creating the output. This function will be exposed in the API in a later commit for the receiver to use when scanning as a light client.

Given a set of private keys, the smallest outpoint, and list of recipients this function handles the entire sender flow: 1. Sum up the private keys 2. Calculate the input_hash 3. For each recipient: 3a. Calculate a shared secret 3b. Create the requested number of outputs This function assumes a single sender context in that it requires the sender to have access to all of the private keys. In the future, this API may be expanded to allow for a multiple senders or for a single sender who does not have access to all private keys at any given time, but for now these modes are considered out of scope / unsafe.

Add function for creating a label tweak. This requires a tagged hash function for labels. This function is used by the receiver for creating labels to be used for a) creating labelled addresses and b) to populate a labels cache when scanning. Add function for creating a labelled spend pubkey. This involves taking a label tweak, turning it into a public key and adding it to the spend public key. This function is used by the receiver to create a labelled silent payment address.

Add data type for passing around the summed input public key (A_sum) and the input hash tweak (input_hash). This data is passed to the scanner before the ECDH step as two separate elements so that the scanner can multiply b_scan * input_hash before doing ECDH. Add functions for deserializing / serializing a public_data object to and from a public key. When serializing a public_data object, the input_hash is multplied into A_sum. This is so the object can be stored as public key for wallet rescanning later, or to vend to light clients. For the light client, a `_parse` function is added which parses the compressed public key serialization into a `public_data` object.

Add routine for scanning a transaction and returning the necessary spending data for any found outputs. This function works with labels via a lookup callback and requires access to the transaction outputs. Requiring access to the transaction outputs is not suitable for light clients, but light client support is enabled in followup commits.

Expose a shared secret routine in the public API and a output_pubkey creation routine. These are simple wrappers around the same functions the module is using under the hood. They are exposed for the light client as the minimal set of operations they need to do scanning without access to the transaction outputs. This means the light client will need to manage their own scanning state, so wherever possible it is preferrable to use the `_recipient_scan_ouputs` function.

Demonstrate sending, scanning, and light client scanning.

Add a benchmark for `scan_outputs` as this is the most performance critical part of silent payments.

The vectors are generated with a Python script that converts the .json file from the BIP to C code: $ ./tools/tests_silentpayments_generate.py test_vectors.json > ./src/modules/silentpayments/vectors.h

josibake · 2024-05-08T12:50:33Z

Rebased on master (following #1518 merge) 92f5920 -> 56ed901 (bip352-silentpayments-module-03-rebase -> bip352-silentpayments-module-04-rebase, compare)

josibake mentioned this pull request Apr 19, 2024

Add silentpayments (BIP352) module #1471

Closed

real-or-random added the feature label Apr 19, 2024

josibake mentioned this pull request Apr 22, 2024

BIP352 tracking issue bitcoin/bitcoin#28536

Open

15 tasks

theStack reviewed Apr 22, 2024

View reviewed changes

josibake force-pushed the bip352-silentpayments-module branch from 3d08027 to 8b48bf1 Compare April 24, 2024 08:38

josibake force-pushed the bip352-silentpayments-module branch from 8b48bf1 to f5585d4 Compare April 24, 2024 09:38

theStack reviewed Apr 28, 2024

View reviewed changes

Sjors mentioned this pull request May 2, 2024

Silent Payments: Implement BIP352 bitcoin/bitcoin#28122

Draft

josibake force-pushed the bip352-silentpayments-module branch 2 times, most recently from 9d75190 to 1a3a00b Compare May 3, 2024 08:21

josibake force-pushed the bip352-silentpayments-module branch from 1a3a00b to 92f5920 Compare May 3, 2024 11:11

josibake mentioned this pull request May 3, 2024

Add secp256k1_pubkey_sort #1518

Merged

theStack and others added 14 commits May 8, 2024 14:42

build: add skeleton for new silentpayments (BIP352) module

957eb03

doc: add module description for silentpayments

31187da

silentpayments: add sortable recipient struct

ac97828

silentpayments: add examples/silentpayments.c

dfe9243

Demonstrate sending, scanning, and light client scanning.

silentpayments: add benchmark for scan_outputs

baafed0

Add a benchmark for `scan_outputs` as this is the most performance critical part of silent payments.

tests: add BIP-352 test vectors

932fe57

The vectors are generated with a Python script that converts the .json file from the BIP to C code: $ ./tools/tests_silentpayments_generate.py test_vectors.json > ./src/modules/silentpayments/vectors.h

ci: enable silentpayments module

56ed901

josibake force-pushed the bip352-silentpayments-module branch from 92f5920 to 56ed901 Compare May 8, 2024 12:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add BIP352 `silentpayments` module #1519

Add BIP352 `silentpayments` module #1519

josibake commented Apr 19, 2024 •

edited

theStack left a comment

josibake commented Apr 24, 2024

josibake commented Apr 24, 2024

theStack left a comment

theStack Apr 28, 2024

josibake May 3, 2024

theStack Apr 28, 2024

josibake May 3, 2024

josibake commented May 3, 2024

josibake commented May 3, 2024

josibake commented May 8, 2024

		&((const secp256k1_silentpayments_recipient *)pk1)->scan_pubkey,
		&((const secp256k1_silentpayments_recipient *)pk2)->scan_pubkey

	ARG_CHECK(recipients != NULL);
	ARG_CHECK(recipients != NULL);
	ARG_CHECK(n_recipients >= 1);

Add BIP352 silentpayments module #1519

Are you sure you want to change the base?

Add BIP352 silentpayments module #1519

Conversation

josibake commented Apr 19, 2024 • edited

For sending

For scanning

Note for reviewers

Notable differences between this PR and the previous version

Outstanding work

theStack left a comment

Choose a reason for hiding this comment

josibake commented Apr 24, 2024

josibake commented Apr 24, 2024

theStack left a comment

Choose a reason for hiding this comment

theStack Apr 28, 2024

Choose a reason for hiding this comment

josibake May 3, 2024

Choose a reason for hiding this comment

theStack Apr 28, 2024

Choose a reason for hiding this comment

josibake May 3, 2024

Choose a reason for hiding this comment

josibake commented May 3, 2024

josibake commented May 3, 2024

josibake commented May 8, 2024

Add BIP352 `silentpayments` module #1519

Add BIP352 `silentpayments` module #1519

josibake commented Apr 19, 2024 •

edited