Skip to content

feat(collateral): declare the epoch collateral requirement + safety margin control methods - #33

Merged
MichaelTaylor3d merged 8 commits into
mainfrom
loop/32-collateral-margin
Aug 28, 2026
Merged

feat(collateral): declare the epoch collateral requirement + safety margin control methods#33
MichaelTaylor3d merged 8 commits into
mainfrom
loop/32-collateral-margin

Conversation

@MichaelTaylor3d

@MichaelTaylor3dMichaelTaylor3d commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

DRAFT — DO NOT MERGE. Gate round has not returned.

Closes#32. Child of epic https://github.com/DIG-Network/dig_ecosystem/issues/3173.

What this declares

Three shell-owned, token-gated methods in a new Category::Collateral:

MethodParamsResult
control.collateral.requirementCollateralRequirementResult
control.collateral.margin.get{margin_bp:u64}
control.collateral.margin.set{margin_bp:u64}{margin_bp:u64}

CollateralRequirementResult is a #[serde(tag = "state")] union:

  • knownepoch, protocol_version, required_per_store_dig_base_units, and the census inputs
    behind the figure: stores, owners, multiplier_micros, handicap_dig_base_units.
  • unknown — a reason from not_censused | behind_finality_depth | record_unreadable |
    no_chain_source.

Unknown is a variant, not an optional number. There is no representable value carrying a figure
the node was not given, which is what dig-app SPEC.md §3.7b requires when it forbids any path that
renders an absent requirement as a zero cost. The four reasons name different missing facts with
different remedies and are deliberately not collapsed.

The protocol version that COMPUTED the epoch travels with the figure, and a reader REFUSES a
known requirement that omits it rather than defaulting — the model is versioned and a client that
knows only the number cannot tell a rule change from a disagreement.

The margin is u64 basis points under the key margin_bp, never converted, matching dig-app
SPEC.md §3.7b and the unit dig_mirror_collateral::apply_safety_margin takes. Two constants are
published: DEFAULT_SAFETY_MARGIN_BP = 100 (+1%) and MAX_SAFETY_MARGIN_BP = 10_000 (+100%).

Above the bound the request is REFUSED as -32602, not clamped. The asymmetry with dig-app is
deliberate and stated in SPEC.md §4.2e: dig-app clamps a value already on disk because refusing it
would leave the node on the lower posting, whereas a .set caller is stating an intent right now and
silently applying a different number would leave stored intent and node behaviour disagreeing about
money. The bound exists because .set is a money-path mutation reachable with an ordinary paired
token and the margin arithmetic saturates rather than failing.

The requirement returns the PRE-margin figure and never the margin, so a node cannot present its
own preference as the network's price. The held-store count is not served here — a client
assembling the recommended-$DIG buffer reads it from control.hostedStores.list, rather than this
creating a second source of truth for an input to a money calculation.

dig-mirror-collateral is deliberately NOT a dependency: it sits at the SAME crate level as this
contract and a same-level edge is forbidden (CLAUDE.md Appendix B). The two constants are restated
here with that reason recorded in their doc comments.

Error codes: NONE minted, and why

No new error code. Every failure this method set has is already covered: an out-of-range margin
is a malformed param (-32602 INVALID_PARAMS), a persistence failure is -32032 CONTROL_ERROR, a
build without the feature is -32031 NOT_SUPPORTED, and an unknown requirement is deliberately not
an error at all but a result variant — which is precisely what stops a client rendering it as zero.

How the space was checked anyway, since #27 records that -32044 already collided. This crate
owns -3204x; declared here are -32040..-32044 and -32046..-32048. A grep of every -320xx
literal in dig-node returns -32000..-32017, -32020..-32022, -32030..-32033,
-32040..-32052, -32060, -32099.

The apparent gap at -32045 is NOT free space — it is a stale claim.dig-node/SPEC.md:5877
still states normatively that -32045 is WALLET_RESERVATIONS_UNAVAILABLE, which #28 moved to
-32047. -32049..-32052 are dig-node's chat band (NO_IDENTITY = -32050, undeclared upstream).
Minting -32045 here would have re-created exactly the -32044 collision #27 records. Logged for
the adopter below rather than fixed in this repo.

Blast radius

Purely ADDITIVE — no existing symbol was edited, renamed or deleted, and no field changed
meaning. gitnexus was not run: this is a fresh worktree and an index build is the ~10-minute
operation §2.0 caps, for a change with no edited symbol to analyse. The radius was measured by
grep + direct read instead, which §2.0 sanctions and which is recorded here.

Consumers of dig_node_control_interface across the ecosystem: dig-node-service
(control.rs, control_cli.rs, meta.rs, server.rs, two test files) and dig-wallet.

  • ControlMethod, Category and ControlErrorCode are all #[non_exhaustive], so a downstream
    match already carries a wildcard.
  • The two dispatch matches in the consumer (control.rs:869, control_cli.rs:478) dispatch on
    &str, not the enum, so a new method cannot break them at all.
  • control_contract_conformance.rs:124 filters on category() == Category::Wallet; a new
    Category variant does not enter that filter.
  • The one thing that WILL react, by design:
    the_node_serves_every_control_method_the_contract_publishes compares ControlMethod::ALL
    against CONTROL_METHODS, so on adoption dig-node must either serve the three methods or list
    them in KNOWN_PREEXISTING_DRIFT. That is the intended pressure, not a regression.

Risk: LOW. No HIGH/CRITICAL finding.

SemVer

0.22.0 → 0.23.0, minor. Additive only: three methods, one Category variant, two result types,
two constants, three trait methods. Adding a method is minor; a changed field meaning would be
breaking and none changed. The trait gains required methods, which is a break for an out-of-tree
implementor — the only implementors are dig-node and this crate's own mock, and every prior method
addition in this crate has shipped the same way.

Verification

  • cargo test168 unit + 9 doc-tests green. cargo clippy --all-targets -D warnings clean.

  • SPEC.md §4.2e added with the three catalog rows; README.md rows added. The existing
    the_spec_and_readme_name_every_catalogued_method guard was RED until both were written, and the
    error-code table guard from feat(spends): declare control.spends.list, the sanctioned reader for the spend audit record #31 passes unchanged.

  • Every normative MUST in §4.2e is true of the code in this diff, and the four load-bearing ones
    were each proved by reverting ONLY their subject (committed first, restored after):

    RevertedTest that failed
    .set stops persistingsetting_the_margin_persists_it_and_getting_it_back_agrees
    the ceiling check made unreachablethe_margin_ceiling_is_pinned_from_both_sides
    protocol_version given #[serde(default)]a_known_requirement_must_declare_its_protocol_version
    CollateralRequirement added to is_open_readthe_collateral_methods_are_named_categorised_and_gated

Fixture notes, since narrowness is where false greens are born:

  • The margin round-trip uses 1 bp, not 100. 100 is +1% and survives a basis-points-to-percent
    conversion as the plausible integer 1; 1 bp collapses to 0 under any such conversion, which is
    the silent no-margin the round-up exists to prevent.
  • The bound is pinned from both sides — at-bound accepted, one-over refused — because a bound
    tested only from below is satisfied by an implementation with no bound at all.
  • .set then .get are two separate dispatcher calls. Asserting result == params in one call
    is satisfied by a handler that echoes and stores nothing.
  • The known fixture gives every field a different value, so a transposition of any pair fails
    rather than passing on a shape they share.
  • the_published_margin_bounds_match_the_declared_constants ties the numbers in normative SPEC.md
    prose to the constants, so the two cannot drift.

Dependencies (§2.4b)

This crate declares no dig-* and no chia-* dependencies — only serde, serde_json,
async-trait, semver, futures. Nothing to bring forward; cargo update -p locks 0 changes.

For the adopter, not for this PR

  • dig-node still declares dig-node-control-interface = "0.21" and does not serve
    control.spends.list from 0.22 (dig-node#385). This method set lands in that same adoption.
  • dig-node/SPEC.md:5877 is stale: it still names -32045 WALLET_RESERVATIONS_UNAVAILABLE
    after feat(wallet)!: move the reservation codes off -32044, which dig-node already owns #28 moved it to -32047. A reimplementer building from that sentence mints a colliding
    code. Worth folding into the same adoption.

…ety margin methods
Refs: #32
Co-Authored-By: Claude <noreply@anthropic.com>
MichaelTaylor3dand others added 5 commits August 27, 2026 21:55
…IP, test target red)
Salvaged from a lane the session cap killed mid-implementation, with 462
insertions uncommitted across five files. Pushed so the work is durable.
State, stated honestly so the next lane does not mistake this for finished:
* `cargo check` on the LIB passes.
* `cargo test` does NOT compile: `src/kats.rs` is missing `use crate::params;`
(E0433). The lane was mid-edit on the handler trait when it died.
So this is work-in-progress, not a green branch. The next step is that import
and then whatever the kats need to actually exercise the new methods.
Refs #32
Co-Authored-By: Claude <noreply@anthropic.com>
…ol methods
Three token-gated, shell-owned methods:
control.collateral.requirement -- this epoch's per-store requirement + census inputs
control.collateral.margin.get -- the local safety margin, in basis points
control.collateral.margin.set -- set it, bounded and refused rather than clamped
Unknown is a tagged variant carrying its reason, so no representable state renders an
absent requirement as a zero cost. The protocol version that computed the epoch travels
with the figure. The margin is u64 basis points and is never converted.
Refs: #32
Co-Authored-By: Claude <noreply@anthropic.com>
…ods normatively
SPEC.md gains the three catalog rows and section 4.2e: the margin is never a consensus
input, is basis points and never converted, is refused rather than clamped above the
bound, defaults to 100bp rather than 0 for a config predating the field, and an unknown
requirement is stated as unknown with a named reason rather than a zero.
Co-Authored-By: Claude <noreply@anthropic.com>
…argin bounds
Co-Authored-By: Claude <noreply@anthropic.com>
Minor: three new methods, one new Category variant, two new result types and two new
constants. ControlMethod, Category and ControlErrorCode are all #[non_exhaustive], so a
downstream match already carries a wildcard arm and no existing field changes meaning.
Co-Authored-By: Claude <noreply@anthropic.com>
@MichaelTaylor3d

Copy link
Copy Markdown
ContributorAuthor

IN PROGRESS — not the verdict. Gate on head 5991ff647e034a405be26ecbd67314a04203dec0.

Confirmed so far:

  1. Merge preconditions, by name.check-merge-preconditions.shBLOCKED, exit 1, on draft=truealone. All four required contexts present and SUCCESS: Format / Clippy / Build / Docs, Coverage (>=80% lines), Lint commit messages, Check version increment. unresolvedReviewThreads=0, mergeStateStatus=CLEAN. No absent-context case.

  2. Authorship coherent. All six commits author and commit as Michael Taylor <michael@michaeltaylor.dev>. The WIP commit 6d3ff31 ("test target red") is genuinely superseded by e99a13d, not left standing as the tip of any surface — the honest red note describes an intermediate state that no longer exists at head.

  3. Unknown-as-a-number: achieved by type, not merely intended.CollateralRequirementResult is #[serde(tag = "state", rename_all = "snake_case")] with Known { .. } carrying no #[serde(default)] on any field (src/results.rs). So a known payload missing required_per_store_dig_base_units, epoch, protocol_version, stores, owners, multiplier_micros or handicap_dig_base_units is a deserialization failure, not a zero. There is no representable known value carrying a figure the node was not given.

  4. Refuse-don't-clamp is total on the dispatch path, and pre-persistence.traits.rs dispatch does self.collateral_margin_set(params.validated()?)validated() runs before the handler is entered, so the bound is checked before anything can be persisted. validated() has exactly one return Err(...) and no clamping arm anywhere; grep finds no min(/clamp( on margin_bp in this crate. Nothing here encodes dig-app's clamping behaviour — the rustdoc on validated() explicitly names the asymmetry and why the two situations differ (a stored value already on disk vs. an intent stated now).

Still verifying: the error-code claim about -32045, the restated dig-mirror-collateral constants, the four load-bearing tests (re-executing the is_open_read one), and SPEC.md truth-in-diff.

@MichaelTaylor3dMichaelTaylor3d left a comment

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gate round 1 (head 5991ff6) — one GATING inline finding attached; verdict comment posted separately.

Comment threadsrc/kats.rs Outdated

@MichaelTaylor3dMichaelTaylor3d left a comment

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gate round 1 — second inline finding.

Comment threadsrc/traits.rs Outdated

@MichaelTaylor3dMichaelTaylor3d left a comment

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gate round 1 — third inline finding.

Comment threadsrc/method.rs
@MichaelTaylor3d

Copy link
Copy Markdown
ContributorAuthor

CHANGES-REQUIRED

Head reviewed: 5991ff647e034a405be26ecbd67314a04203dec0 (resolved from the remote, not from the dispatch brief; head did not move during the review).

Three GATING findings, each posted as its own inline thread. None makes the shipped behaviour wrong today — the contract's semantics are right, and most of what the brief asked me to check came back clean. What is wrong is that two of the guards meant to keep it right do not, and one of the three public enums is missing the attribute that makes this a legitimate minor.

Ranked findings

  1. src/kats.rs:4695the_published_margin_bounds_match_the_declared_constants is a false green for its default row.spec.contains(&value.to_string()) asserts a decimal substring occurs anywhere in a 900-line document. Proved by mutation, full suite each run: (a) DEFAULT_SAFETY_MARGIN_BP: 100 -> 10 with SPEC untouched — this test stayed green (only the separate literal pin caught it); (b) SPEC 4.2e rewritten to teach 250 bp as the default with the constant left at 100all 168 tests passed, i.e. SPEC.md was normatively false about a money-path default and nothing went red. The ceiling row happens to work only because "10000" occurs exactly once, which is a property of today's document rather than of the assertion. It catches a missing row, never a wrong-value row.

  2. src/method.rs:66Category::Collateral added in a minor, but Category has no #[non_exhaustive]. Its siblings ControlMethod and ControlErrorCode both carry it and both document the reason in this same repo. 0.23.0 is therefore a compile break for any downstream exhaustive match. Fix it in this window, when the break is already being taken.

  3. src/traits.rs:691 — an inserted arm stole its neighbour's comment, and the comment is false where it now sits.// Re-validated here idempotently; deserialization already enforced the same rule. was written for SpendsList, which genuinely has a hand-written validating Deserialize (src/params.rs:1473). It now reads as describing the three collateral arms, for which deserialization enforces nothingparams.validated()? is the sole enforcement of MAX_SAFETY_MARGIN_BP. In a crate whose job is to be reimplemented from, that sentence invites a reimplementer to omit the only guard on a money-path bound.

What I verified CLEAN

  • Merge preconditions, by name.check-merge-preconditions.sh gives BLOCKED, exit 1, on draft=truealone. All four required contexts present and SUCCESS (Format / Clippy / Build / Docs, Coverage (>=80% lines), Lint commit messages, Check version increment); no absent or action_required context. unresolvedReviewThreads=0 before this review.
  • Unknown is unrepresentable as a number — achieved, not merely intended.#[serde(tag="state")] with Known { .. } carrying no #[serde(default)] on any of its seven fields. A known payload missing any numeric field is a deserialization failure. a_known_requirement_must_declare_its_protocol_version asserts that refusal directly, and every fixture field carries a distinct value so a transposition cannot pass.
  • Refuse-don't-clamp is total, and pre-persistence.validated() runs in the dispatcher before the handler is entered; one return Err, no clamping arm; no min/clamp/max on margin_bp anywhere in the crate. Ceiling pinned from both sides (at-bound accepted, one-over refused as -32602). Nothing here encodes dig-app's clamping — the rustdoc names the asymmetry and correctly explains why a value already on disk and an intent stated now are different situations. dig-app#308 is neither worsened nor blessed.
  • Authz classification, re-executed as asked. Probe: adding CollateralRequirement to is_open_read turns two tests red — the new the_collateral_methods_are_named_categorised_and_gatedand the pre-existing crate-wide the_token_less_surface_is_exactly_the_bootstrap_plus_the_chain_reads (src/method.rs:699). Genuinely defended, from two independent directions.
  • .set persistence is a real round trip. The mock stores into a thread-local and .get reads it back through the dispatcher, so an echoing handler cannot pass. The fixture is 1 bp, which also kills any percent-quantising implementation.
  • The error-code claim — independently confirmed, and the lane is right. This crate declares -32040..-32044 and -32046..-32048; -32045 is a genuine gap in this table. But dig-nodeorigin/mainSPEC.mdstill names -32045 WALLET_RESERVATIONS_UNAVAILABLE, which this crate moved to -32047 in 0.21.0 (feat(wallet)!: move the reservation codes off -32044, which dig-node already owns #28). So -32045 is a stale SPEC claim, not free space, and minting it would have recreated the -32044 collision -32044 collides: WALLET_COINS_RESERVED vs dig-node's shipped WALLET_NODE_SPEND_DISABLED #27 records. Minting no new codes was correct. Coverage is complete: -32602 bad param, -32032 runtime failure, -32031 unsupported build, -32030 no token — and the interesting cases (no_chain_source, behind_finality_depth) are deliberately in-band on the result union rather than errors, which is the right call.
  • Restated constants match upstream.DEFAULT_SAFETY_MARGIN_BP = 100 equals dig_mirror_collateral::SAFETY_MARGIN_BP_DEFAULT (src/constants.rs:251); the test's 1 bp is SAFETY_MARGIN_BP_TIGHT (:248); multiplier_micros millionths matches MULT_SCALE = 1_000_000 (:69); CENSUS_FINALITY_DEPTH_BLOCKS exists (:218). apply_safety_margin(required, margin_bp) does take basis points, does saturate, and does round up (+ scale - 1), so all three rationale claims resting on it are true. MAX_SAFETY_MARGIN_BP has no upstream counterpart and does not claim one; it is a contract-local policy bound, correctly documented as such. The layering call is right (same level, Appendix B forbids the edge) and the lane's own point stands: nothing can detect future drift, because the rule forbids the dependency that would guard it. Finding 1 matters more in that light, since the SPEC guard is the only drift detector this crate can have.
  • Authorship and history. All six commits author and commit as Michael Taylor <michael@michaeltaylor.dev>. The salvage pushes are coherent; the WIP commit 6d3ff31 ("test target red") is genuinely superseded by e99a13d, and its honest red note describes a state that no longer exists at head.
  • Additive-only otherwise. No field meaning changed, no method renamed, no wire shape altered. ControlMethod and ControlErrorCode are #[non_exhaustive], so the enum additions there are genuinely additive. (Category is finding 2.)
  • SPEC 4.2e is true of the code in this same diff on every claim I could check — the four reason tokens, -32602, owned routing, token-gating, "MUST be rejected rather than defaulted", and the control.hostedStores.list cross-reference, which exists at src/method.rs:242.

The two external findings — accurately stated, with one correction

Both confirmed against origin/main, and neither is this PR's to fix:

  • -32045 in dig-node's SPEC is normatively false. Confirmed — but the line is SPEC.md:6011 on origin/main, not 5877. The lane's line number is stale relative to the current tree; the substance is exactly right. Belongs in the dig-node#385 adoption.
  • dig-node's chat band -32050..-32052 is undeclared upstream. Confirmed: crates/dig-node-core/src/chat.rs:374,378,380 define them, and crates/dig-node-core/src/lib.rs:4881 (the brief said 4882) admits it in its own doc comment. Same private-minting class the -3204x ownership rule exists to prevent.

Re-gate scope

Findings 1 and 3 are test/comment-only; finding 2 is a one-line attribute plus its doc. None touches the wire shape, the auth tier or the refusal logic, all of which I have cleared. So the re-gate should be one correctness pass over the fix diff, not a fresh full round.

Threads: 3 open, all mine, all genuine blockers. No GHAS or third-party threads exist on this PR. The PR is correctly still a draft.

Probes ran in an isolated worktree cut from the PR head under C:/tmp/worktrees/dnci-33-gate, restored to clean after each mutation. No shared checkout was modified.

MichaelTaylor3dand others added 2 commits August 27, 2026 22:41
…d mark Category non_exhaustive
Three gate findings on PR #33:
- kats: the_published_margin_bounds_match_the_declared_constants matched the
constant's DIGITS anywhere in SPEC.md, so it caught a missing row and never a
wrong one. It now reads the figure out of the `NAME` (N bp sentence and
compares it as a number, checking every occurrence.
- method: Category gains a variant in a minor release, so it takes
#[non_exhaustive] like its ControlMethod and ControlErrorCode siblings.
- traits: the collateral arms were inserted under SpendsList's comment, which
said deserialization already validated. It does not for
CollateralMarginSetParams; the arms now say validated() is the sole
enforcement of MAX_SAFETY_MARGIN_BP.
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
@MichaelTaylor3d
MichaelTaylor3d marked this pull request as ready for review August 28, 2026 05:50
@MichaelTaylor3d
MichaelTaylor3d merged commit b6ec659 into mainAug 28, 2026
8 checks passed
@MichaelTaylor3d
MichaelTaylor3d deleted the loop/32-collateral-margin branch August 28, 2026 05:51
Sign up for freeto 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.

Serve the current epoch collateral requirement, so a client can show what a margin costs

1 participant

@MichaelTaylor3d