Skip to content

Restore contract-proxied stake transfers - #3140

Merged
UnArbosFive merged 2 commits into
release-454from
fix/proxy-transfer-stake
Sep 4, 2026
Merged

Restore contract-proxied stake transfers#3140
UnArbosFive merged 2 commits into
release-454from
fix/proxy-transfer-stake

Conversation

@UnArbosFour

@UnArbosFour UnArbosFour commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Context

Release 453 fixed a proxy filter-laundering vulnerability by preserving inherited origin filters across nested proxy calls.

This also blocked legitimate contract deposit flows that execute:

Contract::call_runtime(
    Proxy::proxy(
        real = depositor,
        SubtensorModule::transfer_stake { ... }
    )
)

The inner transfer_stake must now pass both the contract call filter and the user’s proxy-type filter. Because ContractCallFilter only permitted Proxy::proxy, these calls began failing with CallFiltered.

Changes

  • Allow SubtensorModule::transfer_stake through ContractCallFilter.
  • Bump the runtime spec_version from 453 to 454.

Security

This does not reintroduce the proxy vulnerability fixed in release 453.

The effective authorization remains the intersection of all inherited filters:

  • The contract must be explicitly registered as the user’s proxy.
  • The selected proxy type must permit transfer_stake.
  • NonTransfer and NonFungible proxies continue to reject the transfer.
  • SmallTransfer continues to enforce its amount limit.
  • Other nested calls remain blocked.
  • A direct contract-origin transfer_stake can only move stake owned by the contract account itself.

Validation

  • cargo fmt --check --all
  • git diff --check

masterdubs and others added 2 commits September 3, 2026 16:23
The proxy origin-filter inheritance fix in release 453 (do_proxy now
propagates the caller's call filter into the proxied call) closed a real
filter-laundering loophole — but it also broke contracts that legitimately
execute transfer_stake through an explicit user proxy delegation.

An escrow contract deployed on this chain performs deposits as:
  call_runtime(Proxy::proxy(real = depositor, TransferStake { .. }))
where the depositor has registered the contract as a proxy beforehand.
Pre-453 the inner TransferStake ran on a freshly authenticated origin and
succeeded; post-453 it must pass ContractCallFilter, which only whitelists
Proxy::proxy, so the inner call fails with CallFiltered inside
ProxyExecuted and every deposit reverts.

Whitelist the single inner call the flow needs. Security posture is
preserved: the transfer still requires the user's explicit proxy
delegation, and the inherited-filter mechanism keeps every other nested
call blocked.
@vercel

vercel Bot commented Sep 3, 2026

Copy link
Copy Markdown

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

Project Deployment Actions Updated
subtensor Ready Ready Preview Sep 3, 2026 8:26pm UTC

Request Review

@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

🛡️ AI Review — Skeptic (security review)

VERDICT: SAFE

HIGH scrutiny: account is 63 days old, but has repository write access and substantive merged history; no Gittensor association found. Branch targets main.

Static review found the allowlist expansion limited to transfer_stake. Direct contract calls can affect only the contract account’s own stake; proxied calls still require an explicit delegation and must pass the selected proxy-type filter inherited by do_proxy. The runtime version is bumped appropriately.

Findings

No findings.

Conclusion

No malicious behavior or security vulnerability was found in this diff.


🔍 AI Review — Auditor (domain review)

VERDICT: 👍

UNKNOWN Gittensor association; newer account with repository write access and substantive merged contribution history.

The filter expansion is narrowly limited to transfer_stake; proxied calls remain constrained by the delegated proxy type, while direct contract calls can affect only the contract account's stake.

PR #3139 contains the same functional commit but omits this branch's required runtime version bump. This PR is the better candidate. Recommend closing #3139.

Static review only; runtime execution was unnecessary. git diff --check passed and git status --short was clean.

Findings

Sev File Finding
MEDIUM runtime/src/lib.rs:1418 Add regression coverage for the expanded contract filter inline

Conclusion

The implementation restores the intended contract-proxy flow without bypassing inherited proxy restrictions. It is ready after adding focused regression coverage.

@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
Comment on lines +1417 to +1418
RuntimeCall::SubtensorModule(inner) => {
matches!(inner, pallet_subtensor::Call::transfer_stake { .. })

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.

[MEDIUM] Add regression coverage for the expanded contract filter

This production regression fix changes a security-sensitive runtime allowlist but adds no test. Add focused coverage proving that the filter accepts transfer_stake and Proxy::proxy, rejects unrelated calls, and—preferably through dispatch—preserves rejection by NonTransfer, NonFungible, and over-limit SmallTransfer proxies.

@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

🔄 AI review updated — Skeptic: SAFE Auditor: 👍

@UnArbosFour
UnArbosFour changed the base branch from main to release-454 September 4, 2026 13:39
@UnArbosFive
UnArbosFive merged commit 14fe0ad into release-454 Sep 4, 2026
98 of 101 checks passed
@UnArbosFive UnArbosFive mentioned this pull request Sep 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants