Improvments to runtime configuration

[karolk91]

Summary

Add recursive wrapper call inspection to ValidateStorageCalls so that storage authorization is enforced even when store/store_with_cid_config/renew are nested inside Utility::batch, Proxy::proxy, Sudo::sudo_as, etc. Also harden XCM configuration to block storage-mutating calls over XCM, refactor AllowedSignedCalls into a single-source-of-truth allowlist, and add extensive test coverage for both runtimes.

What changed

1. CallInspector trait + TraverseResult (pallets/transaction-storage/src/extension.rs)

New trait that lets the runtime tell the pallet extension how to unwrap wrapper calls. The extension recursively walks the call tree and rejects any wrapper containing storage-mutating calls (store/renew must be direct extrinsics). For wrappers containing only management calls (authorize_, refresh_, remove_expired_*), it validates authorization and accumulates ValidTransaction metadata for correct mempool deduplication.

TraverseResult tracks:

• found_storage — whether any TransactionStorage pallet calls were found

2. Wrapper inspection helpers (pallets/common/src/lib.rs)

Generic helper functions for pallet-utility, pallet-sudo, and pallet-proxy that extract inner calls from wrapper call variants. Returns None for non-wrapper variants (e.g. set_key, add_proxy). Exhaustively matches all variants (including __Ignore) to ensure new upstream variants cause compile errors.

3. StorageCallInspector (both runtimes)

Runtime-specific CallInspector implementation. Also implements Contains<RuntimeCall> for use as XCM SafeCallFilter.

• Polkadot: inspects Utility, Proxy, and Sudo wrappers
• Westend: inspects Utility only. Sudo is intentionally not inspected so the sudo key holder can sudo(store) without authorization (Root origin is accepted by ensure_authorized)

4. AllowedSignedCalls refactor (Polkadot runtime)

Split into validate_signed_call() (recursive allowlist + auth checks) and a priority/longevity assignment block. Adds dedicated priority/longevity constants for Sudo, Proxy, Utility, and Upgrade calls (previously they all borrowed BridgeTxLongevity).

5. XCM SafeCallFilter (both runtimes)

Changed from Everything to EverythingBut<StorageCallInspector>, blocking store/store_with_cid_config/renew over XCM (including when wrapped in batch). Authorization management calls (authorize_account, etc.) remain allowed over XCM.

What is allowed / rejected

Signed transactions — storage calls

Scenario	Result	Reason
Direct store/renew with authorization	OK	Origin transformed to Authorized
Direct store/renew without authorization	Rejected (Payment)	No auth found
batch([store, ...]) with or without auth	Rejected (Call)	Store/renew must be direct extrinsics
sudo(store) (Westend only, sudo key holder)	OK	Sudo not inspected; Root accepted by ensure_authorized
sudo(store) / sudo_as(store) (Polkadot)	Rejected (Call)	Sudo inspected; store inside wrapper rejected
proxy(store) (Polkadot)	Rejected (Call)	Proxy inspected; store inside wrapper rejected
batch_all([authorize_account]) by Authorizer	OK	Management calls allowed in wrappers
batch([authorize_account]) by non-Authorizer	OK at validation, fails at dispatch	Dispatch checks Authorizer origin
Nesting > MAX_WRAPPER_DEPTH (8)	Rejected (ExhaustsResources)	Depth limit prevents complexity attacks

XCM Transact

Scenario	Result
store / store_with_cid_config / renew	Blocked by SafeCallFilter
batch([store])	Blocked (recursive inspection)
authorize_account / authorize_preimage	Allowed (Superuser → Root satisfies Authorizer)

Polkadot solochain signed call allowlist

Allowed: TransactionStorage::*, Session::set_keys/purge_keys, bridge calls (submit_finality_proof, submit_parachain_heads, receive_messages_proof/delivery_proof), Sudo::*, Proxy::*, Utility::* (inner calls recursively validated), System::apply_authorized_upgrade.

Everything else (e.g., System::remark) is rejected with InvalidTransaction::Call, both direct and wrapped.

Design: store/renew must be direct extrinsics

The ValidateStorageCalls extension rejects store/renew inside any wrapper. This is because:

• The extension transforms the origin to Origin::Authorized { who, scope } so that store/renew dispatches pass ensure_authorized
• Only wrappers that preserve the caller's origin (batch) could forward Authorized, but mixing storage + non-storage calls in a batch creates an origin conflict (Authorized vs Signed)
• Wrappers that change the origin (sudo, proxy) would lose the Authorized origin at dispatch
• Rejecting all wrapped store/renew is fail-safe by design — simpler and eliminates edge cases

Exception: On Westend, sudo is not inspected. sudo(store) works because sudo dispatches with Root origin, which ensure_authorized accepts directly (no Authorized origin needed).

Test coverage

~1250 new lines of tests across both runtimes covering:

• Authorization enforcement for all wrapper types (batch, batch_all, force_batch, as_derivative, sudo_as, proxy)
• Store/renew rejection inside wrappers even when authorized
• Mixed batch rejection (storage + management, storage + non-storage)
• XCM SafeCallFilter for direct and wrapped storage calls
• XCM authorization management pass-through
• Recursion depth enforcement
• Call allowlist enforcement in wrappers (Polkadot)
• Authorization consumption verification (not consumed on rejection)
• sudo(store) works for sudo key holder on Westend

[bkontur]

hmm, not sure about this now, maybe we want to allow SC to renew in the future (most probably for renew we will also remove authorization check), but ok we can keep it like this

[bkontur]

I meant this, the Utility is not storage call and it is handled by validate_inner_calls

Suggested change

	RuntimeCall::Utility(utility_call) => {
	for inner in utility_inner_calls(utility_call) {
	validate_storage_calls(who, inner, depth + 1, consume)?;
	}
	Ok(())
	},

[karolk91]

I didn't understand the comment ? could you explain a bit more?

[bkontur]

hmm, this is strange, we reached the max_depth and returning true?
Which means "Returns true if call is a storage-mutating TransactionStorage call" ?

[karolk91]

adding a comment would be fine? or some more explanatory return type? if someone tries to wrap with too many depth levels then returning true will eventually cause the call to be blocked in such case

[bkontur]

for example for recursion depth/count for xcm we are using something like this https://github.com/paritytech/polkadot-sdk/blob/d5c5fbe6b7752779f75e2bc8db4fea354aefc01a/polkadot/xcm/xcm-builder/src/barriers.rs#L593

[karolk91]

I see, lookint at how environmental! works, its using thread-local global state to track recursions - I believe that's mostly not a good thing unless you must do it for some reason. For this use case here - too complicated

[bkontur]

@karolk91 my assistant sees (I will check later also):

 1. extract_signer doc comment says origin is transformed for wrapper calls, but it isn't
  extension.rs:229-232 only transforms origin for direct store/renew calls (when maybe_scope is Some). For wrapper calls containing management calls, the origin stays Signed. The doc comment on extract_signer
  (runtimes/bulletin-polkadot/src/lib.rs:564-567) says:

  ▎ ValidateStorageCalls transforms the origin to Authorized for wrapper calls containing storage operations

  This is inaccurate — it only transforms for direct store/renew. Wrappers containing management calls keep the Signed origin. Consider updating the comment.

  2. Depth limit inconsistency between is_storage_mutating_call and traverse_storage_calls
  In is_storage_mutating_call (extension.rs:63-72), the depth check happens before is_sub_type(), so at depth == MAX_WRAPPER_DEPTH even a direct Call::store would be treated as storage-mutating (which is fine —
  fail-safe). But in traverse_storage_calls (extension.rs:98-101), is_sub_type() is checked before the depth check, so a direct storage call at any depth would pass the visitor. This means at depth >= MAX_WRAPPER_DEPTH,
   is_storage_mutating_call returns true (blocking the call), but traverse_storage_calls would successfully visit it. Since is_storage_mutating_call is always called first to reject store/renew in wrappers, this isn't
  exploitable, but the inconsistent ordering could be confusing for future maintainers.


  Low / Nits

  4. TraverseResult is effectively a bool
  TraverseResult has a single field found_storage: bool. Unless you plan to add more fields, a plain bool would be simpler. If expansion is planned, a comment noting that would help.

  5. Unused import: inspect_sudo_wrapper in bulletin-polkadot
  runtimes/bulletin-polkadot/src/lib.rs:560 imports inspect_sudo_wrapper from pallets_common. It's used in StorageCallInspector::inspect_wrapper, so it's not dead code, but verify clippy is happy.

[karolk91]

@karolk91 my assistant sees (I will check later also):

 1. extract_signer doc comment says origin is transformed for wrapper calls, but it isn't
  extension.rs:229-232 only transforms origin for direct store/renew calls (when maybe_scope is Some). For wrapper calls containing management calls, the origin stays Signed. The doc comment on extract_signer
  (runtimes/bulletin-polkadot/src/lib.rs:564-567) says:

  ▎ ValidateStorageCalls transforms the origin to Authorized for wrapper calls containing storage operations

  This is inaccurate — it only transforms for direct store/renew. Wrappers containing management calls keep the Signed origin. Consider updating the comment.

  2. Depth limit inconsistency between is_storage_mutating_call and traverse_storage_calls
  In is_storage_mutating_call (extension.rs:63-72), the depth check happens before is_sub_type(), so at depth == MAX_WRAPPER_DEPTH even a direct Call::store would be treated as storage-mutating (which is fine —
  fail-safe). But in traverse_storage_calls (extension.rs:98-101), is_sub_type() is checked before the depth check, so a direct storage call at any depth would pass the visitor. This means at depth >= MAX_WRAPPER_DEPTH,
   is_storage_mutating_call returns true (blocking the call), but traverse_storage_calls would successfully visit it. Since is_storage_mutating_call is always called first to reject store/renew in wrappers, this isn't
  exploitable, but the inconsistent ordering could be confusing for future maintainers.


  Low / Nits

  4. TraverseResult is effectively a bool
  TraverseResult has a single field found_storage: bool. Unless you plan to add more fields, a plain bool would be simpler. If expansion is planned, a comment noting that would help.

  5. Unused import: inspect_sudo_wrapper in bulletin-polkadot
  runtimes/bulletin-polkadot/src/lib.rs:560 imports inspect_sudo_wrapper from pallets_common. It's used in StorageCallInspector::inspect_wrapper, so it's not dead code, but verify clippy is happy.

1 Seems just doc is outdated - fixed in: 863d37b
2 Indeed some incosistency was there (at the DEPTH_LEVEL=max) - doesn't lead to any issues just behaves slightly differently - also fixed in: 863d37b

4 I think TraverseResult may stay, at is a bit more self-explanatory
5. Assistant seems drunk - all good - nothing to do here