From 45c9b7e3112e729ec82dbf9fe95c8ec08d1bb178 Mon Sep 17 00:00:00 2001 From: William J Date: Thu, 23 Mar 2023 01:37:32 -0400 Subject: [PATCH 01/20] Add new flexible memo generator option to rth memo builder. --- .../src/memo_builder/rth_memo_builder.rs | 560 +++++++++++++++--- .../builder/src/transaction_builder.rs | 16 +- transaction/core/src/tx_error.rs | 4 +- 3 files changed, 502 insertions(+), 78 deletions(-) diff --git a/transaction/builder/src/memo_builder/rth_memo_builder.rs b/transaction/builder/src/memo_builder/rth_memo_builder.rs index bf97cbf49e..0d5e03e1a9 100644 --- a/transaction/builder/src/memo_builder/rth_memo_builder.rs +++ b/transaction/builder/src/memo_builder/rth_memo_builder.rs @@ -1,21 +1,24 @@ +// Copyright (c) 2018-2023 The MobileCoin Foundation + // Copyright (c) 2018-2022 The MobileCoin Foundation //! Defines the RTHMemoBuilder. //! (RTH is an abbrevation of Recoverable Transaction History.) //! This MemoBuilder policy implements Recoverable Transaction History using //! the encrypted memos, as envisioned in MCIP #4. - use super::MemoBuilder; use crate::ReservedSubaddresses; +use alloc::{boxed::Box, fmt::Debug, format, string::String, sync::Arc}; +use displaydoc::Display; use mc_account_keys::{PublicAddress, ShortAddressHash}; use mc_transaction_core::{ tokens::Mob, Amount, MemoContext, MemoPayload, NewMemoError, Token, TokenId, }; use mc_transaction_extra::{ - AuthenticatedSenderMemo, AuthenticatedSenderWithPaymentIntentIdMemo, - AuthenticatedSenderWithPaymentRequestIdMemo, DestinationMemo, DestinationMemoError, - DestinationWithPaymentIntentIdMemo, DestinationWithPaymentRequestIdMemo, SenderMemoCredential, - UnusedMemo, + compute_authenticated_sender_memo, compute_destination_memo, AuthenticatedSenderMemo, + AuthenticatedSenderWithPaymentIntentIdMemo, AuthenticatedSenderWithPaymentRequestIdMemo, + DestinationMemo, DestinationMemoError, DestinationWithPaymentIntentIdMemo, + DestinationWithPaymentRequestIdMemo, SenderMemoCredential, UnusedMemo, }; /// This memo builder attaches 0x0100 Authenticated Sender Memos to normal @@ -56,10 +59,8 @@ use mc_transaction_extra::{ pub struct RTHMemoBuilder { // The credential used to form 0x0100 and 0x0101 memos, if present. sender_cred: Option, - // The payment request id, if any - payment_request_id: Option, - // The payment intent id, if any - payment_intent_id: Option, + // Different options for the custom memo data + custom_memo_type: Option, // Whether destination memos are enabled. destination_memo_enabled: bool, // Tracks if we already wrote a destination memo, for error reporting @@ -76,12 +77,70 @@ pub struct RTHMemoBuilder { fee: Amount, } +#[derive(Clone, Debug)] +pub enum CustomMemoType { + PaymentRequestId(u64), + PaymentIntentId(u64), + FlexibleMemoGenerator(FlexibleMemoGenerator), +} + +/// A function that returns a flexible memopayload for outputs +pub type FlexibleOutputMemoGenerator = Box< + dyn Fn(FlexibleMemoOutputContext) -> Result + Sync + Send, +>; +/// A function that returns a flexible memopayload for change +pub type FlexibleChangeMemoGenerator = Box< + dyn Fn(FlexibleMemoChangeContext) -> Result + Sync + Send, +>; + +pub struct FlexibleMemoBuilderContext { + pub sender_cred: Option, + // Tracks the last recipient + pub last_recipient: ShortAddressHash, + // Tracks the total outlay so far + pub total_outlay: u64, + // Tracks the total outlay token id + pub outlay_token_id: Option, + // Tracks the number of recipients so far + pub num_recipients: u8, + // Tracks the fee + pub fee: Amount, +} +pub struct FlexibleMemoOutputContext<'a> { + pub amount: Amount, + pub recipient: &'a PublicAddress, + pub memo_context: MemoContext<'a>, + pub builder_context: FlexibleMemoBuilderContext, +} +pub struct FlexibleMemoChangeContext<'a> { + pub memo_context: MemoContext<'a>, + pub amount: Amount, + pub change_destination: &'a ReservedSubaddresses, + pub builder_context: FlexibleMemoBuilderContext, +} + +pub struct FlexibleMemoPayload { + memo_type_bytes: [u8; 2], + memo_data: [u8; 32], +} + +#[derive(Clone)] +pub struct FlexibleMemoGenerator { + flexible_output_memo_generator: Arc, + flexible_change_memo_generator: Arc, +} + +impl Debug for FlexibleMemoGenerator { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + write!(f, "FlexibleMemoGenerator") + } +} + impl Default for RTHMemoBuilder { fn default() -> Self { Self { sender_cred: Default::default(), - payment_request_id: None, - payment_intent_id: None, + custom_memo_type: None, destination_memo_enabled: false, wrote_destination_memo: false, last_recipient: Default::default(), @@ -93,6 +152,18 @@ impl Default for RTHMemoBuilder { } } +/// An error that occurs when setting up a memo builder +/// +/// These errors are usually created setting invalid field combinations on the +/// memo builder. 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(Clone, Debug, Display, Eq, PartialEq)] +pub enum MemoBuilderError { + /// Invalid state change + StateChange(String), +} + impl RTHMemoBuilder { /// Set the sender credential. If no sender credential is provided, /// then authenticated sender memos cannot be produced. @@ -118,23 +189,47 @@ impl RTHMemoBuilder { } /// Set the payment request id. - pub fn set_payment_request_id(&mut self, id: u64) { - self.payment_request_id = Some(id); + pub fn set_payment_request_id(&mut self, id: u64) -> Result<(), MemoBuilderError> { + if self.custom_memo_type.is_some() { + return Err(MemoBuilderError::StateChange(format!( + "Custom Memo Type already set to {:?}", + self.custom_memo_type + ))); + } + self.custom_memo_type = Some(CustomMemoType::PaymentRequestId(id)); + Ok(()) } - /// Clear the payment request id. - pub fn clear_payment_request_id(&mut self) { - self.payment_request_id = None; + /// Clear the custom memo type. + pub fn clear_custom_memo_type(&mut self) { + self.custom_memo_type = None; } /// Set the payment intent id. - pub fn set_payment_intent_id(&mut self, id: u64) { - self.payment_intent_id = Some(id); + pub fn set_payment_intent_id(&mut self, id: u64) -> Result<(), MemoBuilderError> { + if self.custom_memo_type.is_some() { + return Err(MemoBuilderError::StateChange(format!( + "Custom Memo Type already set to {:?}", + self.custom_memo_type + ))); + } + self.custom_memo_type = Some(CustomMemoType::PaymentIntentId(id)); + Ok(()) } - /// Clear the payment intent id. - pub fn clear_payment_intent_id(&mut self) { - self.payment_intent_id = None; + /// Set the flexible memo generator. + pub fn set_flexible_memo_generator( + &mut self, + generator: FlexibleMemoGenerator, + ) -> Result<(), MemoBuilderError> { + if self.custom_memo_type.is_some() { + return Err(MemoBuilderError::StateChange(format!( + "Custom Memo Type already set to {:?}", + self.custom_memo_type + ))); + } + self.custom_memo_type = Some(CustomMemoType::FlexibleMemoGenerator(generator)); + Ok(()) } /// Enable destination memos @@ -146,6 +241,19 @@ impl RTHMemoBuilder { pub fn disable_destination_memo(&mut self) { self.destination_memo_enabled = false; } + + /// Generates a flexible memo builder context from fields set on the rth + /// memo builder to be used with the flexible memo generator function + pub fn get_flexible_memo_builder_context(&self) -> FlexibleMemoBuilderContext { + FlexibleMemoBuilderContext { + sender_cred: self.sender_cred.clone(), + last_recipient: self.last_recipient.clone(), + total_outlay: self.total_outlay, + outlay_token_id: self.outlay_token_id, + num_recipients: self.num_recipients, + fee: self.fee, + } + } } impl MemoBuilder for RTHMemoBuilder { @@ -186,26 +294,53 @@ impl MemoBuilder for RTHMemoBuilder { .checked_add(1) .ok_or(NewMemoError::LimitsExceeded("num_recipients"))?; self.last_recipient = ShortAddressHash::from(recipient); + let payload: MemoPayload = if let Some(cred) = &self.sender_cred { - if self.payment_request_id.is_some() && self.payment_intent_id.is_some() { - return Err(NewMemoError::RequestAndIntentIdSet); - } - if let Some(payment_request_id) = self.payment_request_id { - AuthenticatedSenderWithPaymentRequestIdMemo::new( - cred, - recipient.view_public_key(), - &memo_context.tx_public_key.into(), - payment_request_id, - ) - .into() - } else if let Some(payment_intent_id) = self.payment_intent_id { - AuthenticatedSenderWithPaymentIntentIdMemo::new( - cred, - recipient.view_public_key(), - &memo_context.tx_public_key.into(), - payment_intent_id, - ) - .into() + if let Some(custom_memo_type) = &self.custom_memo_type { + match custom_memo_type { + CustomMemoType::FlexibleMemoGenerator(flexible_memo_generator) => { + let tx_public_key = memo_context.tx_public_key; + let flexible_memo_context = FlexibleMemoOutputContext { + memo_context, + amount, + recipient, + builder_context: self.get_flexible_memo_builder_context(), + }; + let flexible_memo_payload = (flexible_memo_generator + .flexible_output_memo_generator)( + flexible_memo_context + )?; + let memo_data = compute_authenticated_sender_memo( + flexible_memo_payload.memo_type_bytes, + cred, + recipient.view_public_key(), + &tx_public_key.into(), + &flexible_memo_payload.memo_data, + ); + if flexible_memo_payload.memo_type_bytes[0] != 0x01 { + return Err(NewMemoError::FlexibleMemoGenerator(format!("The flexible output memo generator created a memo of the incorrect memo type: {:?}", flexible_memo_payload.memo_type_bytes))); + } + MemoPayload::new(flexible_memo_payload.memo_type_bytes, memo_data) + } + CustomMemoType::PaymentRequestId(payment_request_id) => { + AuthenticatedSenderWithPaymentRequestIdMemo::new( + cred, + recipient.view_public_key(), + &memo_context.tx_public_key.into(), + *payment_request_id, + ) + .into() + } + CustomMemoType::PaymentIntentId(payment_intent_id) => { + AuthenticatedSenderWithPaymentIntentIdMemo::new( + cred, + recipient.view_public_key(), + &memo_context.tx_public_key.into(), + *payment_intent_id, + ) + .into() + } + } } else { AuthenticatedSenderMemo::new( cred, @@ -254,42 +389,73 @@ impl MemoBuilder for RTHMemoBuilder { .total_outlay .checked_add(self.fee.value) .ok_or(NewMemoError::LimitsExceeded("total_outlay"))?; - - if self.payment_request_id.is_some() && self.payment_intent_id.is_some() { - return Err(NewMemoError::RequestAndIntentIdSet); - } - - if let Some(payment_request_id) = self.payment_request_id { - match DestinationWithPaymentRequestIdMemo::new( - self.last_recipient.clone(), - self.total_outlay, - self.fee.value, - payment_request_id, - ) { - Ok(mut d_memo) => { + if let Some(custom_memo_type) = &self.custom_memo_type { + match custom_memo_type { + CustomMemoType::FlexibleMemoGenerator(flexible_memo_generator) => { + let flexible_memo_context = FlexibleMemoChangeContext { + memo_context: _memo_context, + amount, + change_destination: _change_destination, + builder_context: self.get_flexible_memo_builder_context(), + }; + let flexible_memo_payload = (flexible_memo_generator + .flexible_change_memo_generator)( + flexible_memo_context + )?; + if flexible_memo_payload.memo_type_bytes[0] != 0x02 { + return Err(NewMemoError::FlexibleMemoGenerator(format!("The flexible change memo generator created a memo of the incorrect memo type: {:?}", flexible_memo_payload.memo_type_bytes))); + } + let memo_data = compute_destination_memo( + self.last_recipient.clone(), + self.fee.value, + self.num_recipients, + self.total_outlay, + flexible_memo_payload.memo_data, + ); + let payload: MemoPayload = + MemoPayload::new(flexible_memo_payload.memo_type_bytes, memo_data); + //TODO: Check whether fee is too large before setting wrote destination memo self.wrote_destination_memo = true; - d_memo.set_num_recipients(self.num_recipients); - Ok(d_memo.into()) + Ok(payload) } - Err(err) => match err { - DestinationMemoError::FeeTooLarge => Err(NewMemoError::LimitsExceeded("fee")), - }, - } - } else if let Some(payment_intent_id) = self.payment_intent_id { - match DestinationWithPaymentIntentIdMemo::new( - self.last_recipient.clone(), - self.total_outlay, - self.fee.value, - payment_intent_id, - ) { - Ok(mut d_memo) => { - self.wrote_destination_memo = true; - d_memo.set_num_recipients(self.num_recipients); - Ok(d_memo.into()) + CustomMemoType::PaymentRequestId(payment_request_id) => { + match DestinationWithPaymentRequestIdMemo::new( + self.last_recipient.clone(), + self.total_outlay, + self.fee.value, + *payment_request_id, + ) { + Ok(mut d_memo) => { + self.wrote_destination_memo = true; + d_memo.set_num_recipients(self.num_recipients); + Ok(d_memo.into()) + } + Err(err) => match err { + DestinationMemoError::FeeTooLarge => { + Err(NewMemoError::LimitsExceeded("fee")) + } + }, + } + } + CustomMemoType::PaymentIntentId(payment_intent_id) => { + match DestinationWithPaymentIntentIdMemo::new( + self.last_recipient.clone(), + self.total_outlay, + self.fee.value, + *payment_intent_id, + ) { + Ok(mut d_memo) => { + self.wrote_destination_memo = true; + d_memo.set_num_recipients(self.num_recipients); + Ok(d_memo.into()) + } + Err(err) => match err { + DestinationMemoError::FeeTooLarge => { + Err(NewMemoError::LimitsExceeded("fee")) + } + }, + } } - Err(err) => match err { - DestinationMemoError::FeeTooLarge => Err(NewMemoError::LimitsExceeded("fee")), - }, } } else { match DestinationMemo::new( @@ -309,3 +475,251 @@ impl MemoBuilder for RTHMemoBuilder { } } } + +#[cfg(test)] +mod tests { + use super::*; + use mc_account_keys::AccountKey; + use mc_crypto_keys::{CompressedRistrettoPublic, RistrettoPublic}; + use mc_transaction_extra::{ + AuthenticatedSenderWithDataMemo, DestinationWithDataMemo, RegisteredMemoType, + }; + use mc_util_from_random::FromRandom; + use rand::{rngs::StdRng, SeedableRng}; + + const AUTHENTICATED_CUSTOM_MEMO_TYPE_BYTES: [u8; 2] = [0x01, 0x08]; + const DESTINATION_CUSTOM_MEMO_TYPE_BYTES: [u8; 2] = [0x02, 0x08]; + + pub struct RTHMemoTestContext { + sender: AccountKey, + receiver: AccountKey, + funding_public_key: RistrettoPublic, + output_memo: Result, + change_memo: Result, + } + + fn get_valid_flexible_memo_generator() -> FlexibleMemoGenerator { + let flexible_output_memo_generator_closure: FlexibleOutputMemoGenerator = + Box::new(|_context: FlexibleMemoOutputContext| { + let memo_type_bytes = AUTHENTICATED_CUSTOM_MEMO_TYPE_BYTES; + let memo_data = [0x01; 32]; + Ok(FlexibleMemoPayload { + memo_type_bytes, + memo_data, + }) + }); + let flexible_change_memo_generator_closure: FlexibleChangeMemoGenerator = + Box::new(|_context: FlexibleMemoChangeContext| { + let memo_type_bytes = DESTINATION_CUSTOM_MEMO_TYPE_BYTES; + let memo_data = [0x01; 32]; + Ok(FlexibleMemoPayload { + memo_type_bytes, + memo_data, + }) + }); + + FlexibleMemoGenerator { + flexible_output_memo_generator: Arc::new(flexible_output_memo_generator_closure), + flexible_change_memo_generator: Arc::new(flexible_change_memo_generator_closure), + } + } + + fn get_flexible_memo_generator_returning_invalid_types() -> FlexibleMemoGenerator { + let flexible_output_memo_generator_closure: FlexibleOutputMemoGenerator = + Box::new(|_context: FlexibleMemoOutputContext| { + let memo_type_bytes = DESTINATION_CUSTOM_MEMO_TYPE_BYTES; + let memo_data = [0x01; 32]; + Ok(FlexibleMemoPayload { + memo_type_bytes, + memo_data, + }) + }); + let flexible_change_memo_generator_closure: FlexibleChangeMemoGenerator = + Box::new(|_context: FlexibleMemoChangeContext| { + let memo_type_bytes = AUTHENTICATED_CUSTOM_MEMO_TYPE_BYTES; + let memo_data = [0x01; 32]; + Ok(FlexibleMemoPayload { + memo_type_bytes, + memo_data, + }) + }); + + FlexibleMemoGenerator { + flexible_output_memo_generator: Arc::new(flexible_output_memo_generator_closure), + flexible_change_memo_generator: Arc::new(flexible_change_memo_generator_closure), + } + } + + fn build_rth_memos( + sender: AccountKey, + mut builder: RTHMemoBuilder, + funding_amount: Amount, + change_amount: Amount, + fee: Amount, + ) -> RTHMemoTestContext { + // Create simulated context + let mut rng: StdRng = SeedableRng::from_seed([0u8; 32]); + let sender_address_book = ReservedSubaddresses::from(&sender); + + let receiver = AccountKey::random_with_fog(&mut rng); + let receiver_primary_address = receiver.default_subaddress(); + + let funding_public_key = RistrettoPublic::from_random(&mut rng); + let funding_context = MemoContext { + tx_public_key: &funding_public_key, + }; + let change_tx_pubkey = RistrettoPublic::from_random(&mut rng); + let change_context = MemoContext { + tx_public_key: &change_tx_pubkey, + }; + + builder.set_fee(fee).unwrap(); + // Build blank output memo for TxOut at gift code address & funding memo to + // change output + let output_memo = builder.make_memo_for_output( + funding_amount, + &receiver_primary_address, + funding_context, + ); + let change_memo = builder.make_memo_for_change_output( + change_amount, + &sender_address_book, + change_context, + ); + RTHMemoTestContext { + sender, + receiver, + funding_public_key, + output_memo, + change_memo, + } + } + + fn build_test_memo_builder(rng: &mut StdRng) -> (AccountKey, RTHMemoBuilder) { + let sender = AccountKey::random(rng); + let mut memo_builder = RTHMemoBuilder::default(); + memo_builder.set_sender_credential(SenderMemoCredential::from(&sender)); + memo_builder.enable_destination_memo(); + (sender, memo_builder) + } + + #[test] + fn test_funding_memo_built_successfully() { + // Create Memo Builder with data + let fee = Amount::new(1, 0.into()); + let change_amount = Amount::new(1, 0.into()); + let funding_amount = Amount::new(10, 0.into()); + + let mut rng: StdRng = SeedableRng::from_seed([0u8; 32]); + let (sender, builder) = build_test_memo_builder(&mut rng); + // Build the memo payload + let memo_test_context = + build_rth_memos(sender, builder, funding_amount, change_amount, fee); + + let output_memo = memo_test_context + .output_memo + .expect("Expected valid output memo"); + let change_memo = memo_test_context + .change_memo + .expect("Expected valid change memo"); + + // Verify memo type + assert_eq!( + AuthenticatedSenderMemo::MEMO_TYPE_BYTES, + output_memo.get_memo_type().clone() + ); + assert_eq!( + DestinationMemo::MEMO_TYPE_BYTES, + change_memo.get_memo_type().clone() + ); + + // Verify memo data + let authenticated_memo = AuthenticatedSenderMemo::from(output_memo.get_memo_data()); + let destination_memo = DestinationMemo::from(change_memo.get_memo_data()); + + authenticated_memo.validate( + &memo_test_context.sender.default_subaddress(), + memo_test_context.receiver.view_private_key(), + &CompressedRistrettoPublic::from(memo_test_context.funding_public_key), + ); + + let derived_fee = destination_memo.get_fee(); + assert_eq!(fee.value, derived_fee); + } + + #[test] + fn test_flexible_funding_output_memo_built_successfully() { + // Create Memo Builder with data + let fee = Amount::new(1, 0.into()); + let change_amount = Amount::new(1, 0.into()); + let funding_amount = Amount::new(10, 0.into()); + + let mut rng: StdRng = SeedableRng::from_seed([0u8; 32]); + let (sender, mut builder) = build_test_memo_builder(&mut rng); + // Add a flexible memo generator + builder + .set_flexible_memo_generator(get_valid_flexible_memo_generator()) + .expect("No other custom memo type should be set"); + // Build the memo payload + let memo_test_context = + build_rth_memos(sender, builder, funding_amount, change_amount, fee); + + let output_memo = memo_test_context + .output_memo + .expect("Expected valid output memo"); + let change_memo = memo_test_context + .change_memo + .expect("Expected valid change memo"); + + // Verify memo type + assert_eq!( + AUTHENTICATED_CUSTOM_MEMO_TYPE_BYTES, + output_memo.get_memo_type().clone() + ); + assert_eq!( + DESTINATION_CUSTOM_MEMO_TYPE_BYTES, + change_memo.get_memo_type().clone() + ); + + // Verify memo data + let authenticated_memo = AuthenticatedSenderWithDataMemo::from(output_memo.get_memo_data()); + let destination_memo = DestinationWithDataMemo::from(change_memo.get_memo_data()); + + authenticated_memo.validate( + &memo_test_context.sender.default_subaddress(), + memo_test_context.receiver.view_private_key(), + &CompressedRistrettoPublic::from(memo_test_context.funding_public_key), + ); + + let derived_fee = destination_memo.get_fee(); + assert_eq!(fee.value, derived_fee); + + assert_eq!(authenticated_memo.data(), [0x01; 32]); + assert_eq!(destination_memo.get_data(), [0x01; 32]); + } + + #[test] + fn test_flexible_funding_output_memo_rejects_invalid_types() { + // Create Memo Builder with data + let fee = Amount::new(1, 0.into()); + let change_amount = Amount::new(1, 0.into()); + let funding_amount = Amount::new(10, 0.into()); + + let mut rng: StdRng = SeedableRng::from_seed([0u8; 32]); + let (sender, mut builder) = build_test_memo_builder(&mut rng); + // Add a flexible memo generator + builder + .set_flexible_memo_generator(get_flexible_memo_generator_returning_invalid_types()) + .expect("No other custom memo types should be set"); + // Build the memo payload + let memo_test_context = + build_rth_memos(sender, builder, funding_amount, change_amount, fee); + + assert_eq!(memo_test_context + .output_memo + .expect_err("Should have an invalid output memo type"), NewMemoError::FlexibleMemoGenerator("The flexible output memo generator created a memo of the incorrect memo type: [2, 8]".to_owned())); + assert_eq!(memo_test_context + .change_memo + .expect_err("Should have an invalid destination memo type"), NewMemoError::FlexibleMemoGenerator("The flexible change memo generator created a memo of the incorrect memo type: [1, 8]".to_owned())); + } +} diff --git a/transaction/builder/src/transaction_builder.rs b/transaction/builder/src/transaction_builder.rs index 04e1bfb71f..f69b58c4a3 100644 --- a/transaction/builder/src/transaction_builder.rs +++ b/transaction/builder/src/transaction_builder.rs @@ -1845,7 +1845,9 @@ pub mod transaction_builder_tests { let mut memo_builder = RTHMemoBuilder::default(); memo_builder.set_sender_credential(SenderMemoCredential::from(&sender)); memo_builder.enable_destination_memo(); - memo_builder.set_payment_request_id(42); + memo_builder + .set_payment_request_id(42) + .expect("No other memo types should be set"); let mut transaction_builder = TransactionBuilder::new( block_version, @@ -2010,7 +2012,9 @@ pub mod transaction_builder_tests { { let mut memo_builder = RTHMemoBuilder::default(); memo_builder.set_sender_credential(SenderMemoCredential::from(&sender)); - memo_builder.set_payment_request_id(47); + memo_builder + .set_payment_request_id(47) + .expect("No other memo types should be set"); let mut transaction_builder = TransactionBuilder::new( block_version, @@ -2162,7 +2166,9 @@ pub mod transaction_builder_tests { { let mut memo_builder = RTHMemoBuilder::default(); memo_builder.enable_destination_memo(); - memo_builder.set_payment_request_id(47); + memo_builder + .set_payment_request_id(47) + .expect("No other memo types should be set"); let mut transaction_builder = TransactionBuilder::new( block_version, @@ -2310,7 +2316,9 @@ pub mod transaction_builder_tests { let mut memo_builder = RTHMemoBuilder::default(); memo_builder.set_sender_credential(SenderMemoCredential::from(&sender)); memo_builder.enable_destination_memo(); - memo_builder.set_payment_intent_id(4855282172840142080); + memo_builder + .set_payment_intent_id(4855282172840142080) + .expect("No other memo types should be set"); let mut transaction_builder = TransactionBuilder::new( block_version, diff --git a/transaction/core/src/tx_error.rs b/transaction/core/src/tx_error.rs index b352ef5490..540d46c09e 100644 --- a/transaction/core/src/tx_error.rs +++ b/transaction/core/src/tx_error.rs @@ -1,4 +1,4 @@ -// Copyright (c) 2018-2022 The MobileCoin Foundation +// Copyright (c) 2018-2023 The MobileCoin Foundation //! Errors that can occur when creating a new TxOut @@ -100,6 +100,8 @@ pub enum NewMemoError { MaxFeeExceeded(u64, u64), /// Payment request and intent ID both are set RequestAndIntentIdSet, + /// Error in the Flexible memo generation + FlexibleMemoGenerator(String), /// Defragmentation transaction with non-zero change DefragWithChange, /// Other: {0} From 73139239b4d67bd3b45a24ac3ad721d300766c84 Mon Sep 17 00:00:00 2001 From: William J Date: Mon, 27 Mar 2023 03:52:45 -0400 Subject: [PATCH 02/20] Cleanup --- transaction/builder/src/memo_builder/rth_memo_builder.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/transaction/builder/src/memo_builder/rth_memo_builder.rs b/transaction/builder/src/memo_builder/rth_memo_builder.rs index 0d5e03e1a9..cf16f5bb55 100644 --- a/transaction/builder/src/memo_builder/rth_memo_builder.rs +++ b/transaction/builder/src/memo_builder/rth_memo_builder.rs @@ -1,7 +1,5 @@ // Copyright (c) 2018-2023 The MobileCoin Foundation -// Copyright (c) 2018-2022 The MobileCoin Foundation - //! Defines the RTHMemoBuilder. //! (RTH is an abbrevation of Recoverable Transaction History.) //! This MemoBuilder policy implements Recoverable Transaction History using @@ -106,12 +104,14 @@ pub struct FlexibleMemoBuilderContext { // Tracks the fee pub fee: Amount, } + pub struct FlexibleMemoOutputContext<'a> { pub amount: Amount, pub recipient: &'a PublicAddress, pub memo_context: MemoContext<'a>, pub builder_context: FlexibleMemoBuilderContext, } + pub struct FlexibleMemoChangeContext<'a> { pub memo_context: MemoContext<'a>, pub amount: Amount, @@ -162,6 +162,8 @@ impl Default for RTHMemoBuilder { pub enum MemoBuilderError { /// Invalid state change StateChange(String), + /// Other + Other(String), } impl RTHMemoBuilder { From 8ae53d469c55c086fd2ced41794af3a32b65d931 Mon Sep 17 00:00:00 2001 From: William J Date: Mon, 27 Mar 2023 06:10:58 -0400 Subject: [PATCH 03/20] Add helper functions for extracting data from authenticated sender and destination memos --- transaction/extra/src/lib.rs | 4 +++- transaction/extra/src/memo/authenticated_common.rs | 7 +++++++ transaction/extra/src/memo/destination.rs | 7 +++++++ transaction/extra/src/memo/mod.rs | 10 ++++++++-- 4 files changed, 25 insertions(+), 3 deletions(-) diff --git a/transaction/extra/src/lib.rs b/transaction/extra/src/lib.rs index 65157d283f..0fa54a6945 100644 --- a/transaction/extra/src/lib.rs +++ b/transaction/extra/src/lib.rs @@ -19,7 +19,9 @@ mod tx_out_gift_code; mod unsigned_tx; pub use memo::{ - compute_authenticated_sender_memo, compute_destination_memo, AuthenticatedSenderMemo, + compute_authenticated_sender_memo, compute_destination_memo, + get_data_from_authenticated_sender_memo, get_data_from_destination_memo, + validate_authenticated_sender, AuthenticatedSenderMemo, AuthenticatedSenderWithPaymentIntentIdMemo, AuthenticatedSenderWithPaymentRequestIdMemo, BurnRedemptionMemo, DefragmentationMemo, DefragmentationMemoError, DestinationMemo, DestinationMemoError, DestinationWithPaymentIntentIdMemo, DestinationWithPaymentRequestIdMemo, diff --git a/transaction/extra/src/memo/authenticated_common.rs b/transaction/extra/src/memo/authenticated_common.rs index 54bd69fd1a..2d46ad5660 100644 --- a/transaction/extra/src/memo/authenticated_common.rs +++ b/transaction/extra/src/memo/authenticated_common.rs @@ -101,3 +101,10 @@ pub fn compute_authenticated_sender_memo( memo_data[48..].copy_from_slice(&hmac_value); memo_data } + +/// Shared code that returns the payload portion of authenticated sender memos +pub fn get_data_from_authenticated_sender_memo(memo_data: &[u8]) -> [u8; 32] { + let mut data = [0u8; 32]; + data.copy_from_slice(&memo_data[16..48]); + data +} diff --git a/transaction/extra/src/memo/destination.rs b/transaction/extra/src/memo/destination.rs index f0395a6b7d..67c7f763b0 100644 --- a/transaction/extra/src/memo/destination.rs +++ b/transaction/extra/src/memo/destination.rs @@ -177,4 +177,11 @@ pub fn compute_destination_memo( memo_data } +/// Shared code that returns the payload portion of destination memos +pub fn get_data_from_destination_memo(memo_data: &[u8]) -> [u8; 32] { + let mut data = [0u8; 32]; + data.copy_from_slice(&memo_data[32..64]); + data +} + impl_memo_type_conversions! { DestinationMemo } diff --git a/transaction/extra/src/memo/mod.rs b/transaction/extra/src/memo/mod.rs index 38a0e7d9fd..0f1ceddf54 100644 --- a/transaction/extra/src/memo/mod.rs +++ b/transaction/extra/src/memo/mod.rs @@ -48,14 +48,20 @@ //! | 0x0204 | Destination With Payment Intent Id Memo | pub use self::{ - authenticated_common::{compute_authenticated_sender_memo, compute_category1_hmac}, + authenticated_common::{ + compute_authenticated_sender_memo, compute_category1_hmac, + get_data_from_authenticated_sender_memo, validate_authenticated_sender, + }, authenticated_sender::AuthenticatedSenderMemo, authenticated_sender_with_payment_intent_id::AuthenticatedSenderWithPaymentIntentIdMemo, authenticated_sender_with_payment_request_id::AuthenticatedSenderWithPaymentRequestIdMemo, burn_redemption::BurnRedemptionMemo, credential::SenderMemoCredential, defragmentation::{DefragmentationMemo, DefragmentationMemoError}, - destination::{compute_destination_memo, DestinationMemo, DestinationMemoError}, + destination::{ + compute_destination_memo, get_data_from_destination_memo, DestinationMemo, + DestinationMemoError, + }, destination_with_payment_intent_id::DestinationWithPaymentIntentIdMemo, destination_with_payment_request_id::DestinationWithPaymentRequestIdMemo, gift_code_cancellation::GiftCodeCancellationMemo, From f0e99a36e675ba89d3226908fddd270d8ca43ae8 Mon Sep 17 00:00:00 2001 From: William J Date: Mon, 27 Mar 2023 06:12:07 -0400 Subject: [PATCH 04/20] Expose flexible memo types and update test --- transaction/builder/src/lib.rs | 8 ++- transaction/builder/src/memo_builder/mod.rs | 8 ++- .../src/memo_builder/rth_memo_builder.rs | 67 ++++++++++++++----- 3 files changed, 63 insertions(+), 20 deletions(-) diff --git a/transaction/builder/src/lib.rs b/transaction/builder/src/lib.rs index 0aff1c2d5d..47824da2d3 100644 --- a/transaction/builder/src/lib.rs +++ b/transaction/builder/src/lib.rs @@ -1,4 +1,4 @@ -// Copyright (c) 2018-2022 The MobileCoin Foundation +// Copyright (c) 2018-2023 The MobileCoin Foundation //! Utilities for creating MobileCoin transactions, intended for client-side //! use and not intended to be used inside of enclaves. @@ -23,8 +23,10 @@ pub use error::{SignedContingentInputBuilderError, TxBuilderError}; pub use input_credentials::InputCredentials; pub use memo_builder::{ BurnRedemptionMemoBuilder, DefragmentationMemoBuilder, EmptyMemoBuilder, - GiftCodeCancellationMemoBuilder, GiftCodeFundingMemoBuilder, GiftCodeSenderMemoBuilder, - MemoBuilder, RTHMemoBuilder, + FlexibleChangeMemoGenerator, FlexibleMemoBuilderContext, FlexibleMemoChangeContext, + FlexibleMemoGenerator, FlexibleMemoOutputContext, FlexibleMemoPayload, + FlexibleOutputMemoGenerator, GiftCodeCancellationMemoBuilder, GiftCodeFundingMemoBuilder, + GiftCodeSenderMemoBuilder, MemoBuilder, RTHMemoBuilder, }; pub use reserved_subaddresses::ReservedSubaddresses; pub use signed_contingent_input_builder::SignedContingentInputBuilder; diff --git a/transaction/builder/src/memo_builder/mod.rs b/transaction/builder/src/memo_builder/mod.rs index 4eee3ffdc2..f4791ce7ca 100644 --- a/transaction/builder/src/memo_builder/mod.rs +++ b/transaction/builder/src/memo_builder/mod.rs @@ -1,4 +1,4 @@ -// Copyright (c) 2018-2022 The MobileCoin Foundation +// Copyright (c) 2018-2023 The MobileCoin Foundation //! Defines the MemoBuilder trait, and the Default implementation //! The memo builder for recoverable transaction history is defined in a @@ -22,7 +22,11 @@ pub use defragmentation_memo_builder::DefragmentationMemoBuilder; 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; +pub use rth_memo_builder::{ + FlexibleChangeMemoGenerator, FlexibleMemoBuilderContext, FlexibleMemoChangeContext, + FlexibleMemoGenerator, FlexibleMemoOutputContext, FlexibleMemoPayload, + FlexibleOutputMemoGenerator, RTHMemoBuilder, +}; /// The MemoBuilder trait defines the API that the transaction builder uses /// to ask the memo builder to build a memo for a particular TxOut. diff --git a/transaction/builder/src/memo_builder/rth_memo_builder.rs b/transaction/builder/src/memo_builder/rth_memo_builder.rs index cf16f5bb55..41bdcc6fb7 100644 --- a/transaction/builder/src/memo_builder/rth_memo_builder.rs +++ b/transaction/builder/src/memo_builder/rth_memo_builder.rs @@ -91,43 +91,72 @@ pub type FlexibleChangeMemoGenerator = Box< dyn Fn(FlexibleMemoChangeContext) -> Result + Sync + Send, >; +///This is the context provided to the flexible memo generator which is set on +/// the rth memo builder. Each of these fields has a direct corresponds to a +/// field in the RTHMemoBuilder pub struct FlexibleMemoBuilderContext { + /// The credential used to form 0x0100 and 0x0101 memos, if present. pub sender_cred: Option, - // Tracks the last recipient + /// Tracks the last recipient pub last_recipient: ShortAddressHash, - // Tracks the total outlay so far + /// Tracks the total outlay so far pub total_outlay: u64, - // Tracks the total outlay token id + /// Tracks the total outlay token id pub outlay_token_id: Option, - // Tracks the number of recipients so far + /// Tracks the number of recipients so far pub num_recipients: u8, - // Tracks the fee + /// Tracks the fee pub fee: Amount, } +///This is the context provided to the flexible memo generator which is set +/// when generating the output memo pub struct FlexibleMemoOutputContext<'a> { + /// Output amount pub amount: Amount, + /// Memo recipient pub recipient: &'a PublicAddress, + /// Additional context provided for generating the output memo. pub memo_context: MemoContext<'a>, + /// Context available from the RthMemoBuilder pub builder_context: FlexibleMemoBuilderContext, } +///This is the context provided to the flexible memo generator which is set +/// when generating specifically the change memo pub struct FlexibleMemoChangeContext<'a> { + /// Additional context provided for generating the output memo. pub memo_context: MemoContext<'a>, + /// Change amount pub amount: Amount, + /// The subaddress that the change is being sent to. pub change_destination: &'a ReservedSubaddresses, + /// Context available from the RthMemoBuilder pub builder_context: FlexibleMemoBuilderContext, } +/// This is the result of the flexible memo generators. pub struct FlexibleMemoPayload { - memo_type_bytes: [u8; 2], - memo_data: [u8; 32], + /// memo_type_bytes corresponds to the returned memo type. This should be + /// listed in an mcip. + pub memo_type_bytes: [u8; 2], + /// memo_data: corresponds to some 32 byte encoding of data used for + /// the returned memo type, and does not include fields used for the + /// authenticated sender or destination super types like the + /// SenderMemoCredential + pub memo_data: [u8; 32], } +/// This is a pair of generators used for creating flexible output memos and +/// flexible change memos #[derive(Clone)] pub struct FlexibleMemoGenerator { - flexible_output_memo_generator: Arc, - flexible_change_memo_generator: Arc, + /// Flexible_output_memo_generator: Is called when generating output memos. + /// It must return a memo payload with the first byte being 0x01 + pub flexible_output_memo_generator: Arc, + /// Flexible_change_memo_generator: Is called when genearting change memos. + /// It must return a memo payload with the first byte being 0x02 + pub flexible_change_memo_generator: Arc, } impl Debug for FlexibleMemoGenerator { @@ -484,7 +513,8 @@ mod tests { use mc_account_keys::AccountKey; use mc_crypto_keys::{CompressedRistrettoPublic, RistrettoPublic}; use mc_transaction_extra::{ - AuthenticatedSenderWithDataMemo, DestinationWithDataMemo, RegisteredMemoType, + get_data_from_authenticated_sender_memo, get_data_from_destination_memo, + validate_authenticated_sender, RegisteredMemoType, }; use mc_util_from_random::FromRandom; use rand::{rngs::StdRng, SeedableRng}; @@ -684,20 +714,27 @@ mod tests { ); // Verify memo data - let authenticated_memo = AuthenticatedSenderWithDataMemo::from(output_memo.get_memo_data()); - let destination_memo = DestinationWithDataMemo::from(change_memo.get_memo_data()); + let destination_memo = DestinationMemo::from(change_memo.get_memo_data()); - authenticated_memo.validate( + validate_authenticated_sender( &memo_test_context.sender.default_subaddress(), memo_test_context.receiver.view_private_key(), &CompressedRistrettoPublic::from(memo_test_context.funding_public_key), + output_memo.get_memo_type().clone(), + output_memo.get_memo_data(), ); let derived_fee = destination_memo.get_fee(); assert_eq!(fee.value, derived_fee); - assert_eq!(authenticated_memo.data(), [0x01; 32]); - assert_eq!(destination_memo.get_data(), [0x01; 32]); + assert_eq!( + get_data_from_authenticated_sender_memo(output_memo.get_memo_data()), + [1u8; 32] + ); + assert_eq!( + get_data_from_destination_memo(change_memo.get_memo_data()), + [1u8; 32] + ); } #[test] From f0085c414cee12b790f5c8463142d09f77027e17 Mon Sep 17 00:00:00 2001 From: William J Date: Mon, 27 Mar 2023 06:12:30 -0400 Subject: [PATCH 05/20] Expand rth test in transaction builder to use flexible memo generator --- .../builder/src/transaction_builder.rs | 206 +++++++++++++++++- 1 file changed, 204 insertions(+), 2 deletions(-) diff --git a/transaction/builder/src/transaction_builder.rs b/transaction/builder/src/transaction_builder.rs index f69b58c4a3..a16473072a 100644 --- a/transaction/builder/src/transaction_builder.rs +++ b/transaction/builder/src/transaction_builder.rs @@ -922,10 +922,12 @@ pub mod transaction_builder_tests { use crate::{ test_utils::{create_output, get_input_credentials, get_ring, get_transaction}, BurnRedemptionMemoBuilder, DefragmentationMemoBuilder, EmptyMemoBuilder, + FlexibleChangeMemoGenerator, FlexibleMemoChangeContext, FlexibleMemoGenerator, + FlexibleMemoOutputContext, FlexibleMemoPayload, FlexibleOutputMemoGenerator, GiftCodeCancellationMemoBuilder, GiftCodeFundingMemoBuilder, GiftCodeSenderMemoBuilder, RTHMemoBuilder, }; - use alloc::{string::ToString, vec}; + use alloc::{string::ToString, sync::Arc, vec}; use assert_matches::assert_matches; use maplit::btreemap; use mc_account_keys::{ @@ -944,7 +946,10 @@ pub mod transaction_builder_tests { validation::{validate_signature, validate_tx_out}, NewTxError, TokenId, }; - use mc_transaction_extra::{MemoType, SenderMemoCredential, TxOutGiftCode}; + use mc_transaction_extra::{ + AuthenticatedSenderWithPaymentRequestIdMemo, DestinationWithPaymentRequestIdMemo, MemoType, + RegisteredMemoType, SenderMemoCredential, TxOutGiftCode, + }; use rand::{rngs::StdRng, SeedableRng}; // Helper which produces a list of block_version, TokenId pairs to iterate over @@ -959,6 +964,36 @@ pub mod transaction_builder_tests { ] } + fn get_valid_flexible_memo_generator() -> FlexibleMemoGenerator { + let flexible_output_memo_generator_closure: FlexibleOutputMemoGenerator = + Box::new(|_context: FlexibleMemoOutputContext| { + let payment_request_id = 42u64; + let memo_type_bytes = AuthenticatedSenderWithPaymentRequestIdMemo::MEMO_TYPE_BYTES; + let mut memo_data = [0x00; 32]; + memo_data[0..8].copy_from_slice(&payment_request_id.to_be_bytes()); + Ok(FlexibleMemoPayload { + memo_type_bytes, + memo_data, + }) + }); + let flexible_change_memo_generator_closure: FlexibleChangeMemoGenerator = + Box::new(|_context: FlexibleMemoChangeContext| { + let payment_request_id = 42u64; + let memo_type_bytes = DestinationWithPaymentRequestIdMemo::MEMO_TYPE_BYTES; + let mut memo_data = [0u8; 32]; + memo_data[0..8].copy_from_slice(&payment_request_id.to_be_bytes()); + Ok(FlexibleMemoPayload { + memo_type_bytes, + memo_data, + }) + }); + + FlexibleMemoGenerator { + flexible_output_memo_generator: Arc::new(flexible_output_memo_generator_closure), + flexible_change_memo_generator: Arc::new(flexible_change_memo_generator_closure), + } + } + #[test] // Spend a single input and send its full value to a single recipient. fn test_simple_transaction() { @@ -2478,6 +2513,173 @@ pub mod transaction_builder_tests { } } } + // Enable both sender and destination memos, and set a flexible memo generator + { + let mut memo_builder = RTHMemoBuilder::default(); + memo_builder.set_sender_credential(SenderMemoCredential::from(&sender)); + memo_builder.enable_destination_memo(); + memo_builder + .set_flexible_memo_generator(get_valid_flexible_memo_generator()) + .expect("No other memo types should be set"); + + let mut transaction_builder = TransactionBuilder::new( + block_version, + Amount::new(Mob::MINIMUM_FEE, token_id), + fog_resolver.clone(), + memo_builder, + ) + .unwrap(); + + transaction_builder.set_tombstone_block(2000); + + let input_credentials = get_input_credentials( + block_version, + Amount { value, token_id }, + &sender, + &fog_resolver, + &mut rng, + ); + transaction_builder.add_input(input_credentials); + + transaction_builder + .add_output( + Amount::new(value - change_value - Mob::MINIMUM_FEE, token_id), + &recipient_address, + &mut rng, + ) + .unwrap(); + + transaction_builder + .add_change_output( + Amount::new(change_value, token_id), + &sender_change_dest, + &mut rng, + ) + .unwrap(); + + let tx = transaction_builder + .build(&NoKeysRingSigner {}, &mut rng) + .unwrap(); + + // The transaction should have two output. + assert_eq!(tx.prefix.outputs.len(), 2); + + // The tombstone block should be the min of what the user requested, and what + // fog limits it to + assert_eq!(tx.prefix.tombstone_block, 1000); + + let output = tx + .prefix + .outputs + .iter() + .find(|tx_out| { + subaddress_matches_tx_out(&recipient, DEFAULT_SUBADDRESS_INDEX, tx_out) + .unwrap() + }) + .expect("Didn't find recipient's output"); + 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, output).unwrap(); + validate_tx_out(block_version, change).unwrap(); + + assert!( + !subaddress_matches_tx_out(&recipient, DEFAULT_SUBADDRESS_INDEX, change) + .unwrap() + ); + assert!( + !subaddress_matches_tx_out(&sender, DEFAULT_SUBADDRESS_INDEX, change).unwrap() + ); + assert!( + !subaddress_matches_tx_out(&sender, CHANGE_SUBADDRESS_INDEX, output).unwrap() + ); + assert!( + !subaddress_matches_tx_out(&recipient, CHANGE_SUBADDRESS_INDEX, output) + .unwrap() + ); + + // The 1st output should belong to the correct recipient and have correct amount + // and have correct memo + { + let ss = get_tx_out_shared_secret( + recipient.view_private_key(), + &RistrettoPublic::try_from(&output.public_key).unwrap(), + ); + let (amount, _) = output.get_masked_amount().unwrap().get_value(&ss).unwrap(); + assert_eq!(amount.value, value - change_value - Mob::MINIMUM_FEE); + assert_eq!(amount.token_id, token_id); + + if block_version.e_memo_feature_is_supported() { + let memo = output.e_memo.unwrap().decrypt(&ss); + match MemoType::try_from(&memo).expect("Couldn't decrypt memo") { + MemoType::AuthenticatedSenderWithPaymentRequestId(memo) => { + assert_eq!( + memo.sender_address_hash(), + ShortAddressHash::from(&sender_addr), + "lookup based on address hash failed" + ); + assert!( + bool::from( + memo.validate( + &sender_addr, + &recipient + .subaddress_view_private(DEFAULT_SUBADDRESS_INDEX), + &output.public_key, + ) + ), + "hmac validation failed" + ); + assert_eq!(memo.payment_request_id(), 42); + } + _ => { + panic!("unexpected memo type") + } + } + } + } + + // The 2nd output should belong to the correct recipient and have correct amount + // and have correct memo + { + let ss = get_tx_out_shared_secret( + sender.view_private_key(), + &RistrettoPublic::try_from(&change.public_key).unwrap(), + ); + let (amount, _) = change.get_masked_amount().unwrap().get_value(&ss).unwrap(); + assert_eq!(amount.value, change_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::DestinationWithPaymentRequestId(memo) => { + assert_eq!( + memo.get_address_hash(), + &ShortAddressHash::from(&recipient_address), + "lookup based on address hash failed" + ); + assert_eq!(memo.get_num_recipients(), 1); + assert_eq!(memo.get_fee(), Mob::MINIMUM_FEE); + assert_eq!( + memo.get_total_outlay(), + value - change_value, + "outlay should be amount sent to recipient + fee" + ); + assert_eq!(memo.get_payment_request_id(), 42); + } + _ => { + panic!("unexpected memo type") + } + } + } + } + } } } From 1b8651ed6e405a9e5393a38362634acf02bc7d0e Mon Sep 17 00:00:00 2001 From: William J Date: Mon, 27 Mar 2023 12:14:59 -0400 Subject: [PATCH 06/20] lint --- transaction/builder/src/memo_builder/rth_memo_builder.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/transaction/builder/src/memo_builder/rth_memo_builder.rs b/transaction/builder/src/memo_builder/rth_memo_builder.rs index 41bdcc6fb7..1e11861f80 100644 --- a/transaction/builder/src/memo_builder/rth_memo_builder.rs +++ b/transaction/builder/src/memo_builder/rth_memo_builder.rs @@ -720,7 +720,7 @@ mod tests { &memo_test_context.sender.default_subaddress(), memo_test_context.receiver.view_private_key(), &CompressedRistrettoPublic::from(memo_test_context.funding_public_key), - output_memo.get_memo_type().clone(), + output_memo.get_memo_type(), output_memo.get_memo_data(), ); From c9b5ae329f529ece8739161bda8c78a3baddd1f1 Mon Sep 17 00:00:00 2001 From: William J Date: Mon, 27 Mar 2023 13:02:42 -0400 Subject: [PATCH 07/20] lint --- transaction/builder/src/memo_builder/rth_memo_builder.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/transaction/builder/src/memo_builder/rth_memo_builder.rs b/transaction/builder/src/memo_builder/rth_memo_builder.rs index 1e11861f80..413b4b9f10 100644 --- a/transaction/builder/src/memo_builder/rth_memo_builder.rs +++ b/transaction/builder/src/memo_builder/rth_memo_builder.rs @@ -720,7 +720,7 @@ mod tests { &memo_test_context.sender.default_subaddress(), memo_test_context.receiver.view_private_key(), &CompressedRistrettoPublic::from(memo_test_context.funding_public_key), - output_memo.get_memo_type(), + *output_memo.get_memo_type(), output_memo.get_memo_data(), ); From 6ed5b93567c556bbb36b4781476d2f995eea63f7 Mon Sep 17 00:00:00 2001 From: William J Date: Tue, 11 Apr 2023 03:45:41 -0400 Subject: [PATCH 08/20] Remove todo, add fee check. --- .../src/memo_builder/rth_memo_builder.rs | 25 ++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/transaction/builder/src/memo_builder/rth_memo_builder.rs b/transaction/builder/src/memo_builder/rth_memo_builder.rs index 413b4b9f10..9ae61ae1c6 100644 --- a/transaction/builder/src/memo_builder/rth_memo_builder.rs +++ b/transaction/builder/src/memo_builder/rth_memo_builder.rs @@ -416,6 +416,10 @@ impl MemoBuilder for RTHMemoBuilder { return Err(NewMemoError::MixedTokenIds); } + if self.fee.value.to_be_bytes()[0] != 0u8 { + return Err(NewMemoError::LimitsExceeded("fee")); + } + self.total_outlay = self .total_outlay .checked_add(self.fee.value) @@ -445,7 +449,6 @@ impl MemoBuilder for RTHMemoBuilder { ); let payload: MemoPayload = MemoPayload::new(flexible_memo_payload.memo_type_bytes, memo_data); - //TODO: Check whether fee is too large before setting wrote destination memo self.wrote_destination_memo = true; Ok(payload) } @@ -679,6 +682,26 @@ mod tests { assert_eq!(fee.value, derived_fee); } + #[test] + fn test_funding_memo_rejects_fees_which_are_too_large() { + // Create Memo Builder with data + let fee = Amount::new(u64::MAX, 0.into()); + let change_amount = Amount::new(1, 0.into()); + let funding_amount = Amount::new(10, 0.into()); + + let mut rng: StdRng = SeedableRng::from_seed([0u8; 32]); + let (sender, builder) = build_test_memo_builder(&mut rng); + // Build the memo payload + let memo_test_context = + build_rth_memos(sender, builder, funding_amount, change_amount, fee); + assert_eq!( + memo_test_context + .change_memo + .expect_err("Should have an invalid destination memo type"), + NewMemoError::LimitsExceeded("fee") + ); + } + #[test] fn test_flexible_funding_output_memo_built_successfully() { // Create Memo Builder with data From e9905e0a96c97e8a31f6ad2985c4727f0609f715 Mon Sep 17 00:00:00 2001 From: William J Date: Tue, 11 Apr 2023 16:24:05 -0400 Subject: [PATCH 09/20] fmt --- transaction/builder/src/memo_builder/rth_memo_builder.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/transaction/builder/src/memo_builder/rth_memo_builder.rs b/transaction/builder/src/memo_builder/rth_memo_builder.rs index 9ae61ae1c6..3b725f2dd8 100644 --- a/transaction/builder/src/memo_builder/rth_memo_builder.rs +++ b/transaction/builder/src/memo_builder/rth_memo_builder.rs @@ -91,7 +91,7 @@ pub type FlexibleChangeMemoGenerator = Box< dyn Fn(FlexibleMemoChangeContext) -> Result + Sync + Send, >; -///This is the context provided to the flexible memo generator which is set on +/// This is the context provided to the flexible memo generator which is set on /// the rth memo builder. Each of these fields has a direct corresponds to a /// field in the RTHMemoBuilder pub struct FlexibleMemoBuilderContext { @@ -109,7 +109,7 @@ pub struct FlexibleMemoBuilderContext { pub fee: Amount, } -///This is the context provided to the flexible memo generator which is set +/// This is the context provided to the flexible memo generator which is set /// when generating the output memo pub struct FlexibleMemoOutputContext<'a> { /// Output amount @@ -122,7 +122,7 @@ pub struct FlexibleMemoOutputContext<'a> { pub builder_context: FlexibleMemoBuilderContext, } -///This is the context provided to the flexible memo generator which is set +/// This is the context provided to the flexible memo generator which is set /// when generating specifically the change memo pub struct FlexibleMemoChangeContext<'a> { /// Additional context provided for generating the output memo. From f453a245886883f4fd13d3ca1741a54f0f03a9b5 Mon Sep 17 00:00:00 2001 From: William J Date: Mon, 17 Apr 2023 23:26:02 -0400 Subject: [PATCH 10/20] Using trait instead of closures per pr feedback --- transaction/builder/src/lib.rs | 8 +- transaction/builder/src/memo_builder/mod.rs | 5 +- .../src/memo_builder/rth_memo_builder.rs | 131 ++++++++++++------ .../builder/src/transaction_builder.rs | 67 +++++---- 4 files changed, 135 insertions(+), 76 deletions(-) diff --git a/transaction/builder/src/lib.rs b/transaction/builder/src/lib.rs index 47824da2d3..e1b4b0aceb 100644 --- a/transaction/builder/src/lib.rs +++ b/transaction/builder/src/lib.rs @@ -23,10 +23,10 @@ pub use error::{SignedContingentInputBuilderError, TxBuilderError}; pub use input_credentials::InputCredentials; pub use memo_builder::{ BurnRedemptionMemoBuilder, DefragmentationMemoBuilder, EmptyMemoBuilder, - FlexibleChangeMemoGenerator, FlexibleMemoBuilderContext, FlexibleMemoChangeContext, - FlexibleMemoGenerator, FlexibleMemoOutputContext, FlexibleMemoPayload, - FlexibleOutputMemoGenerator, GiftCodeCancellationMemoBuilder, GiftCodeFundingMemoBuilder, - GiftCodeSenderMemoBuilder, MemoBuilder, RTHMemoBuilder, + FlexibleMemoBuilderContext, FlexibleMemoChangeContext, FlexibleMemoGenerator, + FlexibleMemoGeneratorTrait, FlexibleMemoOutputContext, FlexibleMemoPayload, + GiftCodeCancellationMemoBuilder, GiftCodeFundingMemoBuilder, GiftCodeSenderMemoBuilder, + MemoBuilder, RTHMemoBuilder, }; pub use reserved_subaddresses::ReservedSubaddresses; pub use signed_contingent_input_builder::SignedContingentInputBuilder; diff --git a/transaction/builder/src/memo_builder/mod.rs b/transaction/builder/src/memo_builder/mod.rs index f4791ce7ca..ee0769da1b 100644 --- a/transaction/builder/src/memo_builder/mod.rs +++ b/transaction/builder/src/memo_builder/mod.rs @@ -23,9 +23,8 @@ 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::{ - FlexibleChangeMemoGenerator, FlexibleMemoBuilderContext, FlexibleMemoChangeContext, - FlexibleMemoGenerator, FlexibleMemoOutputContext, FlexibleMemoPayload, - FlexibleOutputMemoGenerator, RTHMemoBuilder, + FlexibleMemoBuilderContext, FlexibleMemoChangeContext, FlexibleMemoGenerator, + FlexibleMemoGeneratorTrait, FlexibleMemoOutputContext, FlexibleMemoPayload, RTHMemoBuilder, }; /// The MemoBuilder trait defines the API that the transaction builder uses diff --git a/transaction/builder/src/memo_builder/rth_memo_builder.rs b/transaction/builder/src/memo_builder/rth_memo_builder.rs index 3b725f2dd8..eceee85dab 100644 --- a/transaction/builder/src/memo_builder/rth_memo_builder.rs +++ b/transaction/builder/src/memo_builder/rth_memo_builder.rs @@ -82,17 +82,41 @@ pub enum CustomMemoType { FlexibleMemoGenerator(FlexibleMemoGenerator), } -/// A function that returns a flexible memopayload for outputs -pub type FlexibleOutputMemoGenerator = Box< - dyn Fn(FlexibleMemoOutputContext) -> Result + Sync + Send, ->; -/// A function that returns a flexible memopayload for change -pub type FlexibleChangeMemoGenerator = Box< - dyn Fn(FlexibleMemoChangeContext) -> Result + Sync + Send, ->; +/// This is a trait that must be implemented by generators used for creating +/// flexible output memos and flexible change memos +pub trait FlexibleMemoGeneratorTrait: Sync + Send + Debug { + /// This is called when generating output memos. + /// It must return a memo payload with the first byte being 0x01 + fn create_output_memo( + &self, + context: FlexibleMemoOutputContext, + ) -> Result; + /// This is called when generating change memos. + /// It must return a memo payload with the first byte being 0x02 + fn create_change_memo( + &self, + context: FlexibleMemoChangeContext, + ) -> Result; +} + +/// This is a struct that contains a generator used for creating flexible output +/// memos and flexible change memos +#[derive(Clone)] +pub struct FlexibleMemoGenerator { + /// This is a reference to the actual generator that will be called. + pub generator: Arc>, +} + +impl Debug for FlexibleMemoGenerator { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + f.debug_struct("FlexibleMemoGenerator") + .field("generator", &self.generator) + .finish() + } +} /// This is the context provided to the flexible memo generator which is set on -/// the rth memo builder. Each of these fields has a direct corresponds to a +/// the rth memo builder. Each of these fields has a direct correspondence to a /// field in the RTHMemoBuilder pub struct FlexibleMemoBuilderContext { /// The credential used to form 0x0100 and 0x0101 memos, if present. @@ -147,24 +171,6 @@ pub struct FlexibleMemoPayload { pub memo_data: [u8; 32], } -/// This is a pair of generators used for creating flexible output memos and -/// flexible change memos -#[derive(Clone)] -pub struct FlexibleMemoGenerator { - /// Flexible_output_memo_generator: Is called when generating output memos. - /// It must return a memo payload with the first byte being 0x01 - pub flexible_output_memo_generator: Arc, - /// Flexible_change_memo_generator: Is called when genearting change memos. - /// It must return a memo payload with the first byte being 0x02 - pub flexible_change_memo_generator: Arc, -} - -impl Debug for FlexibleMemoGenerator { - fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { - write!(f, "FlexibleMemoGenerator") - } -} - impl Default for RTHMemoBuilder { fn default() -> Self { Self { @@ -337,10 +343,9 @@ impl MemoBuilder for RTHMemoBuilder { recipient, builder_context: self.get_flexible_memo_builder_context(), }; - let flexible_memo_payload = (flexible_memo_generator - .flexible_output_memo_generator)( - flexible_memo_context - )?; + let flexible_memo_payload = flexible_memo_generator + .generator + .create_output_memo(flexible_memo_context)?; let memo_data = compute_authenticated_sender_memo( flexible_memo_payload.memo_type_bytes, cred, @@ -433,10 +438,9 @@ impl MemoBuilder for RTHMemoBuilder { change_destination: _change_destination, builder_context: self.get_flexible_memo_builder_context(), }; - let flexible_memo_payload = (flexible_memo_generator - .flexible_change_memo_generator)( - flexible_memo_context - )?; + let flexible_memo_payload = flexible_memo_generator + .generator + .create_change_memo(flexible_memo_context)?; if flexible_memo_payload.memo_type_bytes[0] != 0x02 { return Err(NewMemoError::FlexibleMemoGenerator(format!("The flexible change memo generator created a memo of the incorrect memo type: {:?}", flexible_memo_payload.memo_type_bytes))); } @@ -532,6 +536,46 @@ mod tests { output_memo: Result, change_memo: Result, } + /// A function that returns a flexible memopayload for outputs + pub type FlexibleOutputMemoGenerator = Box< + dyn Fn(FlexibleMemoOutputContext) -> Result + + Sync + + Send, + >; + /// A function that returns a flexible memopayload for change + pub type FlexibleChangeMemoGenerator = Box< + dyn Fn(FlexibleMemoChangeContext) -> Result + + Sync + + Send, + >; + + // Test memo generator that allows the use of closures + struct FlexibleMemoGeneratorClosure { + output_memo_generator: FlexibleOutputMemoGenerator, + change_memo_generator: FlexibleChangeMemoGenerator, + } + + impl Debug for FlexibleMemoGeneratorClosure { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + f.debug_struct("FlexibleMemoGeneratorClosure").finish() + } + } + + impl FlexibleMemoGeneratorTrait for FlexibleMemoGeneratorClosure { + fn create_output_memo( + &self, + context: FlexibleMemoOutputContext, + ) -> Result { + (self.output_memo_generator)(context) + } + + fn create_change_memo( + &self, + context: FlexibleMemoChangeContext, + ) -> Result { + (self.change_memo_generator)(context) + } + } fn get_valid_flexible_memo_generator() -> FlexibleMemoGenerator { let flexible_output_memo_generator_closure: FlexibleOutputMemoGenerator = @@ -553,12 +597,15 @@ mod tests { }) }); + let generator_closure = FlexibleMemoGeneratorClosure { + output_memo_generator: flexible_output_memo_generator_closure, + change_memo_generator: flexible_change_memo_generator_closure, + }; + FlexibleMemoGenerator { - flexible_output_memo_generator: Arc::new(flexible_output_memo_generator_closure), - flexible_change_memo_generator: Arc::new(flexible_change_memo_generator_closure), + generator: Arc::new(Box::new(generator_closure)), } } - fn get_flexible_memo_generator_returning_invalid_types() -> FlexibleMemoGenerator { let flexible_output_memo_generator_closure: FlexibleOutputMemoGenerator = Box::new(|_context: FlexibleMemoOutputContext| { @@ -579,9 +626,13 @@ mod tests { }) }); + let generator_closure = FlexibleMemoGeneratorClosure { + output_memo_generator: flexible_output_memo_generator_closure, + change_memo_generator: flexible_change_memo_generator_closure, + }; + FlexibleMemoGenerator { - flexible_output_memo_generator: Arc::new(flexible_output_memo_generator_closure), - flexible_change_memo_generator: Arc::new(flexible_change_memo_generator_closure), + generator: Arc::new(Box::new(generator_closure)), } } diff --git a/transaction/builder/src/transaction_builder.rs b/transaction/builder/src/transaction_builder.rs index a16473072a..1d6f756880 100644 --- a/transaction/builder/src/transaction_builder.rs +++ b/transaction/builder/src/transaction_builder.rs @@ -922,10 +922,9 @@ pub mod transaction_builder_tests { use crate::{ test_utils::{create_output, get_input_credentials, get_ring, get_transaction}, BurnRedemptionMemoBuilder, DefragmentationMemoBuilder, EmptyMemoBuilder, - FlexibleChangeMemoGenerator, FlexibleMemoChangeContext, FlexibleMemoGenerator, - FlexibleMemoOutputContext, FlexibleMemoPayload, FlexibleOutputMemoGenerator, - GiftCodeCancellationMemoBuilder, GiftCodeFundingMemoBuilder, GiftCodeSenderMemoBuilder, - RTHMemoBuilder, + FlexibleMemoChangeContext, FlexibleMemoGenerator, FlexibleMemoGeneratorTrait, + FlexibleMemoOutputContext, FlexibleMemoPayload, GiftCodeCancellationMemoBuilder, + GiftCodeFundingMemoBuilder, GiftCodeSenderMemoBuilder, RTHMemoBuilder, }; use alloc::{string::ToString, sync::Arc, vec}; use assert_matches::assert_matches; @@ -963,34 +962,44 @@ pub mod transaction_builder_tests { (BlockVersion::try_from(2).unwrap(), TokenId::from(2)), ] } + // Test memo generator that creates a payload which corresponds to a payment + // request. + #[derive(Debug)] + struct FlexibleMemoGeneratorPaymentRequest {} + + impl FlexibleMemoGeneratorTrait for FlexibleMemoGeneratorPaymentRequest { + fn create_output_memo( + &self, + _context: FlexibleMemoOutputContext, + ) -> Result { + let payment_request_id = 42u64; + let memo_type_bytes = AuthenticatedSenderWithPaymentRequestIdMemo::MEMO_TYPE_BYTES; + let mut memo_data = [0x00; 32]; + memo_data[0..8].copy_from_slice(&payment_request_id.to_be_bytes()); + Ok(FlexibleMemoPayload { + memo_type_bytes, + memo_data, + }) + } - fn get_valid_flexible_memo_generator() -> FlexibleMemoGenerator { - let flexible_output_memo_generator_closure: FlexibleOutputMemoGenerator = - Box::new(|_context: FlexibleMemoOutputContext| { - let payment_request_id = 42u64; - let memo_type_bytes = AuthenticatedSenderWithPaymentRequestIdMemo::MEMO_TYPE_BYTES; - let mut memo_data = [0x00; 32]; - memo_data[0..8].copy_from_slice(&payment_request_id.to_be_bytes()); - Ok(FlexibleMemoPayload { - memo_type_bytes, - memo_data, - }) - }); - let flexible_change_memo_generator_closure: FlexibleChangeMemoGenerator = - Box::new(|_context: FlexibleMemoChangeContext| { - let payment_request_id = 42u64; - let memo_type_bytes = DestinationWithPaymentRequestIdMemo::MEMO_TYPE_BYTES; - let mut memo_data = [0u8; 32]; - memo_data[0..8].copy_from_slice(&payment_request_id.to_be_bytes()); - Ok(FlexibleMemoPayload { - memo_type_bytes, - memo_data, - }) - }); + fn create_change_memo( + &self, + _context: FlexibleMemoChangeContext, + ) -> Result { + let payment_request_id = 42u64; + let memo_type_bytes = DestinationWithPaymentRequestIdMemo::MEMO_TYPE_BYTES; + let mut memo_data = [0u8; 32]; + memo_data[0..8].copy_from_slice(&payment_request_id.to_be_bytes()); + Ok(FlexibleMemoPayload { + memo_type_bytes, + memo_data, + }) + } + } + fn get_valid_flexible_memo_generator() -> FlexibleMemoGenerator { FlexibleMemoGenerator { - flexible_output_memo_generator: Arc::new(flexible_output_memo_generator_closure), - flexible_change_memo_generator: Arc::new(flexible_change_memo_generator_closure), + generator: Arc::new(Box::new(FlexibleMemoGeneratorPaymentRequest {})), } } From 8c2ffd6fba52d9655862800df82d536c13a0f045 Mon Sep 17 00:00:00 2001 From: William J Date: Tue, 18 Apr 2023 15:29:47 -0400 Subject: [PATCH 11/20] Remove unnecessary struct --- transaction/builder/src/lib.rs | 2 +- transaction/builder/src/memo_builder/mod.rs | 2 +- .../src/memo_builder/rth_memo_builder.rs | 45 ++++++------------- .../builder/src/transaction_builder.rs | 10 ++--- 4 files changed, 19 insertions(+), 40 deletions(-) diff --git a/transaction/builder/src/lib.rs b/transaction/builder/src/lib.rs index e1b4b0aceb..c34ec5b62a 100644 --- a/transaction/builder/src/lib.rs +++ b/transaction/builder/src/lib.rs @@ -24,7 +24,7 @@ pub use input_credentials::InputCredentials; pub use memo_builder::{ BurnRedemptionMemoBuilder, DefragmentationMemoBuilder, EmptyMemoBuilder, FlexibleMemoBuilderContext, FlexibleMemoChangeContext, FlexibleMemoGenerator, - FlexibleMemoGeneratorTrait, FlexibleMemoOutputContext, FlexibleMemoPayload, + FlexibleMemoGeneratorReference, FlexibleMemoOutputContext, FlexibleMemoPayload, GiftCodeCancellationMemoBuilder, GiftCodeFundingMemoBuilder, GiftCodeSenderMemoBuilder, MemoBuilder, RTHMemoBuilder, }; diff --git a/transaction/builder/src/memo_builder/mod.rs b/transaction/builder/src/memo_builder/mod.rs index ee0769da1b..f4f0a53aad 100644 --- a/transaction/builder/src/memo_builder/mod.rs +++ b/transaction/builder/src/memo_builder/mod.rs @@ -24,7 +24,7 @@ pub use gift_code_funding_memo_builder::GiftCodeFundingMemoBuilder; pub use gift_code_sender_memo_builder::GiftCodeSenderMemoBuilder; pub use rth_memo_builder::{ FlexibleMemoBuilderContext, FlexibleMemoChangeContext, FlexibleMemoGenerator, - FlexibleMemoGeneratorTrait, FlexibleMemoOutputContext, FlexibleMemoPayload, RTHMemoBuilder, + FlexibleMemoGeneratorReference, FlexibleMemoOutputContext, FlexibleMemoPayload, RTHMemoBuilder, }; /// The MemoBuilder trait defines the API that the transaction builder uses diff --git a/transaction/builder/src/memo_builder/rth_memo_builder.rs b/transaction/builder/src/memo_builder/rth_memo_builder.rs index eceee85dab..b0dd86bee6 100644 --- a/transaction/builder/src/memo_builder/rth_memo_builder.rs +++ b/transaction/builder/src/memo_builder/rth_memo_builder.rs @@ -79,12 +79,12 @@ pub struct RTHMemoBuilder { pub enum CustomMemoType { PaymentRequestId(u64), PaymentIntentId(u64), - FlexibleMemoGenerator(FlexibleMemoGenerator), + FlexibleMemoGenerator(FlexibleMemoGeneratorReference), } /// This is a trait that must be implemented by generators used for creating /// flexible output memos and flexible change memos -pub trait FlexibleMemoGeneratorTrait: Sync + Send + Debug { +pub trait FlexibleMemoGenerator: Sync + Send + Debug { /// This is called when generating output memos. /// It must return a memo payload with the first byte being 0x01 fn create_output_memo( @@ -99,21 +99,8 @@ pub trait FlexibleMemoGeneratorTrait: Sync + Send + Debug { ) -> Result; } -/// This is a struct that contains a generator used for creating flexible output -/// memos and flexible change memos -#[derive(Clone)] -pub struct FlexibleMemoGenerator { - /// This is a reference to the actual generator that will be called. - pub generator: Arc>, -} - -impl Debug for FlexibleMemoGenerator { - fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { - f.debug_struct("FlexibleMemoGenerator") - .field("generator", &self.generator) - .finish() - } -} +/// This is a thread safe reference to a FlexibleMemoGenerator +pub type FlexibleMemoGeneratorReference = Arc>; /// This is the context provided to the flexible memo generator which is set on /// the rth memo builder. Each of these fields has a direct correspondence to a @@ -257,7 +244,7 @@ impl RTHMemoBuilder { /// Set the flexible memo generator. pub fn set_flexible_memo_generator( &mut self, - generator: FlexibleMemoGenerator, + generator: FlexibleMemoGeneratorReference, ) -> Result<(), MemoBuilderError> { if self.custom_memo_type.is_some() { return Err(MemoBuilderError::StateChange(format!( @@ -343,9 +330,8 @@ impl MemoBuilder for RTHMemoBuilder { recipient, builder_context: self.get_flexible_memo_builder_context(), }; - let flexible_memo_payload = flexible_memo_generator - .generator - .create_output_memo(flexible_memo_context)?; + let flexible_memo_payload = + flexible_memo_generator.create_output_memo(flexible_memo_context)?; let memo_data = compute_authenticated_sender_memo( flexible_memo_payload.memo_type_bytes, cred, @@ -438,9 +424,8 @@ impl MemoBuilder for RTHMemoBuilder { change_destination: _change_destination, builder_context: self.get_flexible_memo_builder_context(), }; - let flexible_memo_payload = flexible_memo_generator - .generator - .create_change_memo(flexible_memo_context)?; + let flexible_memo_payload = + flexible_memo_generator.create_change_memo(flexible_memo_context)?; if flexible_memo_payload.memo_type_bytes[0] != 0x02 { return Err(NewMemoError::FlexibleMemoGenerator(format!("The flexible change memo generator created a memo of the incorrect memo type: {:?}", flexible_memo_payload.memo_type_bytes))); } @@ -561,7 +546,7 @@ mod tests { } } - impl FlexibleMemoGeneratorTrait for FlexibleMemoGeneratorClosure { + impl FlexibleMemoGenerator for FlexibleMemoGeneratorClosure { fn create_output_memo( &self, context: FlexibleMemoOutputContext, @@ -577,7 +562,7 @@ mod tests { } } - fn get_valid_flexible_memo_generator() -> FlexibleMemoGenerator { + fn get_valid_flexible_memo_generator() -> FlexibleMemoGeneratorReference { let flexible_output_memo_generator_closure: FlexibleOutputMemoGenerator = Box::new(|_context: FlexibleMemoOutputContext| { let memo_type_bytes = AUTHENTICATED_CUSTOM_MEMO_TYPE_BYTES; @@ -602,9 +587,7 @@ mod tests { change_memo_generator: flexible_change_memo_generator_closure, }; - FlexibleMemoGenerator { - generator: Arc::new(Box::new(generator_closure)), - } + Arc::new(Box::new(generator_closure)) } fn get_flexible_memo_generator_returning_invalid_types() -> FlexibleMemoGenerator { let flexible_output_memo_generator_closure: FlexibleOutputMemoGenerator = @@ -631,9 +614,7 @@ mod tests { change_memo_generator: flexible_change_memo_generator_closure, }; - FlexibleMemoGenerator { - generator: Arc::new(Box::new(generator_closure)), - } + Arc::new(Box::new(generator_closure)) } fn build_rth_memos( diff --git a/transaction/builder/src/transaction_builder.rs b/transaction/builder/src/transaction_builder.rs index 1d6f756880..71be35b178 100644 --- a/transaction/builder/src/transaction_builder.rs +++ b/transaction/builder/src/transaction_builder.rs @@ -922,7 +922,7 @@ pub mod transaction_builder_tests { use crate::{ test_utils::{create_output, get_input_credentials, get_ring, get_transaction}, BurnRedemptionMemoBuilder, DefragmentationMemoBuilder, EmptyMemoBuilder, - FlexibleMemoChangeContext, FlexibleMemoGenerator, FlexibleMemoGeneratorTrait, + FlexibleMemoChangeContext, FlexibleMemoGenerator, FlexibleMemoGeneratorReference, FlexibleMemoOutputContext, FlexibleMemoPayload, GiftCodeCancellationMemoBuilder, GiftCodeFundingMemoBuilder, GiftCodeSenderMemoBuilder, RTHMemoBuilder, }; @@ -967,7 +967,7 @@ pub mod transaction_builder_tests { #[derive(Debug)] struct FlexibleMemoGeneratorPaymentRequest {} - impl FlexibleMemoGeneratorTrait for FlexibleMemoGeneratorPaymentRequest { + impl FlexibleMemoGenerator for FlexibleMemoGeneratorPaymentRequest { fn create_output_memo( &self, _context: FlexibleMemoOutputContext, @@ -997,10 +997,8 @@ pub mod transaction_builder_tests { } } - fn get_valid_flexible_memo_generator() -> FlexibleMemoGenerator { - FlexibleMemoGenerator { - generator: Arc::new(Box::new(FlexibleMemoGeneratorPaymentRequest {})), - } + fn get_valid_flexible_memo_generator() -> FlexibleMemoGeneratorReference { + Arc::new(Box::new(FlexibleMemoGeneratorPaymentRequest {})) } #[test] From 47fd4d17948f1a66f0aacd47b22d8eee84c6fc11 Mon Sep 17 00:00:00 2001 From: William J Date: Wed, 19 Apr 2023 01:26:33 -0400 Subject: [PATCH 12/20] Remove function based flexible memo generation. Just set the memo payloads directly instead --- transaction/builder/src/lib.rs | 8 +- transaction/builder/src/memo_builder/mod.rs | 5 +- .../src/memo_builder/rth_memo_builder.rs | 281 +++++------------- .../builder/src/transaction_builder.rs | 63 ++-- transaction/core/src/tx_error.rs | 4 +- 5 files changed, 109 insertions(+), 252 deletions(-) diff --git a/transaction/builder/src/lib.rs b/transaction/builder/src/lib.rs index c34ec5b62a..75cfc66cba 100644 --- a/transaction/builder/src/lib.rs +++ b/transaction/builder/src/lib.rs @@ -22,11 +22,9 @@ pub mod test_utils; pub use error::{SignedContingentInputBuilderError, TxBuilderError}; pub use input_credentials::InputCredentials; pub use memo_builder::{ - BurnRedemptionMemoBuilder, DefragmentationMemoBuilder, EmptyMemoBuilder, - FlexibleMemoBuilderContext, FlexibleMemoChangeContext, FlexibleMemoGenerator, - FlexibleMemoGeneratorReference, FlexibleMemoOutputContext, FlexibleMemoPayload, - GiftCodeCancellationMemoBuilder, GiftCodeFundingMemoBuilder, GiftCodeSenderMemoBuilder, - MemoBuilder, RTHMemoBuilder, + BurnRedemptionMemoBuilder, DefragmentationMemoBuilder, EmptyMemoBuilder, FlexibleMemoPayload, + FlexibleMemoPayloads, GiftCodeCancellationMemoBuilder, GiftCodeFundingMemoBuilder, + GiftCodeSenderMemoBuilder, MemoBuilder, RTHMemoBuilder, }; pub use reserved_subaddresses::ReservedSubaddresses; pub use signed_contingent_input_builder::SignedContingentInputBuilder; diff --git a/transaction/builder/src/memo_builder/mod.rs b/transaction/builder/src/memo_builder/mod.rs index f4f0a53aad..17967f5368 100644 --- a/transaction/builder/src/memo_builder/mod.rs +++ b/transaction/builder/src/memo_builder/mod.rs @@ -22,10 +22,7 @@ pub use defragmentation_memo_builder::DefragmentationMemoBuilder; 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::{ - FlexibleMemoBuilderContext, FlexibleMemoChangeContext, FlexibleMemoGenerator, - FlexibleMemoGeneratorReference, FlexibleMemoOutputContext, FlexibleMemoPayload, RTHMemoBuilder, -}; +pub use rth_memo_builder::{FlexibleMemoPayload, FlexibleMemoPayloads, RTHMemoBuilder}; /// The MemoBuilder trait defines the API that the transaction builder uses /// to ask the memo builder to build a memo for a particular TxOut. diff --git a/transaction/builder/src/memo_builder/rth_memo_builder.rs b/transaction/builder/src/memo_builder/rth_memo_builder.rs index b0dd86bee6..3b5321308e 100644 --- a/transaction/builder/src/memo_builder/rth_memo_builder.rs +++ b/transaction/builder/src/memo_builder/rth_memo_builder.rs @@ -6,7 +6,7 @@ //! the encrypted memos, as envisioned in MCIP #4. use super::MemoBuilder; use crate::ReservedSubaddresses; -use alloc::{boxed::Box, fmt::Debug, format, string::String, sync::Arc}; +use alloc::{fmt::Debug, format, string::String}; use displaydoc::Display; use mc_account_keys::{PublicAddress, ShortAddressHash}; use mc_transaction_core::{ @@ -79,77 +79,27 @@ pub struct RTHMemoBuilder { pub enum CustomMemoType { PaymentRequestId(u64), PaymentIntentId(u64), - FlexibleMemoGenerator(FlexibleMemoGeneratorReference), + FlexibleMemos(FlexibleMemoPayloads), } -/// This is a trait that must be implemented by generators used for creating -/// flexible output memos and flexible change memos -pub trait FlexibleMemoGenerator: Sync + Send + Debug { - /// This is called when generating output memos. - /// It must return a memo payload with the first byte being 0x01 - fn create_output_memo( - &self, - context: FlexibleMemoOutputContext, - ) -> Result; - /// This is called when generating change memos. - /// It must return a memo payload with the first byte being 0x02 - fn create_change_memo( - &self, - context: FlexibleMemoChangeContext, - ) -> Result; -} - -/// This is a thread safe reference to a FlexibleMemoGenerator -pub type FlexibleMemoGeneratorReference = Arc>; - -/// This is the context provided to the flexible memo generator which is set on -/// the rth memo builder. Each of these fields has a direct correspondence to a -/// field in the RTHMemoBuilder -pub struct FlexibleMemoBuilderContext { - /// The credential used to form 0x0100 and 0x0101 memos, if present. - pub sender_cred: Option, - /// Tracks the last recipient - pub last_recipient: ShortAddressHash, - /// Tracks the total outlay so far - pub total_outlay: u64, - /// Tracks the total outlay token id - pub outlay_token_id: Option, - /// Tracks the number of recipients so far - pub num_recipients: u8, - /// Tracks the fee - pub fee: Amount, -} - -/// This is the context provided to the flexible memo generator which is set -/// when generating the output memo -pub struct FlexibleMemoOutputContext<'a> { - /// Output amount - pub amount: Amount, - /// Memo recipient - pub recipient: &'a PublicAddress, - /// Additional context provided for generating the output memo. - pub memo_context: MemoContext<'a>, - /// Context available from the RthMemoBuilder - pub builder_context: FlexibleMemoBuilderContext, -} - -/// This is the context provided to the flexible memo generator which is set -/// when generating specifically the change memo -pub struct FlexibleMemoChangeContext<'a> { - /// Additional context provided for generating the output memo. - pub memo_context: MemoContext<'a>, - /// Change amount - pub amount: Amount, - /// The subaddress that the change is being sent to. - pub change_destination: &'a ReservedSubaddresses, - /// Context available from the RthMemoBuilder - pub builder_context: FlexibleMemoBuilderContext, +/// This contains both the output memo payload and the change memo payload which +/// will be used when generating custom output memos and change memos +#[derive(Clone, Debug)] +pub struct FlexibleMemoPayloads { + /// This is used when generating output memos. + /// It must contain a memo type with the first byte being 0x01 + pub output_memo_payload: FlexibleMemoPayload, + /// This is used when generating change memos. + /// It must contain a memo type with the first byte being 0x02 + pub change_memo_payload: FlexibleMemoPayload, } -/// This is the result of the flexible memo generators. +/// This is the payload data used for creating custom MemoPayloads with types +/// which are defined in MCIPs. +#[derive(Clone, Debug)] pub struct FlexibleMemoPayload { /// memo_type_bytes corresponds to the returned memo type. This should be - /// listed in an mcip. + /// listed in an MCIP. pub memo_type_bytes: [u8; 2], /// memo_data: corresponds to some 32 byte encoding of data used for /// the returned memo type, and does not include fields used for the @@ -241,10 +191,10 @@ impl RTHMemoBuilder { Ok(()) } - /// Set the flexible memo generator. - pub fn set_flexible_memo_generator( + /// Set the flexible memos. + pub fn set_flexible_memos( &mut self, - generator: FlexibleMemoGeneratorReference, + memos: FlexibleMemoPayloads, ) -> Result<(), MemoBuilderError> { if self.custom_memo_type.is_some() { return Err(MemoBuilderError::StateChange(format!( @@ -252,7 +202,7 @@ impl RTHMemoBuilder { self.custom_memo_type ))); } - self.custom_memo_type = Some(CustomMemoType::FlexibleMemoGenerator(generator)); + self.custom_memo_type = Some(CustomMemoType::FlexibleMemos(memos)); Ok(()) } @@ -265,19 +215,6 @@ impl RTHMemoBuilder { pub fn disable_destination_memo(&mut self) { self.destination_memo_enabled = false; } - - /// Generates a flexible memo builder context from fields set on the rth - /// memo builder to be used with the flexible memo generator function - pub fn get_flexible_memo_builder_context(&self) -> FlexibleMemoBuilderContext { - FlexibleMemoBuilderContext { - sender_cred: self.sender_cred.clone(), - last_recipient: self.last_recipient.clone(), - total_outlay: self.total_outlay, - outlay_token_id: self.outlay_token_id, - num_recipients: self.num_recipients, - fee: self.fee, - } - } } impl MemoBuilder for RTHMemoBuilder { @@ -322,16 +259,9 @@ impl MemoBuilder for RTHMemoBuilder { let payload: MemoPayload = if let Some(cred) = &self.sender_cred { if let Some(custom_memo_type) = &self.custom_memo_type { match custom_memo_type { - CustomMemoType::FlexibleMemoGenerator(flexible_memo_generator) => { + CustomMemoType::FlexibleMemos(flexible_memos) => { let tx_public_key = memo_context.tx_public_key; - let flexible_memo_context = FlexibleMemoOutputContext { - memo_context, - amount, - recipient, - builder_context: self.get_flexible_memo_builder_context(), - }; - let flexible_memo_payload = - flexible_memo_generator.create_output_memo(flexible_memo_context)?; + let flexible_memo_payload = &flexible_memos.output_memo_payload; let memo_data = compute_authenticated_sender_memo( flexible_memo_payload.memo_type_bytes, cred, @@ -340,7 +270,7 @@ impl MemoBuilder for RTHMemoBuilder { &flexible_memo_payload.memo_data, ); if flexible_memo_payload.memo_type_bytes[0] != 0x01 { - return Err(NewMemoError::FlexibleMemoGenerator(format!("The flexible output memo generator created a memo of the incorrect memo type: {:?}", flexible_memo_payload.memo_type_bytes))); + return Err(NewMemoError::FlexibleMemo(format!("The flexible output memo has a memopayload of the incorrect memo type: {:?}", flexible_memo_payload.memo_type_bytes))); } MemoPayload::new(flexible_memo_payload.memo_type_bytes, memo_data) } @@ -417,17 +347,10 @@ impl MemoBuilder for RTHMemoBuilder { .ok_or(NewMemoError::LimitsExceeded("total_outlay"))?; if let Some(custom_memo_type) = &self.custom_memo_type { match custom_memo_type { - CustomMemoType::FlexibleMemoGenerator(flexible_memo_generator) => { - let flexible_memo_context = FlexibleMemoChangeContext { - memo_context: _memo_context, - amount, - change_destination: _change_destination, - builder_context: self.get_flexible_memo_builder_context(), - }; - let flexible_memo_payload = - flexible_memo_generator.create_change_memo(flexible_memo_context)?; + CustomMemoType::FlexibleMemos(flexible_memos) => { + let flexible_memo_payload = &flexible_memos.change_memo_payload; if flexible_memo_payload.memo_type_bytes[0] != 0x02 { - return Err(NewMemoError::FlexibleMemoGenerator(format!("The flexible change memo generator created a memo of the incorrect memo type: {:?}", flexible_memo_payload.memo_type_bytes))); + return Err(NewMemoError::FlexibleMemo(format!("The flexible change memo has a memopayload of the incorrect memo type: {:?}", flexible_memo_payload.memo_type_bytes))); } let memo_data = compute_destination_memo( self.last_recipient.clone(), @@ -521,100 +444,42 @@ mod tests { output_memo: Result, change_memo: Result, } - /// A function that returns a flexible memopayload for outputs - pub type FlexibleOutputMemoGenerator = Box< - dyn Fn(FlexibleMemoOutputContext) -> Result - + Sync - + Send, - >; - /// A function that returns a flexible memopayload for change - pub type FlexibleChangeMemoGenerator = Box< - dyn Fn(FlexibleMemoChangeContext) -> Result - + Sync - + Send, - >; - - // Test memo generator that allows the use of closures - struct FlexibleMemoGeneratorClosure { - output_memo_generator: FlexibleOutputMemoGenerator, - change_memo_generator: FlexibleChangeMemoGenerator, - } - - impl Debug for FlexibleMemoGeneratorClosure { - fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { - f.debug_struct("FlexibleMemoGeneratorClosure").finish() - } - } - - impl FlexibleMemoGenerator for FlexibleMemoGeneratorClosure { - fn create_output_memo( - &self, - context: FlexibleMemoOutputContext, - ) -> Result { - (self.output_memo_generator)(context) - } - fn create_change_memo( - &self, - context: FlexibleMemoChangeContext, - ) -> Result { - (self.change_memo_generator)(context) + fn get_valid_flexible_memos() -> FlexibleMemoPayloads { + let memo_type_bytes = AUTHENTICATED_CUSTOM_MEMO_TYPE_BYTES; + let memo_data = [0x01; 32]; + let output_memo_payload = FlexibleMemoPayload { + memo_type_bytes, + memo_data, + }; + let memo_type_bytes = DESTINATION_CUSTOM_MEMO_TYPE_BYTES; + let memo_data = [0x01; 32]; + let change_memo_payload = FlexibleMemoPayload { + memo_type_bytes, + memo_data, + }; + FlexibleMemoPayloads { + output_memo_payload, + change_memo_payload, } } - - fn get_valid_flexible_memo_generator() -> FlexibleMemoGeneratorReference { - let flexible_output_memo_generator_closure: FlexibleOutputMemoGenerator = - Box::new(|_context: FlexibleMemoOutputContext| { - let memo_type_bytes = AUTHENTICATED_CUSTOM_MEMO_TYPE_BYTES; - let memo_data = [0x01; 32]; - Ok(FlexibleMemoPayload { - memo_type_bytes, - memo_data, - }) - }); - let flexible_change_memo_generator_closure: FlexibleChangeMemoGenerator = - Box::new(|_context: FlexibleMemoChangeContext| { - let memo_type_bytes = DESTINATION_CUSTOM_MEMO_TYPE_BYTES; - let memo_data = [0x01; 32]; - Ok(FlexibleMemoPayload { - memo_type_bytes, - memo_data, - }) - }); - - let generator_closure = FlexibleMemoGeneratorClosure { - output_memo_generator: flexible_output_memo_generator_closure, - change_memo_generator: flexible_change_memo_generator_closure, + fn get_invalid_flexible_memos() -> FlexibleMemoPayloads { + let memo_type_bytes = DESTINATION_CUSTOM_MEMO_TYPE_BYTES; + let memo_data = [0x01; 32]; + let output_memo_payload = FlexibleMemoPayload { + memo_type_bytes, + memo_data, }; - - Arc::new(Box::new(generator_closure)) - } - fn get_flexible_memo_generator_returning_invalid_types() -> FlexibleMemoGenerator { - let flexible_output_memo_generator_closure: FlexibleOutputMemoGenerator = - Box::new(|_context: FlexibleMemoOutputContext| { - let memo_type_bytes = DESTINATION_CUSTOM_MEMO_TYPE_BYTES; - let memo_data = [0x01; 32]; - Ok(FlexibleMemoPayload { - memo_type_bytes, - memo_data, - }) - }); - let flexible_change_memo_generator_closure: FlexibleChangeMemoGenerator = - Box::new(|_context: FlexibleMemoChangeContext| { - let memo_type_bytes = AUTHENTICATED_CUSTOM_MEMO_TYPE_BYTES; - let memo_data = [0x01; 32]; - Ok(FlexibleMemoPayload { - memo_type_bytes, - memo_data, - }) - }); - - let generator_closure = FlexibleMemoGeneratorClosure { - output_memo_generator: flexible_output_memo_generator_closure, - change_memo_generator: flexible_change_memo_generator_closure, + let memo_type_bytes = AUTHENTICATED_CUSTOM_MEMO_TYPE_BYTES; + let memo_data = [0x01; 32]; + let change_memo_payload = FlexibleMemoPayload { + memo_type_bytes, + memo_data, }; - - Arc::new(Box::new(generator_closure)) + FlexibleMemoPayloads { + output_memo_payload, + change_memo_payload, + } } fn build_rth_memos( @@ -743,9 +608,9 @@ mod tests { let mut rng: StdRng = SeedableRng::from_seed([0u8; 32]); let (sender, mut builder) = build_test_memo_builder(&mut rng); - // Add a flexible memo generator + // Add a flexible memo builder - .set_flexible_memo_generator(get_valid_flexible_memo_generator()) + .set_flexible_memos(get_valid_flexible_memos()) .expect("No other custom memo type should be set"); // Build the memo payload let memo_test_context = @@ -801,19 +666,31 @@ mod tests { let mut rng: StdRng = SeedableRng::from_seed([0u8; 32]); let (sender, mut builder) = build_test_memo_builder(&mut rng); - // Add a flexible memo generator + // Add a flexible memo builder - .set_flexible_memo_generator(get_flexible_memo_generator_returning_invalid_types()) + .set_flexible_memos(get_invalid_flexible_memos()) .expect("No other custom memo types should be set"); // Build the memo payload let memo_test_context = build_rth_memos(sender, builder, funding_amount, change_amount, fee); - assert_eq!(memo_test_context - .output_memo - .expect_err("Should have an invalid output memo type"), NewMemoError::FlexibleMemoGenerator("The flexible output memo generator created a memo of the incorrect memo type: [2, 8]".to_owned())); - assert_eq!(memo_test_context - .change_memo - .expect_err("Should have an invalid destination memo type"), NewMemoError::FlexibleMemoGenerator("The flexible change memo generator created a memo of the incorrect memo type: [1, 8]".to_owned())); + assert_eq!( + memo_test_context + .output_memo + .expect_err("Should have an invalid output memo type"), + NewMemoError::FlexibleMemo( + "The flexible output memo has a memopayload of the incorrect memo type: [2, 8]" + .to_owned() + ) + ); + assert_eq!( + memo_test_context + .change_memo + .expect_err("Should have an invalid destination memo type"), + NewMemoError::FlexibleMemo( + "The flexible change memo has a memopayload of the incorrect memo type: [1, 8]" + .to_owned() + ) + ); } } diff --git a/transaction/builder/src/transaction_builder.rs b/transaction/builder/src/transaction_builder.rs index 71be35b178..a880c33f22 100644 --- a/transaction/builder/src/transaction_builder.rs +++ b/transaction/builder/src/transaction_builder.rs @@ -922,11 +922,10 @@ pub mod transaction_builder_tests { use crate::{ test_utils::{create_output, get_input_credentials, get_ring, get_transaction}, BurnRedemptionMemoBuilder, DefragmentationMemoBuilder, EmptyMemoBuilder, - FlexibleMemoChangeContext, FlexibleMemoGenerator, FlexibleMemoGeneratorReference, - FlexibleMemoOutputContext, FlexibleMemoPayload, GiftCodeCancellationMemoBuilder, + FlexibleMemoPayload, FlexibleMemoPayloads, GiftCodeCancellationMemoBuilder, GiftCodeFundingMemoBuilder, GiftCodeSenderMemoBuilder, RTHMemoBuilder, }; - use alloc::{string::ToString, sync::Arc, vec}; + use alloc::{string::ToString, vec}; use assert_matches::assert_matches; use maplit::btreemap; use mc_account_keys::{ @@ -962,45 +961,31 @@ pub mod transaction_builder_tests { (BlockVersion::try_from(2).unwrap(), TokenId::from(2)), ] } - // Test memo generator that creates a payload which corresponds to a payment + // Returns a flexible memo payload which corresponds to a payment // request. - #[derive(Debug)] - struct FlexibleMemoGeneratorPaymentRequest {} - - impl FlexibleMemoGenerator for FlexibleMemoGeneratorPaymentRequest { - fn create_output_memo( - &self, - _context: FlexibleMemoOutputContext, - ) -> Result { - let payment_request_id = 42u64; - let memo_type_bytes = AuthenticatedSenderWithPaymentRequestIdMemo::MEMO_TYPE_BYTES; - let mut memo_data = [0x00; 32]; - memo_data[0..8].copy_from_slice(&payment_request_id.to_be_bytes()); - Ok(FlexibleMemoPayload { - memo_type_bytes, - memo_data, - }) - } - - fn create_change_memo( - &self, - _context: FlexibleMemoChangeContext, - ) -> Result { - let payment_request_id = 42u64; - let memo_type_bytes = DestinationWithPaymentRequestIdMemo::MEMO_TYPE_BYTES; - let mut memo_data = [0u8; 32]; - memo_data[0..8].copy_from_slice(&payment_request_id.to_be_bytes()); - Ok(FlexibleMemoPayload { - memo_type_bytes, - memo_data, - }) + fn get_valid_flexible_memo() -> FlexibleMemoPayloads { + let payment_request_id = 42u64; + let memo_type_bytes = AuthenticatedSenderWithPaymentRequestIdMemo::MEMO_TYPE_BYTES; + let mut memo_data = [0x00; 32]; + memo_data[0..8].copy_from_slice(&payment_request_id.to_be_bytes()); + let output_memo_payload = FlexibleMemoPayload { + memo_type_bytes, + memo_data, + }; + let payment_request_id = 42u64; + let memo_type_bytes = DestinationWithPaymentRequestIdMemo::MEMO_TYPE_BYTES; + let mut memo_data = [0u8; 32]; + memo_data[0..8].copy_from_slice(&payment_request_id.to_be_bytes()); + let change_memo_payload = FlexibleMemoPayload { + memo_type_bytes, + memo_data, + }; + FlexibleMemoPayloads { + output_memo_payload, + change_memo_payload, } } - fn get_valid_flexible_memo_generator() -> FlexibleMemoGeneratorReference { - Arc::new(Box::new(FlexibleMemoGeneratorPaymentRequest {})) - } - #[test] // Spend a single input and send its full value to a single recipient. fn test_simple_transaction() { @@ -2526,7 +2511,7 @@ pub mod transaction_builder_tests { memo_builder.set_sender_credential(SenderMemoCredential::from(&sender)); memo_builder.enable_destination_memo(); memo_builder - .set_flexible_memo_generator(get_valid_flexible_memo_generator()) + .set_flexible_memos(get_valid_flexible_memo()) .expect("No other memo types should be set"); let mut transaction_builder = TransactionBuilder::new( diff --git a/transaction/core/src/tx_error.rs b/transaction/core/src/tx_error.rs index 540d46c09e..05296c4e34 100644 --- a/transaction/core/src/tx_error.rs +++ b/transaction/core/src/tx_error.rs @@ -100,8 +100,8 @@ pub enum NewMemoError { MaxFeeExceeded(u64, u64), /// Payment request and intent ID both are set RequestAndIntentIdSet, - /// Error in the Flexible memo generation - FlexibleMemoGenerator(String), + /// Error generating a memo with the flexible memo payload. + FlexibleMemo(String), /// Defragmentation transaction with non-zero change DefragWithChange, /// Other: {0} From 73a708bc91178c5775757adbb533a1c9fbdffee4 Mon Sep 17 00:00:00 2001 From: wjuan-mob Date: Wed, 19 Apr 2023 13:54:35 -0400 Subject: [PATCH 13/20] Update transaction/builder/src/memo_builder/rth_memo_builder.rs Co-authored-by: Sam Dealy <33067698+samdealy@users.noreply.github.com> --- transaction/builder/src/memo_builder/rth_memo_builder.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/transaction/builder/src/memo_builder/rth_memo_builder.rs b/transaction/builder/src/memo_builder/rth_memo_builder.rs index 3b5321308e..3cae211530 100644 --- a/transaction/builder/src/memo_builder/rth_memo_builder.rs +++ b/transaction/builder/src/memo_builder/rth_memo_builder.rs @@ -98,7 +98,7 @@ pub struct FlexibleMemoPayloads { /// which are defined in MCIPs. #[derive(Clone, Debug)] pub struct FlexibleMemoPayload { - /// memo_type_bytes corresponds to the returned memo type. This should be + /// Corresponds to the returned memo type. This should be /// listed in an MCIP. pub memo_type_bytes: [u8; 2], /// memo_data: corresponds to some 32 byte encoding of data used for From 9c2b96289b9dbfbcf0e3917e3cbd47bba4d35477 Mon Sep 17 00:00:00 2001 From: wjuan-mob Date: Wed, 19 Apr 2023 13:54:57 -0400 Subject: [PATCH 14/20] Update transaction/builder/src/memo_builder/rth_memo_builder.rs Co-authored-by: Sam Dealy <33067698+samdealy@users.noreply.github.com> --- transaction/builder/src/memo_builder/rth_memo_builder.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/transaction/builder/src/memo_builder/rth_memo_builder.rs b/transaction/builder/src/memo_builder/rth_memo_builder.rs index 3cae211530..6eca57537f 100644 --- a/transaction/builder/src/memo_builder/rth_memo_builder.rs +++ b/transaction/builder/src/memo_builder/rth_memo_builder.rs @@ -101,7 +101,7 @@ pub struct FlexibleMemoPayload { /// Corresponds to the returned memo type. This should be /// listed in an MCIP. pub memo_type_bytes: [u8; 2], - /// memo_data: corresponds to some 32 byte encoding of data used for + /// Corresponds to some 32 byte encoding of data used for /// the returned memo type, and does not include fields used for the /// authenticated sender or destination super types like the /// SenderMemoCredential From 6b63747d407b4605daeb2bb30c95a35dca6d4025 Mon Sep 17 00:00:00 2001 From: wjuan-mob Date: Wed, 19 Apr 2023 13:55:18 -0400 Subject: [PATCH 15/20] Update transaction/builder/src/memo_builder/rth_memo_builder.rs Co-authored-by: Sam Dealy <33067698+samdealy@users.noreply.github.com> --- transaction/builder/src/memo_builder/rth_memo_builder.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/transaction/builder/src/memo_builder/rth_memo_builder.rs b/transaction/builder/src/memo_builder/rth_memo_builder.rs index 6eca57537f..84de2b3681 100644 --- a/transaction/builder/src/memo_builder/rth_memo_builder.rs +++ b/transaction/builder/src/memo_builder/rth_memo_builder.rs @@ -129,7 +129,7 @@ impl Default for RTHMemoBuilder { /// These errors are usually created setting invalid field combinations on the /// memo builder. 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. +/// other error code. #[derive(Clone, Debug, Display, Eq, PartialEq)] pub enum MemoBuilderError { /// Invalid state change From 7f9bef81cc643d8f2501b6fe366ecf5d385c9ebb Mon Sep 17 00:00:00 2001 From: wjuan-mob Date: Wed, 19 Apr 2023 13:55:40 -0400 Subject: [PATCH 16/20] Update transaction/builder/src/memo_builder/rth_memo_builder.rs Co-authored-by: Sam Dealy <33067698+samdealy@users.noreply.github.com> --- transaction/builder/src/memo_builder/rth_memo_builder.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/transaction/builder/src/memo_builder/rth_memo_builder.rs b/transaction/builder/src/memo_builder/rth_memo_builder.rs index 84de2b3681..e76544a549 100644 --- a/transaction/builder/src/memo_builder/rth_memo_builder.rs +++ b/transaction/builder/src/memo_builder/rth_memo_builder.rs @@ -463,6 +463,7 @@ mod tests { change_memo_payload, } } + fn get_invalid_flexible_memos() -> FlexibleMemoPayloads { let memo_type_bytes = DESTINATION_CUSTOM_MEMO_TYPE_BYTES; let memo_data = [0x01; 32]; From ae29bd46a3d2999b7c4b52ea1f388f3c2d585687 Mon Sep 17 00:00:00 2001 From: William J Date: Mon, 24 Apr 2023 15:30:18 -0400 Subject: [PATCH 17/20] Change error name --- .../builder/src/memo_builder/rth_memo_builder.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/transaction/builder/src/memo_builder/rth_memo_builder.rs b/transaction/builder/src/memo_builder/rth_memo_builder.rs index e76544a549..f2016708e3 100644 --- a/transaction/builder/src/memo_builder/rth_memo_builder.rs +++ b/transaction/builder/src/memo_builder/rth_memo_builder.rs @@ -132,8 +132,8 @@ impl Default for RTHMemoBuilder { /// other error code. #[derive(Clone, Debug, Display, Eq, PartialEq)] pub enum MemoBuilderError { - /// Invalid state change - StateChange(String), + /// Unable to set custom memo type + CustomMemoType(String), /// Other Other(String), } @@ -165,7 +165,7 @@ impl RTHMemoBuilder { /// Set the payment request id. pub fn set_payment_request_id(&mut self, id: u64) -> Result<(), MemoBuilderError> { if self.custom_memo_type.is_some() { - return Err(MemoBuilderError::StateChange(format!( + return Err(MemoBuilderError::CustomMemoType(format!( "Custom Memo Type already set to {:?}", self.custom_memo_type ))); @@ -182,7 +182,7 @@ impl RTHMemoBuilder { /// Set the payment intent id. pub fn set_payment_intent_id(&mut self, id: u64) -> Result<(), MemoBuilderError> { if self.custom_memo_type.is_some() { - return Err(MemoBuilderError::StateChange(format!( + return Err(MemoBuilderError::CustomMemoType(format!( "Custom Memo Type already set to {:?}", self.custom_memo_type ))); @@ -197,7 +197,7 @@ impl RTHMemoBuilder { memos: FlexibleMemoPayloads, ) -> Result<(), MemoBuilderError> { if self.custom_memo_type.is_some() { - return Err(MemoBuilderError::StateChange(format!( + return Err(MemoBuilderError::CustomMemoType(format!( "Custom Memo Type already set to {:?}", self.custom_memo_type ))); @@ -463,7 +463,7 @@ mod tests { change_memo_payload, } } - + fn get_invalid_flexible_memos() -> FlexibleMemoPayloads { let memo_type_bytes = DESTINATION_CUSTOM_MEMO_TYPE_BYTES; let memo_data = [0x01; 32]; From 205701ccf9cafdcbba5b4e097b31371c520fe98e Mon Sep 17 00:00:00 2001 From: William J Date: Tue, 25 Apr 2023 13:51:53 -0400 Subject: [PATCH 18/20] Addig defaults --- .../src/memo_builder/rth_memo_builder.rs | 164 ++++++++++-------- .../builder/src/transaction_builder.rs | 15 +- 2 files changed, 98 insertions(+), 81 deletions(-) diff --git a/transaction/builder/src/memo_builder/rth_memo_builder.rs b/transaction/builder/src/memo_builder/rth_memo_builder.rs index f2016708e3..da04fc3e0f 100644 --- a/transaction/builder/src/memo_builder/rth_memo_builder.rs +++ b/transaction/builder/src/memo_builder/rth_memo_builder.rs @@ -87,20 +87,23 @@ pub enum CustomMemoType { #[derive(Clone, Debug)] pub struct FlexibleMemoPayloads { /// This is used when generating output memos. - /// It must contain a memo type with the first byte being 0x01 + /// It is used to generate a memo type with the first byte being 0x01 pub output_memo_payload: FlexibleMemoPayload, /// This is used when generating change memos. - /// It must contain a memo type with the first byte being 0x02 + /// It is used to generate a memo type with the first byte being 0x02 pub change_memo_payload: FlexibleMemoPayload, } +const OUTPUT_MEMO_TYPE: u8 = 0x01; +const CHANGE_MEMO_TYPE: u8 = 0x02; + /// This is the payload data used for creating custom MemoPayloads with types /// which are defined in MCIPs. #[derive(Clone, Debug)] pub struct FlexibleMemoPayload { - /// Corresponds to the returned memo type. This should be + /// Corresponds to the returned memo sub type. This should be /// listed in an MCIP. - pub memo_type_bytes: [u8; 2], + pub memo_type_byte: u8, /// Corresponds to some 32 byte encoding of data used for /// the returned memo type, and does not include fields used for the /// authenticated sender or destination super types like the @@ -108,6 +111,25 @@ pub struct FlexibleMemoPayload { pub memo_data: [u8; 32], } +/// This defaults to a Authenticated Sender and Destination memo +impl Default for FlexibleMemoPayloads { + fn default() -> Self { + Self { + output_memo_payload: Default::default(), + change_memo_payload: Default::default(), + } + } +} + +impl Default for FlexibleMemoPayload { + fn default() -> Self { + Self { + memo_type_byte: 0, + memo_data: [0; 32], + } + } +} + impl Default for RTHMemoBuilder { fn default() -> Self { Self { @@ -132,7 +154,7 @@ impl Default for RTHMemoBuilder { /// other error code. #[derive(Clone, Debug, Display, Eq, PartialEq)] pub enum MemoBuilderError { - /// Unable to set custom memo type + /// Unable to set custom memo typeK CustomMemoType(String), /// Other Other(String), @@ -262,17 +284,17 @@ impl MemoBuilder for RTHMemoBuilder { CustomMemoType::FlexibleMemos(flexible_memos) => { let tx_public_key = memo_context.tx_public_key; let flexible_memo_payload = &flexible_memos.output_memo_payload; + let memo_type_bytes = + [OUTPUT_MEMO_TYPE, flexible_memo_payload.memo_type_byte]; let memo_data = compute_authenticated_sender_memo( - flexible_memo_payload.memo_type_bytes, + memo_type_bytes, cred, recipient.view_public_key(), &tx_public_key.into(), &flexible_memo_payload.memo_data, ); - if flexible_memo_payload.memo_type_bytes[0] != 0x01 { - return Err(NewMemoError::FlexibleMemo(format!("The flexible output memo has a memopayload of the incorrect memo type: {:?}", flexible_memo_payload.memo_type_bytes))); - } - MemoPayload::new(flexible_memo_payload.memo_type_bytes, memo_data) + + MemoPayload::new(memo_type_bytes, memo_data) } CustomMemoType::PaymentRequestId(payment_request_id) => { AuthenticatedSenderWithPaymentRequestIdMemo::new( @@ -349,9 +371,7 @@ impl MemoBuilder for RTHMemoBuilder { match custom_memo_type { CustomMemoType::FlexibleMemos(flexible_memos) => { let flexible_memo_payload = &flexible_memos.change_memo_payload; - if flexible_memo_payload.memo_type_bytes[0] != 0x02 { - return Err(NewMemoError::FlexibleMemo(format!("The flexible change memo has a memopayload of the incorrect memo type: {:?}", flexible_memo_payload.memo_type_bytes))); - } + let memo_type_bytes = [CHANGE_MEMO_TYPE, flexible_memo_payload.memo_type_byte]; let memo_data = compute_destination_memo( self.last_recipient.clone(), self.fee.value, @@ -359,8 +379,7 @@ impl MemoBuilder for RTHMemoBuilder { self.total_outlay, flexible_memo_payload.memo_data, ); - let payload: MemoPayload = - MemoPayload::new(flexible_memo_payload.memo_type_bytes, memo_data); + let payload: MemoPayload = MemoPayload::new(memo_type_bytes, memo_data); self.wrote_destination_memo = true; Ok(payload) } @@ -371,10 +390,10 @@ impl MemoBuilder for RTHMemoBuilder { self.fee.value, *payment_request_id, ) { - Ok(mut d_memo) => { + Ok(mut destination_memo) => { self.wrote_destination_memo = true; - d_memo.set_num_recipients(self.num_recipients); - Ok(d_memo.into()) + destination_memo.set_num_recipients(self.num_recipients); + Ok(destination_memo.into()) } Err(err) => match err { DestinationMemoError::FeeTooLarge => { @@ -429,13 +448,15 @@ mod tests { use mc_crypto_keys::{CompressedRistrettoPublic, RistrettoPublic}; use mc_transaction_extra::{ get_data_from_authenticated_sender_memo, get_data_from_destination_memo, - validate_authenticated_sender, RegisteredMemoType, + validate_authenticated_sender, }; use mc_util_from_random::FromRandom; use rand::{rngs::StdRng, SeedableRng}; - const AUTHENTICATED_CUSTOM_MEMO_TYPE_BYTES: [u8; 2] = [0x01, 0x08]; - const DESTINATION_CUSTOM_MEMO_TYPE_BYTES: [u8; 2] = [0x02, 0x08]; + const AUTHENTICATED_CUSTOM_MEMO_TYPE_BYTE: u8 = 0x08; + const DESTINATION_CUSTOM_MEMO_TYPE_BYTE: u8 = 0x08; + const AUTHENTICATED_SENDER_MEMO_TYPE_BYTE: u8 = 0x00; + const DESTINATION_MEMO_TYPE_BYTE: u8 = 0x00; pub struct RTHMemoTestContext { sender: AccountKey, @@ -446,35 +467,16 @@ mod tests { } fn get_valid_flexible_memos() -> FlexibleMemoPayloads { - let memo_type_bytes = AUTHENTICATED_CUSTOM_MEMO_TYPE_BYTES; - let memo_data = [0x01; 32]; - let output_memo_payload = FlexibleMemoPayload { - memo_type_bytes, - memo_data, - }; - let memo_type_bytes = DESTINATION_CUSTOM_MEMO_TYPE_BYTES; - let memo_data = [0x01; 32]; - let change_memo_payload = FlexibleMemoPayload { - memo_type_bytes, - memo_data, - }; - FlexibleMemoPayloads { - output_memo_payload, - change_memo_payload, - } - } - - fn get_invalid_flexible_memos() -> FlexibleMemoPayloads { - let memo_type_bytes = DESTINATION_CUSTOM_MEMO_TYPE_BYTES; + let memo_type_byte = AUTHENTICATED_CUSTOM_MEMO_TYPE_BYTE; let memo_data = [0x01; 32]; let output_memo_payload = FlexibleMemoPayload { - memo_type_bytes, + memo_type_byte, memo_data, }; - let memo_type_bytes = AUTHENTICATED_CUSTOM_MEMO_TYPE_BYTES; + let memo_type_byte = DESTINATION_CUSTOM_MEMO_TYPE_BYTE; let memo_data = [0x01; 32]; let change_memo_payload = FlexibleMemoPayload { - memo_type_bytes, + memo_type_byte, memo_data, }; FlexibleMemoPayloads { @@ -557,14 +559,14 @@ mod tests { .expect("Expected valid change memo"); // Verify memo type + assert_eq!(OUTPUT_MEMO_TYPE, output_memo.get_memo_type()[0]); + assert_eq!(CHANGE_MEMO_TYPE, change_memo.get_memo_type()[0]); + // Verify memo sub type assert_eq!( - AuthenticatedSenderMemo::MEMO_TYPE_BYTES, - output_memo.get_memo_type().clone() - ); - assert_eq!( - DestinationMemo::MEMO_TYPE_BYTES, - change_memo.get_memo_type().clone() + AUTHENTICATED_SENDER_MEMO_TYPE_BYTE, + output_memo.get_memo_type()[1] ); + assert_eq!(DESTINATION_MEMO_TYPE_BYTE, change_memo.get_memo_type()[1]); // Verify memo data let authenticated_memo = AuthenticatedSenderMemo::from(output_memo.get_memo_data()); @@ -625,13 +627,17 @@ mod tests { .expect("Expected valid change memo"); // Verify memo type + assert_eq!(OUTPUT_MEMO_TYPE, output_memo.get_memo_type()[0]); + assert_eq!(CHANGE_MEMO_TYPE, change_memo.get_memo_type()[0]); + + // Verify memo sub type assert_eq!( - AUTHENTICATED_CUSTOM_MEMO_TYPE_BYTES, - output_memo.get_memo_type().clone() + AUTHENTICATED_CUSTOM_MEMO_TYPE_BYTE, + output_memo.get_memo_type()[1] ); assert_eq!( - DESTINATION_CUSTOM_MEMO_TYPE_BYTES, - change_memo.get_memo_type().clone() + DESTINATION_CUSTOM_MEMO_TYPE_BYTE, + change_memo.get_memo_type()[1] ); // Verify memo data @@ -657,9 +663,8 @@ mod tests { [1u8; 32] ); } - #[test] - fn test_flexible_funding_output_memo_rejects_invalid_types() { + fn test_flexible_funding_output_memo_built_successfully_using_defaults() { // Create Memo Builder with data let fee = Amount::new(1, 0.into()); let change_amount = Amount::new(1, 0.into()); @@ -669,29 +674,42 @@ mod tests { let (sender, mut builder) = build_test_memo_builder(&mut rng); // Add a flexible memo builder - .set_flexible_memos(get_invalid_flexible_memos()) - .expect("No other custom memo types should be set"); + .set_flexible_memos(FlexibleMemoPayloads::default()) + .expect("No other custom memo type should be set"); // Build the memo payload let memo_test_context = build_rth_memos(sender, builder, funding_amount, change_amount, fee); + let output_memo = memo_test_context + .output_memo + .expect("Expected valid output memo"); + let change_memo = memo_test_context + .change_memo + .expect("Expected valid change memo"); + + // Verify memo type + assert_eq!(OUTPUT_MEMO_TYPE, output_memo.get_memo_type()[0]); + assert_eq!(CHANGE_MEMO_TYPE, change_memo.get_memo_type()[0]); + + // Verify memo sub type assert_eq!( - memo_test_context - .output_memo - .expect_err("Should have an invalid output memo type"), - NewMemoError::FlexibleMemo( - "The flexible output memo has a memopayload of the incorrect memo type: [2, 8]" - .to_owned() - ) + AUTHENTICATED_SENDER_MEMO_TYPE_BYTE, + output_memo.get_memo_type()[1] ); - assert_eq!( - memo_test_context - .change_memo - .expect_err("Should have an invalid destination memo type"), - NewMemoError::FlexibleMemo( - "The flexible change memo has a memopayload of the incorrect memo type: [1, 8]" - .to_owned() - ) + assert_eq!(DESTINATION_MEMO_TYPE_BYTE, change_memo.get_memo_type()[1]); + + // Verify memo data + let destination_memo = DestinationMemo::from(change_memo.get_memo_data()); + + validate_authenticated_sender( + &memo_test_context.sender.default_subaddress(), + memo_test_context.receiver.view_private_key(), + &CompressedRistrettoPublic::from(memo_test_context.funding_public_key), + *output_memo.get_memo_type(), + output_memo.get_memo_data(), ); + + let derived_fee = destination_memo.get_fee(); + assert_eq!(fee.value, derived_fee); } } diff --git a/transaction/builder/src/transaction_builder.rs b/transaction/builder/src/transaction_builder.rs index a880c33f22..86fbdbcf10 100644 --- a/transaction/builder/src/transaction_builder.rs +++ b/transaction/builder/src/transaction_builder.rs @@ -944,10 +944,7 @@ pub mod transaction_builder_tests { validation::{validate_signature, validate_tx_out}, NewTxError, TokenId, }; - use mc_transaction_extra::{ - AuthenticatedSenderWithPaymentRequestIdMemo, DestinationWithPaymentRequestIdMemo, MemoType, - RegisteredMemoType, SenderMemoCredential, TxOutGiftCode, - }; + use mc_transaction_extra::{MemoType, SenderMemoCredential, TxOutGiftCode}; use rand::{rngs::StdRng, SeedableRng}; // Helper which produces a list of block_version, TokenId pairs to iterate over @@ -965,19 +962,21 @@ pub mod transaction_builder_tests { // request. fn get_valid_flexible_memo() -> FlexibleMemoPayloads { let payment_request_id = 42u64; - let memo_type_bytes = AuthenticatedSenderWithPaymentRequestIdMemo::MEMO_TYPE_BYTES; + let authenticated_sender_with_payment_request_id_memo_byte = 0x01; + let memo_type_byte = authenticated_sender_with_payment_request_id_memo_byte; let mut memo_data = [0x00; 32]; memo_data[0..8].copy_from_slice(&payment_request_id.to_be_bytes()); let output_memo_payload = FlexibleMemoPayload { - memo_type_bytes, + memo_type_byte, memo_data, }; let payment_request_id = 42u64; - let memo_type_bytes = DestinationWithPaymentRequestIdMemo::MEMO_TYPE_BYTES; + let destination_with_payment_request_id_memo_byte = 0x03; + let memo_type_byte = destination_with_payment_request_id_memo_byte; let mut memo_data = [0u8; 32]; memo_data[0..8].copy_from_slice(&payment_request_id.to_be_bytes()); let change_memo_payload = FlexibleMemoPayload { - memo_type_bytes, + memo_type_byte, memo_data, }; FlexibleMemoPayloads { From 2d92113c30078d43d45d95793063b4e8c2d221c2 Mon Sep 17 00:00:00 2001 From: William J Date: Tue, 25 Apr 2023 14:03:02 -0400 Subject: [PATCH 19/20] Changing constants to use named constants --- .../builder/src/memo_builder/rth_memo_builder.rs | 6 +++--- transaction/builder/src/transaction_builder.rs | 11 ++++++++--- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/transaction/builder/src/memo_builder/rth_memo_builder.rs b/transaction/builder/src/memo_builder/rth_memo_builder.rs index da04fc3e0f..2d06ee01a7 100644 --- a/transaction/builder/src/memo_builder/rth_memo_builder.rs +++ b/transaction/builder/src/memo_builder/rth_memo_builder.rs @@ -448,15 +448,15 @@ mod tests { use mc_crypto_keys::{CompressedRistrettoPublic, RistrettoPublic}; use mc_transaction_extra::{ get_data_from_authenticated_sender_memo, get_data_from_destination_memo, - validate_authenticated_sender, + validate_authenticated_sender, RegisteredMemoType, }; use mc_util_from_random::FromRandom; use rand::{rngs::StdRng, SeedableRng}; const AUTHENTICATED_CUSTOM_MEMO_TYPE_BYTE: u8 = 0x08; const DESTINATION_CUSTOM_MEMO_TYPE_BYTE: u8 = 0x08; - const AUTHENTICATED_SENDER_MEMO_TYPE_BYTE: u8 = 0x00; - const DESTINATION_MEMO_TYPE_BYTE: u8 = 0x00; + const AUTHENTICATED_SENDER_MEMO_TYPE_BYTE: u8 = AuthenticatedSenderMemo::MEMO_TYPE_BYTES[1]; + const DESTINATION_MEMO_TYPE_BYTE: u8 = DestinationMemo::MEMO_TYPE_BYTES[1]; pub struct RTHMemoTestContext { sender: AccountKey, diff --git a/transaction/builder/src/transaction_builder.rs b/transaction/builder/src/transaction_builder.rs index 86fbdbcf10..5d8df35a9d 100644 --- a/transaction/builder/src/transaction_builder.rs +++ b/transaction/builder/src/transaction_builder.rs @@ -944,7 +944,10 @@ pub mod transaction_builder_tests { validation::{validate_signature, validate_tx_out}, NewTxError, TokenId, }; - use mc_transaction_extra::{MemoType, SenderMemoCredential, TxOutGiftCode}; + use mc_transaction_extra::{ + AuthenticatedSenderWithPaymentRequestIdMemo, DestinationWithPaymentRequestIdMemo, MemoType, + RegisteredMemoType, SenderMemoCredential, TxOutGiftCode, + }; use rand::{rngs::StdRng, SeedableRng}; // Helper which produces a list of block_version, TokenId pairs to iterate over @@ -962,7 +965,8 @@ pub mod transaction_builder_tests { // request. fn get_valid_flexible_memo() -> FlexibleMemoPayloads { let payment_request_id = 42u64; - let authenticated_sender_with_payment_request_id_memo_byte = 0x01; + let authenticated_sender_with_payment_request_id_memo_byte = + AuthenticatedSenderWithPaymentRequestIdMemo::MEMO_TYPE_BYTES[1]; let memo_type_byte = authenticated_sender_with_payment_request_id_memo_byte; let mut memo_data = [0x00; 32]; memo_data[0..8].copy_from_slice(&payment_request_id.to_be_bytes()); @@ -971,7 +975,8 @@ pub mod transaction_builder_tests { memo_data, }; let payment_request_id = 42u64; - let destination_with_payment_request_id_memo_byte = 0x03; + let destination_with_payment_request_id_memo_byte = + DestinationWithPaymentRequestIdMemo::MEMO_TYPE_BYTES[1]; let memo_type_byte = destination_with_payment_request_id_memo_byte; let mut memo_data = [0u8; 32]; memo_data[0..8].copy_from_slice(&payment_request_id.to_be_bytes()); From 465ddc683032e3c8c15c1e15882b55e1a1324611 Mon Sep 17 00:00:00 2001 From: William J Date: Tue, 25 Apr 2023 15:13:40 -0400 Subject: [PATCH 20/20] lint --- .../builder/src/memo_builder/rth_memo_builder.rs | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/transaction/builder/src/memo_builder/rth_memo_builder.rs b/transaction/builder/src/memo_builder/rth_memo_builder.rs index 2d06ee01a7..d9edfa5c33 100644 --- a/transaction/builder/src/memo_builder/rth_memo_builder.rs +++ b/transaction/builder/src/memo_builder/rth_memo_builder.rs @@ -84,7 +84,7 @@ pub enum CustomMemoType { /// This contains both the output memo payload and the change memo payload which /// will be used when generating custom output memos and change memos -#[derive(Clone, Debug)] +#[derive(Clone, Debug, Default)] pub struct FlexibleMemoPayloads { /// This is used when generating output memos. /// It is used to generate a memo type with the first byte being 0x01 @@ -111,16 +111,7 @@ pub struct FlexibleMemoPayload { pub memo_data: [u8; 32], } -/// This defaults to a Authenticated Sender and Destination memo -impl Default for FlexibleMemoPayloads { - fn default() -> Self { - Self { - output_memo_payload: Default::default(), - change_memo_payload: Default::default(), - } - } -} - +/// This defaults to a Authenticated Sender and Destination memo respectively impl Default for FlexibleMemoPayload { fn default() -> Self { Self {