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

feat(dojo-core): add selector_from_names for cairo runtime #2219

Merged
merged 1 commit into from
Jul 26, 2024

Conversation

glihm
Copy link
Collaborator

@glihm glihm commented Jul 26, 2024

Description

Dojo compiler plugin proposes a selector_from_tag! macro. However, no variables can be used in their.

This PR adds a selector_from_name utils function, that can compute a selector of a resource providing it's namespace and name.

Summary by CodeRabbit

  • New Features

    • Introduced a new utility function, selector_from_names, for enhanced resource selector computation.
    • Expanded the public module exports to include the new function alongside existing utilities.
  • Tests

    • Added a new test function, test_selector_computation, to validate the new selector functionality, improving overall testing coverage.

Copy link

coderabbitai bot commented Jul 26, 2024

Walkthrough

Ohayo, sensei! The recent updates enhance the dojo-core library by adding the public function selector_from_names in the utils module, allowing the computation of resource selectors. Additionally, a new test function, test_selector_computation, has been introduced to validate this functionality within the test suite. These changes aim to improve both the utility and robustness of the library.

Changes

Files Change Summary
crates/dojo-core/src/lib.cairo Added selector_from_names to the public exports of the utils module.
crates/dojo-core/src/tests/utils.cairo Introduced test_selector_computation to validate the new selector functionality; updated imports.
crates/dojo-core/src/utils/utils.cairo Implemented selector_from_names for computing a selector using namespace and name parameters.

Recent review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between f731597 and 084a74f.

Files selected for processing (3)
  • crates/dojo-core/src/lib.cairo (1 hunks)
  • crates/dojo-core/src/tests/utils.cairo (2 hunks)
  • crates/dojo-core/src/utils/utils.cairo (1 hunks)
Additional comments not posted (5)
crates/dojo-core/src/tests/utils.cairo (2)

2-2: Ohayo, sensei! Import statement looks good.

The import of selector_from_names is correctly added to the dojo::utils module.


22-28: Ohayo, sensei! The new test function is well-structured.

The test_selector_computation function correctly tests the selector computation using the new selector_from_names function. This enhances the test coverage for the new utility function.

crates/dojo-core/src/lib.cairo (1)

48-48: Ohayo, sensei! Export statement looks good.

The export of selector_from_names is correctly added to the utils module, making it publicly accessible.

crates/dojo-core/src/utils/utils.cairo (2)

16-16: Ohayo, sensei! Documentation looks good.

The documentation for selector_from_names clearly explains its purpose and parameters.


17-19: Ohayo, sensei! The new function implementation looks solid.

The selector_from_names function correctly computes the selector by hashing the namespace and name using poseidon_hash_span.

@glihm glihm changed the title feat: add selector_from_names for cairo runtime feat(dojo-core): add selector_from_names for cairo runtime Jul 26, 2024
Copy link

codecov bot commented Jul 26, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 69.42%. Comparing base (f731597) to head (084a74f).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2219      +/-   ##
==========================================
+ Coverage   69.41%   69.42%   +0.01%     
==========================================
  Files         341      341              
  Lines       44473    44473              
==========================================
+ Hits        30870    30876       +6     
+ Misses      13603    13597       -6     

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

@glihm glihm merged commit 7be5dc7 into dojoengine:main Jul 26, 2024
15 checks passed
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.

1 participant