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

refactor(sozo): make event parsing logic modular and reusable #882

Closed
lambda-0x opened this issue Sep 8, 2023 · 11 comments
Closed

refactor(sozo): make event parsing logic modular and reusable #882

lambda-0x opened this issue Sep 8, 2023 · 11 comments
Assignees
Labels
good first issue Good for newcomers ODHack OnlyDust Hack fest sozo
Milestone

Comments

@lambda-0x
Copy link
Contributor

lambda-0x commented Sep 8, 2023

Currently event parsing logic using in sozo events is very declarative and not reusable, it should be refactored in such a way that it could be re-used by other components of dojo.

It can be done in few ways currently like implementing a method on Manifest or creating a new struct which contains manifest (and hence the abi) and events.

related: xJonathanLEI/starknet-rs#286


EDIT

Now cainome has the parser library which can be used to parse cairo types from string.

In sozo, the event parsing may then be enhanced using cainome.

@lambda-0x
Copy link
Contributor Author

https://github.com/glihm/starknet-abigen-rs this has ability to parse EmittedEvents, i will take a look if it works for us.

@ponderingdemocritus
Copy link
Contributor

are you still actioning this @lambda-0x ?

@lambda-0x
Copy link
Contributor Author

@glihm mentioned that the repo is not meant to be consumed directly, waiting for xJonathanLEI/starknet-rs#475 to be merged.

@glihm
Copy link
Collaborator

glihm commented Oct 23, 2023

@glihm mentioned that the repo is not meant to be consumed directly, waiting for xJonathanLEI/starknet-rs#475 to be merged.

I've updated the README in the PR + changed the linked in the repo. If you need to test out something, you can try with this new syntax: https://github.com/glihm/starknet-rs/tree/abigen/starknet-macros.

But yes... until it's merged, it's still on the fork and not the official repo. And may be broken based on the feedback of Jonathan during the review.

@ponderingdemocritus
Copy link
Contributor

Putting this on 0.5.0 release

@ponderingdemocritus ponderingdemocritus added this to the 0.5.0 milestone Nov 30, 2023
@glihm
Copy link
Collaborator

glihm commented Dec 23, 2023

Now abigen! macro lives into cainome repository which should be ready to do this kind of stuff.

@glihm glihm added the ODHack OnlyDust Hack fest label Feb 14, 2024
@Matth26
Copy link
Contributor

Matth26 commented Feb 16, 2024

I've checked how cainome abigen works, can I try this one?

@glihm
Copy link
Collaborator

glihm commented Feb 16, 2024

Nice to see you around @Matth26, do not hesitate if you have questions on this. 👍

@Matth26
Copy link
Contributor

Matth26 commented Feb 17, 2024

Hi @glihm ! Thanks to the !abigen macro I'm able to deserialize EmittedEvent. However, I'm uncertain how this will address the specific need described in this issue. I can display events based on their types through the event enum, but this approach isn't generic. Each event must be individually formatted according to our requirements, and this method does not works with custom events.

I might be missing something. How do you envision resolving this?

@glihm
Copy link
Collaborator

glihm commented Feb 17, 2024

Hi @glihm ! Thanks to the !abigen macro I'm able to deserialize EmittedEvent. However, I'm uncertain how this will address the specific need described in this issue. I can display events based on their types through the event enum, but this approach isn't generic. Each event must be individually formatted according to our requirements, and this method does not works with custom events.

I might be missing something. How do you envision resolving this?

The idea is to avoid parsing string of types, which is already handled by the parser crate. You actually don't need to use abigen! in this one. :)

Take the lines here:

"core::starknet::contract_address::ContractAddress"
| "core::starknet::class_hash::ClassHash" => {
let value = match data.pop_front() {
Some(addr) => addr,
None => continue 'outer,
};
ret.push_str(&format!("{}: {:#x}\n", field.name, value));
}
"core::felt252" => {
let value = match data.pop_front() {
Some(addr) => addr,
None => continue 'outer,
};
let value = match parse_cairo_short_string(&value) {
Ok(v) => v,
Err(_) => format!("{:#x}", value),
};
ret.push_str(&format!("{}: {}\n", field.name, value));
}
"core::integer::u8" => {
let value = match data.pop_front() {
Some(addr) => addr,
None => continue 'outer,
};
let num = match value.to_string().parse::<u8>() {
Ok(num) => num,
Err(_) => continue 'outer,
};
ret.push_str(&format!("{}: {}\n", field.name, num));
}
"dojo_examples::systems::move::Direction" => {
let value = match data.pop_front() {
Some(addr) => addr,
None => continue 'outer,
};
ret.push_str(&format!("{}: {}\n", field.name, value));
}
"core::array::Span::<core::felt252>" => {
let length = match data.pop_front() {
Some(addr) => addr,
None => continue 'outer,
};
let length = match length.to_string().parse::<usize>() {
Ok(len) => len,
Err(_) => continue 'outer,
};
ret.push_str(&format!("{}: ", field.name));
if data.len() >= length {
ret.push_str(&format!(
"{:?}\n",
data.drain(..length)
.map(|e| format!("{:#x}", e))
.collect::<Vec<_>>()
));
.

Instead of parsing each possible type, we will prefer using the token parsing from Cainome, which already does that for us:
https://github.com/cartridge-gg/cainome/blob/54df2a4114c0c61359c2f1a70bc7e5fb57d9eaf2/crates/parser/src/tokens/mod.rs#L32

And instead, we use the cainome token to decide how print the values.
Doing so, the code will be more generic and will apply to any type supported by Cainome instead of manually registered types.

Hope it clarifies, if not don't hesitate to ask other questions. 👍

@Matth26
Copy link
Contributor

Matth26 commented Feb 17, 2024

Yes, that makes sense! I thought we needed to use the abigen macro, which led me in the wrong direction. Thanks you the quick response. I'll reach out if I have any further questions :)

@ponderingdemocritus ponderingdemocritus moved this to 🏗 In progress in Dojo Feb 26, 2024
@linear linear bot closed this as completed Mar 14, 2024
@github-project-automation github-project-automation bot moved this from In Progress to Done in Dojo Mar 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers ODHack OnlyDust Hack fest sozo
Projects
No open projects
Status: Done
Development

No branches or pull requests

4 participants