diff --git a/crates/core/app/src/action_handler/transaction.rs b/crates/core/app/src/action_handler/transaction.rs index 9219a8350b..b289c0938a 100644 --- a/crates/core/app/src/action_handler/transaction.rs +++ b/crates/core/app/src/action_handler/transaction.rs @@ -13,7 +13,9 @@ use super::AppActionHandler; mod stateful; mod stateless; -use self::stateful::{claimed_anchor_is_valid, fee_greater_than_base_fee, fmd_parameters_valid}; +use self::stateful::{ + claimed_anchor_is_valid, fmd_parameters_valid, tx_parameters_historical_check, +}; use stateless::{ check_memo_exists_if_outputs_absent_if_not, num_clues_equal_to_num_outputs, valid_binding_signature, @@ -56,14 +58,13 @@ impl AppActionHandler for Transaction { async fn check_historical(&self, state: Arc) -> Result<()> { let mut action_checks = JoinSet::new(); - // TODO: these could be pushed into the action checks and run concurrently if needed - + // SAFETY: Transaction parameters (chain id, expiry height, fee) against chain state + // that cannot change during transaction execution. + tx_parameters_historical_check(state.clone(), self).await?; // SAFETY: anchors are historical data and cannot change during transaction execution. claimed_anchor_is_valid(state.clone(), self).await?; // SAFETY: FMD parameters cannot change during transaction execution. fmd_parameters_valid(state.clone(), self).await?; - // SAFETY: gas prices cannot change during transaction execution. - fee_greater_than_base_fee(state.clone(), self).await?; // Currently, we need to clone the component actions so that the spawned // futures can have 'static lifetimes. In the future, we could try to diff --git a/crates/core/app/src/action_handler/transaction/stateful.rs b/crates/core/app/src/action_handler/transaction/stateful.rs index 7241d1a6a6..6a5a5c4fdb 100644 --- a/crates/core/app/src/action_handler/transaction/stateful.rs +++ b/crates/core/app/src/action_handler/transaction/stateful.rs @@ -6,10 +6,103 @@ use penumbra_sct::component::tree::VerificationExt; use penumbra_shielded_pool::component::StateReadExt as _; use penumbra_shielded_pool::fmd; use penumbra_transaction::gas::GasCost; -use penumbra_transaction::Transaction; +use penumbra_transaction::{Transaction, TransactionParameters}; + +use crate::app::StateReadExt; const FMD_GRACE_PERIOD_BLOCKS: u64 = 10; +pub async fn tx_parameters_historical_check( + state: S, + transaction: &Transaction, +) -> Result<()> { + let TransactionParameters { + chain_id, + expiry_height, + // This is checked in `fee_greater_than_base_fee` against the whole + // transaction, for convenience. + fee: _, + // IMPORTANT: Adding a transaction parameter? Then you **must** add a SAFETY + // argument here to justify why it is safe to validate against a historical + // state. + } = transaction.transaction_parameters(); + + // SAFETY: This is safe to do in a **historical** check because the chain's actual + // id cannot change during transaction processing. + chain_id_is_correct(&state, chain_id).await?; + // SAFETY: This is safe to do in a **historical** check because the chain's current + // block height cannot change during transaction processing. + expiry_height_is_valid(&state, expiry_height).await?; + // SAFETY: This is safe to do in a **historical** check as long as the current gas prices + // are static, or set in the previous block. + fee_greater_than_base_fee(&state, transaction).await?; + + Ok(()) +} + +pub async fn chain_id_is_correct(state: S, tx_chain_id: String) -> Result<()> { + let chain_id = state.get_chain_id().await?; + + // The chain ID in the transaction must exactly match the current chain ID. + ensure!( + tx_chain_id == chain_id, + "transaction chain ID '{}' must match the current chain ID '{}'", + tx_chain_id, + chain_id + ); + Ok(()) +} + +pub async fn expiry_height_is_valid(state: S, expiry_height: u64) -> Result<()> { + let current_height = state.get_block_height().await?; + + // A zero expiry height means that the transaction is valid indefinitely. + if expiry_height == 0 { + return Ok(()); + } + + // Otherwise, the expiry height must be greater than or equal to the current block height. + ensure!( + expiry_height >= current_height, + "transaction expiry height '{}' must be greater than or equal to the current block height '{}'", + expiry_height, + current_height + ); + + Ok(()) +} + +pub async fn fee_greater_than_base_fee( + state: S, + transaction: &Transaction, +) -> Result<()> { + let current_gas_prices = state + .get_gas_prices() + .await + .expect("gas prices must be present in state"); + + let transaction_base_price = current_gas_prices.fee(&transaction.gas_cost()); + let user_supplied_fee = transaction.transaction_body().transaction_parameters.fee; + let user_supplied_fee_amount = user_supplied_fee.amount(); + let user_supplied_fee_asset_id = user_supplied_fee.asset_id(); + + ensure!( + user_supplied_fee_amount >= transaction_base_price, + "fee must be greater than or equal to the transaction base price (supplied: {}, base: {})", + user_supplied_fee_amount, + transaction_base_price + ); + + // We split the check to provide granular error messages. + ensure!( + user_supplied_fee_asset_id == *penumbra_asset::STAKING_TOKEN_ASSET_ID, + "fee must be paid in staking tokens (found: {})", + user_supplied_fee_asset_id + ); + + Ok(()) +} + pub async fn fmd_parameters_valid(state: S, transaction: &Transaction) -> Result<()> { let previous_fmd_parameters = state .get_previous_fmd_parameters() @@ -78,34 +171,3 @@ pub async fn claimed_anchor_is_valid( ) -> Result<()> { state.check_claimed_anchor(transaction.anchor).await } - -pub async fn fee_greater_than_base_fee( - state: S, - transaction: &Transaction, -) -> Result<()> { - let current_gas_prices = state - .get_gas_prices() - .await - .expect("gas prices must be present in state"); - - let transaction_base_price = current_gas_prices.fee(&transaction.gas_cost()); - let user_supplied_fee = transaction.transaction_body().transaction_parameters.fee; - let user_supplied_fee_amount = user_supplied_fee.amount(); - let user_supplied_fee_asset_id = user_supplied_fee.asset_id(); - - ensure!( - user_supplied_fee_amount >= transaction_base_price, - "fee must be greater than or equal to the transaction base price (supplied: {}, base: {})", - user_supplied_fee_amount, - transaction_base_price - ); - - // We split the check to provide granular error messages. - ensure!( - user_supplied_fee_asset_id == *penumbra_asset::STAKING_TOKEN_ASSET_ID, - "fee must be paid in staking tokens (found: {})", - user_supplied_fee_asset_id - ); - - Ok(()) -} diff --git a/crates/core/app/src/genesis.rs b/crates/core/app/src/genesis.rs index 32d01944bf..8b07b24b68 100644 --- a/crates/core/app/src/genesis.rs +++ b/crates/core/app/src/genesis.rs @@ -173,6 +173,10 @@ impl DomainType for AppState { } impl Content { + pub fn with_chain_id(self, chain_id: String) -> Self { + Self { chain_id, ..self } + } + pub fn with_epoch_duration(self, epoch_duration: u64) -> Self { Self { sct_content: penumbra_sct::genesis::Content { diff --git a/crates/core/app/tests/app_can_define_and_delegate_to_a_validator.rs b/crates/core/app/tests/app_can_define_and_delegate_to_a_validator.rs index e6cf0fdc2d..f3ccc4d4a7 100644 --- a/crates/core/app/tests/app_can_define_and_delegate_to_a_validator.rs +++ b/crates/core/app/tests/app_can_define_and_delegate_to_a_validator.rs @@ -35,8 +35,11 @@ async fn app_can_define_and_delegate_to_a_validator() -> anyhow::Result<()> { let storage = TempStorage::new().await?; // Configure an AppState with slightly shorter epochs than usual. - let app_state = - AppState::Content(genesis::Content::default().with_epoch_duration(EPOCH_DURATION)); + let app_state = AppState::Content( + genesis::Content::default() + .with_epoch_duration(EPOCH_DURATION) + .with_chain_id(TestNode::<()>::CHAIN_ID.to_string()), + ); // Start the test node. let mut node = { diff --git a/crates/core/app/tests/app_can_deposit_into_community_pool.rs b/crates/core/app/tests/app_can_deposit_into_community_pool.rs index ecdee26f92..3c927dca19 100644 --- a/crates/core/app/tests/app_can_deposit_into_community_pool.rs +++ b/crates/core/app/tests/app_can_deposit_into_community_pool.rs @@ -2,7 +2,10 @@ use { self::common::BuilderExt, anyhow::anyhow, cnidarium::TempStorage, - penumbra_app::{genesis::AppState, server::consensus::Consensus}, + penumbra_app::{ + genesis::{self, AppState}, + server::consensus::Consensus, + }, penumbra_asset::asset, penumbra_community_pool::{CommunityPoolDeposit, StateReadExt}, penumbra_keys::test_keys, @@ -29,7 +32,9 @@ async fn app_can_deposit_into_community_pool() -> anyhow::Result<()> { // Define our application state, and start the test node. let mut test_node = { - let app_state = AppState::default(); + let app_state = AppState::Content( + genesis::Content::default().with_chain_id(TestNode::<()>::CHAIN_ID.to_string()), + ); let consensus = Consensus::new(storage.as_ref().clone()); TestNode::builder() .single_validator() diff --git a/crates/core/app/tests/app_can_disable_community_pool_spends.rs b/crates/core/app/tests/app_can_disable_community_pool_spends.rs index ac60680467..7a9b8e9577 100644 --- a/crates/core/app/tests/app_can_disable_community_pool_spends.rs +++ b/crates/core/app/tests/app_can_disable_community_pool_spends.rs @@ -111,6 +111,7 @@ async fn app_can_disable_community_pool_spends() -> anyhow::Result<()> { // Define our application state, and start the test node. let mut test_node = { let mut content = Content { + chain_id: TestNode::<()>::CHAIN_ID.to_string(), governance_content: penumbra_governance::genesis::Content { governance_params: penumbra_governance::params::GovernanceParameters { proposal_deposit_amount: 0_u32.into(), diff --git a/crates/core/app/tests/app_can_propose_community_pool_spends.rs b/crates/core/app/tests/app_can_propose_community_pool_spends.rs index d91daccc9e..a7fbd25f69 100644 --- a/crates/core/app/tests/app_can_propose_community_pool_spends.rs +++ b/crates/core/app/tests/app_can_propose_community_pool_spends.rs @@ -111,6 +111,7 @@ async fn app_can_propose_community_pool_spends() -> anyhow::Result<()> { // Define our application state, and start the test node. let mut test_node = { let mut content = Content { + chain_id: TestNode::<()>::CHAIN_ID.to_string(), governance_content: penumbra_governance::genesis::Content { governance_params: penumbra_governance::params::GovernanceParameters { proposal_deposit_amount: 0_u32.into(), diff --git a/crates/core/app/tests/app_can_spend_notes_and_detect_outputs.rs b/crates/core/app/tests/app_can_spend_notes_and_detect_outputs.rs index ace81a3c72..517a9d95c8 100644 --- a/crates/core/app/tests/app_can_spend_notes_and_detect_outputs.rs +++ b/crates/core/app/tests/app_can_spend_notes_and_detect_outputs.rs @@ -2,7 +2,10 @@ use { self::common::BuilderExt, anyhow::anyhow, cnidarium::TempStorage, - penumbra_app::{genesis::AppState, server::consensus::Consensus}, + penumbra_app::{ + genesis::{self, AppState}, + server::consensus::Consensus, + }, penumbra_keys::test_keys, penumbra_mock_client::MockClient, penumbra_mock_consensus::TestNode, @@ -26,7 +29,9 @@ async fn app_can_spend_notes_and_detect_outputs() -> anyhow::Result<()> { let guard = common::set_tracing_subscriber(); let storage = TempStorage::new().await?; let mut test_node = { - let app_state = AppState::default(); + let app_state = AppState::Content( + genesis::Content::default().with_chain_id(TestNode::<()>::CHAIN_ID.to_string()), + ); let consensus = Consensus::new(storage.as_ref().clone()); TestNode::builder() .single_validator() diff --git a/crates/core/app/tests/app_can_undelegate_from_a_validator.rs b/crates/core/app/tests/app_can_undelegate_from_a_validator.rs index 20bd76cfa5..fba7559b3f 100644 --- a/crates/core/app/tests/app_can_undelegate_from_a_validator.rs +++ b/crates/core/app/tests/app_can_undelegate_from_a_validator.rs @@ -60,6 +60,7 @@ async fn app_can_undelegate_from_a_validator() -> anyhow::Result<()> { // Configure an AppState with slightly shorter epochs than usual. let app_state = AppState::Content( genesis::Content::default() + .with_chain_id(TestNode::<()>::CHAIN_ID.to_string()) .with_epoch_duration(EPOCH_DURATION) .with_unbonding_delay(UNBONDING_DELAY), ); diff --git a/crates/core/app/tests/app_rejects_validator_definitions_with_invalid_auth_sigs.rs b/crates/core/app/tests/app_rejects_validator_definitions_with_invalid_auth_sigs.rs index a245d03488..4013a20524 100644 --- a/crates/core/app/tests/app_rejects_validator_definitions_with_invalid_auth_sigs.rs +++ b/crates/core/app/tests/app_rejects_validator_definitions_with_invalid_auth_sigs.rs @@ -2,7 +2,10 @@ use { self::common::{BuilderExt, ValidatorDataReadExt}, cnidarium::TempStorage, decaf377_rdsa::{SigningKey, SpendAuth, VerificationKey}, - penumbra_app::{genesis::AppState, server::consensus::Consensus}, + penumbra_app::{ + genesis::{self, AppState}, + server::consensus::Consensus, + }, penumbra_keys::test_keys, penumbra_mock_client::MockClient, penumbra_mock_consensus::TestNode, @@ -25,7 +28,9 @@ async fn app_rejects_validator_definitions_with_invalid_auth_sigs() -> anyhow::R // Start the test node. let mut node = { let consensus = Consensus::new(storage.as_ref().clone()); - let app_state = AppState::default(); + let app_state = AppState::Content( + genesis::Content::default().with_chain_id(TestNode::<()>::CHAIN_ID.to_string()), + ); TestNode::builder() .single_validator() .with_penumbra_auto_app_state(app_state)? diff --git a/crates/core/app/tests/app_tracks_uptime_for_genesis_validator_missing_blocks.rs b/crates/core/app/tests/app_tracks_uptime_for_genesis_validator_missing_blocks.rs index e9f93ba232..6d55488163 100644 --- a/crates/core/app/tests/app_tracks_uptime_for_genesis_validator_missing_blocks.rs +++ b/crates/core/app/tests/app_tracks_uptime_for_genesis_validator_missing_blocks.rs @@ -2,7 +2,10 @@ use { self::common::{BuilderExt, ValidatorDataReadExt}, anyhow::Context, cnidarium::TempStorage, - penumbra_app::{genesis::AppState, server::consensus::Consensus}, + penumbra_app::{ + genesis::{self, AppState}, + server::consensus::Consensus, + }, penumbra_mock_consensus::TestNode, penumbra_stake::component::validator_handler::validator_store::ValidatorDataRead, tap::Tap, @@ -19,7 +22,9 @@ async fn app_tracks_uptime_for_genesis_validator_missing_blocks() -> anyhow::Res // Start the test node. let mut node = { - let app_state = AppState::default(); + let app_state = AppState::Content( + genesis::Content::default().with_chain_id(TestNode::<()>::CHAIN_ID.to_string()), + ); let consensus = Consensus::new(storage.as_ref().clone()); TestNode::builder() .single_validator() diff --git a/crates/core/app/tests/app_tracks_uptime_for_genesis_validator_signing_blocks.rs b/crates/core/app/tests/app_tracks_uptime_for_genesis_validator_signing_blocks.rs index 4f20881b8e..9685a37395 100644 --- a/crates/core/app/tests/app_tracks_uptime_for_genesis_validator_signing_blocks.rs +++ b/crates/core/app/tests/app_tracks_uptime_for_genesis_validator_signing_blocks.rs @@ -2,7 +2,10 @@ use { self::common::{BuilderExt, ValidatorDataReadExt}, anyhow::Context, cnidarium::TempStorage, - penumbra_app::{genesis::AppState, server::consensus::Consensus}, + penumbra_app::{ + genesis::{self, AppState}, + server::consensus::Consensus, + }, penumbra_mock_consensus::TestNode, penumbra_stake::component::validator_handler::validator_store::ValidatorDataRead, tap::Tap, @@ -19,7 +22,9 @@ async fn app_tracks_uptime_for_genesis_validator_missing_blocks() -> anyhow::Res // Start the test node. let mut node = { - let app_state = AppState::default(); + let app_state = AppState::Content( + genesis::Content::default().with_chain_id(TestNode::<()>::CHAIN_ID.to_string()), + ); let consensus = Consensus::new(storage.as_ref().clone()); TestNode::builder() .single_validator() diff --git a/crates/core/app/tests/app_tracks_uptime_for_validators_only_once_active.rs b/crates/core/app/tests/app_tracks_uptime_for_validators_only_once_active.rs index 439f45fd9f..1a7f66b18b 100644 --- a/crates/core/app/tests/app_tracks_uptime_for_validators_only_once_active.rs +++ b/crates/core/app/tests/app_tracks_uptime_for_validators_only_once_active.rs @@ -35,8 +35,11 @@ async fn app_tracks_uptime_for_validators_only_once_active() -> anyhow::Result<( let storage = TempStorage::new().await?; // Configure an AppState with slightly shorter epochs than usual. - let app_state = - AppState::Content(genesis::Content::default().with_epoch_duration(EPOCH_DURATION)); + let app_state = AppState::Content( + genesis::Content::default() + .with_epoch_duration(EPOCH_DURATION) + .with_chain_id(TestNode::<()>::CHAIN_ID.to_string()), + ); // Start the test node. let mut node = { diff --git a/crates/core/app/tests/mock_consensus_can_define_a_genesis_validator.rs b/crates/core/app/tests/mock_consensus_can_define_a_genesis_validator.rs index 2d478ea52a..da66bd8575 100644 --- a/crates/core/app/tests/mock_consensus_can_define_a_genesis_validator.rs +++ b/crates/core/app/tests/mock_consensus_can_define_a_genesis_validator.rs @@ -2,7 +2,10 @@ use { self::common::{BuilderExt, ValidatorDataReadExt}, anyhow::anyhow, cnidarium::TempStorage, - penumbra_app::{genesis::AppState, server::consensus::Consensus}, + penumbra_app::{ + genesis::{self, AppState}, + server::consensus::Consensus, + }, penumbra_mock_consensus::TestNode, penumbra_stake::component::validator_handler::ValidatorDataRead as _, tap::{Tap, TapFallible}, @@ -18,7 +21,9 @@ async fn mock_consensus_can_define_a_genesis_validator() -> anyhow::Result<()> { let guard = common::set_tracing_subscriber(); let storage = TempStorage::new().await?; let test_node = { - let app_state = AppState::default(); + let app_state = AppState::Content( + genesis::Content::default().with_chain_id(TestNode::<()>::CHAIN_ID.to_string()), + ); let consensus = Consensus::new(storage.as_ref().clone()); TestNode::builder() .single_validator() diff --git a/crates/core/app/tests/mock_consensus_can_send_a_sequence_of_empty_blocks.rs b/crates/core/app/tests/mock_consensus_can_send_a_sequence_of_empty_blocks.rs index 74e662285b..cf66c15fdc 100644 --- a/crates/core/app/tests/mock_consensus_can_send_a_sequence_of_empty_blocks.rs +++ b/crates/core/app/tests/mock_consensus_can_send_a_sequence_of_empty_blocks.rs @@ -1,7 +1,10 @@ use { self::common::BuilderExt, cnidarium::TempStorage, - penumbra_app::{genesis::AppState, server::consensus::Consensus}, + penumbra_app::{ + genesis::{self, AppState}, + server::consensus::Consensus, + }, penumbra_mock_consensus::TestNode, penumbra_sct::component::clock::EpochRead as _, tap::TapFallible, @@ -17,7 +20,9 @@ async fn mock_consensus_can_send_a_sequence_of_empty_blocks() -> anyhow::Result< let guard = common::set_tracing_subscriber(); let storage = TempStorage::new().await?; let mut test_node = { - let app_state = AppState::default(); + let app_state = AppState::Content( + genesis::Content::default().with_chain_id(TestNode::<()>::CHAIN_ID.to_string()), + ); let consensus = Consensus::new(storage.as_ref().clone()); TestNode::builder() .single_validator()