[frame/im-online] remove network state from heartbeats

client/offchain/src/api.rs

@@ -16,17 +16,17 @@
 // You should have received a copy of the GNU General Public License
 // along with this program. If not, see <https://www.gnu.org/licenses/>.
 
-use std::{collections::HashSet, str::FromStr, sync::Arc, thread::sleep};
+use std::{collections::HashSet, sync::Arc, thread::sleep};
 
 use crate::NetworkProvider;
 use codec::{Decode, Encode};
 use futures::Future;
 pub use http::SharedClient;
-use libp2p::{Multiaddr, PeerId};
+use libp2p::PeerId;
 use sp_core::{
 	offchain::{
-		self, HttpError, HttpRequestId, HttpRequestStatus, OffchainStorage, OpaqueMultiaddr,
-		OpaqueNetworkState, StorageKind, Timestamp,
+		self, HttpError, HttpRequestId, HttpRequestStatus, OffchainStorage, OpaqueNetworkState,
+		StorageKind, Timestamp,
 	},
 	OpaquePeerId,
 };
@@ -159,9 +159,7 @@ impl offchain::Externalities for Api {
 	}
 
 	fn network_state(&self) -> Result<OpaqueNetworkState, ()> {
-		let external_addresses = self.network_provider.external_addresses();
-
-		let state = NetworkState::new(self.network_provider.local_peer_id(), external_addresses);
+		let state = NetworkState::new(self.network_provider.local_peer_id());
 		Ok(OpaqueNetworkState::from(state))
 	}
 
@@ -238,12 +236,11 @@ impl offchain::Externalities for Api {
 #[derive(Clone, Eq, PartialEq, Debug)]
 pub struct NetworkState {
 	peer_id: PeerId,
-	external_addresses: Vec<Multiaddr>,
 }
 
 impl NetworkState {
-	fn new(peer_id: PeerId, external_addresses: Vec<Multiaddr>) -> Self {
-		NetworkState { peer_id, external_addresses }
+	fn new(peer_id: PeerId) -> Self {
+		NetworkState { peer_id }
 	}
 }
 
@@ -252,16 +249,7 @@ impl From<NetworkState> for OpaqueNetworkState {
 		let enc = Encode::encode(&state.peer_id.to_bytes());
 		let peer_id = OpaquePeerId::new(enc);
 
-		let external_addresses: Vec<OpaqueMultiaddr> = state
-			.external_addresses
-			.iter()
-			.map(|multiaddr| {
-				let e = Encode::encode(&multiaddr.to_string());
-				OpaqueMultiaddr::new(e)
-			})
-			.collect();
-
-		OpaqueNetworkState { peer_id, external_addresses }
+		OpaqueNetworkState { peer_id }
 	}
 }
 
@@ -274,20 +262,7 @@ impl TryFrom<OpaqueNetworkState> for NetworkState {
 		let bytes: Vec<u8> = Decode::decode(&mut &inner_vec[..]).map_err(|_| ())?;
 		let peer_id = PeerId::from_bytes(&bytes).map_err(|_| ())?;
 
-		let external_addresses: Result<Vec<Multiaddr>, Self::Error> = state
-			.external_addresses
-			.iter()
-			.map(|enc_multiaddr| -> Result<Multiaddr, Self::Error> {
-				let inner_vec = &enc_multiaddr.0;
-				let bytes = <Vec<u8>>::decode(&mut &inner_vec[..]).map_err(|_| ())?;
-				let multiaddr_str = String::from_utf8(bytes).map_err(|_| ())?;
-				let multiaddr = Multiaddr::from_str(&multiaddr_str).map_err(|_| ())?;
-				Ok(multiaddr)
-			})
-			.collect();
-		let external_addresses = external_addresses?;
-
-		Ok(NetworkState { peer_id, external_addresses })
+		Ok(NetworkState { peer_id })
 	}
 }
 
@@ -324,7 +299,7 @@ impl AsyncApi {
 #[cfg(test)]
 mod tests {
 	use super::*;
-	use libp2p::PeerId;
+	use libp2p::{Multiaddr, PeerId};
 	use sc_client_db::offchain::LocalStorage;
 	use sc_network::{
 		config::MultiaddrWithPeerId, types::ProtocolName, NetworkPeers, NetworkStateInfo,
@@ -410,17 +385,17 @@ mod tests {
 	}
 
 	impl NetworkStateInfo for TestNetwork {
-		fn external_addresses(&self) -> Vec<Multiaddr> {
-			Vec::new()
-		}
-
 		fn local_peer_id(&self) -> PeerId {
 			PeerId::random()
 		}
 
 		fn listen_addresses(&self) -> Vec<Multiaddr> {
 			Vec::new()
 		}
+
+		fn external_addresses(&self) -> Vec<Multiaddr> {
+			Vec::new()
+		}
 	}
 
 	fn offchain_api() -> (Api, AsyncApi) {
@@ -524,13 +499,7 @@ mod tests {
 	#[test]
 	fn should_convert_network_states() {
 		// given
-		let state = NetworkState::new(
-			PeerId::random(),
-			vec![
-				Multiaddr::try_from("/ip4/127.0.0.1/tcp/1234".to_string()).unwrap(),
-				Multiaddr::try_from("/ip6/2601:9:4f81:9700:803e:ca65:66e8:c21").unwrap(),
-			],
-		);
+		let state = NetworkState::new(PeerId::random());
 
 		// when
 		let opaque_state = OpaqueNetworkState::from(state.clone());

frame/im-online/src/benchmarking.rs

@@ -24,7 +24,7 @@ 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_core::OpaquePeerId;
 use sp_runtime::{
 	traits::{ValidateUnsigned, Zero},
 	transaction_validity::TransactionSource,
@@ -33,11 +33,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,
@@ -50,10 +48,7 @@ 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 network_state = OpaqueNetworkState { peer_id: OpaquePeerId::default() };
 	let input_heartbeat = Heartbeat {
 		block_number: T::BlockNumber::zero(),
 		network_state,
@@ -73,15 +68,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)
@@ -90,8 +83,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();
 	}: {

frame/im-online/src/lib.rs

@@ -235,27 +235,18 @@ where
 /// A type that is the same as [`OpaqueNetworkState`] but with [`Vec`] replaced with
 /// [`WeakBoundedVec<Limit>`] where Limit is the respective size limit
 /// `PeerIdEncodingLimit` represents the size limit of the encoding of `PeerId`
-/// `MultiAddrEncodingLimit` represents the size limit of the encoding of `MultiAddr`
-/// `AddressesLimit` represents the size limit of the vector of peers connected
 #[derive(Clone, Eq, PartialEq, Encode, Decode, MaxEncodedLen, TypeInfo)]
 #[codec(mel_bound())]
-#[scale_info(skip_type_params(PeerIdEncodingLimit, MultiAddrEncodingLimit, AddressesLimit))]
-pub struct BoundedOpaqueNetworkState<PeerIdEncodingLimit, MultiAddrEncodingLimit, AddressesLimit>
+#[scale_info(skip_type_params(PeerIdEncodingLimit))]
+pub struct BoundedOpaqueNetworkState<PeerIdEncodingLimit>
 where
 	PeerIdEncodingLimit: Get<u32>,
-	MultiAddrEncodingLimit: Get<u32>,
-	AddressesLimit: Get<u32>,
 {
 	/// PeerId of the local node in SCALE encoded.
 	pub peer_id: WeakBoundedVec<u8, PeerIdEncodingLimit>,
-	/// List of addresses the node knows it can be reached as.
-	pub external_addresses:
-		WeakBoundedVec<WeakBoundedVec<u8, MultiAddrEncodingLimit>, AddressesLimit>,
 }
 
-impl<PeerIdEncodingLimit: Get<u32>, MultiAddrEncodingLimit: Get<u32>, AddressesLimit: Get<u32>>
-	BoundedOpaqueNetworkState<PeerIdEncodingLimit, MultiAddrEncodingLimit, AddressesLimit>
-{
+impl<PeerIdEncodingLimit: Get<u32>> BoundedOpaqueNetworkState<PeerIdEncodingLimit> {
 	fn force_from(ons: &OpaqueNetworkState) -> Self {
 		let peer_id = WeakBoundedVec::<_, PeerIdEncodingLimit>::force_from(
 			ons.peer_id.0.clone(),
@@ -265,28 +256,7 @@ impl<PeerIdEncodingLimit: Get<u32>, MultiAddrEncodingLimit: Get<u32>, AddressesL
   				adjustment may be needed.",
 			),
 		);
-
-		let external_addresses = WeakBoundedVec::<_, AddressesLimit>::force_from(
-			ons.external_addresses
-				.iter()
-				.map(|x| {
-					WeakBoundedVec::<_, MultiAddrEncodingLimit>::force_from(
-						x.0.clone(),
-						Some(
-							"Warning: The size of the encoding of MultiAddr \
-  							is bigger than expected. A runtime configuration \
-  							adjustment may be needed.",
-						),
-					)
-				})
-				.collect(),
-			Some(
-				"Warning: The network has more peers than expected \
-  				A runtime configuration adjustment may be needed.",
-			),
-		);
-
-		Self { peer_id, external_addresses }
+		Self { peer_id }
 	}
 }
 
@@ -418,13 +388,7 @@ pub mod pallet {
 		SessionIndex,
 		Twox64Concat,
 		AuthIndex,
-		WrapperOpaque<
-			BoundedOpaqueNetworkState<
-				T::MaxPeerDataEncodingSize,
-				T::MaxPeerDataEncodingSize,
-				T::MaxPeerInHeartbeats,
-			>,
-		>,
+		WrapperOpaque<BoundedOpaqueNetworkState<T::MaxPeerDataEncodingSize>>,
 	>;
 
 	/// For each session index, we keep a mapping of `ValidatorId<T>` to the
@@ -457,16 +421,13 @@ pub mod pallet {
 	#[pallet::call]
 	impl<T: Config> Pallet<T> {
 		/// ## Complexity:
-		/// - `O(K + E)` where K is length of `Keys` (heartbeat.validators_len) and E is length of
-		///   `heartbeat.network_state.external_address`
+		/// - `O(K)` where K is length of `Keys` (heartbeat.validators_len)
 		///   - `O(K)`: decoding of length `K`
-		///   - `O(E)`: decoding/encoding of length `E`
 		// NOTE: the weight includes the cost of validate_unsigned as it is part of the cost to
 		// import block with such an extrinsic.
 		#[pallet::call_index(0)]
 		#[pallet::weight(<T as Config>::WeightInfo::validate_unsigned_and_then_heartbeat(
 			heartbeat.validators_len as u32,
-			heartbeat.network_state.external_addresses.len() as u32,
 		))]
 		pub fn heartbeat(
 			origin: OriginFor<T>,
@@ -485,11 +446,10 @@ pub mod pallet {
 			if let (false, Some(public)) = (exists, public) {
 				Self::deposit_event(Event::<T>::HeartbeatReceived { authority_id: public.clone() });
 
-				let network_state_bounded = BoundedOpaqueNetworkState::<
-					T::MaxPeerDataEncodingSize,
-					T::MaxPeerDataEncodingSize,
-					T::MaxPeerInHeartbeats,
-				>::force_from(&heartbeat.network_state);
+				let network_state_bounded =
+					BoundedOpaqueNetworkState::<T::MaxPeerDataEncodingSize>::force_from(
+						&heartbeat.network_state,
+					);
 				ReceivedHeartbeats::<T>::insert(
 					&current_session,
 					&heartbeat.authority_index,

frame/im-online/src/tests.rs

@@ -125,10 +125,7 @@ fn heartbeat(
 
 	let heartbeat = Heartbeat {
 		block_number,
-		network_state: OpaqueNetworkState {
-			peer_id: OpaquePeerId(vec![1]),
-			external_addresses: vec![],
-		},
+		network_state: OpaqueNetworkState { peer_id: OpaquePeerId(vec![1]) },
 		session_index,
 		authority_index,
 		validators_len: validators.len() as u32,