DI - IBC

.changelog/unreleased/improvements/3466-di-shielded-token.md

@@ -0,0 +1,2 @@
+- Replaced cross-system dependencies in namada_shielded_token crate with
+  dependency-injection. ([\#3466](https://github.com/anoma/namada/pull/3466))

.changelog/unreleased/improvements/3472-systems-crate.md

@@ -0,0 +1,5 @@
+- Added a new namada_systems crate to contain abstract systems interfaces,
+  previously added to core crate. Also switched to use the concrete
+  storage error and result type instead of the generic associated
+  type which reduces the amount of typing needed one the caller side.
+  ([\#3472](https://github.com/anoma/namada/pull/3472))

.changelog/unreleased/improvements/3482-di-gov.md

@@ -0,0 +1,2 @@
+- Replaced cross-system dependencies in namada_governance crate with dependency-
+  injection. ([\#3482](https://github.com/anoma/namada/pull/3482))

.changelog/unreleased/improvements/3497-di-pos.md

@@ -0,0 +1,2 @@
+- Replaced cross-system dependencies in namada_proof_of_stake crate with
+  dependency-injection. ([\#3497](https://github.com/anoma/namada/pull/3497))

.changelog/unreleased/improvements/3509-di-ibc.md

@@ -0,0 +1,2 @@
+- Replaced cross-system dependencies in namada_ibc crate with dependency-
+  injection. ([\#3509](https://github.com/anoma/namada/pull/3509))

Cargo.toml

@@ -26,6 +26,7 @@ members = [
   "crates/shielded_token",
   "crates/state",
   "crates/storage",
+  "crates/systems",
   "crates/test_utils",
   "crates/tests",
   "crates/token",

crates/benches/native_vps.rs

@@ -51,7 +51,7 @@ use namada_apps_lib::validation::{
     IbcVpContext, MaspVp, MultitokenVp, ParametersVp, PgfVp, PosVp,
 };
 use namada_apps_lib::wallet::defaults;
-use namada_apps_lib::{governance, proof_of_stake, storage, token};
+use namada_apps_lib::{governance, parameters, proof_of_stake, storage, token};
 use namada_node::bench_utils::{
     generate_foreign_key_tx, BenchShell, BenchShieldedCtx,
     ALBERT_PAYMENT_ADDRESS, ALBERT_SPENDING_KEY, BERTHA_PAYMENT_ADDRESS,
@@ -110,9 +110,11 @@ fn governance(c: &mut Criterion) {
             "minimal_proposal" => {
                 let content_section =
                     Section::ExtraData(Code::new(vec![], None));
-                let params =
-                    proof_of_stake::storage::read_pos_params(&shell.state)
-                        .unwrap();
+                let params = proof_of_stake::storage::read_pos_params::<
+                    _,
+                    governance::Store<_>,
+                >(&shell.state)
+                .unwrap();
                 let voting_start_epoch =
                     Epoch(2 + params.pipeline_len + params.unbonding_len);
                 // Must start after current epoch
@@ -163,9 +165,11 @@ fn governance(c: &mut Criterion) {
                     None,
                 ));
 
-                let params =
-                    proof_of_stake::storage::read_pos_params(&shell.state)
-                        .unwrap();
+                let params = proof_of_stake::storage::read_pos_params::<
+                    _,
+                    governance::Store<_>,
+                >(&shell.state)
+                .unwrap();
                 let voting_start_epoch =
                     Epoch(2 + params.pipeline_len + params.unbonding_len);
                 // Must start after current epoch
@@ -1670,16 +1674,20 @@ fn ibc_vp_validate_action(c: &mut Criterion) {
 
         let exec_ctx = IbcVpContext::new(ibc.ctx.pre());
         let ctx = Rc::new(RefCell::new(exec_ctx));
-        let mut actions = IbcActions::new(ctx.clone(), verifiers.clone());
+        let mut actions =
+            IbcActions::<_, parameters::Store<_>, token::Store<()>>::new(
+                ctx.clone(),
+                verifiers.clone(),
+            );
         actions.set_validation_params(ibc.validation_params().unwrap());
 
         let module = TransferModule::new(ctx.clone(), verifiers);
         actions.add_transfer_module(module);
-        let module = NftTransferModule::new(ctx);
+        let module = NftTransferModule::<_, token::Store<()>>::new(ctx);
         actions.add_transfer_module(module);
 
         group.bench_function(bench_name, |b| {
-            b.iter(|| actions.validate(&tx_data).unwrap())
+            b.iter(|| actions.validate::<Transfer>(&tx_data).unwrap())
         });
     }
 
@@ -1727,16 +1735,20 @@ fn ibc_vp_execute_action(c: &mut Criterion) {
         let exec_ctx = IbcVpContext::new(ibc.ctx.pre());
         let ctx = Rc::new(RefCell::new(exec_ctx));
 
-        let mut actions = IbcActions::new(ctx.clone(), verifiers.clone());
+        let mut actions =
+            IbcActions::<_, parameters::Store<_>, token::Store<()>>::new(
+                ctx.clone(),
+                verifiers.clone(),
+            );
         actions.set_validation_params(ibc.validation_params().unwrap());
 
         let module = TransferModule::new(ctx.clone(), verifiers);
         actions.add_transfer_module(module);
-        let module = NftTransferModule::new(ctx);
+        let module = NftTransferModule::<_, token::Store<()>>::new(ctx);
         actions.add_transfer_module(module);
 
         group.bench_function(bench_name, |b| {
-            b.iter(|| actions.execute(&tx_data).unwrap())
+            b.iter(|| actions.execute::<token::Transfer>(&tx_data).unwrap())
         });
     }
 

crates/core/src/ibc.rs

@@ -1,54 +1,21 @@
 //! IBC-related data types
 
-use std::collections::{BTreeMap, BTreeSet};
+use std::collections::BTreeMap;
 use std::fmt::Display;
 use std::str::FromStr;
 
 use borsh::{BorshDeserialize, BorshSchema, BorshSerialize};
 use data_encoding::{DecodePartial, HEXLOWER, HEXLOWER_PERMISSIVE};
+use ibc::core::host::types::identifiers::{ChannelId, PortId};
 pub use ibc::*;
-use masp_primitives::transaction::components::ValueSum;
-use masp_primitives::transaction::TransparentAddress;
 use namada_macros::BorshDeserializer;
 #[cfg(feature = "migrations")]
 use namada_migrations::*;
 use serde::{Deserialize, Serialize};
 
 use super::address::HASH_LEN;
-use crate::address::Address;
 use crate::hash::Hash;
-use crate::masp::TAddrData;
-use crate::{storage, token};
-
-/// Abstract IBC storage read interface
-pub trait Read<S> {
-    /// Storage error
-    type Err;
-
-    /// Extract MASP transaction from IBC envelope
-    fn try_extract_masp_tx_from_envelope(
-        tx_data: &[u8],
-    ) -> Result<Option<masp_primitives::transaction::Transaction>, Self::Err>;
-
-    /// Apply relevant IBC packets to the changed balances structure
-    fn apply_ibc_packet(
-        storage: &S,
-        tx_data: &[u8],
-        acc: ChangedBalances,
-        keys_changed: &BTreeSet<storage::Key>,
-    ) -> Result<ChangedBalances, Self::Err>;
-}
-
-/// Balances changed by a transaction
-#[derive(Default, Debug, Clone)]
-pub struct ChangedBalances {
-    /// Map between MASP transparent address and namada types
-    pub decoder: BTreeMap<TransparentAddress, TAddrData>,
-    /// Balances before the tx
-    pub pre: BTreeMap<TransparentAddress, ValueSum<Address, token::Amount>>,
-    /// Balances after the tx
-    pub post: BTreeMap<TransparentAddress, ValueSum<Address, token::Amount>>,
-}
+use crate::token;
 
 /// IBC token hash derived from a denomination.
 #[derive(
@@ -69,7 +36,7 @@ pub struct ChangedBalances {
 #[repr(transparent)]
 pub struct IbcTokenHash(pub [u8; HASH_LEN]);
 
-impl std::fmt::Display for IbcTokenHash {
+impl Display for IbcTokenHash {
     #[inline(always)]
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         write!(f, "{}", HEXLOWER.encode(&self.0))
@@ -106,3 +73,92 @@ impl FromStr for IbcTxDataRefs {
         serde_json::from_str(s)
     }
 }
+
+/// The target of a PGF payment
+#[derive(
+    Debug,
+    Clone,
+    PartialEq,
+    Serialize,
+    Deserialize,
+    Ord,
+    Eq,
+    PartialOrd,
+    BorshDeserializer,
+    Hash,
+)]
+pub struct PGFIbcTarget {
+    /// The target address on the target chain
+    pub target: String,
+    /// The amount of token to fund the target address
+    pub amount: token::Amount,
+    /// Port ID to fund
+    pub port_id: PortId,
+    /// Channel ID to fund
+    pub channel_id: ChannelId,
+}
+
+impl BorshSerialize for PGFIbcTarget {
+    fn serialize<W: std::io::Write>(
+        &self,
+        writer: &mut W,
+    ) -> std::io::Result<()> {
+        BorshSerialize::serialize(&self.target, writer)?;
+        BorshSerialize::serialize(&self.amount, writer)?;
+        BorshSerialize::serialize(&self.port_id.to_string(), writer)?;
+        BorshSerialize::serialize(&self.channel_id.to_string(), writer)
+    }
+}
+
+impl borsh::BorshDeserialize for PGFIbcTarget {
+    fn deserialize_reader<R: std::io::Read>(
+        reader: &mut R,
+    ) -> std::io::Result<Self> {
+        use std::io::{Error, ErrorKind};
+        let target: String = BorshDeserialize::deserialize_reader(reader)?;
+        let amount: token::Amount =
+            BorshDeserialize::deserialize_reader(reader)?;
+        let port_id: String = BorshDeserialize::deserialize_reader(reader)?;
+        let port_id: PortId = port_id.parse().map_err(|err| {
+            Error::new(
+                ErrorKind::InvalidData,
+                format!("Error decoding port ID: {}", err),
+            )
+        })?;
+        let channel_id: String = BorshDeserialize::deserialize_reader(reader)?;
+        let channel_id: ChannelId = channel_id.parse().map_err(|err| {
+            Error::new(
+                ErrorKind::InvalidData,
+                format!("Error decoding channel ID: {}", err),
+            )
+        })?;
+        Ok(Self {
+            target,
+            amount,
+            port_id,
+            channel_id,
+        })
+    }
+}
+
+impl borsh::BorshSchema for PGFIbcTarget {
+    fn add_definitions_recursively(
+        definitions: &mut BTreeMap<
+            borsh::schema::Declaration,
+            borsh::schema::Definition,
+        >,
+    ) {
+        let fields = borsh::schema::Fields::NamedFields(vec![
+            ("target".into(), String::declaration()),
+            ("amount".into(), token::Amount::declaration()),
+            ("port_id".into(), String::declaration()),
+            ("channel_id".into(), String::declaration()),
+        ]);
+        let definition = borsh::schema::Definition::Struct { fields };
+        definitions.insert(Self::declaration(), definition);
+    }
+
+    fn declaration() -> borsh::schema::Declaration {
+        std::any::type_name::<Self>().into()
+    }
+}

crates/core/src/lib.rs

@@ -21,9 +21,7 @@ pub mod arith;
 pub mod bytes;
 #[cfg(any(test, feature = "control_flow"))]
 pub mod control_flow;
-pub mod governance;
 pub mod hints;
-pub mod proof_of_stake;
 
 // TODO(namada#3248): only re-export v037 `tendermint-rs`
 pub use {masp_primitives, tendermint, tendermint_proto};