feat(bin): restructure init command with explicit rollup/sovereign modes

[kariy]

Refactor katana init command to use explicit subcommands for rollup and sovereign chain modes. Now running katana init requires you to select the mode as a subcommand: katana init rollup or katana init sovereign.

One of the reason why we made this change is because the CLI argument configurations will always require you to provide the arguments instead of falling back to prompting.

The expected behaviour when you literally just run katana init is for it to prompt the arguments like so:

But due to how the CLI arguments is being configured, we instead get an error for not providing the arguments using flags:

This is mainly because of the #[arg(required_unless_present = "sovereign")] tag we're using that forces us to provide the options if --sovereign is not present.

katana/bin/katana/src/cli/init/mod.rs

Lines 44 to 48 in d584d42

	#[arg(long = "settlement-chain")]
	#[arg(required_unless_present = "sovereign")]
	#[arg(requires_all = ["id", "settlement_account", "settlement_account_private_key"])]
	settlement_chain: Option<SettlementChain>,

Afaik it doesn't seem to be possible to 'fix' this using clap derive macro. Even if it does, I believe separating the mode into separate commands feels like a better UX.

Comparison with current behaviour

• Current: katana init --id my-chain --settlement-chain sepolia ...
• New: katana init rollup --id my-chain --settlement-chain sepolia ...

[kariy]

@claude /review

[claude]

Claude finished @kariy's task —— View job

---

Code Review for PR #280

Reviewed the refactoring to separate rollup and sovereign chain initialization into distinct subcommands.

Todo Checklist:

• Read and analyze bin/katana/src/cli/init/mod.rs changes
• Read and analyze bin/katana/src/cli/init/prompt.rs changes
• Read and analyze bin/katana/src/cli/mod.rs changes
• Understand the architectural changes and separation of concerns
• Review for bugs, security issues, and best practices
• Provide comprehensive feedback with specific recommendations

✅ Overall Assessment

This is a solid architectural improvement that enhances the CLI UX by separating rollup and sovereign initialization into distinct subcommands (katana init rollup vs katana init sovereign). The refactoring improves argument validation, provides clearer help text, and maintains backward compatibility through interactive prompting.

🎯 Strengths

1. Excellent Documentation: The new module-level documentation (lines 1-56 in mod.rs) is comprehensive and provides clear usage examples for both modes
2. Better UX: Users can now directly specify their intent instead of selecting modes interactively
3. Type Safety: Argument validation moved to clap level with proper requires_all constraints
4. Comprehensive Testing: Good test coverage for CLI argument parsing edge cases
5. Clean Separation: Each mode has its own dedicated argument struct and execution logic

⚠️ Areas for Improvement

1. Error Handling (mod.rs:285)

let l1_chain_id = settlement_provider.chain_id().await.unwrap();

Issue: Using .unwrap() in async context could panic
Suggestion:

let l1_chain_id = settlement_provider.chain_id().await
    .context("Failed to get settlement chain ID")?;

2. Large Method Complexity (mod.rs:244-326)

The configure_from_args() method is quite large and handles multiple concerns. Consider breaking it down:

// Extract to separate methods:
async fn validate_required_args(&self) -> Option<RequiredArgs> { /* ... */ }
async fn setup_settlement_provider(&self) -> Result<SettlementChainProvider> { /* ... */ }  
async fn handle_deployment(&self, provider: &SettlementChainProvider) -> Result<DeploymentOutcome> { /* ... */ }

3. Code Duplication (mod.rs:224-241 vs 341-359)

Both RollupArgs::execute() and SovereignArgs::execute() share similar logic:

Suggestion: Extract common functionality:

fn write_chain_spec(
    spec: &rollup::ChainSpec, 
    output_path: Option<PathBuf>
) -> anyhow::Result<()> {
    if let Some(path) = output_path {
        let dir = ChainConfigDir::create(path)?;
        rollup::write(&dir, spec).context("failed to write chain spec file")?;
    } else {
        rollup::write_local(spec).context("failed to write chain spec file")?;
    }
    Ok(())
}

4. Complex Slot Paymaster Logic (prompt.rs:196-240)

The Rc<RefCell<Vec<_>>> pattern for validation seems overengineered for CLI input validation:

Suggestion: Consider a simpler approach:

fn prompt_slot_paymasters() -> Result<Option<Vec<slot::PaymasterAccountArgs>>> {
    let mut paymasters = Vec::new();
    
    while Confirm::new("Add Slot paymaster account?").with_default(true).prompt()? {
        let public_key = /* ... */;
        let salt = /* validate against existing paymasters directly */;
        
        if paymasters.iter().any(|pm| pm.public_key == public_key && pm.salt == salt) {
            println!("Public key and salt combination already exists!");
            continue;
        }
        
        paymasters.push(PaymasterAccountArgs { public_key, salt });
    }
    
    Ok(Some(paymasters))
}

5. Minor Issues

• mod.rs:287: Consider adding error context to cairo_short_string_to_felt(&id).unwrap()
• prompt.rs:78-85: The async block could benefit from better error messages for network failures

🚀 Recommendations

1. Consider feature gating: The rollup vs sovereign distinction might benefit from feature flags if one mode is more mature than the other
2. Add integration tests: Consider adding end-to-end tests that exercise both CLI modes
3. Validate chain IDs: Add validation to ensure chain IDs don't conflict with existing networks

📝 Summary

This refactoring successfully improves the CLI architecture while maintaining functionality. The separation of concerns is well-executed, and the comprehensive documentation is exemplary. Address the error handling and consider the code deduplication suggestions to further improve code quality.

---

[kariy]

/claude-review

[codecov]

Codecov Report

❌ Patch coverage is 23.84615% with 99 lines in your changes missing coverage. Please review.
✅ Project coverage is 75.72%. Comparing base (9bde0ae) to head (47e996b).
⚠️ Report is 119 commits behind head on main.

Files with missing lines	Patch %	Lines
bin/katana/src/cli/init/mod.rs	33.33%	62 Missing ⚠️
bin/katana/src/cli/init/prompt.rs	0.00%	33 Missing ⚠️
bin/katana/src/cli/init/deployment.rs	0.00%	3 Missing ⚠️
bin/katana/src/main.rs	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #280      +/-   ##
==========================================
+ Coverage   73.32%   75.72%   +2.40%     
==========================================
  Files         209      229      +20     
  Lines       23132    27098    +3966     
==========================================
+ Hits        16961    20521    +3560     
- Misses       6171     6577     +406     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.