Adopt OpenZeppelin upgradeable proxies for smart contracts (repo-wide checklist) #54
Labels
No labels
agent:fix_bugfix
agent:fix_conflicts
agent:fix_security
agent:gap_analysis
agent:implement
agent:open_issues
agents
agent:security_audit
agent:verify
area
timecurve
arena
arena-v2
audit
block:feature_review
block:security
bug
contracts
design
devops
docs
documentation
enhancement
epic
feature
frontend
indexer
low priority
megaeth
operations
ops
performance
QA
qa checklist
referrals
removal
security
smart-contracts
tech-debt
testing
time-arena
timecurve
visual-qa
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
code/yieldomega#54
Loading…
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
Adopt OpenZeppelin upgradeable proxies (UUPS and/or Transparent +
ProxyAdmin) so production Yieldomega contracts are deployed behind proxies with explicit implementation addresses, initializer flows, storage gaps, and documented upgrade governance.This issue lists repository touchpoints (contracts, tests, scripts, docs, skills, tooling). No implementation is included here.
Scope decisions (must be explicit before coding)
ProxyAdmin). Pick one default for the monorepo and document exceptions.TimeCurve,RabbitTreasury,FeeRouter,ReferralRegistry,LinearCharmPrice, fee sinks (PodiumPool,DoubLPIncentives,EcosystemTreasury,CL8YProtocolTreasury),DoubPresaleVesting(if used in production).Doubloon(ERC20) andLeprechaunNFT(ERC721) — upgrading these changes user expectations (token address = proxy; implementation swaps). Confirm whether these must be upgradeable or remain immutable implementations for trust/UX.DeployDevmocks (MockReserveCl8y,MockLaunchToken),contracts/src/fixtures/AnvilKumbayaFixture.sol, drill scripts — typically stay directnew; document that only canonical envs use proxies.ProxyAdminowner, and/or OZAccessControlroles; align withdocs/onchain/fee-routing-and-governance.mdanddocs/onchain/treasury-contracts.md.openzeppelin-contracts-upgradeable(version aligned with currentopenzeppelin-contracts5.6.x),remappings.txt, andforge install/ submodule docs.Smart contracts (
contracts/src/)For each in-scope contract:
constructorconfiguration withinitializerfunctions; inherit OZInitializableand relevant upgradeable base contracts (Ownable2StepUpgradeable,AccessControlEnumerableUpgradeable,ReentrancyGuardUpgradeable,ERC20Upgradeable/ERC721Upgradeableif tokens are upgradeable, etc.).constructorstate writes from logic contracts; use__X_init/__X_init_unchainedper OZ patterns.uint256[50] private __gap(or project-standard gap size) at end of each upgradeable contract for future storage layout safety.UUPSUpgradeable, implement_authorizeUpgrade, and document who may upgrade.DoubPresaleVesting: today allocations are constructor-locked; upgradeability must be reconciled with product docs (“immutable after deploy”) — either keep non-upgradeable or encode freeze afterstartVesting/ explicit initializer completion.LeprechaunNFT: metadata / admin patterns interact with “upgradeable URI” notes indocs/onchain/security-and-threat-model.md; align behavior and governance text.Deployment scripts (
contracts/script/)DeployDev.s.sol: deploy implementation → deploy proxy →initialize(encoded calldata). Log proxy addresses as the canonical addresses consumers use; optionally log implementation addresses for verification/ops.DeployKumbayaAnvilFixtures.s.sol: likely unchanged (local fixtures); cross-link that these are not the upgradeable production pattern.AnvilSameBlockDrill.s.sol: update if it builds a mini stack with core contracts; either use same proxy helper as tests or document “direct deploy for drill only.”SimulateAnvilRichState.s.sol: ensure it still targets proxy addresses if env registry switches.Tests (
contracts/test/)Every test that does
new TimeCurve,new FeeRouter,new RabbitTreasury, etc. must either:TestDeployutility matching production wiring.Files with heavy
newusage (non-exhaustive):TimeCurve.t.sol,TimeCurveReferral.t.sol,TimeCurveInvariant.t.solRabbitTreasury.t.sol,RabbitTreasuryInvariant.t.solFeeRouter.t.sol,FeeRouterInvariant.t.solDevStackIntegration.t.sol,NonStandardERC20.t.solDoubPresaleVesting.t.sol,ReferralRegistry.t.sol,LinearCharmPrice.t.sol,FeeSinks.t.sol,LeprechaunNFT.t.solNew / extended tests to plan:
upgradeToAndCall/ProxyAdmin.upgradeAndCallsucceeds only for authorized actor; unauthorized reverts.forge inspect/ OZ upgrades plugin patterns if adopted).ABI / registry tooling
contracts/script/export_abi_hashes.sh: today hashes implementation artifact ABIs. Decide whether registries pin proxy-facing ABI (same as implementation for delegatecall) vs document that ABI is implementation’s while address is proxy; update checklist text indocs/operations/deployment-checklist.mdaccordingly.contracts/deployments/*.json: add fields forimplementationAddress,proxyAdmin(if Transparent),kind(uups/transparent), and upgrade timelock / owner addresses as needed.scripts/start-local-anvil-stack.sh,scripts/lib/anvil_deploy_dev.sh,scripts/e2e-anvil.sh,scripts/anvil-export-bot-env.sh: parsing assumes console logs / broadcast JSON from current deploy shape; update if log format or contract order changes.Indexer (
indexer/)src/decoder.rs: event ABIs are inlined viasol!; proxy deployment does not change event topics if the implementation is unchanged, but any interface change on upgrade requires decoder updates and versioned event handling (already has legacy paths — extend policy for post-upgrade events).src/config.rs/ ingestion: ensure start block and contract addresses refer to proxy addresses; document implementation changes as a separate ops concern.Frontend (
frontend/)VITE_*addresses remain proxy addresses; no ABI change for callers if the external interface is stable.Bots (
bots/timecurve/)README.md,SKILL.md, env export scripts: registry JSON shape / address semantics (proxy vs impl).Documentation
Update or add sections for proxy architecture, initializer order, upgrade runbooks, and incident response:
docs/operations/deployment-stages.md,deployment-checklist.md,stage3-mainnet-operator-runbook.md,stage2-run-log.md,docs/qa/*.mddocs/onchain/treasury-contracts.md,fee-routing-and-governance.md,security-and-threat-model.md(constructor → initializer; upgrade threat; token upgrade policy)docs/testing/strategy.md,invariants-and-business-logic.md,e2e-anvil.md,local-swap-testing.md,anvil-same-block-drill.md,contract-fork-smoke.mddocs/integrations/kumbaya.md(only if deploy flow or address table changes)docs/agent-phases.md(especially Phase 15 deployment),docs/agents/metadata-and-skills.mddocs/architecture/overview.md,repository-layout.mdif trust boundaries / addresses are describedREADME.mdif it states “direct deployments”Skills
.cursor/skills/yieldomega-guardrails/SKILL.md: add a bullet that large contract refactors (proxies, initializers, upgrades) must update deployment docs +export_abi_hashes+ registry JSON conventions.skills/(play skills): only if any skill claims addresses are immutable bytecode at the same address forever; otherwise likely no change.CI / static analysis
.github/workflows/slither.yml: Slither oncontracts/may needslither.config.jsonupdates (proxy patterns, false positives ondelegatecall, uninitialized implementation — common exclusions or detectors).FOUNDRY_PROFILE=ci forge teststill passes with longer compile times / contract size limits (watch Spurious Dragon / contract size if implementations grow).Open questions for the issue thread
Related code today: direct
newdeployments incontracts/script/DeployDev.s.sol; constructors acrossTimeCurve,RabbitTreasury,FeeRouter, sinks,ReferralRegistry,LinearCharmPrice,Doubloon,LeprechaunNFT,DoubPresaleVesting, etc.openzeppelin-contracts-upgradeableis not yet incontracts/lib/.mentioned in commit
00482497ffmentioned in commit
0cc6342017Merged to
mainUUPS +
ERC1967Proxyfor core routing contracts is merged (branchissue-54-core-uups).Tip of
main:0cc6342Suggested verification
git pullandgit submodule update --init --recursivecd contracts && forge buildFOUNDRY_PROFILE=ci forge test(178 tests on the merge snapshot)DeployDev.s.sol,AnvilSameBlockDrill.s.soluseUUPSDeployLib; confirm any external deploy/anvil helpers still match proxy addresses if you script around them.cc @brouie — please review/verify on your side. Leaving #54 open until you sign off.
@PlasticDigits verified on
0cc6342merge chain.Verification per your checklist
git pull+git submodule update --init --recursive— openzeppelin-contracts-upgradeable v5.6.1 submodule + OZ sub-submodules (erc4626-tests, forge-std, halmos-cheatcodes) fetched cleancd contracts && forge build— 45 files compiled,Compiler run successful with warnings(lint-level only, no errors)FOUNDRY_PROFILE=ci forge test— 178/178 passing, 0 failed, 0 skipped (matches your expected count exactly). 18 test suites in 2.35s.DeployDev.s.solandAnvilSameBlockDrill.s.solmigrated toUUPSDeployLibfor all core routing contracts. Doubloon + LeprechaunNFT + MockReserveCl8y remain directnew()deploys per your commit note.Code review highlights
UUPSDeployLib.sol(166 lines) — textbook impl→ERC1967Proxy→initialize pattern. Each helper encodes init call viaabi.encodeCall, then wraps in proxy via single atomic constructor call.TimeCurve.solspot-check: proper UUPS hierarchy (Initializable, OwnableUpgradeable, ReentrancyGuard, UUPSUpgradeable),_disableInitializers()in constructor at L219 (critical security — prevents impl-direct initialization),initialize(...)withinitializermodifier at L237.AVE note
This slots cleanly into the
YO-Security-Checklist-DRAFTgap analysis for category 2.11 (storage collision / proxy patterns). Will update the draft to mark 2.11 as covered (UUPS proxy with OZ v5.6.1 +_disableInitializersguard + initializer modifier) rather than TBD.Closing as verified.
mentioned in issue #61
mentioned in issue #55
mentioned in issue #65
mentioned in issue #122
mentioned in issue #223