diff --git a/noir-projects/noir-contracts/contracts/private_fpc_contract/src/main.nr b/noir-projects/noir-contracts/contracts/private_fpc_contract/src/main.nr index c7a6d116d2e..c4d5adea2b1 100644 --- a/noir-projects/noir-contracts/contracts/private_fpc_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/private_fpc_contract/src/main.nr @@ -1,7 +1,7 @@ mod lib; contract PrivateFPC { - use dep::aztec::protocol_types::{abis::log_hash::LogHash, address::AztecAddress}; + use dep::aztec::protocol_types::{abis::log_hash::LogHash, address::AztecAddress, hash::poseidon2_hash}; use dep::aztec::state_vars::SharedImmutable; use dep::private_token::PrivateToken; use crate::lib::emit_randomness_as_unencrypted_log; @@ -20,19 +20,23 @@ contract PrivateFPC { } #[aztec(private)] - fn fund_transaction_privately(amount: Field, asset: AztecAddress, randomness: Field) { + fn fund_transaction_privately(amount: Field, asset: AztecAddress, user_randomness: Field) { assert(asset == storage.other_asset.read_private()); // convince the FPC we are not cheating - context.push_nullifier(randomness, 0); + context.push_nullifier(user_randomness, 0); - // allow the FPC to reconstruct their fee note - emit_randomness_as_unencrypted_log(&mut context, randomness); + // We use different randomness for fee payer to prevent a potential privay leak (see impl + // of PrivatelyRefundable for TokenNote for details). + let fee_payer_randomness = poseidon2_hash([user_randomness]); + // We emit fee payer randomness to ensure FPC admin can reconstruct their fee note + emit_randomness_as_unencrypted_log(&mut context, fee_payer_randomness); PrivateToken::at(asset).setup_refund( storage.admin_npk_m_hash.read_private(), context.msg_sender(), amount, - randomness + user_randomness, + fee_payer_randomness ).call(&mut context); context.set_as_fee_payer(); } diff --git a/noir-projects/noir-contracts/contracts/private_token_contract/src/main.nr b/noir-projects/noir-contracts/contracts/private_token_contract/src/main.nr index 369aa623f31..18873a6e027 100644 --- a/noir-projects/noir-contracts/contracts/private_token_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/private_token_contract/src/main.nr @@ -160,7 +160,8 @@ contract PrivateToken { fee_payer_npk_m_hash: Field, // NpkMHash of the entity which will receive the fee note. user: AztecAddress, // A user for which we are setting up the fee refund. funded_amount: Field, // The amount the user funded the fee payer with (represents fee limit). - randomness: Field // A randomness to mix in with the generated notes. + user_randomness: Field, // A randomness to mix in with the generated refund note for the sponsored user. + fee_payer_randomness: Field // A randomness to mix in with the generated fee note for the fee payer. ) { // 1. This function is called by fee paying contract (fee_payer) when setting up a refund so we need to support // the authwit flow here and check that the user really permitted fee_payer to set up a refund on their behalf. @@ -183,7 +184,8 @@ contract PrivateToken { fee_payer_npk_m_hash, user_npk_m_hash, funded_amount, - randomness + user_randomness, + fee_payer_randomness ); // 5. Set the public teardown function to `complete_refund(...)`. Public teardown is the only time when a public diff --git a/noir-projects/noir-contracts/contracts/private_token_contract/src/test/basic.nr b/noir-projects/noir-contracts/contracts/private_token_contract/src/test/basic.nr index 3f7f75ed93b..97f49cdb5ca 100644 --- a/noir-projects/noir-contracts/contracts/private_token_contract/src/test/basic.nr +++ b/noir-projects/noir-contracts/contracts/private_token_contract/src/test/basic.nr @@ -40,7 +40,8 @@ unconstrained fn setup_refund_success() { let (env, token_contract_address, owner, recipient, mint_amount) = utils::setup_and_mint(true); let funded_amount = 1_000; - let refund_nonce = 42; + let user_randomness = 42; + let fee_payer_randomness = 123; let mut context = env.private(); let recipient_npk_m_hash = context.get_header().get_npk_m_hash(&mut context, recipient); @@ -48,7 +49,8 @@ unconstrained fn setup_refund_success() { recipient_npk_m_hash, // fee payer owner, // sponsored user funded_amount, - refund_nonce + user_randomness, + fee_payer_randomness ); authwit_cheatcodes::add_private_authwit_from_call_interface(owner, recipient, setup_refund_from_call_interface); @@ -70,7 +72,7 @@ unconstrained fn setup_refund_success() { &mut TokenNote { amount: U128::from_integer(funded_amount - 1), npk_m_hash: owner_npk_m_hash, - randomness: refund_nonce, + randomness: user_randomness, header: NoteHeader::empty() }, PrivateToken::storage().balances.slot, @@ -80,7 +82,7 @@ unconstrained fn setup_refund_success() { &mut TokenNote { amount: U128::from_integer(1), npk_m_hash: recipient_npk_m_hash, - randomness: refund_nonce, + randomness: fee_payer_randomness, header: NoteHeader::empty() }, PrivateToken::storage().balances.slot, diff --git a/noir-projects/noir-contracts/contracts/private_token_contract/src/types/token_note.nr b/noir-projects/noir-contracts/contracts/private_token_contract/src/types/token_note.nr index 6d0a9a8cc32..d41b70934b6 100644 --- a/noir-projects/noir-contracts/contracts/private_token_contract/src/types/token_note.nr +++ b/noir-projects/noir-contracts/contracts/private_token_contract/src/types/token_note.nr @@ -19,7 +19,8 @@ trait PrivatelyRefundable { fee_payer_npk_m_hash: Field, user_npk_m_hash: Field, funded_amount: Field, - randomness: Field + user_randomness: Field, + fee_payer_randomness: Field ) -> (EmbeddedCurvePoint, EmbeddedCurvePoint); fn complete_refund( @@ -155,8 +156,8 @@ impl OwnedNote for TokenNote { * deduce what n is. This is the discrete log problem. * * However we can still perform addition/subtraction on points! That is why we generate those two points, which are: - * incomplete_fee_payer_point := (fee_payer_npk + randomness) * G - * incomplete_user_point := (user_npk + funded_amount + randomness) * G + * incomplete_fee_payer_point := (fee_payer_npk + fee_payer_randomness) * G + * incomplete_user_point := (user_npk + funded_amount + user_randomness) * G * * where `funded_amount` is the total amount in tokens that the sponsored user initially supplied, from which the transaction fee will be subtracted. * @@ -167,25 +168,37 @@ impl OwnedNote for TokenNote { * Then we arrive at the final points via addition/subtraction of that transaction fee point: * * fee_payer_point := incomplete_fee_payer_point + fee_point - * = (fee_payer_npk + randomness) * G + transaction_fee * G - * = (fee_payer_npk + randomness + transaction_fee) * G + * = (fee_payer_npk + fee_payer_randomness) * G + transaction_fee * G + * = (fee_payer_npk + fee_payer_randomness + transaction_fee) * G * * user_point := incomplete_user_point - fee_point - * = (user_npk + funded_amount + randomness) * G - transaction_fee * G - * = (user_npk + randomness + (funded_amount - transaction_fee)) * G + * = (user_npk + funded_amount + user_randomness) * G - transaction_fee * G + * = (user_npk + user_randomness + (funded_amount - transaction_fee)) * G * * When we return the x-coordinate of those points, it identically matches the note_content_hash of (and therefore *is*) notes like: * { * amount: (funded_amount - transaction_fee), * npk_m_hash: user_npk, - * randomness: randomness + * randomness: user_randomness * } + * + * Why do we need different randomness for the user and the fee payer notes? + * --> This is because if the randomness values were the same we could fingerprint the user by doing the following: + * 1) randomness_influence = incomplete_fee_payer_point - G * fee_payer_npk = + * = (fee_payer_npk + randomness) * G - G * fee_payer_npk = randomness * G + * 2) user_fingerprint = incomplete_user_point - G * funded_amount - randomness_influence = + * = (user_npk + funded_amount + randomness) * G - funded_amount * G - randomness * G = + * = user_npk * G + * 3) Then the second time the user would use this fee paying contract we would recover the same fingerprint and + * link that the 2 transactions were made by the same user. Given that it's expected that only a limited set + * of fee paying contracts will be used and they will be known searching for fingerprints by trying different + * fee payer npk values of these known contracts is a feasible attack. */ impl PrivatelyRefundable for TokenNote { - fn generate_refund_points(fee_payer_npk_m_hash: Field, user_npk_m_hash: Field, funded_amount: Field, randomness: Field) -> (EmbeddedCurvePoint, EmbeddedCurvePoint) { + fn generate_refund_points(fee_payer_npk_m_hash: Field, user_npk_m_hash: Field, funded_amount: Field, user_randomness: Field, fee_payer_randomness: Field) -> (EmbeddedCurvePoint, EmbeddedCurvePoint) { // 1. To be able to multiply generators with randomness and npk_m_hash using barretneberg's (BB) blackbox function we // first need to convert the fields to high and low limbs. - let (randomness_lo, randomness_hi) = decompose(randomness); + let (fee_payer_randomness_lo, fee_payer_randomness_hi) = decompose(fee_payer_randomness); let (fee_payer_npk_m_hash_lo, fee_payer_npk_m_hash_hi) = decompose(fee_payer_npk_m_hash); // 2. Now that we have correct representationsn of fee payer and randomness we can compute `G ^ (fee_payer_npk + randomness)` @@ -196,12 +209,13 @@ impl PrivatelyRefundable for TokenNote { hi: fee_payer_npk_m_hash_hi }, EmbeddedCurveScalar { - lo: randomness_lo, - hi: randomness_hi + lo: fee_payer_randomness_lo, + hi: fee_payer_randomness_hi }] ); // 3. We do the necessary conversion for values relevant for the sponsored user point. + let (user_randomness_lo, user_randomness_hi) = decompose(user_randomness); // TODO(#7324): representing user with their npk_m_hash here does not work with key rotation let (user_lo, user_hi) = decompose(user_npk_m_hash); let (funded_amount_lo, funded_amount_hi) = decompose(funded_amount); @@ -218,8 +232,8 @@ impl PrivatelyRefundable for TokenNote { hi: funded_amount_hi }, EmbeddedCurveScalar { - lo: randomness_lo, - hi: randomness_hi + lo: user_randomness_lo, + hi: user_randomness_hi }] ); diff --git a/yarn-project/end-to-end/src/e2e_fees/private_refunds.test.ts b/yarn-project/end-to-end/src/e2e_fees/private_refunds.test.ts index f98c200d574..bbbae0727de 100644 --- a/yarn-project/end-to-end/src/e2e_fees/private_refunds.test.ts +++ b/yarn-project/end-to-end/src/e2e_fees/private_refunds.test.ts @@ -8,6 +8,7 @@ import { } from '@aztec/aztec.js'; import { Fr, type GasSettings } from '@aztec/circuits.js'; import { FunctionSelector, FunctionType } from '@aztec/foundation/abi'; +import { poseidon2Hash } from '@aztec/foundation/crypto'; import { type PrivateFPCContract, PrivateTokenContract } from '@aztec/noir-contracts.js'; import { expectMapping } from '../fixtures/utils.js'; @@ -53,7 +54,8 @@ describe('e2e_fees/private_refunds', () => { // 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 bobNpkMHash = t.bobWallet.getCompleteAddress().publicKeys.masterNullifierPublicKey.hash(); - const randomness = Fr.random(); + const aliceRandomness = Fr.random(); // Called user_randomness in contracts + const bobRandomness = poseidon2Hash([aliceRandomness]); // Called fee_payer_randomness in contracts // 2. We call arbitrary `private_get_name(...)` function to check that the fee refund flow works. const tx = await privateToken.methods @@ -65,7 +67,8 @@ describe('e2e_fees/private_refunds', () => { privateToken.address, privateFPC.address, aliceWallet, - randomness, + aliceRandomness, + bobRandomness, bobNpkMHash, // We use Bob's npk_m_hash in the notes that contain the transaction fee. ), }, @@ -81,7 +84,7 @@ describe('e2e_fees/private_refunds', () => { // 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, randomness]); + 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 // should be able to add the note to our PXE. Just calling `pxe.addNote(...)` is enough of a check that the note @@ -102,7 +105,7 @@ describe('e2e_fees/private_refunds', () => { // 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, randomness]); + 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. await t.bobWallet.addNote( @@ -144,10 +147,16 @@ class PrivateRefundPaymentMethod implements FeePaymentMethod { private wallet: Wallet, /** - * A randomness to mix in with the generated notes. + * A randomness to mix in with the generated refund note for the sponsored user. * Use this to reconstruct note preimages for the PXE. */ - private randomness: Fr, + private userRandomness: Fr, + + /** + * A randomness to mix in with the generated fee note for the fee payer. + * Use this to reconstruct note preimages for the PXE. + */ + private feePayerRandomness: Fr, /** * The hash of the master nullifier public key that the FPC sends notes it receives to. @@ -179,8 +188,14 @@ class PrivateRefundPaymentMethod implements FeePaymentMethod { caller: this.paymentContract, action: { name: 'setup_refund', - args: [this.feeRecipientNpkMHash, this.wallet.getCompleteAddress().address, maxFee, this.randomness], - selector: FunctionSelector.fromSignature('setup_refund(Field,(Field),Field,Field)'), + args: [ + this.feeRecipientNpkMHash, + this.wallet.getCompleteAddress().address, + maxFee, + this.userRandomness, + this.feePayerRandomness, + ], + selector: FunctionSelector.fromSignature('setup_refund(Field,(Field),Field,Field,Field)'), type: FunctionType.PRIVATE, isStatic: false, to: this.asset, @@ -195,7 +210,7 @@ class PrivateRefundPaymentMethod implements FeePaymentMethod { selector: FunctionSelector.fromSignature('fund_transaction_privately(Field,(Field),Field)'), type: FunctionType.PRIVATE, isStatic: false, - args: [maxFee, this.asset, this.randomness], + args: [maxFee, this.asset, this.userRandomness], returnTypes: [], }, ]; diff --git a/yarn-project/noir-contracts.js/package.json b/yarn-project/noir-contracts.js/package.json index 1df400f15aa..be2ad23370e 100644 --- a/yarn-project/noir-contracts.js/package.json +++ b/yarn-project/noir-contracts.js/package.json @@ -74,4 +74,4 @@ "engines": { "node": ">=18" } -} \ No newline at end of file +}