diff --git a/.maintain/monitoring/alerting-rules/alerting-rule-tests.yaml b/.maintain/monitoring/alerting-rules/alerting-rule-tests.yaml index 40a489bd09cf0..7ad916f022154 100644 --- a/.maintain/monitoring/alerting-rules/alerting-rule-tests.yaml +++ b/.maintain/monitoring/alerting-rules/alerting-rule-tests.yaml @@ -175,7 +175,7 @@ tests: polkadot-abcdef01234-abcdef has been monotonically increasing for more than 10 minutes." - exp_labels: - severity: critical + severity: warning pod: polkadot-abcdef01234-abcdef instance: polkadot-abcdef01234-abcdef job: polkadot @@ -190,7 +190,7 @@ tests: # same. Thus expect an alert. exp_alerts: - exp_labels: - severity: critical + severity: warning pod: polkadot-abcdef01234-abcdef instance: polkadot-abcdef01234-abcdef job: polkadot diff --git a/.maintain/monitoring/alerting-rules/alerting-rules.yaml b/.maintain/monitoring/alerting-rules/alerting-rules.yaml index 1aed87ad84f88..bc3243d732b4f 100644 --- a/.maintain/monitoring/alerting-rules/alerting-rules.yaml +++ b/.maintain/monitoring/alerting-rules/alerting-rules.yaml @@ -74,7 +74,7 @@ groups: increase(polkadot_sub_txpool_validations_finished[5m]) > 0' for: 30m labels: - severity: critical + severity: warning annotations: message: 'The transaction pool size on node {{ $labels.instance }} has been monotonically increasing for more than 30 minutes.' @@ -83,7 +83,7 @@ groups: polkadot_sub_txpool_validations_finished > 10000' for: 5m labels: - severity: critical + severity: warning annotations: message: 'The transaction pool size on node {{ $labels.instance }} has been above 10_000 for more than 5 minutes.' diff --git a/client/api/src/backend.rs b/client/api/src/backend.rs index 14841d8d3e96f..09e9e0cb2e173 100644 --- a/client/api/src/backend.rs +++ b/client/api/src/backend.rs @@ -475,6 +475,12 @@ pub trait Backend: AuxStore + Send + Sync { revert_finalized: bool, ) -> sp_blockchain::Result<(NumberFor, HashSet)>; + /// Discard non-best, unfinalized leaf block. + fn remove_leaf_block( + &self, + hash: &Block::Hash, + ) -> sp_blockchain::Result<()>; + /// Insert auxiliary data into key-value store. fn insert_aux< 'a, diff --git a/client/api/src/in_mem.rs b/client/api/src/in_mem.rs index fb9ecb9d0d1c9..0d40bb3354cc3 100644 --- a/client/api/src/in_mem.rs +++ b/client/api/src/in_mem.rs @@ -777,6 +777,13 @@ impl backend::Backend for Backend where Block::Hash Ok((Zero::zero(), HashSet::new())) } + fn remove_leaf_block( + &self, + _hash: &Block::Hash, + ) -> sp_blockchain::Result<()> { + Ok(()) + } + fn get_import_lock(&self) -> &RwLock<()> { &self.import_lock } diff --git a/client/api/src/leaves.rs b/client/api/src/leaves.rs index 47cac8b186f4a..0474d5bb8fe17 100644 --- a/client/api/src/leaves.rs +++ b/client/api/src/leaves.rs @@ -216,8 +216,8 @@ impl LeafSet where self.pending_removed.clear(); } - #[cfg(test)] - fn contains(&self, number: N, hash: H) -> bool { + /// Check if given block is a leaf. + pub fn contains(&self, number: N, hash: H) -> bool { self.storage.get(&Reverse(number)).map_or(false, |hashes| hashes.contains(&hash)) } diff --git a/client/consensus/slots/src/slots.rs b/client/consensus/slots/src/slots.rs index b5ce71dfbf4c9..665f7c58ba94b 100644 --- a/client/consensus/slots/src/slots.rs +++ b/client/consensus/slots/src/slots.rs @@ -40,11 +40,12 @@ pub fn duration_now() -> Duration { } /// Returns the duration until the next slot from now. -pub fn time_until_next(slot_duration: Duration) -> Duration { - let remaining_full_millis = slot_duration.as_millis() - - (duration_now().as_millis() % slot_duration.as_millis()) - - 1; - Duration::from_millis(remaining_full_millis as u64) +pub fn time_until_next_slot(slot_duration: Duration) -> Duration { + let now = duration_now().as_millis(); + + let next_slot = (now + slot_duration.as_millis()) / slot_duration.as_millis(); + let remaining_millis = next_slot * slot_duration.as_millis() - now; + Duration::from_millis(remaining_millis as u64) } /// Information about a slot. @@ -86,7 +87,7 @@ impl SlotInfo { duration, chain_head, block_size_limit, - ends_at: Instant::now() + time_until_next(duration), + ends_at: Instant::now() + time_until_next_slot(duration), } } } @@ -132,7 +133,7 @@ where self.inner_delay = match self.inner_delay.take() { None => { // schedule wait. - let wait_dur = time_until_next(self.slot_duration); + let wait_dur = time_until_next_slot(self.slot_duration); Some(Delay::new(wait_dur)) } Some(d) => Some(d), @@ -143,7 +144,12 @@ where } // timeout has fired. - let ends_at = Instant::now() + time_until_next(self.slot_duration); + let ends_in = time_until_next_slot(self.slot_duration); + + // reschedule delay for next slot. + self.inner_delay = Some(Delay::new(ends_in)); + + let ends_at = Instant::now() + ends_in; let chain_head = match self.client.best_chain() { Ok(x) => x, @@ -174,10 +180,6 @@ where let slot = inherent_data_providers.slot(); let inherent_data = inherent_data_providers.create_inherent_data()?; - // reschedule delay for next slot. - let ends_in = time_until_next(self.slot_duration); - self.inner_delay = Some(Delay::new(ends_in)); - // never yield the same slot twice. if slot > self.last_slot { self.last_slot = slot; diff --git a/client/db/src/lib.rs b/client/db/src/lib.rs index 42354908490b0..f88b8b6e41af2 100644 --- a/client/db/src/lib.rs +++ b/client/db/src/lib.rs @@ -654,6 +654,7 @@ impl HeaderMetadata for BlockchainDb { } fn remove_header_metadata(&self, hash: Block::Hash) { + self.header_cache.lock().remove(&hash); self.header_metadata_cache.remove_header_metadata(hash); } } @@ -2002,6 +2003,59 @@ impl sc_client_api::backend::Backend for Backend { Ok((reverted, reverted_finalized)) } + fn remove_leaf_block( + &self, + hash: &Block::Hash, + ) -> ClientResult<()> { + let best_hash = self.blockchain.info().best_hash; + + if best_hash == *hash { + return Err( + sp_blockchain::Error::Backend( + format!("Can't remove best block {:?}", hash) + ) + ) + } + + let hdr = self.blockchain.header_metadata(hash.clone())?; + if !self.have_state_at(&hash, hdr.number) { + return Err( + sp_blockchain::Error::UnknownBlock( + format!("State already discarded for {:?}", hash) + ) + ) + } + + let mut leaves = self.blockchain.leaves.write(); + if !leaves.contains(hdr.number, *hash) { + return Err( + sp_blockchain::Error::Backend( + format!("Can't remove non-leaf block {:?}", hash) + ) + ) + } + + let mut transaction = Transaction::new(); + if let Some(commit) = self.storage.state_db.remove(hash) { + apply_state_commit(&mut transaction, commit); + } + transaction.remove(columns::KEY_LOOKUP, hash.as_ref()); + let changes_trie_cache_ops = self.changes_tries_storage.revert( + &mut transaction, + &cache::ComplexBlockId::new( + *hash, + hdr.number, + ), + )?; + + self.changes_tries_storage.post_commit(Some(changes_trie_cache_ops)); + leaves.revert(hash.clone(), hdr.number); + leaves.prepare_transaction(&mut transaction, columns::META, meta_keys::LEAF_PREFIX); + self.storage.db.commit(transaction)?; + self.blockchain().remove_header_metadata(*hash); + Ok(()) + } + fn blockchain(&self) -> &BlockchainDb { &self.blockchain } @@ -3041,4 +3095,36 @@ pub(crate) mod tests { } } } + + #[test] + fn remove_leaf_block_works() { + let backend = Backend::::new_test_with_tx_storage( + 2, + 10, + TransactionStorageMode::StorageChain + ); + let mut blocks = Vec::new(); + let mut prev_hash = Default::default(); + for i in 0 .. 2 { + let hash = insert_block(&backend, i, prev_hash, None, Default::default(), vec![i.into()], None); + blocks.push(hash); + prev_hash = hash; + } + + // insert a fork at block 2, which becomes best block + let best_hash = insert_block( + &backend, + 1, + blocks[0], + None, + sp_core::H256::random(), + vec![42.into()], + None + ); + assert!(backend.remove_leaf_block(&best_hash).is_err()); + assert!(backend.have_state_at(&prev_hash, 1)); + backend.remove_leaf_block(&prev_hash).unwrap(); + assert_eq!(None, backend.blockchain().header(BlockId::hash(prev_hash.clone())).unwrap()); + assert!(!backend.have_state_at(&prev_hash, 1)); + } } diff --git a/client/finality-grandpa/src/communication/tests.rs b/client/finality-grandpa/src/communication/tests.rs index dc37a1615f415..bfc5b1d10a413 100644 --- a/client/finality-grandpa/src/communication/tests.rs +++ b/client/finality-grandpa/src/communication/tests.rs @@ -295,6 +295,7 @@ fn good_commit_leads_to_relay() { let _ = sender.unbounded_send(NetworkEvent::NotificationStreamOpened { remote: sender_id.clone(), protocol: GRANDPA_PROTOCOL_NAME.into(), + negotiated_fallback: None, role: ObservedRole::Full, }); @@ -308,6 +309,7 @@ fn good_commit_leads_to_relay() { let _ = sender.unbounded_send(NetworkEvent::NotificationStreamOpened { remote: receiver_id.clone(), protocol: GRANDPA_PROTOCOL_NAME.into(), + negotiated_fallback: None, role: ObservedRole::Full, }); @@ -442,6 +444,7 @@ fn bad_commit_leads_to_report() { let _ = sender.unbounded_send(NetworkEvent::NotificationStreamOpened { remote: sender_id.clone(), protocol: GRANDPA_PROTOCOL_NAME.into(), + negotiated_fallback: None, role: ObservedRole::Full, }); let _ = sender.unbounded_send(NetworkEvent::NotificationsReceived { diff --git a/client/finality-grandpa/src/lib.rs b/client/finality-grandpa/src/lib.rs index e1c3a2c131540..672b08d0b7142 100644 --- a/client/finality-grandpa/src/lib.rs +++ b/client/finality-grandpa/src/lib.rs @@ -690,6 +690,7 @@ pub struct GrandpaParams { pub fn grandpa_peers_set_config() -> sc_network::config::NonDefaultSetConfig { sc_network::config::NonDefaultSetConfig { notifications_protocol: communication::GRANDPA_PROTOCOL_NAME.into(), + fallback_names: Vec::new(), // Notifications reach ~256kiB in size at the time of writing on Kusama and Polkadot. max_notification_size: 1024 * 1024, set_config: sc_network::config::SetConfig { @@ -1134,12 +1135,12 @@ fn local_authority_id( voters: &VoterSet, keystore: Option<&SyncCryptoStorePtr>, ) -> Option { - keystore.and_then(|keystore| { + keystore.and_then(|keystore| { voters .iter() .find(|(p, _)| { SyncCryptoStore::has_keys(&**keystore, &[(p.to_raw_vec(), AuthorityId::ID)]) }) - .map(|(p, _)| p.clone()) + .map(|(p, _)| p.clone()) }) } diff --git a/client/light/src/backend.rs b/client/light/src/backend.rs index 621ada13ff61d..d6f86209afe9f 100644 --- a/client/light/src/backend.rs +++ b/client/light/src/backend.rs @@ -246,6 +246,13 @@ impl ClientBackend for Backend> Err(ClientError::NotAvailableOnLightClient) } + fn remove_leaf_block( + &self, + _hash: &Block::Hash, + ) -> ClientResult<()> { + Err(ClientError::NotAvailableOnLightClient) + } + fn get_import_lock(&self) -> &RwLock<()> { &self.import_lock } diff --git a/client/network-gossip/src/bridge.rs b/client/network-gossip/src/bridge.rs index 235ac98dc3968..fd9aac96c0102 100644 --- a/client/network-gossip/src/bridge.rs +++ b/client/network-gossip/src/bridge.rs @@ -188,7 +188,7 @@ impl Future for GossipEngine { Event::SyncDisconnected { remote } => { this.network.remove_set_reserved(remote, this.protocol.clone()); } - Event::NotificationStreamOpened { remote, protocol, role } => { + Event::NotificationStreamOpened { remote, protocol, role, .. } => { if protocol != this.protocol { continue; } @@ -416,6 +416,7 @@ mod tests { Event::NotificationStreamOpened { remote: remote_peer.clone(), protocol: protocol.clone(), + negotiated_fallback: None, role: ObservedRole::Authority, } ).expect("Event stream is unbounded; qed."); @@ -575,6 +576,7 @@ mod tests { Event::NotificationStreamOpened { remote: remote_peer.clone(), protocol: protocol.clone(), + negotiated_fallback: None, role: ObservedRole::Authority, } ).expect("Event stream is unbounded; qed."); diff --git a/client/network/src/behaviour.rs b/client/network/src/behaviour.rs index a73685ed3bf32..17c38b6f95456 100644 --- a/client/network/src/behaviour.rs +++ b/client/network/src/behaviour.rs @@ -124,6 +124,11 @@ pub enum BehaviourOut { remote: PeerId, /// The concerned protocol. Each protocol uses a different substream. protocol: Cow<'static, str>, + /// If the negotiation didn't use the main name of the protocol (the one in + /// `notifications_protocol`), then this field contains which name has actually been + /// used. + /// See also [`crate::Event::NotificationStreamOpened`]. + negotiated_fallback: Option>, /// Object that permits sending notifications to the peer. notifications_sink: NotificationsSink, /// Role of the remote. @@ -324,10 +329,13 @@ Behaviour { &target, &self.block_request_protocol_name, buf, pending_response, IfDisconnected::ImmediateError, ); }, - CustomMessageOutcome::NotificationStreamOpened { remote, protocol, roles, notifications_sink } => { + CustomMessageOutcome::NotificationStreamOpened { + remote, protocol, negotiated_fallback, roles, notifications_sink + } => { self.events.push_back(BehaviourOut::NotificationStreamOpened { remote, protocol, + negotiated_fallback, role: reported_roles_to_observed_role(roles), notifications_sink: notifications_sink.clone(), }); diff --git a/client/network/src/config.rs b/client/network/src/config.rs index 3864b77d88be3..77618f2771148 100644 --- a/client/network/src/config.rs +++ b/client/network/src/config.rs @@ -541,6 +541,13 @@ pub struct NonDefaultSetConfig { /// > **Note**: This field isn't present for the default set, as this is handled internally /// > by the networking code. pub notifications_protocol: Cow<'static, str>, + /// If the remote reports that it doesn't support the protocol indicated in the + /// `notifications_protocol` field, then each of these fallback names will be tried one by + /// one. + /// + /// If a fallback is used, it will be reported in + /// [`crate::Event::NotificationStreamOpened::negotiated_fallback`]. + pub fallback_names: Vec>, /// Maximum allowed size of single notifications. pub max_notification_size: u64, /// Base configuration. @@ -553,6 +560,7 @@ impl NonDefaultSetConfig { NonDefaultSetConfig { notifications_protocol, max_notification_size, + fallback_names: Vec::new(), set_config: SetConfig { in_peers: 0, out_peers: 0, diff --git a/client/network/src/gossip/tests.rs b/client/network/src/gossip/tests.rs index b000cf575ddb3..19ac002aac869 100644 --- a/client/network/src/gossip/tests.rs +++ b/client/network/src/gossip/tests.rs @@ -159,6 +159,7 @@ fn build_nodes_one_proto() extra_sets: vec![ config::NonDefaultSetConfig { notifications_protocol: PROTOCOL_NAME, + fallback_names: Vec::new(), max_notification_size: 1024 * 1024, set_config: Default::default() } @@ -173,6 +174,7 @@ fn build_nodes_one_proto() extra_sets: vec![ config::NonDefaultSetConfig { notifications_protocol: PROTOCOL_NAME, + fallback_names: Vec::new(), max_notification_size: 1024 * 1024, set_config: config::SetConfig { reserved_nodes: vec![config::MultiaddrWithPeerId { diff --git a/client/network/src/protocol.rs b/client/network/src/protocol.rs index e0fa7a1cb467c..6dafd8b85f351 100644 --- a/client/network/src/protocol.rs +++ b/client/network/src/protocol.rs @@ -362,12 +362,24 @@ impl Protocol { genesis_hash, ).encode(); + let sync_protocol_config = notifications::ProtocolConfig { + name: block_announces_protocol, + fallback_names: Vec::new(), + handshake: block_announces_handshake, + max_notification_size: MAX_BLOCK_ANNOUNCE_SIZE, + }; + Notifications::new( peerset, - iter::once((block_announces_protocol, block_announces_handshake, MAX_BLOCK_ANNOUNCE_SIZE)) + iter::once(sync_protocol_config) .chain(network_config.extra_sets.iter() .zip(notifications_protocols_handshakes) - .map(|(s, hs)| (s.notifications_protocol.clone(), hs, s.max_notification_size)) + .map(|(s, hs)| notifications::ProtocolConfig { + name: s.notifications_protocol.clone(), + fallback_names: s.fallback_names.clone(), + handshake: hs, + max_notification_size: s.max_notification_size, + }) ), ) }; @@ -1154,6 +1166,8 @@ pub enum CustomMessageOutcome { NotificationStreamOpened { remote: PeerId, protocol: Cow<'static, str>, + /// See [`crate::Event::NotificationStreamOpened::negotiated_fallback`]. + negotiated_fallback: Option>, roles: Roles, notifications_sink: NotificationsSink }, @@ -1346,9 +1360,13 @@ impl NetworkBehaviour for Protocol { }; let outcome = match event { - NotificationsOut::CustomProtocolOpen { peer_id, set_id, received_handshake, notifications_sink, .. } => { + NotificationsOut::CustomProtocolOpen { + peer_id, set_id, received_handshake, notifications_sink, negotiated_fallback + } => { // Set number 0 is hardcoded the default set of peers we sync from. if set_id == HARDCODED_PEERSETS_SYNC { + debug_assert!(negotiated_fallback.is_none()); + // `received_handshake` can be either a `Status` message if received from the // legacy substream ,or a `BlockAnnouncesHandshake` if received from the block // announces substream. @@ -1408,6 +1426,7 @@ impl NetworkBehaviour for Protocol { CustomMessageOutcome::NotificationStreamOpened { remote: peer_id, protocol: self.notification_protocols[usize::from(set_id) - NUM_HARDCODED_PEERSETS].clone(), + negotiated_fallback, roles, notifications_sink, }, @@ -1419,6 +1438,7 @@ impl NetworkBehaviour for Protocol { CustomMessageOutcome::NotificationStreamOpened { remote: peer_id, protocol: self.notification_protocols[usize::from(set_id) - NUM_HARDCODED_PEERSETS].clone(), + negotiated_fallback, roles: peer.info.roles, notifications_sink, } diff --git a/client/network/src/protocol/event.rs b/client/network/src/protocol/event.rs index fb2e3b33dd680..c13980b3f4302 100644 --- a/client/network/src/protocol/event.rs +++ b/client/network/src/protocol/event.rs @@ -67,7 +67,16 @@ pub enum Event { /// Node we opened the substream with. remote: PeerId, /// The concerned protocol. Each protocol uses a different substream. + /// This is always equal to the value of + /// [`crate::config::NonDefaultSetConfig::notifications_protocol`] of one of the + /// configured sets. protocol: Cow<'static, str>, + /// If the negotiation didn't use the main name of the protocol (the one in + /// `notifications_protocol`), then this field contains which name has actually been + /// used. + /// Always contains a value equal to the value in + /// [`crate::config::NonDefaultSetConfig::fallback_names`]. + negotiated_fallback: Option>, /// Role of the remote. role: ObservedRole, }, diff --git a/client/network/src/protocol/notifications.rs b/client/network/src/protocol/notifications.rs index ef25795758b80..8739eb4948b77 100644 --- a/client/network/src/protocol/notifications.rs +++ b/client/network/src/protocol/notifications.rs @@ -19,7 +19,7 @@ //! Implementation of libp2p's `NetworkBehaviour` trait that establishes communications and opens //! notifications substreams. -pub use self::behaviour::{Notifications, NotificationsOut}; +pub use self::behaviour::{Notifications, NotificationsOut, ProtocolConfig}; pub use self::handler::{NotifsHandlerError, NotificationsSink, Ready}; mod behaviour; diff --git a/client/network/src/protocol/notifications/behaviour.rs b/client/network/src/protocol/notifications/behaviour.rs index d5112a9f981d7..0a883543de526 100644 --- a/client/network/src/protocol/notifications/behaviour.rs +++ b/client/network/src/protocol/notifications/behaviour.rs @@ -17,7 +17,7 @@ // along with this program. If not, see . use crate::protocol::notifications::{ - handler::{NotificationsSink, NotifsHandlerProto, NotifsHandlerOut, NotifsHandlerIn} + handler::{self, NotificationsSink, NotifsHandlerProto, NotifsHandlerOut, NotifsHandlerIn} }; use bytes::BytesMut; @@ -95,10 +95,8 @@ use wasm_timer::Instant; /// accommodates for any number of connections. /// pub struct Notifications { - /// Notification protocols. Entries are only ever added and not removed. - /// Contains, for each protocol, the protocol name and the message to send as part of the - /// initial handshake. - notif_protocols: Vec<(Cow<'static, str>, Arc>>, u64)>, + /// Notification protocols. Entries never change after initialization. + notif_protocols: Vec, /// Receiver for instructions about who to connect to or disconnect from. peerset: sc_peerset::Peerset, @@ -130,6 +128,19 @@ pub struct Notifications { events: VecDeque>, } +/// Configuration for a notifications protocol. +#[derive(Debug, Clone)] +pub struct ProtocolConfig { + /// Name of the protocol. + pub name: Cow<'static, str>, + /// Names of the protocol to use if the main one isn't available. + pub fallback_names: Vec>, + /// Handshake of the protocol. + pub handshake: Vec, + /// Maximum allowed size for a notification. + pub max_notification_size: u64, +} + /// Identifier for a delay firing. #[derive(Debug, Copy, Clone, PartialEq, Eq)] struct DelayId(u64); @@ -311,6 +322,9 @@ pub enum NotificationsOut { peer_id: PeerId, /// Peerset set ID the substream is tied to. set_id: sc_peerset::SetId, + /// If `Some`, a fallback protocol name has been used rather the main protocol name. + /// Always matches one of the fallback names passed at initialization. + negotiated_fallback: Option>, /// Handshake that was sent to us. /// This is normally a "Status" message, but this is out of the concern of this code. received_handshake: Vec, @@ -358,10 +372,15 @@ impl Notifications { /// Creates a `CustomProtos`. pub fn new( peerset: sc_peerset::Peerset, - notif_protocols: impl Iterator, Vec, u64)>, + notif_protocols: impl Iterator, ) -> Self { let notif_protocols = notif_protocols - .map(|(n, hs, sz)| (n, Arc::new(RwLock::new(hs)), sz)) + .map(|cfg| handler::ProtocolConfig { + name: cfg.name, + fallback_names: cfg.fallback_names, + handshake: Arc::new(RwLock::new(cfg.handshake)), + max_notification_size: cfg.max_notification_size, + }) .collect::>(); assert!(!notif_protocols.is_empty()); @@ -385,7 +404,7 @@ impl Notifications { handshake_message: impl Into> ) { if let Some(p) = self.notif_protocols.get_mut(usize::from(set_id)) { - *p.1.write() = handshake_message.into(); + *p.handshake.write() = handshake_message.into(); } else { log::error!(target: "sub-libp2p", "Unknown handshake change set: {:?}", set_id); debug_assert!(false); @@ -1728,7 +1747,9 @@ impl NetworkBehaviour for Notifications { } } - NotifsHandlerOut::OpenResultOk { protocol_index, received_handshake, notifications_sink, .. } => { + NotifsHandlerOut::OpenResultOk { + protocol_index, negotiated_fallback, received_handshake, notifications_sink, .. + } => { let set_id = sc_peerset::SetId::from(protocol_index); trace!(target: "sub-libp2p", "Handler({}, {:?}) => OpenResultOk({:?})", @@ -1748,6 +1769,7 @@ impl NetworkBehaviour for Notifications { let event = NotificationsOut::CustomProtocolOpen { peer_id: source, set_id, + negotiated_fallback, received_handshake, notifications_sink: notifications_sink.clone(), }; diff --git a/client/network/src/protocol/notifications/handler.rs b/client/network/src/protocol/notifications/handler.rs index 99677cc45e54e..3d38182c3c9d6 100644 --- a/client/network/src/protocol/notifications/handler.rs +++ b/client/network/src/protocol/notifications/handler.rs @@ -110,7 +110,7 @@ const INITIAL_KEEPALIVE_TIME: Duration = Duration::from_secs(5); pub struct NotifsHandlerProto { /// Name of protocols, prototypes for upgrades for inbound substreams, and the message we /// send or respond with in the handshake. - protocols: Vec<(Cow<'static, str>, NotificationsIn, Arc>>, u64)>, + protocols: Vec, } /// The actual handler once the connection has been established. @@ -135,20 +135,27 @@ pub struct NotifsHandler { >, } +/// Configuration for a notifications protocol. +#[derive(Debug, Clone)] +pub struct ProtocolConfig { + /// Name of the protocol. + pub name: Cow<'static, str>, + /// Names of the protocol to use if the main one isn't available. + pub fallback_names: Vec>, + /// Handshake of the protocol. The `RwLock` is locked every time a new substream is opened. + pub handshake: Arc>>, + /// Maximum allowed size for a notification. + pub max_notification_size: u64, +} + /// Fields specific for each individual protocol. struct Protocol { - /// Name of the protocol. - name: Cow<'static, str>, + /// Other fields. + config: ProtocolConfig, /// Prototype for the inbound upgrade. in_upgrade: NotificationsIn, - /// Handshake to send when opening a substream or receiving an open request. - handshake: Arc>>, - - /// Maximum allowed size of individual notifications. - max_notification_size: u64, - /// Current state of the substreams for this protocol. state: State, } @@ -214,21 +221,25 @@ impl IntoProtocolsHandler for NotifsHandlerProto { fn inbound_protocol(&self) -> UpgradeCollec { self.protocols.iter() - .map(|(_, p, _, _)| p.clone()) + .map(|cfg| NotificationsIn::new(cfg.name.clone(), cfg.fallback_names.clone(), cfg.max_notification_size)) .collect::>() } fn into_handler(self, peer_id: &PeerId, connected_point: &ConnectedPoint) -> Self::Handler { NotifsHandler { - protocols: self.protocols.into_iter().map(|(name, in_upgrade, handshake, max_size)| { + protocols: self.protocols.into_iter().map(|config| { + let in_upgrade = NotificationsIn::new( + config.name.clone(), + config.fallback_names.clone(), + config.max_notification_size + ); + Protocol { - name, + config, in_upgrade, - handshake, state: State::Closed { pending_opening: false, }, - max_notification_size: max_size, } }).collect(), peer_id: peer_id.clone(), @@ -271,6 +282,8 @@ pub enum NotifsHandlerOut { OpenResultOk { /// Index of the protocol in the list of protocols passed at initialization. protocol_index: usize, + /// Name of the protocol that was actually negotiated, if the default one wasn't available. + negotiated_fallback: Option>, /// The endpoint of the connection that is open for custom protocols. endpoint: ConnectedPoint, /// Handshake that was sent to us. @@ -445,18 +458,10 @@ impl NotifsHandlerProto { /// is always the same whether we open a substream ourselves or respond to handshake from /// the remote. pub fn new( - list: impl Into, Arc>>, u64)>>, + list: impl Into>, ) -> Self { - let protocols = list - .into() - .into_iter() - .map(|(proto_name, msg, max_notif_size)| { - (proto_name.clone(), NotificationsIn::new(proto_name, max_notif_size), msg, max_notif_size) - }) - .collect(); - NotifsHandlerProto { - protocols, + protocols: list.into(), } } } @@ -481,7 +486,7 @@ impl ProtocolsHandler for NotifsHandler { fn inject_fully_negotiated_inbound( &mut self, - ((_remote_handshake, mut new_substream), protocol_index): + (mut in_substream_open, protocol_index): >::Output, (): () ) { @@ -495,7 +500,7 @@ impl ProtocolsHandler for NotifsHandler { )); protocol_info.state = State::OpenDesiredByRemote { - in_substream: new_substream, + in_substream: in_substream_open.substream, pending_opening, }; }, @@ -518,16 +523,16 @@ impl ProtocolsHandler for NotifsHandler { // Create `handshake_message` on a separate line to be sure that the // lock is released as soon as possible. - let handshake_message = protocol_info.handshake.read().clone(); - new_substream.send_handshake(handshake_message); - *in_substream = Some(new_substream); + let handshake_message = protocol_info.config.handshake.read().clone(); + in_substream_open.substream.send_handshake(handshake_message); + *in_substream = Some(in_substream_open.substream); }, } } fn inject_fully_negotiated_outbound( &mut self, - (handshake, substream): >::Output, + new_open: >::Output, protocol_index: Self::OutboundOpenInfo ) { match self.protocols[protocol_index].state { @@ -553,15 +558,16 @@ impl ProtocolsHandler for NotifsHandler { self.protocols[protocol_index].state = State::Open { notifications_sink_rx: stream::select(async_rx.fuse(), sync_rx.fuse()).peekable(), - out_substream: Some(substream), + out_substream: Some(new_open.substream), in_substream: in_substream.take(), }; self.events_queue.push_back(ProtocolsHandlerEvent::Custom( NotifsHandlerOut::OpenResultOk { protocol_index, + negotiated_fallback: new_open.negotiated_fallback, endpoint: self.endpoint.clone(), - received_handshake: handshake, + received_handshake: new_open.handshake, notifications_sink } )); @@ -577,9 +583,10 @@ impl ProtocolsHandler for NotifsHandler { State::Closed { pending_opening } => { if !*pending_opening { let proto = NotificationsOut::new( - protocol_info.name.clone(), - protocol_info.handshake.read().clone(), - protocol_info.max_notification_size + protocol_info.config.name.clone(), + protocol_info.config.fallback_names.clone(), + protocol_info.config.handshake.read().clone(), + protocol_info.config.max_notification_size ); self.events_queue.push_back(ProtocolsHandlerEvent::OutboundSubstreamRequest { @@ -593,13 +600,14 @@ impl ProtocolsHandler for NotifsHandler { }; }, State::OpenDesiredByRemote { pending_opening, in_substream } => { - let handshake_message = protocol_info.handshake.read().clone(); + let handshake_message = protocol_info.config.handshake.read().clone(); if !*pending_opening { let proto = NotificationsOut::new( - protocol_info.name.clone(), + protocol_info.config.name.clone(), + protocol_info.config.fallback_names.clone(), handshake_message.clone(), - protocol_info.max_notification_size, + protocol_info.config.max_notification_size, ); self.events_queue.push_back(ProtocolsHandlerEvent::OutboundSubstreamRequest { diff --git a/client/network/src/protocol/notifications/tests.rs b/client/network/src/protocol/notifications/tests.rs index 8efe897afec3a..4c7461c94b20d 100644 --- a/client/network/src/protocol/notifications/tests.rs +++ b/client/network/src/protocol/notifications/tests.rs @@ -18,7 +18,7 @@ #![cfg(test)] -use crate::protocol::notifications::{Notifications, NotificationsOut}; +use crate::protocol::notifications::{Notifications, NotificationsOut, ProtocolConfig}; use futures::prelude::*; use libp2p::{PeerId, Multiaddr, Transport}; @@ -80,7 +80,12 @@ fn build_nodes() -> (Swarm, Swarm) { }); let behaviour = CustomProtoWithAddr { - inner: Notifications::new(peerset, iter::once(("/foo".into(), Vec::new(), 1024 * 1024))), + inner: Notifications::new(peerset, iter::once(ProtocolConfig { + name: "/foo".into(), + fallback_names: Vec::new(), + handshake: Vec::new(), + max_notification_size: 1024 * 1024 + })), addrs: addrs .iter() .enumerate() diff --git a/client/network/src/protocol/notifications/upgrade.rs b/client/network/src/protocol/notifications/upgrade.rs index b23e5eab06d9e..35ae6917272a2 100644 --- a/client/network/src/protocol/notifications/upgrade.rs +++ b/client/network/src/protocol/notifications/upgrade.rs @@ -19,8 +19,10 @@ pub use self::collec::UpgradeCollec; pub use self::notifications::{ NotificationsIn, + NotificationsInOpen, NotificationsInSubstream, NotificationsOut, + NotificationsOutOpen, NotificationsOutSubstream, NotificationsHandshakeError, NotificationsOutError, diff --git a/client/network/src/protocol/notifications/upgrade/notifications.rs b/client/network/src/protocol/notifications/upgrade/notifications.rs index eba96441bcfde..e2ef26c81eba9 100644 --- a/client/network/src/protocol/notifications/upgrade/notifications.rs +++ b/client/network/src/protocol/notifications/upgrade/notifications.rs @@ -41,7 +41,7 @@ use futures::prelude::*; use asynchronous_codec::Framed; use libp2p::core::{UpgradeInfo, InboundUpgrade, OutboundUpgrade, upgrade}; use log::error; -use std::{borrow::Cow, convert::{Infallible, TryFrom as _}, io, iter, mem, pin::Pin, task::{Context, Poll}}; +use std::{borrow::Cow, convert::{Infallible, TryFrom as _}, io, mem, pin::Pin, task::{Context, Poll}, vec}; use unsigned_varint::codec::UviBytes; /// Maximum allowed size of the two handshake messages, in bytes. @@ -52,7 +52,8 @@ const MAX_HANDSHAKE_SIZE: usize = 1024; #[derive(Debug, Clone)] pub struct NotificationsIn { /// Protocol name to use when negotiating the substream. - protocol_name: Cow<'static, str>, + /// The first one is the main name, while the other ones are fall backs. + protocol_names: Vec>, /// Maximum allowed size for a single notification. max_notification_size: u64, } @@ -62,7 +63,8 @@ pub struct NotificationsIn { #[derive(Debug, Clone)] pub struct NotificationsOut { /// Protocol name to use when negotiating the substream. - protocol_name: Cow<'static, str>, + /// The first one is the main name, while the other ones are fall backs. + protocol_names: Vec>, /// Message to send when we start the handshake. initial_message: Vec, /// Maximum allowed size for a single notification. @@ -106,51 +108,54 @@ pub struct NotificationsOutSubstream { impl NotificationsIn { /// Builds a new potential upgrade. - pub fn new(protocol_name: impl Into>, max_notification_size: u64) -> Self { + pub fn new( + main_protocol_name: impl Into>, + fallback_names: Vec>, + max_notification_size: u64 + ) -> Self { + let mut protocol_names = fallback_names; + protocol_names.insert(0, main_protocol_name.into()); + NotificationsIn { - protocol_name: protocol_name.into(), + protocol_names, max_notification_size, } } } impl UpgradeInfo for NotificationsIn { - type Info = Cow<'static, [u8]>; - type InfoIter = iter::Once; + type Info = StringProtocolName; + type InfoIter = vec::IntoIter; fn protocol_info(&self) -> Self::InfoIter { - let bytes: Cow<'static, [u8]> = match &self.protocol_name { - Cow::Borrowed(s) => Cow::Borrowed(s.as_bytes()), - Cow::Owned(s) => Cow::Owned(s.as_bytes().to_vec()) - }; - iter::once(bytes) + self.protocol_names.iter().cloned().map(StringProtocolName).collect::>().into_iter() } } impl InboundUpgrade for NotificationsIn where TSubstream: AsyncRead + AsyncWrite + Unpin + Send + 'static, { - type Output = (Vec, NotificationsInSubstream); + type Output = NotificationsInOpen; type Future = Pin> + Send>>; type Error = NotificationsHandshakeError; fn upgrade_inbound( self, mut socket: TSubstream, - _: Self::Info, + negotiated_name: Self::Info, ) -> Self::Future { Box::pin(async move { - let initial_message_len = unsigned_varint::aio::read_usize(&mut socket).await?; - if initial_message_len > MAX_HANDSHAKE_SIZE { + let handshake_len = unsigned_varint::aio::read_usize(&mut socket).await?; + if handshake_len > MAX_HANDSHAKE_SIZE { return Err(NotificationsHandshakeError::TooLarge { - requested: initial_message_len, + requested: handshake_len, max: MAX_HANDSHAKE_SIZE, }); } - let mut initial_message = vec![0u8; initial_message_len]; - if !initial_message.is_empty() { - socket.read_exact(&mut initial_message).await?; + let mut handshake = vec![0u8; handshake_len]; + if !handshake.is_empty() { + socket.read_exact(&mut handshake).await?; } let mut codec = UviBytes::default(); @@ -161,11 +166,30 @@ where TSubstream: AsyncRead + AsyncWrite + Unpin + Send + 'static, handshake: NotificationsInSubstreamHandshake::NotSent, }; - Ok((initial_message, substream)) + Ok(NotificationsInOpen { + handshake, + negotiated_fallback: if negotiated_name.0 == self.protocol_names[0] { + None + } else { + Some(negotiated_name.0) + }, + substream, + }) }) } } +/// Yielded by the [`NotificationsIn`] after a successfuly upgrade. +pub struct NotificationsInOpen { + /// Handshake sent by the remote. + pub handshake: Vec, + /// If the negotiated name is not the "main" protocol name but a fallback, contains the + /// name of the negotiated fallback. + pub negotiated_fallback: Option>, + /// Implementation of `Stream` that allows receives messages from the substream. + pub substream: NotificationsInSubstream, +} + impl NotificationsInSubstream where TSubstream: AsyncRead + AsyncWrite + Unpin, { @@ -296,7 +320,8 @@ where TSubstream: AsyncRead + AsyncWrite + Unpin, impl NotificationsOut { /// Builds a new potential upgrade. pub fn new( - protocol_name: impl Into>, + main_protocol_name: impl Into>, + fallback_names: Vec>, initial_message: impl Into>, max_notification_size: u64, ) -> Self { @@ -305,38 +330,47 @@ impl NotificationsOut { error!(target: "sub-libp2p", "Outbound networking handshake is above allowed protocol limit"); } + let mut protocol_names = fallback_names; + protocol_names.insert(0, main_protocol_name.into()); + NotificationsOut { - protocol_name: protocol_name.into(), + protocol_names, initial_message, max_notification_size, } } } +/// Implementation of the `ProtocolName` trait, where the protocol name is a string. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct StringProtocolName(Cow<'static, str>); + +impl upgrade::ProtocolName for StringProtocolName { + fn protocol_name(&self) -> &[u8] { + self.0.as_bytes() + } +} + impl UpgradeInfo for NotificationsOut { - type Info = Cow<'static, [u8]>; - type InfoIter = iter::Once; + type Info = StringProtocolName; + type InfoIter = vec::IntoIter; fn protocol_info(&self) -> Self::InfoIter { - let bytes: Cow<'static, [u8]> = match &self.protocol_name { - Cow::Borrowed(s) => Cow::Borrowed(s.as_bytes()), - Cow::Owned(s) => Cow::Owned(s.as_bytes().to_vec()) - }; - iter::once(bytes) + self.protocol_names.iter().cloned().map(StringProtocolName).collect::>().into_iter() } } impl OutboundUpgrade for NotificationsOut where TSubstream: AsyncRead + AsyncWrite + Unpin + Send + 'static, { - type Output = (Vec, NotificationsOutSubstream); + type Output = NotificationsOutOpen; type Future = Pin> + Send>>; type Error = NotificationsHandshakeError; fn upgrade_outbound( self, mut socket: TSubstream, - _: Self::Info, + negotiated_name: Self::Info, ) -> Self::Future { Box::pin(async move { upgrade::write_with_len_prefix(&mut socket, &self.initial_message).await?; @@ -358,13 +392,32 @@ where TSubstream: AsyncRead + AsyncWrite + Unpin + Send + 'static, let mut codec = UviBytes::default(); codec.set_max_len(usize::try_from(self.max_notification_size).unwrap_or(usize::max_value())); - Ok((handshake, NotificationsOutSubstream { - socket: Framed::new(socket, codec), - })) + Ok(NotificationsOutOpen { + handshake, + negotiated_fallback: if negotiated_name.0 == self.protocol_names[0] { + None + } else { + Some(negotiated_name.0) + }, + substream: NotificationsOutSubstream { + socket: Framed::new(socket, codec), + } + }) }) } } +/// Yielded by the [`NotificationsOut`] after a successfuly upgrade. +pub struct NotificationsOutOpen { + /// Handshake returned by the remote. + pub handshake: Vec, + /// If the negotiated name is not the "main" protocol name but a fallback, contains the + /// name of the negotiated fallback. + pub negotiated_fallback: Option>, + /// Implementation of `Sink` that allows sending messages on the substream. + pub substream: NotificationsOutSubstream, +} + impl Sink> for NotificationsOutSubstream where TSubstream: AsyncRead + AsyncWrite + Unpin, { @@ -436,7 +489,7 @@ pub enum NotificationsOutError { #[cfg(test)] mod tests { - use super::{NotificationsIn, NotificationsOut}; + use super::{NotificationsIn, NotificationsInOpen, NotificationsOut, NotificationsOutOpen}; use async_std::net::{TcpListener, TcpStream}; use futures::{prelude::*, channel::oneshot}; @@ -450,9 +503,9 @@ mod tests { let client = async_std::task::spawn(async move { let socket = TcpStream::connect(listener_addr_rx.await.unwrap()).await.unwrap(); - let (handshake, mut substream) = upgrade::apply_outbound( + let NotificationsOutOpen { handshake, mut substream, .. } = upgrade::apply_outbound( socket, - NotificationsOut::new(PROTO_NAME, &b"initial message"[..], 1024 * 1024), + NotificationsOut::new(PROTO_NAME, Vec::new(), &b"initial message"[..], 1024 * 1024), upgrade::Version::V1 ).await.unwrap(); @@ -465,12 +518,12 @@ mod tests { listener_addr_tx.send(listener.local_addr().unwrap()).unwrap(); let (socket, _) = listener.accept().await.unwrap(); - let (initial_message, mut substream) = upgrade::apply_inbound( + let NotificationsInOpen { handshake, mut substream, .. } = upgrade::apply_inbound( socket, - NotificationsIn::new(PROTO_NAME, 1024 * 1024) + NotificationsIn::new(PROTO_NAME, Vec::new(), 1024 * 1024) ).await.unwrap(); - assert_eq!(initial_message, b"initial message"); + assert_eq!(handshake, b"initial message"); substream.send_handshake(&b"hello world"[..]); let msg = substream.next().await.unwrap().unwrap(); @@ -489,9 +542,9 @@ mod tests { let client = async_std::task::spawn(async move { let socket = TcpStream::connect(listener_addr_rx.await.unwrap()).await.unwrap(); - let (handshake, mut substream) = upgrade::apply_outbound( + let NotificationsOutOpen { handshake, mut substream, .. } = upgrade::apply_outbound( socket, - NotificationsOut::new(PROTO_NAME, vec![], 1024 * 1024), + NotificationsOut::new(PROTO_NAME, Vec::new(), vec![], 1024 * 1024), upgrade::Version::V1 ).await.unwrap(); @@ -504,12 +557,12 @@ mod tests { listener_addr_tx.send(listener.local_addr().unwrap()).unwrap(); let (socket, _) = listener.accept().await.unwrap(); - let (initial_message, mut substream) = upgrade::apply_inbound( + let NotificationsInOpen { handshake, mut substream, .. } = upgrade::apply_inbound( socket, - NotificationsIn::new(PROTO_NAME, 1024 * 1024) + NotificationsIn::new(PROTO_NAME, Vec::new(), 1024 * 1024) ).await.unwrap(); - assert!(initial_message.is_empty()); + assert!(handshake.is_empty()); substream.send_handshake(vec![]); let msg = substream.next().await.unwrap().unwrap(); @@ -528,7 +581,7 @@ mod tests { let socket = TcpStream::connect(listener_addr_rx.await.unwrap()).await.unwrap(); let outcome = upgrade::apply_outbound( socket, - NotificationsOut::new(PROTO_NAME, &b"hello"[..], 1024 * 1024), + NotificationsOut::new(PROTO_NAME, Vec::new(), &b"hello"[..], 1024 * 1024), upgrade::Version::V1 ).await; @@ -543,12 +596,12 @@ mod tests { listener_addr_tx.send(listener.local_addr().unwrap()).unwrap(); let (socket, _) = listener.accept().await.unwrap(); - let (initial_msg, substream) = upgrade::apply_inbound( + let NotificationsInOpen { handshake, substream, .. } = upgrade::apply_inbound( socket, - NotificationsIn::new(PROTO_NAME, 1024 * 1024) + NotificationsIn::new(PROTO_NAME, Vec::new(), 1024 * 1024) ).await.unwrap(); - assert_eq!(initial_msg, b"hello"); + assert_eq!(handshake, b"hello"); // We successfully upgrade to the protocol, but then close the substream. drop(substream); @@ -567,7 +620,7 @@ mod tests { let ret = upgrade::apply_outbound( socket, // We check that an initial message that is too large gets refused. - NotificationsOut::new(PROTO_NAME, (0..32768).map(|_| 0).collect::>(), 1024 * 1024), + NotificationsOut::new(PROTO_NAME, Vec::new(), (0..32768).map(|_| 0).collect::>(), 1024 * 1024), upgrade::Version::V1 ).await; assert!(ret.is_err()); @@ -580,7 +633,7 @@ mod tests { let (socket, _) = listener.accept().await.unwrap(); let ret = upgrade::apply_inbound( socket, - NotificationsIn::new(PROTO_NAME, 1024 * 1024) + NotificationsIn::new(PROTO_NAME, Vec::new(), 1024 * 1024) ).await; assert!(ret.is_err()); }); @@ -597,7 +650,7 @@ mod tests { let socket = TcpStream::connect(listener_addr_rx.await.unwrap()).await.unwrap(); let ret = upgrade::apply_outbound( socket, - NotificationsOut::new(PROTO_NAME, &b"initial message"[..], 1024 * 1024), + NotificationsOut::new(PROTO_NAME, Vec::new(), &b"initial message"[..], 1024 * 1024), upgrade::Version::V1 ).await; assert!(ret.is_err()); @@ -608,11 +661,11 @@ mod tests { listener_addr_tx.send(listener.local_addr().unwrap()).unwrap(); let (socket, _) = listener.accept().await.unwrap(); - let (initial_message, mut substream) = upgrade::apply_inbound( + let NotificationsInOpen { handshake, mut substream, .. } = upgrade::apply_inbound( socket, - NotificationsIn::new(PROTO_NAME, 1024 * 1024) + NotificationsIn::new(PROTO_NAME, Vec::new(), 1024 * 1024) ).await.unwrap(); - assert_eq!(initial_message, b"initial message"); + assert_eq!(handshake, b"initial message"); // We check that a handshake that is too large gets refused. substream.send_handshake((0..32768).map(|_| 0).collect::>()); diff --git a/client/network/src/service.rs b/client/network/src/service.rs index 99036c5effad8..03b71b8c86f5e 100644 --- a/client/network/src/service.rs +++ b/client/network/src/service.rs @@ -1541,7 +1541,7 @@ impl Future for NetworkWorker { } }, Poll::Ready(SwarmEvent::Behaviour(BehaviourOut::NotificationStreamOpened { - remote, protocol, notifications_sink, role + remote, protocol, negotiated_fallback, notifications_sink, role })) => { if let Some(metrics) = this.metrics.as_ref() { metrics.notifications_streams_opened_total @@ -1554,6 +1554,7 @@ impl Future for NetworkWorker { this.event_streams.send(Event::NotificationStreamOpened { remote, protocol, + negotiated_fallback, role, }); }, diff --git a/client/network/src/service/tests.rs b/client/network/src/service/tests.rs index dd4a0597cbcbc..4e5bba8f7d33f 100644 --- a/client/network/src/service/tests.rs +++ b/client/network/src/service/tests.rs @@ -159,6 +159,7 @@ fn build_nodes_one_proto() extra_sets: vec![ config::NonDefaultSetConfig { notifications_protocol: PROTOCOL_NAME, + fallback_names: Vec::new(), max_notification_size: 1024 * 1024, set_config: Default::default() } @@ -172,6 +173,7 @@ fn build_nodes_one_proto() extra_sets: vec![ config::NonDefaultSetConfig { notifications_protocol: PROTOCOL_NAME, + fallback_names: Vec::new(), max_notification_size: 1024 * 1024, set_config: config::SetConfig { reserved_nodes: vec![config::MultiaddrWithPeerId { @@ -328,6 +330,7 @@ fn lots_of_incoming_peers_works() { extra_sets: vec![ config::NonDefaultSetConfig { notifications_protocol: PROTOCOL_NAME, + fallback_names: Vec::new(), max_notification_size: 1024 * 1024, set_config: config::SetConfig { in_peers: u32::max_value(), @@ -353,6 +356,7 @@ fn lots_of_incoming_peers_works() { extra_sets: vec![ config::NonDefaultSetConfig { notifications_protocol: PROTOCOL_NAME, + fallback_names: Vec::new(), max_notification_size: 1024 * 1024, set_config: config::SetConfig { reserved_nodes: vec![config::MultiaddrWithPeerId { @@ -456,6 +460,81 @@ fn notifications_back_pressure() { }); } +#[test] +fn fallback_name_working() { + // Node 1 supports the protocols "new" and "old". Node 2 only supports "old". Checks whether + // they can connect. + + const NEW_PROTOCOL_NAME: Cow<'static, str> = + Cow::Borrowed("/new-shiny-protocol-that-isnt-PROTOCOL_NAME"); + + let listen_addr = config::build_multiaddr![Memory(rand::random::())]; + + let (node1, mut events_stream1) = build_test_full_node(config::NetworkConfiguration { + extra_sets: vec![ + config::NonDefaultSetConfig { + notifications_protocol: NEW_PROTOCOL_NAME.clone(), + fallback_names: vec![PROTOCOL_NAME], + max_notification_size: 1024 * 1024, + set_config: Default::default() + } + ], + listen_addresses: vec![listen_addr.clone()], + transport: config::TransportConfig::MemoryOnly, + .. config::NetworkConfiguration::new_local() + }); + + let (_, mut events_stream2) = build_test_full_node(config::NetworkConfiguration { + extra_sets: vec![ + config::NonDefaultSetConfig { + notifications_protocol: PROTOCOL_NAME, + fallback_names: Vec::new(), + max_notification_size: 1024 * 1024, + set_config: config::SetConfig { + reserved_nodes: vec![config::MultiaddrWithPeerId { + multiaddr: listen_addr, + peer_id: node1.local_peer_id().clone(), + }], + .. Default::default() + } + } + ], + listen_addresses: vec![], + transport: config::TransportConfig::MemoryOnly, + .. config::NetworkConfiguration::new_local() + }); + + let receiver = async_std::task::spawn(async move { + // Wait for the `NotificationStreamOpened`. + loop { + match events_stream2.next().await.unwrap() { + Event::NotificationStreamOpened { protocol, negotiated_fallback, .. } => { + assert_eq!(protocol, PROTOCOL_NAME); + assert_eq!(negotiated_fallback, None); + break + }, + _ => {} + }; + } + }); + + async_std::task::block_on(async move { + // Wait for the `NotificationStreamOpened`. + loop { + match events_stream1.next().await.unwrap() { + Event::NotificationStreamOpened { protocol, negotiated_fallback, .. } => { + assert_eq!(protocol, NEW_PROTOCOL_NAME); + assert_eq!(negotiated_fallback, Some(PROTOCOL_NAME)); + break + }, + _ => {} + }; + } + + receiver.await; + }); +} + #[test] #[should_panic(expected = "don't match the transport")] fn ensure_listen_addresses_consistent_with_transport_memory() { diff --git a/client/network/src/transactions.rs b/client/network/src/transactions.rs index b694182e6a231..8a7dd78c834ce 100644 --- a/client/network/src/transactions.rs +++ b/client/network/src/transactions.rs @@ -136,6 +136,7 @@ impl TransactionsHandlerPrototype { pub fn set_config(&self) -> config::NonDefaultSetConfig { config::NonDefaultSetConfig { notifications_protocol: self.protocol_name.clone(), + fallback_names: Vec::new(), max_notification_size: MAX_TRANSACTIONS_SIZE, set_config: config::SetConfig { in_peers: 0, @@ -318,7 +319,7 @@ impl TransactionsHandler { } }, - Event::NotificationStreamOpened { remote, protocol, role } if protocol == self.protocol_name => { + Event::NotificationStreamOpened { remote, protocol, role, .. } if protocol == self.protocol_name => { let _was_in = self.peers.insert(remote, Peer { known_transactions: LruHashSet::new(NonZeroUsize::new(MAX_KNOWN_TRANSACTIONS) .expect("Constant is nonzero")), diff --git a/client/network/test/src/lib.rs b/client/network/test/src/lib.rs index 689eca8aac5dd..8e56005dad25d 100644 --- a/client/network/test/src/lib.rs +++ b/client/network/test/src/lib.rs @@ -742,6 +742,7 @@ pub trait TestNetFactory: Sized where >: network_config.extra_sets = config.notifications_protocols.into_iter().map(|p| { NonDefaultSetConfig { notifications_protocol: p, + fallback_names: Vec::new(), max_notification_size: 1024 * 1024, set_config: Default::default() } diff --git a/client/state-db/src/lib.rs b/client/state-db/src/lib.rs index 8961f2549b2d8..1340442061aba 100644 --- a/client/state-db/src/lib.rs +++ b/client/state-db/src/lib.rs @@ -364,6 +364,17 @@ impl StateDbSync Option> { + match self.mode { + PruningMode::ArchiveAll => { + Some(CommitSet::default()) + }, + PruningMode::ArchiveCanonical | PruningMode::Constrained(_) => { + self.non_canonical.remove(hash) + }, + } + } + fn pin(&mut self, hash: &BlockHash) -> Result<(), PinError> { match self.mode { PruningMode::ArchiveAll => Ok(()), @@ -509,6 +520,12 @@ impl StateDb Option> { + self.db.write().remove(hash) + } + /// Returns last finalized block number. pub fn best_canonical(&self) -> Option { return self.db.read().best_canonical() diff --git a/client/state-db/src/noncanonical.rs b/client/state-db/src/noncanonical.rs index 3f0c7d132f746..1a680b16ffbee 100644 --- a/client/state-db/src/noncanonical.rs +++ b/client/state-db/src/noncanonical.rs @@ -36,7 +36,7 @@ const MAX_BLOCKS_PER_LEVEL: u64 = 32; #[derive(parity_util_mem_derive::MallocSizeOf)] pub struct NonCanonicalOverlay { last_canonicalized: Option<(BlockHash, u64)>, - levels: VecDeque>>, + levels: VecDeque>, parents: HashMap, pending_canonicalizations: Vec, pending_insertions: Vec, @@ -46,6 +46,36 @@ pub struct NonCanonicalOverlay { pinned_insertions: HashMap, u32)>, } +#[derive(parity_util_mem_derive::MallocSizeOf)] +#[cfg_attr(test, derive(PartialEq, Debug))] +struct OverlayLevel { + blocks: Vec>, + used_indicies: u64, // Bitmask of available journal indicies. +} + +impl OverlayLevel { + fn push(&mut self, overlay: BlockOverlay) { + self.used_indicies |= 1 << overlay.journal_index; + self.blocks.push(overlay) + } + + fn available_index(&self) -> u64 { + self.used_indicies.trailing_ones() as u64 + } + + fn remove(&mut self, index: usize) -> BlockOverlay { + self.used_indicies &= !(1 << self.blocks[index].journal_index); + self.blocks.remove(index) + } + + fn new() -> OverlayLevel { + OverlayLevel { + blocks: Vec::new(), + used_indicies: 0, + } + } +} + #[derive(Encode, Decode)] struct JournalRecord { hash: BlockHash, @@ -62,6 +92,7 @@ fn to_journal_key(block: u64, index: u64) -> Vec { #[derive(parity_util_mem_derive::MallocSizeOf)] struct BlockOverlay { hash: BlockHash, + journal_index: u64, journal_key: Vec, inserted: Vec, deleted: Vec, @@ -93,7 +124,7 @@ fn discard_values(values: &mut HashMap, inserted } fn discard_descendants( - levels: &mut (&mut [Vec>], &mut [Vec>]), + levels: &mut (&mut [OverlayLevel], &mut [OverlayLevel]), mut values: &mut HashMap, parents: &mut HashMap, pinned: &HashMap, @@ -111,36 +142,32 @@ fn discard_descendants( }; let mut pinned_children = 0; if let Some(level) = first { - *level = level.drain(..).filter_map(|overlay| { - let parent = parents.get(&overlay.hash) - .expect("there is a parent entry for each entry in levels; qed"); - - if parent == hash { - let mut num_pinned = discard_descendants( - &mut remainder, - values, - parents, - pinned, - pinned_insertions, - &overlay.hash - ); - if pinned.contains_key(&overlay.hash) { - num_pinned += 1; - } - if num_pinned != 0 { - // save to be discarded later. - pinned_insertions.insert(overlay.hash.clone(), (overlay.inserted, num_pinned)); - pinned_children += num_pinned; - } else { - // discard immediately. - parents.remove(&overlay.hash); - discard_values(&mut values, overlay.inserted); - } - None + while let Some(i) = level.blocks.iter().position(|overlay| parents.get(&overlay.hash) + .expect("there is a parent entry for each entry in levels; qed") + == hash) + { + let overlay = level.remove(i); + let mut num_pinned = discard_descendants( + &mut remainder, + values, + parents, + pinned, + pinned_insertions, + &overlay.hash + ); + if pinned.contains_key(&overlay.hash) { + num_pinned += 1; + } + if num_pinned != 0 { + // save to be discarded later. + pinned_insertions.insert(overlay.hash.clone(), (overlay.inserted, num_pinned)); + pinned_children += num_pinned; } else { - Some(overlay) + // discard immediately. + parents.remove(&overlay.hash); + discard_values(&mut values, overlay.inserted); } - }).collect(); + } } pinned_children } @@ -161,7 +188,7 @@ impl NonCanonicalOverlay { let mut total: u64 = 0; block += 1; loop { - let mut level = Vec::new(); + let mut level = OverlayLevel::new(); for index in 0 .. MAX_BLOCKS_PER_LEVEL { let journal_key = to_journal_key(block, index); if let Some(record) = db.get_meta(&journal_key).map_err(|e| Error::Db(e))? { @@ -169,6 +196,7 @@ impl NonCanonicalOverlay { let inserted = record.inserted.iter().map(|(k, _)| k.clone()).collect(); let overlay = BlockOverlay { hash: record.hash.clone(), + journal_index: index, journal_key, inserted: inserted, deleted: record.deleted, @@ -187,7 +215,7 @@ impl NonCanonicalOverlay { total += 1; } } - if level.is_empty() { + if level.blocks.is_empty() { break; } levels.push_back(level); @@ -235,23 +263,24 @@ impl NonCanonicalOverlay { } } let level = if self.levels.is_empty() || number == front_block_number + self.levels.len() as u64 { - self.levels.push_back(Vec::new()); + self.levels.push_back(OverlayLevel::new()); self.levels.back_mut().expect("can't be empty after insertion; qed") } else { self.levels.get_mut((number - front_block_number) as usize) .expect("number is [front_block_number .. front_block_number + levels.len()) is asserted in precondition; qed") }; - if level.len() >= MAX_BLOCKS_PER_LEVEL as usize { + if level.blocks.len() >= MAX_BLOCKS_PER_LEVEL as usize { return Err(Error::TooManySiblingBlocks); } - let index = level.len() as u64; + let index = level.available_index(); let journal_key = to_journal_key(number, index); let inserted = changeset.inserted.iter().map(|(k, _)| k.clone()).collect(); let overlay = BlockOverlay { hash: hash.clone(), + journal_index: index, journal_key: journal_key.clone(), inserted: inserted, deleted: changeset.deleted.clone(), @@ -279,7 +308,7 @@ impl NonCanonicalOverlay { hash: &BlockHash ) { if let Some(level) = self.levels.get(level_index) { - level.iter().for_each(|overlay| { + level.blocks.iter().for_each(|overlay| { let parent = self.parents.get(&overlay.hash).expect("there is a parent entry for each entry in levels; qed").clone(); if parent == *hash { discarded_journals.push(overlay.journal_key.clone()); @@ -310,7 +339,7 @@ impl NonCanonicalOverlay { let start = self.last_canonicalized_block_number().unwrap_or(0); self.levels .get(self.pending_canonicalizations.len()) - .map(|level| level.iter().map(|r| (r.hash.clone(), start)).collect()) + .map(|level| level.blocks.iter().map(|r| (r.hash.clone(), start)).collect()) .unwrap_or_default() } @@ -323,14 +352,14 @@ impl NonCanonicalOverlay { ) -> Result<(), Error> { trace!(target: "state-db", "Canonicalizing {:?}", hash); let level = self.levels.get(self.pending_canonicalizations.len()).ok_or_else(|| Error::InvalidBlock)?; - let index = level + let index = level.blocks .iter() .position(|overlay| overlay.hash == *hash) .ok_or_else(|| Error::InvalidBlock)?; let mut discarded_journals = Vec::new(); let mut discarded_blocks = Vec::new(); - for (i, overlay) in level.iter().enumerate() { + for (i, overlay) in level.blocks.iter().enumerate() { if i != index { self.discard_journals( self.pending_canonicalizations.len() + 1, @@ -344,7 +373,7 @@ impl NonCanonicalOverlay { } // get the one we need to canonicalize - let overlay = &level[index]; + let overlay = &level.blocks[index]; commit.data.inserted.extend(overlay.inserted.iter() .map(|k| (k.clone(), self.values.get(k).expect("For each key in overlays there's a value in values").1.clone()))); commit.data.deleted.extend(overlay.deleted.clone()); @@ -363,13 +392,13 @@ impl NonCanonicalOverlay { for hash in self.pending_canonicalizations.drain(..) { trace!(target: "state-db", "Post canonicalizing {:?}", hash); let level = self.levels.pop_front().expect("Hash validity is checked in `canonicalize`"); - let index = level + let index = level.blocks .iter() .position(|overlay| overlay.hash == hash) .expect("Hash validity is checked in `canonicalize`"); // discard unfinalized overlays and values - for (i, overlay) in level.into_iter().enumerate() { + for (i, overlay) in level.blocks.into_iter().enumerate() { let mut pinned_children = if i != index { discard_descendants( &mut self.levels.as_mut_slices(), @@ -421,7 +450,7 @@ impl NonCanonicalOverlay { pub fn revert_one(&mut self) -> Option> { self.levels.pop_back().map(|level| { let mut commit = CommitSet::default(); - for overlay in level.into_iter() { + for overlay in level.blocks.into_iter() { commit.meta.deleted.push(overlay.journal_key); self.parents.remove(&overlay.hash); discard_values(&mut self.values, overlay.inserted); @@ -430,6 +459,36 @@ impl NonCanonicalOverlay { }) } + /// Revert a single block. Returns commit set that deletes the journal or `None` if not possible. + pub fn remove(&mut self, hash: &BlockHash) -> Option> { + let mut commit = CommitSet::default(); + let level_count = self.levels.len(); + for (level_index, level) in self.levels.iter_mut().enumerate().rev() { + let index = match level.blocks.iter().position(|overlay| &overlay.hash == hash) { + Some(index) => index, + None => continue, + }; + // Check that it does not have any children + if (level_index != level_count - 1) && self.parents.values().any(|h| h == hash) { + log::debug!(target: "state-db", "Trying to remove block {:?} with children", hash); + return None; + } + let overlay = level.remove(index); + commit.meta.deleted.push(overlay.journal_key); + self.parents.remove(&overlay.hash); + discard_values(&mut self.values, overlay.inserted); + break; + } + if self.levels.back().map_or(false, |l| l.blocks.is_empty()) { + self.levels.pop_back(); + } + if !commit.meta.deleted.is_empty() { + Some(commit) + } else { + None + } + } + fn revert_insertions(&mut self) { self.pending_insertions.reverse(); for hash in self.pending_insertions.drain(..) { @@ -437,12 +496,13 @@ impl NonCanonicalOverlay { // find a level. When iterating insertions backwards the hash is always last in the level. let level_index = self.levels.iter().position(|level| - level.last().expect("Hash is added in `insert` in reverse order").hash == hash) + level.blocks.last().expect("Hash is added in `insert` in reverse order").hash == hash) .expect("Hash is added in insert"); - let overlay = self.levels[level_index].pop().expect("Empty levels are not allowed in self.levels"); + let overlay_index = self.levels[level_index].blocks.len() - 1; + let overlay = self.levels[level_index].remove(overlay_index); discard_values(&mut self.values, overlay.inserted); - if self.levels[level_index].is_empty() { + if self.levels[level_index].blocks.is_empty() { debug_assert_eq!(level_index, self.levels.len() - 1); self.levels.pop_back(); } @@ -1000,4 +1060,67 @@ mod tests { overlay.apply_pending(); assert!(!contains(&overlay, 21)); } + + #[test] + fn index_reuse() { + // This test discards a branch that is journaled under a non-zero index on level 1, + // making sure all journals are loaded for each level even if some of them are missing. + let root = H256::random(); + let h1 = H256::random(); + let h2 = H256::random(); + let h11 = H256::random(); + let h21 = H256::random(); + let mut db = make_db(&[]); + let mut overlay = NonCanonicalOverlay::::new(&db).unwrap(); + db.commit(&overlay.insert::(&root, 10, &H256::default(), make_changeset(&[], &[])).unwrap()); + db.commit(&overlay.insert::(&h1, 11, &root, make_changeset(&[1], &[])).unwrap()); + db.commit(&overlay.insert::(&h2, 11, &root, make_changeset(&[2], &[])).unwrap()); + db.commit(&overlay.insert::(&h11, 12, &h1, make_changeset(&[11], &[])).unwrap()); + db.commit(&overlay.insert::(&h21, 12, &h2, make_changeset(&[21], &[])).unwrap()); + let mut commit = CommitSet::default(); + overlay.canonicalize::(&root, &mut commit).unwrap(); + overlay.canonicalize::(&h2, &mut commit).unwrap(); // h11 should stay in the DB + db.commit(&commit); + overlay.apply_pending(); + + // add another block at top level. It should reuse journal index 0 of previously discarded block + let h22 = H256::random(); + db.commit(&overlay.insert::(&h22, 12, &h2, make_changeset(&[22], &[])).unwrap()); + assert_eq!(overlay.levels[0].blocks[0].journal_index, 1); + assert_eq!(overlay.levels[0].blocks[1].journal_index, 0); + + // Restore into a new overlay and check that journaled value exists. + let overlay = NonCanonicalOverlay::::new(&db).unwrap(); + assert_eq!(overlay.parents.len(), 2); + assert!(contains(&overlay, 21)); + assert!(contains(&overlay, 22)); + } + + #[test] + fn remove_works() { + let root = H256::random(); + let h1 = H256::random(); + let h2 = H256::random(); + let h11 = H256::random(); + let h21 = H256::random(); + let mut db = make_db(&[]); + let mut overlay = NonCanonicalOverlay::::new(&db).unwrap(); + db.commit(&overlay.insert::(&root, 10, &H256::default(), make_changeset(&[], &[])).unwrap()); + db.commit(&overlay.insert::(&h1, 11, &root, make_changeset(&[1], &[])).unwrap()); + db.commit(&overlay.insert::(&h2, 11, &root, make_changeset(&[2], &[])).unwrap()); + db.commit(&overlay.insert::(&h11, 12, &h1, make_changeset(&[11], &[])).unwrap()); + db.commit(&overlay.insert::(&h21, 12, &h2, make_changeset(&[21], &[])).unwrap()); + assert!(overlay.remove(&h1).is_none()); + assert!(overlay.remove(&h2).is_none()); + assert_eq!(overlay.levels.len(), 3); + + db.commit(&overlay.remove(&h11).unwrap()); + assert!(!contains(&overlay, 11)); + + db.commit(&overlay.remove(&h21).unwrap()); + assert_eq!(overlay.levels.len(), 2); + + db.commit(&overlay.remove(&h2).unwrap()); + assert!(!contains(&overlay, 2)); + } } diff --git a/frame/election-provider-multi-phase/src/unsigned.rs b/frame/election-provider-multi-phase/src/unsigned.rs index ebeae3dc472fb..66b985c8efb94 100644 --- a/frame/election-provider-multi-phase/src/unsigned.rs +++ b/frame/election-provider-multi-phase/src/unsigned.rs @@ -437,6 +437,11 @@ impl Pallet { let mut high = assignments.len(); let mut low = 0; + // not much we can do if assignments are already empty. + if high == low { + return Ok(()); + } + while high - low > 1 { let test = (high + low) / 2; if encoded_size_of(&assignments[..test])? <= max_allowed_length { @@ -446,13 +451,13 @@ impl Pallet { } } let maximum_allowed_voters = - if encoded_size_of(&assignments[..low + 1])? <= max_allowed_length { + if low < assignments.len() && encoded_size_of(&assignments[..low + 1])? <= max_allowed_length { low + 1 } else { low }; - // ensure our postconditions are correct + // ensure our post-conditions are correct debug_assert!( encoded_size_of(&assignments[..maximum_allowed_voters]).unwrap() <= max_allowed_length ); @@ -1256,8 +1261,7 @@ mod tests { #[test] fn trim_assignments_length_does_not_modify_when_short_enough() { - let mut ext = ExtBuilder::default().build(); - ext.execute_with(|| { + ExtBuilder::default().build_and_execute(|| { roll_to(25); // given @@ -1281,8 +1285,7 @@ mod tests { #[test] fn trim_assignments_length_modifies_when_too_long() { - let mut ext = ExtBuilder::default().build(); - ext.execute_with(|| { + ExtBuilder::default().build().execute_with(|| { roll_to(25); // given @@ -1307,8 +1310,7 @@ mod tests { #[test] fn trim_assignments_length_trims_lowest_stake() { - let mut ext = ExtBuilder::default().build(); - ext.execute_with(|| { + ExtBuilder::default().build().execute_with(|| { roll_to(25); // given @@ -1340,13 +1342,59 @@ mod tests { }); } + #[test] + fn trim_assignments_length_wont_panic() { + // we shan't panic if assignments are initially empty. + ExtBuilder::default().build_and_execute(|| { + let encoded_size_of = Box::new(|assignments: &[IndexAssignmentOf]| { + CompactOf::::try_from(assignments).map(|compact| compact.encoded_size()) + }); + + let mut assignments = vec![]; + + // since we have 16 fields, we need to store the length fields of 16 vecs, thus 16 bytes + // minimum. + let min_compact_size = encoded_size_of(&assignments).unwrap(); + assert_eq!(min_compact_size, CompactOf::::LIMIT); + + // all of this should not panic. + MultiPhase::trim_assignments_length(0, &mut assignments, encoded_size_of.clone()) + .unwrap(); + MultiPhase::trim_assignments_length(1, &mut assignments, encoded_size_of.clone()) + .unwrap(); + MultiPhase::trim_assignments_length( + min_compact_size as u32, + &mut assignments, + encoded_size_of, + ) + .unwrap(); + }); + + // or when we trim it to zero. + ExtBuilder::default().build_and_execute(|| { + // we need snapshot for `trim_helpers` to work. + roll_to(25); + let TrimHelpers { mut assignments, encoded_size_of, .. } = trim_helpers(); + assert!(assignments.len() > 0); + + // trim to min compact size. + let min_compact_size = CompactOf::::LIMIT as u32; + MultiPhase::trim_assignments_length( + min_compact_size, + &mut assignments, + encoded_size_of, + ) + .unwrap(); + assert_eq!(assignments.len(), 0); + }); + } + // all the other solution-generation functions end up delegating to `mine_solution`, so if we // demonstrate that `mine_solution` solutions are all trimmed to an acceptable length, then // we know that higher-level functions will all also have short-enough solutions. #[test] fn mine_solution_solutions_always_within_acceptable_length() { - let mut ext = ExtBuilder::default().build(); - ext.execute_with(|| { + ExtBuilder::default().build_and_execute(|| { roll_to(25); // how long would the default solution be? diff --git a/frame/im-online/src/benchmarking.rs b/frame/im-online/src/benchmarking.rs index 287a2c6fd3a73..5ab4d16c7fe08 100644 --- a/frame/im-online/src/benchmarking.rs +++ b/frame/im-online/src/benchmarking.rs @@ -29,7 +29,7 @@ use sp_runtime::traits::{ValidateUnsigned, Zero}; use sp_runtime::transaction_validity::TransactionSource; use frame_support::traits::UnfilteredDispatchable; -use crate::Module as ImOnline; +use crate::Pallet as ImOnline; const MAX_KEYS: u32 = 1000; const MAX_EXTERNAL_ADDRESSES: u32 = 100; diff --git a/frame/im-online/src/lib.rs b/frame/im-online/src/lib.rs index d8f3fdc854b16..0290c564ec599 100644 --- a/frame/im-online/src/lib.rs +++ b/frame/im-online/src/lib.rs @@ -15,7 +15,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -//! # I'm online Module +//! # I'm online Pallet //! //! If the local node is a validator (i.e. contains an authority key), this module //! gossips a heartbeat transaction with each new session. The heartbeat functions @@ -32,7 +32,7 @@ //! //! - [`Config`] //! - [`Call`] -//! - [`Module`] +//! - [`Pallet`] //! //! ## Interface //! @@ -54,7 +54,7 @@ //! #[weight = 0] //! pub fn is_online(origin, authority_index: u32) -> dispatch::DispatchResult { //! let _sender = ensure_signed(origin)?; -//! let _is_online = >::is_online(authority_index); +//! let _is_online = >::is_online(authority_index); //! Ok(()) //! } //! } @@ -81,27 +81,19 @@ use sp_std::prelude::*; use sp_std::convert::TryInto; use sp_runtime::{ offchain::storage::StorageValueRef, - traits::{AtLeast32BitUnsigned, Convert, Member, Saturating}, - transaction_validity::{ - InvalidTransaction, TransactionPriority, TransactionSource, TransactionValidity, - ValidTransaction, - }, + traits::{AtLeast32BitUnsigned, Convert, Saturating}, Perbill, Percent, RuntimeDebug, }; use sp_staking::{ SessionIndex, offence::{ReportOffence, Offence, Kind}, }; -use frame_support::{ - decl_error, decl_event, decl_module, decl_storage, - traits::{ - EstimateNextSessionRotation, Get, OneSessionHandler, ValidatorSet, - ValidatorSetWithIdentification, - }, - Parameter, +use frame_support::traits::{ + EstimateNextSessionRotation, OneSessionHandler, ValidatorSet, ValidatorSetWithIdentification, }; -use frame_system::{ensure_none, offchain::{SendTransactionTypes, SubmitTransaction}}; +use frame_system::offchain::{SendTransactionTypes, SubmitTransaction}; pub use weights::WeightInfo; +pub use pallet::*; pub mod sr25519 { mod app_sr25519 { @@ -238,108 +230,152 @@ pub type IdentificationTuple = ( ValidatorSetWithIdentification<::AccountId>>::Identification, ); -pub trait Config: SendTransactionTypes> + frame_system::Config { - /// The identifier type for an authority. - type AuthorityId: Member + Parameter + RuntimeAppPublic + Default + Ord; - - /// The overarching event type. - type Event: From> + Into<::Event>; - - /// A type for retrieving the validators supposed to be online in a session. - type ValidatorSet: ValidatorSetWithIdentification; - - /// A trait that allows us to estimate the current session progress and also the - /// average session length. - /// - /// This parameter is used to determine the longevity of `heartbeat` transaction and a - /// rough time when we should start considering sending heartbeats, since the workers - /// avoids sending them at the very beginning of the session, assuming there is a - /// chance the authority will produce a block and they won't be necessary. - type NextSessionRotation: EstimateNextSessionRotation; - - /// A type that gives us the ability to submit unresponsiveness offence reports. - type ReportUnresponsiveness: ReportOffence< - Self::AccountId, - IdentificationTuple, - UnresponsivenessOffence>, - >; +type OffchainResult = Result::BlockNumber>>; - /// A configuration for base priority of unsigned transactions. - /// - /// This is exposed so that it can be tuned for particular runtime, when - /// multiple pallets send unsigned transactions. - type UnsignedPriority: Get; +#[frame_support::pallet] +pub mod pallet { + use frame_support::{pallet_prelude::*, traits::Get}; + use frame_system::{pallet_prelude::*, ensure_none}; + use sp_runtime::{ + traits::{Member, MaybeSerializeDeserialize}, + transaction_validity::{ + InvalidTransaction, TransactionPriority, TransactionSource, TransactionValidity, ValidTransaction, + }, + }; + use frame_support::Parameter; + use super::*; + + #[pallet::pallet] + #[pallet::generate_store(pub(super) trait Store)] + pub struct Pallet(_); + + #[pallet::config] + pub trait Config: SendTransactionTypes> + frame_system::Config { + /// The identifier type for an authority. + type AuthorityId: Member + Parameter + RuntimeAppPublic + Default + Ord + MaybeSerializeDeserialize; + + /// The overarching event type. + type Event: From> + IsType<::Event>; + + /// A type for retrieving the validators supposed to be online in a session. + type ValidatorSet: ValidatorSetWithIdentification; + + /// A trait that allows us to estimate the current session progress and also the + /// average session length. + /// + /// This parameter is used to determine the longevity of `heartbeat` transaction and a + /// rough time when we should start considering sending heartbeats, since the workers + /// avoids sending them at the very beginning of the session, assuming there is a + /// chance the authority will produce a block and they won't be necessary. + type NextSessionRotation: EstimateNextSessionRotation; + + /// A type that gives us the ability to submit unresponsiveness offence reports. + type ReportUnresponsiveness: ReportOffence< + Self::AccountId, + IdentificationTuple, + UnresponsivenessOffence>, + >; + + /// A configuration for base priority of unsigned transactions. + /// + /// This is exposed so that it can be tuned for particular runtime, when + /// multiple pallets send unsigned transactions. + type UnsignedPriority: Get; - /// Weight information for extrinsics in this pallet. - type WeightInfo: WeightInfo; -} + /// Weight information for extrinsics in this pallet. + type WeightInfo: WeightInfo; + } -decl_event!( - pub enum Event where - ::AuthorityId, - IdentificationTuple = IdentificationTuple, - { + #[pallet::event] + #[pallet::generate_deposit(pub(super) fn deposit_event)] + #[pallet::metadata(T::AuthorityId = "AuthorityId", Vec> = "Vec")] + pub enum Event { /// A new heartbeat was received from `AuthorityId` \[authority_id\] - HeartbeatReceived(AuthorityId), + HeartbeatReceived(T::AuthorityId), /// At the end of the session, no offence was committed. AllGood, /// At the end of the session, at least one validator was found to be \[offline\]. - SomeOffline(Vec), + SomeOffline(Vec>), } -); -decl_storage! { - trait Store for Module as ImOnline { - /// The block number after which it's ok to send heartbeats in the current - /// session. - /// - /// At the beginning of each session we set this to a value that should fall - /// roughly in the middle of the session duration. The idea is to first wait for - /// the validators to produce a block in the current session, so that the - /// heartbeat later on will not be necessary. - /// - /// This value will only be used as a fallback if we fail to get a proper session - /// progress estimate from `NextSessionRotation`, as those estimates should be - /// more accurate then the value we calculate for `HeartbeatAfter`. - HeartbeatAfter get(fn heartbeat_after): T::BlockNumber; - - /// The current set of keys that may issue a heartbeat. - Keys get(fn keys): Vec; - - /// For each session index, we keep a mapping of `AuthIndex` to - /// `offchain::OpaqueNetworkState`. - ReceivedHeartbeats get(fn received_heartbeats): - double_map hasher(twox_64_concat) SessionIndex, hasher(twox_64_concat) AuthIndex - => Option>; - - /// For each session index, we keep a mapping of `ValidatorId` to the - /// number of blocks authored by the given authority. - AuthoredBlocks get(fn authored_blocks): - double_map hasher(twox_64_concat) SessionIndex, hasher(twox_64_concat) ValidatorId - => u32; - } - add_extra_genesis { - config(keys): Vec; - build(|config| Module::::initialize_keys(&config.keys)) - } -} - -decl_error! { - /// Error for the im-online module. - pub enum Error for Module { + #[pallet::error] + pub enum Error { /// Non existent public key. InvalidKey, /// Duplicated heartbeat. DuplicatedHeartbeat, } -} -decl_module! { - pub struct Module for enum Call where origin: T::Origin { - type Error = Error; + /// The block number after which it's ok to send heartbeats in the current + /// session. + /// + /// At the beginning of each session we set this to a value that should fall + /// roughly in the middle of the session duration. The idea is to first wait for + /// the validators to produce a block in the current session, so that the + /// heartbeat later on will not be necessary. + /// + /// This value will only be used as a fallback if we fail to get a proper session + /// progress estimate from `NextSessionRotation`, as those estimates should be + /// more accurate then the value we calculate for `HeartbeatAfter`. + #[pallet::storage] + #[pallet::getter(fn heartbeat_after)] + pub(crate) type HeartbeatAfter = StorageValue<_, T::BlockNumber, ValueQuery>; + + /// The current set of keys that may issue a heartbeat. + #[pallet::storage] + #[pallet::getter(fn keys)] + pub(crate) type Keys = StorageValue<_, Vec, ValueQuery>; + + /// For each session index, we keep a mapping of `AuthIndex` to + /// `offchain::OpaqueNetworkState`. + #[pallet::storage] + #[pallet::getter(fn received_heartbeats)] + pub(crate) type ReceivedHeartbeats = StorageDoubleMap< + _, + Twox64Concat, + SessionIndex, + Twox64Concat, + AuthIndex, + Vec, + >; + + /// For each session index, we keep a mapping of `ValidatorId` to the + /// number of blocks authored by the given authority. + #[pallet::storage] + #[pallet::getter(fn authored_blocks)] + pub(crate) type AuthoredBlocks = StorageDoubleMap< + _, + Twox64Concat, + SessionIndex, + Twox64Concat, + ValidatorId, + u32, + ValueQuery, + >; + + #[pallet::genesis_config] + pub struct GenesisConfig { + pub keys: Vec, + } + + #[cfg(feature = "std")] + impl Default for GenesisConfig { + fn default() -> Self { + GenesisConfig { + keys: Default::default(), + } + } + } - fn deposit_event() = default; + #[pallet::genesis_build] + impl GenesisBuild for GenesisConfig { + fn build(&self) { + Pallet::::initialize_keys(&self.keys); + } + } + #[pallet::call] + impl Pallet { /// # /// - Complexity: `O(K + E)` where K is length of `Keys` (heartbeat.validators_len) /// and E is length of `heartbeat.network_state.external_address` @@ -351,21 +387,21 @@ decl_module! { /// # // NOTE: the weight includes the cost of validate_unsigned as it is part of the cost to // import block with such an extrinsic. - #[weight = ::WeightInfo::validate_unsigned_and_then_heartbeat( + #[pallet::weight(::WeightInfo::validate_unsigned_and_then_heartbeat( heartbeat.validators_len as u32, heartbeat.network_state.external_addresses.len() as u32, - )] - fn heartbeat( - origin, + ))] + pub fn heartbeat( + origin: OriginFor, heartbeat: Heartbeat, // since signature verification is done in `validate_unsigned` // we can skip doing it here again. _signature: ::Signature, - ) { + ) -> DispatchResultWithPostInfo { ensure_none(origin)?; let current_session = T::ValidatorSet::session_index(); - let exists = ::contains_key( + let exists = ReceivedHeartbeats::::contains_key( ¤t_session, &heartbeat.authority_index ); @@ -375,20 +411,24 @@ decl_module! { Self::deposit_event(Event::::HeartbeatReceived(public.clone())); let network_state = heartbeat.network_state.encode(); - ::insert( + ReceivedHeartbeats::::insert( ¤t_session, &heartbeat.authority_index, &network_state ); + + Ok(().into()) } else if exists { Err(Error::::DuplicatedHeartbeat)? } else { Err(Error::::InvalidKey)? } } + } - // Runs after every block. - fn offchain_worker(now: T::BlockNumber) { + #[pallet::hooks] + impl Hooks> for Pallet { + fn offchain_worker(now: BlockNumberFor) { // Only send messages if we are a potential validator. if sp_io::offchain::is_validator() { for res in Self::send_heartbeats(now).into_iter().flatten() { @@ -410,15 +450,69 @@ decl_module! { } } } -} -type OffchainResult = Result::BlockNumber>>; + /// Invalid transaction custom error. Returned when validators_len field in heartbeat is incorrect. + pub(crate) const INVALID_VALIDATORS_LEN: u8 = 10; + + #[pallet::validate_unsigned] + impl ValidateUnsigned for Pallet { + type Call = Call; + + fn validate_unsigned(_source: TransactionSource, call: &Self::Call) -> TransactionValidity { + if let Call::heartbeat(heartbeat, signature) = call { + if >::is_online(heartbeat.authority_index) { + // we already received a heartbeat for this authority + return InvalidTransaction::Stale.into(); + } + + // check if session index from heartbeat is recent + let current_session = T::ValidatorSet::session_index(); + if heartbeat.session_index != current_session { + return InvalidTransaction::Stale.into(); + } + + // verify that the incoming (unverified) pubkey is actually an authority id + let keys = Keys::::get(); + if keys.len() as u32 != heartbeat.validators_len { + return InvalidTransaction::Custom(INVALID_VALIDATORS_LEN).into(); + } + let authority_id = match keys.get(heartbeat.authority_index as usize) { + Some(id) => id, + None => return InvalidTransaction::BadProof.into(), + }; + + // check signature (this is expensive so we do it last). + let signature_valid = heartbeat.using_encoded(|encoded_heartbeat| { + authority_id.verify(&encoded_heartbeat, &signature) + }); + + if !signature_valid { + return InvalidTransaction::BadProof.into(); + } + + ValidTransaction::with_tag_prefix("ImOnline") + .priority(T::UnsignedPriority::get()) + .and_provides((current_session, authority_id)) + .longevity( + TryInto::::try_into( + T::NextSessionRotation::average_session_length() / 2u32.into(), + ) + .unwrap_or(64_u64), + ) + .propagate(true) + .build() + } else { + InvalidTransaction::Call.into() + } + } + } +} /// Keep track of number of authored blocks per authority, uncles are counted as /// well since they're a valid proof of being online. impl< T: Config + pallet_authorship::Config, -> pallet_authorship::EventHandler, T::BlockNumber> for Module +> pallet_authorship::EventHandler, T::BlockNumber> for Pallet { fn note_author(author: ValidatorId) { Self::note_authorship(author); @@ -429,7 +523,7 @@ impl< } } -impl Module { +impl Pallet { /// Returns `true` if a heartbeat has been received for the authority at /// `authority_index` in the authorities series or if the authority has /// authored at least one block, during the current session. Otherwise @@ -449,8 +543,8 @@ impl Module { fn is_online_aux(authority_index: AuthIndex, authority: &ValidatorId) -> bool { let current_session = T::ValidatorSet::session_index(); - ::contains_key(¤t_session, &authority_index) || - >::get( + ReceivedHeartbeats::::contains_key(¤t_session, &authority_index) || + AuthoredBlocks::::get( ¤t_session, authority, ) != 0 @@ -460,14 +554,14 @@ impl Module { /// the authorities series, during the current session. Otherwise `false`. pub fn received_heartbeat_in_current_session(authority_index: AuthIndex) -> bool { let current_session = T::ValidatorSet::session_index(); - ::contains_key(¤t_session, &authority_index) + ReceivedHeartbeats::::contains_key(¤t_session, &authority_index) } /// Note that the given authority has authored a block in the current session. fn note_authorship(author: ValidatorId) { let current_session = T::ValidatorSet::session_index(); - >::mutate( + AuthoredBlocks::::mutate( ¤t_session, author, |authored| *authored += 1, @@ -648,11 +742,11 @@ impl Module { } } -impl sp_runtime::BoundToRuntimeAppPublic for Module { +impl sp_runtime::BoundToRuntimeAppPublic for Pallet { type Public = T::AuthorityId; } -impl OneSessionHandler for Module { +impl OneSessionHandler for Pallet { type Key = T::AuthorityId; fn on_genesis_session<'a, I: 'a>(validators: I) @@ -693,13 +787,13 @@ impl OneSessionHandler for Module { // Remove all received heartbeats and number of authored blocks from the // current session, they have already been processed and won't be needed // anymore. - ::remove_prefix(&T::ValidatorSet::session_index()); - >::remove_prefix(&T::ValidatorSet::session_index()); + ReceivedHeartbeats::::remove_prefix(&T::ValidatorSet::session_index()); + AuthoredBlocks::::remove_prefix(&T::ValidatorSet::session_index()); if offenders.is_empty() { - Self::deposit_event(RawEvent::AllGood); + Self::deposit_event(Event::::AllGood); } else { - Self::deposit_event(RawEvent::SomeOffline(offenders.clone())); + Self::deposit_event(Event::::SomeOffline(offenders.clone())); let validator_set_count = keys.len() as u32; let offence = UnresponsivenessOffence { session_index, validator_set_count, offenders }; @@ -714,61 +808,6 @@ impl OneSessionHandler for Module { } } -/// Invalid transaction custom error. Returned when validators_len field in heartbeat is incorrect. -const INVALID_VALIDATORS_LEN: u8 = 10; - -impl frame_support::unsigned::ValidateUnsigned for Module { - type Call = Call; - - fn validate_unsigned(_source: TransactionSource, call: &Self::Call) -> TransactionValidity { - if let Call::heartbeat(heartbeat, signature) = call { - if >::is_online(heartbeat.authority_index) { - // we already received a heartbeat for this authority - return InvalidTransaction::Stale.into(); - } - - // check if session index from heartbeat is recent - let current_session = T::ValidatorSet::session_index(); - if heartbeat.session_index != current_session { - return InvalidTransaction::Stale.into(); - } - - // verify that the incoming (unverified) pubkey is actually an authority id - let keys = Keys::::get(); - if keys.len() as u32 != heartbeat.validators_len { - return InvalidTransaction::Custom(INVALID_VALIDATORS_LEN).into(); - } - let authority_id = match keys.get(heartbeat.authority_index as usize) { - Some(id) => id, - None => return InvalidTransaction::BadProof.into(), - }; - - // check signature (this is expensive so we do it last). - let signature_valid = heartbeat.using_encoded(|encoded_heartbeat| { - authority_id.verify(&encoded_heartbeat, &signature) - }); - - if !signature_valid { - return InvalidTransaction::BadProof.into(); - } - - ValidTransaction::with_tag_prefix("ImOnline") - .priority(T::UnsignedPriority::get()) - .and_provides((current_session, authority_id)) - .longevity( - TryInto::::try_into( - T::NextSessionRotation::average_session_length() / 2u32.into(), - ) - .unwrap_or(64_u64), - ) - .propagate(true) - .build() - } else { - InvalidTransaction::Call.into() - } - } -} - /// An offence that is filed if a validator didn't send a heartbeat message. #[derive(RuntimeDebug)] #[cfg_attr(feature = "std", derive(Clone, PartialEq, Eq))] diff --git a/frame/im-online/src/tests.rs b/frame/im-online/src/tests.rs index f447a2ade5481..5ce931875b9a6 100644 --- a/frame/im-online/src/tests.rs +++ b/frame/im-online/src/tests.rs @@ -29,7 +29,7 @@ use sp_core::offchain::{ testing::{TestOffchainExt, TestTransactionPoolExt}, }; use frame_support::{dispatch, assert_noop}; -use sp_runtime::{testing::UintAuthorityId, transaction_validity::TransactionValidityError}; +use sp_runtime::{testing::UintAuthorityId, transaction_validity::{TransactionValidityError, InvalidTransaction}}; #[test] fn test_unresponsiveness_slash_fraction() { @@ -114,7 +114,7 @@ fn heartbeat( authority_index: u32, id: UintAuthorityId, validators: Vec, -) -> dispatch::DispatchResult { +) -> dispatch::DispatchResultWithPostInfo { use frame_support::unsigned::ValidateUnsigned; let heartbeat = Heartbeat { diff --git a/frame/nicks/src/lib.rs b/frame/nicks/src/lib.rs index 1afe55756777a..a6d2415ab96ef 100644 --- a/frame/nicks/src/lib.rs +++ b/frame/nicks/src/lib.rs @@ -15,16 +15,16 @@ // See the License for the specific language governing permissions and // limitations under the License. -//! # Nicks Module +//! # Nicks Pallet //! //! - [`Config`] //! - [`Call`] //! //! ## Overview //! -//! Nicks is an example module for keeping track of account names on-chain. It makes no effort to +//! Nicks is an example pallet for keeping track of account names on-chain. It makes no effort to //! create a name hierarchy, be a DNS replacement or provide reverse lookups. Furthermore, the -//! weights attached to this module's dispatchable functions are for demonstration purposes only and +//! weights attached to this pallet's dispatchable functions are for demonstration purposes only and //! have not been designed to be economically secure. Do not use this pallet as-is in production. //! //! ## Interface @@ -45,63 +45,64 @@ use sp_std::prelude::*; use sp_runtime::{ traits::{StaticLookup, Zero} }; -use frame_support::{ - decl_module, decl_event, decl_storage, ensure, decl_error, - traits::{Currency, EnsureOrigin, ReservableCurrency, OnUnbalanced, Get}, -}; -use frame_system::ensure_signed; +use frame_support::traits::{Currency, ReservableCurrency, OnUnbalanced}; +pub use pallet::*; type BalanceOf = <::Currency as Currency<::AccountId>>::Balance; type NegativeImbalanceOf = <::Currency as Currency<::AccountId>>::NegativeImbalance; -pub trait Config: frame_system::Config { - /// The overarching event type. - type Event: From> + Into<::Event>; +#[frame_support::pallet] +pub mod pallet { + use frame_system::{ensure_signed, pallet_prelude::*}; + use frame_support::{ensure, pallet_prelude::*, traits::{EnsureOrigin, Get}}; + use super::*; - /// The currency trait. - type Currency: ReservableCurrency; + #[pallet::config] + pub trait Config: frame_system::Config { + /// The overarching event type. + type Event: From> + IsType<::Event>; - /// Reservation fee. - type ReservationFee: Get>; + /// The currency trait. + type Currency: ReservableCurrency; - /// What to do with slashed funds. - type Slashed: OnUnbalanced>; + /// Reservation fee. + #[pallet::constant] + type ReservationFee: Get>; - /// The origin which may forcibly set or remove a name. Root can always do this. - type ForceOrigin: EnsureOrigin; + /// What to do with slashed funds. + type Slashed: OnUnbalanced>; - /// The minimum length a name may be. - type MinLength: Get; + /// The origin which may forcibly set or remove a name. Root can always do this. + type ForceOrigin: EnsureOrigin; - /// The maximum length a name may be. - type MaxLength: Get; -} + /// The minimum length a name may be. + #[pallet::constant] + type MinLength: Get; -decl_storage! { - trait Store for Module as Nicks { - /// The lookup table for names. - NameOf: map hasher(twox_64_concat) T::AccountId => Option<(Vec, BalanceOf)>; + /// The maximum length a name may be. + #[pallet::constant] + type MaxLength: Get; } -} -decl_event!( - pub enum Event where AccountId = ::AccountId, Balance = BalanceOf { + #[pallet::event] + #[pallet::generate_deposit(pub(super) fn deposit_event)] + #[pallet::metadata(T::AccountId = "AccountId", BalanceOf = "Balance")] + pub enum Event { /// A name was set. \[who\] - NameSet(AccountId), + NameSet(T::AccountId), /// A name was forcibly set. \[target\] - NameForced(AccountId), + NameForced(T::AccountId), /// A name was changed. \[who\] - NameChanged(AccountId), + NameChanged(T::AccountId), /// A name was cleared, and the given balance returned. \[who, deposit\] - NameCleared(AccountId, Balance), + NameCleared(T::AccountId, BalanceOf), /// A name was removed and the given balance slashed. \[target, deposit\] - NameKilled(AccountId, Balance), + NameKilled(T::AccountId, BalanceOf), } -); -decl_error! { - /// Error for the nicks module. - pub enum Error for Module { + /// Error for the nicks pallet. + #[pallet::error] + pub enum Error { /// A name is too short. TooShort, /// A name is too long. @@ -109,24 +110,20 @@ decl_error! { /// An account isn't named. Unnamed, } -} - -decl_module! { - /// Nicks module declaration. - pub struct Module for enum Call where origin: T::Origin { - type Error = Error; - - fn deposit_event() = default; - /// Reservation fee. - const ReservationFee: BalanceOf = T::ReservationFee::get(); + /// The lookup table for names. + #[pallet::storage] + pub(super) type NameOf = StorageMap<_, Twox64Concat, T::AccountId, (Vec, BalanceOf)>; - /// The minimum length a name may be. - const MinLength: u32 = T::MinLength::get() as u32; + #[pallet::pallet] + #[pallet::generate_store(pub(super) trait Store)] + pub struct Pallet(_); - /// The maximum length a name may be. - const MaxLength: u32 = T::MaxLength::get() as u32; + #[pallet::hooks] + impl Hooks> for Pallet {} + #[pallet::call] + impl Pallet { /// Set an account's name. The name should be a UTF-8-encoded string by convention, though /// we don't check it. /// @@ -143,24 +140,25 @@ decl_module! { /// - One storage read/write. /// - One event. /// # - #[weight = 50_000_000] - fn set_name(origin, name: Vec) { + #[pallet::weight(50_000_000)] + pub(super) fn set_name(origin: OriginFor, name: Vec) -> DispatchResultWithPostInfo { let sender = ensure_signed(origin)?; - ensure!(name.len() >= T::MinLength::get(), Error::::TooShort); - ensure!(name.len() <= T::MaxLength::get(), Error::::TooLong); + ensure!(name.len() >= T::MinLength::get() as usize, Error::::TooShort); + ensure!(name.len() <= T::MaxLength::get() as usize, Error::::TooLong); let deposit = if let Some((_, deposit)) = >::get(&sender) { - Self::deposit_event(RawEvent::NameChanged(sender.clone())); + Self::deposit_event(Event::::NameChanged(sender.clone())); deposit } else { let deposit = T::ReservationFee::get(); T::Currency::reserve(&sender, deposit.clone())?; - Self::deposit_event(RawEvent::NameSet(sender.clone())); + Self::deposit_event(Event::::NameSet(sender.clone())); deposit }; >::insert(&sender, (name, deposit)); + Ok(().into()) } /// Clear an account's name and return the deposit. Fails if the account was not named. @@ -173,8 +171,8 @@ decl_module! { /// - One storage read/write. /// - One event. /// # - #[weight = 70_000_000] - fn clear_name(origin) { + #[pallet::weight(70_000_000)] + pub(super) fn clear_name(origin: OriginFor) -> DispatchResultWithPostInfo { let sender = ensure_signed(origin)?; let deposit = >::take(&sender).ok_or(Error::::Unnamed)?.1; @@ -182,7 +180,8 @@ decl_module! { let err_amount = T::Currency::unreserve(&sender, deposit.clone()); debug_assert!(err_amount.is_zero()); - Self::deposit_event(RawEvent::NameCleared(sender, deposit)); + Self::deposit_event(Event::::NameCleared(sender, deposit)); + Ok(().into()) } /// Remove an account's name and take charge of the deposit. @@ -198,8 +197,11 @@ decl_module! { /// - One storage read/write. /// - One event. /// # - #[weight = 70_000_000] - fn kill_name(origin, target: ::Source) { + #[pallet::weight(70_000_000)] + pub(super) fn kill_name( + origin: OriginFor, + target: ::Source + ) -> DispatchResultWithPostInfo { T::ForceOrigin::ensure_origin(origin)?; // Figure out who we're meant to be clearing. @@ -209,7 +211,8 @@ decl_module! { // Slash their deposit from them. T::Slashed::on_unbalanced(T::Currency::slash_reserved(&target, deposit.clone()).0); - Self::deposit_event(RawEvent::NameKilled(target, deposit)); + Self::deposit_event(Event::::NameKilled(target, deposit)); + Ok(().into()) } /// Set a third-party account's name with no deposit. @@ -224,15 +227,20 @@ decl_module! { /// - One storage read/write. /// - One event. /// # - #[weight = 70_000_000] - fn force_name(origin, target: ::Source, name: Vec) { + #[pallet::weight(70_000_000)] + pub(super) fn force_name( + origin: OriginFor, + target: ::Source, + name: Vec + ) -> DispatchResultWithPostInfo { T::ForceOrigin::ensure_origin(origin)?; let target = T::Lookup::lookup(target)?; let deposit = >::get(&target).map(|x| x.1).unwrap_or_else(Zero::zero); >::insert(&target, (name, deposit)); - Self::deposit_event(RawEvent::NameForced(target)); + Self::deposit_event(Event::::NameForced(target)); + Ok(().into()) } } } @@ -308,8 +316,8 @@ mod tests { } parameter_types! { pub const ReservationFee: u64 = 2; - pub const MinLength: usize = 3; - pub const MaxLength: usize = 16; + pub const MinLength: u32 = 3; + pub const MaxLength: u32 = 16; } ord_parameter_types! { pub const One: u64 = 1; diff --git a/frame/offences/benchmarking/src/lib.rs b/frame/offences/benchmarking/src/lib.rs index 4e5160c6673fa..f65bdddd36d02 100644 --- a/frame/offences/benchmarking/src/lib.rs +++ b/frame/offences/benchmarking/src/lib.rs @@ -37,7 +37,7 @@ use sp_staking::offence::{ReportOffence, Offence}; use pallet_balances::Config as BalancesConfig; use pallet_babe::BabeEquivocationOffence; use pallet_grandpa::{GrandpaEquivocationOffence, GrandpaTimeSlot}; -use pallet_im_online::{Config as ImOnlineConfig, Module as ImOnline, UnresponsivenessOffence}; +use pallet_im_online::{Config as ImOnlineConfig, Pallet as ImOnline, UnresponsivenessOffence}; use pallet_offences::{Config as OffencesConfig, Module as Offences}; use pallet_session::historical::{Config as HistoricalConfig, IdentificationTuple}; use pallet_session::{Config as SessionConfig, SessionManager}; diff --git a/frame/support/procedural/src/lib.rs b/frame/support/procedural/src/lib.rs index 4cedf798821a9..069339a9794c8 100644 --- a/frame/support/procedural/src/lib.rs +++ b/frame/support/procedural/src/lib.rs @@ -28,6 +28,7 @@ mod debug_no_bound; mod clone_no_bound; mod partial_eq_no_bound; mod default_no_bound; +mod max_encoded_len; pub(crate) use storage::INHERENT_INSTANCE_NAME; use proc_macro::TokenStream; @@ -432,3 +433,9 @@ pub fn crate_to_pallet_version(input: TokenStream) -> TokenStream { /// The number of module instances supported by the runtime, starting at index 1, /// and up to `NUMBER_OF_INSTANCE`. pub(crate) const NUMBER_OF_INSTANCE: u8 = 16; + +/// Derive `MaxEncodedLen`. +#[proc_macro_derive(MaxEncodedLen)] +pub fn derive_max_encoded_len(input: TokenStream) -> TokenStream { + max_encoded_len::derive_max_encoded_len(input) +} diff --git a/frame/support/procedural/src/max_encoded_len.rs b/frame/support/procedural/src/max_encoded_len.rs new file mode 100644 index 0000000000000..72efa446b3f4d --- /dev/null +++ b/frame/support/procedural/src/max_encoded_len.rs @@ -0,0 +1,133 @@ +// This file is part of Substrate. + +// Copyright (C) 2021 Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +use frame_support_procedural_tools::generate_crate_access_2018; +use quote::{quote, quote_spanned}; +use syn::{ + Data, DeriveInput, Fields, GenericParam, Generics, TraitBound, Type, TypeParamBound, + parse_quote, spanned::Spanned, +}; + +/// impl for `#[derive(MaxEncodedLen)]` +pub fn derive_max_encoded_len(input: proc_macro::TokenStream) -> proc_macro::TokenStream { + let input: DeriveInput = match syn::parse(input) { + Ok(input) => input, + Err(e) => return e.to_compile_error().into(), + }; + + let mel_trait = match max_encoded_len_trait() { + Ok(mel_trait) => mel_trait, + Err(e) => return e.to_compile_error().into(), + }; + + let name = &input.ident; + let generics = add_trait_bounds(input.generics, mel_trait.clone()); + let (impl_generics, ty_generics, where_clause) = generics.split_for_impl(); + + let data_expr = data_length_expr(&input.data); + + quote::quote!( + const _: () = { + impl #impl_generics #mel_trait for #name #ty_generics #where_clause { + fn max_encoded_len() -> usize { + #data_expr + } + } + }; + ) + .into() +} + +fn max_encoded_len_trait() -> syn::Result { + let frame_support = generate_crate_access_2018("frame-support")?; + Ok(parse_quote!(#frame_support::traits::MaxEncodedLen)) +} + +// Add a bound `T: MaxEncodedLen` to every type parameter T. +fn add_trait_bounds(mut generics: Generics, mel_trait: TraitBound) -> Generics { + for param in &mut generics.params { + if let GenericParam::Type(ref mut type_param) = *param { + type_param.bounds.push(TypeParamBound::Trait(mel_trait.clone())); + } + } + generics +} + +/// generate an expression to sum up the max encoded length from several fields +fn fields_length_expr(fields: &Fields) -> proc_macro2::TokenStream { + let type_iter: Box> = match fields { + Fields::Named(ref fields) => Box::new(fields.named.iter().map(|field| &field.ty)), + Fields::Unnamed(ref fields) => Box::new(fields.unnamed.iter().map(|field| &field.ty)), + Fields::Unit => Box::new(std::iter::empty()), + }; + // expands to an expression like + // + // 0 + // .saturating_add(::max_encoded_len()) + // .saturating_add(::max_encoded_len()) + // + // We match the span of each field to the span of the corresponding + // `max_encoded_len` call. This way, if one field's type doesn't implement + // `MaxEncodedLen`, the compiler's error message will underline which field + // caused the issue. + let expansion = type_iter.map(|ty| { + quote_spanned! { + ty.span() => .saturating_add(<#ty>::max_encoded_len()) + } + }); + quote! { + 0_usize #( #expansion )* + } +} + +// generate an expression to sum up the max encoded length of each field +fn data_length_expr(data: &Data) -> proc_macro2::TokenStream { + match *data { + Data::Struct(ref data) => fields_length_expr(&data.fields), + Data::Enum(ref data) => { + // We need an expression expanded for each variant like + // + // 0 + // .max() + // .max() + // .saturating_add(1) + // + // The 1 derives from the discriminant; see + // https://github.com/paritytech/parity-scale-codec/ + // blob/f0341dabb01aa9ff0548558abb6dcc5c31c669a1/derive/src/encode.rs#L211-L216 + // + // Each variant expression's sum is computed the way an equivalent struct's would be. + + let expansion = data.variants.iter().map(|variant| { + let variant_expression = fields_length_expr(&variant.fields); + quote! { + .max(#variant_expression) + } + }); + + quote! { + 0_usize #( #expansion )* .saturating_add(1) + } + } + Data::Union(ref data) => { + // https://github.com/paritytech/parity-scale-codec/ + // blob/f0341dabb01aa9ff0548558abb6dcc5c31c669a1/derive/src/encode.rs#L290-L293 + syn::Error::new(data.union_token.span(), "Union types are not supported") + .to_compile_error() + } + } +} diff --git a/frame/support/src/storage/bounded_btree_map.rs b/frame/support/src/storage/bounded_btree_map.rs new file mode 100644 index 0000000000000..7fd0d175fda99 --- /dev/null +++ b/frame/support/src/storage/bounded_btree_map.rs @@ -0,0 +1,421 @@ +// This file is part of Substrate. + +// Copyright (C) 2017-2021 Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! Traits, types and structs to support a bounded BTreeMap. + +use sp_std::{ + borrow::Borrow, collections::btree_map::BTreeMap, convert::TryFrom, fmt, marker::PhantomData, + ops::Deref, +}; +use crate::{ + storage::StorageDecodeLength, + traits::{Get, MaxEncodedLen}, +}; +use codec::{Encode, Decode}; + +/// A bounded map based on a B-Tree. +/// +/// B-Trees represent a fundamental compromise between cache-efficiency and actually minimizing +/// the amount of work performed in a search. See [`BTreeMap`] for more details. +/// +/// Unlike a standard `BTreeMap`, there is a static, enforced upper limit to the number of items +/// in the map. All internal operations ensure this bound is respected. +#[derive(Encode, Decode)] +pub struct BoundedBTreeMap(BTreeMap, PhantomData); + +impl BoundedBTreeMap +where + S: Get, +{ + /// Get the bound of the type in `usize`. + pub fn bound() -> usize { + S::get() as usize + } +} + +impl BoundedBTreeMap +where + K: Ord, + S: Get, +{ + /// Create a new `BoundedBTreeMap`. + /// + /// Does not allocate. + pub fn new() -> Self { + BoundedBTreeMap(BTreeMap::new(), PhantomData) + } + + /// Create `Self` from a primitive `BTreeMap` without any checks. + unsafe fn unchecked_from(map: BTreeMap) -> Self { + Self(map, Default::default()) + } + + /// Create `Self` from a primitive `BTreeMap` without any checks. + /// + /// Logs warnings if the bound is not being respected. The scope is mentioned in the log message + /// to indicate where overflow is happening. + /// + /// # Example + /// + /// ``` + /// # use sp_std::collections::btree_map::BTreeMap; + /// # use frame_support::{parameter_types, storage::bounded_btree_map::BoundedBTreeMap}; + /// parameter_types! { + /// pub const Size: u32 = 5; + /// } + /// let mut map = BTreeMap::new(); + /// map.insert("foo", 1); + /// map.insert("bar", 2); + /// let bounded_map = unsafe {BoundedBTreeMap::<_, _, Size>::force_from(map, "demo")}; + /// ``` + pub unsafe fn force_from(map: BTreeMap, scope: Scope) -> Self + where + Scope: Into>, + { + if map.len() > Self::bound() { + log::warn!( + target: crate::LOG_TARGET, + "length of a bounded btreemap in scope {} is not respected.", + scope.into().unwrap_or("UNKNOWN"), + ); + } + + Self::unchecked_from(map) + } + + /// Consume self, and return the inner `BTreeMap`. + /// + /// This is useful when a mutating API of the inner type is desired, and closure-based mutation + /// such as provided by [`try_mutate`][Self::try_mutate] is inconvenient. + pub fn into_inner(self) -> BTreeMap { + debug_assert!(self.0.len() <= Self::bound()); + self.0 + } + + /// Consumes self and mutates self via the given `mutate` function. + /// + /// If the outcome of mutation is within bounds, `Some(Self)` is returned. Else, `None` is + /// returned. + /// + /// This is essentially a *consuming* shorthand [`Self::into_inner`] -> `...` -> + /// [`Self::try_from`]. + pub fn try_mutate(mut self, mut mutate: impl FnMut(&mut BTreeMap)) -> Option { + mutate(&mut self.0); + (self.0.len() <= Self::bound()).then(move || self) + } + + // Clears the map, removing all elements. + pub fn clear(&mut self) { + self.0.clear() + } + + /// Return a mutable reference to the value corresponding to the key. + /// + /// The key may be any borrowed form of the map's key type, but the ordering on the borrowed + /// form _must_ match the ordering on the key type. + pub fn get_mut(&mut self, key: &Q) -> Option<&mut V> + where + K: Borrow, + Q: Ord + ?Sized, + { + self.0.get_mut(key) + } + + /// Exactly the same semantics as [`BTreeMap::insert`], but returns an `Err` (and is a noop) if the + /// new length of the map exceeds `S`. + pub fn try_insert(&mut self, key: K, value: V) -> Result<(), ()> { + if self.len() < Self::bound() { + self.0.insert(key, value); + Ok(()) + } else { + Err(()) + } + } + + /// Remove a key from the map, returning the value at the key if the key was previously in the map. + /// + /// The key may be any borrowed form of the map's key type, but the ordering on the borrowed + /// form _must_ match the ordering on the key type. + pub fn remove(&mut self, key: &Q) -> Option + where + K: Borrow, + Q: Ord + ?Sized, + { + self.0.remove(key) + } + + /// Remove a key from the map, returning the value at the key if the key was previously in the map. + /// + /// The key may be any borrowed form of the map's key type, but the ordering on the borrowed + /// form _must_ match the ordering on the key type. + pub fn remove_entry(&mut self, key: &Q) -> Option<(K, V)> + where + K: Borrow, + Q: Ord + ?Sized, + { + self.0.remove_entry(key) + } +} + +impl Default for BoundedBTreeMap +where + K: Ord, + S: Get, +{ + fn default() -> Self { + Self::new() + } +} + +impl Clone for BoundedBTreeMap +where + BTreeMap: Clone, +{ + fn clone(&self) -> Self { + BoundedBTreeMap(self.0.clone(), PhantomData) + } +} + +#[cfg(feature = "std")] +impl fmt::Debug for BoundedBTreeMap +where + BTreeMap: fmt::Debug, + S: Get, +{ + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_tuple("BoundedBTreeMap").field(&self.0).field(&Self::bound()).finish() + } +} + +impl PartialEq for BoundedBTreeMap +where + BTreeMap: PartialEq, +{ + fn eq(&self, other: &Self) -> bool { + self.0 == other.0 + } +} + +impl Eq for BoundedBTreeMap where BTreeMap: Eq {} + +impl PartialEq> for BoundedBTreeMap +where + BTreeMap: PartialEq, +{ + fn eq(&self, other: &BTreeMap) -> bool { + self.0 == *other + } +} + +impl PartialOrd for BoundedBTreeMap +where + BTreeMap: PartialOrd, +{ + fn partial_cmp(&self, other: &Self) -> Option { + self.0.partial_cmp(&other.0) + } +} + +impl Ord for BoundedBTreeMap +where + BTreeMap: Ord, +{ + fn cmp(&self, other: &Self) -> sp_std::cmp::Ordering { + self.0.cmp(&other.0) + } +} + +impl IntoIterator for BoundedBTreeMap { + type Item = (K, V); + type IntoIter = sp_std::collections::btree_map::IntoIter; + + fn into_iter(self) -> Self::IntoIter { + self.0.into_iter() + } +} + +impl MaxEncodedLen for BoundedBTreeMap +where + K: MaxEncodedLen, + V: MaxEncodedLen, + S: Get, +{ + fn max_encoded_len() -> usize { + Self::bound() + .saturating_mul(K::max_encoded_len().saturating_add(V::max_encoded_len())) + .saturating_add(codec::Compact(S::get()).encoded_size()) + } +} + +impl Deref for BoundedBTreeMap +where + K: Ord, +{ + type Target = BTreeMap; + + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +impl AsRef> for BoundedBTreeMap +where + K: Ord, +{ + fn as_ref(&self) -> &BTreeMap { + &self.0 + } +} + +impl From> for BTreeMap +where + K: Ord, +{ + fn from(map: BoundedBTreeMap) -> Self { + map.0 + } +} + +impl TryFrom> for BoundedBTreeMap +where + K: Ord, + S: Get, +{ + type Error = (); + + fn try_from(value: BTreeMap) -> Result { + (value.len() <= Self::bound()).then(move || BoundedBTreeMap(value, PhantomData)).ok_or(()) + } +} + +impl codec::DecodeLength for BoundedBTreeMap { + fn len(self_encoded: &[u8]) -> Result { + // `BoundedBTreeMap` is stored just a `BTreeMap`, which is stored as a + // `Compact` with its length followed by an iteration of its items. We can just use + // the underlying implementation. + as codec::DecodeLength>::len(self_encoded) + } +} + +impl StorageDecodeLength for BoundedBTreeMap {} + +impl codec::EncodeLike> for BoundedBTreeMap where + BTreeMap: Encode +{ +} + +#[cfg(test)] +pub mod test { + use super::*; + use sp_io::TestExternalities; + use sp_std::convert::TryInto; + use crate::Twox128; + + crate::parameter_types! { + pub const Seven: u32 = 7; + pub const Four: u32 = 4; + } + + crate::generate_storage_alias! { Prefix, Foo => Value> } + crate::generate_storage_alias! { Prefix, FooMap => Map<(u32, Twox128), BoundedBTreeMap> } + crate::generate_storage_alias! { + Prefix, + FooDoubleMap => DoubleMap<(u32, Twox128), (u32, Twox128), BoundedBTreeMap> + } + + fn map_from_keys(keys: &[K]) -> BTreeMap + where + K: Ord + Copy, + { + keys.iter().copied().zip(std::iter::repeat(())).collect() + } + + fn boundedmap_from_keys(keys: &[K]) -> BoundedBTreeMap + where + K: Ord + Copy, + S: Get, + { + map_from_keys(keys).try_into().unwrap() + } + + #[test] + fn decode_len_works() { + TestExternalities::default().execute_with(|| { + let bounded = boundedmap_from_keys::(&[1, 2, 3]); + Foo::put(bounded); + assert_eq!(Foo::decode_len().unwrap(), 3); + }); + + TestExternalities::default().execute_with(|| { + let bounded = boundedmap_from_keys::(&[1, 2, 3]); + FooMap::insert(1, bounded); + assert_eq!(FooMap::decode_len(1).unwrap(), 3); + assert!(FooMap::decode_len(0).is_none()); + assert!(FooMap::decode_len(2).is_none()); + }); + + TestExternalities::default().execute_with(|| { + let bounded = boundedmap_from_keys::(&[1, 2, 3]); + FooDoubleMap::insert(1, 1, bounded); + assert_eq!(FooDoubleMap::decode_len(1, 1).unwrap(), 3); + assert!(FooDoubleMap::decode_len(2, 1).is_none()); + assert!(FooDoubleMap::decode_len(1, 2).is_none()); + assert!(FooDoubleMap::decode_len(2, 2).is_none()); + }); + } + + #[test] + fn try_insert_works() { + let mut bounded = boundedmap_from_keys::(&[1, 2, 3]); + bounded.try_insert(0, ()).unwrap(); + assert_eq!(*bounded, map_from_keys(&[1, 0, 2, 3])); + + assert!(bounded.try_insert(9, ()).is_err()); + assert_eq!(*bounded, map_from_keys(&[1, 0, 2, 3])); + } + + #[test] + fn deref_coercion_works() { + let bounded = boundedmap_from_keys::(&[1, 2, 3]); + // these methods come from deref-ed vec. + assert_eq!(bounded.len(), 3); + assert!(bounded.iter().next().is_some()); + assert!(!bounded.is_empty()); + } + + #[test] + fn try_mutate_works() { + let bounded = boundedmap_from_keys::(&[1, 2, 3, 4, 5, 6]); + let bounded = bounded + .try_mutate(|v| { + v.insert(7, ()); + }) + .unwrap(); + assert_eq!(bounded.len(), 7); + assert!(bounded + .try_mutate(|v| { + v.insert(8, ()); + }) + .is_none()); + } + + #[test] + fn btree_map_eq_works() { + let bounded = boundedmap_from_keys::(&[1, 2, 3, 4, 5, 6]); + assert_eq!(bounded, map_from_keys(&[1, 2, 3, 4, 5, 6])); + } +} diff --git a/frame/support/src/storage/bounded_vec.rs b/frame/support/src/storage/bounded_vec.rs index f441ba39b8843..8aecf2dc100b5 100644 --- a/frame/support/src/storage/bounded_vec.rs +++ b/frame/support/src/storage/bounded_vec.rs @@ -58,22 +58,14 @@ impl> BoundedVec { } /// Create `Self` from `t` without any checks. - /// - /// # WARNING - /// - /// Only use when you are sure you know what you are doing. - fn unchecked_from(t: Vec) -> Self { + unsafe fn unchecked_from(t: Vec) -> Self { Self(t, Default::default()) } /// Create `Self` from `t` without any checks. Logs warnings if the bound is not being /// respected. The additional scope can be used to indicate where a potential overflow is /// happening. - /// - /// # WARNING - /// - /// Only use when you are sure you know what you are doing. - pub fn force_from(t: Vec, scope: Option<&'static str>) -> Self { + pub unsafe fn force_from(t: Vec, scope: Option<&'static str>) -> Self { if t.len() > Self::bound() { log::warn!( target: crate::LOG_TARGET, @@ -166,7 +158,8 @@ impl> TryFrom> for BoundedVec { type Error = (); fn try_from(t: Vec) -> Result { if t.len() <= Self::bound() { - Ok(Self::unchecked_from(t)) + // explicit check just above + Ok(unsafe {Self::unchecked_from(t)}) } else { Err(()) } @@ -357,7 +350,7 @@ where // BoundedVec encodes like Vec which encodes like [T], which is a compact u32 // plus each item in the slice: // https://substrate.dev/rustdocs/v3.0.0/src/parity_scale_codec/codec.rs.html#798-808 - codec::Compact::::max_encoded_len() + codec::Compact(S::get()).encoded_size() .saturating_add(Self::bound().saturating_mul(T::max_encoded_len())) } } @@ -434,11 +427,11 @@ pub mod test { // append to a non-existing assert!(FooMap::get(2).is_none()); assert_ok!(FooMap::try_append(2, 4)); - assert_eq!(FooMap::get(2).unwrap(), BoundedVec::::unchecked_from(vec![4])); + assert_eq!(FooMap::get(2).unwrap(), unsafe {BoundedVec::::unchecked_from(vec![4])}); assert_ok!(FooMap::try_append(2, 5)); assert_eq!( FooMap::get(2).unwrap(), - BoundedVec::::unchecked_from(vec![4, 5]) + unsafe {BoundedVec::::unchecked_from(vec![4, 5])} ); }); @@ -458,12 +451,12 @@ pub mod test { assert_ok!(FooDoubleMap::try_append(2, 1, 4)); assert_eq!( FooDoubleMap::get(2, 1).unwrap(), - BoundedVec::::unchecked_from(vec![4]) + unsafe {BoundedVec::::unchecked_from(vec![4])} ); assert_ok!(FooDoubleMap::try_append(2, 1, 5)); assert_eq!( FooDoubleMap::get(2, 1).unwrap(), - BoundedVec::::unchecked_from(vec![4, 5]) + unsafe {BoundedVec::::unchecked_from(vec![4, 5])} ); }); } diff --git a/frame/support/src/storage/mod.rs b/frame/support/src/storage/mod.rs index adcf44a64620e..1eed6f0c4a7f2 100644 --- a/frame/support/src/storage/mod.rs +++ b/frame/support/src/storage/mod.rs @@ -29,6 +29,7 @@ pub use sp_runtime::TransactionOutcome; pub mod unhashed; pub mod hashed; +pub mod bounded_btree_map; pub mod bounded_vec; pub mod child; #[doc(hidden)] @@ -817,6 +818,7 @@ mod private { impl Sealed for Vec {} impl Sealed for Digest {} impl> Sealed for BoundedVec {} + impl Sealed for bounded_btree_map::BoundedBTreeMap {} } impl StorageAppend for Vec {} diff --git a/frame/support/src/traits.rs b/frame/support/src/traits.rs index d15356c1e1b09..2d7fb3db7366d 100644 --- a/frame/support/src/traits.rs +++ b/frame/support/src/traits.rs @@ -82,4 +82,32 @@ mod voting; pub use voting::{CurrencyToVote, SaturatingCurrencyToVote, U128CurrencyToVote}; mod max_encoded_len; +// This looks like an overlapping import/export, but it isn't: +// macros and traits live in distinct namespaces. pub use max_encoded_len::MaxEncodedLen; +/// Derive [`MaxEncodedLen`][max_encoded_len::MaxEncodedLen]. +/// +/// # Examples +/// +/// ``` +/// # use codec::Encode; +/// # use frame_support::traits::MaxEncodedLen; +/// #[derive(Encode, MaxEncodedLen)] +/// struct TupleStruct(u8, u32); +/// +/// assert_eq!(TupleStruct::max_encoded_len(), u8::max_encoded_len() + u32::max_encoded_len()); +/// ``` +/// +/// ``` +/// # use codec::Encode; +/// # use frame_support::traits::MaxEncodedLen; +/// #[derive(Encode, MaxEncodedLen)] +/// enum GenericEnum { +/// A, +/// B(T), +/// } +/// +/// assert_eq!(GenericEnum::::max_encoded_len(), u8::max_encoded_len() + u8::max_encoded_len()); +/// assert_eq!(GenericEnum::::max_encoded_len(), u8::max_encoded_len() + u128::max_encoded_len()); +/// ``` +pub use frame_support_procedural::MaxEncodedLen; diff --git a/frame/support/test/tests/max_encoded_len.rs b/frame/support/test/tests/max_encoded_len.rs new file mode 100644 index 0000000000000..e9e74929108d4 --- /dev/null +++ b/frame/support/test/tests/max_encoded_len.rs @@ -0,0 +1,149 @@ +// This file is part of Substrate. + +// Copyright (C) 2020-2021 Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! Tests for MaxEncodedLen derive macro + +use frame_support::traits::MaxEncodedLen; +use codec::{Compact, Encode}; + +// These structs won't even compile if the macro isn't working right. + +#[derive(Encode, MaxEncodedLen)] +struct Primitives { + bool: bool, + eight: u8, +} + +#[test] +fn primitives_max_length() { + assert_eq!(Primitives::max_encoded_len(), 2); +} + +#[derive(Encode, MaxEncodedLen)] +struct Composites { + fixed_size_array: [u8; 128], + tuple: (u128, u128), +} + +#[test] +fn composites_max_length() { + assert_eq!(Composites::max_encoded_len(), 128 + 16 + 16); +} + +#[derive(Encode, MaxEncodedLen)] +struct Generic { + one: T, + two: T, +} + +#[test] +fn generic_max_length() { + assert_eq!(Generic::::max_encoded_len(), u8::max_encoded_len() * 2); + assert_eq!(Generic::::max_encoded_len(), u32::max_encoded_len() * 2); +} + +#[derive(Encode, MaxEncodedLen)] +struct TwoGenerics { + t: T, + u: U, +} + +#[test] +fn two_generics_max_length() { + assert_eq!( + TwoGenerics::::max_encoded_len(), + u8::max_encoded_len() + u16::max_encoded_len() + ); + assert_eq!( + TwoGenerics::, [u16; 8]>::max_encoded_len(), + Compact::::max_encoded_len() + <[u16; 8]>::max_encoded_len() + ); +} + +#[derive(Encode, MaxEncodedLen)] +struct UnitStruct; + +#[test] +fn unit_struct_max_length() { + assert_eq!(UnitStruct::max_encoded_len(), 0); +} + +#[derive(Encode, MaxEncodedLen)] +struct TupleStruct(u8, u32); + +#[test] +fn tuple_struct_max_length() { + assert_eq!(TupleStruct::max_encoded_len(), u8::max_encoded_len() + u32::max_encoded_len()); +} + +#[derive(Encode, MaxEncodedLen)] +struct TupleGeneric(T, T); + +#[test] +fn tuple_generic_max_length() { + assert_eq!(TupleGeneric::::max_encoded_len(), u8::max_encoded_len() * 2); + assert_eq!(TupleGeneric::::max_encoded_len(), u32::max_encoded_len() * 2); +} + +#[derive(Encode, MaxEncodedLen)] +#[allow(unused)] +enum UnitEnum { + A, + B, +} + +#[test] +fn unit_enum_max_length() { + assert_eq!(UnitEnum::max_encoded_len(), 1); +} + +#[derive(Encode, MaxEncodedLen)] +#[allow(unused)] +enum TupleEnum { + A(u32), + B, +} + +#[test] +fn tuple_enum_max_length() { + assert_eq!(TupleEnum::max_encoded_len(), 1 + u32::max_encoded_len()); +} + +#[derive(Encode, MaxEncodedLen)] +#[allow(unused)] +enum StructEnum { + A { sixty_four: u64, one_twenty_eight: u128 }, + B, +} + +#[test] +fn struct_enum_max_length() { + assert_eq!(StructEnum::max_encoded_len(), 1 + u64::max_encoded_len() + u128::max_encoded_len()); +} + +// ensure that enums take the max of variant length, not the sum +#[derive(Encode, MaxEncodedLen)] +#[allow(unused)] +enum EnumMaxNotSum { + A(u32), + B(u32), +} + +#[test] +fn enum_max_not_sum_max_length() { + assert_eq!(EnumMaxNotSum::max_encoded_len(), 1 + u32::max_encoded_len()); +} diff --git a/frame/support/test/tests/max_encoded_len_ui.rs b/frame/support/test/tests/max_encoded_len_ui.rs new file mode 100644 index 0000000000000..c5c0489da924f --- /dev/null +++ b/frame/support/test/tests/max_encoded_len_ui.rs @@ -0,0 +1,26 @@ +// This file is part of Substrate. + +// Copyright (C) 2020-2021 Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#[rustversion::attr(not(stable), ignore)] +#[test] +fn derive_no_bound_ui() { + // As trybuild is using `cargo check`, we don't need the real WASM binaries. + std::env::set_var("SKIP_WASM_BUILD", "1"); + + let t = trybuild::TestCases::new(); + t.compile_fail("tests/max_encoded_len_ui/*.rs"); +} diff --git a/frame/support/test/tests/max_encoded_len_ui/not_encode.rs b/frame/support/test/tests/max_encoded_len_ui/not_encode.rs new file mode 100644 index 0000000000000..ed6fe94471e58 --- /dev/null +++ b/frame/support/test/tests/max_encoded_len_ui/not_encode.rs @@ -0,0 +1,6 @@ +use frame_support::traits::MaxEncodedLen; + +#[derive(MaxEncodedLen)] +struct NotEncode; + +fn main() {} diff --git a/frame/support/test/tests/max_encoded_len_ui/not_encode.stderr b/frame/support/test/tests/max_encoded_len_ui/not_encode.stderr new file mode 100644 index 0000000000000..f4dbeac040843 --- /dev/null +++ b/frame/support/test/tests/max_encoded_len_ui/not_encode.stderr @@ -0,0 +1,13 @@ +error[E0277]: the trait bound `NotEncode: WrapperTypeEncode` is not satisfied + --> $DIR/not_encode.rs:3:10 + | +3 | #[derive(MaxEncodedLen)] + | ^^^^^^^^^^^^^ the trait `WrapperTypeEncode` is not implemented for `NotEncode` + | + ::: $WORKSPACE/frame/support/src/traits/max_encoded_len.rs + | + | pub trait MaxEncodedLen: Encode { + | ------ required by this bound in `MaxEncodedLen` + | + = note: required because of the requirements on the impl of `frame_support::dispatch::Encode` for `NotEncode` + = note: this error originates in a derive macro (in Nightly builds, run with -Z macro-backtrace for more info) diff --git a/frame/support/test/tests/max_encoded_len_ui/not_mel.rs b/frame/support/test/tests/max_encoded_len_ui/not_mel.rs new file mode 100644 index 0000000000000..6116f30e5272b --- /dev/null +++ b/frame/support/test/tests/max_encoded_len_ui/not_mel.rs @@ -0,0 +1,14 @@ +use codec::Encode; +use frame_support::traits::MaxEncodedLen; + +#[derive(Encode)] +struct NotMel; + +#[derive(Encode, MaxEncodedLen)] +struct Generic { + t: T, +} + +fn main() { + let _ = Generic::::max_encoded_len(); +} diff --git a/frame/support/test/tests/max_encoded_len_ui/not_mel.stderr b/frame/support/test/tests/max_encoded_len_ui/not_mel.stderr new file mode 100644 index 0000000000000..0aabd4b2a393b --- /dev/null +++ b/frame/support/test/tests/max_encoded_len_ui/not_mel.stderr @@ -0,0 +1,21 @@ +error[E0599]: the function or associated item `max_encoded_len` exists for struct `Generic`, but its trait bounds were not satisfied + --> $DIR/not_mel.rs:13:29 + | +5 | struct NotMel; + | -------------- doesn't satisfy `NotMel: MaxEncodedLen` +... +8 | struct Generic { + | ----------------- + | | + | function or associated item `max_encoded_len` not found for this + | doesn't satisfy `Generic: MaxEncodedLen` +... +13 | let _ = Generic::::max_encoded_len(); + | ^^^^^^^^^^^^^^^ function or associated item cannot be called on `Generic` due to unsatisfied trait bounds + | + = note: the following trait bounds were not satisfied: + `NotMel: MaxEncodedLen` + which is required by `Generic: MaxEncodedLen` + = help: items from traits can only be used if the trait is implemented and in scope + = note: the following trait defines an item `max_encoded_len`, perhaps you need to implement it: + candidate #1: `MaxEncodedLen` diff --git a/frame/support/test/tests/max_encoded_len_ui/union.rs b/frame/support/test/tests/max_encoded_len_ui/union.rs new file mode 100644 index 0000000000000..c685b6939e9b8 --- /dev/null +++ b/frame/support/test/tests/max_encoded_len_ui/union.rs @@ -0,0 +1,10 @@ +use codec::Encode; +use frame_support::traits::MaxEncodedLen; + +#[derive(Encode, MaxEncodedLen)] +union Union { + a: u8, + b: u16, +} + +fn main() {} diff --git a/frame/support/test/tests/max_encoded_len_ui/union.stderr b/frame/support/test/tests/max_encoded_len_ui/union.stderr new file mode 100644 index 0000000000000..bc5519d674d9d --- /dev/null +++ b/frame/support/test/tests/max_encoded_len_ui/union.stderr @@ -0,0 +1,11 @@ +error: Union types are not supported + --> $DIR/union.rs:5:1 + | +5 | union Union { + | ^^^^^ + +error: Union types are not supported. + --> $DIR/union.rs:5:1 + | +5 | union Union { + | ^^^^^ diff --git a/frame/support/test/tests/max_encoded_len_ui/unsupported_variant.rs b/frame/support/test/tests/max_encoded_len_ui/unsupported_variant.rs new file mode 100644 index 0000000000000..675f62c168a69 --- /dev/null +++ b/frame/support/test/tests/max_encoded_len_ui/unsupported_variant.rs @@ -0,0 +1,12 @@ +use codec::Encode; +use frame_support::traits::MaxEncodedLen; + +#[derive(Encode)] +struct NotMel; + +#[derive(Encode, MaxEncodedLen)] +enum UnsupportedVariant { + NotMel(NotMel), +} + +fn main() {} diff --git a/frame/support/test/tests/max_encoded_len_ui/unsupported_variant.stderr b/frame/support/test/tests/max_encoded_len_ui/unsupported_variant.stderr new file mode 100644 index 0000000000000..aa10b5e4cc15e --- /dev/null +++ b/frame/support/test/tests/max_encoded_len_ui/unsupported_variant.stderr @@ -0,0 +1,12 @@ +error[E0599]: no function or associated item named `max_encoded_len` found for struct `NotMel` in the current scope + --> $DIR/unsupported_variant.rs:9:9 + | +5 | struct NotMel; + | -------------- function or associated item `max_encoded_len` not found for this +... +9 | NotMel(NotMel), + | ^^^^^^ function or associated item not found in `NotMel` + | + = help: items from traits can only be used if the trait is implemented and in scope + = note: the following trait defines an item `max_encoded_len`, perhaps you need to implement it: + candidate #1: `MaxEncodedLen` diff --git a/primitives/arithmetic/src/biguint.rs b/primitives/arithmetic/src/biguint.rs index 906c4d0cfd316..bfbd57f57013b 100644 --- a/primitives/arithmetic/src/biguint.rs +++ b/primitives/arithmetic/src/biguint.rs @@ -19,6 +19,7 @@ use num_traits::{Zero, One}; use sp_std::{cmp::Ordering, ops, prelude::*, vec, cell::RefCell, convert::TryFrom}; +use codec::{Encode, Decode}; // A sensible value for this would be half of the dword size of the host machine. Since the // runtime is compiled to 32bit webassembly, using 32 and 64 for single and double respectively @@ -78,7 +79,7 @@ fn div_single(a: Double, b: Single) -> (Double, Single) { } /// Simple wrapper around an infinitely large integer, represented as limbs of [`Single`]. -#[derive(Clone, Default)] +#[derive(Encode, Decode, Clone, Default)] pub struct BigUint { /// digits (limbs) of this number (sorted as msb -> lsb). pub(crate) digits: Vec, diff --git a/primitives/core/src/crypto.rs b/primitives/core/src/crypto.rs index 3479fc28c6358..7446ab25ce4be 100644 --- a/primitives/core/src/crypto.rs +++ b/primitives/core/src/crypto.rs @@ -540,8 +540,6 @@ ss58_address_format!( (25, "alphaville", "ZERO testnet, standard account (*25519).") JupiterAccount => (26, "jupiter", "Jupiter testnet, standard account (*25519).") - PatractAccount => - (27, "patract", "Patract mainnet, standard account (*25519).") SubsocialAccount => (28, "subsocial", "Subsocial network, standard account (*25519).") DhiwayAccount => @@ -586,10 +584,13 @@ ss58_address_format!( (65, "aventus", "Aventus Chain mainnet, standard account (*25519).") CrustAccount => (66, "crust", "Crust Network, standard account (*25519).") + EquilibriumAccount => + (67, "equilibrium", "Equilibrium Network, standard account (*25519).") SoraAccount => (69, "sora", "SORA Network, standard account (*25519).") SocialAccount => (252, "social-network", "Social Network, standard account (*25519).") + // Note: 16384 and above are reserved. ); diff --git a/primitives/utils/README.md b/primitives/utils/README.md index b0e04a3f4f198..2da70f09ccbc5 100644 --- a/primitives/utils/README.md +++ b/primitives/utils/README.md @@ -1,3 +1,20 @@ Utilities Primitives for Substrate -License: Apache-2.0 \ No newline at end of file +## Features + +### metered + +This feature changes the behaviour of the function `mpsc::tracing_unbounded`. +With the disabled feature this function is an alias to `futures::channel::mpsc::unbounded`. +However, when the feature is enabled it creates wrapper types to `UnboundedSender` +and `UnboundedReceiver` to register every `send`/`received`/`dropped` action happened on +the channel. + +Also this feature creates and registers a prometheus vector with name `unbounded_channel_len` and labels: + +| Label | Description | +| ------------ | --------------------------------------------- | +| entity | Name of channel passed to `tracing_unbounded` | +| action | One of `send`/`received`/`dropped` | + +License: Apache-2.0 diff --git a/primitives/utils/src/lib.rs b/primitives/utils/src/lib.rs index 430ec1ecb6f6c..6461361c96d1d 100644 --- a/primitives/utils/src/lib.rs +++ b/primitives/utils/src/lib.rs @@ -16,6 +16,23 @@ // limitations under the License. //! Utilities Primitives for Substrate +//! +//! # Features +//! +//! ## metered +//! +//! This feature changes the behaviour of the function `mpsc::tracing_unbounded`. +//! With the disabled feature this function is an alias to `futures::channel::mpsc::unbounded`. +//! However, when the feature is enabled it creates wrapper types to `UnboundedSender` +//! and `UnboundedReceiver` to register every `send`/`received`/`dropped` action happened on +//! the channel. +//! +//! Also this feature creates and registers a prometheus vector with name `unbounded_channel_len` and labels: +//! +//! | Label | Description | +//! | ------------ | --------------------------------------------- | +//! | entity | Name of channel passed to `tracing_unbounded` | +//! | action | One of `send`/`received`/`dropped` | pub mod metrics; pub mod mpsc; diff --git a/ss58-registry.json b/ss58-registry.json index 624d0256a81fe..43d0117f24f95 100644 --- a/ss58-registry.json +++ b/ss58-registry.json @@ -253,15 +253,6 @@ "standardAccount": "*25519", "website": "https://jupiter.patract.io" }, - { - "prefix": 27, - "network": "patract", - "displayName": "Patract", - "symbols": ["pDOT", "pKSM"], - "decimals": [10, 12], - "standardAccount": "*25519", - "website": "https://patract.network" - }, { "prefix": 28, "network": "subsocial", @@ -479,6 +470,15 @@ "website": "https://crust.network" }, { + "prefix": 67, + "network": "equilibrium", + "displayName": "Equilibrium Network", + "symbols": ["Unknown", "USD", "EQ", "ETH", "BTC", "EOS", "DOT", "CRV"], + "decimals": [0,9,9,9,9,9,9,9], + "standardAccount": "*25519", + "website": "https://equilibrium.io" + }, + { "prefix": 69, "network": "sora", "displayName": "SORA Network",