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 #1556

Merged
merged 16 commits into from
Mar 14, 2024

Conversation

Matth26
Copy link
Contributor

@Matth26 Matth26 commented Feb 19, 2024

This PR addresses Issue #882, resolves DOJ-108, focusing on leveraging the cainome parser for ABI parsing and processing Emitted Events.

Current Progress

  • Successfully integrated the cainome parser for parsing ABI and Emitted Events.
  • CoreBasic and Array data structures are correctly parsed.

@glihm, could I have your review on the work done so far to see if it aligns with your expectations?

Copy link
Collaborator

@glihm glihm left a comment

Choose a reason for hiding this comment

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

Awesome, thanks for the first draft of this! Definitely aligned. 🚀

I left a comment about a function, other than that will take a deeper look after the cleanup and testing.

But running sozo locally to test that, seems all good for now compared to the previous implementation.

Appreciate the emphasis on testing you've started. 👍

bin/sozo/src/ops/events.rs Outdated Show resolved Hide resolved
@Matth26
Copy link
Contributor Author

Matth26 commented Feb 21, 2024

Thanks for the review !

But running sozo locally to test that, seems all good for now compared to the previous implementation.

Yes I've checked on events I had locally to be the same as before, but couldn't test them all (hence the test for the Span)
And I fixed the custom event code which was not working if the event had more than 2 keys (which is the case in my game)

To allow some configuration, I think we should have 2 types or choices (mostly for felts and integers)

Sure, I did it quick and dirty with the is_nested but a proper config would be better

This can be done later, just wanted to point out that it's concerning only cainome logic so, may be better to move it there. 👍

Totally agree, I'll move this in cainome repo 👍

@Matth26 Matth26 changed the title [WIP] refactor(sozo): make event parsing logic modular and reusable refactor(sozo): make event parsing logic modular and reusable Feb 23, 2024
@Matth26 Matth26 marked this pull request as ready for review February 23, 2024 09:39
@Matth26
Copy link
Contributor Author

Matth26 commented Feb 23, 2024

I haven't added the print_value method to CoreBasic yet, as I wanted to know how to manage the cainome version.
However, the code is otherwise ready for review. I've cleaned up the code and added tests.

Copy link

codecov bot commented Feb 25, 2024

Codecov Report

Attention: Patch coverage is 93.05556% with 30 lines in your changes are missing coverage. Please review.

Project coverage is 70.10%. Comparing base (88bae9f) to head (440cbb6).
Report is 1 commits behind head on main.

Files Patch % Lines
bin/sozo/src/ops/events.rs 93.05% 30 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1556      +/-   ##
==========================================
+ Coverage   69.55%   70.10%   +0.55%     
==========================================
  Files         266      266              
  Lines       27217    27536     +319     
==========================================
+ Hits        18931    19305     +374     
+ Misses       8286     8231      -55     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@kariy kariy left a comment

Choose a reason for hiding this comment

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

Good work on this @Matth26!

Left some comments after briefly went through it.

bin/sozo/src/commands/events.rs Outdated Show resolved Hide resolved
bin/sozo/src/ops/events.rs Outdated Show resolved Hide resolved
bin/sozo/src/ops/events.rs Outdated Show resolved Hide resolved
bin/sozo/src/ops/events.rs Outdated Show resolved Hide resolved
bin/sozo/src/ops/events.rs Outdated Show resolved Hide resolved
@Matth26
Copy link
Contributor Author

Matth26 commented Feb 26, 2024

Thanks @kariy for the review. I've updated the error handling based on your suggestions, and I think it's better now. Please let me know your thoughts 👍

@Matth26 Matth26 requested a review from kariy March 5, 2024 09:24
Copy link
Collaborator

@glihm glihm left a comment

Choose a reason for hiding this comment

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

Thanks for the work here @Matth26!
Some minor comments. I'll probably check how we can integrate the printing of the core token into cainome. Keep you posted.

bin/sozo/src/ops/events.rs Outdated Show resolved Hide resolved
bin/sozo/src/ops/events.rs Outdated Show resolved Hide resolved
Copy link
Collaborator

@glihm glihm left a comment

Choose a reason for hiding this comment

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

Thank you for the effort @Matth26 and the time dedicated to that!
Cainome integration can be done later, and it looks great for a first rework on event support with cainome, good job! 🚀

Copy link
Member

@kariy kariy left a comment

Choose a reason for hiding this comment

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

looks good to me... just need to address the one thing and then we're good

bin/sozo/src/ops/events.rs Outdated Show resolved Hide resolved
@Matth26
Copy link
Contributor Author

Matth26 commented Mar 12, 2024

I've just added the error as @kariy suggested

Copy link
Collaborator

@glihm glihm left a comment

Choose a reason for hiding this comment

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

Thanks @Matth26 for the work here!
As seen with @kariy we may enhance how the ABI is retrieved from a file (currently being the scarb artifacts). But this is for a next PR.

@glihm glihm merged commit 948233c into dojoengine:main Mar 14, 2024
12 checks passed
glihm added a commit that referenced this pull request Mar 25, 2024
* fix: handle primitive and custom types for enum introspect (#1643)

The function `handle_introspect_enum` handles the derive Introspect attribute for an enum.
At the moment, two cases are supported: tuples and type paths.

A type path may be a primitive type or a custom type (struct or enum).
A tuple may be composed of primitive and/or custom types.

This PR updates the function `handle_introspect_enum` to handle all these cases properly.

* fix: remove old reference to submodule (#1612)

* fix: remove old reference to submodule

* Update submodule

* Will this now work?

* Removed old files

* Correctly updated gitmodules

* chore: update forge std

---------

Co-authored-by: glihm <dev@glihm.net>

* fix(sozo): ensure warnings don't stop tests build (#1648)

* fix: ensure warnings are not blocking the test build

* fix: update default world address for spawn and move example

* fix: [torii/graphql] Added timezone information to timestamps strings (#1657)

* Check model exists on `sozo auth` (#1644)

* feat: check model name and address before granting write access

* Update crates/dojo-world/src/contracts/model.rs

Co-authored-by: glihm <dev@glihm.net>

* Update bin/sozo/src/ops/auth.rs

Co-authored-by: glihm <dev@glihm.net>

---------

Co-authored-by: glihm <dev@glihm.net>

* fix: error on `world.model()` call gets mapped wrongly (#1661)

* sozo: check existing class-hashes before registering model (#1576)

* check existing class-hashes before registering model

* update checking if models registered logic

* apply review feedback

* add initial test fixture for reregister

* check if models need reregistering using remote manifest

* test ouput of the reregister command

* apply further review feedback

* allow dead code for test utils

* feat: add transaction_hash and block_number in the manifest file (#1651)

* feat: add transaction_hash and block_number in the manifest file

Store the `transaction_hash` and the `block_number` of the World deployment in the
manifest file.

* merge transaction_hash and block_number from previous deployed manifest if exists

* update spawn-and-move example

* fix fmt issues

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

* refacto: using cainome parser in order to parse events in sozo

* refacto: clean logs + add tests

* fix: clippy error

* refacto: best error handling

* update how felt are displayed + remove  unnecessary expect in tests

* fix tests

* fix: fmt and artifacts parsing for missing events

* fix: adjust event count as now world and base are included

* return error from parse_and_print_events function

* fix: ensure world address is always present to filter events

---------

Co-authored-by: glihm <dev@glihm.net>

* feat(sozo): add `--receipt` transaction option (#1647)

* feat(katana): add simulate transactions (#1590)

* add simulation to the RPC

* simulate transaction on sequencer

* add `new_query`

* add fee type

* lint

* update to `simulate_transactions`

* remove `simulation.rs`

* individual parameters for `simulate_transactions`

* lint rebase

* update to fit spec

* pin link

* Prepare release: v0.6.0-alpha.6 (#1666)

* feat(relay): persist and expose messages through grpc (#1526)

* feat(libp2p): start working on persistence & grpc

* feat: add relay proto and db migration

* feat(libp2p): relaystorage for message persistence

* feat(libp2p): relay subscription service

* refactor: start implementing models/ty for libp2p

* refactor(torii-core): generic set ty query

* feat: use models for libp2p messages

* feat: rpcs for subscring and retrieving messages

* refactor(torii-core): arc rwlock for sql database & work on messages for models

* refactor: tests & write message to db

* refactor(grpc): grpc service & subscription

* refactor: use entitiy type and models for messages

* chore: fmt & clippy

* refactor: unused async

* chore: world descriptor

* refactor: unused deps & fmt

* chore: remove post types

* chore: unused libp2p deps

* chore: non wasm deps

* chore

* chore: fmt

* fix: test

* refactor: check if entity already exists for libp2p messages

* chore: fmt & clippy

* feat(libp2p): start working on identity and signatures for messages

* refactor(libp2p): use array for signature type and check its ocmponents

* refactor: decouple logic

* refactor(libpo2p): extract identity & signature from validate message

* feat(libp2p): eip 712 typed data

* feat(libp2p): start workling on typed data structure snip-12

* refactor: change name

* feat: handle type encoding

* feat: typed data message encoding

* refactor: rework in progress for using snip-12

* feat(libp2p): testing typed data with mocks & tests

* fix: typed data passing test for enums

* feat: test for presets and refactor type primitives

* refactor: string parsing for felts

* fix: dont include type hash on children objects

* fix: json ordering issue with serde_json

* fix: typed data for shortstring

* fix: message encoding - passes all tests

* feat: start supporting enums

* feat: handle enum variant types

* chore: rebase and fix db rwlock

* refactor: encode all object types & start fixing enums

* fix: enum test

* fix: preset types. full compatibility with starknet js

* refactor: more optimized approach for preset types

* feat(libp2p): valid message signature & identity

* refactor: wip identity and verifying message

* refactor: get entity model identity & check

* wip: start working on encoding typed data to Ty

* refactor: implement ty types for encode typed data

* refactor: parse object to ty function

* refactor: set entity from relay w verification

* feat: sign last signature and check signed last sig against last sig in db

* chore: fmt

* fix: torii grpc

* refactor: sqlx onlyu server side

* chore: clippy

* chore: use char

* refactor: sql reference

* chore: fmt & clippy

* refactor: typed data payload

* fix: clippy

* use RESOURCE_METADATA_MODEL constant

* fix fmt

* skip old ModelRegistered events

* fix(torii): Added timezone informations to naive datetime strings (#1668)

* fix(torii): Added timezone informations to naive datetimes

* fix(torii_naive_datetimes): Quick parameters renaming

* fix(torii_naive_datetimes): Used rust fmt script

* fix(torii_naive_datetimes): Naive datetime refacto

* fix(torii_naive_datetimes): Quick refacotring

* fix(torii_naive_datetimes): Fixed cargo clippy error

* fix(torii_naive_datetimes): Updated datetime retrieval

* get version from dojo::model attribute

* take version into account to generate selector and version functions

* refactor: write manifest file even if migration failed (#1652)

* refactor: write manifest file before trying to deploy them

* fix lints

* sort fs entries for deterministic order in manifest

* write migration output for partial migrations

* make suggested changes

* feat(sozo): add seed to manifest (#1674)

* feat(sozo): add seed to manifest

* make suggested changes

* fix(sozo): don't upload to ipfs if in offline mode (#1678)

* Torii fix queries with keys regex (#1609)

* [sozo] Detect and manage manifests and artifacts discrepancies  (#1672)

feat: detect and manage manifests and artifacts discrepancies

`sozo` uses several file types (previously generated by `sozo build` and `sozo migrate`)
as input for migration. All of these files may be corrupt or may have discrepancies due
to an older format version.

This PR introduces a new `sozo clean` command to clean the project of these files.
`sozo clean --manifest-abis` cleans only manifest and ABI files.
`sozo clean --artifacts` cleans only artifacts.

When an anomaly is detected in a file, the user is informed of the problem
and advised to run `sozo clean` to clean up the project.

* Prepare release: v0.6.0-alpha.7 (#1680)

Co-authored-by: glihm <glihm@users.noreply.github.com>

* Add explorer flag to open World Explorer in browser (#1581)

* Add explorer flag to open World Explorer

* Log error message instead of unwrap when fail to open world explorer on browser

* fix typo

* fix typo 2

---------

Co-authored-by: glihm <dev@glihm.net>

* after review

* refactor(torii): retrieve events and store only relevant txns (#1631)

* feat(torii): parallelize torii sync_range func

* refactor: retrieve events & store only relevant txns

* refactor: fix chunk size overflow & handle next events pages

* chore: fmt

* chore: clippy

* chore: fmt

* refactor: execute all txns at end of sync

* chore: code

* chroe: suggested changes

* feat: configure events page chunk size

* fmt

* refactor: include chink size in config

* fix

* Split sozo ops into its own crate (#1546)

* Split sozo ops into its own crate

* fix: minor fixes and adjust commands documentation

* fix: add missing file

* fix: ensure migration failure is not silent

---------

Co-authored-by: Jonatan Chaverri <jonatan.chaverri@kyndryl.com>
Co-authored-by: glihm <dev@glihm.net>

* fix base_test error

* feat: add eventmessage for emitting models & start refactoring emit macro (#1656)

* feat: add eventmessage for emitting models & start refactoring emit macro

* refactor: emit multiple models event

* feat: event message processor

* feat: emit_message world func emit event mssage

* refactor: catch all event processor

* refactor: check model key as name

* feat: model name keccak as id & rework model events

* fix: storing entities with model hash

* refactor: catch all event message  and store

* feat: emit model evrnt from spawn and move spawn

* fix: pass keys as array

* feat: fix emit macro and correctly index model events

* feat: event messages migrations & set

* feat: store events messages and new id system

* chore: fmt

* fix: keys array

* feat: add grpc endpoint for event messages

* feat: graphql schema for event messages

* cadd comments for moel name hash

* revert world.cario changes

* fix: graphql entity and model connection

* refactor: only test models name ordering

* fix: entity/modeldata relation

* refactor: remove event testing

* fix: cairo code

* refactor: add back event rxample

* fix: merge

* chore: migration

* fix: subscription test

* fix: tests

* fix: subscription test

* fix: sql test

* chore: format model id correctly in test

* fix: model subscription with id

* fix: schema for model

* chore: revert modelmmeber type

* fix: event message query

* fix: ensure sozo clean only affect base (#1685)

The overlays are managed by the user. We don't want to clean them.
The deployments are only output, and can't cause any conflict.
For this reason, only base should be clean.

* sozo: add dry-run mode (#1686)

* fix: rebuild artifacts

---------

Co-authored-by: RareSecond <jasper@codictive.be>
Co-authored-by: glihm <dev@glihm.net>
Co-authored-by: akhercha <akherchache@pm.me>
Co-authored-by: Ammar Arif <evergreenkary@gmail.com>
Co-authored-by: kwkr <kawka.maciej.93@gmail.com>
Co-authored-by: Matthias <matthias.monnier@gmail.com>
Co-authored-by: greged93 <82421016+greged93@users.noreply.github.com>
Co-authored-by: Tarrence van As <tarrencev@users.noreply.github.com>
Co-authored-by: Larko <59736843+Larkooo@users.noreply.github.com>
Co-authored-by: lambda-0x <0xlambda@protonmail.com>
Co-authored-by: Yun <broody@gmail.com>
Co-authored-by: glihm <glihm@users.noreply.github.com>
Co-authored-by: Junichi Sugiura <jun.sugiura.jp@gmail.com>
Co-authored-by: Jonatan Chaverri <jonathan.chaverri12@gmail.com>
Co-authored-by: Jonatan Chaverri <jonatan.chaverri@kyndryl.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants