Skip to content

Commit

Permalink
A0-4213: Move aleph-runtime dependency in finality-aleph to dev-depen…
Browse files Browse the repository at this point in the history
…dencies (#1665)

# Description

This PR moves `aleph-runtime` dependency to `dev-dependencies` in
`finality-aleph`. The only reason we have normal dependency is that in
`finality-aleph` we read storage `Session.QueuedKeys`, and that storage
type is `SessionKeys` type from `aleph-runtime`. We move that type to
`primitives`, which breaks normal dependency in `finality-aleph`.

The reason this can be moved is that currently in `aleph-runtime`,
`SessionKeys` are generated using `impl_opaque_keys` macro. You can use
`cargo expand` too see that
```
use ::sp_runtime::serde as __opaque_keys_serde_import__SessionKeys;
#[serde(crate = "__opaque_keys_serde_import__SessionKeys")]
pub struct SessionKeys {
    pub aura: <Aura as ::sp_runtime::BoundToRuntimeAppPublic>::Public,
    pub aleph: <Aleph as ::sp_runtime::BoundToRuntimeAppPublic>::Public,
}
```
So we see that `aura` and `aleph` fields are some public key types,
retrieved from `pallet-aura` and `pallet-aleph`, that both implement
`BoundToRuntimeAppPublic` trait, example for
[Aura](https://github.com/Cardinal-Cryptography/polkadot-sdk/blob/aleph-v1.3.0/substrate/frame/aura/src/lib.rs#L312).
`AuthorityId` is defined in `aleph-runtime` as
```
impl pallet_aura::Config for Runtime {
    type MaxAuthorities = MaxAuthorities;
    type AuthorityId = AuraId;
    ...
}
```
and for `pallet-aleph`
```
impl pallet_aleph::Config for Runtime {
    type AuthorityId = AlephId;
    ...
}
```
Where `AuraId` and `AlephId` are types from `primitives. So really what
`cargo expand` tell us is `SessionKeys` is just a below struct
```
use ::sp_runtime::serde as __opaque_keys_serde_import__SessionKeys;
#[serde(crate = "__opaque_keys_serde_import__SessionKeys")]
pub struct SessionKeys {
    pub aura: AuraId,
    pub aleph: AlephId,
}
```
In this PR, we propose to just implement session keys in `primites` as
```
impl_opaque_keys! {
    pub struct AlephNodeSessionKeys {
        pub aura: AuraId,
        pub aleph: AuthorityId,
    }
}
```
This expands via `cargo expand` to
```
use ::sp_runtime::serde as __opaque_keys_serde_import__AlephNodeSessionKeys;
#[serde(crate = "__opaque_keys_serde_import__AlephNodeSessionKeys")]
pub struct AlephNodeSessionKeys {
    pub aura: <AuraId as ::sp_runtime::BoundToRuntimeAppPublic>::Public,
    pub aleph: <AuthorityId as ::sp_runtime::BoundToRuntimeAppPublic>::Public,
}
```
Now this is not entirely the same implementation at the first glance,
previously we had `<Aura as
::sp_runtime::BoundToRuntimeAppPublic>::Public` and now we have `<AuraId
as ::sp_runtime::BoundToRuntimeAppPublic>::Public`, where `Aura` is
`pallet-aura::pallet` and `AuraId` is
`sp_consensus_aura::sr25519::AuthorityId`. This compiles successfully
though and import from genesis test works. The reason it compiles is
that there's a [default
implementation](https://github.com/Cardinal-Cryptography/polkadot-sdk/blob/aleph-v1.3.0/substrate/primitives/application-crypto/src/traits.rs#L195-L197)
of `BoundToRuntimeAppPublic` for all types that are `RuntimeAppPublic`,
and both `AuraId` and `AlephId` must implement `RuntimeAppPublic`.

## Type of change

Please delete options that are not relevant.

- New feature (non-breaking change which adds functionality)
  • Loading branch information
Marcin-Radecki authored Mar 27, 2024
1 parent d79d6ac commit db84c42
Show file tree
Hide file tree
Showing 6 changed files with 24 additions and 21 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

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

8 changes: 4 additions & 4 deletions bin/node/src/chain_spec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ use std::{collections::HashSet, str::FromStr, string::ToString};

use aleph_runtime::{
AccountId, AlephConfig, AuraConfig, BalancesConfig, CommitteeManagementConfig, ElectionsConfig,
Feature, FeatureControlConfig, Perbill, RuntimeGenesisConfig, SessionConfig, SessionKeys,
StakingConfig, SudoConfig, SystemConfig, VestingConfig, WASM_BINARY,
Feature, FeatureControlConfig, Perbill, RuntimeGenesisConfig, SessionConfig, StakingConfig,
SudoConfig, SystemConfig, VestingConfig, WASM_BINARY,
};
use libp2p::PeerId;
use pallet_staking::{Forcing, StakerStatus};
Expand All @@ -19,8 +19,8 @@ use sp_core::{sr25519, Pair};

use crate::aleph_primitives::{
staking::{MIN_NOMINATOR_BOND, MIN_VALIDATOR_BOND},
AuraId, AuthorityId as AlephId, SessionValidators, Version as FinalityVersion,
ADDRESSES_ENCODING, LEGACY_FINALITY_VERSION, TOKEN_DECIMALS,
AlephNodeSessionKeys as SessionKeys, AuraId, AuthorityId as AlephId, SessionValidators,
Version as FinalityVersion, ADDRESSES_ENCODING, LEGACY_FINALITY_VERSION, TOKEN_DECIMALS,
};

pub const CHAINTYPE_DEV: &str = "dev";
Expand Down
20 changes: 7 additions & 13 deletions bin/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,12 @@ pub use pallet_timestamp::Call as TimestampCall;
use pallet_transaction_payment::{CurrencyAdapter, Multiplier, TargetedFeeAdjustment};
use parity_scale_codec::{Decode, Encode, MaxEncodedLen};
use primitives::{
staking::MAX_NOMINATORS_REWARDED_PER_VALIDATOR, wrap_methods, ApiError as AlephApiError,
AuraId, AuthorityId as AlephId, Block as AlephBlock, BlockId as AlephBlockId,
BlockNumber as AlephBlockNumber, Header as AlephHeader, SessionAuthorityData, SessionCommittee,
SessionIndex, SessionInfoProvider, SessionValidatorError, Version as FinalityVersion,
ADDRESSES_ENCODING, DEFAULT_BAN_REASON_LENGTH, DEFAULT_MAX_WINNERS, DEFAULT_SESSIONS_PER_ERA,
staking::MAX_NOMINATORS_REWARDED_PER_VALIDATOR, wrap_methods,
AlephNodeSessionKeys as SessionKeys, ApiError as AlephApiError, AuraId, AuthorityId as AlephId,
Block as AlephBlock, BlockId as AlephBlockId, BlockNumber as AlephBlockNumber,
Header as AlephHeader, SessionAuthorityData, SessionCommittee, SessionIndex,
SessionInfoProvider, SessionValidatorError, Version as FinalityVersion, ADDRESSES_ENCODING,
DEFAULT_BAN_REASON_LENGTH, DEFAULT_MAX_WINNERS, DEFAULT_SESSIONS_PER_ERA,
DEFAULT_SESSION_PERIOD, MAX_BLOCK_SIZE, MILLISECS_PER_BLOCK, TOKEN,
};
pub use primitives::{AccountId, AccountIndex, Balance, Hash, Nonce, Signature};
Expand Down Expand Up @@ -381,13 +382,6 @@ impl pallet_vk_storage::Config for Runtime {
type StorageCharge = VkStorageCharge;
}

impl_opaque_keys! {
pub struct SessionKeys {
pub aura: Aura,
pub aleph: Aleph,
}
}

parameter_types! {
pub const SessionPeriod: u32 = DEFAULT_SESSION_PERIOD;
pub const MaximumBanReasonLength: u32 = DEFAULT_BAN_REASON_LENGTH;
Expand Down Expand Up @@ -434,7 +428,7 @@ impl pallet_session::Config for Runtime {
type ShouldEndSession = pallet_session::PeriodicSessions<SessionPeriod, Offset>;
type NextSessionRotation = pallet_session::PeriodicSessions<SessionPeriod, Offset>;
type SessionManager = Aleph;
type SessionHandler = <SessionKeys as OpaqueKeys>::KeyTypeIdProviders;
type SessionHandler = (Aura, Aleph);
type Keys = SessionKeys;
type WeightInfo = pallet_session::weights::SubstrateWeight<Runtime>;
}
Expand Down
3 changes: 2 additions & 1 deletion finality-aleph/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ ip_network = { workspace = true }
log = { workspace = true }
lru = { workspace = true }
parity-scale-codec = { workspace = true, features = ["derive"] }
scale-info = { workspace = true, features = ["derive"] }
parking_lot = { workspace = true }
rand = { workspace = true }
serde = { workspace = true }
Expand Down Expand Up @@ -71,14 +72,14 @@ sp-runtime = { workspace = true }
sp-state-machine = { workspace = true }
sp-timestamp = { workspace = true }
sp-trie = { workspace = true }
aleph-runtime = { workspace = true }

[dev-dependencies]
substrate-test-runtime-client = { workspace = true }
substrate-test-runtime = { workspace = true }
substrate-test-client = { workspace = true }
sc-block-builder = { workspace = true }
sc-basic-authorship = { workspace = true }
aleph-runtime = { workspace = true }

[features]
only_legacy = []
5 changes: 2 additions & 3 deletions finality-aleph/src/runtime_api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ use std::{
sync::Arc,
};

use aleph_runtime::SessionKeys;
use frame_support::StorageHasher;
use parity_scale_codec::{Decode, DecodeAll, Encode, Error as DecodeError};
use sc_client_api::Backend;
Expand All @@ -25,8 +24,6 @@ pub trait RuntimeApi: Clone + Send + Sync + 'static {
-> Result<Vec<(AccountId, AuraId)>, Self::Error>;
}

type QueuedKeys = Vec<(AccountId, SessionKeys)>;

pub struct RuntimeApiImpl<C, B, BE>
where
C: ClientForAleph<B, BE> + Send + Sync + 'static,
Expand Down Expand Up @@ -137,6 +134,8 @@ impl Display for ApiError {
}
}

type QueuedKeys = Vec<(AccountId, primitives::AlephNodeSessionKeys)>;

impl<C, B, BE> RuntimeApi for RuntimeApiImpl<C, B, BE>
where
C: ClientForAleph<B, BE> + Send + Sync + 'static,
Expand Down
8 changes: 8 additions & 0 deletions primitives/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ pub use sp_runtime::{
BoundedVec, ConsensusEngineId, OpaqueExtrinsic as UncheckedExtrinsic, Perbill,
};
use sp_runtime::{
impl_opaque_keys,
traits::{IdentifyAccount, Verify},
MultiSignature, Perquintill,
};
Expand All @@ -35,6 +36,13 @@ sp_application_crypto::with_pair! {
pub type AuthoritySignature = app::Signature;
pub type AuthorityId = app::Public;

impl_opaque_keys! {
pub struct AlephNodeSessionKeys {
pub aura: AuraId,
pub aleph: AuthorityId,
}
}

pub type Balance = u128;
pub type Header = generic::Header<BlockNumber, BlakeTwo256>;
pub type Block = generic::Block<Header, UncheckedExtrinsic>;
Expand Down

0 comments on commit db84c42

Please sign in to comment.