Skip to content

Commit

Permalink
[pallet-staking] Additional check for virtual stakers (#5985)
Browse files Browse the repository at this point in the history
closes #5791.

This is not strictly necessary but serves as a defensive check.

The staking pallet exposes
[apis](https://paritytech.github.io/polkadot-sdk/master/sp_staking/trait.StakingUnchecked.html#tymethod.virtual_bond)
that other runtime pallets (pallet-delegated-staking) can use to create
virtual stakers. However, there’s no way for pallet-staking to ensure
that the staker is truly keyless. If the caller (this is a trusted
caller so this would only happen due to a bug) registers an account with
a private key as a virtual_staker, these accounts could later interact
directly with pallet-staking dispatchables (such as
[bond_extra](https://paritytech.github.io/polkadot-sdk/master/pallet_staking/dispatchables/fn.bond_extra.html))
and bypass any locking mechanism. The check above ensures this scenario
can never occur by performing an integrity check.

---------

Co-authored-by: Bastian Köcher <git@kchr.de>
Co-authored-by: command-bot <>
  • Loading branch information
Ank4n and bkchr authored Nov 5, 2024
1 parent 3c6ea86 commit be26d62
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 3 deletions.
10 changes: 9 additions & 1 deletion substrate/frame/staking/src/pallet/impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1881,8 +1881,12 @@ impl<T: Config> StakingInterface for Pallet<T> {
}

/// Whether `who` is a virtual staker whose funds are managed by another pallet.
///
/// There is an assumption that, this account is keyless and managed by another pallet in the
/// runtime. Hence, it can never sign its own transactions.
fn is_virtual_staker(who: &T::AccountId) -> bool {
VirtualStakers::<T>::contains_key(who)
frame_system::Pallet::<T>::account_nonce(who).is_zero() &&
VirtualStakers::<T>::contains_key(who)
}

fn slash_reward_fraction() -> Perbill {
Expand Down Expand Up @@ -2103,6 +2107,10 @@ impl<T: Config> Pallet<T> {
Ledger::<T>::get(stash.clone()).unwrap().stash == stash,
"ledger corrupted for virtual staker"
);
ensure!(
frame_system::Pallet::<T>::account_nonce(&stash).is_zero(),
"virtual stakers are keyless and should not have any nonce"
);
let reward_destination = <Payee<T>>::get(stash.clone()).unwrap();
if let RewardDestination::Account(payee) = reward_destination {
ensure!(
Expand Down
4 changes: 2 additions & 2 deletions substrate/primitives/panic-handler/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ use std::{
cell::Cell,
io::{self, Write},
marker::PhantomData,
panic::{self, PanicHookInfo},
panic::{self, PanicInfo},
sync::LazyLock,
thread,
};
Expand Down Expand Up @@ -149,7 +149,7 @@ fn strip_control_codes(input: &str) -> std::borrow::Cow<str> {
}

/// Function being called when a panic happens.
fn panic_hook(info: &PanicHookInfo, report_url: &str, version: &str) {
fn panic_hook(info: &PanicInfo, report_url: &str, version: &str) {
let location = info.location();
let file = location.as_ref().map(|l| l.file()).unwrap_or("<unknown>");
let line = location.as_ref().map(|l| l.line()).unwrap_or(0);
Expand Down

0 comments on commit be26d62

Please sign in to comment.