Skip to content

fix(staking): S-1 LDV controller hijack + S-2 adapter swap-under-load#127

Merged
drewstone merged 1 commit into
mainfrom
feat/s1-s2-controller-and-adapter-guards
May 10, 2026
Merged

fix(staking): S-1 LDV controller hijack + S-2 adapter swap-under-load#127
drewstone merged 1 commit into
mainfrom
feat/s1-s2-controller-and-adapter-guards

Conversation

@drewstone
Copy link
Copy Markdown
Contributor

Summary

Two surgical staking-layer fixes from the Round 4 architectural audit:

  • S-1 (LiquidDelegationVault): an authorized ERC-7540 operator could file a redemption with controller != owner, then drive redeem to route the owner's assets to any receiver. Operator-filed requests now require controller == owner. Owner-filed requests may still pick any controller — that's a legitimate ERC-7540 aggregator pattern.
  • S-2 (StakingAssetsFacet): raw registerAdapter / removeAdapter calls silently corrupted accounting when the asset had live deposits — balances either stranded in the old adapter or double-counted in the new one. Both paths now revert with AdapterChangeWhileDepositsExist(token, deposits) when currentDeposits != 0. The M-8 migration framework (startAdapterMigrationcompleteAdapterMigration) remains the intended drain path.

What changed

  • src/staking/LiquidDelegationVault.sol — operator-vs-controller gate in requestRedeem.
  • src/facets/staking/StakingAssetsFacet.solcurrentDeposits == 0 guard on registerAdapter / removeAdapter plus new custom error.
  • src/interfaces/IMultiAssetDelegation.sol — exposes the new error so Rust bindings can decode the revert.
  • test/staking/LiquidDelegationTest.t.sol — two S-1 regressions (operator must set controller == owner; owner can still delegate to a separate controller).
  • test/security/AdapterChangeWhileDepositsExistTest.t.sol — three S-2 regressions (register-blocked-while-deposits, remove-blocked-while-deposits, both allowed at zero deposits).
  • bindings/ — ABI regenerated; new error wired into alloy decoder.

Why these are surgical

  • S-1 preserves the legitimate ERC-7540 aggregator path (owner-as-controller-chooser) — only third-party operators are clamped.
  • S-2 plugs the raw register/remove paths but leaves the M-8 controlled-migration path untouched, so admins still have a clean way to swap adapters under load.

Bindings version

ABI changes only (one new custom error). Version bump deferred to whichever Round 4 PR lands last, to avoid racing four PRs onto v0.15.0.

Test plan

  • forge test --match-path test/security/AdapterChangeWhileDepositsExistTest.t.sol -vv — 3/3 pass
  • forge test --match-path test/staking/LiquidDelegationTest.t.sol --match-test "Operator|Owner" — 5/5 pass
  • forge test --match-path "test/staking/*.t.sol" — 262/262 pass
  • forge test --match-path "test/security/*.t.sol" — 25/25 pass
  • FOUNDRY_PROFILE=local_build forge build clean
  • cargo build --release -p tnt-core-bindings clean
  • CI full forge test (delegated)

S-1 (LiquidDelegationVault.requestRedeem):
  An authorized operator (ERC-7540 `_operators[owner][caller]`) could file a
  redemption with `controller != owner`, then drive `redeem` to route assets
  anywhere. Now an operator-driven request must set `controller == owner`;
  owners filing on their own behalf may still pick any controller (the legit
  ERC-7540 aggregator-vault use case).

S-2 (StakingAssetsFacet.registerAdapter / removeAdapter):
  Switching adapters while the asset has live deposits silently strands
  balances in the old adapter or double-counts them in the new one. Both
  paths now revert with `AdapterChangeWhileDepositsExist(token, deposits)`
  when `currentDeposits != 0`. The M-8 migration framework
  (`startAdapterMigration` -> `completeAdapterMigration`) remains the
  intended drain path for live assets. The new error is exposed on
  `IMultiAssetDelegation` so the Rust bindings decode the revert.

Tests:
  - test_Vault_OperatorCannotPickArbitraryController_S1
  - test_Vault_OwnerMayPickArbitraryController_S1 (preserves legit case)
  - test_registerAdapter_blockedWhileDepositsExist_S2
  - test_removeAdapter_blockedWhileDepositsExist_S2
  - test_registerAdapter_allowedWhenDepositsAreZero_S2

Bindings: ABI regenerated; new custom error decoded by alloy. Version bump
deferred to whichever Round 4 PR lands last so we don't race four PRs onto
v0.15.0.
@drewstone drewstone merged commit de91a36 into main May 10, 2026
1 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant