Skip to content

fix(deps): move chia-wallet-sdk 0.34.0 -> 0.36.0 to heal the internal chia split - #36

Merged
MichaelTaylor3d merged 6 commits into
mainfrom
loop/3161-chia-wallet-sdk-036
Aug 26, 2026
Merged

fix(deps): move chia-wallet-sdk 0.34.0 -> 0.36.0 to heal the internal chia split#36
MichaelTaylor3d merged 6 commits into
mainfrom
loop/3161-chia-wallet-sdk-036

Conversation

@MichaelTaylor3d

@MichaelTaylor3d MichaelTaylor3d commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

WIP — DO NOT MERGE. Gate round pending.

Task

Move chia-wallet-sdk 0.34.0 → 0.36.0, healing the internal chia split in datalayer-driver.

Parent epic: https://github.com/DIG-Network/dig_ecosystem/issues/3161

Published 4.0.0 ships internally split: every chia primitive at 0.36.1 beside chia-wallet-sdk at 0.34.0. Because the split lives inside the published manifest, no consumer can escape it by bumping — which is why this crate is the sole holdout blocking two ready adoptions (digs/digstore-chain, dig-wallet-backend).

Same shape as dig-chainsource-interface 0.3.1, fixed in its #7; the manifest-coherence guard there is adapted here.

The ceiling

chia-wallet-sdk 0.36.0 is the newest published version — verified live against index.crates.io. The primitives publish at 0.48 and are unreachable while the SDK sits at 0.36, so 0.36.1 primitives beside 0.36.0 chia-wallet-sdk is the coherent maximum. The primitives were already at their ceiling and are deliberately left alone; bumping them to 0.48 would re-split the crate, not modernise it.

Feature-flag check

All four declared features exist in 0.36.0 — read from the crates.io index, not assumed. A silently-dropped feature is a capability loss that compiles, so this was checked rather than inferred:

0.36.0 publishes action-layer, chip-0035, chip-0037, native-tls, offer-compression, peer-simulator, rustls — a superset of the four this crate declares (chip-0035, native-tls, peer-simulator, action-layer). Nothing dropped.

API deltas adapted

The churn is a capitalisation rename, not a signature change. DataStore already carried its <M> metadata parameter in 0.34, so no generic parameter was added:

0.34 0.36
DataStore Datastore
DataStoreInfo DatastoreInfo
DataStoreMetadata DatastoreMetadata
UpdateDataStoreMerkleRoot UpdateDatastoreMerkleRoot

Adapted across src/lib.rs (public re-exports and five fn signatures), src/types.rs (SuccessResponse::new_datastore), src/wallet.rs (the bulk — from_spend, owner_create_coin_condition, get_recreation_memos, new_metadata_condition, the mint metadata literal and the KAT), and napi/src/.

No shim, no bridging. Every call site moved to the 0.36 spelling.

Verification / VerificationAsserter / VerifiedData — not applicable here

These are removed in 0.36 and their removal is genuinely breaking in general, so it was checked rather than assumed. A repo-wide grep across *.rs, *.toml, *.ts and *.js returns zero occurrences: this crate never referenced them, so their removal is a non-event for it and does not contribute to the bump rationale.

The npm/TypeScript surface is UNCHANGED

The rename is absorbed at the NAPI alias boundary (Datastore as RustDataStore). The JS-facing DataStore / DataStoreMetadata structs deliberately keep their names, because an upstream Rust capitalisation change is not observable to a JS consumer and renaming them would break every one of them for nothing. napi/index.d.ts is byte-unchanged, and the three DataStore mentions in README.md are all JavaScript examples that remain correct.

Evidence

The KAT reproduces UNMODIFIED — this is the load-bearing result

wallet::melt_kat::from_spend_reports_owner_melt_as_missing_child is a real peer-simulator mint → melt that pins the custody-relevant signal digstore-chain's melt classifier depends on: a childless datastore singleton spend surfaces as Err(DriverError::MissingChild), while a spend that recreates the datastore surfaces as Ok(Some(_)).

Its expected values were not touched. Only the type's spelling changed. Had either signal moved in 0.36 the test would have gone red rather than needing an edit — which is precisely the evidence that the downstream classifier is still safe. It carries its own positive control (the launcher spend), so it distinguishes the real property from an implementation that returns MissingChild for every datastore spend.

Full suite, --locked

running 5 tests   (lib)
test xch_server_coin::tests::test_morph ... ok
test examples::example_nft_minting ... ok
test xch_server_coin::tests::test_puzzle_hash ... ok
test examples::example_key_operations ... ok
test wallet::melt_kat::from_spend_reports_owner_melt_as_missing_child ... ok
test result: ok. 5 passed; 0 failed

running 4 tests   (tests/manifest_chia_coherence.rs)
test result: ok. 4 passed; 0 failed
  • cargo clippy --all-targets --all-features -- -D warnings → exit 0 (read unpiped)
  • cargo fmt --all -- --check → exit 0
  • cargo check --locked --workspace --all-targets → clean, including the napi member
  • Cargo.lock updated via cargo update -p, committed, and every run passes --locked

The lock: what actually changed, and what legitimately did not

The chia-sdk-* family moved 0.34.00.36.0 (10 entries). The crate's declared set is now one line per family:

chia-protocol/chia-bls/chia-consensus/chia-traits/chia-puzzle-types/clvm-traits/clvm-utils at 0.36.1, chia-wallet-sdk at 0.36.0, chia-puzzles 0.20.3, clvmr 0.16.2.

Older chia lines remain in the resolved lock and are correct. Each was traced, not assumed:

  • chia-bls 0.28.2, chia-traits 0.28.2, chia_streamable_macro 0.28.2, chia-sha2 0.34.0 — reached from clvmr 0.16.4, internal to the CLVM evaluator.
  • chia-bls 0.42.1, chia-traits 0.42.1, chia_streamable_macro 0.42.1, chia-ssl 0.42.1 — reached from chialisp 0.4.6, itself pulled by chia-sdk-types and rue-lir.

None is declared by this workspace and none reaches a public signature — proven by the fact that the crate compiles against the SDK's own types, which is what unification means. cargo tree -d cannot be the gate here: chia-wallet-sdk fails it on its own vendored graph, so any correct implementation fails it.

The regression guard

tests/manifest_chia_coherence.rs fails if this workspace ever again declares two minor lines of one chia-* family.

No behavioural test can catch this defect — both halves of a split run correctly in isolation, which is exactly why 4.0.0 shipped. The defect was manifest coherence, and that is mechanically assertable.

It is extended beyond the dig-chainsource-interface original in one way that matters here: this workspace declares the chia set twice (Cargo.toml and napi/Cargo.toml), so the two manifests are pooled into one coherence judgement and a fourth test pins that they agree on every shared dependency. A guard reading only the root manifest would be blind to half the surface.

Revert-proof

Committed first, and reverted by file copy (never git checkout <path>).

(a) A root-manifest split does not even compile. Setting chia-wallet-sdk back to 0.34.0 in Cargo.toml fails the build, before any test runs — 5 errors, e.g. OracleLayer::new expecting BytesImpl<32> and finding [u8]. That is stronger evidence than a red test: it proves the source adaptation is genuinely load-bearing rather than cosmetic.

(b) So the guard was proven on the half that DOES compile silently — a split in napi/Cargo.toml, which is a separate workspace member and builds fine while wrong. This is the real silent case, and two tests fired, each naming the defect:

---- every_chia_family_declares_a_single_minor_line ----
#3161 REGRESSION: `chia-wallet-sdk` is declared at 0.34.0 in `napi/Cargo.toml` but the
chia 0.36 line (chia-wallet-sdk + the chia/clvm primitives) is pinned to 0.36.x.
  left: "0.34"   right: "0.36"

---- the_two_manifests_agree_on_every_shared_chia_dependency ----
`chia-wallet-sdk` is declared at differing versions across this workspace's manifests:
{("Cargo.toml", "0.36.0"), ("napi/Cargo.toml", "0.34.0")}
  left: 2   right: 1

test result: FAILED. 2 passed; 2 failed

Restored, and green again at the real versions — the truthful control that the guard does not fire on the legitimate non-uniform ceiling (0.36.0 SDK beside 0.36.1 primitives): 4 passed; 0 failed.

The lock pollution the guard's own doc warns about happened, and was caught. The --offline proof run silently re-resolved Cargo.lock (md5 4da4235…fd5cb67…). Restoring only the manifest would have shipped a two-line split created by the proof itself. --locked refused loudly on the next run; the lock was restored from its copy and verified byte-identical before committing.

Which direction a wrong version fails in

Loudly, at compile time, in both directions — this adoption cannot fail silently:

  • Source adapted to 0.36 against a 0.34 manifest → 5 hard type errors (measured above).
  • Source left at 0.34 against a 0.36 manifest → the original 2 cannot find type errors this branch started from.
  • A split confined to napi/ compiles, and that is the silent case — which is exactly the hole the new guard closes.

The one genuinely silent risk would have been a behavioural change in the melt signal, and the KAT covers it.

Blast radius checked

Within the crate, the Rust compiler is the exhaustive oracle for a type rename — every call site must compile, and cargo check --locked --workspace --all-targets is clean. Cross-repo, every consumer was grepped directly:

consumer pins affected?
digs/digstore-chain datalayer-driver = "4" the only real one — re-exports DataStore and constructs DataStoreMetadata (src/singleton.rs:13,21,100). Two spellings to change when it adopts 5.x.
dig-collateral-coin "3.0.0" no
dig-wallet-backend comment only; DigDataStoreMetadata is dig_merkle's own type no
anchor-watcher transitive via digstore-chain prose in doc comments only
dig-mirror-coin explicitly does NOT depend on it no

Nothing breaks on merge: every consumer pins ^4 or ^3, so a 5.0.0 release cannot be picked up implicitly. Adoption is the deliberate follow-up this PR exists to unblock.

Tooling note (§2.0 bound 2): gitnexus was not indexed for this worktree; blast radius was established by the compiler (exhaustive for a rename), plus ripgrep across all consuming manifests and sources. Stated rather than glossed.

Bump

Cargo 4.0.05.0.0 (MAJOR). The public re-exports DataStore / DataStoreInfo / DataStoreMetadata are renamed, SuccessResponse::new_datastore changes type, and five public fn signatures change. A consumer writing datalayer_driver::DataStore breaks, so this is breaking and is labelled as such — the BREAKING CHANGE: footer is on the commit.

npm @dignetwork/datalayer-driver 3.0.03.0.1 (patch). The JS surface and index.d.ts are unchanged; only the native addon is rebuilt against 0.36. The bump is required because publish-npm fires on push to main and would otherwise fail on a duplicate version. The two artifacts are already independently versioned (Cargo 4.x vs npm 3.x), so they are bumped on their own merits.

Also in this PR

  • #![allow(clippy::result_large_err)] with a WHY comment. 0.36 grew ClientError to ~136 bytes, pushing WalletError past clippy's 128-byte threshold and failing -D warnings at three sites. Boxing the variant would fix it properly but reshapes the crate's public error enum and every construction and match site — a deliberate refactor, not part of a dependency move. Suppressed explicitly and reported as a follow-up rather than silently widened or quietly ignored.
  • napi/package-lock.json root version corrected from 0.1.38 to match package.json. Pre-existing drift, two lines, in a file this PR already touches.
  • KAT doc comment updated from "under 0.34" to "across the 0.34 → 0.36 move", and its assertion message likewise. Wording only; no expected value changed.

@MichaelTaylor3d

Copy link
Copy Markdown
Contributor Author

PARKED — resume-ready. Stub only; the adaptation work was not pushed.

A lane stalled at the 600s watchdog part-way through src/wallet.rs. It had pushed the stub commit and this draft PR but no source changes (files=0), so that work is gone and must be redone. Nothing here is salvageable; nothing here is wrong either.

The task, restated so it needs no re-derivation

Move chia-wallet-sdk 0.34.0 → 0.36.0. Nothing else.

datalayer-driver 4.0.0 — the latest published release, and main matches it — ships internally split: every primitive is already at the ceiling and only the SDK edge is behind.

chia-protocol      = "0.36.1"   # already correct
chia-bls           = "0.36.1"   # already correct
chia-consensus     = "0.36.1"   # already correct
chia-traits        = "0.36.1"   # already correct
chia-puzzles       = "0.20.3"   # already correct
chia-puzzle-types  = "0.36.1"   # already correct
chia-wallet-sdk    = { version = "0.34.0", features = [...] }   # <-- the only line to move

Because the split lives inside the published manifest, consumers cannot escape it by bumping. Identical shape to dig-chainsource-interface 0.3.1, fixed in its #7 (merged bac9031f) — read that PR for the pattern.

Why it is worth a lane

It is the sole holdout blocking two ready adoptions:

  • digs/digstore-chain — a lane made the change, measured it, and correctly reverted rather than shimming.
  • dig-wallet-backend — same blocker.

The rest of the family already moved: dig-wallet-backend 0.31.0, dig-cat/dig-tips/dig-offers 0.3.0, dig-options 0.4.0.

The API deltas, already measured from the consumer side — expect real churn

  • DataStoreDatastore<DataStoreMetadata> — a generic parameter added, so every mention changes shape.
  • Verification, VerificationAsserter, VerifiedData are REMOVED in 0.36.

src/wallet.rs holds the largest set of affected call sites — that is where the previous attempt was working when it stalled.

Adapt the call sites; never bridge two versions with a shim (§4.1 byte-drift). If Verification/VerifiedData are part of this crate's public API, their removal is genuinely breaking for consumers and the bump must say so.

Check all four feature flags still exist in 0.36.0chip-0035, native-tls, peer-simulator, action-layer. A silently-dropped feature is a capability loss that compiles.

Traps that cost time elsewhere in this epic today

  • The ceiling is chia-wallet-sdk 0.36.0, not the newest on crates.io. The primitives publish at 0.48 and are unreachable while the SDK sits at 0.36. Bumping them would be a defect. Verify live: .claude/scripts/chia-ceiling-audit.sh.
  • A Cargo.toml bump must reach Cargo.lock — CI passes --locked, and an unlocked local build silently rewrites the lock and stays green.
  • If a revert-proof edits the manifest, restore the lock too and re-run with --locked. Running tests in the reverted state silently re-resolved 217 lines on a sibling PR today, leaving a real split behind.
  • If required checks read ABSENT — never ran, do not force it. See https://github.com/DIG-Network/dig_ecosystem/issues/3163 — a fresh PR ref sometimes escapes and sometimes does not. Never --admin past absent checks.
  • Push early. This lane's loss is the argument: the stub survived, three files of adaptation did not.

Parent epic: https://github.com/DIG-Network/dig_ecosystem/issues/3161

MichaelTaylor3d and others added 4 commits August 26, 2026 11:01
Salvaged from a stalled lane's worktree. Not yet compiled; pushed
immediately so the adaptation cannot be lost a second time.

Co-Authored-By: Claude <noreply@anthropic.com>
The two call sites the stalled lane had not reached. `cargo check
--locked --all-targets` is now clean.

Co-Authored-By: Claude <noreply@anthropic.com>
The 4.0.0 split was a manifest-coherence defect, which no behavioural
test can see: both halves of a split run correctly in isolation. Pins
per-family MAJOR.MINOR agreement, pooling Cargo.toml and napi/Cargo.toml
so a split cannot hide in the bindings half.

Co-Authored-By: Claude <noreply@anthropic.com>
The JS-facing `DataStore`/`DataStoreMetadata` structs keep their names
on purpose: they are the published npm/TypeScript surface, and an
upstream capitalisation change is not observable to a JS consumer.

Co-Authored-By: Claude <noreply@anthropic.com>
@MichaelTaylor3d

Copy link
Copy Markdown
Contributor Author

Resumed and COMPLETE — ready for the gate. Do not undraft, do not merge.

Branch loop/3161-chia-wallet-sdk-036 · head bee1976 · was 3ea4413 (stub, files=0)

Correction to the parked note: the work was NOT gone

The note said the stalled lane's source changes "must be redone". They were recoverable — the lane's worktree survived on disk with all of it uncommitted (Cargo.toml, Cargo.lock, napi/Cargo.toml, src/lib.rs, src/types.rs, src/wallet.rs). It was committed as the first action of this lane, before anything else, and only two call sites in src/wallet.rs were still unadapted.

Worth recording for the loop generally: a dead lane's absence of pushed commits does not prove its work is gone — check its worktree before redoing anything. Redoing it would have cost a full re-derivation of ~40 adapted call sites that were already correct.

What is done

  • chia-wallet-sdk 0.34.00.36.0 in both manifests; every other chia dep left at its ceiling.
  • The 0.36 rename (DataStoreDatastore, DataStoreInfo/DataStoreMetadata, UpdateDataStoreMerkleRoot) adapted across src/ and napi/src/. No shim.
  • npm/TypeScript surface unchanged — rename absorbed at the NAPI alias boundary; index.d.ts byte-identical.
  • tests/manifest_chia_coherence.rs added, extended to pool both manifests. Revert-proved: 2 tests fire on a napi/-only split (the silent case that compiles); a root-manifest split fails the build outright.
  • The custody KAT from_spend_reports_owner_melt_as_missing_child passes with expected values unmodified — the melt signal digstore-chain depends on is unchanged in 0.36.
  • Bump: Cargo 4.0.05.0.0 (major, renamed public re-exports); npm 3.0.03.0.1 (patch, JS surface unchanged, required because publish-npm fires on merge).

Gate evidence

rust-checks pass, CodeQL + all three Analyze jobs pass. Locally: clippy -D warnings exit 0, fmt --check exit 0, cargo check --locked --workspace --all-targets clean, 9 tests green under --locked. The 5 cross-platform native build jobs were still running at time of writing — the gate must confirm them before merge.

Full evidence, the traced lock analysis, the blast-radius table and the revert-proof transcripts are in the PR body.

Flagged for the gate

  1. #![allow(clippy::result_large_err)] was added. 0.36 grew ClientError to ~136 bytes, pushing WalletError past clippy's threshold at three sites. Boxing the variant is the real fix but reshapes the crate's public error enum — deliberately out of scope for a dependency move, and called out rather than buried.
  2. main is NOT branch-protected on this repo (gh api .../branches/main/protection → 404, "Branch not protected"). There is no required-context set, so §2.4a's by-name assertion has nothing to read and publish-npm/publish-crates fire on any push to main. Reported, not changed — provisioning is the orchestrator's call.
  3. digs/digstore-chain is the only consumer that must edit code to adopt 5.xsrc/singleton.rs:13,21,100, two spellings. It pins "4", so nothing breaks on merge.

Parent epic: https://github.com/DIG-Network/dig_ecosystem/issues/3161

@MichaelTaylor3d

Copy link
Copy Markdown
Contributor Author

Correction to my parked note above: it was wrong.

It said the stalled lane's work "is gone and must be redone". It was not gone — the worktree survived on disk with every source change uncommitted, and only two call sites in src/wallet.rs were still unadapted. Redoing it would have re-derived ~40 already-correct call sites.

I inferred "lost" from files=0 on the PR, which measures what was pushed, not what exists. A dead lane's absence of pushed commits does not prove its work is gone — check the worktree first. Recorded so the next resume note does not repeat it.

@MichaelTaylor3d

Copy link
Copy Markdown
Contributor Author

loop-security — interim findings (audit in progress)

Head audited: bee197670b5112cc5a86ea059a350717d8334991. Base d3ddffcc.

Verdict not yet formed. Posting as findings land, per the loop's post-as-you-go rule.

1. The rename dispute is SETTLED — no generic was added. Lane's account is correct.

The PR's own pre-image proves it. At base, src/wallet.rs:526 already read:

let new_store = DataStore::<DataStoreMetadata>::from_spend(

The <M> parameter existed in 0.34 and was already being passed explicitly. The
consumer-side account elsewhere in the epic (DataStore -> Datastore<DataStoreMetadata>,
i.e. a generic being added) is WRONG. Confirmed against both published sources:

  • 0.34 chia-sdk-driver-0.34.0/src/primitives/datalayer/datastore.rs:
    pub struct DataStore<M = DataStoreMetadata>
  • 0.36 chia-sdk-driver-0.36.0/src/primitives/datalayer/datastore.rs:
    pub struct Datastore<M = DatastoreMetadata>

Same arity, same default. Nothing is hiding behind a defaulted type parameter.

2. Verification / VerificationAsserter / VerifiedData — zero occurrences, confirmed.

Grepped the whole worktree (excluding target/, node_modules) across .rs/.ts/.js/.json.
Zero hits. Their removal upstream is a non-event for this crate and correctly is NOT
cited as a reason for the major bump.

3. The on-chain custody surface is byte-identical across 0.34 -> 0.36.

This was the question that actually mattered: whether a "capitalisation rename" was
carrying a spend-encoding change with it. It is not.

chia-sdk-types/src/condition.rs differs between the two versions by exactly four
diff lines
— one rename, nothing else. The merkle-root update condition is unchanged
in opcode and field layout (chia-sdk-types-0.36.0/src/condition.rs:196-200):

UpdateDatastoreMerkleRoot {
    opcode: i8 if -13,
    new_merkle_root: Bytes32,
    ...memos: Vec<Bytes>,
},

Identical to 0.34's UpdateDataStoreMerkleRoot. DatastoreMetadata's ToClvm/FromClvm
impls are also unchanged apart from the type name, so metadata tree hashes are stable and
a store minted under 4.0.0 remains updatable under 5.0.0.

4. 0.36 CLOSES two defects that 0.34 shipped. Both are arguments FOR this move.

(a) A reachable panic on an adversarial memo is fixed.
chia-sdk-driver-0.34.0/src/primitives/datalayer/datastore_info.rs:79-81 parsed the
oracle fee as:

let oracle_fee: u64 = BigInt::from_signed_bytes_be(&remaining_memos.remove(0))
    .to_u64_digits()
    .1[0];

to_u64_digits() returns an EMPTY digit vector for zero, so .1[0] is an index-out-of-
bounds panic. An empty atom is the canonical CLVM encoding of zero, so this panics on
a legitimately-encoded zero fee — and on anything a hostile peer chooses to encode that
way. sync_store parses spends fetched from a peer, and under NC-12 every dialled peer is
untrusted, so this was a remote panic primitive.

0.36 (chia-sdk-driver-0.36.0/.../datastore_info.rs:79-86) returns 0 for the empty
memo and Err(DriverError::InvalidMemo) on overflow instead of silently truncating to
the low 64-bit digit. Correct, and fail-closed.

(b) A missing verify-then-use check on the metadata updater is added.
0.34's from_spend called NftStateLayer::get_next_metadata
(chia-sdk-driver-0.34.0/src/layers/nft_state_layer.rs:134-159), which ran the updater
puzzle revealed IN THE SPEND with no check that it matches the updater puzzle hash
committed in the state layer
. 0.36 routes the same call through run_metadata_updater
(chia-sdk-driver-0.36.0/src/primitives/nft/metadata_update.rs:47-49), which gates it:

if tree_hash(allocator, updater_puzzle_reveal) != current_metadata_updater_puzzle_hash.into() {
    return Err(DriverError::MetadataUpdaterPuzzleHashMismatch);
}

Under 0.34 a hostile peer serving a fabricated spend to sync_store could reveal an
arbitrary updater puzzle and have the driver compute an arbitrary new_metadata
including an arbitrary root_hash — and hand it back as the store's current state. 0.36
refuses. Legitimate on-chain spends are unaffected (the state-layer puzzle enforces the
same hash on chain), so the behaviour change is strictly in the fail-closed direction.

Neither of these is claimed in the PR body. They should be, because they change the
character of this PR from "dependency hygiene" to "dependency hygiene that also picks up
two upstream security fixes."

Continuing on: version-bump justification, the npm .d.ts byte-identity claim, the KAT
revert-proof, the result_large_err suppression, feature-flag presence, and consumer
blast radius.

@MichaelTaylor3d

Copy link
Copy Markdown
Contributor Author

loop-security — STOP. The stated merge consequence is wrong, in both directions.

Head audited: bee197670b5112cc5a86ea059a350717d8334991.

The gate brief says a squash-merge "immediately publishes crate 5.0.0 to crates.io
and npm 3.0.1". I measured the trigger. What actually happens on merge is:

registry outcome on merge why
crates.io 5.0.0 DOES NOT PUBLISH no line of the squash message is a bare semver
npm 3.0.1 PUBLISHES (--tag next) an unanchored regex matches PROSE in the commit body

Both publish jobs run, the workflow goes green, and the crates.io step prints
Not a release, skipping crates.io publish. Nothing is red. Nothing is missing from the
rollup. The epic's blocker is simply not removed.

The mechanism

.github/workflows/CI.yml:319-322 and :269-272 gate on push-to-main, but the
publish STEPS gate on a regex over the commit message.

publish-crates, .github/workflows/CI.yml:352-359:

if git log -1 --pretty=%B | grep "^[0-9]\+\.[0-9]\+\.[0-9]\+$";
then
  cargo login $CARGO_REGISTRY_TOKEN
  cargo publish --no-verify --allow-dirty
else
  echo "Not a release, skipping crates.io publish"
fi

It requires a line that is exactly a bare semver. I ran that regex against the six
commit messages this branch will squash: zero matches. The squash subject will be the
PR title (squash_merge_commit_title: COMMIT_OR_PR_TITLE, and with >1 commit GitHub uses
the PR title), which begins fix(deps):.

publish-npm, .github/workflows/CI.yml:303-310, has a second, unanchored-at-the-end
branch:

elif git log -1 --pretty=%B | grep "^[0-9]\+\.[0-9]\+\.[0-9]\+";
then
  npm publish --tag next --access public

grep matches line-by-line over the whole body. squash_merge_commit_message is
COMMIT_MESSAGES, so every commit body is concatenated verbatim at column 0. Line 46 of
that concatenation, from bee1976's own body, is:

0.36.1 beside chia-wallet-sdk at 0.34.0. Because the split lived inside

That line begins with 0.36.1 and matches. So the merge publishes
@dignetwork/datalayer-driver@3.0.1 to npm because of a wrapped sentence in a commit
message.

This is corroborated three ways, not just by my reading of the regex

  1. Release history. Every real release in this repo is a commit whose SUBJECT is a
    bare semver — 7040fc1 = 4.0.0, 92518b2 = 3.0.0, 421147c = 0.1.50, and so on
    back through 0.1.38. That is the npm version convention, and it is what this CI was
    built to detect. This PR uses Conventional Commits instead, so the trigger is absent.

  2. Current registry state. crates.io serves 2.0.0, 3.0.0, 4.0.0 — no 5.0.0. npm
    dist-tags.latest is 3.0.0. The two registries are already divergent.

  3. The base commit says so itself. d3ddffc (PR ci(publish): let npm trusted publishing actually engage, and guard publish on the ref #35, the immediately preceding merge)
    was written by a previous lane and states, verbatim:

    "why npm still serves 3.0.0 against a Cargo.toml at 4.0.0"

    "The elif is also unanchored and matched line-by-line, so any line of a squash body
    beginning with a version string could trigger a --tag next publish."

    PR ci(publish): let npm trusted publishing actually engage, and guard publish on the ref #35 fixed the npm OIDC auth and added the ref guard. It did not fix the trigger,
    and explicitly documented the hazard that is about to fire.

Why this gates rather than being a note

Not because the code is unsafe — the diff is clean and I will say so separately. It gates
because merging produces a green run that means the opposite of what it looks like:

  • digs/digstore-chain and dig-wallet-backend stay blocked. crates.io keeps serving
    datalayer-driver 4.0.0, still internally split at chia-wallet-sdk 0.34.0. Any lane
    dispatched afterwards to adopt datalayer-driver = "5" fails to resolve.
  • The orchestrator records a success that did not happen, bumps a pointer on it, and the
    epic's next step is planned against a version that does not exist. This is the
    half-cut-release class: green run, no artifact, no red anywhere to catch it.
  • Meanwhile npm is published irreversibly, and 3.0.1 reaches every JS consumer on
    ^3 — dist-tags do not restrict range resolution, so --tag next is cosmetic here.
    The result is a registry pair that is MORE divergent after the merge than before it:
    npm carrying a build of source version 5.0.0 (chia-wallet-sdk 0.36), crates.io still
    serving 4.0.0 (chia-wallet-sdk 0.34).

What would make this land as intended

Cheapest correct fix, and it matches every prior release in this repo: land the version
bump as a commit whose subject is the bare version, and merge with a squash subject
that is a bare semver — or replace the commit-message regex with a real trigger (a tag, or
a version-vs-registry comparison). A release control that fires on incidental prose in a
commit body is not a control.

Whatever is chosen, please also reword bee1976's body so no line begins with a version
string
, or the npm half fires unintentionally regardless.

Remaining checks (major-bump justification, .d.ts byte-identity, the KAT revert-proof,
result_large_err, feature flags, consumer blast radius) are still in progress and will
follow. Two of them are already confirmed clean and are in my earlier comment.

@MichaelTaylor3d
MichaelTaylor3d force-pushed the loop/3161-chia-wallet-sdk-036 branch from bee1976 to 3cc1ae9 Compare August 26, 2026 19:23
…plit

datalayer-driver 4.0.0 shipped internally split: every chia primitive sat
at the 0.36.1 ceiling while chia-wallet-sdk stayed on the 0.34 line.
Because the split lived inside the published manifest, no consumer could
escape it by bumping, which blocked digs/digstore-chain and
dig-wallet-backend simultaneously.

BREAKING CHANGE: the re-exported driver types follow chia-wallet-sdk
0.36's rename -- `DataStore`, `DataStoreInfo` and `DataStoreMetadata`
are now `Datastore`, `DatastoreInfo` and `DatastoreMetadata`. The
published npm/TypeScript surface is UNCHANGED: the rename is absorbed at
the NAPI alias boundary, so `DataStore` remains the JS interface name.

Note for anyone editing this message: no line of this body may BEGIN with
a version string. CI.yml's publish-npm step greps the squash body
line-by-line with an unanchored pattern, so a wrapped sentence starting
"0.36.1" fires an unintended npm publish. That is not hypothetical -- it
was the state of this commit until the gate on PR#36 measured it.

Co-Authored-By: Claude <noreply@anthropic.com>
@MichaelTaylor3d

Copy link
Copy Markdown
Contributor Author

loop-security VERDICT: PASS (with two RELEASE-time conditions)

Head audited: 463680d4b5653ff2768228eb7960a7b9cba0a505 - re-resolved from
gh pr view 36 --json headRefOid after the head moved mid-audit. My earlier STOP verdict was
measured against bee1976.

Why the earlier probes still hold at the new head

bee1976 and 463680d resolve to the SAME tree, 269bf43c8295ddd99bba950cb7de89a94f9d3442.
The amend changed the commit message only, not one byte of code. Every code-level probe below
therefore applies verbatim to 463680d; nothing carries over on faith. The one thing that genuinely
changed is the commit text - which is precisely the input to the publish trigger - so I re-ran that
measurement from scratch.

Re-measured trigger at 463680d: the npm half is DEFUSED, the crates half is unchanged

job regex it uses at bee1976 at 463680d
publish-crates bare-semver, anchored both ends 0 matches 0 matches
publish-npm elif bare-semver, anchored at start ONLY 1 match 0 matches

The only body line still starting with a digit is the one beginning 0.36's rename, which cannot
match: after 0.36 comes an apostrophe, not a second dot, so there is no third numeric group.

Merging 463680d publishes NOTHING. Both jobs run and skip. That removes the irreversible
action I gated on - the unintended @dignetwork/datalayer-driver@3.0.1 npm publish will not happen.
The crates.io half still will not fire, now correctly tracked as DataLayer-Driver#37 rather than
papered over; keeping a release-control change out of a dependency PR is the right call.

CI at this head is terminal and green: 18 SUCCESS, 2 SKIPPED (Publish to NPM,
Publish to Crates.io), asserted by name since there is no branch protection to read a required set
from.


The eight brief items - all verified

1. Rename vs added generic - SETTLED: pure capitalisation rename, no generic added.
The PR's own pre-image proves it: at base, src/wallet.rs:526 already read
DataStore::<DataStoreMetadata>::from_spend. Both published sources agree - 0.34 declares
pub struct DataStore<M = DataStoreMetadata>, 0.36 declares
pub struct Datastore<M = DatastoreMetadata>. Same arity, same default.
The earlier consumer-side measurement claiming the generic was ADDED is wrong, and nothing is
hiding behind a defaulted type parameter.

2. Verification / VerificationAsserter / VerifiedData - zero occurrences repo-wide
(.rs/.ts/.js/.json, excluding target/ and node_modules). Their upstream removal is a non-event
here and is correctly NOT used to justify the major bump.

3. Major 4.0.0 -> 5.0.0 - correct; neither over- nor under-bump.
Public-surface diff across lib.rs + types.rs + wallet.rs: 3 items removed, 3 added, all
rename pairs - zero genuine removals or additions.
Confirmed individually: 3 renamed re-exports
(src/lib.rs:31-33); SuccessResponse::new_datastore retyped (src/types.rs:21); 5 public fns in
lib.rs - oracle_spend:276, update_store_metadata:290, update_store_ownership:311,
melt_store:325, async_api::sync_store:554; plus SyncStoreResponse::latest_store
(src/wallet.rs:477) and the wallet:: equivalents, which the lane's count of five understates in
the safe direction. use datalayer_driver::DataStore no longer compiles, so major is required.

4. npm .d.ts byte-identity - CONFIRMED; the PATCH is justified.
napi/index.d.ts and napi/index.js are tracked and unchanged in the diff. The sole
napi_lib.rs hunk is the use block only - no #[napi] item touched. The JS-facing structs
deliberately keep their names (napi/src/napi_lib.rs:106,237), so the generator's input is
unchanged too.

5. Melt KAT - non-vacuous, mutation-proved in BOTH directions.
It recomputes from a live Simulator mint -> melt with both spends landed on chain: no fixtures, no
reloading. "Expected values untouched" is true because there are NO pinned values - only pinned
signals, which is the right shape for what digstore-chain's classifier depends on.

  • Probe 1: melt assertion changed to InvalidMemo -> RED, reporting got Err(MissingChild).
  • Probe 2: positive control changed to is_none() -> RED.

It would have gone red had 0.36 moved the melt signal.

6. #![allow(clippy::result_large_err)] - acceptable in a dependency move.
It is a clippy perf lint with no security meaning, names exactly one lint, and masks nothing else
(cargo clippy --all-targets --all-features -D warnings is green). It is also pre-existing, not
introduced here
: src/wallet.rs:1 already carried the module-level allow, and
src/dig_collateral_coin.rs:152,177,213,266 carry four more, from f6cd59b in 2025-12.
src/lib.rs:24 widens it to crate scope and documents why. Boxing would reshape a public error enum
inside a dependency PR; deferring it is right.

7. Feature flags - all four present in published 0.36.0: action-layer, chip-0035,
native-tls, peer-simulator. Verified from two independent sources (the vendored
chia-wallet-sdk-0.36.0/Cargo.toml and the crates.io index). The feature set is identical
between 0.34.0 and 0.36.0 - nothing silently dropped.

8. Blast radius - pins confirmed safe; nothing breaks on merge.
digstore-chain "4", dig-collateral-coin "3.0.0", npm chia-block-listener ^0.1.35. None
admits 5.0.0 or 3.0.1 automatically.

Correction to item 8: adoption is a two-level cascade, not a three-line edit

The brief cites only src/singleton.rs:13,21,100. In fact digstore-chain/src/singleton.rs names
DataStore/DataStoreMetadata at ~13 sites, and line 21 is load-bearing - it re-exports the
renamed type:

pub use datalayer_driver::{DataStore, DelegatedPuzzle, PublicKey};

So the rename propagates into digstore-chain's OWN public API - it needs its own major bump, and its
consumers (hub.dig.net anchor-watcher) inherit it.
modules/crates/00-foundation/dig-collateral-coin/src/lib.rs:1 is a second re-export layer, pinned
at datalayer-driver = "3.0.0". Plan the epic for two levels.

Additional security checks beyond the brief

  • The on-chain custody surface is byte-identical across 0.34 -> 0.36. This was the question that
    actually mattered - whether a "capitalisation rename" carried a spend-encoding change with it. It
    does not. chia-sdk-types/src/condition.rs differs by exactly four diff lines, the rename
    alone. UpdateDatastoreMerkleRoot keeps opcode -13, new_merkle_root: Bytes32 and its
    trailing memos: Vec<Bytes>. DatastoreMetadata's ToClvm/FromClvm are unchanged apart from
    the type name, so metadata tree hashes are stable and a store minted under 4.0.0 stays updatable
    under 5.0.0.

  • 0.36 CLOSES two upstream defects - both arguments FOR this move, neither claimed in the PR body:

    • 0.34's datastore_info.rs:79-81 parsed the oracle fee by indexing element 0 of
      to_u64_digits(), which panics on the canonical CLVM encoding of zero (an empty atom
      yields an empty digit vector). That is a remote panic primitive, because sync_store parses
      spends served by peers and NC-12 holds every dialled peer untrusted. 0.36 returns 0 for the
      empty memo and errors on overflow instead of silently truncating.
    • 0.34's from_spend ran the metadata-updater puzzle revealed in the spend with no check that
      it matched the updater hash committed in the state layer; 0.36's metadata_update.rs:47-49
      refuses on mismatch, so a hostile peer can no longer make the driver compute an arbitrary
      root_hash and hand it back as the store's current state.

    Both changes are fail-closed. Legitimate on-chain spends are unaffected.

  • The split is genuinely healed. All ten chia-sdk-*/chia-wallet-sdk crates move uniformly
    0.34.0 -> 0.36.0 with no half-move. Crates resolving to more than one version are identical
    before and after
    (5 crates - the upstream-vendored clvmr/chialisp lines the new guard's own
    doc comment documents). Two new packages, include_dir + include_dir_macros 0.7.4, arrive
    transitively via rue-compiler 0.8.5; both established and benign.

  • The new 295-line coherence guard is real, mutation-proved. Reintroducing the exact 4.0.0 split
    fires 2 of 4 tests with the intended message; a patch-level cross-manifest disagreement fires the
    dedicated test. It carries its own anti-vacuity assertion
    (tests/manifest_chia_coherence.rs:165-170) and pins exact membership, so a dropped dependency
    cannot hide.

  • Secrets: clean. No key, token, credential or projectId introduced, logged, or committed.

  • No new endpoint, RPC, permission, privilege elevation, deserialization, path, or amplification
    surface; nothing peer-reachable is added. .gitignore gains .gitnexus, which is correct.


TWO CONDITIONS ON THE RELEASE (not on this merge)

A. #37 does NOT cover the worst part of that job - fixing it narrowly ARMS an unverified publish

I checked #37's body: it does not mention --no-verify or --allow-dirty. But the publish step at
.github/workflows/CI.yml:359 runs cargo publish with both flags.

--no-verify means the crate is never built from its packaged tarball before it is published
irreversibly, and --allow-dirty packages whatever is in the working tree - while the immediately
preceding step (CI.yml:337-349) deliberately runs cargo update and commits Cargo.lock first. A
resolution that does not compile publishes anyway, with no recall.

This is dormant only because the trigger never fires. The moment #37 is fixed it becomes live
on the very next release.
It is the same anti-pattern CLAUDE.md records as a durable guard REMOVED
from chia-l2-consensus after a P0 in which a published crate carried a different rule than its source.

Add this to #37 (or file a sibling) and fix both together. Fixing the trigger alone converts a
silent no-op into an unverified irreversible publish - strictly worse than today.

B. Nothing in this pipeline will ever check that 5.0.0 packages and builds

Because CI publishes with --no-verify, run cargo package --locked (or
cargo publish --dry-run) before the real 5.0.0 release. I did not run it, and no gate does.


Branch protection (dig_ecosystem#3164): recommend PROVISION-FIRST

Reasoning, not preference.

  • The usual case for merge-then-provision is urgency: "this merge publishes irreversibly, do not
    block it." That premise is now measured false - merging 463680d publishes nothing. There is
    no race to win, so the strongest argument for merge-first has evaporated.
  • Merge-and-file is the weakest option: it leaves main unprotected on a repo whose publish jobs
    fire on push to main, with zero required checks, no conversation-resolution gate and no
    force-push protection - contrary to section 3.6b - for as long as the ticket sits.
  • CI.yml must be edited anyway to resolve npm prerelease match is unanchored (prose publishes), and crates.io publishes with --no-verify --allow-dirty #37, so provisioning in the same pass is nearly free.

The two caveats that make provision-first safe, and they are the part worth carrying into #3164:

  1. Derive required contexts from the checks that actually RAN on 463680d, never from
    CI.yml's job keys. Matrix jobs report under expanded name: templates - for example
    stable - x86_64-unknown-linux-gnu - node@20 and Test bindings on Linux-x64-gnu - node@20 -
    which do not match the job keys build and test-linux-x64-gnu-binding. Requiring a key
    that never reports blocks every PR forever under strict=true.
  2. Never require Publish to NPM or Publish to Crates.io. Both report SKIPPED on pull
    requests - confirmed on this PR's rollup at both heads - and SKIPPED is not SUCCESS, so
    requiring either deadlocks every future PR.

Safe minimal required set, taken from this head's rollup: rust-checks; the five
stable - <target> - node@20 contexts; the eight Test bindings on ... contexts; and CodeQL.
With strict=true, required_conversation_resolution=true, enforce_admins=false, squash-only,
required_linear_history=true, zero required approvals.


What I could NOT verify

  • That 5.0.0 packages and builds from its own tarball - condition B above. Nothing checks it.
  • Runtime behaviour of the npm native addon on any platform. The type surface is byte-identical
    and the Rust-side behaviour changes are fail-closed, but I ran no JS consumer. Stated plainly: the
    patch bump DOES carry a real behaviour change - a syncStore that previously succeeded on a
    malformed oracle memo (silently truncating) or on a mismatched metadata updater now throws. The
    direction is safer, but it is more than "identical .d.ts" implies.
  • The claim that 0.36 grew ClientError to ~136 bytes. Not measured; it is a clippy perf
    threshold with no security consequence either way.
  • Whether GitHub renders squash bodies at column 0. Now moot: at 463680d zero lines match
    either regex, so the outcome is publish-nothing under every possible formatting.

Acknowledgement

I understand a PASS authorises a squash-merge into an unprotected main on a repo whose publish
jobs fire on push to main, and that a crates.io or npm publish is irreversible with no recall. I
pass because I measured that this head publishes neither - not because I assume it is safe.
If the commit message is edited again before merge, the trigger measurement must be re-run,
because the message IS the trigger.

Probe disclosure

All mutation probes ran in a worktree I cut myself, C:\tmp\worktrees\dld-secaudit-36 (detached),
now removed. I mutated no shared checkout. The single command run inside the lane's worktree was
cargo test --locked --lib melt_kat, which writes target/ only, while the lane was idle (no cargo
or rustc processes). Its Cargo.lock md5 is ff0a3f66... before and after, and
git status --porcelain there shows only the pre-existing untracked .claude/, AGENTS.md and
CLAUDE.md. The coherence-guard probe deliberately ran the pre-built test binary against
mutated manifests, so Cargo.lock was never re-resolved at all.

@MichaelTaylor3d
MichaelTaylor3d marked this pull request as ready for review August 26, 2026 19:40
@MichaelTaylor3d
MichaelTaylor3d merged commit b72e82a into main Aug 26, 2026
20 checks passed
@MichaelTaylor3d
MichaelTaylor3d deleted the loop/3161-chia-wallet-sdk-036 branch August 26, 2026 19:40
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