Feature/improve flashblocks test framework#3
Open
maoueh wants to merge 29 commits into
Open
Conversation
…mework
Introduces TestEvent{Flashblock, CanonicalBlock} to replace raw Vec<Flashblock>
in run_flashblock_sequence. CanonicalBlock events update GenesisClient's internal
available-block set so state_by_block_number_or_tag returns successfully for that
block number without WS traffic. Adds two new tests covering the provider bootstrap
path; all 12 integration tests pass.
# Conflicts:
# crates/firehose-flashblocks/tests/flashblock_sequence.rs
…_block return TestEvent directly - flash_base and flash_delta now return TestEvent instead of Flashblock, eliminating the need for TestEvent::flashblock(...) wrapping at call sites. - Add a module-level canonical_block free function mirroring flash_base and flash_delta, so all three helpers are uniform. - Add #[derive(Clone)] to TestEvent to support cloning in duplicate-base test. - Remove the now-unused TestEvent::canonical_block method and the unwrap_flashblock internal helper (the run_flashblock_sequence split loop is self-contained). - Rename all delta local variables throughout flashblock_sequence.rs to follow the delta<blockNum>_<flashIndex> convention (e.g. delta1_1, delta1_2, delta2_1). - Rename base local variables to base1, base2, etc. for consistency. - Update module-level doc comment to reflect the new helper signatures. # Conflicts: # crates/firehose-flashblocks/tests/flashblock_sequence.rs
959fa49 to
6aa7496
Compare
…and add canonical FIRE BLOCK emission - Fix GenesisClient::header_by_number to return a synthesised header with the correct block number so that next_evm_env computes block_env.number = parent.number + 1 correctly for blocks beyond genesis; add header_for_block helper. - Change run_flashblock_sequence to process events sequentially without WebSocket indirection: Flashblock events call on_flashblock_received directly; CanonicalBlock events both mark the block available in the provider AND emit a canonical FIRE BLOCK through a dedicated canonical tracer sharing the same buffer. This preserves strict ordering in the output so Flash1/Canonical1/Canonical2/Flash3 sequences are testable. - Remove ws_server_once and associated tokio-tungstenite/futures/url imports; tests are now plain #[test] instead of #[tokio::test]. - Add base_canonical_gap_then_base_emits_four_fire_blocks test: verifies the sequence base1 → canonical_block(1) → canonical_block(2) → base3 emits exactly four FIRE BLOCK lines (Flash1, Canonical1, Canonical2, Flash3). - Update canonical_block_unblocks_next_base and canonical_block_unblocks_non_sequential_gap to account for the canonical FIRE BLOCK now emitted alongside provider availability. All 13 integration tests pass; clippy is clean.
…k_hash and add hash-aware test framework
processor: stop sealing the per-flashblock SealedBlock with B256::ZERO. Use the
latest flashblock's diff.block_hash — the candidate-tip hash the sequencer commits
to in the wire payload (op-rbuilder's payload.rs:1099 computes it via seal_slow
and puts it into ExecutionPayloadFlashblockDeltaV1.block_hash). The FIRE BLOCK
line now carries a meaningful hash downstream for every base and delta emission,
matching the canonical-tracer behaviour.
test framework:
- flash_base/flash_delta/canonical_block builders now take a block_hash:
flash_base(block_number, block_hash, parent_hash, timestamp),
flash_delta(block_number, block_hash, index),
canonical_block(block_number, block_hash).
A hash(label: &str) helper (keccak256 of the label) lets tests use readable
labels like hash("1a"), hash("2a"), hash("3b").
- FireEvent::Block and FireEvent::FlashBlock gained a block_hash: B256 field;
the parser extracts it from the FIRE BLOCK line. Assertion constructors
FireEvent::canonical_block(N, hash) and FireEvent::flash_block(N, hash, idx,
is_final) take the expected hash and the metadata comparator checks it exactly.
- run_flashblock_sequence now writes the two tracers to separate buffers and
merges them into the output with # SOURCE FLASH / # SOURCE CANON marker lines.
The parser tracks the current source via those markers so flashblock-tracer
lines (any flash_idx, including the base at 0) parse as FireEvent::FlashBlock
and only canonical-tracer lines parse as FireEvent::Block. Removes the prior
wire-level ambiguity where flash_idx==0 mapped to FireEvent::Block regardless
of source.
tests: all 13 existing tests updated to the new API; flash_base outputs are now
asserted as FireEvent::flash_block(N, hash, 0, false).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…e execution
- Validate an incoming base's parent_hash at three moments in the lifecycle:
1. on receipt, against the latest known canonical hash for the parent block;
2. on receipt, against the in-flight flashblock chain's tip when no canonical for
the parent has been observed yet;
3. on a later canonical-block notification, against the parent_hash recorded in a
buffered (pending) base.
Any mismatch discards the base and any deltas that would have followed.
- Sequential fast path: when an incoming base's parent_hash matches the in-flight
tip, execute on top of the accumulated in-memory state carried forward from the
previous block — no provider lookup needed.
- Validate that every delta's payload_id matches the in-flight base's payload_id;
a mismatch is treated as a non-sequential gap and resets the sequence.
- Drop flashblocks whose timestamp is more than 5 seconds behind the processor's
clock; the clock is injectable for tests, defaults to wall-clock time in prod.
Processor changes (crates/firehose-flashblocks/src/processor.rs):
- Track `latest_canonical: Option<(u64, B256)>` (sticky across resets) used by checks
(1) and (3) above.
- After a replay triggered by `on_canonical_block`, set
`awaiting_canonical_confirmation = true`. The next FirstOfNextBlock transition is
deferred (pending) until a canonical confirms or contradicts the replayed tip; on
contradiction the buffered next-block flashblocks are discarded.
- New `ClockFn` type alias and `with_clock` constructor; `new` keeps wrapping
`SystemTime::now()` so production behaviour is unchanged.
Test framework (crates/firehose-flashblocks/tests/framework/mod.rs):
- `flash_delta_with_payload_id(...)` helper for exercising payload-id mismatch.
- Runner uses per-tracer buffers merged with `# SOURCE FLASH` / `# SOURCE CANON`
marker lines so the parser can distinguish base-flashblock emissions from canonical
ones (both have `flash_idx == 0` on the wire).
- `run_flashblock_sequence_at(client, events, now_secs)` pins the processor clock;
the default `run_flashblock_sequence` uses the minimum event timestamp so the
saturating age calculation keeps every fixture event at age 0.
Tests added:
- `base_canonical_after_flashblock_flushes_buffer` — replayed-then-contradicted block
3 must discard buffered block 4.
- `base_ordered_wrong_hash` — base whose parent disagrees with a prior canonical is
discarded.
- `base_plus_delta_plus_next_base_but_no_state_yet` — sequential fast path on the
in-memory accumulated_db when parent matches the in-flight tip.
- `delta_with_wrong_payload_id_is_discarded` — payload-id mismatch resets the
sequence.
- `stale_flashblock_is_skipped` — staleness gate drops a flashblock 10 s in the past.
is_final * Trait extension: FlashblocksReceiver::on_flashblock_received_with_peek passes the next queued flashblock as a peek hint (1-element buffer added to FlashblocksSubscriber's consumer task). * Squash: consecutive same-block deltas accumulate in stored_flashblocks but skip EVM execution and FIRE BLOCK emission; the next non-squashed flashblock executes with all held-back transactions gathered in one pass (last_executed_index tracks the boundary). * Single-emission is_final: when peek shows the next-block base, the current flashblock executes once, then the post-execution state_root is computed from the revm bundle, the header is re-sealed via hash_slow, and the tracer's in-progress block is overridden through firehose-tracer 5.2.0's new Tracer::set_final_flash_block. The resulting FIRE BLOCK carries the recomputed hash and the +1000 printed index — no separate non-final + is_final pair. * Hash mismatch on the peek-driven is_final path drops the FIRE BLOCK at the wire layer (mark_failed) and resets state, matching geth's Skipping=true recovery; final_part_sent suppresses the FirstOfNextBlock fallback re-emission to avoid duplicates.
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
* Replay path in `on_canonical_block` now stamps
`last_executed_index = stored.last().index` after executing the
buffered flashblocks. Without this, the next delta arriving through
`process_inner` saw `last_executed_index = None`, the multi-delta
tx-gather filter included every previously-replayed flashblock's
transactions in the EVM call, and the second pass tripped
"nonce too low". Observed in prod at block 46517112.
* `execute_flashblock` now calls
`snapshot_flash_block_for_next_iteration` after the EVM run on
non-final iterations, matching geth's
`eth/tracers/firehose.go:180`. Without it, `on_block_start` cleared
the in-progress block on every iteration and each emitted FIRE
BLOCK carried only the current delta's traces — the wire stream
diverged from the cumulative-per-index contract every downstream
firehose consumer expects.
Both bugs covered by new regression tests
(`canonical_replay_stamps_last_executed_index`,
`snapshot_carries_cumulative_traces_across_replay`); the framework
gained a test-only `RunResult { raw, processor }` returned by
`run_flashblock_sequence_at_with_processor` and the processor gained
a `last_executed_index_for_test()` accessor to support whitebox
assertions.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…y loop
Prod panic at firehose-tracer:1662:
flash block index must be strictly greater than previous:
block=46518875 last_idx=4 new_idx=2
Race between the canonical-state subscription task (running replay)
and the WS subscriber's consumer task (delivering new flashblocks):
1. Several flashblocks for block N+1 buffer while block N is still
pre-canonical (`pending_state = true`).
2. Canonical for N arrives. The replay loop clones stored, drops
state, and starts iterating `execute_flashblock` per buffered
flashblock; the tracer mutex is released between iterations.
3. The WS task grabs state, validates + pushes a freshly-arrived
flashblock, then races into `execute_flashblock` for it.
4. Its `start_flashblock_local` advances the tracer's snapshot
index past the replay's; the next replay iteration trips the
tracer's "strictly greater than previous" check.
Prod trace matches: emit idx=0 (replay i=0), emit idx=1 (replay
i=1), emit idx=4 (WS racing in), panic on replay i=2.
Fix: hold `self.state.lock()` for the ENTIRE replay loop instead
of dropping it before iterating. `execute_flashblock` was already
designed to not touch `self.state` (only `self.tracer`), so there
is no deadlock risk. Concurrent `process_inner` calls block on
`state.lock()` until replay completes — strict mutual exclusion via
the natural mutex primitive, not a soft boolean defer flag.
A boolean (e.g. keeping `pending_state = true` for the loop) would
have been fragile: `pending_state` is also touched by `start_block`
(via `FirstOfNextBlock` / `InvalidNewBlockIndex{0}`) and `reset`
(via `NonSequentialGap`), so a concurrent `process_inner` could
clear it mid-replay and reopen the race window.
Cost: the WS consumer task waits on `state.lock()` during replay.
The upstream mpsc channel (capacity 100) absorbs incoming
flashblocks without dropping; once the loop ends and state is
consistent again, the WS task drains its backlog through the
normal squash + multi-delta tx-gather path.
No regression test added: reproducing the race requires preempting
`execute_flashblock` mid-tracer-call between two real OS threads,
which is nondeterministic and would only flag the regression with
some probability per run. The fix is small and structural; existing
canonical-replay tests cover the post-replay state invariants.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
ca6c51f to
3d4760b
Compare
… canonical Two prod-uncovered bugs in the parent-hash sanity / canonical confirmation flow: 1. The in-flight tip check in process_inner compared `stored.last().diff.block_hash` (the sequencer's wire-reported hash, computed with `state_root = ZERO` for unsealed flashblocks) against the new base's `parent_hash` (the real canonical hash, computed with the post-execution state root). Those always differ in normal op-rbuilder operation, so every next-block base whose canonical commit hadn't yet propagated to `latest_canonical` was rejected — block 46525453's flashblocks were lost in prod. 2. The old `on_canonical_block` Case 2 only checked `awaiting_canonical_confirmation` (the replay case) and used the same broken `diff.block_hash` comparison. There was no path for "canonical(N) arrives before the next-base flashblock for N+1 does" to emit is_final on the flashblock tracer stream. Fixes: * Remove the broken in-flight tip check. The rigorous version still runs inside the `FirstOfNextBlock` branch via `build_is_final_emission`, which recomputes the previous block's hash via `Header::hash_slow(computed_state_root)` and compares against the new base's `parent_hash`. * Refactor `on_canonical_block` to perform the same recompute and emit is_final for the in-flight block when canonical confirms it — covering BOTH "real-time executing" and "awaiting after replay" sub-cases. Either ordering (next-base first or canonical first) now produces the same single is_final FIRE BLOCK on the wire; `final_part_sent` prevents double emission when both signals arrive. * `build_is_final_emission` now stamps `final_index = latest_idx + 1` for the fallback re-emission path. The previous execute_flashblock call stamped the tracer's snapshot at `flash_index = latest_idx`, so reusing the same index would trip the tracer's "strictly-greater-than-previous" check. Mirrors geth's `c.state.CurrentIndex++` at controller.go:298. The `restore_flash_block_snapshot` on the tracer side picks up the cumulative trace from the previous iteration, so the FIRE BLOCK still carries `0..=latest_idx` of transaction traces. Tests: * `next_base_accepted_when_delta_diff_block_hash_diverges_from_recompute` — exercises the prod scenario via the peek path: `delta.diff.block_hash` is set to a wire-like value different from the recompute; block 2's base.parent_hash is the canonical/recomputed hash. The old check would have rejected block 2's base; the fix lets the recompute path validate correctly. * `canonical_emits_is_final_when_no_subsequent_flashblock` — same fixture but with `canonical_block(1, recompute)` instead of a next-block base. Exercises the new `on_canonical_block` Flow 1. * `canonical_with_wrong_hash_resets_state` — the mirror: canonical arrives with a hash that doesn't match the recompute → state reset, no is_final emission. * `next_base_accepted_without_peek_when_recompute_matches` — exercises the FirstOfNextBlock fallback (no peek hint) where the is_final emission goes through `emit_final_if_pending` with the incremented `final_index`. A new `run_flashblock_sequence_without_peek` runner feeds flashblocks through `on_flashblock_received` (no peek hint) to reproduce the production slow-drip case where the WS subscriber's peek slot is empty between deliveries. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…stic fields Prod symptom (block 46530250 idx=0 dropped, deltas hit "no in-flight sequence" warnings): when the FirstOfNextBlock recompute of block N diverged from the new base's parent_hash, we set state.reset() and returned early — dropping the new base and breaking the wire stream for every subsequent flashblock of block N+1 until a much later restart point. Behavior change: on hash mismatch we still reset (the previous block's accumulated EVM bundle proved divergent from canonical and must be discarded), but we now fall through to start_block(flashblock) so the new base is recorded. With accumulated_db now None, the execute path either re-bootstraps from canonical for the new block's parent (when that canonical is already known) or buffers the sequence in pending_state until the canonical notification for the parent arrives; Flow 2 of on_canonical_block then replays the buffered flashblocks naturally. We diverge from geth's `Skipping = true` here because the recompute mismatch can be a false positive in our setup — delivery losing tail flashblocks or header-field disagreements between our BlockAssembler and reth's canonical sealing — and dropping the entire next block is too aggressive a response. The on_canonical_block Flow 1 mismatch path still resets without recovery: it's just a confirmation signal with no new base to accept; the next incoming flashblock will bootstrap fresh. Diagnostic logging: build_is_final_emission now returns a structured BuildIsFinalError::HashMismatch carrying the recomputed hash, computed state_root, the assembled header hash WITHOUT the state_root override (i.e. with the sequencer's wire diff.state_root left in place), the flashblock count, and the cumulative transaction count. The caller's warn! surfaces each as a tracing field so a single prod log line can localise whether the disagreement is in state_root, in another header field, or in delivery (missing tail flashblocks vs. canonical's `txs=N`). Test: `next_base_mismatch_recovers_via_pending_and_canonical_replay` runs the no-peek runner over the prod scenario (delta's diff.block_hash ≠ recompute, new base's parent_hash ≠ recompute, then canonical matches the new base's parent_hash). Asserts that block N+1's base and delta both reach the wire via the pending-buffer → canonical-replay path. Without the fix the new base was dropped at step 3 and neither block N+1 emission would have landed. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…ate_root Prod symptom: every is_final recompute disagreed with canonical. The `computed_state_root` reported in the mismatch warn line was *exactly the parent block's state_root* — for block 46544695 we computed 0x810064777c8ec558… which is the canonical state_root of 46544694. Root cause: revm's `State<DB>` accumulates per-tx state changes in its `cache` / `transition_state` and only promotes them into `bundle_state` when `merge_transitions` is explicitly called. Our `compute_state_root` reads the bundle (`provider.hashed_post_state(&db.bundle_state)`), so without an intervening merge it sees an empty bundle and `StateProvider::state_root` returns the underlying provider's root — i.e. the parent block's pre-execution state_root. Fix: call `accumulated_db.merge_transitions(BundleRetention::Reverts)` after `executor.finish()` so the bundle reflects every executed flashblock's effects by the time `build_is_final_emission` queries it. Mirrors the existing pattern in `crates/execution/flashblocks/src/processor.rs:488`. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…across flashblocks Bumps `firehose-tracer` to <new-version>, which adds three cumulative-across-block offsets (`flashblock_tx_index_offset`, `flashblock_cumulative_gas_offset`, `flashblock_log_block_index_offset`) that rebase the per-iteration receipt fields revm produces into the canonical-across-block values the FIRE BLOCK protobuf is supposed to carry. Removes the local `[patch.crates-io]` path override. Adds `tracer_offsets_carry_cumulative_tx_fields_across_flashblocks`: sends base + delta(tx@nonce=0) + delta(tx@nonce=1) through the no-peek runner and asserts that the third FIRE BLOCK's `transaction_traces[1]` carries the canonical-across-block `index = 1` and `cumulative_gas_used = 42_000` — not the per-iteration `0` and `21_000` values revm hands back. Verified to fail when the tracer-side offsets are reverted. Adds three small dev-deps used only by the new test fixture: `alloy-signer`, `alloy-signer-local`, `alloy-network` (for `TxSignerSync::sign_transaction_sync`).
…e-time debug logs
Two diagnostic improvements so a prod log alone tells you which
is_final path fired and when each flashblock arrived from the WS
subscriber.
Adds debug-level logs at two key handoff points:
* `flashblocks_rpc::subscription` consumer task: one log per
received flashblock with its block/index and the peek slot's
block/index. Surfaces WS arrival timing and whether peek caught
the next one.
* `firehose-flashblocks::process_inner` entry: one log with the
squash flag and the is_final-expected-hash classification, so the
path taken by `classify_peek` is visible.
Renames the three is_final emission info messages so they're
unambiguously distinguishable:
* peek-driven inline path (`execute_flashblock` →
`set_final_flash_block`, single FIRE BLOCK, no idx increment):
"peek-driven is_final emitted inline"
* FirstOfNextBlock fallback (`build_is_final_emission` →
`emit_final_if_pending`, idx = latest + 1 to clear the tracer's
strictly-greater snapshot check):
"FirstOfNextBlock-fallback is_final emitted"
* `on_canonical_block` Flow 1 (canonical confirms the in-flight
block):
"canonical-driven is_final emitted"
The fallback's `final_index = latest_idx + 1` is also a useful tell:
when the log shows `final_index = K + 1` over a prior emit at
`index = K`, you're on the fallback path; equal indices mean the
peek-driven inline path.
To enable in prod:
RUST_LOG=base_firehose_flashblocks=debug,base_flashblocks=debug
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
earliest is_final signal Plumb the consensus engine's ConsensusEngineEvent broadcast through BaseAddOns -> NodeHooks::add_engine_event_listener_hook, and wire FirehoseFlashblocksExtension to drive on_canonical_block from CanonicalBlockAdded — which fires inside the engine before the canonical-state notification — falling back to subscribe_to_canonical_state
FIRE BLOCK on canonical replay When the canonical-driven replay path flushes buffered flashblocks (block N+1 arrived before block N became canonical), assemble the entire buffered slice once, gather all transactions across base + deltas into a single EVM execution, and emit one FIRE BLOCK stamped at the highest buffered index. Downstream firehose consumers only need the state-correct partial; per-flashblock replay emissions re-traversed the same EVM work and streamed redundant intermediate partials.
…ase#2936) * fix(consensus): recover pruned forkchoice checkpoints Persist safe and finalized forkchoice checkpoints in the consensus service so validator restarts can recover when reth has pruned historical block bodies. Use checkpoints only after the specific missing L1 info deposit error and validate them against the reth-labeled block header before sync start accepts them. Co-authored-by: Codex <noreply@openai.com> (cherry picked from commit 0bfc265) * chore(succinct): refresh ELF manifest to match main The reproducible SP1 ELF hashes pinned in manifest.toml drifted from what the build environment currently produces. Main already updated these hashes (commit a052beb); this brings the v0.9.1 backport branch in line so CI can verify reproducibility. Unrelated to the checkpoint backport itself - none of the diff in this branch touches the SP1 program source or its dependency tree. * chore: update succinct manifest and program cargo lock * fix(execution): use LenientRpcModuleValidator to allow base RPC namespace The default RpcModuleValidator rejects unknown namespaces, including 'base'. When operators include 'base' in --http.api, the node crashlooped with 'Invalid RPC module base in http.api: Unknown RPC module: base'. Switch the default Rpc generic on Cli to LenientRpcModuleValidator so operator-configured custom namespaces (like 'base') are accepted. * fix(consensus): recover to earliest unpruned block on pruned forkchoice When base-consensus starts from a GBS snapshot with a stale safe/finalized head whose block body has been pruned, and no forkchoice checkpoint exists, instead of crashing with MissingL1InfoDeposit, binary search between the pruned block and the latest block to find the earliest unpruned block and use that as the recovery point. For safe head: falls back to the recovered finalized value. For finalized head: binary searches for the prune boundary. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai> --------- Co-authored-by: Andreas Bigger <andreas.bigger@coinbase.com> Co-authored-by: Codex <noreply@openai.com> Co-authored-by: Mihir Wadekar <mwadekar2000@gmail.com> Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
10 After execute_flashblock returns for any flashblock at or above idx 10, spawn a tokio::spawn_blocking task that runs hashed_post_state + state_root against a cloned BundleState. When the next-block base arrives and triggers is_final, the peek-driven path checks the cache (keyed by bundle revision) and skips the synchronous trie traversal on hit. Off the critical path the spec runs during the inter-flashblock ~200 ms gap and lands the result ahead of the next message, saving 100-150 ms on is_final emission in the common case where the final flashblock arrives with no new transactions. execute_flashblock now returns whether the bundle actually changed so the caller can drive the revision bump; the on_canonical_block merged-replay caller holds the state mutex for its loop and would otherwise deadlock.
Release v0.9.1
…ered a full block
header lookup races canonical commit State availability and header availability can diverge briefly when a flashblock for block N+1 arrives between the parent's payload-insert (which makes state queryable) and the canonical-chain commit (which writes the header). The header lookup in execute_flashblock then errors with "parent header missing"; the outer process caught the error and called state.reset(), dropping the buffered base and causing subsequent indices for the same block to be lost as "no in-flight sequence". Add an explicit header_by_number pre-check in process_inner after the state bootstrap. On miss, mark the sequence pending so the canonical- block notification replays it once the header lands.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.