Skip to content

fix(sync): wrap EntrySignature in iroh::Signature and lock wire format#101

Merged
matheus23 merged 1 commit into
n0-computer:mainfrom
cbenhagen:fix/entry-signature-wire-stable
May 26, 2026
Merged

fix(sync): wrap EntrySignature in iroh::Signature and lock wire format#101
matheus23 merged 1 commit into
n0-computer:mainfrom
cbenhagen:fix/entry-signature-wire-stable

Conversation

@cbenhagen
Copy link
Copy Markdown
Contributor

Description

iroh-docs 0.99 silently became wire-incompatible with 0.98. iroh 1.0.0-rc.0 pulled ed25519 from 3.0.0-rc.4 to 3.0.0 stable, which rewrote Signature::serialize from serialize_tuple (64 raw bytes) to serdect::array::serialize_hex_upper_or_bin (varint-length-prefix + 64 bytes). Sync responses fail to decode with Hit the end of buffer.

  • Wraps EntrySignature's signatures in iroh::Signature (hand-written wire-stable serialize_tuple impl, the same pattern iroh-base PR #3529 introduced). Restores byte-identical wire format with 0.98.
  • Adds test_postcard_format_stable: golden-byte assertions for SignedEntry, EntrySignature, Author, NamespaceSecret. Roundtrip tests miss this class of bug; golden bytes catch it before shipping.

Breaking Changes

None. SemVer-compatible: public from_parts(&[u8; 64], &[u8; 64]) and to_bytes() -> [u8; 64] API unchanged.

Notes & open questions

Targets 0.99.1. The companion breaking refactor (ed25519_dalek to iroh-base types throughout iroh_docs::keys) is split into a separate PR for 0.100.0.

Change checklist

  • Self-review.
  • Documentation updates following the style guide, if relevant.
  • Tests if relevant.
  • All breaking changes documented.

@n0bot n0bot Bot added this to iroh May 26, 2026
@github-project-automation github-project-automation Bot moved this to 🚑 Needs Triage in iroh May 26, 2026
Copy link
Copy Markdown
Member

@matheus23 matheus23 left a comment

Choose a reason for hiding this comment

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

This is good! Just some requests regarding how this is tested:

  • Please split up the test, so that you're not checking a lot of things in one test.
  • Please reduce the set of things you test: If we already snapshot-test a full SignedEntry, then I don't think we need another check for the EntrySignature alone.

Comment thread src/sync.rs Outdated
Comment thread src/sync.rs Outdated
Comment thread src/sync.rs Outdated
@github-project-automation github-project-automation Bot moved this from 🚑 Needs Triage to 🏗 In progress in iroh May 26, 2026
`EntrySignature` was holding raw `ed25519_dalek::Signature` values, whose
postcard serialization is determined by whatever serde impl the upstream
`ed25519` crate ships. When iroh 1.0.0-rc.0 tightened its dalek pin from
`=3.0.0-pre.6` to `=3.0.0-pre.7`, the transitive `ed25519` crate moved
from `3.0.0-rc.4` to `3.0.0` stable, which rewrote `Signature::serialize`
from `serialize_tuple` (64 raw bytes) to
`serdect::array::serialize_hex_upper_or_bin` (varint-length-prefix + 64
bytes). The result: iroh-docs 0.99 silently became wire-incompatible with
iroh-docs 0.98 - sync responses started failing to decode with
`Hit the end of buffer` / `Serde Deserialization Error`.

`iroh_base::Signature` (re-exported as `iroh::Signature`) wraps
`ed25519_dalek::Signature` with a hand-written, wire-stable
`serialize_tuple` impl - the same pattern iroh-base adopted in PR #3529
("feat(iroh-base)!: reduce external types in the iroh-base API for keys")
to insulate every iroh crate's wire payloads from upstream `ed25519`
serde changes. Switching iroh-docs to that wrapper restores byte-identical
wire format with 0.98 peers.

`from_entry` / `verify` hop between the wrapper and
`ed25519_dalek::Signature` at the `Signer` / `Verifier` trait boundary.
The public `from_parts(&[u8; 64], &[u8; 64])` API is unchanged.
`to_bytes()` on both types returns `[u8; 64]`, so the `Debug` impl and
`store::fs` callers are unaffected.

Adds three postcard snapshot tests that lock the on-wire and on-disk
byte representations against deterministic inputs:
`test_signed_entry_postcard_snapshot`, `test_author_postcard_snapshot`,
`test_namespace_secret_postcard_snapshot`. Roundtrip tests miss this
class of bug since both ends use the same build; snapshots catch it.
The `Author` / `NamespaceSecret` snapshots also lock the on-disk format,
which delegates transparently to `iroh::SecretKey` and through it to
`ed25519_dalek::SigningKey::serialize`, so it remains exposed to the
same class of upstream serde drift until iroh-base wraps `SecretKey`
with a hand-written impl or these types ship their own.
@cbenhagen cbenhagen force-pushed the fix/entry-signature-wire-stable branch from a8fe07c to c6bc8d3 Compare May 26, 2026 07:28
@cbenhagen
Copy link
Copy Markdown
Contributor Author

@matheus23 thanks for the review! I have reduced the verbosity of the comments and split up the tests.

@cbenhagen cbenhagen requested a review from matheus23 May 26, 2026 07:37
@matheus23 matheus23 merged commit 466e541 into n0-computer:main May 26, 2026
25 checks passed
@github-project-automation github-project-automation Bot moved this from 🏗 In progress to ✅ Done in iroh May 26, 2026
dignifiedquire pushed a commit that referenced this pull request May 27, 2026
…102)

## Description

Finishes the migration n0-computer/iroh#3529
started. iroh-docs is one of the last iroh crates still leaking
`ed25519_dalek::{SigningKey, VerifyingKey, SignatureError}` through its
public surface.

- `SigningKey` becomes `iroh::SecretKey` (transparent serde; on-disk
format byte-identical, locked by `test_postcard_format_stable`).
- `VerifyingKey` becomes `iroh::PublicKey` inside `AuthorPublicKey` /
`NamespacePublicKey` (API-only; iroh-docs never serializes these).
- `SignatureError` becomes `iroh::SignatureError` for verification
failures.
- Key-parsing failures now return `iroh::KeyParsingError` instead of
reusing `SignatureError`.
- `SignedEntry::verify` returns a new `SignedEntryVerifyError` enum that
separates key-parsing from signature-mismatch failures.

## Breaking Changes

All breakage is in error-return positions; no method names, parameters,
or semantic behavior change.

Return types changed from `Result<_, SignatureError>` to `Result<_,
KeyParsingError>` on:
- `AuthorPublicKey::from_bytes`, `NamespacePublicKey::from_bytes`
- `AuthorId::public_key`, `AuthorId::into_public_key`
- `NamespaceId::public_key`, `NamespaceId::into_public_key`
- `PublicKeyStore::public_key`, `PublicKeyStore::namespace_key`,
`PublicKeyStore::author_key`
- `TryFrom<AuthorId> for AuthorPublicKey`, `TryFrom<NamespaceId> for
NamespacePublicKey`

Other changes:
- `SignedEntry::verify` now returns `Result<(),
SignedEntryVerifyError>`.
- `From<ed25519_dalek::SigningKey>` impls on `Author` /
`NamespaceSecret` become `From<iroh::SecretKey>`.

Migration: callers using `?` or `anyhow` are unaffected. Callers
pattern-matching on `SignatureError` should switch to `KeyParsingError`
for parse failures or to the new `SignedEntryVerifyError` variants for
verify failures. Custom `PublicKeyStore` impls need to update their
return types.

## Notes & open questions

Targets 0.100.0. Stacked on #101 (the 0.99.1 wire-compat fix lands
first)

## Change checklist

- [x] Self-review.
- [x] Documentation updates following the [style
guide](https://rust-lang.github.io/rfcs/1574-more-api-documentation-conventions.html#appendix-a-full-conventions-text),
if relevant.
- [x] Tests if relevant.
- [x] All breaking changes documented.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: ✅ Done

Development

Successfully merging this pull request may close these issues.

2 participants