Skip to content

Commit

Permalink
Tidy unused variables from op payload builder (#427)
Browse files Browse the repository at this point in the history 
## 📝 Summary

Cleanup PR to remove unused variables.

## 💡 Motivation and Context

<!--- (Optional) Why is this change required? What problem does it
solve? Remove this section if not applicable. -->

---

## ✅ I have completed the following steps:

* [ ] Run `make lint`
* [ ] Run `make test`
* [ ] Added tests (if applicable)
  • Loading branch information
@ferranbt
ferranbt authored Feb 12, 2025
1 parent 7512f28 commit 08f4fae
Show file tree
Hide file tree
Showing 3 changed files with 7 additions and 43 deletions.
9 changes: 3 additions & 6 deletions crates/op-rbuilder/src/generator.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ pub trait PayloadBuilder<Pool, Client>: Send + Sync + Clone {
/// A `Result` indicating the build outcome or an error.
fn try_build(
&self,
args: BuildArguments<Pool, Client, Self::Attributes, Self::BuiltPayload>,
args: BuildArguments<Pool, Client, Self::Attributes>,
best_payload: BlockCell<Self::BuiltPayload>,
) -> Result<(), PayloadBuilderError>;
}
Expand Down Expand Up @@ -323,7 +323,7 @@ where
}
}

pub struct BuildArguments<Pool, Client, Attributes, Payload> {
pub struct BuildArguments<Pool, Client, Attributes> {
/// How to interact with the chain.
pub client: Client,
/// The transaction pool.
Expand All @@ -336,8 +336,6 @@ pub struct BuildArguments<Pool, Client, Attributes, Payload> {
pub config: PayloadConfig<Attributes>,
/// A marker that can be used to cancel the job.
pub cancel: CancellationToken,
/// The best payload achieved so far.
pub best_payload: Option<Payload>,
}

/// A [PayloadJob] is a future that's being polled by the `PayloadBuilderService`
Expand Down Expand Up @@ -368,7 +366,6 @@ where
cached_reads: Default::default(),
config: payload_config,
cancel,
best_payload: None,
};

let result = builder.try_build(args, cell);
Expand Down Expand Up @@ -657,7 +654,7 @@ mod tests {

fn try_build(
&self,
args: BuildArguments<Pool, Client, Self::Attributes, Self::BuiltPayload>,
args: BuildArguments<Pool, Client, Self::Attributes>,
_best_payload: BlockCell<Self::BuiltPayload>,
) -> Result<(), PayloadBuilderError> {
self.new_event(BlockEvent::Started);
Expand Down
16 changes: 2 additions & 14 deletions crates/op-rbuilder/src/payload_builder.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,6 @@ use tokio_tungstenite::WebSocketStream;
/// Optimism's payload builder
#[derive(Debug, Clone)]
pub struct OpPayloadBuilder<EvmConfig, Txs = ()> {
/// The rollup's compute pending block configuration option.
// TODO(clabby): Implement this feature.
pub compute_pending_block: bool,
/// The type responsible for creating the evm.
pub evm_config: EvmConfig,
/// The type responsible for yielding the best transactions for the payload if mempool
Expand All @@ -76,7 +73,6 @@ impl<EvmConfig> OpPayloadBuilder<EvmConfig> {
Self::publish_task(rx, subscribers.clone());

Self {
compute_pending_block: true,
evm_config,
best_transactions: (),
subscribers,
Expand Down Expand Up @@ -157,7 +153,7 @@ where
/// a result indicating success with the payload or an error in case of failure.
fn build_payload<Client, Pool>(
&self,
args: BuildArguments<Pool, Client, OpPayloadBuilderAttributes, OpBuiltPayload>,
args: BuildArguments<Pool, Client, OpPayloadBuilderAttributes>,
best_payload: BlockCell<OpBuiltPayload>,
) -> Result<(), PayloadBuilderError>
where
Expand Down Expand Up @@ -185,7 +181,6 @@ where
initialized_cfg: cfg_env_with_handler_cfg,
initialized_block_env: block_env,
cancel,
best_payload: None, // remove
};

let best = self.best_transactions.clone();
Expand Down Expand Up @@ -312,7 +307,7 @@ where

fn try_build(
&self,
args: BuildArguments<Pool, Client, Self::Attributes, Self::BuiltPayload>,
args: BuildArguments<Pool, Client, Self::Attributes>,
best_payload: BlockCell<Self::BuiltPayload>,
) -> Result<(), PayloadBuilderError> {
self.build_payload(args, best_payload)
Expand Down Expand Up @@ -540,8 +535,6 @@ pub struct OpPayloadBuilderCtx<EvmConfig> {
pub initialized_block_env: BlockEnv,
/// Marker to check whether the job has been cancelled.
pub cancel: CancellationToken,
/// The currently best payload.
pub best_payload: Option<OpBuiltPayload>,
}

impl<EvmConfig> OpPayloadBuilderCtx<EvmConfig> {
Expand Down Expand Up @@ -644,11 +637,6 @@ impl<EvmConfig> OpPayloadBuilderCtx<EvmConfig> {
.is_holocene_active_at_timestamp(self.attributes().timestamp())
}

/// Returns true if the fees are higher than the previous payload.
pub fn is_better_payload(&self, total_fees: U256) -> bool {
is_better_payload(self.best_payload.as_ref(), total_fees)
}

/// Commits the withdrawals from the payload attributes to the state.
pub fn commit_withdrawals<DB>(&self, db: &mut State<DB>) -> Result<Option<B256>, ProviderError>
where
Expand Down
25 changes: 2 additions & 23 deletions crates/op-rbuilder/src/payload_builder_vanilla.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,6 @@ use tracing::{info, trace, warn};
/// Optimism's payload builder
#[derive(Debug, Clone)]
pub struct OpPayloadBuilderVanilla<EvmConfig, Txs = ()> {
// /// The rollup's compute pending block configuration option.
// // TODO(clabby): Implement this feature.
// pub compute_pending_block: bool,
/// The type responsible for creating the evm.
pub evm_config: EvmConfig,
/// The builder's signer key to use for an end of block tx
Expand All @@ -74,7 +71,6 @@ impl<EvmConfig> OpPayloadBuilderVanilla<EvmConfig> {
/// `OpPayloadBuilder` constructor.
pub fn new(evm_config: EvmConfig, builder_signer: Option<Signer>) -> Self {
Self {
// compute_pending_block: true,
evm_config,
builder_signer,
best_transactions: (),
Expand All @@ -94,7 +90,7 @@ where

fn try_build(
&self,
args: BuildArguments<Pool, Client, Self::Attributes, Self::BuiltPayload>,
args: BuildArguments<Pool, Client, Self::Attributes>,
best_payload: BlockCell<Self::BuiltPayload>,
) -> Result<(), PayloadBuilderError> {
let pool = args.pool.clone();
Expand Down Expand Up @@ -145,7 +141,7 @@ where
/// a result indicating success with the payload or an error in case of failure.
fn build_payload<'a, Client, Pool, Txs>(
&self,
args: BuildArguments<Pool, Client, OpPayloadBuilderAttributes, OpBuiltPayload>,
args: BuildArguments<Pool, Client, OpPayloadBuilderAttributes>,
best: impl FnOnce(BestTransactionsAttributes) -> Txs + Send + Sync + 'a,
) -> Result<BuildOutcome<OpBuiltPayload>, PayloadBuilderError>
where
Expand All @@ -167,7 +163,6 @@ where
mut cached_reads,
config,
cancel,
best_payload,
} = args;

let ctx = OpPayloadBuilderCtx {
Expand All @@ -177,7 +172,6 @@ where
initialized_cfg: cfg_env_with_handler_cfg,
initialized_block_env: block_env,
cancel,
best_payload,
builder_signer: self.builder_signer,
metrics: Default::default(),
};
Expand Down Expand Up @@ -310,14 +304,6 @@ where
{
return Ok(BuildOutcomeKind::Cancelled);
}

// check if the new payload is even more valuable
if !ctx.is_better_payload(info.total_fees) {
// can skip building the block
return Ok(BuildOutcomeKind::Aborted {
fees: info.total_fees,
});
}
}

// Add builder tx to the block
Expand Down Expand Up @@ -567,8 +553,6 @@ pub struct OpPayloadBuilderCtx<EvmConfig> {
pub initialized_block_env: BlockEnv,
/// Marker to check whether the job has been cancelled.
pub cancel: CancellationToken,
/// The currently best payload.
pub best_payload: Option<OpBuiltPayload>,
/// The builder signer
pub builder_signer: Option<Signer>,
/// The metrics for the builder
Expand Down Expand Up @@ -692,11 +676,6 @@ impl<EvmConfig> OpPayloadBuilderCtx<EvmConfig> {
self.builder_signer
}

/// Returns true if the fees are higher than the previous payload.
pub fn is_better_payload(&self, total_fees: U256) -> bool {
is_better_payload(self.best_payload.as_ref(), total_fees)
}

/// Commits the withdrawals from the payload attributes to the state.
pub fn commit_withdrawals<DB>(&self, db: &mut State<DB>) -> Result<Option<B256>, ProviderError>
where
Expand Down

0 comments on commit 08f4fae

Please sign in to comment.