chore(deps): drop the placeholder-genesis dig-constants copy, pin to the chia tip - #199
Conversation
21e56a4 to
e18f239
Compare
b7fcb89 to
f5e55cf
Compare
MichaelTaylor3d
left a comment
There was a problem hiding this comment.
VERDICT: CHANGES-REQUIRED (recorded as a comment review - GitHub rejects a request-changes review from the PR author identity, 422).
Correctness gate — CHANGES-REQUIRED (one gating finding)
Head reviewed: f5e55cf590d8257ecac2fd50b63e3586e20b5de4. Read-only, from an isolated worktree.
The chain-identity question, answered: YES, the node is now on ONE correct chain identity.
Verified independently of anything the crate asserts about itself.
-
The genesis value at this head.
dig-constants0.9.0 shipsDIG_MAINNET_GENESIS_CHALLENGE = 0af981862a4df51f51ec59c312315d959931d917c375730b89b9e2b0854d1abf. Checked against the chain, not against the crate:POST api.coinset.org/get_block_record_by_height {"height":9021277}returnsheader_hash 0x0af981862a4df51f51ec59c312315d959931d917c375730b89b9e2b0854d1abf. A real, verifiable Chia mainnet header hash — not a placeholder, and deliberately NOT the Chia L1 genesis (ccd5bb71…, which the crate carries separately asCHIA_L1_MAINNET_AGG_SIG_ME). The DIG-L2 / Chia-L1 split is intentional and correctly documented. -
All six AGG_SIG domains re-derived from scratch.
sha256(genesis || opcode)for opcodes 43,44,45,46,47,48 reproduces all six published values byte-for-byte;agg_sig_me== genesis. Correct percondition_tools.py. -
One identity across the whole binary. The lock still holds four copies (0.4.0 / 0.5.1 / 0.8.0 / 0.9.0), but the full
DIG_MAINNETconst body is identical across all four — field for field, includingmax_block_cost_clvmandcost_per_byte. So the four copies are a type-unification nuisance, not an identity split. The 0.1.0 all-zeros copy is GONE from the lock and nothing regressed to an older copy. That is exactly the improvement #2072 asked for. -
0.4.0 → 0.9is genuinely additive. Programmatic diff of everypub const: 12 added (DIG_ASSET_ID,DIG_TREASURY_*,CHIA_L1_*_AGG_SIG_ME,DEK_SALT,PROFILE_*,SYMMETRIC_KEY_LEN,IDENTITY_IKM_VERSION,DIG_LOCAL_HOST,RPC_DIG_NET_URL), zero removed, zero changed. CLVM cost limits, generator limits, hard-fork heights untouched; the public method set onNetworkConstantsis identical. The 0.9→0.10 claim also checks out — plot-consensus field renames plus upstream's removal ofmax_generator_size; no DIG value moves. -
dig-clvmgit rev → crates.io 0.2.diff -rbetween the git tree at35677103and the publisheddig-clvm-0.2.2:src/is byte-identical, public symbols identical. The only delta is the manifest —dig-constants = "0.1.0"→"0.9". That makes "zero source changes" a consequence, not a coincidence: the API did not move because the code did not move. -
The 0.10 deferral holds. 0.10 moves to chia-protocol 0.36.1 while
[patch.crates-io]vendors the 0.26 fork; a secondchia_protocolin the graph is a real type-unification break, not an excuse.
The PR body, the manifest comments, and the DEVELOPMENT_LOG entry are unusually good — the reasoning is checkable, and every checkable claim I checked held.
What blocks merge
One thing: this is a bugfix that ships without the regression guard that would have caught the bug (§2.2). Detail inline on crates/dig-node-core/Cargo.toml.
Notes that do NOT block (resolved by me)
- Version collision from #189: none. This head is already rebased on
0ea5d252; main is 0.101.0, this PR is 0.102.0. Clean. dig-walletwas NOT bumped by this PR. The 0.12.2 → 0.13.0 move came with #189 and is already on main, so a behaviour-relevant dependency change lands on dig-wallet at an unchanged version. Acceptable: the crate is workspace-internal (thedig-walleton crates.io is an unrelated package), and this repo's stated convention (crates/dig-node-core/Cargo.toml:2-18) bumps a library version on PUBLIC-SURFACE movement, which did not occur. No consumer pin breaks.- dig-constants check (both questions). (1) Nothing in this diff defines a shared value locally — it removes one. (2) One pre-existing candidate, not introduced here:
crates/dig-node-core/src/peer.rs:80definesDEFAULT_NETWORK_ID = "DIG_MAINNET"locally and dig-constants 0.9 does not publish it. That string is the gossip discovery namespace, byte-identical across dig-node / dig-gossip / the relay, so it belongs indig-constantsby the same rule that putDIG_RELAY_URLandDIG_NODE_PORTthere. Moving it needs a dig-constants release plus the cascade this PR already documents as deferred — a ticket for the #2072 family, not a blocker on this diff. - Gates. 15/15 green on this exact head, including
Test + coverage(7m04s) and Clippy.mergeStateStatus: CLEAN, zero pre-existing review threads. I did not re-run the suite locally; CI onf5e55cfis stronger evidence, and the fourdig-node-corelive-chain failures (#2317) reproduce on unmodified main.
…the chia tip dig-node's lock held FOUR dig-constants versions in one binary (0.1.0, 0.4.0, 0.5.1, 0.8.0). Only two of the seven holders are dig-node's own crates; the rest are pinned by upstream crates' PUBLISHED ranges, which a consumer cannot edit. The copy that mattered was 0.1.0, reached through dig-clvm 0.1.1 (a git rev in dig-wallet). dig-constants 0.1.0 shipped an all-zeros PLACEHOLDER DIG L2 genesis challenge with all six AGG_SIG domains derived from it — self-consistent, so no derivation test could see it — and 0.4.0 finalized the real values. dig-wallet's spend-validation ValidationContext therefore described a different chain identity than the rest of the node. Nothing was mis-signed: that call site sets DONT_VALIDATE_SIGNATURE and the signing domain is injected by the caller. Moving dig-clvm to crates.io 0.2 (also closing a §3.6 git-dep) removes the copy. dig-clvm's own requirement was `>=0.1.0, <0.2.0`, so it could never have resolved forward off the placeholder on its own — which is why this sat as long as it did. The suite could not see any of that, before the fix or after it: no source pins the genesis literal, and every runtime check compares `dig_constants::DIG_MAINNET.genesis_challenge()` against itself, which passes identically under the real value and under the placeholder. So the guard goes where the defect is actually decided, in `dependency_tree.rs` against the lock: no dig-constants copy below 0.4.0. Stated as a FLOOR, not `!= "0.1.0"` — 0.2.x and 0.3.x carry the same placeholder, and this is the same property the release gate enforces at 0.4.0 (#178). Proven RED by reverting both dependency edits: 0.1.0 returns to the lock and this is the only test in the workspace that fails. The node's own two crates move 0.4 -> 0.9. Values are identical from 0.4.0 through 0.10.0, so this is value-neutral. 0.9 — not the 0.10.0 tip — is correct: 0.10.0 moved to chia-protocol 0.36.1 / chia-wallet-sdk 0.34 while this workspace builds against 0.26 / 0.30 with dig-gossip's vendored chia-protocol fork, so depending on it links a second chia_protocol and DIG_MAINNET.genesis_challenge() stops type-unifying (measured: 11 errors). Reaching 0.10 is the chia 0.36 migration, not a dependency bump. The three copies that survive (0.4.0/0.5.1/0.8.0/0.9.0) are held down by upstream crates' PUBLISHED ranges. Their full DIG_MAINNET const bodies are IDENTICAL, cost limits included, so what remains is a type-unification nuisance rather than an identity split; collapsing it is a cleanliness cascade, not a correctness fix. Version: minor, for the dig-wallet spend-validation constants changing value (placeholder -> real genesis) — observable behaviour. Workspace 0.101.0 -> 0.102.0; dig-wallet is already 0.13.0 from #189. Refs DIG-Network/dig_ecosystem#2072 Co-Authored-By: Claude <noreply@anthropic.com>
f5e55cf to
ea82230
Compare
|
Merging. The guard is the right shape and I checked it rather than taking the RED on trust: Three of your realizations are worth more than this PR and I want them recorded rather than lost in a lane transcript: A constant compared only against itself is unguarded no matter how many assertions mention it. Both Verifying a constant from inside the dependency graph is structurally impossible. You could only compare crate versions against each other; the coinset
And the self-correction is the one I value most: you scoped a five-step cross-repo publish order as closing an identity split, then re-labelled it as type-unification cleanup once the reviewer diffed the const bodies across the versions you were not changing. Noticing that your own remediation was scoped against an unverified premise, and downgrading it, is worth more than getting it right first time. |
What this does
Removes the
dig-constantscopy that was actually wrong, and puts dig-node's own crates on the tip of the chia line they build against.dig-wallet:dig-clvmmoves from a git rev (resolving to 0.1.1) to crates.io0.2— also closing a §3.6 git-dep.dig-node-core+dig-node-service:dig-constants0.4→0.9.No source changes were needed; both bumps compile as-is.
The finding — one constant value moved, and dig-node was on the wrong side of it
dig-constants0.1.0 shipped an all-zeros PLACEHOLDERDIG_MAINNET_GENESIS_CHALLENGE, with all six AGG_SIG additional-data domains correctly derived from that placeholder. Self-consistent, so no derivation test could catch it. 0.4.0 finalized the real challenge (0af98186…) and recomputed all six domains.That 0.1.0 copy reached the binary through
dig-clvm0.1.1, anddig-wallet'srun_and_validateused its re-exportedDIG_MAINNETas the spend-validationValidationContext— describing a different chain identity than every other subsystem in the same process.Nothing was mis-signed. That call site sets
DONT_VALIDATE_SIGNATURE, andWalletSigner'sagg_sig_datais injected by the caller rather than read fromDIG_MAINNET; the CLVM cost limits that were consulted (max_block_cost_clvm,cost_per_byte) are identical across every version. So it was latent, not live — one refactor away from mattering. This PR closes it.Every other version is value-neutral. 0.4.0 → 0.10.0 is purely additive (
DIG_ASSET_ID, treasury hash/address, DEK labels,dig.local,rpc.dig.net, the profile-sealing label); 0.9.0 → 0.10.0 changes only upstream chia plot-consensus field names, no DIG value.Why 0.9 and not the 0.10.0 tip
dig-constants0.10.0 moved tochia-protocol0.36.1 /chia-wallet-sdk0.34. This workspace builds against 0.26 / 0.30, including thechia-protocolforkdig-gossipvendors through[patch.crates-io]. Depending on 0.10 links a secondchia_protocol, andDIG_MAINNET.genesis_challenge()returns aBytes32no function here accepts — measured, 11 errors of the formexpected BytesImpl<32>, found chia_protocol::bytes::BytesImpl<32>.Being current on
dig-constantsis downstream of migrating dig-node to chia 0.36. It is a platform migration wearing a dependency bump's clothes.The gate in #178 does NOT pass, and cannot pass in this repo
scripts/check-dig-constants-current.shfrom #178, run against this branch's lock:The remaining three copies are held down by published ranges in other repos, which a consumer cannot edit:
dig-gossip(git rev)>=0.2, <0.5dig-nat0.18.0,digstore-chain(git rev)>=0.4, <0.6/^0.5dig-download0.17.0^0.8Collapsing to one 0.9.0 needs a release-first cascade:
dig-gossipanddigstore-chainvia a git-rev move once their mains bump,dig-nat→ 0.18.1 anddig-download→ 0.17.3 as patch releases againstdig-constants 0.9(patch keeps^0.18/^0.17consumers resolving forward, avoiding a second cascade through dig-dht/dig-peer/dig-peer-selector). Reaching 0.10.0 additionally requires the chia 0.36 migration across all of them.The regression guard (gate finding, added)
crates/dig-node-core/tests/dependency_tree.rs—no_dig_constants_copy_predates_the_real_genesis_challenge.The suite could not see this defect before the fix or after it.
grep 0af98186 --include=*.rsfinds nothing — no source pins the genesis literal — and every runtime check (peer.rs:3857,:3927) comparesdig_constants::DIG_MAINNET.genesis_challenge()against itself. That is circular: it passes identically under the real value and under the placeholder.spend.rs:1090is not a guard either; a tampered bundle fails under 0.1.0 constants exactly as under 0.9.So the assertion goes where the defect is decided — the lock — and is stated as a FLOOR: no
dig-constantscopy below 0.4.0. Not!= "0.1.0", because 0.2.x and 0.3.x carry the same placeholder and an equality check is bypassed by the next one. This is deliberately the same property, in the same words, as the 0.4.0 floor #178 now enforces at release time — one rule, two levels.It also asserts the resolved set is non-empty, so a rename or a botched merge that removes
dig-constantsfrom the lock cannot satisfy it vacuously.Proven RED: reverting this PR's two dependency edits returns 0.1.0 to the lock and this is the only test in the workspace that fails:
What the remaining duplicates actually are
Their full
DIG_MAINNETconst bodies are identical across 0.4.0/0.5.1/0.8.0/0.9.0, CLVM cost limits included. The surviving copies are a type-unification nuisance, not an identity split — the cascade below is a cleanliness task, not a correctness emergency.Blast radius
cargo tree -i dig-constants@{0.1.0,0.4.0,0.5.1,0.8.0}onorigin/mainnamed all seven holders; the two owned by this repo are the two edited.dig_constantsis referenced at 10 sites in this workspace (DIG_MAINNET.genesis_challenge(),DIG_RELAY_URL,DIG_NODE_PORT) — all three values unchanged 0.4 → 0.9.dig_clvmis used at exactly one site,dig-wallet/src/sage/spend.rs.How verified
cargo build --workspace --all-targets,cargo fmt --all --check,cargo clippy --workspace --all-targets -- -D warningsall clean.cargo test --workspace: 794 passed, 4 failed indig-node-core --lib— pre-existing, reproduced identically on an unmodifiedorigin/maintree in the same worktree (pinned root is not the current on-chain root, -32005 vs -32008; they reach a live chain resolver).Version: minor. Workspace
0.101.0→0.102.0;dig-walletis already 0.13.0 from #189.Refs DIG-Network/dig_ecosystem#2072