Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

On-chain unresponsiveness reporting #1215

Merged
merged 16 commits into from
Dec 17, 2024
Merged
4 changes: 4 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Binary file modified crates/client/entropy_metadata.scale
Binary file not shown.
2 changes: 2 additions & 0 deletions pallets/attestation/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ pallet-staking-reward-curve ={ version="11.0.0" }
tdx-quote ={ version="0.0.1", features=["mock"] }
rand_core ="0.6.4"

pallet-slashing={ version="0.3.0", path="../slashing", default-features=false }

[features]
default=['std']
runtime-benchmarks=['frame-benchmarking', 'tdx-quote/mock', 'pallet-session']
Expand Down
13 changes: 13 additions & 0 deletions pallets/attestation/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ frame_support::construct_runtime!(
Historical: pallet_session_historical,
BagsList: pallet_bags_list,
Parameters: pallet_parameters,
Slashing: pallet_slashing,
}
);

Expand Down Expand Up @@ -342,6 +343,18 @@ impl pallet_parameters::Config for Test {
type WeightInfo = ();
}

parameter_types! {
pub const ReportThreshold: u32 = 5;
}

impl pallet_slashing::Config for Test {
type RuntimeEvent = RuntimeEvent;
type AuthorityId = UintAuthorityId;
type ReportThreshold = ReportThreshold;
type ValidatorSet = Historical;
type ReportUnresponsiveness = ();
}

// Build genesis storage according to the mock runtime.
pub fn new_test_ext() -> sp_io::TestExternalities {
let mut t = system::GenesisConfig::<Test>::default().build_storage().unwrap();
Expand Down
5 changes: 3 additions & 2 deletions pallets/propagation/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,10 @@ sp-staking ={ version="27.0.0", default-features=false }
entropy-shared={ version="0.3.0", path="../../crates/shared", default-features=false, features=[
"wasm-no-std",
] }
pallet-registry={ version="0.3.0", path="../registry", default-features=false }
pallet-attestation={ version="0.3.0", path="../attestation", default-features=false }
pallet-programs={ version="0.3.0", path="../programs", default-features=false }
pallet-registry={ version="0.3.0", path="../registry", default-features=false }
pallet-staking-extension={ version="0.3.0", path="../staking", default-features=false }
pallet-attestation={ version="0.3.0", path="../attestation", default-features=false }

[dev-dependencies]
parking_lot="0.12.3"
Expand All @@ -49,6 +49,7 @@ sp-keystore ={ version="0.35.0" }
sp-npos-elections ={ version="27.0.0", default-features=false }
pallet-parameters ={ version="0.3.0", path="../parameters", default-features=false }
pallet-oracle ={ version='0.3.0', path='../oracle', default-features=false }
pallet-slashing ={ version="0.3.0", path="../slashing", default-features=false }

[features]
default=['std']
Expand Down
13 changes: 13 additions & 0 deletions pallets/propagation/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ frame_support::construct_runtime!(
Parameters: pallet_parameters,
Attestation: pallet_attestation,
Oracle: pallet_oracle,
Slashing: pallet_slashing,
}
);

Expand Down Expand Up @@ -394,6 +395,18 @@ impl pallet_attestation::Config for Test {
type Randomness = TestPastRandomness;
}

parameter_types! {
pub const ReportThreshold: u32 = 5;
}

impl pallet_slashing::Config for Test {
type RuntimeEvent = RuntimeEvent;
type AuthorityId = UintAuthorityId;
type ReportThreshold = ReportThreshold;
type ValidatorSet = Historical;
type ReportUnresponsiveness = ();
}

// Build genesis storage according to the mock runtime.
pub fn new_test_ext() -> sp_io::TestExternalities {
let mut t = system::GenesisConfig::<Test>::default().build_storage().unwrap();
Expand Down
1 change: 1 addition & 0 deletions pallets/registry/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ sp-io ={ version="31.0.0", default-features=false }
sp-npos-elections ={ version="27.0.0", default-features=false }
sp-staking ={ version="27.0.0", default-features=false }
pallet-oracle ={ version='0.3.0', path='../oracle', default-features=false }
pallet-slashing ={ version="0.3.0", path="../slashing", default-features=false }

[features]

Expand Down
13 changes: 13 additions & 0 deletions pallets/registry/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ frame_support::construct_runtime!(
Programs: pallet_programs,
Parameters: pallet_parameters,
Oracle: pallet_oracle,
Slashing: pallet_slashing,
}
);

Expand Down Expand Up @@ -380,6 +381,18 @@ impl pallet_parameters::Config for Test {
type WeightInfo = ();
}

parameter_types! {
pub const ReportThreshold: u32 = 5;
}

impl pallet_slashing::Config for Test {
type RuntimeEvent = RuntimeEvent;
type AuthorityId = UintAuthorityId;
type ReportThreshold = ReportThreshold;
type ValidatorSet = Historical;
type ReportUnresponsiveness = ();
}

// Build genesis storage according to the mock runtime.
pub fn new_test_ext() -> sp_io::TestExternalities {
let mut t = system::GenesisConfig::<Test>::default().build_storage().unwrap();
Expand Down
2 changes: 2 additions & 0 deletions pallets/staking/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ p256 ={ version="0.13.2", default-features=false, features=["ecdsa"
rand ={ version="0.8.5", default-features=false, features=["alloc"] }

pallet-parameters={ version="0.3.0", path="../parameters", default-features=false }
pallet-slashing={ version="0.3.0", path="../slashing", default-features=false }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure you are not going to run in to problems with needing the staking extension pallet to be a dependency of the slashing pallet in order to call chill? Or can you call chill on pallet-staking directly without having circular dependencies?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Discussed below

entropy-shared={ version="0.3.0", path="../../crates/shared", features=[
"wasm-no-std",
], default-features=false }
Expand Down Expand Up @@ -69,6 +70,7 @@ std=[
'pallet-balances/std',
'pallet-parameters/std',
'pallet-session/std',
'pallet-slashing/std',
'pallet-staking/std',
'scale-info/std',
'sp-consensus-babe/std',
Expand Down
42 changes: 42 additions & 0 deletions pallets/staking/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ use frame_support::{
};
use frame_system::{EventRecord, RawOrigin};
use pallet_parameters::{SignersInfo, SignersSize};
use pallet_slashing::Event as SlashingEvent;
use pallet_staking::{
Event as FrameStakingEvent, MaxNominationsOf, MaxValidatorsCount, Nominations,
Pallet as FrameStaking, RewardDestination, ValidatorPrefs,
Expand Down Expand Up @@ -56,6 +57,16 @@ fn assert_last_event_frame_staking<T: Config>(
assert_eq!(event, &system_event);
}

fn assert_last_event_slashing<T: Config>(
generic_event: <T as pallet_slashing::Config>::RuntimeEvent,
) {
let events = frame_system::Pallet::<T>::events();
let system_event: <T as frame_system::Config>::RuntimeEvent = generic_event.into();
// compare to the last event record
let EventRecord { event, .. } = &events[events.len() - 1];
assert_eq!(event, &system_event);
}

pub fn create_validators<T: Config>(
count: u32,
seed: u32,
Expand Down Expand Up @@ -518,6 +529,37 @@ benchmarks! {
verify {
assert!(NextSigners::<T>::get().is_some());
}

report_unstable_peer {
// We substract `2` here to give room to our test signers
let s in 0 .. (MAX_SIGNERS - 2) as u32;

let threshold_reporter: T::AccountId = whitelisted_caller();
let threshold_offender: T::AccountId = account("threshold_offender", 0, SEED);

let reporter_validator_id = <T as pallet_session::Config>::ValidatorId::try_from(threshold_reporter.clone())
.or(Err(Error::<T>::InvalidValidatorId))
.unwrap();

let offender_validator_id = <T as pallet_session::Config>::ValidatorId::try_from(threshold_offender.clone())
.or(Err(Error::<T>::InvalidValidatorId))
.unwrap();

ThresholdToStash::<T>::insert(&threshold_reporter, &reporter_validator_id);
ThresholdToStash::<T>::insert(&threshold_offender, &offender_validator_id);

let mut signers = vec![reporter_validator_id.clone(); s as usize];
signers.push(reporter_validator_id);
signers.push(offender_validator_id);

Signers::<T>::put(signers.clone());

}: _(RawOrigin::Signed(threshold_reporter.clone()), threshold_offender.clone())
verify {
assert_last_event_slashing::<T>(
pallet_slashing::Event::NoteReport(threshold_reporter, threshold_offender).into(),
);
}
}

impl_benchmark_test_suite!(Staking, crate::mock::new_test_ext(), crate::mock::Test);
53 changes: 53 additions & 0 deletions pallets/staking/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ pub mod pallet {
+ frame_system::Config
+ pallet_staking::Config
+ pallet_parameters::Config
+ pallet_slashing::Config
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know this is gonna trigger Jesse, but in a future PR I want to move this to be loosely coupled

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

idk I just think it makes the code more unreadable and less flexible but wtv

{
/// Because this pallet emits events, it depends on the runtime's definition of an event.
type RuntimeEvent: From<Event<Self>> + IsType<<Self as frame_system::Config>::RuntimeEvent>;
Expand Down Expand Up @@ -336,6 +337,7 @@ pub mod pallet {
InvalidValidatorId,
SigningGroupError,
TssAccountAlreadyExists,
NotSigner,
NotNextSigner,
ReshareNotInProgress,
AlreadyConfirmed,
Expand Down Expand Up @@ -735,6 +737,57 @@ pub mod pallet {
// signers see https://github.com/entropyxyz/entropy-core/issues/985
Ok(Pays::No.into())
}

/// An on-chain hook for TSS servers in the signing committee to report other TSS servers in
/// the committee for misbehaviour.
///
/// Any "conequences" are handled by the configured Slashing pallet and not this pallet
/// itself.
#[pallet::call_index(7)]
#[pallet::weight(<T as Config>::WeightInfo::report_unstable_peer(MAX_SIGNERS as u32))]
pub fn report_unstable_peer(
origin: OriginFor<T>,
offender_tss_account: T::AccountId,
) -> DispatchResultWithPostInfo {
let reporter_tss_account = ensure_signed(origin)?;

// For reporting purposes we need to know the validator account tied to the TSS account.
let reporter_validator_id = Self::threshold_to_stash(&reporter_tss_account)
.ok_or(Error::<T>::NoThresholdKey)?;
let offender_validator_id = Self::threshold_to_stash(&offender_tss_account)
.ok_or(Error::<T>::NoThresholdKey)?;

// Note: This operation is O(n), but with a small enough Signer group this should be
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like a formal justification for the capitilization of Signer.

Suggested change
// Note: This operation is O(n), but with a small enough Signer group this should be
// Note: This operation is O(n), but with a small enough signer group this should be

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The storage struct is called Signers, QED? lol

Pls don't make me run CI again for this 😅

// fine to do on-chain.
let signers = Self::signers();
ensure!(signers.contains(&reporter_validator_id), Error::<T>::NotSigner);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This means relayers cannot use this to report that they were unable to relay a signature request to a member of the signing committee. Which is fine for now but maybe at some point we will want a separate extrinsic for that purpose.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes that's a good point. I meant to bring it up in the PR description but forgot. The
essence of the question here is whether or not we give relayers the power to report. I'm
not entirely sure yet, but it's something we can decide on later

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Opened #1223

ensure!(signers.contains(&offender_validator_id), Error::<T>::NotSigner);

// We do a bit of a weird conversion here since we want the validator's underlying
// `AccountId` for the reporting mechanism, not their `ValidatorId`.
//
// The Session pallet should have this configured to be the same thing, but we can't
// prove that to the compiler.
let encoded_validator_id = T::ValidatorId::encode(&reporter_validator_id);
let reporter_validator_account = T::AccountId::decode(&mut &encoded_validator_id[..])
.expect("A `ValidatorId` should be equivalent to an `AccountId`.");

let encoded_validator_id = T::ValidatorId::encode(&offender_validator_id);
let offending_peer_validator_account =
T::AccountId::decode(&mut &encoded_validator_id[..])
.expect("A `ValidatorId` should be equivalent to an `AccountId`.");

// We don't actually take any action here, we offload the reporting to the Slashing
// pallet.
pallet_slashing::Pallet::<T>::note_report(
reporter_validator_account,
offending_peer_validator_account,
)?;

let actual_weight =
<T as Config>::WeightInfo::report_unstable_peer(signers.len() as u32);
Ok(Some(actual_weight).into())
}
}

impl<T: Config> Pallet<T> {
Expand Down
38 changes: 38 additions & 0 deletions pallets/staking/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ frame_support::construct_runtime!(
Historical: pallet_session_historical,
BagsList: pallet_bags_list,
Parameters: pallet_parameters,
Slashing: pallet_slashing,
}
);

Expand Down Expand Up @@ -418,6 +419,43 @@ impl entropy_shared::AttestationHandler<AccountId> for MockAttestationHandler {
fn request_quote(_attestee: &AccountId, _nonce: [u8; 32]) {}
}

type IdentificationTuple = (u64, pallet_staking::Exposure<AccountId, Balance>);
type Offence = pallet_slashing::UnresponsivenessOffence<IdentificationTuple>;

parameter_types! {
pub static Offences: Vec<Offence> = vec![];
}

/// A mock offence report handler.
pub struct OffenceHandler;
impl sp_staking::offence::ReportOffence<AccountId, IdentificationTuple, Offence>
for OffenceHandler
{
fn report_offence(
_reporters: Vec<u64>,
offence: Offence,
) -> Result<(), sp_staking::offence::OffenceError> {
Offences::mutate(|l| l.push(offence));
Ok(())
}

fn is_known_offence(_offenders: &[IdentificationTuple], _time_slot: &SessionIndex) -> bool {
false
}
}

parameter_types! {
pub const ReportThreshold: u32 = 5;
}

impl pallet_slashing::Config for Test {
type RuntimeEvent = RuntimeEvent;
type AuthorityId = UintAuthorityId;
type ReportThreshold = ReportThreshold;
type ValidatorSet = Historical;
type ReportUnresponsiveness = OffenceHandler;
}

impl pallet_staking_extension::Config for Test {
type AttestationHandler = MockAttestationHandler;
type Currency = Balances;
Expand Down
Loading
Loading