Skip to content

Add optional HeimdallWSAddress configuration and refactor handler_bor methods#1481

Merged
lucca30 merged 11 commits intoheimdall-v2from
lmartins/milestone-ws-events
Mar 12, 2025
Merged

Add optional HeimdallWSAddress configuration and refactor handler_bor methods#1481
lucca30 merged 11 commits intoheimdall-v2from
lmartins/milestone-ws-events

Conversation

@lucca30
Copy link
Copy Markdown
Contributor

@lucca30 lucca30 commented Mar 6, 2025

Description

This PR introduces the optional configuration parameter HeimdallWSAddress, which enables the application to establish a WebSocket connection with Heimdall for event listening rather than relying on polling. This enhancement aims to provide more efficient and real-time event handling.

Changes:

  • New Config:
    Added HeimdallWSAddress as an optional configuration setting to allow connecting via WebSocket to Heimdall.

  • WebSocket Integration:
    Updated the subscription logic to support event listening over a WebSocket connection. This change has been tested on a deployed devnet node, including a scenario where the connection is recovered after stopping bor.

  • Code Refactor:
    In handler_bor.go, refactored the handle and fetch methods to better segregate their roles, improving code clarity and maintainability.

Changes

  • Bugfix (non-breaking change that solves an issue)
  • Hotfix (change that solves an urgent issue, and requires immediate attention)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (change that is not backwards-compatible and/or changes current functionality)
  • Changes only for a subset of nodes

Checklist

  • I have added at least 2 reviewer or the whole pos-v1 team
  • I have added sufficient documentation in code
  • I will be resolving comments - if any - by pushing each fix in a separate commit and linking the commit hash in the comment reply
  • Created a task in Jira and informed the team for implementation in Erigon client (if applicable)
  • Includes RPC methods changes, and the Notion documentation has been updated

Cross repository changes

Testing

  • I have added unit tests
  • I have added tests to CI
  • I have tested this code manually on local environment
  • I have tested this code manually on remote devnet using express-cli
  • I have tested this code manually on mumbai/amoy
  • I have created new e2e tests into express-cli

@lucca30 lucca30 requested review from a team, Raneet10 and avalkov March 6, 2025 14:56
@lucca30 lucca30 changed the base branch from heimdallv2_fast_consensus to heimdall-v2 March 6, 2025 15:22
@pratikspatil024
Copy link
Copy Markdown
Member

Can you also update the config files?
Also, I don't think we need to update the cmd/ module, as we don't use it. Our command line is in internal/server which you have already updated

@lucca30
Copy link
Copy Markdown
Contributor Author

lucca30 commented Mar 12, 2025

Can you also update the config files? Also, I don't think we need to update the cmd/ module, as we don't use it. Our command line is in internal/server which you have already updated

Since we currently not apply this for any other Heimdall configuration, even for heimdall url. I decided not to include this one and lefting as an optional flag.

https://github.com/maticnetwork/bor/blob/e1f7325d433adcbb2596acf6d2c99c2767643cdd/packaging/templates/mainnet-v1/archive/config.toml#L48-L52

@pratikspatil024
Copy link
Copy Markdown
Member

Makes sense. Thanks!

@lucca30 lucca30 merged commit 83ef1c1 into heimdall-v2 Mar 12, 2025
4 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants