Skip to content

Require server.version before any other Electrum RPC method#1295

Open
PeterXMR wants to merge 1 commit into
romanz:masterfrom
PeterXMR:server-version-first-1241
Open

Require server.version before any other Electrum RPC method#1295
PeterXMR wants to merge 1 commit into
romanz:masterfrom
PeterXMR:server-version-first-1241

Conversation

@PeterXMR
Copy link
Copy Markdown

Refs #1241 — checks off the server.version() must-be-first-message item on the v1.6 protocol checklist.

Summary

Electrum protocol v1.6 mandates that server.version is the first message a client sends. Enforce this in electrs.

  • Adds negotiated_version: Option<String> to the per-client state (Client)
  • Set inside fn version() once the version handshake succeeds
  • In single_call, reject any non-Version call when negotiated_version.is_none() with a JSON-RPC error that names the required method

⚠️ Behavior change

Any client that today sends server.ping / server.banner / server.features / etc. before the version handshake will start receiving an error. This is a deliberate v1.6 spec-compliance change.

Happy to soften this to a warn!-then-allow if you'd prefer a phased rollout — let me know.

Test plan

  • New unit test test_client_starts_without_negotiated_version confirms the default-None state the gate keys on
  • cargo test --lib — 22/22 pass
  • cargo clippy --all-targets -- -D warnings — clean
  • cargo fmt --check — clean
  • Live tested against Bitcoin Core v31.0.0 on regtest, three scenarios:
$ # A. pre-version calls should be rejected
$ printf '{"id":1,"method":"server.ping","params":[]}
{"id":2,"method":"server.banner","params":[]}
{"id":3,"method":"server.features","params":[]}
' | nc -w 2 127.0.0.1 60401
{"error":{"code":1,"message":"`server.version` must be called before any other method"},"id":1,"jsonrpc":"2.0"}
{"error":{"code":1,"message":"`server.version` must be called before any other method"},"id":2,"jsonrpc":"2.0"}
{"error":{"code":1,"message":"`server.version` must be called before any other method"},"id":3,"jsonrpc":"2.0"}

$ # B. version first, then a regular call should succeed
$ printf '{"id":4,"method":"server.version","params":["test","1.4"]}
{"id":5,"method":"server.ping","params":[]}
{"id":6,"method":"server.banner","params":[]}
' | nc -w 2 127.0.0.1 60401
{"id":4,"jsonrpc":"2.0","result":["electrs/0.11.1","1.4"]}
{"id":5,"jsonrpc":"2.0","result":null}
{"id":6,"jsonrpc":"2.0","result":"Welcome to electrs 0.11.1 (Electrum Rust Server)!"}

$ # C. batch with version first also works (state propagates within the batch)
$ printf '[{"id":7,"method":"server.version","params":["test","1.4"]},{"id":8,"method":"server.ping","params":[]}]
' | nc -w 2 127.0.0.1 60401
[{"id":7,"jsonrpc":"2.0","result":["electrs/0.11.1","1.4"]},{"id":8,"jsonrpc":"2.0","result":null}]

Electrum protocol v1.6 mandates that the first message a client sends
must be `server.version`. Enforce this in electrs:

  - Add `negotiated_version: Option<String>` to the per-client state.
  - Set it inside `fn version()` once the version handshake succeeds.
  - In `single_call`, reject any non-Version call when
    `negotiated_version` is still `None`, with a JSON-RPC error message
    that names the required method.

Behavior change for clients: anything that today sends `server.ping`,
`server.banner`, etc. before the version handshake will start receiving
an error. This matches the v1.6 specification and aligns electrs with
the rest of the ecosystem.

Spec: https://electrum-protocol.readthedocs.io/en/latest/protocol-changes.html#version-1-6

Refs romanz#1241
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