diff --git a/.changelog/unreleased/bug-fixes/4036-validate-validator-metadata.md b/.changelog/unreleased/bug-fixes/4036-validate-validator-metadata.md new file mode 100644 index 0000000000..f6afd72cb9 --- /dev/null +++ b/.changelog/unreleased/bug-fixes/4036-validate-validator-metadata.md @@ -0,0 +1,2 @@ +- Validate validator metadata from on-chain validator creation and metadata + changes. ([\#4036](https://github.com/anoma/namada/pull/4036)) \ No newline at end of file diff --git a/.changelog/unreleased/improvements/4019-improve-speculative-shielded-ctx.md b/.changelog/unreleased/improvements/4019-improve-speculative-shielded-ctx.md new file mode 100644 index 0000000000..d2171def2b --- /dev/null +++ b/.changelog/unreleased/improvements/4019-improve-speculative-shielded-ctx.md @@ -0,0 +1,3 @@ +- The speculative shielded context now avoids updating its + state if the transaction failed. Added a test for it. + ([\#4019](https://github.com/anoma/namada/pull/4019)) \ No newline at end of file diff --git a/crates/apps_lib/src/client/rpc.rs b/crates/apps_lib/src/client/rpc.rs index 91890192b1..17087858f5 100644 --- a/crates/apps_lib/src/client/rpc.rs +++ b/crates/apps_lib/src/client/rpc.rs @@ -4,6 +4,7 @@ use std::collections::{BTreeMap, BTreeSet}; use std::io; use borsh::BorshDeserialize; +use color_eyre::owo_colors::OwoColorize; use data_encoding::HEXLOWER; use either::Either; use masp_primitives::asset_type::AssetType; @@ -395,6 +396,15 @@ async fn query_shielded_balance( context: &impl Namada, args: args::QueryBalance, ) { + display_line!( + context.io(), + "{}: {}\n", + "WARNING".bold().underline().yellow(), + "The resulting balance could be outdated, make sure to run `namadac \ + shielded-sync` before querying the balance to get the most recent \ + value." + ); + let args::QueryBalance { // Token owner (needs to be a viewing key) owner, diff --git a/crates/apps_lib/src/client/tx.rs b/crates/apps_lib/src/client/tx.rs index 770f7886b8..01a8097c8a 100644 --- a/crates/apps_lib/src/client/tx.rs +++ b/crates/apps_lib/src/client/tx.rs @@ -3,7 +3,9 @@ use std::io::Write; use borsh::BorshDeserialize; use borsh_ext::BorshSerializeExt; +use color_eyre::owo_colors::OwoColorize; use ledger_namada_rs::{BIP44Path, NamadaApp}; +use namada_core::masp::MaspTransaction; use namada_sdk::address::{Address, ImplicitAddress}; use namada_sdk::args::TxBecomeValidator; use namada_sdk::collections::HashSet; @@ -637,11 +639,10 @@ pub async fn submit_become_validator( .is_applied_and_valid(wrapper_hash.as_ref(), &cmt) .is_none() { - display_line!( - namada.io(), + return Err(error::Error::Tx(error::TxSubmitError::Other( "Transaction failed. No key or addresses have been saved." - ); - safe_exit(1) + .to_string(), + ))); } // add validator address and keys to the wallet @@ -829,13 +830,33 @@ pub async fn submit_shielded_transfer( namada: &impl Namada, args: args::TxShieldedTransfer, ) -> Result<(), error::Error> { + display_line!( + namada.io(), + "{}: {}\n", + "WARNING".bold().underline().yellow(), + "Some information might be leaked if your shielded wallet is not up \ + to date, make sure to run `namadac shielded-sync` before running \ + this command.", + ); + let (mut tx, signing_data) = args.clone().build(namada).await?; + let masp_section = tx + .sections + .iter() + .find_map(|section| section.masp_tx()) + .ok_or_else(|| { + error::Error::Other( + "Missing MASP section in shielded transaction".to_string(), + ) + })?; if args.tx.dump_tx || args.tx.dump_wrapper_tx { tx::dump_tx(namada.io(), &args.tx, tx)?; + pre_cache_masp_data(namada, &masp_section).await; } else { sign(namada, &mut tx, &args.tx, signing_data).await?; - namada.submit(tx, &args.tx).await?; + let res = namada.submit(tx, &args.tx).await?; + pre_cache_masp_data_on_tx_result(namada, &res, &masp_section).await; } Ok(()) } @@ -900,13 +921,33 @@ pub async fn submit_unshielding_transfer( namada: &impl Namada, args: args::TxUnshieldingTransfer, ) -> Result<(), error::Error> { + display_line!( + namada.io(), + "{}: {}\n", + "WARNING".bold().underline().yellow(), + "Some information might be leaked if your shielded wallet is not up \ + to date, make sure to run `namadac shielded-sync` before running \ + this command.", + ); + let (mut tx, signing_data) = args.clone().build(namada).await?; + let masp_section = tx + .sections + .iter() + .find_map(|section| section.masp_tx()) + .ok_or_else(|| { + error::Error::Other( + "Missing MASP section in shielded transaction".to_string(), + ) + })?; if args.tx.dump_tx || args.tx.dump_wrapper_tx { tx::dump_tx(namada.io(), &args.tx, tx)?; + pre_cache_masp_data(namada, &masp_section).await; } else { sign(namada, &mut tx, &args.tx, signing_data).await?; - namada.submit(tx, &args.tx).await?; + let res = namada.submit(tx, &args.tx).await?; + pre_cache_masp_data_on_tx_result(namada, &res, &masp_section).await; } Ok(()) } @@ -920,16 +961,25 @@ where { let (tx, signing_data, _) = args.build(namada).await?; + let opt_masp_section = + tx.sections.iter().find_map(|section| section.masp_tx()); if args.tx.dump_tx || args.tx.dump_wrapper_tx { tx::dump_tx(namada.io(), &args.tx, tx)?; + if let Some(masp_section) = opt_masp_section { + pre_cache_masp_data(namada, &masp_section).await; + } } else { - batch_opt_reveal_pk_and_submit( + let res = batch_opt_reveal_pk_and_submit( namada, &args.tx, &[&args.source.effective_address()], (tx, signing_data), ) .await?; + + if let Some(masp_section) = opt_masp_section { + pre_cache_masp_data_on_tx_result(namada, &res, &masp_section).await; + } } // NOTE that the tx could fail when its submission epoch doesn't match // construction epoch @@ -1482,3 +1532,42 @@ pub async fn gen_ibc_shielding_transfer( } Ok(()) } + +// Pre-cache the data for the provided MASP transaction. Log an error on +// failure. +async fn pre_cache_masp_data(namada: &impl Namada, masp_tx: &MaspTransaction) { + if let Err(e) = namada + .shielded_mut() + .await + .pre_cache_transaction(masp_tx) + .await + { + // Just display the error but do not propagate it + edisplay_line!(namada.io(), "Failed to pre-cache masp data: {}.", e); + } +} + +// Check the result of a transaction and pre-cache the masp data accordingly +async fn pre_cache_masp_data_on_tx_result( + namada: &impl Namada, + tx_result: &ProcessTxResponse, + masp_tx: &MaspTransaction, +) { + match tx_result { + ProcessTxResponse::Applied(resp) => { + if let Some(InnerTxResult::Success(_)) = + // If we have the masp data in an ibc transfer it + // means we are unshielding, so there's no reveal pk + // tx in the batch which contains only the ibc tx + resp.batch_result().first().map(|(_, res)| res) + { + pre_cache_masp_data(namada, masp_tx).await; + } + } + ProcessTxResponse::Broadcast(_) => { + pre_cache_masp_data(namada, masp_tx).await; + } + // Do not pre-cache when dry-running + ProcessTxResponse::DryRun(_) => {} + } +} diff --git a/crates/apps_lib/src/config/genesis/transactions.rs b/crates/apps_lib/src/config/genesis/transactions.rs index a359554067..2873488b1d 100644 --- a/crates/apps_lib/src/config/genesis/transactions.rs +++ b/crates/apps_lib/src/config/genesis/transactions.rs @@ -20,7 +20,6 @@ use namada_sdk::collections::HashSet; use namada_sdk::dec::Dec; use namada_sdk::key::common::PublicKey; use namada_sdk::key::{common, ed25519, RefTo, SerializeWithBorsh, SigScheme}; -use namada_sdk::proof_of_stake::parameters::MAX_VALIDATOR_METADATA_LEN; use namada_sdk::proof_of_stake::types::ValidatorMetaData; use namada_sdk::signing::{sign_tx, SigningTxData}; use namada_sdk::string_encoding::StringEncoded; @@ -1385,60 +1384,14 @@ pub fn validate_validator_account( // Check that the validator metadata is not too large let metadata = &signed_tx.data.metadata; - if metadata.email.len() as u64 > MAX_VALIDATOR_METADATA_LEN { + let errors = metadata.validate(); + if !errors.is_empty() { panic!( - "The email metadata of the validator with address {} is too long, \ - must be within {MAX_VALIDATOR_METADATA_LEN} characters", + "Metadata of the validator with address {} are invalid: \ + {errors:#?}", signed_tx.data.address ); } - if let Some(description) = metadata.description.as_ref() { - if description.len() as u64 > MAX_VALIDATOR_METADATA_LEN { - panic!( - "The description metadata of the validator with address {} is \ - too long, must be within {MAX_VALIDATOR_METADATA_LEN} \ - characters", - signed_tx.data.address - ); - } - } - if let Some(website) = metadata.website.as_ref() { - if website.len() as u64 > MAX_VALIDATOR_METADATA_LEN { - panic!( - "The website metadata of the validator with address {} is too \ - long, must be within {MAX_VALIDATOR_METADATA_LEN} characters", - signed_tx.data.address - ); - } - } - if let Some(discord_handle) = metadata.discord_handle.as_ref() { - if discord_handle.len() as u64 > MAX_VALIDATOR_METADATA_LEN { - panic!( - "The discord handle metadata of the validator with address {} \ - is too long, must be within {MAX_VALIDATOR_METADATA_LEN} \ - characters", - signed_tx.data.address - ); - } - } - if let Some(avatar) = metadata.avatar.as_ref() { - if avatar.len() as u64 > MAX_VALIDATOR_METADATA_LEN { - panic!( - "The avatar metadata of the validator with address {} is too \ - long, must be within {MAX_VALIDATOR_METADATA_LEN} characters", - signed_tx.data.address - ); - } - } - if let Some(name) = metadata.name.as_ref() { - if name.len() as u64 > MAX_VALIDATOR_METADATA_LEN { - panic!( - "The name metadata of the validator with address {} is too \ - long, must be within {MAX_VALIDATOR_METADATA_LEN} characters", - signed_tx.data.address - ); - } - } // Check signature let mut is_valid = { diff --git a/crates/node/src/bench_utils.rs b/crates/node/src/bench_utils.rs index 9bbaeb53f3..b59c03e353 100644 --- a/crates/node/src/bench_utils.rs +++ b/crates/node/src/bench_utils.rs @@ -1223,7 +1223,6 @@ impl BenchShieldedCtx { vec![masp_transfer_data], None, expiration, - true, ) .await }) diff --git a/crates/proof_of_stake/src/error.rs b/crates/proof_of_stake/src/error.rs index 165cec9b7a..8eb160aa4d 100644 --- a/crates/proof_of_stake/src/error.rs +++ b/crates/proof_of_stake/src/error.rs @@ -6,6 +6,7 @@ use namada_core::chain::Epoch; use namada_core::dec::Dec; use thiserror::Error; +use crate::parameters::MAX_VALIDATOR_METADATA_LEN; use crate::types::ValidatorState; use crate::{rewards, Error}; @@ -165,6 +166,16 @@ pub enum ConsensusKeyChangeError { MustBeEd25519, } +#[allow(missing_docs)] +#[derive(Error, Debug)] +pub enum ValidatorMetaDataError { + #[error( + "The {0} metadata is too long, must be within \ + {MAX_VALIDATOR_METADATA_LEN} characters" + )] + FieldTooLong(&'static str), +} + impl From for Error { fn from(err: BecomeValidatorError) -> Self { Self::new(err) diff --git a/crates/proof_of_stake/src/storage.rs b/crates/proof_of_stake/src/storage.rs index a3d0a61cea..0fe961d28e 100644 --- a/crates/proof_of_stake/src/storage.rs +++ b/crates/proof_of_stake/src/storage.rs @@ -963,6 +963,35 @@ where Ok(()) } +/// Read validator's metadata. +pub fn read_validator_metadata( + storage: &S, + validator: &Address, +) -> Result> +where + S: StorageRead, +{ + let email = read_validator_email(storage, validator)?; + let description = read_validator_description(storage, validator)?; + let website = read_validator_website(storage, validator)?; + let discord_handle = read_validator_discord_handle(storage, validator)?; + let avatar = read_validator_avatar(storage, validator)?; + let name = read_validator_name(storage, validator)?; + + // Email is the only required field for a validator in storage + match email { + Some(email) => Ok(Some(ValidatorMetaData { + email, + description, + website, + discord_handle, + avatar, + name, + })), + None => Ok(None), + } +} + /// Get the last epoch in which rewards were claimed from storage, if any pub fn get_last_reward_claim_epoch( storage: &S, diff --git a/crates/proof_of_stake/src/types/mod.rs b/crates/proof_of_stake/src/types/mod.rs index 988d1a7396..e0809eeb67 100644 --- a/crates/proof_of_stake/src/types/mod.rs +++ b/crates/proof_of_stake/src/types/mod.rs @@ -21,8 +21,8 @@ pub use rev_order::ReverseOrdTokenAmount; use serde::{Deserialize, Serialize}; use crate::lazy_map::NestedMap; -use crate::parameters::PosParams; -use crate::{Epoch, KeySeg, LazyMap, LazySet, LazyVec}; +use crate::parameters::{PosParams, MAX_VALIDATOR_METADATA_LEN}; +use crate::{Epoch, KeySeg, LazyMap, LazySet, LazyVec, ValidatorMetaDataError}; /// Stored positions of validators in validator sets pub type ValidatorSetPositions = crate::epoched::NestedEpoched< @@ -419,6 +419,46 @@ pub struct ValidatorMetaData { pub name: Option, } +impl ValidatorMetaData { + /// Validator validator metadata. Returns an empty vec only if all fields + /// are valid. + pub fn validate(&self) -> Vec { + let mut errors = vec![]; + if self.email.len() as u64 > MAX_VALIDATOR_METADATA_LEN { + errors.push(ValidatorMetaDataError::FieldTooLong("email")); + } + if let Some(description) = self.description.as_ref() { + if description.len() as u64 > MAX_VALIDATOR_METADATA_LEN { + errors + .push(ValidatorMetaDataError::FieldTooLong("description")); + } + } + if let Some(website) = self.website.as_ref() { + if website.len() as u64 > MAX_VALIDATOR_METADATA_LEN { + errors.push(ValidatorMetaDataError::FieldTooLong("website")); + } + } + if let Some(discord_handle) = self.discord_handle.as_ref() { + if discord_handle.len() as u64 > MAX_VALIDATOR_METADATA_LEN { + errors.push(ValidatorMetaDataError::FieldTooLong( + "discord handle", + )); + } + } + if let Some(avatar) = self.avatar.as_ref() { + if avatar.len() as u64 > MAX_VALIDATOR_METADATA_LEN { + errors.push(ValidatorMetaDataError::FieldTooLong("avatar")); + } + } + if let Some(name) = self.name.as_ref() { + if name.len() as u64 > MAX_VALIDATOR_METADATA_LEN { + errors.push(ValidatorMetaDataError::FieldTooLong("name")); + } + } + errors + } +} + #[cfg(any(test, feature = "testing"))] impl Default for ValidatorMetaData { fn default() -> Self { diff --git a/crates/proof_of_stake/src/vp.rs b/crates/proof_of_stake/src/vp.rs index dcb2d989f1..60a0b6e437 100644 --- a/crates/proof_of_stake/src/vp.rs +++ b/crates/proof_of_stake/src/vp.rs @@ -14,7 +14,7 @@ use namada_tx::BatchedTxRef; use namada_vp_env::{Error, Result, VpEnv}; use thiserror::Error; -use crate::storage::read_owned_pos_params; +use crate::storage::{read_owned_pos_params, read_validator_metadata}; use crate::storage_key::is_params_key; use crate::types::BondId; use crate::{storage_key, token}; @@ -299,6 +299,23 @@ where } } + // Validate new and changed validator metadata + for validator in became_validator.iter().chain(&changed_metadata) { + let metadata = read_validator_metadata(&ctx.post(), validator)?; + let Some(metadata) = metadata else { + return Err(Error::new_alloc(format!( + "Missing validator {validator} metadata" + ))); + }; + let errors = metadata.validate(); + if !errors.is_empty() { + return Err(Error::new_alloc(format!( + "Metadata of the validator with address {validator} are \ + invalid: {errors:#?}", + ))); + } + } + for key in keys_changed { if is_params_key(key) { return Err(Error::new_const( diff --git a/crates/sdk/src/queries/vp/pos.rs b/crates/sdk/src/queries/vp/pos.rs index 0076419c73..5fcf4e06fd 100644 --- a/crates/sdk/src/queries/vp/pos.rs +++ b/crates/sdk/src/queries/vp/pos.rs @@ -22,13 +22,10 @@ use namada_proof_of_stake::storage::{ read_below_capacity_validator_set_addresses_with_stake, read_consensus_validator_set_addresses, read_consensus_validator_set_addresses_with_stake, read_pos_params, - read_total_active_stake, read_total_stake, read_validator_avatar, - read_validator_description, read_validator_discord_handle, - read_validator_email, read_validator_last_slash_epoch, - read_validator_max_commission_rate_change, read_validator_name, - read_validator_stake, read_validator_website, unbond_handle, - validator_commission_rate_handle, validator_incoming_redelegations_handle, - validator_slashes_handle, + read_total_active_stake, read_total_stake, read_validator_last_slash_epoch, + read_validator_max_commission_rate_change, read_validator_metadata, + read_validator_stake, unbond_handle, validator_commission_rate_handle, + validator_incoming_redelegations_handle, validator_slashes_handle, }; pub use namada_proof_of_stake::types::ValidatorStateInfo; use namada_proof_of_stake::types::{ @@ -325,25 +322,7 @@ where D: 'static + DB + for<'iter> DBIter<'iter> + Sync, H: 'static + StorageHasher + Sync, { - let email = read_validator_email(ctx.state, &validator)?; - let description = read_validator_description(ctx.state, &validator)?; - let website = read_validator_website(ctx.state, &validator)?; - let discord_handle = read_validator_discord_handle(ctx.state, &validator)?; - let avatar = read_validator_avatar(ctx.state, &validator)?; - let name = read_validator_name(ctx.state, &validator)?; - - // Email is the only required field for a validator in storage - match email { - Some(email) => Ok(Some(ValidatorMetaData { - email, - description, - website, - discord_handle, - avatar, - name, - })), - _ => Ok(None), - } + read_validator_metadata(ctx.state, &validator) } /// Get the validator state diff --git a/crates/sdk/src/tx.rs b/crates/sdk/src/tx.rs index 802e82a841..ebb021f066 100644 --- a/crates/sdk/src/tx.rs +++ b/crates/sdk/src/tx.rs @@ -2648,7 +2648,6 @@ pub async fn build_ibc_transfer( context, masp_transfer_data, masp_fee_data, - !(args.tx.dry_run || args.tx.dry_run_wrapper), args.tx.expiration.to_datetime(), ) .await?; @@ -3113,7 +3112,6 @@ pub async fn build_shielded_transfer( context, transfer_data, masp_fee_data, - !(args.tx.dry_run || args.tx.dry_run_wrapper), args.tx.expiration.to_datetime(), ) .await? @@ -3280,7 +3278,6 @@ pub async fn build_shielding_transfer( context, transfer_data, None, - !(args.tx.dry_run || args.tx.dry_run_wrapper), args.tx.expiration.to_datetime(), ) .await? @@ -3402,7 +3399,6 @@ pub async fn build_unshielding_transfer( context, transfer_data, masp_fee_data, - !(args.tx.dry_run || args.tx.dry_run_wrapper), args.tx.expiration.to_datetime(), ) .await? @@ -3455,7 +3451,6 @@ async fn construct_shielded_parts( context: &N, data: Vec, fee_data: Option, - update_ctx: bool, expiration: Option, ) -> Result)>> { // Precompute asset types to increase chances of success in decoding @@ -3469,9 +3464,7 @@ async fn construct_shielded_parts( .await; shielded - .gen_shielded_transfer( - context, data, fee_data, expiration, update_ctx, - ) + .gen_shielded_transfer(context, data, fee_data, expiration) .await }; @@ -3852,7 +3845,6 @@ pub async fn gen_ibc_shielding_transfer( // Fees are paid from the transparent balance of the relayer None, args.expiration.to_datetime(), - true, ) .await .map_err(|err| TxSubmitError::MaspError(err.to_string()))? diff --git a/crates/shielded_token/src/masp.rs b/crates/shielded_token/src/masp.rs index 1d08b70fd0..4daacb3337 100644 --- a/crates/shielded_token/src/masp.rs +++ b/crates/shielded_token/src/masp.rs @@ -320,10 +320,10 @@ pub type WitnessMap = HashMap>; #[derive(BorshSerialize, BorshDeserialize, Debug)] /// The possible sync states of the shielded context pub enum ContextSyncStatus { - /// The context contains only data that has been confirmed by the protocol + /// The context contains data that has been confirmed by the protocol Confirmed, - /// The context contains that that has not yet been confirmed by the - /// protocol and could end up being invalid + /// The context possibly contains that that has not yet been confirmed by + /// the protocol and could be incomplete or invalid Speculative, } diff --git a/crates/shielded_token/src/masp/shielded_wallet.rs b/crates/shielded_token/src/masp/shielded_wallet.rs index 69d4e69fca..a8b38dec27 100644 --- a/crates/shielded_token/src/masp/shielded_wallet.rs +++ b/crates/shielded_token/src/masp/shielded_wallet.rs @@ -359,8 +359,9 @@ impl ShieldedWallet { /// Updates the internal state with the data of the newly generated /// transaction. More specifically invalidate the spent notes, but do not /// cache the newly produced output descriptions and therefore the merkle - /// tree - async fn pre_cache_transaction( + /// tree (this is because we don't know the exact position in the tree where + /// the new notes will be appended) + pub async fn pre_cache_transaction( &mut self, masp_tx: &Transaction, ) -> Result<(), eyre::Error> { @@ -1018,7 +1019,6 @@ pub trait ShieldedApi: data: Vec, fee_data: Option, expiration: Option, - update_ctx: bool, ) -> Result, TransferErr> { // Determine epoch in which to submit potential shielded transaction let epoch = Self::query_masp_epoch(context.client()) @@ -1212,12 +1212,6 @@ pub trait ShieldedApi: ) .map_err(|error| TransferErr::Build { error })?; - if update_ctx { - self.pre_cache_transaction(&masp_tx) - .await - .map_err(|e| TransferErr::General(e.to_string()))?; - } - Ok(Some(ShieldedTransfer { builder: builder_clone, masp_tx, diff --git a/crates/tests/src/integration/ledger_tests.rs b/crates/tests/src/integration/ledger_tests.rs index 0a5a39cc01..d609aaddd7 100644 --- a/crates/tests/src/integration/ledger_tests.rs +++ b/crates/tests/src/integration/ledger_tests.rs @@ -26,6 +26,7 @@ use namada_sdk::account::AccountPublicKeysMap; use namada_sdk::collections::HashMap; use namada_sdk::error::TxSubmitError; use namada_sdk::migrations; +use namada_sdk::proof_of_stake::parameters::MAX_VALIDATOR_METADATA_LEN; use namada_sdk::queries::RPC; use namada_sdk::token::{self, DenominatedAmount}; use namada_sdk::tx::{self, Tx, TX_TRANSFER_WASM, VP_USER_WASM}; @@ -2565,6 +2566,95 @@ fn wrap_tx_by_elsewho() -> Result<()> { Ok(()) } +/// Test for PoS validator metadata validation. +/// +/// 1. Run the ledger node. +/// 2. Submit a valid metadata change tx. +/// 3. Check that the metadata has changed. +/// 4. Submit an invalid metadata change tx. +/// 5. Check that the metadata has not changed. +/// 6. Submit a tx to become validator with invalid metadata. +#[test] +fn pos_validator_metadata_validation() -> Result<()> { + // 1. Run the ledger node. + let (node, _services) = setup::setup()?; + + // 2. Submit a valid metadata change tx. + let valid_desc: String = "0123456789".repeat(50); + assert_eq!(valid_desc.len() as u64, MAX_VALIDATOR_METADATA_LEN); + let tx_args = apply_use_device(vec![ + "change-metadata", + "--validator", + "validator-0-validator", + "--description", + &valid_desc, + ]); + let captured = CapturedOutput::of(|| run(&node, Bin::Client, tx_args)); + println!("{:?}", captured.result); + assert_matches!(captured.result, Ok(_)); + assert!(captured.contains(TX_APPLIED_SUCCESS)); + + // 3. Check that the metadata has changed. + let query_args = apply_use_device(vec![ + "validator-metadata", + "--validator", + "validator-0-validator", + ]); + let captured = + CapturedOutput::of(|| run(&node, Bin::Client, query_args.clone())); + println!("{:?}", captured.result); + assert!(captured.contains(&valid_desc)); + + // 4. Submit an invalid metadata change tx. + let invalid_desc: String = format!("N{valid_desc}"); + assert!(invalid_desc.len() as u64 > MAX_VALIDATOR_METADATA_LEN); + let tx_args = apply_use_device(vec![ + "change-metadata", + "--validator", + "validator-0-validator", + "--description", + &invalid_desc, + "--force", + ]); + let captured = CapturedOutput::of(|| run(&node, Bin::Client, tx_args)); + println!("{:?}", captured.result); + assert_matches!(captured.result, Ok(_)); + assert!(captured.contains(TX_REJECTED)); + + // 5. Check that the metadata has not changed. + let captured = CapturedOutput::of(|| run(&node, Bin::Client, query_args)); + println!("{:?}", captured.result); + assert!(captured.contains(&valid_desc)); + + // 6. Submit a tx to become validator with invalid metadata. + let new_validator = "new-validator"; + let tx_args = apply_use_device(vec![ + "init-validator", + "--alias", + new_validator, + "--name", + new_validator, + "--account-keys", + "bertha-key", + "--commission-rate", + "0.05", + "--max-commission-rate-change", + "0.01", + "--email", + "null@null.net", + "--signing-keys", + "bertha-key", + "--description", + &invalid_desc, + "--unsafe-dont-encrypt", + ]); + let captured = CapturedOutput::of(|| run(&node, Bin::Client, tx_args)); + assert_matches!(captured.result, Err(_)); + assert!(captured.contains(TX_REJECTED)); + + Ok(()) +} + fn make_migration_json() -> (Hash, tempfile::NamedTempFile) { let file = tempfile::Builder::new().tempfile().expect("Test failed"); let updates = [migrations::DbUpdateType::Add { diff --git a/crates/tests/src/integration/masp.rs b/crates/tests/src/integration/masp.rs index 1cd8a9e2cd..f88f08ecdd 100644 --- a/crates/tests/src/integration/masp.rs +++ b/crates/tests/src/integration/masp.rs @@ -6073,3 +6073,279 @@ fn tricky_masp_txs() -> Result<()> { Ok(()) } + +// Test generation of transactions and querying balance with the speculative +// context +#[test] +fn speculative_context() -> Result<()> { + // This address doesn't matter for tests. But an argument is required. + let validator_one_rpc = "http://127.0.0.1:26567"; + // Download the shielded pool parameters before starting node + let _ = FsShieldedUtils::new(PathBuf::new()); + let (mut node, _services) = setup::setup()?; + _ = node.next_masp_epoch(); + + // Add the relevant viewing keys to the wallet otherwise the shielded + // context won't precache the masp data + run( + &node, + Bin::Wallet, + vec![ + "add", + "--alias", + "alias_a", + "--value", + AA_VIEWING_KEY, + "--unsafe-dont-encrypt", + ], + )?; + + // 1. Shield some tokens in two steps two generate two different output + // notes + for _ in 0..2 { + let captured = CapturedOutput::of(|| { + run( + &node, + Bin::Client, + vec![ + "shield", + "--source", + ALBERT, + "--target", + AA_PAYMENT_ADDRESS, + "--token", + NAM, + "--amount", + "100", + "--node", + validator_one_rpc, + ], + ) + }); + assert!(captured.result.is_ok()); + assert!(captured.contains(TX_APPLIED_SUCCESS)); + } + + // 2. Sync the shielded context and check the balance + run( + &node, + Bin::Client, + vec![ + "shielded-sync", + "--viewing-keys", + AA_VIEWING_KEY, + "--node", + validator_one_rpc, + ], + )?; + let captured = CapturedOutput::of(|| { + run( + &node, + Bin::Client, + vec![ + "balance", + "--owner", + AA_VIEWING_KEY, + "--token", + NAM, + "--node", + validator_one_rpc, + ], + ) + }); + assert!(captured.result.is_ok()); + assert!(captured.contains("nam: 200")); + + // 3. Spend an amount of tokens which is less than the amount of every + // single note + let captured = CapturedOutput::of(|| { + run( + &node, + Bin::Client, + vec![ + "transfer", + "--source", + A_SPENDING_KEY, + "--target", + AB_PAYMENT_ADDRESS, + "--token", + NAM, + "--amount", + "90", + "--gas-payer", + ALBERT_KEY, + "--node", + validator_one_rpc, + ], + ) + }); + assert!(captured.result.is_ok()); + assert!(captured.contains(TX_APPLIED_SUCCESS)); + + // 4. Check the balance without calling shielded-sync to check the response + // of the speculative context + let captured = CapturedOutput::of(|| { + run( + &node, + Bin::Client, + vec![ + "balance", + "--owner", + AA_VIEWING_KEY, + "--token", + NAM, + "--node", + validator_one_rpc, + ], + ) + }); + assert!(captured.result.is_ok()); + // The speculative context invalidates the entire note spent so we expect to + // see the balance coming only from the second unspent note + assert!(captured.contains("nam: 100")); + + // 5. Try to spend some amount from the remaining note with a tx that will + // fail + let captured = CapturedOutput::of(|| { + run( + &node, + Bin::Client, + vec![ + "transfer", + "--source", + A_SPENDING_KEY, + "--target", + AB_PAYMENT_ADDRESS, + "--token", + NAM, + "--amount", + "90", + "--gas-payer", + ALBERT_KEY, + // Force failure with low gas limit + "--gas-limit", + "10000", + "--node", + validator_one_rpc, + ], + ) + }); + assert!(captured.result.is_ok()); + assert!(captured.contains("Gas error: Transaction gas limit exceeded")); + + // 6. Check that the speculative context was not updated + let captured = CapturedOutput::of(|| { + run( + &node, + Bin::Client, + vec![ + "balance", + "--owner", + AA_VIEWING_KEY, + "--token", + NAM, + "--node", + validator_one_rpc, + ], + ) + }); + assert!(captured.result.is_ok()); + assert!(captured.contains("nam: 100")); + + // 7. Try to spend some amount from the remaining note + let captured = CapturedOutput::of(|| { + run( + &node, + Bin::Client, + vec![ + "transfer", + "--source", + A_SPENDING_KEY, + "--target", + AB_PAYMENT_ADDRESS, + "--token", + NAM, + "--amount", + "90", + "--gas-payer", + ALBERT_KEY, + "--node", + validator_one_rpc, + ], + ) + }); + assert!(captured.result.is_ok()); + assert!(captured.contains(TX_APPLIED_SUCCESS)); + + // 8. Check the balance without calling shielded-sync to check the response + // of the speculative context + let captured = CapturedOutput::of(|| { + run( + &node, + Bin::Client, + vec![ + "balance", + "--owner", + AA_VIEWING_KEY, + "--token", + NAM, + "--node", + validator_one_rpc, + ], + ) + }); + assert!(captured.result.is_ok()); + // The speculative context invalidates the entire note spent so we expect to + // see an empty balance + assert!(captured.contains("nam: 0")); + + // 9. Finally, sync the shielded context and check the confirmed balances + run( + &node, + Bin::Client, + vec![ + "shielded-sync", + "--viewing-keys", + AA_VIEWING_KEY, + AB_VIEWING_KEY, + "--node", + validator_one_rpc, + ], + )?; + let captured = CapturedOutput::of(|| { + run( + &node, + Bin::Client, + vec![ + "balance", + "--owner", + AA_VIEWING_KEY, + "--token", + NAM, + "--node", + validator_one_rpc, + ], + ) + }); + assert!(captured.result.is_ok()); + assert!(captured.contains("nam: 20")); + let captured = CapturedOutput::of(|| { + run( + &node, + Bin::Client, + vec![ + "balance", + "--owner", + AB_VIEWING_KEY, + "--token", + NAM, + "--node", + validator_one_rpc, + ], + ) + }); + assert!(captured.result.is_ok()); + assert!(captured.contains("nam: 180")); + + Ok(()) +}