chore(audit): Round 4 architectural consolidation (C-3 + F5 + G-02) + bindings v0.15.0#129
Merged
Conversation
… v0.15.0
Round 4 audit fix C-3. The five L2-side receivers that accept cross-chain
slashing and bridge messages were immutable, so any post-mainnet bug
required redeploying the receiver and re-authorising every sender on every
bridge. They are now UUPS upgradeable, with state moved to ERC-7201
namespaced slots and `_authorizeUpgrade` gated on the existing owner role.
Contracts converted:
- src/beacon/L2SlashingReceiver.sol (`tangle.beacon.L2SlashingReceiver`)
- src/beacon/bridges/{Arbitrum,Base,Hyperlane,LayerZero}*Receiver
(`tangle.beacon.bridges.<name>`)
Each contract:
- Inherits Initializable + UUPSUpgradeable + OwnableUpgradeable.
- Disables initializers on the implementation (`_disableInitializers()`
in the constructor).
- Holds all mutable state in a single namespaced struct + 50-slot
`__gap` for forward compatibility.
- Routes upgrade authority through `onlyOwner` (production owner should
be a multisig / timelock; the receiver's H-4 SENDER_ACTIVATION_DELAY
timelock on messenger / slasher swaps remains unchanged).
Init order: ownership is granted before any other initialization step so
the configuration calls that follow (`setMessenger`, `setAuthorizedSender`,
`setTrustedSender`, `setPeer`, ...) are guarded from block one. The
bootstrap exemption that allows `setMessenger`/`setSlasher` to write
immediately when the current value is zero is preserved so deploy scripts
do not deadlock for two days.
Migration: existing deployments must be re-deployed behind a fresh
ERC1967 proxy. The receivers were not yet on mainnet, so there is no
in-place storage migration path to support; this is documented in the
bindings CHANGELOG.
Deploy + tests:
- script/DeployL2Slashing.s.sol now deploys impl + ERC1967 proxy and
encodes `initialize(...)` calldata for all four bridge variants.
- New test/beacon/L2SlashingReceiverUUPS.t.sol covers init-is-one-shot,
impl-cannot-be-initialized, owner-gated upgrade, attacker-rejected
upgrade, state-persistence-across-upgrade, and ERC-7201 slot
placement (vm.load on the namespaced slot).
- All 5 affected existing test files were updated to deploy via proxy
and to expect `OwnableUnauthorizedAccount` instead of "Only owner"
for the receivers.
Bindings + versioning:
- cargo xtask gen-bindings re-run.
- bindings/Cargo.toml + fixtures/Cargo.toml bumped to 0.15.0
(coordinated minor for all four Round 4 PRs).
- CHANGELOG entry added.
Round 4 audit G-02: refactor ValidatorPodManager from raw `int256
podOwnerShares` accounting to per-pod share-pool accounting consistent
with the rest of the staking surface (MultiAssetDelegation,
LiquidDelegationVault, RewardsManager).
Per-pod accounting model:
- Each pod owner has an isolated `BeaconPool { totalAssets, totalShares }`.
- Beacon-chain rebases (rewards / slashes) move totalAssets only; share
balances are unaffected.
- Deposits (validator credential proofs) mint shares against the pool at
the current exchange rate. Withdrawals burn shares and transfer the
asset-equivalent ETH out of the pod.
- Virtual offsets (1e3) defend against first-depositor inflation
attacks; matches LiquidDelegationVault exactly.
Slashing semantics: slashes remain isolated to the affected pod (existing
invariant). With one shareholder per pod, "totalAssets -= slashed; shares
unchanged" reduces the owner's claimable assets without spreading the
loss across other pod owners. Same per-pod-isolation guarantee, now with
share-pool math (mulDiv + virtual offset).
Withdrawal queue: snapshot `convertToAssets(shares)` at queue time,
transfer `min(snapshot, live)` at completion. Beacon slashes between
queue and complete still take effect; rebase-up profit between queue and
complete stays with remaining shareholders rather than being captured by
the redeemer.
Public API surface:
- `recordBeaconChainDeposit(podOwner, assets)` — mints shares.
- `recordBeaconChainRebase(podOwner, assetsDelta)` — moves totalAssets only.
- Legacy `recordBeaconChainEthBalanceUpdate(podOwner, sharesDelta)` is
preserved for backwards compatibility (positive delta == deposit,
negative delta == rebase down).
- `getShares(owner)` now returns uint256 (was int256). Negative-share
states are no longer representable; full slashes saturate totalAssets
at zero and `getRestakedAssets` reflects the residual virtual-offset
dust (sub-1000 wei).
- New views: `convertToShares`, `convertToAssets`, `totalAssetsOf`,
`totalSharesOf`, `getRestakedAssets`. `totalShares()` is now a function
returning uint256 (was a public int256 state variable).
Tests: 14 new cases in test/beacon/ValidatorPodManagerSharePool.t.sol
covering proportional minting, rebase up/down, full-slash saturation,
inflation defense, pod isolation, conversion round-trip, withdrawal
flows (normal / slashed mid-queue / rewarded mid-queue capped at
snapshot), and access control. Existing beacon tests updated to the new
share-pool semantics.
Round 4 audit F5: subscription billing prices a period at the operator
stake observed at the bill instant, allowing an operator to ramp stake
just before billing and dump it after. With realistic ramp ratios this
overcharges customers when stake collapses mid-period and undercharges
when it ramps up. Replace flat-rate billing with a TWAP-fair amount
derived from cumulative stake-seconds.
Staking layer (cum-stake-seconds index):
- Add `_cumStakeSeconds[op][asset]` and `_cumStakeSecondsLastUpdate`
in DelegationStorage. Compound-v2 / Aave-v3 style index: every
stake-changing path folds `prevStake × (now - lastUpdate)` into the
running counter BEFORE the underlying stake mutates.
- Hooks (`_accrueOperatorStakeSeconds`) added in:
- OperatorManager: joinAsOperator (native + ERC20), bondMore,
bondMoreERC20, unstakeAsOperator, exitAsOperator.
- RewardsManager._onDelegationChanged: covers all delegate /
undelegate / reward-pool mutations (single funnel).
- SlashingManager: native + asset slash paths.
- First accrual seeds `lastUpdate` without area contribution, so
pre-existing pools begin TWAP cleanly at upgrade time without
back-paying for unobservable history.
- Storage gap reduced by 2 (46 → 44).
- New view `IStaking.getCumStakeSeconds(operator, asset)` exposed via
`StakingViewsFacet`. `ValidatorPodManager` ships a zero stub
(subscription billing is not currently routed through beacon-only
services; documented inline).
Payments layer (TWAP billing):
- `_billSubscriptionInternal` and `_tryBillSubscription` now call
`_computeTwapBillAmount(serviceId, operators, nominalRate, interval)`
which:
1. Aggregates `getCumStakeSeconds` and current stake across the
service's active operators for the bond asset.
2. Forward-projects `cum` to the period boundary so missed/late
bills don't double-count when the function later advances
`lastPaymentAt += interval`.
3. Lazy-initializes the per-service cursor and baseline on first
bill (no migration required); first post-init bill returns
`nominalRate` exactly.
4. From the next bill onward, bills
`nominalRate × cumDeltaPeriod / (baseline × interval)`.
- Append `lastBilledCumStake` and `subscriptionBaselineStake` to
`PaymentLib.ServiceEscrow`. `_serviceEscrows` is a top-level mapping,
so appending preserves all existing storage slots.
Tests: 4 cases in test/security/F5TWAPBilling.t.sol — constant stake
bills nominal, doubled mid-period bills 1.5×, halved mid-period bills
0.75×, mid-period slash bills by post-slash stake. Mock IStaking impls
in CrossChainSlashing and DeploymentScripts tests gain zero-return
`getCumStakeSeconds` to satisfy the interface.
- Expose F5 `getCumStakeSeconds(operator, asset)` via the `IMultiAssetDelegation` interface so Rust callers can read TWAP stake-seconds without resorting to manual selector encoding. Mirrors how `getOperatorStakeForAsset` is exposed. - Regenerate ABIs and Alloy bindings against the consolidated tip (C-3 UUPS receivers + G-02 share-pool VPM + F5 TWAP billing).
4 tasks
Resolve trivial bindings metadata conflicts after #127 (S-1 + S-2) landed on main. Both new public-interface additions — `IMultiAssetDelegation.AdapterChangeWhileDepositsExist` (#127) and `IMultiAssetDelegation.getCumStakeSeconds` (this PR) — coexist after regenerating bindings. - Cargo.lock: take main's lock; cargo build re-pins our 0.15.0 crate. - bindings/TNT_CORE_VERSION + ABI + Rust bindings: regenerated against the merged source so both the new error and the new view decode. - Source-level conflicts auto-resolved: S-1/S-2 hit StakingAssetsFacet, LiquidDelegationVault, IMultiAssetDelegation (error declaration), and added their own tests; F5/G-02 hit disjoint regions.
Closes the gap items called out in the team-lead review of the consolidation PR. Each item is a correctness or API-shape concern, not a polish-only edit; tests added inline. F5 per-operator cursor (correctness) - The original F5 aggregate cum-stake cursor (`escrow.lastBilledCumStake = Σ cum_op` at last bill) was unsound under operator-set changes: when ops joined or left between bills, the next `cumDelta` summed across different sets and produced meaningless attribution. Clamping to zero on underflow was fail-soft but still wrong. - Replace with per-(service, operator) cursors in `TangleStorage._twapCursorByOp`. Each active operator's delta is computed individually against its own stored cursor; leavers are simply absent from `_activeServiceOperators` (their cursor stays parked, harmless); rejoiners are handled by the join hook in `ServicesLifecycle._finalizeJoin` which re-seeds the cursor at the current cum, forgetting off-service-time accrual. - The lazy-init bill seeds every active operator's cursor inline so the first post-init bill produces nominal exactly (matching the prior semantics for new subscriptions) and the next bill measures from the period boundary. Pure-math fallback `PaymentLib.twapBillAmount` handles baseline / interval / overflow degeneracy without reverting. - Retire the aggregate `lastBilledCumStake` field as `__reservedAggregateCursor` so the struct slot layout stays stable across the F5 commit pair. F5 fuzz tests (process) - CLAUDE.md mandates fuzz for financial logic. The TWAP arithmetic is extracted as `PaymentLib.twapBillAmount` so we can fuzz the formula over the full input space without spinning up the staking layer per run. Four fuzz tests × 1000 runs each cover: - bill scales linearly with cumDelta (monotonicity) - canonical case: `cumDelta = baseline * interval` → bill ≈ nominal - no overflow on realistic uint128 inputs - pathological baseline/interval == 0 → nominal fallback F5 operator-set behavioral tests - `test_F5_MidPeriodJoiner_NoRetroactiveBill`: dynamic-membership subscription, operator2 stakes long before joining, then joins mid-period. Asserts the post-join bill is bounded — without the per-op cursor and join hook this test detonates with a several- orders-of-magnitude over-bill (operator2's pre-join cum gets attributed to the subscription). - `test_F5_OperatorExit_BillUsesRemainingSet`: operator2 scheduled-exits through the queue; the remaining operator-1-only bill is strictly positive and strictly below nominal. G-02: drop the `recordBeaconChainEthBalanceUpdate` back-compat shim - Same-selector / silently-different-semantics is the worst class of API break. With exactly one in-tree caller (`ValidatorPod`), the shim is pure surface area for misuse. Remove it; update both call sites in `ValidatorPod` (credential proof → `recordBeaconChainDeposit`, checkpoint finalize → `recordBeaconChainRebase`); migrate every test call site to the explicit pair; drop the now-unused `InvalidDelta` error. G-02: restore `getShares(address)` `int256` ABI - The G-02 refactor silently flipped `getShares` from `int256 → uint256` at the same selector. This is a decode-time break for any downstream consumer (Rust bindings, indexer ABI lookup) and was the worst kind of ABI break in an audit-sensitive contract. Restore `int256` return (lossless cast from the new `uint256` storage; defensive saturation at `int256.max` for completeness) and add `getSharesUint(address)` as the unsigned companion. LiveBeaconTest pre-existing failure (#130) - The `test_validatorFieldsExtraction` test was silently broken on main: `bytes32(uint256(N))` right-aligns N in big-endian order, but `BeaconChainProofs._fromLittleEndianUint64` extracts the leftmost eight bytes and byte-reverses them. The test encoded its inputs wrong, so `getEffectiveBalanceGwei(fields)` returned 0 instead of 32 gwei. Fix by adding a `_leUint64Bytes32` helper that matches the SSZ convention the contract actually implements. Resolves #130. Storage layout (forge inspect validated) - `MultiAssetDelegation`: F5 adds `_cumStakeSeconds` (slot 39) + `_cumStakeSecondsLastUpdate` (slot 40); `__gap` shrinks 46→44; all downstream slots unchanged. UUPS-safe. - `Tangle`: F5 followup adds `_twapCursorByOp` (slot 77); `__gap` shrinks 41→40; all downstream slots unchanged. UUPS-safe. - `ValidatorPodManager`: G-02 breaks layout (slot 7 changed type from `int256 podOwnerShares` to `mapping(address => BeaconPool) _pools`). Contract is not upgradeable — fresh deploy required. Documented in CHANGELOG. Tests - F5: 11/11 (5 unit + 4 fuzz × 1000 runs + 2 operator-set behavior) - Beacon: now 320/320 (LiveBeaconTest fix), no regressions in VPM / pod / integration suites - Staking + Security + Scripts + Scenario: no regressions
This was referenced May 11, 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
1 participant
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
Round 4 audit architectural pass — consolidated three deferred items into one PR plus a single coordinated
bindings v0.15.0cut. Supersedes #128.L2SlashingReceiverand the four bridge-adapter receivers (Arbitrum, Base, Hyperlane, LayerZero). ERC-7201 namespaced storage; explicit_ownerinitializer arg; OZ Ownable error semantics.Tangle.billSubscription(uint64)now billsrate × cumDelta / (baseline × interval)from a Compound/Aave-style cumulative stake-seconds index, defeating the ramp-and-dump exploit on flat-rate billing. Lazy-init at first bill, no migration required for existing subscriptions.ValidatorPodManagerconsistent withMultiAssetDelegation/LiquidDelegationVault. Beacon rebases (rewards/slashes) movetotalAssetsonly; share balances are invariant. Virtual offsets (1e3) match LDV. Withdrawal queue snapshotsconvertToAssetsat queue and pays outmin(snapshot, live)at completion.getCumStakeSecondsexposed viaIMultiAssetDelegationfor Rust callers (mirrorsgetOperatorStakeForAsset).Breaking changes
initialize(slasher, messenger, owner)instead of plainnew Contract(...)); revert reasons changed from strings to OZ Ownable custom errors.IStakinggainsgetCumStakeSeconds;PaymentLib.ServiceEscrowgains two appended fields (storage-safe append).getSharesreturnsuint256(wasint256);totalShares()is now a function (was a public state variable); newrecordBeaconChainDeposit/recordBeaconChainRebaseentry points; legacyrecordBeaconChainEthBalanceUpdatepreserved as a back-compat shim.Test plan
forge test --match-path test/security/F5TWAPBilling.t.sol— 5/5 pass (constant stake → nominal, doubled mid-period → 1.5×, halved → 0.75×, mid-period slash → post-slash stake, lazy-init first-post-upgrade nominal+forward-only)forge test --match-path test/staking/*— 260/260 pass (F5 accrual hooks at every stake-mutating path)forge test --match-path test/security/*— 27/27 passforge test --match-path test/beacon/*— 319/320 pass (1 pre-existing failure onLiveBeaconTest.test_validatorFieldsExtraction, also fails onmain, unrelated to this PR)forge test --match-path test/Integration.t.sol— 36/36 passforge test --match-path test/scenario/*— 1/1 passforge test --match-path test/scripts/*— 9/9 passcargo xtask gen-bindings— clean regeneration; F5getCumStakeSecondsdecodes (selector0xb1cefa3a)Closes #128.