[confidential-transfer] Add transfer with fee proof generation and extraction #6945
Conversation
samkim-crypto
force-pushed
the
transfer-fee-split
branch
3 times, most recently
from
cae9c4b to
ac19d52
Compare
| test_transfer_with_fee_proof_validity(100, 100, 5, 10); | ||
| test_transfer_with_fee_proof_validity(100, 100, 5, 1); |
There was a problem hiding this comment.
Is there a reason to not use big numbers like with the transfer correctness test? ie 2^16 -1, 2^48 -1
There was a problem hiding this comment.
No, there is no specific reason. I had a bunch of tests and it seemed like the test was becoming quite slow and the transfer amount was already somewhat tested for transfer without fee tests, so I thought I would just include the smaller ones. Now, I did add the tests for big numbers and used (5, 10) and (5, 1) for the (basis point, max fee) parameters. I can also add more edge case fee parameters too if you see fit.
| ( | ||
| CiphertextCommitmentEqualityProofData, | ||
| BatchedGroupedCiphertext3HandlesValidityProofData, | ||
| PercentageWithCapProofData, | ||
| BatchedGroupedCiphertext2HandlesValidityProofData, | ||
| BatchedRangeProofU256Data, | ||
| ), |
There was a problem hiding this comment.
nit: normally we should make this into a struct, but I see that the rest of the crate is using tuples, so this can stay. For the future though, would it be worth using structs?
There was a problem hiding this comment.
Yeah, I think this is a good point and I think we can certainly organize it using a structs. Since this issue is shared with transfer without fee, I'll address it in a separate PR.
token/confidential-transfer/proof-generation/src/transfer_with_fee.rs
Outdated
Show resolved
Hide resolved
| ristretto::multiply_ristretto(&transfer_fee_basis_points_scalar, &transfer_amount_point) | ||
| .ok_or(TokenProofExtractionError::CurveArithmetic)?; | ||
|
|
||
| const MAX_FEE_BASIS_POINTS: u64 = 10_000; |
There was a problem hiding this comment.
nit: You can remove this and use the one defined at the top of the file instead
There was a problem hiding this comment.
Yep, thanks for the catch. Fixed!
Co-authored-by: Jon C <[email protected]>
samkim-crypto
force-pushed
the
transfer-fee-split
branch
from
e7b7900 to
6798ef0
Compare
Problem
The logic to generate proof data for transfer with fee have not been added to the repo yet.
Summary
Add the logic to generate the proof data. This proof is quite technical, but it essentially follows the existing logic in token client.