-
Notifications
You must be signed in to change notification settings - Fork 29
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
[WIP] early payment proofs #70
base: master
Are you sure you want to change the base?
Conversation
text/0000-early-payment-proofs.md
Outdated
Payment proofs prevent a payment receiver from claiming they didn't receive payment. | ||
Such fraud prevention is an essential ingredient to commercial adoption. | ||
|
||
Former payment proofs used in Grin didn't apply to invcoice flow, at least not |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo invcoice
-> invoice
text/0000-early-payment-proofs.md
Outdated
They also lacked the ability to specify dating and purpose of payment. | ||
|
||
This RFC changes the transaction building process where payers can require | ||
payees to create a "proof" they've received a payment before the payer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe we should make it clear here that anyone can finalize it since it allows for all tx flows
text/0000-early-payment-proofs.md
Outdated
# Unresolved questions | ||
[unresolved-questions]: #unresolved-questions | ||
|
||
* What limit is imposed on the size of the memo field? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd say at least 32 bytes should be possible by default to be able to store say a sha3-256 hash of an invoice document or similar
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, i set the limit to 32 bytes mentioning the room for a hash
I think it would be better to replace |
A deep enough reorg can always invalidate a payment proof, whether it uses an index or a nonce. |
If after a reorg the tx is on chain then the proof remains the same if you use public nonce instead of index. If it's not then they need to create a new transaction which creates a new proof in both cases. Consider this case:
In this case I've lost the proof and can't get it (unless i bruteforce the kernels) |
That's a good argument. The problem is that looking up an arbitrary kernel is expensive. The node doesn't maintain an index of all kernels. So to avoid making payment proof verification a DOS vector, we'd really like the payer to provide the index. |
I expect that in the future vast majority would be using the nimble node. Do you have any idea how to get the new index with the nimble node to generate the new proof? Also how would someone validate the proof on a nimble node? |
Nimble nodes will presumably rely on full nodes providing merkle membership proofs. |
Does this make it impossible to swap outputs? So if we made a transaction |
The payment proof commits to the receiver excess, so indeed you can compensate for any changes in or addition of inputs/outputs by adjusting the offset. |
I would probably add the payment proof version in the payment proof because if we ever implement a 2 step we will need a different payment proof and when i show you my proof you should know how to verify it. I feel like version seems like an easy way to achieve that. We will never be able to ignore older proofs so we will always need to be able to support verification of all versions of proofs |
Good idea. I will add versioning. |
I don't think you can perform output swapping because to do that the receiver would need to change his blinding factors but you commit to them in your proof (receiver could technically do it if he had at least 2 elements of tx - eg. 2 outputs, but can't do it if he only has one). It does feel like output swapping might be useful when we will try to increase the privacy Edit: no, even with 2 or more inputs you can't do output swapping because you can figure out which they were if you know the sum point |
The receiver only commits to the excess, not any blinding factors. Btw, I don't quite see the point in output swapping. Once the tx is broadcasted, the swapped outputs are right there for the counter party to see, unless you got stem-phase aggregation. |
Yes that's why he can't change blinding factors later (he could if he has 2 elements but it's pointless because you can figure out which things were changed).
It could be used before it was broadcasted, eg. in stem phase or you could have a chain of 2 services, aggregator --> swapper and swapper would then swap stuff. I haven't thought much about it since we are not improving privacy now but to me it sounds interesting |
They can change blinding factors along with the kernel offset, which preserves the excess.
Once you have good aggregation, then the need to hide your output from your counterparty is much reduced, since your spend of it will be well obscured by aggregation. |
sr = rr + e*xr --> does xr here contain offset? If yes then you're right, if no then changing blinding factors would invalidate the proof if i understand correctly
Counterparty knows your inputs/outputs (well at least the finalizer does) and aggregation can't fix that |
The RFC has this equation
which has no blinding factors, only receiver's excess X.
You missed my point. Aggregation makes it much less useful for counterparty to know your output. |
True, but it won't be obscured from anyone seeing the tx prior to aggregation. Output swapping in its extreme version means that if you give me a tx Btw, the swapped outputs are not public for anyone to see - this would not give any privacy. This is why I merged the idea with objective dandelion which has for a given node So the passing of txs would look like:
So only Node1 knows mapping from |
Does X here contain the receiver's offset? If yes then swapping should work
They still know when you spend these outputs so aggregation only makes it harder to see to who you sent it. Some potential improvements on the RFC itself:
Is the proof for sender-initiated transaction the same (flow)? It feels like it could be |
text/0000-early-payment-proofs.md
Outdated
- receiver public excess | ||
- `sender_address` | ||
|
||
The witness is a triple (s,i,C,m) where i is the MMR index of an on-chain kernel K with commitment C, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo, quadruple not triple
I believe https://forum.grin.mw/t/eliminating-finalize-step/7621/106 describes another way to produce payment proofs that are the same for both SRS/RSR flow. One of the differences is that the |
It uses the set of all participant's R_i, which takes more space. And it uses a nonstandard way of commiting to extra data (R * Hash instead of R + Hash * G as used in Grin switch commitments and Bitcoin taproot). Otherwise it's quite similar to my proof type SenderNonce. There is something to be said for reducing the number of proof types, but I'm not sure whether we should stop supporting the existing payment proofs. |
Indeed, it is a similar strategy. One difference big is what it commits to. The payment proof defined in the forum post requires every participant of a transaction to know all the main ingredients of a transaction that were contributed by other participants (leaving out inputs and outputs). The payment proof is the same for every participant and reveals all sends/receives of a tx (in case there were multiple). It seems like there are different kinds of payment proofs because this one is a bit different compared to other approaches which only constructed the proof for the sender. It's a proof that is the same and "open" to all the participants of a tx regardless of how many there are.
We will always need to have a fallback for old payment proofs because if the receiver's wallet is on an old wallet version, there's no other way to do it. However, I do believe that we should default to newer types of proofs if we find them superior e.g. if two wallets support a symmetric payment proofs, it (imo) makes sense to use those. |
I wonder if it would make sense having the invoice fields blinded by default. A payment proof can be thought of as an invoice proof and if it has a signature for the following fields
we can instead sign the following
This allows us to show the kernel_commitment and then selectively show other invoice fields. If we don't want to show the Edit: of course naively doing H(amount) would be bad as one could bruteforce the value. This could be solved by adding a salt to it e.g. have a format |
Seems like a complication that will go unneeded in the vast majority of use cases. |
I agree, this is definitely not needed right now and if at any point we found a use case for it, we could always make a new 'version' of a payment proof. |
text/0000-early-payment-proofs.md
Outdated
There is no need for this proof type in SRS flow, as the simpler Invoice type suffices. | ||
|
||
The witness is a quintuple (s,i,C,Rs',m) where i is the MMR index of an on-chain kernel K with commitment C, satisfying s\*G = R + e\*X, where R is the `receiver_public_nonce`, X is the `receiver_public_excess`, and e is the hash challenge of kernel K. | ||
and e is the hash challenge of kernel K. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the "and e is the hash..." is repeated twice.
The discussion here made me think a bit about payment proofs in multiparty setting. I think we can have a uniform and relatively simple construction where we kill two birds with one stone. In a setting with > 2 parties, we have to randomly scale the contributed pubkeys to solve the problems mentioned in the document (and likely other multisig key canceling problems which is probably why musig2 also scales keys this way). This makes our total excess a linear combination of random scalings of a sequence of pairs (partial_excess, nonce). Slightly changing the sequence of these pubkey pairs would drastically change the Similarly like you describe in Proof type Invoice, we can make every party sign a message with their grin address containing their part
which when through the process of random scaling as described here in the document yield a scaled total excess |
@tromp I'm reading Proof type Invoice and want to check my understanding. The steps for the invoice flow are:
This is safe from the sender computing the |
Correct; payment proofs require an on-chain kernel, and a convincing proof requires that kernel to have sufficient confirmation. |
Regarding the implementation of this, would it make sense to define the payment proof as
While this does add another curve point to the payment proof, it does away with edge-cases e.g. requiring keeping the sender partial sig around in RSR flow until the transaction hits the chain (and removing it later) and potentially modifying the payment proof structure when we observe the kernel on the chain. I think it's simpler to implement and reason about this if the sender gains a complete payment proof data the moment they sign the transaction. Then, they just have to wait for the kernel to appear on the chain to obtain the mmr index of the kernel (which is only used for faster proving, rather than computing the proof itself). Of course, we would have an optional field |
You should write (X - Xr) instead of Xs?!
Keeping context data around is already the task of a wallet slate context; it's not an edge case.
A payment proof is not complete without an on-chain kernel. I don't think we should leave the locating of a kernel to the verifier. As in PoW, verification effort should be minimized. |
No, I meant Xs because in the definition I made above we remember the sender partial signature which is then subtracted from the full kernel signature equation (which leaves us with the signature equation of the receiver partial sig). The reason I want to save the sender partial signature instead of parts of the receiver partial signature is because this one is always available when the sender signs which gives you all the data needed to verify the proof (except for the kernel mmr index).
What I meant with an edge case was some implementation details that came to mind one being that I think we delete the transaction context once we sign (if I recall correctly, that's true for the current implementation as well). When I was doing the contract implementation, I made sure we delete it to avoid any potential double-signing (even though we could in theory just forget the secret keys, but that's not how it works today). I'll take some time to check how exactly we do today's payment proofs and where the data is stored.
Fair point, sounds reasonable. |
Xs is not specified in your suggested payment proof of <M, sig(M, receiver_addr), (ss, Rs), C> though, while X=C and Xr (as part of M) are?!
We only need receiver partial signature to be available once the kernel is confirmed on-chain.
The wallet always needs to keep a record of pending transactions and preferably past transactions as well (at least recent ones). Such records would include all payment proof related data. |
I'll escort myself out. You're right of course, this would need to be a part of the payment proof as well. |
To accomodate the various proof types, the slate will include the following related fields: | ||
|
||
* `receiver_address` - An ed25519 public key for the receiver, typically the public key of the user's v3 onion address. | ||
* `timestamp` - The time at which the receiver generates the payment promise |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not urgent, I'm just considering an experimental implementation of this now to correspond with unified transaction flow.
timestamp
should be unambiguously defined here, assuming it's epoch time?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, the same 64-bit timestamps appearing in Grin headers.
text/0000-early-payment-proofs.md
Outdated
|
||
* `receiver_address` - An ed25519 public key for the receiver, typically the public key of the user's v3 onion address. | ||
* `timestamp` - The time at which the receiver generates the payment promise | ||
* `memo` - A string of size at most 32 bytes that may contain additional payment details, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is the intent here to have this field arbitrarily sized (which has implications for binary serialization, which we support)? It might be more clear to define this as 'an optional 32 bytes of data that may contain, serialized in string-hex representation'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think for definiteness we should allow any memo up to say 1KB, and make this field the mandatory memohash (blake2b of memo).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, that makes sense and covers future possibilities a bit better. I'll draw your attention to the Slate V5 PR once it's ready for review, and we can revise this RFC based on that
Link to rendered text