Commit 95323ea
authored
fix(security): slashing model, manager-hook reentrancy, lifecycle, EIP-712 (#118)
* fix(security): slashing model, manager-hook reentrancy, lifecycle, EIP-712
Closes a broad class of pre-mainnet correctness gaps. Greenfield posture: takes
the right long-term API rather than backwards-compatible shims. All 1478 tests
pass on the fast profile.
Slashing
- New `disputeResolutionDeadline` (default 14d): a disputed slash auto-fails and
becomes executable after this window, so a forgotten or compromised SLASH_ADMIN
cannot lock delegators forever.
- New `disputeBond` (configurable, default 0/disabled): native-asset bond posted
when an operator disputes; forfeit to treasury on auto-fail / execute, refunded
on cancel. Defeats free-DoS via self-dispute.
- New `maxPendingSlashesPerOperator` (default 32): caps concurrent pending
proposals per operator so a malicious proposer can't spam-grief the
pending-slash counter.
- `executeSlash` now routes to `slashForService` when the operator made explicit
per-asset commitments to the offending service. Falls back to blueprint-wide
slashing only for legacy services with no commitments. Closes the over-broad
slashing of uncommitted assets.
- Operators with pending slashes can no longer call `_startLeaving`; pending
slashes must resolve first.
- Operators in `Inactive` status (e.g. forced inactive by being slashed below
minimum) can now exit via `_startLeaving`. Stake no longer stranded.
- `setSlashConfig` signature extended to surface the new knobs to admin.
- Per-operator pending tracker (`_operatorActiveSlashProposals`) increments on
propose, decrements on execute / cancel.
Manager-hook reentrancy + CEI
- `_callManager` / `_tryCallManager` capped at 500k gas. Failures from
`_tryCallManager` now emit `ManagerHookFailed(manager, selector, returnData)`
for off-chain observability.
- `Operators.registerOperator` reordered to write all state BEFORE the BSM hook
(CEI). Both registerOperator entrypoints + unregisterOperator gain
`nonReentrant`.
- `createBlueprint`, `transferBlueprint`, `updateBlueprint`, `deactivateBlueprint`,
`setJobEventRates`, `setBlueprintResourceRequirements` gain `nonReentrant`.
Service lifecycle
- `fundService` re-checks (a) blueprint manager's payment-asset policy and
(b) service TTL before accepting a top-up. Closes top-ups to expired services
and to services whose token policy was revoked.
- New permissionless `expireServiceRequest(requestId)` refunds the requester
after the configured grace period, so stale unapproved requests don't lock
payment indefinitely. All approve paths reject expired requests.
EIP-712 / replay
- Quote domain separator computed per-call against current chainid via new
`_domainSeparatorView()`. Prevents stale-cache replay across chain forks.
- `OperatorStatusRegistry.submitHeartbeat` now full EIP-712 with explicit
`timestamp` parameter and `HEARTBEAT_MAX_AGE` (5 minutes) freshness. Closes
cross-fork / cross-service / cross-deployment replay.
Governance for Base
- `TangleToken.clock()` switched to `block.timestamp` (ERC-6372). `votingDelay`
and `votingPeriod` are now denominated in seconds and chain-agnostic.
- `GovernanceDeployer` defaults updated: mainnet 1 day delay / 7 day period;
testnet 20 minute delay / 3 hour period.
MBSM registry
- Storage gap restored to 50 (greenfield clean).
Tests
- Governance test suite migrated to timestamp-clock semantics (vm.warp +
vm.roll) and updated default-param assertions.
- Heartbeat tests updated for EIP-712 typed digest and timestamp parameter.
- Slashing tests updated for the new 6-arg `setSlashConfig`.
- Slashing edge-case test now exercises the per-operator pending cap.
- 1478 / 1478 passing on fast profile.
* fix(security): address PR-118 review (CRITICAL + 4 HIGH + 7 non-blocking)
Tangletools review on commit 95b956f flagged 1 critical, 4 high, and 7
non-blocking findings. All addressed below. Full suite: 1483/1483.
CRITICAL: expireServiceRequest activated-service drain
- Added `bool activated` to Types.ServiceRequest. Set to true in
TangleServicesFacet._activateService BEFORE any other state mutations.
- expireServiceRequest now reverts on req.rejected || req.activated.
- _requireRequestNotExpired (the approval-path guard) also rejects activated
requests with ServiceRequestAlreadyProcessed.
HIGH: ITangleSlashing.disputeSlash missing payable
- Added `payable` to the interface. The implementation was already payable.
Without this, typed callers could not pass value through ITangleSlashing.
HIGH: dispute resolution deadline live-config bypass
- Added `uint64 disputeDeadline` to SlashProposal.
- Snapshot taken in SlashingLib.disputeSlash: disputeDeadline =
block.timestamp + config.disputeResolutionDeadline.
- isExecutable now reads proposal.disputeDeadline (snapshot), not live
config. Admin shrinking config.disputeResolutionDeadline cannot
retroactively shorten an already-disputed slash's review window.
HIGH: cancelSlash lacked nonReentrant + bond transferred before state
- Added nonReentrant to cancelSlash.
- Reordered: SlashingLib.cancelSlash + decrementPendingSlash +
_decrementOperatorPendingTracker complete BEFORE _settleDisputeBond.
- Same CEI ordering applied to executeSlash and executeSlashBatch
(state mutations, then bond settlement, then BSM hook).
Non-blocking
- _settleDisputeBond now restores the bond on transfer failure for BOTH
refund and forfeit paths (was only the refund path). Bond is never
silently lost. Treasury-unset case also restores rather than stranding.
- Removed dead `if (perOpCap == 0) perOpCap = 32` branch in proposeSlash
(config validates non-zero on init and update).
- SlashConfigUpdated event now emits all six fields.
- unregisterOperator: CEI reordering (state cleared before BSM hook).
- _completeLeaving: clears _operatorBondLessRequests and iterates
_operatorBlueprints to remove all blueprint associations on full exit.
- Governance test: getPastVotes/quorum lookups use block.timestamp - 1
(was block.number - 1, which only worked by Foundry-init coincidence).
New regression tests in SlashingEdgeCases.t.sol:
- test_DisputeSlash_RevertsOnWrongBond
- test_DisputeSlash_BondRefundedOnCancel
- test_DisputeSlash_BondForfeitOnExecute
- test_DisputeSlash_AdminDoesNotPostBond
- test_DisputeDeadlineSnapshot_IgnoresAdminShrink
test_ApproveService_AlreadyApproved_Reverts updated for the new activation
flag (uses a 2-operator request so the request stays non-activated through
the duplicate-approve test).
Storage gap reductions deferred (greenfield posture per project guidance;
gaps are arbitrary on first deploy, not a live-proxy migration).1 parent fdc890b commit 95323ea
34 files changed
Lines changed: 736 additions & 236 deletions
File tree
- script
- src
- core
- facets/tangle
- governance
- interfaces
- libraries
- staking
- test
- blueprints
- fuzz
- staking
- tangle
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
653 | 653 | | |
654 | 654 | | |
655 | 655 | | |
656 | | - | |
657 | | - | |
658 | | - | |
| 656 | + | |
| 657 | + | |
| 658 | + | |
| 659 | + | |
| 660 | + | |
| 661 | + | |
| 662 | + | |
| 663 | + | |
| 664 | + | |
| 665 | + | |
| 666 | + | |
| 667 | + | |
| 668 | + | |
| 669 | + | |
659 | 670 | | |
660 | 671 | | |
661 | | - | |
| 672 | + | |
662 | 673 | | |
663 | 674 | | |
664 | 675 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1393 | 1393 | | |
1394 | 1394 | | |
1395 | 1395 | | |
1396 | | - | |
| 1396 | + | |
1397 | 1397 | | |
1398 | | - | |
1399 | | - | |
1400 | | - | |
1401 | | - | |
| 1398 | + | |
| 1399 | + | |
| 1400 | + | |
| 1401 | + | |
| 1402 | + | |
| 1403 | + | |
| 1404 | + | |
| 1405 | + | |
| 1406 | + | |
| 1407 | + | |
| 1408 | + | |
| 1409 | + | |
| 1410 | + | |
| 1411 | + | |
1402 | 1412 | | |
1403 | 1413 | | |
1404 | 1414 | | |
1405 | 1415 | | |
1406 | 1416 | | |
1407 | 1417 | | |
1408 | 1418 | | |
| 1419 | + | |
1409 | 1420 | | |
1410 | 1421 | | |
1411 | 1422 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
78 | 78 | | |
79 | 79 | | |
80 | 80 | | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
81 | 85 | | |
82 | | - | |
83 | 86 | | |
84 | 87 | | |
85 | 88 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
53 | 53 | | |
54 | 54 | | |
55 | 55 | | |
56 | | - | |
| 56 | + | |
57 | 57 | | |
58 | 58 | | |
59 | 59 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
232 | 232 | | |
233 | 233 | | |
234 | 234 | | |
| 235 | + | |
| 236 | + | |
| 237 | + | |
| 238 | + | |
| 239 | + | |
| 240 | + | |
235 | 241 | | |
236 | 242 | | |
237 | 243 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
62 | 62 | | |
63 | 63 | | |
64 | 64 | | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
65 | 70 | | |
66 | 71 | | |
67 | 72 | | |
| |||
181 | 186 | | |
182 | 187 | | |
183 | 188 | | |
184 | | - | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
185 | 193 | | |
186 | 194 | | |
187 | 195 | | |
188 | 196 | | |
189 | 197 | | |
190 | 198 | | |
| 199 | + | |
| 200 | + | |
| 201 | + | |
| 202 | + | |
| 203 | + | |
| 204 | + | |
| 205 | + | |
191 | 206 | | |
192 | 207 | | |
193 | 208 | | |
| |||
683 | 698 | | |
684 | 699 | | |
685 | 700 | | |
686 | | - | |
| 701 | + | |
| 702 | + | |
| 703 | + | |
| 704 | + | |
| 705 | + | |
| 706 | + | |
| 707 | + | |
| 708 | + | |
| 709 | + | |
687 | 710 | | |
688 | | - | |
| 711 | + | |
689 | 712 | | |
690 | 713 | | |
691 | 714 | | |
| |||
694 | 717 | | |
695 | 718 | | |
696 | 719 | | |
697 | | - | |
| 720 | + | |
| 721 | + | |
| 722 | + | |
698 | 723 | | |
699 | | - | |
700 | | - | |
| 724 | + | |
| 725 | + | |
| 726 | + | |
| 727 | + | |
701 | 728 | | |
702 | 729 | | |
703 | 730 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
30 | 30 | | |
31 | 31 | | |
32 | 32 | | |
| 33 | + | |
33 | 34 | | |
34 | 35 | | |
35 | 36 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
77 | 77 | | |
78 | 78 | | |
79 | 79 | | |
80 | | - | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
81 | 84 | | |
82 | 85 | | |
83 | 86 | | |
| |||
91 | 94 | | |
92 | 95 | | |
93 | 96 | | |
94 | | - | |
| 97 | + | |
95 | 98 | | |
96 | 99 | | |
97 | 100 | | |
| |||
105 | 108 | | |
106 | 109 | | |
107 | 110 | | |
108 | | - | |
| 111 | + | |
109 | 112 | | |
110 | 113 | | |
111 | 114 | | |
| |||
123 | 126 | | |
124 | 127 | | |
125 | 128 | | |
126 | | - | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
127 | 133 | | |
128 | 134 | | |
129 | 135 | | |
| |||
164 | 170 | | |
165 | 171 | | |
166 | 172 | | |
| 173 | + | |
167 | 174 | | |
168 | 175 | | |
169 | 176 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
172 | 172 | | |
173 | 173 | | |
174 | 174 | | |
175 | | - | |
176 | | - | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
177 | 178 | | |
178 | 179 | | |
179 | 180 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
74 | 74 | | |
75 | 75 | | |
76 | 76 | | |
| 77 | + | |
77 | 78 | | |
78 | 79 | | |
79 | 80 | | |
| |||
87 | 88 | | |
88 | 89 | | |
89 | 90 | | |
| 91 | + | |
90 | 92 | | |
91 | 93 | | |
92 | 94 | | |
| |||
150 | 152 | | |
151 | 153 | | |
152 | 154 | | |
153 | | - | |
154 | | - | |
155 | | - | |
156 | | - | |
157 | | - | |
158 | | - | |
159 | | - | |
160 | | - | |
161 | | - | |
162 | | - | |
163 | | - | |
164 | | - | |
165 | | - | |
166 | | - | |
167 | | - | |
168 | | - | |
169 | | - | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
170 | 161 | | |
171 | | - | |
172 | 162 | | |
173 | 163 | | |
174 | 164 | | |
| |||
189 | 179 | | |
190 | 180 | | |
191 | 181 | | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
192 | 192 | | |
193 | 193 | | |
194 | 194 | | |
195 | 195 | | |
196 | | - | |
| 196 | + | |
197 | 197 | | |
198 | 198 | | |
199 | 199 | | |
| |||
207 | 207 | | |
208 | 208 | | |
209 | 209 | | |
210 | | - | |
211 | | - | |
212 | | - | |
213 | | - | |
214 | | - | |
| 210 | + | |
| 211 | + | |
215 | 212 | | |
216 | 213 | | |
217 | 214 | | |
| |||
229 | 226 | | |
230 | 227 | | |
231 | 228 | | |
232 | | - | |
233 | 229 | | |
234 | 230 | | |
235 | 231 | | |
| 232 | + | |
| 233 | + | |
| 234 | + | |
| 235 | + | |
| 236 | + | |
236 | 237 | | |
237 | 238 | | |
238 | 239 | | |
| |||
0 commit comments