Update dependencies and fix test environment setup #324
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
WalkthroughThe series of changes across various files reflect a consistent update to event handling and argument passing, particularly removing the reference operator when calling Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 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 Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to path filters (1)
odra-vm/Cargo.tomlis excluded by:!**/*.toml
Files selected for processing (14)
- .github/workflows/odra-ci.yml (1 hunks)
- examples/bin/build_schema.rs (2 hunks)
- examples/src/contracts/owned_token.rs (1 hunks)
- examples/src/contracts/tlw.rs (2 hunks)
- examples/src/features/access_control.rs (4 hunks)
- examples/src/features/events.rs (1 hunks)
- examples/src/features/module_nesting.rs (1 hunks)
- examples/src/features/native_token.rs (1 hunks)
- justfile (2 hunks)
- modules/src/access/ownable.rs (4 hunks)
- odra-macros/src/ir/attr.rs (1 hunks)
- odra-vm/src/vm/contract_container.rs (3 hunks)
- odra-vm/src/vm/odra_vm.rs (2 hunks)
- odra-vm/src/vm/storage.rs (1 hunks)
Files skipped from review due to trivial changes (1)
- odra-macros/src/ir/attr.rs
Additional comments: 38
.github/workflows/odra-ci.yml (1)
- 29-30: The uncommenting of the
just prepare-test-envandjust testcommands in the CI workflow file re-enables the test environment setup and the execution of tests. This is a positive change as it ensures that tests are run as part of the CI process, which is crucial for maintaining code quality.examples/bin/build_schema.rs (2)
- 1-1: Adding
clippy::single_component_path_importsto theallowattribute is appropriate if the project's linting rules have been updated to allow this pattern. This change should not affect the functionality of the code.- 18-18: Removing the reference operator
&before theformat!macro call is a minor change that simplifies the code. This change is syntactically correct asformat!returns aStringwhich does not need to be referenced when passed toFile::create.examples/src/features/native_token.rs (3)
- 26-26: Removing the
&operator frommy_contract.address()in the balance check assertion is correct if thebalance_offunction now expects anAddressvalue directly instead of a reference. This change should be consistent across all usages ofbalance_of.- 29-29: The same applies to the balance check assertion on line 29. The removal of the
&operator is consistent with the previous change and is correct if thebalance_offunction's signature has been updated accordingly.- 32-32: Again, the removal of the
&operator frommy_contract.address()in the balance check assertion is consistent with the other changes and is correct if thebalance_offunction's signature has been updated to accept anAddressvalue directly.examples/src/features/events.rs (1)
- 34-34: Modifying the argument passed to the
emitted_eventmethod to remove the reference toparty_contract.address()is correct if the method's signature has been updated to accept anAddressvalue directly. This change should be verified across all usages ofemitted_eventto ensure consistency.justfile (5)
- 5-5: Adding the
--all-targetsflag to thecargo clippycommand in theclippytask ensures that lint checks are run on all targets, which is a good practice for comprehensive code quality checks.- 7-7: The addition of
--all-targetsto thecargo clippycommand in theexamplesdirectory is consistent with the change made in theclippytask and is a good practice.- 8-8: The addition of
--all-targetsto thecargo clippycommand in themodulesdirectory is consistent with the changes made in theclippyandexamplestasks and ensures that lint checks are run on all targets.- 20-20: Adding
cargo check --all-targetsto themodulestask ensures that the code compiles for all targets, which is a good practice for maintaining code quality.- 22-22: Including
cargo check --all-targetsin theexamplestask is consistent with the addition made in themodulestask and ensures that the code compiles for all targets.examples/src/contracts/owned_token.rs (2)
- 75-75: The change to the
emitted_eventfunction call to remove the reference totoken.address()is correct if theemitted_eventmethod's signature has been updated to accept anAddressvalue directly. This change should be verified across all usages ofemitted_eventto ensure consistency.- 82-82: The same applies to the
emitted_eventfunction call on line 82. The removal of the&operator is consistent with the previous change and is correct if theemitted_eventmethod's signature has been updated accordingly.examples/src/features/module_nesting.rs (1)
- 154-154: Modifying the argument passed to the
emitted_eventmethod from&nested_odra_types.address()tonested_odra_types.address()is correct if the method's signature has been updated to accept anAddressvalue directly. This change should be verified across all usages ofemitted_eventto ensure consistency.odra-vm/src/vm/contract_container.rs (3)
- 86-86: Changing the
TEST_ENTRYPOINTconstant from&'static strto&stris a simplification that removes unnecessary lifetime specification. This change is syntactically correct and should not affect the functionality of the code.- 178-178: Replacing
odra::odra_test::odra_env()withodra_test::env()is a simplification that likely reflects a change in the module structure or import paths. This change is syntactically correct and should not affect the functionality of the code.- 199-199: The same simplification is applied here, replacing
odra::odra_test::odra_env()withodra_test::env(). This change is consistent with the previous one and is syntactically correct.examples/src/contracts/tlw.rs (4)
- 145-145: The change in functionality involves modifying the arguments passed to the
emitted_eventfunction within themod testmodule. The&contract.address()calls have been replaced withcontract.address()in multiple instances, affecting the event emission logic.- 152-152: The same applies to the
emitted_eventfunction call on line 152. The removal of the&operator is consistent with the previous change and is correct if theemitted_eventmethod's signature has been updated accordingly.- 206-206: Again, the removal of the
&operator fromcontract.address()in theemitted_eventfunction call is consistent with the other changes and is correct if theemitted_eventmethod's signature has been updated to accept anAddressvalue directly.- 213-213: The same applies to the
emitted_eventfunction call on line 213. The removal of the&operator is consistent with the previous changes and is correct if theemitted_eventmethod's signature has been updated accordingly.examples/src/features/access_control.rs (7)
- 122-122: The removal of the reference operator (
&) before thecontract.address()calls within theemitted_eventfunction is correct if the method's signature has been updated to accept anAddressvalue directly. This change should be verified across all usages ofemitted_eventto ensure consistency.- 166-166: The same applies to the
emitted_eventfunction call on line 166. The removal of the&operator is consistent with the previous change and is correct if theemitted_eventmethod's signature has been updated accordingly.- 174-174: Again, the removal of the
&operator fromcontract.address()in theemitted_eventfunction call is consistent with the other changes and is correct if theemitted_eventmethod's signature has been updated to accept anAddressvalue directly.- 182-182: The same applies to the
emitted_eventfunction call on line 182. The removal of the&operator is consistent with the previous changes and is correct if theemitted_eventmethod's signature has been updated accordingly.- 209-209: The removal of the
&operator fromcontract.address()in theemitted_eventfunction call is correct if the method's signature has been updated to accept anAddressvalue directly. This change should be verified across all usages ofemitted_eventto ensure consistency.- 238-238: The same applies to the
emitted_eventfunction call on line 238. The removal of the&operator is consistent with the previous change and is correct if theemitted_eventmethod's signature has been updated accordingly.- 246-246: Again, the removal of the
&operator fromcontract.address()in theemitted_eventfunction call is consistent with the other changes and is correct if theemitted_eventmethod's signature has been updated to accept anAddressvalue directly.odra-vm/src/vm/storage.rs (1)
- 168-168: The simplification of the
serializecall fromserialize(&next_value.clone())toserialize(&next_value)is correct and removes unnecessary cloning of the value before serialization. This change improves performance by avoiding an extra clone operation.modules/src/access/ownable.rs (6)
- 174-174: The removal of the reference operator (
&) beforeownable.address()is consistent with the summary indicating an API change or refactor for cleaner code.- 175-175: The removal of the reference operator (
&) beforeownable_2step.address()is consistent with the summary indicating an API change or refactor for cleaner code.- 191-191: The removal of the reference operator (
&) beforecontract.address()is consistent with the summary indicating an API change or refactor for cleaner code.- 218-218: The removal of the reference operator (
&) beforecontract.address()is consistent with the summary indicating an API change or refactor for cleaner code.- 225-225: The removal of the reference operator (
&) beforecontract.address()is consistent with the summary indicating an API change or refactor for cleaner code.- 291-291: The removal of the reference operator (
&) beforecontract.address()is consistent with the summary indicating an API change or refactor for cleaner code.odra-vm/src/vm/odra_vm.rs (2)
- 316-316: The change of
TEST_ENTRY_POINTfrom&'static strto&stris a simplification that removes unnecessary lifetime specification. This is a good change if the constant does not need to have a'staticlifetime.- 563-563: The function
test_callernow callsodra_test::env()instead ofodra::odra_test::odra_env(). This change simplifies the call and is consistent with the summary indicating a refactor for cleaner code.
This was
linked to
issues
Jan 19, 2024
Closed
|
odradev/cargo-odra#64 needs to be merged first |
zie1ony
approved these changes
Jan 24, 2024
There was a problem hiding this comment.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files ignored due to path filters (2)
core/Cargo.tomlis excluded by:!**/*.tomlodra-vm/Cargo.tomlis excluded by:!**/*.toml
Files selected for processing (9)
- core/src/contract_container.rs (5 hunks)
- core/src/contract_context.rs (1 hunks)
- core/src/host_context.rs (1 hunks)
- core/src/lib.rs (1 hunks)
- examples/src/features/module_nesting.rs (1 hunks)
- odra-macros/src/ast/deployer_item.rs (6 hunks)
- odra-macros/src/ast/test_parts.rs (6 hunks)
- odra-test/src/lib.rs (1 hunks)
- odra-vm/src/vm/odra_vm.rs (7 hunks)
Files skipped from review as they are similar to previous changes (2)
- examples/src/features/module_nesting.rs
- odra-vm/src/vm/odra_vm.rs
Additional comments: 10
odra-test/src/lib.rs (1)
- 11-11: The change in the
envfunction signature to returnHostEnvdirectly is noted. Ensure that all calls to this function are updated to reflect this change.Verification successful
---
--- The ripgrep search has produced a substantial output, showing numerous instances where the `env()` function is called throughout various files in the codebase. The results include calls to `odra_test::env()`, which is the function signature mentioned in the review comment, as well as other variations of `env()` calls within different contexts and files. Given the extensive references to `env()` and related functions across multiple files, it is clear that the `env()` function is widely used throughout the codebase. This confirms that the change in the function signature to return `HostEnv` directly is significant and that all calls to this function should indeed be updated to reflect this change, as initially commented. Based on the provided output, the review comment is verified to be consistent with the codebase. </details> <details> <summary>Scripts Executed</summary> The following scripts were executed for the analysis: --- Script: ```shell #!/bin/bash # Search for calls to the `env` function to ensure they are updated. ast-grep --lang rust --pattern $'env()'Length of output: 40
Script:
#!/bin/bash # Search for any potential calls to the `env` function within the Rust codebase. ast-grep --lang rust --pattern $'_ = env()'Length of output: 44
Script:
#!/bin/bash # Broaden the search to capture all possible usages of the `env` function. ast-grep --lang rust --pattern $'env($_)'Length of output: 42
Script:
#!/bin/bash # Use ripgrep to perform a text-based search for "env" to ensure no usage is missed. rg 'env\(\)'Length of output: 27058
core/src/contract_context.rs (1)
- 4-5: The addition of
#[cfg_attr(test, allow(unreachable_code))]and#[cfg_attr(test, mockall::automock)]attributes to theContractContexttrait is correct for enabling conditional compilation and mocking in tests.core/src/host_context.rs (1)
- 9-9: The addition of the
#[cfg_attr(test, mockall::automock)]attribute to theHostContexttrait is appropriate for enabling mocking in tests.core/src/lib.rs (1)
- 1-1: The use of
#![cfg_attr(not(test), no_std)]is correct for conditionally enabling the standard library for non-test builds.core/src/contract_container.rs (3)
- 79-79: The modification of the
TEST_ENTRYPOINTconstant from&'static strto&stris correct and should not cause any issues as long as the constant is used consistently throughout the codebase.- 113-113: The addition of a boolean argument in the
CallDef::newcalls is noted. Ensure that the new argument is correctly handled whereverCallDef::newis called.- 171-172: Refactoring of the
ContractContainer::emptyfunction to use a mock context and environment is appropriate for testing purposes.odra-macros/src/ast/deployer_item.rs (2)
- 134-178: The addition of the
epcmethod to theErc20Deployerstruct, which initializes and returns anodra::EntryPointsCaller, is a significant change. Ensure that this new method is integrated correctly with the rest of the codebase.- 265-297: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [264-274]
The redefinition of the
initmethod to return anErc20HostRefand the introduction of theloadmethod are significant changes. Verify that these methods are used correctly and that theinitmethod no longer requires thetotal_supplyparameter.odra-macros/src/ast/test_parts.rs (1)
- 232-238: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [219-261]
The refactoring of the
Erc20Deployerstruct's methods, including the alteredinitmethod signature and the newepcandloadmethods, is noted. Ensure that these changes are consistent with the intended usage patterns and that they are correctly reflected in the test parts.
kubaplas
approved these changes
Jan 25, 2024
zie1ony
approved these changes
Jan 26, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Chores