From ca622e85ff66cf3ec512ba28243fbcd1abc40a61 Mon Sep 17 00:00:00 2001 From: Emilia Hane Date: Thu, 25 Jan 2024 17:44:31 +0100 Subject: [PATCH] Validate eth68 announcements --- crates/net/network/src/lib.rs | 1 + .../net/network/src/transactions/fetcher.rs | 7 +- crates/net/network/src/transactions/mod.rs | 2 + .../network/src/transactions/validation.rs | 364 ++++++++++++++++++ crates/primitives/src/transaction/tx_type.rs | 50 +++ 5 files changed, 423 insertions(+), 1 deletion(-) create mode 100644 crates/net/network/src/transactions/validation.rs diff --git a/crates/net/network/src/lib.rs b/crates/net/network/src/lib.rs index 526330b573fa..c6b28fdedbdd 100644 --- a/crates/net/network/src/lib.rs +++ b/crates/net/network/src/lib.rs @@ -147,5 +147,6 @@ pub use session::{ PendingSessionHandle, PendingSessionHandshakeError, SessionCommand, SessionEvent, SessionId, SessionLimits, SessionManager, SessionsConfig, }; +pub use transactions::{Announcement68Filter, FilterAnnouncement68, ValidateTx68}; pub use reth_eth_wire::{DisconnectReason, HelloMessageWithProtocols}; diff --git a/crates/net/network/src/transactions/fetcher.rs b/crates/net/network/src/transactions/fetcher.rs index 56ebd814b096..d3044e552134 100644 --- a/crates/net/network/src/transactions/fetcher.rs +++ b/crates/net/network/src/transactions/fetcher.rs @@ -16,7 +16,9 @@ use std::{ use tokio::sync::{mpsc::error::TrySendError, oneshot, oneshot::error::RecvError}; use tracing::{debug, trace}; -use super::{Peer, PooledTransactions, FULL_TRANSACTIONS_PACKET_SIZE_SOFT_LIMIT}; +use super::{ + Announcement68Filter, Peer, PooledTransactions, FULL_TRANSACTIONS_PACKET_SIZE_SOFT_LIMIT, +}; /// Maximum concurrent [`GetPooledTxRequest`]s to allow per peer. pub(super) const MAX_CONCURRENT_TX_REQUESTS_PER_PEER: u8 = 1; @@ -65,6 +67,8 @@ pub(super) struct TransactionFetcher { pub(super) unknown_hashes: LruMap), Unlimited>, /// Size metadata for unknown eth68 hashes. pub(super) eth68_meta: LruMap, + /// Filter for valid eth68 announcements. + pub(super) eth68_filter_valid: Announcement68Filter, } // === impl TransactionFetcher === @@ -723,6 +727,7 @@ impl Default for TransactionFetcher { ), unknown_hashes: LruMap::new_unlimited(), eth68_meta: LruMap::new_unlimited(), + eth68_filter_valid: Default::default(), } } } diff --git a/crates/net/network/src/transactions/mod.rs b/crates/net/network/src/transactions/mod.rs index a089faa32d17..02fff8406bd9 100644 --- a/crates/net/network/src/transactions/mod.rs +++ b/crates/net/network/src/transactions/mod.rs @@ -65,8 +65,10 @@ use tokio_stream::wrappers::{ReceiverStream, UnboundedReceiverStream}; use tracing::{debug, trace}; mod fetcher; +mod validation; use fetcher::{FetchEvent, TransactionFetcher}; +pub use validation::*; /// Cache limit of transactions to keep track of for a single peer. const PEER_TRANSACTION_CACHE_LIMIT: usize = 1024 * 10; diff --git a/crates/net/network/src/transactions/validation.rs b/crates/net/network/src/transactions/validation.rs new file mode 100644 index 000000000000..12ca3f764793 --- /dev/null +++ b/crates/net/network/src/transactions/validation.rs @@ -0,0 +1,364 @@ +//! Validation of [`NewPooledTransactionHashes68`] entries. These are of the form +//! `(ty, hash, size)`. Validation and filtering of entries is network dependent. [``] + +use derive_more::{Deref, DerefMut, Display}; +use itertools::izip; +use reth_eth_wire::NewPooledTransactionHashes68; +use reth_primitives::{TxHash, TxType}; +use tracing::{debug, trace}; + +/// Interface for validating a `(ty, size, hash)` tuple from a [`NewPooledTransactionHashes68`]. +pub trait ValidateTx68 { + /// Validates a [`NewPooledTransactionHashes68`] entry. Returns [`ValidationOutcome`] which + /// signals to the caller wether to fetch the transaction or wether to drop it, and wether the + /// sender of the announcement should be penalized. + fn should_fetch(&self, ty: u8, hash: TxHash, size: usize) -> ValidationOutcome; +} + +/// Outcomes from validating a `(ty, hash, size)` entry from a [`NewPooledTransactionHashes68`]. +/// Signals to the caller how to deal with an announcement entry and the peer who sent the +/// announcement. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ValidationOutcome { + /// Tells the caller to keep the entry in the announcement for fetch. + Fetch, + /// Tells the caller to filter out the entry from the announcement. + Ignore, + /// Tells the caller to filter out the entry from the announcement and penalize the peer. On + /// this outcome, caller can drop the announcement, that is up to each implementation. + ReportPeer, +} + +/// Filters valid entires in [`NewPooledTransactionHashes68`] in place, and flags misbehaving +/// peers. +pub trait FilterAnnouncement68: ValidateTx68 { + /// Removes invalid entries from a [`NewPooledTransactionHashes68`] announcement. Returns + /// [`FilterOutcome::ReportPeer`] if the caller should penalize the peer, otherwise + /// [`FilterOutcome::Ok`]. + fn filter_valid_entries(&self, msg: &mut NewPooledTransactionHashes68) -> FilterOutcome; +} + +/// Outcome from filtering [`NewPooledTransactionHashes68`]. Signals to caller wether to penalize +/// the sender of the announcement or not. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum FilterOutcome { + /// Peer behaves appropriately. + Ok, + /// A penalty should be flagged for the peer. Peer sent an announcement with unacceptably + /// invalid entries. + ReportPeer, +} + +/// Wrapper for types that implement [`FilterAnnouncement68`]. The definition of a valid +/// announcement is network dependent. For example, different networks support different [`TxType`] +/// s, and different [`TxType`]s have different transaction size constraints. Defaults to +/// [`LayerOne`]. +#[derive(Debug, Default, Deref, DerefMut)] +pub struct Announcement68Filter(N); + +/// L1 Network. +#[derive(Debug, Display, Default)] +pub struct LayerOne; + +impl ValidateTx68 for LayerOne { + fn should_fetch(&self, ty: u8, hash: TxHash, size: usize) -> ValidationOutcome { + // 1. checks if tx type is valid value for this network + let tx_type = match TxType::try_from(ty) { + Ok(ty) => ty, + Err(_) => { + debug!(target: "net::eth-wire", + ty=ty, + size=size, + hash=%hash, + network=%Self, + "invalid tx type in eth68 announcement" + ); + + return ValidationOutcome::ReportPeer + } + }; + // 2. checks if tx's encoded length is within limits for this network + if size > tx_type.max_encoded_tx_length() || size < tx_type.min_encoded_tx_length() { + // todo: min valid tx encoded length on layer 1? signature length? + debug!(target: "net::eth-wire", + ty=ty, + size=size, + hash=%hash, + max_encoded_tx_length=tx_type.max_encoded_tx_length(), + network=%Self, + "invalid tx size in eth68 announcement" + ); + + return ValidationOutcome::Ignore + } + + ValidationOutcome::Fetch + } +} + +impl FilterAnnouncement68 for LayerOne +where + Self: ValidateTx68, +{ + fn filter_valid_entries(&self, msg: &mut NewPooledTransactionHashes68) -> FilterOutcome { + trace!(target: "net::tx::validation", + types=?msg.types, + sizes=?msg.sizes, + hashes=?msg.hashes, + network=%Self, + "validating eth68 announcement data.." + ); + + // 1. checks if the announcement is empty + if msg.hashes.is_empty() { + debug!(target: "net::tx", + network=%Self, + "empty announcement" + ); + return FilterOutcome::ReportPeer + } + + let mut should_report_peer = false; + let mut indices_to_remove = vec![]; + + for (i, (&ty, &hash, &size)) in izip!(&msg.types, &msg.hashes, &msg.sizes).enumerate() { + match self.should_fetch(ty, hash, size) { + ValidationOutcome::Fetch => (), + ValidationOutcome::Ignore => indices_to_remove.push(i), + ValidationOutcome::ReportPeer => { + indices_to_remove.push(i); + should_report_peer = true; + } + } + } + + for (i, &index) in indices_to_remove.iter().enumerate() { + msg.hashes.remove(index - i); + msg.types.remove(index - i); + msg.sizes.remove(index - i); + } + + let mut hashes_sorted = msg.hashes.clone(); + hashes_sorted.sort(); + + for hash_pair in hashes_sorted.windows(2) { + if hash_pair[0] == hash_pair[1] { + let index = msg.hashes.iter().position(|&hash| hash == hash_pair[0]); + if let Some(i) = index { + debug!(target: "net::eth-wire", + ty=msg.types[i], + size=msg.sizes[i], + hash=%msg.hashes[i], + network=%Self, + "duplicate tx hash in eth68 announcement" + ); + + msg.hashes.remove(i); + msg.types.remove(i); + msg.sizes.remove(i); + } + + should_report_peer = true; + } + } + + if should_report_peer { + return FilterOutcome::ReportPeer + } + + FilterOutcome::Ok + } +} + +#[cfg(test)] +mod test { + use std::str::FromStr; + + use reth_primitives::B256; + + use super::*; + + #[test] + fn eth68_empty_announcement() { + let types = vec![]; + let sizes = vec![]; + let hashes = vec![]; + + let mut announcement = NewPooledTransactionHashes68 { types, sizes, hashes }; + + let filter: Announcement68Filter = Announcement68Filter::default(); + + let outcome = filter.filter_valid_entries(&mut announcement); + + assert_eq!(outcome, FilterOutcome::ReportPeer); + } + + #[test] + fn eth68_announcement_unrecognized_tx_type() { + let types = vec![ + TxType::MAX_RESERVED_EIP as u8 + 1, // the first type isn't valid + TxType::Legacy as u8, + ]; + let sizes = vec![ + TxType::MAX_RESERVED_EIP.max_encoded_tx_length(), + TxType::MAX_RESERVED_EIP.max_encoded_tx_length(), + ]; + let hashes = vec![ + B256::from_str("0xbeefcafebeefcafebeefcafebeefcafebeefcafebeefcafebeefcafebeefcafa") + .unwrap(), + B256::from_str("0xbeefcafebeefcafebeefcafebeefcafebeefcafebeefcafebeefcafebeefbbbb") + .unwrap(), + ]; + + let mut announcement = NewPooledTransactionHashes68 { + types: types.clone(), + sizes: sizes.clone(), + hashes: hashes.clone(), + }; + + let filter: Announcement68Filter = Announcement68Filter::default(); + + let outcome = filter.filter_valid_entries(&mut announcement); + + assert_eq!(outcome, FilterOutcome::ReportPeer); + + assert_eq!( + announcement, + NewPooledTransactionHashes68 { + types: vec!(types[1]), + sizes: vec!(sizes[1]), + hashes: vec!(hashes[1]), + } + ); + } + + #[test] + fn eth68_announcement_too_big_tx() { + let types = + vec![TxType::MAX_RESERVED_EIP as u8, TxType::Legacy as u8, TxType::EIP2930 as u8]; + let sizes = vec![ + TxType::MAX_RESERVED_EIP.max_encoded_tx_length(), + TxType::MAX_RESERVED_EIP.max_encoded_tx_length() + 1, // the second length isn't valid + TxType::MAX_RESERVED_EIP.max_encoded_tx_length() - 1, + ]; + let hashes = vec![ + B256::from_str("0xbeefcafebeefcafebeefcafebeefcafebeefcafebeefcafebeefcafebeefcafa") + .unwrap(), + B256::from_str("0xbeefcafebeefcafebeefcafebeefcafebeefcafebeefcafebeefcafebeefbbbb") + .unwrap(), + B256::from_str("0xbeefcafebeefcafebeefcafebeefcafebeefcafebeefcafebeefcafebeefbbbc") + .unwrap(), + ]; + + let mut announcement = NewPooledTransactionHashes68 { + types: types.clone(), + sizes: sizes.clone(), + hashes: hashes.clone(), + }; + + let filter: Announcement68Filter = Announcement68Filter::default(); + + let outcome = filter.filter_valid_entries(&mut announcement); + + assert_eq!(outcome, FilterOutcome::Ok); + + assert_eq!( + announcement, + NewPooledTransactionHashes68 { + types: vec!(types[0], types[2]), + sizes: vec!(sizes[0], sizes[2]), + hashes: vec!(hashes[0], hashes[2]), + } + ); + } + + #[test] + fn eth68_announcement_too_small_tx() { + let types = + vec![TxType::MAX_RESERVED_EIP as u8, TxType::Legacy as u8, TxType::EIP2930 as u8]; + let sizes = vec![ + TxType::MAX_RESERVED_EIP.min_encoded_tx_length() - 1, // the first length isn't valid + TxType::MAX_RESERVED_EIP.max_encoded_tx_length(), + TxType::MAX_RESERVED_EIP.max_encoded_tx_length(), + ]; + let hashes = vec![ + B256::from_str("0xbeefcafebeefcafebeefcafebeefcafebeefcafebeefcafebeefcafebeefcafa") + .unwrap(), + B256::from_str("0xbeefcafebeefcafebeefcafebeefcafebeefcafebeefcafebeefcafebeefbbbb") + .unwrap(), + B256::from_str("0xbeefcafebeefcafebeefcafebeefcafebeefcafebeefcafebeefcafebeef00bb") + .unwrap(), + ]; + + let mut announcement = NewPooledTransactionHashes68 { + types: types.clone(), + sizes: sizes.clone(), + hashes: hashes.clone(), + }; + + let filter: Announcement68Filter = Announcement68Filter::default(); + + let outcome = filter.filter_valid_entries(&mut announcement); + + assert_eq!(outcome, FilterOutcome::Ok); + + assert_eq!( + announcement, + NewPooledTransactionHashes68 { + types: vec!(types[1], types[2]), + sizes: vec!(sizes[1], sizes[2]), + hashes: vec!(hashes[1], hashes[2]), + } + ); + } + + #[test] + fn eth68_announcement_duplicate_tx_hash() { + let types = vec![ + TxType::EIP1559 as u8, + TxType::EIP4844 as u8, + TxType::EIP1559 as u8, + TxType::EIP4844 as u8, + ]; + let sizes = vec![ + TxType::MAX_RESERVED_EIP.max_encoded_tx_length() - 3, + TxType::MAX_RESERVED_EIP.min_encoded_tx_length() + 1, + TxType::MAX_RESERVED_EIP.max_encoded_tx_length() - 1, + TxType::MAX_RESERVED_EIP.min_encoded_tx_length() + 4, + ]; + // first three or the same + let hashes = vec![ + B256::from_str("0xbeefcafebeefcafebeefcafebeefcafebeefcafebeefcafebeefcafebeefcafa") // removed dup + .unwrap(), + B256::from_str("0xbeefcafebeefcafebeefcafebeefcafebeefcafebeefcafebeefcafebeefcafa") // removed dup + .unwrap(), + B256::from_str("0xbeefcafebeefcafebeefcafebeefcafebeefcafebeefcafebeefcafebeefcafa") // dup + .unwrap(), + B256::from_str("0xbeefcafebeefcafebeefcafebeefcafebeefcafebeefcafebeefcafebeefbbbb") + .unwrap(), + ]; + + let mut announcement = NewPooledTransactionHashes68 { + types: types.clone(), + sizes: sizes.clone(), + hashes: hashes.clone(), + }; + + let filter: Announcement68Filter = Announcement68Filter::default(); + + let outcome = filter.filter_valid_entries(&mut announcement); + + assert_eq!(outcome, FilterOutcome::ReportPeer); + + // + + assert_eq!( + announcement, + NewPooledTransactionHashes68 { + types: vec!(types[2], types[3]), + sizes: vec!(sizes[2], sizes[3]), + hashes: vec!(hashes[2], hashes[3]), + } + ); + } +} diff --git a/crates/primitives/src/transaction/tx_type.rs b/crates/primitives/src/transaction/tx_type.rs index 579853a00871..088d99867134 100644 --- a/crates/primitives/src/transaction/tx_type.rs +++ b/crates/primitives/src/transaction/tx_type.rs @@ -45,6 +45,9 @@ pub enum TxType { } impl TxType { + /// The max type reserved by an EIP. + pub const MAX_RESERVED_EIP: TxType = Self::EIP4844; + /// Check if the transaction type has an access list. pub const fn has_access_list(&self) -> bool { match self { @@ -54,6 +57,30 @@ impl TxType { TxType::DEPOSIT => false, } } + + /// Returns the max size for a transaction type. + pub const fn max_encoded_tx_length(&self) -> usize { + match self { + TxType::Legacy => usize::pow(2, 20), // todo: set limit + TxType::EIP2930 => usize::pow(2, 20), // todo: set limit + TxType::EIP1559 => usize::pow(2, 20), // todo: set limit + TxType::EIP4844 => usize::pow(2, 20), + #[cfg(feature = "optimism")] + TxType::DEPOSIT => usize::pow(2, 20), // todo: set limit + } + } + + /// Returns the min size for a transaction type. + pub const fn min_encoded_tx_length(&self) -> usize { + match self { + TxType::Legacy => 1, // todo: set limit + TxType::EIP2930 => 1, // todo: set limit + TxType::EIP1559 => 1, // todo: set limit + TxType::EIP4844 => 1, + #[cfg(feature = "optimism")] + TxType::DEPOSIT => 1, // todo: set limit + } + } } impl From for u8 { @@ -75,6 +102,29 @@ impl From for U8 { } } +impl TryFrom for TxType { + type Error = &'static str; + + fn try_from(value: u8) -> Result { + #[cfg(feature = "optimism")] + if value == TxType::DEPOSIT as u8 { + return Ok(TxType::DEPOSIT) + } + + if value == TxType::Legacy as u8 { + return Ok(TxType::Legacy) + } else if value == TxType::EIP2930 as u8 { + return Ok(TxType::EIP2930) + } else if value == TxType::EIP1559 as u8 { + return Ok(TxType::EIP1559) + } else if value == TxType::EIP4844 as u8 { + return Ok(TxType::EIP4844) + } + + Err("invalid tx type") + } +} + impl Compact for TxType { fn to_compact(self, buf: &mut B) -> usize where