Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Multi-phase elections solution resubmission #8290

Merged
54 commits merged into from
May 3, 2021
Merged
Show file tree
Hide file tree
Changes from 50 commits
Commits
Show all changes
54 commits
Select commit Hold shift + click to select a range
62e1854
not climate
coriolinus Mar 5, 2021
9a8c4a2
explain the intent of the bool in the unsigned phase
coriolinus Mar 5, 2021
19d7251
remove glob imports from unsigned.rs
coriolinus Mar 5, 2021
02ad14b
add OffchainRepeat parameter to ElectionProviderMultiPhase
coriolinus Mar 5, 2021
f6a0fd9
migrate core logic from #7976
coriolinus Mar 8, 2021
59a0fb4
improve formatting
coriolinus Mar 8, 2021
4612d07
fix test build failures
coriolinus Mar 8, 2021
c8ccbe9
cause test to pass
coriolinus Mar 10, 2021
7e4408f
Merge remote-tracking branch 'origin/master' into prgn-runtime-resubmit
coriolinus Mar 10, 2021
f4b50fc
Apply suggestions from code review
coriolinus Mar 16, 2021
a192ba3
collapse imports
coriolinus Mar 16, 2021
81ceca1
threshold acquired directly within try_acquire_offchain_lock
coriolinus Mar 16, 2021
9023b61
Merge remote-tracking branch 'origin/master' into prgn-runtime-resubmit
coriolinus Mar 16, 2021
449b799
add test of resubmission after interval
coriolinus Mar 17, 2021
85f715e
add test that ocw can regenerate a failed cache when resubmitting
coriolinus Mar 17, 2021
c6ecab8
ensure that OCW solutions are of the correct round
coriolinus Mar 19, 2021
ad3b786
add test of pre-dispatch round check
coriolinus Mar 19, 2021
bee456c
Merge remote-tracking branch 'origin/master' into prgn-runtime-resubmit
coriolinus Mar 19, 2021
89cf445
use `RawSolution.round` instead of redundantly externally
coriolinus Mar 22, 2021
458682b
unpack imports
coriolinus Mar 22, 2021
d7dbcc0
rename `OFFCHAIN_HEAD_DB` -> `OFFCHAIN_LOCK`
coriolinus Mar 22, 2021
b3e4950
rename `mine_call` -> `mine_checked_call`
coriolinus Mar 22, 2021
0b77268
eliminate extraneous comma
coriolinus Mar 22, 2021
576f87c
check cached call is current before submitting
coriolinus Mar 22, 2021
d0f031a
Merge remote-tracking branch 'origin/master' into prgn-runtime-resubmit
coriolinus Mar 22, 2021
681e6f0
remove unused consts introduced by bad merge.
coriolinus Mar 24, 2021
0fac518
Merge remote-tracking branch 'origin/master' into prgn-runtime-resubmit
coriolinus Mar 25, 2021
276a06a
Merge branch 'prgn-runtime-resubmit' of github.com:paritytech/substra…
coriolinus Mar 25, 2021
ea3fc29
resubmit when our solution beats queued solution
coriolinus Mar 29, 2021
cc70d37
clear call cache if solution fails to submit
coriolinus Mar 29, 2021
4b46a93
use local storage; clear on ElectionFinalized
coriolinus Mar 29, 2021
224d5e1
Revert "use local storage; clear on ElectionFinalized"
coriolinus Mar 29, 2021
c5668e9
BROKEN: try to filter local events in OCW
coriolinus Mar 29, 2021
e9305be
use local storage; simplify score fetching
coriolinus Mar 29, 2021
fe84141
fix event filter
coriolinus Mar 29, 2021
8b31773
mutate storage instead of setting it
coriolinus Mar 29, 2021
9978693
StorageValueRef::local isn't actually implemented yet
coriolinus Mar 30, 2021
ebcdeaa
add logging for some events of interest in OCW miner
coriolinus Mar 30, 2021
0236288
Merge remote-tracking branch 'origin/master' into prgn-runtime-resubmit
coriolinus Mar 30, 2021
bd7a195
rename kill_solution -> kill_ocw_solution to avoid ambiguity
coriolinus Apr 1, 2021
1c896be
defensive err instead of unreachable given unreachable code
coriolinus Apr 1, 2021
2243501
doc punctuation
coriolinus Apr 1, 2021
2b5d050
distinguish miner errors between "out of date" and "call invalid"
coriolinus Apr 1, 2021
5eb2673
downgrade info logs -> debug
coriolinus Apr 1, 2021
f12d59d
Merge remote-tracking branch 'origin/master' into prgn-runtime-resubmit
coriolinus Apr 19, 2021
70b4f71
ensure encoded call decodes as a call
coriolinus Apr 20, 2021
9c28048
fix semantics of validation of pre-dispatch failure for wrong round
coriolinus Apr 20, 2021
3f1109b
move score check within `and_then`
coriolinus Apr 20, 2021
af2d2c6
add test that offchain workers clear their cache after election
coriolinus Apr 20, 2021
fc71d3d
ensure that bad ocw submissions are not retained for resubmission
coriolinus Apr 21, 2021
840bad3
simplify fn ocw_solution_exists
coriolinus Apr 23, 2021
8eaf481
add feasibility check when restoring cached solution
coriolinus Apr 23, 2021
f4b52f0
simplify checks again
coriolinus Apr 23, 2021
6ae73c4
Merge remote-tracking branch 'origin/master' into prgn-runtime-resubmit
coriolinus May 3, 2021
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
2 changes: 2 additions & 0 deletions bin/node/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -471,6 +471,7 @@ parameter_types! {
pub const SlashDeferDuration: pallet_staking::EraIndex = 24 * 7; // 1/4 the bonding duration.
pub const RewardCurve: &'static PiecewiseLinear<'static> = &REWARD_CURVE;
pub const MaxNominatorRewardedPerValidator: u32 = 256;
pub OffchainRepeat: BlockNumber = 5;
kianenigma marked this conversation as resolved.
Show resolved Hide resolved
}

impl pallet_staking::Config for Runtime {
Expand Down Expand Up @@ -542,6 +543,7 @@ impl pallet_election_provider_multi_phase::Config for Runtime {
type SignedPhase = SignedPhase;
type UnsignedPhase = UnsignedPhase;
type SolutionImprovementThreshold = SolutionImprovementThreshold;
type OffchainRepeat = OffchainRepeat;
type MinerMaxIterations = MinerMaxIterations;
type MinerMaxWeight = MinerMaxWeight;
type MinerMaxLength = MinerMaxLength;
Expand Down
82 changes: 60 additions & 22 deletions frame/election-provider-multi-phase/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -194,10 +194,6 @@
//! **Score based on (byte) size**: We should always prioritize small solutions over bigger ones, if
//! there is a tie. Even more harsh should be to enforce the bound of the `reduce` algorithm.
//!
//! **Offchain resubmit**: Essentially port <https://github.com/paritytech/substrate/pull/7976> to
//! this pallet as well. The `OFFCHAIN_REPEAT` also needs to become an adjustable parameter of the
//! pallet.
//!
//! **Make the number of nominators configurable from the runtime**. Remove `sp_npos_elections`
//! dependency from staking and the compact solution type. It should be generated at runtime, there
//! it should be encoded how many votes each nominators have. Essentially translate
Expand All @@ -224,7 +220,7 @@ use frame_support::{
use frame_system::{ensure_none, offchain::SendTransactionTypes};
use frame_election_provider_support::{ElectionDataProvider, ElectionProvider, onchain};
use sp_npos_elections::{
assignment_ratio_to_staked_normalized, is_score_better, CompactSolution, ElectionScore,
assignment_ratio_to_staked_normalized, CompactSolution, ElectionScore,
EvaluateSupport, PerThing128, Supports, VoteWeight,
};
use sp_runtime::{
Expand All @@ -235,7 +231,10 @@ use sp_runtime::{
DispatchError, PerThing, Perbill, RuntimeDebug, SaturatedConversion,
traits::Bounded,
};
use sp_std::prelude::*;
use sp_std::{
convert::TryInto,
prelude::*,
};
use sp_arithmetic::{
UpperOf,
traits::{Zero, CheckedAdd},
Expand Down Expand Up @@ -304,8 +303,16 @@ pub enum Phase<Bn> {
Off,
/// Signed phase is open.
Signed,
/// Unsigned phase. First element is whether it is open or not, second the starting block
/// Unsigned phase. First element is whether it is active or not, second the starting block
/// number.
///
/// We do not yet check whether the unsigned phase is active or passive. The intent is for the
/// blockchain to be able to declare: "I believe that there exists an adequate signed solution,"
/// advising validators not to bother running the unsigned offchain worker.
///
/// As validator nodes are free to edit their OCW code, they could simply ignore this advisory
/// and always compute their own solution. However, by default, when the unsigned phase is passive,
/// the offchain workers will not bother running.
gui1117 marked this conversation as resolved.
Show resolved Hide resolved
Unsigned((bool, Bn)),
}

Expand All @@ -316,27 +323,27 @@ impl<Bn> Default for Phase<Bn> {
}

impl<Bn: PartialEq + Eq> Phase<Bn> {
/// Weather the phase is signed or not.
/// Whether the phase is signed or not.
pub fn is_signed(&self) -> bool {
matches!(self, Phase::Signed)
}

/// Weather the phase is unsigned or not.
/// Whether the phase is unsigned or not.
pub fn is_unsigned(&self) -> bool {
matches!(self, Phase::Unsigned(_))
}

/// Weather the phase is unsigned and open or not, with specific start.
/// Whether the phase is unsigned and open or not, with specific start.
pub fn is_unsigned_open_at(&self, at: Bn) -> bool {
matches!(self, Phase::Unsigned((true, real)) if *real == at)
}

/// Weather the phase is unsigned and open or not.
/// Whether the phase is unsigned and open or not.
pub fn is_unsigned_open(&self) -> bool {
matches!(self, Phase::Unsigned((true, _)))
}

/// Weather the phase is off or not.
/// Whether the phase is off or not.
coriolinus marked this conversation as resolved.
Show resolved Hide resolved
pub fn is_off(&self) -> bool {
matches!(self, Phase::Off)
}
Expand Down Expand Up @@ -512,7 +519,7 @@ pub mod pallet {

#[pallet::config]
pub trait Config: frame_system::Config + SendTransactionTypes<Call<Self>> {
type Event: From<Event<Self>> + IsType<<Self as frame_system::Config>::Event>;
type Event: From<Event<Self>> + IsType<<Self as frame_system::Config>::Event> + TryInto<Event<Self>>;

/// Currency type.
type Currency: ReservableCurrency<Self::AccountId> + Currency<Self::AccountId>;
Expand All @@ -529,6 +536,13 @@ pub mod pallet {
#[pallet::constant]
type SolutionImprovementThreshold: Get<Perbill>;

/// The repeat threshold of the offchain worker.
///
/// For example, if it is 5, that means that at least 5 blocks will elapse between attempts
/// to submit the worker's solution.
#[pallet::constant]
type OffchainRepeat: Get<Self::BlockNumber>;

/// The priority of the unsigned transaction submitted in the unsigned-phase
type MinerTxPriority: Get<TransactionPriority>;
/// Maximum number of iteration of balancing that will be executed in the embedded miner of
Expand Down Expand Up @@ -638,16 +652,38 @@ pub mod pallet {
}
}

fn offchain_worker(n: T::BlockNumber) {
// We only run the OCW in the first block of the unsigned phase.
if Self::current_phase().is_unsigned_open_at(n) {
match Self::try_acquire_offchain_lock(n) {
Ok(_) => {
let outcome = Self::mine_check_and_submit().map_err(ElectionError::from);
log!(info, "mine_check_and_submit execution done: {:?}", outcome);
}
Err(why) => log!(warn, "denied offchain worker: {:?}", why),
fn offchain_worker(now: T::BlockNumber) {
match Self::current_phase() {
Phase::Unsigned((true, opened)) if opened == now => {
// mine a new solution, cache it, and attempt to submit it
let initial_output = Self::try_acquire_offchain_lock(now)
.and_then(|_| Self::mine_check_save_submit());
log!(info, "initial OCW output at {:?}: {:?}", now, initial_output);
}
Phase::Unsigned((true, opened)) if opened < now => {
// keep trying to submit solutions. worst case, we note that the stored solution
// is better than our cached/computed one, and decide not to submit after all.
//
// the offchain_lock prevents us from spamming submissions too often.
let resubmit_output = Self::try_acquire_offchain_lock(now)
.and_then(|_| Self::restore_or_compute_then_maybe_submit());
log!(info, "resubmit OCW output at {:?}: {:?}", now, resubmit_output);
}
_ => {}
}
// after election finalization, clear OCW solution storage
if <frame_system::Pallet<T>>::events()
kianenigma marked this conversation as resolved.
Show resolved Hide resolved
.into_iter()
.filter_map(|event_record| {
let local_event = <T as Config>::Event::from(event_record.event);
local_event.try_into().ok()
})
.find(|event| {
matches!(event, Event::ElectionFinalized(_))
})
.is_some()
{
unsigned::kill_ocw_solution::<T>();
}
}

Expand Down Expand Up @@ -784,6 +820,8 @@ pub mod pallet {
PreDispatchWrongWinnerCount,
/// Submission was too weak, score-wise.
PreDispatchWeakSubmission,
/// OCW submitted solution for wrong round
OcwCallWrongEra,
}

#[pallet::origin]
Expand Down
4 changes: 3 additions & 1 deletion frame/election-provider-multi-phase/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ frame_support::construct_runtime!(

pub(crate) type Balance = u64;
pub(crate) type AccountId = u64;
pub(crate) type BlockNumber = u32;
pub(crate) type VoterIndex = u32;
pub(crate) type TargetIndex = u16;

Expand Down Expand Up @@ -204,11 +205,11 @@ parameter_types! {
pub static MinerMaxIterations: u32 = 5;
pub static MinerTxPriority: u64 = 100;
pub static SolutionImprovementThreshold: Perbill = Perbill::zero();
pub static OffchainRepeat: BlockNumber = 5;
pub static MinerMaxWeight: Weight = BlockWeights::get().max_block;
pub static MinerMaxLength: u32 = 256;
pub static MockWeightInfo: bool = false;


pub static EpochLength: u64 = 30;
}

Expand Down Expand Up @@ -276,6 +277,7 @@ impl crate::Config for Runtime {
type SignedPhase = SignedPhase;
type UnsignedPhase = UnsignedPhase;
type SolutionImprovementThreshold = SolutionImprovementThreshold;
type OffchainRepeat = OffchainRepeat;
type MinerMaxIterations = MinerMaxIterations;
type MinerMaxWeight = MinerMaxWeight;
type MinerMaxLength = MinerMaxLength;
Expand Down
Loading