From 2ba0ee6a0919de0d44dbcc709340a5284c721add Mon Sep 17 00:00:00 2001 From: kianenigma Date: Thu, 21 Jan 2021 08:16:41 +0000 Subject: [PATCH 01/18] boilerplate --- frame/staking/src/lib.rs | 13 +++++---- frame/staking/src/offchain_election.rs | 37 +++++++++++++++++++------- 2 files changed, 36 insertions(+), 14 deletions(-) diff --git a/frame/staking/src/lib.rs b/frame/staking/src/lib.rs index 6c0bbc33a4e3d..404f81885c8ae 100644 --- a/frame/staking/src/lib.rs +++ b/frame/staking/src/lib.rs @@ -1366,17 +1366,20 @@ decl_module! { /// Check if the current block number is the one at which the election window has been set /// to open. If so, it runs the offchain worker code. fn offchain_worker(now: T::BlockNumber) { - use offchain_election::{set_check_offchain_execution_status, compute_offchain_election}; + use offchain_election::{set_check_offchain_execution_status, compute_save_and_submit, }; if Self::era_election_status().is_open_at(now) { let offchain_status = set_check_offchain_execution_status::(now); if let Err(why) = offchain_status { log!(warn, "๐Ÿ’ธ skipping offchain worker in open election window due to [{}]", why); } else { - if let Err(e) = compute_offchain_election::() { - log!(error, "๐Ÿ’ธ Error in election offchain worker: {:?}", e); - } else { - log!(debug, "๐Ÿ’ธ Executed offchain worker thread without errors."); + match compute_save_and_submit::() { + Ok(_) => { + log!(info, "๐Ÿ’ธ Executed offchain worker thread without errors."); + }, + Err(why) => { + log!(error, "๐Ÿ’ธ Error in election offchain worker: {:?}", why); + } } } } diff --git a/frame/staking/src/offchain_election.rs b/frame/staking/src/offchain_election.rs index 2ab29b7105d9c..9a29f3b0d5606 100644 --- a/frame/staking/src/offchain_election.rs +++ b/frame/staking/src/offchain_election.rs @@ -23,7 +23,6 @@ use crate::{ }; use codec::Decode; use frame_support::{traits::Get, weights::Weight, IterableStorageMap}; -use frame_system::offchain::SubmitTransaction; use sp_npos_elections::{ to_support_map, EvaluateSupport, reduce, Assignment, ElectionResult, ElectionScore, ExtendedBalance, CompactSolution, @@ -32,6 +31,7 @@ use sp_runtime::{ offchain::storage::StorageValueRef, traits::TrailingZeroInput, PerThing, RuntimeDebug, }; use sp_std::{convert::TryInto, prelude::*}; +use frame_system::offchain::SubmitTransaction; /// Error types related to the offchain election machinery. #[derive(RuntimeDebug)] @@ -105,10 +105,27 @@ pub(crate) fn set_check_offchain_execution_status( } } +pub(crate) const OFFCHAIN_QUEUED_CALL: &[u8] = b"parity/staking-election/call"; + +pub(crate) fn save_solution(call: &Call) -> Result<(), OffchainElectionError> { + let storage = StorageValueRef::persistent(&OFFCHAIN_QUEUED_CALL); + // TODO: probably we should do this with a mutate to ensure atomicity. + storage.set(call); +} + +pub(crate) fn get_solution() -> Option> { + let storage = StorageValueRef::persistent(&OFFCHAIN_QUEUED_CALL); + storage.get().flatten() +} + +pub(crate) fn submit_soluton(call: Call) -> Result<(), OffchainElectionError>{ + SubmitTransaction::>::submit_unsigned_transaction(call.clone().into()) + .map_err(|_| OffchainElectionError::PoolSubmissionFailed) +} + /// The internal logic of the offchain worker of this module. This runs the phragmen election, -/// compacts and reduces the solution, computes the score and submits it back to the chain as an -/// unsigned transaction, without any signature. -pub(crate) fn compute_offchain_election() -> Result<(), OffchainElectionError> { +/// compacts and reduces the solution, computes the score returns a call that can be submitted back to the chain. +pub(crate) fn compute_offchain_election() -> Result, OffchainElectionError> { let iters = get_balancing_iters::(); // compute raw solution. Note that we use `OffchainAccuracy`. let ElectionResult { @@ -135,17 +152,19 @@ pub(crate) fn compute_offchain_election() -> Result<(), OffchainElect // defensive-only: current era can never be none except genesis. let current_era = >::current_era().unwrap_or_default(); - // send it. - let call = Call::submit_election_solution_unsigned( + Ok(Call::submit_election_solution_unsigned( winners, compact, score, current_era, size, - ).into(); + )) +} - SubmitTransaction::>::submit_unsigned_transaction(call) - .map_err(|_| OffchainElectionError::PoolSubmissionFailed) +pub(crate) fn compute_save_and_submit() -> Result<(), OffchainElectionError> { + let call = compute_offchain_election::()?; + save_solution::(&call)?; + submit_soluton::(call) } /// Get a random number of iterations to run the balancing. From 363d03a89ad097de0905e5b71a6d6b5a5123478c Mon Sep 17 00:00:00 2001 From: kianenigma Date: Mon, 25 Jan 2021 12:54:41 +0000 Subject: [PATCH 02/18] Finish --- frame/staking/src/lib.rs | 54 +++++++++---- frame/staking/src/offchain_election.rs | 104 ++++++++++++++++--------- frame/staking/src/tests.rs | 19 ++--- frame/staking/src/weights.rs | 6 +- 4 files changed, 121 insertions(+), 62 deletions(-) diff --git a/frame/staking/src/lib.rs b/frame/staking/src/lib.rs index 404f81885c8ae..3f2a43d2eaab1 100644 --- a/frame/staking/src/lib.rs +++ b/frame/staking/src/lib.rs @@ -1363,24 +1363,48 @@ decl_module! { consumed_weight } - /// Check if the current block number is the one at which the election window has been set - /// to open. If so, it runs the offchain worker code. + /// Offchain worker logic. fn offchain_worker(now: T::BlockNumber) { - use offchain_election::{set_check_offchain_execution_status, compute_save_and_submit, }; + use offchain_election::{ + set_check_offchain_execution_status, compute_save_and_submit, submit_queued, + OFFCHAIN_REPEAT, + }; - if Self::era_election_status().is_open_at(now) { - let offchain_status = set_check_offchain_execution_status::(now); - if let Err(why) = offchain_status { - log!(warn, "๐Ÿ’ธ skipping offchain worker in open election window due to [{}]", why); - } else { - match compute_save_and_submit::() { - Ok(_) => { - log!(info, "๐Ÿ’ธ Executed offchain worker thread without errors."); - }, - Err(why) => { - log!(error, "๐Ÿ’ธ Error in election offchain worker: {:?}", why); + let election_status = Self::era_election_status(); + if election_status.is_open_at(now) { + // If era election status is open at the current block, mine the solution, save it + // and submit it. + + // ensure that we don't run OCW in any case more at least with 5 blocks delay. + let threshold: T::BlockNumber = OFFCHAIN_REPEAT.into(); + match set_check_offchain_execution_status::(now, threshold) { + Ok(_) => { + match compute_save_and_submit::() { + Ok(_) => + log!(info, "๐Ÿ’ธ Executed offchain worker thread without errors."), + Err(why) => + log!(error, "๐Ÿ’ธ Error in election offchain worker: {:?}", why), } - } + }, + Err(why) => + log!(debug, "๐Ÿ’ธ skipping offchain worker in open election window due to [{:?}]", why), + } + } else if election_status.is_open() && Self::queued_elected().is_none() { + // If the election window is open, and we don't have a queued solution, if you have + // one stored in the OCW DB, the submit it. We again check to never run the OCW more + // than every 5 blocks. + let threshold: T::BlockNumber = OFFCHAIN_REPEAT.into(); + match set_check_offchain_execution_status::(now, threshold) { + Ok(_) => { + match submit_queued::() { + Ok(_) => + log!(info, "resubmitting a saved solution succeeded at {:?}", now), + Err(why) => + log!(error, "resubmitting a saved solution failed at {:?} because of {:?}", now, why), + } + }, + Err(why) => + log!(debug, "๐Ÿ’ธ skipping offchain worker in open election window due to [{:?}]", why), } } } diff --git a/frame/staking/src/offchain_election.rs b/frame/staking/src/offchain_election.rs index 9a29f3b0d5606..04d71c028f726 100644 --- a/frame/staking/src/offchain_election.rs +++ b/frame/staking/src/offchain_election.rs @@ -35,12 +35,11 @@ use frame_system::offchain::SubmitTransaction; /// Error types related to the offchain election machinery. #[derive(RuntimeDebug)] +#[cfg_attr(feature = "std", derive(PartialEq, Eq))] pub enum OffchainElectionError { /// election returned None. This means less candidate that minimum number of needed /// validators were present. The chain is in trouble and not much that we can do about it. ElectionFailed, - /// Submission to the transaction pool failed. - PoolSubmissionFailed, /// The snapshot data is not available. SnapshotUnavailable, /// Error from npos-election crate. This usually relates to compact operation. @@ -49,6 +48,20 @@ pub enum OffchainElectionError { InvalidWinner, /// A nominator is not available in the snapshot. NominatorSnapshotCorrupt, + /// Failed to write some data to the offchain worker storage. + DBWriteFailed, + /// An offchain thread was not executed because fork was detected (executed with a block + /// number less than the last record one). + Fork, + /// An offchain thread was not executed because the last executed one is too recent (less than + /// `OFFCHAIN_REPEAT`). + TooRecent, + /// An unreachable state was reached. Should never happen. + Unreachable, + /// Submission to the transaction pool failed. + PoolSubmissionFailed, + /// No solution is stored in the offchain DB. + SolutionUnavailable, } impl From for OffchainElectionError { @@ -73,33 +86,32 @@ pub(crate) const DEFAULT_LONGEVITY: u64 = 25; /// Returns `Ok(())` if offchain worker should happen, `Err(reason)` otherwise. pub(crate) fn set_check_offchain_execution_status( now: T::BlockNumber, -) -> Result<(), &'static str> { + threshold: T::BlockNumber, +) -> Result<(), OffchainElectionError> { let storage = StorageValueRef::persistent(&OFFCHAIN_HEAD_DB); - let threshold = T::BlockNumber::from(OFFCHAIN_REPEAT); - - let mutate_stat = - storage.mutate::<_, &'static str, _>(|maybe_head: Option>| { - match maybe_head { - Some(Some(head)) if now < head => Err("fork."), - Some(Some(head)) if now >= head && now <= head + threshold => { - Err("recently executed.") - } - Some(Some(head)) if now > head + threshold => { - // we can run again now. Write the new head. - Ok(now) - } - _ => { - // value doesn't exists. Probably this node just booted up. Write, and run - Ok(now) - } + + let mutate_stat = storage.mutate(|maybe_head: Option>| { + match maybe_head { + Some(Some(head)) if now < head => Err(OffchainElectionError::Fork), + Some(Some(head)) if now >= head && now <= head + threshold => { + Err(OffchainElectionError::TooRecent) } - }); + Some(Some(head)) if now > head + threshold => { + // we can allow again now. Write the new head. + Ok(now) + } + _ => { + // value doesn't exists. Probably this node just booted up. Write, and allow. + Ok(now) + } + } + }); match mutate_stat { // all good Ok(Ok(_)) => Ok(()), // failed to write. - Ok(Err(_)) => Err("failed to write to offchain db."), + Ok(Err(_)) => Err(OffchainElectionError::DBWriteFailed), // fork etc. Err(why) => Err(why), } @@ -107,24 +119,50 @@ pub(crate) fn set_check_offchain_execution_status( pub(crate) const OFFCHAIN_QUEUED_CALL: &[u8] = b"parity/staking-election/call"; -pub(crate) fn save_solution(call: &Call) -> Result<(), OffchainElectionError> { +pub(crate) fn save_solution(call: Call) -> Result<(), OffchainElectionError> { let storage = StorageValueRef::persistent(&OFFCHAIN_QUEUED_CALL); - // TODO: probably we should do this with a mutate to ensure atomicity. - storage.set(call); + let set_outcome = storage.mutate::<_, OffchainElectionError, _>(|maybe_call| { + match maybe_call { + // under any case, write the new value. This means that we don't need to clear the + // solution. Each era, it will be overwritten. + _ => Ok(call), + } + }); + + match set_outcome { + Ok(Ok(_)) => Ok(()), + // failed to write. + Ok(Err(_)) => Err(OffchainElectionError::DBWriteFailed), + _ => { + // Defensive only: should not happen. Inner mutate closure always returns ok. + Err(OffchainElectionError::Unreachable) + } + } } pub(crate) fn get_solution() -> Option> { - let storage = StorageValueRef::persistent(&OFFCHAIN_QUEUED_CALL); - storage.get().flatten() + StorageValueRef::persistent(&OFFCHAIN_QUEUED_CALL).get().flatten() } -pub(crate) fn submit_soluton(call: Call) -> Result<(), OffchainElectionError>{ - SubmitTransaction::>::submit_unsigned_transaction(call.clone().into()) +pub(crate) fn submit_queued() -> Result<(), OffchainElectionError> { + let saved = get_solution().ok_or(OffchainElectionError::SolutionUnavailable)?; + submit_solution::(saved) +} + +pub(crate) fn submit_solution(call: Call) -> Result<(), OffchainElectionError> { + SubmitTransaction::>::submit_unsigned_transaction(call.into()) .map_err(|_| OffchainElectionError::PoolSubmissionFailed) } +pub(crate) fn compute_save_and_submit() -> Result<(), OffchainElectionError> { + let call = compute_offchain_election::()?; + save_solution::(call.clone())?; + submit_solution::(call) +} + /// The internal logic of the offchain worker of this module. This runs the phragmen election, -/// compacts and reduces the solution, computes the score returns a call that can be submitted back to the chain. +/// compacts and reduces the solution, computes the score returns a call that can be submitted back +/// to the chain. pub(crate) fn compute_offchain_election() -> Result, OffchainElectionError> { let iters = get_balancing_iters::(); // compute raw solution. Note that we use `OffchainAccuracy`. @@ -161,12 +199,6 @@ pub(crate) fn compute_offchain_election() -> Result, Offchain )) } -pub(crate) fn compute_save_and_submit() -> Result<(), OffchainElectionError> { - let call = compute_offchain_election::()?; - save_solution::(&call)?; - submit_soluton::(call) -} - /// Get a random number of iterations to run the balancing. /// /// Uses the offchain seed to generate a random number. diff --git a/frame/staking/src/tests.rs b/frame/staking/src/tests.rs index a10c95d0f24d2..d1c23a575b1bb 100644 --- a/frame/staking/src/tests.rs +++ b/frame/staking/src/tests.rs @@ -4134,35 +4134,36 @@ mod offchain_election { ext.execute_with(|| { use offchain_election::OFFCHAIN_HEAD_DB; use sp_runtime::offchain::storage::StorageValueRef; + use crate::offchain_election::{OffchainElectionError, OFFCHAIN_REPEAT}; let storage = StorageValueRef::persistent(&OFFCHAIN_HEAD_DB); run_to_block(12); // first run -- ok assert_eq!( - offchain_election::set_check_offchain_execution_status::(12), + offchain_election::set_check_offchain_execution_status::(12, OFFCHAIN_REPEAT.into()), Ok(()), ); assert_eq!(storage.get::().unwrap().unwrap(), 12); // re-execute after the next. not allowed. assert_eq!( - offchain_election::set_check_offchain_execution_status::(13), - Err("recently executed."), + offchain_election::set_check_offchain_execution_status::(13, OFFCHAIN_REPEAT.into()), + Err(OffchainElectionError::TooRecent), ); // a fork like situation -- re-execute 10, 11, 12. But it won't go through. assert_eq!( - offchain_election::set_check_offchain_execution_status::(10), - Err("fork."), + offchain_election::set_check_offchain_execution_status::(10, OFFCHAIN_REPEAT.into()), + Err(OffchainElectionError::Fork), ); assert_eq!( - offchain_election::set_check_offchain_execution_status::(11), - Err("fork."), + offchain_election::set_check_offchain_execution_status::(11, OFFCHAIN_REPEAT.into()), + Err(OffchainElectionError::Fork), ); assert_eq!( - offchain_election::set_check_offchain_execution_status::(12), - Err("recently executed."), + offchain_election::set_check_offchain_execution_status::(12, OFFCHAIN_REPEAT.into()), + Err(OffchainElectionError::TooRecent), ); }) } diff --git a/frame/staking/src/weights.rs b/frame/staking/src/weights.rs index b70563ccf41b3..36a3eaa496a5f 100644 --- a/frame/staking/src/weights.rs +++ b/frame/staking/src/weights.rs @@ -35,11 +35,13 @@ // --output=./frame/staking/src/weights.rs // --template=./.maintain/frame-weight-template.hbs - #![allow(unused_parens)] #![allow(unused_imports)] -use frame_support::{traits::Get, weights::{Weight, constants::RocksDbWeight}}; +use frame_support::{ + traits::Get, + weights::{Weight, constants::RocksDbWeight}, +}; use sp_std::marker::PhantomData; /// Weight functions needed for pallet_staking. From 394151361e064d7f2cdeaab88e942ceb725478aa Mon Sep 17 00:00:00 2001 From: kianenigma Date: Mon, 25 Jan 2021 13:46:21 +0000 Subject: [PATCH 03/18] Log cleanup. --- frame/staking/src/lib.rs | 36 ++++++-------------------- frame/staking/src/offchain_election.rs | 2 +- 2 files changed, 9 insertions(+), 29 deletions(-) diff --git a/frame/staking/src/lib.rs b/frame/staking/src/lib.rs index 3f2a43d2eaab1..7bad36a6d487c 100644 --- a/frame/staking/src/lib.rs +++ b/frame/staking/src/lib.rs @@ -1369,43 +1369,23 @@ decl_module! { set_check_offchain_execution_status, compute_save_and_submit, submit_queued, OFFCHAIN_REPEAT, }; + // ensure that we don't run OCW in any case more at least with 5 blocks delay. + let threshold: T::BlockNumber = OFFCHAIN_REPEAT.into(); let election_status = Self::era_election_status(); if election_status.is_open_at(now) { // If era election status is open at the current block, mine the solution, save it // and submit it. - - // ensure that we don't run OCW in any case more at least with 5 blocks delay. - let threshold: T::BlockNumber = OFFCHAIN_REPEAT.into(); - match set_check_offchain_execution_status::(now, threshold) { - Ok(_) => { - match compute_save_and_submit::() { - Ok(_) => - log!(info, "๐Ÿ’ธ Executed offchain worker thread without errors."), - Err(why) => - log!(error, "๐Ÿ’ธ Error in election offchain worker: {:?}", why), - } - }, - Err(why) => - log!(debug, "๐Ÿ’ธ skipping offchain worker in open election window due to [{:?}]", why), - } + let initial_output = set_check_offchain_execution_status::(now, threshold) + .and_then(|_| compute_save_and_submit::()); + log!(debug, "initial OCW output at {:?} = {:?}", now, initial_output); } else if election_status.is_open() && Self::queued_elected().is_none() { // If the election window is open, and we don't have a queued solution, if you have // one stored in the OCW DB, the submit it. We again check to never run the OCW more // than every 5 blocks. - let threshold: T::BlockNumber = OFFCHAIN_REPEAT.into(); - match set_check_offchain_execution_status::(now, threshold) { - Ok(_) => { - match submit_queued::() { - Ok(_) => - log!(info, "resubmitting a saved solution succeeded at {:?}", now), - Err(why) => - log!(error, "resubmitting a saved solution failed at {:?} because of {:?}", now, why), - } - }, - Err(why) => - log!(debug, "๐Ÿ’ธ skipping offchain worker in open election window due to [{:?}]", why), - } + let resubmit_output = set_check_offchain_execution_status::(now, threshold) + .and_then(|_| submit_queued::()); + log!(debug, "resubmit OCW output at {:?} = {:?}", now, resubmit_output); } } diff --git a/frame/staking/src/offchain_election.rs b/frame/staking/src/offchain_election.rs index 04d71c028f726..861500214c640 100644 --- a/frame/staking/src/offchain_election.rs +++ b/frame/staking/src/offchain_election.rs @@ -182,7 +182,7 @@ pub(crate) fn compute_offchain_election() -> Result, Offchain crate::log!( info, - "๐Ÿ’ธ prepared a seq-phragmen solution with {} balancing iterations and score {:?}", + "๐Ÿ’ธ [OCW] prepared a seq-phragmen solution with {} balancing iterations and score {:?}", iters, score, ); From 8613776f7dc3dc0d17b245ea606016e5e1f15759 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Mon, 25 Jan 2021 13:55:59 +0000 Subject: [PATCH 04/18] document it. --- frame/staking/src/offchain_election.rs | 12 +++++++++--- frame/staking/src/weights.rs | 6 ++---- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/frame/staking/src/offchain_election.rs b/frame/staking/src/offchain_election.rs index 20e2ad1ccc4f9..f9dc8bab62d42 100644 --- a/frame/staking/src/offchain_election.rs +++ b/frame/staking/src/offchain_election.rs @@ -23,6 +23,7 @@ use crate::{ }; use codec::Decode; use frame_support::{traits::Get, weights::Weight, IterableStorageMap}; +use frame_system::offchain::SubmitTransaction; use sp_npos_elections::{ to_support_map, EvaluateSupport, reduce, Assignment, ElectionResult, ElectionScore, ExtendedBalance, CompactSolution, @@ -30,11 +31,10 @@ use sp_npos_elections::{ use sp_runtime::{ offchain::storage::StorageValueRef, traits::TrailingZeroInput, RuntimeDebug, }; -use sp_std::{convert::TryInto, prelude::*}; -use frame_system::offchain::SubmitTransaction; +use sp_std::{convert::TryInto, prelude::*, fmt::Debug}; /// Error types related to the offchain election machinery. -#[derive(RuntimeDebug)] +#[derive(Debug)] #[cfg_attr(feature = "std", derive(PartialEq, Eq))] pub enum OffchainElectionError { /// election returned None. This means less candidate that minimum number of needed @@ -117,8 +117,10 @@ pub(crate) fn set_check_offchain_execution_status( } } +/// Storage path for the solution `call`. pub(crate) const OFFCHAIN_QUEUED_CALL: &[u8] = b"parity/staking-election/call"; +/// Save a given call OCW storage. pub(crate) fn save_solution(call: Call) -> Result<(), OffchainElectionError> { let storage = StorageValueRef::persistent(&OFFCHAIN_QUEUED_CALL); let set_outcome = storage.mutate::<_, OffchainElectionError, _>(|maybe_call| { @@ -140,20 +142,24 @@ pub(crate) fn save_solution(call: Call) -> Result<(), OffchainElec } } +/// Get a saved OCW solution, if it exists. pub(crate) fn get_solution() -> Option> { StorageValueRef::persistent(&OFFCHAIN_QUEUED_CALL).get().flatten() } +/// Submit a saved OCW solution, if one exists. pub(crate) fn submit_queued() -> Result<(), OffchainElectionError> { let saved = get_solution().ok_or(OffchainElectionError::SolutionUnavailable)?; submit_solution::(saved) } +/// Submit a given solution as an unsigned transaction. pub(crate) fn submit_solution(call: Call) -> Result<(), OffchainElectionError> { SubmitTransaction::>::submit_unsigned_transaction(call.into()) .map_err(|_| OffchainElectionError::PoolSubmissionFailed) } +/// Compute the solution, save it, and submit it. pub(crate) fn compute_save_and_submit() -> Result<(), OffchainElectionError> { let call = compute_offchain_election::()?; save_solution::(call.clone())?; diff --git a/frame/staking/src/weights.rs b/frame/staking/src/weights.rs index 36a3eaa496a5f..b70563ccf41b3 100644 --- a/frame/staking/src/weights.rs +++ b/frame/staking/src/weights.rs @@ -35,13 +35,11 @@ // --output=./frame/staking/src/weights.rs // --template=./.maintain/frame-weight-template.hbs + #![allow(unused_parens)] #![allow(unused_imports)] -use frame_support::{ - traits::Get, - weights::{Weight, constants::RocksDbWeight}, -}; +use frame_support::{traits::Get, weights::{Weight, constants::RocksDbWeight}}; use sp_std::marker::PhantomData; /// Weight functions needed for pallet_staking. From e5e172529e277508bccb4aa8661c6d93c2475aa5 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Mon, 25 Jan 2021 14:51:20 +0000 Subject: [PATCH 05/18] Remove unused. --- frame/staking/src/offchain_election.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/staking/src/offchain_election.rs b/frame/staking/src/offchain_election.rs index f9dc8bab62d42..97caca459dfb0 100644 --- a/frame/staking/src/offchain_election.rs +++ b/frame/staking/src/offchain_election.rs @@ -29,7 +29,7 @@ use sp_npos_elections::{ ExtendedBalance, CompactSolution, }; use sp_runtime::{ - offchain::storage::StorageValueRef, traits::TrailingZeroInput, RuntimeDebug, + offchain::storage::StorageValueRef, traits::TrailingZeroInput, }; use sp_std::{convert::TryInto, prelude::*, fmt::Debug}; From dd4eb08bcebf27a41ec6f0358196afa9695a3f88 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Tue, 26 Jan 2021 09:16:09 +0000 Subject: [PATCH 06/18] Use a better match to ensure we don't run old --- frame/staking/src/lib.rs | 31 ++++++++++++++++++------------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/frame/staking/src/lib.rs b/frame/staking/src/lib.rs index 7495937d6d8c7..31bde1773b31f 100644 --- a/frame/staking/src/lib.rs +++ b/frame/staking/src/lib.rs @@ -1373,19 +1373,24 @@ decl_module! { let threshold: T::BlockNumber = OFFCHAIN_REPEAT.into(); let election_status = Self::era_election_status(); - if election_status.is_open_at(now) { - // If era election status is open at the current block, mine the solution, save it - // and submit it. - let initial_output = set_check_offchain_execution_status::(now, threshold) - .and_then(|_| compute_save_and_submit::()); - log!(debug, "initial OCW output at {:?} = {:?}", now, initial_output); - } else if election_status.is_open() && Self::queued_elected().is_none() { - // If the election window is open, and we don't have a queued solution, if you have - // one stored in the OCW DB, the submit it. We again check to never run the OCW more - // than every 5 blocks. - let resubmit_output = set_check_offchain_execution_status::(now, threshold) - .and_then(|_| submit_queued::()); - log!(debug, "resubmit OCW output at {:?} = {:?}", now, resubmit_output); + log!(trace, "Running OCW at {:?}, election status = {:?}", now, election_status); + match Self::era_election_status() { + ElectionStatus::Open(opened) if opened == now => { + // If era election status is open at the current block, mine the solution, save it + // and submit it. + let initial_output = set_check_offchain_execution_status::(now, threshold) + .and_then(|_| compute_save_and_submit::()); + log!(debug, "initial OCW output at {:?} = {:?}", now, initial_output); + }, + ElectionStatus::Open(opened) if opened > now => { + // If the election window is open, and we don't have a queued solution, if you have + // one stored in the OCW DB, the submit it. We again check to never run the OCW more + // than every 5 blocks. + let resubmit_output = set_check_offchain_execution_status::(now, threshold) + .and_then(|_| submit_queued::()); + log!(debug, "resubmit OCW output at {:?} = {:?}", now, resubmit_output); + }, + _ => {} } } From b2e93d97eb9321e1facf7aae81f6c5eb87fcf635 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Tue, 26 Jan 2021 10:08:46 +0000 Subject: [PATCH 07/18] reformat a bit into a simpler form --- frame/staking/src/lib.rs | 18 ++++--------- frame/staking/src/offchain_election.rs | 37 +++++++++++++++++++------- 2 files changed, 32 insertions(+), 23 deletions(-) diff --git a/frame/staking/src/lib.rs b/frame/staking/src/lib.rs index 31bde1773b31f..35552650f19a5 100644 --- a/frame/staking/src/lib.rs +++ b/frame/staking/src/lib.rs @@ -1366,7 +1366,7 @@ decl_module! { /// Offchain worker logic. fn offchain_worker(now: T::BlockNumber) { use offchain_election::{ - set_check_offchain_execution_status, compute_save_and_submit, submit_queued, + set_check_offchain_execution_status, restore_or_compute_then_submit, OFFCHAIN_REPEAT, }; // ensure that we don't run OCW in any case more at least with 5 blocks delay. @@ -1375,20 +1375,12 @@ decl_module! { let election_status = Self::era_election_status(); log!(trace, "Running OCW at {:?}, election status = {:?}", now, election_status); match Self::era_election_status() { - ElectionStatus::Open(opened) if opened == now => { + ElectionStatus::Open(opened) if opened >= now => { // If era election status is open at the current block, mine the solution, save it // and submit it. - let initial_output = set_check_offchain_execution_status::(now, threshold) - .and_then(|_| compute_save_and_submit::()); - log!(debug, "initial OCW output at {:?} = {:?}", now, initial_output); - }, - ElectionStatus::Open(opened) if opened > now => { - // If the election window is open, and we don't have a queued solution, if you have - // one stored in the OCW DB, the submit it. We again check to never run the OCW more - // than every 5 blocks. - let resubmit_output = set_check_offchain_execution_status::(now, threshold) - .and_then(|_| submit_queued::()); - log!(debug, "resubmit OCW output at {:?} = {:?}", now, resubmit_output); + let output = set_check_offchain_execution_status::(now, threshold) + .and_then(|_| restore_or_compute_then_submit::()); + log!(debug, "OCW output at {:?} = {:?}", now, output); }, _ => {} } diff --git a/frame/staking/src/offchain_election.rs b/frame/staking/src/offchain_election.rs index 97caca459dfb0..f212657963a45 100644 --- a/frame/staking/src/offchain_election.rs +++ b/frame/staking/src/offchain_election.rs @@ -62,6 +62,8 @@ pub enum OffchainElectionError { PoolSubmissionFailed, /// No solution is stored in the offchain DB. SolutionUnavailable, + /// The stored solution belongs to an old era and cannot be used. + SolutionOld, } impl From for OffchainElectionError { @@ -143,14 +145,11 @@ pub(crate) fn save_solution(call: Call) -> Result<(), OffchainElec } /// Get a saved OCW solution, if it exists. -pub(crate) fn get_solution() -> Option> { - StorageValueRef::persistent(&OFFCHAIN_QUEUED_CALL).get().flatten() -} - -/// Submit a saved OCW solution, if one exists. -pub(crate) fn submit_queued() -> Result<(), OffchainElectionError> { - let saved = get_solution().ok_or(OffchainElectionError::SolutionUnavailable)?; - submit_solution::(saved) +pub(crate) fn get_solution() -> Result, OffchainElectionError> { + StorageValueRef::persistent(&OFFCHAIN_QUEUED_CALL) + .get() + .flatten() + .ok_or(OffchainElectionError::SolutionUnavailable) } /// Submit a given solution as an unsigned transaction. @@ -159,10 +158,28 @@ pub(crate) fn submit_solution(call: Call) -> Result<(), OffchainEl .map_err(|_| OffchainElectionError::PoolSubmissionFailed) } -/// Compute the solution, save it, and submit it. -pub(crate) fn compute_save_and_submit() -> Result<(), OffchainElectionError> { +/// Ensure that the given solution call belongs to the current era. Returns `Ok(call)` if so to be +/// used with `Result::and`. +pub(crate) fn ensure_solution_is_recent(call: Call) -> Result, OffchainElectionError> { + let current_era = >::current_era().unwrap_or_default(); + match call { + Call::submit_election_solution_unsigned(_, _, _, era, _) if era == current_era => Ok(call), + _ => Err(OffchainElectionError::SolutionOld) + } +} + +/// Compute a new solution and save it to the OCW storage. +pub(crate) fn compute_and_save() -> Result, OffchainElectionError> { let call = compute_offchain_election::()?; save_solution::(call.clone())?; + Ok(call) +} + +/// Compute the solution, save it, and submit it. +pub(crate) fn restore_or_compute_then_submit() -> Result<(), OffchainElectionError> { + let call = get_solution::() + .and_then(|call| ensure_solution_is_recent::(call)) + .or_else(|_| compute_and_save::())?; submit_solution::(call) } From 2b96aa461d613c204a5a88689113299eb233be90 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Tue, 26 Jan 2021 10:12:09 +0000 Subject: [PATCH 08/18] Revert "reformat a bit into a simpler form" This reverts commit b2e93d97eb9321e1facf7aae81f6c5eb87fcf635. --- frame/staking/src/lib.rs | 18 +++++++++---- frame/staking/src/offchain_election.rs | 37 +++++++------------------- 2 files changed, 23 insertions(+), 32 deletions(-) diff --git a/frame/staking/src/lib.rs b/frame/staking/src/lib.rs index 35552650f19a5..31bde1773b31f 100644 --- a/frame/staking/src/lib.rs +++ b/frame/staking/src/lib.rs @@ -1366,7 +1366,7 @@ decl_module! { /// Offchain worker logic. fn offchain_worker(now: T::BlockNumber) { use offchain_election::{ - set_check_offchain_execution_status, restore_or_compute_then_submit, + set_check_offchain_execution_status, compute_save_and_submit, submit_queued, OFFCHAIN_REPEAT, }; // ensure that we don't run OCW in any case more at least with 5 blocks delay. @@ -1375,12 +1375,20 @@ decl_module! { let election_status = Self::era_election_status(); log!(trace, "Running OCW at {:?}, election status = {:?}", now, election_status); match Self::era_election_status() { - ElectionStatus::Open(opened) if opened >= now => { + ElectionStatus::Open(opened) if opened == now => { // If era election status is open at the current block, mine the solution, save it // and submit it. - let output = set_check_offchain_execution_status::(now, threshold) - .and_then(|_| restore_or_compute_then_submit::()); - log!(debug, "OCW output at {:?} = {:?}", now, output); + let initial_output = set_check_offchain_execution_status::(now, threshold) + .and_then(|_| compute_save_and_submit::()); + log!(debug, "initial OCW output at {:?} = {:?}", now, initial_output); + }, + ElectionStatus::Open(opened) if opened > now => { + // If the election window is open, and we don't have a queued solution, if you have + // one stored in the OCW DB, the submit it. We again check to never run the OCW more + // than every 5 blocks. + let resubmit_output = set_check_offchain_execution_status::(now, threshold) + .and_then(|_| submit_queued::()); + log!(debug, "resubmit OCW output at {:?} = {:?}", now, resubmit_output); }, _ => {} } diff --git a/frame/staking/src/offchain_election.rs b/frame/staking/src/offchain_election.rs index f212657963a45..97caca459dfb0 100644 --- a/frame/staking/src/offchain_election.rs +++ b/frame/staking/src/offchain_election.rs @@ -62,8 +62,6 @@ pub enum OffchainElectionError { PoolSubmissionFailed, /// No solution is stored in the offchain DB. SolutionUnavailable, - /// The stored solution belongs to an old era and cannot be used. - SolutionOld, } impl From for OffchainElectionError { @@ -145,11 +143,14 @@ pub(crate) fn save_solution(call: Call) -> Result<(), OffchainElec } /// Get a saved OCW solution, if it exists. -pub(crate) fn get_solution() -> Result, OffchainElectionError> { - StorageValueRef::persistent(&OFFCHAIN_QUEUED_CALL) - .get() - .flatten() - .ok_or(OffchainElectionError::SolutionUnavailable) +pub(crate) fn get_solution() -> Option> { + StorageValueRef::persistent(&OFFCHAIN_QUEUED_CALL).get().flatten() +} + +/// Submit a saved OCW solution, if one exists. +pub(crate) fn submit_queued() -> Result<(), OffchainElectionError> { + let saved = get_solution().ok_or(OffchainElectionError::SolutionUnavailable)?; + submit_solution::(saved) } /// Submit a given solution as an unsigned transaction. @@ -158,28 +159,10 @@ pub(crate) fn submit_solution(call: Call) -> Result<(), OffchainEl .map_err(|_| OffchainElectionError::PoolSubmissionFailed) } -/// Ensure that the given solution call belongs to the current era. Returns `Ok(call)` if so to be -/// used with `Result::and`. -pub(crate) fn ensure_solution_is_recent(call: Call) -> Result, OffchainElectionError> { - let current_era = >::current_era().unwrap_or_default(); - match call { - Call::submit_election_solution_unsigned(_, _, _, era, _) if era == current_era => Ok(call), - _ => Err(OffchainElectionError::SolutionOld) - } -} - -/// Compute a new solution and save it to the OCW storage. -pub(crate) fn compute_and_save() -> Result, OffchainElectionError> { +/// Compute the solution, save it, and submit it. +pub(crate) fn compute_save_and_submit() -> Result<(), OffchainElectionError> { let call = compute_offchain_election::()?; save_solution::(call.clone())?; - Ok(call) -} - -/// Compute the solution, save it, and submit it. -pub(crate) fn restore_or_compute_then_submit() -> Result<(), OffchainElectionError> { - let call = get_solution::() - .and_then(|call| ensure_solution_is_recent::(call)) - .or_else(|_| compute_and_save::())?; submit_solution::(call) } From 4072fd6085296056c03f1ed30d257c28a2e6129d Mon Sep 17 00:00:00 2001 From: kianenigma Date: Tue, 26 Jan 2021 10:14:37 +0000 Subject: [PATCH 09/18] Fix order --- frame/staking/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/staking/src/lib.rs b/frame/staking/src/lib.rs index 31bde1773b31f..6adce06a6582d 100644 --- a/frame/staking/src/lib.rs +++ b/frame/staking/src/lib.rs @@ -1382,7 +1382,7 @@ decl_module! { .and_then(|_| compute_save_and_submit::()); log!(debug, "initial OCW output at {:?} = {:?}", now, initial_output); }, - ElectionStatus::Open(opened) if opened > now => { + ElectionStatus::Open(opened) if now > opened => { // If the election window is open, and we don't have a queued solution, if you have // one stored in the OCW DB, the submit it. We again check to never run the OCW more // than every 5 blocks. From 0b338a25e77a4c280e57a0b4f4d20af64e9229fd Mon Sep 17 00:00:00 2001 From: kianenigma Date: Tue, 26 Jan 2021 10:30:36 +0000 Subject: [PATCH 10/18] Last touches. --- frame/staking/src/lib.rs | 22 +++++++------- frame/staking/src/offchain_election.rs | 40 ++++++++++++++++++++------ 2 files changed, 44 insertions(+), 18 deletions(-) diff --git a/frame/staking/src/lib.rs b/frame/staking/src/lib.rs index 6adce06a6582d..271341ed152a2 100644 --- a/frame/staking/src/lib.rs +++ b/frame/staking/src/lib.rs @@ -1366,8 +1366,8 @@ decl_module! { /// Offchain worker logic. fn offchain_worker(now: T::BlockNumber) { use offchain_election::{ - set_check_offchain_execution_status, compute_save_and_submit, submit_queued, - OFFCHAIN_REPEAT, + set_check_offchain_execution_status, compute_save_and_submit, + restore_or_compute_then_submit, OFFCHAIN_REPEAT, }; // ensure that we don't run OCW in any case more at least with 5 blocks delay. let threshold: T::BlockNumber = OFFCHAIN_REPEAT.into(); @@ -1376,19 +1376,21 @@ decl_module! { log!(trace, "Running OCW at {:?}, election status = {:?}", now, election_status); match Self::era_election_status() { ElectionStatus::Open(opened) if opened == now => { - // If era election status is open at the current block, mine the solution, save it - // and submit it. + // If era election status is open at the current block, mine a new solution + // then save and submit it. let initial_output = set_check_offchain_execution_status::(now, threshold) .and_then(|_| compute_save_and_submit::()); log!(debug, "initial OCW output at {:?} = {:?}", now, initial_output); }, ElectionStatus::Open(opened) if now > opened => { - // If the election window is open, and we don't have a queued solution, if you have - // one stored in the OCW DB, the submit it. We again check to never run the OCW more - // than every 5 blocks. - let resubmit_output = set_check_offchain_execution_status::(now, threshold) - .and_then(|_| submit_queued::()); - log!(debug, "resubmit OCW output at {:?} = {:?}", now, resubmit_output); + if Self::queued_score().is_none() { + // If the election window is open, and we don't have a queued solution, + // constantly try to challenge it by either resubmitting a saved solution, + // or mining a new one (just in the case that the previous was skipped). + let resubmit_output = set_check_offchain_execution_status::(now, threshold) + .and_then(|_| restore_or_compute_then_submit::()); + log!(debug, "resubmit OCW output at {:?} = {:?}", now, resubmit_output); + } }, _ => {} } diff --git a/frame/staking/src/offchain_election.rs b/frame/staking/src/offchain_election.rs index 97caca459dfb0..dfc69addff815 100644 --- a/frame/staking/src/offchain_election.rs +++ b/frame/staking/src/offchain_election.rs @@ -62,6 +62,8 @@ pub enum OffchainElectionError { PoolSubmissionFailed, /// No solution is stored in the offchain DB. SolutionUnavailable, + /// The stored solution belongs to an old era and cannot be used. + SolutionOld, } impl From for OffchainElectionError { @@ -107,6 +109,7 @@ pub(crate) fn set_check_offchain_execution_status( } }); + crate::log!(trace, "attempting to acquire the OCW lock at {:?} = {:?}", now, mutate_stat); match mutate_stat { // all good Ok(Ok(_)) => Ok(()), @@ -147,22 +150,43 @@ pub(crate) fn get_solution() -> Option> { StorageValueRef::persistent(&OFFCHAIN_QUEUED_CALL).get().flatten() } -/// Submit a saved OCW solution, if one exists. -pub(crate) fn submit_queued() -> Result<(), OffchainElectionError> { - let saved = get_solution().ok_or(OffchainElectionError::SolutionUnavailable)?; - submit_solution::(saved) -} - /// Submit a given solution as an unsigned transaction. pub(crate) fn submit_solution(call: Call) -> Result<(), OffchainElectionError> { SubmitTransaction::>::submit_unsigned_transaction(call.into()) .map_err(|_| OffchainElectionError::PoolSubmissionFailed) } -/// Compute the solution, save it, and submit it. -pub(crate) fn compute_save_and_submit() -> Result<(), OffchainElectionError> { +/// Ensure that the given solution call belongs to the current era. Returns `Ok(call)` if so to be +/// used with `Result::and`. +pub(crate) fn ensure_solution_is_recent( + call: Call, +) -> Result, OffchainElectionError> { + let current_era = >::current_era().unwrap_or_default(); + match call { + Call::submit_election_solution_unsigned(_, _, _, era, _) if era == current_era => Ok(call), + _ => Err(OffchainElectionError::SolutionOld), + } +} + +/// Compute a new solution and save it to the OCW storage. +pub(crate) fn compute_and_save() -> Result, OffchainElectionError> { let call = compute_offchain_election::()?; save_solution::(call.clone())?; + Ok(call) +} + +/// Compute the solution, save it, and submit it. +pub(crate) fn restore_or_compute_then_submit() -> Result<(), OffchainElectionError> { + let call = get_solution::() + .ok_or(OffchainElectionError::SolutionUnavailable) + .and_then(ensure_solution_is_recent) + .or_else(|_| compute_and_save::())?; + submit_solution::(call) +} + +/// Compute the solution, save it, and submit it. +pub(crate) fn compute_save_and_submit() -> Result<(), OffchainElectionError> { + let call = compute_and_save::()?; submit_solution::(call) } From 960fecb456aff187a2b8299d8a3a6d1e911a30c0 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Wed, 27 Jan 2021 07:48:19 +0000 Subject: [PATCH 11/18] Better loggin --- frame/staking/src/lib.rs | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/frame/staking/src/lib.rs b/frame/staking/src/lib.rs index 271341ed152a2..8a3e2cf9457e9 100644 --- a/frame/staking/src/lib.rs +++ b/frame/staking/src/lib.rs @@ -1373,14 +1373,20 @@ decl_module! { let threshold: T::BlockNumber = OFFCHAIN_REPEAT.into(); let election_status = Self::era_election_status(); - log!(trace, "Running OCW at {:?}, election status = {:?}", now, election_status); + + log!( + trace, + "Running OCW at {:?}, election status = {:?}", + now, + election_status, + ); match Self::era_election_status() { ElectionStatus::Open(opened) if opened == now => { // If era election status is open at the current block, mine a new solution // then save and submit it. let initial_output = set_check_offchain_execution_status::(now, threshold) .and_then(|_| compute_save_and_submit::()); - log!(debug, "initial OCW output at {:?} = {:?}", now, initial_output); + log!(info, "initial OCW output at {:?} = {:?}", now, initial_output); }, ElectionStatus::Open(opened) if now > opened => { if Self::queued_score().is_none() { @@ -1389,7 +1395,7 @@ decl_module! { // or mining a new one (just in the case that the previous was skipped). let resubmit_output = set_check_offchain_execution_status::(now, threshold) .and_then(|_| restore_or_compute_then_submit::()); - log!(debug, "resubmit OCW output at {:?} = {:?}", now, resubmit_output); + log!(info, "resubmit OCW output at {:?} = {:?}", now, resubmit_output); } }, _ => {} From 843d98966066d49ed1dcdb8db54afcedd4de6812 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Wed, 27 Jan 2021 07:56:52 +0000 Subject: [PATCH 12/18] Final grumbles. --- frame/staking/src/offchain_election.rs | 42 ++++++++++++-------------- 1 file changed, 19 insertions(+), 23 deletions(-) diff --git a/frame/staking/src/offchain_election.rs b/frame/staking/src/offchain_election.rs index dfc69addff815..555f30b9763e3 100644 --- a/frame/staking/src/offchain_election.rs +++ b/frame/staking/src/offchain_election.rs @@ -92,21 +92,22 @@ pub(crate) fn set_check_offchain_execution_status( ) -> Result<(), OffchainElectionError> { let storage = StorageValueRef::persistent(&OFFCHAIN_HEAD_DB); - let mutate_stat = storage.mutate(|maybe_head: Option>| { - match maybe_head { - Some(Some(head)) if now < head => Err(OffchainElectionError::Fork), - Some(Some(head)) if now >= head && now <= head + threshold => { - Err(OffchainElectionError::TooRecent) - } - Some(Some(head)) if now > head + threshold => { - // we can allow again now. Write the new head. - Ok(now) - } - _ => { - // value doesn't exists. Probably this node just booted up. Write, and allow. - Ok(now) + let mutate_stat = + storage.mutate(|maybe_head: Option>| { + match maybe_head { + Some(Some(head)) if now < head => Err(OffchainElectionError::Fork), + Some(Some(head)) if now >= head && now <= head + threshold => { + Err(OffchainElectionError::TooRecent) + } + Some(Some(head)) if now > head + threshold => { + // we can allow again now. Write the new head. + Ok(now) + } + _ => { + // value doesn't exists. Probably this node just booted up. Write, and allow. + Ok(now) + } } - } }); crate::log!(trace, "attempting to acquire the OCW lock at {:?} = {:?}", now, mutate_stat); @@ -126,13 +127,8 @@ pub(crate) const OFFCHAIN_QUEUED_CALL: &[u8] = b"parity/staking-election/call"; /// Save a given call OCW storage. pub(crate) fn save_solution(call: Call) -> Result<(), OffchainElectionError> { let storage = StorageValueRef::persistent(&OFFCHAIN_QUEUED_CALL); - let set_outcome = storage.mutate::<_, OffchainElectionError, _>(|maybe_call| { - match maybe_call { - // under any case, write the new value. This means that we don't need to clear the - // solution. Each era, it will be overwritten. - _ => Ok(call), - } - }); + // in all cases, just write the new value regardless of the the old one, if any. + let set_outcome = storage.mutate::<_, OffchainElectionError, _>(|_| Ok(call)); match set_outcome { Ok(Ok(_)) => Ok(()), @@ -212,7 +208,7 @@ pub(crate) fn compute_offchain_election() -> Result, Offchain crate::log!( info, - "๐Ÿ’ธ [OCW] prepared a seq-phragmen solution with {} balancing iterations and score {:?}", + "๐Ÿ’ธ prepared a seq-phragmen solution with {} balancing iterations and score {:?}", iters, score, ); @@ -333,7 +329,7 @@ pub fn maximum_compact_len( /// Thus, we reside to stripping away some voters. This means only changing the `compact` struct. /// /// Note that the solution is already computed, and the winners are elected based on the merit of -/// teh entire stake in the system. Nonetheless, some of the voters will be removed further down the +/// the entire stake in the system. Nonetheless, some of the voters will be removed further down the /// line. /// /// Indeed, the score must be computed **after** this step. If this step reduces the score too much, From 7b9af8433f8d7a0ec6dda37edcb897f094fc9c3b Mon Sep 17 00:00:00 2001 From: kianenigma Date: Wed, 27 Jan 2021 08:02:26 +0000 Subject: [PATCH 13/18] Fix doc --- frame/staking/src/offchain_election.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/staking/src/offchain_election.rs b/frame/staking/src/offchain_election.rs index 555f30b9763e3..7ae4fe35b1b49 100644 --- a/frame/staking/src/offchain_election.rs +++ b/frame/staking/src/offchain_election.rs @@ -171,7 +171,7 @@ pub(crate) fn compute_and_save() -> Result, OffchainElectionE Ok(call) } -/// Compute the solution, save it, and submit it. +/// Restore an old solution if exist, else compute a new one and save it, finally submit it. pub(crate) fn restore_or_compute_then_submit() -> Result<(), OffchainElectionError> { let call = get_solution::() .ok_or(OffchainElectionError::SolutionUnavailable) From e961c861b80c123d16b662e3ac9375e129e40942 Mon Sep 17 00:00:00 2001 From: Kian Paimani <5588131+kianenigma@users.noreply.github.com> Date: Mon, 8 Feb 2021 14:24:22 +0000 Subject: [PATCH 14/18] Update frame/staking/src/offchain_election.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Bastian Kรถcher --- frame/staking/src/offchain_election.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/frame/staking/src/offchain_election.rs b/frame/staking/src/offchain_election.rs index 7ae4fe35b1b49..8738c4319e846 100644 --- a/frame/staking/src/offchain_election.rs +++ b/frame/staking/src/offchain_election.rs @@ -152,8 +152,9 @@ pub(crate) fn submit_solution(call: Call) -> Result<(), OffchainEl .map_err(|_| OffchainElectionError::PoolSubmissionFailed) } -/// Ensure that the given solution call belongs to the current era. Returns `Ok(call)` if so to be -/// used with `Result::and`. +/// Ensure that the given solution call belongs to the current era. +/// +/// Returns `Ok(call)` if it belongs to the current era. pub(crate) fn ensure_solution_is_recent( call: Call, ) -> Result, OffchainElectionError> { From 51969da68f60c2f53bbc399fefa98e1e6979e51d Mon Sep 17 00:00:00 2001 From: Kian Paimani <5588131+kianenigma@users.noreply.github.com> Date: Mon, 8 Feb 2021 14:26:07 +0000 Subject: [PATCH 15/18] Update frame/staking/src/offchain_election.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Bastian Kรถcher --- frame/staking/src/offchain_election.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/frame/staking/src/offchain_election.rs b/frame/staking/src/offchain_election.rs index 8738c4319e846..a68f266410ab2 100644 --- a/frame/staking/src/offchain_election.rs +++ b/frame/staking/src/offchain_election.rs @@ -175,7 +175,8 @@ pub(crate) fn compute_and_save() -> Result, OffchainElectionE /// Restore an old solution if exist, else compute a new one and save it, finally submit it. pub(crate) fn restore_or_compute_then_submit() -> Result<(), OffchainElectionError> { let call = get_solution::() - .ok_or(OffchainElectionError::SolutionUnavailable) + .map(Ok) + .transpose() .and_then(ensure_solution_is_recent) .or_else(|_| compute_and_save::())?; submit_solution::(call) From c43fe6bfaa22222dc160ad5eb43b1ec10dd29259 Mon Sep 17 00:00:00 2001 From: Kian Paimani <5588131+kianenigma@users.noreply.github.com> Date: Mon, 8 Feb 2021 14:26:21 +0000 Subject: [PATCH 16/18] Update frame/staking/src/lib.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Bastian Kรถcher --- frame/staking/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/staking/src/lib.rs b/frame/staking/src/lib.rs index 8a3e2cf9457e9..c6f7375ac7772 100644 --- a/frame/staking/src/lib.rs +++ b/frame/staking/src/lib.rs @@ -1389,7 +1389,7 @@ decl_module! { log!(info, "initial OCW output at {:?} = {:?}", now, initial_output); }, ElectionStatus::Open(opened) if now > opened => { - if Self::queued_score().is_none() { + if !QueuedScore::exists() { // If the election window is open, and we don't have a queued solution, // constantly try to challenge it by either resubmitting a saved solution, // or mining a new one (just in the case that the previous was skipped). From 09298cefd92187e0e0b8540ab501ec3f082c6e76 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Mon, 8 Feb 2021 14:28:25 +0000 Subject: [PATCH 17/18] Some logging --- frame/staking/src/lib.rs | 33 +++++++++++++++----------- frame/staking/src/offchain_election.rs | 12 ++++------ 2 files changed, 24 insertions(+), 21 deletions(-) diff --git a/frame/staking/src/lib.rs b/frame/staking/src/lib.rs index 8a3e2cf9457e9..31dd579140d2b 100644 --- a/frame/staking/src/lib.rs +++ b/frame/staking/src/lib.rs @@ -345,7 +345,7 @@ macro_rules! log { ($level:tt, $patter:expr $(, $values:expr)* $(,)?) => { frame_support::debug::$level!( target: crate::LOG_TARGET, - $patter $(, $values)* + concat!("๐Ÿ’ธ ", $patter $(, $values)*) ) }; } @@ -1345,14 +1345,14 @@ decl_module! { ElectionStatus::::Open(now) ); add_weight(0, 1, 0); - log!(info, "๐Ÿ’ธ Election window is Open({:?}). Snapshot created", now); + log!(info, "Election window is Open({:?}). Snapshot created", now); } else { - log!(warn, "๐Ÿ’ธ Failed to create snapshot at {:?}.", now); + log!(warn, "Failed to create snapshot at {:?}.", now); } } } } else { - log!(warn, "๐Ÿ’ธ Estimating next session change failed."); + log!(warn, "Estimating next session change failed."); } add_weight(0, 0, T::NextNewSession::weight(now)) } @@ -1376,9 +1376,13 @@ decl_module! { log!( trace, - "Running OCW at {:?}, election status = {:?}", + "ocw at {:?}, election status = {:?}, queued_score = {:?}, offchain solution size: {}", now, election_status, + Self::queued_score(), + offchain_election::get_solution::() + .map(offchain_election::ensure_solution_is_recent) + .map(|s| s.len()), ); match Self::era_election_status() { ElectionStatus::Open(opened) if opened == now => { @@ -2326,7 +2330,7 @@ impl Module { { log!( warn, - "๐Ÿ’ธ Snapshot size too big [{} <> {}][{} <> {}].", + "Snapshot size too big [{} <> {}][{} <> {}].", num_validators, MAX_VALIDATORS, num_nominators, @@ -2646,7 +2650,7 @@ impl Module { validator_at, ).map_err(|e| { // log the error since it is not propagated into the runtime error. - log!(warn, "๐Ÿ’ธ un-compacting solution failed due to {:?}", e); + log!(warn, "un-compacting solution failed due to {:?}", e); Error::::OffchainElectionBogusCompact })?; @@ -2661,7 +2665,7 @@ impl Module { // all of the indices must map to either a validator or a nominator. If this is ever // not the case, then the locking system of staking is most likely faulty, or we // have bigger problems. - log!(error, "๐Ÿ’ธ detected an error in the staking locking and snapshot."); + log!(error, "detected an error in the staking locking and snapshot."); // abort. return Err(Error::::OffchainElectionBogusNominator.into()); } @@ -2721,7 +2725,8 @@ impl Module { let exposures = Self::collect_exposure(supports); log!( info, - "๐Ÿ’ธ A better solution (with compute {:?} and score {:?}) has been validated and stored on chain.", + "A better solution (with compute {:?} and score {:?}) has been validated and stored \ + on chain.", compute, submitted_score, ); @@ -2921,7 +2926,7 @@ impl Module { log!( info, - "๐Ÿ’ธ new validator set of size {:?} has been elected via {:?} for era {:?}", + "new validator set of size {:?} has been elected via {:?} for era {:?}", elected_stashes.len(), compute, current_era, @@ -2977,7 +2982,7 @@ impl Module { .map_err(|_| log!( error, - "๐Ÿ’ธ on-chain phragmen is failing due to a problem in the result. This must be a bug." + "on-chain phragmen is failing due to a problem in the result. This must be a bug." ) ) .ok()?; @@ -3045,7 +3050,7 @@ impl Module { // If we don't have enough candidates, nothing to do. log!( warn, - "๐Ÿ’ธ Chain does not have enough staking candidates to operate. Era {:?}.", + "Chain does not have enough staking candidates to operate. Era {:?}.", Self::current_era() ); None @@ -3502,13 +3507,13 @@ impl frame_support::unsigned::ValidateUnsigned for Module { let invalid = to_invalid(error_with_post_info); log!( debug, - "๐Ÿ’ธ validate unsigned pre dispatch checks failed due to error #{:?}.", + "validate unsigned pre dispatch checks failed due to error #{:?}.", invalid, ); return invalid.into(); } - log!(debug, "๐Ÿ’ธ validateUnsigned succeeded for a solution at era {}.", era); + log!(debug, "validateUnsigned succeeded for a solution at era {}.", era); ValidTransaction::with_tag_prefix("StakingOffchain") // The higher the score[0], the better a solution is. diff --git a/frame/staking/src/offchain_election.rs b/frame/staking/src/offchain_election.rs index 7ae4fe35b1b49..7b05dabe044cc 100644 --- a/frame/staking/src/offchain_election.rs +++ b/frame/staking/src/offchain_election.rs @@ -60,8 +60,6 @@ pub enum OffchainElectionError { Unreachable, /// Submission to the transaction pool failed. PoolSubmissionFailed, - /// No solution is stored in the offchain DB. - SolutionUnavailable, /// The stored solution belongs to an old era and cannot be used. SolutionOld, } @@ -208,7 +206,7 @@ pub(crate) fn compute_offchain_election() -> Result, Offchain crate::log!( info, - "๐Ÿ’ธ prepared a seq-phragmen solution with {} balancing iterations and score {:?}", + "prepared a seq-phragmen solution with {} balancing iterations and score {:?}", iters, score, ); @@ -361,7 +359,7 @@ where if compact.remove_voter(index) { crate::log!( trace, - "๐Ÿ’ธ removed a voter at index {} with stake {:?} from compact to reduce the size", + "removed a voter at index {} with stake {:?} from compact to reduce the size", index, _stake, ); @@ -375,7 +373,7 @@ where crate::log!( warn, - "๐Ÿ’ธ {} nominators out of {} had to be removed from compact solution due to size limits.", + "{} nominators out of {} had to be removed from compact solution due to size limits.", removed, compact.voter_count() + removed, ); @@ -385,7 +383,7 @@ where // nada, return as-is crate::log!( info, - "๐Ÿ’ธ Compact solution did not get trimmed due to block weight limits.", + "Compact solution did not get trimmed due to block weight limits.", ); Ok(compact) } @@ -467,7 +465,7 @@ pub fn prepare_submission( let maximum_allowed_voters = maximum_compact_len::(winners.len() as u32, size, maximum_weight); - crate::log!(debug, "๐Ÿ’ธ Maximum weight = {:?} // current weight = {:?} // maximum voters = {:?} // current votes = {:?}", + crate::log!(debug, "Maximum weight = {:?} // current weight = {:?} // maximum voters = {:?} // current votes = {:?}", maximum_weight, T::WeightInfo::submit_solution_better( size.validators.into(), From 221b31a4a4c8a90f8986b127035d9741c413287b Mon Sep 17 00:00:00 2001 From: kianenigma Date: Mon, 8 Feb 2021 14:57:46 +0000 Subject: [PATCH 18/18] Ready for merge. --- frame/staking/src/lib.rs | 10 ++++++---- frame/staking/src/offchain_election.rs | 11 +++++------ 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/frame/staking/src/lib.rs b/frame/staking/src/lib.rs index 1576ea02106e4..43a2abcf6ddbc 100644 --- a/frame/staking/src/lib.rs +++ b/frame/staking/src/lib.rs @@ -345,7 +345,7 @@ macro_rules! log { ($level:tt, $patter:expr $(, $values:expr)* $(,)?) => { frame_support::debug::$level!( target: crate::LOG_TARGET, - concat!("๐Ÿ’ธ ", $patter $(, $values)*) + concat!("๐Ÿ’ธ ", $patter) $(, $values)* ) }; } @@ -1376,13 +1376,15 @@ decl_module! { log!( trace, - "ocw at {:?}, election status = {:?}, queued_score = {:?}, offchain solution size: {}", + "ocw at {:?}, election status = {:?}, queued_score = {:?}, offchain solution: {:?}", now, election_status, Self::queued_score(), offchain_election::get_solution::() - .map(offchain_election::ensure_solution_is_recent) - .map(|s| s.len()), + .map(|call| ( + offchain_election::ensure_solution_is_recent(call.clone()).is_ok(), + call.encode().len(), + )) ); match Self::era_election_status() { ElectionStatus::Open(opened) if opened == now => { diff --git a/frame/staking/src/offchain_election.rs b/frame/staking/src/offchain_election.rs index 6debb33d4a671..32b940715a9ab 100644 --- a/frame/staking/src/offchain_election.rs +++ b/frame/staking/src/offchain_election.rs @@ -150,7 +150,7 @@ pub(crate) fn submit_solution(call: Call) -> Result<(), OffchainEl .map_err(|_| OffchainElectionError::PoolSubmissionFailed) } -/// Ensure that the given solution call belongs to the current era. +/// Ensure that the given solution call belongs to the current era. /// /// Returns `Ok(call)` if it belongs to the current era. pub(crate) fn ensure_solution_is_recent( @@ -172,11 +172,10 @@ pub(crate) fn compute_and_save() -> Result, OffchainElectionE /// Restore an old solution if exist, else compute a new one and save it, finally submit it. pub(crate) fn restore_or_compute_then_submit() -> Result<(), OffchainElectionError> { - let call = get_solution::() - .map(Ok) - .transpose() - .and_then(ensure_solution_is_recent) - .or_else(|_| compute_and_save::())?; + let call = match get_solution::() { + Some(call) => ensure_solution_is_recent(call)?, + None => compute_and_save::()?, + }; submit_solution::(call) }