#2106 introduced a limit on how many keys the receiver is allowed to scan. But the sender specification still allows for creating outputs that exceed that limit, risking loosing funds. I tried to organize each commit so to make review as easy as possible:
- 4b9b490: Adds an additional check in the sender specification to disallow creating more keys/outputs than the receiver will surely scan (as per the specification).
- 2028356: Adds an additional functional test requirement for wallet implementors.
- 0de6dfb: Changes the reference implementation so that groups of silent payment addresses are calculated in a more sensible way (see details below). This commit WILL FAIL with the test vectors.
- c14fd21: Implement the proposed check in the reference implementation.
- 58c4ac6: Suggests minor redaction clarifications and fix typos.
Rationale
The specification of the sender and receiver seems to be out of sync, i.e. the sender can create a spec-compliant transaction such that a spec-compliant receiver will not find all funds received, implying the loss of funds. This is an edge case more theoretical than practical. I consulted @RubenSomsen about this possibility and we agreed opening a PR straight away would be safe for current users of silent payments.
#2106 introduced the K_max parameter which spirit is to limit how much effort the receiver is allowed to perform when scanning a candidate silent payment transaction.
But things are not as clear in the sender side: the spec asks the sender to:
Group receiver silent payment addresses by
B_scan(e.g. each group consists of oneB_scanand one or moreB_m). If any of the groups exceed the limit ofK_max(=2323) silent payment addresses, fail.
Let's analyze the case in which one is using a single silent payment address, but want to create 2324 (K_max + 1) outputs for the receiver. This is exactly scenario was covered by a new test case added together with #2106.
Well, there's one silent payment address, so that would make for one group with a single B_m within. Then, the required check (group size should not exceed K_max) should not fail and we are going to proceed with output generation with no more additional checks required by the specification. We would end up with a transaction with 2324 outputs, but the receiver will only scan for 2323 (K_max) and miss one output.
Note that the receiver could continue scanning anyhow and find all his funds, but version 1.1.x of the spec won't allow this.
About the changes in the reference implementation
Turns out that the test case expected result is that the described transaction creation should fail. And it does so in the reference implementation.
#2106 introduced a field count on the recipients specification for tests and added a test case in which the sender is given a single silent payment address and wants to create 2324 outputs for it, just as described above.
Before calling create_outputs(), which is the reference implementation of this specification, the test harness will clone the silent payment address count times and pass those copies to create_outputs(). Thus, in the sender process, it will create a group with 2324 elements, all of which carry the same silent payment address (i.e. the same B_m), and will fail as per the spec.
I don't believe this to be what any developer would implement from reading the specification:
Group receiver silent payment addresses by
B_scan(e.g. each group consists of oneB_scanand one or moreB_m).
Indeed, I found this during a deep dive on the silent payments specification at Vinteum together with @lorenzolfm, @oleonardolima, @lucasdcf, @IsaqueFranklin, @joaozinhom, and @themhv and we all agreed we would not implement the specification like in the reference implementation.
We started looking at what wallets that support silent payments currently do and some didn't even implemented v1.1.0 yet, which introduced the described interpretation.
