Feature/wasm contract env #478
Conversation
WalkthroughThe changes introduce support for the Changes
Sequence Diagram(s)sequenceDiagram
participant Developer
participant Cargo
participant RustCompiler
participant WasmModule
Developer->>Cargo: Add wasm32 dependencies
Cargo->>RustCompiler: Compile with wasm32 target
RustCompiler->>WasmModule: Generate wasm32 specific code
WasmModule-->>RustCompiler: Provide WasmContractEnv
RustCompiler-->>Cargo: Complete build
Cargo-->>Developer: Build successful
Poem
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 (
|
Benchmark report
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (12)
- core/Cargo.toml (1 hunks)
- core/src/contract_env.rs (3 hunks)
- core/src/lib.rs (2 hunks)
- core/src/wasm/consts.rs (1 hunks)
- core/src/wasm/host_functions.rs (1 hunks)
- core/src/wasm/mod.rs (1 hunks)
- core/src/wasm/wasm_contract_env.rs (1 hunks)
- odra-casper/proxy-caller/Cargo.toml (1 hunks)
- odra-casper/wasm-env/src/lib.rs (1 hunks)
- odra-casper/wasm-env/src/wasm_contract_env.rs (1 hunks)
- odra/Cargo.toml (1 hunks)
- odra/src/lib.rs (1 hunks)
Files skipped from review due to trivial changes (5)
- core/src/wasm/consts.rs
- core/src/wasm/mod.rs
- odra-casper/proxy-caller/Cargo.toml
- odra-casper/wasm-env/src/lib.rs
- odra/Cargo.toml
Additional comments not posted (12)
core/Cargo.toml (1)
18-21: Dependencies forwasm32target added correctly.Ensure that these dependencies are compatible with the rest of the project and do not introduce version conflicts.
core/src/lib.rs (1)
4-5: Attributes and wasm module added forwasm32target.Also applies to: 60-61
Verification successful
Attributes
#![allow(internal_features)]and#![cfg_attr(not(test), feature(core_intrinsics))]are present in bothcore/src/lib.rsandodra-casper/wasm-env/src/lib.rs. Let's verify if these attributes are necessary by checking for usage of core intrinsics and internal features in the codebase.
The attributes
#![allow(internal_features)]and#![cfg_attr(not(test), feature(core_intrinsics))]are necessary due to the usage ofcore::intrinsics::abort()and the need to allow internal features for wasm-specific functionality.
core/src/wasm/mod.rs: Usage ofcore::intrinsics::abort()odra-casper/wasm-env/src/lib.rs: Commented-out usage ofcore::intrinsics::abort()Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the necessity and impact of the new attributes on the project. # Test: Search for usage of internal features and core intrinsics in the project. rg --type rust $'internal_features|core_intrinsics'Length of output: 306
Script:
#!/bin/bash # Description: Check for usage of core intrinsics and internal features in the codebase. # Search for core intrinsics usage rg --type rust 'core::intrinsics' # Search for internal features usage rg --type rust 'internal_features'Length of output: 292
odra/src/lib.rs (1)
63-63: Import statement updated to use the newwasmmodule fromodra_core.Ensure that this change is reflected wherever
odra_casper_wasm_envis used in the project.odra-casper/wasm-env/src/wasm_contract_env.rs (1)
105-115: Conditional compilation used for creatingContractEnvinstances based on the target architecture.Verification successful
The search results indicate that the
new_envandnewmethods are indeed used in the codebase, specifically in theodra-casper/wasm-env/src/wasm_contract_env.rsfile. However, to ensure the correct functionality of these conditionally compiled methods, we need to verify their usage in different contexts and ensure they are correctly invoked based on the target architecture.To proceed, let's gather more specific information about the usage of these methods in the context of the target architecture. We will search for the conditional compilation flags (
#[cfg(not(target_arch = "wasm32"))]and#[cfg(target_arch = "wasm32")]) to see how these methods are being utilized.
The search results confirm that the conditional compilation flags
#[cfg(not(target_arch = "wasm32"))]and#[cfg(target_arch = "wasm32")]are used extensively throughout the codebase. Specifically, thenew_envandnewmethods inodra-casper/wasm-env/src/wasm_contract_env.rsare correctly conditionally compiled based on the target architecture.Given the extensive use of these conditional compilation flags and the specific context in which
new_envandneware used, it appears that the implementation is consistent and correctly handles the different target architectures.Therefore, the conditional compilation for creating
ContractEnvinstances based on the target architecture is correctly implemented and necessary.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correct functionality of the conditionally compiled methods. # Test: Search for usage of `new_env` and `new` methods in the project. rg --type rust $'new_env|new'Length of output: 92039
Script:
#!/bin/bash # Description: Verify the correct functionality of the conditionally compiled methods by searching for conditional compilation flags. # Search for the usage of `#[cfg(not(target_arch = "wasm32"))]` and `#[cfg(target_arch = "wasm32")]` in the project. rg --type rust '#\[cfg\(not\(target_arch = "wasm32"\)\)\]' --context 5 rg --type rust '#\[cfg\(target_arch = "wasm32"\)\]' --context 5Length of output: 53047
core/src/wasm/wasm_contract_env.rs (3)
15-101: Implementation ofContractContextforWasmContractEnvlooks comprehensive and well-structured.
109-112: Theborrowmethod correctly returns a reference toself, which is a typical pattern in Rust for borrowing instances.
105-107: Ensure that thenew_envfunction correctly initializesWasmContractEnv.core/src/contract_env.rs (2)
Line range hint
37-62: Conditional compilation forwasm32and non-wasm32targets is correctly implemented inContractEnv.
Line range hint
44-62: Thenewmethod for both wasm32 and non-wasm32 targets correctly initializesContractEnv. Ensure that theindexandmapping_dataare appropriately used throughout the contract logic.Verification successful
The
indexandmapping_datafields in theContractEnvstruct are used appropriately within the contract logic. Theindexis utilized for key generation and creating child environments, whilemapping_datais used for extending key data and adding to the mapping data.
core/src/contract_env.rs
indexis used in key generation and child environment creation.mapping_datais used for extending key data and adding to the mapping data.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that `index` and `mapping_data` are used correctly in contract logic. # Test: Search for usage of `index` and `mapping_data`. Expect: Correct usage in contract logic. rg --type rust $'index|mapping_data'Length of output: 14949
core/src/wasm/host_functions.rs (3)
112-116: Therevertfunction uses inline optimization and correctly reverts the contract execution using the Casper API.
159-183: Theset_valuefunction correctly writes values to the contract's storage. Review the use of unsafe blocks to ensure safety.
51-110: Theinstall_contractfunction correctly handles contract installation with checks for key overrides and upgradability. Ensure that error handling is robust and that theTODOcomment is addressed.
| /// Returns given named argument passed to the host. The result is not deserialized, | ||
| /// is returned as a `Vec<u8>`. | ||
| pub fn get_named_arg(name: &str) -> Result<Vec<u8>, ApiError> { | ||
| let arg_size = get_named_arg_size(name)?; | ||
| if arg_size > 0 { | ||
| let data_non_null_ptr = contract_api::alloc_bytes(arg_size); | ||
| let ret = unsafe { | ||
| ext_ffi::casper_get_named_arg( | ||
| name.as_bytes().as_ptr(), | ||
| name.len(), | ||
| data_non_null_ptr.as_ptr(), | ||
| arg_size | ||
| ) | ||
| }; | ||
| if ret != 0 { | ||
| return Err(ApiError::from(ret as u32)); | ||
| } | ||
| unsafe { | ||
| Ok(Vec::from_raw_parts( | ||
| data_non_null_ptr.as_ptr(), | ||
| arg_size, | ||
| arg_size | ||
| )) | ||
| } | ||
| } else { | ||
| Ok(Vec::new()) | ||
| } | ||
| } |
There was a problem hiding this comment.
Tip
Codebase Verification
The get_named_arg function in core/src/wasm/host_functions.rs does handle errors by returning an ApiError when ext_ffi::casper_get_named_arg returns a non-zero value. However, the error messages are not detailed, which could be improved for better debugging.
core/src/wasm/host_functions.rs: The function handles errors but lacks detailed error messages.
Analysis chain
Ensure that get_named_arg correctly handles errors and returns the expected results. Consider adding more detailed error messages for better debugging.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify error handling in `get_named_arg`.
# Test: Search for error handling in `get_named_arg`. Expect: Detailed error messages.
rg --type rust $'get_named_arg'
Length of output: 5618
Benchmark report
|
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- core/Cargo.toml (1 hunks)
- odra/Cargo.toml (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- core/Cargo.toml
- odra/Cargo.toml
Benchmark report
|
I was wondering what would happen if I move
odra-casper-wasm-envtoodra-coreand conditionally don't usedyninContractEnvforwasm32.Summary by CodeRabbit
New Features
Chores
RUSTFLAGSenvironment variable for benchmarking.Refactor