From a99292574e5424f7977369499ea6e13770533bbe Mon Sep 17 00:00:00 2001 From: benesjan Date: Mon, 19 May 2025 10:45:20 +0000 Subject: [PATCH 1/7] docs: comment on partial note resuse --- .../contracts/app/nft_contract/src/main.nr | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/noir-projects/noir-contracts/contracts/app/nft_contract/src/main.nr b/noir-projects/noir-contracts/contracts/app/nft_contract/src/main.nr index bd16eb5c29a0..4becdd79aa98 100644 --- a/noir-projects/noir-contracts/contracts/app/nft_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/app/nft_contract/src/main.nr @@ -237,9 +237,15 @@ pub contract NFT { context.storage_write(validity_commitment, true); } // docs:end:store_payload_in_transient_storage_unsafe + /// Finalizes a transfer of NFT with `token_id` from public balance of `msg_sender` to a private balance of `to`. /// The transfer must be prepared by calling `prepare_private_balance_increase` from `msg_sender` account and the /// resulting `partial_note` must be passed as an argument to this function. + /// + /// Note that this contract does not protect against a `partial_note` being used multiple times and it is up to + /// the developer to ensure that it doesn't happen. If the same `partial_note` is used multiple times, the NFT + /// with `token_id` would most likely get lost (the partial note log processing functionality would fail to find + /// the pending partial note when trying to complete it). // docs:start:finalize_transfer_to_private #[public] fn finalize_transfer_to_private(token_id: Field, partial_note: PartialNFTNote) { @@ -296,7 +302,9 @@ pub contract NFT { // We verify that the partial note we're completing is valid (i.e. completer is correct, it uses the correct // state variable's storage slot, and it is internally consistent). We *could* clear the storage since each // partial note should only be used once, but since the AVM offers no gas refunds for doing so this would just - // make the transaction be more expensive. + // make the transaction be more expensive. We don't worry about the validity commitment being checked against multiple + // times because it is up to the developer to ensure that the same partial note is not used multiple times (see + // comment of `finalize_transfer_to_private`). assert( context.storage_read(partial_note.compute_validity_commitment(from_and_completer)), "Invalid partial note or completer", From 2c2f9e7b9dece8c6cf047a7a26af4fa202f837b0 Mon Sep 17 00:00:00 2001 From: benesjan Date: Mon, 19 May 2025 10:47:19 +0000 Subject: [PATCH 2/7] wip --- .../noir-contracts/contracts/app/nft_contract/src/main.nr | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/noir-projects/noir-contracts/contracts/app/nft_contract/src/main.nr b/noir-projects/noir-contracts/contracts/app/nft_contract/src/main.nr index 4becdd79aa98..7474930485bf 100644 --- a/noir-projects/noir-contracts/contracts/app/nft_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/app/nft_contract/src/main.nr @@ -302,9 +302,9 @@ pub contract NFT { // We verify that the partial note we're completing is valid (i.e. completer is correct, it uses the correct // state variable's storage slot, and it is internally consistent). We *could* clear the storage since each // partial note should only be used once, but since the AVM offers no gas refunds for doing so this would just - // make the transaction be more expensive. We don't worry about the validity commitment being checked against multiple - // times because it is up to the developer to ensure that the same partial note is not used multiple times (see - // comment of `finalize_transfer_to_private`). + // make the transaction be more expensive. We don't worry about the validity commitment being checked against + // multiple times because it is up to the developer to ensure that a partial note is used only once (see + // function docs of `finalize_transfer_to_private`). assert( context.storage_read(partial_note.compute_validity_commitment(from_and_completer)), "Invalid partial note or completer", From f94b809b960420b6412e045f03a9add5baba5473 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Bene=C5=A1?= Date: Tue, 20 May 2025 09:39:00 +0200 Subject: [PATCH 3/7] Update noir-projects/noir-contracts/contracts/app/nft_contract/src/main.nr MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Nicolás Venturo --- .../noir-contracts/contracts/app/nft_contract/src/main.nr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/noir-projects/noir-contracts/contracts/app/nft_contract/src/main.nr b/noir-projects/noir-contracts/contracts/app/nft_contract/src/main.nr index 7474930485bf..731a031d946c 100644 --- a/noir-projects/noir-contracts/contracts/app/nft_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/app/nft_contract/src/main.nr @@ -303,7 +303,7 @@ pub contract NFT { // state variable's storage slot, and it is internally consistent). We *could* clear the storage since each // partial note should only be used once, but since the AVM offers no gas refunds for doing so this would just // make the transaction be more expensive. We don't worry about the validity commitment being checked against - // multiple times because it is up to the developer to ensure that a partial note is used only once (see + // multiple times because it is up to the caller to ensure that a partial note is used only once (see // function docs of `finalize_transfer_to_private`). assert( context.storage_read(partial_note.compute_validity_commitment(from_and_completer)), From 2f7fc6d6ac8ca89a22efa9e2a46565da2b7c186e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Bene=C5=A1?= Date: Tue, 20 May 2025 09:39:13 +0200 Subject: [PATCH 4/7] Update noir-projects/noir-contracts/contracts/app/nft_contract/src/main.nr MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Nicolás Venturo --- .../noir-contracts/contracts/app/nft_contract/src/main.nr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/noir-projects/noir-contracts/contracts/app/nft_contract/src/main.nr b/noir-projects/noir-contracts/contracts/app/nft_contract/src/main.nr index 731a031d946c..7d7cd517d780 100644 --- a/noir-projects/noir-contracts/contracts/app/nft_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/app/nft_contract/src/main.nr @@ -243,7 +243,7 @@ pub contract NFT { /// resulting `partial_note` must be passed as an argument to this function. /// /// Note that this contract does not protect against a `partial_note` being used multiple times and it is up to - /// the developer to ensure that it doesn't happen. If the same `partial_note` is used multiple times, the NFT + /// the caller of this function to ensure that it doesn't happen. If the same `partial_note` is used multiple times, the NFT /// with `token_id` would most likely get lost (the partial note log processing functionality would fail to find /// the pending partial note when trying to complete it). // docs:start:finalize_transfer_to_private From 72240be8e918ca3872cf3a63f0f5858171dabffe Mon Sep 17 00:00:00 2001 From: benesjan Date: Tue, 20 May 2025 07:41:45 +0000 Subject: [PATCH 5/7] respecting 120 line width --- .../contracts/app/nft_contract/src/main.nr | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/noir-projects/noir-contracts/contracts/app/nft_contract/src/main.nr b/noir-projects/noir-contracts/contracts/app/nft_contract/src/main.nr index 7d7cd517d780..bc8472ef9865 100644 --- a/noir-projects/noir-contracts/contracts/app/nft_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/app/nft_contract/src/main.nr @@ -243,9 +243,9 @@ pub contract NFT { /// resulting `partial_note` must be passed as an argument to this function. /// /// Note that this contract does not protect against a `partial_note` being used multiple times and it is up to - /// the caller of this function to ensure that it doesn't happen. If the same `partial_note` is used multiple times, the NFT - /// with `token_id` would most likely get lost (the partial note log processing functionality would fail to find - /// the pending partial note when trying to complete it). + /// the caller of this function to ensure that it doesn't happen. If the same `partial_note` is used multiple + /// times, the NFT with `token_id` would most likely get lost (the partial note log processing functionality + /// would fail to find the pending partial note when trying to complete it). // docs:start:finalize_transfer_to_private #[public] fn finalize_transfer_to_private(token_id: Field, partial_note: PartialNFTNote) { @@ -303,8 +303,8 @@ pub contract NFT { // state variable's storage slot, and it is internally consistent). We *could* clear the storage since each // partial note should only be used once, but since the AVM offers no gas refunds for doing so this would just // make the transaction be more expensive. We don't worry about the validity commitment being checked against - // multiple times because it is up to the caller to ensure that a partial note is used only once (see - // function docs of `finalize_transfer_to_private`). + // multiple times because it is up to the caller to ensure that a partial note is used only once (see function + // docs of `finalize_transfer_to_private`). assert( context.storage_read(partial_note.compute_validity_commitment(from_and_completer)), "Invalid partial note or completer", From 82b4de45f5b383df98ecfd1ab4028c228dcf0804 Mon Sep 17 00:00:00 2001 From: benesjan Date: Tue, 20 May 2025 07:46:05 +0000 Subject: [PATCH 6/7] comment --- .../noir-contracts/contracts/app/token_contract/src/main.nr | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/noir-projects/noir-contracts/contracts/app/token_contract/src/main.nr b/noir-projects/noir-contracts/contracts/app/token_contract/src/main.nr index 8f19b1498fe2..7a67c8b6b379 100644 --- a/noir-projects/noir-contracts/contracts/app/token_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/app/token_contract/src/main.nr @@ -468,6 +468,11 @@ pub contract Token { /// Finalizes a transfer of token `amount` from public balance of `msg_sender` to a private balance of `to`. /// The transfer must be prepared by calling `prepare_private_balance_increase` from `msg_sender` account and /// the resulting `partial_note` must be passed as an argument to this function. + /// + /// Note that this contract does not protect against a `partial_note` being used multiple times and it is up to + /// the caller of this function to ensure that it doesn't happen. If the same `partial_note` is used multiple + /// times, the token `amount` would most likely get lost (the partial note log processing functionality would fail + /// to find the pending partial note when trying to complete it). #[public] fn finalize_transfer_to_private(amount: u128, partial_note: PartialUintNote) { // Completer is the entity that can complete the partial note. In this case, it's the same as the account From 41d6ed2744c4ef0080d1b9b9943afcbde1932e6f Mon Sep 17 00:00:00 2001 From: benesjan Date: Wed, 21 May 2025 07:30:43 +0000 Subject: [PATCH 7/7] WIP --- .../noir-contracts/contracts/app/token_contract/src/main.nr | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/noir-projects/noir-contracts/contracts/app/token_contract/src/main.nr b/noir-projects/noir-contracts/contracts/app/token_contract/src/main.nr index 7a67c8b6b379..4320eaef86c3 100644 --- a/noir-projects/noir-contracts/contracts/app/token_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/app/token_contract/src/main.nr @@ -526,8 +526,10 @@ pub contract Token { // We verify that the partial note we're completing is valid (i.e. completer is correct, it uses the correct // state variable's storage slot, and it is internally consistent). We *could* clear the storage since each - // partial note should only be used once, but since the AVM offers no gas refunds for doing so this would - // just make the transaction be more expensive. + // partial note should only be used once, but since the AVM offers no gas refunds for doing so this would just + // make the transaction be more expensive. We don't worry about the validity commitment being checked against + // multiple times because it is up to the caller to ensure that a partial note is used only once (see function + // docs of `finalize_transfer_to_private`). assert( context.storage_read(partial_note.compute_validity_commitment(from_and_completer)), "Invalid partial note or completer",