Skip to content

fix(p2p): gracefully handle unknown protocols in Identify#2200

Open
iostat wants to merge 3 commits intodevelopfrom
io/p2p-fix-identify-reject
Open

fix(p2p): gracefully handle unknown protocols in Identify#2200
iostat wants to merge 3 commits intodevelopfrom
io/p2p-fix-identify-reject

Conversation

@iostat
Copy link
Copy Markdown
Contributor

@iostat iostat commented Mar 26, 2026

(stacked PRs: this is 1 of 3, merge this before #2201 and #2202)

Description

Replace the strict parse_protocol() that rejected entire Identify messages
on unknown protocol strings with a StreamProtocolId enum (Known/Unknown)
that preserves unrecognized protocols for forward compatibility. This prevents
older Rust nodes from disconnecting peers that advertise new capability
strings, consistent with how go-libp2p handles unknown protocols.

Related Issue(s)

Closes #2189

Type of Change

  • Bug fix (non-breaking change which fixes an issue)

Testing

  • Existing test suite passes; parse_protocol no longer returns Result,
    so unknown strings are silently preserved rather than causing errors.

Changelog Entry

Fixed: P2P: Identify protocol no longer rejects peers that advertise unknown
protocol strings, improving forward compatibility with newer nodes (#2200)

messages (#2189)

Replace the strict parse_protocol() that rejected entire
Identify messages on unknown protocol strings with a new
StreamProtocolId enum (Known/Unknown) that preserves
unrecognized protocols for forward compatibility.

This prevents older Rust nodes from breaking when peers
advertise new capability strings, consistent with how
go-libp2p and our own multistream-select parser handle
unknown protocols.
@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 26, 2026

OCaml Reference Validation Results

Repository: https://github.com/MinaProtocol/mina.git
Branch: compatible
Status: ❌ Validation failed

Click to see full validation output
Checking OCaml references against https://github.com/MinaProtocol/mina.git (branch: compatible)
Fetching current commit from compatible...
Current OCaml commit: 3618e4372b2d4559c0e316fca6e5423a6f315b8a

Validating references...
========================
[OK] VALID: crates/ledger/src/account/account.rs:1273 -> src/lib/mina_base/account.ml L:201-224
  [WARN] STALE COMMIT: fc6be4c58091c761f827c858229c2edf9519e941 (current: 3618e4372b2d4559c0e316fca6e5423a6f315b8a)
[ERROR] INVALID: crates/ledger/src/scan_state/transaction_logic/for_tests.rs:63
   Code at L:2285-2285 differs between commit 5da42ccd72e791f164d4d200cf1ce300262873b3 and current branch
   Referenced: https://github.com/MinaProtocol/mina/blob/5da42ccd72e791f164d4d200cf1ce300262873b3/src/lib/transaction_logic/mina_transaction_logic.ml#L2285-L2285
   Current:    https://github.com/MinaProtocol/mina/blob/compatible/src/lib/transaction_logic/mina_transaction_logic.ml#L2285-L2285
[ERROR] INVALID: crates/ledger/src/scan_state/transaction_logic/for_tests.rs:67
   Code at L:2351-2356 differs between commit 5da42ccd72e791f164d4d200cf1ce300262873b3 and current branch
   Referenced: https://github.com/MinaProtocol/mina/blob/5da42ccd72e791f164d4d200cf1ce300262873b3/src/lib/transaction_logic/mina_transaction_logic.ml#L2351-L2356
   Current:    https://github.com/MinaProtocol/mina/blob/compatible/src/lib/transaction_logic/mina_transaction_logic.ml#L2351-L2356
[ERROR] INVALID: crates/ledger/src/scan_state/transaction_logic/for_tests.rs:76
   Code at L:2407-2407 differs between commit 5da42ccd72e791f164d4d200cf1ce300262873b3 and current branch
   Referenced: https://github.com/MinaProtocol/mina/blob/5da42ccd72e791f164d4d200cf1ce300262873b3/src/lib/transaction_logic/mina_transaction_logic.ml#L2407-L2407
   Current:    https://github.com/MinaProtocol/mina/blob/compatible/src/lib/transaction_logic/mina_transaction_logic.ml#L2407-L2407
[OK] VALID: crates/ledger/src/scan_state/transaction_logic/mod.rs:118 -> src/lib/mina_base/transaction_status.ml L:9-51
  [WARN] STALE COMMIT: 5da42ccd72e791f164d4d200cf1ce300262873b3 (current: 3618e4372b2d4559c0e316fca6e5423a6f315b8a)
[OK] VALID: crates/ledger/src/scan_state/transaction_logic/mod.rs:234 -> src/lib/mina_base/transaction_status.ml L:452-454
  [WARN] STALE COMMIT: 5da42ccd72e791f164d4d200cf1ce300262873b3 (current: 3618e4372b2d4559c0e316fca6e5423a6f315b8a)
[OK] VALID: crates/ledger/src/scan_state/transaction_logic/mod.rs:250 -> src/lib/mina_base/with_status.ml L:6-10
  [WARN] STALE COMMIT: 5da42ccd72e791f164d4d200cf1ce300262873b3 (current: 3618e4372b2d4559c0e316fca6e5423a6f315b8a)
[OK] VALID: crates/ledger/src/scan_state/transaction_logic/mod.rs:320 -> src/lib/mina_base/fee_transfer.ml L:76-80
  [WARN] STALE COMMIT: 5da42ccd72e791f164d4d200cf1ce300262873b3 (current: 3618e4372b2d4559c0e316fca6e5423a6f315b8a)
[OK] VALID: crates/ledger/src/scan_state/transaction_logic/mod.rs:345 -> src/lib/mina_base/fee_transfer.ml L:68-69
  [WARN] STALE COMMIT: 5da42ccd72e791f164d4d200cf1ce300262873b3 (current: 3618e4372b2d4559c0e316fca6e5423a6f315b8a)
[OK] VALID: crates/ledger/src/scan_state/transaction_logic/mod.rs:420 -> src/lib/mina_base/coinbase.ml L:17-21
  [WARN] STALE COMMIT: 5da42ccd72e791f164d4d200cf1ce300262873b3 (current: 3618e4372b2d4559c0e316fca6e5423a6f315b8a)
[OK] VALID: crates/ledger/src/scan_state/transaction_logic/mod.rs:1049 -> src/lib/transaction/transaction.ml L:8-11
  [WARN] STALE COMMIT: 5da42ccd72e791f164d4d200cf1ce300262873b3 (current: 3618e4372b2d4559c0e316fca6e5423a6f315b8a)
[OK] VALID: crates/ledger/src/scan_state/transaction_logic/signed_command.rs:17 -> src/lib/mina_base/signed_command_payload.ml L:34-48
  [WARN] STALE COMMIT: 5da42ccd72e791f164d4d200cf1ce300262873b3 (current: 3618e4372b2d4559c0e316fca6e5423a6f315b8a)
[OK] VALID: crates/ledger/src/scan_state/transaction_logic/signed_command.rs:43 -> src/lib/mina_base/stake_delegation.ml L:11-13
  [WARN] STALE COMMIT: 5da42ccd72e791f164d4d200cf1ce300262873b3 (current: 3618e4372b2d4559c0e316fca6e5423a6f315b8a)
[OK] VALID: crates/ledger/src/scan_state/transaction_logic/signed_command.rs:70 -> src/lib/mina_base/signed_command_payload.ml L:179-181
  [WARN] STALE COMMIT: 5da42ccd72e791f164d4d200cf1ce300262873b3 (current: 3618e4372b2d4559c0e316fca6e5423a6f315b8a)
[OK] VALID: crates/ledger/src/scan_state/transaction_logic/signed_command.rs:81 -> src/lib/mina_base/signed_command_payload.ml L:239-243
  [WARN] STALE COMMIT: 5da42ccd72e791f164d4d200cf1ce300262873b3 (current: 3618e4372b2d4559c0e316fca6e5423a6f315b8a)
[OK] VALID: crates/ledger/src/scan_state/transaction_logic/signed_command.rs:112 -> src/lib/mina_base/signed_command_payload.ml L:352-362
  [WARN] STALE COMMIT: 5da42ccd72e791f164d4d200cf1ce300262873b3 (current: 3618e4372b2d4559c0e316fca6e5423a6f315b8a)
[ERROR] INVALID: crates/p2p-messages/src/string.rs:14
   Code at L:140-140 differs between commit c0c9d702b8cba34a603a28001c293ca462b1dfec and current branch
   Referenced: https://github.com/MinaProtocol/mina/blob/c0c9d702b8cba34a603a28001c293ca462b1dfec/src/lib/mina_base/zkapp_account.ml#L140-L140
   Current:    https://github.com/MinaProtocol/mina/blob/compatible/src/lib/mina_base/zkapp_account.ml#L140-L140
[OK] VALID: crates/p2p-messages/src/string.rs:16 -> src/lib/mina_base/account.ml L:92-92
  [WARN] STALE COMMIT: c0c9d702b8cba34a603a28001c293ca462b1dfec (current: 3618e4372b2d4559c0e316fca6e5423a6f315b8a)

Summary
=======
Total references found: 18
Valid references: 14
Invalid references: 4
Stale commits: 14

[ERROR] Validation failed: 4 invalid reference(s) found

@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 26, 2026

✓ Code Reference Verification Passed

All code references in the documentation have been verified successfully!

Total references checked: 1
Valid references: 1

The documentation is in sync with the codebase on the develop branch.

richardpringle
richardpringle previously approved these changes Mar 27, 2026
Copy link
Copy Markdown
Collaborator

@richardpringle richardpringle left a comment

Choose a reason for hiding this comment

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

I think you just need a clippy --fix run

pub enum StreamProtocolId {
Known(StreamKind),
Unknown(String),
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

👍

.iter()
.map(String::as_str)
.map(parse_protocol)
.collect();
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🥇

.map(|v| v.name_str().into())
.map(|v| match v {
StreamProtocolId::Known(k) => k.name_str().into(),
StreamProtocolId::Unknown(s) => s.clone().into(),
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

One of the linters is complaining about this. I think into is unnecessary

@iostat iostat force-pushed the io/p2p-fix-identify-reject branch from 4edae62 to 045fa7b Compare March 31, 2026 16:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

p2p: Identify protocol rejects unknown protocol strings

2 participants