From 7919bcd1bb6dc61ae96e916d8db43a20c8f49da4 Mon Sep 17 00:00:00 2001 From: Tiago Carvalho Date: Thu, 22 Dec 2022 13:04:23 +0000 Subject: [PATCH 01/37] Add block space allocator specs --- .../src/base-ledger/images/block-space-allocator-bins.svg | 4 ++++ .../src/base-ledger/images/block-space-allocator-example.svg | 4 ++++ 2 files changed, 8 insertions(+) create mode 100644 documentation/specs/src/base-ledger/images/block-space-allocator-bins.svg create mode 100644 documentation/specs/src/base-ledger/images/block-space-allocator-example.svg diff --git a/documentation/specs/src/base-ledger/images/block-space-allocator-bins.svg b/documentation/specs/src/base-ledger/images/block-space-allocator-bins.svg new file mode 100644 index 0000000000..f9d7209da1 --- /dev/null +++ b/documentation/specs/src/base-ledger/images/block-space-allocator-bins.svg @@ -0,0 +1,4 @@ + + + +
DECRYPTED
DECRYPT...
E
E
E
E
E
E
E
E
E
E
E
E
E
E
bin.try_dump(tx)
bin.try_dump(tx)
PROTOCOL
PROTOC...
ENCRYPTED
ENCRYPT...
Set M of mempool transactions
Set P of proposed transactions
BlockSpaceAllocator
BlockSpaceAllocatorViewer does not support full SVG 1.1 diff --git a/documentation/specs/src/base-ledger/images/block-space-allocator-example.svg b/documentation/specs/src/base-ledger/images/block-space-allocator-example.svg new file mode 100644 index 0000000000..b19ad90ce7 --- /dev/null +++ b/documentation/specs/src/base-ledger/images/block-space-allocator-example.svg @@ -0,0 +1,4 @@ + + + +
Height
Height
H
H
D
D
P
P
E
E
H+1
H+1
D
D
P
P
H+2
H+2
P
P
H+3
H+3
P
P
E
E
P
P
H+4
H+4
E
E
P
P
D
D
Block space
Block spaceViewer does not support full SVG 1.1 \ No newline at end of file From 68b335676d25a70ba247b532acd1d6472979f6ca Mon Sep 17 00:00:00 2001 From: Tiago Carvalho Date: Thu, 22 Dec 2022 13:16:40 +0000 Subject: [PATCH 02/37] Add new deps --- Cargo.lock | 3 +++ apps/Cargo.toml | 3 +++ 2 files changed, 6 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index 8aba5b5646..27d701bf72 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3685,6 +3685,7 @@ version = "0.14.0" dependencies = [ "ark-serialize", "ark-std", + "assert_matches", "async-std", "async-trait", "base64 0.13.0", @@ -3708,6 +3709,7 @@ dependencies = [ "flate2", "futures 0.3.25", "git2", + "index-set", "itertools", "libc", "libloading", @@ -3715,6 +3717,7 @@ dependencies = [ "masp_proofs", "namada", "num-derive", + "num-rational", "num-traits 0.2.15", "num_cpus", "once_cell", diff --git a/apps/Cargo.toml b/apps/Cargo.toml index f1bf95a05d..b9bd06d2b1 100644 --- a/apps/Cargo.toml +++ b/apps/Cargo.toml @@ -75,6 +75,7 @@ ark-serialize = "0.3.0" ark-std = "0.3.0" # branch = "bat/arse-merkle-tree" arse-merkle-tree = {package = "sparse-merkle-tree", git = "https://github.com/heliaxdev/sparse-merkle-tree", rev = "04ad1eeb28901b57a7599bbe433b3822965dabe8", features = ["std", "borsh"]} +assert_matches = "1.5.0" async-std = {version = "=1.11.0", features = ["unstable"]} async-trait = "0.1.51" base64 = "0.13.0" @@ -96,10 +97,12 @@ eyre = "0.6.5" flate2 = "1.0.22" file-lock = "2.0.2" futures = "0.3" +index-set = {git = "https://github.com/heliaxdev/index-set", tag = "v0.7.1"} itertools = "0.10.1" libc = "0.2.97" libloading = "0.7.2" num-derive = "0.3.3" +num-rational = "0.4.1" num-traits = "0.2.14" num_cpus = "1.13.0" once_cell = "1.8.0" From f5d05a0b0f2c02541e526c19f73f949cbab93d13 Mon Sep 17 00:00:00 2001 From: Tiago Carvalho Date: Thu, 22 Dec 2022 15:04:58 +0000 Subject: [PATCH 03/37] Add PosQueries --- proof_of_stake/src/lib.rs | 1 + proof_of_stake/src/pos_queries.rs | 260 ++++++++++++++++++++++++++++++ 2 files changed, 261 insertions(+) create mode 100644 proof_of_stake/src/pos_queries.rs diff --git a/proof_of_stake/src/lib.rs b/proof_of_stake/src/lib.rs index 869708655a..0511ce1350 100644 --- a/proof_of_stake/src/lib.rs +++ b/proof_of_stake/src/lib.rs @@ -15,6 +15,7 @@ pub mod btree_set; pub mod epoched; pub mod parameters; +pub mod pos_queries; pub mod storage; pub mod types; // pub mod validation; diff --git a/proof_of_stake/src/pos_queries.rs b/proof_of_stake/src/pos_queries.rs new file mode 100644 index 0000000000..3171502400 --- /dev/null +++ b/proof_of_stake/src/pos_queries.rs @@ -0,0 +1,260 @@ +//! Storage API for querying data about Proof-of-stake related +//! data. This includes validator and epoch related data. +use std::collections::BTreeSet; + +use borsh::BorshDeserialize; +use namada_core::ledger::parameters::storage::get_max_proposal_bytes_key; +use namada_core::ledger::parameters::EpochDuration; +use namada_core::ledger::storage::types::decode; +use namada_core::ledger::storage::Storage; +use namada_core::ledger::{storage, storage_api}; +use namada_core::tendermint_proto::google::protobuf; +use namada_core::tendermint_proto::types::EvidenceParams; +use namada_core::types::address::Address; +use namada_core::types::chain::ProposalBytes; +use namada_core::types::storage::{BlockHeight, Epoch}; +use namada_core::types::{key, token}; +use thiserror::Error; + +use crate::types::WeightedValidator; +use crate::{PosBase, PosParams}; + +/// Errors returned by [`PosQueries`] operations. +#[derive(Error, Debug)] +pub enum Error { + /// The given address is not among the set of active validators for + /// the corresponding epoch. + #[error( + "The address '{0:?}' is not among the active validator set for epoch \ + {1}" + )] + NotValidatorAddress(Address, Epoch), + /// The given public key does not correspond to any active validator's + /// key at the provided epoch. + #[error( + "The public key '{0}' is not among the active validator set for epoch \ + {1}" + )] + NotValidatorKey(String, Epoch), + /// The given public key hash does not correspond to any active validator's + /// key at the provided epoch. + #[error( + "The public key hash '{0}' is not among the active validator set for \ + epoch {1}" + )] + NotValidatorKeyHash(String, Epoch), + /// An invalid Tendermint validator address was detected. + #[error("Invalid validator tendermint address")] + InvalidTMAddress, +} + +/// Result type returned by [`PosQueries`] operations. +pub type Result = ::std::result::Result; + +/// Methods used to query blockchain proof-of-stake related state, +/// such as the currently active set of validators. +pub trait PosQueries { + /// Get the set of active validators for a given epoch (defaulting to the + /// epoch of the current yet-to-be-committed block). + fn get_active_validators( + &self, + epoch: Option, + ) -> BTreeSet; + + /// Lookup the total voting power for an epoch (defaulting to the + /// epoch of the current yet-to-be-committed block). + fn get_total_voting_power(&self, epoch: Option) -> token::Amount; + + /// Simple helper function for the ledger to get balances + /// of the specified token at the specified address. + fn get_balance(&self, token: &Address, owner: &Address) -> token::Amount; + + /// Return evidence parameters. + // TODO: impove this docstring + fn get_evidence_params( + &self, + epoch_duration: &EpochDuration, + pos_params: &PosParams, + ) -> EvidenceParams; + + /// Lookup data about a validator from their address. + fn get_validator_from_address( + &self, + address: &Address, + epoch: Option, + ) -> Result<(token::Amount, key::common::PublicKey)>; + + /// Given a tendermint validator, the address is the hash + /// of the validators public key. We look up the native + /// address from storage using this hash. + // TODO: We may change how this lookup is done, see + // https://github.com/anoma/namada/issues/200 + fn get_validator_from_tm_address( + &self, + tm_address: &[u8], + epoch: Option, + ) -> Result
; + + /// Check if we are at a given [`BlockHeight`] offset, `height_offset`, + /// within the current [`Epoch`]. + fn is_deciding_offset_within_epoch(&self, height_offset: u64) -> bool; + + /// Given some [`BlockHeight`], return the corresponding [`Epoch`]. + fn get_epoch(&self, height: BlockHeight) -> Option; + + /// Retrieves the [`BlockHeight`] that is currently being decided. + fn get_current_decision_height(&self) -> BlockHeight; + + /// Retrieve the `max_proposal_bytes` consensus parameter from storage. + fn get_max_proposal_bytes(&self) -> ProposalBytes; +} + +impl PosQueries for Storage +where + D: storage::DB + for<'iter> storage::DBIter<'iter>, + H: storage::StorageHasher, +{ + fn get_active_validators( + &self, + epoch: Option, + ) -> BTreeSet { + let epoch = epoch.unwrap_or_else(|| self.get_current_epoch().0); + let validator_set = self.read_validator_set(); + validator_set + .get(epoch) + .expect("Validators for an epoch should be known") + .active + .clone() + } + + fn get_total_voting_power(&self, epoch: Option) -> token::Amount { + self.get_active_validators(epoch) + .iter() + .map(|validator| validator.bonded_stake) + .sum::() + .into() + } + + fn get_balance(&self, token: &Address, owner: &Address) -> token::Amount { + let balance = storage_api::StorageRead::read( + self, + &token::balance_key(token, owner), + ); + // Storage read must not fail, but there might be no value, in which + // case default (0) is returned + balance + .expect("Storage read in the protocol must not fail") + .unwrap_or_default() + } + + fn get_evidence_params( + &self, + epoch_duration: &EpochDuration, + pos_params: &PosParams, + ) -> EvidenceParams { + // Minimum number of epochs before tokens are unbonded and can be + // withdrawn + let len_before_unbonded = + std::cmp::max(pos_params.unbonding_len as i64 - 1, 0); + let max_age_num_blocks: i64 = + epoch_duration.min_num_of_blocks as i64 * len_before_unbonded; + let min_duration_secs = epoch_duration.min_duration.0 as i64; + let max_age_duration = Some(protobuf::Duration { + seconds: min_duration_secs * len_before_unbonded, + nanos: 0, + }); + EvidenceParams { + max_age_num_blocks, + max_age_duration, + ..EvidenceParams::default() + } + } + + fn get_validator_from_address( + &self, + address: &Address, + epoch: Option, + ) -> Result<(token::Amount, key::common::PublicKey)> { + let epoch = epoch.unwrap_or_else(|| self.get_current_epoch().0); + self.get_active_validators(Some(epoch)) + .into_iter() + .find(|validator| address == &validator.address) + .map(|validator| { + let protocol_pk_key = key::protocol_pk_key(&validator.address); + let bytes = self + .read(&protocol_pk_key) + .expect("Validator should have public protocol key") + .0 + .expect("Validator should have public protocol key"); + let protocol_pk: key::common::PublicKey = + BorshDeserialize::deserialize(&mut bytes.as_ref()).expect( + "Protocol public key in storage should be \ + deserializable", + ); + (validator.bonded_stake.into(), protocol_pk) + }) + .ok_or_else(|| Error::NotValidatorAddress(address.clone(), epoch)) + } + + fn get_validator_from_tm_address( + &self, + tm_address: &[u8], + epoch: Option, + ) -> Result
{ + let epoch = epoch.unwrap_or_else(|| self.get_current_epoch().0); + let validator_raw_hash = core::str::from_utf8(tm_address) + .map_err(|_| Error::InvalidTMAddress)?; + self.read_validator_address_raw_hash(validator_raw_hash) + .ok_or_else(|| { + Error::NotValidatorKeyHash( + validator_raw_hash.to_string(), + epoch, + ) + }) + } + + fn is_deciding_offset_within_epoch(&self, height_offset: u64) -> bool { + let current_decision_height = self.get_current_decision_height(); + + // NOTE: the first stored height in `fst_block_heights_of_each_epoch` + // is 0, because of a bug (should be 1), so this code needs to + // handle that case + // + // we can remove this check once that's fixed + if self.get_current_epoch().0 == Epoch(0) { + let height_offset_within_epoch = BlockHeight(1 + height_offset); + return current_decision_height == height_offset_within_epoch; + } + + let fst_heights_of_each_epoch = + self.block.pred_epochs.first_block_heights(); + + fst_heights_of_each_epoch + .last() + .map(|&h| { + let height_offset_within_epoch = h + height_offset; + current_decision_height == height_offset_within_epoch + }) + .unwrap_or(false) + } + + #[inline] + fn get_epoch(&self, height: BlockHeight) -> Option { + self.block.pred_epochs.get_epoch(height) + } + + #[inline] + fn get_current_decision_height(&self) -> BlockHeight { + self.last_height + 1 + } + + fn get_max_proposal_bytes(&self) -> ProposalBytes { + let key = get_max_proposal_bytes_key(); + let (maybe_value, _gas) = self + .read(&key) + .expect("Must be able to read ProposalBytes from storage"); + let value = + maybe_value.expect("ProposalBytes must be present in storage"); + decode(value).expect("Must be able to decode ProposalBytes in storage") + } +} From 94e4eeb47d1bd254041be827ac1b0f7cd6cef067 Mon Sep 17 00:00:00 2001 From: Tiago Carvalho Date: Thu, 22 Dec 2022 15:05:47 +0000 Subject: [PATCH 04/37] Make first block heights of each epoch public --- core/src/types/storage.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/core/src/types/storage.rs b/core/src/types/storage.rs index 5f46105480..255bd69e7f 100644 --- a/core/src/types/storage.rs +++ b/core/src/types/storage.rs @@ -1106,6 +1106,13 @@ impl Epochs { } None } + + /// Return all starting block heights for each successive Epoch. + /// + /// __INVARIANT:__ The returned values are sorted in ascending order. + pub fn first_block_heights(&self) -> &[BlockHeight] { + &self.first_block_heights + } } /// A value of a storage prefix iterator. From b0eb1851ed8d2807dde2dcff5640edbf65a8be38 Mon Sep 17 00:00:00 2001 From: Tiago Carvalho Date: Thu, 22 Dec 2022 15:06:04 +0000 Subject: [PATCH 05/37] Add block space allocator impl --- .../node/ledger/shell/block_space_alloc.rs | 511 ++++++++++++++++++ .../ledger/shell/block_space_alloc/states.rs | 175 ++++++ .../block_space_alloc/states/decrypted_txs.rs | 46 ++ .../block_space_alloc/states/encrypted_txs.rs | 106 ++++ .../block_space_alloc/states/protocol_txs.rs | 82 +++ .../block_space_alloc/states/remaining_txs.rs | 11 + apps/src/lib/node/ledger/shell/mod.rs | 1 + 7 files changed, 932 insertions(+) create mode 100644 apps/src/lib/node/ledger/shell/block_space_alloc.rs create mode 100644 apps/src/lib/node/ledger/shell/block_space_alloc/states.rs create mode 100644 apps/src/lib/node/ledger/shell/block_space_alloc/states/decrypted_txs.rs create mode 100644 apps/src/lib/node/ledger/shell/block_space_alloc/states/encrypted_txs.rs create mode 100644 apps/src/lib/node/ledger/shell/block_space_alloc/states/protocol_txs.rs create mode 100644 apps/src/lib/node/ledger/shell/block_space_alloc/states/remaining_txs.rs diff --git a/apps/src/lib/node/ledger/shell/block_space_alloc.rs b/apps/src/lib/node/ledger/shell/block_space_alloc.rs new file mode 100644 index 0000000000..5cae6ffd20 --- /dev/null +++ b/apps/src/lib/node/ledger/shell/block_space_alloc.rs @@ -0,0 +1,511 @@ +//! Primitives that facilitate keeping track of the number +//! of bytes utilized by some Tendermint consensus round's proposal. +//! +//! This is important, because Tendermint places an upper bound +//! on the size of a block, rejecting blocks whose size exceeds +//! the limit stated in [`RequestPrepareProposal`]. +//! +//! The code in this module doesn't perform any deserializing to +//! verify if we are, in fact, allocating space for the correct +//! kind of tx for the current [`BlockSpaceAllocator`] state. It +//! is up to `PrepareProposal` to dispatch the correct kind of tx +//! into the current state of the allocator. +//! +//! # How space is allocated +//! +//! In the current implementation, we allocate space for transactions +//! in the following order of preference: +//! +//! - First, we allot space for DKG decrypted txs. Decrypted txs take up as much +//! space as needed. We will see, shortly, why in practice this is fine. +//! - Next, we allot space for protocol txs. Protocol txs get half of the +//! remaining block space allotted to them. +//! - Finally, we allot space for DKG encrypted txs. We allow DKG encrypted txs +//! to take up at most 1/3 of the total block space. +//! - If any space remains, we try to fit any leftover protocol txs in the +//! block. +//! +//! Since at some fixed height `H` decrypted txs only take up as +//! much space as the encrypted txs from height `H - 1`, and we +//! restrict the space of encrypted txs to at most 1/3 of the +//! total block space, we roughly divide the Tendermint block +//! space in 3, for each major type of tx. + +pub mod states; + +// TODO: what if a tx has a size greater than the threshold for +// its bin? how do we handle this? if we keep it in the mempool +// forever, it'll be a DoS vec, as we can make nodes run out of +// memory! maybe we should allow block decisions for txs that are +// too big to fit in their respective bin? in these special block +// decisions, we would only decide proposals with "large" txs?? +// +// MAYBE: in the state machine impl, reset to beginning state, and +// and alloc space for large tx right at the start. the problem with +// this is that then we may not have enough space for decrypted txs + +// TODO: panic if we don't have enough space reserved for a +// decrypted tx; in theory, we should always have enough space +// reserved for decrypted txs, given the invariants of the state +// machine + +// TODO: refactor our measure of space to also reflect gas costs. +// the total gas of all chosen txs cannot exceed the configured max +// gas per block, otherwise a proposal will be rejected! + +use std::marker::PhantomData; + +use namada::core::ledger::storage::{self, Storage}; +use namada::proof_of_stake::pos_queries::PosQueries; + +#[allow(unused_imports)] +use crate::facade::tendermint_proto::abci::RequestPrepareProposal; + +/// Block space allocation failure status responses. +#[derive(Debug, Copy, Clone, Eq, PartialEq)] +pub enum AllocFailure { + /// The transaction can only be included in an upcoming block. + /// + /// We return the space left in the tx bin for logging purposes. + Rejected { bin_space_left: u64 }, + /// The transaction would overflow the allotted bin space, + /// therefore it needs to be handled separately. + /// + /// We return the size of the tx bin for logging purposes. + OverflowsBin { bin_size: u64 }, +} + +/// Allotted space for a batch of transactions in some proposed block, +/// measured in bytes. +/// +/// We keep track of the current space utilized by: +/// +/// - Protocol transactions. +/// - DKG decrypted transactions. +/// - DKG encrypted transactions. +#[derive(Debug, Default)] +pub struct BlockSpaceAllocator { + /// The current state of the [`BlockSpaceAllocator`] state machine. + _state: PhantomData<*const State>, + /// The total space Tendermint has allotted to the + /// application for the current block height. + block: TxBin, + /// The current space utilized by protocol transactions. + protocol_txs: TxBin, + /// The current space utilized by DKG encrypted transactions. + encrypted_txs: TxBin, + /// The current space utilized by DKG decrypted transactions. + decrypted_txs: TxBin, +} + +impl From<&Storage> + for BlockSpaceAllocator +where + D: storage::DB + for<'iter> storage::DBIter<'iter>, + H: storage::StorageHasher, +{ + #[inline] + fn from(storage: &Storage) -> Self { + Self::init(storage.get_max_proposal_bytes().get()) + } +} + +impl BlockSpaceAllocator { + /// Construct a new [`BlockSpaceAllocator`], with an upper bound + /// on the max size of all txs in a block defined by Tendermint. + #[inline] + pub fn init(tendermint_max_block_space_in_bytes: u64) -> Self { + let max = tendermint_max_block_space_in_bytes; + Self { + _state: PhantomData, + block: TxBin::init(max), + protocol_txs: TxBin::default(), + encrypted_txs: TxBin::default(), + // decrypted txs can use as much space as needed; in practice, + // we'll only need, at most, the amount of space reserved for + // encrypted txs at the prev block height + decrypted_txs: TxBin::init(max), + } + } +} + +impl BlockSpaceAllocator { + /// Return the amount of space left to initialize in all + /// [`TxBin`] instances. + /// + /// This is calculated based on the difference between the Tendermint + /// block space for a given round and the sum of the allotted space + /// to each [`TxBin`] instance in a [`BlockSpaceAllocator`]. + #[inline] + fn uninitialized_space_in_bytes(&self) -> u64 { + let total_bin_space = self.protocol_txs.allotted_space_in_bytes + + self.encrypted_txs.allotted_space_in_bytes + + self.decrypted_txs.allotted_space_in_bytes; + self.block.allotted_space_in_bytes - total_bin_space + } + + /// Claim all the space used by the [`TxBin`] instances + /// as block space. + #[inline] + fn claim_block_space(&mut self) { + let used_space = self.protocol_txs.occupied_space_in_bytes + + self.encrypted_txs.occupied_space_in_bytes + + self.decrypted_txs.occupied_space_in_bytes; + + self.block.occupied_space_in_bytes = used_space; + + self.decrypted_txs = TxBin::default(); + self.protocol_txs = TxBin::default(); + self.encrypted_txs = TxBin::default(); + } +} + +/// Allotted space for a batch of transactions of the same kind in some +/// proposed block, measured in bytes. +#[derive(Debug, Copy, Clone, Default)] +pub struct TxBin { + /// The current space utilized by the batch of transactions. + occupied_space_in_bytes: u64, + /// The maximum space the batch of transactions may occupy. + allotted_space_in_bytes: u64, +} + +impl TxBin { + /// Return a new [`TxBin`] with a total allotted space equal to the + /// floor of the fraction `frac` of the available block space `max_bytes`. + #[inline] + pub fn init_over_ratio(max_bytes: u64, frac: threshold::Threshold) -> Self { + let allotted_space_in_bytes = frac.over(max_bytes); + Self { + allotted_space_in_bytes, + occupied_space_in_bytes: 0, + } + } + + /// Return the amount of space left in this [`TxBin`]. + #[inline] + pub fn space_left_in_bytes(&self) -> u64 { + self.allotted_space_in_bytes - self.occupied_space_in_bytes + } + + /// Construct a new [`TxBin`], with a capacity of `max_bytes`. + #[inline] + pub fn init(max_bytes: u64) -> Self { + Self { + allotted_space_in_bytes: max_bytes, + occupied_space_in_bytes: 0, + } + } + + /// Shrink the allotted space of this [`TxBin`] to whatever + /// space is currently being utilized. + #[inline] + pub fn shrink_to_fit(&mut self) { + self.allotted_space_in_bytes = self.occupied_space_in_bytes; + } + + /// Try to dump a new transaction into this [`TxBin`]. + /// + /// Signal the caller if the tx is larger than its max + /// allotted bin space. + pub fn try_dump(&mut self, tx: &[u8]) -> Result<(), AllocFailure> { + let tx_len = tx.len() as u64; + if tx_len > self.allotted_space_in_bytes { + let bin_size = self.allotted_space_in_bytes; + return Err(AllocFailure::OverflowsBin { bin_size }); + } + let occupied = self.occupied_space_in_bytes + tx_len; + if occupied <= self.allotted_space_in_bytes { + self.occupied_space_in_bytes = occupied; + Ok(()) + } else { + let bin_space_left = self.space_left_in_bytes(); + Err(AllocFailure::Rejected { bin_space_left }) + } + } +} + +pub mod threshold { + //! Transaction allotment thresholds. + + use num_rational::Ratio; + + /// Threshold over a portion of block space. + #[derive(Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Hash)] + pub struct Threshold(Ratio); + + impl Threshold { + /// Return a new [`Threshold`]. + const fn new(numer: u64, denom: u64) -> Self { + // constrain ratio to a max of 1 + let numer = if numer > denom { denom } else { numer }; + Self(Ratio::new_raw(numer, denom)) + } + + /// Return a [`Threshold`] over some free space. + pub fn over(self, free_space_in_bytes: u64) -> u64 { + (self.0 * free_space_in_bytes).to_integer() + } + } + + /// Divide free space in three. + pub const ONE_THIRD: Threshold = Threshold::new(1, 3); + + /// Divide free space in two. + pub const ONE_HALF: Threshold = Threshold::new(1, 2); +} + +#[cfg(test)] +mod tests { + use std::cell::RefCell; + + use assert_matches::assert_matches; + use proptest::prelude::*; + + use super::states::{ + NextState, NextStateWithEncryptedTxs, NextStateWithoutEncryptedTxs, + TryAlloc, + }; + use super::*; + use crate::node::ledger::shims::abcipp_shim_types::shim::TxBytes; + + /// Proptest generated txs. + #[derive(Debug)] + struct PropTx { + tendermint_max_block_space_in_bytes: u64, + protocol_txs: Vec, + encrypted_txs: Vec, + decrypted_txs: Vec, + } + + /// Check that at most 1/3 of the block space is + /// reserved for each kind of tx type, in the + /// allocator's common path. + #[test] + fn test_txs_are_evenly_split_across_block() { + const BLOCK_SIZE: u64 = 60; + + // reserve block space for decrypted txs + let mut alloc = BlockSpaceAllocator::init(BLOCK_SIZE); + + // assume we got ~1/3 encrypted txs at the prev block + assert!(alloc.try_alloc(&[0; 18]).is_ok()); + + // reserve block space for protocol txs + let mut alloc = alloc.next_state(); + + // the space we allotted to decrypted txs was shrunk to + // the total space we actually used up + assert_eq!(alloc.decrypted_txs.allotted_space_in_bytes, 18); + + // check that the allotted space for protocol txs is correct + assert_eq!(21, (BLOCK_SIZE - 18) / 2); + assert_eq!(alloc.protocol_txs.allotted_space_in_bytes, 21); + + // fill up the block space with protocol txs + assert!(alloc.try_alloc(&[0; 17]).is_ok()); + assert_matches!( + alloc.try_alloc(&[0; (21 - 17) + 1]), + Err(AllocFailure::Rejected { .. }) + ); + + // reserve block space for encrypted txs + let mut alloc = alloc.next_state_with_encrypted_txs(); + + // check that space was shrunk + assert_eq!(alloc.protocol_txs.allotted_space_in_bytes, 17); + + // check that we reserve at most 1/3 of the block space to + // encrypted txs + assert_eq!(25, BLOCK_SIZE - 17 - 18); + assert_eq!(20, BLOCK_SIZE / 3); + assert_eq!(alloc.encrypted_txs.allotted_space_in_bytes, 20); + + // fill up the block space with encrypted txs + assert!(alloc.try_alloc(&[0; 20]).is_ok()); + assert_matches!( + alloc.try_alloc(&[0; 1]), + Err(AllocFailure::Rejected { .. }) + ); + + // check that there is still remaining space left at the end + let mut alloc = alloc.next_state(); + let remaining_space = alloc.block.allotted_space_in_bytes + - alloc.block.occupied_space_in_bytes; + assert_eq!(remaining_space, 5); + + // fill up the remaining space + assert!(alloc.try_alloc(&[0; 5]).is_ok()); + assert_matches!( + alloc.try_alloc(&[0; 1]), + Err(AllocFailure::Rejected { .. }) + ); + } + + // Test that we cannot include encrypted txs in a block + // when the state invariants banish them from inclusion. + #[test] + fn test_encrypted_txs_are_rejected() { + let alloc = BlockSpaceAllocator::init(1234); + let alloc = alloc.next_state(); + let mut alloc = alloc.next_state_without_encrypted_txs(); + assert_matches!( + alloc.try_alloc(&[0; 1]), + Err(AllocFailure::Rejected { .. }) + ); + } + + proptest! { + /// Check if we reject a tx when its respective bin + /// capacity has been reached on a [`BlockSpaceAllocator`]. + #[test] + fn test_reject_tx_on_bin_cap_reached(max in prop::num::u64::ANY) { + proptest_reject_tx_on_bin_cap_reached(max) + } + + /// Check if the sum of all individual bin allotments for a + /// [`BlockSpaceAllocator`] corresponds to the total space ceded + /// by Tendermint. + #[test] + fn test_bin_capacity_eq_provided_space(max in prop::num::u64::ANY) { + proptest_bin_capacity_eq_provided_space(max) + } + + /// Test that dumping txs whose total combined size + /// is less than the bin cap does not fill up the bin. + #[test] + fn test_tx_dump_doesnt_fill_up_bin(args in arb_transactions()) { + proptest_tx_dump_doesnt_fill_up_bin(args) + } + } + + /// Implementation of [`test_reject_tx_on_bin_cap_reached`]. + fn proptest_reject_tx_on_bin_cap_reached( + tendermint_max_block_space_in_bytes: u64, + ) { + let mut bins = + BlockSpaceAllocator::init(tendermint_max_block_space_in_bytes); + + // fill the entire bin of decrypted txs + bins.decrypted_txs.occupied_space_in_bytes = + bins.decrypted_txs.allotted_space_in_bytes; + + // make sure we can't dump any new decrypted txs in the bin + assert_matches!( + bins.try_alloc(b"arbitrary tx bytes"), + Err(AllocFailure::Rejected { .. }) + ); + } + + /// Implementation of [`test_bin_capacity_eq_provided_space`]. + fn proptest_bin_capacity_eq_provided_space( + tendermint_max_block_space_in_bytes: u64, + ) { + let bins = + BlockSpaceAllocator::init(tendermint_max_block_space_in_bytes); + assert_eq!(0, bins.uninitialized_space_in_bytes()); + } + + /// Implementation of [`test_tx_dump_doesnt_fill_up_bin`]. + fn proptest_tx_dump_doesnt_fill_up_bin(args: PropTx) { + let PropTx { + tendermint_max_block_space_in_bytes, + protocol_txs, + encrypted_txs, + decrypted_txs, + } = args; + + // produce new txs until the moment we would have + // filled up the bins. + // + // iterate over the produced txs to make sure we can keep + // dumping new txs without filling up the bins + + let bins = RefCell::new(BlockSpaceAllocator::init( + tendermint_max_block_space_in_bytes, + )); + let decrypted_txs = decrypted_txs.into_iter().take_while(|tx| { + let bin = bins.borrow().decrypted_txs; + let new_size = bin.occupied_space_in_bytes + tx.len() as u64; + new_size < bin.allotted_space_in_bytes + }); + for tx in decrypted_txs { + assert!(bins.borrow_mut().try_alloc(&tx).is_ok()); + } + + let bins = RefCell::new(bins.into_inner().next_state()); + let protocol_txs = protocol_txs.into_iter().take_while(|tx| { + let bin = bins.borrow().protocol_txs; + let new_size = bin.occupied_space_in_bytes + tx.len() as u64; + new_size < bin.allotted_space_in_bytes + }); + for tx in protocol_txs { + assert!(bins.borrow_mut().try_alloc(&tx).is_ok()); + } + + let bins = + RefCell::new(bins.into_inner().next_state_with_encrypted_txs()); + let encrypted_txs = encrypted_txs.into_iter().take_while(|tx| { + let bin = bins.borrow().encrypted_txs; + let new_size = bin.occupied_space_in_bytes + tx.len() as u64; + new_size < bin.allotted_space_in_bytes + }); + for tx in encrypted_txs { + assert!(bins.borrow_mut().try_alloc(&tx).is_ok()); + } + } + + prop_compose! { + /// Generate arbitrarily sized txs of different kinds. + fn arb_transactions() + // create base strategies + ( + (tendermint_max_block_space_in_bytes, protocol_tx_max_bin_size, encrypted_tx_max_bin_size, + decrypted_tx_max_bin_size) in arb_max_bin_sizes(), + ) + // compose strategies + ( + tendermint_max_block_space_in_bytes in Just(tendermint_max_block_space_in_bytes), + protocol_txs in arb_tx_list(protocol_tx_max_bin_size), + encrypted_txs in arb_tx_list(encrypted_tx_max_bin_size), + decrypted_txs in arb_tx_list(decrypted_tx_max_bin_size), + ) + -> PropTx { + PropTx { + tendermint_max_block_space_in_bytes, + protocol_txs, + encrypted_txs, + decrypted_txs, + } + } + } + + /// Return random bin sizes for a [`BlockSpaceAllocator`]. + fn arb_max_bin_sizes() -> impl Strategy + { + const MAX_BLOCK_SIZE_BYTES: u64 = 1000; + (1..=MAX_BLOCK_SIZE_BYTES).prop_map( + |tendermint_max_block_space_in_bytes| { + ( + tendermint_max_block_space_in_bytes, + threshold::ONE_THIRD + .over(tendermint_max_block_space_in_bytes) + as usize, + threshold::ONE_THIRD + .over(tendermint_max_block_space_in_bytes) + as usize, + threshold::ONE_THIRD + .over(tendermint_max_block_space_in_bytes) + as usize, + ) + }, + ) + } + + /// Return a list of txs. + fn arb_tx_list(max_bin_size: usize) -> impl Strategy>> { + const MAX_TX_NUM: usize = 64; + let tx = prop::collection::vec(prop::num::u8::ANY, 0..=max_bin_size); + prop::collection::vec(tx, 0..=MAX_TX_NUM) + } +} diff --git a/apps/src/lib/node/ledger/shell/block_space_alloc/states.rs b/apps/src/lib/node/ledger/shell/block_space_alloc/states.rs new file mode 100644 index 0000000000..e334666d31 --- /dev/null +++ b/apps/src/lib/node/ledger/shell/block_space_alloc/states.rs @@ -0,0 +1,175 @@ +//! All the states of the [`BlockSpaceAllocator`] state machine, +//! over the extent of a Tendermint consensus round +//! block proposal. +//! +//! # States +//! +//! The state machine moves through the following state DAG: +//! +//! 1. [`BuildingDecryptedTxBatch`] - the initial state. In +//! this state, we populate a block with DKG decrypted txs. +//! 2. [`BuildingProtocolTxBatch`] - the second state. In +//! this state, we populate a block with protocol txs. +//! 3. [`BuildingEncryptedTxBatch`] - the third state. In +//! this state, we populate a block with DKG encrypted txs. +//! This state supports two modes of operation, which you can +//! think of as two states diverging from [`BuildingProtocolTxBatch`]: +//! * [`WithoutEncryptedTxs`] - When this mode is active, no encrypted txs are +//! included in a block proposal. +//! * [`WithEncryptedTxs`] - When this mode is active, we are able to include +//! encrypted txs in a block proposal. +//! 4. [`FillingRemainingSpace`] - the fourth and final state. +//! During this phase, we fill all remaining block space with arbitrary +//! protocol transactions that haven't been included in a block, yet. + +mod decrypted_txs; +mod encrypted_txs; +mod protocol_txs; +mod remaining_txs; + +use super::{AllocFailure, BlockSpaceAllocator}; + +/// Convenience wrapper for a [`BlockSpaceAllocator`] state that allocates +/// encrypted transactions. +#[allow(dead_code)] +pub enum EncryptedTxBatchAllocator { + WithEncryptedTxs( + BlockSpaceAllocator>, + ), + WithoutEncryptedTxs( + BlockSpaceAllocator>, + ), +} + +/// The leader of the current Tendermint round is building +/// a new batch of DKG decrypted transactions. +/// +/// For more info, read the module docs of +/// [`crate::node::ledger::shell::prepare_proposal::block_space_alloc::states`]. +pub enum BuildingDecryptedTxBatch {} + +/// The leader of the current Tendermint round is building +/// a new batch of Namada protocol transactions. +/// +/// For more info, read the module docs of +/// [`crate::node::ledger::shell::prepare_proposal::block_space_alloc::states`]. +pub enum BuildingProtocolTxBatch {} + +/// The leader of the current Tendermint round is building +/// a new batch of DKG encrypted transactions. +/// +/// For more info, read the module docs of +/// [`crate::node::ledger::shell::prepare_proposal::block_space_alloc::states`]. +pub struct BuildingEncryptedTxBatch { + /// One of [`WithEncryptedTxs`] and [`WithoutEncryptedTxs`]. + _mode: Mode, +} + +/// The leader of the current Tendermint round is populating +/// all remaining space in a block proposal with arbitrary +/// protocol transactions that haven't been included in the +/// block, yet. +/// +/// For more info, read the module docs of +/// [`crate::node::ledger::shell::prepare_proposal::block_space_alloc::states`]. +pub enum FillingRemainingSpace {} + +/// Allow block proposals to include encrypted txs. +/// +/// For more info, read the module docs of +/// [`crate::node::ledger::shell::prepare_proposal::block_space_alloc::states`]. +pub enum WithEncryptedTxs {} + +/// Prohibit block proposals from including encrypted txs. +/// +/// For more info, read the module docs of +/// [`crate::node::ledger::shell::prepare_proposal::block_space_alloc::states`]. +pub enum WithoutEncryptedTxs {} + +/// Try to allocate a new transaction on a [`BlockSpaceAllocator`] state. +/// +/// For more info, read the module docs of +/// [`crate::node::ledger::shell::prepare_proposal::block_space_alloc::states`]. +pub trait TryAlloc { + /// Try to allocate space for a new transaction. + fn try_alloc(&mut self, tx: &[u8]) -> Result<(), AllocFailure>; +} + +/// Represents a state transition in the [`BlockSpaceAllocator`] state machine. +/// +/// This trait should not be used directly. Instead, consider using one of +/// [`NextState`], [`NextStateWithEncryptedTxs`] or +/// [`NextStateWithoutEncryptedTxs`]. +/// +/// For more info, read the module docs of +/// [`crate::node::ledger::shell::prepare_proposal::block_space_alloc::states`]. +pub trait NextStateImpl { + /// The next state in the [`BlockSpaceAllocator`] state machine. + type Next; + + /// Transition to the next state in the [`BlockSpaceAllocator`] state + /// machine. + fn next_state_impl(self) -> Self::Next; +} + +/// Convenience extension of [`NextStateImpl`], to transition to a new +/// state with encrypted txs in a block. +/// +/// For more info, read the module docs of +/// [`crate::node::ledger::shell::prepare_proposal::block_space_alloc::states`]. +pub trait NextStateWithEncryptedTxs: NextStateImpl { + /// Transition to the next state in the [`BlockSpaceAllocator`] state, + /// ensuring we include encrypted txs in a block. + #[inline] + fn next_state_with_encrypted_txs(self) -> Self::Next + where + Self: Sized, + { + self.next_state_impl() + } +} + +impl NextStateWithEncryptedTxs for S where S: NextStateImpl {} + +/// Convenience extension of [`NextStateImpl`], to transition to a new +/// state without encrypted txs in a block. +/// +/// For more info, read the module docs of +/// [`crate::node::ledger::shell::prepare_proposal::block_space_alloc::states`]. +pub trait NextStateWithoutEncryptedTxs: + NextStateImpl +{ + /// Transition to the next state in the [`BlockSpaceAllocator`] state, + /// ensuring we do not include encrypted txs in a block. + #[inline] + fn next_state_without_encrypted_txs(self) -> Self::Next + where + Self: Sized, + { + self.next_state_impl() + } +} + +impl NextStateWithoutEncryptedTxs for S where + S: NextStateImpl +{ +} + +/// Convenience extension of [`NextStateImpl`], to transition to a new +/// state with a null transition function. +/// +/// For more info, read the module docs of +/// [`crate::node::ledger::shell::prepare_proposal::block_space_alloc::states`]. +pub trait NextState: NextStateImpl { + /// Transition to the next state in the [`BlockSpaceAllocator`] state, + /// using a null transiiton function. + #[inline] + fn next_state(self) -> Self::Next + where + Self: Sized, + { + self.next_state_impl() + } +} + +impl NextState for S where S: NextStateImpl {} diff --git a/apps/src/lib/node/ledger/shell/block_space_alloc/states/decrypted_txs.rs b/apps/src/lib/node/ledger/shell/block_space_alloc/states/decrypted_txs.rs new file mode 100644 index 0000000000..ec49284e19 --- /dev/null +++ b/apps/src/lib/node/ledger/shell/block_space_alloc/states/decrypted_txs.rs @@ -0,0 +1,46 @@ +use std::marker::PhantomData; + +use super::super::{threshold, AllocFailure, BlockSpaceAllocator, TxBin}; +use super::{ + BuildingDecryptedTxBatch, BuildingProtocolTxBatch, NextStateImpl, TryAlloc, +}; + +impl TryAlloc for BlockSpaceAllocator { + #[inline] + fn try_alloc(&mut self, tx: &[u8]) -> Result<(), AllocFailure> { + self.decrypted_txs.try_dump(tx) + } +} + +impl NextStateImpl for BlockSpaceAllocator { + type Next = BlockSpaceAllocator; + + #[inline] + fn next_state_impl(mut self) -> Self::Next { + self.decrypted_txs.shrink_to_fit(); + + // reserve half of the remaining block space for protocol txs. + // using this strategy, we will eventually converge to 1/3 of + // the allotted block space for protocol txs + let remaining_free_space = self.uninitialized_space_in_bytes(); + self.protocol_txs = + TxBin::init_over_ratio(remaining_free_space, threshold::ONE_HALF); + + // cast state + let Self { + block, + protocol_txs, + encrypted_txs, + decrypted_txs, + .. + } = self; + + BlockSpaceAllocator { + _state: PhantomData, + block, + protocol_txs, + encrypted_txs, + decrypted_txs, + } + } +} diff --git a/apps/src/lib/node/ledger/shell/block_space_alloc/states/encrypted_txs.rs b/apps/src/lib/node/ledger/shell/block_space_alloc/states/encrypted_txs.rs new file mode 100644 index 0000000000..019de0a6b3 --- /dev/null +++ b/apps/src/lib/node/ledger/shell/block_space_alloc/states/encrypted_txs.rs @@ -0,0 +1,106 @@ +use std::marker::PhantomData; + +use super::super::{AllocFailure, BlockSpaceAllocator}; +use super::{ + BuildingEncryptedTxBatch, EncryptedTxBatchAllocator, FillingRemainingSpace, + NextStateImpl, TryAlloc, WithEncryptedTxs, WithoutEncryptedTxs, +}; + +impl TryAlloc + for BlockSpaceAllocator> +{ + #[inline] + fn try_alloc(&mut self, tx: &[u8]) -> Result<(), AllocFailure> { + self.encrypted_txs.try_dump(tx) + } +} + +impl NextStateImpl + for BlockSpaceAllocator> +{ + type Next = BlockSpaceAllocator; + + #[inline] + fn next_state_impl(self) -> Self::Next { + next_state(self) + } +} + +impl TryAlloc + for BlockSpaceAllocator> +{ + #[inline] + fn try_alloc(&mut self, _tx: &[u8]) -> Result<(), AllocFailure> { + Err(AllocFailure::Rejected { bin_space_left: 0 }) + } +} + +impl NextStateImpl + for BlockSpaceAllocator> +{ + type Next = BlockSpaceAllocator; + + #[inline] + fn next_state_impl(self) -> Self::Next { + next_state(self) + } +} + +#[inline] +fn next_state( + mut alloc: BlockSpaceAllocator>, +) -> BlockSpaceAllocator { + alloc.encrypted_txs.shrink_to_fit(); + + // reserve space for any remaining txs + alloc.claim_block_space(); + + // cast state + let BlockSpaceAllocator { + block, + protocol_txs, + encrypted_txs, + decrypted_txs, + .. + } = alloc; + + BlockSpaceAllocator { + _state: PhantomData, + block, + protocol_txs, + encrypted_txs, + decrypted_txs, + } +} + +impl TryAlloc for EncryptedTxBatchAllocator { + #[inline] + fn try_alloc(&mut self, tx: &[u8]) -> Result<(), AllocFailure> { + match self { + EncryptedTxBatchAllocator::WithEncryptedTxs(state) => { + state.try_alloc(tx) + } + EncryptedTxBatchAllocator::WithoutEncryptedTxs(state) => { + // NOTE: this operation will cause the allocator to + // run out of memory immediately + state.try_alloc(tx) + } + } + } +} + +impl NextStateImpl for EncryptedTxBatchAllocator { + type Next = BlockSpaceAllocator; + + #[inline] + fn next_state_impl(self) -> Self::Next { + match self { + EncryptedTxBatchAllocator::WithEncryptedTxs(state) => { + state.next_state_impl() + } + EncryptedTxBatchAllocator::WithoutEncryptedTxs(state) => { + state.next_state_impl() + } + } + } +} diff --git a/apps/src/lib/node/ledger/shell/block_space_alloc/states/protocol_txs.rs b/apps/src/lib/node/ledger/shell/block_space_alloc/states/protocol_txs.rs new file mode 100644 index 0000000000..48194047a8 --- /dev/null +++ b/apps/src/lib/node/ledger/shell/block_space_alloc/states/protocol_txs.rs @@ -0,0 +1,82 @@ +use std::marker::PhantomData; + +use super::super::{threshold, AllocFailure, BlockSpaceAllocator, TxBin}; +use super::{ + BuildingEncryptedTxBatch, BuildingProtocolTxBatch, NextStateImpl, TryAlloc, + WithEncryptedTxs, WithoutEncryptedTxs, +}; + +impl TryAlloc for BlockSpaceAllocator { + #[inline] + fn try_alloc(&mut self, tx: &[u8]) -> Result<(), AllocFailure> { + self.protocol_txs.try_dump(tx) + } +} + +impl NextStateImpl + for BlockSpaceAllocator +{ + type Next = BlockSpaceAllocator>; + + #[inline] + fn next_state_impl(mut self) -> Self::Next { + self.protocol_txs.shrink_to_fit(); + + // reserve space for encrypted txs; encrypted txs can use up to + // 1/3 of the max block space; the rest goes to protocol txs, once + // more + let one_third_of_block_space = + threshold::ONE_THIRD.over(self.block.allotted_space_in_bytes); + let remaining_free_space = self.uninitialized_space_in_bytes(); + self.encrypted_txs = TxBin::init(std::cmp::min( + one_third_of_block_space, + remaining_free_space, + )); + + // cast state + let Self { + block, + protocol_txs, + encrypted_txs, + decrypted_txs, + .. + } = self; + + BlockSpaceAllocator { + _state: PhantomData, + block, + protocol_txs, + encrypted_txs, + decrypted_txs, + } + } +} + +impl NextStateImpl + for BlockSpaceAllocator +{ + type Next = + BlockSpaceAllocator>; + + #[inline] + fn next_state_impl(mut self) -> Self::Next { + self.protocol_txs.shrink_to_fit(); + + // cast state + let Self { + block, + protocol_txs, + encrypted_txs, + decrypted_txs, + .. + } = self; + + BlockSpaceAllocator { + _state: PhantomData, + block, + protocol_txs, + encrypted_txs, + decrypted_txs, + } + } +} diff --git a/apps/src/lib/node/ledger/shell/block_space_alloc/states/remaining_txs.rs b/apps/src/lib/node/ledger/shell/block_space_alloc/states/remaining_txs.rs new file mode 100644 index 0000000000..48f3a43df5 --- /dev/null +++ b/apps/src/lib/node/ledger/shell/block_space_alloc/states/remaining_txs.rs @@ -0,0 +1,11 @@ +use super::super::{AllocFailure, BlockSpaceAllocator}; +use super::{FillingRemainingSpace, TryAlloc}; + +impl TryAlloc for BlockSpaceAllocator { + #[inline] + fn try_alloc(&mut self, tx: &[u8]) -> Result<(), AllocFailure> { + // NOTE: tx dispatching is done at at higher level, to prevent + // allocating space for encrypted txs here + self.block.try_dump(tx) + } +} diff --git a/apps/src/lib/node/ledger/shell/mod.rs b/apps/src/lib/node/ledger/shell/mod.rs index 6b4b05b5ad..a74e88b9cc 100644 --- a/apps/src/lib/node/ledger/shell/mod.rs +++ b/apps/src/lib/node/ledger/shell/mod.rs @@ -5,6 +5,7 @@ //! and [`Shell::process_proposal`] must be also reverted //! (unless we can simply overwrite them in the next block). //! More info in . +mod block_space_alloc; mod finalize_block; mod governance; mod init_chain; From 160610e47a9d31e0974a526eb61bc8fe6e53acb3 Mon Sep 17 00:00:00 2001 From: Tiago Carvalho Date: Thu, 22 Dec 2022 16:04:13 +0000 Subject: [PATCH 06/37] Add TODO items --- apps/src/lib/node/ledger/shell/prepare_proposal.rs | 2 ++ apps/src/lib/node/ledger/shell/process_proposal.rs | 1 + 2 files changed, 3 insertions(+) diff --git a/apps/src/lib/node/ledger/shell/prepare_proposal.rs b/apps/src/lib/node/ledger/shell/prepare_proposal.rs index 1783a127fd..44b8ae14e9 100644 --- a/apps/src/lib/node/ledger/shell/prepare_proposal.rs +++ b/apps/src/lib/node/ledger/shell/prepare_proposal.rs @@ -36,6 +36,8 @@ where /// INVARIANT: Any changes applied in this method must be reverted if /// the proposal is rejected (unless we can simply overwrite /// them in the next block). + // TODO: update second paragraph of docstring with block space alloc + // info, and plug block space alloc to PrepareProposal pub fn prepare_proposal( &self, req: RequestPrepareProposal, diff --git a/apps/src/lib/node/ledger/shell/process_proposal.rs b/apps/src/lib/node/ledger/shell/process_proposal.rs index 11deec9e13..c10a75d5af 100644 --- a/apps/src/lib/node/ledger/shell/process_proposal.rs +++ b/apps/src/lib/node/ledger/shell/process_proposal.rs @@ -25,6 +25,7 @@ where /// but we only reject the entire block if the order of the /// included txs violates the order decided upon in the previous /// block. + // TODO: add block space alloc validation logic to ProcessProposal pub fn process_proposal( &self, req: RequestProcessProposal, From 3ee117f70aea5b0e26ad1f425fa1fe583625a065 Mon Sep 17 00:00:00 2001 From: Tiago Carvalho Date: Tue, 3 Jan 2023 09:57:47 +0000 Subject: [PATCH 07/37] Shim PrepareProposal response --- .../node/ledger/shims/abcipp_shim_types.rs | 24 +++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/apps/src/lib/node/ledger/shims/abcipp_shim_types.rs b/apps/src/lib/node/ledger/shims/abcipp_shim_types.rs index cb3145f0e2..d4f4e07d02 100644 --- a/apps/src/lib/node/ledger/shims/abcipp_shim_types.rs +++ b/apps/src/lib/node/ledger/shims/abcipp_shim_types.rs @@ -129,7 +129,7 @@ pub mod shim { InitChain(ResponseInitChain), Info(ResponseInfo), Query(ResponseQuery), - PrepareProposal(ResponsePrepareProposal), + PrepareProposal(response::PrepareProposal), VerifyHeader(response::VerifyHeader), ProcessProposal(response::ProcessProposal), RevertProposal(response::RevertProposal), @@ -176,7 +176,7 @@ pub mod shim { Ok(Resp::ApplySnapshotChunk(inner)) } Response::PrepareProposal(inner) => { - Ok(Resp::PrepareProposal(inner)) + Ok(Resp::PrepareProposal(inner.into())) } #[cfg(feature = "abcipp")] Response::ExtendVote(inner) => Ok(Resp::ExtendVote(inner)), @@ -282,6 +282,26 @@ pub mod shim { types::ConsensusParams, }; + #[derive(Debug, Default)] + pub struct PrepareProposal { + pub txs: Vec, + } + + #[cfg(feature = "abcipp")] + impl From for super::ResponsePrepareProposal { + fn from(_: PrepareProposal) -> Self { + // TODO(namada#198): When abci++ arrives, we should return a + // real response. + Self::default() + } + } + + #[cfg(not(feature = "abcipp"))] + impl From for super::ResponsePrepareProposal { + fn from(resp: PrepareProposal) -> Self { + Self { txs: resp.txs } + } + } #[derive(Debug, Default)] pub struct VerifyHeader; From 513b88621bc3846362799c6b55a6780e70d0ff71 Mon Sep 17 00:00:00 2001 From: Tiago Carvalho Date: Tue, 3 Jan 2023 09:58:08 +0000 Subject: [PATCH 08/37] Remove tx records --- .../lib/node/ledger/shell/prepare_proposal.rs | 190 ++++-------------- 1 file changed, 38 insertions(+), 152 deletions(-) diff --git a/apps/src/lib/node/ledger/shell/prepare_proposal.rs b/apps/src/lib/node/ledger/shell/prepare_proposal.rs index 44b8ae14e9..dcfbef8d21 100644 --- a/apps/src/lib/node/ledger/shell/prepare_proposal.rs +++ b/apps/src/lib/node/ledger/shell/prepare_proposal.rs @@ -9,10 +9,8 @@ use namada::types::transaction::{AffineCurve, DecryptedTx, EllipticCurve}; use super::super::*; use crate::facade::tendermint_proto::abci::RequestPrepareProposal; -#[cfg(feature = "abcipp")] -use crate::facade::tendermint_proto::abci::{tx_record::TxAction, TxRecord}; use crate::node::ledger::shell::{process_tx, ShellMode}; -use crate::node::ledger::shims::abcipp_shim_types::shim::TxBytes; +use crate::node::ledger::shims::abcipp_shim_types::shim::{response, TxBytes}; // TODO: remove this hard-coded value; Tendermint, and thus // Namada uses 20 MiB max block sizes by default; 5 MiB leaves @@ -50,32 +48,6 @@ where // filter in half of the new txs from Tendermint, only keeping // wrappers let mut total_proposal_size = 0; - #[cfg(feature = "abcipp")] - let mut txs: Vec = req - .txs - .into_iter() - .map(|tx_bytes| { - if let Ok(Ok(TxType::Wrapper(_))) = - Tx::try_from(tx_bytes.as_slice()).map(process_tx) - { - record::keep(tx_bytes) - } else { - record::remove(tx_bytes) - } - }) - .take_while(|tx_record| { - let new_size = total_proposal_size + tx_record.tx.len(); - if new_size > HALF_MAX_PROPOSAL_SIZE - || tx_record.action != TxAction::Unmodified as i32 - { - false - } else { - total_proposal_size = new_size; - true - } - }) - .collect(); - #[cfg(not(feature = "abcipp"))] let mut txs: Vec = req .txs .into_iter() @@ -100,28 +72,29 @@ where .collect(); // decrypt the wrapper txs included in the previous block - let decrypted_txs = self.wl_storage.storage.tx_queue.iter().map( - |WrapperTxInQueue { - tx, - #[cfg(not(feature = "mainnet"))] - has_valid_pow, - }| { - Tx::from(match tx.decrypt(privkey) { - Ok(tx) => DecryptedTx::Decrypted { - tx, - #[cfg(not(feature = "mainnet"))] - has_valid_pow: *has_valid_pow, - }, - _ => DecryptedTx::Undecryptable(tx.clone()), - }) - .to_bytes() - }, - ); - #[cfg(feature = "abcipp")] - let mut decrypted_txs: Vec<_> = - decrypted_txs.map(record::add).collect(); - #[cfg(not(feature = "abcipp"))] - let mut decrypted_txs: Vec<_> = decrypted_txs.collect(); + let mut decrypted_txs = self + .wl_storage + .storage + .tx_queue + .iter() + .map( + |WrapperTxInQueue { + tx, + #[cfg(not(feature = "mainnet"))] + has_valid_pow, + }| { + Tx::from(match tx.decrypt(privkey) { + Ok(tx) => DecryptedTx::Decrypted { + tx, + #[cfg(not(feature = "mainnet"))] + has_valid_pow: *has_valid_pow, + }, + _ => DecryptedTx::Undecryptable(tx.clone()), + }) + .to_bytes() + }, + ) + .collect(); txs.append(&mut decrypted_txs); txs @@ -129,50 +102,7 @@ where vec![] }; - #[cfg(feature = "abcipp")] - { - response::PrepareProposal { - tx_records: txs, - ..Default::default() - } - } - #[cfg(not(feature = "abcipp"))] - { - response::PrepareProposal { txs } - } - } -} - -/// Functions for creating the appropriate TxRecord given the -/// numeric code -#[cfg(feature = "abcipp")] -pub(super) mod record { - use super::*; - - /// Keep this transaction in the proposal - pub fn keep(tx: TxBytes) -> TxRecord { - TxRecord { - action: TxAction::Unmodified as i32, - tx, - } - } - - /// A transaction added to the proposal not provided by - /// Tendermint from the mempool - pub fn add(tx: TxBytes) -> TxRecord { - TxRecord { - action: TxAction::Added as i32, - tx, - } - } - - /// Remove this transaction from the set provided - /// by Tendermint from the mempool - pub fn remove(tx: TxBytes) -> TxRecord { - TxRecord { - action: TxAction::Removed as i32, - tx, - } + response::PrepareProposal { txs } } } @@ -200,12 +130,6 @@ mod test_prepare_proposal { max_tx_bytes: 0, ..Default::default() }; - #[cfg(feature = "abcipp")] - assert_eq!( - shell.prepare_proposal(req).tx_records, - vec![record::remove(tx.to_bytes())] - ); - #[cfg(not(feature = "abcipp"))] assert!(shell.prepare_proposal(req).txs.is_empty()); } @@ -248,12 +172,6 @@ mod test_prepare_proposal { max_tx_bytes: 0, ..Default::default() }; - #[cfg(feature = "abcipp")] - assert_eq!( - shell.prepare_proposal(req).tx_records, - vec![record::remove(wrapper)] - ); - #[cfg(not(feature = "abcipp"))] assert!(shell.prepare_proposal(req).txs.is_empty()); } @@ -310,50 +228,18 @@ mod test_prepare_proposal { .iter() .map(|tx| tx.data.clone().expect("Test failed")) .collect(); - #[cfg(feature = "abcipp")] - { - let received: Vec> = shell - .prepare_proposal(req) - .tx_records - .iter() - .filter_map( - |TxRecord { - tx: tx_bytes, - action, - }| { - if *action == (TxAction::Unmodified as i32) - || *action == (TxAction::Added as i32) - { - Some( - Tx::try_from(tx_bytes.as_slice()) - .expect("Test failed") - .data - .expect("Test failed"), - ) - } else { - None - } - }, - ) - .collect(); - // check that the order of the txs is correct - assert_eq!(received, expected_txs); - } - #[cfg(not(feature = "abcipp"))] - { - let received: Vec> = shell - .prepare_proposal(req) - .txs - .into_iter() - .map(|tx_bytes| { - Tx::try_from(tx_bytes.as_slice()) - .expect("Test failed") - .data - .expect("Test failed") - }) - .collect(); - // check that the order of the txs is correct - assert_eq!(received, expected_txs); - } + let received: Vec> = shell + .prepare_proposal(req) + .txs + .into_iter() + .map(|tx_bytes| { + Tx::try_from(tx_bytes.as_slice()) + .expect("Test failed") + .data + .expect("Test failed") + }) + .collect(); + // check that the order of the txs is correct + assert_eq!(received, expected_txs); } } From 4b163f4a1f9997cdf429ed54309e10f586b4de05 Mon Sep 17 00:00:00 2001 From: Tiago Carvalho Date: Tue, 3 Jan 2023 10:26:40 +0000 Subject: [PATCH 09/37] Add compiler hints --- core/src/hints.rs | 44 ++++++++++++++++++++++++++++++++++++++++++++ core/src/lib.rs | 1 + 2 files changed, 45 insertions(+) create mode 100644 core/src/hints.rs diff --git a/core/src/hints.rs b/core/src/hints.rs new file mode 100644 index 0000000000..78d49eeab5 --- /dev/null +++ b/core/src/hints.rs @@ -0,0 +1,44 @@ +//! Compiler hints, to improve the performance of certain operations. + +/// A function that is seldom called. +#[inline] +#[cold] +pub fn cold() {} + +/// A likely path to be taken in an if-expression. +/// +/// # Example +/// +/// ```ignore +/// if likely(frequent_condition()) { +/// // most common path to take +/// } else { +/// // ... +/// } +/// ``` +#[inline] +pub fn likely(b: bool) -> bool { + if !b { + cold() + } + b +} + +/// An unlikely path to be taken in an if-expression. +/// +/// # Example +/// +/// ```ignore +/// if unlikely(rare_condition()) { +/// // ... +/// } else { +/// // most common path to take +/// } +/// ``` +#[inline] +pub fn unlikely(b: bool) -> bool { + if b { + cold() + } + b +} diff --git a/core/src/lib.rs b/core/src/lib.rs index c9bd40084e..44ca420409 100644 --- a/core/src/lib.rs +++ b/core/src/lib.rs @@ -7,6 +7,7 @@ #![deny(rustdoc::private_intra_doc_links)] pub mod bytes; +pub mod hints; pub mod ledger; pub mod proto; pub mod types; From 3fa6aa4ba2cdcdc26d1d0d2342125441b7f99f06 Mon Sep 17 00:00:00 2001 From: Tiago Carvalho Date: Tue, 3 Jan 2023 11:30:32 +0000 Subject: [PATCH 10/37] Add the block space allocator to PrepareProposal --- .../lib/node/ledger/shell/prepare_proposal.rs | 324 +++++++++++++----- 1 file changed, 243 insertions(+), 81 deletions(-) diff --git a/apps/src/lib/node/ledger/shell/prepare_proposal.rs b/apps/src/lib/node/ledger/shell/prepare_proposal.rs index dcfbef8d21..d1782dab77 100644 --- a/apps/src/lib/node/ledger/shell/prepare_proposal.rs +++ b/apps/src/lib/node/ledger/shell/prepare_proposal.rs @@ -1,6 +1,8 @@ //! Implementation of the [`RequestPrepareProposal`] ABCI++ method for the Shell +use namada::core::hints; use namada::ledger::storage::{DBIter, StorageHasher, DB}; +use namada::proof_of_stake::pos_queries::PosQueries; use namada::proto::Tx; use namada::types::internal::WrapperTxInQueue; use namada::types::transaction::tx_types::TxType; @@ -8,17 +10,18 @@ use namada::types::transaction::wrapper::wrapper_tx::PairingEngine; use namada::types::transaction::{AffineCurve, DecryptedTx, EllipticCurve}; use super::super::*; +#[allow(unused_imports)] +use super::block_space_alloc; +use super::block_space_alloc::states::{ + BuildingDecryptedTxBatch, BuildingProtocolTxBatch, + EncryptedTxBatchAllocator, FillingRemainingSpace, NextState, + NextStateWithEncryptedTxs, NextStateWithoutEncryptedTxs, TryAlloc, +}; +use super::block_space_alloc::{AllocFailure, BlockSpaceAllocator}; use crate::facade::tendermint_proto::abci::RequestPrepareProposal; use crate::node::ledger::shell::{process_tx, ShellMode}; use crate::node::ledger::shims::abcipp_shim_types::shim::{response, TxBytes}; -// TODO: remove this hard-coded value; Tendermint, and thus -// Namada uses 20 MiB max block sizes by default; 5 MiB leaves -// plenty of room for header data, evidence and protobuf serialization -// overhead -const MAX_PROPOSAL_SIZE: usize = 5 << 20; -const HALF_MAX_PROPOSAL_SIZE: usize = MAX_PROPOSAL_SIZE / 2; - impl Shell where D: DB + for<'iter> DBIter<'iter> + Sync + 'static, @@ -26,84 +29,246 @@ where { /// Begin a new block. /// - /// We fill half the block space with new wrapper txs given to us - /// from the mempool by tendermint. The rest of the block is filled - /// with decryptions of the wrapper txs from the previously - /// committed block. + /// Block construction is documented in [`block_space_alloc`] + /// and [`block_space_alloc::states`]. /// /// INVARIANT: Any changes applied in this method must be reverted if /// the proposal is rejected (unless we can simply overwrite /// them in the next block). - // TODO: update second paragraph of docstring with block space alloc - // info, and plug block space alloc to PrepareProposal pub fn prepare_proposal( &self, req: RequestPrepareProposal, ) -> response::PrepareProposal { let txs = if let ShellMode::Validator { .. } = self.mode { - // TODO: This should not be hardcoded - let privkey = ::G2Affine::prime_subgroup_generator(); - - // TODO: Craft the Ethereum state update tx - // filter in half of the new txs from Tendermint, only keeping - // wrappers - let mut total_proposal_size = 0; - let mut txs: Vec = req - .txs - .into_iter() - .filter_map(|tx_bytes| { - if let Ok(Ok(TxType::Wrapper(_))) = - Tx::try_from(tx_bytes.as_slice()).map(process_tx) - { - Some(tx_bytes) - } else { - None - } - }) - .take_while(|tx_bytes| { - let new_size = total_proposal_size + tx_bytes.len(); - if new_size > HALF_MAX_PROPOSAL_SIZE { - false - } else { - total_proposal_size = new_size; - true - } - }) - .collect(); + // start counting allotted space for txs + let alloc = BlockSpaceAllocator::from(&self.storage); // decrypt the wrapper txs included in the previous block - let mut decrypted_txs = self - .wl_storage - .storage - .tx_queue - .iter() - .map( - |WrapperTxInQueue { - tx, - #[cfg(not(feature = "mainnet"))] - has_valid_pow, - }| { - Tx::from(match tx.decrypt(privkey) { - Ok(tx) => DecryptedTx::Decrypted { - tx, - #[cfg(not(feature = "mainnet"))] - has_valid_pow: *has_valid_pow, - }, - _ => DecryptedTx::Undecryptable(tx.clone()), - }) - .to_bytes() - }, - ) - .collect(); + let (decrypted_txs, alloc) = self.build_decrypted_txs(alloc); + let mut txs = decrypted_txs; + + // add vote extension protocol txs + let (mut protocol_txs, alloc) = self.build_protocol_txs( + alloc, + #[cfg(feature = "abcipp")] + req.local_last_commit, + #[cfg(not(feature = "abcipp"))] + &req.txs, + ); + txs.append(&mut protocol_txs); + + // add encrypted txs + let (mut encrypted_txs, alloc) = + self.build_encrypted_txs(alloc, &req.txs); + txs.append(&mut encrypted_txs); + + // fill up the remaining block space with + // protocol transactions that haven't been + // selected for inclusion yet, and whose + // size allows them to fit in the free + // space left + let mut remaining_txs = self.build_remaining_batch(alloc, req.txs); + txs.append(&mut remaining_txs); - txs.append(&mut decrypted_txs); txs } else { vec![] }; + tracing::info!( + height = req.height, + num_of_txs = txs.len(), + "Proposing block" + ); + response::PrepareProposal { txs } } + + /// Builds a batch of DKG decrypted transactions. + // NOTE: we won't have frontrunning protection until V2 of the + // Anoma protocol; Namada runs V1, therefore this method is + // essentially a NOOP + // + // sources: + // - https://specs.namada.net/main/releases/v2.html + // - https://github.com/anoma/ferveo + fn build_decrypted_txs( + &self, + mut alloc: BlockSpaceAllocator, + ) -> (Vec, BlockSpaceAllocator) { + // TODO: This should not be hardcoded + let privkey = + ::G2Affine::prime_subgroup_generator(); + + let txs = self + .storage + .tx_queue + .iter() + .map( + |WrapperTxInQueue { + tx, + #[cfg(not(feature = "mainnet"))] + has_valid_pow, + }| { + Tx::from(match tx.decrypt(privkey) { + Ok(tx) => DecryptedTx::Decrypted { + tx, + #[cfg(not(feature = "mainnet"))] + has_valid_pow: *has_valid_pow, + }, + _ => DecryptedTx::Undecryptable(tx.clone()), + }) + .to_bytes() + }, + ) + // TODO: make sure all decrypted txs are accepted + .take_while(|tx_bytes| { + alloc.try_alloc(&tx_bytes[..]).map_or_else( + |status| match status { + AllocFailure::Rejected { bin_space_left } => { + tracing::warn!( + ?tx_bytes, + bin_space_left, + proposal_height = + ?self.storage.get_current_decision_height(), + "Dropping decrypted tx from the current proposal", + ); + false + } + AllocFailure::OverflowsBin { bin_size } => { + tracing::warn!( + ?tx_bytes, + bin_size, + proposal_height = + ?self.storage.get_current_decision_height(), + "Dropping large decrypted tx from the current proposal", + ); + true + } + }, + |()| true, + ) + }) + .collect(); + let alloc = alloc.next_state(); + + (txs, alloc) + } + + /// Builds a batch of protocol transactions. + fn build_protocol_txs( + &self, + alloc: BlockSpaceAllocator, + #[cfg(feature = "abcipp")] _local_last_commit: Option< + ExtendedCommitInfo, + >, + #[cfg(not(feature = "abcipp"))] _txs: &[TxBytes], + ) -> (Vec, EncryptedTxBatchAllocator) { + // no protocol txs are implemented yet + (vec![], self.get_encrypted_txs_allocator(alloc)) + } + + /// Depending on the current block height offset within the epoch, + /// transition state accordingly, from a protocol tx batch allocator + /// to an encrypted tx batch allocator. + /// + /// # How to determine which path to take in the states DAG + /// + /// If we are at the second or third block height offset within an + /// epoch, we do not allow encrypted transactions to be included in + /// a block, therefore we return an allocator wrapped in an + /// [`EncryptedTxBatchAllocator::WithoutEncryptedTxs`] value. + /// Otherwise, we return an allocator wrapped in an + /// [`EncryptedTxBatchAllocator::WithEncryptedTxs`] value. + #[inline] + fn get_encrypted_txs_allocator( + &self, + alloc: BlockSpaceAllocator, + ) -> EncryptedTxBatchAllocator { + let is_2nd_height_off = self.storage.is_deciding_offset_within_epoch(1); + let is_3rd_height_off = self.storage.is_deciding_offset_within_epoch(2); + + if hints::unlikely(is_2nd_height_off || is_3rd_height_off) { + tracing::warn!( + proposal_height = + ?self.storage.get_current_decision_height(), + "No mempool txs are being included in the current proposal" + ); + EncryptedTxBatchAllocator::WithoutEncryptedTxs( + alloc.next_state_without_encrypted_txs(), + ) + } else { + EncryptedTxBatchAllocator::WithEncryptedTxs( + alloc.next_state_with_encrypted_txs(), + ) + } + } + + /// Builds a batch of encrypted transactions, retrieved from + /// Tendermint's mempool. + fn build_encrypted_txs( + &self, + mut alloc: EncryptedTxBatchAllocator, + txs: &[TxBytes], + ) -> (Vec, BlockSpaceAllocator) { + let txs = txs + .iter() + .filter_map(|tx_bytes| { + if let Ok(Ok(TxType::Wrapper(_))) = + Tx::try_from(tx_bytes.as_slice()).map(process_tx) + { + Some(tx_bytes.clone()) + } else { + None + } + }) + .take_while(|tx_bytes| { + alloc.try_alloc(&tx_bytes[..]) + .map_or_else( + |status| match status { + AllocFailure::Rejected { bin_space_left } => { + tracing::debug!( + ?tx_bytes, + bin_space_left, + proposal_height = + ?self.storage.get_current_decision_height(), + "Dropping encrypted tx from the current proposal", + ); + false + } + AllocFailure::OverflowsBin { bin_size } => { + // TODO: handle tx whose size is greater + // than bin size + tracing::warn!( + ?tx_bytes, + bin_size, + proposal_height = + ?self.storage.get_current_decision_height(), + "Dropping large encrypted tx from the current proposal", + ); + true + } + }, + |()| true, + ) + }) + .collect(); + let alloc = alloc.next_state(); + + (txs, alloc) + } + + /// Builds a batch of transactions that can fit in the + /// remaining space of the [`BlockSpaceAllocator`]. + fn build_remaining_batch( + &self, + _alloc: BlockSpaceAllocator, + _txs: Vec, + ) -> Vec { + // since no protocol txs are implemented yet, this state + // doesn't allocate any txs + vec![] + } } #[cfg(test)] @@ -113,21 +278,20 @@ mod test_prepare_proposal { use namada::types::transaction::{Fee, WrapperTx}; use super::*; - use crate::node::ledger::shell::test_utils::{gen_keypair, TestShell}; + use crate::node::ledger::shell::test_utils::{self, gen_keypair}; /// Test that if a tx from the mempool is not a /// WrapperTx type, it is not included in the /// proposed block. #[test] fn test_prepare_proposal_rejects_non_wrapper_tx() { - let (shell, _) = TestShell::new(); + let (shell, _) = test_utils::setup(); let tx = Tx::new( "wasm_code".as_bytes().to_owned(), Some("transaction_data".as_bytes().to_owned()), ); let req = RequestPrepareProposal { txs: vec![tx.to_bytes()], - max_tx_bytes: 0, ..Default::default() }; assert!(shell.prepare_proposal(req).txs.is_empty()); @@ -138,7 +302,7 @@ mod test_prepare_proposal { /// we simply exclude it from the proposal #[test] fn test_error_in_processing_tx() { - let (shell, _) = TestShell::new(); + let (shell, _) = test_utils::setup(); let keypair = gen_keypair(); let tx = Tx::new( "wasm_code".as_bytes().to_owned(), @@ -169,7 +333,6 @@ mod test_prepare_proposal { #[allow(clippy::redundant_clone)] let req = RequestPrepareProposal { txs: vec![wrapper.clone()], - max_tx_bytes: 0, ..Default::default() }; assert!(shell.prepare_proposal(req).txs.is_empty()); @@ -180,14 +343,13 @@ mod test_prepare_proposal { /// corresponding wrappers #[test] fn test_decrypted_txs_in_correct_order() { - let (mut shell, _) = TestShell::new(); + let (mut shell, _) = test_utils::setup(); let keypair = gen_keypair(); let mut expected_wrapper = vec![]; let mut expected_decrypted = vec![]; let mut req = RequestPrepareProposal { txs: vec![], - max_tx_bytes: 0, ..Default::default() }; // create a request with two new wrappers from mempool and @@ -220,15 +382,15 @@ mod test_prepare_proposal { expected_wrapper.push(wrapper.clone()); req.txs.push(wrapper.to_bytes()); } - // we extract the inner data from the txs for testing - // equality since otherwise changes in timestamps would - // fail the test - expected_wrapper.append(&mut expected_decrypted); - let expected_txs: Vec> = expected_wrapper - .iter() - .map(|tx| tx.data.clone().expect("Test failed")) + let expected_txs: Vec = expected_decrypted + .into_iter() + .chain(expected_wrapper.into_iter()) + // we extract the inner data from the txs for testing + // equality since otherwise changes in timestamps would + // fail the test + .map(|tx| tx.data.expect("Test failed")) .collect(); - let received: Vec> = shell + let received: Vec = shell .prepare_proposal(req) .txs .into_iter() From 741a2a3fe4fd4f0a7f8c407afb9a3b5a86e20437 Mon Sep 17 00:00:00 2001 From: Tiago Carvalho Date: Tue, 3 Jan 2023 12:34:12 +0000 Subject: [PATCH 11/37] Remove index-set from apps --- Cargo.lock | 1 - apps/Cargo.toml | 1 - 2 files changed, 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 27d701bf72..a1ba5ac35b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3709,7 +3709,6 @@ dependencies = [ "flate2", "futures 0.3.25", "git2", - "index-set", "itertools", "libc", "libloading", diff --git a/apps/Cargo.toml b/apps/Cargo.toml index b9bd06d2b1..01cc7910f8 100644 --- a/apps/Cargo.toml +++ b/apps/Cargo.toml @@ -97,7 +97,6 @@ eyre = "0.6.5" flate2 = "1.0.22" file-lock = "2.0.2" futures = "0.3" -index-set = {git = "https://github.com/heliaxdev/index-set", tag = "v0.7.1"} itertools = "0.10.1" libc = "0.2.97" libloading = "0.7.2" From 4c0354729f3414458bd75b3d40198005a4f76f09 Mon Sep 17 00:00:00 2001 From: Tiago Carvalho Date: Tue, 3 Jan 2023 13:57:52 +0000 Subject: [PATCH 12/37] Replace Vec with TxBytes --- apps/src/lib/node/ledger/shell/process_proposal.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/apps/src/lib/node/ledger/shell/process_proposal.rs b/apps/src/lib/node/ledger/shell/process_proposal.rs index c10a75d5af..bfa78968bd 100644 --- a/apps/src/lib/node/ledger/shell/process_proposal.rs +++ b/apps/src/lib/node/ledger/shell/process_proposal.rs @@ -7,6 +7,7 @@ use super::*; use crate::facade::tendermint_proto::abci::response_process_proposal::ProposalStatus; use crate::facade::tendermint_proto::abci::RequestProcessProposal; use crate::node::ledger::shims::abcipp_shim_types::shim::response::ProcessProposal; +use crate::node::ledger::shims::abcipp_shim_types::shim::TxBytes; impl Shell where @@ -43,7 +44,7 @@ where } /// Check all the given txs. - pub fn process_txs(&self, txs: &[Vec]) -> Vec { + pub fn process_txs(&self, txs: &[TxBytes]) -> Vec { let mut tx_queue_iter = self.wl_storage.storage.tx_queue.iter(); txs.iter() .map(|tx_bytes| { From 37894f815931eaa966e703ff37f47ec392e22317 Mon Sep 17 00:00:00 2001 From: Tiago Carvalho Date: Tue, 3 Jan 2023 14:26:04 +0000 Subject: [PATCH 13/37] Add allocation error code to ProcessProposal --- apps/src/lib/node/ledger/shell/mod.rs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/apps/src/lib/node/ledger/shell/mod.rs b/apps/src/lib/node/ledger/shell/mod.rs index a74e88b9cc..367efde16b 100644 --- a/apps/src/lib/node/ledger/shell/mod.rs +++ b/apps/src/lib/node/ledger/shell/mod.rs @@ -121,7 +121,7 @@ impl From for TxResult { /// The different error codes that the ledger may /// send back to a client indicating the status /// of their submitted tx -#[derive(Debug, Clone, FromPrimitive, ToPrimitive, PartialEq)] +#[derive(Debug, Copy, Clone, FromPrimitive, ToPrimitive, PartialEq)] pub enum ErrorCodes { Ok = 0, InvalidTx = 1, @@ -130,6 +130,16 @@ pub enum ErrorCodes { InvalidOrder = 4, ExtraTxs = 5, Undecryptable = 6, + AllocationError = 7, /* NOTE: keep these values in sync with + * [`ErrorCodes::is_recoverable`] */ +} + +impl ErrorCodes { + /// Checks if the given [`ErrorCodes`] value is a protocol level error, + /// that can be recovered from at the finalize block stage. + pub const fn is_recoverable(&self) -> bool { + (*self as u32) <= 3 + } } impl From for u32 { From ac83b7998aad7c0d4eb501a1f69b11c1cca7c1f7 Mon Sep 17 00:00:00 2001 From: Tiago Carvalho Date: Tue, 3 Jan 2023 14:26:27 +0000 Subject: [PATCH 14/37] Add block space allocator logic to ProcessProposal --- .../lib/node/ledger/shell/process_proposal.rs | 373 ++++++++++++------ 1 file changed, 261 insertions(+), 112 deletions(-) diff --git a/apps/src/lib/node/ledger/shell/process_proposal.rs b/apps/src/lib/node/ledger/shell/process_proposal.rs index bfa78968bd..44ef8ed682 100644 --- a/apps/src/lib/node/ledger/shell/process_proposal.rs +++ b/apps/src/lib/node/ledger/shell/process_proposal.rs @@ -1,14 +1,57 @@ //! Implementation of the ['VerifyHeader`], [`ProcessProposal`], //! and [`RevertProposal`] ABCI++ methods for the Shell +use data_encoding::HEXUPPER; +use namada::core::hints; +use namada::core::ledger::storage::Storage; +use namada::proof_of_stake::pos_queries::PosQueries; use namada::types::internal::WrapperTxInQueue; use super::*; use crate::facade::tendermint_proto::abci::response_process_proposal::ProposalStatus; use crate::facade::tendermint_proto::abci::RequestProcessProposal; +use crate::node::ledger::shell::block_space_alloc::{ + threshold, AllocFailure, TxBin, +}; use crate::node::ledger::shims::abcipp_shim_types::shim::response::ProcessProposal; use crate::node::ledger::shims::abcipp_shim_types::shim::TxBytes; +/// Validation metadata, to keep track of used resources or +/// transaction numbers, in a block proposal. +#[derive(Default)] +pub struct ValidationMeta { + /// Space utilized by encrypted txs. + pub encrypted_txs_bin: TxBin, + /// Space utilized by all txs. + pub txs_bin: TxBin, + /// Check if the decrypted tx queue has any elements + /// left. + /// + /// This field will only evaluate to true if a block + /// proposer didn't include all decrypted txs in a block. + pub decrypted_queue_has_remaining_txs: bool, +} + +impl From<&Storage> for ValidationMeta +where + D: DB + for<'iter> DBIter<'iter>, + H: StorageHasher, +{ + fn from(storage: &Storage) -> Self { + let max_proposal_bytes = storage.get_max_proposal_bytes().get(); + let encrypted_txs_bin = + TxBin::init_over_ratio(max_proposal_bytes, threshold::ONE_THIRD); + let txs_bin = TxBin::init(max_proposal_bytes); + Self { + #[cfg(feature = "abcipp")] + digests: DigestCounters::default(), + decrypted_queue_has_remaining_txs: false, + encrypted_txs_bin, + txs_bin, + } + } +} + impl Shell where D: DB + for<'iter> DBIter<'iter> + Sync + 'static, @@ -31,26 +74,73 @@ where &self, req: RequestProcessProposal, ) -> ProcessProposal { - let tx_results = self.process_txs(&req.txs); + let (tx_results, metadata) = self.process_txs(&req.txs); + + // Erroneous transactions were detected when processing + // the leader's proposal. We allow txs that do not + // deserialize properly, that have invalid signatures + // and that have invalid wasm code to reach FinalizeBlock. + let invalid_txs = tx_results.iter().any(|res| { + let error = ErrorCodes::from_u32(res.code).expect( + "All error codes returned from process_single_tx are valid", + ); + !error.is_recoverable() + }); + if invalid_txs { + tracing::warn!( + proposer = ?HEXUPPER.encode(&req.proposer_address), + height = req.height, + hash = ?HEXUPPER.encode(&req.hash), + "Found invalid transactions, proposed block will be rejected" + ); + } + + let has_remaining_decrypted_txs = + metadata.decrypted_queue_has_remaining_txs; + if has_remaining_decrypted_txs { + tracing::warn!( + proposer = ?HEXUPPER.encode(&req.proposer_address), + height = req.height, + hash = ?HEXUPPER.encode(&req.hash), + "Not all decrypted txs from the previous height were included in + the proposal, the block will be rejected" + ); + } + + let will_reject_proposal = invalid_txs || has_remaining_decrypted_txs; + + let status = if will_reject_proposal { + ProposalStatus::Reject + } else { + ProposalStatus::Accept + }; ProcessProposal { - status: if tx_results.iter().any(|res| res.code > 3) { - ProposalStatus::Reject as i32 - } else { - ProposalStatus::Accept as i32 - }, + status: status as i32, tx_results, } } /// Check all the given txs. - pub fn process_txs(&self, txs: &[TxBytes]) -> Vec { + pub fn process_txs( + &self, + txs: &[TxBytes], + ) -> (Vec, ValidationMeta) { let mut tx_queue_iter = self.wl_storage.storage.tx_queue.iter(); - txs.iter() + let mut metadata = ValidationMeta::from(&self.storage); + let tx_results = txs + .iter() .map(|tx_bytes| { - self.process_single_tx(tx_bytes, &mut tx_queue_iter) + self.process_single_tx( + tx_bytes, + &mut tx_queue_iter, + &mut metadata, + ) }) - .collect() + .collect(); + metadata.decrypted_queue_has_remaining_txs = + !self.storage.tx_queue.is_empty() && tx_queue_iter.next().is_some(); + (tx_results, metadata) } /// Checks if the Tx can be deserialized from bytes. Checks the fees and @@ -67,6 +157,8 @@ where /// 3: Wasm runtime error /// 4: Invalid order of decrypted txs /// 5. More decrypted txs than expected + /// 6. A transaction could not be decrypted + /// 7. Not enough block space was available for some tx /// /// INVARIANT: Any changes applied in this method must be reverted if the /// proposal is rejected (unless we can simply overwrite them in the @@ -75,128 +167,177 @@ where &self, tx_bytes: &[u8], tx_queue_iter: &mut impl Iterator, + metadata: &mut ValidationMeta, ) -> TxResult { - let tx = match Tx::try_from(tx_bytes) { - Ok(tx) => tx, - Err(_) => { - return TxResult { + // try to allocate space for this tx + if let Err(e) = metadata.txs_bin.try_dump(tx_bytes) { + return TxResult { + code: ErrorCodes::AllocationError.into(), + info: match e { + AllocFailure::Rejected { .. } => { + "No more space left in the block" + } + AllocFailure::OverflowsBin { .. } => { + "The given tx is larger than the max configured \ + proposal size" + } + } + .into(), + }; + } + + let maybe_tx = Tx::try_from(tx_bytes).map_or_else( + |err| { + tracing::debug!( + ?err, + "Couldn't deserialize transaction received during \ + PrepareProposal" + ); + Err(TxResult { code: ErrorCodes::InvalidTx.into(), info: "The submitted transaction was not deserializable" .into(), - }; - } + }) + }, + |tx| { + process_tx(tx).map_err(|err| { + // This occurs if the wrapper / protocol tx signature is + // invalid + TxResult { + code: ErrorCodes::InvalidSig.into(), + info: err.to_string(), + } + }) + }, + ); + let tx = match maybe_tx { + Ok(tx) => tx, + Err(tx_result) => return tx_result, }; + // TODO: This should not be hardcoded let privkey = ::G2Affine::prime_subgroup_generator(); - match process_tx(tx) { - // This occurs if the wrapper / protocol tx signature is invalid - Err(err) => TxResult { - code: ErrorCodes::InvalidSig.into(), - info: err.to_string(), + match tx { + // If it is a raw transaction, we do no further validation + TxType::Raw(_) => TxResult { + code: ErrorCodes::InvalidTx.into(), + info: "Transaction rejected: Non-encrypted transactions are \ + not supported" + .into(), }, - Ok(result) => match result { - // If it is a raw transaction, we do no further validation - TxType::Raw(_) => TxResult { - code: ErrorCodes::InvalidTx.into(), - info: "Transaction rejected: Non-encrypted transactions \ - are not supported" - .into(), - }, - TxType::Protocol(_) => TxResult { - code: ErrorCodes::InvalidTx.into(), - info: "Protocol transactions are a fun new feature that \ - is coming soon to a blockchain near you. Patience." - .into(), + TxType::Protocol(_) => TxResult { + code: ErrorCodes::InvalidTx.into(), + info: "Protocol transactions are a fun new feature that is \ + coming soon to a blockchain near you. Patience." + .into(), + }, + TxType::Decrypted(tx) => match tx_queue_iter.next() { + Some(WrapperTxInQueue { + tx: wrapper, + #[cfg(not(feature = "mainnet"))] + has_valid_pow: _, + }) => { + if wrapper.tx_hash != tx.hash_commitment() { + TxResult { + code: ErrorCodes::InvalidOrder.into(), + info: "Process proposal rejected a decrypted \ + transaction that violated the tx order \ + determined in the previous block" + .into(), + } + } else if verify_decrypted_correctly(&tx, privkey) { + TxResult { + code: ErrorCodes::Ok.into(), + info: "Process Proposal accepted this transaction" + .into(), + } + } else { + TxResult { + code: ErrorCodes::InvalidTx.into(), + info: "The encrypted payload of tx was \ + incorrectly marked as un-decryptable" + .into(), + } + } + } + None => TxResult { + code: ErrorCodes::ExtraTxs.into(), + info: "Received more decrypted txs than expected".into(), }, - TxType::Decrypted(tx) => match tx_queue_iter.next() { - Some(WrapperTxInQueue { - tx: wrapper, - #[cfg(not(feature = "mainnet"))] - has_valid_pow: _, - }) => { - if wrapper.tx_hash != tx.hash_commitment() { - TxResult { - code: ErrorCodes::InvalidOrder.into(), - info: "Process proposal rejected a decrypted \ - transaction that violated the tx order \ - determined in the previous block" - .into(), - } - } else if verify_decrypted_correctly(&tx, privkey) { - TxResult { - code: ErrorCodes::Ok.into(), - info: "Process Proposal accepted this \ - transaction" - .into(), + }, + TxType::Wrapper(tx) => { + // try to allocate space for this encrypted tx + if let Err(e) = metadata.encrypted_txs_bin.try_dump(tx_bytes) { + return TxResult { + code: ErrorCodes::AllocationError.into(), + info: match e { + AllocFailure::Rejected { .. } => { + "No more space left in the block for wrapper \ + txs" } - } else { - TxResult { - code: ErrorCodes::InvalidTx.into(), - info: "The encrypted payload of tx was \ - incorrectly marked as un-decryptable" - .into(), + AllocFailure::OverflowsBin { .. } => { + "The given wrapper tx is larger than 1/3 of \ + the available block space" } } - } - None => TxResult { - code: ErrorCodes::ExtraTxs.into(), - info: "Received more decrypted txs than expected" + .into(), + }; + } + if hints::unlikely(self.encrypted_txs_not_allowed()) { + return TxResult { + code: ErrorCodes::AllocationError.into(), + info: "Wrapper txs not allowed at the current block \ + height" .into(), - }, - }, - TxType::Wrapper(tx) => { - // validate the ciphertext via Ferveo - if !tx.validate_ciphertext() { + }; + } + + // validate the ciphertext via Ferveo + if !tx.validate_ciphertext() { + TxResult { + code: ErrorCodes::InvalidTx.into(), + info: format!( + "The ciphertext of the wrapped tx {} is invalid", + hash_tx(tx_bytes) + ), + } + } else { + // If the public key corresponds to the MASP sentinel + // transaction key, then the fee payer is effectively + // the MASP, otherwise derive + // they payer from public key. + let fee_payer = if tx.pk != masp_tx_key().ref_to() { + tx.fee_payer() + } else { + masp() + }; + // check that the fee payer has sufficient balance + let balance = self.get_balance(&tx.fee.token, &fee_payer); + + // In testnets, tx is allowed to skip fees if it + // includes a valid PoW + #[cfg(not(feature = "mainnet"))] + let has_valid_pow = self.has_valid_pow_solution(&tx); + #[cfg(feature = "mainnet")] + let has_valid_pow = false; + + if has_valid_pow || self.get_wrapper_tx_fees() <= balance { TxResult { - code: ErrorCodes::InvalidTx.into(), - info: format!( - "The ciphertext of the wrapped tx {} is \ - invalid", - hash_tx(tx_bytes) - ), + code: ErrorCodes::Ok.into(), + info: "Process proposal accepted this transaction" + .into(), } } else { - // If the public key corresponds to the MASP sentinel - // transaction key, then the fee payer is effectively - // the MASP, otherwise derive - // they payer from public key. - let fee_payer = if tx.pk != masp_tx_key().ref_to() { - tx.fee_payer() - } else { - masp() - }; - // check that the fee payer has sufficient balance - let balance = - self.get_balance(&tx.fee.token, &fee_payer); - - // In testnets, tx is allowed to skip fees if it - // includes a valid PoW - #[cfg(not(feature = "mainnet"))] - let has_valid_pow = self.has_valid_pow_solution(&tx); - #[cfg(feature = "mainnet")] - let has_valid_pow = false; - - if has_valid_pow - || self.get_wrapper_tx_fees() <= balance - { - TxResult { - code: ErrorCodes::Ok.into(), - info: "Process proposal accepted this \ - transaction" - .into(), - } - } else { - TxResult { - code: ErrorCodes::InvalidTx.into(), - info: "The address given does not have \ - sufficient balance to pay fee" - .into(), - } + TxResult { + code: ErrorCodes::InvalidTx.into(), + info: "The address given does not have sufficient \ + balance to pay fee" + .into(), } } } - }, + } } } @@ -206,6 +347,14 @@ where ) -> shim::response::RevertProposal { Default::default() } + + /// Checks if it is not possible to include encrypted txs at the current + /// block height. + fn encrypted_txs_not_allowed(&self) -> bool { + let is_2nd_height_off = self.storage.is_deciding_offset_within_epoch(1); + let is_3rd_height_off = self.storage.is_deciding_offset_within_epoch(2); + is_2nd_height_off || is_3rd_height_off + } } /// We test the failure cases of [`process_proposal`]. The happy flows From 36dd49c03d4586681d4d3e081c68091b4c20238f Mon Sep 17 00:00:00 2001 From: Tiago Carvalho Date: Tue, 3 Jan 2023 14:26:42 +0000 Subject: [PATCH 15/37] Fix EndBlock shim call on process_txs() --- apps/src/lib/node/ledger/shims/abcipp_shim.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/src/lib/node/ledger/shims/abcipp_shim.rs b/apps/src/lib/node/ledger/shims/abcipp_shim.rs index 74a56a4ddc..8090cb8f28 100644 --- a/apps/src/lib/node/ledger/shims/abcipp_shim.rs +++ b/apps/src/lib/node/ledger/shims/abcipp_shim.rs @@ -137,7 +137,7 @@ impl AbcippShim { } #[cfg(not(feature = "abcipp"))] Req::EndBlock(_) => { - let processing_results = + let (processing_results, _) = self.service.process_txs(&self.delivered_txs); let mut txs = Vec::with_capacity(self.delivered_txs.len()); let mut delivered = vec![]; From 9cf76bee4e7936ce077cad710d88b25c8a3bd5f9 Mon Sep 17 00:00:00 2001 From: Tiago Carvalho Date: Tue, 3 Jan 2023 15:49:16 +0000 Subject: [PATCH 16/37] Fix import errors --- apps/src/lib/node/ledger/shell/prepare_proposal.rs | 2 ++ core/src/ledger/storage/mod.rs | 1 + 2 files changed, 3 insertions(+) diff --git a/apps/src/lib/node/ledger/shell/prepare_proposal.rs b/apps/src/lib/node/ledger/shell/prepare_proposal.rs index d1782dab77..384ccd9eb9 100644 --- a/apps/src/lib/node/ledger/shell/prepare_proposal.rs +++ b/apps/src/lib/node/ledger/shell/prepare_proposal.rs @@ -18,6 +18,8 @@ use super::block_space_alloc::states::{ NextStateWithEncryptedTxs, NextStateWithoutEncryptedTxs, TryAlloc, }; use super::block_space_alloc::{AllocFailure, BlockSpaceAllocator}; +#[cfg(feature = "abcipp")] +use crate::facade::tendermint_proto::abci::ExtendedCommitInfo; use crate::facade::tendermint_proto::abci::RequestPrepareProposal; use crate::node::ledger::shell::{process_tx, ShellMode}; use crate::node::ledger::shims::abcipp_shim_types::shim::{response, TxBytes}; diff --git a/core/src/ledger/storage/mod.rs b/core/src/ledger/storage/mod.rs index e2ac4da235..d81331af01 100644 --- a/core/src/ledger/storage/mod.rs +++ b/core/src/ledger/storage/mod.rs @@ -12,6 +12,7 @@ pub mod write_log; use core::fmt::Debug; +#[cfg(any(feature = "tendermint", feature = "tendermint-abcipp"))] use merkle_tree::StorageBytes; pub use merkle_tree::{ MembershipProof, MerkleTree, MerkleTreeStoresRead, MerkleTreeStoresWrite, From f069e3d4dc028f3e9ad5812fcb34873e0bf300ad Mon Sep 17 00:00:00 2001 From: Tiago Carvalho Date: Tue, 3 Jan 2023 15:50:57 +0000 Subject: [PATCH 17/37] Fix ABCI++ build --- apps/src/lib/node/ledger/shims/abcipp_shim.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/src/lib/node/ledger/shims/abcipp_shim.rs b/apps/src/lib/node/ledger/shims/abcipp_shim.rs index 8090cb8f28..970607f655 100644 --- a/apps/src/lib/node/ledger/shims/abcipp_shim.rs +++ b/apps/src/lib/node/ledger/shims/abcipp_shim.rs @@ -103,7 +103,7 @@ impl AbcippShim { #[cfg(feature = "abcipp")] Req::FinalizeBlock(block) => { let unprocessed_txs = block.txs.clone(); - let processing_results = + let (processing_results, _) = self.service.process_txs(&block.txs); let mut txs = Vec::with_capacity(unprocessed_txs.len()); for (result, tx) in processing_results From a659e4b1f1ff86d432922a61f1a88b62bd9de244 Mon Sep 17 00:00:00 2001 From: Tiago Carvalho Date: Tue, 3 Jan 2023 15:51:30 +0000 Subject: [PATCH 18/37] Fix ProcessProposal unit tests --- .../lib/node/ledger/shell/process_proposal.rs | 92 ++++++++----------- 1 file changed, 40 insertions(+), 52 deletions(-) diff --git a/apps/src/lib/node/ledger/shell/process_proposal.rs b/apps/src/lib/node/ledger/shell/process_proposal.rs index 44ef8ed682..aa0e5d5e39 100644 --- a/apps/src/lib/node/ledger/shell/process_proposal.rs +++ b/apps/src/lib/node/ledger/shell/process_proposal.rs @@ -43,8 +43,6 @@ where TxBin::init_over_ratio(max_proposal_bytes, threshold::ONE_THIRD); let txs_bin = TxBin::init(max_proposal_bytes); Self { - #[cfg(feature = "abcipp")] - digests: DigestCounters::default(), decrypted_queue_has_remaining_txs: false, encrypted_txs_bin, txs_bin, @@ -374,14 +372,14 @@ mod test_process_proposal { use crate::facade::tendermint_proto::abci::RequestInitChain; use crate::facade::tendermint_proto::google::protobuf::Timestamp; use crate::node::ledger::shell::test_utils::{ - gen_keypair, ProcessProposal, TestError, TestShell, + self, gen_keypair, ProcessProposal, TestError, }; /// Test that if a wrapper tx is not signed, it is rejected /// by [`process_proposal`]. #[test] fn test_unsigned_wrapper_rejected() { - let (mut shell, _) = TestShell::new(); + let (mut shell, _) = test_utils::setup(); let keypair = gen_keypair(); let tx = Tx::new( "wasm_code".as_bytes().to_owned(), @@ -429,7 +427,7 @@ mod test_process_proposal { /// Test that a wrapper tx with invalid signature is rejected #[test] fn test_wrapper_bad_signature_rejected() { - let (mut shell, _) = TestShell::new(); + let (mut shell, _) = test_utils::setup(); let keypair = gen_keypair(); let tx = Tx::new( "wasm_code".as_bytes().to_owned(), @@ -514,8 +512,8 @@ mod test_process_proposal { /// non-zero, [`process_proposal`] rejects that tx #[test] fn test_wrapper_unknown_address() { - let (mut shell, _) = TestShell::new(); - let keypair = crate::wallet::defaults::keys().remove(0).1; + let (mut shell, _) = test_utils::setup(); + let keypair = gen_keypair(); let tx = Tx::new( "wasm_code".as_bytes().to_owned(), Some("transaction data".as_bytes().to_owned()), @@ -535,17 +533,19 @@ mod test_process_proposal { ) .sign(&keypair) .expect("Test failed"); - let request = ProcessProposal { - txs: vec![wrapper.to_bytes()], - }; - let response = if let [resp] = shell - .process_proposal(request) - .expect("Test failed") - .as_slice() - { - resp.clone() - } else { - panic!("Test failed") + let response = { + let request = ProcessProposal { + txs: vec![wrapper.to_bytes()], + }; + if let [resp] = shell + .process_proposal(request) + .expect("Test failed") + .as_slice() + { + resp.clone() + } else { + panic!("Test failed") + } }; assert_eq!(response.result.code, u32::from(ErrorCodes::InvalidTx)); assert_eq!( @@ -560,7 +560,7 @@ mod test_process_proposal { /// [`process_proposal`] rejects that tx #[test] fn test_wrapper_insufficient_balance_address() { - let (mut shell, _) = TestShell::new(); + let (mut shell, _) = test_utils::setup(); let keypair = crate::wallet::defaults::daewon_keypair(); // reduce address balance to match the 100 token fee let balance_key = token::balance_key( @@ -619,7 +619,7 @@ mod test_process_proposal { /// validated, [`process_proposal`] rejects it #[test] fn test_decrypted_txs_out_of_order() { - let (mut shell, _) = TestShell::new(); + let (mut shell, _) = test_utils::setup(); let keypair = gen_keypair(); let mut txs = vec![]; for i in 0..3 { @@ -647,38 +647,26 @@ mod test_process_proposal { has_valid_pow: false, }))); } - let req_1 = ProcessProposal { - txs: vec![txs[0].to_bytes()], - }; - let response_1 = if let [resp] = shell - .process_proposal(req_1) - .expect("Test failed") - .as_slice() - { - resp.clone() - } else { - panic!("Test failed") - }; - assert_eq!(response_1.result.code, u32::from(ErrorCodes::Ok)); - - let req_2 = ProcessProposal { - txs: vec![txs[2].to_bytes()], - }; - - let response_2 = if let Err(TestError::RejectProposal(resp)) = - shell.process_proposal(req_2) - { - if let [resp] = resp.as_slice() { - resp.clone() + let response = { + let request = ProcessProposal { + txs: vec![ + txs[0].to_bytes(), + txs[2].to_bytes(), + txs[1].to_bytes(), + ], + }; + if let Err(TestError::RejectProposal(mut resp)) = + shell.process_proposal(request) + { + assert_eq!(resp.len(), 3); + resp.remove(1) } else { panic!("Test failed") } - } else { - panic!("Test failed") }; - assert_eq!(response_2.result.code, u32::from(ErrorCodes::InvalidOrder)); + assert_eq!(response.result.code, u32::from(ErrorCodes::InvalidOrder)); assert_eq!( - response_2.result.info, + response.result.info, String::from( "Process proposal rejected a decrypted transaction that \ violated the tx order determined in the previous block" @@ -690,7 +678,7 @@ mod test_process_proposal { /// is rejected by [`process_proposal`] #[test] fn test_incorrectly_labelled_as_undecryptable() { - let (mut shell, _) = TestShell::new(); + let (mut shell, _) = test_utils::setup(); let keypair = gen_keypair(); let tx = Tx::new( @@ -743,7 +731,7 @@ mod test_process_proposal { /// undecryptable but still accepted #[test] fn test_invalid_hash_commitment() { - let (mut shell, _) = TestShell::new(); + let (mut shell, _) = test_utils::setup(); shell.init_chain(RequestInitChain { time: Some(Timestamp { seconds: 0, @@ -799,7 +787,7 @@ mod test_process_proposal { /// marked undecryptable and the errors handled correctly #[test] fn test_undecryptable() { - let (mut shell, _) = TestShell::new(); + let (mut shell, _) = test_utils::setup(); shell.init_chain(RequestInitChain { time: Some(Timestamp { seconds: 0, @@ -851,7 +839,7 @@ mod test_process_proposal { /// [`process_proposal`] than expected, they are rejected #[test] fn test_too_many_decrypted_txs() { - let (mut shell, _) = TestShell::new(); + let (mut shell, _) = test_utils::setup(); let tx = Tx::new( "wasm_code".as_bytes().to_owned(), @@ -888,7 +876,7 @@ mod test_process_proposal { /// Process Proposal should reject a RawTx, but not panic #[test] fn test_raw_tx_rejected() { - let (mut shell, _) = TestShell::new(); + let (mut shell, _) = test_utils::setup(); let tx = Tx::new( "wasm_code".as_bytes().to_owned(), From ce7e74f7cba28a70cf6bec79152284e3984f8255 Mon Sep 17 00:00:00 2001 From: Tiago Carvalho Date: Thu, 16 Feb 2023 14:28:21 +0000 Subject: [PATCH 19/37] Temporarily disable get_validator_from_tm_address() --- proof_of_stake/src/pos_queries.rs | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/proof_of_stake/src/pos_queries.rs b/proof_of_stake/src/pos_queries.rs index 3171502400..4a20e7afcd 100644 --- a/proof_of_stake/src/pos_queries.rs +++ b/proof_of_stake/src/pos_queries.rs @@ -198,19 +198,20 @@ where fn get_validator_from_tm_address( &self, - tm_address: &[u8], - epoch: Option, + _tm_address: &[u8], + _epoch: Option, ) -> Result
{ - let epoch = epoch.unwrap_or_else(|| self.get_current_epoch().0); - let validator_raw_hash = core::str::from_utf8(tm_address) - .map_err(|_| Error::InvalidTMAddress)?; - self.read_validator_address_raw_hash(validator_raw_hash) - .ok_or_else(|| { - Error::NotValidatorKeyHash( - validator_raw_hash.to_string(), - epoch, - ) - }) + // let epoch = epoch.unwrap_or_else(|| self.get_current_epoch().0); + // let validator_raw_hash = core::str::from_utf8(tm_address) + // .map_err(|_| Error::InvalidTMAddress)?; + // self.read_validator_address_raw_hash(validator_raw_hash) + // .ok_or_else(|| { + // Error::NotValidatorKeyHash( + // validator_raw_hash.to_string(), + // epoch, + // ) + // }) + todo!() } fn is_deciding_offset_within_epoch(&self, height_offset: u64) -> bool { From 57fa752cb3f8a92407d180e29aebbba50bea6397 Mon Sep 17 00:00:00 2001 From: Tiago Carvalho Date: Thu, 16 Feb 2023 15:09:01 +0000 Subject: [PATCH 20/37] Fix PosQueries --- proof_of_stake/src/pos_queries.rs | 41 ++++++++++++++++--------------- 1 file changed, 21 insertions(+), 20 deletions(-) diff --git a/proof_of_stake/src/pos_queries.rs b/proof_of_stake/src/pos_queries.rs index 4a20e7afcd..5e898b9cbb 100644 --- a/proof_of_stake/src/pos_queries.rs +++ b/proof_of_stake/src/pos_queries.rs @@ -1,12 +1,12 @@ //! Storage API for querying data about Proof-of-stake related //! data. This includes validator and epoch related data. -use std::collections::BTreeSet; +use std::collections::HashSet; use borsh::BorshDeserialize; use namada_core::ledger::parameters::storage::get_max_proposal_bytes_key; use namada_core::ledger::parameters::EpochDuration; use namada_core::ledger::storage::types::decode; -use namada_core::ledger::storage::Storage; +use namada_core::ledger::storage::WlStorage; use namada_core::ledger::{storage, storage_api}; use namada_core::tendermint_proto::google::protobuf; use namada_core::tendermint_proto::types::EvidenceParams; @@ -17,7 +17,7 @@ use namada_core::types::{key, token}; use thiserror::Error; use crate::types::WeightedValidator; -use crate::{PosBase, PosParams}; +use crate::{read_consensus_validator_set_addresses_with_stake, PosParams}; /// Errors returned by [`PosQueries`] operations. #[derive(Error, Debug)] @@ -59,7 +59,7 @@ pub trait PosQueries { fn get_active_validators( &self, epoch: Option, - ) -> BTreeSet; + ) -> HashSet; /// Lookup the total voting power for an epoch (defaulting to the /// epoch of the current yet-to-be-committed block). @@ -109,28 +109,27 @@ pub trait PosQueries { fn get_max_proposal_bytes(&self) -> ProposalBytes; } -impl PosQueries for Storage +impl PosQueries for WlStorage where D: storage::DB + for<'iter> storage::DBIter<'iter>, H: storage::StorageHasher, { + #[inline] fn get_active_validators( &self, epoch: Option, - ) -> BTreeSet { - let epoch = epoch.unwrap_or_else(|| self.get_current_epoch().0); - let validator_set = self.read_validator_set(); - validator_set - .get(epoch) - .expect("Validators for an epoch should be known") - .active - .clone() + ) -> HashSet { + let epoch = epoch.unwrap_or_else(|| self.storage.get_current_epoch().0); + // TODO: we can iterate over the database directly, no need to allocate + // here + read_consensus_validator_set_addresses_with_stake(self, epoch) + .expect("Reading the active set of validators shouldn't fail") } fn get_total_voting_power(&self, epoch: Option) -> token::Amount { self.get_active_validators(epoch) .iter() - .map(|validator| validator.bonded_stake) + .map(|validator| u64::from(validator.bonded_stake)) .sum::() .into() } @@ -175,13 +174,14 @@ where address: &Address, epoch: Option, ) -> Result<(token::Amount, key::common::PublicKey)> { - let epoch = epoch.unwrap_or_else(|| self.get_current_epoch().0); + let epoch = epoch.unwrap_or_else(|| self.storage.get_current_epoch().0); self.get_active_validators(Some(epoch)) .into_iter() .find(|validator| address == &validator.address) .map(|validator| { let protocol_pk_key = key::protocol_pk_key(&validator.address); let bytes = self + .storage .read(&protocol_pk_key) .expect("Validator should have public protocol key") .0 @@ -191,7 +191,7 @@ where "Protocol public key in storage should be \ deserializable", ); - (validator.bonded_stake.into(), protocol_pk) + (validator.bonded_stake, protocol_pk) }) .ok_or_else(|| Error::NotValidatorAddress(address.clone(), epoch)) } @@ -222,13 +222,13 @@ where // handle that case // // we can remove this check once that's fixed - if self.get_current_epoch().0 == Epoch(0) { + if self.storage.get_current_epoch().0 == Epoch(0) { let height_offset_within_epoch = BlockHeight(1 + height_offset); return current_decision_height == height_offset_within_epoch; } let fst_heights_of_each_epoch = - self.block.pred_epochs.first_block_heights(); + self.storage.block.pred_epochs.first_block_heights(); fst_heights_of_each_epoch .last() @@ -241,17 +241,18 @@ where #[inline] fn get_epoch(&self, height: BlockHeight) -> Option { - self.block.pred_epochs.get_epoch(height) + self.storage.block.pred_epochs.get_epoch(height) } #[inline] fn get_current_decision_height(&self) -> BlockHeight { - self.last_height + 1 + self.storage.last_height + 1 } fn get_max_proposal_bytes(&self) -> ProposalBytes { let key = get_max_proposal_bytes_key(); let (maybe_value, _gas) = self + .storage .read(&key) .expect("Must be able to read ProposalBytes from storage"); let value = From 0196a7c5374cda092c149d9b51c4df36c6681dcf Mon Sep 17 00:00:00 2001 From: Tiago Carvalho Date: Thu, 16 Feb 2023 15:14:59 +0000 Subject: [PATCH 21/37] Misc 0.14.0 rebase fixes --- .../node/ledger/shell/block_space_alloc.rs | 6 +++--- .../lib/node/ledger/shell/prepare_proposal.rs | 19 +++++++++++-------- .../lib/node/ledger/shell/process_proposal.rs | 17 ++++++++++------- 3 files changed, 24 insertions(+), 18 deletions(-) diff --git a/apps/src/lib/node/ledger/shell/block_space_alloc.rs b/apps/src/lib/node/ledger/shell/block_space_alloc.rs index 5cae6ffd20..e2f774369f 100644 --- a/apps/src/lib/node/ledger/shell/block_space_alloc.rs +++ b/apps/src/lib/node/ledger/shell/block_space_alloc.rs @@ -55,7 +55,7 @@ pub mod states; use std::marker::PhantomData; -use namada::core::ledger::storage::{self, Storage}; +use namada::core::ledger::storage::{self, WlStorage}; use namada::proof_of_stake::pos_queries::PosQueries; #[allow(unused_imports)] @@ -98,14 +98,14 @@ pub struct BlockSpaceAllocator { decrypted_txs: TxBin, } -impl From<&Storage> +impl From<&WlStorage> for BlockSpaceAllocator where D: storage::DB + for<'iter> storage::DBIter<'iter>, H: storage::StorageHasher, { #[inline] - fn from(storage: &Storage) -> Self { + fn from(storage: &WlStorage) -> Self { Self::init(storage.get_max_proposal_bytes().get()) } } diff --git a/apps/src/lib/node/ledger/shell/prepare_proposal.rs b/apps/src/lib/node/ledger/shell/prepare_proposal.rs index 384ccd9eb9..99a201821d 100644 --- a/apps/src/lib/node/ledger/shell/prepare_proposal.rs +++ b/apps/src/lib/node/ledger/shell/prepare_proposal.rs @@ -43,7 +43,7 @@ where ) -> response::PrepareProposal { let txs = if let ShellMode::Validator { .. } = self.mode { // start counting allotted space for txs - let alloc = BlockSpaceAllocator::from(&self.storage); + let alloc = BlockSpaceAllocator::from(&self.wl_storage); // decrypt the wrapper txs included in the previous block let (decrypted_txs, alloc) = self.build_decrypted_txs(alloc); @@ -103,6 +103,7 @@ where ::G2Affine::prime_subgroup_generator(); let txs = self + .wl_storage .storage .tx_queue .iter() @@ -132,7 +133,7 @@ where ?tx_bytes, bin_space_left, proposal_height = - ?self.storage.get_current_decision_height(), + ?self.wl_storage.get_current_decision_height(), "Dropping decrypted tx from the current proposal", ); false @@ -142,7 +143,7 @@ where ?tx_bytes, bin_size, proposal_height = - ?self.storage.get_current_decision_height(), + ?self.wl_storage.get_current_decision_height(), "Dropping large decrypted tx from the current proposal", ); true @@ -187,13 +188,15 @@ where &self, alloc: BlockSpaceAllocator, ) -> EncryptedTxBatchAllocator { - let is_2nd_height_off = self.storage.is_deciding_offset_within_epoch(1); - let is_3rd_height_off = self.storage.is_deciding_offset_within_epoch(2); + let is_2nd_height_off = + self.wl_storage.is_deciding_offset_within_epoch(1); + let is_3rd_height_off = + self.wl_storage.is_deciding_offset_within_epoch(2); if hints::unlikely(is_2nd_height_off || is_3rd_height_off) { tracing::warn!( proposal_height = - ?self.storage.get_current_decision_height(), + ?self.wl_storage.get_current_decision_height(), "No mempool txs are being included in the current proposal" ); EncryptedTxBatchAllocator::WithoutEncryptedTxs( @@ -233,7 +236,7 @@ where ?tx_bytes, bin_space_left, proposal_height = - ?self.storage.get_current_decision_height(), + ?self.wl_storage.get_current_decision_height(), "Dropping encrypted tx from the current proposal", ); false @@ -245,7 +248,7 @@ where ?tx_bytes, bin_size, proposal_height = - ?self.storage.get_current_decision_height(), + ?self.wl_storage.get_current_decision_height(), "Dropping large encrypted tx from the current proposal", ); true diff --git a/apps/src/lib/node/ledger/shell/process_proposal.rs b/apps/src/lib/node/ledger/shell/process_proposal.rs index aa0e5d5e39..f94b23243b 100644 --- a/apps/src/lib/node/ledger/shell/process_proposal.rs +++ b/apps/src/lib/node/ledger/shell/process_proposal.rs @@ -3,7 +3,7 @@ use data_encoding::HEXUPPER; use namada::core::hints; -use namada::core::ledger::storage::Storage; +use namada::core::ledger::storage::WlStorage; use namada::proof_of_stake::pos_queries::PosQueries; use namada::types::internal::WrapperTxInQueue; @@ -32,12 +32,12 @@ pub struct ValidationMeta { pub decrypted_queue_has_remaining_txs: bool, } -impl From<&Storage> for ValidationMeta +impl From<&WlStorage> for ValidationMeta where D: DB + for<'iter> DBIter<'iter>, H: StorageHasher, { - fn from(storage: &Storage) -> Self { + fn from(storage: &WlStorage) -> Self { let max_proposal_bytes = storage.get_max_proposal_bytes().get(); let encrypted_txs_bin = TxBin::init_over_ratio(max_proposal_bytes, threshold::ONE_THIRD); @@ -125,7 +125,7 @@ where txs: &[TxBytes], ) -> (Vec, ValidationMeta) { let mut tx_queue_iter = self.wl_storage.storage.tx_queue.iter(); - let mut metadata = ValidationMeta::from(&self.storage); + let mut metadata = ValidationMeta::from(&self.wl_storage); let tx_results = txs .iter() .map(|tx_bytes| { @@ -137,7 +137,8 @@ where }) .collect(); metadata.decrypted_queue_has_remaining_txs = - !self.storage.tx_queue.is_empty() && tx_queue_iter.next().is_some(); + !self.wl_storage.storage.tx_queue.is_empty() + && tx_queue_iter.next().is_some(); (tx_results, metadata) } @@ -349,8 +350,10 @@ where /// Checks if it is not possible to include encrypted txs at the current /// block height. fn encrypted_txs_not_allowed(&self) -> bool { - let is_2nd_height_off = self.storage.is_deciding_offset_within_epoch(1); - let is_3rd_height_off = self.storage.is_deciding_offset_within_epoch(2); + let is_2nd_height_off = + self.wl_storage.is_deciding_offset_within_epoch(1); + let is_3rd_height_off = + self.wl_storage.is_deciding_offset_within_epoch(2); is_2nd_height_off || is_3rd_height_off } } From f16c2cccb59cb41f788619e5bd10f74a95da2e20 Mon Sep 17 00:00:00 2001 From: Tiago Carvalho Date: Thu, 16 Feb 2023 16:42:59 +0000 Subject: [PATCH 22/37] Remove need to allocate while iterating active validator sets --- proof_of_stake/src/pos_queries.rs | 239 +++++++++++++++++++----------- 1 file changed, 149 insertions(+), 90 deletions(-) diff --git a/proof_of_stake/src/pos_queries.rs b/proof_of_stake/src/pos_queries.rs index 5e898b9cbb..4a4359198e 100644 --- a/proof_of_stake/src/pos_queries.rs +++ b/proof_of_stake/src/pos_queries.rs @@ -1,12 +1,10 @@ //! Storage API for querying data about Proof-of-stake related //! data. This includes validator and epoch related data. -use std::collections::HashSet; - use borsh::BorshDeserialize; use namada_core::ledger::parameters::storage::get_max_proposal_bytes_key; use namada_core::ledger::parameters::EpochDuration; -use namada_core::ledger::storage::types::decode; use namada_core::ledger::storage::WlStorage; +use namada_core::ledger::storage_api::collections::lazy_map::NestedSubKey; use namada_core::ledger::{storage, storage_api}; use namada_core::tendermint_proto::google::protobuf; use namada_core::tendermint_proto::types::EvidenceParams; @@ -16,8 +14,8 @@ use namada_core::types::storage::{BlockHeight, Epoch}; use namada_core::types::{key, token}; use thiserror::Error; -use crate::types::WeightedValidator; -use crate::{read_consensus_validator_set_addresses_with_stake, PosParams}; +use crate::types::{ConsensusValidatorSet, WeightedValidator}; +use crate::{consensus_validator_set_handle, PosParams}; /// Errors returned by [`PosQueries`] operations. #[derive(Error, Debug)] @@ -54,79 +52,75 @@ pub type Result = ::std::result::Result; /// Methods used to query blockchain proof-of-stake related state, /// such as the currently active set of validators. pub trait PosQueries { - /// Get the set of active validators for a given epoch (defaulting to the - /// epoch of the current yet-to-be-committed block). - fn get_active_validators( - &self, - epoch: Option, - ) -> HashSet; + /// The underlying storage type. + type Storage; - /// Lookup the total voting power for an epoch (defaulting to the - /// epoch of the current yet-to-be-committed block). - fn get_total_voting_power(&self, epoch: Option) -> token::Amount; - - /// Simple helper function for the ledger to get balances - /// of the specified token at the specified address. - fn get_balance(&self, token: &Address, owner: &Address) -> token::Amount; - - /// Return evidence parameters. - // TODO: impove this docstring - fn get_evidence_params( - &self, - epoch_duration: &EpochDuration, - pos_params: &PosParams, - ) -> EvidenceParams; - - /// Lookup data about a validator from their address. - fn get_validator_from_address( - &self, - address: &Address, - epoch: Option, - ) -> Result<(token::Amount, key::common::PublicKey)>; - - /// Given a tendermint validator, the address is the hash - /// of the validators public key. We look up the native - /// address from storage using this hash. - // TODO: We may change how this lookup is done, see - // https://github.com/anoma/namada/issues/200 - fn get_validator_from_tm_address( - &self, - tm_address: &[u8], - epoch: Option, - ) -> Result
; + /// Return a handle to [`PosQueries`]. + fn pos_queries(&self) -> PosQueriesHook<'_, Self::Storage>; +} - /// Check if we are at a given [`BlockHeight`] offset, `height_offset`, - /// within the current [`Epoch`]. - fn is_deciding_offset_within_epoch(&self, height_offset: u64) -> bool; +impl PosQueries for WlStorage +where + D: storage::DB + for<'iter> storage::DBIter<'iter>, + H: storage::StorageHasher, +{ + type Storage = Self; - /// Given some [`BlockHeight`], return the corresponding [`Epoch`]. - fn get_epoch(&self, height: BlockHeight) -> Option; + #[inline] + fn pos_queries(&self) -> PosQueriesHook<'_, Self> { + PosQueriesHook { wl_storage: self } + } +} - /// Retrieves the [`BlockHeight`] that is currently being decided. - fn get_current_decision_height(&self) -> BlockHeight; +/// A handle to [`PosQueries`]. +/// +/// This type is a wrapper around a pointer to a +/// [`WlStorage`]. +#[derive(Debug)] +#[repr(transparent)] +pub struct PosQueriesHook<'db, DB> { + wl_storage: &'db DB, +} - /// Retrieve the `max_proposal_bytes` consensus parameter from storage. - fn get_max_proposal_bytes(&self) -> ProposalBytes; +impl<'db, DB> Clone for PosQueriesHook<'db, DB> { + fn clone(&self) -> Self { + Self { + wl_storage: self.wl_storage, + } + } } -impl PosQueries for WlStorage +impl<'db, DB> Copy for PosQueriesHook<'db, DB> {} + +impl<'db, D, H> PosQueriesHook<'db, WlStorage> where D: storage::DB + for<'iter> storage::DBIter<'iter>, H: storage::StorageHasher, { + /// Return a handle to the inner [`WlStorage`]. #[inline] - fn get_active_validators( - &self, + pub fn storage(self) -> &'db WlStorage { + self.wl_storage + } + + /// Get the set of active validators for a given epoch (defaulting to the + /// epoch of the current yet-to-be-committed block). + #[inline] + pub fn get_active_validators( + self, epoch: Option, - ) -> HashSet { - let epoch = epoch.unwrap_or_else(|| self.storage.get_current_epoch().0); - // TODO: we can iterate over the database directly, no need to allocate - // here - read_consensus_validator_set_addresses_with_stake(self, epoch) - .expect("Reading the active set of validators shouldn't fail") + ) -> GetActiveValidators<'db, D, H> { + let epoch = epoch + .unwrap_or_else(|| self.wl_storage.storage.get_current_epoch().0); + GetActiveValidators { + wl_storage: self.wl_storage, + validator_set: consensus_validator_set_handle().at(&epoch), + } } - fn get_total_voting_power(&self, epoch: Option) -> token::Amount { + /// Lookup the total voting power for an epoch (defaulting to the + /// epoch of the current yet-to-be-committed block). + pub fn get_total_voting_power(self, epoch: Option) -> token::Amount { self.get_active_validators(epoch) .iter() .map(|validator| u64::from(validator.bonded_stake)) @@ -134,9 +128,15 @@ where .into() } - fn get_balance(&self, token: &Address, owner: &Address) -> token::Amount { + /// Simple helper function for the ledger to get balances + /// of the specified token at the specified address. + pub fn get_balance( + self, + token: &Address, + owner: &Address, + ) -> token::Amount { let balance = storage_api::StorageRead::read( - self, + self.wl_storage, &token::balance_key(token, owner), ); // Storage read must not fail, but there might be no value, in which @@ -146,8 +146,10 @@ where .unwrap_or_default() } - fn get_evidence_params( - &self, + /// Return evidence parameters. + // TODO: impove this docstring + pub fn get_evidence_params( + self, epoch_duration: &EpochDuration, pos_params: &PosParams, ) -> EvidenceParams { @@ -169,18 +171,22 @@ where } } - fn get_validator_from_address( - &self, + /// Lookup data about a validator from their address. + pub fn get_validator_from_address( + self, address: &Address, epoch: Option, ) -> Result<(token::Amount, key::common::PublicKey)> { - let epoch = epoch.unwrap_or_else(|| self.storage.get_current_epoch().0); + let epoch = epoch + .unwrap_or_else(|| self.wl_storage.storage.get_current_epoch().0); self.get_active_validators(Some(epoch)) - .into_iter() + .iter() .find(|validator| address == &validator.address) .map(|validator| { let protocol_pk_key = key::protocol_pk_key(&validator.address); + // TODO: rewrite this, to use `StorageRead::read` let bytes = self + .wl_storage .storage .read(&protocol_pk_key) .expect("Validator should have public protocol key") @@ -196,8 +202,13 @@ where .ok_or_else(|| Error::NotValidatorAddress(address.clone(), epoch)) } - fn get_validator_from_tm_address( - &self, + /// Given a tendermint validator, the address is the hash + /// of the validators public key. We look up the native + /// address from storage using this hash. + // TODO: We may change how this lookup is done, see + // https://github.com/anoma/namada/issues/200 + pub fn get_validator_from_tm_address( + self, _tm_address: &[u8], _epoch: Option, ) -> Result
{ @@ -214,7 +225,9 @@ where todo!() } - fn is_deciding_offset_within_epoch(&self, height_offset: u64) -> bool { + /// Check if we are at a given [`BlockHeight`] offset, `height_offset`, + /// within the current [`Epoch`]. + pub fn is_deciding_offset_within_epoch(self, height_offset: u64) -> bool { let current_decision_height = self.get_current_decision_height(); // NOTE: the first stored height in `fst_block_heights_of_each_epoch` @@ -222,13 +235,17 @@ where // handle that case // // we can remove this check once that's fixed - if self.storage.get_current_epoch().0 == Epoch(0) { + if self.wl_storage.storage.get_current_epoch().0 == Epoch(0) { let height_offset_within_epoch = BlockHeight(1 + height_offset); return current_decision_height == height_offset_within_epoch; } - let fst_heights_of_each_epoch = - self.storage.block.pred_epochs.first_block_heights(); + let fst_heights_of_each_epoch = self + .wl_storage + .storage + .block + .pred_epochs + .first_block_heights(); fst_heights_of_each_epoch .last() @@ -240,23 +257,65 @@ where } #[inline] - fn get_epoch(&self, height: BlockHeight) -> Option { - self.storage.block.pred_epochs.get_epoch(height) + /// Given some [`BlockHeight`], return the corresponding [`Epoch`]. + pub fn get_epoch(self, height: BlockHeight) -> Option { + self.wl_storage.storage.block.pred_epochs.get_epoch(height) } #[inline] - fn get_current_decision_height(&self) -> BlockHeight { - self.storage.last_height + 1 + /// Retrieves the [`BlockHeight`] that is currently being decided. + pub fn get_current_decision_height(self) -> BlockHeight { + self.wl_storage.storage.last_height + 1 } - fn get_max_proposal_bytes(&self) -> ProposalBytes { - let key = get_max_proposal_bytes_key(); - let (maybe_value, _gas) = self - .storage - .read(&key) - .expect("Must be able to read ProposalBytes from storage"); - let value = - maybe_value.expect("ProposalBytes must be present in storage"); - decode(value).expect("Must be able to decode ProposalBytes in storage") + /// Retrieve the `max_proposal_bytes` consensus parameter from storage. + pub fn get_max_proposal_bytes(self) -> ProposalBytes { + storage_api::StorageRead::read( + self.wl_storage, + &get_max_proposal_bytes_key(), + ) + .expect("Must be able to read ProposalBytes from storage") + .expect("ProposalBytes must be present in storage") + } +} + +/// A handle to the set of active validators in Namada, +/// at some given epoch. +pub struct GetActiveValidators<'db, D, H> +where + D: storage::DB + for<'iter> storage::DBIter<'iter>, + H: storage::StorageHasher, +{ + wl_storage: &'db WlStorage, + validator_set: ConsensusValidatorSet, +} + +impl<'db, D, H> GetActiveValidators<'db, D, H> +where + D: storage::DB + for<'iter> storage::DBIter<'iter>, + H: storage::StorageHasher, +{ + /// Iterate over the set of active validators in Namada, at some given + /// epoch. + pub fn iter<'this: 'db>( + &'this self, + ) -> impl Iterator + 'db { + self.validator_set + .iter(self.wl_storage) + .expect("Must be able to iterate over active validators") + .map(|res| { + let ( + NestedSubKey::Data { + key: bonded_stake, .. + }, + address, + ) = res.expect( + "We should be able to decode validators in storage", + ); + WeightedValidator { + address, + bonded_stake, + } + }) } } From 8e3a7205766db0e1510cb9287cdbfcabda0c7a7d Mon Sep 17 00:00:00 2001 From: Tiago Carvalho Date: Thu, 16 Feb 2023 16:53:01 +0000 Subject: [PATCH 23/37] PosQueries related fixes --- .../node/ledger/shell/block_space_alloc.rs | 2 +- .../lib/node/ledger/shell/prepare_proposal.rs | 20 ++++++++++--------- .../lib/node/ledger/shell/process_proposal.rs | 10 +++++----- 3 files changed, 17 insertions(+), 15 deletions(-) diff --git a/apps/src/lib/node/ledger/shell/block_space_alloc.rs b/apps/src/lib/node/ledger/shell/block_space_alloc.rs index e2f774369f..b27c6c4851 100644 --- a/apps/src/lib/node/ledger/shell/block_space_alloc.rs +++ b/apps/src/lib/node/ledger/shell/block_space_alloc.rs @@ -106,7 +106,7 @@ where { #[inline] fn from(storage: &WlStorage) -> Self { - Self::init(storage.get_max_proposal_bytes().get()) + Self::init(storage.pos_queries().get_max_proposal_bytes().get()) } } diff --git a/apps/src/lib/node/ledger/shell/prepare_proposal.rs b/apps/src/lib/node/ledger/shell/prepare_proposal.rs index 99a201821d..3f1b7c4b2f 100644 --- a/apps/src/lib/node/ledger/shell/prepare_proposal.rs +++ b/apps/src/lib/node/ledger/shell/prepare_proposal.rs @@ -102,6 +102,7 @@ where let privkey = ::G2Affine::prime_subgroup_generator(); + let pos_queries = self.wl_storage.pos_queries(); let txs = self .wl_storage .storage @@ -133,7 +134,7 @@ where ?tx_bytes, bin_space_left, proposal_height = - ?self.wl_storage.get_current_decision_height(), + ?pos_queries.get_current_decision_height(), "Dropping decrypted tx from the current proposal", ); false @@ -143,7 +144,7 @@ where ?tx_bytes, bin_size, proposal_height = - ?self.wl_storage.get_current_decision_height(), + ?pos_queries.get_current_decision_height(), "Dropping large decrypted tx from the current proposal", ); true @@ -188,15 +189,15 @@ where &self, alloc: BlockSpaceAllocator, ) -> EncryptedTxBatchAllocator { - let is_2nd_height_off = - self.wl_storage.is_deciding_offset_within_epoch(1); - let is_3rd_height_off = - self.wl_storage.is_deciding_offset_within_epoch(2); + let pos_queries = self.wl_storage.pos_queries(); + + let is_2nd_height_off = pos_queries.is_deciding_offset_within_epoch(1); + let is_3rd_height_off = pos_queries.is_deciding_offset_within_epoch(2); if hints::unlikely(is_2nd_height_off || is_3rd_height_off) { tracing::warn!( proposal_height = - ?self.wl_storage.get_current_decision_height(), + ?pos_queries.get_current_decision_height(), "No mempool txs are being included in the current proposal" ); EncryptedTxBatchAllocator::WithoutEncryptedTxs( @@ -216,6 +217,7 @@ where mut alloc: EncryptedTxBatchAllocator, txs: &[TxBytes], ) -> (Vec, BlockSpaceAllocator) { + let pos_queries = self.wl_storage.pos_queries(); let txs = txs .iter() .filter_map(|tx_bytes| { @@ -236,7 +238,7 @@ where ?tx_bytes, bin_space_left, proposal_height = - ?self.wl_storage.get_current_decision_height(), + ?pos_queries.get_current_decision_height(), "Dropping encrypted tx from the current proposal", ); false @@ -248,7 +250,7 @@ where ?tx_bytes, bin_size, proposal_height = - ?self.wl_storage.get_current_decision_height(), + ?pos_queries.get_current_decision_height(), "Dropping large encrypted tx from the current proposal", ); true diff --git a/apps/src/lib/node/ledger/shell/process_proposal.rs b/apps/src/lib/node/ledger/shell/process_proposal.rs index f94b23243b..df9da75868 100644 --- a/apps/src/lib/node/ledger/shell/process_proposal.rs +++ b/apps/src/lib/node/ledger/shell/process_proposal.rs @@ -38,7 +38,8 @@ where H: StorageHasher, { fn from(storage: &WlStorage) -> Self { - let max_proposal_bytes = storage.get_max_proposal_bytes().get(); + let max_proposal_bytes = + storage.pos_queries().get_max_proposal_bytes().get(); let encrypted_txs_bin = TxBin::init_over_ratio(max_proposal_bytes, threshold::ONE_THIRD); let txs_bin = TxBin::init(max_proposal_bytes); @@ -350,10 +351,9 @@ where /// Checks if it is not possible to include encrypted txs at the current /// block height. fn encrypted_txs_not_allowed(&self) -> bool { - let is_2nd_height_off = - self.wl_storage.is_deciding_offset_within_epoch(1); - let is_3rd_height_off = - self.wl_storage.is_deciding_offset_within_epoch(2); + let pos_queries = self.wl_storage.pos_queries(); + let is_2nd_height_off = pos_queries.is_deciding_offset_within_epoch(1); + let is_3rd_height_off = pos_queries.is_deciding_offset_within_epoch(2); is_2nd_height_off || is_3rd_height_off } } From 630ea2f7ae9f3b0d0a67e0d9dae70abbbb54fa23 Mon Sep 17 00:00:00 2001 From: Tiago Carvalho Date: Thu, 16 Feb 2023 16:58:55 +0000 Subject: [PATCH 24/37] Rename: GetActiveValidators -> ActiveValidators --- proof_of_stake/src/pos_queries.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/proof_of_stake/src/pos_queries.rs b/proof_of_stake/src/pos_queries.rs index 4a4359198e..7f05c8c02d 100644 --- a/proof_of_stake/src/pos_queries.rs +++ b/proof_of_stake/src/pos_queries.rs @@ -109,10 +109,10 @@ where pub fn get_active_validators( self, epoch: Option, - ) -> GetActiveValidators<'db, D, H> { + ) -> ActiveValidators<'db, D, H> { let epoch = epoch .unwrap_or_else(|| self.wl_storage.storage.get_current_epoch().0); - GetActiveValidators { + ActiveValidators { wl_storage: self.wl_storage, validator_set: consensus_validator_set_handle().at(&epoch), } @@ -281,7 +281,7 @@ where /// A handle to the set of active validators in Namada, /// at some given epoch. -pub struct GetActiveValidators<'db, D, H> +pub struct ActiveValidators<'db, D, H> where D: storage::DB + for<'iter> storage::DBIter<'iter>, H: storage::StorageHasher, @@ -290,7 +290,7 @@ where validator_set: ConsensusValidatorSet, } -impl<'db, D, H> GetActiveValidators<'db, D, H> +impl<'db, D, H> ActiveValidators<'db, D, H> where D: storage::DB + for<'iter> storage::DBIter<'iter>, H: storage::StorageHasher, From 13f195157f822ce0e55af915f591b70de893cf6c Mon Sep 17 00:00:00 2001 From: Tiago Carvalho Date: Fri, 17 Feb 2023 10:26:03 +0000 Subject: [PATCH 25/37] Fix some unit tests --- .../lib/node/ledger/shell/process_proposal.rs | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/apps/src/lib/node/ledger/shell/process_proposal.rs b/apps/src/lib/node/ledger/shell/process_proposal.rs index df9da75868..d509734539 100644 --- a/apps/src/lib/node/ledger/shell/process_proposal.rs +++ b/apps/src/lib/node/ledger/shell/process_proposal.rs @@ -372,8 +372,6 @@ mod test_process_proposal { use namada::types::transaction::{EncryptionKey, Fee, WrapperTx}; use super::*; - use crate::facade::tendermint_proto::abci::RequestInitChain; - use crate::facade::tendermint_proto::google::protobuf::Timestamp; use crate::node::ledger::shell::test_utils::{ self, gen_keypair, ProcessProposal, TestError, }; @@ -735,14 +733,6 @@ mod test_process_proposal { #[test] fn test_invalid_hash_commitment() { let (mut shell, _) = test_utils::setup(); - shell.init_chain(RequestInitChain { - time: Some(Timestamp { - seconds: 0, - nanos: 0, - }), - chain_id: ChainId::default().to_string(), - ..Default::default() - }); let keypair = crate::wallet::defaults::daewon_keypair(); let tx = Tx::new( @@ -791,14 +781,6 @@ mod test_process_proposal { #[test] fn test_undecryptable() { let (mut shell, _) = test_utils::setup(); - shell.init_chain(RequestInitChain { - time: Some(Timestamp { - seconds: 0, - nanos: 0, - }), - chain_id: ChainId::default().to_string(), - ..Default::default() - }); let keypair = crate::wallet::defaults::daewon_keypair(); let pubkey = EncryptionKey::default(); // not valid tx bytes From 9d4478c5f3c47d6fd2335f6ac2a1a892e205dc95 Mon Sep 17 00:00:00 2001 From: Tiago Carvalho Date: Mon, 20 Feb 2023 10:52:21 +0000 Subject: [PATCH 26/37] Fix test_wrapper_insufficient_balance_address() unit test --- apps/src/lib/node/ledger/shell/process_proposal.rs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/apps/src/lib/node/ledger/shell/process_proposal.rs b/apps/src/lib/node/ledger/shell/process_proposal.rs index d509734539..e785360ff2 100644 --- a/apps/src/lib/node/ledger/shell/process_proposal.rs +++ b/apps/src/lib/node/ledger/shell/process_proposal.rs @@ -363,13 +363,14 @@ where #[cfg(test)] mod test_process_proposal { use borsh::BorshDeserialize; + use namada::ledger::parameters::storage::get_wrapper_tx_fees_key; use namada::proto::SignedTxData; use namada::types::hash::Hash; use namada::types::key::*; use namada::types::storage::Epoch; use namada::types::token::Amount; use namada::types::transaction::encrypted::EncryptedTx; - use namada::types::transaction::{EncryptionKey, Fee, WrapperTx}; + use namada::types::transaction::{EncryptionKey, Fee, WrapperTx, MIN_FEE}; use super::*; use crate::node::ledger::shell::test_utils::{ @@ -573,6 +574,14 @@ mod test_process_proposal { .storage .write(&balance_key, Amount::whole(99).try_to_vec().unwrap()) .unwrap(); + shell + .wl_storage + .storage + .write( + &get_wrapper_tx_fees_key(), + token::Amount::whole(MIN_FEE).try_to_vec().unwrap(), + ) + .unwrap(); let tx = Tx::new( "wasm_code".as_bytes().to_owned(), From 457fcfa1888ef06ad056a6679a548c4daf12b3d2 Mon Sep 17 00:00:00 2001 From: Tiago Carvalho Date: Mon, 20 Feb 2023 10:54:16 +0000 Subject: [PATCH 27/37] Fix test_wrapper_unknown_address() unit test --- apps/src/lib/node/ledger/shell/process_proposal.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/apps/src/lib/node/ledger/shell/process_proposal.rs b/apps/src/lib/node/ledger/shell/process_proposal.rs index e785360ff2..be4de9f5f7 100644 --- a/apps/src/lib/node/ledger/shell/process_proposal.rs +++ b/apps/src/lib/node/ledger/shell/process_proposal.rs @@ -515,6 +515,14 @@ mod test_process_proposal { #[test] fn test_wrapper_unknown_address() { let (mut shell, _) = test_utils::setup(); + shell + .wl_storage + .storage + .write( + &get_wrapper_tx_fees_key(), + token::Amount::whole(MIN_FEE).try_to_vec().unwrap(), + ) + .unwrap(); let keypair = gen_keypair(); let tx = Tx::new( "wasm_code".as_bytes().to_owned(), From 7107571be325c91511eea3680a4d3dbcd8c6a2fc Mon Sep 17 00:00:00 2001 From: Tiago Carvalho Date: Mon, 20 Feb 2023 11:25:11 +0000 Subject: [PATCH 28/37] Change docstrs to reflect the new order of txs in the BSA --- apps/src/lib/node/ledger/shell/block_space_alloc.rs | 12 ++++++------ .../node/ledger/shell/block_space_alloc/states.rs | 12 ++++++------ 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/apps/src/lib/node/ledger/shell/block_space_alloc.rs b/apps/src/lib/node/ledger/shell/block_space_alloc.rs index b27c6c4851..3f5801769f 100644 --- a/apps/src/lib/node/ledger/shell/block_space_alloc.rs +++ b/apps/src/lib/node/ledger/shell/block_space_alloc.rs @@ -16,12 +16,12 @@ //! In the current implementation, we allocate space for transactions //! in the following order of preference: //! -//! - First, we allot space for DKG decrypted txs. Decrypted txs take up as much +//! - First, we allot space for DKG encrypted txs. We allow DKG encrypted txs to +//! take up at most 1/3 of the total block space. +//! - Next, we allot space for DKG decrypted txs. Decrypted txs take up as much //! space as needed. We will see, shortly, why in practice this is fine. -//! - Next, we allot space for protocol txs. Protocol txs get half of the +//! - Finally, we allot space for protocol txs. Protocol txs get half of the //! remaining block space allotted to them. -//! - Finally, we allot space for DKG encrypted txs. We allow DKG encrypted txs -//! to take up at most 1/3 of the total block space. //! - If any space remains, we try to fit any leftover protocol txs in the //! block. //! @@ -80,9 +80,9 @@ pub enum AllocFailure { /// /// We keep track of the current space utilized by: /// -/// - Protocol transactions. -/// - DKG decrypted transactions. /// - DKG encrypted transactions. +/// - DKG decrypted transactions. +/// - Protocol transactions. #[derive(Debug, Default)] pub struct BlockSpaceAllocator { /// The current state of the [`BlockSpaceAllocator`] state machine. diff --git a/apps/src/lib/node/ledger/shell/block_space_alloc/states.rs b/apps/src/lib/node/ledger/shell/block_space_alloc/states.rs index e334666d31..1d64b29390 100644 --- a/apps/src/lib/node/ledger/shell/block_space_alloc/states.rs +++ b/apps/src/lib/node/ledger/shell/block_space_alloc/states.rs @@ -6,18 +6,18 @@ //! //! The state machine moves through the following state DAG: //! -//! 1. [`BuildingDecryptedTxBatch`] - the initial state. In -//! this state, we populate a block with DKG decrypted txs. -//! 2. [`BuildingProtocolTxBatch`] - the second state. In -//! this state, we populate a block with protocol txs. -//! 3. [`BuildingEncryptedTxBatch`] - the third state. In +//! 1. [`BuildingEncryptedTxBatch`] - the initial state. In //! this state, we populate a block with DKG encrypted txs. //! This state supports two modes of operation, which you can -//! think of as two states diverging from [`BuildingProtocolTxBatch`]: +//! think of as two sub-states: //! * [`WithoutEncryptedTxs`] - When this mode is active, no encrypted txs are //! included in a block proposal. //! * [`WithEncryptedTxs`] - When this mode is active, we are able to include //! encrypted txs in a block proposal. +//! 2. [`BuildingDecryptedTxBatch`] - the second state. In +//! this state, we populate a block with DKG decrypted txs. +//! 3. [`BuildingProtocolTxBatch`] - the third state. In +//! this state, we populate a block with protocol txs. //! 4. [`FillingRemainingSpace`] - the fourth and final state. //! During this phase, we fill all remaining block space with arbitrary //! protocol transactions that haven't been included in a block, yet. From 97b7aeed0055d44e48efe3fe3cba0ebda64e20a7 Mon Sep 17 00:00:00 2001 From: Tiago Carvalho Date: Mon, 20 Feb 2023 14:01:10 +0000 Subject: [PATCH 29/37] Allocate txs in block space allocator in another order: EDP --- .../node/ledger/shell/block_space_alloc.rs | 36 +-- .../ledger/shell/block_space_alloc/states.rs | 13 -- .../block_space_alloc/states/decrypted_txs.rs | 9 +- .../block_space_alloc/states/encrypted_txs.rs | 22 +- .../block_space_alloc/states/protocol_txs.rs | 77 +------ .../block_space_alloc/states/remaining_txs.rs | 11 - .../lib/node/ledger/shell/prepare_proposal.rs | 218 ++++++++---------- 7 files changed, 122 insertions(+), 264 deletions(-) delete mode 100644 apps/src/lib/node/ledger/shell/block_space_alloc/states/remaining_txs.rs diff --git a/apps/src/lib/node/ledger/shell/block_space_alloc.rs b/apps/src/lib/node/ledger/shell/block_space_alloc.rs index 3f5801769f..2fbcda8705 100644 --- a/apps/src/lib/node/ledger/shell/block_space_alloc.rs +++ b/apps/src/lib/node/ledger/shell/block_space_alloc.rs @@ -22,8 +22,6 @@ //! space as needed. We will see, shortly, why in practice this is fine. //! - Finally, we allot space for protocol txs. Protocol txs get half of the //! remaining block space allotted to them. -//! - If any space remains, we try to fit any leftover protocol txs in the -//! block. //! //! Since at some fixed height `H` decrypted txs only take up as //! much space as the encrypted txs from height `H - 1`, and we @@ -98,8 +96,8 @@ pub struct BlockSpaceAllocator { decrypted_txs: TxBin, } -impl From<&WlStorage> - for BlockSpaceAllocator +impl From<&WlStorage> + for BlockSpaceAllocator> where D: storage::DB + for<'iter> storage::DBIter<'iter>, H: storage::StorageHasher, @@ -110,7 +108,7 @@ where } } -impl BlockSpaceAllocator { +impl BlockSpaceAllocator> { /// Construct a new [`BlockSpaceAllocator`], with an upper bound /// on the max size of all txs in a block defined by Tendermint. #[inline] @@ -120,11 +118,8 @@ impl BlockSpaceAllocator { _state: PhantomData, block: TxBin::init(max), protocol_txs: TxBin::default(), - encrypted_txs: TxBin::default(), - // decrypted txs can use as much space as needed; in practice, - // we'll only need, at most, the amount of space reserved for - // encrypted txs at the prev block height - decrypted_txs: TxBin::init(max), + encrypted_txs: TxBin::init_over_ratio(max, threshold::ONE_THIRD), + decrypted_txs: TxBin::default(), } } } @@ -143,21 +138,6 @@ impl BlockSpaceAllocator { + self.decrypted_txs.allotted_space_in_bytes; self.block.allotted_space_in_bytes - total_bin_space } - - /// Claim all the space used by the [`TxBin`] instances - /// as block space. - #[inline] - fn claim_block_space(&mut self) { - let used_space = self.protocol_txs.occupied_space_in_bytes - + self.encrypted_txs.occupied_space_in_bytes - + self.decrypted_txs.occupied_space_in_bytes; - - self.block.occupied_space_in_bytes = used_space; - - self.decrypted_txs = TxBin::default(); - self.protocol_txs = TxBin::default(); - self.encrypted_txs = TxBin::default(); - } } /// Allotted space for a batch of transactions of the same kind in some @@ -250,12 +230,10 @@ pub mod threshold { /// Divide free space in three. pub const ONE_THIRD: Threshold = Threshold::new(1, 3); - - /// Divide free space in two. - pub const ONE_HALF: Threshold = Threshold::new(1, 2); } -#[cfg(test)] +//#[cfg(test)] +#[cfg(FALSE)] mod tests { use std::cell::RefCell; diff --git a/apps/src/lib/node/ledger/shell/block_space_alloc/states.rs b/apps/src/lib/node/ledger/shell/block_space_alloc/states.rs index 1d64b29390..113842967e 100644 --- a/apps/src/lib/node/ledger/shell/block_space_alloc/states.rs +++ b/apps/src/lib/node/ledger/shell/block_space_alloc/states.rs @@ -18,14 +18,10 @@ //! this state, we populate a block with DKG decrypted txs. //! 3. [`BuildingProtocolTxBatch`] - the third state. In //! this state, we populate a block with protocol txs. -//! 4. [`FillingRemainingSpace`] - the fourth and final state. -//! During this phase, we fill all remaining block space with arbitrary -//! protocol transactions that haven't been included in a block, yet. mod decrypted_txs; mod encrypted_txs; mod protocol_txs; -mod remaining_txs; use super::{AllocFailure, BlockSpaceAllocator}; @@ -65,15 +61,6 @@ pub struct BuildingEncryptedTxBatch { _mode: Mode, } -/// The leader of the current Tendermint round is populating -/// all remaining space in a block proposal with arbitrary -/// protocol transactions that haven't been included in the -/// block, yet. -/// -/// For more info, read the module docs of -/// [`crate::node::ledger::shell::prepare_proposal::block_space_alloc::states`]. -pub enum FillingRemainingSpace {} - /// Allow block proposals to include encrypted txs. /// /// For more info, read the module docs of diff --git a/apps/src/lib/node/ledger/shell/block_space_alloc/states/decrypted_txs.rs b/apps/src/lib/node/ledger/shell/block_space_alloc/states/decrypted_txs.rs index ec49284e19..7fd15671a3 100644 --- a/apps/src/lib/node/ledger/shell/block_space_alloc/states/decrypted_txs.rs +++ b/apps/src/lib/node/ledger/shell/block_space_alloc/states/decrypted_txs.rs @@ -1,6 +1,6 @@ use std::marker::PhantomData; -use super::super::{threshold, AllocFailure, BlockSpaceAllocator, TxBin}; +use super::super::{AllocFailure, BlockSpaceAllocator, TxBin}; use super::{ BuildingDecryptedTxBatch, BuildingProtocolTxBatch, NextStateImpl, TryAlloc, }; @@ -19,12 +19,9 @@ impl NextStateImpl for BlockSpaceAllocator { fn next_state_impl(mut self) -> Self::Next { self.decrypted_txs.shrink_to_fit(); - // reserve half of the remaining block space for protocol txs. - // using this strategy, we will eventually converge to 1/3 of - // the allotted block space for protocol txs + // the remaining space is allocated to protocol txs let remaining_free_space = self.uninitialized_space_in_bytes(); - self.protocol_txs = - TxBin::init_over_ratio(remaining_free_space, threshold::ONE_HALF); + self.protocol_txs = TxBin::init(remaining_free_space); // cast state let Self { diff --git a/apps/src/lib/node/ledger/shell/block_space_alloc/states/encrypted_txs.rs b/apps/src/lib/node/ledger/shell/block_space_alloc/states/encrypted_txs.rs index 019de0a6b3..10b325f616 100644 --- a/apps/src/lib/node/ledger/shell/block_space_alloc/states/encrypted_txs.rs +++ b/apps/src/lib/node/ledger/shell/block_space_alloc/states/encrypted_txs.rs @@ -1,9 +1,10 @@ use std::marker::PhantomData; -use super::super::{AllocFailure, BlockSpaceAllocator}; +use super::super::{AllocFailure, BlockSpaceAllocator, TxBin}; use super::{ - BuildingEncryptedTxBatch, EncryptedTxBatchAllocator, FillingRemainingSpace, - NextStateImpl, TryAlloc, WithEncryptedTxs, WithoutEncryptedTxs, + BuildingDecryptedTxBatch, BuildingEncryptedTxBatch, + EncryptedTxBatchAllocator, NextStateImpl, TryAlloc, WithEncryptedTxs, + WithoutEncryptedTxs, }; impl TryAlloc @@ -18,7 +19,7 @@ impl TryAlloc impl NextStateImpl for BlockSpaceAllocator> { - type Next = BlockSpaceAllocator; + type Next = BlockSpaceAllocator; #[inline] fn next_state_impl(self) -> Self::Next { @@ -38,7 +39,7 @@ impl TryAlloc impl NextStateImpl for BlockSpaceAllocator> { - type Next = BlockSpaceAllocator; + type Next = BlockSpaceAllocator; #[inline] fn next_state_impl(self) -> Self::Next { @@ -49,11 +50,14 @@ impl NextStateImpl #[inline] fn next_state( mut alloc: BlockSpaceAllocator>, -) -> BlockSpaceAllocator { +) -> BlockSpaceAllocator { alloc.encrypted_txs.shrink_to_fit(); - // reserve space for any remaining txs - alloc.claim_block_space(); + // decrypted txs can use as much space as they need - which + // in practice will only be, at most, 1/3 of the block space + // used by encrypted txs at the prev height + let remaining_free_space = alloc.uninitialized_space_in_bytes(); + alloc.protocol_txs = TxBin::init(remaining_free_space); // cast state let BlockSpaceAllocator { @@ -90,7 +94,7 @@ impl TryAlloc for EncryptedTxBatchAllocator { } impl NextStateImpl for EncryptedTxBatchAllocator { - type Next = BlockSpaceAllocator; + type Next = BlockSpaceAllocator; #[inline] fn next_state_impl(self) -> Self::Next { diff --git a/apps/src/lib/node/ledger/shell/block_space_alloc/states/protocol_txs.rs b/apps/src/lib/node/ledger/shell/block_space_alloc/states/protocol_txs.rs index 48194047a8..bc31717ddd 100644 --- a/apps/src/lib/node/ledger/shell/block_space_alloc/states/protocol_txs.rs +++ b/apps/src/lib/node/ledger/shell/block_space_alloc/states/protocol_txs.rs @@ -1,10 +1,5 @@ -use std::marker::PhantomData; - -use super::super::{threshold, AllocFailure, BlockSpaceAllocator, TxBin}; -use super::{ - BuildingEncryptedTxBatch, BuildingProtocolTxBatch, NextStateImpl, TryAlloc, - WithEncryptedTxs, WithoutEncryptedTxs, -}; +use super::super::{AllocFailure, BlockSpaceAllocator}; +use super::{BuildingProtocolTxBatch, TryAlloc}; impl TryAlloc for BlockSpaceAllocator { #[inline] @@ -12,71 +7,3 @@ impl TryAlloc for BlockSpaceAllocator { self.protocol_txs.try_dump(tx) } } - -impl NextStateImpl - for BlockSpaceAllocator -{ - type Next = BlockSpaceAllocator>; - - #[inline] - fn next_state_impl(mut self) -> Self::Next { - self.protocol_txs.shrink_to_fit(); - - // reserve space for encrypted txs; encrypted txs can use up to - // 1/3 of the max block space; the rest goes to protocol txs, once - // more - let one_third_of_block_space = - threshold::ONE_THIRD.over(self.block.allotted_space_in_bytes); - let remaining_free_space = self.uninitialized_space_in_bytes(); - self.encrypted_txs = TxBin::init(std::cmp::min( - one_third_of_block_space, - remaining_free_space, - )); - - // cast state - let Self { - block, - protocol_txs, - encrypted_txs, - decrypted_txs, - .. - } = self; - - BlockSpaceAllocator { - _state: PhantomData, - block, - protocol_txs, - encrypted_txs, - decrypted_txs, - } - } -} - -impl NextStateImpl - for BlockSpaceAllocator -{ - type Next = - BlockSpaceAllocator>; - - #[inline] - fn next_state_impl(mut self) -> Self::Next { - self.protocol_txs.shrink_to_fit(); - - // cast state - let Self { - block, - protocol_txs, - encrypted_txs, - decrypted_txs, - .. - } = self; - - BlockSpaceAllocator { - _state: PhantomData, - block, - protocol_txs, - encrypted_txs, - decrypted_txs, - } - } -} diff --git a/apps/src/lib/node/ledger/shell/block_space_alloc/states/remaining_txs.rs b/apps/src/lib/node/ledger/shell/block_space_alloc/states/remaining_txs.rs deleted file mode 100644 index 48f3a43df5..0000000000 --- a/apps/src/lib/node/ledger/shell/block_space_alloc/states/remaining_txs.rs +++ /dev/null @@ -1,11 +0,0 @@ -use super::super::{AllocFailure, BlockSpaceAllocator}; -use super::{FillingRemainingSpace, TryAlloc}; - -impl TryAlloc for BlockSpaceAllocator { - #[inline] - fn try_alloc(&mut self, tx: &[u8]) -> Result<(), AllocFailure> { - // NOTE: tx dispatching is done at at higher level, to prevent - // allocating space for encrypted txs here - self.block.try_dump(tx) - } -} diff --git a/apps/src/lib/node/ledger/shell/prepare_proposal.rs b/apps/src/lib/node/ledger/shell/prepare_proposal.rs index 3f1b7c4b2f..d9a3635ce7 100644 --- a/apps/src/lib/node/ledger/shell/prepare_proposal.rs +++ b/apps/src/lib/node/ledger/shell/prepare_proposal.rs @@ -14,8 +14,7 @@ use super::super::*; use super::block_space_alloc; use super::block_space_alloc::states::{ BuildingDecryptedTxBatch, BuildingProtocolTxBatch, - EncryptedTxBatchAllocator, FillingRemainingSpace, NextState, - NextStateWithEncryptedTxs, NextStateWithoutEncryptedTxs, TryAlloc, + EncryptedTxBatchAllocator, NextState, TryAlloc, }; use super::block_space_alloc::{AllocFailure, BlockSpaceAllocator}; #[cfg(feature = "abcipp")] @@ -43,14 +42,19 @@ where ) -> response::PrepareProposal { let txs = if let ShellMode::Validator { .. } = self.mode { // start counting allotted space for txs - let alloc = BlockSpaceAllocator::from(&self.wl_storage); + let alloc = self.get_encrypted_txs_allocator(); + + // add encrypted txs + let (encrypted_txs, alloc) = + self.build_encrypted_txs(alloc, &req.txs); + let mut txs = encrypted_txs; // decrypt the wrapper txs included in the previous block - let (decrypted_txs, alloc) = self.build_decrypted_txs(alloc); - let mut txs = decrypted_txs; + let (mut decrypted_txs, alloc) = self.build_decrypted_txs(alloc); + txs.append(&mut decrypted_txs); // add vote extension protocol txs - let (mut protocol_txs, alloc) = self.build_protocol_txs( + let mut protocol_txs = self.build_protocol_txs( alloc, #[cfg(feature = "abcipp")] req.local_last_commit, @@ -59,19 +63,6 @@ where ); txs.append(&mut protocol_txs); - // add encrypted txs - let (mut encrypted_txs, alloc) = - self.build_encrypted_txs(alloc, &req.txs); - txs.append(&mut encrypted_txs); - - // fill up the remaining block space with - // protocol transactions that haven't been - // selected for inclusion yet, and whose - // size allows them to fit in the free - // space left - let mut remaining_txs = self.build_remaining_batch(alloc, req.txs); - txs.append(&mut remaining_txs); - txs } else { vec![] @@ -86,95 +77,9 @@ where response::PrepareProposal { txs } } - /// Builds a batch of DKG decrypted transactions. - // NOTE: we won't have frontrunning protection until V2 of the - // Anoma protocol; Namada runs V1, therefore this method is - // essentially a NOOP - // - // sources: - // - https://specs.namada.net/main/releases/v2.html - // - https://github.com/anoma/ferveo - fn build_decrypted_txs( - &self, - mut alloc: BlockSpaceAllocator, - ) -> (Vec, BlockSpaceAllocator) { - // TODO: This should not be hardcoded - let privkey = - ::G2Affine::prime_subgroup_generator(); - - let pos_queries = self.wl_storage.pos_queries(); - let txs = self - .wl_storage - .storage - .tx_queue - .iter() - .map( - |WrapperTxInQueue { - tx, - #[cfg(not(feature = "mainnet"))] - has_valid_pow, - }| { - Tx::from(match tx.decrypt(privkey) { - Ok(tx) => DecryptedTx::Decrypted { - tx, - #[cfg(not(feature = "mainnet"))] - has_valid_pow: *has_valid_pow, - }, - _ => DecryptedTx::Undecryptable(tx.clone()), - }) - .to_bytes() - }, - ) - // TODO: make sure all decrypted txs are accepted - .take_while(|tx_bytes| { - alloc.try_alloc(&tx_bytes[..]).map_or_else( - |status| match status { - AllocFailure::Rejected { bin_space_left } => { - tracing::warn!( - ?tx_bytes, - bin_space_left, - proposal_height = - ?pos_queries.get_current_decision_height(), - "Dropping decrypted tx from the current proposal", - ); - false - } - AllocFailure::OverflowsBin { bin_size } => { - tracing::warn!( - ?tx_bytes, - bin_size, - proposal_height = - ?pos_queries.get_current_decision_height(), - "Dropping large decrypted tx from the current proposal", - ); - true - } - }, - |()| true, - ) - }) - .collect(); - let alloc = alloc.next_state(); - - (txs, alloc) - } - - /// Builds a batch of protocol transactions. - fn build_protocol_txs( - &self, - alloc: BlockSpaceAllocator, - #[cfg(feature = "abcipp")] _local_last_commit: Option< - ExtendedCommitInfo, - >, - #[cfg(not(feature = "abcipp"))] _txs: &[TxBytes], - ) -> (Vec, EncryptedTxBatchAllocator) { - // no protocol txs are implemented yet - (vec![], self.get_encrypted_txs_allocator(alloc)) - } - /// Depending on the current block height offset within the epoch, - /// transition state accordingly, from a protocol tx batch allocator - /// to an encrypted tx batch allocator. + /// transition state accordingly, return a block space allocator + /// with or without encrypted txs. /// /// # How to determine which path to take in the states DAG /// @@ -185,10 +90,7 @@ where /// Otherwise, we return an allocator wrapped in an /// [`EncryptedTxBatchAllocator::WithEncryptedTxs`] value. #[inline] - fn get_encrypted_txs_allocator( - &self, - alloc: BlockSpaceAllocator, - ) -> EncryptedTxBatchAllocator { + fn get_encrypted_txs_allocator(&self) -> EncryptedTxBatchAllocator { let pos_queries = self.wl_storage.pos_queries(); let is_2nd_height_off = pos_queries.is_deciding_offset_within_epoch(1); @@ -201,11 +103,11 @@ where "No mempool txs are being included in the current proposal" ); EncryptedTxBatchAllocator::WithoutEncryptedTxs( - alloc.next_state_without_encrypted_txs(), + (&self.wl_storage).into(), ) } else { EncryptedTxBatchAllocator::WithEncryptedTxs( - alloc.next_state_with_encrypted_txs(), + (&self.wl_storage).into(), ) } } @@ -216,7 +118,7 @@ where &self, mut alloc: EncryptedTxBatchAllocator, txs: &[TxBytes], - ) -> (Vec, BlockSpaceAllocator) { + ) -> (Vec, BlockSpaceAllocator) { let pos_queries = self.wl_storage.pos_queries(); let txs = txs .iter() @@ -265,15 +167,89 @@ where (txs, alloc) } - /// Builds a batch of transactions that can fit in the - /// remaining space of the [`BlockSpaceAllocator`]. - fn build_remaining_batch( + /// Builds a batch of DKG decrypted transactions. + // NOTE: we won't have frontrunning protection until V2 of the + // Anoma protocol; Namada runs V1, therefore this method is + // essentially a NOOP + // + // sources: + // - https://specs.namada.net/main/releases/v2.html + // - https://github.com/anoma/ferveo + fn build_decrypted_txs( &self, - _alloc: BlockSpaceAllocator, - _txs: Vec, + mut alloc: BlockSpaceAllocator, + ) -> (Vec, BlockSpaceAllocator) { + // TODO: This should not be hardcoded + let privkey = + ::G2Affine::prime_subgroup_generator(); + + let pos_queries = self.wl_storage.pos_queries(); + let txs = self + .wl_storage + .storage + .tx_queue + .iter() + .map( + |WrapperTxInQueue { + tx, + #[cfg(not(feature = "mainnet"))] + has_valid_pow, + }| { + Tx::from(match tx.decrypt(privkey) { + Ok(tx) => DecryptedTx::Decrypted { + tx, + #[cfg(not(feature = "mainnet"))] + has_valid_pow: *has_valid_pow, + }, + _ => DecryptedTx::Undecryptable(tx.clone()), + }) + .to_bytes() + }, + ) + // TODO: make sure all decrypted txs are accepted + .take_while(|tx_bytes| { + alloc.try_alloc(&tx_bytes[..]).map_or_else( + |status| match status { + AllocFailure::Rejected { bin_space_left } => { + tracing::warn!( + ?tx_bytes, + bin_space_left, + proposal_height = + ?pos_queries.get_current_decision_height(), + "Dropping decrypted tx from the current proposal", + ); + false + } + AllocFailure::OverflowsBin { bin_size } => { + tracing::warn!( + ?tx_bytes, + bin_size, + proposal_height = + ?pos_queries.get_current_decision_height(), + "Dropping large decrypted tx from the current proposal", + ); + true + } + }, + |()| true, + ) + }) + .collect(); + let alloc = alloc.next_state(); + + (txs, alloc) + } + + /// Builds a batch of protocol transactions. + fn build_protocol_txs( + &self, + _alloc: BlockSpaceAllocator, + #[cfg(feature = "abcipp")] _local_last_commit: Option< + ExtendedCommitInfo, + >, + #[cfg(not(feature = "abcipp"))] _txs: &[TxBytes], ) -> Vec { - // since no protocol txs are implemented yet, this state - // doesn't allocate any txs + // no protocol txs are implemented yet vec![] } } From 38602088e687c8355e8e87e396821809698e5e20 Mon Sep 17 00:00:00 2001 From: Tiago Carvalho Date: Mon, 20 Feb 2023 16:48:44 +0000 Subject: [PATCH 30/37] Fix BSA unit tests --- .../node/ledger/shell/block_space_alloc.rs | 133 ++++++++---------- .../ledger/shell/block_space_alloc/states.rs | 43 ------ .../block_space_alloc/states/encrypted_txs.rs | 2 +- .../lib/node/ledger/shell/prepare_proposal.rs | 4 +- 4 files changed, 63 insertions(+), 119 deletions(-) diff --git a/apps/src/lib/node/ledger/shell/block_space_alloc.rs b/apps/src/lib/node/ledger/shell/block_space_alloc.rs index 2fbcda8705..0a11ba10e1 100644 --- a/apps/src/lib/node/ledger/shell/block_space_alloc.rs +++ b/apps/src/lib/node/ledger/shell/block_space_alloc.rs @@ -232,8 +232,7 @@ pub mod threshold { pub const ONE_THIRD: Threshold = Threshold::new(1, 3); } -//#[cfg(test)] -#[cfg(FALSE)] +#[cfg(test)] mod tests { use std::cell::RefCell; @@ -241,12 +240,22 @@ mod tests { use proptest::prelude::*; use super::states::{ - NextState, NextStateWithEncryptedTxs, NextStateWithoutEncryptedTxs, - TryAlloc, + BuildingEncryptedTxBatch, NextState, TryAlloc, WithEncryptedTxs, + WithoutEncryptedTxs, }; use super::*; use crate::node::ledger::shims::abcipp_shim_types::shim::TxBytes; + /// Convenience alias for a block space allocator at a state with encrypted + /// txs. + type BsaWrapperTxs = + BlockSpaceAllocator>; + + /// Convenience alias for a block space allocator at a state without + /// encrypted txs. + type BsaNoWrapperTxs = + BlockSpaceAllocator>; + /// Proptest generated txs. #[derive(Debug)] struct PropTx { @@ -263,57 +272,41 @@ mod tests { fn test_txs_are_evenly_split_across_block() { const BLOCK_SIZE: u64 = 60; - // reserve block space for decrypted txs - let mut alloc = BlockSpaceAllocator::init(BLOCK_SIZE); + // reserve block space for encrypted txs + let mut alloc = BsaWrapperTxs::init(BLOCK_SIZE); - // assume we got ~1/3 encrypted txs at the prev block + // allocate ~1/3 of the block space to encrypted txs assert!(alloc.try_alloc(&[0; 18]).is_ok()); - // reserve block space for protocol txs + // reserve block space for decrypted txs let mut alloc = alloc.next_state(); - // the space we allotted to decrypted txs was shrunk to + // the space we allotted to encrypted txs was shrunk to // the total space we actually used up - assert_eq!(alloc.decrypted_txs.allotted_space_in_bytes, 18); + assert_eq!(alloc.encrypted_txs.allotted_space_in_bytes, 18); - // check that the allotted space for protocol txs is correct - assert_eq!(21, (BLOCK_SIZE - 18) / 2); - assert_eq!(alloc.protocol_txs.allotted_space_in_bytes, 21); + // check that the allotted space for decrypted txs is correct + assert_eq!( + alloc.decrypted_txs.allotted_space_in_bytes, + BLOCK_SIZE - 18 + ); - // fill up the block space with protocol txs + // add about ~1/3 worth of decrypted txs assert!(alloc.try_alloc(&[0; 17]).is_ok()); - assert_matches!( - alloc.try_alloc(&[0; (21 - 17) + 1]), - Err(AllocFailure::Rejected { .. }) - ); - // reserve block space for encrypted txs - let mut alloc = alloc.next_state_with_encrypted_txs(); + // reserve block space for protocol txs + let mut alloc = alloc.next_state(); // check that space was shrunk - assert_eq!(alloc.protocol_txs.allotted_space_in_bytes, 17); - - // check that we reserve at most 1/3 of the block space to - // encrypted txs - assert_eq!(25, BLOCK_SIZE - 17 - 18); - assert_eq!(20, BLOCK_SIZE / 3); - assert_eq!(alloc.encrypted_txs.allotted_space_in_bytes, 20); - - // fill up the block space with encrypted txs - assert!(alloc.try_alloc(&[0; 20]).is_ok()); - assert_matches!( - alloc.try_alloc(&[0; 1]), - Err(AllocFailure::Rejected { .. }) + assert_eq!( + alloc.protocol_txs.allotted_space_in_bytes, + BLOCK_SIZE - (18 + 17) ); - // check that there is still remaining space left at the end - let mut alloc = alloc.next_state(); - let remaining_space = alloc.block.allotted_space_in_bytes - - alloc.block.occupied_space_in_bytes; - assert_eq!(remaining_space, 5); + // add protocol txs to the block space allocator + assert!(alloc.try_alloc(&[0; 25]).is_ok()); - // fill up the remaining space - assert!(alloc.try_alloc(&[0; 5]).is_ok()); + // the block should be full at this point assert_matches!( alloc.try_alloc(&[0; 1]), Err(AllocFailure::Rejected { .. }) @@ -324,9 +317,7 @@ mod tests { // when the state invariants banish them from inclusion. #[test] fn test_encrypted_txs_are_rejected() { - let alloc = BlockSpaceAllocator::init(1234); - let alloc = alloc.next_state(); - let mut alloc = alloc.next_state_without_encrypted_txs(); + let mut alloc = BsaNoWrapperTxs::init(1234); assert_matches!( alloc.try_alloc(&[0; 1]), Err(AllocFailure::Rejected { .. }) @@ -341,12 +332,11 @@ mod tests { proptest_reject_tx_on_bin_cap_reached(max) } - /// Check if the sum of all individual bin allotments for a - /// [`BlockSpaceAllocator`] corresponds to the total space ceded - /// by Tendermint. + /// Check if the initial bin capcity of the [`BlockSpaceAllocator`] + /// is correct. #[test] - fn test_bin_capacity_eq_provided_space(max in prop::num::u64::ANY) { - proptest_bin_capacity_eq_provided_space(max) + fn test_initial_bin_capacity(max in prop::num::u64::ANY) { + proptest_initial_bin_capacity(max) } /// Test that dumping txs whose total combined size @@ -361,27 +351,25 @@ mod tests { fn proptest_reject_tx_on_bin_cap_reached( tendermint_max_block_space_in_bytes: u64, ) { - let mut bins = - BlockSpaceAllocator::init(tendermint_max_block_space_in_bytes); + let mut bins = BsaWrapperTxs::init(tendermint_max_block_space_in_bytes); - // fill the entire bin of decrypted txs - bins.decrypted_txs.occupied_space_in_bytes = - bins.decrypted_txs.allotted_space_in_bytes; + // fill the entire bin of encrypted txs + bins.encrypted_txs.occupied_space_in_bytes = + bins.encrypted_txs.allotted_space_in_bytes; - // make sure we can't dump any new decrypted txs in the bin + // make sure we can't dump any new encrypted txs in the bin assert_matches!( bins.try_alloc(b"arbitrary tx bytes"), Err(AllocFailure::Rejected { .. }) ); } - /// Implementation of [`test_bin_capacity_eq_provided_space`]. - fn proptest_bin_capacity_eq_provided_space( - tendermint_max_block_space_in_bytes: u64, - ) { - let bins = - BlockSpaceAllocator::init(tendermint_max_block_space_in_bytes); - assert_eq!(0, bins.uninitialized_space_in_bytes()); + /// Implementation of [`test_initial_bin_capacity`]. + fn proptest_initial_bin_capacity(tendermint_max_block_space_in_bytes: u64) { + let bins = BsaWrapperTxs::init(tendermint_max_block_space_in_bytes); + let expected = tendermint_max_block_space_in_bytes + - threshold::ONE_THIRD.over(tendermint_max_block_space_in_bytes); + assert_eq!(expected, bins.uninitialized_space_in_bytes()); } /// Implementation of [`test_tx_dump_doesnt_fill_up_bin`]. @@ -399,36 +387,35 @@ mod tests { // iterate over the produced txs to make sure we can keep // dumping new txs without filling up the bins - let bins = RefCell::new(BlockSpaceAllocator::init( + let bins = RefCell::new(BsaWrapperTxs::init( tendermint_max_block_space_in_bytes, )); - let decrypted_txs = decrypted_txs.into_iter().take_while(|tx| { - let bin = bins.borrow().decrypted_txs; + let encrypted_txs = encrypted_txs.into_iter().take_while(|tx| { + let bin = bins.borrow().encrypted_txs; let new_size = bin.occupied_space_in_bytes + tx.len() as u64; new_size < bin.allotted_space_in_bytes }); - for tx in decrypted_txs { + for tx in encrypted_txs { assert!(bins.borrow_mut().try_alloc(&tx).is_ok()); } let bins = RefCell::new(bins.into_inner().next_state()); - let protocol_txs = protocol_txs.into_iter().take_while(|tx| { - let bin = bins.borrow().protocol_txs; + let decrypted_txs = decrypted_txs.into_iter().take_while(|tx| { + let bin = bins.borrow().decrypted_txs; let new_size = bin.occupied_space_in_bytes + tx.len() as u64; new_size < bin.allotted_space_in_bytes }); - for tx in protocol_txs { + for tx in decrypted_txs { assert!(bins.borrow_mut().try_alloc(&tx).is_ok()); } - let bins = - RefCell::new(bins.into_inner().next_state_with_encrypted_txs()); - let encrypted_txs = encrypted_txs.into_iter().take_while(|tx| { - let bin = bins.borrow().encrypted_txs; + let bins = RefCell::new(bins.into_inner().next_state()); + let protocol_txs = protocol_txs.into_iter().take_while(|tx| { + let bin = bins.borrow().protocol_txs; let new_size = bin.occupied_space_in_bytes + tx.len() as u64; new_size < bin.allotted_space_in_bytes }); - for tx in encrypted_txs { + for tx in protocol_txs { assert!(bins.borrow_mut().try_alloc(&tx).is_ok()); } } diff --git a/apps/src/lib/node/ledger/shell/block_space_alloc/states.rs b/apps/src/lib/node/ledger/shell/block_space_alloc/states.rs index 113842967e..18712998e3 100644 --- a/apps/src/lib/node/ledger/shell/block_space_alloc/states.rs +++ b/apps/src/lib/node/ledger/shell/block_space_alloc/states.rs @@ -99,49 +99,6 @@ pub trait NextStateImpl { fn next_state_impl(self) -> Self::Next; } -/// Convenience extension of [`NextStateImpl`], to transition to a new -/// state with encrypted txs in a block. -/// -/// For more info, read the module docs of -/// [`crate::node::ledger::shell::prepare_proposal::block_space_alloc::states`]. -pub trait NextStateWithEncryptedTxs: NextStateImpl { - /// Transition to the next state in the [`BlockSpaceAllocator`] state, - /// ensuring we include encrypted txs in a block. - #[inline] - fn next_state_with_encrypted_txs(self) -> Self::Next - where - Self: Sized, - { - self.next_state_impl() - } -} - -impl NextStateWithEncryptedTxs for S where S: NextStateImpl {} - -/// Convenience extension of [`NextStateImpl`], to transition to a new -/// state without encrypted txs in a block. -/// -/// For more info, read the module docs of -/// [`crate::node::ledger::shell::prepare_proposal::block_space_alloc::states`]. -pub trait NextStateWithoutEncryptedTxs: - NextStateImpl -{ - /// Transition to the next state in the [`BlockSpaceAllocator`] state, - /// ensuring we do not include encrypted txs in a block. - #[inline] - fn next_state_without_encrypted_txs(self) -> Self::Next - where - Self: Sized, - { - self.next_state_impl() - } -} - -impl NextStateWithoutEncryptedTxs for S where - S: NextStateImpl -{ -} - /// Convenience extension of [`NextStateImpl`], to transition to a new /// state with a null transition function. /// diff --git a/apps/src/lib/node/ledger/shell/block_space_alloc/states/encrypted_txs.rs b/apps/src/lib/node/ledger/shell/block_space_alloc/states/encrypted_txs.rs index 10b325f616..f5fb2447ff 100644 --- a/apps/src/lib/node/ledger/shell/block_space_alloc/states/encrypted_txs.rs +++ b/apps/src/lib/node/ledger/shell/block_space_alloc/states/encrypted_txs.rs @@ -57,7 +57,7 @@ fn next_state( // in practice will only be, at most, 1/3 of the block space // used by encrypted txs at the prev height let remaining_free_space = alloc.uninitialized_space_in_bytes(); - alloc.protocol_txs = TxBin::init(remaining_free_space); + alloc.decrypted_txs = TxBin::init(remaining_free_space); // cast state let BlockSpaceAllocator { diff --git a/apps/src/lib/node/ledger/shell/prepare_proposal.rs b/apps/src/lib/node/ledger/shell/prepare_proposal.rs index d9a3635ce7..5ec71f4004 100644 --- a/apps/src/lib/node/ledger/shell/prepare_proposal.rs +++ b/apps/src/lib/node/ledger/shell/prepare_proposal.rs @@ -365,9 +365,9 @@ mod test_prepare_proposal { expected_wrapper.push(wrapper.clone()); req.txs.push(wrapper.to_bytes()); } - let expected_txs: Vec = expected_decrypted + let expected_txs: Vec = expected_wrapper .into_iter() - .chain(expected_wrapper.into_iter()) + .chain(expected_decrypted.into_iter()) // we extract the inner data from the txs for testing // equality since otherwise changes in timestamps would // fail the test From c8854f7e22360b52da7f7696f46a315c73d6959d Mon Sep 17 00:00:00 2001 From: Tiago Carvalho Date: Tue, 21 Feb 2023 11:36:15 +0000 Subject: [PATCH 31/37] Log the length of the excluded tx instead of its payload --- apps/src/lib/node/ledger/shell/prepare_proposal.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/apps/src/lib/node/ledger/shell/prepare_proposal.rs b/apps/src/lib/node/ledger/shell/prepare_proposal.rs index 5ec71f4004..cf73517eb9 100644 --- a/apps/src/lib/node/ledger/shell/prepare_proposal.rs +++ b/apps/src/lib/node/ledger/shell/prepare_proposal.rs @@ -137,7 +137,7 @@ where |status| match status { AllocFailure::Rejected { bin_space_left } => { tracing::debug!( - ?tx_bytes, + tx_bytes_len = tx_bytes.len(), bin_space_left, proposal_height = ?pos_queries.get_current_decision_height(), @@ -149,7 +149,7 @@ where // TODO: handle tx whose size is greater // than bin size tracing::warn!( - ?tx_bytes, + tx_bytes_len = tx_bytes.len(), bin_size, proposal_height = ?pos_queries.get_current_decision_height(), @@ -212,7 +212,7 @@ where |status| match status { AllocFailure::Rejected { bin_space_left } => { tracing::warn!( - ?tx_bytes, + tx_bytes_len = tx_bytes.len(), bin_space_left, proposal_height = ?pos_queries.get_current_decision_height(), @@ -222,7 +222,7 @@ where } AllocFailure::OverflowsBin { bin_size } => { tracing::warn!( - ?tx_bytes, + tx_bytes_len = tx_bytes.len(), bin_size, proposal_height = ?pos_queries.get_current_decision_height(), From 5dee51f32f8c8b43acc2c0508b0f359749306b1c Mon Sep 17 00:00:00 2001 From: Tiago Carvalho Date: Tue, 21 Feb 2023 11:38:37 +0000 Subject: [PATCH 32/37] Increase minimum epoch length in e2e tests --- tests/src/e2e/ledger_tests.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/src/e2e/ledger_tests.rs b/tests/src/e2e/ledger_tests.rs index 9437103546..396dddce62 100644 --- a/tests/src/e2e/ledger_tests.rs +++ b/tests/src/e2e/ledger_tests.rs @@ -1787,7 +1787,7 @@ fn pos_bonds() -> Result<()> { let test = setup::network( |genesis| { let parameters = ParametersConfig { - min_num_of_blocks: 2, + min_num_of_blocks: 4, max_expected_time_per_block: 1, epochs_per_year: 31_536_000, ..genesis.parameters @@ -1991,7 +1991,7 @@ fn pos_init_validator() -> Result<()> { let test = setup::network( |genesis| { let parameters = ParametersConfig { - min_num_of_blocks: 2, + min_num_of_blocks: 4, epochs_per_year: 31_536_000, max_expected_time_per_block: 1, ..genesis.parameters @@ -2263,7 +2263,7 @@ fn proposal_submission() -> Result<()> { let parameters = ParametersConfig { epochs_per_year: epochs_per_year_from_min_duration(1), max_proposal_bytes: Default::default(), - min_num_of_blocks: 1, + min_num_of_blocks: 4, max_expected_time_per_block: 1, vp_whitelist: Some(get_all_wasms_hashes( &working_dir, From 0c517bc410e439a7e63e9e02eb28bb72235e0c4c Mon Sep 17 00:00:00 2001 From: Tiago Carvalho Date: Tue, 21 Feb 2023 13:30:14 +0000 Subject: [PATCH 33/37] Fix pos_bonds() e2e test --- tests/src/e2e/ledger_tests.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/src/e2e/ledger_tests.rs b/tests/src/e2e/ledger_tests.rs index 396dddce62..d739a627a5 100644 --- a/tests/src/e2e/ledger_tests.rs +++ b/tests/src/e2e/ledger_tests.rs @@ -1787,7 +1787,7 @@ fn pos_bonds() -> Result<()> { let test = setup::network( |genesis| { let parameters = ParametersConfig { - min_num_of_blocks: 4, + min_num_of_blocks: 6, max_expected_time_per_block: 1, epochs_per_year: 31_536_000, ..genesis.parameters @@ -1920,7 +1920,7 @@ fn pos_bonds() -> Result<()> { epoch, delegation_withdrawable_epoch ); let start = Instant::now(); - let loop_timeout = Duration::new(20, 0); + let loop_timeout = Duration::new(60, 0); loop { if Instant::now().duration_since(start) > loop_timeout { panic!( From 1da044e8e7f456684623c8fc266c28e9316854a1 Mon Sep 17 00:00:00 2001 From: Tiago Carvalho Date: Thu, 23 Feb 2023 15:58:12 +0000 Subject: [PATCH 34/37] Check if no decrypted txs are proposed before wrapper txs --- .../lib/node/ledger/shell/process_proposal.rs | 79 +++++++++++-------- 1 file changed, 48 insertions(+), 31 deletions(-) diff --git a/apps/src/lib/node/ledger/shell/process_proposal.rs b/apps/src/lib/node/ledger/shell/process_proposal.rs index be4de9f5f7..90c626d1b2 100644 --- a/apps/src/lib/node/ledger/shell/process_proposal.rs +++ b/apps/src/lib/node/ledger/shell/process_proposal.rs @@ -30,6 +30,8 @@ pub struct ValidationMeta { /// This field will only evaluate to true if a block /// proposer didn't include all decrypted txs in a block. pub decrypted_queue_has_remaining_txs: bool, + /// Check if a block has decrypted txs. + pub has_decrypted_txs: bool, } impl From<&WlStorage> for ValidationMeta @@ -45,6 +47,7 @@ where let txs_bin = TxBin::init(max_proposal_bytes); Self { decrypted_queue_has_remaining_txs: false, + has_decrypted_txs: false, encrypted_txs_bin, txs_bin, } @@ -232,41 +235,55 @@ where coming soon to a blockchain near you. Patience." .into(), }, - TxType::Decrypted(tx) => match tx_queue_iter.next() { - Some(WrapperTxInQueue { - tx: wrapper, - #[cfg(not(feature = "mainnet"))] - has_valid_pow: _, - }) => { - if wrapper.tx_hash != tx.hash_commitment() { - TxResult { - code: ErrorCodes::InvalidOrder.into(), - info: "Process proposal rejected a decrypted \ - transaction that violated the tx order \ - determined in the previous block" - .into(), - } - } else if verify_decrypted_correctly(&tx, privkey) { - TxResult { - code: ErrorCodes::Ok.into(), - info: "Process Proposal accepted this transaction" - .into(), - } - } else { - TxResult { - code: ErrorCodes::InvalidTx.into(), - info: "The encrypted payload of tx was \ - incorrectly marked as un-decryptable" - .into(), + TxType::Decrypted(tx) => { + metadata.has_decrypted_txs = true; + match tx_queue_iter.next() { + Some(WrapperTxInQueue { + tx: wrapper, + #[cfg(not(feature = "mainnet"))] + has_valid_pow: _, + }) => { + if wrapper.tx_hash != tx.hash_commitment() { + TxResult { + code: ErrorCodes::InvalidOrder.into(), + info: "Process proposal rejected a decrypted \ + transaction that violated the tx order \ + determined in the previous block" + .into(), + } + } else if verify_decrypted_correctly(&tx, privkey) { + TxResult { + code: ErrorCodes::Ok.into(), + info: "Process Proposal accepted this \ + transaction" + .into(), + } + } else { + TxResult { + code: ErrorCodes::InvalidTx.into(), + info: "The encrypted payload of tx was \ + incorrectly marked as un-decryptable" + .into(), + } } } + None => TxResult { + code: ErrorCodes::ExtraTxs.into(), + info: "Received more decrypted txs than expected" + .into(), + }, } - None => TxResult { - code: ErrorCodes::ExtraTxs.into(), - info: "Received more decrypted txs than expected".into(), - }, - }, + } TxType::Wrapper(tx) => { + // decrypted txs shouldn't show up before wrapper txs + if metadata.has_decrypted_txs { + return TxResult { + code: ErrorCodes::InvalidTx.into(), + info: "Decrypted txs should not be proposed before \ + wrapper txs" + .into(), + }; + } // try to allocate space for this encrypted tx if let Err(e) = metadata.encrypted_txs_bin.try_dump(tx_bytes) { return TxResult { From 33d0b6f1dc4c17c3a6a9363aecbf387c25b5ea10 Mon Sep 17 00:00:00 2001 From: Tiago Carvalho Date: Mon, 3 Apr 2023 09:59:03 +0100 Subject: [PATCH 35/37] Read token balances with storage_api::token::read_balance() --- proof_of_stake/src/pos_queries.rs | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/proof_of_stake/src/pos_queries.rs b/proof_of_stake/src/pos_queries.rs index 7f05c8c02d..86284ddcb9 100644 --- a/proof_of_stake/src/pos_queries.rs +++ b/proof_of_stake/src/pos_queries.rs @@ -135,15 +135,8 @@ where token: &Address, owner: &Address, ) -> token::Amount { - let balance = storage_api::StorageRead::read( - self.wl_storage, - &token::balance_key(token, owner), - ); - // Storage read must not fail, but there might be no value, in which - // case default (0) is returned - balance + storage_api::token::read_balance(self.wl_storage, token, owner) .expect("Storage read in the protocol must not fail") - .unwrap_or_default() } /// Return evidence parameters. From 5cf3b18e377b8278ce88d32d4dda0f85b96ce012 Mon Sep 17 00:00:00 2001 From: Tiago Carvalho Date: Mon, 3 Apr 2023 10:06:53 +0100 Subject: [PATCH 36/37] Rename active to consensus validators in PosQueries --- proof_of_stake/src/pos_queries.rs | 44 +++++++++++++++---------------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/proof_of_stake/src/pos_queries.rs b/proof_of_stake/src/pos_queries.rs index 86284ddcb9..bceabd3157 100644 --- a/proof_of_stake/src/pos_queries.rs +++ b/proof_of_stake/src/pos_queries.rs @@ -20,25 +20,25 @@ use crate::{consensus_validator_set_handle, PosParams}; /// Errors returned by [`PosQueries`] operations. #[derive(Error, Debug)] pub enum Error { - /// The given address is not among the set of active validators for + /// The given address is not among the set of consensus validators for /// the corresponding epoch. #[error( - "The address '{0:?}' is not among the active validator set for epoch \ - {1}" + "The address '{0:?}' is not among the consensus validator set for \ + epoch {1}" )] NotValidatorAddress(Address, Epoch), - /// The given public key does not correspond to any active validator's + /// The given public key does not correspond to any consensus validator's /// key at the provided epoch. #[error( - "The public key '{0}' is not among the active validator set for epoch \ - {1}" + "The public key '{0}' is not among the consensus validator set for \ + epoch {1}" )] NotValidatorKey(String, Epoch), - /// The given public key hash does not correspond to any active validator's - /// key at the provided epoch. + /// The given public key hash does not correspond to any consensus + /// validator's key at the provided epoch. #[error( - "The public key hash '{0}' is not among the active validator set for \ - epoch {1}" + "The public key hash '{0}' is not among the consensus validator set \ + for epoch {1}" )] NotValidatorKeyHash(String, Epoch), /// An invalid Tendermint validator address was detected. @@ -50,7 +50,7 @@ pub enum Error { pub type Result = ::std::result::Result; /// Methods used to query blockchain proof-of-stake related state, -/// such as the currently active set of validators. +/// such as the current set of consensus validators. pub trait PosQueries { /// The underlying storage type. type Storage; @@ -103,16 +103,16 @@ where self.wl_storage } - /// Get the set of active validators for a given epoch (defaulting to the + /// Get the set of consensus validators for a given epoch (defaulting to the /// epoch of the current yet-to-be-committed block). #[inline] - pub fn get_active_validators( + pub fn get_consensus_validators( self, epoch: Option, - ) -> ActiveValidators<'db, D, H> { + ) -> ConsensusValidators<'db, D, H> { let epoch = epoch .unwrap_or_else(|| self.wl_storage.storage.get_current_epoch().0); - ActiveValidators { + ConsensusValidators { wl_storage: self.wl_storage, validator_set: consensus_validator_set_handle().at(&epoch), } @@ -121,7 +121,7 @@ where /// Lookup the total voting power for an epoch (defaulting to the /// epoch of the current yet-to-be-committed block). pub fn get_total_voting_power(self, epoch: Option) -> token::Amount { - self.get_active_validators(epoch) + self.get_consensus_validators(epoch) .iter() .map(|validator| u64::from(validator.bonded_stake)) .sum::() @@ -172,7 +172,7 @@ where ) -> Result<(token::Amount, key::common::PublicKey)> { let epoch = epoch .unwrap_or_else(|| self.wl_storage.storage.get_current_epoch().0); - self.get_active_validators(Some(epoch)) + self.get_consensus_validators(Some(epoch)) .iter() .find(|validator| address == &validator.address) .map(|validator| { @@ -272,9 +272,9 @@ where } } -/// A handle to the set of active validators in Namada, +/// A handle to the set of consensus validators in Namada, /// at some given epoch. -pub struct ActiveValidators<'db, D, H> +pub struct ConsensusValidators<'db, D, H> where D: storage::DB + for<'iter> storage::DBIter<'iter>, H: storage::StorageHasher, @@ -283,19 +283,19 @@ where validator_set: ConsensusValidatorSet, } -impl<'db, D, H> ActiveValidators<'db, D, H> +impl<'db, D, H> ConsensusValidators<'db, D, H> where D: storage::DB + for<'iter> storage::DBIter<'iter>, H: storage::StorageHasher, { - /// Iterate over the set of active validators in Namada, at some given + /// Iterate over the set of consensus validators in Namada, at some given /// epoch. pub fn iter<'this: 'db>( &'this self, ) -> impl Iterator + 'db { self.validator_set .iter(self.wl_storage) - .expect("Must be able to iterate over active validators") + .expect("Must be able to iterate over consensus validators") .map(|res| { let ( NestedSubKey::Data { From 9d75163ff6a343441d2ad35f9ba4da7c2e542362 Mon Sep 17 00:00:00 2001 From: Tiago Carvalho Date: Wed, 5 Apr 2023 09:33:04 +0100 Subject: [PATCH 37/37] Improve ErrorCodes::is_recoverable() --- apps/src/lib/node/ledger/shell/mod.rs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/apps/src/lib/node/ledger/shell/mod.rs b/apps/src/lib/node/ledger/shell/mod.rs index 367efde16b..f90dde5862 100644 --- a/apps/src/lib/node/ledger/shell/mod.rs +++ b/apps/src/lib/node/ledger/shell/mod.rs @@ -130,15 +130,20 @@ pub enum ErrorCodes { InvalidOrder = 4, ExtraTxs = 5, Undecryptable = 6, - AllocationError = 7, /* NOTE: keep these values in sync with - * [`ErrorCodes::is_recoverable`] */ + AllocationError = 7, } impl ErrorCodes { /// Checks if the given [`ErrorCodes`] value is a protocol level error, /// that can be recovered from at the finalize block stage. pub const fn is_recoverable(&self) -> bool { - (*self as u32) <= 3 + use ErrorCodes::*; + // NOTE: pattern match on all `ErrorCodes` variants, in order + // to catch potential bugs when adding new codes + match self { + Ok | InvalidTx | InvalidSig | WasmRuntimeError => true, + InvalidOrder | ExtraTxs | Undecryptable | AllocationError => false, + } } }