-
Notifications
You must be signed in to change notification settings - Fork 208
Conversation
...ing rules - Unify reth git deps with arc-node (tag v1.11.3, same commit as previous rev pin) - New rbuilder 'arc' feature pulling arc-evm/arc-execution-config/arc-precompiles - chain module: cfg-switched ChainSpec/EvmConfig/DbNodeTypes/chain parsing - ArcCachedEvmFactory (no precompile cache: arc precompiles are stateful) - arc_support: ProtocolConfig gas limit, EIP-2935-only pre-block calls, post-block SystemAccounting gas values write + extra_data base fee encoding - finalize: chain-specific requests/extra_data handling asd Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- UnfinishedBlockBuildingSink(Factory) traits; MEV-Boost pipeline implements them - SlotSource enum: MevBoost generator or external channel (engine API driver) - EnginePayloadSink: finalizes best blocks into EthBuiltPayload via a watch-cell registry keyed by engine PayloadId (for engine_getPayload) - TimingsConfig::arc() for sub-second engine-driven slots - StateProviderFactory now requires Debug (needed for boxed sink factories) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
New binary launching ArcNode with the payload service wrapped by RbuilderPayloadServiceBuilder: - engine FCU payload jobs are forwarded to rbuilder as building slots (ProtocolConfig gas limit computed on parent state) - engine_getPayload resolves with rbuilder's best finalized block, falling back to the stock Arc payload builder for liveness - rbuilder consumes the node's txpool and provider directly (no relays, no bidding) - docs/ARC.md + example config Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- arc_support tests: named chain parsing, base fee decoded from parent extra_data + exact ProtocolConfig gas limit, EIP-7708 transfer log emitted through the rbuilder EVM factory - bundle refund/simulation tests encode Ethereum SELFDESTRUCT/refund semantics; ignored under the arc feature with reason - test fixtures wrap chain specs via chain::chain_spec_from_inner so the test build compiles for both flavors Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
...lding - rbuilder now uses the node's chain spec instead of the config 'chain' value (a mismatch made every order fail simulation with InvalidChainId) - log failed order simulations (trace) and non-critically discarded engine-payload blocks (debug) — both were previously silent - sim-path regression test executing a signed tx through the ctx EVM Verified end-to-end against arc-localdev driving the engine API like Malachite: FCU(attrs) -> rbuilder builds (mempool txs + payout) -> getPayload returns rbuilder's block -> newPayload validates it VALID (arc executor re-checks state root, ADR-004 extra_data base fee, SystemAccounting gas values) -> head advances to rbuilder's block. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude finished @SozinM's task in 3m 40s —— View job
PR Review: WIP arc
Reviewed the diff against the base branch. The chain-abstraction approach (crates/rbuilder/src/chain.rs compile-time swap, #[cfg(feature = "arc")] gating) is clean and keeps the Ethereum path untouched. Comments below are the issues worth attention; I posted 3 inline.
Blocking / high priority
- Workspace no longer builds without an external
arc-nodecheckout. The arc deps inCargo.toml(L194–201) are relativepathdeps into../../arc-node/, andcrates/arc-rbuilderis added todefault-members(L31). A fresh clone runningcargo check/make lint/make testwill fail for any contributor or CI runner without that sibling checkout. Thearcfeature itself is correctly optional — butdefault-membersundoes that isolation. Recommend droppingarc-rbuilderfromdefault-members(keep it inmembers) and/or pinning git revs instead of relative paths. (inline)
Correctness to verify
- Empty-requests hash on Prague-active Arc blocks (
building/mod.rsL955–959): the arc branch producesSome(Requests::default()), which emits arequests_hashin the header. If arc-node's assembler leaves itNone, headers won't be byte-identical and blocks get rejected. Worth a regression test pinning this field. (inline) expected_block_gas_limit/post_block_callswallow ProtocolConfig read errors (arc_support.rs) and fall back to defaults. This mirrors arc-node, but a divergence here silently produces a block with the wrong gas limit / base fee that the node will reject rather than fail loudly. Acceptable if intentional; flagging for awareness.
Performance
- Sink mutex held across
finalize_block(engine_payload_sink.rsL136–163): the lock spans state-root computation, so multiple builder threads serialize on finalization. For the sub-secondTimingsConfig::arc()path this caps finalization to one block at a time. Consider finalizing outside the lock and re-checkingbest_valueon commit. (inline)
Minor
block_output/mod.rs:pub mod engine_payload_sink;breaks the alphabetical module ordering (inserted beforebidding_service_interface).EnginePayloadSinkignoresbuilt_block_cache(cross-builder coordination cache) — fine if Arc builders don't use it, but worth a one-line note.- PR checklist (
make lint,make test, tests) is unchecked — given thedefault-membersissue above, please confirm these pass on a clean checkout.
Note: I confirmed gas_used passed to post_block_call includes the payout tx (payout is inserted in finalize_block_execution before partial_block.finalize → process_requests), and that merge_transitions runs after the post-block state write — both correct. Passing the full true_block_value as the payout value is consistent with the existing no_competition_bid pattern.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Workspace becomes unbuildable without an external checkout. These are path deps into a sibling ../../arc-node checkout, and crates/arc-rbuilder (which depends on them unconditionally) is in default-members (line 31). That means a plain cargo build / cargo check / make lint / make test at the repo root will fail for anyone — and CI — that doesn't have arc-node checked out at exactly that relative path. The arc feature on the rbuilder crate is properly optional, but adding arc-rbuilder to default-members defeats that isolation.
Consider one of:
- Remove
arc-rbuilderfromdefault-members(keep it inmembers), so default builds skip it. - Gate the whole arc workspace behind an env/profile, or use git deps with a pinned rev instead of relative
pathdeps.
Either way, please confirm cargo check on a fresh clone (no arc-node sibling) still succeeds before merge.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The state mutex is held across the entire finalize_block call, which includes state-root computation — the most expensive step. With multiple building-algorithm threads feeding this sink and sub-second Arc blocks (TimingsConfig::arc), every thread serializes on this lock during finalization, so effective finalization throughput is single-threaded.
The cheap best_value comparison must stay under the lock, but consider releasing it before finalize_block and re-acquiring to commit best_value (re-checking the bound after finalize, since another thread may have advanced it). local_ctx would need to move out of the shared state for that. Not blocking, but worth measuring for the fast-block path.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When Prague is active this yields Some(Requests::default()) (empty), producing a non-None requests_hash in the header (hash of an empty requests list). If arc-node's ArcBlockAssembler omits requests_hash entirely (sets it to None) for these blocks, the header will not be byte-identical and the block will be rejected. Please confirm arc-node sets the empty-requests hash here too (you mention E2E byte-identical verification — worth a regression test pinning this specific field across a Prague-active Arc block).
📝 Summary
💡 Motivation and Context
✅ I have completed the following steps:
make lintmake test