Skip to content
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.

Sign up for GitHub

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

feat: private refunds optimizations #7968

Merged
merged 2 commits into from
Aug 14, 2024
Merged

feat: private refunds optimizations #7968

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1085d09
feat: private refunds optimizations
benesjan Aug 14, 2024
7ecc3ae
optimizing balance subtraction
benesjan Aug 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion noir-projects/noir-contracts/contracts/token_with_refunds_contract/src/main.nr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -545,7 +545,14 @@ contract TokenWithRefunds {
// 3. Deduct the funded amount from the user's balance - this is a maximum fee a user is willing to pay
// (called fee limit in aztec spec). The difference between fee limit and the actual tx fee will be refunded
// to the user in the `complete_refund(...)` function.
storage.balances.sub(user, U128::from_integer(funded_amount)).emit(encode_and_encrypt_note_with_keys(&mut context, user_ovpk, user_ivpk, user));
let change = subtract_balance(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm surprised this function is available here, I thought we had only added it to the token contract. Did we replicate those changes here?

At any point we should likely nuke with_refunds and merge it into the main token.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I updated the token in a PR down the stack.

&mut context,
storage,
user,
U128::from_integer(funded_amount),
INITIAL_TRANSFER_CALL_MAX_NOTES
);
storage.balances.add(user, change).emit(encode_and_encrypt_note_with_keys_unconstrained(&mut context, user_ovpk, user_ivpk, user));

// 4. We create the partial notes for the fee payer and the user.
// --> Called "partial" because they don't have the amount set yet (that will be done in `complete_refund(...)`).
Expand Down
8 changes: 4 additions & 4 deletions yarn-project/end-to-end/src/e2e_fees/private_refunds.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ describe('e2e_fees/private_refunds', () => {
aliceRandomness,
bobRandomness,
t.bobWallet.getAddress(), // Bob is the recipient of the fee notes.
true, // We set max fee/funded amount to zero to trigger the error.
true, // We set max fee/funded amount to 1 to trigger the error.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Had to bump this to 1 instead because otherwise the test would revert here and not in the expected assert.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is rather yucky, we should prioritze #7694

),
},
}),
Expand Down Expand Up @@ -195,10 +195,10 @@ class PrivateRefundPaymentMethod implements FeePaymentMethod {
private feeRecipient: AztecAddress,

/**
* If true, the max fee will be set to 0.
* If true, the max fee will be set to 1.
* TODO(#7694): Remove this param once the lacking feature in TXE is implemented.
*/
private setMaxFeeToZero = false,
private setMaxFeeToOne = false,
) {}

/**
Expand All @@ -221,7 +221,7 @@ class PrivateRefundPaymentMethod implements FeePaymentMethod {
async getFunctionCalls(gasSettings: GasSettings): Promise<FunctionCall[]> {
// We assume 1:1 exchange rate between fee juice and token. But in reality you would need to convert feeLimit
// (maxFee) to be in token denomination.
const maxFee = this.setMaxFeeToZero ? Fr.ZERO : gasSettings.getFeeLimit();
const maxFee = this.setMaxFeeToOne ? Fr.ONE : gasSettings.getFeeLimit();

await this.wallet.createAuthWit({
caller: this.paymentContract,
Expand Down
Loading