chore(audit): multi-subsystem hardening + bindings v0.12.0#124
Merged
Conversation
Eight parallel auditors reviewed services lifecycle, payments/escrow, slashing, staking/MAD/LDV, beacon+bridges, governance/MBSM/blueprints, jobs/RFQ + indexer + bindings, and test-quality + gas. This commit lands the Tier-1 fixes — small, high-confidence, verified before patching — that don't require interface-level redesign or large refactors. Service lifecycle (svc-lc H-1 / H-3 / M-1 / M-2 / M-4) - Register `expireServiceRequest` selector on `TangleServicesFacet`. The aa511c2 release added the function to `ITangleServices` but never registered the selector, so the permissionless cleanup path was unreachable on the proxy. - Active-status gate on every operator-exit entrypoint (`scheduleExit`, `executeExit`, `forceExit`, `leaveService`, `forceRemoveOperator`) so a stale operator can't fire exit paths on a Terminated service. - `approveService` rejects requests past the expiry grace, mirroring what `rejectService` already does. Closes the race where an operator could quietly activate a stale request the requester thought they could clean up. - Reject duplicate operator entries in `requestService*` — without this, `req.operatorCount` exceeds the unique approver count and the request can never reach activation. - `nonReentrant` on `terminateService` and `terminateServiceForNonPayment`. Slashing (slash S-2 / S-3 + zero-evidence) - `nonReentrant` on `proposeSlash` and `disputeSlash`. Only `executeSlash`, `executeSlashBatch`, and `cancelSlash` were guarded. - `proposeSlash` rejects `bytes32(0)` evidence so off-chain monitors keying off non-zero evidence don't see silently-zero entries. - `TIMESTAMP_BUFFER` symmetry on the Disputed branch of `SlashingLib.isExecutable`. The Pending branch carried a 15-second buffer; the Disputed branch did not, so a sequencer with timestamp influence could sandwich the dispute deadline tick. - `ITangleSlashing` event declarations realigned with what the protocol actually emits. `SlashProposed` is 8 fields (was 4), `SlashExecuted` is 4 fields (was 3), and the previously-missing `SlashDisputed`, `SlashCancelled`, `SlashConfigUpdated` events are now declared. Rust consumers wired to the interface could not decode any emitted slash event before this fix. Quote signature binding (pay H-1 / jobs J-1, BREAKING) - `requester` is now part of the `QUOTE_TYPEHASH` and `hashQuote`'s ABI encoding. The field lived on the struct but was excluded from the typed data, so an attacker who observed a signed quote could flip `details.requester` to themselves and the signature still recovered. Off-chain signers MUST update; existing pre-fix signatures are invalid against the new typehash. All in-tree quote test helpers updated. Payments (pay H-2 + M-5) - `_distributePaymentWithEffectiveExposure` reverts when zero operators are active at billing time. The dev/treasury split was paying out while the operator+staker pool (default 60%) remained retained by the contract with no path back. Service owners who lose all operators recover escrow via terminate -> withdrawRemainingEscrow. - `whenNotPaused` on `fundService`, `billSubscription`, and `billSubscriptionBatch`. Reward / refund claim paths remain unguarded so users can always exit. Staking (stk S-7 / S-14) - `OperatorStatusRegistry.registerOperator` resets all per-(serviceId, operator) heartbeat / metrics state on (re-)register. Without this, `isHeartbeatCurrent` could return true before any new heartbeat landed. - `LiquidDelegationVault.requestRedeem` rejects `controller == address(0)` so the redeemer's burned shares aren't permanently locked. Indexer (jobs I-1) - `SlashConfigUpdated` event signature in `indexer/config.yaml` now declares all 6 fields (was 3); the old shape silently dropped every emission. Schema and handler updated to surface `disputeResolutionDeadline`, `disputeBond`, and `maxPendingSlashesPerOperator`. Dead code removed - `src/exposure/` and `test/exposure/` — orphan module reproducing bps math that lives in `src/core/PaymentsEffectiveExposure.sol`. No references from production. Self-contained tests removed with it; `MockPriceOracle` (still used by other suites) moved to `test/mocks/`. - `src/interfaces/IStreamingPaymentAdapter.sol` — the four interfaces in this file aren't implemented or referenced anywhere. Tests + bindings - New `test/security/AuditFixesTest.t.sol` pins five regressions: expireServiceRequest reach, approve-after-grace revert, duplicate- operator rejection, exit-on-terminated revert, zero-evidence slash revert. - `test_DisputeSlash_BondForfeitOnExecute` warp adjusted by the new buffer. - Bindings regenerated via `cargo xtask gen-bindings`. Version bumped to 0.12.0 for the breaking typehash + event-shape changes; CHANGELOG details every fix. - `.evolve/` added to `.gitignore` (tool working dir). The 2 governance upgrade tests already failing on origin/main (`test_GovernorSelfUpgrade`, `test_TokenUpgradeViaGovernance`) are unaffected by this commit and remain pre-existing. 1391 of 1393 tests pass on this branch. Findings deferred to follow-ups (out of scope for this PR) - Beacon SSZ endianness (B-01) — needs careful spec verification - Cross-chain trust boundary hardening (timelocks on `setSlasher`/`setMessenger`, etc.) - LDV redeem controller scoping changes (interface-level) - `registerAdapter` deposit migration logic - `ValidatorPodManager._slash` migration to share-based pool - TNTLockFactory permissionless `getOrCreateLock` - Storage layout snapshot test for upgrades - ~20 missing event handlers in indexer (I-2)
This was referenced May 8, 2026
drewstone
added a commit
to tangle-network/ai-trading-blueprint
that referenced
this pull request
May 8, 2026
…, Rust) (#71) * chore(soldeer): bump tnt-core 0.10.1 → 0.13.0 tnt-core v0.13.0 binds quote signatures to the requester address (PR #124, #125). Refresh the soldeer lockfile and remappings so contracts pick up the new Types.QuoteDetails / Types.JobQuoteDetails layout. * test(contracts): bind QuoteDetails to a non-zero requester (v0.13.0) DebugEip712 / DebugSign hand-roll a Types.QuoteDetails literal so they need the new `requester` field set explicitly. Use address(0xbEEF) as the placeholder — address(0) is rejected by SignatureLib. Also refresh the hardcoded QUOTE_TYPEHASH string in DebugEip712 so the on-chain digest matches the keccak-printed value. Refs: tangle-network/tnt-core#124, #125 * proto(pricing): add requester field to Quote/JobQuote details (v0.13.0) Mirrors tnt-core v0.13.0 which binds the requester address into the EIP-712 typed data for both QuoteDetails and JobQuoteDetails. The on-chain verifier rejects wildcard address(0). Field numbers are appended (not renumbered) to preserve wire compat for in-flight clients during rollout. Regenerate `arena/src/lib/gen/pricing_pb.ts` via `pnpm exec buf generate`. Refs: tangle-network/tnt-core#124, #125 * feat(arena): thread requester through quotes pipeline (v0.13.0) The on-chain createServiceFromQuotes tuple now leads with `requester` and the contract enforces it equals msg.sender (and rejects address(0)). Wire that through end-to-end: - abis.ts: prepend `{ name: 'requester', type: 'address' }` to the quote details tuple components for createServiceFromQuotes - useQuotes.ts: surface `requester` on OperatorQuote.details, take a `requester?: Address` argument so the hook holds the fetch until the wallet is connected, and read the operator-signed value from the pricing-engine response (falling back to the caller's address — a mismatch will fail on-chain verification, which is the desired behaviour) - provision.tsx: pass userAddress into useQuotes and forward q.details.requester into the on-chain tuple as the first field Refs: tangle-network/tnt-core#124, #125, blueprint-sdk#1406 * feat(operator-api): surface requester on JobQuote responses (v0.13.0) tnt-core v0.13.0 requires a non-zero requester bound into every job quote signature. Update the REST job-quote handlers to: - accept `requester: Option<String>` on JobQuoteRequest (Option keeps wire compat for staged rollout) - echo the requester back in the JSON response so callers spot a missing/zero requester before they hit the on-chain reject - log a warning + default to address(0) when the field is omitted, so the on-chain verifier rejects (defensive — never silently wildcard-sign) The actual signed quotes still come from the gRPC pricing engine; these REST endpoints are informational helpers for dashboards / non-gRPC integrations. Refs: tangle-network/tnt-core#124, #125, blueprint-sdk#1406
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
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
Eight parallel auditors fanned out across services lifecycle, payments/escrow, slashing, staking/MAD/LDV, beacon+bridges, governance/MBSM/blueprints, jobs/RFQ + indexer + bindings, and test-quality + gas. This PR lands the Tier-1 fixes: small, high-confidence, verified before patching, no interface-level redesigns or large refactors. Findings beyond this scope are listed at the bottom.
Security fixes
Service lifecycle
expireServiceRequestselector onTangleServicesFacet. The aa511c2 release shipped the function onITangleServicesbut never registered the selector, so the cleanup path was unreachable on the proxy. The recent fix was incomplete.scheduleExit,executeExit,forceExit,leaveService,forceRemoveOperator) so a stale operator can't fire exit paths on a Terminated service.approveServicerejects requests past the expiry grace, mirroringrejectService. Closes the race where an operator could quietly activate a stale request.requestService*. Without this,req.operatorCountexceeds the unique approver count and the request can never reach activation.nonReentrantonterminateServiceandterminateServiceForNonPayment.Slashing
nonReentrantonproposeSlashanddisputeSlash. OnlyexecuteSlash/executeSlashBatch/cancelSlashwere guarded.TIMESTAMP_BUFFERsymmetry on the Disputed branch ofSlashingLib.isExecutable. The Pending branch had a 15s buffer; Disputed didn't.bytes32(0)evidence inproposeSlash.ITangleSlashingevents realigned with what the protocol actually emits.SlashProposedis 8 fields (was 4),SlashExecutedis 4 fields (was 3); previously-missingSlashDisputed,SlashCancelled,SlashConfigUpdateddeclared. Rust consumers wired to the interface couldn't decode any slash event before this fix.Quote signature binding (BREAKING)
requesteradded toQUOTE_TYPEHASHandhashQuoteABI encoding. The field lived on the struct but was excluded from the typed data; an attacker who saw a signed quote could flipdetails.requesterand the signature still recovered. Off-chain signers MUST update; existing pre-fix signatures are invalid against the new typehash. All in-tree signing helpers updated.Payments
_distributePaymentWithEffectiveExposurereverts when zero operators are active at billing time. The dev/treasury split was paying out while the operator+staker pool (default 60%) remained retained by the contract with no path back. Service owners recover escrow viaterminateService->withdrawRemainingEscrow.whenNotPausedonfundService,billSubscription,billSubscriptionBatch. Reward / refund claim paths remain unguarded so users can always exit.Staking
OperatorStatusRegistry.registerOperatorresets per-(serviceId, operator) heartbeat / metrics state on (re-)register. Without this,isHeartbeatCurrentcould return true before any new heartbeat landed.LiquidDelegationVault.requestRedeemrejectscontroller == address(0)so redeemer's burned shares aren't permanently locked.Indexer
SlashConfigUpdatedevent signature inindexer/config.yamlnow declares all 6 fields (was 3); the old shape silently dropped every emission. Schema and handler updated.Dead code removed
src/exposure/andtest/exposure/— orphan module reproducing bps math that already lives insrc/core/PaymentsEffectiveExposure.sol. No production references.MockPriceOracle(still used by other suites) moved totest/mocks/.src/interfaces/IStreamingPaymentAdapter.sol— none of the four interfaces declared in this file are implemented or referenced anywhere.Tests
test/security/AuditFixesTest.t.solpins five regressions: expire reach, approve-after-grace revert, duplicate-operator rejection, exit-on-terminated revert, zero-evidence slash revert.test_DisputeSlash_BondForfeitOnExecutewarp adjusted by the new 15s buffer.Bindings
Regenerated via
cargo xtask gen-bindings. Version bumped to 0.12.0 for the breaking typehash + event-shape changes; CHANGELOG covers every fix.Test plan
forge buildclean (0 errors)test_GovernorSelfUpgrade,test_TokenUpgradeViaGovernance) are pre-existing failures onorigin/main— verified by stash + re-run. Not caused by this PR; recommend a separate fix.Findings deferred to follow-up PRs (out of scope)
Each was either too risky to bundle (architectural change, interface redesign) or needs design discussion before fixing:
BeaconChainProofs; tests pass because fixtures mirror the same packing. Needs spec verification against real EigenPod proofs before fixing.setSlasher/setMessenger, hardenedoriginalSenderpropagation.registerAdaptermigration logic (S-2) — needs deposit migration plan.ValidatorPodManager._slash(G-02) — O(N delegators) DoS; migrate to share-based pool.TNTLockFactory.getOrCreateLock(gov H-4) — front-run-able delegatee theft.expireJobCallentrypoint.🤖 Generated with Claude Code