Add stateful checks for chain ID and expiry height

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<S: StateRead + 'static>(&self, state: Arc<S>) -> 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

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<S: StateRead>(
+    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<S: StateRead>(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<S: StateRead>(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<S: StateRead>(
+    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<S: StateRead>(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<S: StateRead>(
 ) -> Result<()> {
     state.check_claimed_anchor(transaction.anchor).await
 }
-
-pub async fn fee_greater_than_base_fee<S: StateRead>(
-    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(())
-}

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 {

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 = {

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()

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(),

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(),

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()

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),
     );

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)?

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()

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()

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 = {

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()

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()