Skip to content
This repository was archived by the owner on Nov 15, 2023. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
d0b6d95
Experiments with common equivocation trait
davxy Feb 13, 2023
5754be6
Improved equivocation trait
davxy Feb 15, 2023
9029a5c
Fix grandpa equivocation implementation
davxy Feb 15, 2023
0e04ca0
Remove some cruft
davxy Feb 16, 2023
d8c2598
Remove some more cruft
davxy Feb 16, 2023
1674a48
More generic naming
davxy Feb 16, 2023
6f0feff
Simplification of offences manipilation
davxy Feb 16, 2023
239bfbb
More refactory
davxy Feb 17, 2023
4ba1a01
Some prograss with the encapsulation of offence report system
davxy Feb 18, 2023
7c1c06d
Finally unit type works as a universal null report system
davxy Feb 20, 2023
1069c9d
Align substrate node code
davxy Feb 20, 2023
e096669
Further simplification
davxy Feb 20, 2023
09850ee
Fix test utils
davxy Feb 20, 2023
020e71c
Remove not required associated type
davxy Feb 20, 2023
4155c88
Fix benches
davxy Feb 20, 2023
2a3ed07
Rollback to prev field name
davxy Feb 20, 2023
5545901
Merge branch 'master' into equivocation-offence-rework
davxy Feb 20, 2023
c8a8eb6
Box big params
davxy Feb 20, 2023
9e959b7
Fix typo
davxy Feb 20, 2023
6e7e11d
Remove new tag computation
davxy Feb 20, 2023
080e82c
Remove default implementations
davxy Feb 21, 2023
3966ef7
Better docs
davxy Feb 21, 2023
5587720
Return 'Result' instead of bool
davxy Feb 23, 2023
52ee367
Change offence report system return types
davxy Feb 24, 2023
8706a6d
Some renaming and documentation
davxy Feb 24, 2023
7bebe84
Improve documentation
davxy Feb 24, 2023
f9a505b
More abstract offence report system
davxy Feb 24, 2023
aa83011
Rename 'consume_evidence' to 'process_evidence'
davxy Feb 24, 2023
0c5a536
Further docs refinements
davxy Feb 24, 2023
46b9f86
Doc for dummy offence report
davxy Feb 24, 2023
f05c4ea
Merge branch 'master' into equivocation-offence-rework
davxy Feb 24, 2023
3ff71d1
Fix rustdoc
davxy Feb 24, 2023
78f0e9f
Merge branch 'master' into equivocation-offence-rework
davxy Mar 7, 2023
d336264
Fix after master merge
davxy Mar 7, 2023
2e1c338
Apply code review suggestions
davxy Mar 7, 2023
643c745
Improve docs
davxy Mar 7, 2023
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
Prev Previous commit
Next Next commit
Box big params
  • Loading branch information
davxy committed Feb 20, 2023
commit c8a8eb64ad096233dc1b13f7185e64f693cae5aa
13 changes: 9 additions & 4 deletions bin/node/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,11 @@ mod voter_bags;
#[cfg(feature = "std")]
include!(concat!(env!("OUT_DIR"), "/wasm_binary.rs"));

/// Max size for serialized extrinsic params for this testing runtime.
/// This is a quite arbitrary but empirically battle tested value.
#[cfg(test)]
pub const CALL_PARAMS_MAX_SIZE: usize = 208;

/// Wasm binary unwrapped. If built with `SKIP_WASM_BUILD`, the function panics.
#[cfg(feature = "std")]
pub fn wasm_binary_unwrap() -> &'static [u8] {
Expand Down Expand Up @@ -2352,10 +2357,10 @@ mod tests {
fn call_size() {
let size = core::mem::size_of::<RuntimeCall>();
assert!(
size <= 208,
"size of RuntimeCall {} is more than 208 bytes: some calls have too big arguments, use Box to reduce the
size of RuntimeCall.
If the limit is too strong, maybe consider increase the limit to 300.",
size <= CALL_PARAMS_MAX_SIZE,
"size of RuntimeCall {} is more than {CALL_PARAMS_MAX_SIZE} bytes.
Some calls have too big arguments, use Box to reduce the size of RuntimeCall.
If the limit is too strong, maybe consider fine tune the limit.",
size,
);
}
Expand Down
12 changes: 7 additions & 5 deletions frame/babe/src/equivocation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,6 @@ impl<Offender: Clone> Offence<Offender> for EquivocationOffence<Offender> {
/// using existing subsystems that are part of frame (type bounds described
/// below) and will dispatch to them directly, it's only purpose is to wire all
/// subsystems together.
#[derive(Default)]
pub struct EquivocationReportSystem<T, R, P, L>(sp_std::marker::PhantomData<(T, R, P, L)>);

// We use the authorship pallet to fetch the current block author and use
Expand Down Expand Up @@ -142,15 +141,15 @@ where
let epoch_index =
*slot.saturating_sub(crate::GenesisSlot::<T>::get()) / T::EpochDuration::get();

// TODO DAVXY: CAN ALL THESE VALIDITY CHECK BE PERFORMED ONLY BY CHECK-EVIDENCE???

// Check that the slot number is consistent with the session index
// in the key ownership proof (i.e. slot is for that epoch)
// TODO: this should be part of `check_evidence`...
if Pallet::<T>::session_index_for_epoch(epoch_index) != session_index {
return Err(Error::<T>::InvalidKeyOwnershipProof.into())
}

// Check the membership proof and extract the offender's id
// TODO: These checks were alread yperformed by check evidence...
let offender = P::check_proof((KEY_TYPE, offender), key_owner_proof)
.ok_or(Error::<T>::InvalidKeyOwnershipProof)?;

Expand Down Expand Up @@ -185,7 +184,10 @@ where
) -> bool {
use frame_system::offchain::SubmitTransaction;

let call = Call::report_equivocation_unsigned { equivocation_proof, key_owner_proof };
let call = Call::report_equivocation_unsigned {
equivocation_proof: Box::new(equivocation_proof),
key_owner_proof,
};
let res = SubmitTransaction::<T, Call<T>>::submit_unsigned_transaction(call.into());
match res {
Ok(()) => info!(target: LOG_TARGET, "Submitted equivocation report."),
Expand Down Expand Up @@ -216,7 +218,7 @@ impl<T: Config> Pallet<T> {
}

// Check report validity
// TODO DAVXY: propagate error
// TODO DAVXY: propagate check error
T::EquivocationReportSystem::check_evidence(equivocation_proof, key_owner_proof)
.map_err(|_| InvalidTransaction::Stale)?;

Expand Down
12 changes: 6 additions & 6 deletions frame/babe/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -415,13 +415,13 @@ pub mod pallet {
))]
pub fn report_equivocation(
origin: OriginFor<T>,
equivocation_proof: EquivocationProof<T::Header>,
equivocation_proof: Box<EquivocationProof<T::Header>>,
key_owner_proof: T::KeyOwnerProof,
) -> DispatchResultWithPostInfo {
let reporter = Some(ensure_signed(origin)?);
let reporter = ensure_signed(origin)?;
T::EquivocationReportSystem::report_evidence(
reporter,
equivocation_proof,
Some(reporter),
*equivocation_proof,
key_owner_proof,
)?;
// Waive the fee since the report is valid and beneficial
Expand All @@ -442,13 +442,13 @@ pub mod pallet {
))]
pub fn report_equivocation_unsigned(
origin: OriginFor<T>,
equivocation_proof: EquivocationProof<T::Header>,
equivocation_proof: Box<EquivocationProof<T::Header>>,
key_owner_proof: T::KeyOwnerProof,
) -> DispatchResultWithPostInfo {
ensure_none(origin)?;
T::EquivocationReportSystem::report_evidence(
None,
equivocation_proof,
*equivocation_proof,
key_owner_proof,
)?;
// Waive the fee since the report is valid and beneficial
Expand Down
22 changes: 11 additions & 11 deletions frame/babe/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -465,7 +465,7 @@ fn report_equivocation_current_session_works() {
// report the equivocation
Babe::report_equivocation_unsigned(
RuntimeOrigin::none(),
equivocation_proof,
Box::new(equivocation_proof),
key_owner_proof,
)
.unwrap();
Expand Down Expand Up @@ -537,7 +537,7 @@ fn report_equivocation_old_session_works() {
// report the equivocation
Babe::report_equivocation_unsigned(
RuntimeOrigin::none(),
equivocation_proof,
Box::new(equivocation_proof),
key_owner_proof,
)
.unwrap();
Expand Down Expand Up @@ -589,7 +589,7 @@ fn report_equivocation_invalid_key_owner_proof() {
assert_err!(
Babe::report_equivocation_unsigned(
RuntimeOrigin::none(),
equivocation_proof.clone(),
Box::new(equivocation_proof.clone()),
key_owner_proof
),
Error::<Test>::InvalidKeyOwnershipProof,
Expand All @@ -609,7 +609,7 @@ fn report_equivocation_invalid_key_owner_proof() {
assert_err!(
Babe::report_equivocation_unsigned(
RuntimeOrigin::none(),
equivocation_proof,
Box::new(equivocation_proof),
key_owner_proof,
),
Error::<Test>::InvalidKeyOwnershipProof,
Expand Down Expand Up @@ -643,7 +643,7 @@ fn report_equivocation_invalid_equivocation_proof() {
assert_err!(
Babe::report_equivocation_unsigned(
RuntimeOrigin::none(),
equivocation_proof,
Box::new(equivocation_proof),
key_owner_proof.clone(),
),
Error::<Test>::InvalidEquivocationProof,
Expand Down Expand Up @@ -750,7 +750,7 @@ fn report_equivocation_validate_unsigned_prevents_duplicates() {
let key_owner_proof = Historical::prove(key).unwrap();

let inner = Call::report_equivocation_unsigned {
equivocation_proof: equivocation_proof.clone(),
equivocation_proof: Box::new(equivocation_proof.clone()),
key_owner_proof: key_owner_proof.clone(),
};

Expand Down Expand Up @@ -786,7 +786,7 @@ fn report_equivocation_validate_unsigned_prevents_duplicates() {
// we submit the report
Babe::report_equivocation_unsigned(
RuntimeOrigin::none(),
equivocation_proof,
Box::new(equivocation_proof),
key_owner_proof,
)
.unwrap();
Expand Down Expand Up @@ -870,7 +870,7 @@ fn report_equivocation_after_skipped_epochs_works() {
// between epoch index and session index must be checked.
assert!(Babe::report_equivocation_unsigned(
RuntimeOrigin::none(),
equivocation_proof,
Box::new(equivocation_proof),
key_owner_proof
)
.is_ok());
Expand All @@ -897,7 +897,7 @@ fn valid_equivocation_reports_dont_pay_fees() {

// check the dispatch info for the call.
let info = Call::<Test>::report_equivocation_unsigned {
equivocation_proof: equivocation_proof.clone(),
equivocation_proof: Box::new(equivocation_proof.clone()),
key_owner_proof: key_owner_proof.clone(),
}
.get_dispatch_info();
Expand All @@ -910,7 +910,7 @@ fn valid_equivocation_reports_dont_pay_fees() {
// report the equivocation.
let post_info = Babe::report_equivocation_unsigned(
RuntimeOrigin::none(),
equivocation_proof.clone(),
Box::new(equivocation_proof.clone()),
key_owner_proof.clone(),
)
.unwrap();
Expand All @@ -924,7 +924,7 @@ fn valid_equivocation_reports_dont_pay_fees() {
// duplicate.
let post_info = Babe::report_equivocation_unsigned(
RuntimeOrigin::none(),
equivocation_proof,
Box::new(equivocation_proof),
key_owner_proof,
)
.err()
Expand Down
6 changes: 4 additions & 2 deletions frame/grandpa/src/equivocation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,6 @@ impl<Offender: Clone> Offence<Offender> for EquivocationOffence<Offender> {
/// using existing subsystems that are part of frame (type bounds described
/// below) and will dispatch to them directly, it's only purpose is to wire all
/// subsystems together.
#[derive(Default)]
pub struct EquivocationReportSystem<T, R, P, L>(sp_std::marker::PhantomData<(T, R, P, L)>);

// We use the authorship pallet to fetch the current block author and use
Expand Down Expand Up @@ -221,7 +220,10 @@ where
) -> bool {
use frame_system::offchain::SubmitTransaction;

let call = Call::report_equivocation_unsigned { equivocation_proof, key_owner_proof };
let call = Call::report_equivocation_unsigned {
equivocation_proof: Box::new(equivocation_proof),
key_owner_proof,
};
let res = SubmitTransaction::<T, Call<T>>::submit_unsigned_transaction(call.into());
match res {
Ok(()) => info!(target: LOG_TARGET, "Submitted equivocation report."),
Expand Down
12 changes: 6 additions & 6 deletions frame/grandpa/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -196,14 +196,14 @@ pub mod pallet {
#[pallet::weight(T::WeightInfo::report_equivocation(key_owner_proof.validator_count()))]
pub fn report_equivocation(
origin: OriginFor<T>,
equivocation_proof: EquivocationProof<T::Hash, T::BlockNumber>,
equivocation_proof: Box<EquivocationProof<T::Hash, T::BlockNumber>>,
key_owner_proof: T::KeyOwnerProof,
) -> DispatchResultWithPostInfo {
let reporter = Some(ensure_signed(origin)?);
let reporter = ensure_signed(origin)?;

T::EquivocationReportSystem::report_evidence(
reporter,
equivocation_proof,
Some(reporter),
*equivocation_proof,
key_owner_proof,
)?;
// Waive the fee since the report is valid and beneficial
Expand All @@ -223,14 +223,14 @@ pub mod pallet {
#[pallet::weight(T::WeightInfo::report_equivocation(key_owner_proof.validator_count()))]
pub fn report_equivocation_unsigned(
origin: OriginFor<T>,
equivocation_proof: EquivocationProof<T::Hash, T::BlockNumber>,
equivocation_proof: Box<EquivocationProof<T::Hash, T::BlockNumber>>,
key_owner_proof: T::KeyOwnerProof,
) -> DispatchResultWithPostInfo {
ensure_none(origin)?;

T::EquivocationReportSystem::report_evidence(
None,
equivocation_proof,
*equivocation_proof,
key_owner_proof,
)?;
// Waive the fee since the report is valid and beneficial
Expand Down
22 changes: 11 additions & 11 deletions frame/grandpa/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,7 @@ fn report_equivocation_current_set_works() {
// report the equivocation and the tx should be dispatched successfully
assert_ok!(Grandpa::report_equivocation_unsigned(
RuntimeOrigin::none(),
equivocation_proof,
Box::new(equivocation_proof),
key_owner_proof,
),);

Expand Down Expand Up @@ -437,7 +437,7 @@ fn report_equivocation_old_set_works() {
// the old set, the tx should be dispatched successfully
assert_ok!(Grandpa::report_equivocation_unsigned(
RuntimeOrigin::none(),
equivocation_proof,
Box::new(equivocation_proof),
key_owner_proof,
),);

Expand Down Expand Up @@ -500,7 +500,7 @@ fn report_equivocation_invalid_set_id() {
assert_err!(
Grandpa::report_equivocation_unsigned(
RuntimeOrigin::none(),
equivocation_proof,
Box::new(equivocation_proof),
key_owner_proof,
),
Error::<Test>::InvalidEquivocationProof,
Expand Down Expand Up @@ -541,7 +541,7 @@ fn report_equivocation_invalid_session() {
assert_err!(
Grandpa::report_equivocation_unsigned(
RuntimeOrigin::none(),
equivocation_proof,
Box::new(equivocation_proof),
key_owner_proof,
),
Error::<Test>::InvalidEquivocationProof,
Expand Down Expand Up @@ -586,7 +586,7 @@ fn report_equivocation_invalid_key_owner_proof() {
assert_err!(
Grandpa::report_equivocation_unsigned(
RuntimeOrigin::none(),
equivocation_proof,
Box::new(equivocation_proof),
invalid_key_owner_proof,
),
Error::<Test>::InvalidKeyOwnershipProof,
Expand Down Expand Up @@ -617,7 +617,7 @@ fn report_equivocation_invalid_equivocation_proof() {
assert_err!(
Grandpa::report_equivocation_unsigned(
RuntimeOrigin::none(),
equivocation_proof,
Box::new(equivocation_proof),
key_owner_proof.clone(),
),
Error::<Test>::InvalidEquivocationProof,
Expand Down Expand Up @@ -687,7 +687,7 @@ fn report_equivocation_validate_unsigned_prevents_duplicates() {
Historical::prove((sp_finality_grandpa::KEY_TYPE, &equivocation_key)).unwrap();

let call = Call::report_equivocation_unsigned {
equivocation_proof: equivocation_proof.clone(),
equivocation_proof: Box::new(equivocation_proof.clone()),
key_owner_proof: key_owner_proof.clone(),
};

Expand Down Expand Up @@ -726,7 +726,7 @@ fn report_equivocation_validate_unsigned_prevents_duplicates() {
// we submit the report
Grandpa::report_equivocation_unsigned(
RuntimeOrigin::none(),
equivocation_proof,
Box::new(equivocation_proof),
key_owner_proof,
)
.unwrap();
Expand Down Expand Up @@ -879,7 +879,7 @@ fn valid_equivocation_reports_dont_pay_fees() {

// check the dispatch info for the call.
let info = Call::<Test>::report_equivocation_unsigned {
equivocation_proof: equivocation_proof.clone(),
equivocation_proof: Box::new(equivocation_proof.clone()),
key_owner_proof: key_owner_proof.clone(),
}
.get_dispatch_info();
Expand All @@ -891,7 +891,7 @@ fn valid_equivocation_reports_dont_pay_fees() {
// report the equivocation.
let post_info = Grandpa::report_equivocation_unsigned(
RuntimeOrigin::none(),
equivocation_proof.clone(),
Box::new(equivocation_proof.clone()),
key_owner_proof.clone(),
)
.unwrap();
Expand All @@ -905,7 +905,7 @@ fn valid_equivocation_reports_dont_pay_fees() {
// duplicate.
let post_info = Grandpa::report_equivocation_unsigned(
RuntimeOrigin::none(),
equivocation_proof,
Box::new(equivocation_proof),
key_owner_proof,
)
.err()
Expand Down