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

Supervisor handling node updates via subscription#1836

Merged
dhyaniarun1993 merged 27 commits intomainfrom
jk/node-events-subscriber
May 26, 2025
Merged

Supervisor handling node updates via subscription#1836
dhyaniarun1993 merged 27 commits intomainfrom
jk/node-events-subscriber

Conversation

@itschaindev
Copy link
Collaborator

@itschaindev itschaindev commented May 22, 2025

Closes #1681

Things that are still left:

  • Call relevant DB functions on event notification
  • Check subscription with actual node
  • Fallback to HTTP polling using PullEvents if subscription does not work

@codecov
Copy link

codecov bot commented May 22, 2025

Codecov Report

Attention: Patch coverage is 65.65657% with 136 lines in your changes missing coverage. Please review.

Project coverage is 83.4%. Comparing base (43137d7) to head (4f5b26d).
Report is 87 commits behind head on main.

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
crates/supervisor/core/src/syncnode/node.rs 69.8% 112 Missing ⚠️
crates/supervisor/types/src/types.rs 0.0% 23 Missing ⚠️
crates/supervisor/rpc/src/jsonrpsee.rs 0.0% 1 Missing ⚠️

☔ 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.

@itschaindev itschaindev marked this pull request as ready for review May 23, 2025 09:36
Copy link
Contributor

@emhane emhane left a comment

Choose a reason for hiding this comment

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

lgtm

|_| {
use std::io::Write;
let secret = JwtSecret::random();
if let Ok(mut file) = File::create("jwt.hex") {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think we should create the jwt here.
Since we are developing the client side, we shouldn't create the jwt(The should be the server's responsibility).

let auth_header =
format!("Bearer {}", alloy_primitives::hex::encode(jwt_secret.as_bytes()));
headers.insert(
"Authorization",
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

which crate are you referring to here?

Comment on lines +241 to +242
let err_msg = format!("Failed to send stop signal: {:?}", e);
error!("{}", err_msg);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you follow this logging pattern?

Suggested change
let err_msg = format!("Failed to send stop signal: {:?}", e);
error!("{}", err_msg);
error!(
target: "managed_node",
?err,
"Failed to send stop signal"
);

let auth_header =
format!("Bearer {}", alloy_primitives::hex::encode(jwt_secret.as_bytes()));
headers.insert(
"Authorization",
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

which crate are you referring to here?

@dhyaniarun1993 dhyaniarun1993 added this pull request to the merge queue May 26, 2025
github-merge-queue bot pushed a commit that referenced this pull request May 26, 2025
Closes #1681

Things that are still left:

- Call relevant DB functions on event notification
- Check subscription with actual node
- Fallback to HTTP polling using `PullEvents` if subscription does not
work

---------

Co-authored-by: Arun Dhyani <dhyaniarun7@gmail.com>
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 26, 2025
@dhyaniarun1993 dhyaniarun1993 added this pull request to the merge queue May 26, 2025
Merged via the queue into main with commit cb27c27 May 26, 2025
20 of 23 checks passed
@dhyaniarun1993 dhyaniarun1993 deleted the jk/node-events-subscriber branch May 26, 2025 13:56
@github-project-automation github-project-automation bot moved this from In Review to Done in Project Tracking May 26, 2025
@emhane emhane added the W-supervisor Workstream: supervisor label Jun 5, 2025
theochap pushed a commit to ethereum-optimism/optimism that referenced this pull request Dec 10, 2025
Closes op-rs/kona#1681

Things that are still left:

- Call relevant DB functions on event notification
- Check subscription with actual node
- Fallback to HTTP polling using `PullEvents` if subscription does not
work

---------

Co-authored-by: Arun Dhyani <dhyaniarun7@gmail.com>
theochap pushed a commit to ethereum-optimism/optimism that referenced this pull request Jan 14, 2026
Closes #1681

Things that are still left:

- Call relevant DB functions on event notification
- Check subscription with actual node
- Fallback to HTTP polling using `PullEvents` if subscription does not
work

---------

Co-authored-by: Arun Dhyani <dhyaniarun7@gmail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

W-supervisor Workstream: supervisor

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

feat(supervisor/core): Build l2-subscriber

3 participants