Adopt OpenZeppelin upgradeable proxies for smart contracts (repo-wide checklist) #54

Closed
opened 2026-04-23 07:24:11 +00:00 by PlasticDigits · 9 comments
PlasticDigits commented 2026-04-23 07:24:11 +00:00 (Migrated from gitlab.com)

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)

  1. Proxy pattern: UUPS (upgrade logic on implementation; smaller proxy) vs Transparent (admin cannot accidentally call implementation functions; separate ProxyAdmin). Pick one default for the monorepo and document exceptions.
  2. Which contracts are in scope for “always behind a proxy”:
    • Core game / routing: TimeCurve, RabbitTreasury, FeeRouter, ReferralRegistry, LinearCharmPrice, fee sinks (PodiumPool, DoubLPIncentives, EcosystemTreasury, CL8YProtocolTreasury), DoubPresaleVesting (if used in production).
    • Tokens / NFTs: Doubloon (ERC20) and LeprechaunNFT (ERC721) — upgrading these changes user expectations (token address = proxy; implementation swaps). Confirm whether these must be upgradeable or remain immutable implementations for trust/UX.
    • Dev-only / fixtures: DeployDev mocks (MockReserveCl8y, MockLaunchToken), contracts/src/fixtures/AnvilKumbayaFixture.sol, drill scripts — typically stay direct new; document that only canonical envs use proxies.
  3. Upgrade authority: multisig, timelock, ProxyAdmin owner, and/or OZ AccessControl roles; align with docs/onchain/fee-routing-and-governance.md and docs/onchain/treasury-contracts.md.
  4. Dependency: add openzeppelin-contracts-upgradeable (version aligned with current openzeppelin-contracts 5.6.x), remappings.txt, and forge install / submodule docs.

Smart contracts (contracts/src/)

For each in-scope contract:

  • Replace constructor configuration with initializer functions; inherit OZ Initializable and relevant upgradeable base contracts (Ownable2StepUpgradeable, AccessControlEnumerableUpgradeable, ReentrancyGuardUpgradeable, ERC20Upgradeable / ERC721Upgradeable if tokens are upgradeable, etc.).
  • Remove constructor state writes from logic contracts; use __X_init / __X_init_unchained per OZ patterns.
  • Add uint256[50] private __gap (or project-standard gap size) at end of each upgradeable contract for future storage layout safety.
  • If UUPS: inherit UUPSUpgradeable, implement _authorizeUpgrade, and document who may upgrade.
  • Audit storage layout order: parent initializers, linearization, no new state inserted before existing fields on upgrades.
  • DoubPresaleVesting: today allocations are constructor-locked; upgradeability must be reconciled with product docs (“immutable after deploy”) — either keep non-upgradeable or encode freeze after startVesting / explicit initializer completion.
  • LeprechaunNFT: metadata / admin patterns interact with “upgradeable URI” notes in docs/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.
  • Add or extend a shared internal library / abstract Script for “deploy proxy + initialize” to avoid drift between dev and prod scripts.

Tests (contracts/test/)

Every test that does new TimeCurve, new FeeRouter, new RabbitTreasury, etc. must either:

  • Deploy implementation + proxy + init (helper), or
  • Use a TestDeploy utility matching production wiring.

Files with heavy new usage (non-exhaustive):

  • TimeCurve.t.sol, TimeCurveReferral.t.sol, TimeCurveInvariant.t.sol
  • RabbitTreasury.t.sol, RabbitTreasuryInvariant.t.sol
  • FeeRouter.t.sol, FeeRouterInvariant.t.sol
  • DevStackIntegration.t.sol, NonStandardERC20.t.sol
  • DoubPresaleVesting.t.sol, ReferralRegistry.t.sol, LinearCharmPrice.t.sol, FeeSinks.t.sol, LeprechaunNFT.t.sol

New / extended tests to plan:

  • Upgrade path: upgradeToAndCall / ProxyAdmin.upgradeAndCall succeeds only for authorized actor; unauthorized reverts.
  • Initialization: cannot re-initialize; initializer modifiers correct.
  • Storage compatibility: optional contract-size / layout checks (e.g. 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 in docs/operations/deployment-checklist.md accordingly.
  • contracts/deployments/*.json: add fields for implementationAddress, 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 via sol!; 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.
  • If upgrade adds new functions or breaking view layout, update wagmi/viem hooks and any hard-coded selectors (grep for contract-specific assumptions).

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:

Area Examples
Operations docs/operations/deployment-stages.md, deployment-checklist.md, stage3-mainnet-operator-runbook.md, stage2-run-log.md, docs/qa/*.md
Onchain product docs/onchain/treasury-contracts.md, fee-routing-and-governance.md, security-and-threat-model.md (constructor → initializer; upgrade threat; token upgrade policy)
Testing docs/testing/strategy.md, invariants-and-business-logic.md, e2e-anvil.md, local-swap-testing.md, anvil-same-block-drill.md, contract-fork-smoke.md
Integrations docs/integrations/kumbaya.md (only if deploy flow or address table changes)
Agent / contributor docs/agent-phases.md (especially Phase 15 deployment), docs/agents/metadata-and-skills.md
Architecture docs/architecture/overview.md, repository-layout.md if trust boundaries / addresses are described
Root README.md if 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.
  • Root 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 on contracts/ may need slither.config.json updates (proxy patterns, false positives on delegatecall, uninitialized implementation — common exclusions or detectors).
  • Forge CI: ensure FOUNDRY_PROFILE=ci forge test still passes with longer compile times / contract size limits (watch Spurious Dragon / contract size if implementations grow).

Open questions for the issue thread

  1. Single ProxyAdmin per environment vs per-contract admin?
  2. Pause / emergency upgrade policy vs timelock-only upgrades?
  3. ERC20 / ERC721 upgradeability: yes/no and rationale for community communications.

Related code today: direct new deployments in contracts/script/DeployDev.s.sol; constructors across TimeCurve, RabbitTreasury, FeeRouter, sinks, ReferralRegistry, LinearCharmPrice, Doubloon, LeprechaunNFT, DoubPresaleVesting, etc. openzeppelin-contracts-upgradeable is not yet in contracts/lib/.

## 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) 1. **Proxy pattern:** **UUPS** (upgrade logic on implementation; smaller proxy) vs **Transparent** (admin cannot accidentally call implementation functions; separate `ProxyAdmin`). Pick one default for the monorepo and document exceptions. 2. **Which contracts are in scope for “always behind a proxy”:** - **Core game / routing:** `TimeCurve`, `RabbitTreasury`, `FeeRouter`, `ReferralRegistry`, `LinearCharmPrice`, fee sinks (`PodiumPool`, `DoubLPIncentives`, `EcosystemTreasury`, `CL8YProtocolTreasury`), `DoubPresaleVesting` (if used in production). - **Tokens / NFTs:** `Doubloon` (ERC20) and `LeprechaunNFT` (ERC721) — upgrading these changes **user expectations** (token address = proxy; implementation swaps). Confirm whether these must be upgradeable or remain **immutable** implementations for trust/UX. - **Dev-only / fixtures:** `DeployDev` mocks (`MockReserveCl8y`, `MockLaunchToken`), `contracts/src/fixtures/AnvilKumbayaFixture.sol`, drill scripts — typically **stay direct `new`**; document that only **canonical** envs use proxies. 3. **Upgrade authority:** multisig, timelock, `ProxyAdmin` owner, and/or OZ `AccessControl` roles; align with `docs/onchain/fee-routing-and-governance.md` and `docs/onchain/treasury-contracts.md`. 4. **Dependency:** add **`openzeppelin-contracts-upgradeable`** (version aligned with current `openzeppelin-contracts` **5.6.x**), `remappings.txt`, and `forge install` / submodule docs. ## Smart contracts (`contracts/src/`) For each **in-scope** contract: - Replace **`constructor`** configuration with **`initializer`** functions; inherit OZ **`Initializable`** and relevant **upgradeable** base contracts (`Ownable2StepUpgradeable`, `AccessControlEnumerableUpgradeable`, `ReentrancyGuardUpgradeable`, `ERC20Upgradeable` / `ERC721Upgradeable` if tokens are upgradeable, etc.). - Remove **`constructor`** state writes from logic contracts; use `__X_init` / `__X_init_unchained` per OZ patterns. - Add **`uint256[50] private __gap`** (or project-standard gap size) at end of **each** upgradeable contract for future storage layout safety. - If **UUPS:** inherit **`UUPSUpgradeable`**, implement `_authorizeUpgrade`, and document who may upgrade. - Audit **storage layout** order: parent initializers, linearization, no new state inserted before existing fields on upgrades. - **`DoubPresaleVesting`:** today allocations are **constructor-locked**; upgradeability must be reconciled with product docs (“immutable after deploy”) — either keep non-upgradeable or encode **freeze** after `startVesting` / explicit initializer completion. - **`LeprechaunNFT`:** metadata / admin patterns interact with “upgradeable URI” notes in `docs/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. - Add or extend a **shared internal library / abstract Script** for “deploy proxy + initialize” to avoid drift between dev and prod scripts. ## Tests (`contracts/test/`) Every test that does `new TimeCurve`, `new FeeRouter`, `new RabbitTreasury`, etc. must either: - Deploy **implementation + proxy + init** (helper), or - Use a **`TestDeploy`** utility matching production wiring. Files with heavy `new` usage (non-exhaustive): - `TimeCurve.t.sol`, `TimeCurveReferral.t.sol`, `TimeCurveInvariant.t.sol` - `RabbitTreasury.t.sol`, `RabbitTreasuryInvariant.t.sol` - `FeeRouter.t.sol`, `FeeRouterInvariant.t.sol` - `DevStackIntegration.t.sol`, `NonStandardERC20.t.sol` - `DoubPresaleVesting.t.sol`, `ReferralRegistry.t.sol`, `LinearCharmPrice.t.sol`, `FeeSinks.t.sol`, `LeprechaunNFT.t.sol` **New / extended tests to plan:** - Upgrade path: `upgradeToAndCall` / `ProxyAdmin.upgradeAndCall` succeeds only for authorized actor; unauthorized reverts. - Initialization: cannot re-initialize; initializer modifiers correct. - Storage compatibility: optional contract-size / layout checks (e.g. `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 in `docs/operations/deployment-checklist.md` accordingly. - **`contracts/deployments/*.json`:** add fields for `implementationAddress`, `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 via `sol!`; 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. - If upgrade adds **new** functions or breaking view layout, update wagmi/viem hooks and any **hard-coded** selectors (grep for contract-specific assumptions). ## 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: | Area | Examples | |------|----------| | Operations | `docs/operations/deployment-stages.md`, `deployment-checklist.md`, `stage3-mainnet-operator-runbook.md`, `stage2-run-log.md`, `docs/qa/*.md` | | Onchain product | `docs/onchain/treasury-contracts.md`, `fee-routing-and-governance.md`, `security-and-threat-model.md` (constructor → initializer; upgrade threat; token upgrade policy) | | Testing | `docs/testing/strategy.md`, `invariants-and-business-logic.md`, `e2e-anvil.md`, `local-swap-testing.md`, `anvil-same-block-drill.md`, `contract-fork-smoke.md` | | Integrations | `docs/integrations/kumbaya.md` (only if deploy flow or address table changes) | | Agent / contributor | `docs/agent-phases.md` (especially Phase 15 deployment), `docs/agents/metadata-and-skills.md` | | Architecture | `docs/architecture/overview.md`, `repository-layout.md` if trust boundaries / addresses are described | | Root | `README.md` if 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. - **Root `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 on `contracts/` may need `slither.config.json` updates (proxy patterns, false positives on `delegatecall`, uninitialized implementation — common exclusions or detectors). - **Forge CI:** ensure `FOUNDRY_PROFILE=ci forge test` still passes with longer compile times / contract size limits (watch **Spurious Dragon / contract size** if implementations grow). ## Open questions for the issue thread 1. **Single ProxyAdmin** per environment vs per-contract admin? 2. **Pause** / emergency upgrade policy vs **timelock-only** upgrades? 3. **ERC20 / ERC721** upgradeability: yes/no and rationale for community communications. --- **Related code today:** direct `new` deployments in `contracts/script/DeployDev.s.sol`; constructors across `TimeCurve`, `RabbitTreasury`, `FeeRouter`, sinks, `ReferralRegistry`, `LinearCharmPrice`, `Doubloon`, `LeprechaunNFT`, `DoubPresaleVesting`, etc. **`openzeppelin-contracts-upgradeable` is not yet** in `contracts/lib/`.
PlasticDigits commented 2026-04-23 13:32:13 +00:00 (Migrated from gitlab.com)

mentioned in commit 00482497ff

mentioned in commit 00482497fff8f48eb57dac198fd64bdaae7b921e
PlasticDigits commented 2026-04-23 13:32:39 +00:00 (Migrated from gitlab.com)

mentioned in commit 0cc6342017

mentioned in commit 0cc63420171c5b4efcbbc3cba872beb8ff930774
PlasticDigits commented 2026-04-23 13:32:47 +00:00 (Migrated from gitlab.com)

Merged to main

UUPS + ERC1967Proxy for core routing contracts is merged (branch issue-54-core-uups).

Tip of main: 0cc6342

Suggested verification

  • git pull and git submodule update --init --recursive
  • cd contracts && forge build
  • FOUNDRY_PROFILE=ci forge test (178 tests on the merge snapshot)
  • Dev deploy scripts: DeployDev.s.sol, AnvilSameBlockDrill.s.sol use UUPSDeployLib; 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.

## Merged to `main` UUPS + `ERC1967Proxy` for core routing contracts is merged (branch `issue-54-core-uups`). **Tip of `main`:** 0cc6342 **Suggested verification** - `git pull` and `git submodule update --init --recursive` - `cd contracts && forge build` - `FOUNDRY_PROFILE=ci forge test` (178 tests on the merge snapshot) - Dev deploy scripts: `DeployDev.s.sol`, `AnvilSameBlockDrill.s.sol` use `UUPSDeployLib`; 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.
Brouie commented 2026-04-24 02:02:31 +00:00 (Migrated from gitlab.com)

@PlasticDigits verified on 0cc6342 merge chain.

Verification per your checklist

  1. git pull + git submodule update --init --recursive — openzeppelin-contracts-upgradeable v5.6.1 submodule + OZ sub-submodules (erc4626-tests, forge-std, halmos-cheatcodes) fetched clean
  2. cd contracts && forge build — 45 files compiled, Compiler run successful with warnings (lint-level only, no errors)
  3. FOUNDRY_PROFILE=ci forge test — 178/178 passing, 0 failed, 0 skipped (matches your expected count exactly). 18 test suites in 2.35s.
  4. External deploy helpers: verified both DeployDev.s.sol and AnvilSameBlockDrill.s.sol migrated to UUPSDeployLib for all core routing contracts. Doubloon + LeprechaunNFT + MockReserveCl8y remain direct new() deploys per your commit note.

Code review highlights

  • UUPSDeployLib.sol (166 lines) — textbook impl→ERC1967Proxy→initialize pattern. Each helper encodes init call via abi.encodeCall, then wraps in proxy via single atomic constructor call.
  • TimeCurve.sol spot-check: proper UUPS hierarchy (Initializable, OwnableUpgradeable, ReentrancyGuard, UUPSUpgradeable), _disableInitializers() in constructor at L219 (critical security — prevents impl-direct initialization), initialize(...) with initializer modifier at L237.

AVE note

This slots cleanly into the YO-Security-Checklist-DRAFT gap 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 + _disableInitializers guard + initializer modifier) rather than TBD.

Closing as verified.

@PlasticDigits verified on `0cc6342` merge chain. ## Verification per your checklist 1. `git pull` + `git submodule update --init --recursive` — openzeppelin-contracts-upgradeable v5.6.1 submodule + OZ sub-submodules (erc4626-tests, forge-std, halmos-cheatcodes) fetched clean 2. `cd contracts && forge build` — 45 files compiled, `Compiler run successful with warnings` (lint-level only, no errors) 3. `FOUNDRY_PROFILE=ci forge test` — **178/178 passing**, 0 failed, 0 skipped (matches your expected count exactly). 18 test suites in 2.35s. 4. External deploy helpers: verified both `DeployDev.s.sol` and `AnvilSameBlockDrill.s.sol` migrated to `UUPSDeployLib` for all core routing contracts. Doubloon + LeprechaunNFT + MockReserveCl8y remain direct `new()` deploys per your commit note. ## Code review highlights - `UUPSDeployLib.sol` (166 lines) — textbook impl→ERC1967Proxy→initialize pattern. Each helper encodes init call via `abi.encodeCall`, then wraps in proxy via single atomic constructor call. - `TimeCurve.sol` spot-check: proper UUPS hierarchy (`Initializable, OwnableUpgradeable, ReentrancyGuard, UUPSUpgradeable`), `_disableInitializers()` in constructor at L219 (critical security — prevents impl-direct initialization), `initialize(...)` with `initializer` modifier at L237. ## AVE note This slots cleanly into the `YO-Security-Checklist-DRAFT` gap 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 + `_disableInitializers` guard + initializer modifier) rather than TBD. Closing as verified.
Brouie (Migrated from gitlab.com) closed this issue 2026-04-24 02:02:32 +00:00
Brouie commented 2026-04-24 06:45:16 +00:00 (Migrated from gitlab.com)

mentioned in issue #61

mentioned in issue #61
Brouie commented 2026-04-26 05:40:29 +00:00 (Migrated from gitlab.com)

mentioned in issue #55

mentioned in issue #55
Brouie commented 2026-04-27 04:12:19 +00:00 (Migrated from gitlab.com)

mentioned in issue #65

mentioned in issue #65
PlasticDigits commented 2026-05-04 04:19:37 +00:00 (Migrated from gitlab.com)

mentioned in issue #122

mentioned in issue #122
PlasticDigits commented 2026-05-20 04:42:09 +00:00 (Migrated from gitlab.com)

mentioned in issue #223

mentioned in issue #223
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
code/yieldomega#54
No description provided.