Skip to content

feat(control): settable mirror advertise-URL override (control.config.setMirrorAdvertiseUrls) - #51

Merged
MichaelTaylor3d merged 2 commits into
mainfrom
feat/mirror-advertise-control-method
Sep 5, 2026
Merged

feat(control): settable mirror advertise-URL override (control.config.setMirrorAdvertiseUrls)#51
MichaelTaylor3d merged 2 commits into
mainfrom
feat/mirror-advertise-control-method

Conversation

@MichaelTaylor3d

@MichaelTaylor3d MichaelTaylor3d commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Status: gates green, ready for review -- DO NOT MERGE OR UNDRAFT

This is the release-first head of a three-repo cascade: dig-node serves this method next (reading
DIG_MIRROR_ADVERTISE_URLS's replacement config path), then dig-app calls it. Leaving this DRAFT
per the assignment -- the orchestrator/review gate merges once satisfied.

Correction (commit 122024a, after the first green pass)

My first pass asserted the result applies LIVE with no restart, reasoned from dig-node's advertise
decision being recomputed every mirror-coin-creation pass. That premise is true and the conclusion
was wrong: the recomputation reads DIG_MIRROR_ADVERTISE_URLS from the OS ENVIRONMENT of the
running process (advertise.rs:160, std::env::var), and no control call can change a running
process's own environment. So under dig-node's CURRENT implementation this method cannot take
effect without a restart, however often the recomputation runs. Caught by orchestrator review
before anything published -- see "Result" below for the fix (a truthful requires_restart field,
not a contract-fixed guess in either direction).

Task

Add a control method so the mirror advertise URL becomes settable over the control surface,
defaulting to dig-node's own derived public address + content-serving port (verbatim user request).

Today it is DIG_MIRROR_ADVERTISE_URLS, an environment variable with no control method and no UI.
dig-node #562 (merged, 0.254.82) made the node derive a default; nothing let a person set
one. ControlMethod is enumerated (only ConfigGet/ConfigSetUpstream existed), so this genuinely
needed a new variant.

Shape (locked by the brief; ConfigSetUpstream was the template)

  • Wire name: control.config.setMirrorAdvertiseUrls.
  • Params: SetMirrorAdvertiseUrlsParams { urls: Option<Vec<String>> }.
    • null/absent -> CLEAR the override, reverting to dig-node#562's derived default.
    • Some(non-empty) -> SET the override.
    • Some(vec![]) (explicit empty) -> REFUSED as -32602 INVALID_PARAMS. Ambiguous between
      "advertise nothing" and "go back to automatic"; guessing wrong either forfeits mirror rewards
      the operator didn't intend to give up, or silently overrides a deliberate opt-out. Enforced by
      validated(), called from the dispatch arm as the SOLE enforcement (plain Deserialize
      derive enforces nothing beyond field types) -- proven through the real dispatcher, not just the
      bare constructor (set_mirror_advertise_urls_refuses_an_explicit_empty_list_through_dispatch).
    • Each URL is checked for well-formedness ONLY (absolute, scheme + host, via the url crate --
      the same crate + version line dig-node's own mirror module parses with). Deliberately NOT
      routability or "this-machine-only": dig-node#562 established that an operator's LAN/private
      address is a legitimate choice risking only their own stake, while a DERIVED one is a broken
      reading of this node's own position. Applying the stricter rule here would silently break every
      LAN-only deployment while looking like a safety improvement. Proven with a positive control
      (set_mirror_advertise_urls_refuses_a_non_absolute_url_through_dispatch asserts BOTH the
      schemeless-host refusal AND that a 192.168.x.x LAN address is accepted).
  • Result: SetMirrorAdvertiseUrlsResult { mirror_advertise: MirrorAdvertiseView, requires_restart: bool }.
    mirror_advertise is the same view ConfigGet embeds. requires_restart was added after an
    orchestrator review caught a wrong assumption in my first pass
    (see "Correction" below) --
    it reports what the ANSWERING NODE actually did, never a value this contract fixes: dig-node's
    real, env-var-backed implementation answers true unconditionally today, but the wire format
    already carries false too (proven by a golden-vector pair at both values), so a future dig-node
    that persists this somewhere its own advertise pass re-reads can go live without a contract change.
    • state: MirrorAdvertiseState restates dig-node#562's own six AdvertiseState labels verbatim
      (advertising_override / advertising_derived / off / no_public_address /
      uncorroborated_address / no_relay), pinned literally in the_mirror_advertise_states_are_dig_node_562s_own_wire_labels.
      This is the field that lets a UI say "no public address discovered yet" instead of an empty box.
  • ConfigResult gains mirror_advertise: Option<MirrorAdvertiseView>, additive. Option (no
    #[serde(default)] needed -- serde treats a missing Option field as None on its own) carries
    backward compat with a pre-field node, the exact pattern WalletBalanceResult::source already
    uses here; proven with a legacy-payload deserialize test
    (a_pre_advertise_field_nodes_config_get_still_parses_with_it_unknown).
  • Tier: ORDINARY, not master. It outlives the token exactly like chiaPeers.add and the
    proposed setUpstream promotion (sec: control.config.setUpstream belongs on the master tier -- its effect outlives the token AND names a principal #40, still open, NOT touched by this PR), but the
    master-tier rule is narrower than "outlives the token" -- it is whether the effect installs a
    principal the node will thereafter believe, obey, or speak to. This method changes only what the
    node broadcasts ABOUT ITSELF; it never dials, trusts, or forwards to the configured value. See the
    doc comment on ControlMethod::requires_master_token and
    set_mirror_advertise_urls_is_ordinary_tier_and_config_category.

Files touched

  • src/method.rs -- variant, wire name, Category::Config, ALL, summary(), tier doc + test.
  • src/params.rs -- SetMirrorAdvertiseUrlsParams, validated(), is_well_formed_advertise_url.
  • src/results.rs -- MirrorAdvertiseState, MirrorAdvertiseView (embedded in ConfigResult,
    additive field), SetMirrorAdvertiseUrlsResult (this method's actual result, added in the
    correction commit); fixed one pre-existing ConfigResult literal in this file's own test module
    (E0063).
  • src/traits.rs -- ControlHandler::config_set_mirror_advertise_urls (returns
    SetMirrorAdvertiseUrlsResult) + dispatch arm.
  • src/kats.rs -- MockNode impl (both directions, honestly answering requires_restart: true),
    golden request/response vectors (incl. a SetMirrorAdvertiseUrlsResult pair at BOTH
    requires_restart values), backward-compat test, wire-label pins, four dispatcher round-trip
    tests (clear, set, empty-list refusal, malformed-vs-LAN-address control), a runtime double-space
    guard on the one \-continued error string (this ecosystem's cargo fmt has mangled that exact
    pattern twice already).
  • SPEC.md -- new normative §4.2h + method-table row + ConfigResult/MirrorAdvertiseView/
    MirrorAdvertiseState field definitions in §4.1.
  • README.md -- method-table row (the_spec_and_readme_name_every_catalogued_method is a REAL
    test that failed here before this fix: 201 passed / 1 failed, exactly the gap it exists to catch).
  • Cargo.toml/Cargo.lock -- url = "2" dependency (matches dig-node's own pin); version
    0.32.1 -> 0.33.0 (minor: additive capability).

Blast radius

Additive-only change to a leaf contract crate (no dig-*/chia-* deps to bump, §2.4b N/A -- checked
Cargo.toml, only serde/serde_json/async-trait/semver/url). One new #[non_exhaustive] enum
variant, one new Option struct field, two new pub types, one new trait method + dispatch arm. No
existing wire shape, error code, tier, or routing changed for any pre-existing method.

No gitnexus index exists for this repo/worktree (checked list_repos -- only unrelated
better_impact-workspace repos are registered on this MCP server instance, none for
dig-node-control-interface or dig_ecosystem). Per §2.0's sanctioned fallback, blast radius was
verified instead by: (1) grep-driven manual sweep of every ControlMethod/ConfigResult construction
and match site across all 5 source files before editing; (2) the Rust compiler's own EXHAUSTIVE match
checking, which is a stronger guarantee than a call-graph heuristic for exactly this class of change
(adding an enum variant/struct field) -- it caught the one construction site I'd missed
(results.rs:2765, a ConfigResult literal in this file's own test module) as a hard compile error;
(3) the full green test suite below, which exercises every new code path through the real dispatcher.
No consumer of this crate exists in this repo (dig-node adopts it next, per the cascade).

Evidence

$ cargo fmt --all -- --check
(clean, exit 0)

$ cargo clippy --all-targets --all-features --locked -- -D warnings
(clean, exit 0, zero warnings)

$ cargo test --workspace --all-targets --all-features --locked
test result: ok. 202 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out

Re-verified identically on the correction commit 122024a (same three commands, same 202/0 --
the correction enriched existing tests' assertions and added fixture cases rather than new test
functions, so the count is unchanged and still real: confirmed by reading actual test names in the
log tail, not a filtered-to-zero count). All runs via output redirection (never a pipe) with the
exit code read from the file, not the tool-call summary -- this ecosystem's own knowledge base
records a piped/chained exit code as a reassuring number that measures the wrong command.

Dep-freshness check (§2.4b): no dig-*/chia-* dependency exists in this crate to bump.

Closes nothing outside this repo -- dig-node and dig-app adopt this method in their own follow-up
PRs, referencing this crate's 0.33.0.

The mirror advertise URL is settable over the control surface, defaulting to
dig-node's own derived public address + content-serving port (dig-node#562),
so an operator no longer needs to know DIG_MIRROR_ADVERTISE_URLS exists or
edit a service's environment by hand. This is the release-first head of a
three-repo cascade -- dig-node serves it next, dig-app calls it after that.

- method.rs: new ControlMethod::ConfigSetMirrorAdvertiseUrls, Category::Config,
  ordinary auth tier (not master -- it installs no principal the node will
  thereafter believe, obey, or speak to, unlike chiaPeers.add/setUpstream's
  proposed promotion in #40).
- params.rs: SetMirrorAdvertiseUrlsParams { urls: Option<Vec<String>> } --
  None/absent clears the override, Some(non-empty) sets it, Some(empty) is
  refused as -32602 INVALID_PARAMS (ambiguous between "advertise nothing" and
  "go back to automatic"). validated() also refuses a non-well-formed URL via
  the `url` crate, checking absoluteness ONLY -- deliberately not routability
  or this-machine-only, which stays dig-node's own asymmetric derived-vs-
  operator rule (an operator's LAN address is a legitimate choice).
- results.rs: MirrorAdvertiseState (dig-node#562's six outcomes, restated so
  client and node cannot drift) + MirrorAdvertiseView (urls, operator_override,
  state), shared between this method's result and ConfigResult's new additive
  `mirror_advertise: Option<...>` field (Option carries backward compat with a
  pre-field node, the WalletBalanceResult::source pattern).
- traits.rs: ControlHandler::config_set_mirror_advertise_urls + dispatch arm,
  validated() as the sole enforcement point.
- kats.rs: MockNode impl, golden request/response vectors, a pre-field
  backward-compat test, the six wire-label pins, and dispatcher round-trip
  tests for clear/set/empty-list-refusal/malformed-vs-LAN-address.
- SPEC.md/README.md: new normative section + method-table rows (README's
  exhaustiveness test caught the missing row as a real failure before this).
- Cargo.toml: url = "2" dependency (matches dig-node's own pin); no dig-*/
  chia-* deps exist in this crate to bump (§2.4b N/A).

Verified: cargo fmt --all -- --check, cargo clippy --all-targets --all-features
--locked -- -D warnings, and cargo test --workspace --all-targets --all-features
--locked all green (202 passed, 0 failed), each checked via output redirection
(never a pipe) with the exit code read from the file.

Purely additive (new enum variant on a #[non_exhaustive] enum, new Option
field, new pub types, new trait method) -- no breaking change.

Co-Authored-By: Claude <noreply@anthropic.com>
@MichaelTaylor3d
MichaelTaylor3d force-pushed the feat/mirror-advertise-control-method branch from fec779a to 85f6b73 Compare September 5, 2026 14:56
… live

My "applied live, no restart" claim rested on dig-node#562's advertise
decision being recomputed every mirror-coin pass -- true, but irrelevant: it
recomputes from DIG_MIRROR_ADVERTISE_URLS, an OS ENVIRONMENT VARIABLE
(advertise.rs:160, std::env::var), and no control call can change the
environment of a process already running. So as dig-node stands today, this
method cannot take effect without a restart, no matter how often the
recomputation runs -- the crate was asserting a fact about a persistence
layer dig-node has not built.

The fix is not to bake in the opposite guess (requires_restart: true
unconditionally forever), because live IS genuinely achievable once dig-node
persists this somewhere its own advertise pass re-reads. Instead the RESULT
reports what actually happened:

- results.rs: SetMirrorAdvertiseUrlsResult { mirror_advertise: MirrorAdvertiseView,
  requires_restart: bool } replaces the bare MirrorAdvertiseView as this
  method's result -- mirroring SetUpstreamResult's restart-hint shape (the
  precedent already in this file), restated rather than reused because the
  TRUTH VALUE differs per method, not just the field name. MirrorAdvertiseView
  itself is unchanged and stays ConfigResult's embedded field.
- params.rs/traits.rs: the control_call binding and the ControlHandler trait
  method now return SetMirrorAdvertiseUrlsResult; the trait doc rewritten to
  require requires_restart be answered truthfully for the ANSWERING node's
  own implementation, never fixed by the contract in either direction, and to
  spell out why answering false while still env-var-backed is a surface
  lying about whether a privileged action took effect.
- kats.rs: MockNode now returns requires_restart: true (honest for an
  env-var-backed node, like every real dig-node today); the three dispatcher
  tests updated to the nested shape and now assert requires_restart; a new
  golden-vector pair round-trips SetMirrorAdvertiseUrlsResult at BOTH
  requires_restart values, proving the wire format can already carry `false`
  even though nothing produces it yet.
- method.rs/SPEC.md/README.md: every "applied LIVE, no restart" claim
  rewritten to describe the corrected, node-truthful contract.

cargo fmt --all -- --check, cargo clippy --all-targets --all-features
--locked -- -D warnings, and cargo test --workspace --all-targets
--all-features --locked all green (202 passed, 0 failed) -- verified via
output redirection, exit code read from the file, not the tool-call summary.

Co-Authored-By: Claude <noreply@anthropic.com>
@MichaelTaylor3d
MichaelTaylor3d marked this pull request as ready for review September 5, 2026 15:23
@MichaelTaylor3d
MichaelTaylor3d merged commit 574afdf into main Sep 5, 2026
9 checks passed
@MichaelTaylor3d
MichaelTaylor3d deleted the feat/mirror-advertise-control-method branch September 5, 2026 15:23
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