Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

migrate pallet-mixnet to umbrella crate #6986

Open
wants to merge 14 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 4 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
6 changes: 1 addition & 5 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

16 changes: 16 additions & 0 deletions prdoc/pr_6986.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0
# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json

title: '[pallet-mixnet] Migrate to using frame umbrella crate'

doc:
- audience: Runtime Dev
description: This PR migrates the pallet-mixnet to use the frame umbrella crate. This
is part of the ongoing effort to migrate all pallets to use the frame umbrella crate.
The effort is tracked [here](https://github.com/paritytech/polkadot-sdk/issues/6504).

crates:
- name: pallet-mixnet
bump: none
- name: polkadot-sdk-frame
bump: none
22 changes: 4 additions & 18 deletions substrate/frame/mixnet/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,42 +17,28 @@ targets = ["x86_64-unknown-linux-gnu"]

[dependencies]
codec = { features = ["derive", "max-encoded-len"], workspace = true }
frame = { workspace = true, features = ["experimental", "runtime"] }
frame-benchmarking = { optional = true, workspace = true }
frame-support = { workspace = true }
frame-system = { workspace = true }
log = { workspace = true }
scale-info = { features = ["derive"], workspace = true }
serde = { features = ["derive"], workspace = true }
sp-application-crypto = { workspace = true }
sp-arithmetic = { workspace = true }
sp-io = { workspace = true }
sp-mixnet = { workspace = true }
sp-runtime = { workspace = true }

[features]
default = ["std"]
std = [
"codec/std",
"frame-benchmarking?/std",
"frame-support/std",
"frame-system/std",
"frame/std",
"log/std",
"scale-info/std",
"serde/std",
"sp-application-crypto/std",
"sp-arithmetic/std",
"sp-io/std",
"sp-mixnet/std",
"sp-runtime/std",
]
runtime-benchmarks = [
"frame-benchmarking/runtime-benchmarks",
"frame-support/runtime-benchmarks",
"frame-system/runtime-benchmarks",
"sp-runtime/runtime-benchmarks",
"frame/runtime-benchmarks",
]
try-runtime = [
"frame-support/try-runtime",
"frame-system/try-runtime",
"sp-runtime/try-runtime",
"frame/try-runtime",
]
65 changes: 29 additions & 36 deletions substrate/frame/mixnet/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,29 +23,25 @@

extern crate alloc;

pub use pallet::*;

use alloc::vec::Vec;
use codec::{Decode, Encode, MaxEncodedLen};
use core::cmp::Ordering;
use frame_support::{
traits::{EstimateNextSessionRotation, Get, OneSessionHandler},
BoundedVec,
};
use frame_system::{
offchain::{CreateInherent, SubmitTransaction},
pallet_prelude::BlockNumberFor,
};
pub use pallet::*;
use scale_info::TypeInfo;

UtkarshBhardwaj007 marked this conversation as resolved.
Show resolved Hide resolved
use serde::{Deserialize, Serialize};

UtkarshBhardwaj007 marked this conversation as resolved.
Show resolved Hide resolved
use frame::{
deps::{
sp_io::{self, MultiRemovalResults},
sp_runtime,
},
prelude::*,
};
use sp_application_crypto::RuntimeAppPublic;
use sp_arithmetic::traits::{CheckedSub, Saturating, UniqueSaturatedInto, Zero};
use sp_io::MultiRemovalResults;
use sp_mixnet::types::{
AuthorityId, AuthoritySignature, KxPublic, Mixnode, MixnodesErr, PeerId, SessionIndex,
SessionPhase, SessionStatus, KX_PUBLIC_SIZE,
};
UtkarshBhardwaj007 marked this conversation as resolved.
Show resolved Hide resolved
use sp_runtime::RuntimeDebug;

const LOG_TARGET: &str = "runtime::mixnet";

/// Index of an authority in the authority list for a session.
Expand Down Expand Up @@ -168,12 +164,9 @@ fn twox<BlockNumber: UniqueSaturatedInto<u64>>(
// The pallet
////////////////////////////////////////////////////////////////////////////////

#[frame_support::pallet(dev_mode)]
#[frame::pallet(dev_mode)]
Copy link
Contributor

Choose a reason for hiding this comment

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

This dev_mode is not really good for production as well 🙈 not in scope for now, but should be removed later.

Copy link
Author

Choose a reason for hiding this comment

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

I see, will have to dig in the code to understand why but we could create an issue for it if there's isn't one already.

pub mod pallet {
use super::*;
use frame_support::pallet_prelude::*;
use frame_system::pallet_prelude::*;

#[pallet::pallet]
pub struct Pallet<T>(_);

Expand Down Expand Up @@ -254,7 +247,7 @@ pub mod pallet {
StorageDoubleMap<_, Identity, SessionIndex, Identity, AuthorityIndex, BoundedMixnodeFor<T>>;

#[pallet::genesis_config]
#[derive(frame_support::DefaultNoBound)]
#[derive(frame::prelude::DefaultNoBound)]
UtkarshBhardwaj007 marked this conversation as resolved.
Show resolved Hide resolved
pub struct GenesisConfig<T: Config> {
/// The mixnode set for the very first session.
pub mixnodes: BoundedVec<BoundedMixnodeFor<T>, T::MaxAuthorities>,
Expand Down Expand Up @@ -308,7 +301,7 @@ pub mod pallet {

fn validate_unsigned(_source: TransactionSource, call: &Self::Call) -> TransactionValidity {
let Self::Call::register { registration, signature } = call else {
return InvalidTransaction::Call.into()
return InvalidTransaction::Call.into();
};

// Check session index matches
Expand All @@ -320,16 +313,16 @@ pub mod pallet {

// Check authority index is valid
if registration.authority_index >= T::MaxAuthorities::get() {
return InvalidTransaction::BadProof.into()
return InvalidTransaction::BadProof.into();
}
let Some(authority_id) = NextAuthorityIds::<T>::get(registration.authority_index)
else {
return InvalidTransaction::BadProof.into()
return InvalidTransaction::BadProof.into();
};

// Check the authority hasn't registered a mixnode yet
if Self::already_registered(registration.session_index, registration.authority_index) {
return InvalidTransaction::Stale.into()
return InvalidTransaction::Stale.into();
}

// Check signature. Note that we don't use regular signed transactions for registration
Expand All @@ -339,7 +332,7 @@ pub mod pallet {
authority_id.verify(&encoded_registration, signature)
});
if !signature_ok {
return InvalidTransaction::BadProof.into()
return InvalidTransaction::BadProof.into();
}

ValidTransaction::with_tag_prefix("MixnetRegistration")
Expand Down Expand Up @@ -368,12 +361,12 @@ impl<T: Config> Pallet<T> {
.saturating_sub(CurrentSessionStartBlock::<T>::get());
let Some(block_in_phase) = block_in_phase.checked_sub(&T::NumCoverToCurrentBlocks::get())
else {
return SessionPhase::CoverToCurrent
return SessionPhase::CoverToCurrent;
};
let Some(block_in_phase) =
block_in_phase.checked_sub(&T::NumRequestsToCurrentBlocks::get())
else {
return SessionPhase::RequestsToCurrent
return SessionPhase::RequestsToCurrent;
kianenigma marked this conversation as resolved.
Show resolved Hide resolved
};
if block_in_phase < T::NumCoverToPrevBlocks::get() {
SessionPhase::CoverToPrev
Expand Down Expand Up @@ -411,7 +404,7 @@ impl<T: Config> Pallet<T> {
return Err(MixnodesErr::InsufficientRegistrations {
num: 0,
min: T::MinMixnodes::get(),
})
});
};
Self::mixnodes(prev_session_index)
}
Expand All @@ -430,15 +423,15 @@ impl<T: Config> Pallet<T> {
// registering
let block_in_session = block_number.saturating_sub(CurrentSessionStartBlock::<T>::get());
if block_in_session < T::NumRegisterStartSlackBlocks::get() {
return false
return false;
}

let (Some(end_block), _weight) =
T::NextSessionRotation::estimate_next_session_rotation(block_number)
else {
// Things aren't going to work terribly well in this case as all the authorities will
// just pile in after the slack period...
return true
return true;
};

let remaining_blocks = end_block
Expand All @@ -447,7 +440,7 @@ impl<T: Config> Pallet<T> {
if remaining_blocks.is_zero() {
// Into the slack time at the end of the session. Not necessarily too late;
// registrations are accepted right up until the session ends.
return true
return true;
}

// Want uniform distribution over the remaining blocks, so pick this block with probability
Expand Down Expand Up @@ -496,7 +489,7 @@ impl<T: Config> Pallet<T> {
"Session {session_index} registration attempted, \
but current session is {current_session_index}",
);
return false
return false;
}

let block_number = frame_system::Pallet::<T>::block_number();
Expand All @@ -505,30 +498,30 @@ impl<T: Config> Pallet<T> {
target: LOG_TARGET,
"Waiting for the session to progress further before registering",
);
return false
return false;
}

let Some((authority_index, authority_id)) = Self::next_local_authority() else {
log::trace!(
target: LOG_TARGET,
"Not an authority in the next session; cannot register a mixnode",
);
return false
return false;
};

if Self::already_registered(session_index, authority_index) {
log::trace!(
target: LOG_TARGET,
"Already registered a mixnode for the next session",
);
return false
return false;
}

let registration =
Registration { block_number, session_index, authority_index, mixnode: mixnode.into() };
let Some(signature) = authority_id.sign(&registration.encode()) else {
log::debug!(target: LOG_TARGET, "Failed to sign registration");
return false
return false;
};
let call = Call::register { registration, signature };
let xt = T::create_inherent(call.into());
Expand Down
19 changes: 16 additions & 3 deletions substrate/frame/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -203,29 +203,40 @@ pub mod prelude {
/// Dispatch types from `frame-support`, other fundamental traits
#[doc(no_inline)]
pub use frame_support::dispatch::{GetDispatchInfo, PostDispatchInfo};
pub use frame_support::traits::{Contains, IsSubType, OnRuntimeUpgrade};
pub use frame_support::traits::{
Contains, EstimateNextSessionRotation, IsSubType, OnRuntimeUpgrade, OneSessionHandler,
UtkarshBhardwaj007 marked this conversation as resolved.
Show resolved Hide resolved
};

/// Pallet prelude of `frame-system`.
#[doc(no_inline)]
pub use frame_system::pallet_prelude::*;

/// Transaction related helpers to submit transactions.
#[doc(no_inline)]
pub use frame_system::offchain::*;

/// All FRAME-relevant derive macros.
#[doc(no_inline)]
pub use super::derive::*;

/// All hashing related things
pub use super::hashing::*;

/// All arithmetic types and traits used for safe math.
pub use super::arithmetic::*;

/// Runtime traits
#[doc(no_inline)]
pub use sp_runtime::traits::{
BlockNumberProvider, Bounded, DispatchInfoOf, Dispatchable, SaturatedConversion,
Saturating, StaticLookup, TrailingZeroInput,
};

/// Other error/result types for runtime
/// Other runtime types and traits
#[doc(no_inline)]
pub use sp_runtime::{DispatchErrorWithPostInfo, DispatchResultWithInfo, TokenError};
pub use sp_runtime::{
BoundToRuntimeAppPublic, DispatchErrorWithPostInfo, DispatchResultWithInfo, TokenError,
};
}

#[cfg(any(feature = "try-runtime", test))]
Expand Down Expand Up @@ -509,6 +520,8 @@ pub mod traits {
}

/// The arithmetic types used for safe math.
///
/// This is already part of the [`prelude`].
pub mod arithmetic {
pub use sp_arithmetic::{traits::*, *};
}
Expand Down
Loading