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

Sorting modules by dependencies #549

Merged
merged 34 commits into from
Jul 27, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
a501995
Generating ModuleTrait.dependencies and implementing sort_by_dependen…
LukaszRozmej Jul 24, 2023
c196edd
fix whitespace
LukaszRozmej Jul 24, 2023
82ade25
Add ModuleInfo macro dependencies test
LukaszRozmej Jul 24, 2023
061bdb2
change sorting modules to have additional type
LukaszRozmej Jul 24, 2023
63ed336
change genesis macro
LukaszRozmej Jul 24, 2023
9a93a74
Merge remote-tracking branch 'upstream/nightly' into lukasz/sorted_de…
LukaszRozmej Jul 25, 2023
78b0975
Merge remote-tracking branch 'upstream/nightly' into lukasz/sorted_de…
LukaszRozmej Jul 25, 2023
a7d6326
add sorting modules tests
LukaszRozmej Jul 25, 2023
0e9ce50
formatting
LukaszRozmej Jul 25, 2023
f9e0630
make Vec fully qualified in macros
LukaszRozmej Jul 25, 2023
b52ad78
Merge remote-tracking branch 'upstream/nightly' into lukasz/sorted_de…
LukaszRozmej Jul 25, 2023
0d212a5
split sort_modules_by_dependencies and sort_values_by_modules_depende…
LukaszRozmej Jul 25, 2023
b08d093
rename param
LukaszRozmej Jul 25, 2023
984b8b0
Merge branch 'nightly' into lukasz/sorted_dependencies
LukaszRozmej Jul 25, 2023
a45eca2
fix whitespace
LukaszRozmej Jul 26, 2023
5978dd3
refactor sort_modules_by_dependencies to use ModuleVisitor struct for…
LukaszRozmej Jul 26, 2023
879bdfd
fix whitespace
LukaszRozmej Jul 26, 2023
cb8fed9
refactor
LukaszRozmej Jul 26, 2023
b1eec25
Add support for cyclic dependencies
LukaszRozmej Jul 26, 2023
4bda64b
Change sort_values_by_modules_dependencies from &Tvalue to TValue
LukaszRozmej Jul 26, 2023
10025d4
remove & in genesic macro identyfiers
LukaszRozmej Jul 26, 2023
fe6d217
Merge remote-tracking branch 'upstream/nightly' into lukasz/sorted_de…
LukaszRozmej Jul 26, 2023
9c20b74
fix test
LukaszRozmej Jul 26, 2023
543057e
fix
LukaszRozmej Jul 27, 2023
edd574e
fix test
LukaszRozmej Jul 27, 2023
be7b91e
move Hash trait implementation for addresses to macros
LukaszRozmej Jul 27, 2023
780d4fb
reverse cycle order in error message
LukaszRozmej Jul 27, 2023
82d45b1
I give up, make test order invariant
LukaszRozmej Jul 27, 2023
795a153
Try fixing test
LukaszRozmej Jul 27, 2023
00963a5
simplify tests
LukaszRozmej Jul 27, 2023
dbe24ff
improve cycle detection with stack
LukaszRozmej Jul 27, 2023
128b104
fix too much cloning
LukaszRozmej Jul 27, 2023
1a94301
slim tests setup
LukaszRozmej Jul 27, 2023
250e97e
Merge branch 'nightly' into lukasz/sorted_dependencies
LukaszRozmej Jul 27, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion adapters/celestia/src/celestia.rs
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,7 @@ impl AsRef<[u8]> for Sha2Hash {
}
}

#[derive(Deserialize, Serialize, PartialEq, Debug, Clone, Eq)]
#[derive(Deserialize, Serialize, PartialEq, Debug, Clone, Eq, Hash)]
pub struct H160(#[serde(deserialize_with = "hex::deserialize")] pub [u8; 20]);

impl AsRef<[u8]> for H160 {
Expand Down
4 changes: 3 additions & 1 deletion adapters/celestia/src/verifier/address.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@ use thiserror::Error;

const HRP: &str = "celestia";

#[derive(Debug, PartialEq, Clone, Eq, Serialize, Deserialize, BorshDeserialize, BorshSerialize)]
#[derive(
Debug, PartialEq, Clone, Eq, Serialize, Deserialize, BorshDeserialize, BorshSerialize, Hash,
)]
// Raw ASCII bytes, including HRP
// TODO: https://github.com/Sovereign-Labs/sovereign-sdk/issues/469
pub struct CelestiaAddress(Vec<u8>);
Expand Down
2 changes: 1 addition & 1 deletion examples/demo-simple-stf/tests/stf_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use sov_rollup_interface::mocks::{MockZkvm, TestBlob};
use sov_rollup_interface::stf::StateTransitionFunction;
use sov_rollup_interface::AddressTrait;

#[derive(PartialEq, Debug, Clone, Eq, serde::Serialize, serde::Deserialize)]
#[derive(PartialEq, Debug, Clone, Eq, serde::Serialize, serde::Deserialize, Hash)]
pub struct DaAddress {
pub addr: [u8; 32],
}
Expand Down
124 changes: 122 additions & 2 deletions module-system/sov-modules-api/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ pub mod macros {
}

use core::fmt::{self, Debug, Display};
use std::collections::{HashMap, HashSet};
use std::str::FromStr;

use borsh::{BorshDeserialize, BorshSerialize};
Expand All @@ -55,7 +56,7 @@ impl AsRef<[u8]> for Address {
impl AddressTrait for Address {}

#[cfg_attr(feature = "native", derive(schemars::JsonSchema))]
#[derive(PartialEq, Clone, Eq, borsh::BorshDeserialize, borsh::BorshSerialize)]
#[derive(PartialEq, Clone, Eq, borsh::BorshDeserialize, borsh::BorshSerialize, Hash)]
pub struct Address {
addr: [u8; 32],
}
Expand Down Expand Up @@ -299,11 +300,14 @@ pub trait ModuleCallJsonSchema: Module {
}

/// Every module has to implement this trait.
pub trait ModuleInfo: Default {
pub trait ModuleInfo {
type Context: Context;

/// Returns address of the module.
fn address(&self) -> &<Self::Context as Spec>::Address;

/// Returns addresses of all the other modules this module is dependent on
fn dependencies(&self) -> Vec<&<Self::Context as Spec>::Address>;
}

/// A StateTransitionRunner needs to implement this if
Expand All @@ -313,6 +317,122 @@ pub trait RpcRunner {
fn get_storage(&self) -> <Self::Context as Spec>::Storage;
}

struct ModuleVisitor<'a, C: Context> {
visited: HashSet<&'a <C as Spec>::Address>,
visited_on_this_path: Vec<&'a <C as Spec>::Address>,
sorted_modules: std::vec::Vec<&'a dyn ModuleInfo<Context = C>>,
}

impl<'a, C: Context> ModuleVisitor<'a, C> {
pub fn new() -> Self {
Self {
visited: HashSet::new(),
sorted_modules: Vec::new(),
visited_on_this_path: Vec::new(),
}
}

/// Visits all the modules and their dependencies, and populates a Vec of modules sorted by their dependencies
fn visit_modules(
&mut self,
modules: Vec<&'a dyn ModuleInfo<Context = C>>,
) -> Result<(), anyhow::Error> {
let mut module_map = HashMap::new();

for module in &modules {
module_map.insert(module.address(), *module);
}

for module in modules {
self.visited_on_this_path.clear();
self.visit_module(module, &module_map)?;
}

Ok(())
}

/// Visits a module and its dependencies, and populates a Vec of modules sorted by their dependencies
fn visit_module(
&mut self,
module: &'a dyn ModuleInfo<Context = C>,
LukaszRozmej marked this conversation as resolved.
Show resolved Hide resolved
module_map: &HashMap<&<C as Spec>::Address, &'a (dyn ModuleInfo<Context = C>)>,
) -> Result<(), anyhow::Error> {
LukaszRozmej marked this conversation as resolved.
Show resolved Hide resolved
let address = module.address();

// if the module have been visited on this path, then we have a cycle dependency
if let Some((index, _)) = self
.visited_on_this_path
.iter()
.enumerate()
.find(|(_, &x)| x == address)
{
let cycle = &self.visited_on_this_path[index..];

anyhow::bail!(
"Cyclic dependency of length {} detected: {:?}",
cycle.len(),
cycle
);
} else {
self.visited_on_this_path.push(address)
}

// if the module hasn't been visited yet, visit it and its dependencies
if self.visited.insert(address) {
for dependency_address in module.dependencies() {
let dependency_module = *module_map.get(dependency_address).ok_or_else(|| {
anyhow::Error::msg(format!("Module not found: {:?}", dependency_address))
})?;
self.visit_module(dependency_module, module_map)?;
}

self.sorted_modules.push(module);
}

// remove the module from the visited_on_this_path list
self.visited_on_this_path.pop();

Ok(())
}
}

/// Sorts ModuleInfo objects by their dependencies
pub fn sort_modules_by_dependencies<C: Context>(
modules: Vec<&dyn ModuleInfo<Context = C>>,
) -> Result<Vec<&dyn ModuleInfo<Context = C>>, anyhow::Error> {
let mut module_visitor = ModuleVisitor::<C>::new();
module_visitor.visit_modules(modules)?;
Ok(module_visitor.sorted_modules)
}

/// Accepts Vec<> of tuples (&ModuleInfo, &TValue), and returns Vec<&TValue> sorted by mapped module dependencies
pub fn sort_values_by_modules_dependencies<C: Context, TValue>(
module_value_tuples: Vec<(&dyn ModuleInfo<Context = C>, TValue)>,
) -> Result<Vec<TValue>, anyhow::Error>
where
TValue: Clone,
{
let sorted_modules = sort_modules_by_dependencies(
module_value_tuples
.iter()
.map(|(module, _)| *module)
.collect(),
)?;

let mut value_map = HashMap::new();

for module in module_value_tuples {
value_map.insert(module.0.address(), module.1);
}

let mut sorted_values = Vec::new();
for module in sorted_modules {
sorted_values.push(value_map.get(module.address()).unwrap().clone());
}

Ok(sorted_values)
}

/// This trait is implemented by types that can be used as arguments in the sov-cli wallet.
/// The recommended way to implement this trait is using the provided derive macro (`#[derive(CliWalletArg)]`).
/// Currently, this trait is a thin wrapper around [`clap::Parser`]
Expand Down
95 changes: 94 additions & 1 deletion module-system/sov-modules-api/src/tests.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
use borsh::{BorshDeserialize, BorshSerialize};

use crate::default_context::DefaultContext;
use crate::default_signature::private_key::DefaultPrivateKey;
use crate::default_signature::{DefaultPublicKey, DefaultSignature};
use crate::Signature;
use crate::{Address, ModuleInfo, Signature};

#[test]
fn test_account_bech32m_display() {
Expand Down Expand Up @@ -44,3 +45,95 @@ fn test_hex_conversion() {
let deserialized_pub_key = DefaultPrivateKey::from_hex(&hex).unwrap().pub_key();
assert_eq!(priv_key.pub_key(), deserialized_pub_key)
}

struct Module {
address: Address,
dependencies: Vec<Address>,
}

impl crate::ModuleInfo for Module {
type Context = DefaultContext;

fn address(&self) -> &<Self::Context as crate::Spec>::Address {
&self.address
}

fn dependencies(&self) -> Vec<&<Self::Context as crate::Spec>::Address> {
self.dependencies.iter().collect()
}
}

#[test]
fn test_sorting_modules() {
let module_a = Module {
address: Address::from([1; 32]),
dependencies: vec![],
};
let module_b = Module {
address: Address::from([2; 32]),
dependencies: vec![module_a.address.clone()],
};
let module_c = Module {
address: Address::from([3; 32]),
dependencies: vec![module_a.address.clone(), module_b.address.clone()],
};

let modules: Vec<(&dyn ModuleInfo<Context = DefaultContext>, i32)> =
vec![(&module_b, 2), (&module_c, 3), (&module_a, 1)];

let sorted_modules = crate::sort_values_by_modules_dependencies(modules).unwrap();

assert_eq!(vec![1, 2, 3], sorted_modules);
}

#[test]
fn test_sorting_modules_missing_module() {
let module_a_address = Address::from([1; 32]);
let module_b = Module {
address: Address::from([2; 32]),
dependencies: vec![module_a_address.clone()],
};
let module_c = Module {
address: Address::from([3; 32]),
dependencies: vec![module_a_address, module_b.address.clone()],
};

let modules: Vec<(&dyn ModuleInfo<Context = DefaultContext>, i32)> =
vec![(&module_b, 2), (&module_c, 3)];

let sorted_modules = crate::sort_values_by_modules_dependencies(modules);

assert!(sorted_modules.is_err());
let error_string = sorted_modules.err().unwrap().to_string();
assert_eq!("Module not found: AddressBech32 { value: \"sov1qyqszqgpqyqszqgpqyqszqgpqyqszqgpqyqszqgpqyqszqgpqyqs259tk3\" }", error_string);
}

#[test]
fn test_sorting_modules_cycle() {
let module_e_address = Address::from([5; 32]);
let module_a = Module {
address: Address::from([1; 32]),
dependencies: vec![],
};
let module_b = Module {
address: Address::from([2; 32]),
dependencies: vec![module_a.address.clone()],
};
let module_d = Module {
address: Address::from([4; 32]),
dependencies: vec![module_e_address.clone()],
};
let module_e = Module {
address: module_e_address,
dependencies: vec![module_a.address.clone(), module_d.address.clone()],
};

let modules: Vec<&dyn ModuleInfo<Context = DefaultContext>> =
vec![&module_b, &module_d, &module_a, &module_e];

let sorted_modules = crate::sort_modules_by_dependencies(modules);

assert!(sorted_modules.is_err());
let error_string = sorted_modules.err().unwrap().to_string();
assert_eq!("Cyclic dependency of length 2 detected: [AddressBech32 { value: \"sov1qszqgpqyqszqgpqyqszqgpqyqszqgpqyqszqgpqyqszqgpqyqszqnu4g3u\" }, AddressBech32 { value: \"sov1q5zs2pg9q5zs2pg9q5zs2pg9q5zs2pg9q5zs2pg9q5zs2pg9q5zskwvj87\" }]", error_string);
}
36 changes: 26 additions & 10 deletions module-system/sov-modules-macros/src/dispatch/genesis.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,25 +42,41 @@ impl GenesisMacro {
type Config = GenesisConfig #type_generics;

fn genesis(&self, config: &Self::Config, working_set: &mut sov_state::WorkingSet<<<Self as sov_modules_api::Genesis>::Context as sov_modules_api::Spec>::Storage>) -> core::result::Result<(), sov_modules_api::Error> {
#(#genesis_fn_body)*
#genesis_fn_body
Ok(())
}
}
}
.into())
}

fn make_genesis_fn_body(fields: &[StructNamedField]) -> Vec<proc_macro2::TokenStream> {
fields
.iter()
.map(|field| {
let ident = &field.ident;
fn make_genesis_fn_body(fields: &[StructNamedField]) -> proc_macro2::TokenStream {
let idents = fields.iter().enumerate().map(|(i, field)| {
let ident = &field.ident;

quote::quote! {
::sov_modules_api::Genesis::genesis(&self.#ident, &config.#ident, working_set)?;
quote::quote! {
(&self.#ident, #i)
}
});

let matches = fields.iter().enumerate().map(|(i, field)| {
let ident = &field.ident;

quote::quote! {
#i => ::sov_modules_api::Genesis::genesis(&self.#ident, &config.#ident, working_set),
}
});

quote::quote! {
let modules: ::std::vec::Vec<(&dyn ::sov_modules_api::ModuleInfo<Context = <Self as sov_modules_api::Genesis>::Context>, usize)> = ::std::vec![#(#idents),*];
let sorted_modules = ::sov_modules_api::sort_values_by_modules_dependencies(modules)?;
for module in sorted_modules {
match module {
#(#matches)*
_ => Err(::sov_modules_api::Error::ModuleError(::anyhow::Error::msg(format!("Module not found: {:?}", module)))),
}?
}
})
.collect()
}
}

fn make_genesis_config(
Expand Down
Loading