feat(initia move cli): add decode cli & add docker image to use cli in docker container #208
Conversation
WalkthroughThis update adds a Docker build and push job to the CI workflow, integrates the Changes
Poem
✨ Finishing Touches
🪧 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 (
|
djm07073
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
tools/initia-move-cli/Dockerfile (1)
7-11: Enhance container security by using a non-root user
For better security posture, consider creating a dedicated non-root user and switching to it before setting theENTRYPOINT.
Example diff:RUN chmod +x initia-move-cli + +RUN addgroup --system app && adduser --system --ingroup app app \ + && chown app:app initia-move-cli + +USER app.github/workflows/move-cli.yml (1)
144-144: Add a newline at end of file
YAML linter warns about missing end-of-line character. Please ensure a newline is present at the end of the file.🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 144-144: no new line character at the end of file
(new-line-at-end-of-file)
📜 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 (9)
.github/workflows/move-cli.yml(1 hunks)Cargo.toml(1 hunks)libmovevm/src/lib.rs(2 hunks)libmovevm/src/move_api/handler.rs(5 hunks)tools/initia-move-cli/Cargo.toml(1 hunks)tools/initia-move-cli/Dockerfile(1 hunks)tools/initia-move-cli/src/decode.rs(1 hunks)tools/initia-move-cli/src/execute.rs(2 hunks)tools/initia-move-cli/src/main.rs(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
libmovevm/src/lib.rs (1)
libmovevm/src/memory.rs (2)
destroy_unmanaged_vector(275-277)new_unmanaged_vector(256-272)
tools/initia-move-cli/src/main.rs (1)
tools/initia-move-cli/src/decode.rs (2)
decode(58-58)decode(68-120)
🪛 YAMLlint (1.37.1)
.github/workflows/move-cli.yml
[error] 144-144: no new line character at the end of file
(new-line-at-end-of-file)
🔇 Additional comments (18)
tools/initia-move-cli/Dockerfile (1)
1-11: Minimalist Dockerfile is well-structured
The base image and package installs are lean, and the entrypoint is correctly set.Cargo.toml (1)
66-66: Root workspace dependency added correctly
Themovevmcrate is now included under[workspace.dependencies]matching the workspace member inCargo.toml. No version conflicts detected..github/workflows/move-cli.yml (1)
106-144: CI workflow stepdocker-build-pushlooks correct
The job correctly downloads the AMD64 artifact, extracts the binary, and builds/pushes the Docker image.🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 144-144: no new line character at the end of file
(new-line-at-end-of-file)
tools/initia-move-cli/Cargo.toml (1)
24-27: New CLI dependencies integrated correctly
Themovevm,serde, andserde_jsonworkspace dependencies are added in alignment with the decoding feature. Dependency ordering is consistent.libmovevm/src/lib.rs (1)
3-4: Reordered modules and exports improve clarity
Themove_apimodule is now public, and key APIs are cleanly re-exported. This enhances discoverability without altering functionality.Also applies to: 19-26
tools/initia-move-cli/src/execute.rs (3)
1-1: LGTM: Import simplificationThe removal of explicit
anyhow::Errorimport is appropriate sinceanyhow::Result<()>is equivalent toanyhow::Result<(), Error>.
10-10: LGTM: Return type simplificationThe simplified return type
anyhow::Result<()>is cleaner and equivalent to the previous explicit error type.
17-17: LGTM: Defensive programming with unreachable armThe
unreachable!()arm is appropriate here since theDecodecommands are now handled separately in the main function's dispatch logic, ensuring this match only receives the commands it's designed to handle.libmovevm/src/move_api/handler.rs (3)
20-31: LGTM: Appropriate API exposure for CLI integrationMaking
read_module_infopublic enables the CLI decode functionality. The function provides safe module information extraction with proper error handling.
84-90: LGTM: Script decoding API made publicThe
decode_script_bytesfunction is appropriately made public to support CLI script decoding operations with proper error handling.
92-98: LGTM: Module decoding API made publicThe
decode_module_bytesfunction is correctly exposed for CLI module decoding functionality with consistent error handling patterns.tools/initia-move-cli/src/main.rs (3)
2-2: LGTM: Clean module integrationThe decode module import properly integrates the new functionality into the CLI structure.
17-35: LGTM: Well-structured command enum extensionThe addition of the
Decodecommand variant and consistentflatten_help = trueattributes across all commands improve the CLI help system's consistency.
49-56: LGTM: Clean command dispatch logicThe main function properly separates decode operations from other commands, providing clear separation of concerns. The pattern matching handles all command variants appropriately.
tools/initia-move-cli/src/decode.rs (4)
7-16: LGTM: Well-structured command definitionThe
Decodestruct and command configuration provide clear documentation and proper CLI structure for the decode functionality.
18-55: LGTM: Comprehensive subcommand definitionsThe
DecodeCommandsenum provides well-documented subcommands with helpful examples and clear argument specifications.
61-65: LGTM: Robust file reading with proper error contextThe
read_filefunction correctly handles path resolution and provides meaningful error messages usingwith_context.
118-118: LGTM: Appropriate use of unreachableThe
unreachable!()arm is justified here since the main function's dispatch logic ensures onlyDecodecommands reach this method.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (4)
tools/initia-move-cli/src/decode.rs (4)
103-105: Address the identified path traversal vulnerability.The path construction using user-provided
package_nameandmodule_namestill lacks input validation, which was previously flagged as a security concern.
129-132: Apply consistent input validation for script subcommand.The path construction for the Script subcommand also needs the same input validation as identified in previous reviews.
143-144: Apply consistent input validation for module subcommand.The Module subcommand path construction requires the same security validation as previously identified.
115-117: Fix the hex conversion type casting bug.The hex conversion logic still has the type casting issue where
as_u64()is called on byte values that should be cast tou8.
🧹 Nitpick comments (3)
tools/initia-move-cli/src/decode.rs (3)
153-153: Consider removing unreachable!() or improving error handling.The
unreachable!()statement suggests this method should only be called withDecodecommands, but this constraint isn't enforced by the type system. Consider either making this a more specific method or handling unexpected cases more gracefully.- _ => unreachable!(), + _ => Err(anyhow::anyhow!("decode() called with non-Decode command")),
109-121: Extract address conversion logic into a helper function.The address conversion logic could be extracted to reduce code complexity and improve reusability.
+fn convert_address_to_hex(json: &mut serde_json::Value) { + if let Some(address) = json.get_mut("address") { + if let serde_json::Value::Array(bytes) = address { + let hex = format!( + "0x{}", + bytes + .iter() + .filter_map(|b| b.as_u64().map(|n| n as u8)) + .map(|b| format!("{:02x}", b)) + .collect::<String>() + ); + *address = serde_json::json!(hex); + } + } +} let mut json: serde_json::Value = serde_json::from_slice(&result)?; - if let Some(address) = json.get_mut("address") { - if let serde_json::Value::Array(bytes) = address { - let hex = format!( - "0x{}", - bytes - .iter() - .filter_map(|b| b.as_u64()) - .map(|b| format!("{:02x}", b)) - .collect::<String>() - ); - *address = serde_json::json!(hex); - } - } + convert_address_to_hex(&mut json);
98-149: Consider consolidating similar decode operations.The Read and Module commands have very similar logic except for the address conversion step. Consider extracting common functionality to reduce duplication.
You could create a helper function like:
fn decode_module_with_options( package_name: &str, module_name: &str, package_path: &Option<PathBuf>, convert_address: bool ) -> anyhow::Result<()> { let path = format!("build/{}/bytecode_modules/{}.mv", package_name, module_name); let bytes = read_file(package_path, &path)?; let result = if convert_address { read_module_info(&bytes)? } else { decode_module_bytes(bytes)? }; let mut json: serde_json::Value = serde_json::from_slice(&result)?; if convert_address { convert_address_to_hex(&mut json); } println!("{}", serde_json::to_string_pretty(&json)?); Ok(()) }
Summary by CodeRabbit
New Features
Decodesubcommand for reading and decoding Move modules and scripts, outputting info and ABI in JSON.Improvements
Documentation