Skip to content

Fix: Proxy version storage collision and KPI reward window snipe - #783

Merged
marvinkruse merged 2 commits into
devfrom
fix/kvz-reports
Oct 22, 2026
Merged

marvinkruse merged 2 commits into
devfrom
fix/kvz-reports

Conversation

@marvinkruse

@marvinkruse marvinkruse commented Oct 22, 2026 •

Copy link
Copy Markdown
Member

Summary

Fixes two issues reported privately by an external security researcher. Neither affects a live deployment and no funds are at risk (details under Severity). The fixes are here so the code is correct for anyone deploying or reviewing it in the future.

  1. InverterTransparentUpgradeableProxy_v1 stored its version in slots 0-2. The proxy kept majorVersion, minorVersion and patchVersion as plain state variables, so they occupied the same slots as the implementation it delegates to. upgradeToNewestVersion (and the constructor) wrote the beacon version into slots 0-2, which overwrites __Module_orchestrator in every module (and moduleFactory in the orchestrator). With a 1.x beacon the orchestrator becomes address(1), isTrustedForwarder then reverts, and the module stops working. Pulling the same version again rewrites the same values, so it can't self-repair.
  2. LM_PC_KPIRewarder_v2 could be sniped at settlement. The resolve callback calls _setRewards(rewardAmount, 1) and clears assertionPending in the same call. Since UMA's settleAssertion is permissionless, a contract can settle the assertion and stake in the same transaction. It is then staked for the one-second reward window and takes a pro-rata share of a round it was never part of.

Fix

Proxy: move the version into an ERC-7201 namespaced struct (erc7201:inverter.storage.InverterTransparentUpgradeableProxy_v1), so the proxy no longer writes to any sequential slot. version() reads from the namespace. No change to the external interface.

KPI rewarder: record rewardWindowEnd = block.timestamp + 1 when a true assertion is resolved and revert stake with Module__LM_PC_KPIRewarder_v2__CannotStakeDuringRewardWindow while block.timestamp < rewardWindowEnd. Anyone staking after the window gets nothing from the resolved round, because rewards only accrue up to rewardsEnd. The new variable takes one slot from __gap (50 → 49), so the layout stays upgrade-safe. Also corrected the contract description, which still described the old staking queue.

Severity

Both reported as High. In practice:

  • Proxy: only reachable on workflows created with independentUpdates = true, and only through the workflow's own update admin. Standard workflows use InverterBeaconProxy_v1, which stores nothing, so beacon upgrades are unaffected (confirmed on deployed modules at 1.1.0 / 1.0.2). Across all chains with a deployed factory, the only independent-update workflow is an unused, empty test deployment on Optimism.
  • KPI rewarder: not deployed on any chain, and UMA no longer supports the ASSERT_TRUTH identifier on Optimism, Polygon or Ethereum, so postAssertion currently reverts anyway.

Changes

  • src/proxies/InverterTransparentUpgradeableProxy_v1.sol: version stored in an ERC-7201 namespace.
  • src/modules/logicModule/LM_PC_KPIRewarder_v2.sol: rewardWindowEnd, set in the resolve callback and checked in stake, gap reduced to 49, description updated.
  • src/modules/logicModule/interfaces/ILM_PC_KPIRewarder_v2.sol: new error.
  • Regression tests:
    • test_upgradeToNewestVersionKeepsImplementationStorage: slots 0-2 survive upgradeToNewestVersion and version() reports the new version.
    • test_RevertWhen_TheRewardWindowIsOpen: staking in the same second as the resolution reverts, staking one second later works and earns nothing from the round, and existing stakers keep the full round.
    • Both fail on the pre-fix code and pass on the fix.

Deployment Steps

✅ No deployment needed right now.

Neither fix changes a live contract. The proxy fix only affects proxies created after the factories are redeployed with the new proxy bytecode; existing proxies keep their original code. The KPI rewarder isn't deployed anywhere. If either is ever needed on a live deployment, it ships with a regular factory or module beacon release.

Test Plan

Pre-merge checklist

  • forge build succeeds
  • forge test --match-path 'test/unit/proxies/*' passes
  • forge test --match-path test/unit/modules/logicModule/LM_PC_KPIRewarder_v2.t.sol passes
  • New regression tests fail on the pre-fix code and pass on the fix
  • Full suite passes (1570 tests, including the KPI rewarder e2e lifecycle test repaired in CI: Pin Foundry to v1.3.6 and repair KPI e2e test #784)

@marvinkruse
marvinkruse force-pushed the fix/kvz-reports branch 2 times, most recently from 9df066a to e8adc0d Compare October 22, 2026 16:58
@marvinkruse
marvinkruse merged commit ff45162 into dev Oct 22, 2026
5 checks passed
@marvinkruse
marvinkruse deleted the fix/kvz-reports branch October 22, 2026 19:17
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