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.

Sign up for GitHub

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

chore(sdk): define helper trait BlockPrimitives #12721

Closed
wants to merge 2 commits into from
Closed

chore(sdk): define helper trait BlockPrimitives #12721

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
831a6da
Define BlockPrimitives, links block types in NodePrimitives
emhane Nov 24, 2024
31e5826
Fix merge conflicts
emhane Nov 24, 2024
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
1 change: 1 addition & 0 deletions Cargo.lock
View file
Open in desktop

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

1 change: 1 addition & 0 deletions crates/consensus/beacon/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ reth-engine-primitives.workspace = true
reth-network-p2p.workspace = true
reth-node-types.workspace = true
reth-chainspec = { workspace = true, optional = true }
reth-primitives-traits.workspace = true

# ethereum
alloy-primitives.workspace = true
Expand Down
5 changes: 3 additions & 2 deletions crates/consensus/beacon/src/engine/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ use reth_payload_builder_primitives::PayloadBuilder;
use reth_payload_primitives::{PayloadAttributes, PayloadBuilderAttributes};
use reth_payload_validator::ExecutionPayloadValidator;
use reth_primitives::{Head, SealedBlock, SealedHeader};
use reth_primitives_traits::BlockPrimitives;
use reth_provider::{
providers::ProviderNodeTypes, BlockIdReader, BlockReader, BlockSource, CanonChainTracker,
ChainSpecProvider, ProviderError, StageCheckpointReader,
Expand Down Expand Up @@ -226,7 +227,7 @@ where

impl<N, BT, Client> BeaconConsensusEngine<N, BT, Client>
where
N: EngineNodeTypes,
N: EngineNodeTypes<Primitives: BlockPrimitives>,
BT: BlockchainTreeEngine
+ BlockReader
+ BlockIdReader
Expand Down Expand Up @@ -1795,7 +1796,7 @@ where
/// receiver and forwarding them to the blockchain tree.
impl<N, BT, Client> Future for BeaconConsensusEngine<N, BT, Client>
where
N: EngineNodeTypes,
N: EngineNodeTypes<Primitives: BlockPrimitives>,
Client: EthBlockClient + 'static,
BT: BlockchainTreeEngine
+ BlockReader
Expand Down
4 changes: 1 addition & 3 deletions crates/node/builder/src/setup.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,9 +87,7 @@ pub fn build_pipeline<N, H, B, Executor>(
where
N: ProviderNodeTypes,
H: HeaderDownloader<Header = alloy_consensus::Header> + 'static,
B: BodyDownloader<
Body = <<N::Primitives as NodePrimitives>::Block as reth_node_api::Block>::Body,
> + 'static,
B: BodyDownloader<Body = <N::Primitives as NodePrimitives>::BlockBody> + 'static,
Executor: BlockExecutorProvider,
N::Primitives: FullNodePrimitives<BlockBody = reth_primitives::BlockBody>,
{
Expand Down
8 changes: 3 additions & 5 deletions crates/primitives-traits/src/block/body.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,7 @@

use alloc::fmt;

use alloy_consensus::Transaction;

use crate::{FullSignedTx, InMemorySize, MaybeArbitrary, MaybeSerde};
use crate::{FullSignedTx, InMemorySize, MaybeArbitrary, MaybeSerde, SignedTransaction};

/// Helper trait that unifies all behaviour required by transaction to support full node operations.
pub trait FullBlockBody: BlockBody<Transaction: FullSignedTx> {}
Expand All @@ -28,8 +26,8 @@ pub trait BlockBody:
+ MaybeSerde
+ MaybeArbitrary
{
/// Ordered list of signed transactions as committed in block.
type Transaction: Transaction;
/// Signed transaction.
type Transaction: SignedTransaction;

/// Returns reference to transactions in block.
fn transactions(&self) -> &[Self::Transaction];
Expand Down
4 changes: 3 additions & 1 deletion crates/primitives-traits/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,9 @@ pub use size::InMemorySize;

/// Node traits
pub mod node;
pub use node::{FullNodePrimitives, NodePrimitives, ReceiptTy};
pub use node::{
BlockPrimitives, FullBlockPrimitives, FullNodePrimitives, NodePrimitives, ReceiptTy,
};

/// Helper trait that requires arbitrary implementation if the feature is enabled.
#[cfg(any(feature = "test-utils", feature = "arbitrary"))]
Expand Down
97 changes: 63 additions & 34 deletions crates/primitives-traits/src/node.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
use core::fmt;

use crate::{
FullBlock, FullBlockBody, FullBlockHeader, FullReceipt, FullSignedTx, FullTxType, MaybeSerde,
Block, BlockBody, BlockHeader, FullBlock, FullBlockBody, FullBlockHeader, FullReceipt,
FullSignedTx, FullTxType, MaybeSerde, SignedTransaction,
};

/// Configures all the primitive types of the node.
Expand Down Expand Up @@ -77,46 +78,74 @@ impl NodePrimitives for () {
}

/// Helper trait that sets trait bounds on [`NodePrimitives`].
pub trait FullNodePrimitives
where
Self: NodePrimitives<
Block: FullBlock<Header = Self::BlockHeader, Body = Self::BlockBody>,
BlockHeader: FullBlockHeader,
BlockBody: FullBlockBody<Transaction = Self::SignedTx>,
SignedTx: FullSignedTx,
TxType: FullTxType,
Receipt: FullReceipt,
> + Send
+ Sync
+ Unpin
+ Clone
+ Default
+ fmt::Debug
+ PartialEq
+ Eq
+ 'static,
pub trait FullNodePrimitives:
NodePrimitives<
Block: FullBlock<Header = Self::BlockHeader, Body = Self::BlockBody> + 'static,
BlockHeader: FullBlockHeader + 'static,
BlockBody: FullBlockBody<Transaction = Self::SignedTx> + 'static,
SignedTx: FullSignedTx + 'static,
TxType: FullTxType + 'static,
Receipt: FullReceipt + 'static,
>
{
}

impl<T> FullNodePrimitives for T where
T: NodePrimitives<
Block: FullBlock<Header = Self::BlockHeader, Body = Self::BlockBody>,
BlockHeader: FullBlockHeader,
BlockBody: FullBlockBody<Transaction = Self::SignedTx>,
SignedTx: FullSignedTx,
TxType: FullTxType,
Receipt: FullReceipt,
> + Send
+ Sync
+ Unpin
+ Clone
+ Default
+ fmt::Debug
+ PartialEq
+ Eq
+ 'static
Block: FullBlock<Header = Self::BlockHeader, Body = Self::BlockBody> + 'static,
BlockHeader: FullBlockHeader + 'static,
BlockBody: FullBlockBody<Transaction = Self::SignedTx> + 'static,
SignedTx: FullSignedTx + 'static,
TxType: FullTxType + 'static,
Receipt: FullReceipt + 'static,
>
{
}

/// Helper adapter type for accessing [`NodePrimitives`] receipt type.
pub type ReceiptTy<N> = <N as NodePrimitives>::Receipt;

/// Helper trait that links [`NodePrimitives::Block`] to [`NodePrimitives::BlockHeader`],
/// [`NodePrimitives::BlockBody`] and [`NodePrimitives::SignedTx`].
pub trait BlockPrimitives:
Copy link
Member

Choose a reason for hiding this comment

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

unsure I understand why we need a separate trait for this? those bounds already exist for FullNodePrimitives, and I believe we could also move them to NodePrimitives?

Copy link
Member Author

Choose a reason for hiding this comment

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

for the same reason we don't have the compact feature reth-codec always enabled, cc @joshieDo . it makes testing more straight forward and it makes it more apparent to us how data flows in reth.

if you want to test for example consensus engine, you should just need an impl like

impl NodePrimitives for TestPrimitives {
    type Block = Block;
    type BlockHeader = Header;
    type BlockBody = BlockBody;
    type SignedTx = TransactionSigned;
    type TxType = ();
    type Receipt = ();
}

it's always misleading a weird for someone new to the code, to try and figure out how the code works by looking at the test, and then realise half way through that types are in scope, that are not used.

Copy link
Member Author

Choose a reason for hiding this comment

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

nice side effect of having NodePrimitives as super trait of BlockPrimitives, is that in case it's needed, a trait impl using BlockPrimitives, can still return a receipt type, just not use the receipt functionality in its scope

Copy link
Member

Choose a reason for hiding this comment

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

ProviderFactory won't work unless primitives are FullNodePrimitives so I don't think you will be able to test anything with such primitives implementation

Copy link
Member

Choose a reason for hiding this comment

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

I think it might make sense to move those bounds to NodePrimitives but separate trait doesn't seem very useful

Copy link
Member Author

Choose a reason for hiding this comment

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

provider is expected to use FullNodePrimitives in some places. node builder is expected to use FullNodePrimitives everywhere

Comment on lines +108 to +110
Copy link
Collaborator

Choose a reason for hiding this comment

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

unclear what the benefit here is
what does this unblock?

Copy link
Member Author

Choose a reason for hiding this comment

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

it links the block type to the header, body and signed tx types. it should be used in places where FullNodePrimitives is not needed, and simplifies having to specify the super trait of BlockPrimitives everywhere, i.e. N: NodePrimitives<Block: Block<Header = N::BlockHeader, ..>, ..> to link the blockparts and tx to the block type

NodePrimitives<
Block: Block<Header = Self::BlockHeader, Body = Self::BlockBody>,
BlockHeader: BlockHeader,
BlockBody: BlockBody<Transaction = Self::SignedTx>,
SignedTx: SignedTransaction,
>
{
}

impl<T> BlockPrimitives for T where
T: NodePrimitives<
Block: Block<Header = T::BlockHeader, Body = T::BlockBody>,
BlockHeader: BlockHeader,
BlockBody: BlockBody<Transaction = T::SignedTx>,
SignedTx: SignedTransaction,
>
{
}

/// Helper trait like [`BlockPrimitives`], but with encoding trait bounds on
/// [`NodePrimitives::Block`], [`NodePrimitives::BlockHeader`], [`NodePrimitives::BlockBody`] and
/// [`NodePrimitives::SignedTx`].
pub trait FullBlockPrimitives:
NodePrimitives<
Block: FullBlock<Header = Self::BlockHeader, Body = Self::BlockBody>,
BlockHeader: FullBlockHeader,
BlockBody: FullBlockBody<Transaction = Self::SignedTx>,
SignedTx: FullSignedTx,
>
{
}

impl<T> FullBlockPrimitives for T where
T: NodePrimitives<
Block: FullBlock<Header = T::BlockHeader, Body = T::BlockBody>,
BlockHeader: FullBlockHeader,
BlockBody: FullBlockBody<Transaction = T::SignedTx>,
SignedTx: FullSignedTx,
>
{
}
3 changes: 2 additions & 1 deletion crates/primitives-traits/src/transaction/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,10 @@ pub mod execute;
pub mod signed;
pub mod tx_type;

use crate::{InMemorySize, MaybeArbitrary, MaybeCompact, MaybeSerde};
use core::{fmt, hash::Hash};

use crate::{InMemorySize, MaybeArbitrary, MaybeCompact, MaybeSerde};

/// Helper trait that unifies all behaviour required by transaction to support full node operations.
pub trait FullTransaction: Transaction + MaybeCompact {}

Expand Down
22 changes: 18 additions & 4 deletions crates/primitives-traits/src/transaction/signed.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,26 @@
//! API of a signed transaction.

use crate::{FillTxEnv, InMemorySize, MaybeArbitrary, MaybeCompact, MaybeSerde, TxType};
use alloc::fmt;
use core::hash::Hash;

use alloy_eips::eip2718::{Decodable2718, Encodable2718};
use alloy_primitives::{keccak256, Address, PrimitiveSignature, TxHash, B256};
use core::hash::Hash;

use crate::{
FillTxEnv, FullTransaction, FullTxType, InMemorySize, MaybeArbitrary, MaybeCompact, MaybeSerde,
Transaction, TxType,
};

/// Helper trait that unifies all behaviour required by block to support full node operations.
pub trait FullSignedTx: SignedTransaction + FillTxEnv + MaybeCompact {}
pub trait FullSignedTx:
SignedTransaction<Transaction: FullTransaction, Type: FullTxType> + FillTxEnv + MaybeCompact
{
}

impl<T> FullSignedTx for T where T: SignedTransaction + FillTxEnv + MaybeCompact {}
impl<T> FullSignedTx for T where
T: SignedTransaction<Transaction: FullTransaction, Type: FullTxType> + FillTxEnv + MaybeCompact
{
}

/// A signed transaction.
#[auto_impl::auto_impl(&, Arc)]
Expand All @@ -32,6 +43,9 @@ pub trait SignedTransaction:
+ MaybeArbitrary
+ InMemorySize
{
/// Unsigned transaction type.
type Transaction: Transaction;

/// Transaction envelope type ID.
type Type: TxType;

Expand Down
1 change: 1 addition & 0 deletions crates/primitives/src/transaction/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1265,6 +1265,7 @@ impl TransactionSigned {

impl SignedTransaction for TransactionSigned {
type Type = TxType;
type Transaction = Transaction;
Copy link
Collaborator

Choose a reason for hiding this comment

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

this was removed because this AT, didn't make much sense that a transaction defines a transaction


fn tx_hash(&self) -> &TxHash {
self.hash_ref()
Expand Down
1 change: 1 addition & 0 deletions crates/primitives/src/transaction/pooled.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -579,6 +579,7 @@ impl alloy_consensus::Transaction for PooledTransactionsElement {
}

impl SignedTransaction for PooledTransactionsElement {
type Transaction = Transaction;
type Type = TxType;

fn tx_hash(&self) -> &TxHash {
Expand Down
4 changes: 2 additions & 2 deletions crates/storage/provider/src/providers/database/chain.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
use crate::{providers::NodeTypes, DatabaseProvider};
use reth_db::transaction::{DbTx, DbTxMut};
use reth_node_types::FullNodePrimitives;
use reth_primitives::EthPrimitives;
use reth_primitives_traits::BlockPrimitives;
use reth_storage_api::{ChainStorageWriter, EthStorage};

/// Trait that provides access to implementations of [`ChainStorage`]
pub trait ChainStorage<Primitives: FullNodePrimitives>: Send + Sync {
pub trait ChainStorage<Primitives: BlockPrimitives>: Send + Sync {
/// Provides access to the chain writer.
fn writer<TX, Types>(&self) -> impl ChainStorageWriter<DatabaseProvider<TX, Types>, Primitives>
where
Expand Down
10 changes: 5 additions & 5 deletions crates/storage/provider/src/providers/database/provider.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,11 @@ use reth_execution_types::{Chain, ExecutionOutcome};
use reth_network_p2p::headers::downloader::SyncTarget;
use reth_node_types::{NodeTypes, TxTy};
use reth_primitives::{
Account, Block, BlockBody, BlockWithSenders, Bytecode, GotExpected, NodePrimitives, Receipt,
SealedBlock, SealedBlockWithSenders, SealedHeader, StaticFileSegment, StorageEntry,
TransactionMeta, TransactionSigned, TransactionSignedNoHash,
Account, Block, BlockBody, BlockWithSenders, Bytecode, GotExpected, Receipt, SealedBlock,
SealedBlockWithSenders, SealedHeader, StaticFileSegment, StorageEntry, TransactionMeta,
TransactionSigned, TransactionSignedNoHash,
};
use reth_primitives_traits::{BlockBody as _, SignedTransaction};
use reth_primitives_traits::{BlockBody as _, NodePrimitives, SignedTransaction};
use reth_prune_types::{PruneCheckpoint, PruneModes, PruneSegment};
use reth_stages_types::{StageCheckpoint, StageId};
use reth_storage_api::{StateProvider, StorageChangeSetReader, TryIntoHistoricalStateProvider};
Expand Down Expand Up @@ -2751,7 +2751,7 @@ impl<TX: DbTxMut + DbTx + 'static, N: NodeTypesForProvider + 'static> BlockExecu
impl<TX: DbTxMut + DbTx + 'static, N: NodeTypesForProvider + 'static> BlockWriter
for DatabaseProvider<TX, N>
{
type Body = <<N::Primitives as NodePrimitives>::Block as reth_primitives_traits::Block>::Body;
type Body = <N::Primitives as NodePrimitives>::BlockBody;

/// Inserts the block into the database, always modifying the following tables:
/// * [`CanonicalHeaders`](tables::CanonicalHeaders)
Expand Down
10 changes: 5 additions & 5 deletions crates/storage/storage-api/src/chain.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use reth_db::{
transaction::DbTxMut,
DbTxUnwindExt,
};
use reth_primitives_traits::{Block, BlockBody, FullNodePrimitives};
use reth_primitives_traits::{BlockBody, BlockPrimitives};
use reth_storage_errors::provider::ProviderResult;

/// Trait that implements how block bodies are written to the storage.
Expand All @@ -32,12 +32,12 @@ pub trait BlockBodyWriter<Provider, Body: BlockBody> {
}

/// Trait that implements how chain-specific types are written to the storage.
pub trait ChainStorageWriter<Provider, Primitives: FullNodePrimitives>:
BlockBodyWriter<Provider, <Primitives::Block as Block>::Body>
pub trait ChainStorageWriter<Provider, Primitives: BlockPrimitives>:
BlockBodyWriter<Provider, Primitives::BlockBody>
{
}
impl<T, Provider, Primitives: FullNodePrimitives> ChainStorageWriter<Provider, Primitives> for T where
T: BlockBodyWriter<Provider, <Primitives::Block as Block>::Body>
impl<T, Provider, Primitives: BlockPrimitives> ChainStorageWriter<Provider, Primitives> for T where
T: BlockBodyWriter<Provider, Primitives::BlockBody>
{
}

Expand Down
Loading