Skip to content

Commit

Permalink
pallet epm: add TrimmingStatus to the mined solution (#1659)
Browse files Browse the repository at this point in the history 
For tools such that is using the `Miner` it's useful to know whether a
solution was trimmed or not and also how much that was trimmed.

---------

Co-authored-by: Alexandru Vasile <60601340+lexnv@users.noreply.github.com>
  • Loading branch information
@niklasad1 @lexnv
niklasad1 and lexnv committed Sep 22, 2023
1 parent 22d9e2a commit f79fa6c
Show file tree
Hide file tree
Showing 4 changed files with 73 additions and 36 deletions.
4 changes: 1 addition & 3 deletions polkadot/tests/common.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,7 @@ pub async fn wait_n_finalized_blocks(n: usize, url: &str) {
let mut interval = tokio::time::interval(Duration::from_secs(6));

loop {
let Ok(rpc) = ws_client(url).await else {
continue;
};
let Ok(rpc) = ws_client(url).await else { continue };

if let Ok(block) = ChainApi::<(), Hash, Header, Block>::finalized_head(&rpc).await {
built_blocks.insert(block);
Expand Down
4 changes: 2 additions & 2 deletions substrate/frame/election-provider-multi-phase/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2365,7 +2365,7 @@ mod tests {
assert_eq!(MultiPhase::desired_targets().unwrap(), 2);

// mine seq_phragmen solution with 2 iters.
let (solution, witness) = MultiPhase::mine_solution().unwrap();
let (solution, witness, _) = MultiPhase::mine_solution().unwrap();

// ensure this solution is valid.
assert!(MultiPhase::queued_solution().is_none());
Expand Down Expand Up @@ -2647,7 +2647,7 @@ mod tests {
// set the solution balancing to get the desired score.
crate::mock::Balancing::set(Some(BalancingConfig { iterations: 2, tolerance: 0 }));

let (solution, _) = MultiPhase::mine_solution().unwrap();
let (solution, _, _) = MultiPhase::mine_solution().unwrap();
// Default solution's score.
assert!(matches!(solution.score, ElectionScore { minimal_stake: 50, .. }));

Expand Down
2 changes: 1 addition & 1 deletion substrate/frame/election-provider-multi-phase/src/signed.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1348,7 +1348,7 @@ mod tests {
roll_to_signed();
assert!(MultiPhase::current_phase().is_signed());

let (raw, witness) = MultiPhase::mine_solution().unwrap();
let (raw, witness, _) = MultiPhase::mine_solution().unwrap();
let solution_weight = <Runtime as MinerConfig>::solution_weight(
witness.voters,
witness.targets,
Expand Down
99 changes: 69 additions & 30 deletions substrate/frame/election-provider-multi-phase/src/unsigned.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,27 @@ impl From<FeasibilityError> for MinerError {
}
}

/// Reports the trimming result of a mined solution
#[derive(Debug, Clone)]
pub struct TrimmingStatus {
weight: usize,
length: usize,
}

impl TrimmingStatus {
pub fn is_trimmed(&self) -> bool {
self.weight > 0 || self.length > 0
}

pub fn trimmed_weight(&self) -> usize {
self.weight
}

pub fn trimmed_length(&self) -> usize {
self.length
}
}

/// Save a given call into OCW storage.
fn save_solution<T: Config>(call: &Call<T>) -> Result<(), MinerError> {
log!(debug, "saving a call to the offchain storage.");
Expand Down Expand Up @@ -162,16 +183,21 @@ impl<T: Config> Pallet<T> {
///
/// The Npos Solver type, `S`, must have the same AccountId and Error type as the
/// [`crate::Config::Solver`] in order to create a unified return type.
pub fn mine_solution(
) -> Result<(RawSolution<SolutionOf<T::MinerConfig>>, SolutionOrSnapshotSize), MinerError> {
pub fn mine_solution() -> Result<
(RawSolution<SolutionOf<T::MinerConfig>>, SolutionOrSnapshotSize, TrimmingStatus),
MinerError,
> {
let RoundSnapshot { voters, targets } =
Self::snapshot().ok_or(MinerError::SnapshotUnAvailable)?;
let desired_targets = Self::desired_targets().ok_or(MinerError::SnapshotUnAvailable)?;
let (solution, score, size) = Miner::<T::MinerConfig>::mine_solution_with_snapshot::<
T::Solver,
>(voters, targets, desired_targets)?;
let (solution, score, size, is_trimmed) =
Miner::<T::MinerConfig>::mine_solution_with_snapshot::<T::Solver>(
voters,
targets,
desired_targets,
)?;
let round = Self::round();
Ok((RawSolution { solution, score, round }, size))
Ok((RawSolution { solution, score, round }, size, is_trimmed))
}

/// Attempt to restore a solution from cache. Otherwise, compute it fresh. Either way, submit
Expand Down Expand Up @@ -232,7 +258,7 @@ impl<T: Config> Pallet<T> {
/// Mine a new solution as a call. Performs all checks.
pub fn mine_checked_call() -> Result<Call<T>, MinerError> {
// get the solution, with a load of checks to ensure if submitted, IT IS ABSOLUTELY VALID.
let (raw_solution, witness) = Self::mine_and_check()?;
let (raw_solution, witness, _) = Self::mine_and_check()?;

let score = raw_solution.score;
let call: Call<T> = Call::submit_unsigned { raw_solution: Box::new(raw_solution), witness };
Expand Down Expand Up @@ -282,11 +308,13 @@ impl<T: Config> Pallet<T> {
/// If you want an unchecked solution, use [`Pallet::mine_solution`].
/// If you want a checked solution and submit it at the same time, use
/// [`Pallet::mine_check_save_submit`].
pub fn mine_and_check(
) -> Result<(RawSolution<SolutionOf<T::MinerConfig>>, SolutionOrSnapshotSize), MinerError> {
let (raw_solution, witness) = Self::mine_solution()?;
pub fn mine_and_check() -> Result<
(RawSolution<SolutionOf<T::MinerConfig>>, SolutionOrSnapshotSize, TrimmingStatus),
MinerError,
> {
let (raw_solution, witness, is_trimmed) = Self::mine_solution()?;
Self::basic_checks(&raw_solution, "mined")?;
Ok((raw_solution, witness))
Ok((raw_solution, witness, is_trimmed))
}

/// Checks if an execution of the offchain worker is permitted at the given block number, or
Expand Down Expand Up @@ -408,7 +436,7 @@ impl<T: MinerConfig> Miner<T> {
voters: Vec<(T::AccountId, VoteWeight, BoundedVec<T::AccountId, T::MaxVotesPerVoter>)>,
targets: Vec<T::AccountId>,
desired_targets: u32,
) -> Result<(SolutionOf<T>, ElectionScore, SolutionOrSnapshotSize), MinerError>
) -> Result<(SolutionOf<T>, ElectionScore, SolutionOrSnapshotSize, TrimmingStatus), MinerError>
where
S: NposSolver<AccountId = T::AccountId>,
{
Expand Down Expand Up @@ -436,7 +464,8 @@ impl<T: MinerConfig> Miner<T> {
voters: Vec<(T::AccountId, VoteWeight, BoundedVec<T::AccountId, T::MaxVotesPerVoter>)>,
targets: Vec<T::AccountId>,
desired_targets: u32,
) -> Result<(SolutionOf<T>, ElectionScore, SolutionOrSnapshotSize), MinerError> {
) -> Result<(SolutionOf<T>, ElectionScore, SolutionOrSnapshotSize, TrimmingStatus), MinerError>
{
// now make some helper closures.
let cache = helpers::generate_voter_cache::<T>(&voters);
let voter_index = helpers::voter_index_fn::<T>(&cache);
Expand Down Expand Up @@ -495,13 +524,13 @@ impl<T: MinerConfig> Miner<T> {
// trim assignments list for weight and length.
let size =
SolutionOrSnapshotSize { voters: voters.len() as u32, targets: targets.len() as u32 };
Self::trim_assignments_weight(
let weight_trimmed = Self::trim_assignments_weight(
desired_targets,
size,
T::MaxWeight::get(),
&mut index_assignments,
);
Self::trim_assignments_length(
let length_trimmed = Self::trim_assignments_length(
T::MaxLength::get(),
&mut index_assignments,
&encoded_size_of,
Expand All @@ -513,7 +542,9 @@ impl<T: MinerConfig> Miner<T> {
// re-calc score.
let score = solution.clone().score(stake_of, voter_at, target_at)?;

Ok((solution, score, size))
let is_trimmed = TrimmingStatus { weight: weight_trimmed, length: length_trimmed };

Ok((solution, score, size, is_trimmed))
}

/// Greedily reduce the size of the solution to fit into the block w.r.t length.
Expand All @@ -534,7 +565,7 @@ impl<T: MinerConfig> Miner<T> {
max_allowed_length: u32,
assignments: &mut Vec<IndexAssignmentOf<T>>,
encoded_size_of: impl Fn(&[IndexAssignmentOf<T>]) -> Result<usize, sp_npos_elections::Error>,
) -> Result<(), MinerError> {
) -> Result<usize, MinerError> {
// Perform a binary search for the max subset of which can fit into the allowed
// length. Having discovered that, we can truncate efficiently.
let max_allowed_length: usize = max_allowed_length.saturated_into();
Expand All @@ -543,7 +574,7 @@ impl<T: MinerConfig> Miner<T> {

// not much we can do if assignments are already empty.
if high == low {
return Ok(())
return Ok(0)
}

while high - low > 1 {
Expand Down Expand Up @@ -577,16 +608,18 @@ impl<T: MinerConfig> Miner<T> {
// after this point, we never error.
// check before edit.

let remove = assignments.len().saturating_sub(maximum_allowed_voters);

log_no_system!(
debug,
"from {} assignments, truncating to {} for length, removing {}",
assignments.len(),
maximum_allowed_voters,
assignments.len().saturating_sub(maximum_allowed_voters),
remove
);
assignments.truncate(maximum_allowed_voters);

Ok(())
Ok(remove)
}

/// Greedily reduce the size of the solution to fit into the block w.r.t. weight.
Expand All @@ -609,7 +642,7 @@ impl<T: MinerConfig> Miner<T> {
size: SolutionOrSnapshotSize,
max_weight: Weight,
assignments: &mut Vec<IndexAssignmentOf<T>>,
) {
) -> usize {
let maximum_allowed_voters =
Self::maximum_voter_for_weight(desired_targets, size, max_weight);
let removing: usize =
Expand All @@ -622,6 +655,8 @@ impl<T: MinerConfig> Miner<T> {
removing,
);
assignments.truncate(maximum_allowed_voters as usize);

removing
}

/// Find the maximum `len` that a solution can have in order to fit into the block weight.
Expand Down Expand Up @@ -1230,7 +1265,7 @@ mod tests {
assert_eq!(MultiPhase::desired_targets().unwrap(), 2);

// mine seq_phragmen solution with 2 iters.
let (solution, witness) = MultiPhase::mine_solution().unwrap();
let (solution, witness, _) = MultiPhase::mine_solution().unwrap();

// ensure this solution is valid.
assert!(MultiPhase::queued_solution().is_none());
Expand Down Expand Up @@ -1268,7 +1303,7 @@ mod tests {
roll_to_unsigned();
assert!(MultiPhase::current_phase().is_unsigned());

let (raw, witness) = MultiPhase::mine_solution().unwrap();
let (raw, witness, t) = MultiPhase::mine_solution().unwrap();
let solution_weight = <Runtime as MinerConfig>::solution_weight(
witness.voters,
witness.targets,
Expand All @@ -1278,11 +1313,12 @@ mod tests {
// default solution will have 5 edges (5 * 5 + 10)
assert_eq!(solution_weight, Weight::from_parts(35, 0));
assert_eq!(raw.solution.voter_count(), 5);
assert_eq!(t.trimmed_weight(), 0);

// now reduce the max weight
<MinerMaxWeight>::set(Weight::from_parts(25, u64::MAX));

let (raw, witness) = MultiPhase::mine_solution().unwrap();
let (raw, witness, t) = MultiPhase::mine_solution().unwrap();
let solution_weight = <Runtime as MinerConfig>::solution_weight(
witness.voters,
witness.targets,
Expand All @@ -1292,6 +1328,7 @@ mod tests {
// default solution will have 5 edges (5 * 5 + 10)
assert_eq!(solution_weight, Weight::from_parts(25, 0));
assert_eq!(raw.solution.voter_count(), 3);
assert_eq!(t.trimmed_weight(), 2);
})
}

Expand All @@ -1303,7 +1340,7 @@ mod tests {
assert!(MultiPhase::current_phase().is_unsigned());

// Force the number of winners to be bigger to fail
let (mut solution, _) = MultiPhase::mine_solution().unwrap();
let (mut solution, _, _) = MultiPhase::mine_solution().unwrap();
solution.solution.votes1[0].1 = 4;

assert_eq!(
Expand Down Expand Up @@ -1342,7 +1379,7 @@ mod tests {
let RoundSnapshot { voters, targets } = MultiPhase::snapshot().unwrap();
let desired_targets = MultiPhase::desired_targets().unwrap();

let (raw, score, witness) =
let (raw, score, witness, _) =
Miner::<Runtime>::prepare_election_result_with_snapshot(
result,
voters.clone(),
Expand Down Expand Up @@ -1371,7 +1408,7 @@ mod tests {
},
],
};
let (raw, score, _) = Miner::<Runtime>::prepare_election_result_with_snapshot(
let (raw, score, _, _) = Miner::<Runtime>::prepare_election_result_with_snapshot(
result,
voters.clone(),
targets.clone(),
Expand Down Expand Up @@ -1400,7 +1437,7 @@ mod tests {
},
],
};
let (raw, score, witness) =
let (raw, score, witness, _) =
Miner::<Runtime>::prepare_election_result_with_snapshot(
result,
voters.clone(),
Expand Down Expand Up @@ -1769,7 +1806,7 @@ mod tests {
let solution_clone = solution.clone();

// when
Miner::<Runtime>::trim_assignments_length(
let trimmed_len = Miner::<Runtime>::trim_assignments_length(
encoded_len,
&mut assignments,
encoded_size_of,
Expand All @@ -1779,6 +1816,7 @@ mod tests {
// then
let solution = SolutionOf::<Runtime>::try_from(assignments.as_slice()).unwrap();
assert_eq!(solution, solution_clone);
assert_eq!(trimmed_len, 0);
});
}

Expand All @@ -1794,7 +1832,7 @@ mod tests {
let solution_clone = solution.clone();

// when
Miner::<Runtime>::trim_assignments_length(
let trimmed_len = Miner::<Runtime>::trim_assignments_length(
encoded_len as u32 - 1,
&mut assignments,
encoded_size_of,
Expand All @@ -1805,6 +1843,7 @@ mod tests {
let solution = SolutionOf::<Runtime>::try_from(assignments.as_slice()).unwrap();
assert_ne!(solution, solution_clone);
assert!(solution.encoded_size() < encoded_len);
assert_eq!(trimmed_len, 1);
});
}

Expand Down

0 comments on commit f79fa6c

Please sign in to comment.