feat(katana): non-interactive init #2988
Conversation
|
Ohayo sensei! WalkthroughThis pull request introduces several asynchronous enhancements and refactors across CLI components. A new dependency ( Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant P as Prompt Module
participant RPC as JSON RPC Client
participant O as Outcome Constructor
U->>P: Enter chain ID & select settlement option
P->>RPC: Validate settlement RPC URL & contract
RPC-->>P: Return validation result
P->>U: Request account address and private key
U->>P: Submit account info
P->>O: Construct Outcome with config details
O-->>P: Outcome ready
P-->>U: Return Outcome
sequenceDiagram
participant CLI as CLI Runner
participant RT as Tokio Runtime
participant CMD as Command
CLI->>RT: execute_async(command.execute())
RT->>CMD: Process command asynchronously
CMD-->>RT: Return command result
RT-->>CLI: Deliver result
Possibly related PRs
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
bin/katana/src/cli/mod.rs (2)
68-70: Ohayo sensei, consider customizing error messages.
execute_asyncis clear, but ifbuild_tokio_runtimefails, it might help to provide a more detailed error log or fallback strategy.
72-74: Ohayo sensei, reviewing multi-thread runtime.
new_multi_thread()is a solid default, but some users may require explicit thread-count control.bin/katana/src/cli/init/prompt.rs (1)
80-88: Ohayo sensei, well-done with the private key input.
Consider using a masked input or specialized widget to safeguard sensitive info from shoulder surfers.bin/katana/src/cli/init/mod.rs (3)
51-56: Ohayo sensei, movingexecuteto async is a good step for concurrency.
Just ensure that potential blocking calls (e.g., file I/O) are carefully handled with async I/O if needed.
92-122: Ohayo sensei, defensive checks on existing vs. newly deployed contracts.
Verifying the contract program info is a good security measure. Recommended to wrap any unavoidableunwrap()in more descriptive errors if there's an edge case.
163-168: Ohayo sensei, enumerating settlement chain variants.
Well-structured for expansions. Possibly consider an Off chain or a more advanced scenario in future.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
bin/katana/Cargo.toml(1 hunks)bin/katana/src/cli/init/mod.rs(4 hunks)bin/katana/src/cli/init/prompt.rs(1 hunks)bin/katana/src/cli/mod.rs(3 hunks)crates/katana/cli/src/args.rs(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: fmt
🔇 Additional comments (18)
bin/katana/src/cli/mod.rs (4)
1-2: Ohayo sensei, nice addition for async support!
Bringing inFuturefrom the standard library paves the way for concurrency-savvy CLI commands.
8-8: Ohayo sensei, good job usingtokio::runtime::Runtimefor concurrency.
This is essential for asynchronous tasks.
30-31: Ohayo sensei, verify error handling on async command.
Wrapping theInitcommand inexecute_asyncis a sound approach. Ensure any panics insideargs.execute()are gracefully handled.
35-35: Ohayo sensei, consistent async strategy.
The sameexecute_asyncusage keeps the code uniform. Looks good!bin/katana/src/cli/init/prompt.rs (4)
19-31: Ohayo sensei, user input forchain_idis well-defined.
Validating ASCII ensures a clean chain identifier. Consider clarifying if uppercase vs. lowercase matters.
33-53: Ohayo sensei, settlement chain selection logic looks good.
The dynamic#[cfg(feature = "init-custom-settlement-chain")]approach is neat. It's easy to extend in the future for more settlement chains.
98-119: Ohayo sensei, mindful deployment logic.
Deploying vs re-using a settlement contract is straightforward. Confirm that thedeploymentinstructions have thorough retry or crash recovery steps in production scenarios.
121-128: Ohayo sensei, finalOutcomestructure is clear and well-organized.
Great job consolidating user inputs here for easy consumption downstream.bin/katana/src/cli/init/mod.rs (8)
26-46: Ohayo sensei, nice usage ofrequires_allinInitArgs.
This elegantly ensures that the user either provides all settlement-related arguments or none.
59-63: Ohayo sensei, settlement layer struct usage is intuitive.
Neatly compartmentalizes chain ID, account, and contract.
76-90: Ohayo sensei,process_argsgating logic is robust!
Clap'srequires_allhelps, but double-check all side effects if partial arguments are supplied.
130-145: Ohayo sensei,Outcomestruct is well-labeled.
Makes it easy to see which fields are used for settlement configuration and storing user-provided data.
157-161: Ohayo sensei, informative error type.
Mentioning the invalid chain ID inSettlementChainTryFromStrErroris good for debugging.
170-185: Ohayo sensei,FromStrimplementation is user-friendly.
This pattern elegantly captures recognized chain aliases or custom URLs, if the feature is enabled.
187-192: Ohayo sensei, bridgingtry_fromis consistent.
No duplication of logic — nicely callsfrom_str.
194-230: Ohayo sensei, solid test coverage forSettlementChain.
Nice to see corner cases, including uppercase variants and invalid inputs. Continue adding more tests for dynamic use cases if required.crates/katana/cli/src/args.rs (1)
116-119: Ohayo! The async implementation looks great, sensei!The conversion of
executeto an async function is well-implemented, properly awaiting thestart_nodecall.bin/katana/Cargo.toml (1)
35-35: Ohayo! Good choice with the url crate, sensei!Using the workspace-managed url crate is a solid choice for URL handling and maintains version consistency across the project.
| let contract_exist_parser = &|input: &str| { | ||
| let block_id = BlockId::Tag(BlockTag::Pending); | ||
| let address = Felt::from_str(input).map_err(|_| ())?; | ||
| let result = tokio::task::block_in_place(|| { | ||
| Handle::current().block_on(l1_provider.clone().get_class_hash_at(block_id, address)) | ||
| }); | ||
|
|
||
| match result { | ||
| Ok(..) => Ok(ContractAddress::from(address)), | ||
| Err(..) => Err(()), | ||
| } | ||
| }; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Ohayo sensei, blocking within block_in_place can stall other tasks.
To avoid blocking the entire thread pool, consider a non-blocking approach, such as fully async calls without block_in_place.
- let result = tokio::task::block_in_place(|| {
- Handle::current().block_on(l1_provider.clone().get_class_hash_at(block_id, address))
- });
+ let result = l1_provider
+ .clone()
+ .get_class_hash_at(block_id, address)
+ .await;Committable suggestion skipped: line range outside the PR's diff.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2988 +/- ##
==========================================
- Coverage 56.99% 56.97% -0.02%
==========================================
Files 425 426 +1
Lines 56303 56376 +73
==========================================
+ Hits 32088 32120 +32
- Misses 24215 24256 +41 ☔ View full report in Codecov by Sentry. |
If no arguments are provided,
katana initretains its interactive prompt (current behavior). This PR adds a non-interactive flow where all required values can be passed directly via CLI options to skip the prompts.