Skip to content

Conversation

@prestwich
Copy link
Contributor

Noticed compilation is broken with --no-default-features due to missing cfg(feature="optimism") in src/orderpool2/prioritized_pool/step.rs. This lead to adding missing CI steps, and noticing that documentation is also broken with default features and with --all-features

So:

  • Add missing CI steps:
    • clippy with --no-default-features
    • doc with --all-features and -D warnings
  • Resolve all errors and warnings
    • fix a bunch of doclinks
    • a few doc corrections (e.g. missed rename of RevertProtection)
    • mark an example non-rust, as it appears to be diagrammatic
  • small API change to aid doc fixes
    • Executable::execute_transaction and Executable::execute_bundle are now pub
      • this is safe as outside users already have unchecked access via Executable::execute

Copilot AI review requested due to automatic review settings December 26, 2025 18:32
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes compilation issues with --no-default-features and broken documentation, while adding CI steps to catch these issues in the future.

Key Changes:

  • Added CI jobs for clippy with --no-default-features and documentation building with --all-features
  • Fixed feature gating in src/orderpool2/prioritized_pool/step.rs to prevent compilation errors when optimism feature is disabled
  • Corrected numerous broken documentation links and improved API documentation

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
.github/workflows/sanity.yaml Added new CI jobs for clippy without default features and documentation validation
src/orderpool2/prioritized_pool/step.rs Added proper cfg(feature = "optimism") gates for Optimism-specific imports and implementations
src/payload/exec.rs Changed execute_transaction and execute_bundle visibility to pub and updated doc references to RemoveRevertedTransactions
src/test_utils/platform.rs Added proper doclink for rblib_test macro
src/test_utils/mod.rs Fixed Platform doclink and added explicit links for Ethereum and Optimism
src/test_utils/exts/mock.rs Fixed broken doclink to PipelineServiceBuilder and added link for PayloadJobGenerator
src/pool/setup.rs Improved documentation with concrete feature list instead of broken HostNode link
src/platform/optimism/ext.rs Added proper doclinks for Ecotone, Holocene, and Jovian hard forks
src/platform/mod.rs Fixed EthereumNode doclink and made OpNode link conditional on optimism feature
src/pipelines/step/context.rs Added proper doclink for BlockBuilder::apply_pre_execution_changes
src/pipelines/exec/scope.rs Changed code block from rust to text for diagrammatic content
src/pipelines/exec/mod.rs Added doclink for BlockBuilder::apply_pre_execution_changes
src/payload/ext/cached_state.rs Removed reference to non-existent CachedStateMetrics parameter
src/payload/block.rs Fixed doclink for PayloadJobGenerator::new_payload_job
src/lib.rs Changed doc comment to regular comment for conditionally compiled test_utils module

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Member

@julio4 julio4 left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks a lot for contributing

@karim-agha karim-agha merged commit 8768d1a into flashbots:main Dec 28, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants