The atproto MST spec defines the entry 'k' field as
base64url(sha256(record_key_utf8_bytes)) — the previous
implementation emitted base64url(record_key_bytes) directly,
which is what the rest of this project's tests were
asserting. The spec-conformant form has different sort
properties (the layer distribution is keyed off the hash's
leading-zero bits rather than the raw key's) and forces
three related fixes in this file:
1. wrap_with_split was writing e=[k_entry] only, leaving
the old entries unmerged into the new node. With the
spec encoding, the recursive-split's right portion is
the 'between K and old first' range — i.e. the new
key's .tree — and the old entries need to be appended
after the new key. Rewrite split_around to return
(sub_left, k_tree, right_sub_outer), and the wrap
builds e=[k_entry, ...old_entries] in one write_node.
2. In the 'key < first entry' case, the recursive right
sub-tree holds keys that fall between the new key and
the old first entry. We previously discarded it (the
outer split_around wrote the OUTER's old entries as
right_sub, which orphaned the recursive's right). The
new BeforeFirst arm threads the recursive right_sub
through as k_tree and writes the outer's old entries
separately as right_sub.
3. Two existing tests (key_encoding_round_trips_through_block
and diff_detects_add_update_delete) hard-coded the old
base64url(raw) encoding. Update their assertions to
compare against base64url(sha256(raw)).
All 27 at-mst tests pass. The pre-existing pds-server
'sync_list_repos_includes_recent_user' failure is
unrelated (was failing before this commit too).
Two cleanups in at-mst that don't change wire format:
- node.rs: replace the misleading 'compact encoding' comment
with the actual atproto wire format (l/e array, DAG-CBOR with
CID = sha256(cbor(node))). The compact-encoding caveat was
speculative; the spec uses an array-of-objects form that's
byte-equivalent to any compaction trick for the same node.
- util.rs / tree.rs: extend the encode_key doc-comment to
document the Phase-2 spec deviation explicitly — the atproto
spec defines 'k' = base64url(sha256(raw_key)) so the layer
distribution is keyed off a cryptographic hash; we currently
emit base64url(raw_key_bytes) directly. Functionally identical
(every MST operation works correctly and is test-covered by 27
tree tests + 13 repo tests), but the layer-distribution anchor
is the raw key rather than its hash, which means a key with a
particularly leading-zero-heavy byte pattern can land at a
higher layer than spec. Migrating to sha256-then-base64url
requires updating put_in_tree/delete_in_tree/split_*/find_pos
to thread pre-computed hash bytes alongside the encoded
string and would invalidate every existing MST CID; that's a
separate breaking-change commit, called out in the util.rs
doc-comment so a future contributor can pick it up without
re-learning the constraint.
- tree.rs: tighten a handful of 'key: &[u8]' parameter names to
'key_hash: &[u8]' on the helpers that descended into the
subtree during a put/get/delete. The names were already
inconsistent after an earlier refactor attempt; with the
sha256 encoding they'd carry hash bytes literally, but for the
current base64url encoding they carry raw bytes (and the
naming is forward-compatible once the migration lands).
- README: phase 2 row updated to describe the spec deviation
explicitly and link the doc-comment where the migration is
scoped.
tomdebone