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

Add get_key_string method to RuntimeEvent #1107

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions examples/demo-rollup/stf/src/runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ use sov_modules_api::macros::DefaultRuntime;
use sov_modules_api::macros::{expose_rpc, CliWallet};
#[cfg(feature = "native")]
use sov_modules_api::Spec;
use sov_modules_api::{Context, DispatchCall, Genesis, MessageCodec};
use sov_modules_api::{Context, DispatchCall, Event, Genesis, MessageCodec};
#[cfg(feature = "native")]
use sov_nft_module::{NonFungibleTokenRpcImpl, NonFungibleTokenRpcServer};
use sov_rollup_interface::da::DaSpec;
Expand All @@ -64,7 +64,7 @@ use crate::genesis_config::GenesisPaths;

/// The `demo-stf runtime`.
#[cfg_attr(feature = "native", derive(CliWallet), expose_rpc)]
#[derive(Genesis, DispatchCall, MessageCodec, DefaultRuntime)]
#[derive(Genesis, DispatchCall, Event, MessageCodec, DefaultRuntime)]
#[serialization(borsh::BorshDeserialize, borsh::BorshSerialize)]
#[cfg_attr(
feature = "native",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,21 @@ pub enum CallMessage {
),
}

/// This enumeration represents the available events that result from interacting with the `sov-value-setter` module.
#[cfg_attr(
feature = "native",
derive(serde::Serialize),
derive(serde::Deserialize)
)]
#[derive(borsh::BorshDeserialize, borsh::BorshSerialize, Debug, PartialEq, Clone)]
pub enum Event {
/// Value set
ValueSet(
/// new value
u32,
),
}

/// Example of a custom error.
#[derive(Debug, Error)]
enum SetValueError {
Expand All @@ -50,6 +65,7 @@ impl<C: sov_modules_api::Context> ValueSetter<C> {

// This is how we set a new value:
self.value.set(&new_value, working_set);
// TODO: replace add event functionality to be similar to self.event.add()
working_set.add_event("set", &format!("value_set: {new_value:?}"));

Ok(CallResponse::default())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ impl<C: sov_modules_api::Context> sov_modules_api::Module for ValueSetter<C> {

type CallMessage = call::CallMessage;

type Event = ();
type Event = call::Event;

fn genesis(&self, config: &Self::Config, working_set: &mut WorkingSet<C>) -> Result<(), Error> {
// The initialization logic
Expand Down
45 changes: 43 additions & 2 deletions module-system/sov-modules-macros/src/event.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,44 @@ impl<'a> StructDef<'a> {
})
.collect()
}

fn create_get_key_string_impl(&self) -> proc_macro2::TokenStream {
let enum_ident = self.enum_ident(EVENT);

let match_legs: Vec<proc_macro2::TokenStream> = self
.fields
.iter()
.map(|field| {
let module_name = &field.ident;
let module_name_str = &field.ident.to_string();
quote::quote!(
#enum_ident::#module_name(inner)=>{
let enum_name: String = format!("{:?}", inner)
.split('(')
.collect::<Vec<&str>>()[0].to_string();
format!("{}-{}", #module_name_str, enum_name)
},
)
})
.collect();

let impl_generics = &self.impl_generics;
let enum_ident = self.enum_ident(EVENT);
let where_clause = &self.where_clause;
let ty_generics = &self.type_generics;

quote::quote! {
impl #impl_generics #enum_ident #ty_generics #where_clause {

/// Returns a string that identifies both the module and the event type
pub fn get_key_string(&self) -> String {
match self {
#(#match_legs)*
}
}
}
}
}
}

impl EventMacro {
Expand Down Expand Up @@ -62,12 +100,15 @@ impl EventMacro {
where_clause,
);

let event_enum_legs = struct_def.create_event_enum_legs();
let event_enum = struct_def.create_enum(&event_enum_legs, EVENT, &serialization_methods);
let enum_legs = struct_def.create_event_enum_legs();
let event_enum = struct_def.create_enum(&enum_legs, EVENT, &serialization_methods);
let get_key_string_impl = struct_def.create_get_key_string_impl();

Ok(quote::quote! {
#[doc="This enum is generated from the underlying Runtime, the variants correspond to events from the relevant modules"]
#event_enum

#get_key_string_impl
}
.into())
}
Expand Down
33 changes: 28 additions & 5 deletions module-system/sov-modules-macros/tests/dispatch/derive_event.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
mod modules;
use modules::{first_test_module, second_test_module};
use modules::{first_test_module, second_test_module, third_test_module};
use sov_modules_api::default_context::DefaultContext;
use sov_modules_api::macros::DefaultRuntime;
use sov_modules_api::{
Expand All @@ -12,12 +12,35 @@ struct Runtime<C: Context>
{
pub first: first_test_module::FirstTestStruct<C>,
pub second: second_test_module::SecondTestStruct<C>,
pub third: third_test_module::ThirdTestStruct<C, u32>,
}

fn main() {
// Check to see if the runtime events are getting initialized correctly
let _event = RuntimeEvent::<DefaultContext>::first(first_test_module::Event::FirstModuleEnum1(10));
let _event = RuntimeEvent::<DefaultContext>::first(first_test_module::Event::FirstModuleEnum2);
let _event = RuntimeEvent::<DefaultContext>::first(first_test_module::Event::FirstModuleEnum3(vec![1;3]));
let _event = RuntimeEvent::<DefaultContext>::second(second_test_module::Event::SecondModuleEnum);
{
let event = RuntimeEvent::<DefaultContext>::first(first_test_module::Event::FirstModuleEnum1(10));
assert_eq!(event.get_key_string(), "first-FirstModuleEnum1");
Copy link
Member

@bkolad bkolad Oct 25, 2023

Choose a reason for hiding this comment

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

Will this generate an expected string even if Event is struct/tuple/array/nested enum instead of an enum type?

}

{
let event = RuntimeEvent::<DefaultContext>::first(first_test_module::Event::FirstModuleEnum2);
assert_eq!(event.get_key_string(), "first-FirstModuleEnum2");
}

{
let event = RuntimeEvent::<DefaultContext>::first(first_test_module::Event::FirstModuleEnum3(vec![1; 3]));
assert_eq!(event.get_key_string(), "first-FirstModuleEnum3");
Copy link
Member

@bkolad bkolad Oct 25, 2023

Choose a reason for hiding this comment

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

To create the key string using the get_key_string function, we require a value of Event::FirstModuleEnum3(..). However, obtaining this value necessitates providing a Vec this is fine when we save to the db.

Now, let's consider a scenario where a user wishes to query the database using the first-FirstModuleEnum3 key. What will be the method for the user to calculate the first-FirstModuleEnum3? Since he doesn't have a value of the Event::FirstModuleEnum3(...) he can't call get_key_string(&self). We have to think about the cases where the enums contain arbitrary complex structures and possibly other nested enums.

}

{
let event = RuntimeEvent::<DefaultContext>::second(second_test_module::Event::SecondModuleEnum);
assert_eq!(event.get_key_string(), "second-SecondModuleEnum");
}

{
// Not sure if this is how we'd want to keep this. But wanted to highlight this edge case.
let event = RuntimeEvent::<DefaultContext>::third(());
assert_eq!(event.get_key_string(), "third-");
}

}