refactor(vara.eth/processor): apply MB transactions as an ordered list#5499
refactor(vara.eth/processor): apply MB transactions as an ordered list#5499grishasobol wants to merge 3 commits into
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request refactors the processor's execution pipeline to enforce strict transaction ordering. By consolidating disparate input fields into a unified, ordered list of transactions, the processor now processes events, injected transactions, and queue tasks in the precise sequence determined by the Malachite block. This change improves determinism and simplifies the execution logic within the processor. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request refactors the ethexe execution model to use an ordered list of ProcessorTransactions within ExecutableData, replacing the previous approach of handling events, injected transactions, and gas allowances in fixed sequential stages. This change allows the processor to apply transactions exactly as they were sequenced in a Malachite block. Feedback focuses on ensuring the promise_sink is handled robustly to support multiple queue-draining transactions per block and optimizing the compute layer to avoid emitting empty event transactions.
| // `take` hands the sink to this single (by MB shape) | ||
| // `ProcessQueues`, leaving `None` for any other. | ||
| self.process_queues( | ||
| transitions, | ||
| height, | ||
| timestamp, | ||
| limits.gas_allowance, | ||
| promise_sink.take(), |
There was a problem hiding this comment.
Using promise_sink.take() is brittle as it assumes only one ProcessQueues transaction exists per block. While the current sequencer might follow this "MB shape", the refactor's goal is to support an ordered list of transactions. If a block contains multiple ProcessQueues transactions (e.g., interleaved with other transactions), any injected transactions processed in subsequent drains will fail to emit promises because the sink was consumed by the first one. Using clone() (assuming BoundPromiseSink is clonable, which it should be as it wraps a channel sender) makes the processor more robust and truly generic.
self.process_queues(
transitions,
height,
timestamp,
limits.gas_allowance,
promise_sink.clone(),
)| events.extend(block_events.into_iter().filter_map(|e| e.to_request())); | ||
| } | ||
| current_anchor = block_hash; | ||
| processor_txs.push(ProcessorTransaction::EthereumEvents { events }); |
There was a problem hiding this comment.
Pushing a ProcessorTransaction::EthereumEvents when the events list is empty is unnecessary and adds a small overhead to the processor loop (which will create a ProcessingHandler only to do nothing). It's better to skip it if no events were collected during the advance walk.
| processor_txs.push(ProcessorTransaction::EthereumEvents { events }); | |
| if !events.is_empty() { | |
| processor_txs.push(ProcessorTransaction::EthereumEvents { events }); | |
| } |
298cbba to
af52969
Compare
Replaces the manual `#[cfg(test)] impl Default` with `#[cfg_attr(test, derive(Default))]`. Silences clippy::derivable_impls; behavior is identical (every field already has a `Default` impl).
Resolves #5501
Summary
injected_transactions,events,gas_allowance) onExecutableDatawith a single orderedVec<ProcessorTransaction>, so the processor applies MB transactions in the exact order the malachite block sequenced them.ethexe-computenow maps eachmalachite::Transaction1:1 into aProcessorTransaction, resolvingAdvanceTillEthereumBlockinto the concrete Ethereum events it pins (compute has DB access).Processor::process_programsiterates the transaction list dispatchingEthereumEvents/Injected/ProgressTasks/ProcessQueues;handle_injected_and_eventsis split intohandle_events+handle_injected. The promise sink istake()n by the singleProcessQueuestransaction.Notes
assert_migration_types_hashdoc explaining why migration types must stay byte-stable.db.rstype-info hash updated to reflect theExecutableDatashape change.Test plan
cargo nextest run -p ethexe-processorcargo nextest run -p ethexe-computecargo nextest run -p "ethexe-*" --no-fail-fast🤖 Generated with Claude Code