Feature/platform upgradeable

[robertocarlous]

Option chosen

Option A — Upgradeable platform contracts

I chose Option A because it maps directly to Phase P3 ("contract upgrade flows") in Developer/ROADMAP.md. The four platform contracts hold long-lived economic and coordination state; making them upgradeable preserves proxy addresses and on-chain state while allowing logic fixes over time. Task-level contracts (DINTaskCoordinator, DINTaskAuditor) were intentionally left as redeployable per the issue doc and task scope i was assigned to

---

Proxy pattern: Transparent Proxy

Chosen: OpenZeppelin Transparent Proxy + ProxyAdmin (via @openzeppelin/hardhat-upgrades).

Why not UUPS:

• Upgrade authority is separated from implementation logic — a bad V2 deployment cannot brick the proxy by omitting upgrade functions.
• Fits the current operational model where the DIN-Representative / deployer EOA controls upgrades today; governance/multisig can take ownership of ProxyAdmin later without changing the pattern.
• All four platform contracts share one coherent deployment model.

Toolchain:

• @openzeppelin/contracts-upgradeable ^5.3.0
• @openzeppelin/hardhat-upgrades ^3.9.1
• PROXY_KIND = "transparent" in hardhat/deploy/constants.ts

---

Storage layout rules followed

• No reordering or removal of existing storage variables across upgrades.
• New variables only appended in future versions (V2 test contracts add no new storage slots).
• immutable cross-contract references removed — proxies read storage by slot, not bytecode.
• constant values (MIN_STAKE, UNBONDING_PERIOD) unchanged — not stored in proxy slots.
• hardhat-upgrades validateUpgrade() run in tests before each V1 → V2 upgrade.

---

Per-contract changes

DinToken

Before	After
Constructor owner_	initialize()
immutable OWNER	coordinator storage
Custom onlyOwner for mint	onlyCoordinator for mint; OwnableUpgradeable for setup
Deployed inside DinCoordinator	Independently deployed proxy

Mint authority is wired post-deploy via setCoordinator(address) (one-shot, onlyOwner).

DinCoordinator

Before	After
new DinToken(address(this)) in constructor	initialize(address dinToken_)
immutable dinToken	dinToken storage
Ownable constructor	OwnableUpgradeable + _disableInitializers()

ReentrancyGuardTransient kept as-is (stateless / EIP-1153 — no proxy storage impact).

DinValidatorStake

Before	After
Constructor (dinToken, dinCoordinator)	initialize(dinToken, dinCoordinator)
immutable DIN_TOKEN, immutable DIN_COORDINATOR	Regular storage (getter names unchanged for ABI compatibility)
Ownable	OwnableUpgradeable

DINModelRegistry

Before	After
Constructor + daoAdmin = msg.sender	initialize(dinValidatorStake_)
daoAdmin + onlyDAOAdmin + setDAOAdmin()	OwnableUpgradeable + onlyOwner + transferOwnership()
Fee defaults on storage declarations	Fee defaults set in initialize() (proxy storage starts at zero)

Access model deviation: daoAdmin / setDAOAdmin / DAOAdminUpdated removed in favour of standard OZ owner() / transferOwnership(). Functionally equivalent for admin migration; dincli ABIs referencing the old interface are out of scope for this PR but will need a follow-up when testnet is redeployed.

---

Deployment order

Proxies cannot use nested new Contract() — wiring is explicit:

1. Deploy DinToken proxy → initialize()
2. Deploy DinCoordinator proxy → initialize(dinTokenAddress)
3. DinToken.setCoordinator(dinCoordinatorAddress)
4. Deploy DinValidatorStake proxy → initialize(dinTokenAddress, dinCoordinatorAddress)
5. DinCoordinator.updateValidatorStakeContract(stakeAddress)
6. Deploy DINModelRegistry proxy → initialize(stakeAddress)

Scripts:

cd hardhat
npm run deploy:platform -- --network hardhat

CONTRACT=DinToken npm run upgrade:platform -- --network localhost


## Tests

12 upgrade-safety tests (3 per platform contract):

**DinToken**
- State persistence: balances + coordinator after upgrade
- Access control: mint restricted to coordinator; `setCoordinator` owner-only
- Storage / wiring: `validateUpgrade()`

**DinCoordinator**
- State persistence: `dinPerEth`, token ref, mint flow
- Access control: `updateDinPerEth` owner-only
- Storage / wiring: slasher wiring to stake survives upgrade

**DinValidatorStake**
- State persistence: stake balances + validator status
- Access control: `addSlasher` coordinator-only; blacklist owner-only
- Storage / wiring: `validateUpgrade()`

**DINModelRegistry**
- State persistence: fees + approved model data
- Access control: fee setters owner-only
- Storage / wiring: pending requests readable post-upgrade

Run tests:

cd hardhat && npm test

Expected: **12 passing**

<img width="1792" height="1120" alt="Screenshot 2026-06-25 at 3 21 10 PM" src="https://github.com/user-attachments/assets/a5c21b84-d914-477a-b83b-3ff17f2922c1" />


<img width="1792" height="1120" alt="Screenshot 2026-06-25 at 3 21 47 PM" src="https://github.com/user-attachments/assets/6ccf758e-3e84-4290-9c70-00b1a8ceadc3" />


V2 implementations: `hardhat/contracts/upgrade/*V2.sol` (test-only; adds `version()` only).

---

## Doc inaccuracies (`Developer/issues/upgradable-contracts.md`)

- **Platform contracts use constructors** — Was true; converted to `initialize()` in this PR.
- **`DinToken` immutable `OWNER`** — Was true; now `coordinator` storage.
- **`DinCoordinator` immutable `dinToken` + inline `new DinToken()`** — Was true; external token proxy + storage ref.
- **`DinValidatorStake` immutable `DIN_TOKEN` / `DIN_COORDINATOR`** — Was true; now storage.
- **`DINModelRegistry` uses mutable `daoAdmin` but is not proxy-based** — Was true; now proxy + `OwnableUpgradeable`.
- **`hardhat/test/DinValidatorStake.test.ts` exists** — Incorrect; no platform contract tests on `develop`; this PR adds the first suite.
- **`hardhat-upgrades` in toolchain** — Was missing; added in commit 1.
- **Deploy scripts for platform proxies** — Were missing; added `scripts/deploy-platform.ts` and `scripts/upgrade-platform.ts`.

Task-level contracts remain redeployable by design — no change needed in the issue doc for that point.

---

## Out of scope (unchanged)

- Task-level contracts (`DINTaskCoordinator`, `DINTaskAuditor`)
- `dincli` changes / ABI updates / testnet address config
- Governance/DAO redesign (`Developer/issues/decentralized-governance.md`)
- Migration from existing Optimism Sepolia non-upgradeable deployments
- Production `ProxyAdmin` multisig / timelock

---

## With more time

1. Transfer `ProxyAdmin` ownership to a multisig or timelock before mainnet.
2. Testnet migration script: deploy proxies, document new addresses, coordinate `dincli` config update.
3. Update `dincli` ABIs for registry (`owner()` vs `daoAdmin()`) and publish redeployment runbook.
4. Add `updateCoordinator()` on token / stake if coordinator proxy replacement (new address) is ever required — current design assumes stable proxy addresses with implementation-only upgrades.
5. Negative upgrade tests (intentionally incompatible storage layout) to demonstrate `validateUpgrade` rejection in CI.

[robertocarlous]

@umermjd11 @abrahamnash

Please can you review this PR. Thank you

[umeradl]

PR Review — Robbert Abimbola

PR: #13
Branch: feature/platform-upgradeable
Option chosen: A (Upgradeable platform contracts)
Reviewed: 2026-06-26
Reviewers: Umer (lead), Abraham (secondary)
Test result on branch: 12 passing, 1 failing (pre-existing regression — see §3)

---

1. Overall Assessment

Strong, well-scoped submission. Robbert read the codebase carefully, made defensible architecture decisions, documented them clearly, and shipped a clean implementation with tests. The storage layout reasoning, deployment order, and proxy pattern justification all show genuine understanding — not copy-paste from a tutorial.

Abraham's independent read: "If I were reviewing this PR and the code matched the write-up, I'd probably approve it after a small compatibility discussion around DINModelRegistry." His strongest positive signals: correct proxy choice, proper handling of immutables and constructors, explicit deployment wiring, state-persistence tests rather than superficial upgrade tests, awareness of future governance migration. His only flagged weakness: the unnecessary API break from removing daoAdmin()/setDAOAdmin() in favour of pure OZ OwnableUpgradeable — a shim was available.

Verdict: Hold — and to be clear, this is not a reflection on the quality of Robbert's work. The Solidity implementation is solid and the hold is not on anything he did wrong architecturally. The hold exists because of a gap on our side: a dincli CLI-level test harness does not yet exist, and merging upgraded contracts without it means we have no safety net to confirm the Python CLI still works against the new interfaces. That is a precaution we owe to the codebase, not a criticism of this PR. Three conditions must be met before merge: (1) fix the broken pre-existing test (§3), (2) resolve the daoAdmin compatibility question (§4), and (3) CLI-level integration tests covering dincli calls against the upgraded contracts must be in place. The contract ABI changes — notably daoAdmin() removal and immutable → storage conversions — almost certainly affect existing dincli command paths, but without a test harness at the CLI layer we cannot safely confirm or bound that impact. See §8 for detail.

---

1a. Follow-up Commit (4c58299 — pushed after initial review)

Robbert pushed a second commit on 2026-06-26 titled "fix(contracts): address upgrade review concerns" before being sent this review. It proactively addressed several minor items:

Item	Status
transfer() → call{value:} in DINModelRegistry.withdrawFees() + TransferFailed error	✅ Fixed — matches DinCoordinator pattern
DinToken.initialize() visibility public → external	✅ Fixed — all 4 now consistent
Explanatory comments on all 4 V2 contracts justifying @custom:oz-upgrades-unsafe-allow missing-initializer	✅ Added
Implementation addresses tracked in deployments JSON after each upgrade (types.ts + upgrade-platform.ts)	✅ Added — partially addresses the ProxyAdmin recording concern

What this commit does NOT address (all items from §3–§5 below remain open):

• DinValidatorStake.test.ts regression (required, §3)
• require() string errors in DINModelRegistry.requestModelRegistration (§4)
• daoAdmin compatibility shim question (§4)
• Missing _disableInitializers() runtime tests (§4)
• __gap storage slots (§5 minor)
• Coordinator replacement intentionally undocumented (§5 minor)

The commit shows Robbert is actively iterating on feedback — positive signal.

---

2. Outstanding Work

Architecture / Design

• Proxy pattern justification is correct and specific. Transparent Proxy over UUPS because "a bad V2 cannot brick the proxy by omitting upgrade functions" — this is exactly the right risk model for an early-stage protocol with a single deployer EOA and governance handoff pending. The reasoning maps to the current operational stage.
• Deployment order script is non-trivial and correct. The circular token↔coordinator dependency (DinToken needs coordinator address; DinCoordinator needs token address) is resolved cleanly via a post-deploy setCoordinator(coordinatorAddress) wire. Getting the 6-step ordering right — including updateValidatorStakeContract before DINModelRegistry deploys — requires actually understanding the contract graph.
• DIN_TOKEN / DIN_COORDINATOR ALL_CAPS naming preserved on DinValidatorStake despite converting from immutable to mutable storage. This avoids a silent ABI break for any existing callers of DIN_TOKEN() and DIN_COORDINATOR(). Conscious and correct.
• ReentrancyGuardTransient reasoning is sound. Correctly identified that it uses transient storage (EIP-1153), not contract storage, so no proxy storage layout impact and no __init call needed.

Implementation

• _disableInitializers() present in all 4 constructors. Abraham independently flagged this as a critical check — verified present in DinToken, DinCoordinator, DinValidatorStake, and DINModelRegistry. This prevents attackers from initializing implementation contracts directly and taking ownership.
• CoordinatorAlreadySet one-shot guard on setCoordinator. The coordinator wiring is set-once, not any-owner-can-override. Tight.
• Fee defaults in DINModelRegistry.initialize(), not at declaration. Correct for proxy storage — storage slots start at zero, so defaults must be set in the initializer, not via storage declarations.
• validateUpgrade() called in all 4 test suites before actually upgrading. This exercises the OZ storage-layout checker, not just runtime behaviour.

Tests

• deployPlatform() + mintDinViaDeposit() helper is clean. Factored into test/helpers/platform.ts, reused across all 4 suites — no copy-paste setup.
• @custom:oz-upgrades-from annotation on all V2 contracts. Correct usage required by OZ upgrades plugin for cross-contract upgrade validation.
• Test structure hits all 3 required axes per contract: state persistence, access control, storage layout. No gaps against the task spec axes.

Documentation

• Doc inaccuracy list is accurate and complete. All 8 items he flagged were genuinely stale before this PR and are now resolved by the implementation.
• "With more time" section shows good awareness of what's deferred: ProxyAdmin multisig/timelock, testnet migration script, dincli ABI updates, updateCoordinator() escape hatch, negative upgrade tests.

---

3. Required Fix Before Merge

Pre-existing DinValidatorStake.test.ts is broken (regression)

Test result: 12 passing, 1 failing — the 1 failure is all 4 tests in the original DinValidatorStake.test.ts (they share a beforeEach that errors before any test body runs).

Root cause: The original test deploys contracts directly via constructor:

// test/DinValidatorStake.test.ts line 22-31
const DinCoordinator = await ethers.getContractFactory("DinCoordinator");
dinCoordinator = await DinCoordinator.deploy();           // <-- no-arg constructor

dinValidatorStake = await DinValidatorStake.deploy(
    dinTokenAddress, dinCoordinatorAddress               // <-- 2-arg constructor
);

Both constructors now call _disableInitializers(), so direct .deploy() calls fail with "incorrect number of arguments to constructor."

Fix required: Update DinValidatorStake.test.ts to use deployPlatform() from test/helpers/platform.ts (already exists) — the same pattern used in all 4 new upgrade tests. The old test logic (unbonding window, slash-pending-withdrawals, blacklist) is valuable and should be kept; just rewire the setup to proxy deployment.

The PR description incorrectly states "no platform contract tests on develop; this PR adds the first suite" — DinValidatorStake.test.ts existed on develop and was passing. The PR broke it without acknowledging it.

---

4. Should Fix (Pre-Merge or Fast Follow)

daoAdmin removal — unnecessary API break; compatibility shim preferred

This is Abraham's primary concern and the most significant design discussion in the PR.

What happened: DINModelRegistry had a custom daoAdmin storage variable with onlyDAOAdmin modifier, setDAOAdmin(address) setter, and DAOAdminUpdated event. Robbert replaced all of this with standard OwnableUpgradeable (owner() / transferOwnership()).

Why it's a problem: dincli currently calls daoAdmin() on DINModelRegistry. If this branch is deployed to testnet, every DAO-admin CLI command against the registry fails. The conversion also breaks any tooling, off-chain monitors, or scripts that listen for DAOAdminUpdated or poll daoAdmin().

What could have been done instead — compatibility shims:

// Backward-compat shims — delegates to OwnableUpgradeable internals
function daoAdmin() external view returns (address) {
    return owner();
}
function setDAOAdmin(address newAdmin) external onlyOwner {
    transferOwnership(newAdmin);
}

These two view/wrapper functions are cheap, preserve the existing ABI surface, and defer the dincli update to a planned migration window rather than making it a hard prerequisite of this PR. The underlying storage and access model is still cleanly OwnableUpgradeable — the shims are just a facade.

Ask from Robbert: Was the shim option considered and rejected, or just not considered? If the latter, add the two shims above. If it was a deliberate choice to force a clean break, document that decision explicitly in the PR and make the dincli ABI update a hard pre-condition tracked before testnet deployment.

require() string errors in DINModelRegistry.requestModelRegistration

Lines 146–153 use require(condition, "string"):

require(dinValidatorStake.isSlasherContract(taskCoordinator), "Invalid Coordinator");
require(dinValidatorStake.isSlasherContract(taskAuditor), "Invalid Auditor");

The approval-time revalidation (same conditions, lines 192–195) already uses the correct custom errors CoordinatorNoLongerSlasher / AuditorNoLongerSlasher. The inconsistency was inherited from the original code — but since he rewrote the file, these two calls should be converted to match. Also a minor gas issue (string encoding is more expensive than custom errors).

Fix: Replace with if (!...) revert CoordinatorNoLongerSlasher(); / if (!...) revert AuditorNoLongerSlasher();

Missing tests: implementation contracts cannot be initialized directly

Abraham flagged this. _disableInitializers() is present in all 4 constructors, but there are no tests proving it actually prevents direct initialization. A security reviewer expecting test coverage here would not find it.

Add one test per contract, e.g.:

it("blocks direct initialization of the implementation contract", async function () {
    const DinToken = await ethers.getContractFactory("DinToken");
    const impl = await DinToken.deploy();
    await expect(impl.initialize()).to.be.revertedWithCustomError(
        impl, "InvalidInitialization"
    );
});

This is not cosmetic — it demonstrates the security property, not just that the code compiles.

---

5. Minor / Nice-to-Have

Coordinator replacement is intentionally impossible — but undocumented. DinToken.setCoordinator() is one-shot (CoordinatorAlreadySet guard). Once the coordinator address is wired, it cannot be changed. With Transparent Proxy, the coordinator proxy address is stable (you upgrade the implementation, not the address), so this is safe today. But if the coordinator proxy ever needs to be replaced with a new proxy address, the token is bricked for minting. Abraham flagged this as a question. The design is probably correct as-is, but deserves a code comment or a test asserting the revert, so future developers understand it's intentional.

ProxyAdmin is recorded but not committed. The deploy script saves the proxyAdmin address to deployments/${network}.json — so it IS captured. However, the deployments directory is presumably gitignored (no committed artifacts visible on the branch). The ProxyAdmin address is also not deterministic (CREATE opcode, depends on deployer nonce). For testnet, the deployer needs to ensure the deployments/ JSON is kept safe; losing it means reconstructing the ProxyAdmin address from the transaction receipt. Worth noting in the deploy script comments or README.

initialize() visibility inconsistency — ✅ Fixed in commit 4c58299: DinToken.initialize() changed to external, all 4 now consistent.

No __gap storage slots — The 4 contracts don't define uint256[50] private __gap. For leaf contracts not intended as base contracts, this is lower priority. But if a future V2 is structured via inheritance and adds state variables, storage collisions become a risk. Standard OZ guidance recommends __gap in any upgradeable contract. Low priority, one-liner to add.

DinCoordinator imports DinToken.sol concretely — Uses the DinToken type directly rather than an interface. Not a bug, but creates tighter coupling that could complicate future multi-contract versioning.

---

6. Checklist Against Task Spec

Deliverable	Status	Notes
Proxy pattern chosen and justified	✅	Transparent, well-argued
All 4 contracts converted to initializer pattern	✅	All 4 clean
Storage-layout safe	✅	immutables removed, defaults in initialize(), validateUpgrade() run
Hardhat deploy scripts for proxy deploy	✅	deploy-platform.ts with correct wiring order
Hardhat deploy scripts for upgrade flow	✅	upgrade-platform.ts with CONTRACT= env var
Upgrade-safety tests per contract	✅	12 tests, 3 per contract
State persistence tests	✅	All 4
Access control tests	✅	All 4
Storage layout validation	✅	validateUpgrade() in all 4 suites
Write-up of changes and rationale	✅	Thorough PR description
Doc-inaccuracy notes	✅	8 items, all accurate
Pre-existing tests still pass	❌	DinValidatorStake.test.ts broken (4 tests) — not fixed in 4c58299
Tests for _disableInitializers()	❌	Not present — implementation init not tested
initialize() visibility consistent	✅	Fixed in 4c58299
Implementation address tracked post-upgrade	✅	Added in 4c58299
withdrawFees uses safe call pattern	✅	Fixed in 4c58299

---

7. Questions for Robbert

1. Broken test (required): DinValidatorStake.test.ts fails with "incorrect number of arguments to constructor" — did you run the full test suite before submitting? The fix is straightforward (use deployPlatform() helper) but must be in the PR.

2. daoAdmin shim (design discussion): Why remove daoAdmin()/setDAOAdmin() entirely rather than adding thin compatibility wrappers over owner()/transferOwnership()? Was the clean break intentional, or was the shim option not considered? If intentional, the dincli ABI update should be a tracked pre-condition before testnet deployment, not just a "with more time" item.

3. Coordinator replacement (clarification): DinToken.setCoordinator() is one-shot — once the coordinator address is set it can never change. Is this intentional? With Transparent Proxy, coordinator proxy addresses are stable so this is safe today, but it's worth a comment in the code confirming it's a deliberate design constraint.

4. _disableInitializers() tests (gap): _disableInitializers() is present in all 4 constructors — confirmed. But there are no tests proving implementation contracts actually revert on direct initialization. Can you add one per contract? This turns a code-review check into a tested security property.

5. ProxyAdmin recording: The deploy script saves the ProxyAdmin address to deployments/${network}.json. Is that file committed, or gitignored? If gitignored, what's the plan for keeping the address safe for testnet? Losing it means reconstructing from transaction receipts.

6. require() vs custom errors: Were the two require(condition, "string") calls in requestModelRegistration intentionally left, or missed? The rest of the contract uses custom errors; these are inconsistent.

7. Testnet migration plan: The existing Optimism Sepolia deployment has live state (stakers, models). When we migrate to proxy-based contracts, is the plan a clean redeploy (accept state loss for devnet) or a migration script? Doesn't need to be in this PR, but worth aligning on now.

---

8. Merge Recommendation

Hold — three blockers before merge:

1. Fix DinValidatorStake.test.ts regression (§3) — straightforward, shouldn't take long.
2. Resolve the daoAdmin compatibility question (§4) — either add the two-line shim or document the clean-break decision with a hard tracked prerequisite on the dincli side.
3. CLI-level integration tests must exist before this merges to develop.

On point 3: the upgraded contracts change ABIs in ways that are almost certain to break dincli call sites:

• daoAdmin() is gone from DINModelRegistry — any dindao.py path that calls it fails.
• DIN_TOKEN() / DIN_COORDINATOR() on DinValidatorStake are now mutable storage rather than immutable — the getter ABI is the same but the behaviour at a freshly deployed address differs.
• DinCoordinator no longer deploys DinToken in its constructor — any dincli bootstrap flow that assumed dinToken = coordinator.dinToken() after a single deploy needs the new two-step wire.

There is currently no test harness at the dincli layer that exercises these paths against a local chain. Contract-level Hardhat tests prove the Solidity is correct; they say nothing about whether the Python CLI correctly handles the new contract interfaces. Until CLI-level tests exist — even minimal ones covering dindao calls and the token/coordinator bootstrap path against a local Hardhat node — we cannot safely merge this branch without risking a broken devnet CLI for everyone.

What "CLI-level tests" means here: tests in the dincli Python test suite (following the monkeypatch/CliRunner pattern in tests/) that spin up a local chain (or mock the contract layer), deploy the proxy contracts via the new scripts, and assert that the key dincli commands complete without error. This is a prerequisite that belongs on develop before or alongside this PR — it is not Robbert's work to own, but it is a gate on merging his branch.

Once blocker 3 is resolved (and 1 and 2 addressed), add the _disableInitializers() tests from §4 as a fast follow. The core Solidity work — proxy pattern, storage layout, deployment wiring, upgrade tests — is competent and ready to land once the CLI safety net is in place.

[robertocarlous]

Thanks for the thorough review, Umer.
I've pushed a follow-up commit addressing all the code-level items.
Responses to each question below.

Broken test (§3)
Missed this DinValidatorStake.test.ts was not in the diff I was reviewing locally because my fork hadn't synced it from upstream. Fixed: the test file has been rewritten to use deployPlatform() from test/helpers/platform.ts. All 32 tests now pass (12 original upgrade tests + 4 _disableInitializers tests + 16 behavioural staking tests).

daoAdmin shim (§7 Q2)
The shim was not considered the clean break to OwnableUpgradeable was a reflex, not a deliberate decision. I've added both compatibility wrappers so the existing dincli ABI surface is preserved:

function daoAdmin() external view returns (address) { return owner(); }
function setDAOAdmin(address newAdmin) external onlyOwner {
address old = owner();
transferOwnership(newAdmin);
emit DAOAdminUpdated(old, newAdmin);
}
The underlying auth model stays OwnableUpgradeable. DAOAdminUpdated is emitted to keep off-chain indexers intact.

Coordinator replacement (§7 Q3)
Yes, intentional. DinCoordinator sits behind a Transparent Proxy so its address is stable across implementation upgrades setCoordinator() never needs to be called again once the proxy is wired. Added a code comment confirming this constraint.

_disableInitializers() tests (§7 Q4)
Added one per contract in each upgrade test file. Each deploys the implementation directly and asserts Invalid Initialization on a direct initialize() call.

ProxyAdmin recording (§7 Q5)
hardhat/.gitignore has /deployments listed, so the file was being gitignored. I Removed that line deployments/{network}.json will now be committed. It only contains public contract addresses so there is nothing sensitive in it.

require() strings (§7 Q6)
Not intentional missed them during the refactor. Replaced both require(condition, "string") calls in requestModelRegistration with revert CoordinatorNoLongerSlasher() and revert AuditorNoLongerSlasher() to match the approval-time checks.

Testnet migration plan (§7 Q7)
For devnet the plan is a clean redeploy existing state (stakers, models) on the current Optimism Sepolia deployment is test data and can be discarded. A migration script to carry state forward is not needed at this stage. If the team wants to preserve any specific test registrations before the cutover, that can be handled operationally before running the new deploy script.

[umeradl]

Follow-up — Code-Level Blockers Resolved ✅

@robertocarlous — thanks for the fast and complete iteration. Checked commit b32b299 against all review items.

Everything from §3–§5 is now addressed

Item	Status
DinValidatorStake.test.ts regression	✅ Fixed — 16 behavioural tests using deployPlatform()
_disableInitializers() guard tests (all 4 contracts)	✅ Added
require() strings → custom errors in requestModelRegistration	✅ Fixed
daoAdmin() / setDAOAdmin() backward-compat shims	✅ Added — DAOAdminUpdated event preserved
uint256[50] __gap on all 4 contracts	✅ Added
setCoordinator() one-shot constraint documented	✅ Added
/deployments removed from .gitignore	✅ Fixed

Your responses to the review questions were clear and correct — in particular the testnet migration plan (clean redeploy) and the coordinator one-shot reasoning.

---

One remaining gate before merge (team-side, not yours)

The Solidity work is done and ready to land. The single remaining blocker is CLI-level integration tests in the dincli Python test suite — tests that exercise key dincli call paths against a local chain running the new proxy contracts. This is tracked on our side as WP 6.1 and is not work we're asking you to own.

Once that harness exists, this PR merges. We'll circle back when it's in place.

---

Good work on this. The implementation is clean, the decisions are well-reasoned, and the follow-up was thorough. The hold is not on anything you did wrong — it's a gap in our own tooling we need to close before landing any ABI-changing contract PR.

[robertocarlous]

Understood, thanks for the clear sign-off. I'll keep the branch up to date with develop while WP 6.1 is in progress. You can always let me know whenever the CLI harness is ready and I'll rebase if needed.