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 3 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
6 changes: 6 additions & 0 deletions adapters/celestia/src/celestia.rs
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,12 @@ impl Display for H160 {
}
}

impl std::hash::Hash for H160 {
fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
self.0.as_ref().hash(state);
}
}

impl AddressTrait for H160 {}

pub fn parse_pfb_namespace(
Expand Down
6 changes: 6 additions & 0 deletions adapters/celestia/src/verifier/address.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,12 @@ impl FromStr for CelestiaAddress {
}
}

impl std::hash::Hash for CelestiaAddress {
fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
self.0.hash(state);
}
}

impl AddressTrait for CelestiaAddress {}

#[cfg(test)]
Expand Down
6 changes: 6 additions & 0 deletions examples/demo-simple-stf/tests/stf_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,12 @@ impl Display for DaAddress {
}
}

impl std::hash::Hash for DaAddress {
fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
self.addr.hash(state);
}
}

#[test]
fn test_stf() {
let address = DaAddress { addr: [1; 32] };
Expand Down
53 changes: 52 additions & 1 deletion module-system/sov-modules-api/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ pub use sov_rollup_interface::crypto::SimpleHasher as Hasher;
pub use sov_rollup_interface::AddressTrait;
use sov_state::{Storage, Witness, WorkingSet};
use thiserror::Error;
use std::collections::{HashMap, HashSet};

pub use crate::bech32::AddressBech32;

Expand All @@ -35,6 +36,12 @@ impl AsRef<[u8]> for Address {
}
}

impl std::hash::Hash for Address {
fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
self.addr.hash(state);
}
}

impl AddressTrait for Address {}

#[cfg_attr(feature = "native", derive(schemars::JsonSchema))]
Expand Down Expand Up @@ -252,11 +259,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 @@ -265,3 +275,44 @@ pub trait RpcRunner {
type Context: Context;
fn get_storage(&self) -> <Self::Context as Spec>::Storage;
}

/// Sorts ModuleInfo objects by their dependencies
pub fn sort_by_dependencies<C: Context>(
modules: Vec<&dyn ModuleInfo<Context = C>>,
) -> Result<Vec<&dyn ModuleInfo<Context = C>>, anyhow::Error> {

fn visit_module<'a, C: Context>(
module: &'a dyn ModuleInfo<Context = C>,
LukaszRozmej marked this conversation as resolved.
Show resolved Hide resolved
visited: &mut HashSet<&'a C::Address>,
sorted_modules: &mut std::vec::Vec<&'a (dyn ModuleInfo<Context = C> + 'a)>,
module_map: &HashMap<&<C as Spec>::Address, &'a (dyn ModuleInfo<Context = C> + 'a)>
) -> Result<(), anyhow::Error> {
LukaszRozmej marked this conversation as resolved.
Show resolved Hide resolved
let address = module.address();
if visited.insert(address) {
LukaszRozmej marked this conversation as resolved.
Show resolved Hide resolved
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)))?;
visit_module(dependency_module, visited, sorted_modules, module_map)?;
}

sorted_modules.push(module);
}

Ok(())
}

let mut module_map = HashMap::new();

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

let mut sorted_modules = Vec::new();
let mut visited = HashSet::new();
for module in modules {
visit_module(module, &mut visited, &mut sorted_modules, &module_map)?;
}

Ok(sorted_modules)
}
20 changes: 19 additions & 1 deletion module-system/sov-modules-macros/src/module_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ impl<'a> StructDef<'a> {

let mut impl_self_init = Vec::default();
let mut impl_self_body = Vec::default();
let mut modules = Vec::default();

let mut module_address = None;
for field in fields.iter() {
Expand All @@ -106,6 +107,7 @@ impl<'a> StructDef<'a> {
FieldKind::Module(field) => {
impl_self_init.push(make_init_module(field)?);
impl_self_body.push(&field.ident);
modules.push(&field.ident);
}
FieldKind::Address(field) => {
impl_self_init.push(make_init_address(
Expand All @@ -127,6 +129,7 @@ impl<'a> StructDef<'a> {
let where_clause = self.where_clause;

let fn_address = make_fn_address(module_address)?;
let fn_dependencies = make_fn_dependencies(modules);

Ok(quote::quote! {
use ::sov_modules_api::AddressTrait;
Expand All @@ -145,6 +148,8 @@ impl<'a> StructDef<'a> {
type Context = #generic_param;

#fn_address

#fn_dependencies
}
})
}
Expand Down Expand Up @@ -238,7 +243,7 @@ fn make_fn_address(
) -> Result<proc_macro2::TokenStream, syn::Error> {
match address_ident {
Some(address_ident) => Ok(quote::quote! {
fn address(&self) -> &<Self::Context as sov_modules_api::Spec>::Address{
fn address(&self) -> &<Self::Context as sov_modules_api::Spec>::Address {
&self.#address_ident
}
}),
Expand All @@ -249,6 +254,19 @@ fn make_fn_address(
}
}

fn make_fn_dependencies(modules: Vec<&proc_macro2::Ident>) -> proc_macro2::TokenStream {
let address_tokens = modules.iter().map(|ident| {
quote::quote! {
&self.#ident.address()
}
});

quote::quote! {
fn dependencies(&self) -> Vec<&<Self::Context as sov_modules_api::Spec>::Address> {
LukaszRozmej marked this conversation as resolved.
Show resolved Hide resolved
vec![#(#address_tokens),*]
}
}
}
fn make_init_state(field: &StructNamedField) -> Result<proc_macro2::TokenStream, syn::Error> {
let prefix_fun = prefix_func_ident(&field.ident);
let field_ident = &field.ident;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use sov_modules_api::default_context::ZkDefaultContext;
use sov_modules_api::Context;
use sov_modules_macros::ModuleInfo;
use sov_state::StateMap;
use sov_modules_api::ModuleInfo;

pub mod first_test_module {
use super::*;
Expand Down Expand Up @@ -70,4 +71,6 @@ fn main() {
)
.into()
);

assert_eq!(second_test_struct.dependencies(), [second_test_struct.module_in_second_struct_1.address()]);
}
6 changes: 6 additions & 0 deletions rollup-interface/src/state_machine/mocks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,12 @@ impl Display for MockAddress {
}
}

impl std::hash::Hash for MockAddress {
fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
self.addr.hash(state);
}
}

impl AddressTrait for MockAddress {}

#[derive(
Expand Down
1 change: 1 addition & 0 deletions rollup-interface/src/state_machine/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,5 +26,6 @@ pub trait AddressTrait:
+ Send
+ Sync
+ core::fmt::Display
+ std::hash::Hash
{
}