Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(ibc): remove dependency on penumbra-chain specific state keys #3528

Closed
wants to merge 10 commits into from
13 changes: 13 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ members = [
"crates/misc/tct-visualize",
"crates/bench",
"tools/summonerd",
"crates/util/wrapper-derive",
]

# Optimize for small binaries in just the wasm crate.
Expand Down
2 changes: 2 additions & 0 deletions crates/bin/pd/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ download-proving-keys = ["penumbra-proof-params/download-proving-keys"]
# Workspace dependencies
penumbra-proto = { path = "../../proto" }
cnidarium = { path = "../../cnidarium", features = ["migration", "rpc"] }
cnidarium-component = { path = "../../cnidarium-component" }
penumbra-asset = { path = "../../core/asset" }
penumbra-keys = { path = "../../core/keys" }
penumbra-shielded-pool = { path = "../../core/component/shielded-pool", features = [
Expand All @@ -44,6 +45,7 @@ penumbra-app = { path = "../../core/app" }
penumbra-custody = { path = "../../custody" }
penumbra-tower-trace = { path = "../../util/tower-trace" }
penumbra-tendermint-proxy = { path = "../../util/tendermint-proxy" }
wrapper-derive = { path = "../../util/wrapper-derive" }

# Penumbra dependencies
decaf377 = { version = "0.5", features = ["parallel"] }
Expand Down
32 changes: 32 additions & 0 deletions crates/bin/pd/src/ibc.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
use anyhow::Result;
use cnidarium::StateRead;
use cnidarium::Storage;
use cnidarium_component::ChainStateReadExt;
use ibc_types::core::commitment::MerkleProof;
// implemented by [`SnapshotWrapper`]
use penumbra_chain::component::StateReadExt as _;

#[derive(wrapper_derive::StateRead, wrapper_derive::ChainStateReadExt, Clone)]
pub struct SnapshotWrapper<S: StateRead>(S);

#[async_trait::async_trait]
impl penumbra_ibc::component::rpc::Snapshot for SnapshotWrapper<cnidarium::Snapshot> {
fn version(&self) -> u64 {
self.0.version()
}

async fn get_with_proof(&self, key: Vec<u8>) -> Result<(Option<Vec<u8>>, MerkleProof)> {
self.0.get_with_proof(key).await
}
}

#[derive(Clone)]
pub struct StorageWrapper(pub Storage);

impl penumbra_ibc::component::rpc::Storage<SnapshotWrapper<cnidarium::Snapshot>>
for StorageWrapper
{
fn latest_snapshot(&self) -> SnapshotWrapper<cnidarium::Snapshot> {
SnapshotWrapper(self.0.latest_snapshot())
}
}
2 changes: 2 additions & 0 deletions crates/bin/pd/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#![recursion_limit = "512"]

mod consensus;
mod ibc;
mod info;
mod mempool;
mod metrics;
Expand All @@ -16,6 +17,7 @@ pub mod upgrade;

pub use crate::metrics::register_metrics;
pub use consensus::Consensus;
pub use ibc::{SnapshotWrapper, StorageWrapper};
pub use info::Info;
pub use mempool::Mempool;
pub use penumbra_app::app::App;
Expand Down
3 changes: 2 additions & 1 deletion crates/bin/pd/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,8 @@ async fn main() -> anyhow::Result<()> {
)
.expect("failed to spawn abci server");

let ibc = penumbra_ibc::component::rpc::IbcQuery::new(storage.clone());
let ibc =
penumbra_ibc::component::rpc::IbcQuery::new(pd::StorageWrapper(storage.clone()));

// TODO: Once we migrate to Tonic 0.10.0, we'll be able to use the
// `Routes` structure to have each component define a method that
Expand Down
11 changes: 11 additions & 0 deletions crates/cnidarium-component/src/chain_state_read_ext.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
use anyhow::Result;
use async_trait::async_trait;
use cnidarium::StateRead;

#[async_trait]
pub trait ChainStateReadExt: StateRead {
async fn get_chain_id(&self) -> Result<String>;
async fn get_revision_number(&self) -> Result<u64>;
async fn get_block_height(&self) -> Result<u64>;
async fn get_block_timestamp(&self) -> Result<tendermint::Time>;
}
Comment on lines +6 to +11
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this should go in cnidarium-component, since this is really an IBC host chain interface, that should be named as such and go in the ibc crate, right?

2 changes: 2 additions & 0 deletions crates/cnidarium-component/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,9 @@
#![deny(clippy::unwrap_used)]

mod action_handler;
mod chain_state_read_ext;
mod component;

pub use action_handler::ActionHandler;
pub use chain_state_read_ext::ChainStateReadExt;
pub use component::Component;
1 change: 1 addition & 0 deletions crates/core/app/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ penumbra-ibc = { path = "../component/ibc", features = ["component"] }
penumbra-distributions = { path = "../component/distributions" }
penumbra-compact-block = { path = "../component/compact-block" }
penumbra-transaction = { path = "../transaction", features = ["parallel"] }
wrapper-derive = { path = "../../util/wrapper-derive" }

# Penumbra dependencies
decaf377 = { version = "0.5" }
Expand Down
6 changes: 5 additions & 1 deletion crates/core/app/src/action_handler/actions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ use penumbra_txhash::TransactionContext;

mod submit;

use crate::state_delta_wrapper::StateDeltaWrapper;

use super::ActionHandler;
use cnidarium_component::ActionHandler as _;

Expand Down Expand Up @@ -109,10 +111,12 @@ impl ActionHandler for Action {
Action::Spend(action) => action.execute(state).await,
Action::Output(action) => action.execute(state).await,
Action::IbcRelay(action) => {
let mut state = state;
let wrapper = StateDeltaWrapper(&mut state);
action
.clone()
.with_handler::<Ics20Transfer>()
.execute(state)
.execute(wrapper)
.await
}
Action::Ics20Withdrawal(action) => action.execute(state).await,
Expand Down
5 changes: 3 additions & 2 deletions crates/core/app/src/app/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ use tracing::Instrument;

use crate::action_handler::ActionHandler;
use crate::params::AppParameters;
use crate::state_delta_wrapper::ArcStateDeltaWrapper;
use crate::{genesis, CommunityPoolStateReadExt};

pub mod state_key;
Expand Down Expand Up @@ -213,11 +214,11 @@ impl App {
}
}

// Run each of the begin block handlers for each component, in sequence:
// Run each of the begin block handlers for each component, in sequence.
let mut arc_state_tx = Arc::new(state_tx);
ShieldedPool::begin_block(&mut arc_state_tx, begin_block).await;
Distributions::begin_block(&mut arc_state_tx, begin_block).await;
IBCComponent::begin_block(&mut arc_state_tx, begin_block).await;
IBCComponent::begin_block(&mut ArcStateDeltaWrapper(&mut arc_state_tx), begin_block).await;
Governance::begin_block(&mut arc_state_tx, begin_block).await;
Staking::begin_block(&mut arc_state_tx, begin_block).await;
Fee::begin_block(&mut arc_state_tx, begin_block).await;
Expand Down
1 change: 1 addition & 0 deletions crates/core/app/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
mod action_handler;
mod community_pool_ext;
mod mock_client;
mod state_delta_wrapper;
mod temp_storage_ext;

pub use action_handler::ActionHandler;
Expand Down
17 changes: 17 additions & 0 deletions crates/core/app/src/state_delta_wrapper.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
use anyhow::Result;
use cnidarium::StateRead;
use cnidarium::StateWrite;
use cnidarium_component::ChainStateReadExt;
use penumbra_chain::component::StateReadExt;

#[derive(
wrapper_derive::StateRead, wrapper_derive::StateWrite, wrapper_derive::ChainStateReadExt,
)]
pub(crate) struct StateDeltaWrapper<'a, S: StateRead + StateWrite>(pub(crate) &'a mut S);

#[derive(
wrapper_derive::StateRead, wrapper_derive::StateWrite, wrapper_derive::ChainStateReadExt,
)]
pub(crate) struct ArcStateDeltaWrapper<'a, S: StateRead + StateWrite>(
pub(crate) &'a mut std::sync::Arc<S>,
);
1 change: 0 additions & 1 deletion crates/core/component/chain/src/component/view.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ use crate::{
///
/// Note: the `get_` methods in this trait assume that the state store has been
/// initialized, so they will error on an empty state.
//#[async_trait(?Send)]
#[async_trait]
pub trait StateReadExt: StateRead {
/// Indicates if the chain parameters have been updated in this block.
Expand Down
7 changes: 5 additions & 2 deletions crates/core/component/ibc/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,18 @@ component = [
default = ["component", "std"]
std = ["ibc-types/std"]
docsrs = []
rpc = ["dep:tonic", "ibc-proto/client", "ibc-proto/server"]
rpc = ["dep:tonic", "dep:penumbra-chain", "dep:wrapper-derive", "ibc-proto/client", "ibc-proto/server"]

[dependencies]
# Workspace dependencies
penumbra-proto = { path = "../../../proto", default-features = false }
cnidarium = { path = "../../../cnidarium", optional = true }
cnidarium-component = { path = "../../../cnidarium-component", optional = true }
penumbra-chain = { path = "../chain", default-features = false }
penumbra-asset = { path = "../../../core/asset", default-features = false }
penumbra-num = { path = "../../../core/num", default-features = false }
penumbra-keys = { path = "../../../core/keys", default-features = false }
wrapper-derive = { path = "../../../util/wrapper-derive", optional = true }
penumbra-chain = { path = "../chain", default-features = false, optional = true }
penumbra-txhash = { path = "../../../core/txhash", default-features = false }

# Penumbra dependencies
Expand Down Expand Up @@ -55,4 +56,6 @@ tonic = { version = "0.10", optional = true }
tower = "0.4"

[dev-dependencies]
penumbra-chain = { path = "../chain", default-features = false }
tokio = { version = "1.3", features = ["full"] }
wrapper-derive = { path = "../../../util/wrapper-derive" }
Original file line number Diff line number Diff line change
@@ -1,19 +1,16 @@
use std::sync::Arc;

use anyhow::{Context, Result};
use async_trait::async_trait;
use cnidarium::{StateRead, StateWrite};
use cnidarium_component::ActionHandler;
use cnidarium_component::ChainStateReadExt;

use crate::{
component::{app_handler::AppHandler, MsgHandler as _},
IbcActionWithHandler, IbcRelay,
};

#[async_trait]
impl<H: AppHandler> ActionHandler for IbcActionWithHandler<H> {
type CheckStatelessContext = ();
async fn check_stateless(&self, _context: ()) -> Result<()> {
impl<H: AppHandler> IbcActionWithHandler<H> {
pub async fn check_stateless(&self, _context: ()) -> Result<()> {
let action = self.action();
match action {
IbcRelay::CreateClient(msg) => msg.check_stateless::<H>().await?,
Expand Down Expand Up @@ -41,12 +38,12 @@ impl<H: AppHandler> ActionHandler for IbcActionWithHandler<H> {
Ok(())
}

async fn check_stateful<S: StateRead + 'static>(&self, _state: Arc<S>) -> Result<()> {
pub async fn check_stateful<_S: StateRead + 'static>(&self, _state: Arc<_S>) -> Result<()> {
// No-op: IBC actions merge check_stateful and execute.
Ok(())
}

async fn execute<S: StateWrite>(&self, state: S) -> Result<()> {
pub async fn execute<S: StateWrite + ChainStateReadExt>(&self, state: S) -> Result<()> {
let action = self.action();
match action {
IbcRelay::CreateClient(msg) => msg
Expand Down
Loading
Loading