Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,11 @@ impl pallet_staking_async_rc_client::Config for Runtime {
type ValidatorSetExportSession = ConstU32<4>;
type RelayChainSessionKeys = RelayChainSessionKeys;
type Balance = Balance;
// Hardcoded anti-spam threshold: 10k WND active bond to set session keys.
// Prevents unbounded relay chain storage growth via bond-validate-set_keys-chill loops.
// For a testnet like Westend AH though, we just prioritize ease of testing so we set it
// ridiculously low. On Polkadot, this should be set to a more meaningful value.
type MinSetKeysBond = ConstU128<{ 10 * UNITS }>;
// | Key | Crypto | Public Key | Signature |
// |---------------------|---------|------------|-----------|
// | grandpa | Ed25519 | 32 bytes | 64 bytes |
Expand Down
16 changes: 16 additions & 0 deletions prdoc/pr_11168.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
title: Add MinSetKeysBond check in rc_client::set_keys to prevent relay chain storage
spam
doc:
- audience: Runtime Dev
description: "With minValidatorBond = 0, an attacker can bond ED then loop validate\
\ \u2192 set_keys \u2192 chill to store unlimited session keys on the relay chain\
\ at negligible cost.\nAdd a configurable MinSetKeysBond threshold (hardcoded\
\ to 10 WND on asset-hub-westend) that rejects set_keys when active bond\
\ is insufficient.\nSet to 0 to disable (useful e.g. for asset-hub-kusama)."
Comment thread
sigurpol marked this conversation as resolved.
Outdated
crates:
- name: asset-hub-westend-runtime
bump: major
- name: pallet-staking-async-rc-client
bump: major
- name: pallet-staking-async
bump: major
Original file line number Diff line number Diff line change
Expand Up @@ -491,6 +491,10 @@ frame::deps::sp_runtime::impl_opaque_keys! {
}
}

parameter_types! {
pub static MinSetKeysBond: Balance = 0;
}

impl pallet_staking_async_rc_client::Config for Runtime {
type AHStakingInterface = Staking;
type SendToRelayChain = DeliverToRelay;
Expand All @@ -499,6 +503,7 @@ impl pallet_staking_async_rc_client::Config for Runtime {
type ValidatorSetExportSession = ValidatorSetExportSession;
type RelayChainSessionKeys = RCSessionKeys;
type Balance = Balance;
type MinSetKeysBond = MinSetKeysBond;
type WeightInfo = ();
}

Expand Down
54 changes: 53 additions & 1 deletion substrate/frame/staking-async/integration-tests/src/ah/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1415,7 +1415,7 @@ mod poll_operations {
mod session_keys {
use super::*;
use crate::ah::mock::{
Balances, LocalQueue, OutgoingMessages, ProxyType, PurgeKeysExecutionCost,
Balances, LocalQueue, MinSetKeysBond, OutgoingMessages, ProxyType, PurgeKeysExecutionCost,
SetKeysExecutionCost,
};
use codec::Encode;
Expand Down Expand Up @@ -1881,6 +1881,58 @@ mod session_keys {
});
}

#[test]
fn set_keys_insufficient_bond() {
ExtBuilder::default().local_queue().build().execute_with(|| {
let validator: AccountId = 1;
let (keys, proof) = make_session_keys_and_proof(validator);

// GIVEN: MinSetKeysBond is set higher than the validator's active bond (100)
MinSetKeysBond::set(101);

// WHEN: Validator tries to set keys
// THEN: InsufficientBond error is returned
assert_noop!(
rc_client::Pallet::<T>::set_keys(
RuntimeOrigin::signed(validator),
keys.clone(),
proof.clone(),
None,
),
rc_client::Error::<T>::InsufficientBond
);

// GIVEN: MinSetKeysBond equals the validator's active bond
MinSetKeysBond::set(100);

// WHEN: Validator sets keys with exact bond
// THEN: Succeeds
assert_ok!(rc_client::Pallet::<T>::set_keys(
RuntimeOrigin::signed(validator),
keys.clone(),
proof.clone(),
None,
));
});
}

#[test]
fn set_keys_min_bond_zero_disables_check() {
ExtBuilder::default().local_queue().build().execute_with(|| {
// GIVEN: MinSetKeysBond is 0 (default in tests) — check is disabled
let validator: AccountId = 1;
let (keys, proof) = make_session_keys_and_proof(validator);

// WHEN/THEN: set_keys succeeds regardless of bond amount
assert_ok!(rc_client::Pallet::<T>::set_keys(
RuntimeOrigin::signed(validator),
keys,
proof,
None,
));
});
}

/// End-to-end test: set keys on AssetHub, verify on RelayChain, then purge and verify.
#[test]
fn set_and_purge_keys_e2e() {
Expand Down
30 changes: 29 additions & 1 deletion substrate/frame/staking-async/rc-client/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -835,6 +835,8 @@ where
pub trait AHStakingInterface {
/// The validator account id type.
type AccountId;
/// The balance type.
type Balance: BalanceTrait;
/// Maximum number of validators that the staking system may have.
type MaxValidatorSet: Get<u32>;

Expand Down Expand Up @@ -868,6 +870,9 @@ pub trait AHStakingInterface {
///
/// Returns true if the account has called `validate()` and is in the `Validators` storage.
fn is_validator(who: &Self::AccountId) -> bool;

/// Returns the active bonded amount for a stash, or `None` if not bonded.
fn active_bond(who: &Self::AccountId) -> Option<Self::Balance>;
Comment thread
sigurpol marked this conversation as resolved.
Outdated
}

/// The communication trait of `pallet-staking-async` -> `pallet-staking-async-rc-client`.
Expand Down Expand Up @@ -1010,7 +1015,10 @@ pub mod pallet {
type RelayChainOrigin: EnsureOrigin<Self::RuntimeOrigin>;

/// Our communication handle to the local staking pallet.
type AHStakingInterface: AHStakingInterface<AccountId = Self::AccountId>;
type AHStakingInterface: AHStakingInterface<
AccountId = Self::AccountId,
Balance = Self::Balance,
>;

/// Our communication handle to the relay chain.
type SendToRelayChain: SendToRelayChain<
Expand Down Expand Up @@ -1058,6 +1066,16 @@ pub mod pallet {
/// The balance type used for delivery fee limits.
type Balance: BalanceTrait;

/// Minimum active bond required to call `set_keys`.
///
/// Prevents relay chain storage spam: without this, an attacker could bond the
/// existential deposit, call `validate → set_keys → chill` in a loop, storing
/// unlimited session keys on the relay chain at negligible cost.
///
/// Set to 0 to disable the check.
#[pallet::constant]
type MinSetKeysBond: Get<BalanceOf<Self>>;

/// Weight information for extrinsics in this pallet.
type WeightInfo: WeightInfo;
}
Expand All @@ -1078,6 +1096,8 @@ pub mod pallet {
InvalidProof,
/// Delivery fees exceeded the specified maximum.
FeesExceededMax,
/// The stash's active bond is below `MinSetKeysBond`.
InsufficientBond,
}

#[pallet::event]
Expand Down Expand Up @@ -1297,6 +1317,14 @@ pub mod pallet {
// Only registered validators can set session keys
ensure!(T::AHStakingInterface::is_validator(&stash), Error::<T>::NotValidator);

// Ensure active bond meets the minimum to prevent RC storage spam
let min_bond = T::MinSetKeysBond::get();
if !min_bond.is_zero() {
let active = T::AHStakingInterface::active_bond(&stash)
.ok_or(Error::<T>::InsufficientBond)?;
ensure!(active >= min_bond, Error::<T>::InsufficientBond);
}

// Validate keys: decode as RelayChainSessionKeys to ensure correct format
let session_keys = T::RelayChainSessionKeys::decode(&mut &keys[..])
.map_err(|_| Error::<T>::InvalidKeys)?;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -487,6 +487,7 @@ impl pallet_staking_async_rc_client::Config for Runtime {
type ValidatorSetExportSession = ConstU32<4>;
type RelayChainSessionKeys = RelayChainSessionKeys;
type Balance = Balance;
type MinSetKeysBond = ConstU128<{ 10_000 * UNITS }>;
type WeightInfo = ();
}

Expand Down
5 changes: 5 additions & 0 deletions substrate/frame/staking-async/src/pallet/impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1112,6 +1112,7 @@ impl<T: Config> ElectionDataProvider for Pallet<T> {

impl<T: Config> rc_client::AHStakingInterface for Pallet<T> {
type AccountId = T::AccountId;
type Balance = BalanceOf<T>;
type MaxValidatorSet = T::MaxValidatorSet;

/// When we receive a session report from the relay chain, it kicks off the next session.
Expand Down Expand Up @@ -1345,6 +1346,10 @@ impl<T: Config> rc_client::AHStakingInterface for Pallet<T> {
fn is_validator(who: &Self::AccountId) -> bool {
Validators::<T>::contains_key(who)
}

fn active_bond(who: &Self::AccountId) -> Option<BalanceOf<T>> {
Self::ledger(StakingAccount::Stash(who.clone())).ok().map(|l| l.active)
}
}

impl<T: Config> ScoreProvider<T::AccountId> for Pallet<T> {
Expand Down
Loading