Skip to content

core/priority: add leading slash to protocol ID - #4605

Open
pinebit wants to merge 1 commit into
mainfrom
pinebit/fix-priority-slash
Open

core/priority: add leading slash to protocol ID#4605
pinebit wants to merge 1 commit into
mainfrom
pinebit/fix-priority-slash

Conversation

@pinebit

@pinebit pinebit commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator

Normalises the priority protocol ID to /charon/priority/2.0.0, matching every other charon libp2p protocol. It was the only one registered without a leading slash, which defeats /charon/-prefix matching.

The wire format does not change, so the version stays at 2.0.0 and the old spelling charon/priority/2.0.0 is kept as a legacy alias. Nodes offer both IDs when dialling, preferring the slash-prefixed one, and serve both. This keeps patched and unpatched nodes interoperable in all four combinations:

Dialer Listener Negotiated
patched patched /charon/priority/2.0.0
patched unpatched charon/priority/2.0.0
unpatched patched charon/priority/2.0.0
unpatched unpatched charon/priority/2.0.0

The two IDs are registered as separate exact-match handlers rather than via a single RegisterHandler call with WithDelimitedProtocol. They share no common prefix, so one call would collapse protocolPrefix to the bare wildcard "*", and that string is what gets advertised to peers through libp2p identify — peers would be told we support "*" and neither real ID. A code comment, a protocolPrefix doc comment, and a test assertion all pin this down.

TODO(v1.14) markers on the alias, the dual registration, and the send-side fallback mark them for removal once no supported version dials the legacy ID.

Tests: TestProtocols pins the wire-visible strings and their precedence, TestProtocolNegotiation covers what each peer type advertises and negotiates, and TestPrioritiserLegacyPeer runs a full exchange between a patched and an unpatched node. The latter isolates one dial direction per case — with both nodes dialling, either can reach quorum off its peer's inbound request and mask a broken outbound dial.

category: refactor
ticket: none

Normalise the priority protocol ID to /charon/priority/2.0.0, matching every
other charon libp2p protocol. The wire format is unchanged, so the version
stays at 2.0.0 and the old spelling is kept as a legacy alias so that patched
and unpatched nodes interoperate.

Nodes now offer both IDs when dialling, preferring the slash-prefixed one, and
serve both via separate exact-match handler registrations. The two IDs share no
common prefix, so registering them in a single RegisterHandler call would
collapse protocolPrefix to the bare wildcard "*" and advertise that to peers via
libp2p identify instead of the real protocol IDs.

category: refactor
ticket: none

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@pinebit
pinebit requested a review from KaloyanTanev July 27, 2026 12:50
@sonarqubecloud

Copy link
Copy Markdown

@codecov

codecov Bot commented Jul 27, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 87.50000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 57.61%. Comparing base (2eb6798) to head (5979094).

Files with missing lines Patch % Lines
core/priority/prioritiser.go 87.50% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4605      +/-   ##
==========================================
+ Coverage   57.58%   57.61%   +0.02%     
==========================================
  Files         246      246              
  Lines       33675    33678       +3     
==========================================
+ Hits        19392    19402      +10     
+ Misses      11845    11836       -9     
- Partials     2438     2440       +2     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.

1 participant