Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Plug BlockSpaceAllocator logic into ProcessProposal #895

Merged
Show file tree
Hide file tree
Changes from 34 commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
890edd9
Update index-set to 0.7.1
sug0 Dec 13, 2022
fbe4ed5
Move block space allocator to shell/
sug0 Dec 13, 2022
c5f0e62
Fix imports
sug0 Dec 13, 2022
f33df5d
Add a state tracker for a block space allocator
sug0 Dec 13, 2022
b276e13
Inline some simple method calls
sug0 Dec 13, 2022
a878447
Seal state tracker API to valid block space allocator states
sug0 Dec 13, 2022
7aa56f3
Move tracker to a new file
sug0 Dec 13, 2022
28cd78b
Make tracker mod public
sug0 Dec 13, 2022
e606d12
Add tracker consts
sug0 Dec 13, 2022
0db25a4
WIP: Add block space allocator to ProcessProposal
sug0 Dec 13, 2022
0a549a7
Improve docstr
sug0 Dec 14, 2022
fd95717
Return reason for rejecting vote ext from ProcessProposal
sug0 Dec 14, 2022
0b3109e
Add a fused block space allocator
sug0 Dec 14, 2022
97d29b2
Add CurrentState impl to the FusedBlockSpaceAllocator
sug0 Dec 14, 2022
8c3df34
WIP: ProcessProposal validation with the block space allocator
sug0 Dec 14, 2022
640c3d1
Improve field identifiers in the fused block space allocator
sug0 Dec 14, 2022
e2996d3
Removing unnecessary TryAlloc bounds
sug0 Dec 14, 2022
3fcb10f
WIP: Checking allocator state in ProcessProposal
sug0 Dec 14, 2022
e855810
Use generic param instead of impl in check_proposal_tx()
sug0 Dec 14, 2022
1d83824
Generalize FusedBlockSpaceAllocator into CtxBlockSpaceAllocator
sug0 Dec 14, 2022
52e4418
Remove Option from ctx allocator
sug0 Dec 14, 2022
58b6c4a
Remove all the state tracker non-sense
sug0 Dec 14, 2022
9c7f664
Return reason for rejecting vote ext from ProcessProposal
sug0 Dec 14, 2022
6fa26f5
Make threshold and tx bins public
sug0 Dec 14, 2022
1426518
Keep track of utilized by encrypted txs in ProcessProposal
sug0 Dec 14, 2022
a91e928
Add allocation tx error code
sug0 Dec 14, 2022
eda49d1
Reject proposal on allocation errors
sug0 Dec 14, 2022
b7c025e
Decrypted tx queue meta bug fix
sug0 Dec 14, 2022
fd5344e
Check if the decrypted tx queue has any remaining txs left
sug0 Dec 14, 2022
c7cbe10
Fix docstrs
sug0 Dec 15, 2022
949ffa8
Fix test_too_many_decrypted_txs() unit test
sug0 Dec 15, 2022
49f96b2
Allow dead code on TestShell:new()
sug0 Dec 15, 2022
8af117a
Fix test_decrypted_txs_out_of_order() unit test
sug0 Dec 15, 2022
3ec3ea6
Merge tiago/ethbridge/fix-abcipp-prepare-proposal
sug0 Dec 15, 2022
2dbf090
Merge branch 'tiago/ethbridge/fix-abcipp-prepare-proposal' into tiago…
sug0 Dec 16, 2022
fcf1ee5
Merge branch 'tiago/ethbridge/optimize-block-proposal' into tiago/eth…
sug0 Dec 20, 2022
908305b
Re-order tx building method defs in PrepareProposal
sug0 Dec 20, 2022
93fcdda
Add missing tx error code in ProcessProposal docstr
sug0 Dec 20, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion apps/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ 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.5.0"}
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"
Expand Down
2 changes: 1 addition & 1 deletion apps/src/lib/client/tx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -778,7 +778,7 @@ impl ShieldedContext {
/// associated to notes, memos, and diversifiers. And the set of notes that
/// we have spent are updated. The witness map is maintained to make it
/// easier to construct note merkle paths in other code. See
/// https://zips.z.cash/protocol/protocol.pdf#scan
/// <https://zips.z.cash/protocol/protocol.pdf#scan>
pub fn scan_tx(
&mut self,
height: BlockHeight,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,9 @@ use std::marker::PhantomData;
use namada::core::ledger::storage::{self, Storage};
use namada::ledger::queries_ext::QueriesExt;

#[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 {
Expand Down Expand Up @@ -160,7 +163,7 @@ impl<State> BlockSpaceAllocator<State> {
/// Allotted space for a batch of transactions of the same kind in some
/// proposed block, measured in bytes.
#[derive(Debug, Copy, Clone, Default)]
struct TxBin {
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.
Expand All @@ -171,7 +174,7 @@ 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]
fn init_over_ratio(max_bytes: u64, frac: threshold::Threshold) -> Self {
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,
Expand All @@ -181,13 +184,13 @@ impl TxBin {

/// Return the amount of space left in this [`TxBin`].
#[inline]
fn space_left_in_bytes(&self) -> u64 {
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]
fn init(max_bytes: u64) -> Self {
pub fn init(max_bytes: u64) -> Self {
Self {
allotted_space_in_bytes: max_bytes,
occupied_space_in_bytes: 0,
Expand All @@ -197,15 +200,15 @@ impl TxBin {
/// Shrink the allotted space of this [`TxBin`] to whatever
/// space is currently being utilized.
#[inline]
fn shrink_to_fit(&mut self) {
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.
fn try_dump(&mut self, tx: &[u8]) -> Result<(), AllocFailure> {
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;
Expand All @@ -222,7 +225,7 @@ impl TxBin {
}
}

mod threshold {
pub mod threshold {
//! Transaction allotment thresholds.

use num_rational::Ratio;
Expand Down
6 changes: 4 additions & 2 deletions apps/src/lib/node/ledger/shell/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 <https://github.com/anoma/namada/issues/362>.
mod block_space_alloc;
mod finalize_block;
mod governance;
mod init_chain;
Expand Down Expand Up @@ -124,8 +125,8 @@ pub enum ErrorCodes {
ExtraTxs = 5,
Undecryptable = 6,
InvalidVoteExtension = 7,
// NOTE: keep these values in sync with
// [`ErrorCodes::is_recoverable`]
AllocationError = 8, /* NOTE: keep these values in sync with
* [`ErrorCodes::is_recoverable`] */
Comment on lines +128 to +129
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ErrorCodes::is_recoverable should be matching on this enum rather than returning (self as u32) <= 3, to enforce this (related to this NOTE but not the PR itself though, we shouldn't change this as part of this PR)

Copy link
Collaborator Author

@sug0 sug0 Dec 16, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's how it was defined in ProcessProposal before, this is_recoverable() method came from a ProcessProposal refactor from back when I was working on eth events vote extensions

}

impl ErrorCodes {
Expand Down Expand Up @@ -1019,6 +1020,7 @@ mod test_utils {
/// Same as [`TestShell::new_at_height`], but returns a shell at block
/// height 0.
#[inline]
#[allow(dead_code)]
pub fn new() -> (Self, UnboundedReceiver<Vec<u8>>, Sender<EthereumEvent>)
{
Self::new_at_height(BlockHeight(1))
Expand Down
8 changes: 3 additions & 5 deletions apps/src/lib/node/ledger/shell/prepare_proposal.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
//! Implementation of the [`RequestPrepareProposal`] ABCI++ method for the Shell

mod block_space_alloc;

#[cfg(not(feature = "abcipp"))]
use index_set::vec::VecIndexSet;
use namada::core::hints;
Expand All @@ -18,13 +16,13 @@ use namada::types::transaction::{AffineCurve, DecryptedTx, EllipticCurve};
#[cfg(feature = "abcipp")]
use namada::types::vote_extensions::VoteExtensionDigest;

use self::block_space_alloc::states::{
use super::super::*;
use super::block_space_alloc::states::{
BuildingDecryptedTxBatch, BuildingProtocolTxBatch,
EncryptedTxBatchAllocator, FillingRemainingSpace, NextState,
NextStateWithEncryptedTxs, NextStateWithoutEncryptedTxs, TryAlloc,
};
use self::block_space_alloc::{AllocFailure, BlockSpaceAllocator};
use super::super::*;
use super::block_space_alloc::{AllocFailure, BlockSpaceAllocator};
#[cfg(feature = "abcipp")]
use crate::facade::tendermint_proto::abci::ExtendedCommitInfo;
use crate::facade::tendermint_proto::abci::RequestPrepareProposal;
Expand Down
Loading