feat: private fee payment with private rebate #4891
Conversation
Benchmark resultsMetrics with a significant change:
Detailed resultsAll benchmarks are run on txs on the This benchmark source data is available in JSON format on S3 here. Values are compared against data from master at commit L2 block published to L1Each column represents the number of txs on an L2 block published to L1.
L2 chain processingEach column represents the number of blocks on the L2 chain where each block has 16 txs.
Circuits statsStats on running time and I/O sizes collected for every circuit run across all benchmarks.
Tree insertion statsThe duration to insert a fixed batch of leaves into each tree type.
MiscellaneousTransaction sizes based on how many contracts are deployed in the tx.
Transaction processing duration by data writes.
|
alexghr
force-pushed
the
feat/private-fee-rebate
branch
3 times, most recently
from
9b66ea1
to
e14d356
Compare
alexghr
changed the title
alexghr
force-pushed
the
feat/private-fee-rebate
branch
from
e14d356
to
15d9710
Compare
|
I have some concerns with some of these changes, but it might take me some time to understand and articulate them. Also, @LHerskind and his team might need to be looped into such large aztec.nr changes. I'm not sure who the 'owner' of aztec.nr is anymore/yet (I know there's going to be some application process). Edit: but I do appreciate this looks like a lot of great effort, and can't have been an easy set of changes to make. I also wouldn't want to block progress on fees work. I'm just conscious there were recent concerns raised that aztec.nr might not have "one voice" anymore, and that people were worried stuff was being added to aztec.nr by different teams without some 'visionary' ensuring consistency. I'm also conscious that if we discover that this approach to partial notes isn't generic enough, it's going to be quite difficult to unpick or maintain. |
|
|
||
| assert(storage.partial_notes.at(hash).read(), "Partial notes not authorized"); | ||
| // only completable once | ||
| context.push_new_nullifier(hash.to_field(), 0); |
There was a problem hiding this comment.
Is this temporary? The idea is to set the public slot to 0 isn't it?
There was a problem hiding this comment.
I actually thought this would be neater as it signals both that it was an action that was allowed at some point in the past and that it was consumed and can not be run anymore (in way, similar to contract initialiasation)
There was a problem hiding this comment.
Yeah that makes sense. I was just thinking that this uses 32 bytes more DA. Setting the storage to 0 would presumably be squashed onto the setting of it to 1 in auth_partial_notes_pair()
|
I'll keep this PR unmerged until we reach consensus :D
@iAmMichaelConnor one advantage this "patching" implementation has (that this PR does a very poor job of conveying) is that the "partial note" and "note completion" events are entirely under the contract's control. Once we have event selectors (and once encrypted logs are generalised) all the PXE needs to do is to keep track of the partial note and the completion patch (by e.g. requiring them to have a known shape or use a specific log selector). The PXE could then defer control to the contract implementation to complete a note, similar to contract TokenContract {
unconstrained fn complete_partial_note(
note_type_id: Field,
partial_note_content: [Field; 20],
note_patch: [Field; 20]
) -> TokenNote {
if note_type_id == TokenNote::get_note_type_id() {
TokenNote::complete_note(partial_note_content, note_patch)
} else {
// ...
}
}
}
impl TokenNote {
fn complete_note(content: [Field; 3], patch: [Field; 1] {
TokenNote { amount: patch[0], owner: content[1], randomness: content[2] }
}
}(the partial content above could be optimized down to just two fields too!) Later edit: and all the information will live on chain so even if I restore my private key into a new PXE, I can recreate all of my partial notes back to full notes (which is not the case with e.g. pending shields -- if my human brain forgets the preimage to the secret hash before I claim it, then it's lost forever) |
There was a problem hiding this comment.
Something I don't particularly like is that it increase complexity of all notes, but the benefit is only to the very few types of notes that people will be using to pay fees.
The amount of extra stuff to handle is a bit concerning. We have approval in public storage (64 bytes) but it can be reset afterwards to not broadcast it, we have an extra nullifier (32 bytes), we have two partial notes (~200bytes each?), unencrypted logs (~32+64 bytes for the contract address and things for each partial note?) sounds like we are somewhere around 700 bytes to handle the fee then?
If a transaction is private, the gas-limit is essentially fixed and if there is a base-fee it can be fairly well predicted. This means that the refunds would likely be small. We need to be mindful, that if the refund is small, the cost of the refund could be larger than the fee.
Alternative that is a bit less neat from wallet POV, but don't change life-cycle: Use pending shields. As part of the execution provide a secretHash, a note is inserted. And you can collect the refund later 🤷♂ If you have plenty of refunds you can aggregate them at once, or you can ignore the ones that might be to expensive to fully retrieve.
Ran a test. The mint_public transaction in the test emits 1857 bytes when it is paying the fee. Without the fee only 185, so fee is 9/10 or the transaction.
|
As @alexghr says, this can be made completely generic. Once we have event selectors everything comes down to a contract's implementation as to what it means to build a partial note and then complete it. The general principle of building a private note containing information sourced from both private and public domains seems pretty useful, so I disagree that this would only be used for fee payments. Why wouldn't shielding be done using a similar mechanism? This seems much more elegant than storing a secret and having to submit a second transaction. Regarding some of the other concerns. In this example Alice creates 3 notes:
We could get rid of the note in 1. Alice could collect enough notes to be >= the max fee, and get everything back minus the fee in note 2 as a refund. Then we would be down to 2 notes, 2 events and the public storage write which I'd like to think we could remove completely as it resets back to the value it was before the transaction was sent. I can't think of a way to keep the whole exchange using private notes and get the data requirements any lower. Of course all of this is optional. Alice could pay directly in AZT. Or Alice could be doing a private only transaction, pays privately and uses a 'don't bother with the refund' method on the FPC. In both of these sceanrios there is no need for the setup and teardown phases either. |
|
I should've used stacked PRs because I think things are getting muddied wrt the implementation enabling partial notes ( I think it would be great to decide if we want to merge:
Change 1 - has a cost in terms of developers need to be aware of it and implement the new Note interface accordingly (it leaks into the Aztec.nr API). I had to make changes to the global Change 2 - a specific feature using the partial note implementation. Yes, this could be made more efficient in the future by: emitting just two fields instead of all three for partial ntoes (which would require event selectors), using two public storage writes (which would get squashed down to one) instead of public storage + nullifiers.
I've brought this up as well and the consensus was to make the rebate step optional. We should support multiple fee paying methods: some can use partial notes, some can use shield/unshield, some are public, others can be without rebate so you'd have to be really really tight during simulation.
👍. But should this be the only option?
Up to max-number-of-fn-calls in a tx (currently 4). Using shields just defers the cost to later, the sender still has to pay to turn a shield into a note. With that partial token notes implementation here that cost is paid immediately, with shields it's some future tx (which could be cheaper if base gas price goes down). |
|
I agree with the view that this would be useful, but is not strictly necessary since we can achieve private refunds with pending shields, and it comes with significant costs, particularly on DevEx (mental burden) and UX (expensive). I'd speculate a user would prefer two cheap transactions (one to do the thing, one to get their money back) over 1 expensive transaction, especially since the wallet can handle the complexity for them. I think it was useful to demonstrate the patching, and how partial notes could be implemented, but I'd rather we incorporate them in response to demand, since again, they are not strictly necessary for private refunds. So should we revisit this when we have a larger community of developers and understanding of the real costs? |
alexghr
force-pushed
the
feat/private-fee-rebate
branch
from
15d9710
to
fe30702
Compare
|
Here's an example of an alternative approach to creating notes, that might require no changes to aztec.nr. Instead, it puts the burden of interpreting app-specific events on the javascript dapp itself. I'm not necessarily saying it's an approach I like, yet: I haven't thought about this subject enough, and there could be better approaches. Maybe I'm just playing devil's advocate, or something. The flow for partial notes, as I understand it, is:
A half-thought-out idea:
The MyNote declaration file: // Declare a note, which adheres to the NoteInterface, as normal. Give it some optional data members, to emulate "partial-ness" of this note.
struct MyNote {
a: Option<Field>,
b: Option<Field>,
c: Option<Field>
rand: Field,
}
// I don't know if enums are a thing in rust.
enum MyNoteGeneratorEnum {
domain_separator,
a,
a_null,
b,
b_null,
c,
c_null,
rand,
}
// computing the generators at compile time not shown here.
global GENERATOR_DOMAIN_SEP: GrumpkinPoint = compute_generator(MyNoteGeneratorEnum::domain_separator);
global GENERATOR_A: GrumpkinPoint = compute_generator(MyNoteGeneratorEnum::a);
global GENERATOR_A_NULL: GrumpkinPoint = compute_generator(MyNoteGeneratorEnum::a_null);
global GENERATOR_B: GrumpkinPoint = compute_generator(MyNoteGeneratorEnum::b);
global GENERATOR_B_NULL: GrumpkinPoint = compute_generator(MyNoteGeneratorEnum::b_null);
global GENERATOR_C: GrumpkinPoint = compute_generator(MyNoteGeneratorEnum::c);
global GENERATOR_C_NULL: GrumpkinPoint = compute_generator(MyNoteGeneratorEnum::c_null);
global GENERATOR_RAND: GrumpkinPoint = compute_generator(MyNoteGeneratorEnum::rand);
impl NoteInterface for MyNote {
compute_note_body_hash(&self) { // I don't recall the name or syntax, sorry
if (self.a.is_none || self.b.is_none || self.c.is_none) {
throw new Error("this is a partial note: it's missing some values");
}
pedersen("MyNote".to_field(), self.a.unwrap_unsafe(), self.b.unwrap_unsafe(), self.c.unwrap_unsafe().to_field(), self.rand)
}
}
// Also implement some custom partial note methods. Not sure where these should live, but I'll write them here.
// Very inefficient function, but it's illustrative:
fn compute_partial_note_commitment(note: MyNote) -> GrumpkinPoint {
// Multiply the scalars by the corresponding points, and add the results together.
// This is basically a homomorphic pedersen hash:
Grumpkin::multi_scalar_mul(
scalars: [
"MyNote".to_field(),
self.a.is_none ? 1 : self.a.unwrap_unsafe(),
self.b.is_none ? 1 : self.b.unwrap_unsafe(),
self.c.is_none ? 1 : self.c.unwrap_unsafe(),
rand,
],
points: [
GENERATOR_DOMAIN_SEP,
self.a.is_none ? GENERATOR_A_NULL : GENERATOR_A,
self.b.is_none ? GENERATOR_B_NULL : GENERATOR_B,
self.c.is_none ? GENERATOR_C_NULL : GENERATOR_C,
GENERATOR_RAND,
]
)
}
// Note: the `NULL` generators are required to allow `0` to be a valid field of the completed note.The Contract which makes use of the note: MyContract {
storage {
interaction_nonce: PublicMutableState<u64>;
partial_commitments: Map<Field, PublicMutableState<GrumpkinPoint>>;
}
// Create a partial note with data members `b` and `c` missing:
#[aztec(private)]
fn create_the_partial_note_commitment_with_b_and_c_missing(a, rand) {
let partial_note_commitment: GrumpkinPoint = compute_partial_note_commitment(
MyNote {
a,
Option::none,
Option::none
rand,
}
);
// Encrypt and emit `a` and `rand` as an outgoing message (NOT as a note).
// Syntax not shown because I don't know what it is.
// Then store the partial note in public-land:
this.register_partial_note_commitment(partial_note_commitment);
}
// Register the partial commitment, to be completed some time later
#[aztec(public)]
fn register_partial_note_commitment(partial_note_commitment: GrumpkinPoint) {
// store it somewhere:
let nonce = interaction_nonce.read();
partial_commitments.at(++nonce).write(partial_note_commitment); // we've stored it for later.
interaction_nonce.write(nonce);
// The caller will subscribe to this contract and nonce, somehow.
emit NewPartialNoteRegistered(nonce, partial_note_commitment); // emit an event with the interaction nonce
}
#[aztec(public)]
fn complete_partial_note_commitment_with_b_and_c(interaction_nonce: u64, b: Field, c: Field) {
let partial_note_commitment: GrumpkinPoint = partial_commitments.at(interaction_nonce).read();
// maybe delete this entry, so it can't be 'completed' again:
partial_commitments.at(interaction_nonce).write(GrumpkinPoint{ 0, 0, });
// I'm being lazier with notation here: `+` and `*` are elliptic curve operations here:
let completed_note_commitment: GrumpkinPoint = partial_note_commitment +
(b * GENERATOR_B - GENERATOR_B_NULL) +
(c * GENERATOR_C - GENERATOR_C_NULL);
let completed_note_hash: Field = completed_note_commitment.x; // assuming taking the x-coord is collision resistant, which it probably isn't.
// Inform the owner of the note (who should be subscribed to this event and interaction_nonce) about the completion of the note.
emit NewlyCompletedNote(interaction_nonce, b, c, note_commitment);
}
} // MyContract |
|
I haven't delved deeply enough into the details of this PR to compare these two approaches, and I'm biased because I worked on Aztec Connect's circuits for so long. But it might be helpful if someone could provide pros/cons to these approaches (or propose another approach). |
|
Thanks for submitting the code Mike, this is amazing! If I understand the cryptography correctly we can subtract the "null" points and add in the missing ones once we know the values. One thing that's not clear to me (and this is related to aztec.nr's current design): what about the storage slot associated with the note? I can see partial note hashes are stored at a known location so that the contract can find them later (sidenote I think the Other than the advanced crypto I think the flows are the same:
Out of all of these steps, I think 1, 3, 5, 6 are concerns for aztec.nr, while steps 2, 4 are the responsibility of the contract developer (along with the data emitted by steps 3 and 6 and the data necessary for steps 1 and 5). I don't think this gets away from having to patch the note in the PXE as the server would still have to somehow recreate the full note and store it in its database (so that --
yes, we can (and this is how I assumed we'd implement them tbf) but there's big downsides to them in the context of fee rebates:
Regarding the costs of the above, things could be made cheaper if Alice calls |
Oh good point. I'd forgotten to consider storage slots in my code snippets, and indeed in my example function With this homomorphic approach to commitments, the I'm not sure how my proposed approach would work if the storage slot is hashed into the note in a non-homomorphic way (which is the way storage slots are included at the moment with aztec.nr). Edit: I'll need to think more on your other comments later in the week :) |
Nice. I had been hopeful we could solve our problems with math 😁. If I understand, if we took that general approach, a token could create a trait Partialable<T> where
T: NoteInterface<N> {
fn compute_partial_note_content_hash(self) -> Field;
}plus have a few new helpers for just emitting relevant logs and helping with the crypto.
PXE still needs a way to record these partial note logs, but I don't think we strictly need the ability to patch, since I think that once we see the complete note, we just drop the corresponding partial note? Side note, I think if we remove the constraint that emitted encrypted logs are always notes (and e.g. prefix logs with a byte that encodes their type), this becomes a lot more clean. I think some weirdness could come though if we enshrine a log type of "partial note" without enshrining the concept of "partialable" in aztec-nr. But maybe its not a big deal, since probably from the PXE perspective a "partial note" would just be treated as "something for which I eventually expect a full note". I think the only reason you would need to "patch" is if you fill in some more of an existing partial note without completing it. But that seems like it could be its own interface that extends "partial-able" which we could define and implement in the future. Am I understanding correctly? Generally I like this much more than obliging all note types to adhere to an inner/outer hash. And I think homomorphic crypto could be a good implementation: it feels like it fits very well with what we're trying to achieve here. Side note, I expect we could eventually pull that trait and helpers into aztec-nr once they are stable; the nice thing is that it would be an extension, and not change any existing api or implementation. In the meantime, if we're all on board, I'd say to design concretely what this could look to get it working end-to-end with just the TokenNote. |
That would be amazing indeed but we somehow need to mix in the storage slot in private.
Maybe I'm missing something, but my understanding (based on working with the code in this PR) is that the PXE sees hashes being inserted into the notes tree from public. It somehow needs to get the preimage to that hash in order to add a note to its database. I called it "patching" the partial encrypted log with public data (ie. "I got an array of fields from private execution, I got another array of fields from public execution, put two and two together and gimme a note"), we can call it something else but I don't see how it can be avoided?
I agree and iirc there's a ticket to track it. Edit: found it #1988
Me too, but we just need to udnerstand how the storage slot fits into all of this and if it changes how note hashing is done. |
|
It almost feels then like we solve #1988 and come back to this?
Ah yes, you're right! We spoke past each other. In my mind "patch" implies you can keep patching, but here it's a special patch that is "merge/patch and complete". |
|
Closing this |
Private fee payment with private rebate
This PR implements a new fee payment flow whereby a sender pays tx fees by creating two partial notes: one for itself (to receive any rebates back) and one for the fee payment contract that will pay tx costs in the gas token.
Changes to note hashes
In order to achieve this the way notes are hashed has been changed: previously a note hash was defined as:
(where
note_content_hashis up to the note to implement)Now hashing requires one extra step:
inner_note_content_hashandouter_note_content_hashare left to the note implementation to define.The
partial_note_hashhides away the storage slot (as long asinner_note_content_hashcontains randomness so that the whole hash is slated) and enables the note to be completed in public without revealing who the owner of the note is.Costs of an extra hashing operation
The extra level of hasing has to be paid for every note so it might look expensive, but this will mostly be done in private execution (where most notes are created and consumed) so it won't affect total tx costs. Notes fully created in public (like the
TransparentNote) will have to pay this cost though.Partial note pairs in the token contract
The concept of "partial note pairs" is added to the Token contract. A user takes an amount out of their private balance, creates two partial notes and allows a 3rd party to complete these partial notes. The Token contract enforces this and also enforces that the two completed notes add up exactly to the original amount (similar to the public authwit scheme for account contract). This is used by fee payment contracts to "escrow" an amount of tokens from the sender's private balance and at the end build two notes, one for itself with an amount proportional to the tx fee and one for the sender with the leftover.
Cost of setting an extra storage slot
This will increase how much a tx costs, but it only affects txs that use this fee payment flow.
Disabled test
This PR disables the dapp subscription entrypoint E2E test due to a bug in how nonRevertible and normal side effects are currently tracked in public. This issue will be fixed in a future PR. See #4958