diff --git a/crates/namada/src/ledger/governance/mod.rs b/crates/namada/src/ledger/governance/mod.rs index 5152e5bd9b..36cfa36448 100644 --- a/crates/namada/src/ledger/governance/mod.rs +++ b/crates/namada/src/ledger/governance/mod.rs @@ -632,8 +632,8 @@ where if !is_valid_activation_epoch { let error = native_vp::Error::new_alloc(format!( "Expected min duration between the end and grace epoch \ - {min_grace_epoch}, but got grace = {grace_epoch}, end = \ - {end_epoch}", + {min_grace_epochs}, but got activation = {activation_epoch}, \ + end = {end_epoch}", )) .into(); tracing::info!("{error}"); @@ -644,8 +644,8 @@ where if !is_valid_max_proposal_period { let error = native_vp::Error::new_alloc(format!( "Expected max duration between the start and grace epoch \ - {max_proposal_period}, but got grace = {grace_epoch}, start \ - = {start_epoch}", + {max_proposal_period}, but got activation = \ + {activation_epoch}, start = {start_epoch}", )) .into(); tracing::info!("{error}"); @@ -1206,7 +1206,6 @@ mod test { use namada_sdk::key::RefTo; use namada_sdk::time::DateTimeUtc; use namada_sdk::token; - use namada_sdk::validity_predicate::VpSentinel; use namada_state::mockdb::MockDB; use namada_state::testing::TestState; use namada_state::{ @@ -1261,7 +1260,6 @@ mod test { wasm::compilation_cache::common::testing::cache(); let tx_index = TxIndex::default(); - let sentinel = RefCell::new(VpSentinel::default()); let signer = keypair_1(); let signer_address = Address::from(&signer.clone().ref_to()); @@ -1286,7 +1284,6 @@ mod test { &tx, &tx_index, &gas_meter, - &sentinel, &keys_changed, &verifiers, vp_wasm_cache, @@ -1294,10 +1291,9 @@ mod test { let governance_vp = GovernanceVp { ctx }; // this should return true because state has been stored - assert!( - governance_vp - .validate_tx(&tx, &keys_changed, &verifiers) - .expect("validation failed") + assert_matches!( + governance_vp.validate_tx(&tx, &keys_changed, &verifiers), + Ok(_) ); } @@ -1488,7 +1484,6 @@ mod test { wasm::compilation_cache::common::testing::cache(); let tx_index = TxIndex::default(); - let sentinel = RefCell::new(VpSentinel::default()); let signer = keypair_1(); let signer_address = Address::from(&signer.clone().ref_to()); @@ -1536,7 +1531,6 @@ mod test { &tx, &tx_index, &gas_meter, - &sentinel, &keys_changed, &verifiers, vp_wasm_cache, @@ -1544,10 +1538,9 @@ mod test { let governance_vp = GovernanceVp { ctx }; // this should return true because state has been stored - assert!( - governance_vp - .validate_tx(&tx, &keys_changed, &verifiers) - .expect("validation failed") + assert_matches!( + governance_vp.validate_tx(&tx, &keys_changed, &verifiers), + Ok(_) ); state.write_log_mut().commit_tx(); @@ -1585,7 +1578,6 @@ mod test { wasm::compilation_cache::common::testing::cache(); let tx_index = TxIndex::default(); - let sentinel = RefCell::new(VpSentinel::default()); let signer = keypair_1(); let signer_address = Address::from(&signer.clone().ref_to()); @@ -1633,20 +1625,17 @@ mod test { &tx, &tx_index, &gas_meter, - &sentinel, &keys_changed, &verifiers, vp_wasm_cache, ); let governance_vp = GovernanceVp { ctx }; - // this should return true because state has been stored - let vp_result = governance_vp - .validate_tx(&tx, &keys_changed, &verifiers) - .expect("validation failed"); - assert!(!vp_result); + let result = governance_vp.validate_tx(&tx, &keys_changed, &verifiers); + // this should fail + assert_matches!(&result, Err(_)); - if !vp_result { + if result.is_err() { state.write_log_mut().drop_tx(); } else { state.write_log_mut().commit_tx(); @@ -1685,7 +1674,6 @@ mod test { wasm::compilation_cache::common::testing::cache(); let tx_index = TxIndex::default(); - let sentinel = RefCell::new(VpSentinel::default()); let signer = keypair_1(); let signer_address = Address::from(&signer.clone().ref_to()); @@ -1733,19 +1721,16 @@ mod test { &tx, &tx_index, &gas_meter, - &sentinel, &keys_changed, &verifiers, vp_wasm_cache, ); let governance_vp = GovernanceVp { ctx }; - let vp_result = governance_vp - .validate_tx(&tx, &keys_changed, &verifiers) - .expect("validation failed"); - assert!(vp_result); + let result = governance_vp.validate_tx(&tx, &keys_changed, &verifiers); + assert_matches!(&result, Ok(_)); - if !vp_result { + if result.is_err() { state.write_log_mut().drop_tx(); } else { state.write_log_mut().commit_tx(); @@ -1784,7 +1769,6 @@ mod test { wasm::compilation_cache::common::testing::cache(); let tx_index = TxIndex::default(); - let sentinel = RefCell::new(VpSentinel::default()); let signer = keypair_1(); let signer_address = Address::from(&signer.clone().ref_to()); @@ -1832,7 +1816,6 @@ mod test { &tx, &tx_index, &gas_meter, - &sentinel, &keys_changed, &verifiers, vp_wasm_cache, @@ -1840,10 +1823,9 @@ mod test { let governance_vp = GovernanceVp { ctx }; // this should return true because state has been stored - assert!( - !governance_vp - .validate_tx(&tx, &keys_changed, &verifiers) - .expect("validation failed") + assert_matches!( + governance_vp.validate_tx(&tx, &keys_changed, &verifiers), + Err(_) ); } @@ -1863,7 +1845,6 @@ mod test { wasm::compilation_cache::common::testing::cache(); let tx_index = TxIndex::default(); - let sentinel = RefCell::new(VpSentinel::default()); let signer = keypair_1(); let signer_address = Address::from(&signer.clone().ref_to()); @@ -1911,7 +1892,6 @@ mod test { &tx, &tx_index, &gas_meter, - &sentinel, &keys_changed, &verifiers, vp_wasm_cache, @@ -1919,10 +1899,9 @@ mod test { let governance_vp = GovernanceVp { ctx }; // this should return true because state has been stored - assert!( - !governance_vp - .validate_tx(&tx, &keys_changed, &verifiers) - .expect("validation failed") + assert_matches!( + governance_vp.validate_tx(&tx, &keys_changed, &verifiers), + Err(_) ); } @@ -1942,7 +1921,6 @@ mod test { wasm::compilation_cache::common::testing::cache(); let tx_index = TxIndex::default(); - let sentinel = RefCell::new(VpSentinel::default()); let signer = keypair_1(); let signer_address = Address::from(&signer.clone().ref_to()); @@ -1990,7 +1968,6 @@ mod test { &tx, &tx_index, &gas_meter, - &sentinel, &keys_changed, &verifiers, vp_wasm_cache, @@ -1998,10 +1975,9 @@ mod test { let governance_vp = GovernanceVp { ctx }; // this should return true because state has been stored - assert!( - !governance_vp - .validate_tx(&tx, &keys_changed, &verifiers) - .expect("validation failed") + assert_matches!( + governance_vp.validate_tx(&tx, &keys_changed, &verifiers), + Err(_) ); } @@ -2039,7 +2015,6 @@ mod test { wasm::compilation_cache::common::testing::cache(); let tx_index = TxIndex::default(); - let sentinel = RefCell::new(VpSentinel::default()); let signer = keypair_1(); let signer_address = Address::from(&signer.clone().ref_to()); @@ -2087,7 +2062,6 @@ mod test { &tx, &tx_index, &gas_meter, - &sentinel, &keys_changed, &verifiers, vp_wasm_cache, @@ -2095,10 +2069,9 @@ mod test { let governance_vp = GovernanceVp { ctx }; // this should return true because state has been stored - assert!( - !governance_vp - .validate_tx(&tx, &keys_changed, &verifiers) - .expect("validation failed") + assert_matches!( + governance_vp.validate_tx(&tx, &keys_changed, &verifiers), + Err(_) ); } @@ -2136,7 +2109,6 @@ mod test { wasm::compilation_cache::common::testing::cache(); let tx_index = TxIndex::default(); - let sentinel = RefCell::new(VpSentinel::default()); let signer = keypair_1(); let signer_address = Address::from(&signer.clone().ref_to()); @@ -2184,7 +2156,6 @@ mod test { &tx, &tx_index, &gas_meter, - &sentinel, &keys_changed, &verifiers, vp_wasm_cache, @@ -2192,10 +2163,9 @@ mod test { let governance_vp = GovernanceVp { ctx }; // this should return true because state has been stored - assert!( - !governance_vp - .validate_tx(&tx, &keys_changed, &verifiers) - .expect("validation failed") + assert_matches!( + governance_vp.validate_tx(&tx, &keys_changed, &verifiers), + Err(_) ); } @@ -2215,7 +2185,6 @@ mod test { wasm::compilation_cache::common::testing::cache(); let tx_index = TxIndex::default(); - let sentinel = RefCell::new(VpSentinel::default()); let signer = keypair_1(); let signer_address = Address::from(&signer.clone().ref_to()); @@ -2263,7 +2232,6 @@ mod test { &tx, &tx_index, &gas_meter, - &sentinel, &keys_changed, &verifiers, vp_wasm_cache.clone(), @@ -2271,10 +2239,9 @@ mod test { let governance_vp = GovernanceVp { ctx }; // this should return true because state has been stored - assert!( - governance_vp - .validate_tx(&tx, &keys_changed, &verifiers) - .expect("validation failed") + assert_matches!( + governance_vp.validate_tx(&tx, &keys_changed, &verifiers), + Ok(_) ); state.write_log_mut().commit_tx(); @@ -2308,7 +2275,6 @@ mod test { &tx, &tx_index, &gas_meter, - &sentinel, &keys_changed, &verifiers, vp_wasm_cache, @@ -2316,10 +2282,9 @@ mod test { let governance_vp = GovernanceVp { ctx }; - assert!( - governance_vp - .validate_tx(&tx, &keys_changed, &verifiers) - .expect("validation failed") + assert_matches!( + governance_vp.validate_tx(&tx, &keys_changed, &verifiers), + Ok(_) ); } @@ -2339,7 +2304,6 @@ mod test { wasm::compilation_cache::common::testing::cache(); let tx_index = TxIndex::default(); - let sentinel = RefCell::new(VpSentinel::default()); let signer = keypair_1(); let signer_address = Address::from(&signer.clone().ref_to()); @@ -2387,7 +2351,6 @@ mod test { &tx, &tx_index, &gas_meter, - &sentinel, &keys_changed, &verifiers, vp_wasm_cache.clone(), @@ -2395,10 +2358,9 @@ mod test { let governance_vp = GovernanceVp { ctx }; // this should return true because state has been stored - assert!( - governance_vp - .validate_tx(&tx, &keys_changed, &verifiers) - .expect("validation failed") + assert_matches!( + governance_vp.validate_tx(&tx, &keys_changed, &verifiers), + Ok(_) ); state.write_log_mut().commit_tx(); @@ -2432,7 +2394,6 @@ mod test { &tx, &tx_index, &gas_meter, - &sentinel, &keys_changed, &verifiers, vp_wasm_cache, @@ -2440,10 +2401,9 @@ mod test { let governance_vp = GovernanceVp { ctx }; - assert!( - !governance_vp - .validate_tx(&tx, &keys_changed, &verifiers) - .expect("validation failed") + assert_matches!( + governance_vp.validate_tx(&tx, &keys_changed, &verifiers), + Err(_) ); } @@ -2463,7 +2423,6 @@ mod test { wasm::compilation_cache::common::testing::cache(); let tx_index = TxIndex::default(); - let sentinel = RefCell::new(VpSentinel::default()); let signer = keypair_1(); let signer_address = Address::from(&signer.clone().ref_to()); @@ -2511,7 +2470,6 @@ mod test { &tx, &tx_index, &gas_meter, - &sentinel, &keys_changed, &verifiers, vp_wasm_cache.clone(), @@ -2519,10 +2477,9 @@ mod test { let governance_vp = GovernanceVp { ctx }; // this should return true because state has been stored - assert!( - governance_vp - .validate_tx(&tx, &keys_changed, &verifiers) - .expect("validation failed") + assert_matches!( + governance_vp.validate_tx(&tx, &keys_changed, &verifiers), + Ok(_) ); state.write_log_mut().commit_tx(); @@ -2556,7 +2513,6 @@ mod test { &tx, &tx_index, &gas_meter, - &sentinel, &keys_changed, &verifiers, vp_wasm_cache, @@ -2564,10 +2520,9 @@ mod test { let governance_vp = GovernanceVp { ctx }; - assert!( - !governance_vp - .validate_tx(&tx, &keys_changed, &verifiers) - .expect("validation failed") + assert_matches!( + governance_vp.validate_tx(&tx, &keys_changed, &verifiers), + Err(_) ); } @@ -2587,7 +2542,6 @@ mod test { wasm::compilation_cache::common::testing::cache(); let tx_index = TxIndex::default(); - let sentinel = RefCell::new(VpSentinel::default()); let signer = keypair_1(); let signer_address = Address::from(&signer.clone().ref_to()); @@ -2635,7 +2589,6 @@ mod test { &tx, &tx_index, &gas_meter, - &sentinel, &keys_changed, &verifiers, vp_wasm_cache.clone(), @@ -2643,10 +2596,9 @@ mod test { let governance_vp = GovernanceVp { ctx }; - assert!( - governance_vp - .validate_tx(&tx, &keys_changed, &verifiers) - .expect("validation failed") + assert_matches!( + governance_vp.validate_tx(&tx, &keys_changed, &verifiers), + Ok(_) ); state.write_log_mut().commit_tx(); @@ -2697,7 +2649,6 @@ mod test { &tx, &tx_index, &gas_meter, - &sentinel, &keys_changed, &verifiers, vp_wasm_cache, @@ -2705,10 +2656,9 @@ mod test { let governance_vp = GovernanceVp { ctx }; - assert!( - governance_vp - .validate_tx(&tx, &keys_changed, &verifiers) - .expect("validation failed") + assert_matches!( + governance_vp.validate_tx(&tx, &keys_changed, &verifiers), + Ok(_) ); } @@ -2728,7 +2678,6 @@ mod test { wasm::compilation_cache::common::testing::cache(); let tx_index = TxIndex::default(); - let sentinel = RefCell::new(VpSentinel::default()); let signer = keypair_1(); let signer_address = Address::from(&signer.clone().ref_to()); @@ -2776,7 +2725,6 @@ mod test { &tx, &tx_index, &gas_meter, - &sentinel, &keys_changed, &verifiers, vp_wasm_cache.clone(), @@ -2784,10 +2732,9 @@ mod test { let governance_vp = GovernanceVp { ctx }; - assert!( - governance_vp - .validate_tx(&tx, &keys_changed, &verifiers) - .expect("validation failed") + assert_matches!( + governance_vp.validate_tx(&tx, &keys_changed, &verifiers), + Ok(_) ); state.write_log_mut().commit_tx(); @@ -2838,7 +2785,6 @@ mod test { &tx, &tx_index, &gas_meter, - &sentinel, &keys_changed, &verifiers, vp_wasm_cache, @@ -2846,10 +2792,9 @@ mod test { let governance_vp = GovernanceVp { ctx }; - assert!( - !governance_vp - .validate_tx(&tx, &keys_changed, &verifiers) - .expect("validation failed") + assert_matches!( + governance_vp.validate_tx(&tx, &keys_changed, &verifiers), + Err(_) ); } @@ -2869,7 +2814,6 @@ mod test { wasm::compilation_cache::common::testing::cache(); let tx_index = TxIndex::default(); - let sentinel = RefCell::new(VpSentinel::default()); let signer = keypair_1(); let signer_address = Address::from(&signer.clone().ref_to()); @@ -2917,7 +2861,6 @@ mod test { &tx, &tx_index, &gas_meter, - &sentinel, &keys_changed, &verifiers, vp_wasm_cache.clone(), @@ -2925,10 +2868,9 @@ mod test { let governance_vp = GovernanceVp { ctx }; - assert!( - governance_vp - .validate_tx(&tx, &keys_changed, &verifiers) - .expect("validation failed") + assert_matches!( + governance_vp.validate_tx(&tx, &keys_changed, &verifiers), + Ok(_) ); state.write_log_mut().commit_tx(); @@ -2979,7 +2921,6 @@ mod test { &tx, &tx_index, &gas_meter, - &sentinel, &keys_changed, &verifiers, vp_wasm_cache, @@ -2987,10 +2928,9 @@ mod test { let governance_vp = GovernanceVp { ctx }; - assert!( - !governance_vp - .validate_tx(&tx, &keys_changed, &verifiers) - .expect("validation failed") + assert_matches!( + governance_vp.validate_tx(&tx, &keys_changed, &verifiers), + Err(_) ); } } diff --git a/crates/namada/src/ledger/native_vp/ethereum_bridge/bridge_pool_vp.rs b/crates/namada/src/ledger/native_vp/ethereum_bridge/bridge_pool_vp.rs index 1f1fb423e5..1e7e7cf388 100644 --- a/crates/namada/src/ledger/native_vp/ethereum_bridge/bridge_pool_vp.rs +++ b/crates/namada/src/ledger/native_vp/ethereum_bridge/bridge_pool_vp.rs @@ -554,7 +554,10 @@ where tracing::debug!( "Rejecting transaction, since the Ethereum bridge is disabled." ); - return Ok(false); + return Err(native_vp::Error::SimpleMessage( + "Rejecting transaction, since the Ethereum bridge is disabled.", + ) + .into()); } let Some(tx_data) = tx.data() else { return Err(native_vp::Error::SimpleMessage( diff --git a/crates/namada/src/ledger/native_vp/ibc/mod.rs b/crates/namada/src/ledger/native_vp/ibc/mod.rs index 0641eda30a..3e83ecf737 100644 --- a/crates/namada/src/ledger/native_vp/ibc/mod.rs +++ b/crates/namada/src/ledger/native_vp/ibc/mod.rs @@ -909,9 +909,9 @@ mod tests { let ibc = Ibc { ctx }; // this should return true because state has been stored - assert!( - ibc.validate_tx(&outer_tx, &keys_changed, &verifiers) - .is_ok() + assert_matches!( + ibc.validate_tx(&outer_tx, &keys_changed, &verifiers), + Ok(_) ); } @@ -1110,7 +1110,7 @@ mod tests { ); let ibc = Ibc { ctx }; // this should return true because state has been stored - assert!(ibc.validate_tx(&tx, &keys_changed, &verifiers).is_ok()); + assert_matches!(ibc.validate_tx(&tx, &keys_changed, &verifiers), Ok(_)); } #[test] @@ -1432,7 +1432,7 @@ mod tests { ); let ibc = Ibc { ctx }; // this should return true because state has been stored - assert!(ibc.validate_tx(&tx, &keys_changed, &verifiers).is_ok()); + assert_matches!(ibc.validate_tx(&tx, &keys_changed, &verifiers), Ok(_)); } #[test] @@ -1539,9 +1539,9 @@ mod tests { vp_wasm_cache, ); let ibc = Ibc { ctx }; - assert!( - ibc.validate_tx(&outer_tx, &keys_changed, &verifiers) - .is_ok() + assert_matches!( + ibc.validate_tx(&outer_tx, &keys_changed, &verifiers), + Ok(_) ); } @@ -1635,9 +1635,9 @@ mod tests { vp_wasm_cache, ); let ibc = Ibc { ctx }; - assert!( - ibc.validate_tx(&outer_tx, &keys_changed, &verifiers) - .is_ok() + assert_matches!( + ibc.validate_tx(&outer_tx, &keys_changed, &verifiers), + Ok(_) ); } @@ -1759,9 +1759,9 @@ mod tests { vp_wasm_cache, ); let ibc = Ibc { ctx }; - assert!( - ibc.validate_tx(&outer_tx, &keys_changed, &verifiers) - .is_ok() + assert_matches!( + ibc.validate_tx(&outer_tx, &keys_changed, &verifiers), + Ok(_) ); } @@ -1882,9 +1882,9 @@ mod tests { vp_wasm_cache, ); let ibc = Ibc { ctx }; - assert!( - ibc.validate_tx(&outer_tx, &keys_changed, &verifiers) - .is_ok() + assert_matches!( + ibc.validate_tx(&outer_tx, &keys_changed, &verifiers), + Ok(_) ); } @@ -1990,9 +1990,9 @@ mod tests { vp_wasm_cache, ); let ibc = Ibc { ctx }; - assert!( - ibc.validate_tx(&outer_tx, &keys_changed, &verifiers) - .is_ok() + assert_matches!( + ibc.validate_tx(&outer_tx, &keys_changed, &verifiers), + Ok(_) ); } @@ -2093,7 +2093,7 @@ mod tests { vp_wasm_cache, ); let ibc = Ibc { ctx }; - assert!(ibc.validate_tx(&tx, &keys_changed, &verifiers).is_ok()); + assert_matches!(ibc.validate_tx(&tx, &keys_changed, &verifiers), Ok(_)); } // skip test_close_init_channel() and test_close_confirm_channel() since it @@ -2247,7 +2247,7 @@ mod tests { vp_wasm_cache, ); let ibc = Ibc { ctx }; - assert!(ibc.validate_tx(&tx, &keys_changed, &verifiers).is_ok()); + assert_matches!(ibc.validate_tx(&tx, &keys_changed, &verifiers), Ok(_)); } #[test] @@ -2456,7 +2456,7 @@ mod tests { vp_wasm_cache, ); let ibc = Ibc { ctx }; - assert!(ibc.validate_tx(&tx, &keys_changed, &verifiers).is_ok()); + assert_matches!(ibc.validate_tx(&tx, &keys_changed, &verifiers), Ok(_)); } #[test] @@ -2599,7 +2599,7 @@ mod tests { vp_wasm_cache, ); let ibc = Ibc { ctx }; - assert!(ibc.validate_tx(&tx, &keys_changed, &verifiers).is_ok()); + assert_matches!(ibc.validate_tx(&tx, &keys_changed, &verifiers), Ok(_)); } #[test] @@ -2759,7 +2759,7 @@ mod tests { vp_wasm_cache, ); let ibc = Ibc { ctx }; - assert!(ibc.validate_tx(&tx, &keys_changed, &verifiers).is_ok()); + assert_matches!(ibc.validate_tx(&tx, &keys_changed, &verifiers), Ok(_)); } #[test] @@ -2920,7 +2920,7 @@ mod tests { vp_wasm_cache, ); let ibc = Ibc { ctx }; - assert!(ibc.validate_tx(&tx, &keys_changed, &verifiers).is_ok()); + assert_matches!(ibc.validate_tx(&tx, &keys_changed, &verifiers), Ok(_)); } #[test] @@ -3079,23 +3079,18 @@ mod tests { wasm::compilation_cache::common::testing::cache(); let verifiers = BTreeSet::new(); - let sentinel = RefCell::new(VpSentinel::default()); let ctx = Ctx::new( &ADDRESS, &state, &tx, &tx_index, &gas_meter, - &sentinel, &keys_changed, &verifiers, vp_wasm_cache, ); let ibc = Ibc { ctx }; - assert!( - ibc.validate_tx(&tx, &keys_changed, &verifiers) - .expect("validation failed") - ); + assert_matches!(ibc.validate_tx(&tx, &keys_changed, &verifiers), Ok(_)); } #[test] @@ -3316,22 +3311,17 @@ mod tests { wasm::compilation_cache::common::testing::cache(); let verifiers = BTreeSet::new(); - let sentinel = RefCell::new(VpSentinel::default()); let ctx = Ctx::new( &ADDRESS, &state, &tx, &tx_index, &gas_meter, - &sentinel, &keys_changed, &verifiers, vp_wasm_cache, ); let ibc = Ibc { ctx }; - assert!( - ibc.validate_tx(&tx, &keys_changed, &verifiers) - .expect("validation failed") - ); + assert_matches!(ibc.validate_tx(&tx, &keys_changed, &verifiers), Ok(_)); } } diff --git a/crates/namada/src/ledger/native_vp/masp.rs b/crates/namada/src/ledger/native_vp/masp.rs index bab703cc23..ad8989a628 100644 --- a/crates/namada/src/ledger/native_vp/masp.rs +++ b/crates/namada/src/ledger/native_vp/masp.rs @@ -810,13 +810,8 @@ where _ => {} } - // TODO: propagate errors inside of `verify_shielded_tx` - // up to the client - verify_shielded_tx(&shielded_tx).ok_or_else(|| { - native_vp::Error::new_const( - "Verification failed on the shielded tx", - ) - .into() - }) + // Verify the proofs + verify_shielded_tx(&shielded_tx, |gas| self.ctx.charge_gas(gas)) + .map_err(Error::NativeVpError) } } diff --git a/crates/namada/src/ledger/native_vp/multitoken.rs b/crates/namada/src/ledger/native_vp/multitoken.rs index 22d4281cea..c005a9a168 100644 --- a/crates/namada/src/ledger/native_vp/multitoken.rs +++ b/crates/namada/src/ledger/native_vp/multitoken.rs @@ -86,7 +86,11 @@ where tracing::debug!( "Native token deposit isn't allowed" ); - return Ok(false); + return Err(Error::NativeVpError( + native_vp::Error::SimpleMessage( + "Native token deposit isn't allowed", + ), + )); } let change = inc_changes.entry(token.clone()).or_default(); @@ -104,7 +108,11 @@ where tracing::debug!( "Native token withdraw isn't allowed" ); - return Ok(false); + return Err(Error::NativeVpError( + native_vp::Error::SimpleMessage( + "Native token deposit isn't allowed", + ), + )); } let diff = pre .checked_sub(post) @@ -126,7 +134,11 @@ where tracing::debug!( "Minting/Burning native token isn't allowed" ); - return Ok(false); + return Err(Error::NativeVpError( + native_vp::Error::SimpleMessage( + "Minting/Burning native token isn't allowed", + ), + )); } let pre: Amount = self.ctx.read_pre(key)?.unwrap_or_default(); @@ -281,6 +293,7 @@ where mod tests { use std::cell::RefCell; + use assert_matches::assert_matches; use borsh_ext::BorshSerializeExt; use namada_gas::TxGasMeter; use namada_parameters::storage::get_native_token_transferable_key; @@ -739,24 +752,19 @@ mod tests { let (vp_wasm_cache, _vp_cache_dir) = wasm_cache(); let mut verifiers = BTreeSet::new(); verifiers.insert(src); - let sentinel = RefCell::new(VpSentinel::default()); let ctx = Ctx::new( &ADDRESS, &state, &tx, &tx_index, &gas_meter, - &sentinel, &keys_changed, &verifiers, vp_wasm_cache, ); let vp = MultitokenVp { ctx }; - assert!( - !vp.validate_tx(&tx, &keys_changed, &verifiers) - .expect("validation failed") - ); + assert_matches!(vp.validate_tx(&tx, &keys_changed, &verifiers), Err(_)); } #[test] @@ -778,24 +786,19 @@ mod tests { let (vp_wasm_cache, _vp_cache_dir) = wasm_cache(); let mut verifiers = BTreeSet::new(); verifiers.insert(src); - let sentinel = RefCell::new(VpSentinel::default()); let ctx = Ctx::new( &ADDRESS, &state, &tx, &tx_index, &gas_meter, - &sentinel, &keys_changed, &verifiers, vp_wasm_cache, ); let vp = MultitokenVp { ctx }; - assert!( - vp.validate_tx(&tx, &keys_changed, &verifiers) - .expect("validation failed") - ); + assert_matches!(vp.validate_tx(&tx, &keys_changed, &verifiers), Ok(_)); } #[test] @@ -817,23 +820,18 @@ mod tests { let (vp_wasm_cache, _vp_cache_dir) = wasm_cache(); let mut verifiers = BTreeSet::new(); verifiers.insert(src); - let sentinel = RefCell::new(VpSentinel::default()); let ctx = Ctx::new( &ADDRESS, &state, &tx, &tx_index, &gas_meter, - &sentinel, &keys_changed, &verifiers, vp_wasm_cache, ); let vp = MultitokenVp { ctx }; - assert!( - !vp.validate_tx(&tx, &keys_changed, &verifiers) - .expect("validation failed") - ); + assert_matches!(vp.validate_tx(&tx, &keys_changed, &verifiers), Err(_)); } } diff --git a/crates/sdk/src/masp.rs b/crates/sdk/src/masp.rs index 089f8fa653..73d47363f2 100644 --- a/crates/sdk/src/masp.rs +++ b/crates/sdk/src/masp.rs @@ -64,6 +64,7 @@ use namada_ibc::IbcMessage; use namada_macros::BorshDeserializer; #[cfg(feature = "migrations")] use namada_migrations::*; +use namada_state::StorageError; use namada_token::{self as token, Denomination, MaspDigitPos, Transfer}; use namada_tx::data::{TxResult, WrapperTx}; use namada_tx::Tx; @@ -333,23 +334,27 @@ pub fn partial_deauthorize( pub fn verify_shielded_tx( transaction: &Transaction, mut consume_verify_gas: F, -) -> Result +) -> Result<(), StorageError> where - F: FnMut(u64) -> std::result::Result<(), namada_state::StorageError>, + F: FnMut(u64) -> std::result::Result<(), StorageError>, { tracing::info!("entered verify_shielded_tx()"); let sapling_bundle = if let Some(bundle) = transaction.sapling_bundle() { bundle } else { - return Ok(false); + return Err(StorageError::SimpleMessage("no sapling bundle")); }; let tx_data = transaction.deref(); // Partially deauthorize the transparent bundle let unauth_tx_data = match partial_deauthorize(tx_data) { Some(tx_data) => tx_data, - None => return Ok(false), + None => { + return Err(StorageError::SimpleMessage( + "Failed to partially de-authorize", + )); + } }; let txid_parts = unauth_tx_data.digest(TxIdDigester); @@ -374,19 +379,21 @@ where for spend in &sapling_bundle.shielded_spends { consume_verify_gas(namada_gas::MASP_VERIFY_SPEND_GAS)?; if !check_spend(spend, sighash.as_ref(), &mut ctx, spend_vk) { - return Ok(false); + return Err(StorageError::SimpleMessage("Invalid shielded spend")); } } for convert in &sapling_bundle.shielded_converts { consume_verify_gas(namada_gas::MASP_VERIFY_CONVERT_GAS)?; if !check_convert(convert, &mut ctx, convert_vk) { - return Ok(false); + return Err(StorageError::SimpleMessage( + "Invalid shielded conversion", + )); } } for output in &sapling_bundle.shielded_outputs { consume_verify_gas(namada_gas::MASP_VERIFY_OUTPUT_GAS)?; if !check_output(output, &mut ctx, output_vk) { - return Ok(false); + return Err(StorageError::SimpleMessage("Invalid shielded output")); } } @@ -406,7 +413,10 @@ where sapling_bundle.authorization.binding_sig, ); tracing::info!("final check result {result}"); - Ok(result) + if !result { + return Err(StorageError::SimpleMessage("MASP final check failed")); + } + Ok(()) } /// Get the path to MASP parameters from [`ENV_VAR_MASP_PARAMS_DIR`] env var or diff --git a/wasm/wasm_source/src/vp_user.rs b/wasm/wasm_source/src/vp_user.rs index 5dd7dfa8cb..a18b705350 100644 --- a/wasm/wasm_source/src/vp_user.rs +++ b/wasm/wasm_source/src/vp_user.rs @@ -221,30 +221,27 @@ fn validate_pos_changes( // Metadata changes must be signed by the validator whose // metadata is manipulated - let is_valid_metadata_change = || { - let is_valid = match is_validator_metadata_key(key) { - Some(address) => { - let metadata = ctx.post().read::(key)?; - let valid_len = if let Some(metadata) = metadata { - (metadata.len() as u64) <= MAX_VALIDATOR_METADATA_LEN - } else { - true - }; - if !valid_len { - return VpError::Erased( - "The metadata exceeds the maximum length".into(), - ); - } - gadget.borrow_mut().verify_signatures_when( - address == owner, - ctx, - tx_data, - owner, - ) + let is_valid_metadata_change = || match is_validator_metadata_key(key) { + Some(address) => { + let metadata = ctx.post().read::(key).into_vp_error()?; + let valid_len = if let Some(metadata) = metadata { + (metadata.len() as u64) <= MAX_VALIDATOR_METADATA_LEN + } else { + true + }; + if !valid_len { + return Err(VpError::Erased( + "The metadata exceeds the maximum length".into(), + )); } - None => VpError::Unspecified, - }; - VpResult::Ok(is_valid) + gadget.borrow_mut().verify_signatures_when( + || address == owner, + ctx, + tx_data, + owner, + ) + } + None => Err(VpError::Unspecified), }; // Changes in validator state