silentpayments: return 0 instead of `ARG_CHECK`ing for empty inputs/outputs #1983

secp256k1_silentpayments_recipient_prevouts_summary_create and secp256k1_silentpayments_recipient_scan_outputs treated a transaction with no eligible inputs, or no taproot outputs, as an illegal argument. With the default illegal callback, that calls abort().

That contradicts the API documentation. Both functions are documented to return 0 if the transaction is not a Silent Payments transaction, and a transaction with no eligible inputs or no taproot outputs is exactly that case. It also puts the burden on callers. Anyone scanning a block or building an index has to filter these transactions out before calling the library. If they forget, a transaction someone else created can crash their process.

This PR makes both functions return 0 in these cases:

• prevouts_summary_create: drops the ARG_CHECK((n_pubkeys > 0) || (n_xonly_pubkeys > 0)). With no public keys, the sum is the point at infinity, so the existing "sum is infinity" check already returns 0. No new code path is needed.
• scan_outputs: drops the ARG_CHECK(n_tx_outputs > 0). It returns 0 after all other arguments have been validated and before the scan key is loaded. *n_found_outputs is still set to 0.
• scan_outputs: tx_outputs and found_outputs are now only checked for NULL if n_tx_outputs > 0, and their SECP256K1_ARG_NONNULL attributes are removed. This lets callers pass an empty array as NULL (e.g., data() of an empty std::vector). It matches how prevouts_summary_create and sender_create_outputs already treat empty key arrays (see the previous "Fixed" entry in the CHANGELOG added in PR #1932).

ACK 160590d1922914ddabe7c4d39fe14846b009212e

I built the head and master (b88cbc7) with CMake and Apple clang 21, and tests -t=silentpayments and the example pass on both. A small program calls the two functions with an empty input or output set and a counting illegal callback. On master each call fires the callback, which by default aborts. On the head each returns 0 without it, the summary is zeroed and *n_found_outputs is 0. The tests are built with SECP256K1_BUILD, which turns SECP256K1_ARG_NONNULL off, so they cannot see the header change. I compiled a caller passing NULL arrays with n_tx_outputs 0 under -Wnonnull -Werror: it fails on master and builds on the head. The program, the commands and the output are in https://gist.github.com/fametrano/5f4db35678dfd8a5b4afd45803e92cca/4c6057f286d646f9ab235141d1155163794d65ae.

The new contract matches BIP352, where a transaction with no eligible input or no taproot output is not scanned, as with an input sum at infinity. The sender keeps its checks, in line with the BIP's MUST for at least one eligible input. To check the tests, I made six changes to main_impl.h one at a time. I put back each removed ARG_CHECK, made the two NULL checks unconditional, and removed the new early return and the *n_found_outputs = 0. Each change makes the module's tests fail, and the unchanged head passes.

utACK 160590d1922914ddabe7c4d39fe14846b009212e

Let me add that this has been reported to secp256k1-security@bitcoin.org independently of the Antrophic scan (but without explicit permission to quote the report here, so I'm not doing it).

this has been reported to secp256k1-security@bitcoin.org independently of the Antrophic scan

And let me also add that we don't consider this a vulnerability. Technically, crashing can be a security issue, but this crash is clean and will be caught by minimal testing on the caller side, so that no one should hit this case in production.

Let me add that this has been reported to secp256k1-security@bitcoin.org independently of the Antrophic scan (but without explicit permission to quote the report here, so I'm not doing it).

Crediting @KillMalware for the independent report, thanks again!