DI - PoS

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

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

@@ -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

crates/core/src/ibc.rs

@@ -1,54 +1,18 @@
 //! IBC-related data types
 
-use std::collections::{BTreeMap, BTreeSet};
 use std::fmt::Display;
 use std::str::FromStr;
 
 use borsh::{BorshDeserialize, BorshSchema, BorshSerialize};
 use data_encoding::{DecodePartial, HEXLOWER, HEXLOWER_PERMISSIVE};
 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>>,
-}
 
 /// IBC token hash derived from a denomination.
 #[derive(
@@ -69,7 +33,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))

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};

crates/core/src/parameters.rs

@@ -12,41 +12,6 @@ use super::hash::Hash;
 use super::time::DurationSecs;
 use super::token;
 use crate::borsh::{BorshDeserialize, BorshSchema, BorshSerialize};
-use crate::storage;
-
-/// Abstract parameters storage keys interface
-pub trait Keys {
-    /// Key for implicit VP
-    fn implicit_vp_key() -> storage::Key;
-}
-
-/// Abstract parameters storage read interface
-pub trait Read<S> {
-    /// Storage error
-    type Err;
-
-    /// Read all parameters
-    fn read(storage: &S) -> Result<Parameters, Self::Err>;
-
-    /// Read MASP epoch multiplier
-    fn masp_epoch_multiplier(storage: &S) -> Result<u64, Self::Err>;
-
-    /// Read the the epoch duration parameter from store
-    fn epoch_duration_parameter(
-        storage: &S,
-    ) -> Result<EpochDuration, Self::Err>;
-
-    /// Helper function to retrieve the `is_native_token_transferable` protocol
-    /// parameter from storage
-    fn is_native_token_transferable(storage: &S) -> Result<bool, Self::Err>;
-}
-
-/// Abstract parameters storage write interface
-pub trait Write<S>: Read<S> {
-    /// Write all parameters
-    fn write(storage: &mut S, parameters: &Parameters)
-    -> Result<(), Self::Err>;
-}
 
 /// Protocol parameters
 #[derive(

crates/core/src/token.rs

@@ -14,73 +14,12 @@ use namada_migrations::*;
 use serde::{Deserialize, Serialize};
 use thiserror::Error;
 
-use crate::address::Address;
 use crate::arith::{self, checked, CheckedAdd, CheckedSub};
 use crate::dec::{Dec, POS_DECIMAL_PRECISION};
 use crate::storage;
 use crate::storage::{DbKeySeg, KeySeg};
 use crate::uint::{self, Uint, I256};
 
-/// Abstract token keys interface
-pub trait Keys {
-    /// Key for transparent token balance
-    fn balance_key(token: &Address, owner: &Address) -> storage::Key;
-
-    /// Returns the owner address if the given storage key is a balance key for
-    /// the given token.
-    fn is_balance_key<'a>(
-        token_addr: &Address,
-        key: &'a storage::Key,
-    ) -> Option<&'a Address>;
-
-    /// Check if the given storage key is a balance key for an unspecified
-    /// token. If it is, return the token and owner address.
-    fn is_any_token_balance_key(key: &storage::Key) -> Option<[&Address; 2]>;
-
-    /// Obtain a storage key for the multitoken minter.
-    fn minter_key(token_addr: &Address) -> storage::Key;
-}
-
-/// Abstract token storage read interface
-pub trait Read<S> {
-    /// Storage error
-    type Err;
-}
-
-/// Abstract token storage write interface
-pub trait Write<S>: Read<S> {
-    /// Transfer `token` from `src` to `dest`. Returns an `Err` if `src` has
-    /// insufficient balance or if the transfer the `dest` would overflow (This
-    /// can only happen if the total supply doesn't fit in `token::Amount`).
-    fn transfer(
-        storage: &mut S,
-        token: &Address,
-        src: &Address,
-        dest: &Address,
-        amount: Amount,
-    ) -> Result<(), Self::Err>;
-
-    /// Burn a specified amount of tokens from some address. If the burn amount
-    /// is larger than the total balance of the given address, then the
-    /// remaining balance is burned. The total supply of the token is
-    /// properly adjusted.
-    fn burn_tokens(
-        storage: &mut S,
-        token: &Address,
-        source: &Address,
-        amount: Amount,
-    ) -> Result<(), Self::Err>;
-
-    /// Credit tokens to an account, to be used only by protocol. In
-    /// transactions, this would get rejected by the default `vp_token`.
-    fn credit_tokens(
-        storage: &mut S,
-        token: &Address,
-        dest: &Address,
-        amount: Amount,
-    ) -> Result<(), Self::Err>;
-}
-
 /// Amount in micro units. For different granularity another representation
 /// might be more appropriate.
 #[derive(

crates/ethereum_bridge/Cargo.toml

@@ -19,6 +19,7 @@ testing = [
     "namada_account",
     "namada_core/testing",
     "namada_state/testing",
+    "namada_governance",
 ]
 migrations = [
     "namada_migrations",
@@ -29,12 +30,14 @@ migrations = [
 namada_account = {path = "../account", optional = true}
 namada_core = {path = "../core", default-features = false, features = ["ethers-derive"]}
 namada_events = { path = "../events", default-features = false }
+namada_governance = {path = "../governance", optional = true}
 namada_macros = {path = "../macros"}
 namada_migrations = {path = "../migrations", optional = true}
 namada_parameters = {path = "../parameters"}
 namada_proof_of_stake = {path = "../proof_of_stake", default-features = false}
 namada_state = {path = "../state"}
 namada_storage = {path = "../storage"}
+namada_systems = { path = "../systems" }
 namada_trans_token = {path = "../trans_token"}
 namada_tx = {path = "../tx"}
 namada_vote_ext = {path = "../vote_ext"}
@@ -52,10 +55,10 @@ thiserror.workspace = true
 tracing = "0.1.30"
 
 [dev-dependencies]
-# Added "testing" feature.
 namada_account = {path = "../account"}
 namada_core = {path = "../core", default-features = false, features = ["ethers-derive", "testing"]}
 namada_gas = {path = "../gas"}
+namada_governance = {path = "../governance"}
 namada_proof_of_stake = {path = "../proof_of_stake", default-features = false, features = ["testing"]}
 namada_state = { path = "../state", features = ["testing"] }
 namada_token = {path = "../token", features = ["testing"]}