tests: Silent Payments ctime test never reaches a match or label lookup #1898

l0rinc commented at 11:53 PM on July 27, 2026: contributor

Problem: The constant-time test generates an output for one scan key, then mutates the key before scanning, so the scan cannot find it. It also passes no label callback, so label-candidate serialization and batching never run. The test still passes because it never checks the scan result. Nothing verifies the match path, or that the label callback only receives public data, while the scan key is marked undefined.

Fix: Regenerate the output for the actual scan key, prepend a nonmatching output, and require one direct, unlabeled match. Pass a label callback that verifies each serialized 33-byte candidate is defined and counts the misses.

l0rinc force-pushed on Jul 28, 2026

l0rinc marked this as a draft on Jul 28, 2026

real-or-random added the label assurance on Jul 28, 2026

real-or-random added the label tweak/refactor on Jul 28, 2026

l0rinc renamed this:
~~RFC/tests: cover Silent Payments label scanning~~
tests: cover Silent Payments label scanning
on Jul 29, 2026

l0rinc marked this as ready for review on Jul 29, 2026

theStack commented at 12:13 PM on August 7, 2026: contributor

Concept ACK

l0rinc commented at 8:06 PM on November 6, 2026: contributor

Is there any change you want me to do here?

in src/ctime_tests.c:357 in b561d6c58c outdated

 357 | -     * and then reused for the rest of the scanning logic.
 358 | -     */
 359 | -    CHECK(secp256k1_silentpayments_recipient_scan_outputs(ctx, found_outputs_ptrs, &n_found_outputs, tx_outputs, 2, key, &prevouts_summary, &recipient.spend_pubkey, NULL, NULL));
 360 | +    label_lookups = 0;
 361 | +    CHECK(secp256k1_silentpayments_recipient_scan_outputs(ctx, found_outputs_ptrs, &n_found_outputs, tx_outputs, 2, key, &prevouts_summary, &recipient.spend_pubkey, label_lookup_none, NULL));
 362 | +    CHECK(n_found_outputs == 1);

theStack commented at 2:14 PM on November 11, 2026:

nit: could move this check into the first commit (f7de6f7bdfbb11510001c57184ec5732cfba1bb3), so that it proves its own claim

l0rinc commented at 8:15 PM on November 11, 2026:

Moved the n_found_outputs and found_with_label checks into the first commit as suggested.

theStack commented at 2:29 PM on November 11, 2026: contributor

ACK b561d6c58c541e38433b3475092b6bfacbd952ec

Sorry for the late reply!

Even though the removed code comment still applies ("It is sufficient to check _recipient_scan_outputs without a label lookup function, since the shared secret is created once (which is where the constant timeness matters) and then reused for the rest of the scanning logic"), I think it makes sense to exercise as many code paths as possible in the ctime tests, in particular to check that the label lookup callback only ever receives public data. We could go further and also exercise the labeled-match path, but that can be done in a different PR, possibly the follow-up planned for #1974 (see #1974 (review)), since it depends on whether the label tweak is treated as secret in the scanning function (still to be discussed).

tests: make Silent Payments ctime scan find output

The ctime test marks the scan key undefined, but generates its output for a scan public key created before later key mutations.
The scan therefore cannot find that output, leaving the successful-match path untested even though the test passes.

Regenerate the output for the actual scan key and prepend a nonmatching output for the label-batch scenario.
Remove the earlier scan-public-key creation and sender call because this setup replaces both.
Require one direct, unlabeled match so the test fails if the output is generated for a different scan key.

078f67f86b

tests: cover Silent Payments label scanning

The ctime test passes no label callback, so label-candidate serialization and batching never run while the scan key is marked undefined.

Check that every serialized 33-byte candidate is defined and count callback misses.
For `k = 0`, the first output contributes two candidates that flush before the second output matches directly.
For `k = 1`, both outputs miss and four candidates flush at the end.

The six callback calls prove both flush paths execute, `SECP256K1_CHECKMEM_CHECK` proves the candidate bytes are defined, and the existing result assertions prove the intended direct match is still found.
The six-call argument relies on `LABEL_BATCH_SIZE >= 2` (currently 8).
With `LABEL_BATCH_SIZE == 1`, the first miss would flush immediately and no longer cover the pre-direct-match flush.

f391539a3b

l0rinc force-pushed on Nov 11, 2026

l0rinc renamed this:
~~tests: cover Silent Payments label scanning~~
tests: Silent Payments ctime test never reaches a match or label lookup
on Nov 11, 2026

fametrano commented at 5:26 PM on November 13, 2026: contributor

ACK f391539a3b2d16059883dbd281bee33e1f995423

A branch on shared_secret added to secp256k1_silentpayments_recipient_scan_outputs makes valgrind fail ctime_tests with this PR, both when placed in the match path and when placed where label candidates are built (exit 42). On master f43a345 the same mutations pass unnoticed. Unmodified, both commits of this PR pass valgrind with no errors.

Thank you l0rinc for your work, here and elsewhere ;-)