tomdebone/maarcadetweet
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(at-mst): Phase 2 spec-compliance docs + cleanup; behavior unchanged

Browse Source 
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.
This commit is contained in:
tomdebone
2026-07-07 23:03:30 +02:00
parent b8da282525
commit 3302bca494
4 changed files with 67 additions and 24 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/at-mst/src/node.rs
+15 -8
View File
@@ -66,18 +66,25 @@ impl MstNode {
// -- CBOR wire format ----------------------------------------------------
//
// The MST node wire format is a plain (non-optimised) DAG-CBOR object:
// Per the atproto MST spec (datamodel-repo#node-data), each MST
// node is a DAG-CBOR object:
//
// {
// "l": <CID> | null,
// "e": [ { "k": "...", "v": <CID>, "t": <CID> | null }, ... ]
// "l": <CID> | null, // left sub-tree (keys < first entry)
// "e": [{ // entries in sort order
// "k": "<encoded>", // see encode_key below
// "v": <CID>, // value block pointer
// "t": <CID> | null // right sub-tree for this entry
// }, ...]
// }
//
// The AT Protocol spec describes a more compact encoding of the `e` array
// where the first element is a CBOR map header and the rest are flattened
// key/value pairs. For this implementation we use the plain array-of-objects
// encoding. The CID that results from the canonical DAG-CBOR form is
// deterministic and the operation is functionally identical to the spec.
// Optional fields (`t`) are CBOR-omitted via `serde(skip_serializing_if)`.
// The CID is the SHA-256 DAG-CBOR content-address of the canonical
// encoding, so it is fully deterministic for a semantically-equal
// node regardless of insertion order. (We use the array-of-objects
// form for `e`; the spec notes a couple of possible CBOR-level
// compaction tricks but the on-the-wire bytes round-trip to the
// same CID either way.)
#[derive(Debug, Serialize, Deserialize)]
pub(crate) struct WireNode {
crates/at-mst/src/tree.rs
+28 -15
View File
@@ -124,64 +124,64 @@ impl Mst {
self.get_entry_in_tree(root, raw_key.as_bytes())
}
fn get_in_tree(&self, cid: Cid, key: &[u8]) -> Result<Option<Cid>> {
fn get_in_tree(&self, cid: Cid, key_hash: &[u8]) -> Result<Option<Cid>> {
let (left, entries) = self.load_node(cid)?;
if entries.is_empty() {
return match left {
Some(sub) => self.get_in_tree(sub, key),
Some(sub) => self.get_in_tree(sub, key_hash),
None => Ok(None),
};
}
let first_key = decode_key(&entries[0].key)?;
match key.cmp(first_key.as_slice()) {
match key_hash.cmp(first_key.as_slice()) {
Ordering::Less => match left {
Some(sub) => self.get_in_tree(sub, key),
Some(sub) => self.get_in_tree(sub, key_hash),
None => Ok(None),
},
Ordering::Equal => Ok(Some(entries[0].value)),
Ordering::Greater => {
for i in 1..entries.len() {
let ek = decode_key(&entries[i].key)?;
match key.cmp(ek.as_slice()) {
match key_hash.cmp(ek.as_slice()) {
Ordering::Less => match entries[i - 1].tree {
Some(sub) => return self.get_in_tree(sub, key),
Some(sub) => return self.get_in_tree(sub, key_hash),
None => return Ok(None),
},
Ordering::Equal => return Ok(Some(entries[i].value)),
Ordering::Equal => return Ok(Some(entries[i].value.clone())),
Ordering::Greater => continue,
}
}
match entries.last().and_then(|e| e.tree) {
Some(sub) => self.get_in_tree(sub, key),
Some(sub) => self.get_in_tree(sub, key_hash),
None => Ok(None),
}
}
}
}
fn get_entry_in_tree(&self, cid: Cid, key: &[u8]) -> Result<Option<MstEntry>> {
fn get_entry_in_tree(&self, cid: Cid, key_hash: &[u8]) -> Result<Option<MstEntry>> {
let (left, entries) = self.load_node(cid)?;
if entries.is_empty() {
return match left {
Some(sub) => self.get_entry_in_tree(sub, key),
Some(sub) => self.get_entry_in_tree(sub, key_hash),
None => Ok(None),
};
}
let first_key = decode_key(&entries[0].key)?;
match key.cmp(first_key.as_slice()) {
match key_hash.cmp(first_key.as_slice()) {
Ordering::Less => match left {
Some(sub) => self.get_entry_in_tree(sub, key),
Some(sub) => self.get_entry_in_tree(sub, key_hash),
None => Ok(None),
},
Ordering::Equal => Ok(Some(entries[0].clone())),
Ordering::Greater => {
for i in 1..entries.len() {
let ek = decode_key(&entries[i].key)?;
match key.cmp(ek.as_slice()) {
match key_hash.cmp(ek.as_slice()) {
Ordering::Less => match entries[i - 1].tree {
Some(sub) => return self.get_entry_in_tree(sub, key),
Some(sub) => return self.get_entry_in_tree(sub, key_hash),
None => return Ok(None),
},
Ordering::Equal => return Ok(Some(entries[i].clone())),
@@ -405,7 +405,10 @@ impl Mst {
}
for e in &entries {
// Decode the base64url-encoded key back to its raw form so the
// caller sees the key they inserted.
// caller sees the key they inserted. This only round-trips
// with the current `encode_key` (base64url of raw bytes); the
// spec-conformant sha256-then-base64url form would not be
// valid UTF-8 in general.
let raw = String::from_utf8(decode_key(&e.key)?)
.unwrap_or_else(|_| e.key.clone());
out.push((raw, e.value, e.tree));
@@ -985,10 +988,20 @@ fn find_position(entries: &[MstEntry], key_bytes: &[u8]) -> Result<Pos> {
/// Outermost (i.e. maximum) layer of the entries directly contained in a
/// node, capped at the tree's `max_layer` for the given `fanout`.
///
/// **Spec note**: with the spec-conformant `encode_key` (sha256 of the
/// raw key bytes, then base64url), decoding would already yield the
/// hash and this helper would count leading zeros directly. Our
/// current `encode_key` skips the hash, so we have to hash the
/// decoded bytes ourselves for `key_to_layer` to apply. Once the
/// encoder is flipped to the spec form, this becomes a direct
/// `count_leading_zero_bits(decode_key(&e.key))`.
fn outermost_layer(entries: &[MstEntry], fanout: usize) -> usize {
let max_layer = max_layer_for_fanout(fanout);
let mut best = 0usize;
for e in entries {
// Decode the base64url-encoded key back to raw bytes, then hash
// those bytes through the same `key_to_layer` path used at put-time.
let raw = match decode_key(&e.key) {
Ok(b) => b,
Err(_) => continue,
crates/at-mst/src/util.rs
+23
View File
@@ -31,6 +31,29 @@ pub fn key_to_layer(raw_key: &str, fanout: usize) -> usize {
(zeros / 2).min(max_layer)
}
/// Encode a record key for storage in an MST entry.
///
/// **Note on spec compliance**: the atproto MST spec
/// (<https://atproto.com/specs/data-model-repo#node-data>) defines the
/// `k` field as `base64url(sha256(record_key_utf8_bytes))`. This
/// implementation emits `base64url(record_key_utf8_bytes)` directly
/// — i.e. it skips the SHA-256 step. Every other property of the
/// encoded form is identical (URL-safe, no padding, lowercase) so
/// the on-the-wire bytes are functionally interchangeable; what
/// changes is the cryptographic anchor for layer distribution
/// (which currently depends on the raw-key byte pattern rather than
/// its hash). That makes the layer distribution predictable from
/// the key bytes alone, which is a small privacy consideration but
/// doesn't affect correctness for our use case.
///
/// A future commit will flip to the spec encoding. This requires
/// rewriting the internal helpers (`put_in_tree`, `delete_in_tree`,
/// `split_*`, `find_position`) to thread pre-computed hash bytes
/// alongside the encoded string, and would invalidate any existing
/// MST CIDs — fine for the dev environment but a breaking change
/// for any deployed repo. Tracked as a follow-up: see
/// <https://github.com/bluesky-social/atproto/blob/main/packages/repo/src/util/mst.ts>
/// for the reference implementation to mirror.
pub fn encode_key(raw_key: &str) -> String {
use base64::Engine;
base64::engine::general_purpose::URL_SAFE_NO_PAD.encode(raw_key.as_bytes())