From d4197279de776fd2e30c49b5673621d4800a9168 Mon Sep 17 00:00:00 2001 From: Ava Howell Date: Mon, 25 Sep 2023 16:57:33 -0700 Subject: [PATCH 1/2] ibc: use APP_VERSION when construction IBC heights --- crates/bin/pd/src/info.rs | 8 +++++--- crates/core/component/chain/src/lib.rs | 5 +++++ crates/core/component/ibc/src/component/ibc_component.rs | 3 +-- .../ibc/src/component/msg_handler/connection_open_ack.rs | 7 +++++-- .../ibc/src/component/msg_handler/connection_open_try.rs | 3 ++- .../ibc/src/component/msg_handler/recv_packet.rs | 4 ++-- .../component/ibc/src/component/proof_verification.rs | 4 +++- crates/narsil/narsil/src/ledger/info.rs | 3 +-- 8 files changed, 24 insertions(+), 13 deletions(-) diff --git a/crates/bin/pd/src/info.rs b/crates/bin/pd/src/info.rs index 6c573cc1d1..0eccec1413 100644 --- a/crates/bin/pd/src/info.rs +++ b/crates/bin/pd/src/info.rs @@ -26,7 +26,10 @@ use ibc_types::core::channel::{ChannelId, PortId}; use ibc_types::core::client::ClientId; use ibc_types::core::connection::ConnectionId; use ibc_types::core::connection::IdentifiedConnectionEnd; -use penumbra_chain::component::{AppHashRead, StateReadExt}; +use penumbra_chain::{ + component::{AppHashRead, StateReadExt}, + APP_VERSION, +}; use penumbra_ibc::component::ChannelStateReadExt as _; use penumbra_ibc::component::ClientStateReadExt as _; use penumbra_ibc::component::ConnectionStateReadExt as _; @@ -44,7 +47,6 @@ use tracing::Instrument; use penumbra_tower_trace::v034::RequestExt; const ABCI_INFO_VERSION: &str = env!("VERGEN_GIT_SEMVER"); -const APP_VERSION: u64 = 1; /// Implements service traits for Tonic gRPC services. /// @@ -80,7 +82,7 @@ impl Info { .unwrap_or_default() .try_into()?; - tracing::info!(?info, state_version = ?state.version(), app_version = ?last_block_height, "reporting height in info query"); + tracing::info!(?info, state_version = ?state.version(), last_block_height = ?last_block_height, "reporting height in info query"); let last_block_app_hash = state.app_hash().await?.0.to_vec().try_into()?; diff --git a/crates/core/component/chain/src/lib.rs b/crates/core/component/chain/src/lib.rs index 102007e084..d10c77de57 100644 --- a/crates/core/component/chain/src/lib.rs +++ b/crates/core/component/chain/src/lib.rs @@ -9,6 +9,11 @@ mod note_source; #[cfg(feature = "component")] pub mod component; +// This corresponds to the "revision number" in Cosmos SDK chains, and the "app_version" field in the Tendermint RPC. +// Currently, we don't use a revision number, because we don't have +// any further namespacing of blocks than the block height. So, we just set this to 0. +pub const APP_VERSION: u64 = 0; + pub mod genesis; pub mod params; pub mod state_key; diff --git a/crates/core/component/ibc/src/component/ibc_component.rs b/crates/core/component/ibc/src/component/ibc_component.rs index bbc23ded06..83eb3153e1 100644 --- a/crates/core/component/ibc/src/component/ibc_component.rs +++ b/crates/core/component/ibc/src/component/ibc_component.rs @@ -48,8 +48,7 @@ impl Component for IBCComponent { // Currently, we don't use a revision number, because we don't have // any further namespacing of blocks than the block height. - let revision_number = 0; - let height = Height::new(revision_number, begin_block.header.height.into()) + let height = Height::new(APP_VERSION, begin_block.header.height.into()) .expect("block height cannot be zero"); state.put_penumbra_consensus_state(height, cs); diff --git a/crates/core/component/ibc/src/component/msg_handler/connection_open_ack.rs b/crates/core/component/ibc/src/component/msg_handler/connection_open_ack.rs index 5a925050cb..b80efa66a3 100644 --- a/crates/core/component/ibc/src/component/msg_handler/connection_open_ack.rs +++ b/crates/core/component/ibc/src/component/msg_handler/connection_open_ack.rs @@ -7,7 +7,10 @@ use ibc_types::core::{ }; use ibc_types::lightclients::tendermint::client_state::ClientState as TendermintClientState; use ibc_types::path::{ClientConsensusStatePath, ClientStatePath, ConnectionPath}; -use penumbra_chain::component::{StateReadExt as _, PENUMBRA_COMMITMENT_PREFIX}; +use penumbra_chain::{ + component::{StateReadExt as _, PENUMBRA_COMMITMENT_PREFIX}, + APP_VERSION, +}; use penumbra_storage::{StateRead, StateWrite}; use crate::component::{ @@ -194,7 +197,7 @@ async fn consensus_height_is_correct( state: S, msg: &MsgConnectionOpenAck, ) -> anyhow::Result<()> { - let current_height = Height::new(0, state.get_block_height().await?)?; + let current_height = Height::new(APP_VERSION, state.get_block_height().await?)?; if msg.consensus_height_of_a_on_b > current_height { anyhow::bail!("consensus height is greater than the current block height",); } diff --git a/crates/core/component/ibc/src/component/msg_handler/connection_open_try.rs b/crates/core/component/ibc/src/component/msg_handler/connection_open_try.rs index c7619ab428..e055a9dd4d 100644 --- a/crates/core/component/ibc/src/component/msg_handler/connection_open_try.rs +++ b/crates/core/component/ibc/src/component/msg_handler/connection_open_try.rs @@ -13,6 +13,7 @@ use ibc_types::{ }, }; use penumbra_chain::component::{StateReadExt as _, PENUMBRA_COMMITMENT_PREFIX}; +use penumbra_chain::APP_VERSION; use penumbra_storage::{StateRead, StateWrite}; use crate::component::{ @@ -203,7 +204,7 @@ async fn consensus_height_is_correct( state: S, msg: &MsgConnectionOpenTry, ) -> anyhow::Result<()> { - let current_height = IBCHeight::new(0, state.get_block_height().await?)?; + let current_height = IBCHeight::new(APP_VERSION, state.get_block_height().await?)?; if msg.consensus_height_of_b_on_a > current_height { anyhow::bail!("consensus height is greater than the current block height",); } diff --git a/crates/core/component/ibc/src/component/msg_handler/recv_packet.rs b/crates/core/component/ibc/src/component/msg_handler/recv_packet.rs index e89b8cee36..398c954593 100644 --- a/crates/core/component/ibc/src/component/msg_handler/recv_packet.rs +++ b/crates/core/component/ibc/src/component/msg_handler/recv_packet.rs @@ -10,7 +10,7 @@ use ibc_types::core::{ client::Height as IBCHeight, connection::State as ConnectionState, }; -use penumbra_chain::component::StateReadExt; +use penumbra_chain::{component::StateReadExt, APP_VERSION}; use penumbra_storage::StateWrite; use crate::component::{ @@ -65,7 +65,7 @@ impl MsgHandler for MsgRecvPacket { } let block_height = state.get_block_height().await?; - let height = IBCHeight::new(0, block_height)?; + let height = IBCHeight::new(APP_VERSION, block_height)?; if self.packet.timeout_height_on_b.has_expired(height) { anyhow::bail!("packet has timed out"); diff --git a/crates/core/component/ibc/src/component/proof_verification.rs b/crates/core/component/ibc/src/component/proof_verification.rs index aee4a7ed87..a425310ef8 100644 --- a/crates/core/component/ibc/src/component/proof_verification.rs +++ b/crates/core/component/ibc/src/component/proof_verification.rs @@ -355,6 +355,8 @@ pub trait PacketProofVerifier: StateReadExt + inner::Inner { impl PacketProofVerifier for T {} mod inner { + use penumbra_chain::APP_VERSION; + use super::*; #[async_trait] @@ -396,7 +398,7 @@ mod inner { TendermintClientState::verify_delay_passed( current_timestamp.into(), - Height::new(0, current_height)?, + Height::new(APP_VERSION, current_height)?, processed_time, processed_height, delay_period_time, diff --git a/crates/narsil/narsil/src/ledger/info.rs b/crates/narsil/narsil/src/ledger/info.rs index fd07328fc3..0bc89aa67c 100644 --- a/crates/narsil/narsil/src/ledger/info.rs +++ b/crates/narsil/narsil/src/ledger/info.rs @@ -5,7 +5,7 @@ use std::{ }; use futures::FutureExt; -use penumbra_chain::component::AppHashRead; +use penumbra_chain::{component::AppHashRead, APP_VERSION}; use penumbra_storage::Storage; use penumbra_tower_trace::v034::RequestExt; use tendermint::v0_34::abci::{self, response::Echo, InfoRequest, InfoResponse}; @@ -14,7 +14,6 @@ use tracing::Instrument; // const ABCI_INFO_VERSION: &str = env!("VERGEN_GIT_SEMVER"); const ABCI_INFO_VERSION: &str = "wut"; -const APP_VERSION: u64 = 1; /// Implements service traits for Tonic gRPC services. /// From 8730380a69ea138ef5b6a6c09b2bd80ffd752328 Mon Sep 17 00:00:00 2001 From: Ava Howell Date: Mon, 25 Sep 2023 18:28:25 -0700 Subject: [PATCH 2/2] compute revision number from chain id instead of hardcoding --- crates/bin/pd/src/info.rs | 10 +++++----- crates/core/component/chain/src/component/view.rs | 8 ++++++++ crates/core/component/chain/src/lib.rs | 5 ----- .../core/component/ibc/src/component/ibc_component.rs | 11 +++++++++-- .../src/component/msg_handler/connection_open_ack.rs | 10 +++++----- .../src/component/msg_handler/connection_open_try.rs | 6 ++++-- .../ibc/src/component/msg_handler/recv_packet.rs | 4 ++-- .../component/ibc/src/component/proof_verification.rs | 4 +--- crates/narsil/narsil/src/ledger/info.rs | 4 ++-- 9 files changed, 36 insertions(+), 26 deletions(-) diff --git a/crates/bin/pd/src/info.rs b/crates/bin/pd/src/info.rs index 0eccec1413..494288cde8 100644 --- a/crates/bin/pd/src/info.rs +++ b/crates/bin/pd/src/info.rs @@ -26,10 +26,7 @@ use ibc_types::core::channel::{ChannelId, PortId}; use ibc_types::core::client::ClientId; use ibc_types::core::connection::ConnectionId; use ibc_types::core::connection::IdentifiedConnectionEnd; -use penumbra_chain::{ - component::{AppHashRead, StateReadExt}, - APP_VERSION, -}; +use penumbra_chain::component::{AppHashRead, StateReadExt}; use penumbra_ibc::component::ChannelStateReadExt as _; use penumbra_ibc::component::ClientStateReadExt as _; use penumbra_ibc::component::ConnectionStateReadExt as _; @@ -82,6 +79,9 @@ impl Info { .unwrap_or_default() .try_into()?; + // likewise, we want to return 0 if we can't get the revision number + let app_version = state.get_revision_number().await.unwrap_or_default(); + tracing::info!(?info, state_version = ?state.version(), last_block_height = ?last_block_height, "reporting height in info query"); let last_block_app_hash = state.app_hash().await?.0.to_vec().try_into()?; @@ -89,7 +89,7 @@ impl Info { Ok(response::Info { data: "penumbra".to_string(), version: ABCI_INFO_VERSION.to_string(), - app_version: APP_VERSION, + app_version, last_block_height, last_block_app_hash, }) diff --git a/crates/core/component/chain/src/component/view.rs b/crates/core/component/chain/src/component/view.rs index 12abc8ae5f..724991b9cc 100644 --- a/crates/core/component/chain/src/component/view.rs +++ b/crates/core/component/chain/src/component/view.rs @@ -2,6 +2,7 @@ use std::str::FromStr; use anyhow::{anyhow, Context, Result}; use async_trait::async_trait; +use ibc_types::core::connection::ChainId; use penumbra_proto::{StateReadProto, StateWriteProto}; use penumbra_storage::{StateRead, StateWrite}; use tendermint::Time; @@ -54,6 +55,13 @@ pub trait StateReadExt: StateRead { self.get_chain_params().await.map(|params| params.chain_id) } + /// Gets the chain revision number, from the chain ID + async fn get_revision_number(&self) -> Result { + let cid_str = self.get_chain_id().await?; + + Ok(ChainId::from_string(&cid_str).version()) + } + /// Gets the current block height from the JMT async fn get_block_height(&self) -> Result { let height_bytes: u64 = self diff --git a/crates/core/component/chain/src/lib.rs b/crates/core/component/chain/src/lib.rs index d10c77de57..102007e084 100644 --- a/crates/core/component/chain/src/lib.rs +++ b/crates/core/component/chain/src/lib.rs @@ -9,11 +9,6 @@ mod note_source; #[cfg(feature = "component")] pub mod component; -// This corresponds to the "revision number" in Cosmos SDK chains, and the "app_version" field in the Tendermint RPC. -// Currently, we don't use a revision number, because we don't have -// any further namespacing of blocks than the block height. So, we just set this to 0. -pub const APP_VERSION: u64 = 0; - pub mod genesis; pub mod params; pub mod state_key; diff --git a/crates/core/component/ibc/src/component/ibc_component.rs b/crates/core/component/ibc/src/component/ibc_component.rs index 83eb3153e1..446270f707 100644 --- a/crates/core/component/ibc/src/component/ibc_component.rs +++ b/crates/core/component/ibc/src/component/ibc_component.rs @@ -5,6 +5,7 @@ use async_trait::async_trait; use ibc_types::{ core::client::Height, lightclients::tendermint::ConsensusState as TendermintConsensusState, }; +use penumbra_chain::component::StateReadExt; use penumbra_chain::genesis; use penumbra_component::Component; use penumbra_storage::StateWrite; @@ -48,8 +49,14 @@ impl Component for IBCComponent { // Currently, we don't use a revision number, because we don't have // any further namespacing of blocks than the block height. - let height = Height::new(APP_VERSION, begin_block.header.height.into()) - .expect("block height cannot be zero"); + let height = Height::new( + state + .get_revision_number() + .await + .expect("must be able to get revision number in begin block"), + begin_block.header.height.into(), + ) + .expect("block height cannot be zero"); state.put_penumbra_consensus_state(height, cs); } diff --git a/crates/core/component/ibc/src/component/msg_handler/connection_open_ack.rs b/crates/core/component/ibc/src/component/msg_handler/connection_open_ack.rs index b80efa66a3..ebb3f95317 100644 --- a/crates/core/component/ibc/src/component/msg_handler/connection_open_ack.rs +++ b/crates/core/component/ibc/src/component/msg_handler/connection_open_ack.rs @@ -7,10 +7,7 @@ use ibc_types::core::{ }; use ibc_types::lightclients::tendermint::client_state::ClientState as TendermintClientState; use ibc_types::path::{ClientConsensusStatePath, ClientStatePath, ConnectionPath}; -use penumbra_chain::{ - component::{StateReadExt as _, PENUMBRA_COMMITMENT_PREFIX}, - APP_VERSION, -}; +use penumbra_chain::component::{StateReadExt as _, PENUMBRA_COMMITMENT_PREFIX}; use penumbra_storage::{StateRead, StateWrite}; use crate::component::{ @@ -197,7 +194,10 @@ async fn consensus_height_is_correct( state: S, msg: &MsgConnectionOpenAck, ) -> anyhow::Result<()> { - let current_height = Height::new(APP_VERSION, state.get_block_height().await?)?; + let current_height = Height::new( + state.get_revision_number().await?, + state.get_block_height().await?, + )?; if msg.consensus_height_of_a_on_b > current_height { anyhow::bail!("consensus height is greater than the current block height",); } diff --git a/crates/core/component/ibc/src/component/msg_handler/connection_open_try.rs b/crates/core/component/ibc/src/component/msg_handler/connection_open_try.rs index e055a9dd4d..eb586bfad7 100644 --- a/crates/core/component/ibc/src/component/msg_handler/connection_open_try.rs +++ b/crates/core/component/ibc/src/component/msg_handler/connection_open_try.rs @@ -13,7 +13,6 @@ use ibc_types::{ }, }; use penumbra_chain::component::{StateReadExt as _, PENUMBRA_COMMITMENT_PREFIX}; -use penumbra_chain::APP_VERSION; use penumbra_storage::{StateRead, StateWrite}; use crate::component::{ @@ -204,7 +203,10 @@ async fn consensus_height_is_correct( state: S, msg: &MsgConnectionOpenTry, ) -> anyhow::Result<()> { - let current_height = IBCHeight::new(APP_VERSION, state.get_block_height().await?)?; + let current_height = IBCHeight::new( + state.get_revision_number().await?, + state.get_block_height().await?, + )?; if msg.consensus_height_of_b_on_a > current_height { anyhow::bail!("consensus height is greater than the current block height",); } diff --git a/crates/core/component/ibc/src/component/msg_handler/recv_packet.rs b/crates/core/component/ibc/src/component/msg_handler/recv_packet.rs index 398c954593..65fcfe97de 100644 --- a/crates/core/component/ibc/src/component/msg_handler/recv_packet.rs +++ b/crates/core/component/ibc/src/component/msg_handler/recv_packet.rs @@ -10,7 +10,7 @@ use ibc_types::core::{ client::Height as IBCHeight, connection::State as ConnectionState, }; -use penumbra_chain::{component::StateReadExt, APP_VERSION}; +use penumbra_chain::component::StateReadExt; use penumbra_storage::StateWrite; use crate::component::{ @@ -65,7 +65,7 @@ impl MsgHandler for MsgRecvPacket { } let block_height = state.get_block_height().await?; - let height = IBCHeight::new(APP_VERSION, block_height)?; + let height = IBCHeight::new(state.get_revision_number().await?, block_height)?; if self.packet.timeout_height_on_b.has_expired(height) { anyhow::bail!("packet has timed out"); diff --git a/crates/core/component/ibc/src/component/proof_verification.rs b/crates/core/component/ibc/src/component/proof_verification.rs index a425310ef8..8cf74896e2 100644 --- a/crates/core/component/ibc/src/component/proof_verification.rs +++ b/crates/core/component/ibc/src/component/proof_verification.rs @@ -355,8 +355,6 @@ pub trait PacketProofVerifier: StateReadExt + inner::Inner { impl PacketProofVerifier for T {} mod inner { - use penumbra_chain::APP_VERSION; - use super::*; #[async_trait] @@ -398,7 +396,7 @@ mod inner { TendermintClientState::verify_delay_passed( current_timestamp.into(), - Height::new(APP_VERSION, current_height)?, + Height::new(self.get_revision_number().await?, current_height)?, processed_time, processed_height, delay_period_time, diff --git a/crates/narsil/narsil/src/ledger/info.rs b/crates/narsil/narsil/src/ledger/info.rs index 0bc89aa67c..1f886ee06e 100644 --- a/crates/narsil/narsil/src/ledger/info.rs +++ b/crates/narsil/narsil/src/ledger/info.rs @@ -5,7 +5,7 @@ use std::{ }; use futures::FutureExt; -use penumbra_chain::{component::AppHashRead, APP_VERSION}; +use penumbra_chain::component::{AppHashRead, StateReadExt}; use penumbra_storage::Storage; use penumbra_tower_trace::v034::RequestExt; use tendermint::v0_34::abci::{self, response::Echo, InfoRequest, InfoResponse}; @@ -47,7 +47,7 @@ impl Info { Ok(abci::response::Info { data: "penumbra".to_string(), version: ABCI_INFO_VERSION.to_string(), - app_version: APP_VERSION, + app_version: state.get_revision_number().await?, last_block_height, last_block_app_hash, })