docs: fix "fill or kill type or order" typo in allow_partial doc comments#2875
Conversation
…ents Co-authored-by: Cursor <cursoragent@cursor.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🛡️ AI Review — Skeptic (security review)VERDICT: VULNERABLE MEDIUM scrutiny — 118-day-old contributor with repository write access and substantial history; no Gittensor association; fix-fill-or-kill-doc-typo → main. The PR is no longer doc-only: it includes the merged runtime-upgrade signing and CI changes. The previously identified signing trust-boundary issue remains. Findings
Prior-comment reconciliation
ConclusionThe change appears legitimate, but the signing command still permits mainnet approval without independently verifying the runtime bytes. 📜 Previous run (superseded)
🔍 AI Review — Auditor (domain review)VERDICT: 👍 UNKNOWN Gittensor association; established high-volume contributor with repository write access. The changes are non-runtime and require neither tests nor a Description discrepancy: the PR body calls this a doc-comment-only change, but the branch also changes Docker-pull retry behavior and adds a scoped deprecation allowance to a migration test. The body should mention those ancillary changes. Findings
Prior-comment reconciliation
ConclusionThe PR is safe to merge, though the remaining awkward wording should be corrected consistently before or during merge. 📜 Previous run (superseded)
|
| /// * `allow_partial`: Allows partial execution of the amount. If set to false, this becomes | ||
| /// fill or kill type or order. | ||
| /// fill or kill type of order. |
There was a problem hiding this comment.
[LOW] Replacement remains grammatically awkward
“Fill-or-kill” is the established compound adjective; “fill or kill type of order” remains ungrammatical. Use the clearer wording below consistently across all five changed comments.
| /// * `allow_partial`: Allows partial execution of the amount. If set to false, this becomes | |
| /// fill or kill type or order. | |
| /// fill or kill type of order. | |
| /// * `allow_partial`: Allows partial execution of the amount. If set to false, this becomes | |
| /// a fill-or-kill order. |
|
🔄 AI review updated — Skeptic: SAFE Auditor: 👍 |
…ration test The doc comment typo fix propagates into runtime metadata, so the generated Python calls.py must be regenerated to keep the metadata drift gate green. Also silence clippy::deprecated on the LastTxBlockDelegateTake migration test, which intentionally exercises the deprecated storage aliases. Co-authored-by: Cursor <cursoragent@cursor.com>
| /// | ||
| /// * `allow_partial`: Allows partial execution of the amount. If set to false, this becomes | ||
| /// fill or kill type or order. | ||
| /// fill or kill type of order. |
There was a problem hiding this comment.
[LOW] Replacement remains grammatically awkward
“Fill or kill type of order” is still unnatural. Use the standard compound term “a fill-or-kill order,” and apply the same wording to the other source and generated occurrences in this PR.
| /// fill or kill type of order. | |
| /// a fill-or-kill order. |
|
🔄 AI review updated — Skeptic: SAFE Auditor: 👍 |
…ials With ~120 parallel e2e jobs pulling the same localnet image, GHCR occasionally returns a spurious permission_denied. Retry with backoff instead of failing the shard on the first attempt. Co-authored-by: Cursor <cursoragent@cursor.com>
| /// | ||
| /// * `allow_partial`: Allows partial execution of the amount. If set to false, this becomes | ||
| /// fill or kill type or order. | ||
| /// fill or kill type of order. |
There was a problem hiding this comment.
[LOW] Replacement remains grammatically awkward
“Fill or kill type of order” is still unnatural and does not use the standard term. Use “a fill-or-kill order” here and in every corresponding source/generated occurrence.
| /// fill or kill type of order. | |
| /// a fill-or-kill order. |
|
🔄 AI review updated — Skeptic: SAFE Auditor: 👍 |
…eleases) The release train now publishes each mainnet upgrade proposal as a pre-release v<spec> whose page URL is the single thing to share with signers: it carries the signer instructions and the artifacts (proxy_proxy_blob.hex, subtensor.wasm, digest, upgrade-manifest.json) as stable-URL assets. The mainnet watcher promotes the pre-release to the final release once the upgrade executes instead of creating a new one. On the signer side, a new `btcli upgrade` command group replaces the manual prod-approval.js flow: `pending` discovers proposals purely from chain state (sudo.key() -> SudoUncheckedSetCode proxy -> pending multisig ops), `check` verifies the call data end to end (exact structural re-encoding against live metadata, srtool digest, live sudo key, on-chain call hash — with --wasm pinning it to a self-built runtime), and `sign` runs every check then auto-detects the first/interior/final signer position from chain state and submits the matching approval. `btcli multisig pending` surfaces pending upgrades when the multisig is the chain's sudo key. Co-authored-by: Cursor <cursoragent@cursor.com>
| if bundle.wasm_source != "local": | ||
| app_ctx.output.message( | ||
| "[yellow]note:[/yellow] the runtime was taken from the release, not a local " | ||
| "srtool build — anyone reproducing the build should pass --wasm" | ||
| ) |
There was a problem hiding this comment.
[HIGH] Require an independently built runtime before signing
This warning still allows signing when both the proposal blob and WASM came from the release. Those artifacts share the CI/GitHub trust domain that submitted the on-chain proposal, so their byte equality does not independently establish that the runtime corresponds to reviewed source. If CI, its credentials, or release publication is compromised, the recommended btcli upgrade sign --url ... flow lets triumvirate members approve arbitrary structurally valid runtime code. Preserve the multisig’s independent security boundary by refusing to sign unless --wasm supplies a locally reproduced build.
| if bundle.wasm_source != "local": | |
| app_ctx.output.message( | |
| "[yellow]note:[/yellow] the runtime was taken from the release, not a local " | |
| "srtool build — anyone reproducing the build should pass --wasm" | |
| ) | |
| if bundle.wasm_source != "local": | |
| app_ctx.output.error( | |
| "refusing to sign without an independently built runtime; pass --wasm " | |
| "with the byte-identical srtool build produced from reviewed source" | |
| ) | |
| raise typer.Exit(1) |
|
🔄 AI review updated — Skeptic: VULNERABLE |
| if bundle.wasm_source != "local": | ||
| app_ctx.output.message( | ||
| "[yellow]note:[/yellow] the runtime was taken from the release, not a local " | ||
| "srtool build — anyone reproducing the build should pass --wasm" | ||
| ) |
There was a problem hiding this comment.
[HIGH] Require an independently built runtime before signing
This remains only a warning, so every triumvirate signer can approve runtime bytes supplied by the same CI/release trust domain that proposed them. If that domain is compromised, all documented checks can agree on malicious bytes without any signer independently deriving the runtime from source. Refuse signing unless --wasm supplied a locally reproduced build.
| if bundle.wasm_source != "local": | |
| app_ctx.output.message( | |
| "[yellow]note:[/yellow] the runtime was taken from the release, not a local " | |
| "srtool build — anyone reproducing the build should pass --wasm" | |
| ) | |
| if bundle.wasm_source != "local": | |
| app_ctx.output.error( | |
| "refusing to sign without an independently built runtime", | |
| help="build the tagged source with srtool and pass the result with --wasm", | |
| ) | |
| raise typer.Exit(1) |
|
🔄 AI review updated — Skeptic: VULNERABLE |
Summary
allow_partialparameter docs (5 occurrences acrossdispatches.rs,add_stake.rs,remove_stake.rs).pallets/, so it exercises the split builds, the required try-runtime snapshot path, and all four clone shards end to end.Test plan
Made with Cursor