Skip to content

Conversation

@davidtaikocha
Copy link
Collaborator

@davidtaikocha davidtaikocha commented Jul 24, 2025

We can simply trigger a beacon sync by a FCU call.

@davidtaikocha davidtaikocha marked this pull request as ready for review July 26, 2025 02:45
@davidtaikocha davidtaikocha enabled auto-merge July 26, 2025 02:46
@codecov
Copy link

codecov bot commented Jul 26, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 27.03%. Comparing base (211d8fb) to head (64029d1).
⚠️ Report is 16 commits behind head on main.

Additional details and impacted files
Files with missing lines Coverage Δ
...ko-client/driver/chain_syncer/beaconsync/syncer.go 0.00% <ø> (ø)

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 55d286b...64029d1. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@cyberhorsey
Copy link
Contributor

why is this call unnecessary? reth doesn't support it and we need it removed?

@davidtaikocha
Copy link
Collaborator Author

why is this call unnecessary? reth doesn't support it and we need it removed?

Its unnecessary if you take a look at both geth and reth FCU source code, but we can still call it. It's also ok, the problem is our request body is incompatible with the spec, we left executableData.transactions as nil, it seems its ok to geth, but can't pass reth's check. So removing it will bring our calls closer to the spec.

Another choice is we update this method to fill the executableData.transactions before sending the request.

But yeah, better @cyberhorsey @YoGhurt111 can double-check the taiko-geth source code.

@dantaik
Copy link
Member

dantaik commented Jul 29, 2025

@claude please review.

@claude
Copy link

claude bot commented Jul 29, 2025

Claude encountered an error —— View job


I'll analyze this and get back to you.

@davidtaikocha
Copy link
Collaborator Author

Closed by #19860

auto-merge was automatically disabled August 2, 2025 01:25

Pull request was closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants