Skip to content
This repository was archived by the owner on Jan 16, 2026. It is now read-only.

fix(test/kona): fix l1 confirmation delay for kona and sequence attribute builder pipeline#2770

Closed
theochap wants to merge 9 commits intotheo/add-ws-rpcfrom
theo/fix-confirmation-delay
Closed

fix(test/kona): fix l1 confirmation delay for kona and sequence attribute builder pipeline#2770
theochap wants to merge 9 commits intotheo/add-ws-rpcfrom
theo/fix-confirmation-delay

Conversation

@theochap
Copy link
Member

@theochap theochap commented Aug 26, 2025

Description

Try to fix the l1 confirmation delay in the devstack for kona which should reduce transfer flakiness in sysgo
Fixes the sequencer attribute builder pipeline to include txs from the mempool automatically

Depends on #2768

@claude
Copy link
Contributor

claude bot commented Aug 26, 2025

Claude finished @theochap's task —— View job


PR Review: fix(test/kona): fix l1 confirmation delay for kona

Review Checklist

  • Analyze changed files and understand dependency updates
  • Review code quality and best practices
  • Check for potential bugs or issues
  • Assess performance considerations
  • Evaluate security concerns
  • Review test coverage impact

Summary

This PR updates the dependency on theochap/optimism fork from commit 765ea670b027 (2025-08-26 11:12:33) to commit 01a87b97dc75 (2025-08-26 11:39:38), specifically targeting L1 confirmation delay fixes for kona to reduce transfer flakiness in sysgo tests.

Review Findings

✅ Code Quality & Best Practices

  • Clean dependency management: The change follows proper Go module practices by updating both go.mod and go.sum files
  • Focused scope: Changes are limited to the testing infrastructure, not affecting production code
  • Clear intent: The commit message and PR description clearly explain the purpose (fixing L1 confirmation delay)

✅ Potential Bugs & Issues

  • Low risk: This is a dependency version bump of a fork, likely containing configuration changes
  • Test isolation: Changes are contained to the tests/ directory which handles e2e testing with kurtosis/devstack
  • Backward compatibility: The version update appears to be a patch-level change within the same fork

⚠️ Performance Considerations

  • Potential improvement: The stated goal is to "reduce transfer flakiness in sysgo", suggesting this should improve test reliability and potentially reduce test execution time
  • Confirmation delay optimization: L1 confirmation delay fixes typically reduce waiting times in blockchain operations

✅ Security Concerns

  • Fork dependency: Using a personal fork (theochap/optimism) instead of upstream is noted - ensure this fork is trusted and up-to-date with security patches
  • Limited scope: Changes are in test infrastructure only, not affecting production security posture
  • Commit verification: The short time window between commits (27 minutes) suggests these are targeted fixes

✅ Test Coverage Impact

  • Test reliability improvement: The primary goal is reducing test flakiness, which should improve CI reliability
  • No test removal: No test files were modified, only dependency versions
  • E2e testing focus: Changes target the kurtosis-based e2e testing framework for kona-node

Recommendations

  1. Monitor test stability: After merging, track whether the sysgo transfer tests show improved reliability
  2. Fork maintenance: Consider establishing a process to regularly sync the theochap/optimism fork with upstream security updates
  3. Documentation: The change is well-documented in the commit message and PR description

Conclusion

This is a low-risk, focused improvement that should enhance test stability. The dependency update follows proper practices and targets a specific issue (L1 confirmation delay) in the testing infrastructure.

Recommendation: ✅ Approve - The changes are well-scoped, properly implemented, and address a legitimate testing concern without introducing security or stability risks.


@codecov
Copy link

codecov bot commented Aug 26, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.0%. Comparing base (6fef405) to head (b6ce647).
✅ All tests successful. No failed tests found.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@theochap theochap force-pushed the theo/fix-confirmation-delay branch from d5415f1 to dc0ddd8 Compare August 26, 2025 12:35
@theochap theochap changed the base branch from theo/add-ws-rpc to main August 26, 2025 12:56
@theochap theochap changed the base branch from main to theo/add-ws-rpc August 26, 2025 13:02
@theochap theochap force-pushed the theo/fix-confirmation-delay branch from 1f27482 to 5cca647 Compare August 26, 2025 13:02
@theochap theochap force-pushed the theo/fix-confirmation-delay branch from 5cca647 to d88a661 Compare August 26, 2025 13:05
@theochap theochap self-assigned this Aug 26, 2025
@theochap theochap added M-tests Meta: Testing related A-node Area: cl node (eq. Go op-node) handles single-chain consensus labels Aug 26, 2025
@theochap theochap moved this to In Review in Project Tracking Aug 26, 2025
@theochap theochap changed the title fix(test/kona): fix l1 confirmation delay for kona fix(test/kona): fix l1 confirmation delay for kona and sequence attribute builder pipeline Aug 26, 2025
theochap added a commit that referenced this pull request Aug 27, 2025
theochap added a commit that referenced this pull request Aug 27, 2025
theochap added a commit that referenced this pull request Aug 27, 2025
@theochap
Copy link
Member Author

Superseeded by #2783

@theochap theochap closed this Aug 27, 2025
@github-project-automation github-project-automation bot moved this from In Review to Done in Project Tracking Aug 27, 2025
github-merge-queue bot pushed a commit that referenced this pull request Aug 27, 2025
## Description

Adds a restart test for the sequencer. Ensure that the unsafe head of
the other nodes doesn't progress

Rebased on top of #2770
theochap added a commit to ethereum-optimism/optimism that referenced this pull request Dec 10, 2025
## Description

Adds a restart test for the sequencer. Ensure that the unsafe head of
the other nodes doesn't progress

Rebased on top of op-rs/kona#2770
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

A-node Area: cl node (eq. Go op-node) handles single-chain consensus M-tests Meta: Testing related

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant