-
Notifications
You must be signed in to change notification settings - Fork 166
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
feat(dojo-lang): add systems to contract's manifest and fix DojoAuxData
gathering
#2309
Conversation
WalkthroughOhayo, sensei! The recent changes enhance contract management in the Dojo framework by introducing a Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Contract
participant System
User->>Contract: Deploy contract
Contract->>System: Register systems
System->>Contract: Acknowledge registration
Contract->>User: Provide contract details
sequenceDiagram
participant User
participant Contract
participant Plugin
User->>Contract: Call function
Contract->>Plugin: Process request
Plugin->>Contract: Return results
Contract->>User: Send response
TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (3)
crates/dojo-lang/src/plugin.rs (2)
101-101
: Review added logging for debugging.A logging statement has been added to print
module_ast
. Confirm that this addition aids debugging without exposing sensitive information.
68-69
: Ohayo, sensei! It looks like the transition fromsystems
tocontracts
in theDojoAuxData
struct isn't fully consistent across the codebase. Here are some points to consider:
systems
Field: This field is still present and used inplugin.rs
,contract.rs
, andcompiler.rs
.contracts
Field: This field is correctly used in several places, but the presence ofsystems
suggests incomplete transition.Please review these areas to ensure that the
DojoAuxData
struct consistently usescontracts
where intended.Analysis chain
Check reorganization in
DojoAuxData
.The
DojoAuxData
struct now holdscontracts
instead ofsystems
. Ensure this change is consistent across the codebase.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of `DojoAuxData` to ensure the new `contracts` field is correctly utilized. # Test: Search for the struct usage. Expect: Correct handling of the new field. rg --type rust 'DojoAuxData' crates/dojo-lang/src/Length of output: 1039
Script:
#!/bin/bash # Description: Verify the usage of the `contracts` field in `DojoAuxData` and check for any remaining `systems` references. # Test: Search for the usage of `contracts` in DojoAuxData. rg --type rust 'contracts' crates/dojo-lang/src/ # Test: Search for any remaining references to `systems` in DojoAuxData. rg --type rust 'systems' crates/dojo-lang/src/Length of output: 6196
crates/dojo-lang/src/compiler.rs (1)
136-136
: Remove debug print statements before merging.The
println!
statement is likely used for debugging and should be removed for cleaner production code.- println!("Decl qualified path: {:?}", qualified_path);
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2309 +/- ##
==========================================
- Coverage 69.59% 67.04% -2.55%
==========================================
Files 352 352
Lines 46032 45819 -213
==========================================
- Hits 32034 30718 -1316
- Misses 13998 15101 +1103 ☔ View full report in Codecov by Sentry. |
Coverage is affected by this PR due to the fact that we don't build the 3 tests in debug mode, which is almost doubling the test time. We anyway build those projects in the CI, but not directly from the code, so codecov can't see this coverage. |
DojoAuxData
gatheringDojoAuxData
gathering
All dojo contracts are starknet contract under the hood. However, the way we were gathering the data was always falling back into
StarkNetAuxData
instead of actually using theDojoAuxData
.This was due to the fact that a dojo contract
module_id
was actually the parent module, and not the contract's module itself, hence never found in the artifacts paths.This PR also adds a new entry in the manifest for dojo contract with the list of systems, which are all the functions that are not returning any data.
Summary by CodeRabbit
New Features
systems
fields in various contract configurations to enhance functionality and organization.DojoContract
structure to include a list of systems, improving its capability to manage system-related information.Bug Fixes
Documentation
Chores