refactor: private refund cleanup (#7403)

noir-projects/noir-contracts/contracts/private_token_contract/src/main.nr

@@ -174,7 +174,7 @@ contract PrivateToken {
         // 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.
-        // TODO(#7324): using npk_m_hash here does not work with key rotation
+        // TODO(#7324), TODO(#7323): using npk_m_hash here is vulnerable in 2 ways described in the linked issues.
         storage.balances.sub(user_npk_m_hash, U128::from_integer(funded_amount)).emit(encode_and_encrypt_note_with_keys(&mut context, user_ovpk, user_ivpk));
 
         // 4. We generate the refund points.
@@ -190,22 +190,17 @@ contract PrivateToken {
         // function has access to the final transaction fee, which is needed to compute the actual refund amount.
         context.set_public_teardown_function(
             context.this_address(),
-            FunctionSelector::from_signature("complete_refund(Field,Field,Field,Field)"),
-            [fee_payer_point.x, fee_payer_point.y, user_point.x, user_point.y]
+            FunctionSelector::from_signature("complete_refund((Field,Field,bool),(Field,Field,bool))"),
+            [
+            fee_payer_point.x, fee_payer_point.y, fee_payer_point.is_infinite as Field, user_point.x, user_point.y, user_point.is_infinite as Field
+        ]
         );
     }
 
     #[aztec(public)]
     #[aztec(internal)]
-    fn complete_refund(
-        fee_payer_point_x: Field,
-        fee_payer_point_y: Field,
-        user_point_x: Field,
-        user_point_y: Field
-    ) {
+    fn complete_refund(fee_payer_point: Point, user_point: Point) {
         // 1. We get the final note content hashes by calling the `complete_refund` on the note.
-        let fee_payer_point = Point { x: fee_payer_point_x, y: fee_payer_point_y, is_infinite: false };
-        let user_point = Point { x: user_point_x, y: user_point_y, is_infinite: false };
         let tx_fee = context.transaction_fee();
         let (fee_payer_note_content_hash, user_note_content_hash) = TokenNote::complete_refund(fee_payer_point, user_point, tx_fee);
 

yarn-project/end-to-end/src/e2e_fees/private_refunds.test.ts

@@ -77,16 +77,21 @@ describe('e2e_fees/private_refunds', () => {
 
     expect(tx.transactionFee).toBeGreaterThan(0);
 
-    // 3. Now we compute the contents of the note containing the refund for Alice. The refund note value is simply
-    // the fee limit less the final transaction fee. The other 2 fields in the note are Alice's npk_m_hash and
+    // 3. We check that randomness for Bob was correctly emitted as an unencrypted log (Bobs needs it to reconstruct his note).
+    const resp = await aliceWallet.getUnencryptedLogs({ txHash: tx.txHash });
+    const bobRandomnessFromLog = Fr.fromBuffer(resp.logs[0].log.data);
+    expect(bobRandomnessFromLog).toEqual(bobRandomness);
+
+    // 4. Now we compute the contents of the note containing the refund for Alice. The refund note value is simply
+    // the fee limit minus the final transaction fee. The other 2 fields in the note are Alice's npk_m_hash and
     // the randomness.
     const refundNoteValue = t.gasSettings.getFeeLimit().sub(new Fr(tx.transactionFee!));
     // TODO(#7324): The values in complete address are currently not updated after the keys are rotated so this does
     // not work with key rotation as the key might be the old one and then we would fetch a new one in the contract.
     const aliceNpkMHash = t.aliceWallet.getCompleteAddress().publicKeys.masterNullifierPublicKey.hash();
     const aliceRefundNote = new Note([refundNoteValue, aliceNpkMHash, aliceRandomness]);
 
-    // 4. If the refund flow worked it should have added emitted a note hash of the note we constructed above and we
+    // 5. If the refund flow worked it should have added emitted a note hash of the note we constructed above and we
     // should be able to add the note to our PXE. Just calling `pxe.addNote(...)` is enough of a check that the note
     // hash was emitted because the endpoint will compute the hash and then it will try to find it in the note hash
     // tree. If the note hash is not found in the tree, an error is thrown.
@@ -101,13 +106,13 @@ describe('e2e_fees/private_refunds', () => {
       ),
     );
 
-    // 5. Now we reconstruct the note for the final fee payment. It should contain the transaction fee, Bob's
+    // 6. Now we reconstruct the note for the final fee payment. It should contain the transaction fee, Bob's
     // npk_m_hash (set in the paymentMethod above) and the randomness.
     // Note that FPC emits randomness as unencrypted log and the tx fee is publicly know so Bob is able to reconstruct
     // his note just from on-chain data.
     const bobFeeNote = new Note([new Fr(tx.transactionFee!), bobNpkMHash, bobRandomness]);
 
-    // 6. Once again we add the note to PXE which computes the note hash and checks that it is in the note hash tree.
+    // 7. Once again we add the note to PXE which computes the note hash and checks that it is in the note hash tree.
     await t.bobWallet.addNote(
       new ExtendedNote(
         bobFeeNote,
@@ -119,7 +124,7 @@ describe('e2e_fees/private_refunds', () => {
       ),
     );
 
-    // 7. At last we check that the gas balance of FPC has decreased exactly by the transaction fee ...
+    // 8. At last we check that the gas balance of FPC has decreased exactly by the transaction fee ...
     await expectMapping(t.getGasBalanceFn, [privateFPC.address], [initialFPCGasBalance - tx.transactionFee!]);
     // ... and that the transaction fee was correctly transferred from Alice to Bob.
     await expectMapping(