feat: add cli methods for ciphernode registration [skip-line-limit]#1089
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
WalkthroughAdds a new Ciphernode CLI subsystem: a ChainContext for chain-aware provider/signer setup, license and ticket command handlers, lifecycle operations, ERC20 helpers, contract bindings and artifacts for EnclaveTicketToken, and two workspace dependencies ( Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant ChainContext
participant Provider
participant ERC20
participant BondingRegistry
User->>CLI: ciphernode tickets buy <amount> --chain <x>
CLI->>ChainContext: ChainContext::new(config, selection)
ChainContext->>Provider: init RPC + signer
Provider-->>ChainContext: provider ready
ChainContext-->>CLI: context
CLI->>ChainContext: ticket_token_address()
ChainContext->>BondingRegistry: bonding.ticketToken()
BondingRegistry-->>ChainContext: ticket_token_addr
ChainContext-->>CLI: ticket_token_addr
CLI->>ChainContext: erc20(ticket_token_addr)
ChainContext-->>CLI: ERC20 instance
CLI->>ERC20: decimals()
ERC20-->>CLI: decimals
CLI->>CLI: parse_amount(input, decimals)
CLI->>ChainContext: ensure_allowance(token, bonding_addr, amount)
ChainContext->>ERC20: allowance(operator, bonding_addr)
ERC20-->>ChainContext: current_allowance
alt allowance < amount
ChainContext->>ERC20: approve(bonding_addr, amount)
ERC20-->>Provider: tx
Provider-->>ChainContext: receipt
end
ChainContext-->>CLI: allowance ensured
CLI->>ChainContext: bonding()
ChainContext-->>CLI: BondingRegistry instance
CLI->>BondingRegistry: addTicketBalance(amount)
BondingRegistry-->>Provider: tx
Provider-->>CLI: tx_receipt
CLI->>User: print tx hash / confirmation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
hmzakhalid
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: 0
🧹 Nitpick comments (4)
crates/cli/src/ciphernode/tickets.rs (1)
34-37: Unused variableticket_contract.The
ticket_contractvariable is fetched on line 35 but never used. The decimals are fetched directly on line 36 usingctx.ticket_token_address().await?again, making line 35 redundant. Consider removing it or using the variable consistently.TicketCommands::Burn { amount } => { - let ticket_contract = ctx.ticket_token_address().await?; - let decimals = ctx.erc20(ticket_contract).decimals().call().await?; + let ticket_token = ctx.ticket_token_address().await?; + let decimals = ctx.erc20(ticket_token).decimals().call().await?; let parsed = parse_amount(&amount, decimals)?;crates/cli/src/ciphernode/utils.rs (1)
91-109: Consider informing users about approval transactions.The
ensure_allowancefunction silently approves tokens when needed. For CLI transparency, consider printing a message when an approval transaction is sent, so users are aware of the additional transaction.+ if current < amount { + println!("Approving {} to spend tokens...", spender); + } + erc20 .approve(spender, amount) .send() .await? .get_receipt() .await?;crates/cli/src/ciphernode/lifecycle.rs (1)
45-47: Clarify the relationship betweenactivateandregister.The
activatefunction simply delegates toregister. While this may be intentionally aliased (the smart contract'sregisterOperatormight handle both initial registration and reactivation), the current implementation could confuse users who expect different behavior. Consider adding a comment explaining this design decision.+/// Activate the operator. This is functionally equivalent to calling register, +/// as the contract's registerOperator handles both initial registration and +/// reactivation after meeting collateral requirements. pub(crate) async fn activate(ctx: &ChainContext) -> Result<()> { register(ctx).await }crates/cli/src/ciphernode/context.rs (1)
123-139: Consider caching token addresses if called multiple times.The async methods
license_token_address(),ticket_token_address(), andticket_underlying_address()make RPC calls on each invocation. For CLI commands that only call these once, this is fine. If any command path calls these multiple times, caching the results would reduce RPC overhead.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (9)
crates/cli/Cargo.toml(2 hunks)crates/cli/src/ciphernode/context.rs(1 hunks)crates/cli/src/ciphernode/license.rs(1 hunks)crates/cli/src/ciphernode/lifecycle.rs(1 hunks)crates/cli/src/ciphernode/mod.rs(1 hunks)crates/cli/src/ciphernode/tickets.rs(1 hunks)crates/cli/src/ciphernode/utils.rs(1 hunks)crates/cli/src/cli.rs(3 hunks)crates/cli/src/main.rs(1 hunks)
🧰 Additional context used
🧠 Learnings (13)
📓 Common learnings
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 107
File: packages/ciphernode/sortition/src/distance.rs:1-1
Timestamp: 2024-09-26T03:11:29.311Z
Learning: In `packages/ciphernode/core/src/events.rs`, the import statements use the correct and updated `alloy::primitives` module.
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 107
File: packages/ciphernode/sortition/src/distance.rs:1-1
Timestamp: 2024-10-08T19:45:18.209Z
Learning: In `packages/ciphernode/core/src/events.rs`, the import statements use the correct and updated `alloy::primitives` module.
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 156
File: packages/ciphernode/enclave/src/commands/wallet/set.rs:17-23
Timestamp: 2024-10-28T12:00:09.010Z
Learning: In the `enclave` package of the `ciphernode` project, prefer using `println!` over logging macros like `error!` from the `tracing` crate for error output in CLI commands.
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 156
File: packages/ciphernode/keyshare/src/encryption.rs:45-45
Timestamp: 2024-10-23T02:03:02.008Z
Learning: In the `packages/ciphernode/keyshare/src/encryption.rs` file, the environment variable `CIPHERNODE_SECRET` is used for the encryption password. A secure secret management solution is not currently available, but may be considered in future iterations.
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 173
File: packages/ciphernode/config/src/app_config.rs:13-21
Timestamp: 2024-11-05T06:48:58.177Z
Learning: In the `packages/ciphernode/config/src/app_config.rs` file, for the `Contract` enum, the team prefers to use `String` type for `address` fields, relying on parsing to handle validation, instead of using the `Address` type.
📚 Learning: 2024-10-08T19:45:18.209Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 107
File: packages/ciphernode/sortition/src/distance.rs:1-1
Timestamp: 2024-10-08T19:45:18.209Z
Learning: In `packages/ciphernode/core/src/events.rs`, the import statements use the correct and updated `alloy::primitives` module.
Applied to files:
crates/cli/src/cli.rscrates/cli/src/main.rscrates/cli/src/ciphernode/utils.rscrates/cli/src/ciphernode/lifecycle.rscrates/cli/src/ciphernode/license.rscrates/cli/src/ciphernode/context.rscrates/cli/Cargo.tomlcrates/cli/src/ciphernode/mod.rs
📚 Learning: 2024-10-28T12:00:09.010Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 156
File: packages/ciphernode/enclave/src/commands/wallet/set.rs:17-23
Timestamp: 2024-10-28T12:00:09.010Z
Learning: In the `enclave` package of the `ciphernode` project, prefer using `println!` over logging macros like `error!` from the `tracing` crate for error output in CLI commands.
Applied to files:
crates/cli/src/cli.rscrates/cli/src/main.rscrates/cli/src/ciphernode/license.rscrates/cli/src/ciphernode/mod.rs
📚 Learning: 2024-10-23T02:03:02.008Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 156
File: packages/ciphernode/keyshare/src/encryption.rs:45-45
Timestamp: 2024-10-23T02:03:02.008Z
Learning: In the `packages/ciphernode/keyshare/src/encryption.rs` file, the environment variable `CIPHERNODE_SECRET` is used for the encryption password. A secure secret management solution is not currently available, but may be considered in future iterations.
Applied to files:
crates/cli/src/cli.rscrates/cli/src/main.rs
📚 Learning: 2024-10-28T12:04:26.578Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 156
File: packages/ciphernode/enclave_node/src/ciphernode.rs:26-28
Timestamp: 2024-10-28T12:04:26.578Z
Learning: In the `setup_ciphernode` function in `packages/ciphernode/enclave_node/src/ciphernode.rs`, panicking on errors during setup is acceptable.
Applied to files:
crates/cli/src/cli.rscrates/cli/src/main.rscrates/cli/src/ciphernode/mod.rs
📚 Learning: 2024-10-28T10:59:42.202Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 156
File: packages/ciphernode/config/src/app_config.rs:146-175
Timestamp: 2024-10-28T10:59:42.202Z
Learning: In `packages/ciphernode/config/src/app_config.rs`, the `normalize_path` function uses custom path normalization instead of `std::fs::canonicalize` because using `canonicalize` requires actual files to exist, making testing more difficult.
Applied to files:
crates/cli/src/cli.rs
📚 Learning: 2024-10-23T01:59:42.967Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 156
File: packages/ciphernode/tests/tests/test_aggregation_and_decryption.rs:274-274
Timestamp: 2024-10-23T01:59:42.967Z
Learning: In the `packages/ciphernode/tests/tests/test_aggregation_and_decryption.rs` file and other test files within this project, hardcoding `CIPHERNODE_SECRET` is acceptable for testing purposes.
Applied to files:
crates/cli/src/cli.rscrates/cli/src/main.rs
📚 Learning: 2024-11-05T06:48:58.177Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 173
File: packages/ciphernode/config/src/app_config.rs:13-21
Timestamp: 2024-11-05T06:48:58.177Z
Learning: In the `packages/ciphernode/config/src/app_config.rs` file, for the `Contract` enum, the team prefers to use `String` type for `address` fields, relying on parsing to handle validation, instead of using the `Address` type.
Applied to files:
crates/cli/src/cli.rscrates/cli/src/ciphernode/context.rs
📚 Learning: 2024-10-29T01:03:50.414Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 156
File: packages/ciphernode/config/src/app_config.rs:21-26
Timestamp: 2024-10-29T01:03:50.414Z
Learning: In `packages/ciphernode/config/src/app_config.rs`, the `rpc_url` field in the `ChainConfig` struct is not considered sensitive and does not need to be encrypted.
Applied to files:
crates/cli/src/cli.rs
📚 Learning: 2024-11-05T06:49:46.285Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 173
File: packages/ciphernode/enclave_node/src/datastore.rs:14-16
Timestamp: 2024-11-05T06:49:46.285Z
Learning: In `packages/ciphernode/enclave_node/src/datastore.rs`, for internal functions like `get_in_mem_store`, adding extensive documentation and error handling may not be necessary if they are not client-facing.
Applied to files:
crates/cli/src/ciphernode/utils.rs
📚 Learning: 2024-10-12T10:24:07.572Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 139
File: packages/ciphernode/aggregator/src/publickey_aggregator.rs:46-46
Timestamp: 2024-10-12T10:24:07.572Z
Learning: In `packages/ciphernode/router/src/hooks.rs`, the `src_chain_id` parameter in `PublicKeyAggregator::new` is correctly handled, even if not explicitly provided during instantiation.
Applied to files:
crates/cli/src/ciphernode/context.rs
📚 Learning: 2025-08-25T10:28:56.174Z
Learnt from: ctrlc03
Repo: gnosisguild/enclave PR: 657
File: Cargo.toml:32-34
Timestamp: 2025-08-25T10:28:56.174Z
Learning: The examples/CRISP directory has its own Cargo.toml workspace configuration with members like "server", "wasm-crypto", "program/core", "program/client", etc. The root workspace intentionally excludes "examples/CRISP/server", "examples/CRISP/program", and "examples/CRISP/wasm-crypto" to prevent double workspace membership, which is the correct approach for self-contained example workspaces.
Applied to files:
crates/cli/Cargo.toml
📚 Learning: 2024-10-22T02:36:01.448Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 145
File: packages/ciphernode/data/Cargo.toml:0-0
Timestamp: 2024-10-22T02:36:01.448Z
Learning: In `packages/ciphernode/data/Cargo.toml`, `actix-rt` is only used in tests and should remain in `[dev-dependencies]`.
Applied to files:
crates/cli/Cargo.toml
🧬 Code graph analysis (6)
crates/cli/src/cli.rs (1)
crates/cli/src/ciphernode/mod.rs (1)
execute(123-163)
crates/cli/src/ciphernode/tickets.rs (1)
crates/cli/src/ciphernode/utils.rs (2)
ensure_allowance(91-110)parse_amount(33-69)
crates/cli/src/ciphernode/utils.rs (1)
crates/cli/src/ciphernode/context.rs (1)
erc20(141-146)
crates/cli/src/ciphernode/lifecycle.rs (3)
crates/utils/src/actix.rs (1)
bail(12-14)crates/cli/src/ciphernode/utils.rs (3)
format_amount(14-31)parse_amount(33-69)parse_u256_list(71-77)crates/cli/src/ciphernode/context.rs (2)
chain_label(115-117)operator(111-113)
crates/cli/src/ciphernode/license.rs (4)
crates/cli/src/ciphernode/utils.rs (2)
ensure_allowance(91-110)parse_amount(33-69)crates/cli/src/ciphernode/mod.rs (1)
execute(123-163)crates/cli/src/ciphernode/tickets.rs (1)
execute(13-53)crates/cli/src/ciphernode/context.rs (2)
chain_label(115-117)erc20(141-146)
crates/cli/src/ciphernode/mod.rs (5)
crates/cli/src/ciphernode/license.rs (1)
execute(14-90)crates/cli/src/ciphernode/tickets.rs (1)
execute(13-53)crates/cli/src/cli.rs (1)
execute(85-200)crates/cli/src/ciphernode/context.rs (1)
new(78-99)crates/cli/src/ciphernode/lifecycle.rs (5)
register(13-27)deregister(29-43)activate(45-47)deactivate(49-96)status(98-145)
⏰ 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). (9)
- GitHub Check: integration_prebuild
- GitHub Check: build_enclave_cli
- GitHub Check: test_net
- GitHub Check: test_contracts
- GitHub Check: build_ciphernode_image
- GitHub Check: build_sdk
- GitHub Check: crisp_unit
- GitHub Check: rust_unit
- GitHub Check: rust_integration
🔇 Additional comments (19)
crates/cli/src/main.rs (1)
11-11: LGTM!The module declaration follows the existing pattern and correctly integrates the new ciphernode subsystem into the CLI crate.
crates/cli/Cargo.toml (1)
15-15: LGTM!The new workspace dependencies (
alloyande3-evm) are correctly declared and align with the blockchain interaction requirements of the ciphernode subsystem.Also applies to: 24-24
crates/cli/src/cli.rs (1)
9-9: LGTM!The ciphernode command integration follows the established CLI patterns:
- Import mirrors other command module imports
- Dispatch follows the same pattern as existing subcommands (Password, Wallet, Net)
- Enum variant is correctly structured with the flattened subcommand
Also applies to: 192-192, 293-297
crates/cli/src/ciphernode/tickets.rs (1)
13-53: LGTM!The ticket operations are well-structured:
Buycorrectly ensures allowance on the underlying token before depositingBurnproperly retrieves decimals for amount parsing- Both commands print transaction receipts per project conventions
crates/cli/src/ciphernode/license.rs (2)
14-90: LGTM!The license command implementation is well-structured:
Acquirecorrectly handles dev/faucet minting with allocation metadataBonduses the helper function with proper allowance checkingUnbondqueues exits appropriatelyClaimusesU256::MAXas a sensible default to claim all available exits- Error handling and receipt printing are consistent throughout
92-110: LGTM onbond_licensehelper.The helper correctly ensures ERC20 allowance is set before calling the bonding contract, following the same pattern used in tickets.
crates/cli/src/ciphernode/utils.rs (2)
14-31: LGTM!The
format_amountfunction correctly handles:
- Zero fractional parts (returns integer only)
- Trailing zero trimming in fractional parts
- Proper padding for fractional parts shorter than decimals
33-69: LGTM!The
parse_amountfunction correctly:
- Normalizes input by stripping underscores (visual separators)
- Validates decimal precision against token decimals
- Handles edge cases like empty fractional parts after decimal point
- Preserves leading zeros in fractional parts (e.g., "1.01" → correct value)
crates/cli/src/ciphernode/lifecycle.rs (2)
98-144: LGTM on status output.The
statusfunction provides comprehensive and well-formatted output covering:
- Registration and activation state
- Ticket and license balances with proper decimal formatting
- Pending exits
- System requirements
The sequential RPC calls are acceptable for a CLI status command.
13-27: LGTM on lifecycle operations.The
register,deregister, anddeactivatefunctions are well-implemented:
- Proper validation in
deactivateensuring at least one withdrawal type is specified- Correct use of
parse_u256_listfor IMT proof siblings- Consistent error handling and receipt printing
Also applies to: 29-43, 49-96
crates/cli/src/ciphernode/mod.rs (4)
19-30: LGTM!The
ChainArgsstruct with theselection()helper provides clean chain selection that integrates well with clap's flatten pattern.
32-79: LGTM on command definitions.The
CiphernodeCommandsenum is well-structured:
- Clear help text for each command
- Proper use of
#[command(flatten)]for the sharedChainArgsDeregisterusesvalue_delimiterfor comma-separated IMT proof nodesDeactivateuses optional amounts allowing flexible withdrawal combinations
81-121: LGTM on subcommand definitions.
LicenseCommandsandTicketCommandsare properly structured with:
- Clear help text describing each operation
- String-based amounts to support decimal input with the
parse_amountutility- Sensible default for allocation in
Acquire
123-163: LGTM on the execute dispatcher.The dispatcher correctly:
- Creates a
ChainContextfor each command variant- Delegates to the appropriate module (license, tickets, lifecycle)
- Follows the established pattern from other CLI modules
- Properly propagates errors with
?crates/cli/src/ciphernode/context.rs (5)
1-17: LGTM!License header and imports are well-organized. The use of
alloy::primitives::Addressaligns with established patterns in the codebase.
19-68: Clean contract binding organization.The sol! macro bindings are well-structured. The inline IERC20Metadata interface definition avoids external ABI dependencies while providing just the needed functions. The separate module pattern keeps generated code isolated.
70-75: LGTM!The struct design cleanly encapsulates chain-specific state with appropriate visibility.
77-99: Well-structured async constructor.The initialization flow is clear: chain selection → address parsing → cipher/signer loading → provider setup. Error propagation with
?ensures failures surface with context from the underlying calls.
156-172: LGTM!Clear error messages guide users toward resolution. The
select_chainfunction correctly returns a reference, andparse_addressfollows the team's pattern of using String in config with validation at parse time. Based on learnings, this aligns with the established approach in the codebase.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
crates/cli/src/ciphernode/context.rs (1)
138-154: Optional: enrich address parsing errors with chain/field context
parse_addresscurrently wraps failures with a generic"Invalid address in configuration"message. If you ever find debugging misconfigured chains painful, consider threading through the chain name and field (e.g.,"bonding_registry") so the error points directly at the offending entry. Not urgent, but it would improve operator UX when configs are wrong.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
crates/cli/src/ciphernode/context.rs(1 hunks)crates/cli/src/ciphernode/license.rs(1 hunks)crates/cli/src/ciphernode/mod.rs(1 hunks)
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 107
File: packages/ciphernode/sortition/src/distance.rs:1-1
Timestamp: 2024-10-08T19:45:18.209Z
Learning: In `packages/ciphernode/core/src/events.rs`, the import statements use the correct and updated `alloy::primitives` module.
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 107
File: packages/ciphernode/sortition/src/distance.rs:1-1
Timestamp: 2024-09-26T03:11:29.311Z
Learning: In `packages/ciphernode/core/src/events.rs`, the import statements use the correct and updated `alloy::primitives` module.
📚 Learning: 2024-10-08T19:45:18.209Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 107
File: packages/ciphernode/sortition/src/distance.rs:1-1
Timestamp: 2024-10-08T19:45:18.209Z
Learning: In `packages/ciphernode/core/src/events.rs`, the import statements use the correct and updated `alloy::primitives` module.
Applied to files:
crates/cli/src/ciphernode/mod.rscrates/cli/src/ciphernode/context.rs
📚 Learning: 2024-10-12T10:24:07.572Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 139
File: packages/ciphernode/aggregator/src/publickey_aggregator.rs:46-46
Timestamp: 2024-10-12T10:24:07.572Z
Learning: In `packages/ciphernode/router/src/hooks.rs`, the `src_chain_id` parameter in `PublicKeyAggregator::new` is correctly handled, even if not explicitly provided during instantiation.
Applied to files:
crates/cli/src/ciphernode/mod.rscrates/cli/src/ciphernode/context.rs
📚 Learning: 2024-10-28T12:00:09.010Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 156
File: packages/ciphernode/enclave/src/commands/wallet/set.rs:17-23
Timestamp: 2024-10-28T12:00:09.010Z
Learning: In the `enclave` package of the `ciphernode` project, prefer using `println!` over logging macros like `error!` from the `tracing` crate for error output in CLI commands.
Applied to files:
crates/cli/src/ciphernode/mod.rs
📚 Learning: 2024-10-28T12:04:26.578Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 156
File: packages/ciphernode/enclave_node/src/ciphernode.rs:26-28
Timestamp: 2024-10-28T12:04:26.578Z
Learning: In the `setup_ciphernode` function in `packages/ciphernode/enclave_node/src/ciphernode.rs`, panicking on errors during setup is acceptable.
Applied to files:
crates/cli/src/ciphernode/mod.rs
📚 Learning: 2024-11-05T06:48:58.177Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 173
File: packages/ciphernode/config/src/app_config.rs:13-21
Timestamp: 2024-11-05T06:48:58.177Z
Learning: In the `packages/ciphernode/config/src/app_config.rs` file, for the `Contract` enum, the team prefers to use `String` type for `address` fields, relying on parsing to handle validation, instead of using the `Address` type.
Applied to files:
crates/cli/src/ciphernode/context.rs
🧬 Code graph analysis (1)
crates/cli/src/ciphernode/mod.rs (4)
crates/cli/src/ciphernode/license.rs (1)
execute(14-72)crates/cli/src/ciphernode/tickets.rs (1)
execute(13-53)crates/cli/src/ciphernode/context.rs (1)
new(67-88)crates/cli/src/ciphernode/lifecycle.rs (5)
register(13-27)deregister(29-43)activate(45-47)deactivate(49-96)status(98-145)
⏰ 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). (9)
- GitHub Check: crisp_unit
- GitHub Check: build_sdk
- GitHub Check: rust_unit
- GitHub Check: rust_integration
- GitHub Check: integration_prebuild
- GitHub Check: test_contracts
- GitHub Check: build_enclave_cli
- GitHub Check: test_net
- GitHub Check: build_ciphernode_image
🔇 Additional comments (5)
crates/cli/src/ciphernode/license.rs (2)
14-69: License command dispatch looks consistent and correctly typedThe Bond/Unbond/Claim branches correctly resolve token addresses, fetch decimals, parse user amounts, and call the expected bonding methods, matching the patterns used in
tickets.rsandlifecycle.rs. Error propagation is clean and the user-facing messages are clear.
74-92: Bond flow correctly handles decimals, allowance, and bonding
bond_licenseresolves the license token, uses on-chain decimals withparse_amount, ensures allowance to the bonding registry before callingbondLicense, and prints a concise receipt. This is a solid, reusable pattern for future token flows.crates/cli/src/ciphernode/context.rs (1)
66-88: ChainContext::new is cohesive and keeps concerns well‑isolatedThe constructor cleanly handles chain selection, address parsing, key/cipher loading, signer derivation, and provider setup, then stores only what the CLI needs (label, registry address, provider, signer address). This keeps on‑chain wiring out of the command modules and should be easy to extend if the chain config grows.
crates/cli/src/ciphernode/mod.rs (2)
19-79: CLI command shapes and clap wiring align with the described UXThe
CiphernodeCommands,LicenseCommands, andTicketCommandsenums map cleanly onto the intended CLI (license,tickets,register,deregister,activate,deactivate,status), and the clap attributes (flattenforChainArgs,value_delimiterfor--proof, explicit--amountflags) should give a predictable UX for operators.If you haven’t already, a quick
enclave ciphernode --help/enclave ciphernode license --helppass to visually confirm the generated help text matches the docs would be worthwhile.
116-156: Dispatcher cleanly constructs per‑command ChainContext instancesThe top‑level
executefunction consistently builds aChainContextfromChainArgs::selection()and delegates to the appropriate module (license,tickets,lifecycle), keeping each branch simple and side‑effect free beyond the intended on‑chain call. The small duplication aroundChainContext::newis fine at this scale and keeps each arm easy to read.
hmzakhalid
changed the title
ctrlc03
left a comment
ctrlc03
left a comment
There was a problem hiding this comment.
utACK - just one question
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
crates/cli/src/ciphernode/context.rs (1)
67-80: Consider stricter validation and clearer errors for the bonding registry addressRight now
bonding_registryis only parsed, andparse_addressreports a generic"Invalid address in configuration"error. You might want to (a) treat the zero address as invalid and (b) add chain/field context to the error (bonding_registryfor chain X) so misconfigurations are easier to diagnose from the CLI.Also applies to: 138-154
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
crates/cli/src/ciphernode/context.rs(1 hunks)packages/enclave-contracts/.gitignore(1 hunks)packages/enclave-contracts/artifacts/contracts/interfaces/IBondingRegistry.sol/IBondingRegistry.json(3 hunks)packages/enclave-contracts/artifacts/contracts/interfaces/IEnclave.sol/IEnclave.json(1 hunks)packages/enclave-contracts/artifacts/contracts/token/EnclaveTicketToken.sol/EnclaveTicketToken.json(1 hunks)packages/enclave-contracts/contracts/interfaces/IBondingRegistry.sol(1 hunks)packages/enclave-contracts/contracts/registry/BondingRegistry.sol(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/enclave-contracts/artifacts/contracts/interfaces/IEnclave.sol/IEnclave.json
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 156
File: packages/ciphernode/enclave/src/commands/wallet/set.rs:17-23
Timestamp: 2024-10-28T12:00:09.010Z
Learning: In the `enclave` package of the `ciphernode` project, prefer using `println!` over logging macros like `error!` from the `tracing` crate for error output in CLI commands.
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 156
File: packages/ciphernode/keyshare/src/encryption.rs:45-45
Timestamp: 2024-10-23T02:03:02.008Z
Learning: In the `packages/ciphernode/keyshare/src/encryption.rs` file, the environment variable `CIPHERNODE_SECRET` is used for the encryption password. A secure secret management solution is not currently available, but may be considered in future iterations.
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 107
File: packages/ciphernode/sortition/src/distance.rs:1-1
Timestamp: 2024-10-08T19:45:18.209Z
Learning: In `packages/ciphernode/core/src/events.rs`, the import statements use the correct and updated `alloy::primitives` module.
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 107
File: packages/ciphernode/sortition/src/distance.rs:1-1
Timestamp: 2024-09-26T03:11:29.311Z
Learning: In `packages/ciphernode/core/src/events.rs`, the import statements use the correct and updated `alloy::primitives` module.
📚 Learning: 2025-09-19T11:16:53.825Z
Learnt from: cedoor
Repo: gnosisguild/enclave PR: 752
File: packages/enclave-contracts/contracts/Enclave.sol:15-17
Timestamp: 2025-09-19T11:16:53.825Z
Learning: The Enclave contract in the gnosisguild/enclave repository has not been deployed yet as of September 2025, so storage layout considerations for upgradeable contracts don't apply to current changes.
Applied to files:
packages/enclave-contracts/artifacts/contracts/token/EnclaveTicketToken.sol/EnclaveTicketToken.json
📚 Learning: 2024-11-05T06:48:58.177Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 173
File: packages/ciphernode/config/src/app_config.rs:13-21
Timestamp: 2024-11-05T06:48:58.177Z
Learning: In the `packages/ciphernode/config/src/app_config.rs` file, for the `Contract` enum, the team prefers to use `String` type for `address` fields, relying on parsing to handle validation, instead of using the `Address` type.
Applied to files:
crates/cli/src/ciphernode/context.rs
📚 Learning: 2024-10-12T10:24:07.572Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 139
File: packages/ciphernode/aggregator/src/publickey_aggregator.rs:46-46
Timestamp: 2024-10-12T10:24:07.572Z
Learning: In `packages/ciphernode/router/src/hooks.rs`, the `src_chain_id` parameter in `PublicKeyAggregator::new` is correctly handled, even if not explicitly provided during instantiation.
Applied to files:
crates/cli/src/ciphernode/context.rs
📚 Learning: 2024-10-08T19:45:18.209Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 107
File: packages/ciphernode/sortition/src/distance.rs:1-1
Timestamp: 2024-10-08T19:45:18.209Z
Learning: In `packages/ciphernode/core/src/events.rs`, the import statements use the correct and updated `alloy::primitives` module.
Applied to files:
crates/cli/src/ciphernode/context.rs
📚 Learning: 2024-10-22T03:39:29.448Z
Learnt from: ryardley
Repo: gnosisguild/enclave PR: 145
File: packages/ciphernode/evm/src/enclave_sol_reader.rs:87-89
Timestamp: 2024-10-22T03:39:29.448Z
Learning: In the `ciphernode` project, specifically in `packages/ciphernode/evm/src/enclave_sol_reader.rs`, the method `EnclaveSolReader::attach` should be retained even if it directly calls `EvmEventReader::attach` without additional processing. Avoid suggesting its removal in future reviews.
Applied to files:
crates/cli/src/ciphernode/context.rs
🔇 Additional comments (6)
packages/enclave-contracts/.gitignore (1)
11-11: LGTM!The unignore rules follow the established pattern for tracking generated contract artifacts. Lines 11, 15, and 19 correctly unignore the token directory, the EnclaveTicketToken.sol subdirectory, and the corresponding JSON artifact—mirroring the existing structure for interfaces and registry contracts.
Also applies to: 15-15, 19-19
packages/enclave-contracts/artifacts/contracts/token/EnclaveTicketToken.sol/EnclaveTicketToken.json (1)
1-1222: Artifact ABI and path look consistent with CLI usageThe EnclaveTicketToken artifact is well‑formed, exposes the expected methods (
underlying, deposit/mint/burn/payout), and its path matches the sol! import in the CLI; no issues from a contracts/CLI integration perspective.packages/enclave-contracts/contracts/interfaces/IBondingRegistry.sol (1)
116-127: New token address getters are consistent and non‑breakingAdding
getLicenseToken/getTicketTokenas view functions returningaddresscleanly complements the existing setters and doesn’t affect storage or existing callers; interface shape looks good.packages/enclave-contracts/contracts/registry/BondingRegistry.sol (1)
189-197: Getter implementations correctly expose token addresses without changing storage
getLicenseToken/getTicketTokenreuse existing public fields and only cast toaddress, so they’re upgrade‑safe and align with the IBondingRegistry interface and CLI needs.packages/enclave-contracts/artifacts/contracts/interfaces/IBondingRegistry.sol/IBondingRegistry.json (1)
362-374: ABI updates for token getters match the Solidity interface
getLicenseToken/getTicketTokenare correctly reflected as no‑arg view functions returningaddress, and the updatedbuildInfoIdis expected from recompilation; this artifact now lines up with IBondingRegistry.sol and the Rust bindings.Also applies to: 418-430, 880-880
crates/cli/src/ciphernode/context.rs (1)
19-57: ChainContext wiring and contract bindings look solidThe sol! bindings, provider setup, and helpers (
license_token_address,ticket_token_address,ticket_underlying_address,erc20) are cohesive and match the contract ABIs and config model (addresses as strings parsed intoalloy::primitives::Address). The artifact paths mirror the established pattern from the evm crate, so the CLI‑side contract access layer looks well‑integrated. Based on learnings, usingalloy::primitives::Addresshere is aligned with existing ciphernode conventions.Also applies to: 66-136
2 participants
enclave ciphernode status [--chain <name>]enclave ciphernode license bond --amount <ENCL>enclave ciphernode license unbond --amount <ENCL>enclave ciphernode license claim [--max-ticket <amount>] [--max-license <amount>]enclave ciphernode tickets buy --amount <stablecoin>enclave ciphernode tickets burn --amount <tickets>enclave ciphernode registerenclave ciphernode activateenclave ciphernode deactivate [--tickets <amount>] [--license <amount>]enclave ciphernode deregister --proof <node1,node2,...>Amounts accept decimals (
123.45) or visual separators (1_000). The CLI automatically fetchestoken decimals, normalizes the number, and sets ERC-20 allowances as needed before calling the
contracts.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.