Skip to content

Refund Unused Extrinsic Weight (fee) - #2997

Open
UnArbosSix wants to merge 17 commits into
mainfrom
feat/subtensor-actual-weight-refunds
Open

Refund Unused Extrinsic Weight (fee)#2997
UnArbosSix wants to merge 17 commits into
mainfrom
feat/subtensor-actual-weight-refunds

Conversation

@UnArbosSix

@UnArbosSix UnArbosSix commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator

Motivation

Subtensor extrinsics with state-dependent work currently charge their declared worst-case weight without consistently returning the weight actually consumed. This overcharges transaction fees and contract chain-extension gas for smaller executions.

Changes

  • Precharge variable-work Subtensor calls with the largest admissible normal-dispatch weight and return benchmark-derived actual weight through post-dispatch information.
  • Preserve actual-weight information on both successful and failed dispatches.
  • Account for transaction and dispatch-extension overhead when determining the maximum admissible call weight.
  • Update chain extensions to adjust charged weight after variable-weight calls.
  • Parameterize affected benchmarks and weight interfaces by observed collection sizes and state fan-out.
  • Update hotkey and coldkey swap accounting and related tests.

Files of interest

  • pallets/subtensor/src/weight_accounting.rs
  • pallets/subtensor/src/macros/dispatches.rs
  • pallets/subtensor/src/swap/swap_hotkey.rs
  • pallets/subtensor/src/benchmarks/
  • pallets/subtensor/src/weights.rs
  • chain-extensions/src/lib.rs
  • runtime/src/lib.rs

Behavioral impact

Variable-work extrinsics reserve sufficient weight for admission and refund unused weight after execution. Failed dispatches also report their actual consumed weight. Contract chain-extension callers receive corresponding gas adjustments.

Runtime upgrade

This changes runtime dispatch and fee behavior. spec_version is bumped from 440 to 441. No storage migration is introduced.

Testing

Unit and runtime tests cover post-dispatch weights, failure-path refunds, chain-extension adjustments, and claim-root accounting. The affected benchmarks must be regenerated and checked to ensure each variable component produces the expected weight slope before merge.

@UnArbosSix UnArbosSix added the run-benchmarks run benchmarks label Jul 27, 2026
@vercel

vercel Bot commented Jul 27, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
subtensor Ready Ready Preview Jul 30, 2026 12:49am

Request Review

@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 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 runtime/src/lib.rs Outdated
@github-actions

github-actions Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

🛡️ AI Review — Skeptic (security review)

VERDICT: SAFE

VERY HIGH scrutiny due to account age/public-repo tier, mitigated by repository write permission and explained benchmark-bot commits; no Gittensor association found. Branch: feat/subtensor-actual-weight-refunds → main.

The maximum dispatch reservation now belongs to each variable-work call, so nested dispatch wrappers inherit it and propagate the call’s measured post-dispatch refund. No security vulnerabilities or malicious behavior were found by static analysis.

Findings

No findings.

Prior-comment reconciliation

  • 79708346: addressed — Variable-work calls now declare the maximum admissible call weight directly and return measured post-dispatch weight, including when nested through utility dispatch.

Conclusion

The previously reported nested-dispatch undercharge is addressed, and no new security findings remain.


📜 Previous run (superseded)
Sev File Finding Status
HIGH pallets/subtensor/src/swap/swap_hotkey.rs:39 Nested dispatch bypasses the maximum-weight reserve ✅ Addressed
Variable-work calls now declare the maximum admissible call weight directly and return measured post-dispatch weight, including when nested through utility dispatch.

🔍 AI Review — Auditor (domain review)

VERDICT: 👍

Gittensor association UNKNOWN; repository write access and substantial Subtensor activity, but the account is under 90 days old.

Static inspection was sufficient; no runtime test or auto-fix was needed.

Description discrepancy

The runtime bumps spec_version from 440 to 442, while the PR body says 441. It also bumps transaction_version from 1 to 2, which the Runtime upgrade section should mention.

Overlapping work

This PR is the better candidate for the weight-accounting changes also touched by #2993 because it integrates claim-root accounting into the comprehensive refund implementation and associated tests. Recommend closing #2993 once its release-branch requirements are confirmed covered.

Findings

No findings.

Prior-comment reconciliation

  • 71b5b838: addressed — The generated weight functions retain nonzero parameter slopes and variable database/proof-size costs.

Conclusion

The variable dispatch paths conservatively precharge admission weight and return benchmark-derived actual weight on success and failure, with corresponding chain-extension refunds and parameterized generated weights. The PR is ready from a domain-review perspective.

@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 pallets/subtensor/src/weights.rs Outdated
@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 pallets/subtensor/src/weights.rs Outdated
@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 pallets/subtensor/src/weights.rs Outdated
@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 pallets/subtensor/src/weights.rs Outdated
@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 pallets/subtensor/src/extensions/subtensor.rs Outdated
@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

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 on lines +38 to +39
pub fn precharge_maximum(benchmark_model: Weight) -> Weight {
benchmark_model

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] Nested dispatch bypasses the maximum-weight reserve

This leaves nested variable-work calls charged only at their benchmark-model metadata weight, while the maximum reserve is added exclusively by SubtensorTransactionExtension when the top-level call matches reserves_maximum_dispatch_weight. Wrappers such as utility batch, proxy, multisig, and sudo present their outer call to that extension, so an inner claim_root, swap_coldkey_announced, or root swap_coldkey receives no reserve. Several such calls now declare state-dependent parameters as zero and report the larger value only post-dispatch; FRAME's nested-dispatch accounting caps refunds/actual weight against the inner declared weight. An attacker can therefore batch expensive state fan-out while admission and block accounting see the small zero-parameter weights. Provide a conservative composable declaration for nested calls, or make every wrapper path reserve and propagate the uncapped actual weight safely.

@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: 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

run-benchmarks run benchmarks

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant