diff --git a/.changelog/unreleased/improvements/4832-dry-run-mock-sigs.md b/.changelog/unreleased/improvements/4832-dry-run-mock-sigs.md new file mode 100644 index 00000000000..e1d1af3c9b1 --- /dev/null +++ b/.changelog/unreleased/improvements/4832-dry-run-mock-sigs.md @@ -0,0 +1,8 @@ +- Closes #4714 + - When dry-running, dummy signatures are used in + txs + - Validation does not perform signature checks when dry-running + +I have tested that the gas estimation hasn't changed between dry-running +with signatures and with dummies. It is curious however that in both cases there is a small discrepancy between dry-running and the actual tx. +([\#4714](https://github.com/namada-net/namada/issues/4714)) \ No newline at end of file diff --git a/crates/apps_lib/src/cli.rs b/crates/apps_lib/src/cli.rs index fbecf648b95..24fbf760f5c 100644 --- a/crates/apps_lib/src/cli.rs +++ b/crates/apps_lib/src/cli.rs @@ -7849,15 +7849,10 @@ pub mod args { )) .conflicts_with(DRY_RUN_TX.name), ) - .arg( - DEVICE_TRANSPORT - .def() - .help(wrap!( - "Select transport for hardware wallet from \"hid\" \ - (default) or \"tcp\"." - )) - .conflicts_with(DRY_RUN_TX.name), - ) + .arg(DEVICE_TRANSPORT.def().help(wrap!( + "Select transport for hardware wallet from \"hid\" (default) \ + or \"tcp\"." + ))) .arg( MEMO_OPT .def() diff --git a/crates/core/src/key/common.rs b/crates/core/src/key/common.rs index 16ca93e3166..020930bb429 100644 --- a/crates/core/src/key/common.rs +++ b/crates/core/src/key/common.rs @@ -516,6 +516,17 @@ impl super::SigScheme for SigScheme { _ => Err(VerifySigError::MismatchedScheme), } } + + fn mock(keypair: &SecretKey) -> Self::Signature { + match keypair { + SecretKey::Ed25519(kp) => { + Signature::Ed25519(ed25519::SigScheme::mock(kp)) + } + SecretKey::Secp256k1(kp) => { + Signature::Secp256k1(secp256k1::SigScheme::mock(kp)) + } + } + } } #[cfg(test)] diff --git a/crates/core/src/key/ed25519.rs b/crates/core/src/key/ed25519.rs index 8e53e051a86..0eefd596a4b 100644 --- a/crates/core/src/key/ed25519.rs +++ b/crates/core/src/key/ed25519.rs @@ -402,6 +402,10 @@ impl super::SigScheme for SigScheme { Ok(()) } } + + fn mock(_: &Self::SecretKey) -> Self::Signature { + Signature(ed25519_consensus::Signature::from([0; 64])) + } } #[cfg(feature = "arbitrary")] diff --git a/crates/core/src/key/mod.rs b/crates/core/src/key/mod.rs index 330ff64ce7b..447cc6a68fd 100644 --- a/crates/core/src/key/mod.rs +++ b/crates/core/src/key/mod.rs @@ -268,6 +268,9 @@ pub trait SigScheme: Eq + Ord + Debug + Serialize + Default { ) -> Result<(), VerifySigError> { Self::verify_signature_with_hasher::(pk, data, sig) } + + /// Provide a dummy signature + fn mock(keypair: &Self::SecretKey) -> Self::Signature; } /// Public key hash derived from `common::Key` borsh encoded bytes (hex string diff --git a/crates/core/src/key/secp256k1.rs b/crates/core/src/key/secp256k1.rs index 3a0a9e2ead3..d02f455af35 100644 --- a/crates/core/src/key/secp256k1.rs +++ b/crates/core/src/key/secp256k1.rs @@ -602,6 +602,17 @@ impl super::SigScheme for SigScheme { Ok(()) } } + + fn mock(_: &Self::SecretKey) -> Self::Signature { + Signature( + k256::ecdsa::Signature::from_scalars( + k256::Scalar::ONE, + k256::Scalar::ONE, + ) + .unwrap(), + RecoveryId::from_byte(0).unwrap(), + ) + } } #[cfg(feature = "arbitrary")] diff --git a/crates/node/src/bench_utils.rs b/crates/node/src/bench_utils.rs index 137d1bd8cfc..91b64c59efd 100644 --- a/crates/node/src/bench_utils.rs +++ b/crates/node/src/bench_utils.rs @@ -285,6 +285,7 @@ impl BenchShellInner { &mut self.inner.vp_wasm_cache, &mut self.inner.tx_wasm_cache, run::GasMeterKind::MutGlobal, + false, ) .unwrap() } diff --git a/crates/node/src/dry_run_tx.rs b/crates/node/src/dry_run_tx.rs index f01ac059bae..bc2cd898e04 100644 --- a/crates/node/src/dry_run_tx.rs +++ b/crates/node/src/dry_run_tx.rs @@ -30,7 +30,6 @@ where CA: 'static + WasmCacheAccess + Sync, { let tx = Tx::try_from_bytes(&request.data[..]).into_storage_result()?; - tx.validate_tx().into_storage_result()?; let gas_scale = parameters::get_gas_scale(&state)?; let height = state.in_mem().get_block_height().0; @@ -59,6 +58,7 @@ where &tx_gas_meter, &mut shell_params, None, + true, ) .into_storage_result()?; @@ -107,6 +107,7 @@ where &mut vp_wasm_cache, &mut tx_wasm_cache, protocol::GasMeterKind::MutGlobal, + true, ) .map_err(|err| err.error) .into_storage_result()?; diff --git a/crates/node/src/protocol.rs b/crates/node/src/protocol.rs index f74cb220ba9..7865ea4078f 100644 --- a/crates/node/src/protocol.rs +++ b/crates/node/src/protocol.rs @@ -252,6 +252,7 @@ where vp_wasm_cache, tx_wasm_cache, GasMeterKind::MutGlobal, + false, ) } else { // Governance proposal. We don't allow tx batches in this case, @@ -270,6 +271,7 @@ where tx_wasm_cache, }, GasMeterKind::MutGlobal, + false, ) .map_err(|e| Box::new(DispatchError::from(e)))?; @@ -328,6 +330,7 @@ where tx_gas_meter, &mut shell_params, Some(block_proposer), + false, ) .map_err(|e| { Box::new(Error::WrapperRunnerError(e.to_string()).into()) @@ -348,6 +351,7 @@ pub(crate) fn dispatch_inner_txs<'a, S, D, H, CA>( vp_wasm_cache: &'a mut VpCache, tx_wasm_cache: &'a mut TxCache, gas_meter_kind: GasMeterKind, + dry_run: bool, ) -> std::result::Result, Box> where S: 'static @@ -384,6 +388,7 @@ where tx_wasm_cache, }, gas_meter_kind, + dry_run, ) { Err(Error::GasError(msg)) => { // Gas error aborts the execution of the entire batch @@ -489,6 +494,7 @@ pub(crate) fn apply_wrapper_tx( tx_gas_meter: &RefCell, shell_params: &mut ShellParams<'_, S, D, H, CA>, block_proposer: Option<&Address>, + dry_run: bool, ) -> Result> where S: 'static @@ -514,7 +520,7 @@ where Some(block_proposer) => { transfer_fee(shell_params, block_proposer, tx, wrapper, tx_index)? } - None => check_fees(shell_params, tx, wrapper)?, + None => check_fees(shell_params, tx, wrapper, dry_run)?, }; // Commit tx to the block write log even in case of subsequent errors (if @@ -627,7 +633,7 @@ where } else { // See if the first inner transaction of the batch pays the fees // with a masp unshield - match try_masp_fee_payment(shell_params, tx, tx_index) { + match try_masp_fee_payment(shell_params, tx, tx_index, false) { Ok(valid_batched_tx_result) => { #[cfg(not(fuzzing))] let balance = token::read_balance( @@ -784,6 +790,7 @@ fn try_masp_fee_payment( }: &mut ShellParams<'_, S, D, H, CA>, tx: &Tx, tx_index: &TxIndex, + dry_run: bool, ) -> std::result::Result where S: 'static @@ -833,6 +840,7 @@ where tx_wasm_cache, }, GasMeterKind::MutGlobal, + dry_run, ) { Ok(result) => { // NOTE: do not commit yet cause this could be exploited to get @@ -958,6 +966,7 @@ pub fn check_fees( shell_params: &mut ShellParams<'_, S, D, H, CA>, tx: &Tx, wrapper: &WrapperTx, + dry_run: bool, ) -> Result> where S: 'static @@ -994,6 +1003,7 @@ where shell_params, tx, &TxIndex::default(), + dry_run, )?; let balance = token::read_balance( shell_params.state, @@ -1030,6 +1040,7 @@ fn apply_wasm_tx( tx_index: &TxIndex, shell_params: ShellParams<'_, S, D, H, CA>, gas_meter_kind: GasMeterKind, + dry_run: bool, ) -> Result where S: 'static + State + ReadConversionState + Sync, @@ -1053,6 +1064,7 @@ where vp_wasm_cache, tx_wasm_cache, gas_meter_kind, + dry_run, )?; let vps_result = check_vps(CheckVps { @@ -1063,6 +1075,7 @@ where verifiers_from_tx: &verifiers, vp_wasm_cache, gas_meter_kind, + dry_run, })?; let initialized_accounts = state.write_log().get_initialized_accounts(); @@ -1172,6 +1185,7 @@ fn execute_tx( vp_wasm_cache: &mut VpCache, tx_wasm_cache: &mut TxCache, gas_meter_kind: GasMeterKind, + dry_run: bool, ) -> Result> where S: State, @@ -1189,6 +1203,7 @@ where vp_wasm_cache, tx_wasm_cache, gas_meter_kind, + dry_run, ) .map_err(|err| match err { wasm::run::Error::GasError(msg) => Error::GasError(msg), @@ -1210,6 +1225,7 @@ where verifiers_from_tx: &'a BTreeSet
, vp_wasm_cache: &'a mut VpCache, gas_meter_kind: GasMeterKind, + dry_run: bool, } /// Check the acceptance of a transaction by validity predicates @@ -1222,6 +1238,7 @@ fn check_vps( verifiers_from_tx, vp_wasm_cache, gas_meter_kind, + dry_run, }: CheckVps<'_, S, CA>, ) -> Result where @@ -1241,6 +1258,7 @@ where tx_gas_meter, vp_wasm_cache, gas_meter_kind, + dry_run, )?; tracing::debug!("Total VPs gas cost {:?}", vps_gas); @@ -1262,6 +1280,7 @@ fn execute_vps( tx_gas_meter: &TxGasMeter, vp_wasm_cache: &mut VpCache, gas_meter_kind: GasMeterKind, + dry_run: bool, ) -> Result<(VpsResult, namada_sdk::gas::Gas)> where S: 'static + ReadConversionState + State + Sync, @@ -1298,6 +1317,7 @@ where &verifiers, vp_wasm_cache.clone(), gas_meter_kind, + dry_run, ) .map_err(|err| match err { wasm::run::Error::GasError(msg) => { @@ -1755,6 +1775,7 @@ mod tests { &gas_meter, &mut vp_cache, GasMeterKind::MutGlobal, + false, ); assert!(matches!(result.unwrap_err(), Error::GasError(_))); } @@ -1808,6 +1829,7 @@ mod tests { &Default::default(), vp_cache.clone(), GasMeterKind::MutGlobal, + false, ) .is_ok() ); @@ -1838,6 +1860,7 @@ mod tests { &Default::default(), vp_cache.clone(), GasMeterKind::MutGlobal, + false, ) .is_err() ); diff --git a/crates/node/src/shell/mod.rs b/crates/node/src/shell/mod.rs index 84b6f7b0155..a640d9e1b95 100644 --- a/crates/node/src/shell/mod.rs +++ b/crates/node/src/shell/mod.rs @@ -1519,7 +1519,7 @@ where ))))?; fee_data_check(wrapper, minimum_gas_price, shell_params)?; - protocol::check_fees(shell_params, tx, wrapper) + protocol::check_fees(shell_params, tx, wrapper, false) .map_err(Error::TxApply) .map(|_| ()) } diff --git a/crates/sdk/src/signing.rs b/crates/sdk/src/signing.rs index 46eaadfa29c..d0a4074c453 100644 --- a/crates/sdk/src/signing.rs +++ b/crates/sdk/src/signing.rs @@ -17,6 +17,7 @@ use namada_core::address::{Address, ImplicitAddress, InternalAddress, MASP}; use namada_core::arith::checked; use namada_core::collections::{HashMap, HashSet}; use namada_core::ibc::primitives::IntoHostTime; +use namada_core::key::common::{CommonPublicKey, CommonSignature}; use namada_core::key::*; use namada_core::masp::{AssetData, MaspTxId, PaymentAddress}; use namada_core::tendermint::Time as TmTime; @@ -37,7 +38,9 @@ use namada_token::storage_key::balance_key; use namada_tx::data::pgf::UpdateStewardCommission; use namada_tx::data::pos::BecomeValidator; use namada_tx::data::{Fee, pos}; -use namada_tx::{Authorization, MaspBuilder, Section, SignatureIndex, Tx}; +use namada_tx::{ + Authorization, MaspBuilder, Section, SignatureIndex, Signer, Tx, +}; use rand::rngs::OsRng; use serde::{Deserialize, Serialize}; use tokio::sync::RwLock; @@ -218,6 +221,43 @@ pub async fn default_sign( ))) } +/// When mocking signatures, if a key is in a hardware wallet, +/// use this function instead to mock the signature +fn mock_hw_sig(tx: &mut Tx, pk: common::PublicKey, signable: Signable) { + let targets = match signable { + Signable::FeeRawHeader => { + vec![tx.sechashes(), vec![tx.raw_header_hash()]] + } + Signable::RawHeader => vec![vec![tx.raw_header_hash()]], + }; + tx.protocol_filter(); + let sig = match pk { + CommonPublicKey::Ed25519(_) => { + // this key has no effect other than to satisfy api constraints + let kp = ed25519::SigScheme::from_bytes([0; 32]); + CommonSignature::Ed25519(ed25519::SigScheme::mock(&kp)) + } + CommonPublicKey::Secp256k1(_) => { + // this key has no effect other than to satisfy api constraints + const SECRET_KEY_HEX: &str = + "c2c72dfbff11dfb4e9d5b0a20c620c58b15bb7552753601f043db91331b0db15"; + let sk_bytes = HEXLOWER.decode(SECRET_KEY_HEX.as_bytes()).unwrap(); + let kp = + secp256k1::SecretKey::try_from_slice(&sk_bytes[..]).unwrap(); + CommonSignature::Secp256k1(secp256k1::SigScheme::mock(&kp)) + } + }; + for t in targets { + let mut signatures = BTreeMap::new(); + signatures.insert(0, sig.clone()); + tx.add_section(Section::Authorization(Authorization { + targets: t, + signer: Signer::PubKeys(vec![pk.clone()]), + signatures, + })); + } +} + /// Sign a transaction with a given signing key or public key of a given signer. /// If no explicit signer given, use the `default`. If no `default` is given, /// Error. @@ -280,11 +320,19 @@ where } if !signing_tx_keypairs.is_empty() { - tx.sign_raw( - signing_tx_keypairs, - account_public_keys_map, - signing_data.owner, - ); + if args.dry_run || args.dry_run_wrapper { + tx.mock( + signing_tx_keypairs, + account_public_keys_map, + signing_data.owner, + ) + } else { + tx.sign_raw( + signing_tx_keypairs, + account_public_keys_map, + signing_data.owner, + ) + }; } } @@ -295,7 +343,10 @@ where either::Either::Left((fee_payer, _)) if pubkey == fee_payer => { } _ => { - if let Ok(ntx) = sign( + if args.dry_run || args.dry_run_wrapper { + mock_hw_sig(tx, pubkey.clone(), Signable::RawHeader); + used_pubkeys.insert(pubkey.clone()); + } else if let Ok(ntx) = sign( tx.clone(), pubkey.clone(), Signable::RawHeader, @@ -329,16 +380,28 @@ where }; match key { Ok(fee_payer_keypair) => { - tx.sign_wrapper(fee_payer_keypair); + if args.dry_run || args.dry_run_wrapper { + tx.mock_sign_wrapper(fee_payer_keypair); + } else { + tx.sign_wrapper(fee_payer_keypair); + } } Err(_) => { - *tx = sign( - tx.clone(), - fee_payer.clone(), - Signable::FeeRawHeader, - user_data, - ) - .await?; + if args.dry_run || args.dry_run_wrapper { + mock_hw_sig( + tx, + fee_payer.clone(), + Signable::FeeRawHeader, + ); + } else { + *tx = sign( + tx.clone(), + fee_payer.clone(), + Signable::FeeRawHeader, + user_data, + ) + .await?; + } if signing_data.public_keys.contains(fee_payer) { used_pubkeys.insert(fee_payer.clone()); } diff --git a/crates/tests/src/integration/ledger_tests.rs b/crates/tests/src/integration/ledger_tests.rs index 6a007e957fa..2c46f26df94 100644 --- a/crates/tests/src/integration/ledger_tests.rs +++ b/crates/tests/src/integration/ledger_tests.rs @@ -356,6 +356,122 @@ fn ledger_txs_and_queries() -> Result<()> { Ok(()) } +/// We test that we can dry-run transactions without +/// producing valid signatures. The gas estimation cost +/// should be the close to the same between dry-run and actual execution. +/// +/// Steps: +/// 1. Add a public key to the software wallet, but not its corresponding +/// secret. This mocks the scenario where a hardware wallet is being used +/// 2. Dry-run a transaction from Bertha to the implicit account of the above +/// public key. Get the gas cost. +/// 3. Actually perform the transfer and get the gas cost. +/// 4. Check that the difference in gas cost between the dry-run and actual tx +/// is within 10% of the actual gas cost +/// 5. Test that we dry-run a tx requiring a signature from the public key in +/// the first step without using a hardware wallet. +#[test] +fn test_dry_run_transaction() -> Result<()> { + // This address doesn't matter for tests. But an argument is required. + let validator_one_rpc = "http://127.0.0.1:26567"; + // This is a public key whose secret counterpart is not in the software + // wallet. + const HARDWARE_WALLET_PK: &str = + "tpknam1qpxpjvd9tm8jef9y07zsxhyhr2maeav06m9crh24ldkhtwrt7wj0y4gvem8"; + // The implicit address corresponding to HARDWARE_WALLET_PK + const HARDWARE_WALLET_IMPLICIT: &str = + "tnam1qqre7pwsjp9hgdfefehspujks0mdt3p90c7m3f7p"; + + let (node, _services) = setup::setup()?; + + let tx_args = vec![ + "add", + "--alias", + "hw_pk", + "--value", + HARDWARE_WALLET_PK, + "--unsafe-dont-encrypt", + ]; + let captured = CapturedOutput::of(|| run(&node, Bin::Wallet, tx_args)); + assert_matches!(captured.result, Ok(_)); + + let tx_args = vec![ + "transparent-transfer", + "--source", + BERTHA, + "--target", + HARDWARE_WALLET_IMPLICIT, + "--token", + NAM, + "--amount", + "1000.1", + "--signing-keys", + BERTHA_KEY, + "--dry-run-wrapper", + "--node", + &validator_one_rpc, + ]; + let captured = CapturedOutput::of(|| run(&node, Bin::Client, tx_args)); + assert_matches!(captured.result, Ok(_)); + assert!(!captured.contains(TX_REJECTED)); + let matches = captured + .matches("batch consumed [0-9]+") + .expect("Test failed"); + let dry_run_gas = + u64::from_str(matches.split(' ').next_back().expect("Test failed")) + .expect("Test failed"); + let tx_args = vec![ + "transparent-transfer", + "--source", + BERTHA, + "--target", + HARDWARE_WALLET_IMPLICIT, + "--token", + NAM, + "--amount", + "1000.1", + "--signing-keys", + BERTHA_KEY, + "--node", + &validator_one_rpc, + ]; + let captured = CapturedOutput::of(|| run(&node, Bin::Client, tx_args)); + assert_matches!(captured.result, Ok(_)); + assert!(captured.contains(TX_APPLIED_SUCCESS)); + let matches = captured + .matches("batch consumed [0-9]+") + .expect("Test failed"); + let actual_gas = + u64::from_str(matches.split(' ').next_back().expect("Test failed")) + .expect("Test failed"); + + // check that the gas difference is at most 10% different from the actual + assert!(10 * dry_run_gas.abs_diff(actual_gas) <= actual_gas); + + let tx_args = apply_use_device(vec![ + "transparent-transfer", + "--source", + HARDWARE_WALLET_IMPLICIT, + "--target", + ALBERT, + "--token", + NAM, + "--amount", + "10.1", + "--dry-run", + "--signing-keys", + HARDWARE_WALLET_PK, + "--gas-payer", + HARDWARE_WALLET_IMPLICIT, + "--node", + &validator_one_rpc, + ]); + let captured = CapturedOutput::of(|| run(&node, Bin::Client, tx_args)); + assert_matches!(captured.result, Ok(_)); + assert!(!captured.contains(TX_REJECTED)); + Ok(()) +} + /// In this test we: /// 1. Run the ledger node /// 2. Submit an invalid transaction (disallowed by state machine) diff --git a/crates/tests/src/vm_host_env/tx.rs b/crates/tests/src/vm_host_env/tx.rs index c0db1f08b35..36430728e8f 100644 --- a/crates/tests/src/vm_host_env/tx.rs +++ b/crates/tests/src/vm_host_env/tx.rs @@ -249,6 +249,7 @@ impl TestTxEnv { &mut self.vp_wasm_cache, &mut self.tx_wasm_cache, wasm::run::GasMeterKind::MutGlobal, + false, ) .and(Ok(())); diff --git a/crates/tx/src/section.rs b/crates/tx/src/section.rs index d288de3e058..19728e58d65 100644 --- a/crates/tx/src/section.rs +++ b/crates/tx/src/section.rs @@ -510,6 +510,35 @@ impl Authorization { secret_keys: BTreeMap, signer: Option
, ) -> Self { + Self::create_signatures( + targets, + secret_keys, + signer, + common::SigScheme::sign, + ) + } + + /// Place mock signatures over the given section hash with + /// the given key and return a section + pub fn mock( + targets: Vec, + secret_keys: BTreeMap, + signer: Option
, + ) -> Self { + Self::create_signatures(targets, secret_keys, signer, |kp, _| { + common::SigScheme::mock(kp) + }) + } + + fn create_signatures( + targets: Vec, + secret_keys: BTreeMap, + signer: Option
, + create_sig: F, + ) -> Self + where + F: Fn(&common::SecretKey, namada_core::hash::Hash) -> common::Signature, + { // If no signer address is given, then derive the signer's public keys // from the given secret keys. let signer = if let Some(addr) = signer { @@ -539,9 +568,7 @@ impl Authorization { // commitment made above let signatures = secret_keys .iter() - .map(|(index, secret_key)| { - (*index, common::SigScheme::sign(secret_key, target)) - }) + .map(|(index, secret_key)| (*index, create_sig(secret_key, target))) .collect(); Self { signatures, @@ -663,6 +690,81 @@ impl Authorization { Ok(verifications) } + + /// Mock a signature verification while still consuming gas. Used + /// for dry-running txs. + pub fn dry_run_signature( + &self, + verified_pks: &mut HashSet, + public_keys_index_map: &AccountPublicKeysMap, + signer: &Option
, + consume_verify_sig_gas: &mut F, + ) -> std::result::Result + where + F: FnMut() -> std::result::Result<(), namada_gas::Error>, + { + // Records whether there are any successful verifications + let mut verifications = 0; + match &self.signer { + // Verify the signatures against the given public keys if the + // account addresses match + Signer::Address(addr) if Some(addr) == signer.as_ref() => { + for idx in self.signatures.keys() { + if public_keys_index_map + .get_public_key_from_index(*idx) + .is_some() + { + consume_verify_sig_gas()?; + verified_pks.insert(*idx); + // Cannot overflow + #[allow(clippy::arithmetic_side_effects)] + { + verifications += 1; + } + } + } + } + // If the account addresses do not match, then there is no efficient + // way to map signatures to the given public keys + Signer::Address(_) => {} + // Verify the signatures against the subset of this section's public + // keys that are also in the given map + Signer::PubKeys(pks) => { + if pks.len() > usize::from(u8::MAX) { + return Err(VerifySigError::PksOverflow); + } + #[allow(clippy::disallowed_types)] // ordering doesn't matter + let unique_pks: std::collections::HashSet< + &common::PublicKey, + > = std::collections::HashSet::from_iter(pks.iter()); + if unique_pks.len() != pks.len() { + return Err(VerifySigError::RepeatedPks); + } + for (idx, pk) in pks.iter().enumerate() { + let map_idx = + public_keys_index_map.get_index_from_public_key(pk); + + if let Some(map_idx) = map_idx { + let sig_idx = u8::try_from(idx) + .map_err(|_| VerifySigError::PksOverflow)?; + consume_verify_sig_gas()?; + _ = self + .signatures + .get(&sig_idx) + .ok_or(VerifySigError::MissingSignature)?; + verified_pks.insert(map_idx); + // Cannot overflow + #[allow(clippy::arithmetic_side_effects)] + { + verifications += 1; + } + } + } + } + } + + Ok(verifications) + } } /// A section representing a multisig over another section diff --git a/crates/tx/src/types.rs b/crates/tx/src/types.rs index 9d1663aaf68..51642ff1fb4 100644 --- a/crates/tx/src/types.rs +++ b/crates/tx/src/types.rs @@ -464,17 +464,69 @@ impl Tx { } /// Verify that the sections with the given hashes have been signed by the - /// given public keys + /// given public keys signatures pub fn verify_signatures( + &self, + hashes: &HashSet, + public_keys_index_map: AccountPublicKeysMap, + signer: &Option
, + threshold: u8, + consume_verify_sig_gas: F, + ) -> std::result::Result, VerifySigError> + where + F: FnMut() -> std::result::Result<(), namada_gas::Error>, + { + self.verify_signatures_aux( + hashes, + public_keys_index_map, + signer, + threshold, + consume_verify_sig_gas, + Authorization::verify_signature::, + ) + } + + /// Same checks as [`Self::verify_signatures`] without performing the actual + /// signature checks. Used for dry-running txs. + pub fn dry_run_signatures( + &self, + hashes: &HashSet, + public_keys_index_map: AccountPublicKeysMap, + signer: &Option
, + threshold: u8, + consume_verify_sig_gas: F, + ) -> std::result::Result, VerifySigError> + where + F: FnMut() -> std::result::Result<(), namada_gas::Error>, + { + self.verify_signatures_aux( + hashes, + public_keys_index_map, + signer, + threshold, + consume_verify_sig_gas, + Authorization::dry_run_signature::, + ) + } + + fn verify_signatures_aux( &self, hashes: &HashSet, public_keys_index_map: AccountPublicKeysMap, signer: &Option
, threshold: u8, mut consume_verify_sig_gas: F, + mut verifying_func: G, ) -> std::result::Result, VerifySigError> where F: FnMut() -> std::result::Result<(), namada_gas::Error>, + G: FnMut( + &Authorization, + &mut HashSet, + &AccountPublicKeysMap, + &Option
, + &mut F, + ) -> std::result::Result, { // Records the public key indices used in successful signatures let mut verified_pks = HashSet::new(); @@ -527,18 +579,18 @@ impl Tx { if matching_hashes { // Finally verify that the signature itself is valid - let amt_verifieds = signatures - .verify_signature( - &mut verified_pks, - &public_keys_index_map, - signer, - &mut consume_verify_sig_gas, + let amt_verifieds = verifying_func( + signatures, + &mut verified_pks, + &public_keys_index_map, + signer, + &mut consume_verify_sig_gas, + ) + .map_err(|_e| { + VerifySigError::InvalidSectionSignature( + "found invalid signature.".to_string(), ) - .map_err(|_e| { - VerifySigError::InvalidSectionSignature( - "found invalid signature.".to_string(), - ) - }); + }); // Record the section witnessing these signatures if amt_verifieds? > 0 { witnesses.push(signatures); @@ -771,8 +823,31 @@ impl Tx { /// Add fee payer keypair to the tx builder pub fn sign_wrapper(&mut self, keypair: common::SecretKey) -> &mut Self { + self.create_wrapper_sig(keypair, Authorization::new) + } + + /// Mock adding fee payer keypair to the tx builder + pub fn mock_sign_wrapper( + &mut self, + keypair: common::SecretKey, + ) -> &mut Self { + self.create_wrapper_sig(keypair, Authorization::mock) + } + + fn create_wrapper_sig( + &mut self, + keypair: common::SecretKey, + create_sig: F, + ) -> &mut Self + where + F: Fn( + Vec, + BTreeMap, + Option
, + ) -> Authorization, + { self.protocol_filter(); - self.add_section(Section::Authorization(Authorization::new( + self.add_section(Section::Authorization(create_sig( self.sechashes(), [(0, keypair)].into_iter().collect(), None, @@ -787,6 +862,44 @@ impl Tx { account_public_keys_map: AccountPublicKeysMap, signer: Option
, ) -> &mut Self { + self.create_sig_raw( + keypairs, + account_public_keys_map, + signer, + Authorization::new, + ) + } + + /// Add signing keys to the tx builder with mock + /// signatures + pub fn mock( + &mut self, + keypairs: Vec, + account_public_keys_map: AccountPublicKeysMap, + signer: Option
, + ) -> &mut Self { + self.create_sig_raw( + keypairs, + account_public_keys_map, + signer, + Authorization::mock, + ) + } + + fn create_sig_raw( + &mut self, + keypairs: Vec, + account_public_keys_map: AccountPublicKeysMap, + signer: Option
, + create_sig: F, + ) -> &mut Self + where + F: Fn( + Vec, + BTreeMap, + Option
, + ) -> Authorization, + { // The inner tx signer signs the Raw version of the Header let hashes = vec![self.raw_header_hash()]; self.protocol_filter(); @@ -797,7 +910,7 @@ impl Tx { (0..).zip(keypairs).collect() }; - self.add_section(Section::Authorization(Authorization::new( + self.add_section(Section::Authorization(create_sig( hashes, secret_keys, signer, diff --git a/crates/vm/src/host_env.rs b/crates/vm/src/host_env.rs index 1cf98b962c4..ead41019254 100644 --- a/crates/vm/src/host_env.rs +++ b/crates/vm/src/host_env.rs @@ -100,6 +100,9 @@ where pub memory: MEM, /// The tx context contains references to host structures. pub ctx: TxCtx, + /// Indicates whether the transaction executed in this environment + /// is a dry-run or not. + pub dry_run: bool, } /// A transaction's host context @@ -165,6 +168,7 @@ where #[allow(clippy::too_many_arguments)] pub fn new( memory: MEM, + dry_run: bool, write_log: &mut WriteLog, in_mem: &InMemory, db: &D, @@ -220,7 +224,11 @@ where cache_access: std::marker::PhantomData, }; - Self { memory, ctx } + Self { + memory, + ctx, + dry_run, + } } /// Access state from within a tx @@ -240,6 +248,7 @@ where Self { memory: self.memory.clone(), ctx: self.ctx.clone(), + dry_run: self.dry_run, } } } @@ -320,6 +329,9 @@ where pub memory: MEM, /// The VP context contains references to host structures. pub ctx: VpCtx, + /// Indicates whether we are executing VPs as part of a + /// dry-run of a transaction or not. + pub dry_run: bool, } /// A validity predicate's host context @@ -407,6 +419,7 @@ where #[allow(clippy::too_many_arguments)] pub fn new( memory: MEM, + dry_run: bool, address: &Address, write_log: &WriteLog, in_mem: &InMemory, @@ -442,7 +455,11 @@ where vp_wasm_cache, ); - Self { memory, ctx } + Self { + memory, + ctx, + dry_run, + } } /// Access state from within a VP @@ -463,6 +480,7 @@ where Self { memory: self.memory.clone(), ctx: self.ctx.clone(), + dry_run: self.dry_run, } } } @@ -1964,17 +1982,31 @@ where let tx = unsafe { env.ctx.tx.get() }; - match tx.verify_signatures( - &HashSet::from_iter(hashes), - public_keys_map, - &Some(signer), - threshold, - || { - gas_meter - .borrow_mut() - .consume(gas::VERIFY_TX_SIG_GAS.into()) - }, - ) { + match if !env.dry_run { + tx.verify_signatures( + &HashSet::from_iter(hashes), + public_keys_map, + &Some(signer), + threshold, + || { + gas_meter + .borrow_mut() + .consume(gas::VERIFY_TX_SIG_GAS.into()) + }, + ) + } else { + tx.dry_run_signatures( + &HashSet::from_iter(hashes), + public_keys_map, + &Some(signer), + threshold, + || { + gas_meter + .borrow_mut() + .consume(gas::VERIFY_TX_SIG_GAS.into()) + }, + ) + } { Ok(_) => Ok(()), Err(err) => match err { namada_tx::VerifySigError::Gas(inner) => { @@ -2120,17 +2152,31 @@ where let tx = unsafe { env.ctx.tx.get() }; let (gas_meter, sentinel) = env.ctx.gas_meter_and_sentinel(); - match tx.verify_signatures( - &HashSet::from_iter(hashes), - public_keys_map, - &None, - threshold, - || { - gas_meter - .borrow_mut() - .consume(gas::VERIFY_TX_SIG_GAS.into()) - }, - ) { + match if !env.dry_run { + tx.verify_signatures( + &HashSet::from_iter(hashes), + public_keys_map, + &None, + threshold, + || { + gas_meter + .borrow_mut() + .consume(gas::VERIFY_TX_SIG_GAS.into()) + }, + ) + } else { + tx.dry_run_signatures( + &HashSet::from_iter(hashes), + public_keys_map, + &None, + threshold, + || { + gas_meter + .borrow_mut() + .consume(gas::VERIFY_TX_SIG_GAS.into()) + }, + ) + } { Ok(_) => Ok(HostEnvResult::Success.to_i64()), Err(err) => match err { namada_tx::VerifySigError::Gas(inner) => { @@ -2386,6 +2432,7 @@ pub mod testing { let (write_log, in_mem, db) = state.split_borrow(); TxVmEnv::new( NativeMemory, + false, write_log, in_mem, db, @@ -2436,6 +2483,7 @@ pub mod testing { let (write_log, in_mem, db) = state.split_borrow(); let mut env = TxVmEnv::new( WasmMemory::new(Rc::downgrade(&store)), + false, write_log, in_mem, db, @@ -2483,6 +2531,7 @@ pub mod testing { { VpVmEnv::new( NativeMemory, + false, address, state.write_log(), state.in_mem(), diff --git a/crates/vm/src/wasm/run.rs b/crates/vm/src/wasm/run.rs index 588bb7a9af3..7d8d7fefc31 100644 --- a/crates/vm/src/wasm/run.rs +++ b/crates/vm/src/wasm/run.rs @@ -157,6 +157,7 @@ pub fn tx( vp_wasm_cache: &mut VpCache, tx_wasm_cache: &mut TxCache, gas_meter_kind: GasMeterKind, + dry_run: bool, ) -> Result> where S: StateRead + State + StorageRead, @@ -241,6 +242,7 @@ where let mut env = TxVmEnv::new( WasmMemory::new(Rc::downgrade(&store)), + dry_run, write_log, in_mem, db, @@ -389,6 +391,7 @@ pub fn vp( verifiers: &BTreeSet
, mut vp_wasm_cache: VpCache, gas_meter_kind: GasMeterKind, + dry_run: bool, ) -> Result<()> where S: StateRead, @@ -424,6 +427,7 @@ where let mut env = VpVmEnv::new( WasmMemory::new(Rc::downgrade(&store)), + dry_run, address, state.write_log(), state.in_mem(), @@ -849,6 +853,7 @@ where let mut env = VpVmEnv { memory: WasmMemory::new(Rc::downgrade(&store)), ctx, + dry_run: false, }; let yielded_value_borrow = env.ctx.yielded_value; let imports = { @@ -1604,6 +1609,7 @@ mod tests { &mut vp_cache, &mut tx_cache, GasMeterKind::MutGlobal, + false, ); assert!(result.is_ok(), "Expected success, got {:?}", result); @@ -1624,6 +1630,7 @@ mod tests { &mut vp_cache, &mut tx_cache, GasMeterKind::MutGlobal, + false, ) .expect_err("Expected to run out of memory"); @@ -1697,6 +1704,7 @@ mod tests { &verifiers, vp_cache.clone(), GasMeterKind::MutGlobal, + false, ) .is_ok() ); @@ -1730,6 +1738,7 @@ mod tests { &verifiers, vp_cache, GasMeterKind::MutGlobal, + false, ) .is_err() ); @@ -1781,6 +1790,7 @@ mod tests { &verifiers, vp_cache.clone(), GasMeterKind::MutGlobal, + false, ); assert!(result.is_ok(), "Expected success, got {:?}", result); @@ -1801,6 +1811,7 @@ mod tests { &verifiers, vp_cache, GasMeterKind::MutGlobal, + false, ) .expect_err("Expected to run out of memory"); @@ -1852,6 +1863,7 @@ mod tests { &mut vp_cache, &mut tx_cache, GasMeterKind::MutGlobal, + false, ); // Depending on platform, we get a different error from the running out // of memory @@ -1909,6 +1921,7 @@ mod tests { &verifiers, vp_cache, GasMeterKind::MutGlobal, + false, ); // Depending on platform, we get a different error from the running out // of memory @@ -1971,6 +1984,7 @@ mod tests { &mut vp_cache, &mut tx_cache, GasMeterKind::MutGlobal, + false, ) .expect_err("Expected to run out of memory"); @@ -2029,6 +2043,7 @@ mod tests { &verifiers, vp_cache, GasMeterKind::MutGlobal, + false, ) .expect_err("Expected to run out of memory"); @@ -2107,6 +2122,7 @@ mod tests { &verifiers, vp_cache, GasMeterKind::MutGlobal, + false ) .is_err() ); @@ -2218,6 +2234,7 @@ mod tests { &mut vp_cache, &mut tx_cache, GasMeterKind::MutGlobal, + false, ); assert!(matches!(result.unwrap_err(), Error::GasError(_))); @@ -2260,6 +2277,7 @@ mod tests { &mut vp_cache, &mut tx_cache, GasMeterKind::MutGlobal, + false, ); assert!(matches!(result.unwrap_err(), Error::GasError(_))); @@ -2304,6 +2322,7 @@ mod tests { &verifiers, vp_cache.clone(), GasMeterKind::MutGlobal, + false, ); assert!(matches!(result.unwrap_err(), Error::GasError(_))); @@ -2349,6 +2368,7 @@ mod tests { &verifiers, vp_cache.clone(), GasMeterKind::MutGlobal, + false, ); assert!(matches!(result.unwrap_err(), Error::GasError(_))); @@ -2525,6 +2545,7 @@ mod tests { &verifiers, vp_cache.clone(), GasMeterKind::MutGlobal, + false, ) } @@ -2572,6 +2593,7 @@ mod tests { vp_cache, tx_cache, GasMeterKind::MutGlobal, + false, ) } @@ -2627,6 +2649,7 @@ mod tests { &mut vp_cache, &mut tx_cache, GasMeterKind::MutGlobal, + false, ) .unwrap();