From 2e325939bc3bb513c94feb5900862cb728d881a1 Mon Sep 17 00:00:00 2001 From: Murisi Tarusenga Date: Sun, 8 Sep 2024 20:30:39 +0200 Subject: [PATCH] Now ensure that the MASP crate gets the correct key ak. --- Cargo.lock | 6 +-- Cargo.toml | 4 +- crates/apps_lib/src/cli/context.rs | 21 +++++----- crates/apps_lib/src/client/tx.rs | 15 +++++-- crates/core/src/masp.rs | 9 +++-- crates/sdk/src/args.rs | 4 +- crates/sdk/src/lib.rs | 10 ++--- crates/sdk/src/masp.rs | 53 +++++++++++-------------- crates/sdk/src/tx.rs | 6 +-- crates/shielded_token/src/validation.rs | 3 +- crates/tx/src/types.rs | 12 ++++++ 11 files changed, 80 insertions(+), 63 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 3e13da1ec3..3fca809348 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4401,7 +4401,7 @@ dependencies = [ [[package]] name = "masp_note_encryption" version = "1.0.0" -source = "git+https://github.com/anoma/masp?rev=e6451ecf64d519409f9b1a67aa1d8322a9fe0717#e6451ecf64d519409f9b1a67aa1d8322a9fe0717" +source = "git+https://github.com/anoma/masp?rev=f2b0cae3e495e4f7d482e587432ec4e5f2793528#f2b0cae3e495e4f7d482e587432ec4e5f2793528" dependencies = [ "arbitrary", "borsh", @@ -4415,7 +4415,7 @@ dependencies = [ [[package]] name = "masp_primitives" version = "1.0.0" -source = "git+https://github.com/anoma/masp?rev=e6451ecf64d519409f9b1a67aa1d8322a9fe0717#e6451ecf64d519409f9b1a67aa1d8322a9fe0717" +source = "git+https://github.com/anoma/masp?rev=f2b0cae3e495e4f7d482e587432ec4e5f2793528#f2b0cae3e495e4f7d482e587432ec4e5f2793528" dependencies = [ "aes", "arbitrary", @@ -4448,7 +4448,7 @@ dependencies = [ [[package]] name = "masp_proofs" version = "1.0.0" -source = "git+https://github.com/anoma/masp?rev=e6451ecf64d519409f9b1a67aa1d8322a9fe0717#e6451ecf64d519409f9b1a67aa1d8322a9fe0717" +source = "git+https://github.com/anoma/masp?rev=f2b0cae3e495e4f7d482e587432ec4e5f2793528#f2b0cae3e495e4f7d482e587432ec4e5f2793528" dependencies = [ "bellman", "blake2b_simd", diff --git a/Cargo.toml b/Cargo.toml index 230d1d75eb..3c315683ee 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -138,8 +138,8 @@ libc = "0.2.97" libloading = "0.7.2" linkme = "0.3.24" # branch = "tomas/arbitrary" -masp_primitives = { git = "https://github.com/anoma/masp", rev = "e6451ecf64d519409f9b1a67aa1d8322a9fe0717" } -masp_proofs = { git = "https://github.com/anoma/masp", rev = "e6451ecf64d519409f9b1a67aa1d8322a9fe0717", default-features = false, features = ["local-prover"] } +masp_primitives = { git = "https://github.com/anoma/masp", rev = "f2b0cae3e495e4f7d482e587432ec4e5f2793528" } +masp_proofs = { git = "https://github.com/anoma/masp", rev = "f2b0cae3e495e4f7d482e587432ec4e5f2793528", default-features = false, features = ["local-prover"] } num256 = "0.3.5" num_cpus = "1.13.0" num-derive = "0.4" diff --git a/crates/apps_lib/src/cli/context.rs b/crates/apps_lib/src/cli/context.rs index 633a37c587..cdcd16450a 100644 --- a/crates/apps_lib/src/cli/context.rs +++ b/crates/apps_lib/src/cli/context.rs @@ -16,7 +16,10 @@ use namada_sdk::masp::fs::FsShieldedUtils; use namada_sdk::masp::{ShieldedContext, *}; use namada_sdk::wallet::{DatedSpendingKey, DatedViewingKey, Wallet}; use namada_sdk::{Namada, NamadaImpl}; -use masp_primitives::zip32::sapling::PseudoExtendedSpendingKey; +use masp_primitives::zip32::sapling::PseudoExtendedKey; +use masp_primitives::zip32::{ + ExtendedFullViewingKey as MaspExtendedViewingKey, ExtendedSpendingKey as MaspExtendedSpendingKey, +}; use super::args; use crate::cli::utils; @@ -44,7 +47,7 @@ pub type WalletAddrOrNativeToken = FromContext; /// A raw extended spending key (bech32m encoding) or an alias of an extended /// spending key in the wallet -pub type WalletSpendingKey = FromContext; +pub type WalletSpendingKey = FromContext; /// A raw dated extended spending key (bech32m encoding) or an alias of an /// extended spending key in the wallet @@ -585,7 +588,7 @@ impl ArgFromMutContext for ExtendedSpendingKey { } } -impl ArgFromMutContext for PseudoExtendedSpendingKey { +impl ArgFromMutContext for PseudoExtendedKey { fn arg_from_mut_ctx( ctx: &mut ChainContext, raw: impl AsRef, @@ -593,23 +596,23 @@ impl ArgFromMutContext for PseudoExtendedSpendingKey { let raw = raw.as_ref(); // Either the string is a raw extended spending key ExtendedSpendingKey::from_str(raw).map( - |x| PseudoExtendedSpendingKey::from_spending_key(x.into()) + |x| PseudoExtendedKey::from(MaspExtendedSpendingKey::from(x)) ).or_else(|_parse_err| { ExtendedViewingKey::from_str(raw).map( - |x| PseudoExtendedSpendingKey::from_viewing_key(x.into()) + |x| PseudoExtendedKey::from(MaspExtendedViewingKey::from(x)) ) }).or_else(|_parse_err| { // Or it is a stored alias of one ctx.wallet .find_spending_key(raw, None) - .map(|k| PseudoExtendedSpendingKey::from_spending_key(k.key.into())) + .map(|k| PseudoExtendedKey::from(MaspExtendedSpendingKey::from(k.key))) .map_err(|_find_err| format!("Unknown spending key {}", raw)) }).or_else(|_parse_err| { // Or it is a stored alias of one ctx.wallet .find_viewing_key(raw) .copied() - .map(|k| PseudoExtendedSpendingKey::from_viewing_key(k.key.into())) + .map(|k| PseudoExtendedKey::from(MaspExtendedViewingKey::from(k.key))) .map_err(|_find_err| format!("Unknown viewing key {}", raw)) }) } @@ -694,11 +697,11 @@ impl ArgFromMutContext for TransferSource { .map(Self::Address) .or_else(|_| { ExtendedSpendingKey::arg_from_mut_ctx(ctx, raw) - .map(|x| Self::ExtendedSpendingKey(PseudoExtendedSpendingKey::from_spending_key(x.into()))) + .map(|x| Self::ExtendedSpendingKey(PseudoExtendedKey::from(MaspExtendedSpendingKey::from(x)))) }) .or_else(|_| { ExtendedViewingKey::arg_from_mut_ctx(ctx, raw) - .map(|x| Self::ExtendedSpendingKey(PseudoExtendedSpendingKey::from_viewing_key(x.into()))) + .map(|x| Self::ExtendedSpendingKey(PseudoExtendedKey::from(MaspExtendedViewingKey::from(x)))) }) } } diff --git a/crates/apps_lib/src/client/tx.rs b/crates/apps_lib/src/client/tx.rs index 74e8a0d4ed..4c2d18716e 100644 --- a/crates/apps_lib/src/client/tx.rs +++ b/crates/apps_lib/src/client/tx.rs @@ -29,6 +29,8 @@ use namada_sdk::collections::HashMap; use masp_primitives::sapling::redjubjub; use masp_primitives::transaction::components::sapling::fees::InputView; use crate::masp_primitives::transaction::components::sapling; +use masp_primitives::zip32::ExtendedKey; +use masp_primitives::sapling::redjubjub::PrivateKey; use masp_primitives::transaction::components::sapling::builder::{ BuildParams, ConvertBuildParams, OutputBuildParams, RngBuildParams, @@ -838,7 +840,7 @@ pub async fn submit_shielded_transfer( // Augment the pseudo spending key with a proof authorization key for data in &mut args.data { // Only attempt an augmentation if proof authorization is not there - if data.source.partial_spending_key().is_none() { + if data.source.to_spending_key().is_none() { // First find the derivation path corresponding to this viewing // key let viewing_key = @@ -900,11 +902,14 @@ pub async fn submit_shielded_transfer( hardware wallet: {}.", err, )))?; - // Finally augment the pseudo spending key - data.source.augment(pgk).map_err(|_| error::Error::Other(format!( + // Augment the pseudo spending key + data.source.augment_proof_generation_key(pgk).map_err(|_| error::Error::Other(format!( "Proof generation key in response from the hardware wallet \ does not correspond to stored viewing key.", )))?; + // Finally, augment an incorrect spend authorization key just to + // make sure that the Transaction is built. + data.source.augment_spend_authorizing_key_unchecked(PrivateKey(jubjub::Fr::default())); shielded_hw_keys.insert(path.path, viewing_key); } } @@ -988,7 +993,7 @@ pub async fn submit_shielded_transfer( for (path, vk) in shielded_hw_keys { // Sign the MASP Transaction using the current viewing key let path = BIP44Path { path: path.to_string() }; - let response = app + app .sign_masp(&path, &tx.serialize_to_vec()) .await .map_err(|err| error::Error::Other(err.to_string()))?; @@ -1027,6 +1032,8 @@ pub async fn submit_shielded_transfer( err, )))?; } + tx.remove_masp_section(&shielded_hash); + tx.add_section(Section::MaspTx(masp_tx)); } sign(namada, &mut tx, &args.tx, signing_data).await?; namada.submit(tx, &args.tx).await?; diff --git a/crates/core/src/masp.rs b/crates/core/src/masp.rs index e8aa961d06..08bf1be6fb 100644 --- a/crates/core/src/masp.rs +++ b/crates/core/src/masp.rs @@ -10,7 +10,8 @@ use borsh_ext::BorshSerializeExt; use masp_primitives::asset_type::AssetType; use masp_primitives::sapling::ViewingKey; use masp_primitives::transaction::TransparentAddress; -use masp_primitives::zip32::sapling::PseudoExtendedSpendingKey; +use masp_primitives::zip32::sapling::PseudoExtendedKey; +use masp_primitives::zip32::ExtendedKey; pub use masp_primitives::transaction::TxId as TxIdInner; use namada_macros::BorshDeserializer; #[cfg(feature = "migrations")] @@ -68,7 +69,7 @@ pub struct MaspTxId( serialize_with = "serialize_txid", deserialize_with = "deserialize_txid" )] - TxIdInner, + pub TxIdInner, ); impl From for MaspTxId { @@ -518,7 +519,7 @@ pub enum TransferSource { /// A transfer coming from a transparent address Address(Address), /// A transfer coming from a shielded address - ExtendedSpendingKey(PseudoExtendedSpendingKey), + ExtendedSpendingKey(PseudoExtendedKey), } impl TransferSource { @@ -533,7 +534,7 @@ impl TransferSource { } /// Get the contained ExtendedSpendingKey contained, if any - pub fn spending_key(&self) -> Option { + pub fn spending_key(&self) -> Option { match self { Self::ExtendedSpendingKey(x) => Some(*x), _ => None, diff --git a/crates/sdk/src/args.rs b/crates/sdk/src/args.rs index 91c76d7201..78d0df2f6f 100644 --- a/crates/sdk/src/args.rs +++ b/crates/sdk/src/args.rs @@ -6,7 +6,7 @@ use std::str::FromStr; use std::time::Duration as StdDuration; use masp_primitives::transaction::components::sapling::builder::BuildParams; -use masp_primitives::zip32::sapling::PseudoExtendedSpendingKey; +use masp_primitives::zip32::sapling::PseudoExtendedKey; use namada_core::address::Address; use namada_core::chain::{BlockHeight, ChainId, Epoch}; @@ -121,7 +121,7 @@ impl NamadaTypes for SdkTypes { type MaspIndexerAddress = String; type PaymentAddress = namada_core::masp::PaymentAddress; type PublicKey = namada_core::key::common::PublicKey; - type SpendingKey = PseudoExtendedSpendingKey; + type SpendingKey = PseudoExtendedKey; type TendermintAddress = tendermint_rpc::Url; type TransferSource = namada_core::masp::TransferSource; type TransferTarget = namada_core::masp::TransferTarget; diff --git a/crates/sdk/src/lib.rs b/crates/sdk/src/lib.rs index ccab6ea824..0a73ac6bb5 100644 --- a/crates/sdk/src/lib.rs +++ b/crates/sdk/src/lib.rs @@ -51,7 +51,7 @@ pub use std::marker::Sync as MaybeSync; use std::path::PathBuf; use std::str::FromStr; -use masp_primitives::zip32::sapling::PseudoExtendedSpendingKey; +use masp_primitives::zip32::sapling::PseudoExtendedKey; use args::{DeviceTransport, InputAmount, SdkTypes}; use io::Io; use masp::{ShieldedContext, ShieldedUtils}; @@ -62,7 +62,7 @@ use namada_core::dec::Dec; use namada_core::ethereum_events::EthAddress; use namada_core::ibc::core::host::types::identifiers::{ChannelId, PortId}; use namada_core::key::*; -use namada_core::masp::{ExtendedSpendingKey, PaymentAddress, TransferSource}; +use namada_core::masp::{PaymentAddress, TransferSource}; use namada_tx::data::wrapper::GasLimit; use namada_tx::Tx; use rpc::{denominate_amount, format_denominated_amount, query_native_token}; @@ -190,7 +190,7 @@ pub trait Namada: Sized + MaybeSync + MaybeSend { fn new_shielded_transfer( &self, data: Vec, - gas_spending_keys: Vec, + gas_spending_keys: Vec, disposable_signing_key: bool, ) -> args::TxShieldedTransfer { args::TxShieldedTransfer { @@ -221,9 +221,9 @@ pub trait Namada: Sized + MaybeSync + MaybeSend { /// arguments fn new_unshielding_transfer( &self, - source: PseudoExtendedSpendingKey, + source: PseudoExtendedKey, data: Vec, - gas_spending_keys: Vec, + gas_spending_keys: Vec, disposable_signing_key: bool, ) -> args::TxUnshieldingTransfer { args::TxUnshieldingTransfer { diff --git a/crates/sdk/src/masp.rs b/crates/sdk/src/masp.rs index 927c430671..1224bf3831 100644 --- a/crates/sdk/src/masp.rs +++ b/crates/sdk/src/masp.rs @@ -29,7 +29,7 @@ use masp_primitives::transaction::builder::{self, *}; use masp_primitives::transaction::components::sapling::builder::{ BuildParams, RngBuildParams, SaplingMetadata, }; -use masp_primitives::zip32::sapling::PseudoExtendedSpendingKey; +use masp_primitives::zip32::sapling::PseudoExtendedKey; use masp_primitives::transaction::components::{ I128Sum, TxOut, U64Sum, ValueSum, }; @@ -86,6 +86,7 @@ use crate::{ control_flow, display_line, edisplay_line, query_native_token, rpc, MaybeSend, MaybeSync, Namada, }; +use masp_primitives::zip32::ExtendedKey; /// Randomness seed for MASP integration tests to build proofs with /// deterministic rng. @@ -111,7 +112,7 @@ pub struct ShieldedTransfer { #[allow(missing_docs)] #[derive(Debug)] pub struct MaspFeeData { - pub sources: Vec, + pub sources: Vec, pub target: Address, pub token: Address, pub amount: token::DenominatedAmount, @@ -160,7 +161,7 @@ struct MaspTxReorderedData { // Data about the unspent amounts for any given shielded source coming from the // spent notes in their posses that have been added to the builder. Can be used // to either pay fees or to return a change -type Changes = HashMap; +type Changes = HashMap; /// Shielded pool data for a token #[allow(missing_docs)] @@ -198,20 +199,20 @@ struct WalletMap; impl masp_primitives::transaction::components::sapling::builder::MapBuilder< P1, - MaspExtendedSpendingKey, + PseudoExtendedKey, (), ExtendedFullViewingKey, > for WalletMap { fn map_params(&self, _s: P1) {} - fn map_key(&self, s: MaspExtendedSpendingKey) -> ExtendedFullViewingKey { - (&s).into() + fn map_key(&self, s: PseudoExtendedKey) -> ExtendedFullViewingKey { + s.to_viewing_key() } } impl - MapBuilder + MapBuilder for WalletMap { fn map_notifier(&self, _s: N1) {} @@ -942,7 +943,7 @@ impl ShieldedContext { &mut self, context: &impl Namada, spent_notes: &mut SpentNotesTracker, - sk: PseudoExtendedSpendingKey, + sk: PseudoExtendedKey, is_native_token: bool, target: I128Sum, target_epoch: MaspEpoch, @@ -1227,7 +1228,7 @@ impl ShieldedContext { u32::MAX - 20 } }; - let mut builder = Builder::::new( + let mut builder = Builder::::new( NETWORK, // NOTE: this is going to add 20 more blocks to the actual // expiration but there's no other exposed function that we could @@ -1437,7 +1438,7 @@ impl ShieldedContext { #[allow(clippy::too_many_arguments)] async fn add_inputs( context: &impl Namada, - builder: &mut Builder, + builder: &mut Builder, source: &TransferSource, token: &Address, amount: &token::DenominatedAmount, @@ -1497,11 +1498,7 @@ impl ShieldedContext { for (diversifier, note, merkle_path) in unspent_notes { builder .add_sapling_spend( - sk.partial_spending_key().ok_or_else(|| { - Error::Other(format!( - "Unable to get proof authorization" - )) - })?, + sk, diversifier, note, merkle_path, @@ -1570,7 +1567,7 @@ impl ShieldedContext { #[allow(clippy::too_many_arguments)] async fn add_outputs( context: &impl Namada, - builder: &mut Builder, + builder: &mut Builder, source: TransferSource, target: &TransferTarget, token: Address, @@ -1711,9 +1708,9 @@ impl ShieldedContext { #[allow(clippy::too_many_arguments)] async fn add_fees( context: &impl Namada, - builder: &mut Builder, + builder: &mut Builder, source_data: &HashMap, - sources: Vec, + sources: Vec, target: &Address, token: &Address, amount: &token::DenominatedAmount, @@ -1949,21 +1946,17 @@ impl ShieldedContext { // `add_fees` cause we might have some change coming from there too #[allow(clippy::result_large_err)] fn add_changes( - builder: &mut Builder, + builder: &mut Builder, changes: Changes, ) -> Result<(), TransferErr> { for (sp, changes) in changes.into_iter() { for (asset_type, amt) in changes.components() { if let Ordering::Greater = amt.cmp(&0) { - let sk = sp.partial_spending_key().ok_or_else(|| { - Error::Other(format!( - "Unable to get proof authorization" - )) - })?; + let sk = sp.to_viewing_key(); // Send the change in this asset type back to the sender builder .add_sapling_output( - Some(sk.expsk.ovk), + Some(sk.fvk.ovk), sk.default_address().1, *asset_type, *amt as u64, @@ -2662,7 +2655,7 @@ pub mod testing { mut rng in arb_rng().prop_map(TestCsprng), bparams_rng in arb_rng().prop_map(TestCsprng), prover_rng in arb_rng().prop_map(TestCsprng), - ) -> (MaspExtendedSpendingKey, Diversifier, Note, Node) { + ) -> (PseudoExtendedKey, Diversifier, Note, Node) { let mut spending_key_seed = [0; 32]; rng.fill_bytes(&mut spending_key_seed); let spending_key = MaspExtendedSpendingKey::master(spending_key_seed.as_ref()); @@ -2673,7 +2666,7 @@ pub mod testing { .to_payment_address(div) .expect("a PaymentAddress"); - let mut builder = Builder::::new( + let mut builder = Builder::::new( NETWORK, // NOTE: this is going to add 20 more blocks to the actual // expiration but there's no other exposed function that we could @@ -2707,7 +2700,7 @@ pub mod testing { assert_eq!(payment_addr, pa); // Make a path to out new note let node = Node::new(shielded_output.cmu.to_repr()); - (spending_key, div, note, node) + (PseudoExtendedKey::from(spending_key), div, note, node) } } @@ -2757,7 +2750,7 @@ pub mod testing { ).unwrap(), *value, )).collect::>() - ) -> Vec<(MaspExtendedSpendingKey, Diversifier, Note, Node)> { + ) -> Vec<(PseudoExtendedKey, Diversifier, Note, Node)> { spend_description } } @@ -2833,7 +2826,7 @@ pub mod testing { assets in Just(assets), ) -> ( Transfer, - Builder::, + Builder::, HashMap, ) { // Enable assets to be more easily decoded diff --git a/crates/sdk/src/tx.rs b/crates/sdk/src/tx.rs index 343e30163b..ac7d60babc 100644 --- a/crates/sdk/src/tx.rs +++ b/crates/sdk/src/tx.rs @@ -16,7 +16,7 @@ use masp_primitives::transaction::components::sapling::fees::{ use masp_primitives::transaction::components::transparent::fees::{ InputView as TransparentInputView, OutputView as TransparentOutputView, }; -use masp_primitives::zip32::sapling::PseudoExtendedSpendingKey; +use masp_primitives::zip32::sapling::PseudoExtendedKey; use masp_primitives::transaction::components::sapling::builder::{BuildParams, RngBuildParams}; use masp_primitives::transaction::components::I128Sum; use masp_primitives::transaction::{builder, Transaction as MaspTransaction}; @@ -39,7 +39,7 @@ use namada_core::ibc::core::host::types::identifiers::{ChannelId, PortId}; use namada_core::ibc::primitives::Timestamp as IbcTimestamp; use namada_core::key::{self, *}; use namada_core::masp::{ - AssetData, ExtendedSpendingKey, MaspEpoch, TransferSource, TransferTarget, + AssetData, MaspEpoch, TransferSource, TransferTarget, }; use namada_core::storage; use namada_core::time::DateTimeUtc; @@ -3131,7 +3131,7 @@ async fn get_masp_fee_payment_amount( args: &args::Tx, fee_amount: DenominatedAmount, fee_payer: &common::PublicKey, - gas_spending_keys: Vec, + gas_spending_keys: Vec, ) -> Result> { let fee_payer_address = Address::from(fee_payer); let balance_key = balance_key(&args.fee_token, &fee_payer_address); diff --git a/crates/shielded_token/src/validation.rs b/crates/shielded_token/src/validation.rs index 1567791acd..cfa155cac6 100644 --- a/crates/shielded_token/src/validation.rs +++ b/crates/shielded_token/src/validation.rs @@ -20,6 +20,7 @@ use masp_proofs::bellman::groth16::VerifyingKey; use masp_proofs::sapling::BatchValidator; use rand_core::OsRng; use smooth_operator::checked; +use masp_primitives::zip32::ExtendedSpendingKey; use crate::{Error, Result}; @@ -55,7 +56,7 @@ pub struct PartialAuthorized; impl Authorization for PartialAuthorized { type SaplingAuth = ::SaplingAuth; - type TransparentAuth = ::TransparentAuth; + type TransparentAuth = as Authorization>::TransparentAuth; } /// MASP verifying keys diff --git a/crates/tx/src/types.rs b/crates/tx/src/types.rs index 2bc0fda9b0..bd26af4d44 100644 --- a/crates/tx/src/types.rs +++ b/crates/tx/src/types.rs @@ -1293,6 +1293,18 @@ impl Tx { None } + /// Remove the transaction section with the given hash + pub fn remove_masp_section(&mut self, hash: &MaspTxId) { + self.sections.retain(|section| { + if let Section::MaspTx(masp) = section { + if MaspTxId::from(masp.txid()) == *hash { + return false; + } + } + true + }); + } + /// Get the MASP builder section with the given hash pub fn get_masp_builder(&self, hash: &MaspTxId) -> Option<&MaspBuilder> { for section in &self.sections {