feat(mirror): a bond page names WHICH WALLET its amounts are about - #43
Merged
Conversation
Every figure on a bond page -- each `unfunded` shortfall, each bonded amount, `locked_dig_base_units` -- is a statement about the node's own machine-custody operator wallet, not the user's. The payload named an amount and no wallet, which is the literal shape of the misdiagnosis this family exists to fix: a node said `unfunded, short 1010` while its operator's own wallet held 1,015,000 base units of $DIG. Both true, each about a different wallet, and nobody could tell from the answer. `MirrorBondStatesResult::Known` gains `funding_wallet`, typed as the same `WalletOperatorAddressResult` the dedicated method returns -- reused rather than restated, so the two surfaces cannot drift into two spellings of one fact, and so a node with no wallet says `not_initialized` here too instead of a blank string a client might render as a destination. On the ANSWER, not per row, and the distinction is not cosmetic. The funding wallet is node-wide, so a per-row copy would be one string repeated for every entry: a field that cannot vary while reading as though it could, and two rows of one answer could then be written to disagree. One value per answer can be wrong; it cannot be inconsistent with itself. Carried rather than left to a second call for the same reason `epoch` is carried, in that field's own words: a second call is a second observation and may have moved. A page of amounts can be rendered, screenshotted and acted on before a follow-up returns. Also corrects a README row that still said "seven states, six of which" after 0.28.0 added `unadvertised` -- drift introduced by that PR and caught here. Co-Authored-By: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The question, and the answer
Does a mirror bond row carry the funding address? It did not.
MirrorBondStatesResult::Knowncarriedentries,complete,cursor,locked_dig_base_unitsandepoch— every one of them a figure, and none of them a wallet.That is the literal shape of the misdiagnosis this family exists to fix: a node reported
unfunded, short_dig_base_units: 1010while its operator's own wallet held 1,015,000 base units of $DIG. Both statements were true, each was about a different wallet, and the payload named an amount and no wallet, so nobody could tell which. So: added.But on the ANSWER, not on each row — and that is a design decision, not a shortcut
Sage's transaction table carries a per-row address because a transaction's counterparty genuinely varies per row. A bond page's funding wallet does not: it is node-wide, one wallet for every entry.
So a per-row copy would be the same string repeated N times — a field that cannot vary while reading as though it could. Worse, two rows of one answer could then be written to disagree, and a client would have to pick which to believe. One value per answer can be wrong; it cannot be inconsistent with itself.
funding_wallettherefore sits onKnown, besideepoch, and it is carried rather than left tocontrol.wallet.operatorAddressfor the reasonepoch's own doc already gives — "without consulting a second method whose answer may have moved in between". A page of amounts can be rendered, screenshotted and acted on before a follow-up call returns.Typed as the method's own result, reused rather than restated
funding_wallet: WalletOperatorAddressResult— the exact typecontrol.wallet.operatorAddressreturns. Two consequences, both deliberate:not_initializedhere too, rather than a blank string a client might render as a destination on a funding screen.This follows the crate's existing pattern of reusing a taxonomy instead of copying it —
Deferred { reason: CollateralUnknownReason }does the same.The one judgement call, stated plainly
A required field is stricter than a new enum variant. A new variant leaves old payloads decoding fine; a required key means an old node's payload will not decode in a new client. I am shipping it as required, and as a MINOR, because that is this contract's stated posture rather than an oversight:
completedeliberately carries nodefaultandcursorusesrequired_option, both on the reasoning that an absent key must not decode into a value. Afunding_walletthat could be absent-and-defaulted would reintroduce exactly the silence this field removes.If that reads as major to a reviewer, the counter-argument is legitimate and the remedy is a version label, not a design change.
Version — 0.30.0, not 0.28.0
The instruction asked for all three changes in one 0.28.0. Two had already merged and published while those messages were in flight, and a published version is immutable:
MirrorBondState::Unadvertised4e346a3(#41)control.wallet.operatorAddress941eb4d(#42)funding_walleton the bond answerAll three are additive minors, and dig-node adopts 0.30.0 in one dep bump, so the consumer sees one cascade regardless.
The other two requirements, already satisfied by 0.29.0
Known { address, puzzle_hash }. The address is thexch1…string; the puzzle hash rides beside it because a client matching this wallet against a coin record compares puzzle hashes, and re-deriving one from an address is a bech32m decode no consumer should reimplement.Unavailable { reason: not_initialized | unreadable }, a tagged arm rather than a nullable field, on exactly the reasoning that madeUnfundedvsUnadvertisedworth splitting.The spend-source requirement — measured, and pinned in dig-node
The concern was that
server.rs:2713-2721prefers the signer'sowner_puzzle_hashand only falls back tooperator_puzzle_hash, so a method built on the fallback could show a funded address while collateral failed from an empty one.Measured: they cannot diverge.
open_signer(lifecycle.rs:723) opensOperatorWallet::open, which isopen_operator_phrase(paths)→from_phrase→derive_wallet_keys(phrase).owner_puzzle_hash.operator_puzzle_hashisopen_operator_phrase(paths)→derive_wallet_keys(phrase).owner_puzzle_hash. Same phrase source, same derivation, andagg_sig_datadoes not enter the puzzle hash.Pinned rather than asserted in prose, as required:
the_published_address_derives_from_the_hash_the_mirror_lifecycle_spends_from(dig-node,operator_wallet.rs) bootstraps one real sealed wallet on disk and takes both routes over it, so a future change giving either route its own derivation, phrase source or key slot fails there. It also opens the signer twice under differentagg_sig_data, so one wallet cannot acquire two addresses because two call sites signed for two networks.Verification
cargo test --lib— 191 passed, 0 failed, 0 filtered out. Count checked, not exit status.Four existing guards fired on the addition, which is how a required wire field should behave and is the evidence they are load-bearing:
The new KAT,
a_page_of_bond_amounts_names_the_wallet_those_amounts_are_about, is built against its two nearest wrong implementations rather than against the happy path:funding_walletagainstwallet_operator_address()through the handler, so both come by the route a client uses. A second hardcoded expectation would have agreed with itself while the two surfaces disagreed.knowntag and a non-empty address, because a blank destination on a funding screen is the money statement this family exists to prevent.The fixture deliberately contains an
unfundedrow, so the assertion is made on an answer that is actually demanding money — the exact case where naming the wrong wallet costs somebody a transfer.cargo clippy --all-targets -- -D warningsclean;cargo fmt --checkclean.A drift this PR also fixes
The
README.mdbond row still said "Seven states, six of which mean no coin yet" after 0.28.0 addedunadvertised— drift introduced by that PR and missed by its own catalogue KAT, which checks that every method is NAMED, not that its prose is current. Corrected here, withunadvertiseddescribed and thedisabled/unadvertisedfault distinction stated.