From a80519d3514785cba74c64dc0f044f75b60adf40 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=81lvaro=20Rodr=C3=ADguez?= Date: Tue, 19 Mar 2024 12:31:14 +0100 Subject: [PATCH] feat: Ensure claimer is owner of the note in claim contract (#5135) --- .../contracts/claim_contract/src/main.nr | 7 ++- .../src/e2e_crowdfunding_and_claim.test.ts | 63 +++++++++++++++++-- 2 files changed, 63 insertions(+), 7 deletions(-) diff --git a/noir-projects/noir-contracts/contracts/claim_contract/src/main.nr b/noir-projects/noir-contracts/contracts/claim_contract/src/main.nr index c86fc04e8da..7fac0994785 100644 --- a/noir-projects/noir-contracts/contracts/claim_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/claim_contract/src/main.nr @@ -24,12 +24,13 @@ contract Claim { } #[aztec(private)] - fn claim(proof_note: ValueNote) { - // 1) Check that the note corresponds to the target contract + fn claim(proof_note: ValueNote, recipient: AztecAddress) { + // 1) Check that the note corresponds to the target contract and belongs to the sender let target_address = storage.target_contract.read_private(); assert( target_address == proof_note.header.contract_address, "Note does not correspond to the target contract" ); + assert_eq(proof_note.owner, context.msg_sender(), "Note does not belong to the sender"); // 2) Prove that the note hash exists in the note hash tree prove_note_inclusion(proof_note, context); @@ -42,6 +43,6 @@ contract Claim { // 4) Finally we mint the reward token to the sender of the transaction let reward_token = Token::at(storage.reward_token.read_private()); - reward_token.mint_public(&mut context, context.msg_sender(), proof_note.value); + reward_token.mint_public(&mut context, recipient, proof_note.value); } } diff --git a/yarn-project/end-to-end/src/e2e_crowdfunding_and_claim.test.ts b/yarn-project/end-to-end/src/e2e_crowdfunding_and_claim.test.ts index 7b95950f124..1f2bd159972 100644 --- a/yarn-project/end-to-end/src/e2e_crowdfunding_and_claim.test.ts +++ b/yarn-project/end-to-end/src/e2e_crowdfunding_and_claim.test.ts @@ -9,6 +9,7 @@ import { Note, PXE, TxHash, + computeAuthWitMessageHash, computeMessageSecretHash, generatePublicKey, } from '@aztec/aztec.js'; @@ -223,7 +224,11 @@ describe('e2e_crowdfunding_and_claim', () => { // 3) We claim the reward token via the Claim contract { - await claimContract.withWallet(donorWallets[0]).methods.claim(valueNote).send().wait(); + await claimContract + .withWallet(donorWallets[0]) + .methods.claim(valueNote, donorWallets[0].getAddress()) + .send() + .wait(); } // Since the RWT is minted 1:1 with the DNT, the balance of the reward token should be equal to the donation amount @@ -248,7 +253,51 @@ describe('e2e_crowdfunding_and_claim', () => { it('cannot claim twice', async () => { // The first claim was executed in the previous test - await expect(claimContract.withWallet(donorWallets[0]).methods.claim(valueNote).send().wait()).rejects.toThrow(); + await expect( + claimContract.withWallet(donorWallets[0]).methods.claim(valueNote, donorWallets[0].getAddress()).send().wait(), + ).rejects.toThrow(); + }); + + it('cannot claim with a different address than the one that donated', async () => { + const donationAmount = 1000n; + { + const action = donationToken + .withWallet(donorWallets[1]) + .methods.transfer(donorWallets[1].getAddress(), crowdfundingContract.address, donationAmount, 0); + const messageHash = computeAuthWitMessageHash(crowdfundingContract.address, action.request()); + const witness = await donorWallets[1].createAuthWit(messageHash); + await donorWallets[1].addAuthWitness(witness); + } + + // 2) We donate to the crowdfunding contract + + const donateTxReceipt = await crowdfundingContract + .withWallet(donorWallets[1]) + .methods.donate(donationAmount) + .send() + .wait({ + debug: true, + }); + + // Get the notes emitted by the Crowdfunding contract and check that only 1 was emitted (the value note) + const notes = donateTxReceipt.debugInfo?.visibleNotes.filter(x => + x.contractAddress.equals(crowdfundingContract.address), + ); + expect(notes!.length).toEqual(1); + + // Set the value note in a format which can be passed to claim function + const anotherDonationNote = await processExtendedNote(notes![0]); + + // 3) We claim the reward token via the Claim contract + { + await expect( + claimContract + .withWallet(donorWallets[0]) + .methods.claim(anotherDonationNote, donorWallets[1].getAddress()) + .send() + .wait(), + ).rejects.toThrow('Note does not belong to the sender'); + } }); it('cannot claim with a non-existent note', async () => { @@ -257,7 +306,11 @@ describe('e2e_crowdfunding_and_claim', () => { nonExistentNote.randomness = Fr.random(); await expect( - claimContract.withWallet(donorWallets[0]).methods.claim(nonExistentNote).send().wait(), + claimContract + .withWallet(donorWallets[0]) + .methods.claim(nonExistentNote, donorWallets[0].getAddress()) + .send() + .wait(), ).rejects.toThrow(); }); @@ -280,7 +333,9 @@ describe('e2e_crowdfunding_and_claim', () => { await inclusionsProofsContract.methods.test_note_inclusion(owner, false, 0n, true).send().wait(); // 4) Finally, check that the claim process fails - await expect(claimContract.withWallet(donorWallets[0]).methods.claim(note).send().wait()).rejects.toThrow(); + await expect( + claimContract.withWallet(donorWallets[0]).methods.claim(note, donorWallets[0].getAddress()).send().wait(), + ).rejects.toThrow(); }); it('cannot donate after a deadline', async () => {