Alpha deposit support in contract #164
Conversation
WalkthroughAdds alpha-collateral support across DB schema, persistence layer, Solidity contracts (V1/V2) and ABIs, Rust CLI/library, deployment and helper scripts, tests, and tooling; extends deposit/reclaim/slash call signatures to include alpha hot/cold keys and amounts and integrates staking/neuron interactions. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User / CLI
participant R as Rust lib / CLI
participant C as Collateral Proxy (V1/V2)
participant S as Staking (Alpha)
rect rgb(243,255,242)
U->>R: deposit(hotkey, nodeId, alphaHotkey, alphaAmount, value)
R->>C: deposit(...)
alt alphaAmount > 0
C->>S: transferAlpha(alphaHotkey, alphaAmount)
S-->>C: ok
end
C-->>R: emit Deposit(..., alphaHotkey, alphaAmount)
R-->>U: tx receipt
end
sequenceDiagram
autonumber
participant U as User / CLI
participant R as Rust lib
participant C as Collateral Proxy
participant S as Staking (Alpha)
U->>R: reclaimCollateral(hotkey, nodeId, alphaColdkey, url, md5)
R->>C: reclaimCollateral(...)
C-->>R: emit ReclaimProcessStarted(..., alphaColdkey, alphaAmount)
U->>R: finalizeReclaim(id)
R->>C: finalizeReclaim(id)
alt success and alphaAmount > 0
C->>S: withdrawAlpha(alphaColdkey, alphaAmount)
S-->>C: ok
C-->>R: emit Reclaimed(..., alphaColdkey, alphaAmount)
else deny or fail
C-->>R: emit Denied(...)
end
R-->>U: tx receipt
sequenceDiagram
autonumber
participant T as Trustee / CLI
participant R as Rust lib
participant C as Collateral Proxy
T->>R: slashCollateral(hotkey,nodeId,slashAmount,slashAlphaAmount,url,md5)
R->>C: slashCollateral(...)
C->>C: validate normal and alpha collateral availability
alt sufficient amounts
C-->>R: emit Slashed(..., slashAmount, slashAlphaAmount)
else insufficient
C-->>R: revert InsufficientCollateralForSlash
end
R-->>T: tx receipt / error
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
crates/basilica-validator/migrations/001_initial_schema.sql (1)
272-277: Do not retrofit the initial migration; add a forward migration foralpha_collateral.Editing
001_initial_schema.sqlwon’t touch databases that already ran this migration, so production nodes will still have acollateral_statustable withoutalpha_collateral. The updated persistence code now selects and updates that column, so those nodes will hitno such column: alpha_collateralat runtime. Please revert this change in001_initial_schema.sql, add a new migration that runsALTER TABLE collateral_status ADD COLUMN alpha_collateral TEXT NOT NULL DEFAULT '0';, and (if needed) backfill existing rows to'0'.crates/collateral-contract/README.md (1)
198-208: CLI docs diverge from actual flags
collateral-cli tx slash-collateralstill defines the flag as--amountinTxCommands::SlashCollateral(seesrc/main.rs, Line 128), so the newly documented--slash-amountwill be rejected. At the same time, the CLI now requires--alpha-hotkeyand--alpha-amounton deposits/slashes, but none of the usage examples include them. Please either update the Clap definitions to accept the documented names or adjust the README examples so users can run the commands without hitting missing/misnamed argument errors.
🧹 Nitpick comments (4)
crates/collateral-contract/src/CollateralUpgradableV2ABI.json (4)
515-525: Align naming: setContractColdkey vs alphaColdkey parameter.Function name suggests “contract coldkey” but parameter is alphaColdkey and events use AlphaColdkeyUpdated. Rename function to setAlphaColdkey (or rename param to contractColdkey) for ABI clarity.
- "name": "setContractColdkey", + "name": "setAlphaColdkey", "inputs": [ { - "name": "alphaColdkey", + "name": "alphaColdkey", "type": "bytes32", "internalType": "bytes32" } ],
207-215: Alpha identity asymmetry (hotkey on deposit, coldkey on reclaim). Please confirm.Deposit uses alphaHotkey; reclaim/events use alphaColdkey. If intentional (hotkey at deposit, coldkey at reclaim), document clearly in CLI and docs to avoid misuse; otherwise, harmonize input types.
Also applies to: 410-413, 780-790, 861-871, 928-938
753-791: Event indexing trade-off: alpha not indexed in Deposit.All three indexed slots are used (hotkey, nodeId, miner). If alpha-centric analytics are needed, consider an additional event (e.g., AlphaDeposit) with alphaHotkey indexed, or swapping one indexed field, before ABI freeze.
3-14: Upgradeable constructor/receive/fallback: sanity check.Confirm constructor disables initializers (_disableInitializers) and that receive/fallback won’t trap funds unintentionally (e.g., ETH sent without deposit path). If ETH is only accepted via deposit, consider guarding receive/fallback or emitting an event.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (24)
Cargo.toml(2 hunks)crates/basilica-miner/tests/mod.rs(4 hunks)crates/basilica-validator/migrations/001_initial_schema.sql(1 hunks)crates/basilica-validator/src/persistence/collateral_persistence.rs(14 hunks)crates/collateral-contract/Cargo.toml(1 hunks)crates/collateral-contract/README.md(2 hunks)crates/collateral-contract/deploy.sh(1 hunks)crates/collateral-contract/flow.sh(3 hunks)crates/collateral-contract/query.sh(1 hunks)crates/collateral-contract/run_unit_test.sh(1 hunks)crates/collateral-contract/script/DeployUpgradeable.s.sol(3 hunks)crates/collateral-contract/src/Collateral.sol(1 hunks)crates/collateral-contract/src/CollateralUpgradableABI.json(15 hunks)crates/collateral-contract/src/CollateralUpgradableV2ABI.json(1 hunks)crates/collateral-contract/src/CollateralUpgradeable.sol(18 hunks)crates/collateral-contract/src/CollateralUpgradeableV2.sol(14 hunks)crates/collateral-contract/src/lib.rs(11 hunks)crates/collateral-contract/src/main.rs(18 hunks)crates/collateral-contract/src/tests.rs(2 hunks)crates/collateral-contract/test/CollateralBasic.t.sol(1 hunks)crates/collateral-contract/test/CollateralUpgradeable.t.sol(8 hunks)crates/collateral-contract/test/IStakingIntegration.t.sol(1 hunks)crates/collateral-contract/update_abi.py(1 hunks)docs/collateral-contract.md(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
crates/collateral-contract/flow.sh (2)
crates/collateral-contract/src/lib.rs (1)
deposit(184-207)crates/basilica-validator/src/persistence/collateral_persistence.rs (4)
deposit(66-66)deposit(67-67)deposit(91-91)deposit(92-92)
crates/collateral-contract/src/main.rs (1)
crates/collateral-contract/src/lib.rs (6)
get_version(315-322)contract_coldkey(324-333)contract_hotkey(335-344)alpha_collaterals(425-442)reclaims(443-460)deposit(184-207)
crates/basilica-validator/src/persistence/collateral_persistence.rs (2)
crates/collateral-contract/src/lib.rs (2)
deposit(184-207)from(38-54)crates/basilica-validator/src/persistence/simple_persistence.rs (1)
for_testing(25-42)
crates/collateral-contract/src/tests.rs (1)
crates/collateral-contract/src/lib.rs (9)
netuid(346-353)trustee(355-362)decision_timeout(364-373)from(38-54)contract_hotkey(335-344)contract_coldkey(324-333)collaterals(406-423)alpha_collaterals(425-442)min_collateral_increase(375-384)
🪛 Gitleaks (8.28.0)
crates/collateral-contract/deploy.sh
[high] 10-10: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
crates/collateral-contract/run_unit_test.sh
[high] 5-5: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
crates/collateral-contract/src/tests.rs
[high] 44-44: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🪛 Shellcheck (0.11.0)
crates/collateral-contract/run_unit_test.sh
[warning] 22-22: flags appears unused. Verify use (or export if used externally).
(SC2034)
🔇 Additional comments (6)
crates/collateral-contract/Cargo.toml (1)
37-38: Dev deps wired up correctlyPointing the contract crate’s tests to the shared
blake2andsp-ioworkspace crates keeps versions centralized—looks good.Cargo.toml (1)
75-96: Workspace crypto deps in syncAdding
blake2 = 0.10andsp-io = 34.0.0alongside the existing Substrate stack keeps the workspace aligned—no concerns.crates/collateral-contract/src/tests.rs (1)
44-44: Static analysis false positive: test key exposure.The static analysis tool flagged this line as potentially exposing a secret. However, this is the well-known Alith development account private key used for local testing with Substrate-based chains. It's documented in Substrate/Polkadot development guides and is safe to use in test code.
crates/collateral-contract/src/CollateralUpgradableABI.json (1)
1-1287: LGTM: ABI properly reflects alpha collateral support.The ABI has been correctly updated to include:
- New functions:
alphaCollaterals,burnRegister,getContractStake,setContractColdkey,CONTRACT_COLDKEY,CONTRACT_HOTKEY- Extended parameters:
deposit,reclaimCollateral,slashCollateralnow include alpha-related fields- New events:
AlphaColdkeyUpdated, and extendedDeposit,ReclaimProcessStarted,Reclaimed,Slashedevents with alpha data- New errors:
InsufficientCollateralForSlash,InvalidAlphaColdkeyThe type annotations and parameter ordering are consistent throughout.
crates/collateral-contract/src/CollateralUpgradableV2ABI.json (2)
652-668: Verify UUPS gating and role checks on upgradeToAndCall in implementation.ABI looks correct (payable). Ensure onlyProxy/onlyDelegateCall and UPGRADER_ROLE are enforced to prevent direct implementation calls.
267-276: Versioning consistency.getVersion (uint256, pure) and ContractUpgraded.newVersion (uint256) align. Ensure UPGRADE_INTERFACE_VERSION string reflects OZ expectations and that Initialized event signature matches tooling.
Also applies to: 690-706
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
crates/collateral-contract/src/main.rs (4)
239-268: Remove duplicate code and consolidate output.Lines 248-252 contain duplicate parsing and printing logic. The
node_uuidis parsed twice (lines 244 and 252), and there are two differentprintln!statements (lines 248-251 and 254-257) that will both execute, causing confusing output.Apply this diff to remove the duplicates:
let alpha_hotkey_bytes = parse_hotkey(&alpha_hotkey)?; let alpha_amount_u256 = parse_u256(&alpha_amount)?; - - println!( - "Depositing {} in wei for node {} with hotkey {} and {} for alpha hotkey {}", - amount, node_uuid, hotkey, alpha_amount, alpha_hotkey, - ); - let node_uuid = Uuid::parse_str(&node_id)?; println!( - "Depositing {} wei for node {} with hotkey {}", - amount, node_id, hotkey + "Depositing {} wei for node {} with hotkey {} and {} wei alpha collateral with alpha hotkey {}", + amount, node_id, hotkey, alpha_amount, alpha_hotkey );
329-365: Remove duplicate println statement.Lines 344-347 and 349-352 both execute and print similar messages, causing duplicate and confusing output. Also, ensure variable naming aligns with the corrected CLI argument names.
Apply this diff to remove the duplicate:
let amount_u256 = parse_u256(&amount)?; let alpha_amount_u256 = parse_u256(&alpha_amount)?; - - println!( - "Slashing all collateral for executor {} with hotkey {}", - node_uuid, hotkey - ); println!( - "Slashing collateral for node {} with hotkey {}", - node_id, hotkey + "Slashing {} wei and {} wei alpha collateral for node {} with hotkey {}", + amount, alpha_amount, node_id, hotkey );Note: If the earlier suggestion to rename CLI args to
slash_amountandslash_alpha_amountis applied, update the variable names here accordingly.
375-427: Remove duplicate println statement in Collaterals query.Lines 425-426 both execute and print the same result with different terminology ("executor" vs "node"), causing duplicate output.
Apply this diff to remove the duplicate:
) .await?; - println!("Collateral for executor {}: {} in wei", node_uuid, result); println!("Collateral for node {}: {} wei", node_id_clone, result); }
601-612: Use consistent field names in JSON output for Slashed events.The JSON output uses generic field names
amountandalphaAmount(lines 607-608) instead ofslashAmountandslashAlphaAmountto match the event structure and differentiate from deposit amounts.Apply this diff to align naming:
serde_json::json!({ "type": "Slashed", "hotkey": hex::encode(slashed.hotkey.as_slice()), "nodeId": hex::encode(slashed.nodeId.as_slice()), "miner": slashed.miner.to_string(), - "amount": slashed.slashAmount.to_string(), - "alphaAmount": slashed.slashAlphaAmount.to_string(), + "slashAmount": slashed.slashAmount.to_string(), + "slashAlphaAmount": slashed.slashAlphaAmount.to_string(), "url": slashed.url, "urlContentMd5Checksum": hex::encode(slashed.urlContentMd5Checksum.as_slice()) })crates/collateral-contract/src/CollateralUpgradeableV2.sol (2)
339-343: Fix finalizeReclaim validation to check both collateral types.The validation only checks
reclaim.amount == 0, but should also verifyreclaim.alphaAmount == 0. If a reclaim request has only alpha collateral (amount=0, alphaAmount>0), this incorrectly reverts withReclaimNotFound. V1 correctly checks both at line 338.Apply this diff:
- if (reclaim.amount == 0) { + if (reclaim.amount == 0 && reclaim.alphaAmount == 0) { revert ReclaimNotFound(); }
352-371: Critical: Missing alphaCollateralUnderPendingReclaims decrement in finalizeReclaim.After deleting the reclaim at line 353,
collateralUnderPendingReclaimsis decremented (line 354) butalphaCollateralUnderPendingReclaimsis never decremented. This breaks accounting for pending alpha reclaims. ThedenyReclaimRequestfunction correctly decrements both (lines 415-417), sofinalizeReclaimmust do the same.Apply this diff:
delete reclaims[reclaimRequestId]; collateralUnderPendingReclaims[hotkey][nodeId] -= amount; + alphaCollateralUnderPendingReclaims[hotkey][nodeId] -= alphaAmount;crates/collateral-contract/src/CollateralUpgradeable.sol (1)
352-371: Critical: Missing alphaCollateralUnderPendingReclaims decrement in finalizeReclaim.After deleting the reclaim at line 352,
collateralUnderPendingReclaimsis decremented (line 353) butalphaCollateralUnderPendingReclaimsis never decremented. This breaks accounting for pending alpha reclaims. ThedenyReclaimRequestfunction correctly decrements both (lines 418-420), sofinalizeReclaimmust do the same. This issue also exists in V2.Apply this diff:
delete reclaims[reclaimRequestId]; collateralUnderPendingReclaims[hotkey][nodeId] -= amount; + alphaCollateralUnderPendingReclaims[hotkey][nodeId] -= alphaAmount;
♻️ Duplicate comments (1)
crates/collateral-contract/src/CollateralUpgradeableV2.sol (1)
448-450: Fix slash validation logic to prevent partial over-slashing.The validation uses
&&(AND), allowing slash operations that exceed available collateral for one type as long as the other has sufficient collateral. For example, ifamount=100,alphaAmount=100,slashAmount=200,slashAlphaAmount=50, this check passes even though TAO is over-slashed. V1 correctly uses||at line 451.Apply this diff:
- if (slashAmount > amount && slashAlphaAmount > alphaAmount) { + if (slashAmount > amount || slashAlphaAmount > alphaAmount) { revert InsufficientCollateralForSlash(); }
🧹 Nitpick comments (1)
crates/collateral-contract/src/main.rs (1)
63-68: Clarify help text for alpha-collateral fields.The help text for
alpha_hotkeyandalpha_amountis identical to the regularhotkeyandamountfields. Users may not understand what "alpha" collateral represents or how it differs from the primary collateral.Apply this diff to improve the help text:
- /// Hotkey as hex string (32 bytes) + /// Alpha hotkey as hex string (32 bytes) for alpha collateral #[arg(long)] alpha_hotkey: String, - /// Amount to deposit in wei + /// Alpha collateral amount to deposit in wei #[arg(long)] alpha_amount: String,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
crates/collateral-contract/src/CollateralUpgradeable.sol(18 hunks)crates/collateral-contract/src/CollateralUpgradeableV2.sol(14 hunks)crates/collateral-contract/src/main.rs(18 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
crates/collateral-contract/src/main.rs (1)
crates/collateral-contract/src/lib.rs (6)
get_version(315-322)contract_coldkey(324-333)contract_hotkey(335-344)alpha_collaterals(425-442)reclaims(443-460)deposit(184-207)
🔇 Additional comments (5)
crates/collateral-contract/src/main.rs (3)
143-182: LGTM!The new query commands are well-structured with appropriate help text.
270-298: LGTM!The alpha_coldkey field is correctly parsed and passed to the reclaim_collateral function.
552-564: LGTM!The event field names
slashAmountandslashAlphaAmountare correctly displayed in the pretty-print output.crates/collateral-contract/src/CollateralUpgradeable.sol (1)
451-453: LGTM: Slash validation correctly uses OR logic.The validation correctly uses
||(OR) to revert when eitherslashAmount > amountorslashAlphaAmount > alphaAmount, preventing partial over-slashing. This is the correct implementation. V2 incorrectly uses&&at line 448 and needs to be fixed to match this logic.crates/collateral-contract/src/CollateralUpgradeableV2.sol (1)
9-29: Mark IStaking.transferStake and moveStake as payable
In CollateralUpgradeableV2.sol’s IStaking interface, change both methods toexternal payableto mirror V1 and support ETH transfers.
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
crates/collateral-contract/src/main.rs (2)
263-272: Remove duplicate code and consolidate logging.There are two issues here:
- Line 267 duplicates the UUID parsing already done at line 259
- Lines 263-266 and 269-272 both print deposit information, causing duplicate/confusing output
Apply this diff to fix both issues:
let alpha_amount_u256 = parse_u256(&alpha_amount)?; println!( "Depositing {} in wei for node {} with hotkey {} and {} for alpha hotkey {}", amount, node_uuid, hotkey, alpha_amount, alpha_hotkey, ); - let node_uuid = Uuid::parse_str(&node_id)?; - - println!( - "Depositing {} wei for node {} with hotkey {}", - amount, node_id, hotkey - ); collateral_contract::deposit(
359-367: Consolidate duplicate logging statements.Lines 359-362 and 364-367 both print similar information about slashing collateral, using inconsistent terminology ("executor" vs "node"). This creates duplicate and confusing output.
Apply this diff to remove the duplicate:
let alpha_amount_u256 = parse_u256(&alpha_amount)?; - println!( - "Slashing all collateral for executor {} with hotkey {}", - node_uuid, hotkey - ); - println!( "Slashing collateral for node {} with hotkey {}", node_id, hotkeycrates/collateral-contract/src/CollateralUpgradeableV2.sol (2)
340-386: Fix alpha-only finalize and accounting; use actual balances to clear nodeCurrent logic rejects alpha-only reclaims, doesn’t release alpha pending, doesn’t decrement alphaCollaterals, and clears nodeToMiner based on reclaim alphaAmount.
- if (reclaim.amount == 0) { + if (reclaim.amount == 0 && reclaim.alphaAmount == 0) { revert ReclaimNotFound(); } @@ - address miner = reclaim.miner; - uint256 amount = reclaim.amount; + address miner = reclaim.miner; + uint256 amount = reclaim.amount; + bytes32 alphaColdkey = reclaim.alphaColdkey; + uint256 alphaAmount = reclaim.alphaAmount; @@ - collateralUnderPendingReclaims[hotkey][nodeId] -= amount; + collateralUnderPendingReclaims[hotkey][nodeId] -= amount; + alphaCollateralUnderPendingReclaims[hotkey][nodeId] -= alphaAmount; @@ - if (reclaim.alphaAmount > 0) { - withdrawAlpha(reclaim.alphaColdkey, reclaim.alphaAmount); + if (alphaAmount > 0) { + alphaCollaterals[hotkey][nodeId] -= alphaAmount; + withdrawAlpha(alphaColdkey, alphaAmount); } @@ - if (collaterals[hotkey][nodeId] == 0 && reclaim.alphaAmount == 0) { + if ( + collaterals[hotkey][nodeId] == 0 && + alphaCollaterals[hotkey][nodeId] == 0 + ) { nodeToMiner[hotkey][nodeId] = address(0); } @@ - amount, - reclaim.alphaColdkey, - reclaim.alphaAmount + amount, + alphaColdkey, + alphaAmount );
399-421: Allow denying alpha-only reclaims and free alpha pending on denyCheck both amounts; you already free alpha pending—good. Adjust not-found condition.
- if (reclaim.amount == 0) { + if (reclaim.amount == 0 && reclaim.alphaAmount == 0) { revert ReclaimNotFound(); }crates/collateral-contract/src/CollateralUpgradeable.sol (2)
352-371: Release alpha pending on finalizealphaCollateralUnderPendingReclaims must be decremented to avoid blocking future reclaims.
- collateralUnderPendingReclaims[hotkey][nodeId] -= amount; + collateralUnderPendingReclaims[hotkey][nodeId] -= amount; + alphaCollateralUnderPendingReclaims[hotkey][nodeId] -= alphaAmount;
407-421: Allow denying alpha-only reclaimsCheck both amounts when validating existence.
- if (reclaim.amount == 0) { + if (reclaim.amount == 0 && reclaim.alphaAmount == 0) { revert ReclaimNotFound(); }crates/collateral-contract/README.md (1)
192-210: Align CLI flags in README Replace--slash-amountwith--amountand include--alpha-amountin both examples to match the actual CLI flags (amountandalpha_amount). crates/collateral-contract/README.md:192–210
♻️ Duplicate comments (6)
crates/collateral-contract/src/main.rs (3)
475-482: Remove duplicate println statements (unaddressed past review).As flagged in a previous review, lines 475-478 and 479-482 both print the same alpha collateral result with different labels ("executor" vs "node"), causing duplicate output.
Apply this diff to remove the duplicate:
) .await?; - println!( - "Alpha collateral for executor {}: {} in wei", - node_uuid, result - ); println!( "Alpha collateral for node {}: {} in wei", node_id_clone, result
81-83: Fix incorrect help text for alpha_coldkey (unaddressed past review).Despite being marked as addressed in commit ccbd4ad, the help text still incorrectly says "Hotkey" when the field is
alpha_coldkey.Apply this diff to correct the help text:
- /// Hotkey as hex string (32 bytes) + /// Alpha coldkey as hex string (32 bytes) #[arg(long)] alpha_coldkey: String,
124-132: Address past review feedback on parameter naming (unaddressed).The previous review requested renaming
amounttoslash_amountand improving the help text foralpha_amount. This remains unaddressed and affects CLI clarity.Apply this diff to address both issues:
- /// Amount to slash in wei - #[arg(long)] - amount: String, - /// Amount to slash alpha in wei - #[arg(long)] - alpha_amount: String, + /// Amount to slash in wei + #[arg(long = "slash-amount")] + slash_amount: String, + /// Alpha collateral amount to slash in wei + #[arg(long = "slash-alpha-amount")] + slash_alpha_amount: String,Then update all references to
amountandalpha_amountin the SlashCollateral handler (lines 344-380) to useslash_amountandslash_alpha_amount.crates/collateral-contract/src/CollateralUpgradeable.sol (1)
9-29: IStaking functions shouldn’t be payable per precompile ABItransferStake/moveStake are nonpayable in StakingPrecompileV2; drop payable for consistency.
- ) external payable; + ) external; @@ - ) external payable; + ) external;Confirm StakingPrecompileV2 ABI mutability for transferStake and moveStake (nonpayable vs payable)?crates/collateral-contract/flow.sh (1)
7-12: Don’t hard-code secrets; require env-provided PRIVATE_KEYSwitch to env enforcement to avoid accidental key leaks and ensure script fails fast if not set.
-export HOTKEY=0x0000000000000000000000000000000000000000000000000000000000000001 -export ALPHA_HOTKEY=0x0000000000000000000000000000000000000000000000000000000000000002 -export CONTRACT_COLDKEY=0x0000000000000000000000000000000000000000000000000000000000000003 +export HOTKEY=${HOTKEY:-0x0000000000000000000000000000000000000000000000000000000000000001} +export ALPHA_HOTKEY=${ALPHA_HOTKEY:-0x0000000000000000000000000000000000000000000000000000000000000002} +export CONTRACT_COLDKEY=${CONTRACT_COLDKEY:-0x0000000000000000000000000000000000000000000000000000000000000003} export NODE_ID=6339ba4f-60f9-45c2-9d95-2b755bb57ca6 -export PRIVATE_KEY=0x +: "${PRIVATE_KEY:?set PRIVATE_KEY in your environment before running}"crates/collateral-contract/src/CollateralUpgradeableV2.sol (1)
448-450: Prevent partial over-slashing (use OR, not AND)Replace && with || to forbid exceeding either balance.
- if (slashAmount > amount && slashAlphaAmount > alphaAmount) { + if (slashAmount > amount || slashAlphaAmount > alphaAmount) { revert InsufficientCollateralForSlash(); }
🧹 Nitpick comments (6)
crates/collateral-contract/src/main.rs (1)
63-68: Clarify help text for alpha parameters.The help text for
alpha_hotkeyandalpha_amountis identical to the regularhotkeyandamountparameters. This makes it unclear what these alpha-specific parameters represent and how they differ from the regular parameters.Apply this diff to improve the help text:
- /// Hotkey as hex string (32 bytes) + /// Alpha hotkey as hex string (32 bytes) #[arg(long)] alpha_hotkey: String, - /// Amount to deposit in wei + /// Alpha collateral amount to deposit in wei #[arg(long)] alpha_amount: String,crates/collateral-contract/src/lib.rs (1)
209-232: Consider consolidating duplicate deposit functions.The
deposit_with_configfunction (lines 209-232) is identical to thedepositfunction (lines 184-207). This duplication increases maintenance burden and the risk of divergence.Consider removing
deposit_with_configand usingdepositeverywhere, or clarify the distinction if there's a meaningful difference not evident in the current implementation.docs/collateral-contract.md (4)
15-21: Expand “Main adaptations” to explicitly describe alpha flowsSpell out alpha fields and rules to reduce ambiguity.
- Support the Alpha in subnet 39 as collateral + - Alpha collateral support: + - New fields across flows: alphaHotkey (deposit), alphaAmount (deposit/slash), alphaColdkey (reclaim/finalize). + - CLI flags: --alpha-hotkey, --alpha-amount, --alpha-coldkey; queries: alpha-collaterals. + - Accounting: alphaCollaterals mirrors collaterals; nodeToMiner is cleared only when both are zero. + - Validation: reclaim requires alphaColdkey when reclaiming alpha; slashing validates each amount independently.
111-122: Grammar/clarity fixes for owner stepsMinor edits improve readability.
-**Owner set the contract coldkey** - - The owner get the contract address of deployed proxy contract - - Convert the contract address to substrate account public key - - The owner call **setContractColdkey** to set the coldkey value in contract +**Owner sets the contract coldkey** + - The owner gets the deployed proxy contract address. + - Convert the contract address to a Substrate account public key. + - The owner calls **setContractColdkey** to set the coldkey in the contract. -**Owner call burned register to do neuron registration** - - The owner **call burnedRegister** to register neuron with configured hotkey and coldkey in contract - - After successful registration, the proxy will be the owner (as coldkey) of hotkey in bittensor network - - Later, contract can do **transferStake** via precompile +**Owner performs burnedRegister to register the neuron** + - The owner calls **burnedRegister** with the configured hotkey and coldkey stored in the contract. + - After success, the proxy becomes the owner (as coldkey) of the hotkey in the Bittensor network. + - Later, the contract can invoke **transferStake** via the staking precompile.
171-187: Unify variable name NODE_ID (case) across examplesUse NODE_ID consistently to avoid copy/paste errors.
-export node_ID=6339ba4f-60f9-45c2-9d95-2b755bb57ca6 +export NODE_ID=6339ba4f-60f9-45c2-9d95-2b755bb57ca6 @@ - --node-id "$node_ID" + --node-id "$NODE_ID" @@ - --node-id "$node_ID" + --node-id "$NODE_ID" @@ - --node-id "$node_ID" + --node-id "$NODE_ID"Also applies to: 197-211, 214-216
189-191: Add alpha-collateral query note after depositYou already include it below; consider moving the alpha-collaterals query note up with the standard collaterals check to keep the flow together.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
crates/collateral-contract/README.md(2 hunks)crates/collateral-contract/flow.sh(3 hunks)crates/collateral-contract/src/CollateralUpgradeable.sol(18 hunks)crates/collateral-contract/src/CollateralUpgradeableV2.sol(14 hunks)crates/collateral-contract/src/lib.rs(11 hunks)crates/collateral-contract/src/main.rs(17 hunks)docs/collateral-contract.md(8 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
crates/collateral-contract/src/main.rs (2)
crates/collateral-contract/src/lib.rs (8)
set_contract_coldkey(314-325)burn_register(327-337)get_version(340-347)contract_coldkey(349-358)contract_hotkey(360-369)alpha_collaterals(450-467)reclaims(468-485)deposit(184-207)crates/collateral-contract/src/tests.rs (1)
burn_register(806-835)
crates/collateral-contract/flow.sh (1)
crates/collateral-contract/src/lib.rs (1)
deposit(184-207)
crates/collateral-contract/src/lib.rs (1)
crates/collateral-contract/src/tests.rs (1)
burn_register(806-835)
🪛 LanguageTool
docs/collateral-contract.md
[style] ~50-~50: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...llateral into the validator's contract. Miners can now specify a GPU node UUID (re...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[grammar] ~111-~111: Ensure spelling is correct
Context: ...ed amounts. - Owner set the contract coldkey - The owner get the contract address of de...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~113-~113: There might be a mistake here.
Context: ... - The owner get the contract address of deployed proxy contract - Convert the...
(QB_NEW_EN)
[grammar] ~114-~114: There might be a mistake here.
Context: ...ntract - Convert the contract address to substrate account public key - The ow...
(QB_NEW_EN)
[grammar] ~115-~115: There might be a mistake here.
Context: ...tractColdkey** to set the coldkey value in contract - **Owner call burned registe...
(QB_NEW_EN)
[grammar] ~119-~119: There might be a mistake here.
Context: ... - The owner call burnedRegister to register neuron with configured hotkey and coldk...
(QB_NEW_EN)
[grammar] ~119-~119: There might be a mistake here.
Context: ...all burnedRegister** to register neuron with configured hotkey and coldkey in contra...
(QB_NEW_EN)
[grammar] ~119-~119: There might be a mistake here.
Context: ...uron with configured hotkey and coldkey in contract - After successful registrat...
(QB_NEW_EN)
[grammar] ~120-~120: There might be a mistake here.
Context: ...istration, the proxy will be the owner (as coldkey) of hotkey in bittensor network...
(QB_NEW_EN)
[grammar] ~120-~120: There might be a mistake here.
Context: ...he proxy will be the owner (as coldkey) of hotkey in bittensor network - Later, ...
(QB_NEW_EN)
[grammar] ~121-~121: There might be a mistake here.
Context: ...of hotkey in bittensor network - Later, contract can do transferStake via p...
(QB_NEW_EN)
⏰ 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). (10)
- GitHub Check: lint-complex
- GitHub Check: build-validator (stable)
- GitHub Check: test-python-sdk (3.12)
- GitHub Check: build-payments (stable)
- GitHub Check: test-python-sdk (3.10)
- GitHub Check: build-cli (stable)
- GitHub Check: test-python-sdk (3.11)
- GitHub Check: build-billing (stable)
- GitHub Check: build-api (stable)
- GitHub Check: build-miner (stable)
🔇 Additional comments (11)
crates/collateral-contract/src/main.rs (2)
573-595: LGTM!The event printing logic correctly includes the new alpha-related fields with consistent formatting.
621-646: LGTM!The JSON serialization correctly includes all alpha-related fields with appropriate naming conventions.
crates/collateral-contract/src/lib.rs (6)
184-207: LGTM!The
depositfunction correctly accepts and passes alpha_hotkey and alpha_amount parameters to the contract, consistent with the contract ABI.
234-255: LGTM!The
reclaim_collateralfunction correctly accepts and passes the alpha_coldkey parameter to the contract.
289-312: LGTM!The
slash_collateralfunction correctly usesslash_amountandslash_alpha_amountparameter names, which are clearer than genericamount/alpha_amount. This aligns with the recommended naming convention.
314-337: LGTM!The new transaction functions
set_contract_coldkeyandburn_registerfollow the established pattern and are implemented correctly.
340-369: LGTM!The new query functions
get_version,contract_coldkey, andcontract_hotkeyare implemented correctly and follow the established pattern for read-only contract queries.
450-467: LGTM!The
alpha_collateralsfunction is implemented correctly and consistently with other query functions. It properly retrieves alpha collateral balances for a given hotkey and node ID.crates/collateral-contract/flow.sh (3)
12-20: Alpha coldkey flow LGTMSetting contract coldkey and burn-register steps are correctly sequenced.
22-29: Deposit flow with alpha looks goodThe new --alpha-hotkey/--alpha-amount path and alpha-collaterals query checks are consistent.
83-85: SlashCollateral flags match CLI definitions
flow.sh’s use of--amountand--alpha-amountaligns with theSlashCollateralcommand’s#[arg(long)] amountandalpha_amountinmain.rs. No changes required.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
crates/collateral-contract/src/lib.rs (1)
209-232: Eliminate code duplication.The
deposit_with_configfunction is identical to thedepositfunction (lines 184-207). This duplication violates the DRY principle and increases maintenance burden.Consider one of these approaches:
- If the distinction is no longer needed, remove
deposit_with_configand update all call sites to usedeposit.- If future differentiation is planned, add a comment explaining the intent.
- If both are genuinely needed for API compatibility, have one delegate to the other:
pub async fn deposit_with_config( private_key: &str, hotkey: [u8; 32], node_id: [u8; 16], amount: U256, alpha_hotkey: [u8; 32], alpha_amount: U256, network_config: &CollateralNetworkConfig, ) -> Result<(), anyhow::Error> { - let contract = get_collateral(private_key, network_config).await?; - - let tx = contract - .deposit( - FixedBytes::from_slice(&hotkey), - FixedBytes::from_slice(&node_id), - FixedBytes::from_slice(&alpha_hotkey), - alpha_amount, - ) - .value(amount); - let tx = tx.send().await?; - let receipt = tx.get_receipt().await?; - tracing::info!("{receipt:?}"); - Ok(()) + deposit(private_key, hotkey, node_id, amount, alpha_hotkey, alpha_amount, network_config).await }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
crates/collateral-contract/src/lib.rs(11 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
crates/collateral-contract/src/lib.rs (1)
crates/collateral-contract/src/tests.rs (1)
burn_register(806-835)
⏰ 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). (8)
- GitHub Check: lint-complex
- GitHub Check: test-python-sdk (3.13)
- GitHub Check: build-payments (stable)
- GitHub Check: build-validator (stable)
- GitHub Check: build-miner (stable)
- GitHub Check: build-billing (stable)
- GitHub Check: build-cli (stable)
- GitHub Check: build-api (stable)
🔇 Additional comments (5)
crates/collateral-contract/src/lib.rs (5)
184-207: LGTM!The addition of
alpha_hotkeyandalpha_amountparameters to thedepositfunction correctly extends the API to support alpha collateral deposits, consistent with the PR objectives. The parameters are properly passed to the underlying contract call.
234-255: LGTM!The
alpha_coldkeyparameter addition toreclaim_collateralcorrectly extends reclaim functionality to handle alpha collateral, consistent with the PR objectives.
289-313: LGTM!The addition of separate
slash_amountandslash_alpha_amountparameters appropriately extends slashing to support both collateral types. The#[allow(clippy::too_many_arguments)]attribute is justified given the function's role as a smart contract wrapper that must pass all required parameters.
315-370: LGTM!The new functions (
set_contract_coldkey,burn_register,get_version,contract_coldkey,contract_hotkey) are well-implemented:
- Write operations (
set_contract_coldkey,burn_register) correctly use authenticated contracts and await receipts.- Read operations create read-only providers, which is efficient for queries.
- All functions follow consistent error handling patterns.
- The implementations align with the test usage shown in
crates/collateral-contract/src/tests.rs.
451-468: LGTM!The
alpha_collateralsgetter function follows the same consistent read-only pattern as the existingcollateralsfunction, properly extending the API to query alpha collateral balances.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
crates/collateral-contract/src/CollateralUpgradeableV2.sol (2)
353-355: Finalize must free alpha pending before withdrawalalphaCollateralUnderPendingReclaims isn’t decremented, locking future alpha reclaims or causing underflow on available calculation.
delete reclaims[reclaimRequestId]; collateralUnderPendingReclaims[hotkey][nodeId] -= amount; + alphaCollateralUnderPendingReclaims[hotkey][nodeId] -= reclaim.alphaAmount; if (collaterals[hotkey][nodeId] < amount) {Also applies to: 369-372
409-414: Fix deny guard to support alpha-only reclaimsDenying a reclaim with zero TAO but non-zero alpha currently reverts as “not found”.
- if (reclaim.amount == 0) { + if (reclaim.amount == 0 && reclaim.alphaAmount == 0) { revert ReclaimNotFound(); }crates/collateral-contract/src/CollateralUpgradeable.sol (2)
356-365: Finalize must free alpha pending before withdrawalalphaCollateralUnderPendingReclaims isn’t decremented; this locks alpha or causes underflow in future reclaims.
delete reclaims[reclaimRequestId]; collateralUnderPendingReclaims[hotkey][nodeId] -= amount; + alphaCollateralUnderPendingReclaims[hotkey][nodeId] -= alphaAmount; if (collaterals[hotkey][nodeId] < amount) {Also applies to: 372-375
411-414: Fix deny guard to support alpha-only reclaimsDeny path checks only TAO amount; alpha-only reclaims would be incorrectly rejected.
- if (reclaim.amount == 0) { + if (reclaim.amount == 0 && reclaim.alphaAmount == 0) { revert ReclaimNotFound(); }Also applies to: 419-425
♻️ Duplicate comments (2)
crates/collateral-contract/src/main.rs (1)
464-482: Remove duplicate AlphaCollaterals printsTwo lines print the same amount with different labels. Keep one for clarity.
- println!( - "Alpha collateral for executor {}: {} in wei", - node_uuid, result - ); println!( "Alpha collateral for node {}: {} in wei", node_id_clone, result );crates/collateral-contract/src/CollateralUpgradeableV2.sol (1)
452-454: Use OR in slash validation to prevent partial over-slashingCurrent condition allows over-slashing one pool if the other has enough, and underflow later. Revert if either requested slash exceeds balance.
- if (slashAmount > amount && slashAlphaAmount > alphaAmount) { + if (slashAmount > amount || slashAlphaAmount > alphaAmount) { revert InsufficientCollateralForSlash(); }
🧹 Nitpick comments (6)
crates/collateral-contract/src/CollateralUpgradeableV2.sol (1)
9-29: IStaking precompile methods should be nonpayable (consistency with ABI)transferStake/moveStake declared payable; Staking V2 precompile ABI is nonpayable. Align signatures to reduce confusion and accidental value sends.
- ) external payable; + ) external; ... - ) external payable; + ) external;If you want, I can fetch the official precompile ABI to confirm. Based on learnings
crates/collateral-contract/src/main.rs (2)
263-272: Remove duplicate deposit logsTwo deposit messages are printed. Keep the richer one and drop the second.
- let node_uuid = Uuid::parse_str(&node_id)?; - - println!( - "Depositing {} wei for node {} with hotkey {}", - amount, node_id, hotkey - ); + let node_uuid = Uuid::parse_str(&node_id)?;
359-367: Remove redundant slash logThis duplicates the next log and uses “executor” wording inconsistently.
- println!( - "Slashing all collateral for executor {} with hotkey {}", - node_uuid, hotkey - );crates/collateral-contract/src/lib.rs (1)
28-55: Expose alpha fields in Reclaim to match ABI and CLI needsReclaim currently drops alphaColdkey/alphaAmount; add them for complete data and future CLI printing.
#[derive(Debug, Clone)] pub struct Reclaim { pub hotkey: [u8; 32], pub node_id: [u8; 16], pub miner: Address, pub amount: U256, + pub alpha_coldkey: [u8; 32], + pub alpha_amount: U256, pub deny_timeout: u64, } -impl From<(FixedBytes<32>, FixedBytes<16>, Address, U256, u64)> for Reclaim { - fn from(tuple: (FixedBytes<32>, FixedBytes<16>, Address, U256, u64)) -> Self { +impl From<(FixedBytes<32>, FixedBytes<16>, Address, U256, FixedBytes<32>, U256, u64)> for Reclaim { + fn from(tuple: (FixedBytes<32>, FixedBytes<16>, Address, U256, FixedBytes<32>, U256, u64)) -> Self { Self { hotkey: { let mut hk = [0u8; 32]; hk.copy_from_slice(tuple.0.as_slice()); hk }, node_id: { let mut b = [0u8; 16]; b.copy_from_slice(tuple.1.as_slice()); b }, miner: tuple.2, - amount: tuple.3, - deny_timeout: tuple.4, + amount: tuple.3, + alpha_coldkey: { + let mut ck = [0u8; 32]; + ck.copy_from_slice(tuple.4.as_slice()); + ck + }, + alpha_amount: tuple.5, + deny_timeout: tuple.6, } } }And update reclaims():
- let reclaim = Reclaim::from(( - result.hotkey, - result.nodeId, - result.miner, - result.amount, - result.denyTimeout, - )); + let reclaim = Reclaim::from(( + result.hotkey, + result.nodeId, + result.miner, + result.amount, + result.alphaColdkey, + result.alphaAmount, + result.denyTimeout, + ));Also applies to: 37-55, 469-486
crates/collateral-contract/src/CollateralUpgradeable.sol (1)
9-29: IStaking methods likely nonpayable; align with precompile ABIRecommend removing payable from transferStake/moveStake for accuracy and to avoid accidental value sends.
- ) external payable; + ) external; ... - ) external payable; + ) external;If desired, I can pull the precompile ABI to confirm. Based on learnings
crates/collateral-contract/src/tests.rs (1)
295-301: Fix misleading comment: slashes all, not halfThe code slashes full amounts. Update comment to match behavior.
- // slash half of the collaterals + // slash full TAO and alpha collaterals
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
crates/collateral-contract/src/CollateralUpgradableABI.json(15 hunks)crates/collateral-contract/src/CollateralUpgradableV2ABI.json(1 hunks)crates/collateral-contract/src/CollateralUpgradeable.sol(18 hunks)crates/collateral-contract/src/CollateralUpgradeableV2.sol(15 hunks)crates/collateral-contract/src/lib.rs(11 hunks)crates/collateral-contract/src/main.rs(17 hunks)crates/collateral-contract/src/tests.rs(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/collateral-contract/src/CollateralUpgradableV2ABI.json
🧰 Additional context used
🧬 Code graph analysis (3)
crates/collateral-contract/src/main.rs (2)
crates/collateral-contract/src/lib.rs (8)
set_contract_coldkey(315-326)burn_register(328-338)get_version(341-348)contract_coldkey(350-359)contract_hotkey(361-370)alpha_collaterals(451-468)reclaims(469-486)deposit(184-207)crates/collateral-contract/src/tests.rs (1)
burn_register(806-835)
crates/collateral-contract/src/tests.rs (2)
crates/bittensor/metadata/local.rs (13)
proxy(4223-4225)proxy(4294-4296)proxy(4372-4374)proxy(32721-32742)tx(75-77)disable_whitelist(39909-39925)add_stake(12230-12252)force_set_balance(7355-7371)transfer_stake(13411-13436)call(2028-2065)call(39787-39821)storage(72-74)total_networks(19776-19796)crates/collateral-contract/src/lib.rs (10)
netuid(372-379)trustee(381-388)decision_timeout(390-399)from(38-54)contract_hotkey(361-370)contract_coldkey(350-359)collaterals(432-449)alpha_collaterals(451-468)burn_register(328-338)min_collateral_increase(401-410)
crates/collateral-contract/src/lib.rs (1)
crates/collateral-contract/src/tests.rs (1)
burn_register(806-835)
🪛 Gitleaks (8.28.0)
crates/collateral-contract/src/tests.rs
[high] 44-44: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🔇 Additional comments (3)
crates/collateral-contract/src/CollateralUpgradableABI.json (3)
237-263: ABI surface for deposit looks consistent with contract (alphaHotkey/alphaAmount included)No issues spotted.
265-276: ABI for finalizeReclaim matches function signatureNonpayable with reclaimRequestId only; OK.
278-295: getContractStake(hotkey) view entry aligns with V2 changesOK.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
crates/collateral-contract/flow.sh (1)
11-19: Respect caller-provided PRIVATE_KEYThe script exports
PRIVATE_KEY=0x, so even if someone injects a real key via the environment, this line clobbers it and every subsequent CLI call fails. Switch to an env-backed value with a hard fail when unset, e.g.:-export PRIVATE_KEY=0x +export PRIVATE_KEY="${PRIVATE_KEY:?set PRIVATE_KEY in your environment before running}"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
crates/collateral-contract/flow.sh(3 hunks)docs/collateral-contract.md(8 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
crates/collateral-contract/flow.sh (1)
crates/collateral-contract/src/lib.rs (1)
deposit(184-207)
🪛 LanguageTool
docs/collateral-contract.md
[style] ~50-~50: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...llateral into the validator's contract. Miners can now specify a GPU node UUID (re...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[grammar] ~111-~111: Ensure spelling is correct
Context: ...ed amounts. - Owner set the contract coldkey - The owner get the contract address of de...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~113-~113: There might be a mistake here.
Context: ... - The owner get the contract address of deployed proxy contract - Convert the...
(QB_NEW_EN)
[grammar] ~114-~114: There might be a mistake here.
Context: ...ntract - Convert the contract address to substrate account public key - The ow...
(QB_NEW_EN)
[grammar] ~115-~115: There might be a mistake here.
Context: ...tractColdkey** to set the coldkey value in contract - **Owner call burned registe...
(QB_NEW_EN)
[grammar] ~119-~119: There might be a mistake here.
Context: ... - The owner call burnedRegister to register neuron with configured hotkey and coldk...
(QB_NEW_EN)
[grammar] ~119-~119: There might be a mistake here.
Context: ...all burnedRegister** to register neuron with configured hotkey and coldkey in contra...
(QB_NEW_EN)
[grammar] ~119-~119: There might be a mistake here.
Context: ...uron with configured hotkey and coldkey in contract - After successful registrat...
(QB_NEW_EN)
[grammar] ~120-~120: There might be a mistake here.
Context: ...istration, the proxy will be the owner (as coldkey) of hotkey in bittensor network...
(QB_NEW_EN)
[grammar] ~120-~120: There might be a mistake here.
Context: ...he proxy will be the owner (as coldkey) of hotkey in bittensor network - Later, ...
(QB_NEW_EN)
[grammar] ~121-~121: There might be a mistake here.
Context: ...of hotkey in bittensor network - Later, contract can do transferStake via p...
(QB_NEW_EN)
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/collateral-contract.md (1)
171-187: Fix deposit example flags and env var naming
- Replace slashing flags with deposit flags: use
--amountand--alpha-amountinstead of--slash-amount/--slash-alpha-amount.- Rename
node_IDtoNODE_IDand update--node-idusage accordingly.- export node_ID=6339ba4f-60f9-45c2-9d95-2b755bb57ca6 + export NODE_ID=6339ba4f-60f9-45c2-9d95-2b755bb57ca6 ... - --slash-amount 10 \ + --amount 10 \ ... - --slash-alpha-amount 10 \ + --alpha-amount 10 \ - --node-id "$node_ID" + --node-id "$NODE_ID"
♻️ Duplicate comments (1)
docs/collateral-contract.md (1)
15-21: Expand alpha-specific bullets in “Main adaptations.”The single alpha bullet is too terse. Summarize alpha params (alphaHotkey, alphaAmount, alphaColdkey if applicable), deposit/reclaim/slash entry points, queries (alpha-collaterals), mapping keys, and validation/clearing semantics for alpha so readers get the end-to-end alpha flow.
🧹 Nitpick comments (3)
docs/collateral-contract.md (3)
127-135: Align deposit function/signature with alpha support.Docs show
deposit(nodeUuid)but examples include alpha fields later. Update this to reflect the actual ABI/CLI parameters (e.g., amounts and alpha fields) so the function shape and examples match.Would you confirm the current deposit ABI/CLI signature and update this line accordingly?
227-227: Minor style tweak (optional).Consider “Validators will not list miners’ nodes as available unless those nodes are backed by collateral.”
264-264: Document ALPHA_HOTKEY source/derivation.Briefly note where this value comes from (e.g., subnet 39 alpha hotkey), and how to obtain/verify it.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/collateral-contract.md(8 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/collateral-contract.md
[style] ~50-~50: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...llateral into the validator's contract. Miners can now specify a GPU node UUID (re...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[grammar] ~111-~111: Ensure spelling is correct
Context: ...ed amounts. - Owner set the contract coldkey - The owner get the contract address of de...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~113-~113: There might be a mistake here.
Context: ... - The owner get the contract address of deployed proxy contract - Convert the...
(QB_NEW_EN)
[grammar] ~114-~114: There might be a mistake here.
Context: ...ntract - Convert the contract address to substrate account public key - The ow...
(QB_NEW_EN)
[grammar] ~115-~115: There might be a mistake here.
Context: ...tractColdkey** to set the coldkey value in contract - **Owner call burned registe...
(QB_NEW_EN)
[grammar] ~119-~119: There might be a mistake here.
Context: ... - The owner call burnedRegister to register neuron with configured hotkey and coldk...
(QB_NEW_EN)
[grammar] ~119-~119: There might be a mistake here.
Context: ...all burnedRegister** to register neuron with configured hotkey and coldkey in contra...
(QB_NEW_EN)
[grammar] ~119-~119: There might be a mistake here.
Context: ...uron with configured hotkey and coldkey in contract - After successful registrat...
(QB_NEW_EN)
[grammar] ~120-~120: There might be a mistake here.
Context: ...istration, the proxy will be the owner (as coldkey) of hotkey in bittensor network...
(QB_NEW_EN)
[grammar] ~120-~120: There might be a mistake here.
Context: ...he proxy will be the owner (as coldkey) of hotkey in bittensor network - Later, ...
(QB_NEW_EN)
[grammar] ~121-~121: There might be a mistake here.
Context: ...of hotkey in bittensor network - Later, contract can do transferStake via p...
(QB_NEW_EN)
🔇 Additional comments (4)
docs/collateral-contract.md (4)
5-5: Terminology note looks good.Clear shift to node UUID/ID terminology.
50-50: LGTM on node-bound deposit note.Good clarity on associating collateral with a node UUID.
189-217: Query examples look consistent with node/alpha mappings.Good inclusion of
node-to-miner,collaterals, andalpha-collateralsqueries with--node-id.Consider adding one sample output line for each to set user expectations.
294-295: Slash behavior clarification reads well.Clear on partial slashing and clearing semantics.
Summary
Related Issues
Closes #(issue number)
Type of Change
Changes Made
List the main changes in this PR:
Testing
How Has This Been Tested?
Describe the tests you ran to verify your changes.
cargo test)Test Configuration
Checklist
cargo fmtto format my codecargo clippyand addressed all warningsAdditional Context
Add any other context about the PR here.
Summary by CodeRabbit
New Features
Documentation
Tests
Chores