Skip to content
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.

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

Ensure p2p protocol matches new Starknet spec #2380

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

wojciechos
Copy link
Contributor

@wojciechos wojciechos commented Jan 15, 2025

  • Updated protocol paths to include chain_ID and /sync segment as it is latest starknet p2p spec
  • All protocol paths now follow the format: /starknet/<chain_id>/sync/...
  • Added tests to verify protocol paths

Protocol Path Changes:

Before:

  • /starknet/headers/0.1.0-rc.0
  • /starknet/events/0.1.0-rc.0

After:

  • /starknet/SN_MAIN/sync/headers/0.1.0-rc.0
  • /starknet/SN_SEPOLIA/sync/events/0.1.0-rc.0

@wojciechos wojciechos force-pushed the wojciechos/adjust-p2p-naming branch 2 times, most recently from 7a7d296 to fc245de Compare January 15, 2025 12:56
Copy link

codecov bot commented Jan 15, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 74.34%. Comparing base (61d810a) to head (8ce0d16).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2380      +/-   ##
==========================================
- Coverage   74.48%   74.34%   -0.14%     
==========================================
  Files         113      113              
  Lines       13020    13020              
==========================================
- Hits         9698     9680      -18     
- Misses       2587     2601      +14     
- Partials      735      739       +4     

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

@wojciechos wojciechos added the P2P label Jan 15, 2025
@wojciechos wojciechos force-pushed the wojciechos/adjust-p2p-naming branch 3 times, most recently from b0b5de8 to 8dec96e Compare January 16, 2025 09:28
@wojciechos wojciechos marked this pull request as ready for review January 16, 2025 09:31
Copy link
Contributor

@AnkushinDaniil AnkushinDaniil left a comment

Choose a reason for hiding this comment

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

Have you tested sync with Pathfinder?

@wojciechos wojciechos force-pushed the wojciechos/adjust-p2p-naming branch from 8dec96e to 8ce0d16 Compare January 27, 2025 09:53
@wojciechos
Copy link
Contributor Author

wojciechos commented Jan 27, 2025

Have you tested sync with Pathfinder?

@AnkushinDaniil Yep, I tested, but Pathfinder hasn’t updated to the latest P2P spec, which is why it can’t sync with each other. See here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants