From 652999a802d1d1f9287fc58af051fc9459478ee1 Mon Sep 17 00:00:00 2001 From: Herr Seppia Date: Mon, 9 Sep 2024 09:16:57 +0200 Subject: [PATCH 1/4] node: check failed iterations length --- node/src/chain/header_validation.rs | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/node/src/chain/header_validation.rs b/node/src/chain/header_validation.rs index 56a07ba39b..435ec759a9 100644 --- a/node/src/chain/header_validation.rs +++ b/node/src/chain/header_validation.rs @@ -8,7 +8,7 @@ use crate::database; use crate::database::Ledger; use anyhow::anyhow; use dusk_bytes::Serializable; -use dusk_consensus::config::MINIMUM_BLOCK_TIME; +use dusk_consensus::config::{MINIMUM_BLOCK_TIME, RELAX_ITERATION_THRESHOLD}; use dusk_consensus::operations::Voter; use dusk_consensus::quorum::verifiers; use dusk_consensus::quorum::verifiers::QuorumResult; @@ -205,12 +205,13 @@ impl<'a, DB: database::DB> Validator<'a, DB> { ) -> anyhow::Result { let mut failed_atts = 0u8; - for (iter, att) in candidate_block - .failed_iterations - .att_list - .iter() - .enumerate() - { + let att_list = &candidate_block.failed_iterations.att_list; + + if att_list.len() > RELAX_ITERATION_THRESHOLD as usize { + anyhow::bail!("Too many failed iterations {}", att_list.len()) + } + + for (iter, att) in att_list.iter().enumerate() { if let Some((att, pk)) = att { info!(event = "verify_att", att_type = "failed_att", iter); From 65340407bd6b7b2ac1f2eb75abc2922bf161f110 Mon Sep 17 00:00:00 2001 From: Herr Seppia Date: Mon, 9 Sep 2024 09:19:49 +0200 Subject: [PATCH 2/4] node-data: add `size()` for Block Additionally add `size()` for the following struct: - Header - Fault - Vote - Transaction --- node-data/src/encoding.rs | 2 ++ node-data/src/ledger.rs | 2 +- node-data/src/ledger/block.rs | 6 ++++++ node-data/src/ledger/faults.rs | 22 ++++++++++++++++++++++ node-data/src/ledger/header.rs | 8 ++++++++ node-data/src/ledger/transaction.rs | 21 ++++++++++++++++++++- node-data/src/message.rs | 14 ++++++++++++++ 7 files changed, 73 insertions(+), 2 deletions(-) diff --git a/node-data/src/encoding.rs b/node-data/src/encoding.rs index 95b987bf7a..2941989eb6 100644 --- a/node-data/src/encoding.rs +++ b/node-data/src/encoding.rs @@ -87,6 +87,7 @@ impl Serializable for Transaction { let tx_type = Self::read_u32_le(r)?; let protocol_tx = Self::read_var_le_bytes32(r)?; + let tx_size = protocol_tx.len(); let inner = ProtocolTransaction::from_slice(&protocol_tx[..]) .map_err(|_| io::Error::from(io::ErrorKind::InvalidData))?; @@ -94,6 +95,7 @@ impl Serializable for Transaction { inner, version, r#type: tx_type, + size: Some(tx_size), }) } } diff --git a/node-data/src/ledger.rs b/node-data/src/ledger.rs index 1423fc1c0a..c1bf9fa5c7 100644 --- a/node-data/src/ledger.rs +++ b/node-data/src/ledger.rs @@ -8,7 +8,7 @@ mod header; pub use header::{Header, Seed}; mod block; -pub use block::{Block, BlockWithLabel, Hash, Label}; +pub use block::*; mod transaction; pub use transaction::{SpendingId, SpentTransaction, Transaction}; diff --git a/node-data/src/ledger/block.rs b/node-data/src/ledger/block.rs index 49273dd743..0c4a3a1759 100644 --- a/node-data/src/ledger/block.rs +++ b/node-data/src/ledger/block.rs @@ -65,6 +65,12 @@ impl Block { pub fn set_attestation(&mut self, att: Attestation) { self.header.att = att; } + + pub fn size(&self) -> io::Result { + let mut buf = vec![]; + self.write(&mut buf)?; + Ok(buf.len()) + } } #[derive(Debug, Clone, Copy, PartialEq)] diff --git a/node-data/src/ledger/faults.rs b/node-data/src/ledger/faults.rs index 960ffc0357..a2806b7ff0 100644 --- a/node-data/src/ledger/faults.rs +++ b/node-data/src/ledger/faults.rs @@ -35,6 +35,28 @@ pub enum Fault { DoubleValidationVote(FaultData, FaultData), } +impl Fault { + pub fn size(&self) -> usize { + // prev_block_hash + round + iter + const FAULT_CONSENSUS_HEADER_SIZE: usize = 32 + u64::SIZE + u8::SIZE; + // signer + signature + const FAULT_SIG_INFO_SIZE: usize = + BlsMultisigPublicKey::SIZE + BlsMultisigSignature::SIZE; + + const HEADERS: usize = FAULT_CONSENSUS_HEADER_SIZE * 2; + const SIG_INFOS: usize = FAULT_SIG_INFO_SIZE * 2; + let faults_data_size = match self { + Fault::DoubleCandidate(..) => 32 * 2, + Fault::DoubleRatificationVote(a, b) => { + a.data.size() + b.data.size() + } + Fault::DoubleValidationVote(a, b) => a.data.size() + b.data.size(), + }; + + HEADERS + SIG_INFOS + faults_data_size + } +} + #[derive(Debug, Error)] pub enum InvalidFault { #[error("Inner faults have same data")] diff --git a/node-data/src/ledger/header.rs b/node-data/src/ledger/header.rs index 26fc8c9dca..4cae8cdc44 100644 --- a/node-data/src/ledger/header.rs +++ b/node-data/src/ledger/header.rs @@ -44,6 +44,14 @@ pub struct Header { pub att: Attestation, } +impl Header { + pub fn size(&self) -> io::Result { + let mut buf = vec![]; + self.write(&mut buf)?; + Ok(buf.len()) + } +} + impl std::fmt::Debug for Header { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { let timestamp = diff --git a/node-data/src/ledger/transaction.rs b/node-data/src/ledger/transaction.rs index 7a2b413535..8d04b259c2 100644 --- a/node-data/src/ledger/transaction.rs +++ b/node-data/src/ledger/transaction.rs @@ -4,17 +4,35 @@ // // Copyright (c) DUSK NETWORK. All rights reserved. -use dusk_bytes::Serializable; +use std::io; + +use dusk_bytes::Serializable as DuskSerializable; use execution_core::signatures::bls; use execution_core::transfer::Transaction as ProtocolTransaction; use execution_core::BlsScalar; use serde::Serialize; +use crate::Serializable; + #[derive(Debug, Clone)] pub struct Transaction { pub version: u32, pub r#type: u32, pub inner: ProtocolTransaction, + pub(crate) size: Option, +} + +impl Transaction { + pub fn size(&self) -> io::Result { + match self.size { + Some(size) => Ok(size), + None => { + let mut buf = vec![]; + self.write(&mut buf)?; + Ok(buf.len()) + } + } + } } impl From for Transaction { @@ -23,6 +41,7 @@ impl From for Transaction { inner: value, r#type: 1, version: 1, + size: None, } } } diff --git a/node-data/src/message.rs b/node-data/src/message.rs index 9f70161ff3..1f84d14aa4 100644 --- a/node-data/src/message.rs +++ b/node-data/src/message.rs @@ -493,6 +493,20 @@ pub mod payload { NoQuorum = 3, } + impl Vote { + pub fn size(&self) -> usize { + const ENUM_BYTE: usize = 1; + + let data_size: usize = match &self { + Vote::NoCandidate => 0, + Vote::Valid(_) => 32, + Vote::Invalid(_) => 32, + Vote::NoQuorum => 0, + }; + ENUM_BYTE + data_size + } + } + impl fmt::Debug for Vote { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { let (desc, hash) = match &self { From af3478aca83cbde4e33864a5b16ca09299b8507d Mon Sep 17 00:00:00 2001 From: Herr Seppia Date: Mon, 9 Sep 2024 09:24:58 +0200 Subject: [PATCH 3/4] consensus: check block size - Add check for block size during candidate verification - Add bound for transactions size while generating a block --- consensus/src/commons.rs | 2 + consensus/src/config.rs | 2 + consensus/src/operations.rs | 1 + consensus/src/proposal/block_generator.rs | 79 ++++++++++++++--------- consensus/src/proposal/handler.rs | 12 +++- 5 files changed, 64 insertions(+), 32 deletions(-) diff --git a/consensus/src/commons.rs b/consensus/src/commons.rs index 854d53380f..e268e3a2ea 100644 --- a/consensus/src/commons.rs +++ b/consensus/src/commons.rs @@ -107,6 +107,7 @@ impl From for StepSigError { pub enum ConsensusError { InvalidBlock, InvalidBlockHash, + InvalidBlockSize(usize), InvalidSignature(BlsSigError), InvalidMsgType, InvalidValidationStepVotes(StepSigError), @@ -125,6 +126,7 @@ pub enum ConsensusError { VoteAlreadyCollected, TooManyTransactions(usize), TooManyFaults(usize), + UnknownBlockSize, } impl From for ConsensusError { diff --git a/consensus/src/config.rs b/consensus/src/config.rs index 763fbd0890..3ea3ae40ce 100644 --- a/consensus/src/config.rs +++ b/consensus/src/config.rs @@ -18,6 +18,8 @@ pub const RELAX_ITERATION_THRESHOLD: u8 = 8; pub const MAX_NUMBER_OF_TRANSACTIONS: usize = 1_000; pub const MAX_NUMBER_OF_FAULTS: usize = 100; +pub const MAX_BLOCK_SIZE: usize = 1_024 * 1_024; + /// Emergency mode is enabled after 16 iterations pub const EMERGENCY_MODE_ITERATION_THRESHOLD: u8 = 16; diff --git a/consensus/src/operations.rs b/consensus/src/operations.rs index d4808e16fc..342090196c 100644 --- a/consensus/src/operations.rs +++ b/consensus/src/operations.rs @@ -53,6 +53,7 @@ pub struct CallParams { pub generator_pubkey: node_data::bls::PublicKey, pub to_slash: Vec, pub voters_pubkey: Option>, + pub max_txs_bytes: usize, } #[derive(Default)] diff --git a/consensus/src/proposal/block_generator.rs b/consensus/src/proposal/block_generator.rs index e557c8addb..6859a9c733 100644 --- a/consensus/src/proposal/block_generator.rs +++ b/consensus/src/proposal/block_generator.rs @@ -6,14 +6,12 @@ use crate::commons::RoundUpdate; use crate::operations::{CallParams, Operations, Voter}; -use node_data::ledger::{ - to_str, Attestation, Block, Fault, IterationsInfo, Seed, Slash, -}; +use node_data::ledger::{to_str, Block, Fault, IterationsInfo, Seed, Slash}; use std::cmp::max; use crate::merkle::merkle_root; -use crate::config::{MAX_NUMBER_OF_FAULTS, MINIMUM_BLOCK_TIME}; +use crate::config::{MAX_BLOCK_SIZE, MAX_NUMBER_OF_FAULTS, MINIMUM_BLOCK_TIME}; use dusk_bytes::Serializable; use node_data::message::payload::Candidate; use node_data::message::{ConsensusHeader, Message, SignInfo, StepMessage}; @@ -99,52 +97,71 @@ impl Generator { faults }; + let block_gas_limit = self.executor.get_block_gas_limit().await; let to_slash = Slash::from_iterations_and_faults(&failed_iterations, faults)?; + let prev_block_hash = ru.hash(); + let mut blk_header = ledger::Header { + version: 0, + height: ru.round, + gas_limit: block_gas_limit, + prev_block_hash, + seed, + generator_bls_pubkey: *ru.pubkey_bls.bytes(), + prev_block_cert: *ru.att(), + iteration, + failed_iterations, + ..Default::default() + }; + + let header_size = blk_header.size().map_err(|e| { + crate::operations::Error::InvalidEST(anyhow::anyhow!( + "Cannot get header size {e}. This should be a bug" + )) + })?; + + // We always write the faults len in a u32 + let mut faults_size = u32::SIZE; + let faults_hashes: Vec<_> = faults + .iter() + .map(|f| { + faults_size += f.size(); + f.hash() + }) + .collect(); + + blk_header.faultroot = merkle_root(&faults_hashes); + + // We know for sure that this operation cannot underflow + let max_txs_bytes = MAX_BLOCK_SIZE - header_size - faults_size; + let call_params = CallParams { round: ru.round, generator_pubkey: ru.pubkey_bls.clone(), to_slash, voters_pubkey: Some(voters.to_owned()), + max_txs_bytes, }; let result = self.executor.execute_state_transition(call_params).await?; - let block_gas_limit = self.executor.get_block_gas_limit().await; + blk_header.state_hash = result.verification_output.state_root; + blk_header.event_hash = result.verification_output.event_hash; let tx_hashes: Vec<_> = result.txs.iter().map(|t| t.inner.hash()).collect(); let txs: Vec<_> = result.txs.into_iter().map(|t| t.inner).collect(); - let txroot = merkle_root(&tx_hashes[..]); + blk_header.txroot = merkle_root(&tx_hashes[..]); - let faults = Vec::::new(); - let faults_hashes: Vec<_> = faults.iter().map(|f| f.hash()).collect(); - let faultroot = merkle_root(&faults_hashes); - let timestamp = + blk_header.timestamp = max(ru.timestamp() + MINIMUM_BLOCK_TIME, get_current_timestamp()); - let prev_block_hash = ru.hash(); - let blk_header = ledger::Header { - version: 0, - height: ru.round, - timestamp, - gas_limit: block_gas_limit, - prev_block_hash, - seed, - generator_bls_pubkey: *ru.pubkey_bls.bytes(), - state_hash: result.verification_output.state_root, - event_hash: result.verification_output.event_hash, - hash: [0; 32], - att: Attestation::default(), - prev_block_cert: *ru.att(), - txroot, - faultroot, - iteration, - failed_iterations, - }; - - Ok(Block::new(blk_header, txs, faults).expect("block should be valid")) + Block::new(blk_header, txs, faults.to_vec()).map_err(|e| { + crate::operations::Error::InvalidEST(anyhow::anyhow!( + "Cannot create new block {e}", + )) + }) } } diff --git a/consensus/src/proposal/handler.rs b/consensus/src/proposal/handler.rs index 96686b69fe..b60a0928fa 100644 --- a/consensus/src/proposal/handler.rs +++ b/consensus/src/proposal/handler.rs @@ -5,7 +5,9 @@ // Copyright (c) DUSK NETWORK. All rights reserved. use crate::commons::{ConsensusError, Database, RoundUpdate}; -use crate::config::{MAX_NUMBER_OF_FAULTS, MAX_NUMBER_OF_TRANSACTIONS}; +use crate::config::{ + MAX_BLOCK_SIZE, MAX_NUMBER_OF_FAULTS, MAX_NUMBER_OF_TRANSACTIONS, +}; use crate::merkle::merkle_root; use crate::msg_handler::{HandleMsgOutput, MsgHandler}; use crate::user::committee::Committee; @@ -89,6 +91,14 @@ impl ProposalHandler { return Err(ConsensusError::NotCommitteeMember); } + let candidate_size = p + .candidate + .size() + .map_err(|_| ConsensusError::UnknownBlockSize)?; + if candidate_size > MAX_BLOCK_SIZE { + return Err(ConsensusError::InvalidBlockSize(candidate_size)); + } + // Verify new_block msg signature p.verify_signature()?; From 7e61d0d50e32b3f0daa747395d2fb47ff98010c3 Mon Sep 17 00:00:00 2001 From: Herr Seppia Date: Mon, 9 Sep 2024 09:25:38 +0200 Subject: [PATCH 4/4] rusk: add bound for transactions size while generating a block --- rusk/benches/block_ingestion.rs | 12 ++---------- rusk/src/lib/node/rusk.rs | 19 ++++++++++++++++++- rusk/tests/common/state.rs | 3 ++- 3 files changed, 22 insertions(+), 12 deletions(-) diff --git a/rusk/benches/block_ingestion.rs b/rusk/benches/block_ingestion.rs index 1bcb328320..d1f92e3a70 100644 --- a/rusk/benches/block_ingestion.rs +++ b/rusk/benches/block_ingestion.rs @@ -42,11 +42,7 @@ fn load_phoenix_txs() -> Vec { let line = line.unwrap(); let tx_bytes = hex::decode(line).unwrap(); let tx = ProtocolTransaction::from_slice(&tx_bytes).unwrap(); - txs.push(Transaction { - version: 1, - r#type: 0, - inner: tx, - }); + txs.push(tx.into()); } preverify(&txs); @@ -65,11 +61,7 @@ fn load_moonlight_txs() -> Vec { let line = line.unwrap(); let tx_bytes = hex::decode(line).unwrap(); let tx = ProtocolTransaction::from_slice(&tx_bytes).unwrap(); - txs.push(Transaction { - version: 1, - r#type: 0, - inner: tx, - }); + txs.push(tx.into()); } preverify(&txs); diff --git a/rusk/src/lib/node/rusk.rs b/rusk/src/lib/node/rusk.rs index 8757b4b807..935270224d 100644 --- a/rusk/src/lib/node/rusk.rs +++ b/rusk/src/lib/node/rusk.rs @@ -15,7 +15,7 @@ use sha3::{Digest, Sha3_256}; use tokio::task; use tracing::{debug, info, warn}; -use dusk_bytes::DeserializableSlice; +use dusk_bytes::{DeserializableSlice, Serializable}; use dusk_consensus::config::{ ratification_extra, ratification_quorum, validation_extra, validation_quorum, MAX_NUMBER_OF_TRANSACTIONS, @@ -129,6 +129,9 @@ impl Rusk { let mut event_hasher = Sha3_256::new(); + // We always write the faults len in a u32 + let mut size_left = params.max_txs_bytes - u32::SIZE; + for unspent_tx in txs { if let Some(timeout) = self.generation_timeout { if started.elapsed() > timeout { @@ -150,6 +153,18 @@ impl Rusk { continue; } + let tx_len = unspent_tx.size().unwrap_or_default(); + + if tx_len == 0 { + info!("Skipping {tx_id_hex} due to error while calculating the len"); + continue; + } + + if tx_len > size_left { + info!("Skipping {tx_id_hex} due size greater than bytes left: {size_left}"); + continue; + } + match execute( &mut session, &unspent_tx.inner, @@ -180,6 +195,8 @@ impl Rusk { continue; } + size_left -= tx_len; + // We're currently ignoring the result of successful calls let err = receipt.data.err().map(|e| format!("{e}")); info!("Tx {tx_id_hex} executed with {gas_spent} gas and err {err:?}"); diff --git a/rusk/tests/common/state.rs b/rusk/tests/common/state.rs index a4d1df42e5..798591e65f 100644 --- a/rusk/tests/common/state.rs +++ b/rusk/tests/common/state.rs @@ -4,8 +4,8 @@ // // Copyright (c) DUSK NETWORK. All rights reserved. -use std::path::Path; use std::sync::LazyLock; +use std::{path::Path, usize}; use dusk_bytes::Serializable; use node::vm::VMExecution; @@ -127,6 +127,7 @@ pub fn generator_procedure( generator_pubkey, to_slash, voters_pubkey: None, + max_txs_bytes: usize::MAX, }; let (transfer_txs, discarded, execute_output) =