Skip to content

Commit

Permalink
[frame/im-online] remove network state from heartbeats (paritytech#14251
Browse files Browse the repository at this point in the history
)

* [frame/im-online] remove `external_addresses` from heartbeats

Users should use DHT for discovering new nodes. The reason for adding external addresses was
unstable work of authority discovery (see paritytech#2719),
which is now stable. Hence we can safely remove `external_addresses`.

Refs https://github.com/paritytech/polkadot/issues/7181

* remove unused import

* run benchmark

* remove external_addresses from offchain NetworkState

* add missing fn to TestNetwork

* Revert "run benchmark"

This reverts commit a282042.

* update weights

* address @bkchr comments

* remove duplicate fn

* cleanup benchmarking.rs

* fix executor tests

* remove peer_id from hearbeat as well

paritytech#14251 (comment)

* remove MaxPeerDataEncodingSize

* change storage value type to `()`

paritytech#14251 (comment)

* scaffold storage migration

* no need to check the type actually

* remove unnecessary types from v0 mod

* add a test for migration

* expose Config types

+ pre_upgrade and post_upgrade working fn

* fix test

* replace dummy type with ConstU32

* add some comments to migration test

* fix comment

* respond to @bkchr comments

* use BoundedOpaqueNetworkState::default

intead of using default for each field
  • Loading branch information
melekes authored and nathanwhit committed Jul 19, 2023
1 parent 0cd8efc commit 08e357c
Show file tree
Hide file tree
Showing 10 changed files with 226 additions and 198 deletions.
1 change: 0 additions & 1 deletion bin/node/executor/tests/submit_transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ fn should_submit_unsigned_transaction() {
pallet_im_online::sr25519::AuthoritySignature::try_from(vec![0; 64]).unwrap();
let heartbeat_data = pallet_im_online::Heartbeat {
block_number: 1,
network_state: Default::default(),
session_index: 1,
authority_index: 0,
validators_len: 0,
Expand Down
7 changes: 3 additions & 4 deletions bin/node/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -328,8 +328,9 @@ impl InstanceFilter<RuntimeCall> for ProxyType {
RuntimeCall::Elections(..) |
RuntimeCall::Treasury(..)
),
ProxyType::Staking =>
matches!(c, RuntimeCall::Staking(..) | RuntimeCall::FastUnstake(..)),
ProxyType::Staking => {
matches!(c, RuntimeCall::Staking(..) | RuntimeCall::FastUnstake(..))
},
}
}
fn is_superset(&self, o: &Self) -> bool {
Expand Down Expand Up @@ -1261,7 +1262,6 @@ parameter_types! {
pub const MaxAuthorities: u32 = 100;
pub const MaxKeys: u32 = 10_000;
pub const MaxPeerInHeartbeats: u32 = 10_000;
pub const MaxPeerDataEncodingSize: u32 = 1_000;
}

impl<LocalCall> frame_system::offchain::CreateSignedTransaction<LocalCall> for Runtime
Expand Down Expand Up @@ -1329,7 +1329,6 @@ impl pallet_im_online::Config for Runtime {
type WeightInfo = pallet_im_online::weights::SubstrateWeight<Runtime>;
type MaxKeys = MaxKeys;
type MaxPeerInHeartbeats = MaxPeerInHeartbeats;
type MaxPeerDataEncodingSize = MaxPeerDataEncodingSize;
}

impl pallet_offences::Config for Runtime {
Expand Down
1 change: 0 additions & 1 deletion client/offchain/src/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -324,7 +324,6 @@ impl AsyncApi {
#[cfg(test)]
mod tests {
use super::*;
use libp2p::PeerId;
use sc_client_db::offchain::LocalStorage;
use sc_network::{
config::MultiaddrWithPeerId, types::ProtocolName, NetworkPeers, NetworkStateInfo,
Expand Down
17 changes: 3 additions & 14 deletions frame/im-online/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ use super::*;
use frame_benchmarking::v1::benchmarks;
use frame_support::{traits::UnfilteredDispatchable, WeakBoundedVec};
use frame_system::RawOrigin;
use sp_core::{offchain::OpaqueMultiaddr, OpaquePeerId};
use sp_runtime::{
traits::{ValidateUnsigned, Zero},
transaction_validity::TransactionSource,
Expand All @@ -33,11 +32,9 @@ use sp_runtime::{
use crate::Pallet as ImOnline;

const MAX_KEYS: u32 = 1000;
const MAX_EXTERNAL_ADDRESSES: u32 = 100;

pub fn create_heartbeat<T: Config>(
k: u32,
e: u32,
) -> Result<
(crate::Heartbeat<T::BlockNumber>, <T::AuthorityId as RuntimeAppPublic>::Signature),
&'static str,
Expand All @@ -50,13 +47,8 @@ pub fn create_heartbeat<T: Config>(
.map_err(|()| "More than the maximum number of keys provided")?;
Keys::<T>::put(bounded_keys);

let network_state = OpaqueNetworkState {
peer_id: OpaquePeerId::default(),
external_addresses: vec![OpaqueMultiaddr::new(vec![0; 32]); e as usize],
};
let input_heartbeat = Heartbeat {
block_number: T::BlockNumber::zero(),
network_state,
session_index: 0,
authority_index: k - 1,
validators_len: keys.len() as u32,
Expand All @@ -73,15 +65,13 @@ benchmarks! {
#[extra]
heartbeat {
let k in 1 .. MAX_KEYS;
let e in 1 .. MAX_EXTERNAL_ADDRESSES;
let (input_heartbeat, signature) = create_heartbeat::<T>(k, e)?;
let (input_heartbeat, signature) = create_heartbeat::<T>(k)?;
}: _(RawOrigin::None, input_heartbeat, signature)

#[extra]
validate_unsigned {
let k in 1 .. MAX_KEYS;
let e in 1 .. MAX_EXTERNAL_ADDRESSES;
let (input_heartbeat, signature) = create_heartbeat::<T>(k, e)?;
let (input_heartbeat, signature) = create_heartbeat::<T>(k)?;
let call = Call::heartbeat { heartbeat: input_heartbeat, signature };
}: {
ImOnline::<T>::validate_unsigned(TransactionSource::InBlock, &call)
Expand All @@ -90,8 +80,7 @@ benchmarks! {

validate_unsigned_and_then_heartbeat {
let k in 1 .. MAX_KEYS;
let e in 1 .. MAX_EXTERNAL_ADDRESSES;
let (input_heartbeat, signature) = create_heartbeat::<T>(k, e)?;
let (input_heartbeat, signature) = create_heartbeat::<T>(k)?;
let call = Call::heartbeat { heartbeat: input_heartbeat, signature };
let call_enc = call.encode();
}: {
Expand Down
Loading

0 comments on commit 08e357c

Please sign in to comment.