[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

client/offchain/src/api.rs

@@ -16,7 +16,7 @@
 // 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};
@@ -25,8 +25,8 @@ pub use http::SharedClient;
 use libp2p::{Multiaddr, PeerId};
 use sp_core::{
 	offchain::{
-		self, HttpError, HttpRequestId, HttpRequestStatus, OffchainStorage, OpaqueMultiaddr,
-		OpaqueNetworkState, StorageKind, Timestamp,
+		self, HttpError, HttpRequestId, HttpRequestStatus, OffchainStorage, OpaqueNetworkState,
+		StorageKind, Timestamp,
 	},
 	OpaquePeerId,
 };
@@ -252,16 +252,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 +265,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, external_addresses: vec![] })
 	}
 }
 

frame/im-online/src/benchmarking.rs

@@ -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,

primitives/core/src/offchain/mod.rs

@@ -189,8 +189,6 @@ impl TryFrom<u32> for HttpRequestStatus {
 pub struct OpaqueNetworkState {
 	/// PeerId of the local node in SCALE encoded.
 	pub peer_id: OpaquePeerId,
-	/// List of addresses the node knows it can be reached as.
-	pub external_addresses: Vec<OpaqueMultiaddr>,
 }
 
 /// Simple blob to hold a `Multiaddr` without committing to its format.

primitives/core/src/offchain/testing.rs

@@ -217,7 +217,7 @@ impl offchain::Externalities for TestOffchainExt {
 	}
 
 	fn network_state(&self) -> Result<OpaqueNetworkState, ()> {
-		Ok(OpaqueNetworkState { peer_id: Default::default(), external_addresses: vec![] })
+		Ok(OpaqueNetworkState { peer_id: Default::default() })
 	}
 
 	fn timestamp(&mut self) -> Timestamp {