Repository navigation
Fix: Proxy version storage collision and KPI reward window snipe - #783
Merged
Merged
Conversation
2 of 3 tasks
marvinkruse
force-pushed
the
fix/kvz-reports
branch
2 times, most recently
from
October 22, 2026 16:58
9df066a to
e8adc0d
Compare
marvinkruse
force-pushed
the
fix/kvz-reports
branch
from
October 22, 2026 19:01
e8adc0d to
e8d34ec
Compare
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
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.
InverterTransparentUpgradeableProxy_v1stored its version in slots 0-2. The proxy keptmajorVersion,minorVersionandpatchVersionas 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_orchestratorin every module (andmoduleFactoryin the orchestrator). With a 1.x beacon the orchestrator becomesaddress(1),isTrustedForwarderthen reverts, and the module stops working. Pulling the same version again rewrites the same values, so it can't self-repair.LM_PC_KPIRewarder_v2could be sniped at settlement. The resolve callback calls_setRewards(rewardAmount, 1)and clearsassertionPendingin the same call. Since UMA'ssettleAssertionis permissionless, a contract can settle the assertion andstakein 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 + 1when a true assertion is resolved and revertstakewithModule__LM_PC_KPIRewarder_v2__CannotStakeDuringRewardWindowwhileblock.timestamp < rewardWindowEnd. Anyone staking after the window gets nothing from the resolved round, because rewards only accrue up torewardsEnd. 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:
independentUpdates = true, and only through the workflow's own update admin. Standard workflows useInverterBeaconProxy_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.ASSERT_TRUTHidentifier on Optimism, Polygon or Ethereum, sopostAssertioncurrently 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 instake, gap reduced to 49, description updated.src/modules/logicModule/interfaces/ILM_PC_KPIRewarder_v2.sol: new error.test_upgradeToNewestVersionKeepsImplementationStorage: slots 0-2 surviveupgradeToNewestVersionandversion()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.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 buildsucceedsforge test --match-path 'test/unit/proxies/*'passesforge test --match-path test/unit/modules/logicModule/LM_PC_KPIRewarder_v2.t.solpasses