[MEL] - Implement a Unified Replay Binary for MEL#4553
[MEL] - Implement a Unified Replay Binary for MEL#4553rauljordan wants to merge 18 commits intomasterfrom
Conversation
…ro into raul/mel-unified-binary
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #4553 +/- ##
==========================================
- Coverage 34.16% 33.99% -0.18%
==========================================
Files 494 499 +5
Lines 58932 59271 +339
==========================================
+ Hits 20137 20152 +15
- Misses 35249 35581 +332
+ Partials 3546 3538 -8 |
❌ 15 Tests Failed:
View the top 3 failed tests by shortest run time
📣 Thoughts on this report? Let Codecov know! | Powered by Codecov |
…ro into raul/mel-unified-binary
…ro into raul/mel-unified-binary
| fn bytes32_last_non_zero_index(&self) -> usize { | ||
| let last_non_zero_idx = self | ||
| .bytes32_vals | ||
| .iter() | ||
| .enumerate() | ||
| .rev() | ||
| .find(|&(_, &val)| val != Bytes32::default()) | ||
| .map(|(i, _)| i); | ||
|
|
||
| match last_non_zero_idx { | ||
| Some(idx) => std::cmp::max(1, idx), | ||
| None => 1, | ||
| } | ||
| } |
There was a problem hiding this comment.
[Critical] This backward-compat hashing trick is consensus-load-bearing: a legacy 2-slot state [H, S] must hash identically to a 4-slot state [H, S, 0, 0]. The logic here looks correct, but there are no tests asserting this invariant anywhere in crates/prover/src/ — crates/prover/src/test.rs contains only reject_reexports, reject_ambiguous_imports, and test_compress; none touch GlobalState.
A one-character regression here (e.g. max(1, idx) → max(2, idx), or off-by-one on 0..=end_idx) silently invalidates every existing assertion proof, and the failure mode only surfaces during a challenge.
Please add a golden-vector unit test covering at minimum:
- all-zeros
- only slot 0 set
- only slot 1 set
- slot 2 set (new behavior, intermediate zeros preserved)
- slot 3 set
- legacy-vs-new equality:
hash({a, b, 0, 0}) == hash_legacy({a, b})
~15 lines of Rust — cheapest possible risk reduction for a consensus invariant.
| "github.com/offchainlabs/nitro/wavmio" | ||
| ) | ||
|
|
||
| func main() { |
There was a problem hiding this comment.
[Critical] This PR adds ~800 lines of consensus-adjacent code (cmd/unified-replay/ + melwavmio/ + the new GetEndParentChainBlockHash WAVM opcode) with zero new test coverage. The melwavmio/stub.go build-tag split exists specifically to allow native (non-WASM) testing — that infrastructure is already in place, it's just not being used.
At minimum, please consider:
- One Go integration test driving
produceBlockandextractMessagesUpTothrough the stub, seeded from the same preimage fixtures used bycmd/replay. - A Rust interpreter test for the new
GetEndParentChainBlockHashopcode (nothing currently exercises thestore_slice_alignedpath atmachine.rs:2519).
A bug that only surfaces inside the prover is very expensive to debug.
| } | ||
|
|
||
| func StubFinal() { | ||
| log.Info("endMelStateHash", endMelStateHash.Hex()) |
There was a problem hiding this comment.
[Important] go-ethereum's log.Info expects msg, key, value…. As written, endMelStateHash.Hex() is being parsed as a bare key with no value — the logger will emit a "missing value" formatter error rather than the hash you want to see.
| log.Info("endMelStateHash", endMelStateHash.Hex()) | |
| log.Info("final MEL state hash", "endMelStateHash", endMelStateHash.Hex()) |
| newBalance, err := builder.L2.Client.BalanceAt(ctx, txOpts.From, nil) | ||
| if err == nil && newBalance.Cmp(expectedBalance) == 0 { | ||
| break | ||
| } | ||
| select { | ||
| case <-tick.C: | ||
| case <-timeout.C: | ||
| t.Fatalf("timed out waiting for MEL and TransactionStreamer to detect reorg") | ||
| latestBalance, _ := builder.L2.Client.BalanceAt(ctx, txOpts.From, nil) | ||
| t.Fatalf("timed out waiting for balance to reflect reorg handling: got %v, want %v (MEL reorg logged=%v, TxStreamer reorg logged=%v)", | ||
| latestBalance, expectedBalance, | ||
| logHandler.WasLogged("MEL detected L1 reorg"), | ||
| logHandler.WasLogged("TransactionStreamer: Reorg detected!")) | ||
| } |
There was a problem hiding this comment.
[Important] This polling loop silently swallows every BalanceAt error on each 100ms tick, and the timeout message at line 646 discards the error too (latestBalance, _). Pre-PR, the test fail-loud on RPC errors via t.Fatalf("BalanceAt... unexpected error: %v", err) — that signal is now lost, so a broken RPC client becomes an opaque 1-minute timeout.
Suggest tracking lastErr across ticks and surfacing it in the timeout message, e.g.:
var lastErr error
for {
newBalance, err := builder.L2.Client.BalanceAt(ctx, txOpts.From, nil)
if err == nil && newBalance.Cmp(expectedBalance) == 0 { break }
if err != nil { lastErr = err }
select {
case <-tick.C:
case <-timeout.C:
latestBalance, balErr := builder.L2.Client.BalanceAt(ctx, txOpts.From, nil)
t.Fatalf("... got %v (balErr=%v, lastPollErr=%v), want %v ...",
latestBalance, balErr, lastErr, expectedBalance, ...)
}
}| return currentState | ||
| } | ||
|
|
||
| // Extracts all block header hashes in the range from startHash to endHash. |
There was a problem hiding this comment.
[Important] The doc is inclusivity-ambiguous. The actual semantics are half-open (startHash, endHash] — endHash is included, startHash is excluded. Two edge cases are also silently undocumented: when startHash == endHash it returns nil, and on line 304 it panics if it walks past genesis without finding startHash.
Suggest:
// walkBackwards returns the parent-chain header hashes in the half-open range
// (startHash, endHash], ordered from endHash back to startHash's child.
// Returns nil if startHash == endHash.
// Panics if walking back reaches the zero hash without finding startHash.| /// Sets the ending parent chain block hash used for a machine when executing message extraction | ||
| /// algorithms. |
There was a problem hiding this comment.
[Suggestion] Thin for an FFI boundary. Doesn't document: what "end" means (inclusive upper bound? target to walk to?), that *const Bytes32 must be a readable 32-byte region the callee dereferences and copies, or that null/unaligned is UB. FFI contracts generally warrant explicit # Safety sections — suggest adding one, plus a sentence clarifying that "end parent chain block hash" is the inclusive end of the parent-chain block range over which MEL message extraction executes.
| // Globalstate holds: | ||
| // bytes32 - last_block_hash | ||
| // bytes32 - send_root | ||
| // bytes32 - mel_state_hash |
There was a problem hiding this comment.
Optional(nit): Naming inconsistency between Rust and Go sides — Rust calls this slot mel_state_hash, while melwavmio/higher.go:23 calls it IDX_MEL_ROOT, with GetStartMELRoot/SetEndMELRoot accessors. Indexes line up (0/1/2/3 on both sides) but a reader jumping between files has to translate. Happy either way, just flagging.
| /// Finds the last non-zero index in the bytes32 values and returns | ||
| /// the max of it and 1. This is used to determine how many | ||
| /// bytes32 values to include in the hash and serialization of a global state, | ||
| /// and at least the first two values must be included for backwards compatibility. |
There was a problem hiding this comment.
[Suggestion] Doc is technically accurate but incomplete: when every bytes32 value is zero, find returns None and the function returns 1, not 0. The current comment doesn't cover that branch. Suggest:
/// Returns the index of the last non-zero bytes32 value, or 1 if all values
/// past index 1 are zero. Always returns at least 1, so the first two slots
/// (block_hash, send_root) are always serialized — preserving the pre-MEL
/// GlobalState hash format for backwards compatibility.
| // Globalstate holds: | ||
| // bytes32 - last_block_hash | ||
| // bytes32 - send_root | ||
| // bytes32 - mel_state_hash |
There was a problem hiding this comment.
[Suggestion] The // Globalstate holds: line just above should match the type name GlobalState on line 821 — tiny typo worth fixing while this comment block is being touched.
| var hash [32]byte | ||
| copy(hash[:], key) | ||
| if len(key) == 32 { | ||
| copy(hash[:], key) | ||
| } else if len(key) == len(rawdb.CodePrefix)+32 && bytes.HasPrefix(key, rawdb.CodePrefix) { | ||
| // Retrieving code | ||
| copy(hash[:], key[len(rawdb.CodePrefix):]) | ||
| } else { | ||
| return nil, fmt.Errorf("preimage DB attempted to access non-hash key %v", hex.EncodeToString(key)) | ||
| } |
There was a problem hiding this comment.
[Suggestion] The unconditional copy(hash[:], key) on line 34 is dead code: if len(key) == 32, line 36 repeats the same copy; if it's the code prefix, line 39 overwrites; in the else branch at line 40 we return before hash is ever used. This mirrors the same pattern in cmd/replay/db.go:33-42, so it's pre-existing rather than introduced here — but since the file is brand new, good moment to drop the redundant line.
3 participants
This PR brings over changes from #4152 into master.
Summary
unified-replaybinary (cmd/unified-replay/) that combines both MEL (Message Extraction Layer) message extraction and block production into a single WASM-compilable replay programGetEndParentChainBlockHashhost I/O opcode to support MEL provingmelwavmiopackage providing WASM imports and native stubs for MELHow it Works
The unified replay binary is quite easy to understand in pythonic pseudocode. It essentially either runs message extraction or block production depending on some of the input or computed values. This means that validation of Arbitrum becomes a message extraction followed by execution of those extracted messages:
Changes
New files
cmd/unified-replay/main.go— Unified replay entry point handling two modes: block production (when a MEL message hash is present) and message extraction (walking parent chain blocks backward to extract messages)cmd/unified-replay/db.go— Preimage-backed database adapter for state trie lookups during replaycmd/unified-replay/preimage_resolver.go— WAVM preimage resolver and blob reader implementationsmelwavmio/— MEL-specific WAVM I/O package with WASM importsModified files
GlobalStateto 4 bytes32 slots with backward-compatible hashing, addedGetEndParentChainBlockHashopcode, and made binary output filenames configurablebuild-unified-replay-envandbuild-unified-wasm-bintargets