diff --git a/.changelog/unreleased/features/3653-masp-key-birthdays.md b/.changelog/unreleased/features/3653-masp-key-birthdays.md new file mode 100644 index 0000000000..1b85948570 --- /dev/null +++ b/.changelog/unreleased/features/3653-masp-key-birthdays.md @@ -0,0 +1,4 @@ + - Partially addresses Issue [\#2900](https://github.com/anoma/namada/issues/2900). Viewing and spending keys can now + be given birthdays in the form of block heights which are loaded into + shielded sync. Shielded sync will not try to decrypt a block before a + keys birthday with said key. ([\#3653](https://github.com/anoma/namada/pull/3653)) \ No newline at end of file diff --git a/crates/apps_lib/src/cli.rs b/crates/apps_lib/src/cli.rs index af64f76c82..b7deb07b81 100644 --- a/crates/apps_lib/src/cli.rs +++ b/crates/apps_lib/src/cli.rs @@ -3211,6 +3211,7 @@ pub mod args { Err(_) => config::get_default_namada_folder(), }), ); + pub const BIRTHDAY: ArgOpt = arg_opt("birthday"); pub const BLOCK_HEIGHT: Arg = arg("block-height"); pub const BLOCK_HEIGHT_OPT: ArgOpt = arg_opt("height"); pub const BLOCK_HEIGHT_FROM_OPT: ArgOpt = @@ -3255,6 +3256,10 @@ pub mod args { arg_opt("success-sleep"); pub const DATA_PATH_OPT: ArgOpt = arg_opt("data-path"); pub const DATA_PATH: Arg = arg("data-path"); + pub const DATED_SPENDING_KEYS: ArgMulti = + arg_multi("spending-keys"); + pub const DATED_VIEWING_KEYS: ArgMulti = + arg_multi("viewing-keys"); pub const DB_KEY: Arg = arg("db-key"); pub const DB_COLUMN_FAMILY: ArgDefault = arg_default( "db-column-family", @@ -6598,8 +6603,8 @@ pub mod args { let ledger_address = CONFIG_RPC_LEDGER_ADDRESS.parse(matches); let start_query_height = BLOCK_HEIGHT_FROM_OPT.parse(matches); let last_query_height = BLOCK_HEIGHT_TO_OPT.parse(matches); - let spending_keys = SPENDING_KEYS.parse(matches); - let viewing_keys = VIEWING_KEYS.parse(matches); + let spending_keys = DATED_SPENDING_KEYS.parse(matches); + let viewing_keys = DATED_VIEWING_KEYS.parse(matches); let with_indexer = WITH_INDEXER.parse(matches); Self { ledger_address, @@ -6621,13 +6626,17 @@ pub mod args { .def() .help(wrap!("Option block height to sync from.")), ) - .arg(SPENDING_KEYS.def().help(wrap!( + .arg(DATED_SPENDING_KEYS.def().help(wrap!( "List of new spending keys with which to check note \ - ownership. These will be added to the shielded context." + ownership. These will be added to the shielded context. \ + Appending \"<<$BLOCKHEIGHT\" to the end of each key adds \ + a birthday." ))) - .arg(VIEWING_KEYS.def().help(wrap!( + .arg(DATED_VIEWING_KEYS.def().help(wrap!( "List of new viewing keys with which to check note \ - ownership. These will be added to the shielded context." + ownership. These will be added to the shielded context. \ + Appending \"<<$BLOCKHEIGHT\" to the end of each key adds \ + a birthday." ))) .arg(WITH_INDEXER.def().help(wrap!( "Address of a `namada-masp-indexer` live instance. If \ @@ -6982,6 +6991,8 @@ pub mod args { type BpConversionTable = PathBuf; type ConfigRpcTendermintAddress = ConfigRpcAddress; type Data = PathBuf; + type DatedSpendingKey = WalletDatedSpendingKey; + type DatedViewingKey = WalletDatedViewingKey; type EthereumAddress = String; type Keypair = WalletKeypair; type MaspIndexerAddress = String; @@ -7338,7 +7349,8 @@ pub mod args { find_viewing_key(&mut wallet) } else { find_viewing_key(&mut ctx.borrow_mut_chain_or_exit().wallet) - }; + } + .key; Ok(PayAddressGen:: { alias: self.alias, @@ -7377,6 +7389,7 @@ pub mod args { let shielded = SHIELDED.parse(matches); let alias = ALIAS.parse(matches); let alias_force = ALIAS_FORCE.parse(matches); + let birthday = BIRTHDAY.parse(matches); let unsafe_dont_encrypt = UNSAFE_DONT_ENCRYPT.parse(matches); let derivation_path = HD_DERIVATION_PATH.parse(matches); let allow_non_compliant = @@ -7396,6 +7409,7 @@ pub mod args { prompt_bip39_passphrase, use_device, device_transport, + birthday, } } @@ -7417,6 +7431,11 @@ pub mod args { "Force overwrite the alias if it already exists." )), ) + .arg(BIRTHDAY.def().help(wrap!( + "A block height after which this key is being created. Used \ + for optimizing MASP operations. If none is provided, \ + defaults to the first block height." + ))) .arg(UNSAFE_DONT_ENCRYPT.def().help(wrap!( "UNSAFE: Do not encrypt the keypair. Do not use this for keys \ used in a live network." @@ -7465,6 +7484,7 @@ pub mod args { let raw = RAW_KEY_GEN.parse(matches); let alias = ALIAS.parse(matches); let alias_force = ALIAS_FORCE.parse(matches); + let birthday = BIRTHDAY.parse(matches); let unsafe_dont_encrypt = UNSAFE_DONT_ENCRYPT.parse(matches); let derivation_path = HD_DERIVATION_PATH.parse(matches); let allow_non_compliant = @@ -7477,6 +7497,7 @@ pub mod args { raw, alias, alias_force, + birthday, unsafe_dont_encrypt, derivation_path, allow_non_compliant, @@ -7509,6 +7530,11 @@ pub mod args { .arg(ALIAS_FORCE.def().help(wrap!( "Override the alias without confirmation if it already exists." ))) + .arg(BIRTHDAY.def().help(wrap!( + "A block height after which this key is being created. Used \ + for optimizing MASP operations. If none is provided, \ + defaults to the first block height." + ))) .arg(UNSAFE_DONT_ENCRYPT.def().help(wrap!( "UNSAFE: Do not encrypt the keypair. Do not use this for keys \ used in a live network." @@ -7680,11 +7706,13 @@ pub mod args { fn parse(matches: &ArgMatches) -> Self { let alias = ALIAS.parse(matches); let alias_force = ALIAS_FORCE.parse(matches); + let birthday = BIRTHDAY.parse(matches); let value = VALUE.parse(matches); let unsafe_dont_encrypt = UNSAFE_DONT_ENCRYPT.parse(matches); Self { alias, alias_force, + birthday, value, unsafe_dont_encrypt, } @@ -7699,6 +7727,11 @@ pub mod args { .arg(ALIAS_FORCE.def().help(wrap!( "Override the alias without confirmation if it already exists." ))) + .arg(BIRTHDAY.def().help(wrap!( + "A block height after which this key is being created. Used \ + for optimizing MASP operations. If none is provided, \ + defaults to the first block height." + ))) .arg(VALUE.def().help(wrap!( "Any value of the following:\n- transparent pool secret \ key\n- transparent pool public key\n- transparent pool \ diff --git a/crates/apps_lib/src/cli/client.rs b/crates/apps_lib/src/cli/client.rs index 61ce4a5512..4a6d802b51 100644 --- a/crates/apps_lib/src/cli/client.rs +++ b/crates/apps_lib/src/cli/client.rs @@ -1,7 +1,6 @@ use std::io::Read; use color_eyre::eyre::Result; -use masp_primitives::zip32::ExtendedFullViewingKey; use namada_sdk::io::Io; use namada_sdk::{display_line, Namada, NamadaImpl}; @@ -351,15 +350,8 @@ impl CliApi { .get_viewing_keys() .values() .copied() - .map(|vk| ExtendedFullViewingKey::from(vk).fvk.vk) - .chain(args.viewing_keys.into_iter().map(|vk| { - ExtendedFullViewingKey::from(vk).fvk.vk - })) - .collect::>(); - let sks = args - .spending_keys - .into_iter() - .map(|sk| sk.into()) + .chain(args.viewing_keys) + .map(|vk| vk.map(|vk| vk.as_viewing_key())) .collect::>(); crate::client::masp::syncing( chain_ctx.shielded, @@ -368,7 +360,7 @@ impl CliApi { &io, args.start_query_height, args.last_query_height, - &sks, + &args.spending_keys, &vks, ) .await?; diff --git a/crates/apps_lib/src/cli/context.rs b/crates/apps_lib/src/cli/context.rs index aa0e6c6141..de4220b040 100644 --- a/crates/apps_lib/src/cli/context.rs +++ b/crates/apps_lib/src/cli/context.rs @@ -14,7 +14,7 @@ use namada_sdk::io::Io; use namada_sdk::key::*; use namada_sdk::masp::fs::FsShieldedUtils; use namada_sdk::masp::{ShieldedContext, *}; -use namada_sdk::wallet::Wallet; +use namada_sdk::wallet::{DatedSpendingKey, DatedViewingKey, Wallet}; use namada_sdk::{Namada, NamadaImpl}; use super::args; @@ -45,6 +45,10 @@ pub type WalletAddrOrNativeToken = FromContext; /// spending key in the wallet pub type WalletSpendingKey = FromContext; +/// A raw dated extended spending key (bech32m encoding) or an alias of an +/// extended spending key in the wallet +pub type WalletDatedSpendingKey = FromContext; + /// A raw payment address (bech32m encoding) or an alias of a payment address /// in the wallet pub type WalletPaymentAddr = FromContext; @@ -53,6 +57,10 @@ pub type WalletPaymentAddr = FromContext; /// in the wallet pub type WalletViewingKey = FromContext; +/// A raw full dated viewing key (bech32m encoding) or an alias of a full +/// viewing key in the wallet +pub type WalletDatedViewingKey = FromContext; + /// A raw address or a raw extended spending key (bech32m encoding) or an alias /// of either in the wallet pub type WalletTransferSource = FromContext; @@ -561,6 +569,23 @@ impl ArgFromContext for common::PublicKey { } impl ArgFromMutContext for ExtendedSpendingKey { + fn arg_from_mut_ctx( + ctx: &mut ChainContext, + raw: impl AsRef, + ) -> Result { + let raw = raw.as_ref(); + // Either the string is a raw extended spending key + FromStr::from_str(raw).or_else(|_parse_err| { + // Or it is a stored alias of one + ctx.wallet + .find_spending_key(raw, None) + .map(|k| k.key) + .map_err(|_find_err| format!("Unknown spending key {}", raw)) + }) + } +} + +impl ArgFromMutContext for DatedSpendingKey { fn arg_from_mut_ctx( ctx: &mut ChainContext, raw: impl AsRef, @@ -577,6 +602,24 @@ impl ArgFromMutContext for ExtendedSpendingKey { } impl ArgFromMutContext for ExtendedViewingKey { + fn arg_from_mut_ctx( + ctx: &mut ChainContext, + raw: impl AsRef, + ) -> Result { + let raw = raw.as_ref(); + // Either the string is a raw full viewing key + FromStr::from_str(raw).or_else(|_parse_err| { + // Or it is a stored alias of one + ctx.wallet + .find_viewing_key(raw) + .copied() + .map(|k| k.key) + .map_err(|_find_err| format!("Unknown viewing key {}", raw)) + }) + } +} + +impl ArgFromMutContext for DatedViewingKey { fn arg_from_mut_ctx( ctx: &mut ChainContext, raw: impl AsRef, diff --git a/crates/apps_lib/src/cli/wallet.rs b/crates/apps_lib/src/cli/wallet.rs index b5f61578b1..ebdce5d35c 100644 --- a/crates/apps_lib/src/cli/wallet.rs +++ b/crates/apps_lib/src/cli/wallet.rs @@ -9,7 +9,7 @@ use borsh_ext::BorshSerializeExt; use color_eyre::eyre::Result; use itertools::sorted; use ledger_namada_rs::{BIP44Path, NamadaApp}; -use masp_primitives::zip32::ExtendedFullViewingKey; +use namada_core::storage::BlockHeight; use namada_sdk::address::{Address, DecodeError}; use namada_sdk::io::Io; use namada_sdk::key::*; @@ -193,6 +193,7 @@ fn shielded_key_derive( allow_non_compliant, prompt_bip39_passphrase, use_device, + birthday, .. }: args::KeyDerive, ) { @@ -216,6 +217,7 @@ fn shielded_key_derive( .derive_store_spending_key_from_mnemonic_code( alias, alias_force, + birthday, derivation_path, None, prompt_bip39_passphrase, @@ -254,6 +256,7 @@ fn shielded_key_gen( derivation_path, allow_non_compliant, prompt_bip39_passphrase, + birthday, .. }: args::KeyGen, ) { @@ -261,7 +264,13 @@ fn shielded_key_gen( let alias = alias.to_lowercase(); let password = read_and_confirm_encryption_password(unsafe_dont_encrypt); let alias = if raw { - wallet.gen_store_spending_key(alias, password, alias_force, &mut OsRng) + wallet.gen_store_spending_key( + alias, + birthday, + password, + alias_force, + &mut OsRng, + ) } else { let derivation_path = decode_shielded_derivation_path(derivation_path) .unwrap_or_else(|err| { @@ -281,9 +290,10 @@ fn shielded_key_gen( &mut OsRng, prompt_bip39_passphrase, ); - wallet.derive_store_hd_spendind_key( + wallet.derive_store_hd_spending_key( alias, alias_force, + birthday, seed, derivation_path, password, @@ -319,7 +329,7 @@ fn payment_address_gen( ) { let mut wallet = load_wallet(ctx); let alias = alias.to_lowercase(); - let viewing_key = ExtendedFullViewingKey::from(viewing_key).fvk.vk; + let viewing_key = viewing_key.as_viewing_key(); let (div, _g_d) = find_valid_diversifier(&mut OsRng); let masp_payment_addr = viewing_key .to_payment_address(div) @@ -346,6 +356,7 @@ fn shielded_key_address_add( io: &impl Io, alias: String, alias_force: bool, + birthday: Option, masp_value: MaspValue, unsafe_dont_encrypt: bool, ) { @@ -354,7 +365,7 @@ fn shielded_key_address_add( let (alias, typ) = match masp_value { MaspValue::FullViewingKey(viewing_key) => { let alias = wallet - .insert_viewing_key(alias, viewing_key, alias_force) + .insert_viewing_key(alias, viewing_key, birthday, alias_force) .unwrap_or_else(|| { edisplay_line!(io, "Viewing key not added"); cli::safe_exit(1); @@ -369,6 +380,7 @@ fn shielded_key_address_add( alias, alias_force, spending_key, + birthday, password, None, ) @@ -444,8 +456,8 @@ async fn transparent_key_and_address_derive( allow_non_compliant, prompt_bip39_passphrase, use_device, - shielded: _, device_transport, + .. }: args::KeyDerive, ) { let mut wallet = load_wallet(ctx); @@ -788,6 +800,7 @@ fn add_key_or_address( io: &impl Io, alias: String, alias_force: bool, + birthday: Option, value: KeyAddrAddValue, unsafe_dont_encrypt: bool, ) { @@ -813,6 +826,7 @@ fn add_key_or_address( io, alias, alias_force, + birthday, masp_value, unsafe_dont_encrypt, ), @@ -827,8 +841,8 @@ fn key_address_add( alias, alias_force, value, + birthday, unsafe_dont_encrypt, - .. }: args::KeyAddressAdd, ) { let value = KeyAddrAddValue::from_str(&value).unwrap_or_else(|err| { @@ -836,7 +850,15 @@ fn key_address_add( display_line!(io, "No changes are persisted. Exiting."); cli::safe_exit(1) }); - add_key_or_address(ctx, io, alias, alias_force, value, unsafe_dont_encrypt) + add_key_or_address( + ctx, + io, + alias, + alias_force, + birthday, + value, + unsafe_dont_encrypt, + ) } /// Remove keys and addresses @@ -1293,6 +1315,7 @@ fn key_import( io, alias, alias_force, + None, masp_value, unsafe_dont_encrypt, ); diff --git a/crates/apps_lib/src/client/masp.rs b/crates/apps_lib/src/client/masp.rs index 29e30ee246..f357835d60 100644 --- a/crates/apps_lib/src/client/masp.rs +++ b/crates/apps_lib/src/client/masp.rs @@ -4,7 +4,6 @@ use std::time::Duration; use color_eyre::owo_colors::OwoColorize; use masp_primitives::sapling::ViewingKey; -use masp_primitives::zip32::ExtendedSpendingKey; use namada_sdk::error::Error; use namada_sdk::io::Io; use namada_sdk::masp::utils::{ @@ -14,6 +13,7 @@ use namada_sdk::masp::utils::{ use namada_sdk::masp::{IndexedNoteEntry, ShieldedContext, ShieldedUtils}; use namada_sdk::queries::Client; use namada_sdk::storage::BlockHeight; +use namada_sdk::wallet::{DatedKeypair, DatedSpendingKey}; use namada_sdk::{display, display_line, MaybeSend, MaybeSync}; #[allow(clippy::too_many_arguments)] @@ -28,8 +28,8 @@ pub async fn syncing< io: &IO, start_query_height: Option, last_query_height: Option, - sks: &[ExtendedSpendingKey], - fvks: &[ViewingKey], + sks: &[DatedSpendingKey], + fvks: &[DatedKeypair], ) -> Result, Error> { if indexer_addr.is_some() { display_line!( diff --git a/crates/benches/native_vps.rs b/crates/benches/native_vps.rs index b8b206e582..2410f5b446 100644 --- a/crates/benches/native_vps.rs +++ b/crates/benches/native_vps.rs @@ -401,7 +401,7 @@ fn prepare_ibc_tx_and_ctx(bench_name: &str) -> (BenchShieldedCtx, BatchedTx) { shielded_ctx.shell.commit_block(); shielded_ctx.generate_shielded_action( Amount::native_whole(10), - TransferSource::ExtendedSpendingKey(albert_spending_key), + TransferSource::ExtendedSpendingKey(albert_spending_key.key), defaults::bertha_address().to_string(), ) } @@ -579,12 +579,12 @@ fn setup_storage_for_masp_verification( ), "unshielding" => shielded_ctx.generate_masp_tx( amount, - TransferSource::ExtendedSpendingKey(albert_spending_key), + TransferSource::ExtendedSpendingKey(albert_spending_key.key), TransferTarget::Address(defaults::albert_address()), ), "shielded" => shielded_ctx.generate_masp_tx( amount, - TransferSource::ExtendedSpendingKey(albert_spending_key), + TransferSource::ExtendedSpendingKey(albert_spending_key.key), TransferTarget::PaymentAddress(bertha_payment_addr), ), _ => panic!("Unexpected bench test"), diff --git a/crates/core/src/masp.rs b/crates/core/src/masp.rs index a3b6736b05..b814532091 100644 --- a/crates/core/src/masp.rs +++ b/crates/core/src/masp.rs @@ -8,6 +8,7 @@ use std::str::FromStr; use borsh::{BorshDeserialize, BorshSchema, BorshSerialize}; use borsh_ext::BorshSerializeExt; use masp_primitives::asset_type::AssetType; +use masp_primitives::sapling::ViewingKey; use masp_primitives::transaction::TransparentAddress; use namada_macros::BorshDeserializer; #[cfg(feature = "migrations")] @@ -257,6 +258,11 @@ impl ExtendedViewingKey { masp_primitives::zip32::ExtendedFullViewingKey::read(&mut &bytes[..]) .map(Self) } + + /// Get the underlying viewing key + pub fn as_viewing_key(&self) -> ViewingKey { + self.0.fvk.vk + } } impl string_encoding::Format for ExtendedViewingKey { diff --git a/crates/node/src/bench_utils.rs b/crates/node/src/bench_utils.rs index c7a8dfdd93..0025c6ba35 100644 --- a/crates/node/src/bench_utils.rs +++ b/crates/node/src/bench_utils.rs @@ -1017,12 +1017,14 @@ impl Default for BenchShieldedCtx { chain_ctx.wallet.gen_store_spending_key( ALBERT_SPENDING_KEY.to_string(), None, + None, true, &mut OsRng, ); chain_ctx.wallet.gen_store_spending_key( BERTHA_SPENDING_KEY.to_string(), None, + None, true, &mut OsRng, ); @@ -1040,6 +1042,7 @@ impl Default for BenchShieldedCtx { .wallet .find_viewing_key(viewing_alias) .unwrap() + .key .to_string(), ); let viewing_key = ExtendedFullViewingKey::from( @@ -1091,7 +1094,7 @@ impl BenchShieldedCtx { &StdIo, None, None, - &[spending_key.into()], + &[spending_key.key.into()], &[], )) .unwrap(); diff --git a/crates/sdk/src/args.rs b/crates/sdk/src/args.rs index 3a90d7fbba..55473603e0 100644 --- a/crates/sdk/src/args.rs +++ b/crates/sdk/src/args.rs @@ -28,6 +28,7 @@ use zeroize::Zeroizing; use crate::eth_bridge::bridge_pool; use crate::ibc::core::host::types::identifiers::{ChannelId, PortId}; use crate::signing::SigningTxData; +use crate::wallet::{DatedSpendingKey, DatedViewingKey}; use crate::{rpc, tx, Namada}; /// [`Duration`](StdDuration) wrapper that provides a @@ -66,6 +67,10 @@ pub trait NamadaTypes: Clone + std::fmt::Debug { type ViewingKey: Clone + std::fmt::Debug; /// Represents a shielded spending key type SpendingKey: Clone + std::fmt::Debug; + /// Represents a shielded viewing key + type DatedViewingKey: Clone + std::fmt::Debug; + /// Represents a shielded spending key + type DatedSpendingKey: Clone + std::fmt::Debug; /// Represents a shielded payment address type PaymentAddress: Clone + std::fmt::Debug; /// Represents the owner of a balance @@ -106,6 +111,8 @@ impl NamadaTypes for SdkTypes { type BpConversionTable = HashMap; type ConfigRpcTendermintAddress = tendermint_rpc::Url; type Data = Vec; + type DatedSpendingKey = DatedSpendingKey; + type DatedViewingKey = DatedViewingKey; type EthereumAddress = (); type Keypair = namada_core::key::common::SecretKey; type MaspIndexerAddress = (); @@ -2125,9 +2132,9 @@ pub struct ShieldedSync { /// Height to sync up to. Defaults to most recent pub last_query_height: Option, /// Spending keys used to determine note ownership - pub spending_keys: Vec, + pub spending_keys: Vec, /// Viewing keys used to determine note ownership - pub viewing_keys: Vec, + pub viewing_keys: Vec, /// Address of a `namada-masp-indexer` live instance /// /// If present, the shielded sync will be performed @@ -2474,6 +2481,9 @@ pub struct KeyGen { pub prompt_bip39_passphrase: bool, /// Allow non-compliant derivation path pub allow_non_compliant: bool, + /// Optional block height after which this key was created. + /// Only used for MASP keys. + pub birthday: Option, } /// Wallet restore key and implicit address arguments @@ -2499,6 +2509,9 @@ pub struct KeyDerive { pub use_device: bool, /// Hardware Wallet transport - HID (USB) or TCP pub device_transport: DeviceTransport, + /// Optional blockheight after which this key was created. + /// Only used for MASP keys + pub birthday: Option, } /// Wallet list arguments @@ -2576,6 +2589,9 @@ pub struct KeyAddressAdd { pub alias_force: bool, /// Any supported value pub value: String, + /// Optional block height after which this key was created. + /// Only used for MASP keys. + pub birthday: Option, /// Don't encrypt the key pub unsafe_dont_encrypt: bool, } diff --git a/crates/sdk/src/masp.rs b/crates/sdk/src/masp.rs index ddedee83d4..8da5b6fd67 100644 --- a/crates/sdk/src/masp.rs +++ b/crates/sdk/src/masp.rs @@ -78,6 +78,7 @@ use crate::masp::utils::{ }; use crate::queries::Client; use crate::rpc::{query_conversion, query_denom}; +use crate::wallet::{DatedKeypair, DatedSpendingKey}; use crate::{ control_flow, display_line, edisplay_line, query_native_token, rpc, MaybeSend, MaybeSync, Namada, @@ -602,8 +603,8 @@ impl ShieldedContext { start_query_height: Option, last_query_height: Option, retry: RetryStrategy, - sks: &[MaspExtendedSpendingKey], - fvks: &[ViewingKey], + sks: &[DatedSpendingKey], + fvks: &[DatedKeypair], ) -> Result<(), Error> where IO: Io, @@ -698,8 +699,8 @@ impl ShieldedContext { start_query_height: Option, last_query_height: Option, retry: RetryStrategy, - sks: &[MaspExtendedSpendingKey], - fvks: &[ViewingKey], + sks: &[DatedSpendingKey], + fvks: &[DatedKeypair], mut shutdown_signal: ShutdownSignal, ) -> Result<(), Error> where @@ -728,12 +729,32 @@ impl ShieldedContext { ..Default::default() }; } - for esk in sks { - let vk = to_viewing_key(esk).vk; - self.vk_heights.entry(vk).or_default(); - } - for vk in fvks { - self.vk_heights.entry(*vk).or_default(); + for vk in sks + .iter() + .map(|esk| { + esk.map(|k| { + to_viewing_key(&MaspExtendedSpendingKey::from(k)).vk + }) + }) + .chain(fvks.iter().copied()) + { + if let Some(h) = self.vk_heights.entry(vk.key).or_default() { + let birthday = IndexedTx { + height: vk.birthday, + index: Default::default(), + }; + if birthday > *h { + *h = birthday; + } + } else { + self.vk_heights.insert( + vk.key, + Some(IndexedTx { + height: vk.birthday, + index: Default::default(), + }), + ); + } } // Save the context to persist newly added keys @@ -2393,7 +2414,7 @@ impl ShieldedContext { .await .get_viewing_keys() .values() - .map(|evk| ExtendedFullViewingKey::from(*evk).fvk.vk) + .map(|evk| ExtendedFullViewingKey::from(evk.key).fvk.vk) .collect(); let last_witnessed_tx = self.tx_note_map.keys().max(); // This data will be discarded at the next fetch so we don't need to @@ -3567,6 +3588,7 @@ mod test_shielded_sync { test_client, TestUnscannedTracker, TestingMaspClient, }; use crate::masp::utils::{DefaultTracker, ProgressTracker, RetryStrategy}; + use crate::wallet::{DatedKeypair, DatedSpendingKey, StoredKeypair}; // A viewing key derived from A_SPENDING_KEY pub const AA_VIEWING_KEY: &str = "zvknam1qqqqqqqqqqqqqq9v0sls5r5de7njx8ehu49pqgmqr9ygelg87l5x8y4s9r0pjlvu6x74w9gjpw856zcu826qesdre628y6tjc26uhgj6d9zqur9l5u3p99d9ggc74ald6s8y3sdtka74qmheyqvdrasqpwyv2fsmxlz57lj4grm2pthzj3sflxc0jx0edrakx3vdcngrfjmru8ywkguru8mxss2uuqxdlglaz6undx5h8w7g70t2es850g48xzdkqay5qs0yw06rtxcpjdve6"; @@ -3707,7 +3729,7 @@ mod test_shielded_sync { None, RetryStrategy::Times(1), &[], - &[vk], + &[vk.into()], ) .await .unwrap_err(); @@ -3751,7 +3773,7 @@ mod test_shielded_sync { None, RetryStrategy::Times(2), &[], - &[vk], + &[vk.into()], ) .await .expect("Test failed"); @@ -3784,6 +3806,116 @@ mod test_shielded_sync { assert_eq!(shielded_ctx.note_map.len(), 2); } + /// Test the the birthdays of keys are properly reflected in the key + /// sync heights when starting shielded sync. + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn test_key_birthdays() { + let temp_dir = tempdir().unwrap(); + let mut shielded_ctx = + FsShieldedUtils::new(temp_dir.path().to_path_buf()); + let (client, masp_tx_sender) = test_client(2.into()); + let io = StdIo; + let progress = DefaultTracker::new(&io); + // First test the case where no keys have been seen yet + let mut vk = DatedKeypair::new( + ExtendedFullViewingKey::from( + ExtendedViewingKey::from_str(AA_VIEWING_KEY) + .expect("Test failed"), + ) + .fvk + .vk, + Some(10.into()), + ); + let StoredKeypair::Raw(mut sk) = serde_json::from_str::<'_, StoredKeypair::>(r#""unencrypted:zsknam1q02rgh4mqqqqpqqm68m2lmd0xe9k5vf4fscmdxuvewqhdhwl0h492fj40tzl5f6gwfk6kgnaxpgct7mx9cw2he4724858jdfhrzdh3e4hu3us463gphqyl6k5hvkjwkv9r7rx3jtcueurgflgj6dx9qn4rg0caf0t9zawfcdwt3ramxlrs4jyan4wyp4nh9hj8s806ru0smk3437ejy56ewtw9ljz8rc3vkyznxdf3l5c70skcw6aatpv5de9zhxuxs5k6l6jz6zktgg0udvl<<30""#).expect("Test failed") else { + panic!("Test failed") + }; + + masp_tx_sender.send(None).expect("Test failed"); + let result = shielded_ctx + .fetch( + TestingMaspClient::new(&client), + &progress, + None, + None, + RetryStrategy::Times(1), + &[sk], + &[vk], + ) + .await + .unwrap_err(); + match result { + Error::Other(msg) => assert_eq!( + msg.as_str(), + "After retrying, could not fetch all MASP txs." + ), + other => panic!("{:?} does not match Error::Other(_)", other), + } + shielded_ctx.load_confirmed().await.expect("Test failed"); + let birthdays = shielded_ctx + .vk_heights + .values() + .cloned() + .collect::>(); + assert_eq!( + birthdays, + vec![ + Some(IndexedTx { + height: BlockHeight(30), + index: Default::default() + }), + Some(IndexedTx { + height: BlockHeight(10), + index: Default::default() + }) + ] + ); + + // Test two cases: + // * A birthday is less than the synced height of key + // * A birthday is greater than the synced height of key + vk.birthday = 5.into(); + sk.birthday = 60.into(); + masp_tx_sender.send(None).expect("Test failed"); + let result = shielded_ctx + .fetch( + TestingMaspClient::new(&client), + &progress, + None, + None, + RetryStrategy::Times(1), + &[sk], + &[vk], + ) + .await + .unwrap_err(); + match result { + Error::Other(msg) => assert_eq!( + msg.as_str(), + "After retrying, could not fetch all MASP txs." + ), + other => panic!("{:?} does not match Error::Other(_)", other), + } + shielded_ctx.load_confirmed().await.expect("Test failed"); + let birthdays = shielded_ctx + .vk_heights + .values() + .cloned() + .collect::>(); + assert_eq!( + birthdays, + vec![ + Some(IndexedTx { + height: BlockHeight(60), + index: Default::default() + }), + Some(IndexedTx { + height: BlockHeight(10), + index: Default::default() + }) + ] + ) + } + /// Test that upon each retry, we either resume from the /// latest height that had been previously stored in the /// `tx_note_map`, or from the minimum height stored in @@ -3868,7 +4000,7 @@ mod test_shielded_sync { None, RetryStrategy::Times(1), &[], - &[vk], + &[vk.into()], ) .await .unwrap_err(); @@ -3893,7 +4025,7 @@ mod test_shielded_sync { None, RetryStrategy::Times(1), &[], - &[vk], + &[vk.into()], ) .await .unwrap_err(); @@ -3909,7 +4041,7 @@ mod test_shielded_sync { None, RetryStrategy::Times(1), &[], - &[vk], + &[vk.into()], ) .await .unwrap_err(); @@ -3927,7 +4059,7 @@ mod test_shielded_sync { None, RetryStrategy::Times(1), &[], - &[vk], + &[vk.into()], ) .await .unwrap_err(); @@ -3963,7 +4095,7 @@ mod test_shielded_sync { None, RetryStrategy::Times(1), &[], - &[vk], + &[vk.into()], ) .await .expect("Test failed"); @@ -4016,7 +4148,7 @@ mod test_shielded_sync { None, RetryStrategy::Times(2), &[], - &[vk], + &[vk.into()], ) .await .expect("Test failed"); @@ -4075,7 +4207,7 @@ mod test_shielded_sync { None, RetryStrategy::Forever, &[], - &[vk], + &[vk.into()], shutdown_signal, ) .await diff --git a/crates/sdk/src/wallet/keys.rs b/crates/sdk/src/wallet/keys.rs index 7c94df49d6..faef0f9b1c 100644 --- a/crates/sdk/src/wallet/keys.rs +++ b/crates/sdk/src/wallet/keys.rs @@ -7,6 +7,8 @@ use std::str::FromStr; use borsh::{BorshDeserialize, BorshSerialize}; use borsh_ext::BorshSerializeExt; use data_encoding::HEXLOWER; +use namada_core::masp::{ExtendedSpendingKey, ExtendedViewingKey}; +use namada_core::storage::BlockHeight; use orion::{aead, kdf}; use serde::{Deserialize, Serialize}; use thiserror::Error; @@ -17,6 +19,11 @@ use crate::wallet::WalletIo; const ENCRYPTED_KEY_PREFIX: &str = "encrypted:"; const UNENCRYPTED_KEY_PREFIX: &str = "unencrypted:"; +/// Type alias for a viewing key with a birthday. +pub type DatedViewingKey = DatedKeypair; +/// Type alias for a spending key with a birthday. +pub type DatedSpendingKey = DatedKeypair; + /// A keypair stored in a wallet #[derive(Debug)] pub enum StoredKeypair @@ -113,6 +120,121 @@ pub enum DeserializeStoredKeypairError { MissingPrefix, } +#[allow(missing_docs)] +#[derive(Error, Debug)] +pub enum DeserializeDatedKeypairError { + #[error("The stored keypair is not valid: {0}")] + InvalidKeypairString(String), + #[error("The stored keypair contains an invalid birthday: {0}")] + InvalidBirthday(String), +} + +/// A keypair with a block height after which it was created +#[derive(Debug, Serialize, Deserialize, BorshSerialize, BorshDeserialize)] +pub struct DatedKeypair +where + T: BorshSerialize + BorshDeserialize, +{ + /// The keypair itself + pub key: T, + /// A blockheight that precedes the creation of the keypair + pub birthday: BlockHeight, +} + +impl Copy for DatedKeypair where + T: Copy + BorshSerialize + BorshDeserialize +{ +} + +impl Clone for DatedKeypair +where + T: Clone + BorshSerialize + BorshDeserialize, +{ + fn clone(&self) -> Self { + Self { + key: self.key.clone(), + birthday: self.birthday, + } + } +} + +impl DatedKeypair +where + T: BorshSerialize + BorshDeserialize, +{ + /// Create a new dated keypair. If no birthday is provided, + /// defaults to the first blockheight. + pub fn new(key: T, birthday: Option) -> Self { + Self { + key, + birthday: birthday.unwrap_or_else(BlockHeight::first), + } + } + + /// Map the inner key type while maintaining the birthday. + pub fn map(self, func: F) -> DatedKeypair + where + F: Fn(T) -> U, + U: BorshSerialize + BorshDeserialize, + { + DatedKeypair { + key: func(self.key), + birthday: self.birthday, + } + } +} + +impl From for DatedKeypair +where + T: BorshSerialize + BorshDeserialize, +{ + fn from(key: T) -> Self { + Self::new(key, None) + } +} + +impl Display for DatedKeypair +where + T: BorshSerialize + BorshDeserialize + Display, +{ + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{}<<{}", self.key, self.birthday,) + } +} + +impl FromStr for DatedKeypair +where + T: Serialize + BorshSerialize + BorshDeserialize + FromStr, + ::Err: Display, +{ + type Err = DeserializeDatedKeypairError; + + fn from_str(s: &str) -> Result { + let mut pieces = s.split("<<"); + let key_ser = pieces.next().ok_or( + DeserializeDatedKeypairError::InvalidKeypairString( + "Provided string was empty".to_string(), + ), + )?; + let birthday = pieces + .next() + .map(|b| { + BlockHeight::from_str(b).map_err(|_| { + DeserializeDatedKeypairError::InvalidBirthday(b.to_string()) + }) + }) + .transpose()?; + Ok(Self::new( + T::from_str(key_ser).map_err(|e| { + DeserializeDatedKeypairError::InvalidKeypairString( + e.to_string(), + ) + })?, + birthday, + )) + } +} + /// An encrypted keypair stored in a wallet #[derive(Debug)] pub struct EncryptedKeypair( diff --git a/crates/sdk/src/wallet/mod.rs b/crates/sdk/src/wallet/mod.rs index 3719b97663..a54930878e 100644 --- a/crates/sdk/src/wallet/mod.rs +++ b/crates/sdk/src/wallet/mod.rs @@ -21,6 +21,7 @@ use namada_core::key::*; use namada_core::masp::{ ExtendedSpendingKey, ExtendedViewingKey, PaymentAddress, }; +use namada_core::storage::BlockHeight; use namada_core::time::DateTimeUtc; use namada_ibc::trace::is_ibc_denom; pub use pre_genesis::gen_key_to_store; @@ -31,7 +32,10 @@ use thiserror::Error; use zeroize::Zeroizing; pub use self::derivation_path::{DerivationPath, DerivationPathError}; -pub use self::keys::{DecryptionError, StoredKeypair}; +pub use self::keys::{ + DatedKeypair, DatedSpendingKey, DatedViewingKey, DecryptionError, + StoredKeypair, +}; pub use self::store::{ConfirmationResponse, ValidatorData, ValidatorKeys}; use crate::wallet::store::{derive_hd_secret_key, derive_hd_spending_key}; @@ -257,7 +261,7 @@ pub struct Wallet { utils: U, store: Store, decrypted_key_cache: HashMap, - decrypted_spendkey_cache: HashMap, + decrypted_spendkey_cache: HashMap, } impl From> for Store { @@ -419,7 +423,7 @@ impl Wallet { pub fn find_viewing_key( &self, alias: impl AsRef, - ) -> Result<&ExtendedViewingKey, FindKeyError> { + ) -> Result<&DatedViewingKey, FindKeyError> { self.store.find_viewing_key(alias.as_ref()).ok_or_else(|| { FindKeyError::KeyNotFound(alias.as_ref().to_string()) }) @@ -484,7 +488,7 @@ impl Wallet { } /// Get all known viewing keys by their alias - pub fn get_viewing_keys(&self) -> HashMap { + pub fn get_viewing_keys(&self) -> HashMap { self.store .get_viewing_keys() .iter() @@ -495,7 +499,7 @@ impl Wallet { /// Get all known viewing keys by their alias pub fn get_spending_keys( &self, - ) -> HashMap> { + ) -> HashMap> { self.store .get_spending_keys() .iter() @@ -544,10 +548,12 @@ impl Wallet { /// provided, will prompt for password from stdin. /// Stores the key in decrypted key cache and returns the alias of the key /// and a reference-counting pointer to the key. + #[allow(clippy::too_many_arguments)] pub fn derive_store_spending_key_from_mnemonic_code( &mut self, alias: String, alias_force: bool, + birthday: Option, derivation_path: DerivationPath, mnemonic_passphrase: Option<(Mnemonic, Zeroizing)>, prompt_bip39_passphrase: bool, @@ -573,6 +579,7 @@ impl Wallet { alias, alias_force, spend_key, + birthday, password, Some(derivation_path), ) @@ -633,13 +640,21 @@ impl Wallet { pub fn gen_store_spending_key( &mut self, alias: String, + birthday: Option, password: Option>, force_alias: bool, csprng: &mut (impl CryptoRng + RngCore), ) -> Option<(String, ExtendedSpendingKey)> { let spend_key = gen_spending_key(csprng); - self.insert_spending_key(alias, force_alias, spend_key, password, None) - .map(|alias| (alias, spend_key)) + self.insert_spending_key( + alias, + force_alias, + spend_key, + birthday, + password, + None, + ) + .map(|alias| (alias, spend_key)) } /// Generate a new keypair, derive an implicit address from its public key @@ -736,14 +751,20 @@ impl Wallet { /// into the store with the provided alias, converted to lower case. If the /// alias already exists, optionally force overwrite the key for the /// alias. + /// + /// An optional birthday can be provided saying that this key was created + /// after this blockheight. + /// /// If no encryption password is provided, the key will be stored raw /// without encryption. + /// /// Stores the key in decrypted key cache and returns the alias of the key /// and the key itself. - pub fn derive_store_hd_spendind_key( + pub fn derive_store_hd_spending_key( &mut self, alias: String, force_alias: bool, + birthday: Option, seed: Seed, derivation_path: DerivationPath, password: Option>, @@ -754,6 +775,7 @@ impl Wallet { alias, force_alias, spend_key, + birthday, password, Some(derivation_path), ) @@ -886,7 +908,7 @@ impl Wallet { &mut self, alias: impl AsRef, password: Option>, - ) -> Result { + ) -> Result { // Try cache first if let Some(cached_key) = self .decrypted_spendkey_cache @@ -1082,10 +1104,16 @@ impl Wallet { &mut self, alias: String, view_key: ExtendedViewingKey, + birthday: Option, force_alias: bool, ) -> Option { self.store - .insert_viewing_key::(alias.into(), view_key, force_alias) + .insert_viewing_key::( + alias.into(), + view_key, + birthday, + force_alias, + ) .map(Into::into) } @@ -1095,6 +1123,7 @@ impl Wallet { alias: String, force_alias: bool, spend_key: ExtendedSpendingKey, + birthday: Option, password: Option>, path: Option, ) -> Option { @@ -1102,14 +1131,17 @@ impl Wallet { .insert_spending_key::( alias.into(), spend_key, + birthday, password, path, force_alias, ) .map(|alias| { // Cache the newly added key - self.decrypted_spendkey_cache - .insert(alias.clone(), spend_key); + self.decrypted_spendkey_cache.insert( + alias.clone(), + DatedKeypair::new(spend_key, birthday), + ); alias }) .map(Into::into) diff --git a/crates/sdk/src/wallet/store.rs b/crates/sdk/src/wallet/store.rs index c7c8edc01b..bf071b13df 100644 --- a/crates/sdk/src/wallet/store.rs +++ b/crates/sdk/src/wallet/store.rs @@ -15,12 +15,14 @@ use namada_core::key::*; use namada_core::masp::{ ExtendedSpendingKey, ExtendedViewingKey, PaymentAddress, }; +use namada_core::storage::BlockHeight; use serde::{Deserialize, Serialize}; use zeroize::Zeroizing; use super::alias::{self, Alias}; use super::derivation_path::DerivationPath; use super::pre_genesis; +use crate::wallet::keys::{DatedKeypair, DatedSpendingKey, DatedViewingKey}; use crate::wallet::{StoredKeypair, WalletIo}; /// Actions that can be taken when there is an alias conflict @@ -62,9 +64,9 @@ pub struct ValidatorData { #[derive(Serialize, Deserialize, Debug, Default)] pub struct Store { /// Known viewing keys - view_keys: BTreeMap, + view_keys: BTreeMap, /// Known spending keys - spend_keys: BTreeMap>, + spend_keys: BTreeMap>, /// Payment address book payment_addrs: BiBTreeMap, /// Cryptographic keypairs @@ -133,7 +135,7 @@ impl Store { pub fn find_spending_key( &self, alias: impl AsRef, - ) -> Option<&StoredKeypair> { + ) -> Option<&StoredKeypair> { self.spend_keys.get(&alias.into()) } @@ -141,7 +143,7 @@ impl Store { pub fn find_viewing_key( &self, alias: impl AsRef, - ) -> Option<&ExtendedViewingKey> { + ) -> Option<&DatedViewingKey> { self.view_keys.get(&alias.into()) } @@ -252,14 +254,14 @@ impl Store { } /// Get all known viewing keys by their alias. - pub fn get_viewing_keys(&self) -> &BTreeMap { + pub fn get_viewing_keys(&self) -> &BTreeMap { &self.view_keys } /// Get all known spending keys by their alias. pub fn get_spending_keys( &self, - ) -> &BTreeMap> { + ) -> &BTreeMap> { &self.spend_keys } @@ -368,6 +370,7 @@ impl Store { &mut self, alias: Alias, spendkey: ExtendedSpendingKey, + birthday: Option, password: Option>, path: Option, force: bool, @@ -388,19 +391,22 @@ impl Store { ConfirmationResponse::Replace => {} ConfirmationResponse::Reselect(new_alias) => { return self.insert_spending_key::( - new_alias, spendkey, password, path, false, + new_alias, spendkey, birthday, password, path, false, ); } ConfirmationResponse::Skip => return None, } } self.remove_alias(&alias); + let (spendkey_to_store, _raw_spendkey) = - StoredKeypair::new(spendkey, password); + StoredKeypair::new(DatedKeypair::new(spendkey, birthday), password); self.spend_keys.insert(alias.clone(), spendkey_to_store); // Simultaneously add the derived viewing key to ease balance viewing - let viewkey = - zip32::ExtendedFullViewingKey::from(&spendkey.into()).into(); + let viewkey = DatedKeypair::new( + zip32::ExtendedFullViewingKey::from(&spendkey.into()).into(), + birthday, + ); self.view_keys.insert(alias.clone(), viewkey); path.map(|p| self.derivation_paths.insert(alias.clone(), p)); Some(alias) @@ -411,6 +417,7 @@ impl Store { &mut self, alias: Alias, viewkey: ExtendedViewingKey, + birthday: Option, force: bool, ) -> Option { // abort if the alias is reserved @@ -427,14 +434,16 @@ impl Store { match U::show_overwrite_confirmation(&alias, "a viewing key") { ConfirmationResponse::Replace => {} ConfirmationResponse::Reselect(new_alias) => { - return self - .insert_viewing_key::(new_alias, viewkey, false); + return self.insert_viewing_key::( + new_alias, viewkey, birthday, false, + ); } ConfirmationResponse::Skip => return None, } } self.remove_alias(&alias); - self.view_keys.insert(alias.clone(), viewkey); + self.view_keys + .insert(alias.clone(), DatedKeypair::new(viewkey, birthday)); Some(alias) }