Skip to content

Conversation

@steinerkelvin
Copy link
Collaborator

@steinerkelvin steinerkelvin commented Oct 9, 2025

Summary

Added explicit rules to both CLAUDE.md and CONTRIBUTING.md prohibiting the use of Rust keywords (type, use, trait, impl, etc.) as struct field names, especially in public APIs.

Addresses: https://github.com/renlabs-dev/torus-substrate/pull/140/files#r2337166593

Problem

Rust keywords require the r# prefix (e.g., r#type), which causes serialization inconsistencies across language bindings:

  • Polkadot.js converts to r_type or rType
  • Serde serializes as type
  • Creates confusion and breaks API consistency

Solution

Use alternative names like kind, variant, scope, category, or mode instead. This guideline applies to all public structs, especially those in api crates and types exposed to external clients.

Changes

  • Added "API Design - MANDATORY" section to CLAUDE.md with strict rules for AI code generation
  • Added bullet point to "Pallet-specific guidelines" in CONTRIBUTING.md for human developers

Test plan

  • Documentation changes only, no code changes
  • Reviewed for clarity and consistency with existing style

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added a Torus command-line interface with commands for keys, balances (check/transfer), agents (info/register/unregister), namespaces (info/register/unregister), and staking (view/add/remove/transfer).
    • Supports fee estimation, dry-run, JSON output, testnet mode, and interactive confirmations.
    • Introduced fee estimation methods in the client for transactions.
    • Updated permission validation to allow zero-weight recipients when permissions are revocable.
  • Documentation

    • Expanded contributor guidelines with mandatory API design rules and Rust keyword guidance.
    • Added a CLI README with command references.
  • Chores

    • Added CLI to the workspace and updated the Rust toolchain.

Added explicit rules to both CLAUDE.md and CONTRIBUTING.md prohibiting the use of Rust keywords (type, use, trait, impl, etc.) as struct field names, especially in public APIs.

Problem: Rust keywords require the r# prefix (e.g., r#type), which causes serialization inconsistencies across language bindings:
- Polkadot.js converts to r_type or rType
- Serde serializes as type
- Creates confusion and breaks API consistency

Solution: Use alternative names like kind, variant, scope, category, or mode instead. This guideline applies to all public structs, especially those in api crates and types exposed to external clients.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings October 9, 2025 17:16
@coderabbitai
Copy link

coderabbitai bot commented Oct 9, 2025

Walkthrough

Adds a new CLI crate to the workspace with actions for agents, balances, keys, namespaces, and staking; introduces an action framework and key management with encryption; updates codegen to emit fee estimation methods; refactors tests to use named account helpers; makes minor logic/style tweaks across pallets and tooling; updates Rust toolchain.

Changes

Cohort / File(s) Summary
Docs
CLAUDE.md, CONTRIBUTING.md
Expanded guidelines on API design and Rust keyword usage; added/checklist notes; formatting adjustments.
Workspace & Tooling
Cargo.toml, rust-toolchain.toml, justfile
Added cli to workspace; bumped toolchain to 1.90.0; tightened Clippy for tests (-- -D clippy::all).
New CLI crate: config & docs
cli/Cargo.toml, cli/README.md
Added CLI crate manifest with dependencies/features; documented commands and flags.
CLI framework & entrypoint
cli/src/main.rs, cli/src/cli.rs, cli/src/action/mod.rs
Introduced async CLI runner, action dispatch, Action/ActionContext traits, JSON/dry-run/confirm flows.
CLI actions: agent
cli/src/action/agent.rs
Implemented agent info/register/unregister actions with fee estimation and prompts.
CLI actions: balance
cli/src/action/balance.rs
Implemented balance check and transfer with fee estimation and formatted output.
CLI actions: key management
cli/src/action/key.rs
Implemented list/create/delete/info for keys with prompts, mnemonic handling, and tables.
CLI actions: namespace
cli/src/action/namespace.rs
Implemented namespace info/register/unregister with fees and confirmations.
CLI actions: stake
cli/src/action/stake.rs
Implemented given/received/add/remove/transfer stake actions, fees, and summaries.
CLI support: keypairs/store/util
cli/src/keypair.rs, cli/src/store.rs, cli/src/util.rs
Added multi-crypto Keypair, encrypted key store with file ops, and amount formatting.
Client codegen
client/codegen/src/codegen/calls.rs, .../parser/{calls.rs,mod.rs,storage.rs}
Generated <call>_fee methods for signed calls; parser refactors combining conditions.
Test utilities
test-utils/src/lib.rs
Added accounts module, account! macro, and named account helpers (e.g., alice()).
Permission pallet tests
pallets/permission0/tests/{fixed.rs,lifetime.rs,namespace.rs,stream.rs,wallet.rs}
Switched to named accounts/macros; adjusted setups, balances, and some expected weights/errors.
Pallet logic tweaks
pallets/permission0/src/ext/stream_impl.rs, pallets/governance/src/{application.rs,proposal.rs}, pallets/emission0/src/distribute/math.rs
Small conditional changes (recipient weight rule, status update guard, is_multiple_of usage).
Torus staking tests
pallets/torus0/tests/stake.rs
Refactored to account-based helpers; updated flows and assertions accordingly.
Node/tooling minor
node/src/chain_spec.rs, mcp/src/main.rs, xtask/src/main.rs, project-selector/src/main.rs
Minor style/condition consolidations; allow dead code; adjusted Cargo invocation flags.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor User
  participant CLI as torus-cli
  participant Ctx as CliActionContext
  participant Act as Action<A>
  participant Net as TorusClient

  User->>CLI: Parse args (clap)
  CLI->>Ctx: Build context (json/testnet/dry-run)
  CLI->>Act: A::create(ctx, params)
  alt dry-run
    CLI->>Act: estimate_fee(ctx)
    CLI-->>User: Print DryRunResponse { fee }
  else execute
    CLI->>Act: confirmation_phrase(ctx)
    CLI->>Ctx: confirm(desc)
    CLI->>Act: execute(ctx)
    Act->>Net: network call(s) (mainnet/testnet)
    Act-->>CLI: ResponseData
    CLI-->>User: Output (JSON or Display)
  end
Loading
sequenceDiagram
  autonumber
  participant Gen as Codegen
  participant API as Generated API
  participant Signer as Signer<Keypair>
  participant Node as Substrate Node

  Gen->>API: Emit call(), wait(), and call_fee() for signed calls
  User->>API: call_fee(args..., signer)
  API->>Node: create_signed(call, signer).partial_fee_estimate()
  Node-->>API: u128 fee
  API-->>User: fee estimate
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~75 minutes

Possibly related PRs

Suggested reviewers

  • functor-flow
  • devwckd

Poem

In burrows of code, I twitch my nose,
A CLI sprouts where the toolkit grows.
Keys tucked safe, with secret seed,
Fees foretold before we proceed.
Named friends hop through every test—
Alice, Bob—our fluffy best.
Thump-thump: shipped, and well-pressed! 🐇✨

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Description Check ⚠️ Warning The pull request description does not follow the repository template because it omits the initial checklist section and the required “## Description” and “## Related Issues” headings, instead using custom headings like Summary, Problem, Solution, Changes, and Test plan. This mismatch prevents consistent structure across PRs. While the content is thorough, it fails to conform to the prescribed template. Update the description to include the checklist at the top, add a “## Description” section summarizing the changes, and include a “## Related Issues” heading linking relevant issues, moving or summarizing the existing content under these template headings.
Docstring Coverage ⚠️ Warning Docstring coverage is 44.12% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (1 passed)
Check name Status Explanation
Title Check ✅ Passed The title succinctly conveys that documentation guidelines are being added to avoid using Rust keywords as field names, directly reflecting the primary changes in CLAUDE.md and CONTRIBUTING.md. It follows the conventional “docs:” prefix and avoids unnecessary detail, making it clear and specific. Reviewers scanning the history will immediately understand the PR’s focus.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch docs/avoid-rust-keywords-in-api

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

coderabbitai[bot]

This comment was marked as off-topic.

@github-actions
Copy link

github-actions bot commented Oct 9, 2025

Code Coverage

Package Line Rate Health
pallets.permission0.src.ext 90%
pallets.governance.src 93%
pallets.permission0.src.permission 84%
pallets.torus0.src 96%
pallets.emission0.src.distribute 90%
pallets.emission0.src 92%
pallets.torus0.api.src 79%
pallets.permission0.api.src 100%
pallets.permission0.src 90%
Summary 91% (3511 / 3843)

@github-actions
Copy link

github-actions bot commented Oct 9, 2025

Detailed coverage report

@steinerkelvin steinerkelvin changed the base branch from main to dev October 9, 2025 17:27
@renlabs-dev renlabs-dev deleted a comment from coderabbitai bot Oct 9, 2025
@renlabs-dev renlabs-dev deleted a comment from coderabbitai bot Oct 9, 2025
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@saiintbrisson saiintbrisson merged commit 236928a into dev Oct 13, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants