Skip to content

Convert transaction extensions to dispatch extensions#2773

Merged
sam0x17 merged 22 commits into
devnet-readyfrom
tx-ext-to-dispatch-ext
Jun 23, 2026
Merged

Convert transaction extensions to dispatch extensions#2773
sam0x17 merged 22 commits into
devnet-readyfrom
tx-ext-to-dispatch-ext

Conversation

@l0r1s

@l0r1s l0r1s commented Jun 21, 2026

Copy link
Copy Markdown
Collaborator

Summary

This PR moves Subtensor-specific pre-dispatch checks out of SubtensorTransactionExtension and into FRAME DispatchExtensions.

The dispatch extensions are now the source of truth for these checks, so they run for both top-level extrinsics and nested dispatch paths such as proxies and precompile-dispatched runtime calls. SubtensorTransactionExtension remains only as a transaction-pool adapter for top-level validation, including Pays::No admission checks.

Changes

  • Added dedicated Subtensor dispatch extensions:
    • CheckWeights: input length checks, minimum stake checks, commit/reveal validation, and invalid reveal-round checks.
    • CheckRateLimits: weight commit/set rate limits and network registration rate limits.
    • CheckDelegateTake: delegate-take bounds and hotkey owner checks.
    • CheckServingEndpoints: axon, axon TLS, and prometheus serving endpoint validation.
    • CheckEvmKeyAssociation: subnet UID existence and EVM-key association cooldown checks.
  • Kept CheckColdkeySwap as a dispatch extension and expanded tests to cover direct, proxied, and nested-proxy execution paths.
  • Wired all Subtensor dispatch extensions into frame_system::Config::DispatchExtension as a tuple in the runtime and test runtimes.
  • Simplified SubtensorTransactionExtension:
    • Removes duplicated rule-specific validation logic.
    • Calls the configured runtime dispatch extension from validate.
    • Maps Subtensor pallet DispatchError::Module values into CustomTransactionError.
  • Simplified precompile dispatch:
    • Removes manual SubtensorTransactionExtension validate/prepare/post-dispatch calls.
    • Relies on normal call.dispatch(origin), which invokes dispatch extensions.
  • Moved rule-specific tests into the corresponding dispatch extension files.
  • Deleted the old transaction_extension_pays_no test module and colocated transaction-extension adapter tests with SubtensorTransactionExtension.
  • Removed legacy transaction-extension validation tests from serving and weights tests where the same behavior is now covered by dispatch-extension tests.

@l0r1s l0r1s added the skip-cargo-audit This PR fails cargo audit but needs to be merged anyway label Jun 21, 2026
@github-actions

Copy link
Copy Markdown
Contributor

eco-tests changed — indexer review required

This PR modifies files under eco-tests/. and may affect downstream indexing.
cc @evgeny-s — please review manually

Changed files
  • eco-tests/src/mock.rs

@github-actions github-actions Bot requested a review from evgeny-s June 21, 2026 22:40

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

AI review — see the sticky summary comment for the verdict and the inline comments below for specific findings.

}

fn commit_epoch(hash: H256) -> Result<u64, Error<T>> {
Pallet::<T>::find_commit_epoch_via_hash(hash).ok_or(Error::<T>::NoWeightsCommitFound)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[HIGH] Unbounded commit scan now runs during dispatch

commit_epoch() calls Pallet::<T>::find_commit_epoch_via_hash, which is implemented as WeightCommits::<T>::iter() over all stored commits. Because this helper now runs from CheckWeights::pre_dispatch, every reveal_weights, reveal_mechanism_weights, and each item in batch_reveal_weights can perform a global storage scan during block execution, proxy dispatch, or EVM-precompile dispatch. The extension weight is fixed, so a staked caller can force work proportional to global commit storage while paying only the normal fixed/no-fee reveal weight, creating a block-weight underestimation and DoS path.

Bound this check to the caller's own commit queue, e.g. read WeightCommits for (netuid_index, who) and search that bounded deque, matching the dispatch body's logic. Alternatively, keep the global pre-admission scan only in the transaction-pool adapter and rely on the dispatch body for execution-time validation.

@github-actions

github-actions Bot commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

🛡️ AI Review — Skeptic (security review)

VERDICT: VULNERABLE

BASELINE scrutiny: l0r1s has write permission, a long-lived account, substantial prior Subtensor history, no Gittensor allowlist hit, and no in-PR author/committer mismatch; branch tx-ext-to-dispatch-ext -> devnet-ready.

No .github/ai-review, copilot-instruction, dependency manifest, lockfile, or build-script changes are present. The previous WeightCommits global-iteration concern remains addressed by the caller-scoped WeightCommits(netuid_index, who) lookup, but the precompile dispatch bypass concern is still present on the same runtime call path.

Findings

Sev File Finding
HIGH precompiles/src/extensions.rs:98 Precompile dispatch bypasses the new guard extensions inline

Prior-comment reconciliation

  • c6c7271c: not addressed — The current diff adds a precompile test and wires the mock runtime's DispatchExtension, but the production helper still performs plain call.dispatch(origin) after removing the explicit Subtensor transaction-extension validation.

Conclusion

The PR still removes the explicit Subtensor transaction-extension validation from precompile dispatch while leaving this path on plain call.dispatch(origin). That keeps EVM-dispatched Subtensor calls as a bypass for the new guard extensions, so the verdict remains VULNERABLE.


📜 Previous run (superseded)
Sev File Finding Status
HIGH precompiles/src/extensions.rs:98 Precompile dispatch bypasses the new guard extensions ➡️ Carried forward to current findings
The current diff adds a precompile test and wires the mock runtime's DispatchExtension, but the production helper still performs plain call.dispatch(origin) after removing the explicit Subtensor transaction-extension validation.

🔍 AI Review — Auditor (domain review)

VERDICT: 👍

Gittensor: LIKELY by repository-history heuristic; l0r1s is an established write-permission contributor with substantial prior Subtensor history.

The PR body is substantive and matches the implementation. I found no duplicate open PR that is a better candidate for this transaction-extension to dispatch-extension refactor.

No auto-fixes were applied. I attempted a targeted cargo test for the changed CheckWeights guard path, but the environment's rustup setup tried to write under the read-only home directory before Cargo could start. The devnet spec-version RPC query also failed DNS resolution from this runner, so I could not independently confirm whether an auto-bump is needed; local runtime/src/lib.rs currently reports spec_version: 420.

Findings

No findings.

Conclusion

Static domain review found the moved pre-dispatch checks wired consistently into the runtime and test runtimes, with focused guard tests covering the new dispatch-extension behavior. I do not have a blocking finding for this PR.

@github-actions

Copy link
Copy Markdown
Contributor

🔄 AI review updated — Skeptic: VULNERABLE

@github-actions

Copy link
Copy Markdown
Contributor

🔄 AI review updated — Skeptic: SAFE Auditor: 👍

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

AI review — see the sticky summary comment for the verdict and the inline comments below for specific findings.

Comment thread precompiles/src/extensions.rs
@github-actions

Copy link
Copy Markdown
Contributor

🔄 AI review updated — Skeptic: VULNERABLE

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

AI review — see the sticky summary comment for the verdict and the inline comments below for specific findings.

Comment thread precompiles/src/extensions.rs
@github-actions

Copy link
Copy Markdown
Contributor

🔄 AI review updated — Skeptic: VULNERABLE

@sam0x17 sam0x17 merged commit 4c3fd8c into devnet-ready Jun 23, 2026
235 of 240 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip-cargo-audit This PR fails cargo audit but needs to be merged anyway

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants