fix(digstore-chain): stop clearing a store's on-chain size on every commit

[MichaelTaylor3d]

DO NOT MERGE — draft, awaiting review

Summary

Confirms and fixes an on-chain data-loss defect: every digstore commit (owner path
and --writer-key path) silently cleared the store's on-chain size (bytes) and
size-proof (size_proof) fields, because update_store_metadata replaces the whole
on-chain metadata condition on every spend, and none of the three update builders in
crates/digstore-chain/src/singleton.rs had any way to carry the store's existing values
forward — all three hard-coded None, None.

The report vs. what I measured — narrower than the cited line, same defect

The report cited crates/digstore-chain/src/singleton.rs:447. That line belongs to
build_update_unsigned (the single-address variant) — which has zero callers anywhere
in this repo, dig-node, or hub.dig.net today (verified by grep across all three repos;
it's reachable only via its own build_update wrapper, which is itself uncalled outside
this crate's own tests). So the literal claim "every digstore commit" does not run
through that line.

The real, live path is different code with the identical bug, copy-pasted across
three sibling functions:

Function	Called by	Reachable from digstore commit?
build_update_unsigned (line ~433, cited line 447 is inside it)	build_update only	No — exported public API, zero callers anywhere in the ecosystem
build_update_unsigned_multi (line ~479)	anchor::build_advance_store_bundle ← ChainAnchor::update_root	Yes — this is what the CLI's commit.rs calls on every owner commit
build_update_unsigned_writer (line ~619)	anchor::build_advance_store_writer_bundle ← ChainAnchor::update_root_writer	Yes — the --writer-key deploy-token path

So the effect described ("every commit clears the size") is true, just attributed to
a dead sibling rather than the two live functions. All three shared the bug; all three
are fixed here.

Is the size actually lost today, on real stores?

bytes/size_proof are never set to Some(...) anywhere in this crate — mint always
passes None too (unchanged by this PR; a brand-new store genuinely has nothing prior to
preserve). So a store whose entire lifecycle has gone through this CLI exclusively
never had a value to lose.

The real exposure is cross-repo: bytes/size_proof are real CHIP-0035
DatastoreMetadata fields, and chip35_dl_coin (hub.dig.net's spend-builder, the
app/app/lib/convert.ts wasm-JSON bridge) round-trips metadata.bytes as ordinary data —
i.e. some other CHIP-0035-compliant tool (hub.dig.net, Sage, the reference dl CLI) can
set a real value. Any store that tool touches and this CLI later commits to would have
had that value silently erased, pre-fix. This crate has no local record of a value it
never stored, so that loss is not retroactively recoverable by this fix — it only stops
future erasure. No format/migration is needed: bytes/size_proof are optional,
additive CHIP-0035 fields and this PR does not touch the wire shape at all, only which
value gets asserted for them (§5.1 unaffected).

The fix

Read the store's own current on-chain bytes/size_proof (already in hand via the
freshly-synced Datastore each builder receives) into local bindings before the store
is moved into update_store_metadata, and pass those through instead of None, None.
This is the same "preserve, don't lose" direction already used for label/description
(re-sent from local config) and already documented for the delegated-puzzle set on
updateStoreOwnership ("always re-send existing delegates... or you silently drop the
admin") — same replace-semantics hazard, same remedy, now applied to the third field pair
that needed it.

Deliberately not attempting to recompute a real size from local content — the task
brief for this class of fix is explicit: prefer preserving an existing value over writing
a computed guess. A stale-but-real size is recoverable; a wrong one asserted confidently
is not distinguishable from a true one.

Regression tests (TDD: written first, watched fail for the right reason, then fixed)

All three new tests failed before the fix with the identical shape —
assertion left == right failed ... left: None, right: Some(...) — and pass after:

• build_update_preserves_existing_on_chain_size_and_size_proof — direct return-value
  check on build_update/build_update_unsigned.
• owner_update_preserves_existing_on_chain_size_on_simulator — the actual
  digstore commit path, validated on a real in-process Chia simulator
  (chia_sdk_test::Simulator): mints a store with a seeded bytes/size_proof, submits
  a real owner-authorized root-advance transaction, and asserts the resulting metadata
  still carries the original values.
• writer_update_preserves_existing_on_chain_size_on_simulator — same, for the
  --writer-key path.

Verified with the exact commands CI runs (not the workspace---all-features guess)

Read .github/workflows/ci.yml rather than assuming — its real gate omits
--all-features at the workspace level (two clippy lints are explicitly allowed) and
uses cargo nextest, not cargo test. Ran exactly that:

• cargo build -p digstore-guest --target wasm32-unknown-unknown --release --locked — guest wasm prereq, green.
• cargo fmt --all --check — clean.
• cargo clippy --workspace --all-targets --locked -- -D warnings -A clippy::default_constructed_unit_structs -A clippy::field_reassign_with_default — 0 warnings.
  (An earlier attempt with --all-features hit an unrelated LNK1120 link failure in
  digstore-prover's RISC0 build script — not part of CI's real command, not caused by
  this change; confirmed by matching CI's actual invocation, which is clean.)
• cargo build --workspace --locked — clean (after regenerating Cargo.lock for the
  version bump below — a cargo build started microseconds before the Cargo.toml edit
  landed on disk, so it silently built against the stale 0.29.1 manifest; caught by
  checking the compile log's version strings rather than trusting the exit code, then
  re-verified after cargo update --workspace --offline).
• cargo nextest run --workspace --locked --retries 2 — 1575 tests run: 1575 passed
  (3 slow), 17 skipped, 0 failed. All three new tests present and green; the rest of the
  workspace shows zero regressions.
• cargo test --doc --workspace --locked — clean (no runnable doc examples in this
  workspace; 0 failures).

Installed-binary integration (§3.5 HARD requirement — not unit tests alone)

• cargo install --path crates/digstore-cli --force --locked → dig-store.exe,
  digs.exe installed to ~/.cargo/bin.
• dig-store --version (PATH-resolved, confirmed not target/debug) → dig-store 0.29.2, matching the bump.
• Full end-to-end smoke cycle against the installed binary, DIGSTORE_ANCHOR_MOCK=1:
  dig-store init → dig-store add -A → dig-store commit — mint confirmed, content
  staged, commit "submitted", "confirmed", "Published a new version — it's live and
  permanent." (The mock backend doesn't itself exercise build_update_unsigned_multi —
  that's proven separately by the real-simulator regression test above — but this proves
  the CLI as a whole builds, installs, and runs this change with zero breakage.)
• The crates/digstore-cli/tests/* integration suite (599 assert_cmd-driven subprocess
  tests spawning the freshly-built binary) is included in the nextest run above — all
  green.

Version

digstore-chain — and the whole workspace, which shares one version via
[workspace.package] — bumped 0.29.1 → 0.29.2 (patch: behavior fix, no format/API
break). Read from Cargo.toml on disk after the fact (not the commit log), and
scripts/check-workspace-dep-versions.sh confirms all 11 in-repo path deps agree.

Ticket

No existing issue describes this defect (searched DIG-Network/digs open issues for
"size", "on-chain size", "singleton.rs", "update_store_metadata", "data loss",
vulnerabilities — no match). Nothing to Closes.

Scope note

This repo is one of the five whose nightly cron can cut an unattended stable release
(its stable job accepts event_name == 'schedule') — so this stays a draft, small, and
self-contained rather than something to "finish later" on a half-done branch.

Co-Authored-By: Claude noreply@anthropic.com

[MichaelTaylor3d]

IN PROGRESS — not the verdict. Correctness gate, fresh context.

Verified by RUNNING (fresh shallow clone of the PR head + dig-node) and by READING:

1. Read (diff + crates/digstore-chain/src/singleton.rs fetched at head SHA 6785bd67...):
   the store's bytes/size_proof are read into prev_bytes/prev_size_proof locals
   before store is moved by value into update_store_metadata(...) in all three sites
   (build_update_unsigned ~L444, build_update_unsigned_multi ~L494, build_update_unsigned_writer ~L640).
   Ordering is correct; nothing recomputes a size, it's a straight carry-forward.

2. Ran grep over a fresh shallow clone of digs at the PR head:
   build_update_unsigned_multi/_writer are called only from anchor.rs's
   update_root/update_root_writer, and digstore-cli/commands/commit.rs goes through
   that anchor path. build_update_unsigned (the cited line's function) has no caller in
   this repo besides its own build_update wrapper, and build_update/build_update_unsigned
   have zero callers outside this crate's own unit tests. So the PR's scope correction
   (live defect is in _multi/_writer, not the originally-cited function) checks out.

3. Ran grep over a fresh shallow clone of dig-node: zero hits for
   build_update_unsigned/singleton::build_update. Confirms dig-node does not call the
   dead function either.

4. Read chip35_dl_coin (core/wasm/app) — metadata.bytes/size_proof are real fields
   round-tripped in core/src/store.rs, wasm/src/types.rs, app/app/lib/convert.ts.
   Confirms the cross-repo exposure claim (a store touched by hub/Sage/dl before this
   CLI commits would have had the value erased pre-fix).

5. Mint path unchanged in the diff — confirmed by diff inspection, no mint-builder hunks.

6. Regression tests: read the 3 new tests. Two are simulator-backed
   (owner_update_preserves_existing_on_chain_size_on_simulator,
   writer_update_preserves_existing_on_chain_size_on_simulator) exercising real
   chia_sdk_test::Simulator spends, not just struct assertions — good, exercises the
   real commit path. Did not re-run the suite myself (draft PR, not my worktree to build
   in) — relying on the PR's own reported 1575 passed nextest run plus the 3 new tests'
   described pre-fix failure shape (left: None, right: Some(...)), which is the correct
   red-test shape for this bug (a cleared field reads back as None).

7. Cargo.toml/Cargo.lock diff: mechanical version bump 0.29.1→0.29.2 across the
   11 workspace members, consistent everywhere, patch-level correct for a behavior fix.
   No wire/format change — the diff touches only singleton.rs, version files, and
   DEVELOPMENT_LOG.md. §5.1 unaffected, confirmed by diff inspection (bytes/size_proof
   are pre-existing optional fields, not newly added).

8. CI status at head: two build & test jobs still IN_PROGRESS, CodeQL (rust) IN_PROGRESS,
   and supply-chain audit (cargo-deny) shows FAILURE. Checked recent main CI runs —
   all green — so this is not something the diff visibly caused (no new deps in this PR;
   only path-dep version bumps). Likely a fresh RUSTSEC advisory landing independent of
   this change, but NOT independently confirmed against main at the same moment — flag
   this as something to re-check before the PR leaves draft, not a correctness objection
   to the fix itself.

On the dead build_update_unsigned/build_update: given zero callers anywhere in the
ecosystem (verified above), it should be DELETED rather than carried forward fixed-but-dead
— a fourth copy of this exact bug pattern is how the third one (this PR fixes 2 of 3 live
copies plus this 1 dead one) survived un-caught for as long as it did. Recommend a follow-up
(or amend this PR) to delete build_update/build_update_unsigned and their 3 dedicated
unit tests, or state explicitly why it's kept as public API surface (e.g. external crate
consumers) — the PR itself doesn't say why it's kept.

[MichaelTaylor3d]

VERDICT: PASS (with one non-blocking recommendation, and one item to recheck before undraft)

Head SHA reviewed: 6785bd6

Summary: the fix is mechanically correct (read-before-move of the store's own
bytes/size_proof, carried forward instead of hard-coded None), the scope
correction (live defect is build_update_unsigned_multi/_writer, not the originally
cited build_update_unsigned) is verified true by grep across digs + dig-node, the
cross-repo chip35_dl_coin round-trip claim is verified true, mint is untouched, no wire
format change, version bump is correct and consistent, and the regression tests
(including two real-simulator spend tests) exercise the actual digstore commit path
and fail in the correct shape pre-fix.

Two non-blocking items, both stated as comments above and not gating this correctness
review:

1. build_update_unsigned/build_update are dead code (zero callers ecosystem-wide,
   verified). Recommend deleting them rather than leaving a fixed-but-dead fourth copy
   of the pattern — or state explicitly why they're kept as public API.
2. supply-chain audit (cargo-deny) is FAILURE at head while the two build & test
   jobs and CodeQL(rust) are still in-progress. Not traceable to this diff (no new deps;
   only internal path-dep version bumps), but re-verify it's pre-existing/unrelated to
   main before this leaves draft — do not undraft/merge on a red required check
   without checking that first (§2.4a).

Not verified by me directly (relied on the PR's own reported run): the full
1575 passed nextest run and the installed-binary (cargo install --force --locked)
smoke cycle — I did not rebuild in my own worktree. The mechanical read of the diff and
the independent grep-verification of the scope/cross-repo claims give me high confidence
in the fix itself regardless.