refactor(payment)!: PaymentRails set + PaymentRouter (replace PaymentMode)

[drewstone]

Greenfield cleanup — the best-in-class, extensible payment primitive (no prod consumers yet, so no backward-compat shims).

Why

PaymentMode was an enum cross-product — None / Shielded / Direct / Both. "Both" is a special case; a third rail would force More / AllThree… variants. That doesn't scale.

What

• PaymentRails — a composable set of bool flags (shielded, direct). Rails compose freely; enabling several lets one endpoint accept any of them. Adding a future rail = one more flag + a PaymentProvider impl, never a new enum cross-product.
• PaymentRouter (replaces CompositeProvider) — the single universal provider. Holds enabled rails as Option<Provider>, dispatches each request to the matching one by proof type, and rejects a proof for a disabled rail up front (no chain call). create_provider(rails, ..) → Noop for the empty set, else a PaymentRouter.
• Config: BillingConfig.payment_mode → payment_rails (default shielded). TOML: payment_rails = { shielded = true, direct = true }.

Tests

Rails serde/compose, both-rails build, router-rejects-disabled-rail. Suite green: 16 lib + 49 integration.

Consumed by llm-inference-blueprint#feat/payment-rails.

[tangletools]

✅ Auto-approved PR — 4476f077

Blanket team auto-approval is enabled for this reviewer service.
The full PR reviewer audit still runs separately and will publish findings if it detects issues.

_{tangletools · auto-approval · reason: blanket_auto_approve · 2026-06-11T20:59:14Z}

[tangletools]

✅ No Blockers — 4476f077

Readiness 47/100 · Confidence 90/100 · 17 findings (5 medium, 12 low)

	deepseek	glm	aggregate
Readiness	57	47	47
Confidence	90	90	90
Correctness	57	47	47
Security	57	47	47
Testing	57	47	47
Architecture	57	47	47

Full multi-shot audit completed 6/6 planned shots over 6 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 6/6 planned shots over 6 changed files. Global verifier still owns final merge decision.

🟠 MEDIUM Breaking config format change: old payment_mode enum values will silently deserialize to default — src/config.rs

An existing TOML/JSON config containing payment_mode = "shielded" or payment_mode = "both" will no longer parse. The field was renamed from payment_mode to payment_rails and its type changed from a #[serde(rename_all = "snake_case")] enum to a struct with bool fields. Because #[serde(default)] is used on the new payment_rails field, an old config that only has payment_mode will silently get PaymentRails::default() (shielded-only). For operators who had payment_mode = "none" or payment_mode = "direct", this silently changes billing behavior — a "none" operator now requires shielded payment by default. Mitigation: add #[serde(alias = "payment_mode")] on the field and/or add a deny-unknown-fields guard + migration guide in the PR description. If this is a pre-1.0 cra

🟠 MEDIUM PaymentRouter::build parses operator_key redundantly — src/payment.rs

Lines 460-465: PaymentRouter::build parses tangle.operator_key into a PrivateKeySigner just to extract the address, but every sub-provider (ShieldedProvider::new, DirectProvider::new) independently parses the same key for the same purpose. This is triple-parsing a private key material string. If any provider changes its derivation, the router's stored operator_address silently diverges. Consider deriving the address once at the factory level (create_provider) and passing Address down, or having each provider expose its address from a single canonical derivation. Minor perf concern (secp256k1 parsing 3x) but primarily a correctness/DRY issue — any

🟠 MEDIUM PaymentRouter::build() allows empty rails (pub API sharp edge) — src/payment.rs

Line 459: PaymentRouter::build() is a pub fn that accepts any PaymentRails without validating that at least one rail is enabled. Calling build(PaymentRails::NONE, ...) produces a router where both shielded and direct are None, operator_address() returns a valid address, but every authorize()/settle() call fails with 'not enabled' errors. The create_provider factory (line 547) guards this with rails.is_empty(), but build() does not. Fix: add the same guard at the start of build(), returning `anyhow::bail!("no payment

🟠 MEDIUM UsedTxStore::persist silently drops write errors and has no crash-recovery atomicity guarantee — src/payment.rs

Lines 355-368: persist uses write-to-tmp + rename which is correct for atomicity on POSIX, but: (1) std::fs::write(&tmp, &data).is_ok() silently discards the write error — if the disk is full, the operator gets no error surface and the tx is already committed in-memory. On restart, the tx is gone from the persisted set but was already served, allowing replay. (2) The .tmp file path is deterministic per-store — concurrent processes (unlikely in single-operator but possible if supervisor restarts fast) could race on the tmp file. Consider: (a) bubbling the write error up from commit (or at minimum logging at error level on write failure rather than silentl

🟠 MEDIUM Lost serde round-trip coverage for payment rails — tests/integration_test.rs

The old payment_mode_serde_all_variants test (lines 897-910 in base) verified full serialization→deserialization round-trip for every variant. The replacement tests (payment_rails_default_is_shielded_only at L892, payment_rails_compose_freely at L900) only test serde_json::from_str (deserialization). If a #[serde] attribute or field name change breaks serialization, no test catches it. Add serde_json::to_string(&PaymentRails::SHIELDED) and round-trip assertions for at least SHIELDED, DIRECT, BOTH, and NONE.

🟡 LOW Example creates duplicate BillingClient (pre-existing architectural debt) — examples/minimal_operator.rs

Line 153 creates billing = BillingClient::new(...) and passes it via .billing(Arc::new(billing)). Since .payment_provider(...) is not called, AppStateBuilder::build() (server.rs:368-387) falls back to create_provider(SHIELDED, ...) which constructs a ShieldedProvider that internally calls BillingClient::new(...) again, yielding two separate BillingClient instances. Not a bug — the example doesn't exercise billing — but anyone copying this pattern gets duplicated provider state. Consider either calling .payment_provider(...) explicitly or using AppState::from_config(...) ([line 136](https://github.com/tangle-network/tangle-inference-c

🟡 LOW Hardcoded Anvil test private key in example — examples/minimal_operator.rs

Line 121: operator_key: "ac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80" — the well-known Anvil/Hardhat account #0 key. Pre-existing, not introduced by this PR. Low severity since this is an example that 'will fail at startup' (line 14), but worth a comment or env-var substitution to prevent copy-paste into production.

🟡 LOW Missing payment_rails default assertion in billing config test — src/config.rs

test_billing_config_defaults (line 269) deserializes BillingConfig but never asserts that payment_rails defaults to SHIELDED. The only assertions are billing_required, claim_max_retries, clock_skew_tolerance_secs, and max_gas_price_gwei. Since this file's primary change is PaymentMode→PaymentRails with a Default impl, the serde default path should be explicitly verified to prevent regression. Add: assert_eq!(cfg.payment_rails, PaymentRails::SHIELDED);

🟡 LOW PaymentRails silently accepts unknown fields (no deny_unknown_fields) — src/config.rs

PaymentRails (line 86) derives Deserialize without #[serde(deny_unknown_fields)]. A config typo like {"shielded": true, "direc": true} silently deserializes as SHIELDED (direct=false), leaving the operator believing direct payments are enabled when they are not. This is a configuration footgun for security-sensitive payment settings. Add #[serde(deny_unknown_fields)] on the struct to reject typos at config load time.

🟡 LOW test_billing_config_defaults doesn't assert payment_rails default — src/config.rs

The unit test at line 269 constructs a BillingConfig from JSON with only max_spend_per_request and min_credit_balance, but never asserts cfg.payment_rails. The payment_rails_default_is_shielded_only test in integration_test.rs covers this indirectly, but the config unit test should also verify the field since it's testing BillingConfig defaults. Minor coverage gap.

🟡 LOW No happy-path routing dispatch test for PaymentRouter with BOTH rails — src/payment.rs

The integration test router_rejects_proof_for_disabled_rail (test line 919) tests only the rejection case: SHIELDED-only router correctly rejects a DirectTransfer proof. But there is no test that PaymentRouter::build(PaymentRails::BOTH, ...).authorize(&SpendAuthProof) dispatches to ShieldedProvider, or that a DirectTransfer proof routes to DirectProvider. The individual providers are tested for their own authorize/settle logic, but the match-arm dispatch in PaymentRouter (lines 479-495) is unverified for the correct path. A swap in the

🟡 LOW PaymentRouter dispatch duplicates match arms in authorize and settle — src/payment.rs

Lines 480-510: authorize and settle both have identical match-on-proof-type → ok_or_else → delegate patterns with duplicated error messages ('shielded rail not enabled' / 'direct rail not enabled'). If a new proof variant is added to PaymentProof, both match arms must be updated in lockstep. Consider a helper method fn rail_for(&self, proof: &PaymentProof) -> anyhow::Result<&dyn PaymentProvider> that centralizes the dispatch, or a macro, to eliminate the duplication. Not a bug today, but a maintenance hazard.

🟡 LOW Builder fallback silently swaps rail config on error — src/server.rs

Lines 370-386: When create_provider fails (e.g., DIRECT rail configured but no payment_token_address), the builder silently falls back to ShieldedProvider instead of propagating the error. This means an operator who explicitly configured payment_rails: { direct: true } without a token address gets shielded-only instead of a clear startup failure. This is pre-existing behavior (not introduced by this PR), but the rail-based config makes misconfiguration more likely since operators may toggle individual flags. Consider propagating the error instead of silently downgrading, or at minimum logging a warning that the fallback was triggered.

🟡 LOW BOTH-rail dispatch not tested — tests/integration_test.rs

create_provider_both_rails (L908) only asserts is_ok() on construction. It never verifies that a router built with PaymentRails::BOTH accepts both a SpendAuth proof and a DirectTransfer proof — meaning a regression where BOTH only dispatches to one rail would not be caught by this test file. Consider adding a router_dispatches_both_rails test.

🟡 LOW Missing serde round-trip and NONE-deserialization coverage — tests/integration_test.rs

The old payment_mode_serde_all_variants test verified serialize→deserialize round-trip for all three variants (none, shielded, direct). The replacement tests (payment_rails_default_is_shielded_only, payment_rails_compose_freely) verify deserialization of {"direct":true} and {"shielded":true,"direct":true} but never call serde_json::to_string and compare. Also no test deserializes {} or {"shielded":false,"direct":false} to verify NONE round-trips correctly. Low severity — the underlying PaymentRails bitfield type likely makes this trivial, but the old test's exhaustive coverage regressed slightly.

🟡 LOW Stale comment references removed type — tests/integration_test.rs

Line 711: // - PaymentMode config creates the right provider — the type was renamed from PaymentMode to PaymentRails. Should read PaymentRails config.

🟡 LOW Stale comment referencing removed PaymentMode — tests/integration_test.rs

Line 711 has a comment '// - PaymentMode config creates the right provider' referencing the now-removed type. Outside this shot's scope but worth noting for housekeeping.

---

_{tangletools · 2026-06-11T21:12:31Z · trace}

[tangletools]

✅ Auto-approved PR — 0c7d0225

Blanket team auto-approval is enabled for this reviewer service.
The full PR reviewer audit still runs separately and will publish findings if it detects issues.

_{tangletools · auto-approval · reason: blanket_auto_approve · 2026-06-11T21:28:38Z}