From 6eee02b63b1ee57113f690ddd40bcd128caf2e82 Mon Sep 17 00:00:00 2001 From: iamalwaysuncomfortable Date: Fri, 13 May 2022 12:38:36 -0400 Subject: [PATCH 01/12] Refactor memo builder into separate memo builders --- transaction/core/src/memo.rs | 2 +- transaction/core/src/tx_error.rs | 27 +++- transaction/std/src/lib.rs | 7 +- .../gift_code_cancellation_memo_builder.rs | 99 +++++++++++++ .../gift_code_funding_memo_builder.rs | 136 ++++++++++++++++++ .../gift_code_sender_memo_builder.rs | 100 +++++++++++++ transaction/std/src/memo_builder/mod.rs | 7 + 7 files changed, 373 insertions(+), 5 deletions(-) create mode 100644 transaction/std/src/memo_builder/gift_code_cancellation_memo_builder.rs create mode 100644 transaction/std/src/memo_builder/gift_code_funding_memo_builder.rs create mode 100644 transaction/std/src/memo_builder/gift_code_sender_memo_builder.rs diff --git a/transaction/core/src/memo.rs b/transaction/core/src/memo.rs index 3943b09e60..1a9d7ab991 100644 --- a/transaction/core/src/memo.rs +++ b/transaction/core/src/memo.rs @@ -223,7 +223,7 @@ derive_serde_from_repr_bytes!(MemoPayload); derive_prost_message_from_repr_bytes!(MemoPayload); /// An error which can occur when handling memos -#[derive(Display, Debug)] +#[derive(Debug, Display, PartialEq, Eq)] pub enum MemoError { /// Wrong length for memo payload: {0} BadLength(usize), diff --git a/transaction/core/src/tx_error.rs b/transaction/core/src/tx_error.rs index 29e5c0162c..b2ecfec35c 100644 --- a/transaction/core/src/tx_error.rs +++ b/transaction/core/src/tx_error.rs @@ -2,8 +2,8 @@ //! Errors that can occur when creating a new TxOut -use crate::AmountError; -use alloc::string::String; +use crate::{AmountError, MemoError}; +use alloc::{format, string::String}; use displaydoc::Display; use mc_crypto_keys::KeyError; @@ -71,8 +71,31 @@ pub enum NewMemoError { MultipleOutputs, /// Missing output MissingOutput, + /// Missing required input to build the memo: {0} + MissingInput(String), /// Mixed Token Ids are not supported in these memos MixedTokenIds, + /// Destination memo is not supported + DestinationMemoNotAllowed, + /// Improperly configured input: {0} + BadInputs(String), + /// Creation + Creation(MemoError), + /// Utf-8 did not properly decode + Utf8Decoding, /// Other: {0} Other(String), } + +impl From for NewMemoError { + fn from (src: MemoError) -> Self { + match src { + MemoError::Utf8Decoding => Self::Utf8Decoding, + MemoError::BadLength(byte_len) => { + Self::BadInputs( + format!("Input of length: {} exceeded max byte length", byte_len) + ) + } + } + } +} \ No newline at end of file diff --git a/transaction/std/src/lib.rs b/transaction/std/src/lib.rs index c83e23c4a8..83be69d35d 100644 --- a/transaction/std/src/lib.rs +++ b/transaction/std/src/lib.rs @@ -23,10 +23,13 @@ pub use error::{SignedContingentInputBuilderError, TxBuilderError}; pub use input_credentials::InputCredentials; pub use memo::{ AuthenticatedSenderMemo, AuthenticatedSenderWithPaymentRequestIdMemo, BurnRedemptionMemo, - DestinationMemo, DestinationMemoError, MemoDecodingError, MemoType, RegisteredMemoType, + DestinationMemo, DestinationMemoError, GiftCodeCancellationMemo, GiftCodeFundingMemo, + GiftCodeSenderMemo, MemoDecodingError, MemoType, RegisteredMemoType, SenderMemoCredential, UnusedMemo, }; -pub use memo_builder::{BurnRedemptionMemoBuilder, EmptyMemoBuilder, MemoBuilder, RTHMemoBuilder}; +pub use memo_builder::{BurnRedemptionMemoBuilder, EmptyMemoBuilder, + GiftCodeCancellationMemoBuilder, GiftCodeFundingMemoBuilder, + GiftCodeSenderMemoBuilder, MemoBuilder, RTHMemoBuilder}; pub use reserved_destination::ReservedDestination; pub use signed_contingent_input_builder::SignedContingentInputBuilder; pub use transaction_builder::{DefaultTxOutputsOrdering, TransactionBuilder, TxOutputsOrdering}; diff --git a/transaction/std/src/memo_builder/gift_code_cancellation_memo_builder.rs b/transaction/std/src/memo_builder/gift_code_cancellation_memo_builder.rs new file mode 100644 index 0000000000..14436ed99c --- /dev/null +++ b/transaction/std/src/memo_builder/gift_code_cancellation_memo_builder.rs @@ -0,0 +1,99 @@ +// Copyright (c) 2018-2022 The MobileCoin Foundation + +//! Defines the Memo Builder for the gift code cancellation memo (0x0202) +//! specified in MCIP #32 + +use super::{memo::{UnusedMemo, GiftCodeCancellationMemo}, ReservedDestination, MemoBuilder}; +use mc_account_keys::PublicAddress; +use mc_transaction_core::{Amount, MemoContext, MemoPayload, NewMemoError}; + +/// There are three possible gift code memo types specified in MCIP #32 +/// | Memo type bytes | Name | +/// | ----------- | ----------- | +/// | 0x0002 | Gift Code Sender Memo | +/// | 0x0201 | Gift Code Funding Memo | +/// | -->0x0202<-- | Gift Code Cancellation Memo | +/// This memo builder builds a gift code cancellation memo (0x0202). Gift code +/// cancellation is defined as the sender sending the gift code TxOut at the +/// gift code subaddress back to their default address prior to it being spent +/// by the receiver. When that happens a zero valued TxOut is sent to the gift +/// code sender's change subaddress with a gift code cancellation memo that +/// stores the index of the TxOut originally sent to the gift code subaddress +/// when the gift code was funded. +#[derive(Clone, Debug)] +pub struct GiftCodeCancellationMemoBuilder { + // Index of the gift code TxOut that was originally funded + gift_code_tx_out_global_index: Option, + // Whether or not to enable change memos + gift_code_change_memo_enabled: bool, + // Whether we've already written the change memo + wrote_change_memo: bool, +} + +impl GiftCodeCancellationMemoBuilder { + /// Create a new cancellation gift code memo builder + pub fn new() -> Self { + Self { + gift_code_tx_out_global_index: None, + gift_code_change_memo_enabled: true, + wrote_change_memo: false, + } + } + /// Set the index of the gift code TxOut that was cancelled + pub fn set_gift_code_tx_out_index(&mut self, tx_out_global_index: u64) { + self.gift_code_tx_out_global_index = Some(tx_out_global_index) + } + /// Clear the index + pub fn clear_gift_code_tx_out_index(&mut self) { + self.gift_code_tx_out_global_index = None; + } + /// Enable change memos + pub fn enable_gift_code_change_memo(&mut self) { + self.gift_code_change_memo_enabled = true; + } + /// Disable change memos + pub fn disable_gift_code_change_memo(&mut self) { + self.gift_code_change_memo_enabled = false; + } +} + +impl MemoBuilder for GiftCodeCancellationMemoBuilder { + /// Set the fee + fn set_fee(&mut self, _fee: Amount) -> Result<(), NewMemoError> { + Ok(()) + } + + /// Gift code destination memos are not allowed - all gift code + /// memos accompany TxOuts sent to the change address + fn make_memo_for_output( + &mut self, + _amount: Amount, + _recipient: &PublicAddress, + _memo_context: MemoContext, + ) -> Result { + Err(NewMemoError::DestinationMemoNotAllowed) + } + + /// Build a memo for a gift code change output + fn make_memo_for_change_output( + &mut self, + _amount: Amount, + _change_destination: &ReservedDestination, + _memo_context: MemoContext, + ) -> Result { + if !self.gift_code_change_memo_enabled { + return Ok(UnusedMemo {}.into()); + } + if self.wrote_change_memo { + return Err(NewMemoError::MultipleChangeOutputs); + } + self.wrote_change_memo = true; + if let Some(tx_out_global_index) = self.gift_code_tx_out_global_index.take() { + return Ok(GiftCodeCancellationMemo::from(tx_out_global_index).into()); + } else { + return Err(NewMemoError::MissingInput( + "Missing global index of TxOut to be cancelled".into(), + )); + } + } +} diff --git a/transaction/std/src/memo_builder/gift_code_funding_memo_builder.rs b/transaction/std/src/memo_builder/gift_code_funding_memo_builder.rs new file mode 100644 index 0000000000..99997ff392 --- /dev/null +++ b/transaction/std/src/memo_builder/gift_code_funding_memo_builder.rs @@ -0,0 +1,136 @@ +// Copyright (c) 2018-2022 The MobileCoin Foundation + +//! Defines the Memo Builder for the gift code funding memo (0x0201) +//! specified in MCIP #32 + +use crate::{memo::{UnusedMemo, GiftCodeFundingMemo}, MemoBuilder, ReservedDestination}; +use mc_account_keys::PublicAddress; +use mc_crypto_keys::RistrettoPublic; +use mc_transaction_core::{Amount, MemoContext, MemoPayload, NewMemoError}; + +/// There are three possible gift code memo types specified in MCIP #32 +/// | Memo type bytes | Name | +/// | ----------- | ----------- | +/// | 0x0002 | Gift Code Sender Memo | +/// | -->0x0201<-- | Gift Code Funding Memo | +/// | 0x0202 | Gift Code Cancellation Memo | +/// This memo builder builds a gift code funding memo (0x0201). When a gift +/// code is funded, the amount of the gift code is sent to a TxOut at the +/// Sender's reserved gift code subaddress and a second zero valued TxOut +/// is sent to the sender's reserved change subaddress with the gift code +/// funding memo attached. The funding memo will include the first 4 bytes +/// of the hash of the gift code TxOut sent to the sender's reserved gift +/// code subaddress and 60 bytes for an optional utf-8 memo. +/// +/// IMPORTANT NOTE: The public_key of the zero valued TxOut that the Gift Code +/// Funding Memo is written to is NOT the public_key that should be passed into +/// set_gift_code_tx_out_public_key(tx_out_public_key). Instead the public_key +/// of the TxOut sent to the gift code subaddress is what should be passed into +/// set_gift_code_tx_out_public_key(tx_out_public_key) +#[derive(Clone, Debug)] +pub struct GiftCodeFundingMemoBuilder { + // Utf-8 note within the memo that can be up to 60 bytes long + note: String, + // TxOut Public Key of the gift code TxOut sent to the gift code subaddress + gift_code_tx_out_public_key: Option, + // Whether or not to enable change memo + gift_code_change_memo_enabled: bool, + // Whether we've already written the change memo + wrote_change_memo: bool, +} + +impl GiftCodeFundingMemoBuilder { + /// Create a new gift code funding memo builder + pub fn new() -> Self { + Self { + note: "".into(), + gift_code_tx_out_public_key: None, + gift_code_change_memo_enabled: true, + wrote_change_memo: false, + } + } + /// Set a utf-8 note (up to 60 bytes) onto the funding memo indicating + /// what the gift code was for. This method will enforce the 60 byte + /// limit with a NewMemoErr if the &str passed is longer than + /// 60 bytes. + pub fn set_funding_note(&mut self, note: &str) -> Result<(), NewMemoError> { + if note.len() > GiftCodeFundingMemo::NOTE_DATA_LEN { + return Err(NewMemoError::BadInputs("Note memo cannot be greater than 60 bytes".into())) + } + self.note = note.into(); + Ok(()) + } + /// Clear the gift code funding note + pub fn clear_funding_note(&mut self) { + self.note = "".into(); + } + /// Set the TxOut public_key of the gift code TxOut sent to the + /// reserved gift code subaddress. + /// + /// IMPORTANT NOTE: Do NOT pass the public_key of the zero valued change + /// TxOut that the gift code memo is attached to as an argument. Doing + /// so will result in an error when attempting to build the memo. + pub fn set_gift_code_tx_out_public_key( + &mut self, + tx_out_public_key: RistrettoPublic, + ) -> Result<(), NewMemoError> { + self.gift_code_tx_out_public_key = Some(tx_out_public_key); + Ok(()) + } + /// Clear the gift code tx_out_public_key + pub fn clear_gift_code_tx_out_public_key(&mut self) { + self.gift_code_tx_out_public_key = None; + } + /// Enable change memos + pub fn enable_gift_code_change_memo(&mut self) { + self.gift_code_change_memo_enabled = true; + } + /// Disable change memos + pub fn disable_gift_code_change_memo(&mut self) { + self.gift_code_change_memo_enabled = false; + } +} + +impl MemoBuilder for GiftCodeFundingMemoBuilder { + /// Set the fee + fn set_fee(&mut self, _fee: Amount) -> Result<(), NewMemoError> { + Ok(()) + } + + /// Gift code destination memos are not allowed - all gift code + /// memos accompany TxOuts sent to the change address + fn make_memo_for_output( + &mut self, + _amount: Amount, + _recipient: &PublicAddress, + _memo_context: MemoContext, + ) -> Result { + Err(NewMemoError::DestinationMemoNotAllowed) + } + + /// Build a memo for a gift code change output + fn make_memo_for_change_output( + &mut self, + _amount: Amount, + _change_destination: &ReservedDestination, + memo_context: MemoContext, + ) -> Result { + if !self.gift_code_change_memo_enabled { + return Ok(UnusedMemo {}.into()); + } + if self.wrote_change_memo { + return Err(NewMemoError::MultipleChangeOutputs); + } + if self.gift_code_tx_out_public_key.as_ref() == Some(memo_context.tx_public_key) { + return Err(NewMemoError::BadInputs("The public_key in the memo should be the TxOut at the gift code subaddress and not that of the memo TxOut".into())); + } + if let Some(tx_out_public_key) = self.gift_code_tx_out_public_key.take() { + self.wrote_change_memo = true; + return Ok(GiftCodeFundingMemo::new(&tx_out_public_key, self.note.as_str())?.into()); + } else { + return Err(NewMemoError::MissingInput( + "Missing gift code TxOut public key".into(), + )); + } + } +} diff --git a/transaction/std/src/memo_builder/gift_code_sender_memo_builder.rs b/transaction/std/src/memo_builder/gift_code_sender_memo_builder.rs new file mode 100644 index 0000000000..a892ee3bb4 --- /dev/null +++ b/transaction/std/src/memo_builder/gift_code_sender_memo_builder.rs @@ -0,0 +1,100 @@ +// Copyright (c) 2018-2022 The MobileCoin Foundation + +//! Defines the Memo Builder for the gift code sender memo (0x0002) +//! specified in MCIP #32 + +use crate::{memo::{GiftCodeSenderMemo, UnusedMemo}, MemoBuilder, ReservedDestination}; +use mc_account_keys::PublicAddress; +use mc_transaction_core::{Amount, MemoContext, MemoPayload, NewMemoError}; + +/// There are three possible gift code memo types specified in MCIP #32 +/// | Memo type bytes | Name | +/// | ----------- | ----------- | +/// | -->0x0002<-- | Gift Code Sender Memo | +/// | 0x0201 | Gift Code Funding Memo | +/// | 0x0202 | Gift Code Cancellation Memo | +/// This memo builder builds a gift code sender memo (0x0002). A gift code +/// considered redeemed when the Receiver uses the TxOut spend private key +/// of the gift code TxOut they received from the Sender to send the TxOut +/// from the sender's gift code subaddress to their own change subaddress. +/// The destination memo is written into that TxOut at the change address +/// and includes an optional Utf-8 note up to 64 bytes long the Receiver +/// can use to record any information they desire about the gift code. +#[derive(Clone, Debug)] +pub struct GiftCodeSenderMemoBuilder { + // Utf-8 formatted note within the memo that can be up to 64 bytes long + note: String, + // Whether or not to enable change memos + gift_code_change_memo_enabled: bool, + // Whether we've already written the change memo + wrote_change_memo: bool, +} + +impl GiftCodeSenderMemoBuilder { + /// Create a new gift code sender memo builder + pub fn new() -> Self { + Self { + note: "".into(), + gift_code_change_memo_enabled: true, + wrote_change_memo: false, + } + } + /// Set a utf-8 note (up to 64 bytes) onto the sender memo indicating + /// any desired info about the gift code. This method will enforce the + /// 64 byte limit with a NewMemoErr if the &str passed is longer than + /// 64 bytes. + pub fn set_gift_code_sender_note(&mut self, note: &str) -> Result<(), NewMemoError> { + if note.len() > GiftCodeSenderMemo::MEMO_DATA_LEN { + return Err(NewMemoError::BadInputs("Sender note cannot be longer than 64 bytes".into())); + } + self.note = note.into(); + Ok(()) + } + /// Clear the gift code sender note + pub fn clear_sender_note(&mut self) { + self.note = "".into(); + } + /// Enable change memos + pub fn enable_gift_code_change_memo(&mut self) { + self.gift_code_change_memo_enabled = true; + } + /// Disable change memos + pub fn disable_gift_code_change_memo(&mut self) { + self.gift_code_change_memo_enabled = false; + } +} + +impl MemoBuilder for GiftCodeSenderMemoBuilder { + /// Set the fee + fn set_fee(&mut self, _fee: Amount) -> Result<(), NewMemoError> { + Ok(()) + } + + /// Gift code destination memos are not allowed - all gift code + /// memos accompany TxOuts sent to the change address + fn make_memo_for_output( + &mut self, + _amount: Amount, + _recipient: &PublicAddress, + _memo_context: MemoContext, + ) -> Result { + Err(NewMemoError::DestinationMemoNotAllowed) + } + + /// Build a memo for a gift code change output + fn make_memo_for_change_output( + &mut self, + _amount: Amount, + _change_destination: &ReservedDestination, + _memo_context: MemoContext, + ) -> Result { + if !self.gift_code_change_memo_enabled { + return Ok(UnusedMemo {}.into()); + }; + if self.wrote_change_memo { + return Err(NewMemoError::MultipleChangeOutputs); + }; + self.wrote_change_memo = true; + Ok(GiftCodeSenderMemo::new(self.note.as_str())?.into()) + } +} diff --git a/transaction/std/src/memo_builder/mod.rs b/transaction/std/src/memo_builder/mod.rs index f9f04942f6..8e6dd64a40 100644 --- a/transaction/std/src/memo_builder/mod.rs +++ b/transaction/std/src/memo_builder/mod.rs @@ -10,9 +10,16 @@ use mc_account_keys::PublicAddress; use mc_transaction_core::{Amount, MemoContext, MemoPayload, NewMemoError}; mod burn_redemption_memo_builder; +mod gift_code_cancellation_memo_builder; +mod gift_code_funding_memo_builder; +mod gift_code_sender_memo_builder; mod rth_memo_builder; + pub use burn_redemption_memo_builder::BurnRedemptionMemoBuilder; +pub use gift_code_cancellation_memo_builder::GiftCodeCancellationMemoBuilder; +pub use gift_code_funding_memo_builder::GiftCodeFundingMemoBuilder; +pub use gift_code_sender_memo_builder::GiftCodeSenderMemoBuilder; pub use rth_memo_builder::RTHMemoBuilder; /// The MemoBuilder trait defines the API that the transaction builder uses From bc5267990c9a9987f21b760018445930451c7b72 Mon Sep 17 00:00:00 2001 From: iamalwaysuncomfortable Date: Fri, 13 May 2022 14:03:56 -0400 Subject: [PATCH 02/12] Lint fix --- transaction/core/src/tx_error.rs | 13 ++++----- transaction/std/src/lib.rs | 11 ++++---- .../gift_code_cancellation_memo_builder.rs | 14 +++++++++- .../gift_code_funding_memo_builder.rs | 28 ++++++++++++++----- .../gift_code_sender_memo_builder.rs | 18 ++++++++++-- transaction/std/src/memo_builder/mod.rs | 1 - 6 files changed, 62 insertions(+), 23 deletions(-) diff --git a/transaction/core/src/tx_error.rs b/transaction/core/src/tx_error.rs index b2ecfec35c..3ebbb77c93 100644 --- a/transaction/core/src/tx_error.rs +++ b/transaction/core/src/tx_error.rs @@ -88,14 +88,13 @@ pub enum NewMemoError { } impl From for NewMemoError { - fn from (src: MemoError) -> Self { + fn from(src: MemoError) -> Self { match src { MemoError::Utf8Decoding => Self::Utf8Decoding, - MemoError::BadLength(byte_len) => { - Self::BadInputs( - format!("Input of length: {} exceeded max byte length", byte_len) - ) - } + MemoError::BadLength(byte_len) => Self::BadInputs(format!( + "Input of length: {} exceeded max byte length", + byte_len + )), } } -} \ No newline at end of file +} diff --git a/transaction/std/src/lib.rs b/transaction/std/src/lib.rs index 83be69d35d..9b2212bfac 100644 --- a/transaction/std/src/lib.rs +++ b/transaction/std/src/lib.rs @@ -24,12 +24,13 @@ pub use input_credentials::InputCredentials; pub use memo::{ AuthenticatedSenderMemo, AuthenticatedSenderWithPaymentRequestIdMemo, BurnRedemptionMemo, DestinationMemo, DestinationMemoError, GiftCodeCancellationMemo, GiftCodeFundingMemo, - GiftCodeSenderMemo, MemoDecodingError, MemoType, RegisteredMemoType, - SenderMemoCredential, UnusedMemo, + GiftCodeSenderMemo, MemoDecodingError, MemoType, RegisteredMemoType, SenderMemoCredential, + UnusedMemo, +}; +pub use memo_builder::{ + BurnRedemptionMemoBuilder, EmptyMemoBuilder, GiftCodeCancellationMemoBuilder, + GiftCodeFundingMemoBuilder, GiftCodeSenderMemoBuilder, MemoBuilder, RTHMemoBuilder, }; -pub use memo_builder::{BurnRedemptionMemoBuilder, EmptyMemoBuilder, - GiftCodeCancellationMemoBuilder, GiftCodeFundingMemoBuilder, - GiftCodeSenderMemoBuilder, MemoBuilder, RTHMemoBuilder}; pub use reserved_destination::ReservedDestination; pub use signed_contingent_input_builder::SignedContingentInputBuilder; pub use transaction_builder::{DefaultTxOutputsOrdering, TransactionBuilder, TxOutputsOrdering}; diff --git a/transaction/std/src/memo_builder/gift_code_cancellation_memo_builder.rs b/transaction/std/src/memo_builder/gift_code_cancellation_memo_builder.rs index 14436ed99c..a51925febd 100644 --- a/transaction/std/src/memo_builder/gift_code_cancellation_memo_builder.rs +++ b/transaction/std/src/memo_builder/gift_code_cancellation_memo_builder.rs @@ -3,7 +3,10 @@ //! Defines the Memo Builder for the gift code cancellation memo (0x0202) //! specified in MCIP #32 -use super::{memo::{UnusedMemo, GiftCodeCancellationMemo}, ReservedDestination, MemoBuilder}; +use super::{ + memo::{GiftCodeCancellationMemo, UnusedMemo}, + MemoBuilder, ReservedDestination, +}; use mc_account_keys::PublicAddress; use mc_transaction_core::{Amount, MemoContext, MemoPayload, NewMemoError}; @@ -97,3 +100,12 @@ impl MemoBuilder for GiftCodeCancellationMemoBuilder { } } } + +mod tests { + use super::*; + + #[test] + fn test_gift_code() { + // Tests forthcoming + } +} diff --git a/transaction/std/src/memo_builder/gift_code_funding_memo_builder.rs b/transaction/std/src/memo_builder/gift_code_funding_memo_builder.rs index 99997ff392..70986484bd 100644 --- a/transaction/std/src/memo_builder/gift_code_funding_memo_builder.rs +++ b/transaction/std/src/memo_builder/gift_code_funding_memo_builder.rs @@ -3,7 +3,10 @@ //! Defines the Memo Builder for the gift code funding memo (0x0201) //! specified in MCIP #32 -use crate::{memo::{UnusedMemo, GiftCodeFundingMemo}, MemoBuilder, ReservedDestination}; +use crate::{ + memo::{GiftCodeFundingMemo, UnusedMemo}, + MemoBuilder, ReservedDestination, +}; use mc_account_keys::PublicAddress; use mc_crypto_keys::RistrettoPublic; use mc_transaction_core::{Amount, MemoContext, MemoPayload, NewMemoError}; @@ -15,11 +18,11 @@ use mc_transaction_core::{Amount, MemoContext, MemoPayload, NewMemoError}; /// | -->0x0201<-- | Gift Code Funding Memo | /// | 0x0202 | Gift Code Cancellation Memo | /// This memo builder builds a gift code funding memo (0x0201). When a gift -/// code is funded, the amount of the gift code is sent to a TxOut at the -/// Sender's reserved gift code subaddress and a second zero valued TxOut -/// is sent to the sender's reserved change subaddress with the gift code -/// funding memo attached. The funding memo will include the first 4 bytes -/// of the hash of the gift code TxOut sent to the sender's reserved gift +/// code is funded, the amount of the gift code is sent to a TxOut at the +/// Sender's reserved gift code subaddress and a second zero valued TxOut +/// is sent to the sender's reserved change subaddress with the gift code +/// funding memo attached. The funding memo will include the first 4 bytes +/// of the hash of the gift code TxOut sent to the sender's reserved gift /// code subaddress and 60 bytes for an optional utf-8 memo. /// /// IMPORTANT NOTE: The public_key of the zero valued TxOut that the Gift Code @@ -55,7 +58,9 @@ impl GiftCodeFundingMemoBuilder { /// 60 bytes. pub fn set_funding_note(&mut self, note: &str) -> Result<(), NewMemoError> { if note.len() > GiftCodeFundingMemo::NOTE_DATA_LEN { - return Err(NewMemoError::BadInputs("Note memo cannot be greater than 60 bytes".into())) + return Err(NewMemoError::BadInputs( + "Note memo cannot be greater than 60 bytes".into(), + )); } self.note = note.into(); Ok(()) @@ -134,3 +139,12 @@ impl MemoBuilder for GiftCodeFundingMemoBuilder { } } } + +mod tests { + use super::*; + + #[test] + fn test_gift_code() { + // Tests forthcoming + } +} diff --git a/transaction/std/src/memo_builder/gift_code_sender_memo_builder.rs b/transaction/std/src/memo_builder/gift_code_sender_memo_builder.rs index a892ee3bb4..3c100267e1 100644 --- a/transaction/std/src/memo_builder/gift_code_sender_memo_builder.rs +++ b/transaction/std/src/memo_builder/gift_code_sender_memo_builder.rs @@ -3,7 +3,10 @@ //! Defines the Memo Builder for the gift code sender memo (0x0002) //! specified in MCIP #32 -use crate::{memo::{GiftCodeSenderMemo, UnusedMemo}, MemoBuilder, ReservedDestination}; +use crate::{ + memo::{GiftCodeSenderMemo, UnusedMemo}, + MemoBuilder, ReservedDestination, +}; use mc_account_keys::PublicAddress; use mc_transaction_core::{Amount, MemoContext, MemoPayload, NewMemoError}; @@ -45,7 +48,9 @@ impl GiftCodeSenderMemoBuilder { /// 64 bytes. pub fn set_gift_code_sender_note(&mut self, note: &str) -> Result<(), NewMemoError> { if note.len() > GiftCodeSenderMemo::MEMO_DATA_LEN { - return Err(NewMemoError::BadInputs("Sender note cannot be longer than 64 bytes".into())); + return Err(NewMemoError::BadInputs( + "Sender note cannot be longer than 64 bytes".into(), + )); } self.note = note.into(); Ok(()) @@ -98,3 +103,12 @@ impl MemoBuilder for GiftCodeSenderMemoBuilder { Ok(GiftCodeSenderMemo::new(self.note.as_str())?.into()) } } + +mod tests { + use super::*; + + #[test] + fn test_gift_code() { + // Tests forthcoming + } +} diff --git a/transaction/std/src/memo_builder/mod.rs b/transaction/std/src/memo_builder/mod.rs index 8e6dd64a40..a9e4dc2db7 100644 --- a/transaction/std/src/memo_builder/mod.rs +++ b/transaction/std/src/memo_builder/mod.rs @@ -15,7 +15,6 @@ mod gift_code_funding_memo_builder; mod gift_code_sender_memo_builder; mod rth_memo_builder; - pub use burn_redemption_memo_builder::BurnRedemptionMemoBuilder; pub use gift_code_cancellation_memo_builder::GiftCodeCancellationMemoBuilder; pub use gift_code_funding_memo_builder::GiftCodeFundingMemoBuilder; From fa8705a5f6731168cb0e34170ff346f23b774d88 Mon Sep 17 00:00:00 2001 From: iamalwaysuncomfortable Date: Fri, 13 May 2022 14:15:04 -0400 Subject: [PATCH 03/12] Lint fix part deux --- .../gift_code_cancellation_memo_builder.rs | 16 +++++++++------- .../gift_code_funding_memo_builder.rs | 16 +++++++++------- .../gift_code_sender_memo_builder.rs | 10 ++++++---- 3 files changed, 24 insertions(+), 18 deletions(-) diff --git a/transaction/std/src/memo_builder/gift_code_cancellation_memo_builder.rs b/transaction/std/src/memo_builder/gift_code_cancellation_memo_builder.rs index a51925febd..32c4e4a9f1 100644 --- a/transaction/std/src/memo_builder/gift_code_cancellation_memo_builder.rs +++ b/transaction/std/src/memo_builder/gift_code_cancellation_memo_builder.rs @@ -33,15 +33,18 @@ pub struct GiftCodeCancellationMemoBuilder { wrote_change_memo: bool, } -impl GiftCodeCancellationMemoBuilder { - /// Create a new cancellation gift code memo builder - pub fn new() -> Self { +// Create an empty GiftCodeCancellationMemoBuilder +impl Default for GiftCodeCancellationMemoBuilder { + fn default() -> Self { Self { gift_code_tx_out_global_index: None, gift_code_change_memo_enabled: true, wrote_change_memo: false, } } +} + +impl GiftCodeCancellationMemoBuilder { /// Set the index of the gift code TxOut that was cancelled pub fn set_gift_code_tx_out_index(&mut self, tx_out_global_index: u64) { self.gift_code_tx_out_global_index = Some(tx_out_global_index) @@ -92,17 +95,16 @@ impl MemoBuilder for GiftCodeCancellationMemoBuilder { } self.wrote_change_memo = true; if let Some(tx_out_global_index) = self.gift_code_tx_out_global_index.take() { - return Ok(GiftCodeCancellationMemo::from(tx_out_global_index).into()); + Ok(GiftCodeCancellationMemo::from(tx_out_global_index).into()) } else { - return Err(NewMemoError::MissingInput( + Err(NewMemoError::MissingInput( "Missing global index of TxOut to be cancelled".into(), - )); + )) } } } mod tests { - use super::*; #[test] fn test_gift_code() { diff --git a/transaction/std/src/memo_builder/gift_code_funding_memo_builder.rs b/transaction/std/src/memo_builder/gift_code_funding_memo_builder.rs index 70986484bd..74dd04bcb1 100644 --- a/transaction/std/src/memo_builder/gift_code_funding_memo_builder.rs +++ b/transaction/std/src/memo_builder/gift_code_funding_memo_builder.rs @@ -42,9 +42,9 @@ pub struct GiftCodeFundingMemoBuilder { wrote_change_memo: bool, } -impl GiftCodeFundingMemoBuilder { - /// Create a new gift code funding memo builder - pub fn new() -> Self { +// Create an empty GiftCodeFundingMemoBuilder +impl Default for GiftCodeFundingMemoBuilder { + fn default() -> Self { Self { note: "".into(), gift_code_tx_out_public_key: None, @@ -52,6 +52,9 @@ impl GiftCodeFundingMemoBuilder { wrote_change_memo: false, } } +} + +impl GiftCodeFundingMemoBuilder { /// Set a utf-8 note (up to 60 bytes) onto the funding memo indicating /// what the gift code was for. This method will enforce the 60 byte /// limit with a NewMemoErr if the &str passed is longer than @@ -131,17 +134,16 @@ impl MemoBuilder for GiftCodeFundingMemoBuilder { } if let Some(tx_out_public_key) = self.gift_code_tx_out_public_key.take() { self.wrote_change_memo = true; - return Ok(GiftCodeFundingMemo::new(&tx_out_public_key, self.note.as_str())?.into()); + Ok(GiftCodeFundingMemo::new(&tx_out_public_key, self.note.as_str())?.into()) } else { - return Err(NewMemoError::MissingInput( + Err(NewMemoError::MissingInput( "Missing gift code TxOut public key".into(), - )); + )) } } } mod tests { - use super::*; #[test] fn test_gift_code() { diff --git a/transaction/std/src/memo_builder/gift_code_sender_memo_builder.rs b/transaction/std/src/memo_builder/gift_code_sender_memo_builder.rs index 3c100267e1..817bc78823 100644 --- a/transaction/std/src/memo_builder/gift_code_sender_memo_builder.rs +++ b/transaction/std/src/memo_builder/gift_code_sender_memo_builder.rs @@ -33,15 +33,18 @@ pub struct GiftCodeSenderMemoBuilder { wrote_change_memo: bool, } -impl GiftCodeSenderMemoBuilder { - /// Create a new gift code sender memo builder - pub fn new() -> Self { +// Create an empty GiftCodeSenderMemoBuilder +impl Default for GiftCodeSenderMemoBuilder { + fn default() -> Self { Self { note: "".into(), gift_code_change_memo_enabled: true, wrote_change_memo: false, } } +} + +impl GiftCodeSenderMemoBuilder { /// Set a utf-8 note (up to 64 bytes) onto the sender memo indicating /// any desired info about the gift code. This method will enforce the /// 64 byte limit with a NewMemoErr if the &str passed is longer than @@ -105,7 +108,6 @@ impl MemoBuilder for GiftCodeSenderMemoBuilder { } mod tests { - use super::*; #[test] fn test_gift_code() { From acfb5c78270a7f624cc3b7d1ad7732f36926cca6 Mon Sep 17 00:00:00 2001 From: iamalwaysuncomfortable Date: Fri, 13 May 2022 20:10:17 -0400 Subject: [PATCH 04/12] Test coverage for cancellation and sender memo builder --- transaction/core/src/tx_error.rs | 7 + .../gift_code_cancellation_memo_builder.rs | 114 +++++++++++++- .../gift_code_funding_memo_builder.rs | 35 +++- .../gift_code_sender_memo_builder.rs | 149 +++++++++++++++++- transaction/std/src/test_utils.rs | 35 +++- 5 files changed, 330 insertions(+), 10 deletions(-) diff --git a/transaction/core/src/tx_error.rs b/transaction/core/src/tx_error.rs index 3ebbb77c93..d1a12d9255 100644 --- a/transaction/core/src/tx_error.rs +++ b/transaction/core/src/tx_error.rs @@ -4,6 +4,7 @@ use crate::{AmountError, MemoError}; use alloc::{format, string::String}; +use core::str::Utf8Error; use displaydoc::Display; use mc_crypto_keys::KeyError; @@ -98,3 +99,9 @@ impl From for NewMemoError { } } } + +impl From for NewMemoError { + fn from(_: Utf8Error) -> Self { + Self::Utf8Decoding + } +} diff --git a/transaction/std/src/memo_builder/gift_code_cancellation_memo_builder.rs b/transaction/std/src/memo_builder/gift_code_cancellation_memo_builder.rs index 32c4e4a9f1..c20c49b381 100644 --- a/transaction/std/src/memo_builder/gift_code_cancellation_memo_builder.rs +++ b/transaction/std/src/memo_builder/gift_code_cancellation_memo_builder.rs @@ -83,7 +83,7 @@ impl MemoBuilder for GiftCodeCancellationMemoBuilder { /// Build a memo for a gift code change output fn make_memo_for_change_output( &mut self, - _amount: Amount, + amount: Amount, _change_destination: &ReservedDestination, _memo_context: MemoContext, ) -> Result { @@ -93,6 +93,16 @@ impl MemoBuilder for GiftCodeCancellationMemoBuilder { if self.wrote_change_memo { return Err(NewMemoError::MultipleChangeOutputs); } + if self.gift_code_tx_out_global_index.is_none() { + return Err(NewMemoError::MissingInput( + "Must specify gift code TxOut global index".into(), + )); + } + if amount.value > 0 { + return Err(NewMemoError::BadInputs( + "Cancellation memo TxOut should be zero valued".into(), + )); + } self.wrote_change_memo = true; if let Some(tx_out_global_index) = self.gift_code_tx_out_global_index.take() { Ok(GiftCodeCancellationMemo::from(tx_out_global_index).into()) @@ -104,10 +114,108 @@ impl MemoBuilder for GiftCodeCancellationMemoBuilder { } } +#[cfg(test)] mod tests { + use super::*; + use crate::test_utils::build_zero_value_change_memo; + use std::convert::TryInto; + + #[test] + fn test_gift_code_cancellation_memo_built_successfully_with_index() { + // Create memo builder + let mut builder = GiftCodeCancellationMemoBuilder::default(); + + // Set the cancellation index + let index = 666; + builder.set_gift_code_tx_out_index(index); + + // Build the memo payload and get the data + let memo_payload = build_zero_value_change_memo(&mut builder).unwrap(); + let memo_data = memo_payload.get_memo_data(); + + // Verify memo data + let derived_index = u64::from_le_bytes(memo_data[0..8].try_into().unwrap()); + assert_eq!(index, derived_index); + } + + #[test] + fn test_gift_code_cancellation_memo_fails_without_index() { + // Instantiate memo builder + let mut builder = GiftCodeCancellationMemoBuilder::default(); + + // Build the memo payload + let memo_payload = build_zero_value_change_memo(&mut builder); + + // Assert we've created the correct error + assert!(matches!(memo_payload, Err(NewMemoError::MissingInput(_)))); + } + + #[test] + fn test_gift_code_cancellation_memo_fails_for_more_than_one_change_memo() { + // Create memo builder + let mut builder = GiftCodeCancellationMemoBuilder::default(); + + // Set the cancellation index + let index = 666; + builder.set_gift_code_tx_out_index(index); + + // Build the memo payload + build_zero_value_change_memo(&mut builder).unwrap(); + let memo_payload = build_zero_value_change_memo(&mut builder); + assert!(matches!( + memo_payload, + Err(NewMemoError::MultipleChangeOutputs) + )); + } + + #[test] + fn test_gift_code_cancellation_memo_fields_are_set_and_cleared_properly() { + // Create memo builder + let mut builder = GiftCodeCancellationMemoBuilder::default(); + + // Set the cancellation index, and then replace it + let index = 666; + builder.set_gift_code_tx_out_index(index); + let replacement_index = 420; + builder.set_gift_code_tx_out_index(replacement_index); + + // Build the memo payload and get the data + let memo_payload = build_zero_value_change_memo(&mut builder).unwrap(); + let memo_data = memo_payload.get_memo_data(); + + // Verify memo data + let derived_index = u64::from_le_bytes(memo_data[0..8].try_into().unwrap()); + assert_eq!(replacement_index, derived_index); + + // Create another memo builder + let mut builder_2 = GiftCodeCancellationMemoBuilder::default(); + + // Set the cancellation index and then clear it + let index_2 = 666; + builder_2.set_gift_code_tx_out_index(index_2); + builder_2.clear_gift_code_tx_out_index(); + + // Build the memo payload and get the data + let memo_payload_2 = build_zero_value_change_memo(&mut builder_2); + assert!(matches!(memo_payload_2, Err(NewMemoError::MissingInput(_)))); + } #[test] - fn test_gift_code() { - // Tests forthcoming + fn test_gift_code_cancellation_memo_writes_unused_if_change_disabled() { + // Create memo builder + let mut builder = GiftCodeCancellationMemoBuilder::default(); + + // Set the cancellation index + let index = 666; + builder.set_gift_code_tx_out_index(index); + + // Disable change outputs + builder.disable_gift_code_change_memo(); + + // Build the memo payload and get the data + let memo_payload = build_zero_value_change_memo(&mut builder).unwrap(); + + // Verify memo data is blank + assert_eq!(memo_payload.get_memo_data(), &[0u8; 64]); } } diff --git a/transaction/std/src/memo_builder/gift_code_funding_memo_builder.rs b/transaction/std/src/memo_builder/gift_code_funding_memo_builder.rs index 74dd04bcb1..27087c01f7 100644 --- a/transaction/std/src/memo_builder/gift_code_funding_memo_builder.rs +++ b/transaction/std/src/memo_builder/gift_code_funding_memo_builder.rs @@ -119,7 +119,7 @@ impl MemoBuilder for GiftCodeFundingMemoBuilder { /// Build a memo for a gift code change output fn make_memo_for_change_output( &mut self, - _amount: Amount, + amount: Amount, _change_destination: &ReservedDestination, memo_context: MemoContext, ) -> Result { @@ -129,6 +129,11 @@ impl MemoBuilder for GiftCodeFundingMemoBuilder { if self.wrote_change_memo { return Err(NewMemoError::MultipleChangeOutputs); } + if amount.value > 0 { + return Err(NewMemoError::BadInputs( + "Funding memo TxOut should be zero valued".into(), + )); + } if self.gift_code_tx_out_public_key.as_ref() == Some(memo_context.tx_public_key) { return Err(NewMemoError::BadInputs("The public_key in the memo should be the TxOut at the gift code subaddress and not that of the memo TxOut".into())); } @@ -143,10 +148,36 @@ impl MemoBuilder for GiftCodeFundingMemoBuilder { } } +#[cfg(test)] mod tests { #[test] - fn test_gift_code() { + fn test_gift_code_funding_memo_built_successfully_with_note() { + // Tests forthcoming + } + + #[test] + fn test_gift_code_funding_memo_built_successfully_without_note() { + // Tests forthcoming + } + + #[test] + fn test_gift_code_funding_memo_fails_to_build_with_key_from_context() { + // Tests forthcoming + } + + #[test] + fn test_gift_code_funding_memo_fails_for_more_than_one_change_memo() { + // Tests forthcoming + } + + #[test] + fn test_gift_code_funding_memo_fields_are_cleared_properly() { + // Tests forthcoming + } + + #[test] + fn test_gift_code_funding_memo_writes_unused_if_change_disabled() { // Tests forthcoming } } diff --git a/transaction/std/src/memo_builder/gift_code_sender_memo_builder.rs b/transaction/std/src/memo_builder/gift_code_sender_memo_builder.rs index 817bc78823..80195a50cd 100644 --- a/transaction/std/src/memo_builder/gift_code_sender_memo_builder.rs +++ b/transaction/std/src/memo_builder/gift_code_sender_memo_builder.rs @@ -31,6 +31,8 @@ pub struct GiftCodeSenderMemoBuilder { gift_code_change_memo_enabled: bool, // Whether we've already written the change memo wrote_change_memo: bool, + // Whetever we've set a valid note + wrote_valid_note: bool, } // Create an empty GiftCodeSenderMemoBuilder @@ -40,6 +42,7 @@ impl Default for GiftCodeSenderMemoBuilder { note: "".into(), gift_code_change_memo_enabled: true, wrote_change_memo: false, + wrote_valid_note: true, } } } @@ -51,6 +54,7 @@ impl GiftCodeSenderMemoBuilder { /// 64 bytes. pub fn set_gift_code_sender_note(&mut self, note: &str) -> Result<(), NewMemoError> { if note.len() > GiftCodeSenderMemo::MEMO_DATA_LEN { + self.wrote_valid_note = false; return Err(NewMemoError::BadInputs( "Sender note cannot be longer than 64 bytes".into(), )); @@ -102,15 +106,156 @@ impl MemoBuilder for GiftCodeSenderMemoBuilder { if self.wrote_change_memo { return Err(NewMemoError::MultipleChangeOutputs); }; + if !self.wrote_valid_note { + return Err(NewMemoError::BadInputs( + "Tried to set a note longer than 64 bytes".into(), + )); + } self.wrote_change_memo = true; Ok(GiftCodeSenderMemo::new(self.note.as_str())?.into()) } } +#[cfg(test)] mod tests { + use super::*; + use crate::test_utils::build_change_memo_with_amount; + + /// Get the sender note + fn get_sender_note(memo_data: &[u8; GiftCodeSenderMemo::MEMO_DATA_LEN]) -> &str { + let index = if let Some(terminator) = memo_data.iter().position(|b| b == &0u8) { + terminator + } else { + GiftCodeSenderMemo::MEMO_DATA_LEN + }; + + std::str::from_utf8(&memo_data[0..index]).unwrap() + } + + #[test] + fn test_gift_code_sender_memo_built_successfully_with_note() { + // Create memo builder + let mut builder = GiftCodeSenderMemoBuilder::default(); + + // Set the note + let note = "It's MEMO TIME!!"; + builder.set_gift_code_sender_note(note).unwrap(); + + // Build the memo payload and get the data + let amount = Amount::new(42, 0.into()); + let memo_payload = build_change_memo_with_amount(&mut builder, amount).unwrap(); + + // Verify memo data + let derived_note = get_sender_note(memo_payload.get_memo_data()); + assert_eq!(note, derived_note); + } #[test] - fn test_gift_code() { - // Tests forthcoming + fn test_gift_code_sender_memo_built_successfully_without_note() { + // Create memo builder + let mut builder = GiftCodeSenderMemoBuilder::default(); + + // Build the memo payload and get the data + let amount = Amount::new(42, 0.into()); + let memo_payload = build_change_memo_with_amount(&mut builder, amount).unwrap(); + + // Verify memo data + let blank_note = ""; + let derived_note = get_sender_note(memo_payload.get_memo_data()); + assert_eq!(blank_note, derived_note); + } + + #[test] + fn test_gift_code_sender_memo_fails_for_more_than_one_change_memo() { + // Create memo builder + let mut builder = GiftCodeSenderMemoBuilder::default(); + + // Set the note + let note = "It's MEMO TIME!!"; + builder.set_gift_code_sender_note(note).unwrap(); + + // Build the memo payload and get the data + let amount = Amount::new(42, 0.into()); + let memo_payload = build_change_memo_with_amount(&mut builder, amount).unwrap(); + + // Verify memo data works once + let derived_note = get_sender_note(memo_payload.get_memo_data()); + assert_eq!(note, derived_note); + + // Verify memo_data doesn't work more than once + let amount_2 = Amount::new(42, 0.into()); + let memo_payload_2 = build_change_memo_with_amount(&mut builder, amount_2); + + let matches = matches!(memo_payload_2, Err(NewMemoError::MultipleChangeOutputs)); + assert!(matches); + } + + #[test] + fn test_gift_code_sender_memo_fields_are_cleared_properly() { + // Create memo builder + let mut builder = GiftCodeSenderMemoBuilder::default(); + + // Set the note + let note = "It's MEMO TIME!!"; + builder.set_gift_code_sender_note(note).unwrap(); + builder.clear_sender_note(); + + // Build the memo payload + let amount = Amount::new(42, 0.into()); + let memo_payload = build_change_memo_with_amount(&mut builder, amount).unwrap(); + + // Verify memo data + let blank_note = ""; + let derived_note = get_sender_note(memo_payload.get_memo_data()); + assert_eq!(blank_note, derived_note); + + // Create another memo builder + let mut builder_2 = GiftCodeSenderMemoBuilder::default(); + + // Set the cancellation index and then set another note on top of it + let note_2 = "Is anything actually real?"; + builder_2.set_gift_code_sender_note(note).unwrap(); + builder_2.set_gift_code_sender_note(note_2).unwrap(); + + // Build the memo payload and get the data + let amount_2 = Amount::new(666, 0.into()); + let memo_payload_2 = build_change_memo_with_amount(&mut builder_2, amount_2).unwrap(); + let derived_note_2 = get_sender_note(memo_payload_2.get_memo_data()); + assert_eq!(note_2, derived_note_2); + } + + #[test] + fn test_gift_code_sender_memo_writes_unused_if_change_disabled() { + // Create memo builder + let mut builder = GiftCodeSenderMemoBuilder::default(); + + // Set the note + let note = "It's MEMO TIME!!"; + builder.set_gift_code_sender_note(note).unwrap(); + + // Disable change memos + builder.disable_gift_code_change_memo(); + + // Build the memo payload + let amount = Amount::new(42, 0.into()); + let memo_payload = build_change_memo_with_amount(&mut builder, amount).unwrap(); + assert_eq!(memo_payload.get_memo_data(), &[0u8; 64]); + } + + #[test] + fn test_gift_code_sender_builder_doesnt_allow_invalid_note_length() { + // Create memo builder + let mut builder = GiftCodeSenderMemoBuilder::default(); + + // Set an invalid note + let note_bytes = [b'6'; GiftCodeSenderMemo::MEMO_DATA_LEN + 1]; + let note = std::str::from_utf8(¬e_bytes).unwrap(); + let result = builder.set_gift_code_sender_note(note); + assert!(matches!(result, Err(NewMemoError::BadInputs(_)))); + + // Try to build after failing to set note + let amount = Amount::new(42, 0.into()); + let memo_payload = build_change_memo_with_amount(&mut builder, amount); + assert!(matches!(memo_payload, Err(NewMemoError::BadInputs(_)))); } } diff --git a/transaction/std/src/test_utils.rs b/transaction/std/src/test_utils.rs index d87595a037..a217a7ddab 100644 --- a/transaction/std/src/test_utils.rs +++ b/transaction/std/src/test_utils.rs @@ -2,7 +2,10 @@ //! Utilities that help with testing the transaction builder and related objects -use crate::{EmptyMemoBuilder, InputCredentials, MemoPayload, TransactionBuilder, TxBuilderError}; +use crate::{ + EmptyMemoBuilder, InputCredentials, MemoBuilder, MemoPayload, ReservedDestination, + TransactionBuilder, TxBuilderError, +}; use core::convert::TryFrom; use mc_account_keys::{AccountKey, PublicAddress, DEFAULT_SUBADDRESS_INDEX}; use mc_crypto_keys::RistrettoPublic; @@ -11,9 +14,10 @@ use mc_transaction_core::{ onetime_keys::*, tokens::Mob, tx::{Tx, TxOut, TxOutMembershipProof}, - Amount, BlockVersion, Token, TokenId, + Amount, BlockVersion, MemoContext, NewMemoError, Token, TokenId, }; -use rand::{CryptoRng, RngCore}; +use mc_util_from_random::FromRandom; +use rand::{rngs::StdRng, CryptoRng, RngCore, SeedableRng}; /// Creates a TxOut that sends `value` to `recipient`. /// @@ -201,3 +205,28 @@ pub fn get_transaction transaction_builder.build(rng) } + +/// Build simulated test memo with zero amount +pub fn build_zero_value_change_memo( + builder: &mut impl MemoBuilder, +) -> Result { + build_change_memo_with_amount(builder, Amount::new(0, 0.into())) +} + +/// Build simulated test memo with amount +pub fn build_change_memo_with_amount( + builder: &mut impl MemoBuilder, + change_amount: Amount, +) -> Result { + // Create simulated context + let mut rng: StdRng = SeedableRng::from_seed([0u8; 32]); + let alice = AccountKey::random_with_fog(&mut rng); + let alice_address_book = ReservedDestination::from(&alice); + let change_tx_pubkey = RistrettoPublic::from_random(&mut rng); + let memo_context = MemoContext { + tx_public_key: &change_tx_pubkey, + }; + + //Build memo + builder.make_memo_for_change_output(change_amount, &alice_address_book, memo_context) +} From 57262522901c18ff2c11aab5342c373b3c042d3c Mon Sep 17 00:00:00 2001 From: iamalwaysuncomfortable Date: Fri, 13 May 2022 20:15:50 -0400 Subject: [PATCH 05/12] Address sorting & comment structure nits --- transaction/core/src/memo.rs | 2 +- transaction/core/src/tx_error.rs | 2 +- .../gift_code_cancellation_memo_builder.rs | 10 +++++----- .../src/memo_builder/gift_code_funding_memo_builder.rs | 10 +++++----- .../src/memo_builder/gift_code_sender_memo_builder.rs | 10 +++++----- 5 files changed, 17 insertions(+), 17 deletions(-) diff --git a/transaction/core/src/memo.rs b/transaction/core/src/memo.rs index 1a9d7ab991..cd9fd6b39d 100644 --- a/transaction/core/src/memo.rs +++ b/transaction/core/src/memo.rs @@ -223,7 +223,7 @@ derive_serde_from_repr_bytes!(MemoPayload); derive_prost_message_from_repr_bytes!(MemoPayload); /// An error which can occur when handling memos -#[derive(Debug, Display, PartialEq, Eq)] +#[derive(Debug, Display, Eq, PartialEq)] pub enum MemoError { /// Wrong length for memo payload: {0} BadLength(usize), diff --git a/transaction/core/src/tx_error.rs b/transaction/core/src/tx_error.rs index d1a12d9255..3434c527f5 100644 --- a/transaction/core/src/tx_error.rs +++ b/transaction/core/src/tx_error.rs @@ -56,7 +56,7 @@ impl From for ViewKeyMatchError { /// We have included error codes for some known useful error conditions. /// For a custom MemoBuilder, you can try to reuse those, or use the Other /// error code. -#[derive(Debug, Display, PartialEq, Eq)] +#[derive(Debug, Display, Eq, PartialEq)] pub enum NewMemoError { /// Limits for '{0}' value exceeded LimitsExceeded(&'static str), diff --git a/transaction/std/src/memo_builder/gift_code_cancellation_memo_builder.rs b/transaction/std/src/memo_builder/gift_code_cancellation_memo_builder.rs index c20c49b381..3b298611df 100644 --- a/transaction/std/src/memo_builder/gift_code_cancellation_memo_builder.rs +++ b/transaction/std/src/memo_builder/gift_code_cancellation_memo_builder.rs @@ -11,11 +11,11 @@ use mc_account_keys::PublicAddress; use mc_transaction_core::{Amount, MemoContext, MemoPayload, NewMemoError}; /// There are three possible gift code memo types specified in MCIP #32 -/// | Memo type bytes | Name | -/// | ----------- | ----------- | -/// | 0x0002 | Gift Code Sender Memo | -/// | 0x0201 | Gift Code Funding Memo | -/// | -->0x0202<-- | Gift Code Cancellation Memo | +/// | Memo type bytes | Name | +/// | ----------- | ----------- | +/// | 0x0002 | Gift Code Sender Memo | +/// | 0x0201 | Gift Code Funding Memo | +/// | -->0x0202<-- | Gift Code Cancellation Memo | /// This memo builder builds a gift code cancellation memo (0x0202). Gift code /// cancellation is defined as the sender sending the gift code TxOut at the /// gift code subaddress back to their default address prior to it being spent diff --git a/transaction/std/src/memo_builder/gift_code_funding_memo_builder.rs b/transaction/std/src/memo_builder/gift_code_funding_memo_builder.rs index 27087c01f7..ea6c00280f 100644 --- a/transaction/std/src/memo_builder/gift_code_funding_memo_builder.rs +++ b/transaction/std/src/memo_builder/gift_code_funding_memo_builder.rs @@ -12,11 +12,11 @@ use mc_crypto_keys::RistrettoPublic; use mc_transaction_core::{Amount, MemoContext, MemoPayload, NewMemoError}; /// There are three possible gift code memo types specified in MCIP #32 -/// | Memo type bytes | Name | -/// | ----------- | ----------- | -/// | 0x0002 | Gift Code Sender Memo | -/// | -->0x0201<-- | Gift Code Funding Memo | -/// | 0x0202 | Gift Code Cancellation Memo | +/// | Memo type bytes | Name | +/// | ----------- | ----------- | +/// | 0x0002 | Gift Code Sender Memo | +/// | -->0x0201<-- | Gift Code Funding Memo | +/// | 0x0202 | Gift Code Cancellation Memo | /// This memo builder builds a gift code funding memo (0x0201). When a gift /// code is funded, the amount of the gift code is sent to a TxOut at the /// Sender's reserved gift code subaddress and a second zero valued TxOut diff --git a/transaction/std/src/memo_builder/gift_code_sender_memo_builder.rs b/transaction/std/src/memo_builder/gift_code_sender_memo_builder.rs index 80195a50cd..4cc89349ab 100644 --- a/transaction/std/src/memo_builder/gift_code_sender_memo_builder.rs +++ b/transaction/std/src/memo_builder/gift_code_sender_memo_builder.rs @@ -11,11 +11,11 @@ use mc_account_keys::PublicAddress; use mc_transaction_core::{Amount, MemoContext, MemoPayload, NewMemoError}; /// There are three possible gift code memo types specified in MCIP #32 -/// | Memo type bytes | Name | -/// | ----------- | ----------- | -/// | -->0x0002<-- | Gift Code Sender Memo | -/// | 0x0201 | Gift Code Funding Memo | -/// | 0x0202 | Gift Code Cancellation Memo | +/// | Memo type bytes | Name | +/// | ----------- | ----------- | +/// | -->0x0002<-- | Gift Code Sender Memo | +/// | 0x0201 | Gift Code Funding Memo | +/// | 0x0202 | Gift Code Cancellation Memo | /// This memo builder builds a gift code sender memo (0x0002). A gift code /// considered redeemed when the Receiver uses the TxOut spend private key /// of the gift code TxOut they received from the Sender to send the TxOut From 49b2819c147bed4c588e8c14a8a9e881b0f2353a Mon Sep 17 00:00:00 2001 From: iamalwaysuncomfortable Date: Sat, 14 May 2022 12:17:32 -0400 Subject: [PATCH 06/12] Add even more tests --- .../gift_code_cancellation_memo_builder.rs | 17 +- .../gift_code_funding_memo_builder.rs | 281 +++++++++++++++++- .../gift_code_sender_memo_builder.rs | 65 ++-- transaction/std/src/test_utils.rs | 50 +++- 4 files changed, 374 insertions(+), 39 deletions(-) diff --git a/transaction/std/src/memo_builder/gift_code_cancellation_memo_builder.rs b/transaction/std/src/memo_builder/gift_code_cancellation_memo_builder.rs index 3b298611df..a36cd39a6b 100644 --- a/transaction/std/src/memo_builder/gift_code_cancellation_memo_builder.rs +++ b/transaction/std/src/memo_builder/gift_code_cancellation_memo_builder.rs @@ -117,7 +117,7 @@ impl MemoBuilder for GiftCodeCancellationMemoBuilder { #[cfg(test)] mod tests { use super::*; - use crate::test_utils::build_zero_value_change_memo; + use crate::test_utils::{build_change_memo_with_amount, build_zero_value_change_memo}; use std::convert::TryInto; #[test] @@ -168,6 +168,21 @@ mod tests { )); } + #[test] + fn test_gift_code_cancellation_memo_fails_for_nonzero_amount() { + // Create memo builder + let mut builder = GiftCodeCancellationMemoBuilder::default(); + + // Set the cancellation index + let index = 666; + builder.set_gift_code_tx_out_index(index); + + // Build the memo payload + let amount = Amount::new(666, 0.into()); + let memo_payload = build_change_memo_with_amount(&mut builder, amount); + assert!(matches!(memo_payload, Err(NewMemoError::BadInputs(_)))); + } + #[test] fn test_gift_code_cancellation_memo_fields_are_set_and_cleared_properly() { // Create memo builder diff --git a/transaction/std/src/memo_builder/gift_code_funding_memo_builder.rs b/transaction/std/src/memo_builder/gift_code_funding_memo_builder.rs index ea6c00280f..3d41c7f91b 100644 --- a/transaction/std/src/memo_builder/gift_code_funding_memo_builder.rs +++ b/transaction/std/src/memo_builder/gift_code_funding_memo_builder.rs @@ -40,6 +40,9 @@ pub struct GiftCodeFundingMemoBuilder { gift_code_change_memo_enabled: bool, // Whether we've already written the change memo wrote_change_memo: bool, + // Whether our last note setting attempt was invalid and the note hasn't + // been reset with a valid note or cleared + attempted_invalid_note: bool, } // Create an empty GiftCodeFundingMemoBuilder @@ -50,6 +53,7 @@ impl Default for GiftCodeFundingMemoBuilder { gift_code_tx_out_public_key: None, gift_code_change_memo_enabled: true, wrote_change_memo: false, + attempted_invalid_note: false, } } } @@ -61,15 +65,18 @@ impl GiftCodeFundingMemoBuilder { /// 60 bytes. pub fn set_funding_note(&mut self, note: &str) -> Result<(), NewMemoError> { if note.len() > GiftCodeFundingMemo::NOTE_DATA_LEN { + self.attempted_invalid_note = true; return Err(NewMemoError::BadInputs( "Note memo cannot be greater than 60 bytes".into(), )); } + self.attempted_invalid_note = false; self.note = note.into(); Ok(()) } /// Clear the gift code funding note pub fn clear_funding_note(&mut self) { + self.attempted_invalid_note = false; self.note = "".into(); } /// Set the TxOut public_key of the gift code TxOut sent to the @@ -80,9 +87,9 @@ impl GiftCodeFundingMemoBuilder { /// so will result in an error when attempting to build the memo. pub fn set_gift_code_tx_out_public_key( &mut self, - tx_out_public_key: RistrettoPublic, + tx_out_public_key: &RistrettoPublic, ) -> Result<(), NewMemoError> { - self.gift_code_tx_out_public_key = Some(tx_out_public_key); + self.gift_code_tx_out_public_key = Some(*tx_out_public_key); Ok(()) } /// Clear the gift code tx_out_public_key @@ -134,6 +141,12 @@ impl MemoBuilder for GiftCodeFundingMemoBuilder { "Funding memo TxOut should be zero valued".into(), )); } + // Prevent callers from writing memo if last note set attempt was a failure + if self.attempted_invalid_note { + return Err(NewMemoError::BadInputs( + "Tried to set a note longer than 64 bytes".into(), + )); + } if self.gift_code_tx_out_public_key.as_ref() == Some(memo_context.tx_public_key) { return Err(NewMemoError::BadInputs("The public_key in the memo should be the TxOut at the gift code subaddress and not that of the memo TxOut".into())); } @@ -150,34 +163,282 @@ impl MemoBuilder for GiftCodeFundingMemoBuilder { #[cfg(test)] mod tests { + use super::*; + use crate::test_utils::{ + build_change_memo_with_amount, build_zero_value_change_memo, MemoDecoder, + }; + use mc_account_keys::AccountKey; + use mc_util_from_random::FromRandom; + use rand::{rngs::StdRng, SeedableRng}; #[test] fn test_gift_code_funding_memo_built_successfully_with_note() { - // Tests forthcoming + // Create memo builder + let mut builder = GiftCodeFundingMemoBuilder::default(); + + // Set the gift code TxOut public key and note + let mut rng: StdRng = SeedableRng::from_seed([0u8; 32]); + let gift_code_public_key = RistrettoPublic::from_random(&mut rng); + let note = "It's MEMO TIME!!"; + builder + .set_gift_code_tx_out_public_key(&gift_code_public_key) + .unwrap(); + builder.set_funding_note(note).unwrap(); + + // Build the memo payload and get the data + let memo_payload = build_zero_value_change_memo(&mut builder).unwrap(); + let memo_data = memo_payload.get_memo_data(); + + // Verify memo data + let derived_note = MemoDecoder::decode_funding_note(memo_data); + let expected_hash = MemoDecoder::tx_out_public_key_short_hash(&gift_code_public_key); + assert_eq!(note, derived_note); + assert_eq!( + expected_hash, + memo_data[0..GiftCodeFundingMemo::HASH_DATA_LEN] + ); } #[test] fn test_gift_code_funding_memo_built_successfully_without_note() { - // Tests forthcoming + // Create memo builder + let mut builder = GiftCodeFundingMemoBuilder::default(); + + // Set the gift code TxOut public key only + let mut rng: StdRng = SeedableRng::from_seed([0u8; 32]); + let gift_code_public_key = RistrettoPublic::from_random(&mut rng); + builder + .set_gift_code_tx_out_public_key(&gift_code_public_key) + .unwrap(); + + // Build the memo payload and get the data + let memo_payload = build_zero_value_change_memo(&mut builder).unwrap(); + let memo_data = memo_payload.get_memo_data(); + + // Verify memo data + let derived_note = MemoDecoder::decode_funding_note(memo_data); + let expected_hash = MemoDecoder::tx_out_public_key_short_hash(&gift_code_public_key); + let blank_note = ""; + assert_eq!(blank_note, derived_note); + assert_eq!( + expected_hash, + memo_data[0..GiftCodeFundingMemo::HASH_DATA_LEN] + ); + } + + #[test] + fn test_gift_code_funding_memo_fails_to_build_if_key_matches_empty_change_memo_public_key() { + // Create memo builder + let mut builder = GiftCodeFundingMemoBuilder::default(); + + // Set the gift code note and the public_key as the empty gift code public_key + let mut rng: StdRng = SeedableRng::from_seed([0u8; 32]); + let change_tx_public_key = RistrettoPublic::from_random(&mut rng); + let note = "It's MEMO TIME!!"; + builder + .set_gift_code_tx_out_public_key(&change_tx_public_key) + .unwrap(); + builder.set_funding_note(note).unwrap(); + + // Build a memo + let mut rng: StdRng = SeedableRng::from_seed([0u8; 32]); + let alice = AccountKey::random_with_fog(&mut rng); + let alice_address_book = ReservedDestination::from(&alice); + let change_amount = Amount::new(666, 666.into()); + let memo_context = MemoContext { + tx_public_key: &change_tx_public_key, + }; + let memo_payload = + builder.make_memo_for_change_output(change_amount, &alice_address_book, memo_context); + + // Assert memo creation fails + assert!(matches!(memo_payload, Err(NewMemoError::BadInputs(_)))); } #[test] - fn test_gift_code_funding_memo_fails_to_build_with_key_from_context() { - // Tests forthcoming + fn test_gift_code_funding_memo_fails_for_nonzero_amount() { + // Create memo builder + let mut builder = GiftCodeFundingMemoBuilder::default(); + + // Set the gift code TxOut public key and note + let mut rng: StdRng = SeedableRng::from_seed([0u8; 32]); + let gift_code_public_key = RistrettoPublic::from_random(&mut rng); + let note = "It's MEMO TIME!!"; + builder + .set_gift_code_tx_out_public_key(&gift_code_public_key) + .unwrap(); + builder.set_funding_note(note).unwrap(); + + // Build the memo payload and get the data + let amount = Amount::new(666, 0.into()); + let memo_payload = build_change_memo_with_amount(&mut builder, amount); + assert!(matches!(memo_payload, Err(NewMemoError::BadInputs(_)))); } #[test] fn test_gift_code_funding_memo_fails_for_more_than_one_change_memo() { - // Tests forthcoming + // Create memo builder + let mut builder = GiftCodeFundingMemoBuilder::default(); + + // Set the gift code TxOut public key and note + let mut rng: StdRng = SeedableRng::from_seed([0u8; 32]); + let gift_code_public_key = RistrettoPublic::from_random(&mut rng); + let note = "It's MEMO TIME!!"; + builder + .set_gift_code_tx_out_public_key(&gift_code_public_key) + .unwrap(); + builder.set_funding_note(note).unwrap(); + + // Build the memo payload and get the data + let memo_payload = build_zero_value_change_memo(&mut builder).unwrap(); + let memo_data = memo_payload.get_memo_data(); + + // Verify memo data + let derived_note = MemoDecoder::decode_funding_note(memo_data); + let expected_hash = MemoDecoder::tx_out_public_key_short_hash(&gift_code_public_key); + assert_eq!(note, derived_note); + assert_eq!( + expected_hash, + memo_data[0..GiftCodeFundingMemo::HASH_DATA_LEN] + ); + + // Try building another memo and assert failure + let memo_payload = build_zero_value_change_memo(&mut builder); + assert!(matches!( + memo_payload, + Err(NewMemoError::MultipleChangeOutputs) + )) } #[test] - fn test_gift_code_funding_memo_fields_are_cleared_properly() { - // Tests forthcoming + fn test_gift_code_funding_memo_fields_are_cleared_properly_and_fails_if_no_public_key() { + // Create memo builder + let mut builder = GiftCodeFundingMemoBuilder::default(); + + // Set the gift code TxOut public key and note + let mut rng: StdRng = SeedableRng::from_seed([0u8; 32]); + let gift_code_public_key = RistrettoPublic::from_random(&mut rng); + let note = "It's MEMO TIME!!"; + builder + .set_gift_code_tx_out_public_key(&gift_code_public_key) + .unwrap(); + builder.set_funding_note(note).unwrap(); + + // Clear the funding note + builder.clear_funding_note(); + + // Build the memo payload and get the data + let memo_payload = build_zero_value_change_memo(&mut builder).unwrap(); + let memo_data = memo_payload.get_memo_data(); + + // Verify note was cleared + let blank_note = ""; + let derived_note = MemoDecoder::decode_funding_note(memo_data); + let expected_hash = MemoDecoder::tx_out_public_key_short_hash(&gift_code_public_key); + assert_eq!(blank_note, derived_note); + assert_eq!( + expected_hash, + memo_data[0..GiftCodeFundingMemo::HASH_DATA_LEN] + ); + + // Create another memo builder + let mut builder_2 = GiftCodeFundingMemoBuilder::default(); + + // Set the gift code TxOut public key and note + builder_2 + .set_gift_code_tx_out_public_key(&gift_code_public_key) + .unwrap(); + builder_2.set_funding_note(note).unwrap(); + builder_2.clear_gift_code_tx_out_public_key(); + + // Build the memo payload and ensure we can't build it without the tx public key + let memo_payload_2 = build_zero_value_change_memo(&mut builder_2); + assert!(matches!(memo_payload_2, Err(NewMemoError::MissingInput(_)))); } #[test] fn test_gift_code_funding_memo_writes_unused_if_change_disabled() { - // Tests forthcoming + // Create memo builder + let mut builder = GiftCodeFundingMemoBuilder::default(); + + // Set the gift code TxOut public key and note + let mut rng: StdRng = SeedableRng::from_seed([0u8; 32]); + let gift_code_public_key = RistrettoPublic::from_random(&mut rng); + let note = "It's MEMO TIME!!"; + builder + .set_gift_code_tx_out_public_key(&gift_code_public_key) + .unwrap(); + builder.set_funding_note(note).unwrap(); + + // Disable change memos + builder.disable_gift_code_change_memo(); + + // Build the memo payload and get the data + let memo_payload = build_zero_value_change_memo(&mut builder).unwrap(); + let memo_data = memo_payload.get_memo_data(); + + // Assert data is equal to zero byte array + assert_eq!(memo_data, &[0u8; GiftCodeFundingMemo::MEMO_DATA_LEN]); + } + + #[test] + fn test_gift_code_funding_memo_fails_if_last_note_setting_attempt_failed() { + // Create memo builder + let mut builder = GiftCodeFundingMemoBuilder::default(); + + // Set the gift code TxOut public key + let mut rng: StdRng = SeedableRng::from_seed([0u8; 32]); + let gift_code_public_key = RistrettoPublic::from_random(&mut rng); + + // Attempt to set an invalid note longer than allowed bytes + let note_bytes = [b'6'; GiftCodeFundingMemo::NOTE_DATA_LEN + 1]; + let bad_note = std::str::from_utf8(¬e_bytes).unwrap(); + builder + .set_gift_code_tx_out_public_key(&gift_code_public_key) + .unwrap(); + let result = builder.set_funding_note(bad_note); + + // Ensure set method errors correctly + assert!(matches!(result, Err(NewMemoError::BadInputs(_)))); + + // Attempt to set the memo anyways and assert our attempt is a failure + let memo_payload = build_zero_value_change_memo(&mut builder); + assert!(matches!(memo_payload, Err(NewMemoError::BadInputs(_)))); + } + + #[test] + fn test_gift_code_funding_memo_succeeds_after_invalid_note_is_reset_with_valid_note() { + // Create memo builder + let mut builder = GiftCodeFundingMemoBuilder::default(); + + // Set the gift code TxOut public key + let mut rng: StdRng = SeedableRng::from_seed([0u8; 32]); + let gift_code_public_key = RistrettoPublic::from_random(&mut rng); + + // Attempt to set an invalid note longer than allowed bytes + let note_bytes = [b'6'; GiftCodeFundingMemo::NOTE_DATA_LEN + 1]; + let bad_note = std::str::from_utf8(¬e_bytes).unwrap(); + builder + .set_gift_code_tx_out_public_key(&gift_code_public_key) + .unwrap(); + let result = builder.set_funding_note(bad_note); + + // Ensure set method errors correctly + assert!(matches!(result, Err(NewMemoError::BadInputs(_)))); + + // Set a correct memo and ensure we can build a valid memo + let correct_note = "I'm also bad mwa-ha (but in a healthy, fun way)"; + builder.set_funding_note(correct_note).unwrap(); + let memo_payload = build_zero_value_change_memo(&mut builder).unwrap(); + let memo_data = memo_payload.get_memo_data(); + + // Verify memo data + let derived_note = MemoDecoder::decode_funding_note(memo_data); + let expected_hash = MemoDecoder::tx_out_public_key_short_hash(&gift_code_public_key); + assert_eq!(correct_note, derived_note); + assert_eq!( + expected_hash, + memo_data[0..GiftCodeFundingMemo::HASH_DATA_LEN] + ); } } diff --git a/transaction/std/src/memo_builder/gift_code_sender_memo_builder.rs b/transaction/std/src/memo_builder/gift_code_sender_memo_builder.rs index 4cc89349ab..5451df2507 100644 --- a/transaction/std/src/memo_builder/gift_code_sender_memo_builder.rs +++ b/transaction/std/src/memo_builder/gift_code_sender_memo_builder.rs @@ -31,8 +31,9 @@ pub struct GiftCodeSenderMemoBuilder { gift_code_change_memo_enabled: bool, // Whether we've already written the change memo wrote_change_memo: bool, - // Whetever we've set a valid note - wrote_valid_note: bool, + // Whether our last note setting attempt was invalid and the note hasn't + // been reset with a valid note or cleared + attempted_invalid_note: bool, } // Create an empty GiftCodeSenderMemoBuilder @@ -42,7 +43,7 @@ impl Default for GiftCodeSenderMemoBuilder { note: "".into(), gift_code_change_memo_enabled: true, wrote_change_memo: false, - wrote_valid_note: true, + attempted_invalid_note: false, } } } @@ -54,16 +55,18 @@ impl GiftCodeSenderMemoBuilder { /// 64 bytes. pub fn set_gift_code_sender_note(&mut self, note: &str) -> Result<(), NewMemoError> { if note.len() > GiftCodeSenderMemo::MEMO_DATA_LEN { - self.wrote_valid_note = false; + self.attempted_invalid_note = true; return Err(NewMemoError::BadInputs( "Sender note cannot be longer than 64 bytes".into(), )); } self.note = note.into(); + self.attempted_invalid_note = false; Ok(()) } /// Clear the gift code sender note pub fn clear_sender_note(&mut self) { + self.attempted_invalid_note = false; self.note = "".into(); } /// Enable change memos @@ -106,7 +109,8 @@ impl MemoBuilder for GiftCodeSenderMemoBuilder { if self.wrote_change_memo { return Err(NewMemoError::MultipleChangeOutputs); }; - if !self.wrote_valid_note { + // Prevent callers from writing memo if last note set attempt was a failure + if self.attempted_invalid_note { return Err(NewMemoError::BadInputs( "Tried to set a note longer than 64 bytes".into(), )); @@ -119,18 +123,7 @@ impl MemoBuilder for GiftCodeSenderMemoBuilder { #[cfg(test)] mod tests { use super::*; - use crate::test_utils::build_change_memo_with_amount; - - /// Get the sender note - fn get_sender_note(memo_data: &[u8; GiftCodeSenderMemo::MEMO_DATA_LEN]) -> &str { - let index = if let Some(terminator) = memo_data.iter().position(|b| b == &0u8) { - terminator - } else { - GiftCodeSenderMemo::MEMO_DATA_LEN - }; - - std::str::from_utf8(&memo_data[0..index]).unwrap() - } + use crate::test_utils::{build_change_memo_with_amount, MemoDecoder}; #[test] fn test_gift_code_sender_memo_built_successfully_with_note() { @@ -146,7 +139,7 @@ mod tests { let memo_payload = build_change_memo_with_amount(&mut builder, amount).unwrap(); // Verify memo data - let derived_note = get_sender_note(memo_payload.get_memo_data()); + let derived_note = MemoDecoder::decode_sender_note(memo_payload.get_memo_data()); assert_eq!(note, derived_note); } @@ -161,7 +154,7 @@ mod tests { // Verify memo data let blank_note = ""; - let derived_note = get_sender_note(memo_payload.get_memo_data()); + let derived_note = MemoDecoder::decode_sender_note(memo_payload.get_memo_data()); assert_eq!(blank_note, derived_note); } @@ -179,7 +172,7 @@ mod tests { let memo_payload = build_change_memo_with_amount(&mut builder, amount).unwrap(); // Verify memo data works once - let derived_note = get_sender_note(memo_payload.get_memo_data()); + let derived_note = MemoDecoder::decode_sender_note(memo_payload.get_memo_data()); assert_eq!(note, derived_note); // Verify memo_data doesn't work more than once @@ -206,7 +199,7 @@ mod tests { // Verify memo data let blank_note = ""; - let derived_note = get_sender_note(memo_payload.get_memo_data()); + let derived_note = MemoDecoder::decode_sender_note(memo_payload.get_memo_data()); assert_eq!(blank_note, derived_note); // Create another memo builder @@ -220,7 +213,7 @@ mod tests { // Build the memo payload and get the data let amount_2 = Amount::new(666, 0.into()); let memo_payload_2 = build_change_memo_with_amount(&mut builder_2, amount_2).unwrap(); - let derived_note_2 = get_sender_note(memo_payload_2.get_memo_data()); + let derived_note_2 = MemoDecoder::decode_sender_note(memo_payload_2.get_memo_data()); assert_eq!(note_2, derived_note_2); } @@ -243,19 +236,41 @@ mod tests { } #[test] - fn test_gift_code_sender_builder_doesnt_allow_invalid_note_length() { + fn test_gift_code_sender_note_builder_fails_after_attempting_invalid_note() { // Create memo builder let mut builder = GiftCodeSenderMemoBuilder::default(); - // Set an invalid note + // Attempt to set an invalid note let note_bytes = [b'6'; GiftCodeSenderMemo::MEMO_DATA_LEN + 1]; let note = std::str::from_utf8(¬e_bytes).unwrap(); let result = builder.set_gift_code_sender_note(note); assert!(matches!(result, Err(NewMemoError::BadInputs(_)))); - // Try to build after failing to set note + // Fail to build memo after failing to set valid note let amount = Amount::new(42, 0.into()); let memo_payload = build_change_memo_with_amount(&mut builder, amount); assert!(matches!(memo_payload, Err(NewMemoError::BadInputs(_)))); } + + #[test] + fn test_gift_code_sender_note_builder_succeeds_after_resetting_invalid_note_with_valid_note() { + // Create memo builder + let mut builder = GiftCodeSenderMemoBuilder::default(); + + // Attempt to set an invalid note + let note_bytes = [b'6'; GiftCodeSenderMemo::MEMO_DATA_LEN + 1]; + let note = std::str::from_utf8(¬e_bytes).unwrap(); + let result = builder.set_gift_code_sender_note(note); + assert!(matches!(result, Err(NewMemoError::BadInputs(_)))); + + // Reset with valid note + let valid_note = "Im within the byte length, but validity is a subjective concept"; + builder.set_gift_code_sender_note(valid_note).unwrap(); + let amount = Amount::new(42, 0.into()); + let memo_payload = build_change_memo_with_amount(&mut builder, amount).unwrap(); + + // Assert we can build the memo and get the correct output + let derived_note = MemoDecoder::decode_sender_note(memo_payload.get_memo_data()); + assert_eq!(valid_note, derived_note); + } } diff --git a/transaction/std/src/test_utils.rs b/transaction/std/src/test_utils.rs index a217a7ddab..48d6fb989c 100644 --- a/transaction/std/src/test_utils.rs +++ b/transaction/std/src/test_utils.rs @@ -3,11 +3,12 @@ //! Utilities that help with testing the transaction builder and related objects use crate::{ - EmptyMemoBuilder, InputCredentials, MemoBuilder, MemoPayload, ReservedDestination, - TransactionBuilder, TxBuilderError, + EmptyMemoBuilder, GiftCodeFundingMemo, GiftCodeSenderMemo, InputCredentials, MemoBuilder, + MemoPayload, ReservedDestination, TransactionBuilder, TxBuilderError, }; -use core::convert::TryFrom; +use core::convert::{TryFrom, TryInto}; use mc_account_keys::{AccountKey, PublicAddress, DEFAULT_SUBADDRESS_INDEX}; +use mc_crypto_hashes::{Blake2b512, Digest}; use mc_crypto_keys::RistrettoPublic; use mc_fog_report_validation::FogPubkeyResolver; use mc_transaction_core::{ @@ -230,3 +231,46 @@ pub fn build_change_memo_with_amount( //Build memo builder.make_memo_for_change_output(change_amount, &alice_address_book, memo_context) } + +/// Utilities for decoding and verifying memo data +pub struct MemoDecoder; + +impl MemoDecoder { + /// Get the sender note + pub fn decode_sender_note(memo_data: &[u8; GiftCodeSenderMemo::MEMO_DATA_LEN]) -> &str { + let index = if let Some(terminator) = memo_data.iter().position(|b| b == &0u8) { + terminator + } else { + GiftCodeSenderMemo::MEMO_DATA_LEN + }; + + std::str::from_utf8(&memo_data[0..index]).unwrap() + } + + /// Get funding note from memo + pub fn decode_funding_note(memo_data: &[u8; GiftCodeFundingMemo::MEMO_DATA_LEN]) -> &str { + let index = if let Some(terminator) = memo_data + .iter() + .enumerate() + .position(|(i, b)| i >= GiftCodeFundingMemo::HASH_DATA_LEN && b == &0u8) + { + terminator + } else { + GiftCodeFundingMemo::MEMO_DATA_LEN + }; + + std::str::from_utf8(&memo_data[GiftCodeFundingMemo::HASH_DATA_LEN..index]).unwrap() + } + + /// Get the first four bytes of a gift code TxOut public key + pub fn tx_out_public_key_short_hash( + tx_out_public_key: &RistrettoPublic, + ) -> [u8; GiftCodeFundingMemo::HASH_DATA_LEN] { + let mut hasher = Blake2b512::new(); + hasher.update("mc-gift-funding-tx-pub-key"); + hasher.update(tx_out_public_key.as_ref().compress().as_bytes()); + hasher.finalize().as_slice()[0..GiftCodeFundingMemo::HASH_DATA_LEN] + .try_into() + .unwrap() + } +} From 70ce48b92481c62d8d0708f9794739dc6483d218 Mon Sep 17 00:00:00 2001 From: iamalwaysuncomfortable Date: Sat, 14 May 2022 12:59:47 -0400 Subject: [PATCH 07/12] Small comment fixes --- transaction/std/src/test_utils.rs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/transaction/std/src/test_utils.rs b/transaction/std/src/test_utils.rs index 48d6fb989c..8e55cb4cf5 100644 --- a/transaction/std/src/test_utils.rs +++ b/transaction/std/src/test_utils.rs @@ -207,14 +207,14 @@ pub fn get_transaction transaction_builder.build(rng) } -/// Build simulated test memo with zero amount +/// Build simulated change memo with zero amount pub fn build_zero_value_change_memo( builder: &mut impl MemoBuilder, ) -> Result { build_change_memo_with_amount(builder, Amount::new(0, 0.into())) } -/// Build simulated test memo with amount +/// Build simulated change memo with amount pub fn build_change_memo_with_amount( builder: &mut impl MemoBuilder, change_amount: Amount, @@ -236,7 +236,7 @@ pub fn build_change_memo_with_amount( pub struct MemoDecoder; impl MemoDecoder { - /// Get the sender note + /// Decode a sender note from bytes pub fn decode_sender_note(memo_data: &[u8; GiftCodeSenderMemo::MEMO_DATA_LEN]) -> &str { let index = if let Some(terminator) = memo_data.iter().position(|b| b == &0u8) { terminator @@ -247,7 +247,7 @@ impl MemoDecoder { std::str::from_utf8(&memo_data[0..index]).unwrap() } - /// Get funding note from memo + /// Decode a funding note from bytes pub fn decode_funding_note(memo_data: &[u8; GiftCodeFundingMemo::MEMO_DATA_LEN]) -> &str { let index = if let Some(terminator) = memo_data .iter() @@ -262,7 +262,8 @@ impl MemoDecoder { std::str::from_utf8(&memo_data[GiftCodeFundingMemo::HASH_DATA_LEN..index]).unwrap() } - /// Get the first four bytes of a gift code TxOut public key + /// Get the first four bytes of the Blake2b hash of a gift code TxOut public + /// key pub fn tx_out_public_key_short_hash( tx_out_public_key: &RistrettoPublic, ) -> [u8; GiftCodeFundingMemo::HASH_DATA_LEN] { From 69143fd5a575e35f12605b56d703f953b5555d66 Mon Sep 17 00:00:00 2001 From: iamalwaysuncomfortable Date: Mon, 16 May 2022 12:02:59 -0400 Subject: [PATCH 08/12] Remove zero value restrictions, allow empty destination memos on cancellation/funding --- .../gift_code_cancellation_memo_builder.rs | 31 ++-------- .../gift_code_funding_memo_builder.rs | 62 +++++++------------ transaction/std/src/transaction_builder.rs | 2 +- 3 files changed, 28 insertions(+), 67 deletions(-) diff --git a/transaction/std/src/memo_builder/gift_code_cancellation_memo_builder.rs b/transaction/std/src/memo_builder/gift_code_cancellation_memo_builder.rs index a36cd39a6b..f4442e006f 100644 --- a/transaction/std/src/memo_builder/gift_code_cancellation_memo_builder.rs +++ b/transaction/std/src/memo_builder/gift_code_cancellation_memo_builder.rs @@ -19,7 +19,7 @@ use mc_transaction_core::{Amount, MemoContext, MemoPayload, NewMemoError}; /// This memo builder builds a gift code cancellation memo (0x0202). Gift code /// cancellation is defined as the sender sending the gift code TxOut at the /// gift code subaddress back to their default address prior to it being spent -/// by the receiver. When that happens a zero valued TxOut is sent to the gift +/// by the receiver. When that happens a change TxOut is sent to the gift /// code sender's change subaddress with a gift code cancellation memo that /// stores the index of the TxOut originally sent to the gift code subaddress /// when the gift code was funded. @@ -69,21 +69,20 @@ impl MemoBuilder for GiftCodeCancellationMemoBuilder { Ok(()) } - /// Gift code destination memos are not allowed - all gift code - /// memos accompany TxOuts sent to the change address + /// Gift code cancellation memos write blank memos to destination TxOut(s) fn make_memo_for_output( &mut self, _amount: Amount, _recipient: &PublicAddress, _memo_context: MemoContext, ) -> Result { - Err(NewMemoError::DestinationMemoNotAllowed) + Ok(UnusedMemo {}.into()) } /// Build a memo for a gift code change output fn make_memo_for_change_output( &mut self, - amount: Amount, + _amount: Amount, _change_destination: &ReservedDestination, _memo_context: MemoContext, ) -> Result { @@ -98,11 +97,6 @@ impl MemoBuilder for GiftCodeCancellationMemoBuilder { "Must specify gift code TxOut global index".into(), )); } - if amount.value > 0 { - return Err(NewMemoError::BadInputs( - "Cancellation memo TxOut should be zero valued".into(), - )); - } self.wrote_change_memo = true; if let Some(tx_out_global_index) = self.gift_code_tx_out_global_index.take() { Ok(GiftCodeCancellationMemo::from(tx_out_global_index).into()) @@ -117,7 +111,7 @@ impl MemoBuilder for GiftCodeCancellationMemoBuilder { #[cfg(test)] mod tests { use super::*; - use crate::test_utils::{build_change_memo_with_amount, build_zero_value_change_memo}; + use crate::test_utils::build_zero_value_change_memo; use std::convert::TryInto; #[test] @@ -168,21 +162,6 @@ mod tests { )); } - #[test] - fn test_gift_code_cancellation_memo_fails_for_nonzero_amount() { - // Create memo builder - let mut builder = GiftCodeCancellationMemoBuilder::default(); - - // Set the cancellation index - let index = 666; - builder.set_gift_code_tx_out_index(index); - - // Build the memo payload - let amount = Amount::new(666, 0.into()); - let memo_payload = build_change_memo_with_amount(&mut builder, amount); - assert!(matches!(memo_payload, Err(NewMemoError::BadInputs(_)))); - } - #[test] fn test_gift_code_cancellation_memo_fields_are_set_and_cleared_properly() { // Create memo builder diff --git a/transaction/std/src/memo_builder/gift_code_funding_memo_builder.rs b/transaction/std/src/memo_builder/gift_code_funding_memo_builder.rs index 3d41c7f91b..c11cf9cd70 100644 --- a/transaction/std/src/memo_builder/gift_code_funding_memo_builder.rs +++ b/transaction/std/src/memo_builder/gift_code_funding_memo_builder.rs @@ -19,13 +19,14 @@ use mc_transaction_core::{Amount, MemoContext, MemoPayload, NewMemoError}; /// | 0x0202 | Gift Code Cancellation Memo | /// This memo builder builds a gift code funding memo (0x0201). When a gift /// code is funded, the amount of the gift code is sent to a TxOut at the -/// Sender's reserved gift code subaddress and a second zero valued TxOut -/// is sent to the sender's reserved change subaddress with the gift code -/// funding memo attached. The funding memo will include the first 4 bytes -/// of the hash of the gift code TxOut sent to the sender's reserved gift -/// code subaddress and 60 bytes for an optional utf-8 memo. +/// Sender's reserved gift code subaddress and a second (potentially zero +/// valued) change TxOut is sent to the sender's reserved change subaddress +/// with the gift code funding memo attached. The funding memo will include +/// the first 4 bytes of the hash of the gift code TxOut sent to the +/// sender's reserved gift code subaddress and 60 bytes for an optional +/// utf-8 memo. /// -/// IMPORTANT NOTE: The public_key of the zero valued TxOut that the Gift Code +/// IMPORTANT NOTE: The public_key of the change TxOut that the Gift Code /// Funding Memo is written to is NOT the public_key that should be passed into /// set_gift_code_tx_out_public_key(tx_out_public_key). Instead the public_key /// of the TxOut sent to the gift code subaddress is what should be passed into @@ -40,6 +41,8 @@ pub struct GiftCodeFundingMemoBuilder { gift_code_change_memo_enabled: bool, // Whether we've already written the change memo wrote_change_memo: bool, + // Whether we've written an unused memo to the destination + wrote_destination_memo: bool, // Whether our last note setting attempt was invalid and the note hasn't // been reset with a valid note or cleared attempted_invalid_note: bool, @@ -53,6 +56,7 @@ impl Default for GiftCodeFundingMemoBuilder { gift_code_tx_out_public_key: None, gift_code_change_memo_enabled: true, wrote_change_memo: false, + wrote_destination_memo: false, attempted_invalid_note: false, } } @@ -82,8 +86,8 @@ impl GiftCodeFundingMemoBuilder { /// Set the TxOut public_key of the gift code TxOut sent to the /// reserved gift code subaddress. /// - /// IMPORTANT NOTE: Do NOT pass the public_key of the zero valued change - /// TxOut that the gift code memo is attached to as an argument. Doing + /// IMPORTANT NOTE: Do NOT pass the public_key of the change TxOut + /// that the gift code memo is attached to as an argument. Doing /// so will result in an error when attempting to build the memo. pub fn set_gift_code_tx_out_public_key( &mut self, @@ -112,21 +116,26 @@ impl MemoBuilder for GiftCodeFundingMemoBuilder { Ok(()) } - /// Gift code destination memos are not allowed - all gift code - /// memos accompany TxOuts sent to the change address + /// Gift code destination memos applied to the gift code TxOut + /// are empty. fn make_memo_for_output( &mut self, _amount: Amount, _recipient: &PublicAddress, _memo_context: MemoContext, ) -> Result { - Err(NewMemoError::DestinationMemoNotAllowed) + // Only one gift code should be funded + if self.wrote_destination_memo { + return Err(NewMemoError::OutputsAfterChange); + } + self.wrote_destination_memo = true; + Ok(UnusedMemo {}.into()) } /// Build a memo for a gift code change output fn make_memo_for_change_output( &mut self, - amount: Amount, + _amount: Amount, _change_destination: &ReservedDestination, memo_context: MemoContext, ) -> Result { @@ -136,11 +145,6 @@ impl MemoBuilder for GiftCodeFundingMemoBuilder { if self.wrote_change_memo { return Err(NewMemoError::MultipleChangeOutputs); } - if amount.value > 0 { - return Err(NewMemoError::BadInputs( - "Funding memo TxOut should be zero valued".into(), - )); - } // Prevent callers from writing memo if last note set attempt was a failure if self.attempted_invalid_note { return Err(NewMemoError::BadInputs( @@ -164,9 +168,7 @@ impl MemoBuilder for GiftCodeFundingMemoBuilder { #[cfg(test)] mod tests { use super::*; - use crate::test_utils::{ - build_change_memo_with_amount, build_zero_value_change_memo, MemoDecoder, - }; + use crate::test_utils::{build_zero_value_change_memo, MemoDecoder}; use mc_account_keys::AccountKey; use mc_util_from_random::FromRandom; use rand::{rngs::StdRng, SeedableRng}; @@ -255,26 +257,6 @@ mod tests { assert!(matches!(memo_payload, Err(NewMemoError::BadInputs(_)))); } - #[test] - fn test_gift_code_funding_memo_fails_for_nonzero_amount() { - // Create memo builder - let mut builder = GiftCodeFundingMemoBuilder::default(); - - // Set the gift code TxOut public key and note - let mut rng: StdRng = SeedableRng::from_seed([0u8; 32]); - let gift_code_public_key = RistrettoPublic::from_random(&mut rng); - let note = "It's MEMO TIME!!"; - builder - .set_gift_code_tx_out_public_key(&gift_code_public_key) - .unwrap(); - builder.set_funding_note(note).unwrap(); - - // Build the memo payload and get the data - let amount = Amount::new(666, 0.into()); - let memo_payload = build_change_memo_with_amount(&mut builder, amount); - assert!(matches!(memo_payload, Err(NewMemoError::BadInputs(_)))); - } - #[test] fn test_gift_code_funding_memo_fails_for_more_than_one_change_memo() { // Create memo builder diff --git a/transaction/std/src/transaction_builder.rs b/transaction/std/src/transaction_builder.rs index 2fe93f5935..1c77492da8 100644 --- a/transaction/std/src/transaction_builder.rs +++ b/transaction/std/src/transaction_builder.rs @@ -152,7 +152,7 @@ impl TransactionBuilder { /// /// Note: Before adding a signed_contingent_input, you probably want to: /// * validate it (call .validate()) - /// * check if key image appreared already (call .key_image()) + /// * check if key image appeared already (call .key_image()) /// * provide merkle proofs of membership for each ring member (see /// .tx_out_global_indices) /// From cac8cf9d05513c6a043f74e559c494cbbc599cf0 Mon Sep 17 00:00:00 2001 From: iamalwaysuncomfortable Date: Wed, 18 May 2022 00:27:01 -0400 Subject: [PATCH 09/12] Add transaction builder test for gift code flow + remove default pattern for memo builder in favor of new() The data needed to create the memo builders should only need to be initialized once. Doing so removes the room for error and in some cases, is necessary at initialization. Additionally tests of a simulated gift code funding & redemption flow were added. --- account-keys/src/account_keys.rs | 5 + account-keys/src/lib.rs | 2 +- .../gift_code_cancellation_memo_builder.rs | 151 +----- .../gift_code_funding_memo_builder.rs | 447 ++++++--------- .../gift_code_sender_memo_builder.rs | 235 +++----- transaction/std/src/test_utils.rs | 134 ++--- transaction/std/src/transaction_builder.rs | 511 +++++++++++++++++- 7 files changed, 843 insertions(+), 642 deletions(-) diff --git a/account-keys/src/account_keys.rs b/account-keys/src/account_keys.rs index f51bb29eb1..2ffd354bc1 100644 --- a/account-keys/src/account_keys.rs +++ b/account-keys/src/account_keys.rs @@ -417,6 +417,11 @@ impl AccountKey { self.subaddress_spend_private(CHANGE_SUBADDRESS_INDEX) } + /// The private spend key for the gift code subaddress + pub fn gift_code_subaddress_spend_private(&self) -> RistrettoPrivate { + self.subaddress_spend_private(GIFT_CODE_SUBADDRESS_INDEX) + } + /// The private spend key for the i^th subaddress. pub fn subaddress_spend_private(&self, index: u64) -> RistrettoPrivate { let a: &Scalar = self.view_private_key.as_ref(); diff --git a/account-keys/src/lib.rs b/account-keys/src/lib.rs index 015d3b6e84..d09e30603e 100644 --- a/account-keys/src/lib.rs +++ b/account-keys/src/lib.rs @@ -18,7 +18,7 @@ mod identity; pub use crate::{ account_keys::{ AccountKey, PublicAddress, CHANGE_SUBADDRESS_INDEX, DEFAULT_SUBADDRESS_INDEX, - INVALID_SUBADDRESS_INDEX, + GIFT_CODE_SUBADDRESS_INDEX, INVALID_SUBADDRESS_INDEX, }, address_hash::ShortAddressHash, burn_address::{burn_address, burn_address_view_private, BURN_ADDRESS_VIEW_PRIVATE}, diff --git a/transaction/std/src/memo_builder/gift_code_cancellation_memo_builder.rs b/transaction/std/src/memo_builder/gift_code_cancellation_memo_builder.rs index f4442e006f..aa38b61479 100644 --- a/transaction/std/src/memo_builder/gift_code_cancellation_memo_builder.rs +++ b/transaction/std/src/memo_builder/gift_code_cancellation_memo_builder.rs @@ -3,10 +3,7 @@ //! Defines the Memo Builder for the gift code cancellation memo (0x0202) //! specified in MCIP #32 -use super::{ - memo::{GiftCodeCancellationMemo, UnusedMemo}, - MemoBuilder, ReservedDestination, -}; +use super::{memo::GiftCodeCancellationMemo, MemoBuilder, ReservedDestination}; use mc_account_keys::PublicAddress; use mc_transaction_core::{Amount, MemoContext, MemoPayload, NewMemoError}; @@ -18,93 +15,58 @@ use mc_transaction_core::{Amount, MemoContext, MemoPayload, NewMemoError}; /// | -->0x0202<-- | Gift Code Cancellation Memo | /// This memo builder builds a gift code cancellation memo (0x0202). Gift code /// cancellation is defined as the sender sending the gift code TxOut at the -/// gift code subaddress back to their default address prior to it being spent -/// by the receiver. When that happens a change TxOut is sent to the gift -/// code sender's change subaddress with a gift code cancellation memo that -/// stores the index of the TxOut originally sent to the gift code subaddress -/// when the gift code was funded. +/// gift code subaddress back to their change address prior to it being spent +/// by the receiver. When that happens a gift code cancellation memo is +/// written to the change TxOut that stores the index of the TxOut originally +/// sent to the gift code subaddress when the gift code was funded. #[derive(Clone, Debug)] pub struct GiftCodeCancellationMemoBuilder { // Index of the gift code TxOut that was originally funded - gift_code_tx_out_global_index: Option, - // Whether or not to enable change memos - gift_code_change_memo_enabled: bool, + gift_code_tx_out_global_index: u64, // Whether we've already written the change memo wrote_change_memo: bool, } -// Create an empty GiftCodeCancellationMemoBuilder -impl Default for GiftCodeCancellationMemoBuilder { - fn default() -> Self { +impl GiftCodeCancellationMemoBuilder { + /// Initialize memo builder with the index of the originally + /// funded gift code TxOut + pub fn new(gift_code_tx_out_global_index: u64) -> Self { Self { - gift_code_tx_out_global_index: None, - gift_code_change_memo_enabled: true, + gift_code_tx_out_global_index, wrote_change_memo: false, } } } -impl GiftCodeCancellationMemoBuilder { - /// Set the index of the gift code TxOut that was cancelled - pub fn set_gift_code_tx_out_index(&mut self, tx_out_global_index: u64) { - self.gift_code_tx_out_global_index = Some(tx_out_global_index) - } - /// Clear the index - pub fn clear_gift_code_tx_out_index(&mut self) { - self.gift_code_tx_out_global_index = None; - } - /// Enable change memos - pub fn enable_gift_code_change_memo(&mut self) { - self.gift_code_change_memo_enabled = true; - } - /// Disable change memos - pub fn disable_gift_code_change_memo(&mut self) { - self.gift_code_change_memo_enabled = false; - } -} - impl MemoBuilder for GiftCodeCancellationMemoBuilder { /// Set the fee fn set_fee(&mut self, _fee: Amount) -> Result<(), NewMemoError> { Ok(()) } - /// Gift code cancellation memos write blank memos to destination TxOut(s) + /// Destination memos for gift code cancellation memos are not allowed fn make_memo_for_output( &mut self, _amount: Amount, _recipient: &PublicAddress, _memo_context: MemoContext, ) -> Result { - Ok(UnusedMemo {}.into()) + Err(NewMemoError::DestinationMemoNotAllowed) } - /// Build a memo for a gift code change output + /// Write the cancellation memo to the change output the gift code is + /// refunded to fn make_memo_for_change_output( &mut self, _amount: Amount, _change_destination: &ReservedDestination, _memo_context: MemoContext, ) -> Result { - if !self.gift_code_change_memo_enabled { - return Ok(UnusedMemo {}.into()); - } if self.wrote_change_memo { return Err(NewMemoError::MultipleChangeOutputs); } - if self.gift_code_tx_out_global_index.is_none() { - return Err(NewMemoError::MissingInput( - "Must specify gift code TxOut global index".into(), - )); - } self.wrote_change_memo = true; - if let Some(tx_out_global_index) = self.gift_code_tx_out_global_index.take() { - Ok(GiftCodeCancellationMemo::from(tx_out_global_index).into()) - } else { - Err(NewMemoError::MissingInput( - "Missing global index of TxOut to be cancelled".into(), - )) - } + Ok(GiftCodeCancellationMemo::from(self.gift_code_tx_out_global_index).into()) } } @@ -116,12 +78,9 @@ mod tests { #[test] fn test_gift_code_cancellation_memo_built_successfully_with_index() { - // Create memo builder - let mut builder = GiftCodeCancellationMemoBuilder::default(); - - // Set the cancellation index + // Set the cancellation index and create memo builder let index = 666; - builder.set_gift_code_tx_out_index(index); + let mut builder = GiftCodeCancellationMemoBuilder::new(index); // Build the memo payload and get the data let memo_payload = build_zero_value_change_memo(&mut builder).unwrap(); @@ -132,84 +91,20 @@ mod tests { assert_eq!(index, derived_index); } - #[test] - fn test_gift_code_cancellation_memo_fails_without_index() { - // Instantiate memo builder - let mut builder = GiftCodeCancellationMemoBuilder::default(); - - // Build the memo payload - let memo_payload = build_zero_value_change_memo(&mut builder); - - // Assert we've created the correct error - assert!(matches!(memo_payload, Err(NewMemoError::MissingInput(_)))); - } - #[test] fn test_gift_code_cancellation_memo_fails_for_more_than_one_change_memo() { - // Create memo builder - let mut builder = GiftCodeCancellationMemoBuilder::default(); - - // Set the cancellation index + // Set the cancellation index and create memo builder let index = 666; - builder.set_gift_code_tx_out_index(index); + let mut builder = GiftCodeCancellationMemoBuilder::new(index); - // Build the memo payload + // Attempt to build two change outputs build_zero_value_change_memo(&mut builder).unwrap(); let memo_payload = build_zero_value_change_memo(&mut builder); + + // Assert failure for the second output assert!(matches!( memo_payload, Err(NewMemoError::MultipleChangeOutputs) )); } - - #[test] - fn test_gift_code_cancellation_memo_fields_are_set_and_cleared_properly() { - // Create memo builder - let mut builder = GiftCodeCancellationMemoBuilder::default(); - - // Set the cancellation index, and then replace it - let index = 666; - builder.set_gift_code_tx_out_index(index); - let replacement_index = 420; - builder.set_gift_code_tx_out_index(replacement_index); - - // Build the memo payload and get the data - let memo_payload = build_zero_value_change_memo(&mut builder).unwrap(); - let memo_data = memo_payload.get_memo_data(); - - // Verify memo data - let derived_index = u64::from_le_bytes(memo_data[0..8].try_into().unwrap()); - assert_eq!(replacement_index, derived_index); - - // Create another memo builder - let mut builder_2 = GiftCodeCancellationMemoBuilder::default(); - - // Set the cancellation index and then clear it - let index_2 = 666; - builder_2.set_gift_code_tx_out_index(index_2); - builder_2.clear_gift_code_tx_out_index(); - - // Build the memo payload and get the data - let memo_payload_2 = build_zero_value_change_memo(&mut builder_2); - assert!(matches!(memo_payload_2, Err(NewMemoError::MissingInput(_)))); - } - - #[test] - fn test_gift_code_cancellation_memo_writes_unused_if_change_disabled() { - // Create memo builder - let mut builder = GiftCodeCancellationMemoBuilder::default(); - - // Set the cancellation index - let index = 666; - builder.set_gift_code_tx_out_index(index); - - // Disable change outputs - builder.disable_gift_code_change_memo(); - - // Build the memo payload and get the data - let memo_payload = build_zero_value_change_memo(&mut builder).unwrap(); - - // Verify memo data is blank - assert_eq!(memo_payload.get_memo_data(), &[0u8; 64]); - } } diff --git a/transaction/std/src/memo_builder/gift_code_funding_memo_builder.rs b/transaction/std/src/memo_builder/gift_code_funding_memo_builder.rs index c11cf9cd70..409f108758 100644 --- a/transaction/std/src/memo_builder/gift_code_funding_memo_builder.rs +++ b/transaction/std/src/memo_builder/gift_code_funding_memo_builder.rs @@ -25,88 +25,35 @@ use mc_transaction_core::{Amount, MemoContext, MemoPayload, NewMemoError}; /// the first 4 bytes of the hash of the gift code TxOut sent to the /// sender's reserved gift code subaddress and 60 bytes for an optional /// utf-8 memo. -/// -/// IMPORTANT NOTE: The public_key of the change TxOut that the Gift Code -/// Funding Memo is written to is NOT the public_key that should be passed into -/// set_gift_code_tx_out_public_key(tx_out_public_key). Instead the public_key -/// of the TxOut sent to the gift code subaddress is what should be passed into -/// set_gift_code_tx_out_public_key(tx_out_public_key) #[derive(Clone, Debug)] pub struct GiftCodeFundingMemoBuilder { // Utf-8 note within the memo that can be up to 60 bytes long note: String, // TxOut Public Key of the gift code TxOut sent to the gift code subaddress gift_code_tx_out_public_key: Option, - // Whether or not to enable change memo - gift_code_change_memo_enabled: bool, // Whether we've already written the change memo wrote_change_memo: bool, - // Whether we've written an unused memo to the destination + // Whether we've already written the gift code TxOut wrote_destination_memo: bool, - // Whether our last note setting attempt was invalid and the note hasn't - // been reset with a valid note or cleared - attempted_invalid_note: bool, -} - -// Create an empty GiftCodeFundingMemoBuilder -impl Default for GiftCodeFundingMemoBuilder { - fn default() -> Self { - Self { - note: "".into(), - gift_code_tx_out_public_key: None, - gift_code_change_memo_enabled: true, - wrote_change_memo: false, - wrote_destination_memo: false, - attempted_invalid_note: false, - } - } } impl GiftCodeFundingMemoBuilder { - /// Set a utf-8 note (up to 60 bytes) onto the funding memo indicating - /// what the gift code was for. This method will enforce the 60 byte - /// limit with a NewMemoErr if the &str passed is longer than - /// 60 bytes. - pub fn set_funding_note(&mut self, note: &str) -> Result<(), NewMemoError> { + /// Initialize memo builder with a utf-8 note (up to 60 bytes) + /// indicating what the gift code was for. This method will enforce + /// the 60 byte limit with a NewMemoErr if the &str passed is longer + /// than 60 bytes. + pub fn new(note: &str) -> Result { if note.len() > GiftCodeFundingMemo::NOTE_DATA_LEN { - self.attempted_invalid_note = true; return Err(NewMemoError::BadInputs( "Note memo cannot be greater than 60 bytes".into(), )); } - self.attempted_invalid_note = false; - self.note = note.into(); - Ok(()) - } - /// Clear the gift code funding note - pub fn clear_funding_note(&mut self) { - self.attempted_invalid_note = false; - self.note = "".into(); - } - /// Set the TxOut public_key of the gift code TxOut sent to the - /// reserved gift code subaddress. - /// - /// IMPORTANT NOTE: Do NOT pass the public_key of the change TxOut - /// that the gift code memo is attached to as an argument. Doing - /// so will result in an error when attempting to build the memo. - pub fn set_gift_code_tx_out_public_key( - &mut self, - tx_out_public_key: &RistrettoPublic, - ) -> Result<(), NewMemoError> { - self.gift_code_tx_out_public_key = Some(*tx_out_public_key); - Ok(()) - } - /// Clear the gift code tx_out_public_key - pub fn clear_gift_code_tx_out_public_key(&mut self) { - self.gift_code_tx_out_public_key = None; - } - /// Enable change memos - pub fn enable_gift_code_change_memo(&mut self) { - self.gift_code_change_memo_enabled = true; - } - /// Disable change memos - pub fn disable_gift_code_change_memo(&mut self) { - self.gift_code_change_memo_enabled = false; + Ok(Self { + note: note.into(), + gift_code_tx_out_public_key: None, + wrote_change_memo: false, + wrote_destination_memo: false, + }) } } @@ -116,41 +63,41 @@ impl MemoBuilder for GiftCodeFundingMemoBuilder { Ok(()) } - /// Gift code destination memos applied to the gift code TxOut - /// are empty. + /// This method is called when writing the gift TxOut to the reserved + /// gift code subaddress and can only be called once. Once called it + /// will store the public key of the gift code TxOut in the memo + /// builder. fn make_memo_for_output( &mut self, _amount: Amount, _recipient: &PublicAddress, - _memo_context: MemoContext, + memo_context: MemoContext, ) -> Result { // Only one gift code should be funded if self.wrote_destination_memo { + return Err(NewMemoError::MultipleOutputs); + } + if self.wrote_change_memo { return Err(NewMemoError::OutputsAfterChange); } + self.gift_code_tx_out_public_key = Some(*memo_context.tx_public_key); self.wrote_destination_memo = true; Ok(UnusedMemo {}.into()) } - /// Build a memo for a gift code change output + /// Write the funding memo onto the change output of the gift code TxOut fn make_memo_for_change_output( &mut self, _amount: Amount, _change_destination: &ReservedDestination, memo_context: MemoContext, ) -> Result { - if !self.gift_code_change_memo_enabled { - return Ok(UnusedMemo {}.into()); + if !self.wrote_destination_memo { + return Err(NewMemoError::MissingOutput); } if self.wrote_change_memo { return Err(NewMemoError::MultipleChangeOutputs); } - // Prevent callers from writing memo if last note set attempt was a failure - if self.attempted_invalid_note { - return Err(NewMemoError::BadInputs( - "Tried to set a note longer than 64 bytes".into(), - )); - } if self.gift_code_tx_out_public_key.as_ref() == Some(memo_context.tx_public_key) { return Err(NewMemoError::BadInputs("The public_key in the memo should be the TxOut at the gift code subaddress and not that of the memo TxOut".into())); } @@ -168,259 +115,203 @@ impl MemoBuilder for GiftCodeFundingMemoBuilder { #[cfg(test)] mod tests { use super::*; - use crate::test_utils::{build_zero_value_change_memo, MemoDecoder}; use mc_account_keys::AccountKey; use mc_util_from_random::FromRandom; use rand::{rngs::StdRng, SeedableRng}; - #[test] - fn test_gift_code_funding_memo_built_successfully_with_note() { - // Create memo builder - let mut builder = GiftCodeFundingMemoBuilder::default(); - - // Set the gift code TxOut public key and note - let mut rng: StdRng = SeedableRng::from_seed([0u8; 32]); - let gift_code_public_key = RistrettoPublic::from_random(&mut rng); - let note = "It's MEMO TIME!!"; - builder - .set_gift_code_tx_out_public_key(&gift_code_public_key) - .unwrap(); - builder.set_funding_note(note).unwrap(); - - // Build the memo payload and get the data - let memo_payload = build_zero_value_change_memo(&mut builder).unwrap(); - let memo_data = memo_payload.get_memo_data(); - - // Verify memo data - let derived_note = MemoDecoder::decode_funding_note(memo_data); - let expected_hash = MemoDecoder::tx_out_public_key_short_hash(&gift_code_public_key); - assert_eq!(note, derived_note); - assert_eq!( - expected_hash, - memo_data[0..GiftCodeFundingMemo::HASH_DATA_LEN] - ); - } - - #[test] - fn test_gift_code_funding_memo_built_successfully_without_note() { - // Create memo builder - let mut builder = GiftCodeFundingMemoBuilder::default(); - - // Set the gift code TxOut public key only - let mut rng: StdRng = SeedableRng::from_seed([0u8; 32]); - let gift_code_public_key = RistrettoPublic::from_random(&mut rng); - builder - .set_gift_code_tx_out_public_key(&gift_code_public_key) - .unwrap(); - - // Build the memo payload and get the data - let memo_payload = build_zero_value_change_memo(&mut builder).unwrap(); - let memo_data = memo_payload.get_memo_data(); - - // Verify memo data - let derived_note = MemoDecoder::decode_funding_note(memo_data); - let expected_hash = MemoDecoder::tx_out_public_key_short_hash(&gift_code_public_key); - let blank_note = ""; - assert_eq!(blank_note, derived_note); - assert_eq!( - expected_hash, - memo_data[0..GiftCodeFundingMemo::HASH_DATA_LEN] - ); - } - - #[test] - fn test_gift_code_funding_memo_fails_to_build_if_key_matches_empty_change_memo_public_key() { - // Create memo builder - let mut builder = GiftCodeFundingMemoBuilder::default(); - - // Set the gift code note and the public_key as the empty gift code public_key - let mut rng: StdRng = SeedableRng::from_seed([0u8; 32]); - let change_tx_public_key = RistrettoPublic::from_random(&mut rng); - let note = "It's MEMO TIME!!"; - builder - .set_gift_code_tx_out_public_key(&change_tx_public_key) - .unwrap(); - builder.set_funding_note(note).unwrap(); - - // Build a memo + fn build_gift_code_memos( + builder: &mut impl MemoBuilder, + funding_tx_pubkey: &RistrettoPublic, + ) -> Result { + // Create simulated context let mut rng: StdRng = SeedableRng::from_seed([0u8; 32]); let alice = AccountKey::random_with_fog(&mut rng); let alice_address_book = ReservedDestination::from(&alice); - let change_amount = Amount::new(666, 666.into()); - let memo_context = MemoContext { - tx_public_key: &change_tx_public_key, + let change_tx_pubkey = RistrettoPublic::from_random(&mut rng); + let change_amount = Amount::new(1, 0.into()); + let funding_amount = Amount::new(10, 0.into()); + let funding_context = MemoContext { + tx_public_key: &funding_tx_pubkey, + }; + let change_context = MemoContext { + tx_public_key: &change_tx_pubkey, }; - let memo_payload = - builder.make_memo_for_change_output(change_amount, &alice_address_book, memo_context); - // Assert memo creation fails - assert!(matches!(memo_payload, Err(NewMemoError::BadInputs(_)))); + // Build blank output memo for TxOut at gift code address & funding memo to + // change output + builder + .make_memo_for_output( + funding_amount, + &alice_address_book.gift_code_subaddress, + funding_context, + ) + .unwrap(); + builder.make_memo_for_change_output(change_amount, &alice_address_book, change_context) } #[test] - fn test_gift_code_funding_memo_fails_for_more_than_one_change_memo() { - // Create memo builder - let mut builder = GiftCodeFundingMemoBuilder::default(); - - // Set the gift code TxOut public key and note + fn test_gift_code_funding_memo_built_successfully_with_note() { + // Create Memo Builder with note let mut rng: StdRng = SeedableRng::from_seed([0u8; 32]); let gift_code_public_key = RistrettoPublic::from_random(&mut rng); let note = "It's MEMO TIME!!"; - builder - .set_gift_code_tx_out_public_key(&gift_code_public_key) - .unwrap(); - builder.set_funding_note(note).unwrap(); + let mut builder = GiftCodeFundingMemoBuilder::new(note).unwrap(); // Build the memo payload and get the data - let memo_payload = build_zero_value_change_memo(&mut builder).unwrap(); - let memo_data = memo_payload.get_memo_data(); + let memo_payload = build_gift_code_memos(&mut builder, &gift_code_public_key).unwrap(); // Verify memo data - let derived_note = MemoDecoder::decode_funding_note(memo_data); - let expected_hash = MemoDecoder::tx_out_public_key_short_hash(&gift_code_public_key); + let memo = GiftCodeFundingMemo::from(memo_payload.get_memo_data()); + let derived_note = memo.funding_note().unwrap(); assert_eq!(note, derived_note); - assert_eq!( - expected_hash, - memo_data[0..GiftCodeFundingMemo::HASH_DATA_LEN] - ); - - // Try building another memo and assert failure - let memo_payload = build_zero_value_change_memo(&mut builder); - assert!(matches!( - memo_payload, - Err(NewMemoError::MultipleChangeOutputs) - )) + assert!(memo.public_key_matches(&gift_code_public_key)); } #[test] - fn test_gift_code_funding_memo_fields_are_cleared_properly_and_fails_if_no_public_key() { - // Create memo builder - let mut builder = GiftCodeFundingMemoBuilder::default(); - - // Set the gift code TxOut public key and note + fn test_gift_code_funding_memo_built_successfully_with_edge_case_notes() { + // Create memo builder with blank note + let blank_note = ""; + let note_minus_one = + std::str::from_utf8(&[b'6'; GiftCodeFundingMemo::NOTE_DATA_LEN - 1]).unwrap(); + let note_exact = std::str::from_utf8(&[b'6'; GiftCodeFundingMemo::NOTE_DATA_LEN]).unwrap(); let mut rng: StdRng = SeedableRng::from_seed([0u8; 32]); let gift_code_public_key = RistrettoPublic::from_random(&mut rng); - let note = "It's MEMO TIME!!"; - builder - .set_gift_code_tx_out_public_key(&gift_code_public_key) - .unwrap(); - builder.set_funding_note(note).unwrap(); - - // Clear the funding note - builder.clear_funding_note(); - // Build the memo payload and get the data - let memo_payload = build_zero_value_change_memo(&mut builder).unwrap(); - let memo_data = memo_payload.get_memo_data(); + // Test blank note is okay + { + let mut builder = GiftCodeFundingMemoBuilder::new(blank_note).unwrap(); - // Verify note was cleared - let blank_note = ""; - let derived_note = MemoDecoder::decode_funding_note(memo_data); - let expected_hash = MemoDecoder::tx_out_public_key_short_hash(&gift_code_public_key); - assert_eq!(blank_note, derived_note); - assert_eq!( - expected_hash, - memo_data[0..GiftCodeFundingMemo::HASH_DATA_LEN] - ); + // Build the memo payload and get the data + let memo_payload = build_gift_code_memos(&mut builder, &gift_code_public_key).unwrap(); - // Create another memo builder - let mut builder_2 = GiftCodeFundingMemoBuilder::default(); - - // Set the gift code TxOut public key and note - builder_2 - .set_gift_code_tx_out_public_key(&gift_code_public_key) - .unwrap(); - builder_2.set_funding_note(note).unwrap(); - builder_2.clear_gift_code_tx_out_public_key(); + // Verify memo data + let memo = GiftCodeFundingMemo::from(memo_payload.get_memo_data()); + let derived_note = memo.funding_note().unwrap(); + assert_eq!(blank_note, derived_note); + assert!(memo.public_key_matches(&gift_code_public_key)); + } - // Build the memo payload and ensure we can't build it without the tx public key - let memo_payload_2 = build_zero_value_change_memo(&mut builder_2); - assert!(matches!(memo_payload_2, Err(NewMemoError::MissingInput(_)))); - } + // Test note with max length minus one is okay + { + let mut builder = GiftCodeFundingMemoBuilder::new(note_minus_one).unwrap(); - #[test] - fn test_gift_code_funding_memo_writes_unused_if_change_disabled() { - // Create memo builder - let mut builder = GiftCodeFundingMemoBuilder::default(); + // Build the memo payload and get the data + let memo_payload = build_gift_code_memos(&mut builder, &gift_code_public_key).unwrap(); - // Set the gift code TxOut public key and note - let mut rng: StdRng = SeedableRng::from_seed([0u8; 32]); - let gift_code_public_key = RistrettoPublic::from_random(&mut rng); - let note = "It's MEMO TIME!!"; - builder - .set_gift_code_tx_out_public_key(&gift_code_public_key) - .unwrap(); - builder.set_funding_note(note).unwrap(); + // Verify memo data + let memo = GiftCodeFundingMemo::from(memo_payload.get_memo_data()); + let derived_note = memo.funding_note().unwrap(); + assert_eq!(note_minus_one, derived_note); + assert!(memo.public_key_matches(&gift_code_public_key)); + } - // Disable change memos - builder.disable_gift_code_change_memo(); + // Test max length note is okay + { + let mut builder = GiftCodeFundingMemoBuilder::new(note_exact).unwrap(); - // Build the memo payload and get the data - let memo_payload = build_zero_value_change_memo(&mut builder).unwrap(); - let memo_data = memo_payload.get_memo_data(); + // Build the memo payload and get the data + let memo_payload = build_gift_code_memos(&mut builder, &gift_code_public_key).unwrap(); - // Assert data is equal to zero byte array - assert_eq!(memo_data, &[0u8; GiftCodeFundingMemo::MEMO_DATA_LEN]); + // Verify memo data + let memo = GiftCodeFundingMemo::from(memo_payload.get_memo_data()); + let derived_note = memo.funding_note().unwrap(); + assert_eq!(note_exact, derived_note); + assert!(memo.public_key_matches(&gift_code_public_key)); + } } #[test] - fn test_gift_code_funding_memo_fails_if_last_note_setting_attempt_failed() { - // Create memo builder - let mut builder = GiftCodeFundingMemoBuilder::default(); - - // Set the gift code TxOut public key + fn test_gift_code_funding_memo_fails_to_build_if_key_matches_change_memo_public_key() { + // Create memo builder with note let mut rng: StdRng = SeedableRng::from_seed([0u8; 32]); - let gift_code_public_key = RistrettoPublic::from_random(&mut rng); + let note = "It's MEMO TIME!!"; + let mut builder = GiftCodeFundingMemoBuilder::new(note).unwrap(); - // Attempt to set an invalid note longer than allowed bytes - let note_bytes = [b'6'; GiftCodeFundingMemo::NOTE_DATA_LEN + 1]; - let bad_note = std::str::from_utf8(¬e_bytes).unwrap(); + // Build a memo + let alice = AccountKey::random_with_fog(&mut rng); + let alice_address_book = ReservedDestination::from(&alice); + let change_amount = Amount::new(666, 666.into()); + + // Erroneously set change TxOut public address to the funding TxOut + let change_tx_public_key = RistrettoPublic::from_random(&mut rng); builder - .set_gift_code_tx_out_public_key(&gift_code_public_key) + .make_memo_for_output( + change_amount, + &alice_address_book.gift_code_subaddress, + MemoContext { + tx_public_key: &change_tx_public_key, + }, + ) .unwrap(); - let result = builder.set_funding_note(bad_note); - - // Ensure set method errors correctly - assert!(matches!(result, Err(NewMemoError::BadInputs(_)))); + let memo_payload = builder.make_memo_for_change_output( + change_amount, + &alice_address_book, + MemoContext { + tx_public_key: &change_tx_public_key, + }, + ); - // Attempt to set the memo anyways and assert our attempt is a failure - let memo_payload = build_zero_value_change_memo(&mut builder); + // Assert memo creation fails assert!(matches!(memo_payload, Err(NewMemoError::BadInputs(_)))); } #[test] - fn test_gift_code_funding_memo_succeeds_after_invalid_note_is_reset_with_valid_note() { - // Create memo builder - let mut builder = GiftCodeFundingMemoBuilder::default(); - - // Set the gift code TxOut public key + fn test_gift_code_funding_memos_fail_for_multiple_change_memos() { + // Create memo builder with note let mut rng: StdRng = SeedableRng::from_seed([0u8; 32]); - let gift_code_public_key = RistrettoPublic::from_random(&mut rng); + let note = "It's MEMO TIME!!"; + let mut builder = GiftCodeFundingMemoBuilder::new(note).unwrap(); - // Attempt to set an invalid note longer than allowed bytes - let note_bytes = [b'6'; GiftCodeFundingMemo::NOTE_DATA_LEN + 1]; - let bad_note = std::str::from_utf8(¬e_bytes).unwrap(); + // Build a memo + let alice = AccountKey::random_with_fog(&mut rng); + let alice_address_book = ReservedDestination::from(&alice); + let change_amount = Amount::new(666, 666.into()); + + // Write 2 change outputs + let funding_tx_out_public_key = RistrettoPublic::from_random(&mut rng); + let change_tx_public_key = RistrettoPublic::from_random(&mut rng); + let change_tx_public_key_2 = RistrettoPublic::from_random(&mut rng); + builder + .make_memo_for_output( + change_amount, + &alice_address_book.gift_code_subaddress, + MemoContext { + tx_public_key: &funding_tx_out_public_key, + }, + ) + .unwrap(); builder - .set_gift_code_tx_out_public_key(&gift_code_public_key) + .make_memo_for_change_output( + change_amount, + &alice_address_book, + MemoContext { + tx_public_key: &change_tx_public_key, + }, + ) .unwrap(); - let result = builder.set_funding_note(bad_note); + let memo_payload = builder.make_memo_for_change_output( + change_amount, + &alice_address_book, + MemoContext { + tx_public_key: &change_tx_public_key_2, + }, + ); - // Ensure set method errors correctly - assert!(matches!(result, Err(NewMemoError::BadInputs(_)))); + // Assert memo creation fails for second change output + assert!(matches!( + memo_payload, + Err(NewMemoError::MultipleChangeOutputs) + )); + } - // Set a correct memo and ensure we can build a valid memo - let correct_note = "I'm also bad mwa-ha (but in a healthy, fun way)"; - builder.set_funding_note(correct_note).unwrap(); - let memo_payload = build_zero_value_change_memo(&mut builder).unwrap(); - let memo_data = memo_payload.get_memo_data(); + #[test] + fn test_gift_code_sender_note_builder_creation_fails_with_invalid_note() { + // Create Memo Builder with Bad Inputs + let note_bytes = [b'6'; GiftCodeFundingMemo::NOTE_DATA_LEN + 1]; + let note = std::str::from_utf8(¬e_bytes).unwrap(); + let builder = GiftCodeFundingMemoBuilder::new(note); - // Verify memo data - let derived_note = MemoDecoder::decode_funding_note(memo_data); - let expected_hash = MemoDecoder::tx_out_public_key_short_hash(&gift_code_public_key); - assert_eq!(correct_note, derived_note); - assert_eq!( - expected_hash, - memo_data[0..GiftCodeFundingMemo::HASH_DATA_LEN] - ); + //Assert memo creation fails + assert!(matches!(builder, Err(NewMemoError::BadInputs(_)))); } } diff --git a/transaction/std/src/memo_builder/gift_code_sender_memo_builder.rs b/transaction/std/src/memo_builder/gift_code_sender_memo_builder.rs index 5451df2507..0b39cc1214 100644 --- a/transaction/std/src/memo_builder/gift_code_sender_memo_builder.rs +++ b/transaction/std/src/memo_builder/gift_code_sender_memo_builder.rs @@ -3,10 +3,7 @@ //! Defines the Memo Builder for the gift code sender memo (0x0002) //! specified in MCIP #32 -use crate::{ - memo::{GiftCodeSenderMemo, UnusedMemo}, - MemoBuilder, ReservedDestination, -}; +use crate::{memo::GiftCodeSenderMemo, MemoBuilder, ReservedDestination}; use mc_account_keys::PublicAddress; use mc_transaction_core::{Amount, MemoContext, MemoPayload, NewMemoError}; @@ -27,55 +24,25 @@ use mc_transaction_core::{Amount, MemoContext, MemoPayload, NewMemoError}; pub struct GiftCodeSenderMemoBuilder { // Utf-8 formatted note within the memo that can be up to 64 bytes long note: String, - // Whether or not to enable change memos - gift_code_change_memo_enabled: bool, // Whether we've already written the change memo wrote_change_memo: bool, - // Whether our last note setting attempt was invalid and the note hasn't - // been reset with a valid note or cleared - attempted_invalid_note: bool, -} - -// Create an empty GiftCodeSenderMemoBuilder -impl Default for GiftCodeSenderMemoBuilder { - fn default() -> Self { - Self { - note: "".into(), - gift_code_change_memo_enabled: true, - wrote_change_memo: false, - attempted_invalid_note: false, - } - } } impl GiftCodeSenderMemoBuilder { - /// Set a utf-8 note (up to 64 bytes) onto the sender memo indicating - /// any desired info about the gift code. This method will enforce the - /// 64 byte limit with a NewMemoErr if the &str passed is longer than - /// 64 bytes. - pub fn set_gift_code_sender_note(&mut self, note: &str) -> Result<(), NewMemoError> { + /// Initialize memo builder with a utf-8 note (up to 64 bytes) + /// indicating what the gift code was for. This method will enforce + /// the 64 byte limit with a NewMemoErr if the &str passed is longer + /// than 64 bytes. + pub fn new(note: &str) -> Result { if note.len() > GiftCodeSenderMemo::MEMO_DATA_LEN { - self.attempted_invalid_note = true; return Err(NewMemoError::BadInputs( "Sender note cannot be longer than 64 bytes".into(), )); } - self.note = note.into(); - self.attempted_invalid_note = false; - Ok(()) - } - /// Clear the gift code sender note - pub fn clear_sender_note(&mut self) { - self.attempted_invalid_note = false; - self.note = "".into(); - } - /// Enable change memos - pub fn enable_gift_code_change_memo(&mut self) { - self.gift_code_change_memo_enabled = true; - } - /// Disable change memos - pub fn disable_gift_code_change_memo(&mut self) { - self.gift_code_change_memo_enabled = false; + Ok(Self { + note: note.into(), + wrote_change_memo: false, + }) } } @@ -85,8 +52,7 @@ impl MemoBuilder for GiftCodeSenderMemoBuilder { Ok(()) } - /// Gift code destination memos are not allowed - all gift code - /// memos accompany TxOuts sent to the change address + /// Destination memos are not allowed for gift code sender memos fn make_memo_for_output( &mut self, _amount: Amount, @@ -96,25 +62,17 @@ impl MemoBuilder for GiftCodeSenderMemoBuilder { Err(NewMemoError::DestinationMemoNotAllowed) } - /// Build a memo for a gift code change output + /// Write the sender memo to the TxOut the receiver sends to their change + /// address when the gift code is redeemed fn make_memo_for_change_output( &mut self, _amount: Amount, _change_destination: &ReservedDestination, _memo_context: MemoContext, ) -> Result { - if !self.gift_code_change_memo_enabled { - return Ok(UnusedMemo {}.into()); - }; if self.wrote_change_memo { return Err(NewMemoError::MultipleChangeOutputs); }; - // Prevent callers from writing memo if last note set attempt was a failure - if self.attempted_invalid_note { - return Err(NewMemoError::BadInputs( - "Tried to set a note longer than 64 bytes".into(), - )); - } self.wrote_change_memo = true; Ok(GiftCodeSenderMemo::new(self.note.as_str())?.into()) } @@ -122,155 +80,100 @@ impl MemoBuilder for GiftCodeSenderMemoBuilder { #[cfg(test)] mod tests { + use super::*; - use crate::test_utils::{build_change_memo_with_amount, MemoDecoder}; + use crate::test_utils::build_change_memo_with_amount; #[test] fn test_gift_code_sender_memo_built_successfully_with_note() { - // Create memo builder - let mut builder = GiftCodeSenderMemoBuilder::default(); - - // Set the note + // Create memo builder with note let note = "It's MEMO TIME!!"; - builder.set_gift_code_sender_note(note).unwrap(); + let mut builder = GiftCodeSenderMemoBuilder::new(note).unwrap(); // Build the memo payload and get the data let amount = Amount::new(42, 0.into()); let memo_payload = build_change_memo_with_amount(&mut builder, amount).unwrap(); // Verify memo data - let derived_note = MemoDecoder::decode_sender_note(memo_payload.get_memo_data()); - assert_eq!(note, derived_note); + let derived_note = GiftCodeSenderMemo::from(memo_payload.get_memo_data()); + assert_eq!(note, derived_note.sender_note().unwrap()); } #[test] - fn test_gift_code_sender_memo_built_successfully_without_note() { - // Create memo builder - let mut builder = GiftCodeSenderMemoBuilder::default(); - - // Build the memo payload and get the data + fn test_gift_code_sender_memo_built_successfully_with_blank_note_and_notes_close_to_max_length() + { + // Create edge case notes let amount = Amount::new(42, 0.into()); - let memo_payload = build_change_memo_with_amount(&mut builder, amount).unwrap(); - - // Verify memo data let blank_note = ""; - let derived_note = MemoDecoder::decode_sender_note(memo_payload.get_memo_data()); - assert_eq!(blank_note, derived_note); - } - - #[test] - fn test_gift_code_sender_memo_fails_for_more_than_one_change_memo() { - // Create memo builder - let mut builder = GiftCodeSenderMemoBuilder::default(); - - // Set the note - let note = "It's MEMO TIME!!"; - builder.set_gift_code_sender_note(note).unwrap(); - - // Build the memo payload and get the data - let amount = Amount::new(42, 0.into()); - let memo_payload = build_change_memo_with_amount(&mut builder, amount).unwrap(); + let note_minus_one = + std::str::from_utf8(&[b'6'; GiftCodeSenderMemo::MEMO_DATA_LEN - 1]).unwrap(); + let note_exact = std::str::from_utf8(&[b'6'; GiftCodeSenderMemo::MEMO_DATA_LEN]).unwrap(); - // Verify memo data works once - let derived_note = MemoDecoder::decode_sender_note(memo_payload.get_memo_data()); - assert_eq!(note, derived_note); + // Verify blank note is okay + { + let mut builder = GiftCodeSenderMemoBuilder::new(blank_note).unwrap(); - // Verify memo_data doesn't work more than once - let amount_2 = Amount::new(42, 0.into()); - let memo_payload_2 = build_change_memo_with_amount(&mut builder, amount_2); + // Build the memo payload and get the data + let memo_payload = build_change_memo_with_amount(&mut builder, amount).unwrap(); - let matches = matches!(memo_payload_2, Err(NewMemoError::MultipleChangeOutputs)); - assert!(matches); - } + // Verify memo data + let derived_note = GiftCodeSenderMemo::from(memo_payload.get_memo_data()); + assert_eq!(blank_note, derived_note.sender_note().unwrap()); + } - #[test] - fn test_gift_code_sender_memo_fields_are_cleared_properly() { - // Create memo builder - let mut builder = GiftCodeSenderMemoBuilder::default(); + // Verify note with max length minus one is okay + { + let mut builder = GiftCodeSenderMemoBuilder::new(note_minus_one).unwrap(); - // Set the note - let note = "It's MEMO TIME!!"; - builder.set_gift_code_sender_note(note).unwrap(); - builder.clear_sender_note(); + // Build the memo payload and get the data + let memo_payload = build_change_memo_with_amount(&mut builder, amount).unwrap(); - // Build the memo payload - let amount = Amount::new(42, 0.into()); - let memo_payload = build_change_memo_with_amount(&mut builder, amount).unwrap(); - - // Verify memo data - let blank_note = ""; - let derived_note = MemoDecoder::decode_sender_note(memo_payload.get_memo_data()); - assert_eq!(blank_note, derived_note); + // Verify memo data + let derived_note = GiftCodeSenderMemo::from(memo_payload.get_memo_data()); + assert_eq!(note_minus_one, derived_note.sender_note().unwrap()); + } - // Create another memo builder - let mut builder_2 = GiftCodeSenderMemoBuilder::default(); + // Verify note with max length okay + { + let mut builder = GiftCodeSenderMemoBuilder::new(note_exact).unwrap(); - // Set the cancellation index and then set another note on top of it - let note_2 = "Is anything actually real?"; - builder_2.set_gift_code_sender_note(note).unwrap(); - builder_2.set_gift_code_sender_note(note_2).unwrap(); + // Build the memo payload and get the data + let memo_payload = build_change_memo_with_amount(&mut builder, amount).unwrap(); - // Build the memo payload and get the data - let amount_2 = Amount::new(666, 0.into()); - let memo_payload_2 = build_change_memo_with_amount(&mut builder_2, amount_2).unwrap(); - let derived_note_2 = MemoDecoder::decode_sender_note(memo_payload_2.get_memo_data()); - assert_eq!(note_2, derived_note_2); + // Verify memo data + let derived_note = GiftCodeSenderMemo::from(memo_payload.get_memo_data()); + assert_eq!(note_exact, derived_note.sender_note().unwrap()); + } } #[test] - fn test_gift_code_sender_memo_writes_unused_if_change_disabled() { - // Create memo builder - let mut builder = GiftCodeSenderMemoBuilder::default(); - - // Set the note + fn test_gift_code_sender_memo_fails_for_more_than_one_change_memo() { + // Create memo builder with note let note = "It's MEMO TIME!!"; - builder.set_gift_code_sender_note(note).unwrap(); - - // Disable change memos - builder.disable_gift_code_change_memo(); + let mut builder = GiftCodeSenderMemoBuilder::new(note).unwrap(); - // Build the memo payload + // Build the memo payload and get the data let amount = Amount::new(42, 0.into()); let memo_payload = build_change_memo_with_amount(&mut builder, amount).unwrap(); - assert_eq!(memo_payload.get_memo_data(), &[0u8; 64]); - } - #[test] - fn test_gift_code_sender_note_builder_fails_after_attempting_invalid_note() { - // Create memo builder - let mut builder = GiftCodeSenderMemoBuilder::default(); - - // Attempt to set an invalid note - let note_bytes = [b'6'; GiftCodeSenderMemo::MEMO_DATA_LEN + 1]; - let note = std::str::from_utf8(¬e_bytes).unwrap(); - let result = builder.set_gift_code_sender_note(note); - assert!(matches!(result, Err(NewMemoError::BadInputs(_)))); + // Verify memo data can be verified after writing the first change memo + let derived_note = GiftCodeSenderMemo::from(memo_payload.get_memo_data()); + assert_eq!(note, derived_note.sender_note().unwrap()); - // Fail to build memo after failing to set valid note - let amount = Amount::new(42, 0.into()); - let memo_payload = build_change_memo_with_amount(&mut builder, amount); - assert!(matches!(memo_payload, Err(NewMemoError::BadInputs(_)))); + // Verify attempting to write two change memos fail + let memo_payload_2 = build_change_memo_with_amount(&mut builder, amount); + assert!(matches!( + memo_payload_2, + Err(NewMemoError::MultipleChangeOutputs) + )); } #[test] - fn test_gift_code_sender_note_builder_succeeds_after_resetting_invalid_note_with_valid_note() { - // Create memo builder - let mut builder = GiftCodeSenderMemoBuilder::default(); - - // Attempt to set an invalid note + fn test_gift_code_sender_note_builder_creation_fails_with_invalid_note() { + // Create Memo Builder with an input longer than allowed let note_bytes = [b'6'; GiftCodeSenderMemo::MEMO_DATA_LEN + 1]; let note = std::str::from_utf8(¬e_bytes).unwrap(); - let result = builder.set_gift_code_sender_note(note); - assert!(matches!(result, Err(NewMemoError::BadInputs(_)))); - - // Reset with valid note - let valid_note = "Im within the byte length, but validity is a subjective concept"; - builder.set_gift_code_sender_note(valid_note).unwrap(); - let amount = Amount::new(42, 0.into()); - let memo_payload = build_change_memo_with_amount(&mut builder, amount).unwrap(); - - // Assert we can build the memo and get the correct output - let derived_note = MemoDecoder::decode_sender_note(memo_payload.get_memo_data()); - assert_eq!(valid_note, derived_note); + let builder = GiftCodeSenderMemoBuilder::new(note); + assert!(matches!(builder, Err(NewMemoError::BadInputs(_)))); } } diff --git a/transaction/std/src/test_utils.rs b/transaction/std/src/test_utils.rs index 8e55cb4cf5..d8b43d9fb5 100644 --- a/transaction/std/src/test_utils.rs +++ b/transaction/std/src/test_utils.rs @@ -3,12 +3,11 @@ //! Utilities that help with testing the transaction builder and related objects use crate::{ - EmptyMemoBuilder, GiftCodeFundingMemo, GiftCodeSenderMemo, InputCredentials, MemoBuilder, - MemoPayload, ReservedDestination, TransactionBuilder, TxBuilderError, + EmptyMemoBuilder, InputCredentials, MemoBuilder, MemoPayload, ReservedDestination, + TransactionBuilder, TxBuilderError, }; -use core::convert::{TryFrom, TryInto}; +use core::convert::TryFrom; use mc_account_keys::{AccountKey, PublicAddress, DEFAULT_SUBADDRESS_INDEX}; -use mc_crypto_hashes::{Blake2b512, Digest}; use mc_crypto_keys::RistrettoPublic; use mc_fog_report_validation::FogPubkeyResolver; use mc_transaction_core::{ @@ -55,6 +54,31 @@ pub fn create_output( ) } +// Make fake outputs for the ring +fn add_fake_outputs_to_ring( + mut ring: Vec, + block_version: BlockVersion, + amount: Amount, + ring_size: usize, + fog_resolver: &FPR, + rng: &mut RNG, +) -> Vec { + // Create ring_size - 1 mixins with assorted token ids + for idx in 0..ring_size - 1 { + let address = AccountKey::random(rng).default_subaddress(); + let token_id = if block_version.masked_token_id_feature_is_supported() { + TokenId::from(idx as u64) + } else { + Mob::ID + }; + let amount = Amount::new(amount.value, token_id); + let (tx_out, _) = + create_output(block_version, amount, &address, fog_resolver, rng).unwrap(); + ring.push(tx_out); + } + ring +} + /// Creates a ring of of TxOuts. /// /// # Arguments @@ -75,21 +99,15 @@ pub fn get_ring( fog_resolver: &FPR, rng: &mut RNG, ) -> (Vec, usize) { - let mut ring: Vec = Vec::new(); - // Create ring_size - 1 mixins with assorted token ids - for idx in 0..ring_size - 1 { - let address = AccountKey::random(rng).default_subaddress(); - let token_id = if block_version.masked_token_id_feature_is_supported() { - TokenId::from(idx as u64) - } else { - Mob::ID - }; - let amount = Amount::new(amount.value, token_id); - let (tx_out, _) = - create_output(block_version, amount, &address, fog_resolver, rng).unwrap(); - ring.push(tx_out); - } + let mut ring: Vec = add_fake_outputs_to_ring( + Vec::new(), + block_version, + amount, + ring_size, + fog_resolver, + rng, + ); // Insert the real element. let real_index = (rng.next_u64() % ring_size as u64) as usize; @@ -107,6 +125,42 @@ pub fn get_ring( (ring, real_index) } +/// Get ring for existing TxOut +/// +/// # Arguments +/// * `tx_out` - The "real" TxOut to build the ring for +/// * `block_version` - The block version for the TxOut's +/// * `token_id` - The token id for the real element +/// * `ring_size` - Number of elements in the ring. +/// * `value` - Value of the real element. +/// * `fog_resolver` - Fog public keys +/// * `rng` - Randomness. +/// +/// Returns (ring, real_index) +pub fn get_ring_for_txout( + tx_out: &TxOut, + block_version: BlockVersion, + amount: Amount, + ring_size: usize, + fog_resolver: &FPR, + rng: &mut RNG, +) -> (Vec, usize) { + // Create ring_size - 1 mixins with assorted token ids + let mut ring: Vec = add_fake_outputs_to_ring( + Vec::new(), + block_version, + amount, + ring_size, + fog_resolver, + rng, + ); + let real_index = (rng.next_u64() % ring_size as u64) as usize; + ring.insert(real_index, tx_out.clone()); + assert_eq!(ring.len(), ring_size); + + (ring, real_index) +} + /// Creates an `InputCredentials` for an account. /// /// # Arguments @@ -231,47 +285,3 @@ pub fn build_change_memo_with_amount( //Build memo builder.make_memo_for_change_output(change_amount, &alice_address_book, memo_context) } - -/// Utilities for decoding and verifying memo data -pub struct MemoDecoder; - -impl MemoDecoder { - /// Decode a sender note from bytes - pub fn decode_sender_note(memo_data: &[u8; GiftCodeSenderMemo::MEMO_DATA_LEN]) -> &str { - let index = if let Some(terminator) = memo_data.iter().position(|b| b == &0u8) { - terminator - } else { - GiftCodeSenderMemo::MEMO_DATA_LEN - }; - - std::str::from_utf8(&memo_data[0..index]).unwrap() - } - - /// Decode a funding note from bytes - pub fn decode_funding_note(memo_data: &[u8; GiftCodeFundingMemo::MEMO_DATA_LEN]) -> &str { - let index = if let Some(terminator) = memo_data - .iter() - .enumerate() - .position(|(i, b)| i >= GiftCodeFundingMemo::HASH_DATA_LEN && b == &0u8) - { - terminator - } else { - GiftCodeFundingMemo::MEMO_DATA_LEN - }; - - std::str::from_utf8(&memo_data[GiftCodeFundingMemo::HASH_DATA_LEN..index]).unwrap() - } - - /// Get the first four bytes of the Blake2b hash of a gift code TxOut public - /// key - pub fn tx_out_public_key_short_hash( - tx_out_public_key: &RistrettoPublic, - ) -> [u8; GiftCodeFundingMemo::HASH_DATA_LEN] { - let mut hasher = Blake2b512::new(); - hasher.update("mc-gift-funding-tx-pub-key"); - hasher.update(tx_out_public_key.as_ref().compress().as_bytes()); - hasher.finalize().as_slice()[0..GiftCodeFundingMemo::HASH_DATA_LEN] - .try_into() - .unwrap() - } -} diff --git a/transaction/std/src/transaction_builder.rs b/transaction/std/src/transaction_builder.rs index 1c77492da8..9557ec3fe8 100644 --- a/transaction/std/src/transaction_builder.rs +++ b/transaction/std/src/transaction_builder.rs @@ -574,7 +574,7 @@ pub(crate) fn create_output_with_fog_hint( if !block_version.masked_token_id_feature_is_supported() { tx_out.masked_amount.masked_token_id.clear(); } - + // let shared_secret = create_shared_secret(recipient.view_public_key(), &private_key); Ok((tx_out, shared_secret)) } @@ -612,26 +612,27 @@ pub(crate) fn create_fog_hint( pub mod transaction_builder_tests { use super::*; use crate::{ - test_utils::{get_input_credentials, get_ring, get_transaction}, - BurnRedemptionMemoBuilder, EmptyMemoBuilder, MemoType, RTHMemoBuilder, + test_utils::{get_input_credentials, get_ring, get_ring_for_txout, get_transaction}, + BurnRedemptionMemoBuilder, EmptyMemoBuilder, GiftCodeCancellationMemoBuilder, + GiftCodeFundingMemoBuilder, GiftCodeSenderMemoBuilder, MemoType, RTHMemoBuilder, SenderMemoCredential, }; use assert_matches::assert_matches; use maplit::btreemap; use mc_account_keys::{ burn_address, burn_address_view_private, AccountKey, ShortAddressHash, - CHANGE_SUBADDRESS_INDEX, DEFAULT_SUBADDRESS_INDEX, + CHANGE_SUBADDRESS_INDEX, DEFAULT_SUBADDRESS_INDEX, GIFT_CODE_SUBADDRESS_INDEX, }; use mc_fog_report_validation_test_utils::{FullyValidatedFogPubkey, MockFogResolver}; use mc_transaction_core::{ constants::{MAX_INPUTS, MAX_OUTPUTS, MILLIMOB_TO_PICOMOB}, get_tx_out_shared_secret, onetime_keys::*, - ring_signature::KeyImage, + ring_signature::{InputSecret, KeyImage}, subaddress_matches_tx_out, tx::TxOutMembershipProof, validation::{validate_signature, validate_tx_out}, - NewTxError, TokenId, + NewTxError, TokenId, TxOutGiftCode, }; use rand::{rngs::StdRng, SeedableRng}; use std::convert::TryFrom; @@ -1377,7 +1378,7 @@ pub mod transaction_builder_tests { ); transaction_builder.add_input(input_credentials); - let (_txout, _confirmation) = transaction_builder + transaction_builder .add_output( Amount::new(value - change_value - 4 * Mob::MINIMUM_FEE, token_id), &recipient_address, @@ -3104,4 +3105,500 @@ pub mod transaction_builder_tests { assert!(test_fn(block_version, tx_out1_wrong_amount).is_err()); } } + + #[test] + // Transaction builder with gift codes + fn test_gift_code_transactions() { + let mut rng: StdRng = SeedableRng::from_seed([1u8; 32]); + let block_version = BlockVersion::MAX; + let token_id = TokenId::from(5); + let fog_resolver = MockFogResolver::default(); + let sender = AccountKey::random(&mut rng); + let receiver = AccountKey::random(&mut rng); + let (funding_input_amt, funding_output_amt, fee) = (1000, 10, 1); + let sender_reserved_destinations = ReservedDestination::from(&sender); + let receiver_reserved_destinations = ReservedDestination::from(&receiver); + let note = "It's funding time"; + + // Test gift code funding and sending + { + // Initialize funding memo & transaction builders + let funding_memo_builder = GiftCodeFundingMemoBuilder::new(note).unwrap(); + let funding_input_amount = Amount::new(funding_input_amt, token_id); + let funding_output_amount = Amount::new(funding_output_amt, token_id); + let funding_change_amount = + Amount::new(funding_input_amt - funding_output_amt - fee, token_id); + let mut funding_transaction_builder = TransactionBuilder::new( + block_version, + Amount::new(fee, token_id), + fog_resolver.clone(), + funding_memo_builder, + ) + .unwrap(); + + // Make sample input supply + let funding_input_credentials = get_input_credentials( + block_version, + funding_input_amount, + &sender, + &fog_resolver, + &mut rng, + ); + funding_transaction_builder.add_input(funding_input_credentials); + + // Fund gift code TxOut with blank memo + funding_transaction_builder + .add_output( + funding_output_amount, + &sender_reserved_destinations.gift_code_subaddress, + &mut rng, + ) + .unwrap(); + + // Make change output with funding memo + funding_transaction_builder + .add_change_output( + funding_change_amount, + &sender_reserved_destinations, + &mut rng, + ) + .unwrap(); + + let funding_tx = funding_transaction_builder.build(&mut rng).unwrap(); + + // The transaction should have exactly 2 outputs + assert_eq!(funding_tx.prefix.outputs.len(), 2); + + let funding_output = funding_tx + .prefix + .outputs + .iter() + .find(|tx_out| { + subaddress_matches_tx_out(&sender, GIFT_CODE_SUBADDRESS_INDEX, tx_out).unwrap() + }) + .expect("Didn't find recipient's output"); + + let funding_change = funding_tx + .prefix + .outputs + .iter() + .find(|tx_out| { + subaddress_matches_tx_out(&sender, CHANGE_SUBADDRESS_INDEX, tx_out).unwrap() + }) + .expect("Didn't find sender's output"); + + validate_tx_out(block_version, funding_output).unwrap(); + validate_tx_out(block_version, funding_change).unwrap(); + + assert!( + subaddress_matches_tx_out(&sender, GIFT_CODE_SUBADDRESS_INDEX, funding_output) + .unwrap() + ); + assert!( + subaddress_matches_tx_out(&sender, CHANGE_SUBADDRESS_INDEX, funding_change) + .unwrap() + ); + + // Ensure funding output & change memos are correct + let gift_code_tx_out_public_key = + &RistrettoPublic::try_from(&funding_output.public_key).unwrap(); + let funding_change_tx_out_public_key = + &RistrettoPublic::try_from(&funding_change.public_key).unwrap(); + let gift_code_ss = + get_tx_out_shared_secret(sender.view_private_key(), gift_code_tx_out_public_key); + let funding_change_ss = get_tx_out_shared_secret( + sender.view_private_key(), + funding_change_tx_out_public_key, + ); + + let (funding_amount, _) = funding_output + .masked_amount + .get_value(&gift_code_ss) + .unwrap(); + assert_eq!(funding_amount.value, funding_output_amount.value); + assert_eq!(funding_amount.token_id, token_id); + + if block_version.e_memo_feature_is_supported() { + let funding_change_memo = + funding_change.e_memo.unwrap().decrypt(&funding_change_ss); + let funding_output_memo = funding_output.e_memo.unwrap().decrypt(&gift_code_ss); + match MemoType::try_from(&funding_change_memo) + .expect("Couldn't decrypt funding change memo") + { + MemoType::GiftCodeFunding(memo) => { + assert!(memo.public_key_matches(gift_code_tx_out_public_key),); + assert_eq!(memo.funding_note().unwrap(), note,); + } + _ => { + panic!("unexpected memo type") + } + } + match MemoType::try_from(&funding_output_memo) + .expect("Couldn't decrypt gift code funding_output memo") + { + MemoType::Unused(_) => {} + _ => { + panic!("unexpected memo type") + } + } + } + + // MCIP #32 specifies that the receiver will receive the TxOut index, + // shared_secret and onetime_private_key. Send a gift code from a + // simulated sender to a simulated receiver in the manner specified + // in the MCIP, by using the data from gift code TxOut funded above + + // Get the data we're pretending sender sends to the receiver and + // construct TxOutGiftCode object from it + let global_index = 42; + let gift_code_tx_out_private_key = recover_onetime_private_key( + &gift_code_tx_out_public_key, + &sender.spend_private_key(), + &sender.gift_code_subaddress_spend_private(), + ); + let tx_out_gift_code = TxOutGiftCode { + global_index, + onetime_private_key: gift_code_tx_out_private_key.clone(), + shared_secret: gift_code_ss, + }; + + // Values we pretend we get from locating the TxOut using the global index + let masked_amount = funding_output.masked_amount.clone(); + + // Construct the sender Tx from the combo of "located" and "sent" information + let (sending_input_amount, blinding) = masked_amount + .get_value(&tx_out_gift_code.shared_secret) + .unwrap(); + println!("{:?}", sending_input_amount); + let sending_output_amount = Amount::new(sending_input_amount.value - fee, token_id); + + let (ring, real_index) = get_ring_for_txout( + &funding_output, + block_version, + sending_input_amount, + 3, + &fog_resolver, + &mut rng, + ); + + let membership_proofs: Vec = ring + .iter() + .map(|_tx_out| TxOutMembershipProof::default()) + .collect(); + + // Construct our sending memo builder + let note = "It's sending time"; + let sending_memo_builder = GiftCodeSenderMemoBuilder::new(note).unwrap(); + + let mut transaction_builder = TransactionBuilder::new( + block_version, + Amount::new(fee, token_id), + fog_resolver.clone(), + sending_memo_builder, + ) + .unwrap(); + + // Create our inputs from reconstructed info + let input_credentials = InputCredentials { + ring, + membership_proofs, + real_index, + input_secret: InputSecret { + onetime_private_key: gift_code_tx_out_private_key, + amount: sending_input_amount, + blinding: blinding, + }, + }; + + transaction_builder.add_input(input_credentials); + + // Add the output and build the transaction + transaction_builder + .add_change_output( + sending_output_amount, + &receiver_reserved_destinations, + &mut rng, + ) + .unwrap(); + + let tx = transaction_builder.build(&mut rng).unwrap(); + + // Verify the sender transaction was valid + assert_eq!(tx.prefix.outputs.len(), 1); + + let change = tx + .prefix + .outputs + .iter() + .find(|tx_out| { + subaddress_matches_tx_out(&receiver, CHANGE_SUBADDRESS_INDEX, tx_out).unwrap() + }) + .expect("Didn't find sender's output"); + + validate_tx_out(block_version, change).unwrap(); + + assert!(subaddress_matches_tx_out(&receiver, CHANGE_SUBADDRESS_INDEX, change).unwrap()); + + // Ensure change memo is correct + let ss = get_tx_out_shared_secret( + receiver.view_private_key(), + &RistrettoPublic::try_from(&change.public_key).unwrap(), + ); + let (amount, _) = change.masked_amount.get_value(&ss).unwrap(); + assert_eq!(amount.value, sending_output_amount.value); + assert_eq!(amount.token_id, token_id); + + if block_version.e_memo_feature_is_supported() { + let memo = change.e_memo.unwrap().decrypt(&ss); + match MemoType::try_from(&memo).expect("Couldn't decrypt memo") { + MemoType::GiftCodeSender(memo) => { + assert_eq!(memo.sender_note().unwrap(), note,); + } + _ => { + panic!("unexpected memo type") + } + } + } + } + + // Test gift code cancellation + { + let sample_index = 1; + let cancellation_memo_builder = GiftCodeCancellationMemoBuilder::new(sample_index); + + let cancellation_input_amount = Amount::new(funding_output_amt, token_id); + let cancellation_output_amount = Amount::new(funding_output_amt - fee, token_id); + + let mut transaction_builder = TransactionBuilder::new( + block_version, + Amount::new(fee, token_id), + fog_resolver.clone(), + cancellation_memo_builder, + ) + .unwrap(); + + // Make sample input supply + let input_credentials = get_input_credentials( + block_version, + cancellation_input_amount, + &sender, + &fog_resolver, + &mut rng, + ); + transaction_builder.add_input(input_credentials); + + // Cancel gift code + transaction_builder + .add_change_output( + cancellation_output_amount, + &sender_reserved_destinations, + &mut rng, + ) + .unwrap(); + + let tx = transaction_builder.build(&mut rng).unwrap(); + + // The transaction should have exactly 1 output + assert_eq!(tx.prefix.outputs.len(), 1); + + let change = tx + .prefix + .outputs + .iter() + .find(|tx_out| { + subaddress_matches_tx_out(&sender, CHANGE_SUBADDRESS_INDEX, tx_out).unwrap() + }) + .expect("Didn't find sender's output"); + + validate_tx_out(block_version, change).unwrap(); + + // Ensure it was sent to the change address + assert!(subaddress_matches_tx_out(&sender, CHANGE_SUBADDRESS_INDEX, change).unwrap()); + + // Ensure change memo is correct + let ss = get_tx_out_shared_secret( + sender.view_private_key(), + &RistrettoPublic::try_from(&change.public_key).unwrap(), + ); + let (amount, _) = change.masked_amount.get_value(&ss).unwrap(); + assert_eq!(amount.value, cancellation_output_amount.value); + assert_eq!(amount.token_id, token_id); + + if block_version.e_memo_feature_is_supported() { + let memo = change.e_memo.unwrap().decrypt(&ss); + match MemoType::try_from(&memo).expect("Couldn't decrypt memo") { + MemoType::GiftCodeCancellation(memo) => { + assert_eq!(memo.cancelled_gift_code_index(), sample_index,); + } + _ => { + panic!("unexpected memo type") + } + } + } + } + } + + #[test] + // Test errors in gift code building + fn test_gift_code_transaction_builder_errors() { + // Test funding multiple gift at once codes fails + let mut rng: StdRng = SeedableRng::from_seed([1u8; 32]); + let sender = AccountKey::random(&mut rng); + let sender_reserved_destinations = ReservedDestination::from(&sender); + let token_id = TokenId::from(5); + let note = "I'm a note"; + + // Ensure we can't do more than one gift code TxOut output + { + let funding_memo_builder = GiftCodeFundingMemoBuilder::new(note).unwrap(); + + let mut transaction_builder = TransactionBuilder::new( + BlockVersion::MAX, + Amount::new(1, token_id), + MockFogResolver::default(), + funding_memo_builder, + ) + .unwrap(); + + transaction_builder + .add_output( + Amount::new(100, token_id), + &sender_reserved_destinations.gift_code_subaddress, + &mut rng, + ) + .unwrap(); + + let result = transaction_builder.add_output( + Amount::new(100, token_id), + &sender_reserved_destinations.gift_code_subaddress, + &mut rng, + ); + assert_matches!( + result, + Err(TxBuilderError::NewTx(NewTxError::Memo( + NewMemoError::MultipleOutputs + ))) + ); + } + + // Ensure we can't fund after change + { + let funding_memo_builder = GiftCodeFundingMemoBuilder::new(note).unwrap(); + + let mut transaction_builder = TransactionBuilder::new( + BlockVersion::MAX, + Amount::new(1, token_id), + MockFogResolver::default(), + funding_memo_builder, + ) + .unwrap(); + + // Try to write change before funding gift code and assert it errors + let result_change_before_output = transaction_builder.add_change_output( + Amount::new(100, token_id), + &sender_reserved_destinations, + &mut rng, + ); + + assert_matches!( + result_change_before_output, + Err(TxBuilderError::NewTx(NewTxError::Memo( + NewMemoError::MissingOutput + ))) + ); + + // Fund gift code & add change output in proper order + transaction_builder + .add_output( + Amount::new(100, token_id), + &sender_reserved_destinations.gift_code_subaddress, + &mut rng, + ) + .unwrap(); + + // Attempt to fund second gift code + let second_output = transaction_builder.add_output( + Amount::new(100, token_id), + &sender_reserved_destinations.gift_code_subaddress, + &mut rng, + ); + + assert_matches!( + second_output, + Err(TxBuilderError::NewTx(NewTxError::Memo( + NewMemoError::MultipleOutputs + ))) + ); + + transaction_builder + .add_change_output( + Amount::new(100, token_id), + &sender_reserved_destinations, + &mut rng, + ) + .unwrap(); + + // Attempt to write an output after change + let output_after_change = transaction_builder.add_output( + Amount::new(100, token_id), + &sender_reserved_destinations.gift_code_subaddress, + &mut rng, + ); + + assert_matches!( + output_after_change, + Err(TxBuilderError::NewTx(NewTxError::Memo( + NewMemoError::MultipleOutputs + ))) + ); + } + + // Ensure we can't write destination TxOuts for Cancellation & Sending + { + let sender_memo_builder = GiftCodeSenderMemoBuilder::new(note).unwrap(); + let cancellation_memo_builder = GiftCodeCancellationMemoBuilder::new(50); + + let mut sending_transaction_builder = TransactionBuilder::new( + BlockVersion::MAX, + Amount::new(1, token_id), + MockFogResolver::default(), + sender_memo_builder, + ) + .unwrap(); + + let mut cancellation_transaction_builder = TransactionBuilder::new( + BlockVersion::MAX, + Amount::new(1, token_id), + MockFogResolver::default(), + cancellation_memo_builder, + ) + .unwrap(); + + let sender_result = sending_transaction_builder.add_output( + Amount::new(100, token_id), + &sender_reserved_destinations.gift_code_subaddress, + &mut rng, + ); + + let cancellation_result = cancellation_transaction_builder.add_output( + Amount::new(100, token_id), + &sender_reserved_destinations.gift_code_subaddress, + &mut rng, + ); + + assert_matches!( + sender_result, + Err(TxBuilderError::NewTx(NewTxError::Memo( + NewMemoError::DestinationMemoNotAllowed + ))) + ); + + assert_matches!( + cancellation_result, + Err(TxBuilderError::NewTx(NewTxError::Memo( + NewMemoError::DestinationMemoNotAllowed + ))) + ); + } + } } From 5557cda5914ba09dc25647f25e513d2be531d7f6 Mon Sep 17 00:00:00 2001 From: iamalwaysuncomfortable Date: Wed, 18 May 2022 01:41:16 -0400 Subject: [PATCH 10/12] Lint fix --- .../src/memo_builder/gift_code_funding_memo_builder.rs | 2 +- transaction/std/src/transaction_builder.rs | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/transaction/std/src/memo_builder/gift_code_funding_memo_builder.rs b/transaction/std/src/memo_builder/gift_code_funding_memo_builder.rs index 409f108758..bffb1b1bde 100644 --- a/transaction/std/src/memo_builder/gift_code_funding_memo_builder.rs +++ b/transaction/std/src/memo_builder/gift_code_funding_memo_builder.rs @@ -131,7 +131,7 @@ mod tests { let change_amount = Amount::new(1, 0.into()); let funding_amount = Amount::new(10, 0.into()); let funding_context = MemoContext { - tx_public_key: &funding_tx_pubkey, + tx_public_key: funding_tx_pubkey, }; let change_context = MemoContext { tx_public_key: &change_tx_pubkey, diff --git a/transaction/std/src/transaction_builder.rs b/transaction/std/src/transaction_builder.rs index 9557ec3fe8..2329736be1 100644 --- a/transaction/std/src/transaction_builder.rs +++ b/transaction/std/src/transaction_builder.rs @@ -3252,13 +3252,13 @@ pub mod transaction_builder_tests { // construct TxOutGiftCode object from it let global_index = 42; let gift_code_tx_out_private_key = recover_onetime_private_key( - &gift_code_tx_out_public_key, - &sender.spend_private_key(), + gift_code_tx_out_public_key, + sender.spend_private_key(), &sender.gift_code_subaddress_spend_private(), ); let tx_out_gift_code = TxOutGiftCode { global_index, - onetime_private_key: gift_code_tx_out_private_key.clone(), + onetime_private_key: gift_code_tx_out_private_key, shared_secret: gift_code_ss, }; @@ -3273,7 +3273,7 @@ pub mod transaction_builder_tests { let sending_output_amount = Amount::new(sending_input_amount.value - fee, token_id); let (ring, real_index) = get_ring_for_txout( - &funding_output, + funding_output, block_version, sending_input_amount, 3, @@ -3306,7 +3306,7 @@ pub mod transaction_builder_tests { input_secret: InputSecret { onetime_private_key: gift_code_tx_out_private_key, amount: sending_input_amount, - blinding: blinding, + blinding, }, }; From 29e3e11dcd482a9149f6cdbebd596c4081d1a60e Mon Sep 17 00:00:00 2001 From: iamalwaysuncomfortable Date: Wed, 18 May 2022 08:48:11 -0400 Subject: [PATCH 11/12] Comment clarifications --- .../gift_code_cancellation_memo_builder.rs | 7 +- .../gift_code_funding_memo_builder.rs | 11 ++- .../gift_code_sender_memo_builder.rs | 17 +++-- transaction/std/src/test_utils.rs | 8 ++- transaction/std/src/transaction_builder.rs | 69 ++++++++++--------- 5 files changed, 58 insertions(+), 54 deletions(-) diff --git a/transaction/std/src/memo_builder/gift_code_cancellation_memo_builder.rs b/transaction/std/src/memo_builder/gift_code_cancellation_memo_builder.rs index aa38b61479..61b297d573 100644 --- a/transaction/std/src/memo_builder/gift_code_cancellation_memo_builder.rs +++ b/transaction/std/src/memo_builder/gift_code_cancellation_memo_builder.rs @@ -15,8 +15,8 @@ use mc_transaction_core::{Amount, MemoContext, MemoPayload, NewMemoError}; /// | -->0x0202<-- | Gift Code Cancellation Memo | /// This memo builder builds a gift code cancellation memo (0x0202). Gift code /// cancellation is defined as the sender sending the gift code TxOut at the -/// gift code subaddress back to their change address prior to it being spent -/// by the receiver. When that happens a gift code cancellation memo is +/// gift code subaddress to their change address prior to it being spent by +/// the receiver. When that happens a gift code cancellation memo is /// written to the change TxOut that stores the index of the TxOut originally /// sent to the gift code subaddress when the gift code was funded. #[derive(Clone, Debug)] @@ -54,8 +54,7 @@ impl MemoBuilder for GiftCodeCancellationMemoBuilder { Err(NewMemoError::DestinationMemoNotAllowed) } - /// Write the cancellation memo to the change output the gift code is - /// refunded to + /// Write the cancellation memo to the change output fn make_memo_for_change_output( &mut self, _amount: Amount, diff --git a/transaction/std/src/memo_builder/gift_code_funding_memo_builder.rs b/transaction/std/src/memo_builder/gift_code_funding_memo_builder.rs index bffb1b1bde..6d957b3ba6 100644 --- a/transaction/std/src/memo_builder/gift_code_funding_memo_builder.rs +++ b/transaction/std/src/memo_builder/gift_code_funding_memo_builder.rs @@ -38,10 +38,9 @@ pub struct GiftCodeFundingMemoBuilder { } impl GiftCodeFundingMemoBuilder { - /// Initialize memo builder with a utf-8 note (up to 60 bytes) - /// indicating what the gift code was for. This method will enforce - /// the 60 byte limit with a NewMemoErr if the &str passed is longer - /// than 60 bytes. + /// Initialize memo builder with a utf-8 note (up to 60 bytes), This + /// method will enforce the 60 byte limit with a NewMemoErr if the + /// note passed is longer than 60 bytes. pub fn new(note: &str) -> Result { if note.len() > GiftCodeFundingMemo::NOTE_DATA_LEN { return Err(NewMemoError::BadInputs( @@ -169,7 +168,7 @@ mod tests { #[test] fn test_gift_code_funding_memo_built_successfully_with_edge_case_notes() { - // Create memo builder with blank note + // Create blank notes and notes near max length let blank_note = ""; let note_minus_one = std::str::from_utf8(&[b'6'; GiftCodeFundingMemo::NOTE_DATA_LEN - 1]).unwrap(); @@ -232,7 +231,7 @@ mod tests { let alice_address_book = ReservedDestination::from(&alice); let change_amount = Amount::new(666, 666.into()); - // Erroneously set change TxOut public address to the funding TxOut + // Erroneously set funding TxOut pubkey to the change TxOut pubkey let change_tx_public_key = RistrettoPublic::from_random(&mut rng); builder .make_memo_for_output( diff --git a/transaction/std/src/memo_builder/gift_code_sender_memo_builder.rs b/transaction/std/src/memo_builder/gift_code_sender_memo_builder.rs index 0b39cc1214..11a4b6801d 100644 --- a/transaction/std/src/memo_builder/gift_code_sender_memo_builder.rs +++ b/transaction/std/src/memo_builder/gift_code_sender_memo_builder.rs @@ -13,13 +13,14 @@ use mc_transaction_core::{Amount, MemoContext, MemoPayload, NewMemoError}; /// | -->0x0002<-- | Gift Code Sender Memo | /// | 0x0201 | Gift Code Funding Memo | /// | 0x0202 | Gift Code Cancellation Memo | -/// This memo builder builds a gift code sender memo (0x0002). A gift code +/// This memo builder builds the gift code sender memo (0x0002). A gift code /// considered redeemed when the Receiver uses the TxOut spend private key /// of the gift code TxOut they received from the Sender to send the TxOut /// from the sender's gift code subaddress to their own change subaddress. -/// The destination memo is written into that TxOut at the change address -/// and includes an optional Utf-8 note up to 64 bytes long the Receiver -/// can use to record any information they desire about the gift code. +/// The sender memo is written into the TxOut the receiver sends to the +/// change address and includes an optional Utf-8 note up to 64 bytes long +/// the Receiver can use to record any information they desire about the +/// gift code. #[derive(Clone, Debug)] pub struct GiftCodeSenderMemoBuilder { // Utf-8 formatted note within the memo that can be up to 64 bytes long @@ -29,10 +30,9 @@ pub struct GiftCodeSenderMemoBuilder { } impl GiftCodeSenderMemoBuilder { - /// Initialize memo builder with a utf-8 note (up to 64 bytes) - /// indicating what the gift code was for. This method will enforce - /// the 64 byte limit with a NewMemoErr if the &str passed is longer - /// than 64 bytes. + /// Initialize memo builder with a utf-8 note (up to 64 bytes), This + /// method will enforce the 64 byte limit with a NewMemoErr if the + /// note passed is longer than 64 bytes. pub fn new(note: &str) -> Result { if note.len() > GiftCodeSenderMemo::MEMO_DATA_LEN { return Err(NewMemoError::BadInputs( @@ -80,7 +80,6 @@ impl MemoBuilder for GiftCodeSenderMemoBuilder { #[cfg(test)] mod tests { - use super::*; use crate::test_utils::build_change_memo_with_amount; diff --git a/transaction/std/src/test_utils.rs b/transaction/std/src/test_utils.rs index d8b43d9fb5..56ebf4a7c1 100644 --- a/transaction/std/src/test_utils.rs +++ b/transaction/std/src/test_utils.rs @@ -54,7 +54,7 @@ pub fn create_output( ) } -// Make fake outputs for the ring +// Make fake outputs for a ring fn add_fake_outputs_to_ring( mut ring: Vec, block_version: BlockVersion, @@ -63,7 +63,7 @@ fn add_fake_outputs_to_ring( fog_resolver: &FPR, rng: &mut RNG, ) -> Vec { - // Create ring_size - 1 mixins with assorted token ids + // Create fake mixins with assorted token ids for idx in 0..ring_size - 1 { let address = AccountKey::random(rng).default_subaddress(); let token_id = if block_version.masked_token_id_feature_is_supported() { @@ -109,7 +109,7 @@ pub fn get_ring( rng, ); - // Insert the real element. + // Insert the "real" element. let real_index = (rng.next_u64() % ring_size as u64) as usize; let (tx_out, _) = create_output( block_version, @@ -154,6 +154,8 @@ pub fn get_ring_for_txout( fog_resolver, rng, ); + + // Insert the TxOut let real_index = (rng.next_u64() % ring_size as u64) as usize; ring.insert(real_index, tx_out.clone()); assert_eq!(ring.len(), ring_size); diff --git a/transaction/std/src/transaction_builder.rs b/transaction/std/src/transaction_builder.rs index 2329736be1..dec12a25b0 100644 --- a/transaction/std/src/transaction_builder.rs +++ b/transaction/std/src/transaction_builder.rs @@ -3126,7 +3126,7 @@ pub mod transaction_builder_tests { let funding_memo_builder = GiftCodeFundingMemoBuilder::new(note).unwrap(); let funding_input_amount = Amount::new(funding_input_amt, token_id); let funding_output_amount = Amount::new(funding_output_amt, token_id); - let funding_change_amount = + let funding_change_output_amount = Amount::new(funding_input_amt - funding_output_amt - fee, token_id); let mut funding_transaction_builder = TransactionBuilder::new( block_version, @@ -3146,7 +3146,7 @@ pub mod transaction_builder_tests { ); funding_transaction_builder.add_input(funding_input_credentials); - // Fund gift code TxOut with blank memo + // Fund gift code TxOut funding_transaction_builder .add_output( funding_output_amount, @@ -3155,10 +3155,9 @@ pub mod transaction_builder_tests { ) .unwrap(); - // Make change output with funding memo funding_transaction_builder .add_change_output( - funding_change_amount, + funding_change_output_amount, &sender_reserved_destinations, &mut rng, ) @@ -3176,57 +3175,62 @@ pub mod transaction_builder_tests { .find(|tx_out| { subaddress_matches_tx_out(&sender, GIFT_CODE_SUBADDRESS_INDEX, tx_out).unwrap() }) - .expect("Didn't find recipient's output"); + .expect("Didn't find gift code funding output"); - let funding_change = funding_tx + let funding_change_output = funding_tx .prefix .outputs .iter() .find(|tx_out| { subaddress_matches_tx_out(&sender, CHANGE_SUBADDRESS_INDEX, tx_out).unwrap() }) - .expect("Didn't find sender's output"); + .expect("Didn't gift code funding change output"); validate_tx_out(block_version, funding_output).unwrap(); - validate_tx_out(block_version, funding_change).unwrap(); + validate_tx_out(block_version, funding_change_output).unwrap(); assert!( subaddress_matches_tx_out(&sender, GIFT_CODE_SUBADDRESS_INDEX, funding_output) .unwrap() ); - assert!( - subaddress_matches_tx_out(&sender, CHANGE_SUBADDRESS_INDEX, funding_change) - .unwrap() - ); + assert!(subaddress_matches_tx_out( + &sender, + CHANGE_SUBADDRESS_INDEX, + funding_change_output + ) + .unwrap()); // Ensure funding output & change memos are correct - let gift_code_tx_out_public_key = + let funding_output_public_key = &RistrettoPublic::try_from(&funding_output.public_key).unwrap(); - let funding_change_tx_out_public_key = - &RistrettoPublic::try_from(&funding_change.public_key).unwrap(); - let gift_code_ss = - get_tx_out_shared_secret(sender.view_private_key(), gift_code_tx_out_public_key); - let funding_change_ss = get_tx_out_shared_secret( + let funding_change_output_tx_out_public_key = + &RistrettoPublic::try_from(&funding_change_output.public_key).unwrap(); + let funding_output_ss = + get_tx_out_shared_secret(sender.view_private_key(), funding_output_public_key); + let funding_change_output_ss = get_tx_out_shared_secret( sender.view_private_key(), - funding_change_tx_out_public_key, + funding_change_output_tx_out_public_key, ); let (funding_amount, _) = funding_output .masked_amount - .get_value(&gift_code_ss) + .get_value(&funding_output_ss) .unwrap(); assert_eq!(funding_amount.value, funding_output_amount.value); assert_eq!(funding_amount.token_id, token_id); if block_version.e_memo_feature_is_supported() { - let funding_change_memo = - funding_change.e_memo.unwrap().decrypt(&funding_change_ss); - let funding_output_memo = funding_output.e_memo.unwrap().decrypt(&gift_code_ss); - match MemoType::try_from(&funding_change_memo) + let funding_change_output_memo = funding_change_output + .e_memo + .unwrap() + .decrypt(&funding_change_output_ss); + let funding_output_memo = + funding_output.e_memo.unwrap().decrypt(&funding_output_ss); + match MemoType::try_from(&funding_change_output_memo) .expect("Couldn't decrypt funding change memo") { MemoType::GiftCodeFunding(memo) => { - assert!(memo.public_key_matches(gift_code_tx_out_public_key),); + assert!(memo.public_key_matches(funding_output_public_key),); assert_eq!(memo.funding_note().unwrap(), note,); } _ => { @@ -3244,22 +3248,23 @@ pub mod transaction_builder_tests { } // MCIP #32 specifies that the receiver will receive the TxOut index, - // shared_secret and onetime_private_key. Send a gift code from a - // simulated sender to a simulated receiver in the manner specified - // in the MCIP, by using the data from gift code TxOut funded above + // shared_secret and onetime_private_key in a message. Below we + // simulate a receiver sending the gift code to themselves with the + // those 3 pieces of information from the gift code TxOut funded above // Get the data we're pretending sender sends to the receiver and - // construct TxOutGiftCode object from it + // construct TxOutGiftCode object from it. This data would + // normally be sent via a protobuf message let global_index = 42; let gift_code_tx_out_private_key = recover_onetime_private_key( - gift_code_tx_out_public_key, + funding_output_public_key, sender.spend_private_key(), &sender.gift_code_subaddress_spend_private(), ); let tx_out_gift_code = TxOutGiftCode { global_index, onetime_private_key: gift_code_tx_out_private_key, - shared_secret: gift_code_ss, + shared_secret: funding_output_ss, }; // Values we pretend we get from locating the TxOut using the global index @@ -3481,7 +3486,7 @@ pub mod transaction_builder_tests { ); } - // Ensure we can't fund after change + // Ensure we can't write change before funding or fund after change { let funding_memo_builder = GiftCodeFundingMemoBuilder::new(note).unwrap(); From 16352f72778c099163853048657fac30c3e7414b Mon Sep 17 00:00:00 2001 From: iamalwaysuncomfortable Date: Wed, 18 May 2022 18:04:21 -0400 Subject: [PATCH 12/12] Code cleanup suggestions --- .../gift_code_funding_memo_builder.rs | 23 ++--- .../gift_code_sender_memo_builder.rs | 9 +- transaction/std/src/test_utils.rs | 87 ++++--------------- transaction/std/src/transaction_builder.rs | 63 ++++++-------- 4 files changed, 54 insertions(+), 128 deletions(-) diff --git a/transaction/std/src/memo_builder/gift_code_funding_memo_builder.rs b/transaction/std/src/memo_builder/gift_code_funding_memo_builder.rs index 6d957b3ba6..83ed505a82 100644 --- a/transaction/std/src/memo_builder/gift_code_funding_memo_builder.rs +++ b/transaction/std/src/memo_builder/gift_code_funding_memo_builder.rs @@ -29,12 +29,11 @@ use mc_transaction_core::{Amount, MemoContext, MemoPayload, NewMemoError}; pub struct GiftCodeFundingMemoBuilder { // Utf-8 note within the memo that can be up to 60 bytes long note: String, - // TxOut Public Key of the gift code TxOut sent to the gift code subaddress + // TxOut Public Key of the gift code TxOut sent to the gift code subaddress. + // This is populated when the output is created gift_code_tx_out_public_key: Option, // Whether we've already written the change memo wrote_change_memo: bool, - // Whether we've already written the gift code TxOut - wrote_destination_memo: bool, } impl GiftCodeFundingMemoBuilder { @@ -43,15 +42,15 @@ impl GiftCodeFundingMemoBuilder { /// note passed is longer than 60 bytes. pub fn new(note: &str) -> Result { if note.len() > GiftCodeFundingMemo::NOTE_DATA_LEN { - return Err(NewMemoError::BadInputs( - "Note memo cannot be greater than 60 bytes".into(), - )); + return Err(NewMemoError::BadInputs(format!( + "Note memo cannot be greater than {} bytes", + GiftCodeFundingMemo::NOTE_DATA_LEN + ))); } Ok(Self { note: note.into(), gift_code_tx_out_public_key: None, wrote_change_memo: false, - wrote_destination_memo: false, }) } } @@ -73,14 +72,13 @@ impl MemoBuilder for GiftCodeFundingMemoBuilder { memo_context: MemoContext, ) -> Result { // Only one gift code should be funded - if self.wrote_destination_memo { + if self.gift_code_tx_out_public_key.is_some() { return Err(NewMemoError::MultipleOutputs); } if self.wrote_change_memo { return Err(NewMemoError::OutputsAfterChange); } self.gift_code_tx_out_public_key = Some(*memo_context.tx_public_key); - self.wrote_destination_memo = true; Ok(UnusedMemo {}.into()) } @@ -91,9 +89,6 @@ impl MemoBuilder for GiftCodeFundingMemoBuilder { _change_destination: &ReservedDestination, memo_context: MemoContext, ) -> Result { - if !self.wrote_destination_memo { - return Err(NewMemoError::MissingOutput); - } if self.wrote_change_memo { return Err(NewMemoError::MultipleChangeOutputs); } @@ -104,9 +99,7 @@ impl MemoBuilder for GiftCodeFundingMemoBuilder { self.wrote_change_memo = true; Ok(GiftCodeFundingMemo::new(&tx_out_public_key, self.note.as_str())?.into()) } else { - Err(NewMemoError::MissingInput( - "Missing gift code TxOut public key".into(), - )) + Err(NewMemoError::MissingOutput) } } } diff --git a/transaction/std/src/memo_builder/gift_code_sender_memo_builder.rs b/transaction/std/src/memo_builder/gift_code_sender_memo_builder.rs index 11a4b6801d..66ea79ae33 100644 --- a/transaction/std/src/memo_builder/gift_code_sender_memo_builder.rs +++ b/transaction/std/src/memo_builder/gift_code_sender_memo_builder.rs @@ -19,7 +19,7 @@ use mc_transaction_core::{Amount, MemoContext, MemoPayload, NewMemoError}; /// from the sender's gift code subaddress to their own change subaddress. /// The sender memo is written into the TxOut the receiver sends to the /// change address and includes an optional Utf-8 note up to 64 bytes long -/// the Receiver can use to record any information they desire about the +/// the Receiver can use to record any information they desire about the /// gift code. #[derive(Clone, Debug)] pub struct GiftCodeSenderMemoBuilder { @@ -35,9 +35,10 @@ impl GiftCodeSenderMemoBuilder { /// note passed is longer than 64 bytes. pub fn new(note: &str) -> Result { if note.len() > GiftCodeSenderMemo::MEMO_DATA_LEN { - return Err(NewMemoError::BadInputs( - "Sender note cannot be longer than 64 bytes".into(), - )); + return Err(NewMemoError::BadInputs(format!( + "Note memo cannot be greater than {} bytes", + GiftCodeSenderMemo::MEMO_DATA_LEN + ))); } Ok(Self { note: note.into(), diff --git a/transaction/std/src/test_utils.rs b/transaction/std/src/test_utils.rs index 56ebf4a7c1..c23ed11daf 100644 --- a/transaction/std/src/test_utils.rs +++ b/transaction/std/src/test_utils.rs @@ -54,31 +54,6 @@ pub fn create_output( ) } -// Make fake outputs for a ring -fn add_fake_outputs_to_ring( - mut ring: Vec, - block_version: BlockVersion, - amount: Amount, - ring_size: usize, - fog_resolver: &FPR, - rng: &mut RNG, -) -> Vec { - // Create fake mixins with assorted token ids - for idx in 0..ring_size - 1 { - let address = AccountKey::random(rng).default_subaddress(); - let token_id = if block_version.masked_token_id_feature_is_supported() { - TokenId::from(idx as u64) - } else { - Mob::ID - }; - let amount = Amount::new(amount.value, token_id); - let (tx_out, _) = - create_output(block_version, amount, &address, fog_resolver, rng).unwrap(); - ring.push(tx_out); - } - ring -} - /// Creates a ring of of TxOuts. /// /// # Arguments @@ -99,17 +74,23 @@ pub fn get_ring( fog_resolver: &FPR, rng: &mut RNG, ) -> (Vec, usize) { + let mut ring: Vec = Vec::new(); + // Create ring_size - 1 mixins with assorted token ids - let mut ring: Vec = add_fake_outputs_to_ring( - Vec::new(), - block_version, - amount, - ring_size, - fog_resolver, - rng, - ); + for idx in 0..ring_size - 1 { + let address = AccountKey::random(rng).default_subaddress(); + let token_id = if block_version.masked_token_id_feature_is_supported() { + TokenId::from(idx as u64) + } else { + Mob::ID + }; + let amount = Amount::new(amount.value, token_id); + let (tx_out, _) = + create_output(block_version, amount, &address, fog_resolver, rng).unwrap(); + ring.push(tx_out); + } - // Insert the "real" element. + // Insert the real element. let real_index = (rng.next_u64() % ring_size as u64) as usize; let (tx_out, _) = create_output( block_version, @@ -125,44 +106,6 @@ pub fn get_ring( (ring, real_index) } -/// Get ring for existing TxOut -/// -/// # Arguments -/// * `tx_out` - The "real" TxOut to build the ring for -/// * `block_version` - The block version for the TxOut's -/// * `token_id` - The token id for the real element -/// * `ring_size` - Number of elements in the ring. -/// * `value` - Value of the real element. -/// * `fog_resolver` - Fog public keys -/// * `rng` - Randomness. -/// -/// Returns (ring, real_index) -pub fn get_ring_for_txout( - tx_out: &TxOut, - block_version: BlockVersion, - amount: Amount, - ring_size: usize, - fog_resolver: &FPR, - rng: &mut RNG, -) -> (Vec, usize) { - // Create ring_size - 1 mixins with assorted token ids - let mut ring: Vec = add_fake_outputs_to_ring( - Vec::new(), - block_version, - amount, - ring_size, - fog_resolver, - rng, - ); - - // Insert the TxOut - let real_index = (rng.next_u64() % ring_size as u64) as usize; - ring.insert(real_index, tx_out.clone()); - assert_eq!(ring.len(), ring_size); - - (ring, real_index) -} - /// Creates an `InputCredentials` for an account. /// /// # Arguments diff --git a/transaction/std/src/transaction_builder.rs b/transaction/std/src/transaction_builder.rs index dec12a25b0..151b2d8be4 100644 --- a/transaction/std/src/transaction_builder.rs +++ b/transaction/std/src/transaction_builder.rs @@ -574,7 +574,6 @@ pub(crate) fn create_output_with_fog_hint( if !block_version.masked_token_id_feature_is_supported() { tx_out.masked_amount.masked_token_id.clear(); } - // let shared_secret = create_shared_secret(recipient.view_public_key(), &private_key); Ok((tx_out, shared_secret)) } @@ -612,7 +611,7 @@ pub(crate) fn create_fog_hint( pub mod transaction_builder_tests { use super::*; use crate::{ - test_utils::{get_input_credentials, get_ring, get_ring_for_txout, get_transaction}, + test_utils::{create_output, get_input_credentials, get_ring, get_transaction}, BurnRedemptionMemoBuilder, EmptyMemoBuilder, GiftCodeCancellationMemoBuilder, GiftCodeFundingMemoBuilder, GiftCodeSenderMemoBuilder, MemoType, RTHMemoBuilder, SenderMemoCredential, @@ -3189,17 +3188,6 @@ pub mod transaction_builder_tests { validate_tx_out(block_version, funding_output).unwrap(); validate_tx_out(block_version, funding_change_output).unwrap(); - assert!( - subaddress_matches_tx_out(&sender, GIFT_CODE_SUBADDRESS_INDEX, funding_output) - .unwrap() - ); - assert!(subaddress_matches_tx_out( - &sender, - CHANGE_SUBADDRESS_INDEX, - funding_change_output - ) - .unwrap()); - // Ensure funding output & change memos are correct let funding_output_public_key = &RistrettoPublic::try_from(&funding_output.public_key).unwrap(); @@ -3236,15 +3224,11 @@ pub mod transaction_builder_tests { _ => { panic!("unexpected memo type") } - } - match MemoType::try_from(&funding_output_memo) - .expect("Couldn't decrypt gift code funding_output memo") - { - MemoType::Unused(_) => {} - _ => { - panic!("unexpected memo type") - } - } + }; + assert_matches!( + MemoType::try_from(&funding_output_memo), + Ok(MemoType::Unused(_)) + ); } // MCIP #32 specifies that the receiver will receive the TxOut index, @@ -3274,17 +3258,27 @@ pub mod transaction_builder_tests { let (sending_input_amount, blinding) = masked_amount .get_value(&tx_out_gift_code.shared_secret) .unwrap(); - println!("{:?}", sending_input_amount); let sending_output_amount = Amount::new(sending_input_amount.value - fee, token_id); - let (ring, real_index) = get_ring_for_txout( - funding_output, - block_version, - sending_input_amount, - 3, - &fog_resolver, - &mut rng, - ); + let ring_size = 3; + let mut ring: Vec = Vec::new(); + for idx in 0..ring_size - 1 { + let address = AccountKey::random(&mut rng).default_subaddress(); + let mixed_token_id = if block_version.masked_token_id_feature_is_supported() { + TokenId::from(idx as u64) + } else { + token_id + }; + let amount = Amount::new(sending_output_amount.value, mixed_token_id); + let (tx_out, _) = + create_output(block_version, amount, &address, &fog_resolver, &mut rng) + .unwrap(); + ring.push(tx_out); + } + + let real_index = (rng.next_u64() % ring_size as u64) as usize; + ring.insert(real_index, funding_output.clone()); + assert_eq!(ring.len(), ring_size); let membership_proofs: Vec = ring .iter() @@ -3342,8 +3336,6 @@ pub mod transaction_builder_tests { validate_tx_out(block_version, change).unwrap(); - assert!(subaddress_matches_tx_out(&receiver, CHANGE_SUBADDRESS_INDEX, change).unwrap()); - // Ensure change memo is correct let ss = get_tx_out_shared_secret( receiver.view_private_key(), @@ -3417,9 +3409,6 @@ pub mod transaction_builder_tests { validate_tx_out(block_version, change).unwrap(); - // Ensure it was sent to the change address - assert!(subaddress_matches_tx_out(&sender, CHANGE_SUBADDRESS_INDEX, change).unwrap()); - // Ensure change memo is correct let ss = get_tx_out_shared_secret( sender.view_private_key(), @@ -3553,7 +3542,7 @@ pub mod transaction_builder_tests { assert_matches!( output_after_change, Err(TxBuilderError::NewTx(NewTxError::Memo( - NewMemoError::MultipleOutputs + NewMemoError::OutputsAfterChange ))) ); }