Skip to content

test(wallet): make the TxStatus fixture actually call chia_query's ack_to_tx_status - #444

Merged
MichaelTaylor3d merged 3 commits into
mainfrom
loop/438-fixture-coupling
Aug 31, 2026
Merged

test(wallet): make the TxStatus fixture actually call chia_query's ack_to_tx_status#444
MichaelTaylor3d merged 3 commits into
mainfrom
loop/438-fixture-coupling

Conversation

@MichaelTaylor3d

Copy link
Copy Markdown
Contributor

Closes #438. Child of https://github.com/DIG-Network/dig_ecosystem/issues/3166.

What changed

crates/dig-wallet/src/sage/spend.rs test fixture only. No production code.

The ack() fixture hand-built a chia_query::TxStatus from its own string
literals while its doc claimed a change to ack_to_tx_status "shows up here as a
failure". It could not — the crate's mapping was never called, so the coupling the
doc asserted did not exist.

ack() now takes a status byte and returns
chia_query::peer::translate::ack_to_tx_status(status). Named ACK_SUCCESS /
ACK_PENDING / ACK_FAILED / ACK_UNKNOWN constants keep the call sites reading
as the acks a full node actually sends.

Why it mattered

accepted_by_mempool compares against the literal "SUCCESS". If chia-query
renamed that label nothing in this repo would notice: the node would silently
refuse every push, and both tests would stay green. It fails closed, so it was
non-gating — but it is the born-false-coupling class this family has now hit five
times, and deleting the doc sentence would close the claim while leaving the gap.

Blast radius

impact on ack — the symbol is a #[cfg(test)] helper private to
sage::spend::tests, with exactly two callers, both in that module
(a_pending_ack_is_not_an_accepted_broadcast,
only_a_success_ack_reports_an_accepted_broadcast). Nothing outside the test
module can reach it, so the radius is the two tests changed here. Verified against
the shipped crate source that chia_query::peer::translate::ack_to_tx_status is
public and reachable under the native feature dig-wallet already enables
(lib.rs:34peer/mod.rs:8). accepted_by_mempool and
ChiaQueryBroadcaster::broadcast are untouched — sibling lane #441 is live on that
production path.

How verified

Proof the test is now load-bearing: with the label for status 1 changed in the
vendored chia-query source, the fixture goes RED; restored, it goes green. Both
outputs are in a comment below.

Version

0.179.00.179.1 (root workspace) and dig-wallet 0.42.00.42.1
patch, test-only, no behaviour change.

@MichaelTaylor3d

Copy link
Copy Markdown
Contributor Author

The mutation proof — the test is load-bearing

The edit is only worth anything if the fixture now catches what its doc claims. Proved by
renaming the label ack_to_tx_status produces for status 1, in a local vendored copy of
chia-query-0.19.0 patched in via [patch.crates-io] (worktree-local, uncommitted, and the
registry source itself was never touched):

-        1 => "SUCCESS",
+        1 => "ACCEPTED",

RED — with the label renamed

test sage::spend::tests::only_a_success_ack_reports_an_accepted_broadcast ... FAILED

thread 'sage::spend::tests::only_a_success_ack_reports_an_accepted_broadcast'
panicked at crates\dig-wallet\src\sage\spend.rs:1054:48:
status 1 is mempool admission: Error { kind: Api, message: "the network did not admit the
transaction to its mempool (ack: ACCEPTED); nothing is pending on chain for this bundle" }

test result: FAILED. 15 passed; 1 failed; 0 ignored; 0 measured; 712 filtered out

That panic message is exactly the production failure the ticket predicted: a renamed label makes
accepted_by_mempool refuse an ack the full node meant as admission. Before this change the same
rename produced a clean green.

GREEN — patch removed, Cargo.lock restored

test sage::spend::tests::a_pending_ack_is_not_an_accepted_broadcast ... ok
test sage::spend::tests::only_a_success_ack_reports_an_accepted_broadcast ... ok

test result: ok. 16 passed; 0 failed; 0 ignored; 0 measured; 712 filtered out

What it does and does not cover

It catches a label rename, which is the defect filed here. It does not catch a change to
the success flag alone — accepted_by_mempool deliberately ignores success because of the
1 || 2 conflation, which is the separate crate-level defect DIG-Network/chia-query#48. Stating
that explicitly so this fixture is not later read as guarding more than it does.

@MichaelTaylor3d

Copy link
Copy Markdown
Contributor Author

Doc overclaim corrected — 2d3a12e

The doc I added in 61e4ccd said a flipped success flag would also fail these tests. It would
not. accepted_by_mempool reads only status, deliberately, because success conflates ACCEPTED
with PENDING (DIG-Network/chia-query#48). That sentence reproduced the born-false coupling claim
this ticket exists to remove, one line below the fix. Corrected to state what the fixture actually
catches and to name the flag as out of scope by design, not by omission:

/// This CALLS `chia_query::peer::translate::ack_to_tx_status` rather than restating its
/// mapping, so the coupling is real: rename a label there and these tests fail instead of
/// silently accommodating it. A hand-built `TxStatus` would leave [`accepted_by_mempool`]'s
/// `"SUCCESS"` literal free to drift out of agreement with the crate, and the node would refuse
/// every push with both tests still green.
///
/// Boundary: this does NOT guard the `success` flag. [`accepted_by_mempool`] reads only
/// `status`, deliberately, because `success` conflates ACCEPTED with PENDING
/// (DIG-Network/chia-query#48) — so a change to that flag is invisible here by design, not by
/// omission.

Mutation proof re-run against the corrected doc

Still load-bearing. Same technique — a worktree-local vendored chia-query-0.19.0 patched in via
[patch.crates-io], so the shared registry source stayed untouched for the sibling lanes.

RED, with 1 => "SUCCESS" renamed to "ACCEPTED":

test sage::spend::tests::only_a_success_ack_reports_an_accepted_broadcast ... FAILED
panicked at crates\dig-wallet\src\sage\spend.rs:1059:48:
status 1 is mempool admission: Error { kind: Api, message: "the network did not admit the
transaction to its mempool (ack: ACCEPTED); nothing is pending on chain for this bundle" }
test result: FAILED. 15 passed; 1 failed

GREEN, patch removed and Cargo.lock restored:

test sage::spend::tests::a_pending_ack_is_not_an_accepted_broadcast ... ok
test sage::spend::tests::only_a_success_ack_reports_an_accepted_broadcast ... ok
test result: ok. 16 passed; 0 failed

Versions read from disk before pushing: workspace 0.179.1, dig-wallet 0.42.1. Diff remains
the spend.rs test module plus the two bumps.

@MichaelTaylor3d
MichaelTaylor3d force-pushed the loop/438-fixture-coupling branch from 2d3a12e to d376c2c Compare August 31, 2026 04:12
@MichaelTaylor3d

Copy link
Copy Markdown
Contributor Author

Rebased onto main (past #437's 2bcf045c) — d376c2c

git rebase --onto origin/main 649d25a (the squash-safe form). Conflicts were the two version
manifests only; resolved to 0.185.0 in Cargo.toml and Cargo.lock. dig-wallet stays
0.42.1, still above main's 0.42.0. No dropping in the rebase output.

Diff sanity after the rebase — 46 lines, no CRLF blowup:

 Cargo.lock                          |  4 ++--
 Cargo.toml                          |  2 +-
 crates/dig-wallet/Cargo.toml        |  2 +-
 crates/dig-wallet/src/sage/spend.rs | 38 +++++++++++++++++++++----------------
 4 files changed, 26 insertions(+), 20 deletions(-)

spend.rs has zero CR bytes (it was already an LF file and stayed one), and
cargo fmt --all -- --check is clean workspace-wide.

Mutation proof re-run on the rebased tree — still load-bearing

Checking the test count, not the exit status, so a filter that matched nothing could not read as
a red run. Both runs report running 16 tests.

RED, with 1 => "SUCCESS" renamed to "ACCEPTED" in a worktree-local chia-query patched in via
[patch.crates-io]:

running 16 tests
test sage::spend::tests::only_a_success_ack_reports_an_accepted_broadcast ... FAILED
panicked at crates\dig-wallet\src\sage\spend.rs:1059:48:
status 1 is mempool admission: Error { kind: Api, message: "the network did not admit the
transaction to its mempool (ack: ACCEPTED); nothing is pending on chain for this bundle" }
test result: FAILED. 15 passed; 1 failed; 0 ignored; 0 measured; 712 filtered out

GREEN, patch removed and Cargo.lock restored:

running 16 tests
test sage::spend::tests::a_pending_ack_is_not_an_accepted_broadcast ... ok
test sage::spend::tests::only_a_success_ack_reports_an_accepted_broadcast ... ok
test result: ok. 16 passed; 0 failed; 0 ignored; 0 measured; 712 filtered out

Checks at d376c2c, read unpiped

All 14 SUCCESS (Attach packages to the release SKIPPED — not a PR-path job). Still DRAFT.

The doc overclaim you flagged was already fixed in 2d3a12e before that message arrived; it is
carried through the rebase and the boundary sentence is in the shipped diff.

@MichaelTaylor3d
MichaelTaylor3d marked this pull request as ready for review August 31, 2026 04:38
MichaelTaylor3d and others added 3 commits August 30, 2026 21:41
…stating it

The `ack()` fixture hand-built a `TxStatus` from its own string literals while
its doc claimed a change to `ack_to_tx_status` would "show up here as a failure".
It could not: the crate mapping was never called. `accepted_by_mempool` compares
against the literal "SUCCESS", so a rename in chia-query would refuse every push
with both tests still green.

The fixture now takes a status byte and returns
`chia_query::peer::translate::ack_to_tx_status(status)`, making the asserted
coupling real.

Refs #438

Co-Authored-By: Claude <noreply@anthropic.com>
The doc added in the previous commit claimed the tests catch a flipped `success`
flag. They do not: `accepted_by_mempool` reads only `status`, deliberately,
because `success` conflates ACCEPTED with PENDING (DIG-Network/chia-query#48).
Claiming otherwise reproduced -- one line below the fix -- the born-false coupling
claim this change exists to remove.

State what the fixture does catch (a label rename, proven by mutation) and name
the flag as out of scope by design rather than by omission.

Refs #438

Co-Authored-By: Claude <noreply@anthropic.com>
@MichaelTaylor3d
MichaelTaylor3d force-pushed the loop/438-fixture-coupling branch from d376c2c to 2078873 Compare August 31, 2026 04:41
@MichaelTaylor3d

Copy link
Copy Markdown
Contributor Author

Rebased onto main past #440's d089ab1 (3953a2a, v0.183.0) — 2078873

git rebase --onto origin/main 2bcf045c. Conflicts were the two version manifests only, exactly as
predicted; resolved to 0.185.0 in Cargo.toml and Cargo.lock. No dropping in the output,
and the versions were re-read from disk immediately before the push, not from git log.

Version sanity against the new main:

mine main
workspace 0.185.0 0.183.0
dig-wallet 0.42.1 0.42.0

Diff vs origin/main is unchanged at 46 lines, and spend.rs still has zero CR bytes — the
conflict resolution read and wrote with newline='', so no line endings flipped:

 Cargo.lock                          |  4 ++--
 Cargo.toml                          |  2 +-
 crates/dig-wallet/Cargo.toml        |  2 +-
 crates/dig-wallet/src/sage/spend.rs | 38 +++++++++++++++++++++----------------
 4 files changed, 26 insertions(+), 20 deletions(-)

cargo fmt --all -- --check clean workspace-wide; working tree clean.

Checks at 2078873, read unpiped

All 14 SUCCESS, zero non-green (Attach packages to the release SKIPPED — not a PR-path job).

The mutation proof was not re-run for this rebase: the conflict touched only the two version
manifests, and crates/dig-wallet/src/sage/spend.rs is byte-identical to the tree the proof ran
against at d376c2c (RED on the label rename, GREEN restored, running 16 tests both times).
Saying so explicitly rather than implying fresh evidence.

@MichaelTaylor3d
MichaelTaylor3d merged commit 9fdd797 into main Aug 31, 2026
15 checks passed
@MichaelTaylor3d
MichaelTaylor3d deleted the loop/438-fixture-coupling branch August 31, 2026 04:57
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.

the TxStatus fixture claims a coupling to ack_to_tx_status that does not exist — a rename would silently refuse every push

1 participant