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 all commits
Commits
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
1 change: 0 additions & 1 deletion client/allocator/src/freeing_bump.rs
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,6 @@ impl Link {
/// | 0 | next element link |
/// +--------------+-------------------+
/// ```
///
/// ## Occupied header
/// ```ignore
/// 64 32 0
Expand Down
59 changes: 34 additions & 25 deletions frame/election-provider-multi-phase/src/unsigned.rs
Original file line number Diff line number Diff line change
Expand Up @@ -169,27 +169,6 @@ impl<T: Config> Pallet<T> {
Ok((RawSolution { solution, score, round }, size))
}

/// Convert a raw solution from [`sp_npos_elections::ElectionResult`] to [`RawSolution`], which
/// is ready to be submitted to the chain.
///
/// Will always reduce the solution as well.
pub fn prepare_election_result<Accuracy: PerThing128>(
election_result: ElectionResult<T::AccountId, Accuracy>,
) -> Result<(RawSolution<SolutionOf<T::MinerConfig>>, SolutionOrSnapshotSize), 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>::prepare_election_result_with_snapshot(
election_result,
voters,
targets,
desired_targets,
)?;
let round = Self::round();
Ok((RawSolution { solution, score, round }, size))
}

/// Attempt to restore a solution from cache. Otherwise, compute it fresh. Either way, submit
/// if our call's score is greater than that of the cached solution.
pub fn restore_or_compute_then_maybe_submit() -> Result<(), MinerError> {
Expand Down Expand Up @@ -441,7 +420,10 @@ impl<T: MinerConfig> Miner<T> {
})
}

/// Same as [`Pallet::prepare_election_result`], but the input snapshot mut be given as inputs.
/// Convert a raw solution from [`sp_npos_elections::ElectionResult`] to [`RawSolution`], which
/// is ready to be submitted to the chain.
///
/// Will always reduce the solution as well.
pub fn prepare_election_result_with_snapshot<Accuracy: PerThing128>(
election_result: ElectionResult<T::AccountId, Accuracy>,
voters: Vec<(T::AccountId, VoteWeight, BoundedVec<T::AccountId, T::MaxVotesPerVoter>)>,
Expand Down Expand Up @@ -1118,7 +1100,19 @@ mod tests {
distribution: vec![(10, PerU16::one())],
}],
};
let (solution, witness) = MultiPhase::prepare_election_result(result).unwrap();

let RoundSnapshot { voters, targets } = MultiPhase::snapshot().unwrap();
let desired_targets = MultiPhase::desired_targets().unwrap();

let (raw, score, witness) =
Miner::<Runtime>::prepare_election_result_with_snapshot(
result,
voters.clone(),
targets.clone(),
Comment on lines +1110 to +1111
Copy link
Contributor

Choose a reason for hiding this comment

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

Are those clones necessary? Can't we change the prepare_election_result_with_snapshot to take &[T] instead of a Vec<T> for those? It doesn't seem to actually need those owned from what I can see?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

in the production code prepare_election_result_with_snapshot is the last place that we use these values so IMO it is cleaner to consume everything. Clones are only needed in tests.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah okay, that's fine then.

desired_targets,
)
.unwrap();
let solution = RawSolution { solution: raw, score, round: MultiPhase::round() };
assert_ok!(MultiPhase::unsigned_pre_dispatch_checks(&solution));
assert_ok!(MultiPhase::submit_unsigned(
Origin::none(),
Expand All @@ -1139,7 +1133,14 @@ mod tests {
},
],
};
let (solution, _) = MultiPhase::prepare_election_result(result).unwrap();
let (raw, score, _) = Miner::<Runtime>::prepare_election_result_with_snapshot(
result,
voters.clone(),
targets.clone(),
desired_targets,
)
.unwrap();
let solution = RawSolution { solution: raw, score, round: MultiPhase::round() };
// 12 is not 50% more than 10
assert_eq!(solution.score.minimal_stake, 12);
assert_noop!(
Expand All @@ -1161,7 +1162,15 @@ mod tests {
},
],
};
let (solution, witness) = MultiPhase::prepare_election_result(result).unwrap();
let (raw, score, witness) =
Miner::<Runtime>::prepare_election_result_with_snapshot(
result,
voters.clone(),
targets.clone(),
desired_targets,
)
.unwrap();
let solution = RawSolution { solution: raw, score, round: MultiPhase::round() };
assert_eq!(solution.score.minimal_stake, 17);

// and it is fine
Expand Down
4 changes: 2 additions & 2 deletions frame/support/src/storage/types/nmap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -544,7 +544,7 @@ mod test {
use crate::{
hash::{StorageHasher as _, *},
metadata::{StorageEntryModifier, StorageHasher},
storage::types::{Key, Key as NMapKey, ValueQuery},
storage::types::{Key, ValueQuery},
};
use sp_io::{hashing::twox_128, TestExternalities};

Expand Down Expand Up @@ -590,7 +590,7 @@ mod test {

{
#[crate::storage_alias]
type Foo = StorageNMap<test, (NMapKey<Blake2_128Concat, u16>), u32>;
type Foo = StorageNMap<test, (Key<Blake2_128Concat, u16>), u32>;

assert_eq!(Foo::contains_key((3,)), true);
assert_eq!(Foo::get((3,)), Some(10));
Expand Down
23 changes: 11 additions & 12 deletions primitives/npos-elections/src/assignments.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,18 +120,17 @@ impl<AccountId> StakedAssignment<AccountId> {
AccountId: IdentifierT,
{
let stake = self.total();
let distribution = self
.distribution
.into_iter()
.filter_map(|(target, w)| {
let per_thing = P::from_rational(w, stake);
if per_thing == Bounded::min_value() {
None
} else {
Some((target, per_thing))
}
})
.collect::<Vec<(AccountId, P)>>();
// most likely, the size of the staked assignment and normal assignments will be the same,
// so we pre-allocate it to prevent a sudden 2x allocation. `filter_map` starts with a size
Copy link
Member

Choose a reason for hiding this comment

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

the context about filter map makes less sense if you dont know the diff.

// of 0 by default.
// https://www.reddit.com/r/rust/comments/3spfh1/does_collect_allocate_more_than_once_while/
let mut distribution = Vec::<(AccountId, P)>::with_capacity(self.distribution.len());
self.distribution.into_iter().for_each(|(target, w)| {
let per_thing = P::from_rational(w, stake);
if per_thing != Bounded::min_value() {
distribution.push((target, per_thing));
}
});

Assignment { who: self.who, distribution }
}
Expand Down
5 changes: 5 additions & 0 deletions utils/frame/try-runtime/cli/src/commands/execute_block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,11 @@ where
.overwrite_online_at(parent_hash.to_owned());

let builder = if command.overwrite_wasm_code {
log::info!(
target: LOG_TARGET,
"replacing the in-storage :code: with the local code from {}'s chain_spec (your local repo)",
config.chain_spec.name(),
);
let (code_key, code) = extract_code(&config.chain_spec)?;
builder.inject_hashed_key_value(&[(code_key, code)])
} else {
Expand Down
5 changes: 5 additions & 0 deletions utils/frame/try-runtime/cli/src/commands/offchain_worker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,11 @@ where
let builder = command.state.builder::<Block>()?;

let builder = if command.overwrite_wasm_code {
log::info!(
target: LOG_TARGET,
"replacing the in-storage :code: with the local code from {}'s chain_spec (your local repo)",
config.chain_spec.name(),
);
let (code_key, code) = extract_code(&config.chain_spec)?;
builder.inject_hashed_key_value(&[(code_key, code)])
} else {
Expand Down
2 changes: 1 addition & 1 deletion utils/frame/try-runtime/cli/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -722,7 +722,7 @@ pub(crate) fn state_machine_call<Block: BlockT, D: NativeExecutionDispatch + 'st
sp_core::testing::TaskExecutor::new(),
)
.execute(execution.into())
.map_err(|e| format!("failed to execute 'TryRuntime_on_runtime_upgrade': {}", e))
.map_err(|e| format!("failed to execute '{}': {}", method, e))
.map_err::<sc_cli::Error, _>(Into::into)?;

Ok((changes, encoded_results))
Expand Down