feat(payload builder): transaction pool filter #10542
Conversation
greged93
changed the title
There was a problem hiding this comment.
sorry about the delay.
this direction makes sense,
I'd like to move the trait to txpool crate so we can also reuse it for OP and because this is only really relevant when used on the pool, then we can easily add more filters and helpers to combine with the BestTransactions for example
crates/ethereum/payload/src/lib.rs
Outdated
| default_ethereum_payload_builder( | ||
| self.evm_config.clone(), | ||
| args, | ||
| NoOpTransactionFilter::new(), | ||
| ) |
There was a problem hiding this comment.
I'd like to tackle this feature differently, by giving the
reth/crates/ethereum/payload/src/lib.rs
Line 52 in 5fc7755
an instance of a filter
mattsse
added
A-block-building
greged93
force-pushed
the
feat/pool-tx-attributes
branch
from
bd2ca25 to
33ecdbc
Compare
crates/ethereum/node/src/node.rs
Outdated
| let payload_builder = reth_ethereum_payload_builder::EthereumPayloadBuilder::new( | ||
| self.evm_config, | ||
| NoopTransactionFilter::new(), | ||
| ); |
There was a problem hiding this comment.
I know this isn't great and would probably be better to have the filter in EthereumPayloadBuilder here too, but this added a lot of constraints:
-
either we added a generic
TxFilterto it as done lower, which meant we add to add this generic toEthereumNode::componentsand the toNodetrait implementation. I think this generic shouldn't be present on either of these. -
we have a default filter which would be
NoopTransactionFilterbut it requires a GATTransactionwhich represents the transaction we are filtering over (in this case a pool transaction).
I feel like the second solution is better but not quite doable in a clean way yet, since I think we need FullNodeTypes to expose the pool transaction type.
crates/ethereum/node/src/node.rs
Outdated
| reth_ethereum_payload_builder::EthereumPayloadBuilder::new(self.evm_config); | ||
| let payload_builder = reth_ethereum_payload_builder::EthereumPayloadBuilder::new( | ||
| self.evm_config, | ||
| NoopTransactionFilter::new(), |
There was a problem hiding this comment.
this should not break, noop is default and should not be necessary here
crates/ethereum/payload/src/lib.rs
Outdated
| use tracing::{debug, trace, warn}; | ||
|
|
||
| /// Ethereum payload builder | ||
| #[derive(Debug, Clone, Copy, PartialEq, Eq)] | ||
| pub struct EthereumPayloadBuilder<EvmConfig = EthEvmConfig> { | ||
| pub struct EthereumPayloadBuilder<TxFilter, EvmConfig = EthEvmConfig> { |
There was a problem hiding this comment.
the positions should be flipped with TxFilter as a default
| pub const fn new() -> Self { | ||
| Self(std::marker::PhantomData) | ||
| } |
There was a problem hiding this comment.
this function should be replaced with a default impl
There was a problem hiding this comment.
these changes can be reverted if we keep the new function as is
greged93
force-pushed
the
feat/pool-tx-attributes
branch
2 times, most recently
from
9062b0c to
2d62b26
Compare
|
@mattsse slight bump on this :) |
greged93
force-pushed
the
feat/pool-tx-attributes
branch
2 times, most recently
from
dc2b554 to
42000c9
Compare
greged93
force-pushed
the
feat/pool-tx-attributes
branch
from
42000c9 to
0d3a3ea
Compare
mattsse
force-pushed
the
feat/pool-tx-attributes
branch
from
696d1d8 to
8838b81
Compare
mattsse
force-pushed
the
feat/pool-tx-attributes
branch
from
8838b81 to
98b3f8a
Compare
|
I've undone the payload builder integration because I have something else in mind now |
TransactionFiltertrait, exposing ais_validfunction that can be called in order to verify if the provided transaction passes the filtering implemented.NoOpTransactionFilterwhich lets all transactions pass.Closes #10437