Payments rearchitecture: RFQ hardening + multi-asset bill weighting + facet split for EIP-170#133
Merged
Merged
Conversation
Three security findings on the RFQ quote paths, all closed: - Job RFQ `requester` is signed but was not verified. Thread `msg.sender` through `verifyAndMarkJobQuoteUsed`; reject when the verified address does not match the quote's bound requester. Wildcard requesters (`address(0)`) are also rejected. - Service-creation quotes lacked the freshness check the per-job path enforces. Add the same `block.timestamp <= details.timestamp + maxQuoteAge` gate to `verifyQuoteBatch` so stale quotes cannot be redeemed long after the protocol's max-quote-age window. - `extendServiceFromQuotes` allowed unbounded cumulative TTL. Cap the cumulative service TTL at `MAX_SERVICE_TTL` so repeated extensions cannot push a single service past the protocol's lifetime ceiling. New errors: `JobQuoteRequesterMismatch`, `QuoteTimestampStale`, `CumulativeTtlExceeded`. Tests added in `QuoteVerification.t.sol`, `QuoteExtension.t.sol`, `RFQPaymentDistribution.t.sol` covering each guard. Full RFQ suite 57/57 passes.
Restore the per-asset commitment system to subscription bills. Previously the bill weighted by a single bond-asset cum-stake-seconds × the operator's overall `ServiceOperator.exposureBps`, collapsing the protocol's multi-asset security model. Now the bill iterates the operator's `AssetSecurityCommitment[]` for the service and sums `cumDelta_op_asset × commitmentBps_asset` (USD-normalized via oracle when configured) across every (op, asset) pair the service requires. - TWAP cursors are now keyed `(serviceId, operator, assetHash)` so per-asset joins, leaves, and rejoins compute correct deltas. - Activation pins a multi-asset baseline of `Σ_op Σ_asset (delegation × commitmentBps × price)`. - Bill amount + operator payout weight share the same per-asset stake-time × exposure aggregate, so customer-fairness and operator-fairness move together across assets. - Fallback: when an operator has no per-asset commitments stored (services that don't opt into the multi-asset commitment system), bill against the bond asset at `ServiceOperator.exposureBps`. Mirrors the legacy single-asset semantics so existing tests / services keep working. Storage: replaces `_twapCursorByOp` (2-level) with `_twapCursorByOpAsset` (3-level). Greenfield rename — no migration. Full regression: 1450 / 1450 ✅.
Three facets exceeded the 24,576-byte runtime ceiling (TanglePaymentsFacet 31,846, TangleServicesFacet 26,805, TanglePaymentsDistributionFacet 28,971), blocking deployment on any chain that enforces EIP-170. Split the abstract `Payments` contract into focused mixins so each facet inherits only its slice, and pull validation pure-compute out of `ServicesApprovals` into an external library. Payments split: - `PaymentsCore` — shared events, `_activeServiceOperators`, `_depositToEscrow` - `PaymentsEscrow` — `fundService`, `withdrawRemainingEscrow` - `PaymentsBilling` — subscription bill flow, TWAP weighting, `_accrueOperatorWeights` - `PaymentsDistribution` — distribution core, exposure fallback, baseline init - `PaymentsRewards` — already-split rewards/admin (+ `getBillableServices`) Subscription billing's distribution path is reached via a diamond self-call (`ITanglePaymentsInternal.distributeBillWithKeeper`) so the distribution machinery only contributes bytecode to the facet that hosts it. Same for `initSubscriptionBaseline` (now on `TanglePaymentsDistributionFacet`). `hasSecurityCommitments` is now computed inline in `_accrueOperatorWeights` instead of via a second `_calculateEffectiveExposures` pass, which lets `PaymentsBilling` drop its `PaymentsEffectiveExposure` inheritance entirely. Services split: - `AttestationLib` — shared `teeNonce` + `blsPopMessage` pure compute - `ServiceValidationLib` — external library hosting `_validateTeeCommitments`, `_validateSecurityCommitments`, `_requireBlsProofOfPossession` - `ServicesApprovalsViews` — views facet, no longer inherited by `ServicesApprovals` Final runtime sizes (default profile, optimizer on, runs=1): - TanglePaymentsFacet 24,160 (-416 under EIP-170) - TanglePaymentsDistributionFacet 18,144 - TanglePaymentsRewardsFacet 15,266 - TangleServicesFacet 21,741 - ServiceValidationLib 3,802 (deployed separately, auto-linked) No storage layout changes. No diamond-side ABI changes — same selectors, just routed to different physical facet contracts. Full regression: 1,450/1,450 tests pass.
This was referenced May 15, 2026
drewstone
added a commit
that referenced
this pull request
May 16, 2026
Regenerates tnt-core-bindings + fixtures against current main (f64a4e4) and bumps to v0.17.0. Covers PRs #133, #134, #136 since the v0.16.0 release pin. Binding-surface delta is small: ITanglePaymentsInternal gains distributeBillWithKeeper(BillDistribution) selector 0x68cdf660 — the diamond self-call used by billSubscription to hand per-operator weights computed during accrual directly to the distribution facet. Every other facet selector still routes to the Tangle diamond unchanged. Most of the behavioral changes (RFQ requester binding + freshness + cumulative-TTL caps, multi-asset bill weighting, EIP-170 facet split, O(1) operator stake aggregate, VPM share-pool slashing, claimRewardsAll griefing isolation) are observable to indexers via events emitted from the facet contracts. Those event ABIs (OperatorPoolSlashed, RewardsClaimSkipped, KeeperRebateAccrued, TntPaymentDiscountApplied, StakerShareRefundedToEscrow, SubscriptionBaseline*, SubscriptionBill*) are not surfaced on the ITangle / ITangleFull interfaces and therefore are not present in the generated bindings; the changelog enumerates them so indexer consumers can decode by topic hash against the facet source ABIs. Cargo.lock is updated to 0.17.0 — the lockfile on main was still pinning 0.15.0 (the v0.16.0 release commit did not refresh it). DO NOT publish to crates.io until: dApp sync verified green, indexer schema PR merged, at least one downstream blueprint compiles green. Co-authored-by: Drew Stone <drewstone329@gmail.com>
5 tasks
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.
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
2 participants
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.
Summary
Three independent improvements bundled for testnet/mainnet launch readiness.
1. RFQ quote security hardening (
a223d83)verifyAndMarkJobQuoteUsedrejects quotes whoserequesterfield does not matchmsg.sender, blocking front-run / replay across callers.verifyQuoteBatchenforcesblock.timestamp <= details.timestamp + maxQuoteAgeso stale operator quotes cannot be drained after price changes.QuotesExtendclamps repeated extensions atMAX_SERVICE_TTL, preventing a service from accumulating unbounded lifetime via successive extension quotes.2. Multi-asset subscription bill weighting (
5d4e382)Restored the stake-and-exposure-weighted bill calculation per the protocol design: customers select which assets secure a service, operators commit
exposureBpsper asset, and bills are weighted bycum-stake-seconds × commitmentBpssummed across the operator'sAssetSecurityCommitment[]. USD-normalized through the price oracle when configured so heterogeneous assets aggregate by value._twapCursorByOpAssetis now a 3-level mapping(serviceId → operator → assetHash → cursor)so per-asset stake ramps are tracked independently._serviceSecurityCommitmentsare treated as a single implicit commitment to the bond asset at the operator'sServiceOperator.exposureBps. Preserves legacy single-asset semantics for services that don't opt into the multi-asset commitment system._finalizeJoinseeds per-asset cursors at operator join time._initSubscriptionBaselinepins the multi-asset baseline at activation so post-activation stake ramps cannot inflate the customer's bill.3. Facet split for EIP-170 (
06468b0)Three facets exceeded the 24,576-byte runtime ceiling before this PR, blocking deployment on any chain that enforces EIP-170.
Payments split:
Payments.soldecomposed into focused mixins so each facet inherits only the slice it serves.PaymentsCore— shared events,_activeServiceOperators,_depositToEscrowPaymentsEscrow—fundService,withdrawRemainingEscrowPaymentsBilling— subscription bill flow, TWAP weighting,_accrueOperatorWeightsPaymentsDistribution— distribution core, exposure fallback, baseline initPaymentsRewards— rewards/admin (+ movedgetBillableServices)Subscription billing's distribution path is reached via a diamond self-call (
ITanglePaymentsInternal.distributeBillWithKeeper) so the distribution machinery only contributes bytecode to the facet that exposes it.initSubscriptionBaselinefollows the same pattern.hasSecurityCommitmentsis now computed inline in_accrueOperatorWeightsinstead of via a duplicate_calculateEffectiveExposurespass, lettingPaymentsBillingdrop itsPaymentsEffectiveExposureinheritance entirely.Services split:
AttestationLib— sharedteeNonce+blsPopMessagepure computeServiceValidationLib— external library hosting_validateTeeCommitments/_validateSecurityCommitments/_requireBlsProofOfPossessionServicesApprovalsViews— views facet, no longer inherited byServicesApprovalsFinal runtime sizes
Behavioral preservation
Errors.*names.Test plan
forge test --no-match-path "test/Integration.t.sol"→ 1450/1450 passforge build --sizes)Follow-ups (not in this PR)
_operatorStakeForAssetaggregate (getDelegationForAssetcurrently walks delegator lists)claimRewardsAllper-token try/catch (one griefing ERC20 can brick the path)tnt-core-bindingsfrom new main, fixTNT_CORE_VERSIONpin (currently stale at241b09d)blueprint/,ai-trading-blueprints,ai-agent-sandbox-blueprint,voice-inference-blueprint,llm-inference-blueprint, dApp (cloud/app)