Skip to content

feat(payments): subscription billing rearchitecture + bindings v0.16.0#132

Merged
drewstone merged 1 commit into
mainfrom
fix/zero-operator-bill-livelock
May 13, 2026
Merged

feat(payments): subscription billing rearchitecture + bindings v0.16.0#132
drewstone merged 1 commit into
mainfrom
fix/zero-operator-bill-livelock

Conversation

@drewstone

Copy link
Copy Markdown
Contributor

Closes ten prior audit findings on the subscription billing path with one cohesive redesign. Substantive contract-behavior changes — Rust bindings, indexer, and dapp must update.

Bill amount

  • TWAP-fair, capped at nominal. Bill = nominalRate × cumDeltaPeriod / (baseline × interval), clamped to nominalRate. Operators ramping stake mid-period cannot inflate a customer's bill; ramp-down still reduces it. Kills both customer-overpayment surprise AND the rate-vs-amount termination livelock window.
  • Baseline pinned at activation (not first bill). Closes the stake-ramp attack window the previous lazy-init path opened.
  • Manager QoS hook computeBillAdjustmentBps(serviceId, periodStart, periodEnd) → uint16 can discount the bill (clamped `[0, 10_000]`), called via gas-capped staticcall. queryDeveloperPaymentAddress is now gas-capped too. Malicious managers cannot grief keepers.
  • Cursor commit deferred until after the escrow-balance check passes — a failed try-bill no longer leaves cursors advanced. Closes the free-period exploit on the batch path that the final security audit caught.
  • Dust bills skip cleanly via minBillAmount rather than reverting deep in _distributeBill. A manager hook returning a tiny qosBps can't brick a service.
  • Overflow reverts with BillingArithmeticOverflow instead of silently returning nominal.

Operator payout

  • Same weights drive bill amount AND payout share. weights[i] = cumDelta[i] × exposureBps[i]. Customer-fairness and operator-fairness coupled — an operator who ramps stake earns a larger slice of the SAME (capped) pool.
  • Zero-operator skip. Empty active set advances the cursor without billing, emits SubscriptionBillSkippedNoOperators, escrow untouched. No livelock when a Dynamic service loses all operators.
  • Staker pool refund-to-escrow when the fee distributor is unset or reverts (was: silent capture by treasury). ERC20 already-transferred failures surfaced via StakerShareRefundedToEscrow with reason.

Payment split

  • PaymentSplit gained a fifth field keeperBps. setPaymentSplit requires the 5-field sum equal 10_000. paymentSplit() returns a 5-tuple.
  • Default unchanged at 20/20/40/20/0 — admins opt in to a non-zero keeper rebate via setPaymentSplit per network policy.
  • Non-subscription distributions (PayOnce, RFQ, per-job) fold keeperBps into the operator pool so totals still sum to 10_000.

EventDriven funding

  • Upfront paymentAmount > 0 at request rejected with UpfrontPaymentNotAllowedForEventDriven. Customer funds never collected on a path that can't pay them out properly. Quote-create flow has the matching guard.

Activation paths

  • _handleInitialPayments (request flow) and _activateQuoteService (quote flow) both seed per-op TWAP cursors and pin subscriptionBaselineStake at activation. A bill against an unbaselined service reverts loudly with SubscriptionBaselineNotInitialized instead of silently re-seeding.

Per-service operator ceiling

  • ProtocolConfig.MAX_OPERATORS_PER_SERVICE = 64. maxOperators = 0 on a blueprint config is interpreted as "use protocol ceiling" — never "unlimited". Bounds every per-operator loop in the bill / distribute / terminate paths. Closes the storage-audit Critical that let a blueprint creator unbound the bill loop.

Events / errors / dead code

  • New events: SubscriptionBillSkippedNoOperators, SubscriptionBillAdjustedByManager, KeeperRebateAccrued, StakerShareRefundedToEscrow, SubscriptionBaselineInitialized. PaymentSplitUpdated extended with keeperBps.
  • New errors: BillingArithmeticOverflow, SubscriptionBaselineNotInitialized, UpfrontPaymentNotAllowedForEventDriven.
  • Removed: PaymentLib.calculateOperatorPayments, validatePaymentAmount, bpsShareRoundUp, divUp, _seedTwapCursorForOp, _aggregateOperatorStakeAtNow. Orphaned test/payments/EffectiveExposurePayments.t.sol deleted.

Audits

Four independent reviewers ran against this branch (security, architectural correctness, hardening/gas/code-quality, timing+storage). Their actionable findings:

# Severity Status
Security HIGH: cursor-mutation persistence across failed try-bill High Fixed (commit-deferred cursors)
Security MEDIUM: quote-path EventDriven bypass Medium Fixed (guard added in QuotesCreate)
Storage CRITICAL C-1: maxOperators = 0 unbounded Critical Fixed (MAX_OPERATORS_PER_SERVICE = 64)
10 architectural-correctness items All Fixed
Hardening: dead helpers / gas caps / naming Mostly Minor Fixed
Hardening: 1 Blocking (return-arity) Blocking Fixed
Timing H-1 (job RFQ requester binding) High Captured below
Timing H-2 (service-quote freshness gap) High Captured below
Timing H-3 (extendServiceFromQuotes cumulative TTL) High Captured below
Storage C-2 (O(N) stake reads per bill) Critical (perf) Captured below
Storage H-1 (ValidatorPodManager unbounded delegator slash) High Captured below
Storage H-3 (claimRewardsAll bricked by single bad token) High Captured below

Tests

  • test/security/SubscriptionBilling.t.sol (renamed from F5TWAPBilling.t.sol): 15 tests — 7 unit + 6 fuzz × 1000 runs. Covers bill cap, zero-op skip, keeper rebate, QoS waiver, EventDriven rejection, try-bill cursor invariant, overflow revert.
  • All PaymentSplit literals + destructures migrated across test/ and script/ for the 5-tuple.
  • Fixed two pre-existing test failures that reproduce on main, both rooted in the same via-IR optimizer quirk where the optimizer caches block.timestamp across vm.warp calls: test_GovernorSelfUpgrade / test_TokenUpgradeViaGovernance in Governance.t.sol, and testFuzz_DisputeWindow_Enforcement in SlashingFuzz.t.sol (1000 runs).
  • Full regression: 1,445 / 1,445 ✅ across 94 suites.

Bindings + docs

  • bindings v0.16.0 regenerated; CHANGELOG written. Will publish to crates.io after merge.
  • docs/PRICING.md rewritten end-to-end against the v0.16.0 source.
  • README.md updated for the 5-tuple split + payment-model summary.
  • docs/DEPLOYMENT_RUNBOOK.md + docs/full-deploy.md path corrections.
  • Stale AUDIT-*.md, pursue-launch-readiness.md, COMMIT_MSG.md removed.

Local testnet smoke

Deployed end-to-end against a fresh anvil and exercised: PaymentSplit 5-tuple return, Subscription escrow drain, operator pending rewards, EventDriven UpfrontPaymentNotAllowedForEventDriven revert at request. All ✅.

Flagged from smoke: TanglePaymentsFacet + TangleServicesFacet exceed EIP-170 (24,576 bytes) — anvil deploys only with --disable-code-size-limit. Will block mainnet/L2 deployment until the facets are split. Captured below.

Follow-up (separate PRs)

  • Storage C-2: O(1) _operatorStakeForAsset aggregate to remove the per-bill stake-derivation loop.
  • Storage H-1: ValidatorPodManager share-pool delegator slashing.
  • Storage H-3: try/catch on claimRewardsAll per-token transfer.
  • Timing H-1: bind requester on job-RFQ quote verification.
  • Timing H-2: enforce maxQuoteAge on service-creation quotes.
  • Timing H-3: cap cumulative TTL on extendServiceFromQuotes.
  • Contract-size: split TanglePaymentsFacet and TangleServicesFacet to meet EIP-170 before mainnet.

Test plan

  • forge build clean
  • forge test --no-match-path test/Integration.t.sol: 1445/1445 ✅
  • test/security/SubscriptionBilling.t.sol: 15/15 ✅ (incl. 6 × 1000 fuzz runs)
  • Pre-existing governance + dispute-window-fuzz failures fixed
  • Local testnet smoke: deploy + subscription bill + EventDriven reject ✅
  • Mainnet contract-size split (separate PR; captured above)
  • Publish tnt-core-bindings v0.16.0 to crates.io after merge
  • Downstream consumer updates (blueprint, dapp) for the 5-tuple PaymentSplit ABI break — separate PRs in those repos

Closes ten prior audit findings on the subscription billing path with one
cohesive redesign. Substantive contract behavior changes — downstream
consumers (Rust bindings v0.16.0, indexer, dapp) must update.

## What changed

**Bill amount**
- TWAP-fair from activation: `nominalRate × cumDeltaPeriod / (baseline ×
  interval)`, capped at `nominalRate`. Operators ramping stake mid-period
  cannot inflate a customer's bill; ramp-down still reduces it.
- Baseline pinned at activation (not first bill). Removes the stake-ramp
  attack window that the previous lazy-init path opened.
- Manager QoS hook `computeBillAdjustmentBps(serviceId, periodStart,
  periodEnd)` can discount the bill (clamped `[0, 10_000]`); called via
  gas-capped staticcall. `queryDeveloperPaymentAddress` is now gas-capped
  too. Malicious managers cannot grief keepers.
- Per-operator TWAP cursors are mutated only AFTER the escrow-balance check
  passes — a failed try-bill no longer leaves cursors advanced (closes the
  free-period exploit on the batch path).
- Dust bills (rounding to less than 1 wei per recipient) skip cleanly via
  `minBillAmount` rather than reverting deep in `_distributeBill`.
- Overflow in `twapBillAmount` now reverts with `BillingArithmeticOverflow`
  instead of silently returning nominal.

**Operator payout**
- Same `cumDelta × exposureBps` weights drive bill amount AND payout share —
  customer-fairness and operator-fairness coupled. An operator who ramped
  stake earns a larger slice of the SAME (capped) pool.
- Zero active operators: bill is skipped, cursor advances, escrow untouched,
  `SubscriptionBillSkippedNoOperators` emitted. No livelock when a Dynamic
  service loses all its operators.
- Staker pool refund-to-escrow when the fee distributor is unset or reverts
  (was: silent capture by treasury).
- `claimRewardsAll` semantics unchanged for the user; the keeper rebate
  uses the same pull-pattern mapping.

**Payment split**
- `PaymentSplit` gained a fifth field `keeperBps`. `setPaymentSplit` requires
  the five-field sum equal 10_000. `paymentSplit()` returns a 5-tuple.
- Default split unchanged at 20/20/40/20/0 — admins opt in to a non-zero
  keeper rebate via `setPaymentSplit` per network policy.
- Non-subscription distributions (PayOnce, RFQ, per-job) fold `keeperBps`
  into the operator pool so the total still sums to 10_000.

**EventDriven funding**
- Upfront `paymentAmount > 0` at request is rejected with
  `UpfrontPaymentNotAllowedForEventDriven`. Customer funds are never
  collected on a path that cannot pay them out properly. Quote-create flow
  has the matching guard.

**Activation paths**
- `_handleInitialPayments` (request flow) and `_activateQuoteService`
  (quote flow) both seed per-op TWAP cursors and pin `subscriptionBaselineStake`
  at activation. A bill against an unbaselined service reverts loudly with
  `SubscriptionBaselineNotInitialized` instead of silently re-seeding.

**Per-service operator ceiling**
- `ProtocolConfig.MAX_OPERATORS_PER_SERVICE = 64`. `maxOperators = 0` on a
  blueprint config is interpreted as "use the protocol ceiling" — never
  "unlimited". Bounds every per-operator loop in the bill / distribute /
  terminate paths. Hard fix for the storage-audit Critical that let a
  blueprint creator unbound the bill loop.

**Events**
- New: `SubscriptionBillSkippedNoOperators`, `SubscriptionBillAdjustedByManager`,
  `KeeperRebateAccrued`, `StakerShareRefundedToEscrow`,
  `SubscriptionBaselineInitialized`.
- `PaymentSplitUpdated` extended with `keeperBps`.

**Errors**
- New: `BillingArithmeticOverflow`, `SubscriptionBaselineNotInitialized`,
  `UpfrontPaymentNotAllowedForEventDriven`.

**Dead code removed**
- `PaymentLib.calculateOperatorPayments`, `validatePaymentAmount`,
  `bpsShareRoundUp`, `divUp` — superseded by inline per-weight distribution
  and `minBillAmount`. Orphaned test file deleted.
- `_seedTwapCursorForOp`, `_aggregateOperatorStakeAtNow` — unreachable
  helpers stripped.

## Audits

Four independent reviewers ran against this branch:
- Security: 0 Critical / 1 High (cursor-mutation persistence — fixed in this
  commit) / 1 Medium (quote-path EventDriven bypass — fixed) plus Lows
- Architectural correctness: all 10 prior audit findings verified fixed
- Hardening + gas + code quality: dead-code cleanup, gas captures, naming
- Timing intervals + storage / O(1) audits: 3 timing Highs and 2 storage
  Criticals filed; storage C-1 (`maxOperators = 0` unbounded) fixed inline,
  remaining items captured as follow-up issues.

## Tests

- New `test/security/SubscriptionBilling.t.sol` (renamed from F5TWAPBilling):
  15 tests — 7 unit + 6 fuzz (× 1000 runs each), covering bill cap, zero-op
  skip, keeper rebate, QoS waiver, EventDriven rejection, try-bill cursor
  invariant, and overflow revert.
- Updated all PaymentSplit literals + destructures across `test/` and
  `script/` for the 5-tuple.
- Fixed two pre-existing test failures rooted in the same via-IR optimizer
  quirk on `vm.warp` + `block.timestamp`: `test_GovernorSelfUpgrade` /
  `test_TokenUpgradeViaGovernance` (Governance), and
  `testFuzz_DisputeWindow_Enforcement` (SlashingFuzz, 1000 runs).
- Full regression: 1445 tests pass across 94 suites.

## Bindings + docs

- `bindings v0.16.0`: regenerated, CHANGELOG written.
- `docs/PRICING.md` rewritten end-to-end against v0.16.0 source.
- `README.md` updated for the 5-tuple split + new payment-model summary.
- `docs/DEPLOYMENT_RUNBOOK.md` + `docs/full-deploy.md` path corrections.
- Stale AUDIT-*.md / pursue-launch-readiness.md / COMMIT_MSG.md removed.

## Follow-up (NOT in this PR)

Captured for future PRs:
- Storage C-2: O(1) operator-asset stake aggregate (perf, not correctness).
- Storage H-1: ValidatorPodManager unbounded delegator array on slash.
- Storage H-3: `claimRewardsAll` try/catch on per-token transfer.
- Timing H-1: Job RFQ quote `requester` binding not verified.
- Timing H-2: Service-creation quote freshness gap.
- Timing H-3: `extendServiceFromQuotes` cumulative TTL cap.
- Contract-size: TanglePaymentsFacet + TangleServicesFacet exceed EIP-170
  (deploys on anvil require `--disable-code-size-limit`); needs facet split
  before mainnet.
@drewstone drewstone merged commit 8b2777b into main May 13, 2026
1 check failed
drewstone added a commit that referenced this pull request May 17, 2026
…144)

* audit: reentrancy + griefing red-team 2026-05-16

3 medium / 2 low / 1 informational. No critical or high-severity issues.

Medium: `_distributeBill` push to BSM-resolved developer recipient can
be permanently griefed by a malicious manager; `withdrawRemainingEscrow`
has no fallback when the escrow token is broken; `getNonPaymentTerminationPolicy`
is invoked without a gas cap, letting a malicious BSM grief the livelock
escape.

Low: `_settleDisputeBond` strands dispute bonds on executed proposals
when the treasury push fails; `_operatorActiveSlashProposals` cap check
ordering is inverted (cosmetic).

Confirmed clean: every token-moving entry point holds `nonReentrant`;
the OpenZeppelin ReentrancyGuard storage slot is genuinely shared
across facets via ERC-7201; operator and keeper payments are pull via
`_pendingRewards`; self-call gates fire before any state read; CEI
ordering holds on every withdrawal / claim / refund path; the
subscription livelock escape (`terminateServiceForNonPayment`) is
permissionless.

* audit: economic + oracle red-team 2026-05-16

Read-only audit of subscription billing, share-pool inflation defense,
slashing replay, oracle adapters, and TWAP weighting under adversarial
sequences. Customer is protected by cap-at-nominal; per-operator weight
split is the residual attack surface.

Severity counts: 0 CRITICAL, 1 HIGH, 3 MEDIUM, 2 LOW, 2 INFORMATIONAL.

Top findings:
- H-1: oracle-manipulated weight inflation captures the operator pool
  share of a (capped) bill, redistributing rent from honest operators
- M-1: oracle revert during billing bricks subscriptions for the
  configured assets (no fund loss, denial-of-billing)
- M-3: stake-ramp at periodEnd inflates within-period TWAP weight
  beyond the operator's time-averaged backing

* audit: DoS + access control red-team 2026-05-16

Read-only static audit of DoS surfaces and access-control gaps across
src/core/, src/staking/, and src/beacon/.

Two HIGH findings on the permissionless billing path: unbounded
security-commitment arrays brick subscription billing, and
try IBlueprintServiceManager(...) callsites forward 63/64 of remaining
gas instead of capping at MANAGER_HOOK_GAS_LIMIT.

Four MEDIUM and four LOW notes plus 12 clean checks.

* audit: storage + UUPS upgrade red-team 2026-05-16

* feat(local-env): end-to-end stress test harness + runbook

Single-file harness at scripts/local-env/stress-test.sh that boots a fresh
anvil + LocalTestnet deployment and walks 17 ordered economic checks against
the merged-PR surface (#132 subscription billing rearchitecture, #133
multi-asset bill weighting + EIP-170 facet split, #134 O(1) operator stake
aggregate + share-pool slashing, #136 claimRewardsAll griefing isolation,
#138 indexer event handlers).

Highlights:
- Idempotent: clean cleanup of anvil, broadcast artifacts, indexer state.
- Per-step pass/fail with timing + headline metric. Single-line summary.
- Optional --with-indexer / --with-dapp / --with-operator flags for the
  off-chain side processes; none required for the 17-step protocol surface.
- Griefing-token isolation step deploys a RevertingTransferERC20 and seeds
  Tangle's _pendingRewards + _pendingRewardTokens AddressSet via
  anvil_setStorageAt (vm.store from a broadcast script doesn't propagate
  to anvil; the harness drives the seed via curl directly).

Companion docs at scripts/local-env/STRESS-TEST.md cover prereqs, per-step
PR mapping, log locations, debugging recipes, and an extension guide.

A full green run takes ~40-85s on a warm-cache checkout.

* chore(v0.17.1): audit batch — reports + stress harness + remediation

Consolidates the four 2026-05-16 red-team audit reports, the local stress
harness, and remediation fixes for 2 HIGH and 5 MEDIUM findings into a single
batch. Originally PRs #139 #140 #141 #142 #143 — supersedes those.

## Audit reports landed (docs/audits/REDTEAM-*-2026-05-16.md)

- Reentrancy + griefing: 0 H / 3 M / 2 L / 1 Info
- Economic + oracle: 1 H / 3 M / 2 L / 2 Info
- DoS + access control: 2 H / 4 M / 4 L / 3 Info
- Storage + UUPS upgrade: 0 H / 0 M / 1 L / 3 Info

## Stress harness landed

`scripts/local-env/stress-test.sh` drives a 17-step end-to-end flow against a
local anvil node: deploys the protocol, registers operators, opens services,
funds escrow, fires bills, exercises slashing, tests the griefing-token path
via `script/StressGriefingSeed.s.sol` + `anvil_setStorageAt`, and asserts
final state. 17/17 green across five runs. Runbook in `STRESS-TEST.md`.

## Remediations

HIGH — DoS H-1: cap customer-supplied security-requirement arrays at
`MAX_SECURITY_REQUIREMENTS_PER_REQUEST = 16` in `_validateSecurityRequirements`.
An unbounded array let a customer brick their own subscription bills by
forcing the per-bill `O(operators × requirements)` walk past the block gas
limit. 16 is well above any realistic heterogeneous-asset blueprint and
keeps worst-case `64 × 16 = 1024` inner iterations within one block.

HIGH — DoS H-2 (also closes Reentrancy M-3): every bare
`try IBlueprintServiceManager(bp.manager).fn() catch` callsite now routes
through `_tryStaticcallManager(addr, calldata, minReturnLen)` which forwards
exactly `MANAGER_HOOK_GAS_LIMIT` (500_000) gas. Without the cap, Solidity's
`try/catch` forwards 63/64 of remaining gas, letting a malicious BSM drain
the keeper/proposer's budget. Covers `querySlashingOrigin`,
`requiresAggregation` (×2), `getNonPaymentTerminationPolicy`, `canJoin`,
`canLeave`, `forceRemoveAllowsBelowMin`, `getExitConfig`, `getMinOperatorStake`
(×2), `getHeartbeatInterval`, `getHeartbeatThreshold`,
`getRequiredResultCount`, `queryIsPaymentAssetAllowed`,
`getAggregationThreshold`, and converts the mutating `onAggregatedResult`
hook to `_tryCallManager`.

MEDIUM — Reentrancy M-1: developer / TNT-discount / treasury push transfers
in `_distributeBill` wrapped in `PaymentLib.tryTransferPayment`. On failure,
the un-sent amount folds into the operator pool and emits `PushTransferFailed`
with a structured destination tag. A malicious BSM-resolved developer
recipient (or paused/blocklisting token) can no longer brick distribution
for honest operators.

MEDIUM — Economic M-1: `oracle.toUSD` on the billing hot path wrapped in
`_safeToUSD` / `_safeToUSDView` helpers (capped at `ORACLE_QUERY_GAS_LIMIT =
250_000`) with raw-amount fallback + `PriceOracleFallback` event. A stalled
or reverting oracle now degrades to raw token-second weighting instead of
freezing all bills. `_accrueOperatorWeights` drops `view` since the fallback
path emits.

LOW — Storage L-1: `MultiAssetDelegation._authorizeUpgrade` now requires
`UPGRADER_ROLE` (was `ADMIN_ROLE`), restoring the defense-in-depth role
separation the protocol-level contracts already use. Role added to the
initializer.

## Greenfield cleanup

The pre-launch protocol carried audit-round tag comments left over from prior
remediation rounds — `M-8 FIX:`, `H-1 FIX:`, `Round 4 audit S-1:`,
`G-02 follow-up:`, `C-3 (Round 4):`. Stripped across 30 files. Descriptive
content that explains current behavior is retained; historical narrative is
deleted.

VPM also carried `_legacy*` mappings with comments suggesting they were kept
for "storage layout preservation" in a contract that turns out not to be
upgradeable (`ValidatorPodManager` is constructor-deployed `Ownable`, not
`UUPSUpgradeable`). The mappings and misleading comments are deleted
entirely.

Also: the `if (config.currentDeposits >= amountReturned) { … } else { …
clamp to 0 }` patterns in `DepositManager` and `StakingDelegationsFacet` are
collapsed to a single checked subtraction. The clamp described a defensive
case that is structurally unreachable under the current accounting.

## Fuzz coverage

`test/tangle/SubscriptionEscrowInvariant.t.sol` grew two new invariants and
an adversarial actor:

- `invariant_billAmountNeverExceedsNominalRate` — catches a regression of
  the cap-at-nominal clamp under adversarial stake-ramp sequences.
- `invariant_baselinePinnedAtActivation` — catches any post-activation code
  path that re-pins `subscriptionBaselineStake`.
- `stakeRamper` handler actor that `depositAndDelegate`s / schedules
  unstakes against `operator1` during the run.

## What's deliberately deferred to a follow-up

- HIGH Economic H-1 (oracle weight inflation): the proper fix is to snapshot
  per-(op, asset) USD prices at activation and reuse them at every bill,
  matching the baseline pin. The fix is architecturally involved (touches
  `TangleStorage`, `PaymentsBilling._accrueOperatorWeights`,
  `PaymentsDistribution._initSubscriptionBaseline`,
  `ServicesLifecycle._finalizeJoin`) and warrants its own focused PR with
  negative-tested invariants. The customer is safe today (cap-at-nominal
  bounds total damage); the residual risk is distributional between
  operators in the same service.
- MEDIUM Reentrancy M-2 (escrow rescue path): admin-rescue route for stuck
  escrow tokens when the customer's token is centrally paused / blocklists
  the service owner. Moderate scope; better as its own PR.

Supersedes #139 #140 #141 #142 #143.
@drewstone drewstone deleted the fix/zero-operator-bill-livelock branch June 11, 2026 15:11
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