feat: add register-contract script to rs-scripts#3744
Conversation
Adds a new `register-contract` bin under `rs-scripts` that takes a contract JSON file, an identity id, and a private key (WIF or hex), fetches the identity from Platform, picks the public key that matches the supplied private key (and satisfies the AUTHENTICATION + CRITICAL + ECDSA_SECP256K1 triple DPP requires for a contract-create signature), overrides the JSON's `id`/`ownerId` so fixture contracts under `packages/rs-drive/tests/supporting_files/contract/` register correctly, and broadcasts the `DataContractCreate` state transition via the SDK. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughA new Changesregister-contract CLI Binary
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/rs-scripts/src/bin/register_contract.rs (1)
59-64: 💤 Low valueConsider file-based private key input for improved security.
Passing the private key as a command-line argument exposes it in shell history and process listings. For improved operational security, consider supporting file-based input (e.g.,
--private-key-file) or reading from stdin when-is passed.The current approach is acceptable for development/scripting workflows, so this is not a blocker.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/rs-scripts/src/bin/register_contract.rs` around lines 59 - 64, Add a file/stdin option for supplying the private key to avoid exposing secrets on the command line: extend the CLI struct (the arg currently named private_key) to accept either a direct private_key or a new private_key_file: Option<String> (or change private_key to Option<String>) and add parsing logic where the args are processed (e.g., in main or the register handler) to prefer private_key_file if present; if private_key_file == "-" read from stdin, otherwise read the file contents, trim whitespace/newlines, and use that as the key; if no source provided, return a clear error. Ensure you reference the existing private_key symbol and replace usages with the resolved key value after loading from file/stdin.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/rs-scripts/src/bin/register_contract.rs`:
- Around line 59-64: Add a file/stdin option for supplying the private key to
avoid exposing secrets on the command line: extend the CLI struct (the arg
currently named private_key) to accept either a direct private_key or a new
private_key_file: Option<String> (or change private_key to Option<String>) and
add parsing logic where the args are processed (e.g., in main or the register
handler) to prefer private_key_file if present; if private_key_file == "-" read
from stdin, otherwise read the file contents, trim whitespace/newlines, and use
that as the key; if no source provided, return a clear error. Ensure you
reference the existing private_key symbol and replace usages with the resolved
key value after loading from file/stdin.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3dacc0df-1a50-497c-8845-79f60665071e
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
packages/rs-scripts/Cargo.tomlpackages/rs-scripts/README.mdpackages/rs-scripts/src/bin/register_contract.rs
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## v3.1-dev #3744 +/- ##
============================================
+ Coverage 87.16% 87.31% +0.15%
============================================
Files 2607 2590 -17
Lines 319420 316913 -2507
============================================
- Hits 278413 276721 -1692
+ Misses 41007 40192 -815
🚀 New features to boost your workflow:
|
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
The PR’s new register-contract binary is close to usable, but two in-scope correctness issues block it: one documented network mode can never succeed, and the contract JSON is decoded against the local latest protocol rules before the SDK has learned the target network’s real version. The private-key handling is also a real security regression for a tool that explicitly supports mainnet, although that is advisory rather than blocking.
🔴 2 blocking | 🟡 1 suggestion(s)
3 finding(s)
blocking: `regtest` is advertised as a supported network even though this command always aborts on regtest
packages/rs-scripts/src/bin/register_contract.rs (line 70)
This CLI accepts --network regtest in both the clap help and parse_network(), but run() always constructs a TrustedHttpContextProvider for the selected network. That provider rejects Network::Regtest unconditionally in packages/rs-sdk-trusted-context-provider/src/lib.rs:54-56, so every register-contract --network regtest ... invocation fails before any identity lookup or broadcast happens. Because the unsupported mode is introduced and documented by this PR, this is an in-scope user-visible bug that needs to be fixed either by removing regtest from the interface or by using a context provider that actually supports it.
blocking: The contract JSON is parsed with `PlatformVersion::latest()` before the SDK knows the target node’s protocol version
packages/rs-scripts/src/bin/register_contract.rs (line 107)
run() hard-codes let platform_version = PlatformVersion::latest(); and immediately feeds that into DataContract::from_json(...). In packages/rs-dpp/src/data_contract/conversion/json/mod.rs:19-29, that version decides whether the JSON is decoded as DataContractV0 or DataContractV1. The SDK built later is left in auto-detect mode, and its own documentation in packages/rs-sdk/src/sdk.rs:353-365 and 486-491 states that an auto-detect SDK falls back to PlatformVersion::latest() until a proof response has already been parsed successfully. That means this command always interprets the local contract file using the newest contract structure before it has learned the remote network version. This is a concrete compatibility bug, not just a theoretical one: packages/rs-platform-version/src/version/dpp_versions/dpp_contract_versions/v1.rs:15 uses contract_structure_version = 0, while .../v2.rs:15 switches it to 1. On an older network, otherwise valid contract JSON can therefore be rejected or misread before the CLI ever reaches the first request that could teach the SDK the correct version.
suggestion: The signing key is required on the command line, which exposes it through process listings and shell history
packages/rs-scripts/src/bin/register_contract.rs (line 59)
This binary requires the identity private key via --private-key / -k. On Unix-like systems, argv is routinely exposed through ps, /proc, shell history, terminal scrollback, and CI logs, so anyone who can observe the machine while the command runs can recover the key and act as that identity. Because this new CLI explicitly supports mainnet, secret entry should avoid argv exposure and use a safer channel such as an environment variable, stdin, or an interactive hidden prompt.
Inline posting hit GitHub HTTP 422, so I posted the same verified findings as a top-level review body.
2 participants
Issue being fixed or feature implemented
Need a quick CLI for registering data contracts on Platform from local JSON files. Useful for spinning up test contracts on devnet / testnet without writing one-off Rust glue code each time.
What was done?
Adds a new
register-contractbin underpackages/rs-scripts/:packages/rs-drive/tests/supporting_files/contract/), an identity id (base58), and a private key (WIF or 64-char hex).TrustedHttpContextProvider.AUTHENTICATION+CRITICAL+ECDSA_SECP256K1triple DPP requires on a contract-create signature. If a key matches the private key but doesn't meet the requirement, the error message lists every matched key (purpose / security level / key type / disabled) so the caller can see why nothing was usable.id/ownerIdbefore broadcasting —DataContractCreateTransition::new_from_data_contractregenerates the contract id deterministically from(identity_id, identity_nonce)and sets the owner id anyway, so fixture contracts with hard-coded ids register cleanly.PutContract::put_to_platform_and_wait_for_responseand prints the confirmedcontract_id/owner_id/version.Usage:
README.mdforrs-scriptsupdated with full usage docs.How Has This Been Tested?
cargo build -p rs-scripts --bin register-contract— clean.cargo clippy -p rs-scripts --bin register-contract -- -D warnings— clean.cargo fmt --all— clean.cargo run -p rs-scripts --bin register-contract -- --help— clap surface matches the README.No automated end-to-end run against a live network yet — the script needs a funded identity + a reachable DAPI endpoint. Reusing the SDK's
PutContractpath means it inherits the SDK's broadcast/wait coverage.Breaking Changes
None — new binary; no existing API changes.
Checklist:
Summary by CodeRabbit
New Features
Documentation