Conversation
WalkthroughThis update introduces a new Rust CLI tool under the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI (newcli)
participant AuroraClient
participant NEAR RPC
User->>CLI (newcli): Run command with arguments
CLI (newcli)->>CLI (newcli): Parse CLI args (clap)
CLI (newcli)->>AuroraClient: Initialize client with context
CLI (newcli)->>AuroraClient: Dispatch command (async)
AuroraClient->>NEAR RPC: Perform blockchain operation (view/call)
NEAR RPC-->>AuroraClient: Return result
AuroraClient-->>CLI (newcli): Process and format result
CLI (newcli)-->>User: Output result (plain/JSON)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~40 minutes Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
🧰 Additional context used🧬 Code Graph Analysis (1)newcli/src/command/near.rs (3)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
🔇 Additional comments (13)
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
🧹 Nitpick comments (11)
Cargo.toml (1)
23-23: Pinbase64with a compatible MSRV or feature set.
base64 0.22raises the MSRV to 1.63+. As the workspace statesrust-version = "1.86.0"there is no immediate conflict, but double-check that leaf crates (especially anything built for Wasm) do not rely on the#![no_std]path wheredefault-features = falseis required.sdk/src/near/mod.rs (1)
7-7: Alias may shadow the upstreamcryptocrate.Re-exporting
near_cryptoas a flatcryptomodule is convenient but can collide with the popularcryptocrate name in downstream code and breakuse crypto::digest-style imports. Considerpub use near_crypto as near_crypto;(keep original) or a more specific alias likenc_crypto.newcli/src/context.rs (1)
5-8: Derive common traits and add a constructor for ergonomics.Public field exposure is handy for quick prototyping, yet:
- You’ll likely need
Clone,Debug, maybeDefaultonContextfor tests and async re-entrance.- A
newconstructor can centralise client initialisation logic later.-pub struct Context { - pub cli: Cli, - pub client: Client, -} +#[derive(Clone, Debug)] +pub struct Context { + pub cli: Cli, + pub client: Client, +} + +impl Context { + pub fn new(cli: Cli, client: Client) -> Self { + Self { cli, client } + } +}sdk/Cargo.toml (1)
14-19: Re-evaluatestdfeature flags for library portability.
aurora-engine-sdk,libsecp256k1, and others are compiled with"std".
Ifaurora-sdk-rsis ever built forwasm32-unknown-unknown(e.g. browser or smart-contract integration tests), these flags prevent compilation. Consider gating them behind a Cargo feature:[features] default = ["with-std"] with-std = ["aurora-engine-sdk/std", "libsecp256k1/std"]Then depend on
with-stdonly in the CLI crates.Also applies to: 25-26
newcli/src/common/mod.rs (2)
24-34: Complex error handling chain may mask decode/deserialize errors.The
reference_hashparsing uses a complex chain that ends with.ok(), which silently discards any base64 decode or borsh deserialization errors. This could make debugging difficult.Consider logging or propagating specific errors:
- reference_hash: json - .get("reference_hash") - .and_then(serde_json::Value::as_str) - .ok_or_else(|| anyhow::anyhow!("reference_hash must be a string")) - .and_then(|s| { - base64::engine::general_purpose::STANDARD - .decode(s) - .map_err(Into::into) - }) - .and_then(|bytes| FungibleReferenceHash::try_from_slice(&bytes).map_err(Into::into)) - .ok(), + reference_hash: json + .get("reference_hash") + .and_then(serde_json::Value::as_str) + .map(|s| { + base64::engine::general_purpose::STANDARD + .decode(s) + .map_err(Into::into) + .and_then(|bytes| FungibleReferenceHash::try_from_slice(&bytes).map_err(Into::into)) + }) + .transpose()?,
35-39: Unnecessary JSON value cloning.The
decimalsfield clones the JSON value unnecessarily.- decimals: serde_json::from_value( - json.get("decimals") - .ok_or_else(|| anyhow::anyhow!("Missing decimals field"))? - .clone(), - )?, + decimals: json + .get("decimals") + .ok_or_else(|| anyhow::anyhow!("Missing decimals field"))? + .as_u64() + .ok_or_else(|| anyhow::anyhow!("decimals must be a number"))? as u8,newcli/src/cli.rs (1)
47-49: Consider more specific error handling.The generic
Into::intoconversion could be more explicit about the expected error type.pub(crate) fn signer(&self) -> anyhow::Result<Signer> { - InMemorySigner::from_file(&self.near_key_path).map_err(Into::into) + InMemorySigner::from_file(&self.near_key_path).map_err(anyhow::Error::from) }sdk/src/aurora/abi.rs (2)
24-43: Consider supporting native JSON types for complex arguments.The current implementation expects all argument values to be strings, which requires users to stringify complex types like arrays. Consider supporting native JSON representations for better usability.
66-66: Remove unnecessary reference operators.The
&operator is not needed before.len()calls sincelen()returns a value, not a reference.- if &bytes.len() != size { + if bytes.len() != size {And on line 74:
- if &tokens.len() != size { + if tokens.len() != size {Also applies to: 74-74
sdk/src/aurora/contract/mod.rs (1)
81-93: Simplify the error creation block.The nested block is unnecessary and can be simplified.
fn parse(value: Vec<u8>) -> Result<Self, Error> { Self::from_eth(U256::from_big_endian(&value)).ok_or_else(|| { - { - io::Error::new( - io::ErrorKind::InvalidData, - "Failed to convert bytes to WeiU256", - ) - } - .into() + io::Error::new( + io::ErrorKind::InvalidData, + "Failed to convert bytes to WeiU256", + ) + .into() }) }newcli/src/command/mod.rs (1)
1029-1029: Remove unnecessary double references.Using
&&creates a reference to a reference, which is unnecessary here.- &&context.cli.output_format, + &context.cli.output_format,Also applies to: 1204-1204, 1211-1211
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (18)
Cargo.toml(2 hunks)newcli/Cargo.toml(1 hunks)newcli/src/cli.rs(1 hunks)newcli/src/command/mod.rs(1 hunks)newcli/src/command/near.rs(1 hunks)newcli/src/common/mod.rs(1 hunks)newcli/src/common/output.rs(1 hunks)newcli/src/context.rs(1 hunks)newcli/src/main.rs(1 hunks)sdk/Cargo.toml(1 hunks)sdk/src/aurora/abi.rs(1 hunks)sdk/src/aurora/common.rs(3 hunks)sdk/src/aurora/contract/mod.rs(2 hunks)sdk/src/aurora/contract/read.rs(2 hunks)sdk/src/aurora/contract/write.rs(4 hunks)sdk/src/aurora/error.rs(2 hunks)sdk/src/aurora/mod.rs(2 hunks)sdk/src/near/mod.rs(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (6)
newcli/src/main.rs (1)
newcli/src/command/mod.rs (1)
run(489-1218)
newcli/src/cli.rs (1)
cli/src/config.rs (1)
from_file(16-20)
sdk/src/aurora/mod.rs (1)
sdk/src/aurora/contract/mod.rs (16)
parse(24-26)parse(30-32)parse(36-38)parse(42-44)parse(48-50)parse(54-56)parse(60-64)parse(68-72)parse(76-78)parse(82-92)parse(96-98)parse(102-104)parse(108-111)parse(115-117)parse(121-123)parse(127-130)
sdk/src/aurora/common.rs (4)
sdk/src/aurora/client.rs (1)
new(14-16)sdk/src/aurora/error.rs (4)
from(19-29)from(33-35)from(39-41)from(258-367)newcli/src/command/near.rs (1)
gen_key_pair(473-475)sdk/src/aurora/mod.rs (3)
parse(73-73)parse(128-132)parse(136-138)
sdk/src/aurora/contract/mod.rs (1)
sdk/src/aurora/mod.rs (3)
parse(73-73)parse(128-132)parse(136-138)
sdk/src/aurora/contract/write.rs (2)
sdk/src/aurora/contract/read.rs (28)
method_name(67-69)method_name(87-89)method_name(107-109)method_name(131-133)method_name(151-153)method_name(171-173)method_name(195-197)method_name(215-217)method_name(235-237)method_name(253-255)method_name(277-279)method_name(301-303)method_name(325-327)method_name(345-347)params(75-77)params(95-97)params(115-117)params(139-141)params(159-161)params(175-177)params(203-205)params(223-225)params(243-245)params(257-259)params(285-287)params(309-311)params(333-335)params(353-355)sdk/src/aurora/mod.rs (3)
method_name(45-45)params(56-58)deposit(52-54)
🔇 Additional comments (15)
Cargo.toml (1)
13-15: Confirm build-matrix covers the new member.Adding
newclito the workspace list is fine, but CI scripts (e.g.cargo check --workspace --all-targets) must already include it, otherwise the new crate may silently miss compilation in some jobs.newcli/src/main.rs (1)
1-13: Clean and well-structured CLI entry point.The main function follows standard Rust CLI conventions with proper async setup, clear module organization, and appropriate error handling delegation to the command module.
sdk/src/aurora/mod.rs (3)
8-8: New ABI module addition enhances contract interaction capabilities.The addition of the
abimodule aligns with the expanded ABI parsing functionality needed for the CLI tool.
15-16: Useful re-exports for enhanced SDK functionality.The new re-exports (
near_account_to_evm_address,transactions,ethabi) provide convenient access to essential functionality for Aurora/NEAR interactions.Also applies to: 21-21
72-74: AllContractMethodResponseimplementations include explicitparsemethodsVerified that every
impl ContractMethodResponse for …insdk/src/aurora/{mod.rs,contract/mod.rs}provides its ownfn parse(…)implementation. No further changes required.newcli/Cargo.toml (1)
1-21: Well-structured Cargo manifest with appropriate dependencies.The package configuration properly uses workspace inheritance and includes all necessary dependencies for a CLI tool interacting with Aurora and NEAR blockchain. The feature configurations for
clapandlibsecp256k1are appropriate.sdk/src/aurora/abi.rs (1)
9-22: LGTM!The function correctly handles both ABI file formats (raw array and object with "abi" field) with proper error propagation.
sdk/src/aurora/common.rs (3)
40-50: LGTM!Both
hex_to_vecandaddress_from_secret_keyare well-implemented with proper error handling.
52-73: LGTM!The transaction signing implementation correctly follows EIP-155 with proper chain ID handling.
95-101: LGTM!The function correctly attempts to parse as hex address first, then falls back to AccountId parsing with proper error handling.
newcli/src/common/output.rs (1)
5-65: LGTM!The
CommandResultenum and formatting functions are well-structured with proper handling of both plain text and JSON output formats.sdk/src/aurora/contract/mod.rs (2)
75-143: LGTM for the remaining implementations!The implementations for
SubmitResult,Option<SiloParamsArgs>,WhitelistStatusArgs,Erc20Metadata,PausedMask,serde_json::Value,StorageBalance, andTransactionStatusproperly use appropriate deserialization methods with error handling.
120-124: ConfirmH256::from_slicebehavior or add an explicit length checkWe weren’t able to locate the
from_sliceimplementation in our repo (it lives in the externalaurora_engine_typescrate), so please verify whether it panics or returns an error on slices not exactly 32 bytes. If it doesn’t already guard against invalid lengths, update theparseimpl insdk/src/aurora/contract/mod.rs:• File: sdk/src/aurora/contract/mod.rs
Lines 120–124 (insideimpl ContractMethodResponse for H256)Suggested diff:
- fn parse(value: Vec<u8>) -> Result<Self, Error> { - Ok(Self::from_slice(&value)) - } + fn parse(value: Vec<u8>) -> Result<Self, Error> { + if value.len() != 32 { + return Err(Error::InvalidInput(format!( + "H256 slice must be exactly 32 bytes, got {} bytes", + value.len() + ))); + } + Ok(Self::from_slice(&value)) + }– If
Error::InvalidInput(or a similar variant) doesn’t exist yet, introduce it.
– Otherwise, confirmH256::from_slicealready enforces this invariant and document it here.newcli/src/command/mod.rs (2)
26-487: LGTM!The
Commandenum is well-structured with comprehensive documentation and appropriate clap attributes for argument parsing.
1220-1256: LGTM!The parsing helper functions are well-implemented with proper error handling and informative error messages.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
sdk/src/aurora/contract/read.rs (1)
294-296: Make theaddressfield publicThe
addressfield is private, which prevents instantiation of this struct from outside the module. This makes theFtBalanceOfEthview method unusable.pub struct FtBalanceOfEth { - address: Address, + pub address: Address, }sdk/src/aurora/contract/write.rs (1)
68-73: Make theargsfield public for consistencyThe
argsfield is private, which is inconsistent with other similar structs likeRemoveRelayerKeyand prevents external usage.pub struct AddRelayerKey { #[contract_param(serialize_as = "json")] - args: RelayerKeyArgs, + pub args: RelayerKeyArgs, }
🧹 Nitpick comments (1)
newcli/src/command/near.rs (1)
1073-1075: Extract duplicate secret key parsing logic into a helper functionThe secret key parsing logic is duplicated from lines 453-455. Consider extracting this into a reusable helper function to follow the DRY principle.
Add a helper function:
fn parse_aurora_secret_key(key: &str) -> anyhow::Result<SecretKeyEth> { common::hex_to_arr(key.trim()) .and_then(|bytes| SecretKeyEth::parse(&bytes).map_err(Into::into)) .map_err(|e| anyhow::anyhow!("Couldn't create secret key from hex: {e}")) }Then update both occurrences:
- let secret_key = common::hex_to_arr(aurora_secret_key.trim()) - .and_then(|bytes| SecretKeyEth::parse(&bytes).map_err(Into::into)) - .map_err(|e| anyhow::anyhow!("Couldn't create secret key from hex: {e}"))?; + let secret_key = parse_aurora_secret_key(&aurora_secret_key)?;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
newcli/src/cli.rs(1 hunks)newcli/src/command/near.rs(1 hunks)newcli/src/common/mod.rs(1 hunks)newcli/src/common/output.rs(1 hunks)sdk/src/aurora/abi.rs(1 hunks)sdk/src/aurora/common.rs(3 hunks)sdk/src/aurora/contract/read.rs(2 hunks)sdk/src/aurora/contract/write.rs(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- sdk/src/aurora/abi.rs
- newcli/src/cli.rs
- sdk/src/aurora/common.rs
- newcli/src/common/mod.rs
- newcli/src/common/output.rs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: Shared / Static Analysis (aurora-engine-cli-rs, near, cli, 2, ., 1.18, false, --filter-paths "node_modules...
- GitHub Check: Linux
- GitHub Check: Clippy
- GitHub Check: macOS
- GitHub Check: Tests Silo CLI
- GitHub Check: Tests Simple CLI
🔇 Additional comments (1)
newcli/src/command/near.rs (1)
822-822: Relay account ID parsing enforces NEAR sub-account rulesBoth occurrences of constructing the relay ID use
.parse()?on the formatted string, which applies NEAR’sAccountIdvalidation (including sub-account naming). Any invalid format will error at parse time, so no additional checks are required:
- newcli/src/command/near.rs (line 822)
- cli/src/cli/simple/command/mod.rs
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
newcli/src/command/mod.rs (1)
489-723: Consider refactoring this large functionThe
runfunction is over 230 lines long with repetitive patterns. This was previously identified as an issue that impacts maintainability.
🧹 Nitpick comments (1)
newcli/src/command/mod.rs (1)
26-487: Consider breaking down the large Command enumThe Command enum spans over 460 lines with 60+ variants. Consider organizing related commands into sub-commands or separate modules to improve maintainability and discoverability.
For example:
- Token-related commands (FT*, ERC20*, NEP141*)
- Contract management (Deploy, Upgrade, Init)
- Account management (CreateAccount, ViewAccount)
- Configuration (SetOwner, SetFixedGas, etc.)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
newcli/src/command/mod.rs(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: Shared / Static Analysis (aurora-engine-cli-rs, near, cli, 2, ., 1.18, false, --filter-paths "node_modules...
- GitHub Check: macOS
- GitHub Check: Linux
- GitHub Check: Tests Simple CLI
- GitHub Check: Clippy
- GitHub Check: Tests Advanced CLI
🔇 Additional comments (2)
newcli/src/command/mod.rs (2)
605-608: Remove the underscore prefix fromaccount_idThe underscore prefix suggests this field is unused, but it's actually part of the command structure. This is misleading.
Command::GenerateNearKey { - account_id: _, + account_id, key_type, } => handle_generate_near_key(context, key_type).await,Likely an incorrect or invalid review comment.
1642-1646: Review wei parsing and conversion logicThe
parse_weifunction’s name implies it should parse a raw wei amount, but it callsWei::from_eth(u), which—by convention—takes an ETH-denominatedU256and multiplies by 10¹⁸. This is likely inverting the intended units.Please verify and adjust accordingly:
- Confirm that
Wei::from_ethindeed multiplies its input by 10¹⁸ (i.e., expects an ETH value).- Check whether there’s a corresponding constructor for raw wei (e.g.,
Wei::from_weior similar).- Either:
- Change the implementation to parse raw wei (drop the
from_ethconversion), or- Rename
parse_weito something likeparse_ethif the intention is to accept ETH amounts.Location:
- newcli/src/command/mod.rs lines 1642–1646
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
newcli/src/command/mod.rs (2)
735-735: Fix double reference operatorUsing
&&creates a reference to a reference, which is unnecessary.
815-815: Use proper hex encoding instead of Debug formattingThe current code uses Debug formatting on an already hex-encoded string, which may add quotes. Use direct string formatting instead.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
newcli/src/command/mod.rs(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: Shared / Static Analysis (aurora-engine-cli-rs, near, cli, 2, ., 1.18, false, --filter-paths "node_modules...
- GitHub Check: Clippy
- GitHub Check: macOS
- GitHub Check: Linux
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
sdk/src/aurora/contract/mod.rs (1)
59-73: Add bounds checking to prevent panics.The
copy_from_slicecalls will panic if the input is not exactly the expected size. This is the same issue identified in the previous review.Apply this diff to add proper bounds checking:
impl ContractMethodResponse for u64 { fn parse(rsp: Vec<u8>) -> Result<Self, Error> { + if rsp.len() != 8 { + return Err(io::Error::new( + io::ErrorKind::InvalidData, + format!("Expected 8 bytes for u64, got {}", rsp.len()) + ).into()); + } let mut buffer = [0; 8]; buffer.copy_from_slice(&rsp); Ok(Self::from_le_bytes(buffer)) } } impl ContractMethodResponse for u32 { fn parse(rsp: Vec<u8>) -> Result<Self, Error> { + if rsp.len() != 4 { + return Err(io::Error::new( + io::ErrorKind::InvalidData, + format!("Expected 4 bytes for u32, got {}", rsp.len()) + ).into()); + } let mut buffer = [0; 4]; buffer.copy_from_slice(&rsp); Ok(Self::from_le_bytes(buffer)) } }
🧹 Nitpick comments (3)
README.md (3)
25-30: Polish phrasing & fix minor grammar glitchesThe new paragraph reads awkwardly (“bootstrap … with rapid speed built with rust” and “Aurora CLI comes pre-configuration”). A quick re-phrase improves clarity without altering meaning.
-Aurora CLI is a command line interface to bootstrap Aurora Engine with rapid -speed built with rust. - -Aurora CLI comes pre-configuration with opinionated, sensible defaults for +Aurora CLI is a Rust-based command-line interface that bootstraps Aurora +Engine at high speed. + +Aurora CLI is pre-configured with opinionated, sensible defaults for
215-217: Missing space in “andadvanced”The sentence currently renders as “more options andadvanced features”. Insert a space.
-Advanced CLI provides more options andadvanced features. You can try it by +Advanced CLI provides more options and advanced features. You can try it by
458-459: Make--block-numberoption description consistent with the rest of the tableAll other long options show a value placeholder (
<...>). Add one for--block-numberand move the description to the right column for uniformity.- --block-number Block number to get data from + --block-number <BLOCK_NUMBER> Block number to query
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
README.md(13 hunks)sdk/src/aurora/contract/mod.rs(2 hunks)sdk/src/aurora/contract/read.rs(2 hunks)
🧰 Additional context used
🪛 LanguageTool
README.md
[grammar] ~21-~21: Use proper spacing conventions.
Context: ...e is the Aurora's implementation for it. ## What is Aurora CLI? Aurora CLI is a com...
(QB_NEW_EN_OTHER_ERROR_IDS_000007)
[grammar] ~29-~29: There might be a mistake here.
Context: ...projects mention testing on Aurora, they are referring to the settings defined in...
(QB_NEW_EN_OTHER)
[grammar] ~30-~30: Use proper spacing conventions.
Context: ...ng to the settings defined in this repo. **Aurora CLI has the following advantages ...
(QB_NEW_EN_OTHER_ERROR_IDS_000007)
[grammar] ~32-~32: Use proper spacing conventions.
Context: ...I has the following advantages over api:** - 📝 **Easily modifiable EVM states ...
(QB_NEW_EN_OTHER_ERROR_IDS_000007)
[grammar] ~53-~53: Use proper spacing conventions.
Context: ...um](https://forum.aurora.dev/) ## Usage In the following example, we will see ho...
(QB_NEW_EN_OTHER_ERROR_IDS_000007)
[grammar] ~94-~94: Use proper spacing conventions.
Context: ...rt NEAR node (Apple's M-based hardware): shell nearup run localnet --home /tmp/localnet --binary-path /path/to/nearcore/target/release Replace `/path/to/nearcore/target/releas...
(QB_NEW_EN_OTHER_ERROR_IDS_000007)
[grammar] ~178-~178: Place a period at the end of declarative sentences.
Context: ... ### Interact with the smart contract First, let's check that the current valu...
(QB_NEW_EN_OTHER_ERROR_IDS_000178)
[grammar] ~215-~215: Combining words like “every day” changes the meaning.
Context: ...)** Advanced CLI provides more options andadvanced features. You can try it by building wi...
(QB_NEW_EN_OTHER_ERROR_IDS_000001)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: Shared / Static Analysis (aurora-engine-cli-rs, near, cli, 2, ., 1.18, false, --filter-paths "node_modules...
- GitHub Check: Linux
- GitHub Check: macOS
- GitHub Check: Clippy
- GitHub Check: Tests Simple CLI
- GitHub Check: Tests Advanced CLI
🔇 Additional comments (5)
sdk/src/aurora/contract/mod.rs (1)
75-149: Well-implemented parsing methods with proper error handling.The new
ContractMethodResponseimplementations demonstrate good practices:
- Using
try_from_slicefor types that support it (safer than manual bounds checking)- Proper error handling with meaningful error messages
- Appropriate deserialization methods for each type (Borsh, JSON, or custom)
sdk/src/aurora/contract/read.rs (4)
15-31: Efficient macro implementation reduces code duplication.The
view_method!macro is well-designed and eliminates repetitive boilerplate code for simple view methods.
33-58: Comprehensive view method definitions using the macro.The macro usage creates clean, consistent implementations for various view methods. The response types are appropriately chosen for each method.
60-78: Improved GetBalance implementation with Wei response type.The change from a basic numeric type to
Weiprovides better type safety and semantic clarity for balance queries.
80-356: Well-structured contract method implementations.The new contract method implementations demonstrate good practices:
- Consistent struct naming and field visibility
- Appropriate parameter serialization methods for each type
- Custom response parsing where needed (hex encoding, UTF-8 conversion)
- Proper error handling in serialization/deserialization
All implementations follow the
ContractMethodtrait contract correctly.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
newcli/src/command/near.rs (1)
145-148: Remove redundant string manipulationThe
encode()method already returns a hex string without the "0x" prefix, making thetrim_start_matches()call unnecessary.- |address| address.encode().trim_start_matches("0x").to_string(), + |address| address.encode(),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
newcli/src/command/near.rs(1 hunks)sdk/macros/src/lib.rs(2 hunks)sdk/src/aurora/client.rs(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- sdk/src/aurora/client.rs
- sdk/macros/src/lib.rs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: Shared / Static Analysis (aurora-engine-cli-rs, near, cli, 2, ., 1.18, false, --filter-paths "node_modules...
- GitHub Check: Linux
- GitHub Check: macOS
- GitHub Check: Clippy
- GitHub Check: Tests Simple CLI
- GitHub Check: Tests Advanced CLI
🔇 Additional comments (3)
newcli/src/command/near.rs (3)
71-102: Well-implemented account creation logicThe function correctly handles both sub-account and root contract account creation with proper key generation and permissions setup.
445-468: Robust Ethereum transaction submission implementationThe function properly handles ABI encoding, secret key parsing, and transaction submission with appropriate error handling.
1170-1202: Secure Ethereum transaction handlingThe function correctly implements transaction signing with proper nonce management, chain ID for replay protection, and Aurora-specific gas parameters.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
sdk/Cargo.toml (1)
24-24: Disablenear-sdkdefault-features when used off-chain
near-sdkdefaults to pulling in the full WASM tool-chain and contract macro plumbing.
For purely off-chain helper logic the minimal set is usually:-near-sdk = { workspace = true, features = ["non-contract-usage"] } +near-sdk = { workspace = true, default-features = false, features = ["non-contract-usage"] }Avoids compiling
wee_alloc,near-bindgen-macros, etc., shaving build time and binary size.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (1)
sdk/Cargo.toml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: Shared / Static Analysis (aurora-engine-cli-rs, near, cli, 2, ., 1.18, false, --filter-paths "node_modules...
- GitHub Check: Tests Simple CLI
- GitHub Check: Tests Silo CLI
- GitHub Check: Tests Advanced CLI
- GitHub Check: Linux
- GitHub Check: macOS
- GitHub Check: Clippy
🔇 Additional comments (1)
sdk/Cargo.toml (1)
25-26: Unifyrandusage to avoid multiple major versions
libsecp256k1andnear-cryptoalready bringrand<0.9 in via their trees. Explicitly addingrandhere without a version pin can cause duplicated major versions and longer linking times. Unless you need direct RNG calls consider removing the top-level entry or pinning the same minor version used by upstream crates (rand = "0.8"today).
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
newcli/src/context.rs (1)
20-26: Use the.near()accessor method for consistencyThe code directly accesses the
nearfield instead of using the public accessor method.let view_query = self .client - .near + .near() .view(&self.cli.engine, method_name) .args(params);newcli/src/command/near.rs (1)
1087-1089: Usehex_to_arrutility for consistent hex parsingFor consistency with the rest of the codebase, use the
hex_to_arrutility function which properly handles the "0x" prefix.let input = input.map_or(Ok(vec![]), |input| { - Ok(hex::decode(input.trim_start_matches("0x"))?) + hex_to_arr::<256>(&input).map(|arr| arr.to_vec()) })?;Note: Adjust the array size or use a dynamic version of the function if needed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
newcli/src/cli.rs(1 hunks)newcli/src/command/mod.rs(1 hunks)newcli/src/command/near.rs(1 hunks)newcli/src/context.rs(1 hunks)sdk/macros/src/lib.rs(5 hunks)sdk/src/aurora/client.rs(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- newcli/src/command/mod.rs
🚧 Files skipped from review as they are similar to previous changes (3)
- sdk/macros/src/lib.rs
- sdk/src/aurora/client.rs
- newcli/src/cli.rs
🧰 Additional context used
🧬 Code Graph Analysis (2)
newcli/src/context.rs (4)
sdk/src/aurora/client.rs (2)
near(107-109)view(68-74)sdk/src/aurora/contract/read.rs (33)
method_name(67-69)method_name(87-89)method_name(107-109)method_name(131-133)method_name(151-153)method_name(171-173)method_name(195-197)method_name(215-217)method_name(235-237)method_name(253-255)method_name(277-279)method_name(301-303)method_name(325-327)method_name(345-347)params(75-77)params(95-97)params(115-117)params(139-141)params(159-161)params(175-177)params(203-205)params(223-225)params(243-245)params(257-259)params(285-287)params(309-311)params(333-335)params(353-355)parse_response(119-121)parse_response(183-185)parse_response(265-267)parse_response(289-291)parse_response(313-315)sdk/src/aurora/mod.rs (4)
method_name(45-45)params(56-58)parse_response(60-62)parse_error(64-69)sdk/src/aurora/contract/write.rs (3)
method_name(99-101)params(103-105)parse_response(167-174)
newcli/src/command/near.rs (8)
sdk/src/aurora/common.rs (5)
hex_to_arr(28-34)str_to_identifier(95-101)id(97-97)address_from_secret_key(45-50)sign_transaction(52-73)sdk/src/aurora/client.rs (3)
near(107-109)new(15-17)call(19-45)newcli/src/common/mod.rs (1)
parse_ft_metadata(9-47)newcli/src/cli.rs (3)
signer(50-52)root_contract_id(54-64)from_str(77-83)sdk/src/aurora/contract/write.rs (5)
deposit(331-333)deposit(351-353)deposit(375-377)params(103-105)params(147-149)sdk/src/aurora/abi.rs (2)
read_contract(9-22)parse_args(24-43)sdk/src/aurora/contract/read.rs (14)
params(75-77)params(95-97)params(115-117)params(139-141)params(159-161)params(175-177)params(203-205)params(223-225)params(243-245)params(257-259)params(285-287)params(309-311)params(333-335)params(353-355)cli/src/client/near.rs (1)
contract_id(563-576)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: Shared / Static Analysis (aurora-engine-cli-rs, near, cli, 2, ., 1.18, false, --filter-paths "node_modules...
- GitHub Check: Tests Simple CLI
- GitHub Check: Tests Silo CLI
- GitHub Check: Tests Advanced CLI
- GitHub Check: Linux
- GitHub Check: macOS
- GitHub Check: Clippy
🔇 Additional comments (4)
newcli/src/context.rs (1)
34-41: Well-structured error handlingThe error handling properly distinguishes between successful responses, contract method errors, and other RPC errors. This provides detailed error information to callers.
newcli/src/command/near.rs (3)
71-102: Correct account creation logicThe function properly handles both sub-account and root account creation with appropriate key generation and balance transfer.
1169-1201: Well-implemented transaction signing and submissionThe helper function correctly implements Ethereum transaction creation, signing with proper chain ID, and submission. The gas parameters (zero price, max limit) are appropriate for Aurora.
145-148: Confirm hex prefix handling in Address::encode()The
Address::encode()method is defined in the externalaurora_sdk_rscrate. If it already returns a clean hex string (i.e., without a leading “0x”), then the.trim_start_matches("0x")call is redundant. Please verify its behavior in theaurora_sdk_rs::aurora::types::Addressimplementation and remove the extra trimming if unnecessary.Location:
- newcli/src/command/near.rs lines 145–148
Suggested cleanup (if encode never adds “0x”):
- |address| address.encode().trim_start_matches("0x").to_string(), + |address| address.encode(),
| custodian_address: Address, | ||
| ft_metadata_path: P, | ||
| ) -> anyhow::Result<()> { | ||
| let ft_metadata = parse_ft_metadata(std::fs::read_to_string(ft_metadata_path).ok())?; |
There was a problem hiding this comment.
Silent error handling may hide file read failures
Using .ok() on file read operations silently converts errors to None, which could lead to unexpected behavior if the file is inaccessible.
- let ft_metadata = parse_ft_metadata(std::fs::read_to_string(ft_metadata_path).ok())?;
+ let ft_metadata = parse_ft_metadata(Some(std::fs::read_to_string(ft_metadata_path)?))?;🤖 Prompt for AI Agents
In newcli/src/command/near.rs at line 777, the use of `.ok()` on the file read
operation silently converts errors to None, potentially hiding file read
failures. Replace `.ok()` with proper error handling by propagating the error
using `?` or matching the result to handle errors explicitly, ensuring that file
read failures are not ignored and are reported appropriately.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/cli.yml (1)
20-27: Deduplicate the identicalargsvalues across the matrix entriesAll three matrix items now pass the same
--path ./newcliargument. Repeating the key/value block increases maintenance overhead and risks going out-of-sync if the path changes again.YAML examples to DRY this:
strategy: matrix: interface: [ Advanced, Simple, Silo ] + args: [ "--path ./newcli" ] include: - - interface: Advanced - args: --path ./newcli + - interface: Advanced script: scripts/advanced.sh - - interface: Simple - args: --path ./newcli + - interface: Simple script: scripts/simple.sh - - interface: Silo - args: --path ./newcli + - interface: Silo script: scripts/simple-silo.shOr hoist the constant into a job-level
env:block and reference it as${{ env.CARGO_ARGS }}.
Either approach keeps the workflow concise and makes future path updates one-liner changes.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/cli.yml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Shared / Static Analysis (aurora-engine-cli-rs, near, cli, 2, ., 1.18, false, --filter-paths "node_modules...
- GitHub Check: Linux
- GitHub Check: Clippy
- GitHub Check: Silo CLI
- GitHub Check: macOS
- GitHub Check: Simple CLI
- GitHub Check: Advanced CLI
- GitHub Check: semgrep-cloud-platform/scan
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (4)
newcli/src/command/mod.rs (4)
491-727: Large function needs refactoringThe
runfunction with its extensive match statement could benefit from a more maintainable pattern such as a command registry or macro-based dispatch.
819-819: Use proper hex encoding instead of Debug formattingThe Debug formatter on hex-encoded data may include unwanted artifacts.
- result_object!("address" => format!("{:?}", address), "code" => format!("0x{:?}", hex::encode(code))) + result_object!("address" => format!("{:?}", address), "code" => format!("0x{}", hex::encode(code)))
1502-1502: Remove redundant Debug formattingThe
.to_string()already provides string representation, Debug formatting adds unnecessary quotes.- result_object!("latest_hashchain" => latest_hashchain.to_string()) + result_object!("latest_hashchain" => latest_hashchain.to_string())
748-748: Remove double reference operatorUsing
&&creates an unnecessary reference to a reference.- &&context.cli.output_format, + &context.cli.output_format,
🧹 Nitpick comments (1)
newcli/src/command/mod.rs (1)
26-489: Consider splitting this large enum into logical groupsThe Command enum has 60+ variants, making it difficult to navigate and maintain. Consider grouping related commands into sub-enums or modules (e.g.,
AccountCommands,ContractCommands,TokenCommands,WhitelistCommands).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
newcli/src/command/mod.rs(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Shared / Static Analysis (aurora-engine-cli-rs, near, cli, 2, ., 1.18, false, --filter-paths "node_modules...
- GitHub Check: Clippy
- GitHub Check: Linux
- GitHub Check: macOS
- GitHub Check: Advanced CLI
- GitHub Check: Silo CLI
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (1)
newcli/src/command/mod.rs (1)
1-24: Well-organized importsThe imports are properly structured following the standard convention: std → external crates → local modules.
| fn parse_wei(s: &str) -> anyhow::Result<Wei> { | ||
| U256::from_str_radix(s, 10) | ||
| .map_err(|s| anyhow::anyhow!("Invalid wei value: {s}")) | ||
| .and_then(|u| Wei::from_eth(u).ok_or(anyhow::anyhow!("Wei overflow"))) | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify the wei parsing logic
The function parse_wei uses Wei::from_eth() which typically expects values in ether units, not wei. This could lead to incorrect value conversions.
If the input is already in wei units, consider using:
fn parse_wei(s: &str) -> anyhow::Result<Wei> {
U256::from_str_radix(s, 10)
.map_err(|s| anyhow::anyhow!("Invalid wei value: {s}"))
- .and_then(|u| Wei::from_eth(u).ok_or(anyhow::anyhow!("Wei overflow")))
+ .map(Wei::new)
}Or rename the function to parse_eth if it's meant to parse ether values.
🏁 Script executed:
#!/bin/bash
# Locate the definition of Wei and inspect from_eth implementation
rg "struct Wei" -A5
rg "enum Wei" -A5
rg "impl\\s+Wei" -A5
rg "fn from_eth" -A5Length of output: 86
🏁 Script executed:
#!/bin/bash
# Find where Wei is imported in mod.rs
rg "^use .*Wei" -n newcli/src/command/mod.rs
# Show context around parse_wei definition
sed -n '1620,1680p' newcli/src/command/mod.rsLength of output: 2240
🏁 Script executed:
#!/bin/bash
# Show import statements in mod.rs to locate the Wei type's origin
sed -n '1,200p' newcli/src/command/mod.rsLength of output: 5857
🌐 Web query:
What does Wei::from_eth do in the aurora_sdk_rs crate? Does it convert an Ether amount (U256) to Wei by multiplying by 10^18, or is it a simple constructor for a Wei value?
💡 Result:
The function Wei::from_eth in the aurora_sdk_rs crate acts as a constructor for a Wei value given an Ether amount in U256, and it performs the conversion by multiplying the provided Ether amount by (10^{18}).
Here is the rationale:
- In Ethereum, 1 Ether is equal to (10^{18}) Wei, making Wei the smallest denomination[1][2].
- The function name
from_ethstrongly implies that it takes a value in Ether and returns the equivalent value in Wei. - Standard practice (and the norm in most SDKs or Ethereum-related codebases) is to perform this conversion by multiplying the ether amount by (10^{18}) when going from ether to wei, and dividing when converting from wei to ether[1][2].
Unless aurora_sdk_rs specifically documents otherwise, you can confidently expect Wei::from_eth to perform the conversion as follows:
[
\text{Wei} = \text{Ether} \times 10^{18}
]
So, Wei::from_eth accepts a U256 Ether amount and converts it to Wei by multiplying by (10^{18}), and is not simply a raw value constructor.
Citations:
- 1: https://dev.to/shlok2740/mastering-ether-units-a-guide-to-solidity-conversions-255a
- 2: https://betterprogramming.pub/solidity-tutorial-all-about-ether-units-eaebe55dd4dc
Incorrect wei parsing: use direct Wei constructor for raw wei values
The parse_wei helper currently treats its input as an ether amount—calling Wei::from_eth(u) multiplies by 10¹⁸—so passing in a wei value will inflate it by 10¹⁸ (or overflow). If the string is already in wei, switch to the raw constructor:
fn parse_wei(s: &str) -> anyhow::Result<Wei> {
U256::from_str_radix(s, 10)
.map_err(|s| anyhow::anyhow!("Invalid wei value: {s}"))
- .and_then(|u| Wei::from_eth(u).ok_or(anyhow::anyhow!("Wei overflow")))
+ .map(Wei::new)
}If instead you meant to parse ether, rename this function (e.g. to parse_eth).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| fn parse_wei(s: &str) -> anyhow::Result<Wei> { | |
| U256::from_str_radix(s, 10) | |
| .map_err(|s| anyhow::anyhow!("Invalid wei value: {s}")) | |
| .and_then(|u| Wei::from_eth(u).ok_or(anyhow::anyhow!("Wei overflow"))) | |
| } | |
| fn parse_wei(s: &str) -> anyhow::Result<Wei> { | |
| U256::from_str_radix(s, 10) | |
| .map_err(|s| anyhow::anyhow!("Invalid wei value: {s}")) | |
| .map(Wei::new) | |
| } |
🤖 Prompt for AI Agents
In newcli/src/command/mod.rs around lines 1646 to 1650, the parse_wei function
incorrectly treats the input string as an ether amount by calling Wei::from_eth,
which multiplies the value by 10^18 and causes overflow for raw wei inputs. To
fix this, replace the call to Wei::from_eth with the direct Wei constructor that
accepts the raw U256 value without scaling. If the intention was to parse ether
amounts, rename the function accordingly instead.
… formatting references
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
newcli/src/command/mod.rs (2)
819-819: Use proper hex encoding instead of Debug formattingUsing
{:?}with hex::encode might include quotes or other debug artifacts. Use hex encoding for clean output.- result_object!("address" => format!("{:?}", address), "code" => format!("0x{:?}", hex::encode(code))) + result_object!("address" => format!("{:?}", address), "code" => format!("0x{}", hex::encode(code)))
1646-1650: Incorrect wei parsing: use direct Wei constructor for raw wei valuesThe
parse_weihelper currently treats its input as an ether amount—callingWei::from_eth(u)multiplies by 10¹⁸—so passing in a wei value will inflate it by 10¹⁸ (or overflow). If the string is already in wei, switch to the raw constructor:fn parse_wei(s: &str) -> anyhow::Result<Wei> { U256::from_str_radix(s, 10) .map_err(|s| anyhow::anyhow!("Invalid wei value: {s}")) - .and_then(|u| Wei::from_eth(u).ok_or(anyhow::anyhow!("Wei overflow"))) + .map(Wei::new) }If instead you meant to parse ether, rename this function (e.g. to
parse_eth).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
newcli/src/cli.rs(1 hunks)newcli/src/command/mod.rs(1 hunks)newcli/src/command/near.rs(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- newcli/src/command/near.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- newcli/src/cli.rs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: Shared / Static Analysis (aurora-engine-cli-rs, near, cli, 2, ., 1.18, false, --filter-paths "node_modules...
- GitHub Check: macOS
- GitHub Check: Linux
- GitHub Check: Clippy
- GitHub Check: Silo CLI
- GitHub Check: Advanced CLI
- GitHub Check: Simple CLI
newcli/src/command/mod.rs
Outdated
| async fn handle_get_storage_at( | ||
| context: &Context, | ||
| address: Address, | ||
| key: H256, | ||
| ) -> anyhow::Result<()> { | ||
| near::get_storage_at(context, address, key).await?; | ||
| Ok(()) | ||
| } |
There was a problem hiding this comment.
Missing output for get_storage_at command
The handler calls near::get_storage_at but doesn't capture or output the result. Users won't see any response from this command.
async fn handle_get_storage_at(
context: &Context,
address: Address,
key: H256,
) -> anyhow::Result<()> {
- near::get_storage_at(context, address, key).await?;
+ let value = near::get_storage_at(context, address, key).await?;
+ output!(
+ &context.cli.output_format,
+ result_object!("address" => format!("{:?}", address), "key" => format!("{:?}", key), "value" => format!("{:?}", value))
+ );
Ok(())
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async fn handle_get_storage_at( | |
| context: &Context, | |
| address: Address, | |
| key: H256, | |
| ) -> anyhow::Result<()> { | |
| near::get_storage_at(context, address, key).await?; | |
| Ok(()) | |
| } | |
| async fn handle_get_storage_at( | |
| context: &Context, | |
| address: Address, | |
| key: H256, | |
| ) -> anyhow::Result<()> { | |
| let value = near::get_storage_at(context, address, key).await?; | |
| output!( | |
| &context.cli.output_format, | |
| result_object!( | |
| "address" => format!("{:?}", address), | |
| "key" => format!("{:?}", key), | |
| "value" => format!("{:?}", value) | |
| ) | |
| ); | |
| Ok(()) | |
| } |
🤖 Prompt for AI Agents
In newcli/src/command/mod.rs around lines 878 to 885, the function
handle_get_storage_at calls near::get_storage_at but does not capture or print
its result, so users receive no output. Modify the function to await and store
the result from near::get_storage_at, then print or otherwise output this result
to the user before returning Ok(()). This ensures the command provides visible
feedback.
newcli/src/command/mod.rs
Outdated
| async fn handle_register_relayer(context: &Context, address: Address) -> anyhow::Result<()> { | ||
| near::register_relayer(context, address).await?; | ||
| Ok(()) | ||
| } |
There was a problem hiding this comment.
Missing output for register_relayer command
The handler doesn't provide any feedback to the user after registering the relayer.
async fn handle_register_relayer(context: &Context, address: Address) -> anyhow::Result<()> {
near::register_relayer(context, address).await?;
+ output!(
+ &context.cli.output_format,
+ CommandResult::success("Relayer registered successfully")
+ );
Ok(())
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async fn handle_register_relayer(context: &Context, address: Address) -> anyhow::Result<()> { | |
| near::register_relayer(context, address).await?; | |
| Ok(()) | |
| } | |
| async fn handle_register_relayer(context: &Context, address: Address) -> anyhow::Result<()> { | |
| near::register_relayer(context, address).await?; | |
| output!( | |
| &context.cli.output_format, | |
| CommandResult::success("Relayer registered successfully") | |
| ); | |
| Ok(()) | |
| } |
🤖 Prompt for AI Agents
In newcli/src/command/mod.rs around lines 887 to 890, the
handle_register_relayer function lacks user feedback after successfully
registering the relayer. Modify the function to print or log a confirmation
message indicating the relayer was registered successfully before returning
Ok(()). This will provide clear output to the user confirming the command's
success.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
newcli/src/command/mod.rs (1)
679-682: Remove unusedwithdraw_serparameterThe
withdraw_serparameter is destructured but never used. Either use it or remove it from the function signature.Command::SetEthConnectorContractAccount { account_id, - withdraw_ser: _, } => handlers::set_eth_connector_contract_account(context, account_id).await,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
newcli/src/command/handlers.rs(1 hunks)newcli/src/command/mod.rs(1 hunks)
🧬 Code Graph Analysis (1)
newcli/src/command/handlers.rs (6)
newcli/src/common/output.rs (1)
success(11-13)sdk/src/near/query.rs (1)
block_height(37-40)newcli/src/command/near.rs (2)
encode_to_address(469-471)gen_near_key_pair(477-479)cli/src/cli/simple/command/silo.rs (1)
None(62-62)cli/src/eth_method.rs (1)
name(16-27)cli/src/client/near.rs (1)
contract_id(563-576)
🪛 GitHub Actions: CI
newcli/src/command/mod.rs
[error] 505-505: Rustfmt formatting check failed. Multi-line match arms should be single line. Run 'cargo fmt' to fix code style issues.
[error] 541-541: Rustfmt formatting check failed. Multi-line match arms should be single line. Run 'cargo fmt' to fix code style issues.
[error] 550-550: Rustfmt formatting check failed. Multi-line match arms should be single line. Run 'cargo fmt' to fix code style issues.
[error] 571-571: Rustfmt formatting check failed. Multi-line match arms should be single line. Run 'cargo fmt' to fix code style issues.
[error] 615-615: Rustfmt formatting check failed. Multi-line match arms should be single line. Run 'cargo fmt' to fix code style issues.
[error] 631-631: Rustfmt formatting check failed. Multi-line match arms should be single line. Run 'cargo fmt' to fix code style issues.
[error] 703-703: Rustfmt formatting check failed. Multi-line match arms should be single line. Run 'cargo fmt' to fix code style issues.
[error] 720-720: Rustfmt formatting check failed. Multi-line match arms should be single line. Run 'cargo fmt' to fix code style issues.
[error] 742-742: Rustfmt formatting check failed. Multi-line match arms should be single line. Run 'cargo fmt' to fix code style issues.
[error] 751-751: Rustfmt formatting check failed. Multi-line match arms should be single line. Run 'cargo fmt' to fix code style issues.
newcli/src/command/handlers.rs
[error] 168-168: Rustfmt formatting check failed. Function signatures should be on a single line. Run 'cargo fmt' to fix code style issues.
[error] 272-272: Rustfmt formatting check failed. Function signatures should be on a single line. Run 'cargo fmt' to fix code style issues.
[error] 457-457: Rustfmt formatting check failed. Function signatures should be on a single line. Run 'cargo fmt' to fix code style issues.
[error] 532-532: Rustfmt formatting check failed. Function signatures should be on a single line. Run 'cargo fmt' to fix code style issues.
[error] 608-608: Rustfmt formatting check failed. Function signatures should be on a single line. Run 'cargo fmt' to fix code style issues.
[error] 638-638: Rustfmt formatting check failed. Function signatures should be on a single line. Run 'cargo fmt' to fix code style issues.
[error] 651-651: Rustfmt formatting check failed. Function signatures should be on a single line. Run 'cargo fmt' to fix code style issues.
[error] 924-924: Rustfmt formatting check failed. Function signatures should be on a single line. Run 'cargo fmt' to fix code style issues.
[error] 936-936: Rustfmt formatting check failed. Function signatures should be on a single line. Run 'cargo fmt' to fix code style issues.
🧰 Additional context used
🧬 Code Graph Analysis (1)
newcli/src/command/handlers.rs (6)
newcli/src/common/output.rs (1)
success(11-13)sdk/src/near/query.rs (1)
block_height(37-40)newcli/src/command/near.rs (2)
encode_to_address(469-471)gen_near_key_pair(477-479)cli/src/cli/simple/command/silo.rs (1)
None(62-62)cli/src/eth_method.rs (1)
name(16-27)cli/src/client/near.rs (1)
contract_id(563-576)
🪛 GitHub Actions: CI
newcli/src/command/mod.rs
[error] 505-505: Rustfmt formatting check failed. Multi-line match arms should be single line. Run 'cargo fmt' to fix code style issues.
[error] 541-541: Rustfmt formatting check failed. Multi-line match arms should be single line. Run 'cargo fmt' to fix code style issues.
[error] 550-550: Rustfmt formatting check failed. Multi-line match arms should be single line. Run 'cargo fmt' to fix code style issues.
[error] 571-571: Rustfmt formatting check failed. Multi-line match arms should be single line. Run 'cargo fmt' to fix code style issues.
[error] 615-615: Rustfmt formatting check failed. Multi-line match arms should be single line. Run 'cargo fmt' to fix code style issues.
[error] 631-631: Rustfmt formatting check failed. Multi-line match arms should be single line. Run 'cargo fmt' to fix code style issues.
[error] 703-703: Rustfmt formatting check failed. Multi-line match arms should be single line. Run 'cargo fmt' to fix code style issues.
[error] 720-720: Rustfmt formatting check failed. Multi-line match arms should be single line. Run 'cargo fmt' to fix code style issues.
[error] 742-742: Rustfmt formatting check failed. Multi-line match arms should be single line. Run 'cargo fmt' to fix code style issues.
[error] 751-751: Rustfmt formatting check failed. Multi-line match arms should be single line. Run 'cargo fmt' to fix code style issues.
newcli/src/command/handlers.rs
[error] 168-168: Rustfmt formatting check failed. Function signatures should be on a single line. Run 'cargo fmt' to fix code style issues.
[error] 272-272: Rustfmt formatting check failed. Function signatures should be on a single line. Run 'cargo fmt' to fix code style issues.
[error] 457-457: Rustfmt formatting check failed. Function signatures should be on a single line. Run 'cargo fmt' to fix code style issues.
[error] 532-532: Rustfmt formatting check failed. Function signatures should be on a single line. Run 'cargo fmt' to fix code style issues.
[error] 608-608: Rustfmt formatting check failed. Function signatures should be on a single line. Run 'cargo fmt' to fix code style issues.
[error] 638-638: Rustfmt formatting check failed. Function signatures should be on a single line. Run 'cargo fmt' to fix code style issues.
[error] 651-651: Rustfmt formatting check failed. Function signatures should be on a single line. Run 'cargo fmt' to fix code style issues.
[error] 924-924: Rustfmt formatting check failed. Function signatures should be on a single line. Run 'cargo fmt' to fix code style issues.
[error] 936-936: Rustfmt formatting check failed. Function signatures should be on a single line. Run 'cargo fmt' to fix code style issues.
There was a problem hiding this comment.
Actionable comments posted: 4
🔭 Outside diff range comments (1)
scripts/different-outputs.sh (1)
23-37:NEARD_VERSIONcan be empty → subsequent download silently breaks
curl -s … | jq -r .version.versionrelies on two external commands and a live main-net endpoint.
If either:
jqis not installed,- the RPC endpoint is unavailable / rate-limited, or
- the JSON schema changes,
NEARD_VERSIONbecomes an empty string.
download_neard()will then construct…/$NEARD_VERSION/neard.tar.gzwhich evaluates to
…//neard.tar.gzandcurl -ssuppresses the HTTP 404 – the script continues untiltarfails with a cryptic error.Add a guard (and a sane fallback) right after the assignment:
-NEARD_VERSION=$(curl -s https://rpc.mainnet.near.org/status | jq -r .version.version) +NEARD_VERSION=$(curl -s https://rpc.mainnet.near.org/status 2>/dev/null | jq -r .version.version 2>/dev/null || true) +if [[ -z "$NEARD_VERSION" ]]; then + echo "⚠️ Unable to fetch NEAR version – falling back to 2.6.5" >&2 + NEARD_VERSION="2.6.5" +fi +# Optional: verify that the artefact actually exists +# curl --head --fail -s "$url" || error_exitConsider also exiting early if
jqis missing:command -v jq >/dev/null || { echo "jq is required"; exit 1; }
♻️ Duplicate comments (6)
scripts/simple.sh (1)
23-37: Same empty-version pitfall asdifferent-outputs.sh– please apply the guard/fallback logic suggested there.scripts/simple-silo.sh (1)
14-28: Same empty-version pitfall asdifferent-outputs.sh– please apply the guard/fallback logic suggested there.scripts/advanced.sh (1)
10-24: Same empty-version pitfall asdifferent-outputs.sh– please apply the guard/fallback logic suggested there.newcli/src/command/mod.rs (2)
504-762: Fix Rustfmt formatting issuesThe pipeline failures indicate formatting issues with match arms. Run
cargo fmtto fix these multi-line match arms.
771-775: Fix incorrect wei parsing logicThe
parse_weifunction incorrectly usesWei::from_eth()which multiplies the input by 10^18, treating it as ether. This will cause massive inflation of wei values or overflow errors.Apply this fix:
fn parse_wei(s: &str) -> anyhow::Result<Wei> { U256::from_str_radix(s, 10) .map_err(|s| anyhow::anyhow!("Invalid wei value: {s}")) - .and_then(|u| Wei::from_eth(u).ok_or(anyhow::anyhow!("Wei overflow"))) + .map(Wei::new) }newcli/src/command/handlers.rs (1)
94-112: Fix Rustfmt formatting issuesThe pipeline failures indicate function signatures need formatting. Run
cargo fmtto fix these issues.Also applies to: 171-182, 262-266, 460-469, 509-516, 586-594, 641-653
🧹 Nitpick comments (1)
newcli/src/command/handlers.rs (1)
25-25: Use English for code commentsReplace the non-English comment with an English version for consistency and international collaboration.
-// Макрос для обработки результатов - поддерживает лямбда-функции и блоки +// Macro for result handling - supports lambda functions and blocks
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
newcli/src/command/handlers.rs(1 hunks)newcli/src/command/mod.rs(1 hunks)scripts/advanced.sh(1 hunks)scripts/different-outputs.sh(1 hunks)scripts/simple-silo.sh(1 hunks)scripts/simple.sh(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
newcli/src/command/mod.rs (5)
sdk/src/aurora/client.rs (3)
near(107-109)new(15-17)call(19-45)newcli/src/command/handlers.rs (28)
create_account(58-70)view_account(72-83)deploy_aurora(85-92)init(94-112)get_chain_id(114-118)get_nonce(120-127)get_block_hash(129-141)get_code(143-150)get_balance(152-164)upgrade(304-307)deploy(318-345)call(347-388)view_call(390-411)submit(413-437)mirror_erc20_token(625-636)set_eth_connector_contract_account(638-640)get_eth_connector_contract_account(642-653)set_eth_connector_contract_data(655-662)add_relayer(677-693)withdraw_wnear_to_router(695-701)mirror_erc20_token_callback(703-713)ft_transfer(768-784)ft_transfer_call(786-804)ft_on_transfer(806-817)storage_deposit(832-842)storage_unregister(844-850)storage_withdraw(852-858)storage_balance_of(860-872)newcli/src/command/near.rs (28)
create_account(71-102)view_account(104-111)deploy_aurora(113-125)init(127-162)get_chain_id(354-360)get_nonce(362-368)get_block_hash(370-376)get_code(378-384)get_balance(386-392)upgrade(311-317)deploy(1068-1078)call(1080-1108)view_call(1110-1145)submit(444-467)mirror_erc20_token(724-742)set_eth_connector_contract_account(744-761)get_eth_connector_contract_account(763-769)set_eth_connector_contract_data(771-793)add_relayer(816-850)withdraw_wnear_to_router(852-870)mirror_erc20_token_callback(872-890)ft_transfer(925-945)ft_transfer_call(947-969)ft_on_transfer(971-991)storage_deposit(1008-1026)storage_unregister(1028-1039)storage_withdraw(1041-1052)storage_balance_of(1054-1066)cli/src/client/near.rs (1)
contract_id(563-576)newcli/src/common/mod.rs (1)
parse_ft_metadata(9-47)
newcli/src/command/handlers.rs (4)
newcli/src/common/output.rs (1)
success(11-13)newcli/src/command/near.rs (2)
encode_to_address(469-471)gen_near_key_pair(477-479)cli/src/eth_method.rs (1)
name(16-27)cli/src/client/near.rs (1)
contract_id(563-576)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: Shared / Static Analysis (aurora-engine-cli-rs, near, cli, 2, ., 1.18, false, --filter-paths "node_modules...
- GitHub Check: Linux
- GitHub Check: macOS
- GitHub Check: Clippy
- GitHub Check: Simple CLI
- GitHub Check: Silo CLI
- GitHub Check: Advanced CLI
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
newcli/src/command/handlers.rs (1)
95-110: Past unwrap() issues have been properly addressedAll file reading operations now use the
read_wasm_filehelper function, eliminating the unwrap() calls mentioned in previous reviews. The error handling is now consistent and graceful.Also applies to: 286-291, 324-329, 331-336
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
newcli/src/cli.rs(1 hunks)newcli/src/command/handlers.rs(1 hunks)newcli/src/common/mod.rs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- newcli/src/cli.rs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Shared / Static Analysis (aurora-engine-cli-rs, near, cli, 2, ., 1.18, false, --filter-paths "node_modules...
- GitHub Check: Linux
- GitHub Check: macOS
- GitHub Check: Clippy
🔇 Additional comments (4)
newcli/src/common/mod.rs (2)
55-65: LGTM!The default metadata function is well-structured with appropriate defaults for a local ETH token.
67-116: Well-implemented key file reading with proper legacy supportThe implementation correctly handles both key file formats and maintains backward compatibility through serde aliases. The TODO comment appropriately notes this should be removed once the upstream changes are released.
newcli/src/command/handlers.rs (2)
26-37: Good error handling pattern for WASM file readingThe helper function properly handles file reading errors without panicking and provides clear error messages to users.
39-66: Well-designed macro for consistent error handlingThe
handle_near_callmacro effectively standardizes error handling across all handlers with flexible success handling patterns.
| reference_hash: json | ||
| .get("reference_hash") | ||
| .and_then(serde_json::Value::as_str) | ||
| .ok_or_else(|| anyhow::anyhow!("reference_hash must be a string")) | ||
| .and_then(|s| { | ||
| base64::engine::general_purpose::STANDARD | ||
| .decode(s) | ||
| .map_err(Into::into) | ||
| }) | ||
| .and_then(|bytes| FungibleReferenceHash::try_from_slice(&bytes).map_err(Into::into)) | ||
| .ok(), |
There was a problem hiding this comment.
Fix inconsistent error handling for reference_hash field
The reference_hash field validation creates an error if the field exists but is not a string, but then discards this error with .ok() on line 46. This results in silent failure when the field has an invalid type.
Either make it fully optional (remove the error on line 39) or propagate validation errors:
reference_hash: json
.get("reference_hash")
.and_then(serde_json::Value::as_str)
- .ok_or_else(|| anyhow::anyhow!("reference_hash must be a string"))
- .and_then(|s| {
+ .map(|s| {
base64::engine::general_purpose::STANDARD
.decode(s)
- .map_err(Into::into)
+ .map_err(|e| anyhow::anyhow!("Invalid base64 in reference_hash: {}", e))
+ .and_then(|bytes| FungibleReferenceHash::try_from_slice(&bytes)
+ .map_err(|e| anyhow::anyhow!("Invalid reference_hash format: {}", e)))
})
- .and_then(|bytes| FungibleReferenceHash::try_from_slice(&bytes).map_err(Into::into))
- .ok(),
+ .transpose()?,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| reference_hash: json | |
| .get("reference_hash") | |
| .and_then(serde_json::Value::as_str) | |
| .ok_or_else(|| anyhow::anyhow!("reference_hash must be a string")) | |
| .and_then(|s| { | |
| base64::engine::general_purpose::STANDARD | |
| .decode(s) | |
| .map_err(Into::into) | |
| }) | |
| .and_then(|bytes| FungibleReferenceHash::try_from_slice(&bytes).map_err(Into::into)) | |
| .ok(), | |
| reference_hash: json | |
| .get("reference_hash") | |
| .and_then(serde_json::Value::as_str) | |
| .map(|s| { | |
| base64::engine::general_purpose::STANDARD | |
| .decode(s) | |
| .map_err(|e| anyhow::anyhow!("Invalid base64 in reference_hash: {}", e)) | |
| .and_then(|bytes| FungibleReferenceHash::try_from_slice(&bytes) | |
| .map_err(|e| anyhow::anyhow!("Invalid reference_hash format: {}", e))) | |
| }) | |
| .transpose()?, |
🤖 Prompt for AI Agents
In newcli/src/common/mod.rs around lines 36 to 46, the error handling for the
reference_hash field is inconsistent because it generates an error if the field
is not a string but then discards it with .ok(), causing silent failures. To fix
this, either remove the error creation on line 39 to make the field fully
optional or remove the .ok() call on line 46 to propagate validation errors
properly, ensuring that invalid types are reported instead of ignored.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
newcli/src/command/mod.rs (1)
770-774: Incorrect wei parsing: use direct Wei constructor for raw wei values
🧹 Nitpick comments (1)
newcli/src/command/mod.rs (1)
473-475: Document the optional parameters in StorageDepositThe
StorageDepositcommand has two optional parameters without clear documentation about their behavior when omitted. Add doc comments to clarify.StorageDeposit { + /// Account ID to deposit storage for. If None, deposits for the caller account_id: Option<AccountId>, + /// If true, only registers the account without depositing extra storage registration_only: Option<bool>, },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
newcli/src/command/mod.rs(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: Shared / Static Analysis (aurora-engine-cli-rs, near, cli, 2, ., 1.18, false, --filter-paths "node_modules...
- GitHub Check: Advanced CLI
- GitHub Check: macOS
- GitHub Check: Linux
- GitHub Check: Clippy
| #[arg(long, value_parser = parse_withdraw_serialize_type, default_value = "borsh")] | ||
| withdraw_ser: Option<WithdrawSerializeType>, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Confusing pattern: optional parameter with default value
The withdraw_ser parameter is marked as Option<WithdrawSerializeType> but also has a default_value = "borsh". This creates confusion - if the parameter is optional, the default value won't be used when it's None. Either make it non-optional to use the default, or remove the default value.
- #[arg(long, value_parser = parse_withdraw_serialize_type, default_value = "borsh")]
- withdraw_ser: Option<WithdrawSerializeType>,
+ #[arg(long, value_parser = parse_withdraw_serialize_type, default_value = "borsh")]
+ withdraw_ser: WithdrawSerializeType,🤖 Prompt for AI Agents
In newcli/src/command/mod.rs at lines 387-388, the withdraw_ser parameter is
declared as Option<WithdrawSerializeType> but also has a default_value set,
which is contradictory. To fix this, either remove the Option wrapper and keep
the default_value so the parameter is always set with a default, or remove the
default_value and keep it as an Option to allow None values without a default.
Choose one consistent approach to avoid confusion.
| account_id, | ||
| withdraw_ser: _, | ||
| } => handlers::set_eth_connector_contract_account(context, account_id).await, |
There was a problem hiding this comment.
Unused parameter: withdraw_ser
The withdraw_ser parameter is accepted but ignored (marked with _). If this parameter is not needed, consider removing it from the command definition. If it will be used in the future, add a TODO comment.
Either remove the unused parameter or document why it's ignored:
Command::SetEthConnectorContractAccount {
account_id,
- withdraw_ser: _,
+ withdraw_ser,
- } => handlers::set_eth_connector_contract_account(context, account_id).await,
+ } => {
+ // TODO: Implement withdraw_ser handling
+ handlers::set_eth_connector_contract_account(context, account_id).await
+ }Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In newcli/src/command/mod.rs around lines 694 to 696, the parameter withdraw_ser
is accepted but unused and marked with an underscore. To fix this, either remove
the withdraw_ser parameter entirely from the command definition if it is not
needed, or if it is intended for future use, add a clear TODO comment explaining
why it is currently ignored.
| fn parse_ft_metadata_path(s: &str) -> anyhow::Result<FungibleTokenMetadata> { | ||
| common::parse_ft_metadata(std::fs::read_to_string(s).ok()) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve error handling for file reading
The function silently converts file read errors to None using .ok(), which could hide important errors like permission issues or missing files.
fn parse_ft_metadata_path(s: &str) -> anyhow::Result<FungibleTokenMetadata> {
- common::parse_ft_metadata(std::fs::read_to_string(s).ok())
+ let content = std::fs::read_to_string(s)
+ .map_err(|e| anyhow::anyhow!("Failed to read metadata file {}: {}", s, e))?;
+ common::parse_ft_metadata(Some(content))
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| fn parse_ft_metadata_path(s: &str) -> anyhow::Result<FungibleTokenMetadata> { | |
| common::parse_ft_metadata(std::fs::read_to_string(s).ok()) | |
| } | |
| fn parse_ft_metadata_path(s: &str) -> anyhow::Result<FungibleTokenMetadata> { | |
| let content = std::fs::read_to_string(s) | |
| .map_err(|e| anyhow::anyhow!("Failed to read metadata file {}: {}", s, e))?; | |
| common::parse_ft_metadata(Some(content)) | |
| } |
🤖 Prompt for AI Agents
In newcli/src/command/mod.rs around lines 806 to 808, the function
parse_ft_metadata_path uses .ok() to convert file read errors into None, which
hides important errors such as permission issues or missing files. Modify the
function to propagate the file reading errors by using a method like ? to return
the error wrapped in anyhow::Result instead of silently ignoring it. This will
ensure that any file reading failure is properly reported and handled.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
newcli/src/command/handlers.rs (2)
68-85: Consider adding a security warning for secret key output.The handler correctly outputs the secret key for newly created accounts. Consider adding a warning in the output or documentation to remind users to securely store this sensitive information.
354-368: Simplify conditional ABI parsing logic.The nested conditional logic can be simplified for better readability.
- let input = - if let Some((abi_path, args)) = abi_path.and_then(|path| args.map(|args| (path, args))) { - let contract = abi::read_contract(abi_path)?; - let constructor = contract - .constructor() - .ok_or_else(|| anyhow::anyhow!("Constructor not found in ABI"))?; - let args: serde_json::Value = serde_json::from_str(&args)?; - let tokens = abi::parse_args(&constructor.inputs, &args)?; - let code = hex::decode(code)?; - constructor.encode_input(code, &tokens)? - } else { - hex::decode(code)? - }; + let input = match (abi_path, args) { + (Some(abi_path), Some(args)) => { + let contract = abi::read_contract(abi_path)?; + let constructor = contract + .constructor() + .ok_or_else(|| anyhow::anyhow!("Constructor not found in ABI"))?; + let args: serde_json::Value = serde_json::from_str(&args)?; + let tokens = abi::parse_args(&constructor.inputs, &args)?; + let code = hex::decode(code)?; + constructor.encode_input(code, &tokens)? + } + _ => hex::decode(code)? + };newcli/src/command/near.rs (1)
372-378: Inconsistent view method usage.This function uses
context.view()while all other view functions in the file usecontext.client.view(). Consider using the consistent pattern for maintainability.pub async fn get_nonce(context: &Context, address: Address) -> anyhow::Result<u64> { context + .client .view(&context.cli.engine, GetNonce { address }) .await .map(|n| n.as_u64()) .map_err(Into::into) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
newcli/src/command/handlers.rs(1 hunks)newcli/src/command/near.rs(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: Shared / Static Analysis (aurora-engine-cli-rs, near, cli, 2, ., 1.18, false, --filter-paths "node_modules...
- GitHub Check: macOS
- GitHub Check: Clippy
- GitHub Check: Linux
- GitHub Check: Simple CLI
- GitHub Check: Silo CLI
- GitHub Check: Advanced CLI
🔇 Additional comments (10)
newcli/src/command/handlers.rs (4)
25-38: Good error handling pattern for CLI context.The helper function appropriately handles file read errors by outputting them directly to the CLI context and returning
None, which aligns well with the handler pattern used throughout this file.
39-66: Well-designed macro for consistent error handling.The
handle_near_call!macro effectively reduces boilerplate and ensures consistent error handling across all handler functions. The three patterns cover common use cases elegantly.
377-418: Comprehensive transaction status handling.The function properly handles all possible transaction execution statuses with appropriate output formatting for each case.
798-814: Correct NEP141Wei conversion for FT transfers.The function properly converts NearToken to NEP141Wei using yoctonear units, ensuring accurate token amount handling.
newcli/src/command/near.rs (6)
73-104: Secure account creation with proper key generation.The function correctly handles both sub-account and root contract account creation patterns, using secure random key generation.
129-172: Proper dual initialization of Aurora and ETH connector.The batch transaction correctly initializes both components with appropriate gas allocation and argument serialization.
454-477: Secure transaction submission with ABI encoding.The function properly handles secret key parsing, ABI encoding, and transaction submission with comprehensive error handling.
1179-1211: Correct Aurora transaction construction.The function properly constructs legacy Ethereum transactions with Aurora-specific parameters (zero gas price, max gas limit) and signs them correctly.
1120-1155: Robust view call implementation with ABI support.The function properly handles ABI encoding for inputs and decoding for outputs, with appropriate error handling for failed view calls.
1157-1177: Clean whitelist argument conversion logic.The function properly handles different whitelist entry types with clear error messages for parsing failures.
| pub async fn set_eth_connector_contract_data<P: AsRef<Path> + Send>( | ||
| context: &Context, | ||
| prover_id: AccountId, | ||
| custodian_address: Address, | ||
| ft_metadata_path: P, | ||
| ) -> anyhow::Result<()> { | ||
| let ft_metadata = parse_ft_metadata(std::fs::read_to_string(ft_metadata_path).ok())?; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify intentional silent error handling for metadata file.
The use of .ok() silently converts file read errors to None. If this is intentional (e.g., to use default metadata when file is missing), consider adding a comment to clarify. Otherwise, propagate the error.
🏁 Script executed:
#!/bin/bash
# Check how parse_ft_metadata handles None input
ast-grep --pattern 'fn parse_ft_metadata($_) { $$$ }'Length of output: 55
🏁 Script executed:
#!/bin/bash
# Locate the definition and usages of parse_ft_metadata
rg -n "parse_ft_metadata" .Length of output: 1094
🏁 Script executed:
#!/bin/bash
# Display the definition of parse_ft_metadata in newcli/src/common/mod.rs
sed -n '1,200p' newcli/src/common/mod.rsLength of output: 4161
Clarify Default Fallback Behavior for FT Metadata
The use of .ok() here is intentional—parse_ft_metadata(None) will return a built-in default (localETH) when the file is missing or unreadable. To make this explicit for future readers, please add a brief comment at each call site (or update the function’s doc) describing that missing/unreadable metadata falls back to default values.
• In newcli/src/command/near.rs (at the parse_ft_metadata(std::fs::read_to_string…).ok()) call)
• In newcli/src/command/mod.rs (in parse_ft_metadata_path)
Suggested diff:
--- a/newcli/src/command/near.rs
+++ b/newcli/src/command/near.rs
@@ -784,7 +784,9 @@ pub async fn set_eth_connector_contract_data<P: AsRef<Path> + Send>(
prover_id: AccountId,
custodian_address: Address,
ft_metadata_path: P,
-) let ft_metadata = parse_ft_metadata(std::fs::read_to_string(ft_metadata_path).ok())?;
+ // If the metadata file is missing or unreadable, fallback to default FT metadata
+ let ft_metadata = parse_ft_metadata(std::fs::read_to_string(ft_metadata_path).ok())?;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| pub async fn set_eth_connector_contract_data<P: AsRef<Path> + Send>( | |
| context: &Context, | |
| prover_id: AccountId, | |
| custodian_address: Address, | |
| ft_metadata_path: P, | |
| ) -> anyhow::Result<()> { | |
| let ft_metadata = parse_ft_metadata(std::fs::read_to_string(ft_metadata_path).ok())?; | |
| pub async fn set_eth_connector_contract_data<P: AsRef<Path> + Send>( | |
| context: &Context, | |
| prover_id: AccountId, | |
| custodian_address: Address, | |
| ft_metadata_path: P, | |
| ) -> anyhow::Result<()> { | |
| // If the metadata file is missing or unreadable, fallback to default FT metadata | |
| let ft_metadata = parse_ft_metadata(std::fs::read_to_string(ft_metadata_path).ok())?; | |
| // … rest of function … |
🤖 Prompt for AI Agents
In newcli/src/command/near.rs around lines 781 to 787, add a brief comment
before the call to
parse_ft_metadata(std::fs::read_to_string(ft_metadata_path).ok()) explaining
that if the metadata file is missing or unreadable, parse_ft_metadata(None)
returns a built-in default (localETH). Similarly, update the doc or add a
comment in newcli/src/command/mod.rs at the parse_ft_metadata_path function to
clarify this fallback behavior for future readers.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
newcli/src/cli.rs(1 hunks)newcli/src/command/handlers.rs(1 hunks)scripts/simple.sh(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- scripts/simple.sh
- newcli/src/cli.rs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: Shared / Static Analysis (aurora-engine-cli-rs, near, cli, 2, ., 1.18, false, --filter-paths "node_modules...
- GitHub Check: Linux
- GitHub Check: Clippy
- GitHub Check: macOS
- GitHub Check: Silo CLI
- GitHub Check: Simple CLI
- GitHub Check: Advanced CLI
🔇 Additional comments (19)
newcli/src/command/handlers.rs (19)
1-26: Well-organized importsThe module imports are properly structured and grouped by source.
27-40: Good centralized error handling for WASM file readingThe helper function properly handles file read errors and provides consistent error output formatting.
41-68: Excellent macro design for consistent error handlingThe
handle_near_call!macro provides a clean abstraction with three useful patterns for handling async NEAR calls and formatting responses consistently.
102-138: Well-implemented deployment and initialization handlersGood use of the centralized WASM reading helper and proper default value handling.
140-227: Consistent implementation of view functionsGood attention to detail with special Plain format handling for version output and consistent address formatting throughout.
228-302: Well-structured contract management handlersClear success messages and consistent implementation across all administrative functions.
303-357: Consistent use of centralized WASM file readingGood refactoring to use the
read_wasm_filehelper across all functions that need to read WASM files.
359-386: Well-implemented contract deployment with ABI supportGood handling of optional ABI encoding and proper error propagation throughout the complex deployment logic.
388-429: Comprehensive execution status handlingGood implementation with proper handling of all execution status cases and result deserialization.
431-478: Well-integrated ABI-based contract interaction handlersProper token formatting for display and consistent parameter passing.
480-510: Consistent implementation of key and address utilitiesGood formatting of cryptographic values and consistent error handling.
512-544: Well-structured gas and silo parameter managementGood use of Option type for enable/disable semantics and proper struct construction.
546-569: Clean whitelist management implementationGood API design with the u8 to bool conversion for CLI usability.
571-607: Secure implementation of key and relayer managementProper handling of cryptographic keys and allowances with consistent patterns.
609-681: Well-designed cross-chain token bridge handlersGood implementation of bidirectional mappings and metadata management.
683-717: Consistent ETH connector management implementationProper configuration handling and flag management.
718-767: Flexible implementation of relayer and withdrawal operationsGood handling of multiple key types and flexible JSON response for hashchain data.
769-858: Standard-compliant fungible token implementationsGood adherence to NEP-141 patterns with proper type conversions between NearToken and NEP141Wei.
860-914: Well-implemented storage management following NEP-145Proper handling of optional parameters and consistent type conversions.
| pub async fn create_account(context: &Context, account: AccountId, balance: NearToken) { | ||
| let account_str = account.to_string(); | ||
| handle_near_call!( | ||
| context, | ||
| near::create_account( | ||
| context, | ||
| account, | ||
| NearToken::from_near(balance.as_yoctonear()) | ||
| ) | ||
| .await, | ||
| |key_pair: SecretKey| { | ||
| output!( | ||
| &context.cli.output_format, | ||
| result_object!("account_id" => account_str, "public_key" => key_pair.public_key().to_string(), "secret_key" => key_pair.to_string()) | ||
| ); | ||
| } | ||
| ); | ||
| } |
There was a problem hiding this comment.
Remove redundant NearToken conversion
The balance parameter is already of type NearToken, so the conversion is unnecessary.
handle_near_call!(
context,
near::create_account(
context,
account,
- NearToken::from_near(balance.as_yoctonear())
+ balance
)
.await,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| pub async fn create_account(context: &Context, account: AccountId, balance: NearToken) { | |
| let account_str = account.to_string(); | |
| handle_near_call!( | |
| context, | |
| near::create_account( | |
| context, | |
| account, | |
| NearToken::from_near(balance.as_yoctonear()) | |
| ) | |
| .await, | |
| |key_pair: SecretKey| { | |
| output!( | |
| &context.cli.output_format, | |
| result_object!("account_id" => account_str, "public_key" => key_pair.public_key().to_string(), "secret_key" => key_pair.to_string()) | |
| ); | |
| } | |
| ); | |
| } | |
| pub async fn create_account(context: &Context, account: AccountId, balance: NearToken) { | |
| let account_str = account.to_string(); | |
| handle_near_call!( | |
| context, | |
| near::create_account( | |
| context, | |
| account, | |
| balance | |
| ) | |
| .await, | |
| |key_pair: SecretKey| { | |
| output!( | |
| &context.cli.output_format, | |
| result_object!( | |
| "account_id" => account_str, | |
| "public_key" => key_pair.public_key().to_string(), | |
| "secret_key" => key_pair.to_string() | |
| ) | |
| ); | |
| } | |
| ); | |
| } |
🤖 Prompt for AI Agents
In newcli/src/command/handlers.rs around lines 70 to 87, the balance parameter
is already a NearToken, so remove the redundant conversion using
NearToken::from_near(balance.as_yoctonear()) and pass balance directly to
near::create_account to simplify the code.
…dle output formatting
2 participants
Summary by CodeRabbit
New Features
Improvements
Chores