Skip to content

fix(consensus): mark future timestamps as transient #133

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
panos-xyz wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fix(consensus): mark future timestamps as transient #133

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7f44d69
fix(consensus): mark future timestamps as transient
panos-xyz Jun 16, 2026
cc0c65d
fix(engine-api): pass through rpc errors
panos-xyz Jun 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions crates/consensus/src/validation.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,10 @@ impl Consensus<Block> for MorphConsensus {

Ok(())
}

fn is_transient_error(&self, error: &ConsensusError) -> bool {
matches!(error, ConsensusError::TimestampIsInFuture { .. })
}
Comment on lines +332 to +334

@coderabbitai coderabbitai Bot Jun 16, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Verify matches! macro usage and check for syntax issues

# Check Rust version and toolchain
rustc --version || echo "rustc not available"

# Search for other uses of matches! in the codebase to see if it's used elsewhere
rg -n 'matches!\(' --type=rust -C2

# Check if there are any known issues with matches! in similar contexts
ast-grep --pattern $'impl Consensus<$_> for $_ {
  $$$
  fn is_transient_error(&$_, error: &ConsensusError) -> bool {
    $$$
  }
  $$$
}'

Repository: morph-l2/morph-reth

Length of output: 25311

🏁 Script executed:

# Check the test implementation at lines 1138-1148
sed -n '1138,1148p' crates/consensus/src/validation.rs

# Search for other tests or usages of is_transient_error
rg -n 'is_transient_error' crates/consensus/src/validation.rs

Repository: morph-l2/morph-reth

Length of output: 663

The matches! macro warning is a false positive; add a test for negative cases.

The implementation is correct—matches! is used successfully throughout the codebase including in this same file. However, the test only verifies that TimestampIsInFuture is transient. Add a test case asserting that other error variants (e.g., ConsensusError::ExtraDataExceedsMax) return false from is_transient_error.

🧰 Tools
🪛 GitHub Actions: PR `#133` / 0_Analyze (rust).txt

[warning] 333-333: Rustc warning: macro expansion failed for 'matches' (macro expansion failed)

🪛 GitHub Actions: PR `#133` / Analyze (rust)

[warning] 333-333: Rust warning: macro expansion failed for 'matches'.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/consensus/src/validation.rs` around lines 332 - 334, The
is_transient_error method implementation is correct, but the existing test only
covers the positive case where TimestampIsInFuture is transient. Add additional
test cases to the test for is_transient_error that verify negative scenarios,
asserting that other ConsensusError variants such as ExtraDataExceedsMax return
false from is_transient_error. This ensures the method correctly identifies
which errors are transient and which are not.

}

// ============================================================================
Expand Down Expand Up @@ -1131,6 +1135,18 @@ mod tests {
));
}

#[test]
fn test_timestamp_in_future_is_transient_error() {
let chain_spec = create_test_chainspec();
let consensus = MorphConsensus::new(chain_spec);
let error = ConsensusError::TimestampIsInFuture {
timestamp: 2,
present_timestamp: 1,
};

assert!(Consensus::<Block>::is_transient_error(&consensus, &error));
}

#[test]
fn test_validate_header_gas_limit_exceeds_max() {
let chain_spec = create_test_chainspec();
Expand Down
44 changes: 16 additions & 28 deletions crates/engine-api/src/rpc.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,10 +118,10 @@ where
) -> RpcResult<ExecutableL2Data> {
tracing::debug!(target: "morph::engine", block_number = params.number, "assembling L2 block");

self.inner.assemble_l2_block(params).await.map_err(|e| {
tracing::error!(target: "morph::engine", error = %e, "failed to assemble L2 block");
e.into()
})
self.inner
.assemble_l2_block(params)
.await
.map_err(|e| e.into())
}

async fn assemble_l2_block_v2(
Expand All @@ -138,10 +138,7 @@ where
self.inner
.assemble_l2_block_v2(params)
.await
.map_err(|e| {
tracing::error!(target: "morph::engine", error = %e, "failed to assemble L2 block (v2)");
e.into()
})
.map_err(|e| e.into())
}

async fn validate_l2_block(&self, data: ExecutableL2Data) -> RpcResult<GenericResponse> {
Expand All @@ -152,10 +149,10 @@ where
"validating L2 block"
);

self.inner.validate_l2_block(data).await.map_err(|e| {
tracing::error!(target: "morph::engine", error = %e, "failed to validate L2 block");
e.into()
})
self.inner
.validate_l2_block(data)
.await
.map_err(|e| e.into())
}

async fn new_l2_block(&self, data: ExecutableL2Data) -> RpcResult<()> {
Expand All @@ -166,10 +163,7 @@ where
"RPC newL2Block called"
);

self.inner.new_l2_block(data).await.map_err(|e| {
tracing::error!(target: "morph::engine", error = %e, "failed to import L2 block");
e.into()
})
self.inner.new_l2_block(data).await.map_err(|e| e.into())
}

async fn new_l2_block_v2(&self, data: ExecutableL2Data) -> RpcResult<MorphHeader> {
Expand All @@ -180,10 +174,7 @@ where
"RPC newL2BlockV2 called"
);

self.inner.new_l2_block_v2(data).await.map_err(|e| {
tracing::error!(target: "morph::engine", error = %e, "failed to import L2 block (v2)");
e.into()
})
self.inner.new_l2_block_v2(data).await.map_err(|e| e.into())
}

async fn new_safe_l2_block(&self, data: SafeL2Data) -> RpcResult<MorphHeader> {
Expand All @@ -193,10 +184,10 @@ where
"RPC newSafeL2Block called"
);

self.inner.new_safe_l2_block(data).await.map_err(|e| {
tracing::error!(target: "morph::engine", error = %e, "failed to import safe L2 block");
e.into()
})
self.inner
.new_safe_l2_block(data)
.await
.map_err(|e| e.into())
}

async fn set_block_tags(
Expand All @@ -214,10 +205,7 @@ where
self.inner
.set_block_tags(safe_block_hash, finalized_block_hash)
.await
.map_err(|e| {
tracing::error!(target: "morph::engine", error = %e, "failed to set block tags");
e.into()
})
.map_err(|e| e.into())
}
}

Expand Down