fix(security): timelock storage slot, drip mutex, inflation pool deregister#116
Merged
Merged
Conversation
…gister Code-level follow-ups from the pre-mainnet audit (#115). TangleTimelock - updateDelay restored to onlySelf (parent's policy); the previous override silently degraded it to onlyRole(DEFAULT_ADMIN_ROLE). - _setMinDelay now writes to the correct OZ ERC-7201 namespaced slot for TimelockControllerStorage._minDelay (TIMELOCK_CONTROLLER_STORAGE_LOCATION + 1) instead of slot 0x33. The old write was a no-op for getMinDelay() and could silently corrupt slot 51 of the proxy. - Bounds (MIN_DELAY=1d, MAX_DELAY=30d) still enforced. StreamingPaymentManager - dripAndGetChunk and dripOperatorStreams now nonReentrant. Closes the same-tx double-drip race where the transfer-to-distributor hook could re-enter and process additional chunks before the first frame finished. InflationPool - New deregisterOperator / deregisterOperators / deregisterCustomer / deregisterDeveloper paths. Inactive operators no longer accumulate in trackedOperators, capping distribution-loop gas growth over time. The swap-and-pop helper clears membership flags and registration epochs; pending rewards remain claimable. Re-registration is permitted. Tests - test/security/TimelockSetMinDelayTest.t.sol: 3 tests (caller auth, persistence through getMinDelay, bounds enforcement). - test/security/InflationPoolDeregisterTest.t.sol: 3 tests (removal, non-admin revert, re-registration). - Full suite: 1478/1478 passing.
Contributor
🔍 Reviewing
|
| Pass | Status | ETA |
|---|---|---|
| 🔬 Kimi agent | 🟡 Running (2 min) | ~5-15 min |
Agent review running. Reads the actual code. This comment updates in place.
tangletools · #116 · model: kimi-for-coding · started 2026-05-04T17:02:48Z
This was referenced May 4, 2026
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
Follows up #115 with the code-level remaining items from the pre-mainnet audit. Three operational items (populate
base-mainnet.jsonmultisigs, deploy/pin Timelock+Governor, choose Hyperlane ISM / LayerZero DVN config) need your input and aren't in this PR.Changes
TangleTimelock — fix
_setMinDelayslot + restoreonlySelfThe previous override of
updateDelayhad two bugs:onlySelftoonlyRole(DEFAULT_ADMIN_ROLE). Compromised admin could change the delay outside a queued proposal.sstore(0x33, newDelay). OZ 5.xTimelockControllerUpgradeableuses ERC-7201 namespaced storage at0x9a37c2aa…fb3600;_minDelayis at that location + 1. Writing to slot 51 was a silent no-op forgetMinDelay()and could corrupt unrelated proxy storage.Now:
onlySelfreinstated (callable only via a queued, scheduled proposal)._setMinDelaywrites toTIMELOCK_CONTROLLER_STORAGE_LOCATION + 1.StreamingPaymentManager — drip mutex
dripAndGetChunkanddripOperatorStreamsare nownonReentrant. Closes the same-tx double-drip race where the transfer-to-distributor hook could re-enter and process additional chunks before the first frame finished updatinglastDripTime.InflationPool — deregister inactive operators
trackedOperatorspreviously grew monotonically; every distribution iterated stale entries. New paths:deregisterOperator(address)deregisterOperators(address[])deregisterCustomer(address)deregisterDeveloper(address)Each clears the
isTracked*flag, swap-and-pops the address out of the tracked list, and clears*RegistrationEpoch. Pending rewards remain claimable. Re-registration is permitted.Tests
Two new files under
test/security/:TimelockSetMinDelayTest.t.sol(3 tests): non-self caller reverts,getMinDelay()reflects new value, bounds enforced.InflationPoolDeregisterTest.t.sol(3 tests): removes from tracked list, non-admin reverts, re-registration allowed.Full suite: 1478/1478 passing (1472 baseline + 6 new).
Test plan
forge buildcleanforge test-- 1478/1478 passing onfastprofile@openzeppelin-contracts-upgradeable 5.1.0. TheTIMELOCK_CONTROLLER_STORAGE_LOCATIONconstant must be re-verified before bumping.Still operational, not in this PR
deploy/config/base-mainnet.jsonwith the production multisig addresses.TangleTimelock+TangleGovernorand either pin their addresses or wire them throughFullDeploy.AggregationIsm[MultisigIsm + TrustedRelayer]) and LayerZero DVN/ULN.