Skip to content

fix(cli): remove broken non-proxy dinrep deploy, fix add-slasher crash; refresh contract docs (#203) - #204

Merged
umeradl merged 5 commits into
InfiniteZeroFoundation:developfrom
umermjd11:fix/dinrep-legacy-deploy-and-contract-docs
Sep 30, 2026
Merged

umeradl merged 5 commits into
InfiniteZeroFoundation:developfrom
umermjd11:fix/dinrep-legacy-deploy-and-contract-docs

Conversation

@umermjd11

Copy link
Copy Markdown
Collaborator

Summary

Closes #203.

Two dincli fixes, one hardhat cleanup, and the contract docs brought up to date with foundry/src. No foundry contract, ABI or storage layout changes.

dincli (dincli/cli/dinrep.py)

  • Removed the dinrep deploy sub-app (din-coordinator, din-validator-stake, din-model-registry). All three called constructors on proxy-only contracts:

    • din-validator-stake and din-model-registry passed arguments to constructors that take none;
    • din-coordinator deployed an implementation that can never be initialised, then wrote a zero token address into din_info.json.

    Deployment is foundry/script/DeployPlatform.s.sol followed by dincli system import-deployments.

  • dinrep add-slasher no longer crashes without a target. With none of --contract / --taskCoordinator / --taskAuditor it raised UnboundLocalError. It now prints a usage message and exits 1, and loads DinCoordinator only after the target is resolved.

  • Help text fixes for add-slasher and the registry sub-app.

  • New tests/test_dinrep_add_slasher.py (3 tests): no target exits 1 and sends nothing; --contract sends addSlasherContract; dinrep deploy is not a command. The first and third fail on develop.

hardhat

Docs

Doc Change
contracts/DinCoordinator.md withdraw() section removed (the function is gone). Adds sweepFeesToRouter, mintEmission, setMintCap, retireFaucet, ZeroMintAmount, the deploy-script wiring order
contracts/DinValidatorStake.md Partial slashing with S5 escalation, S6, jailing and reactivate, 50% burn / 50% treasury, per-model floors, registration caps, encryption keys
contracts/DINTaskCoordinator.md Restructured. Keeps the seed lock (#191) and T1/T2 commit-reveal (#197); adds the dispute flow and dispute seed, GIStateChanged, reward weights, treasury forwarding through slashTreasury(), a state table with current ordinals
contracts/DINTaskAuditor.md Reward pools and claims, test-data disputes, S1/S3 slashing, createAuditorsBatches(gi, seed), treasury forwarding
contracts/DINShared.md slashTreasury() on IDinValidatorStake; the dispute-seed errors and the 12 T1/T2 commit-reveal errors, none of which were listed
contracts/DinToken.md burn() / TokensBurned, one-shot setCoordinator
mechanisms/staking-mechanism.md Jailing is reachable; slashes are burned/forwarded, not kept
storage_layout.md OpenZeppelin v5 bases are ERC-7201 namespaced, not sequential slots
testing/dincli-testing-guide.md Foundry is the default toolchain
public/roles/dinrep.md Deployment section rewritten for local anvil and Optimism Sepolia; explore-request documented
hardhat docs, decision record, Developer/ notes Hardhat marked as the secondary toolchain; references to the removed commands and shims updated

Known issues recorded in the docs, not fixed here:

Verification

Run on this branch (dc8cb77, based on develop @ e373c8d):

Check Result
pytest -m "not integration" -q 385 passed
forge build && forge test 476 passed, 0 failed
npx hardhat compile && npx hardhat test 32 passing
python .github/scripts/check_doc_links.py Documentation all 257 relative links resolve
Every error in foundry/src/DINShared.sol appears in DINShared.md yes (scripted check)
Identifiers named in the six contract docs exist in foundry/src yes (scripted check; only inherited OpenZeppelin names are outside src)

git grep -nE "dinrep deploy|setDAOAdmin|daoAdmin\(" over dincli tests hardhat Documentation returns only the notes about the planned native deploy and the change-log lines recording the shim removal.

🤖 Generated with Claude Code

umermjd11 and others added 5 commits September 30, 2026 18:01
…ash (InfiniteZeroFoundation#203)

The three `dinrep deploy` commands called constructors on contracts that
are proxy-only: din-validator-stake and din-model-registry passed
arguments to constructors that take none, and din-coordinator deployed an
implementation that can never be initialised. Platform deployment goes
through foundry/script/DeployPlatform.s.sol and
`dincli system import-deployments`.

`dinrep add-slasher` with none of --contract / --taskCoordinator /
--taskAuditor raised UnboundLocalError. It now exits 1 with a usage
message, and loads DinCoordinator only after the target is resolved.

Also fixes the add-slasher and registry help texts.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…egistry (InfiniteZeroFoundation#203)

The shims existed for dincli's `set-dao-admin`, which PR No. 183 removed.
Nothing in dincli, the tests or the hardhat scripts calls them, and the
foundry contract has no such functions. The hardhat docs now describe the
contract without them and mark hardhat as the secondary toolchain.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…h foundry/src (InfiniteZeroFoundation#203)

Contract docs (DinCoordinator, DinToken, DinValidatorStake,
DINTaskCoordinator, DINTaskAuditor, DINShared) rewritten against the
current source:
- DinCoordinator: withdraw() is gone; documents sweepFeesToRouter,
  mintEmission, the mint cap and faucet retirement.
- DinValidatorStake: partial slashing with S5 escalation, S6, jailing,
  50% burn / 50% treasury, per-model floors and registration caps.
- DINTaskCoordinator / DINTaskAuditor: reward pools and claims, dispute
  flows and the dispute seed, treasury forwarding via slashTreasury(),
  GIStateChanged, alongside the batch-assignment seed lock (PR No. 191)
  and T1/T2 commit-reveal (PR No. 197).
- DINShared: slashTreasury() on IDinValidatorStake, the dispute-seed
  errors and the 12 T1/T2 commit-reveal errors.

Also:
- staking-mechanism.md and storage_layout.md (ERC-7201 namespaced bases).
- dincli-testing-guide.md: Foundry is the default toolchain.
- roles/dinrep.md: deployment via DeployPlatform.s.sol and
  import-deployments for local anvil and Optimism Sepolia, replacing the
  removed `dinrep deploy` commands.
- Decision record, backlog and roadmap references updated.

Open caveats recorded, not fixed: DINTaskCoordinator is 24,585 bytes at
runtime, over the EIP-170 limit, and committed-but-unrevealed is slashed
as a liveness miss (issue No. 201); the dincli auditor commit retry and
aggregate-t2 batch id bugs (issue No. 202).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…Foundation#203)

setDAOAdmin() was its only emitter and went with the shims. The hardhat
docs no longer say the declaration remains.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- DinValidatorStake.md, staking-mechanism.md: every mention of the 50/50
  slash split now says the treasury half is also burned when no treasury
  is set.
- ROADMAP P4-IDX2: refer to list_pending_requests by name, not by a line
  range that shifts.
- dincli-native-proxy-deployment.md: "Interim state" describes the
  Foundry flow (DeployPlatform.s.sol, then import-deployments) as the
  current one, with the Hardhat script as the secondary path.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@umermjd11

Copy link
Copy Markdown
Collaborator Author

Issue No. 203 review amendments

Applied the amendments from the approval review. Three were already met by the first push (dc8cb77); the other four are in 2f04896 and 82abbcd.

# Amendment Status
1 Keep get_contract_instance; only time, load_din_info, save_din_info become unused Already met. dinrep.py keeps the import and drops only those three
2 Remove DAOAdminUpdated with the shims Done in 2f04896. The event is removed from hardhat/contracts/DINModelRegistry.sol, and the three hardhat docs (both DINModelRegistry.upgrade.test.md paths and upgradable-contracts/hardhat/README.md) no longer say the declaration remains
3 Slash split: the treasury half is also burned if no treasury is set Done in 82abbcd. The main statements already said so; the four short mentions now do too (DinValidatorStake.md overview and change log, staking-mechanism.md contract summary and slashing flow)
4 storage_layout.md: one ERC-7201 note, DinToken.coordinator as slot 0, other slot numbers left alone Met, with one difference (below)
5 ROADMAP P4-IDX2: name list_pending_requests, no line range Done in 82abbcd
6 dincli-native-proxy-deployment.md: new commands, and "Interim state" on the Foundry flow Done in 82abbcd. "Interim state" now gives DeployPlatform.s.sol then import-deployments (--foundry default) as the current flow, with the Hardhat script as the secondary path. The "new commands" bullet was already in dc8cb77
7 Test pins that get_deployed_din_coordinator_contract is never called in the no-flag case Already met. test_add_slasher_without_target_exits_before_loading_coordinator asserts coordinator_loads == 0

On amendment 4: no slot number changed for any contract, and DinToken gets coordinator slot 0 / __gap slots 1–50 (checked with forge inspect DinToken storageLayout). Beyond the one note, each contract's base-contract rows are collapsed from

[Initializable]
  _initialized      : uint64
[OwnableUpgradeable]
  _owner            : address

to [Initializable] [OwnableUpgradeable] (ERC-7201 namespaced). Listing _owner : address above slot 0 is what made those rows read as sequential slots, so a note alone would have left the blocks contradicting it. If you'd prefer the original rows kept with only the note added, I'll revert that part.

Re-run after these commits: npx hardhat compile and npx hardhat test (32 passing), the doc link check (257 links resolve), and the dinrep unit tests (20 passed). No foundry or Python source changed since dc8cb77.

@umeradl

umeradl commented Sep 30, 2026

Copy link
Copy Markdown
Member

Deep verification — PR No. 204 (issue No. 203)

Reviewed against develop in an isolated worktree at PR head 82abbcd (5 commits). The branch sits directly on the current develop tip, e373c8d, which is also the merge-base; develop has not moved since the branch was cut. No conflicts: git merge-tree --write-tree exits 0, and GitHub reports mergeable: MERGEABLE, mergeStateStatus: CLEAN. All 4 CI checks are green (Solidity, Python, Docs, CI OK). I ran every checkable claim, read all six rewritten contract docs line by line against foundry/src, and went through every comment on the PR and on issue No. 203, including the approval review and its 7 amendments.

dincli

Claimed: dinrep deploy is removed; add-slasher with no target exits 1 without sending anything and loads DinCoordinator only after resolving the target; the help text is fixed; 2 of the 3 new tests fail on develop.

Verified, holds:

  • Break-then-fix: with develop's dinrep.py swapped into the PR tree, tests/test_dinrep_add_slasher.py gives 2 failed, 1 passed. The failures are the real UnboundLocalError: cannot access local variable 'contract_address' at dinrep.py:243, and dinrep deploy still existing. With the PR's file restored: 3 passed.
  • Real CLI under an isolated HOME: dincli dinrep --help lists only add-slasher / registry / coordinator. dinrep deploy --help exits 2 with No such command 'deploy'. The new add-slasher help reads correctly, and the registry help now names DINModelRegistry.
  • Imports: time, load_din_info and save_din_info are gone. get_contract_instance stays, and set-fees still uses it at dinrep.py:305 (amendment 1).
  • Nothing else calls the removed commands. The repo-wide git grep hits for dinrep deploy are the future-native-deploy notes (system.py:994, tests/dincli/NOTES.md:19, test_01_platform.py:24-26, the issue doc) plus the doc lines recording the removal. The local wiki clone has no dinrep deploy either.
  • pytest -m "not integration" -q: 385 passed, 133 deselected, run from the worktree's own dincli (not the main checkout's editable install).

hardhat

Claimed: daoAdmin(), setDAOAdmin() and DAOAdminUpdated are gone, and nothing referenced them.

Verified, holds: the grep over dincli tests hardhat foundry finds no callers. The remaining mentions are historical change-log lines and Developer/issues/upgradable-contracts.md (history). Amendment 2's second path, contracts/hardhat/tests/DINModelRegistry.upgrade.test.md:75, is updated too. npx hardhat compile && npx hardhat test: compiled 30 files, 32 passing.

Docs: every contract claim checked against foundry/src

  • DINShared.md: a script extracted all 140 error declarations from DINShared.sol, and 0 are missing from the doc. The IDinValidatorStake interface block matches the source's 14 functions in order.
  • Identifier check on the six contract docs: every backticked identifier resolves in foundry/src, except inherited OpenZeppelin names, test-contract names (all exist in foundry/test/DeployPlatform.t.sol), DeployPlatform env keys, and names quoted only in change logs as removed (withdraw, setAuditScorenEligibility, OWNER, owner_).
  • GIstates ordinals in the DINTaskCoordinator.md §6 table (4/5 … 25, plus 18 and 21 in the change log) match the enum exactly.
  • Function bodies read against the doc:
    • DinCoordinator: every error, event, guard, the nonReentrant set, and the 95/5 router ETH split.
    • DinValidatorStake: _applySlash 50/50 with burn fallback, slashPartial S5 ring and escalation, the S6 formula, _jailInternal, unblacklist → Jailed/None, reactivate guards, and all defaults.
    • DINTaskCoordinator: _startGI pool gate; registerDINaggregator check order; commit/reveal guards and the sender-bound hash; finalize tie-break, quorum and weight-for-every-assigned-aggregator; slashAggregators S2 vs bad consensus; setTier2Score states; releaseGIRegistrationSlots string requires; all dispute paths and the _burnAndForward split.
    • DINTaskAuditor: all Params and setter defaults, and the resolveTestDataDispute / closeExpiredDispute behavior.
    • DeployPlatform.s.sol steps 1–16 as cited in three docs.
  • Staking caveats checked against source: DinValidatorStake.md §13 No. 2 is real: slashPartial escalation calls _jailInternal, which reverts on Blacklisted after _applySlash, so the whole slash reverts. No. 3 is also real: _syncValidatorStatus leaves Jailed once jailedUntil passes, so reactivate() isn't the only way out.
  • Size figure (24,585 B, 9 over EIP-170) is issue No. 201's own measurement. foundry/anvil.sh does pass --code-size-limit 4294967295.
  • storage_layout.md: DinToken inherits Initializable, ERC20Upgradeable, OwnableUpgradeable (the new bracket order matches). forge inspect DinToken storageLayout: coordinator slot 0, __gap slots 1–50, as documented.
  • dincli-testing-guide.md: PLATFORM_DEPLOY_TOOLCHAIN defaults to "foundry" (constants.py:110). The conftest always runs npx hardhat compile and adds forge build for foundry, and the log names (forge_build.log, anvil_node.log, hardhat_node.log) match conftest.py:102-234. anvil.sh has --accounts 70 --chain-id 1337.
  • roles/dinrep.md deploy flow: the chain-id → file mapping (1337/31337 → localhost, 11155420 → sepolia_op_devnet, anything else reverts unless DEPLOYMENTS_NETWORK is set) matches DeploymentsPath.sol. localhost.json is gitignored (foundry/.gitignore:15) and sepolia_op_devnet.json is not. The optimism-sepolia alias exists in foundry.toml. SEPOLIA_OP_DEVNET_RPC_URL is in .env.example. import-deployments has --file / --foundry (default) / --hardhat. list-pending-requests -t and explore-request -t <type> <id> match the Typer signatures. Both disable-model claims hold: approveManifestUpdate re-checks modelDisabled, and no task contract reads it. Ran §1a end to end on a separate anvil (port 8546, same anvil.sh flags): forge script … --unlocked completed and wrote foundry/deployments/localhost.json (7 proxies + 7 ProxyAdmins), and dincli --network local system import-deployments imported all of them. din_info.json was restored afterwards.
  • python .github/scripts/check_doc_links.py Documentation: 257 links resolve. The same check on Developer: 237 links resolve.

Foundry

forge build and forge test on the real via_ir = true profile, after forge clean: 476 passed, 0 failed (39 suites), matching the PR's count. No foundry/src or foundry/test file changes in this PR.

Issue No. 203 amendments 1–7

All 7 hold at 82abbcd: No. 1 and No. 2 are covered above. No. 3: the four short slash-split mentions now say "or burned if no treasury is set". No. 4: one ERC-7201 note plus coordinator slot 0. No. 5: ROADMAP names list_pending_requests. No. 6: the issue doc says "new" commands and its "Interim state" section shows the Foundry flow. No. 7: coordinator_loads == 0 is asserted.

On the question in the amendments comment, collapsing the [Initializable] / [OwnableUpgradeable] rows: I'd keep the collapse. The listed _owner : address rows above slot 0 were exactly what made them read as sequential slots, and no slot number changed.


Findings

No blocker. The code change is correct and tested, and the docs are accurate against foundry/src apart from the small items below.

  • No. 1 (nit, dinrep.py): the no-target check still runs after the wallet unlock. add_slasher calls ctx.obj.get_en_w3_account_console() first, and that loads and decrypts the wallet. With an encrypted keystore, dincli dinrep add-slasher with no flags prompts for the password and only then prints "No slasher contract given". Checking the three flags before loading the account, using ctx.obj.console for the message, would make the usage error immediate. Optional.
  • No. 2 (nit, DINTaskAuditor.md §3.3): the setS1SlashFractionBps bound is given as "(≤ 10 000)". The setter also rejects 0 (bps == 0 || bps > 10_000), so it should read "(1 – 10 000)", matching the coordinator doc's S2 row.
  • No. 3 (low, roles/dinrep.md §1b): the Optimism Sepolia deploy never mentions that DeployPlatform.s.sol reads 11 tokenomics env keys (DIN_PER_ETH, MINT_CAP, EMISSION_*, MIN_STAKE, S5_*, S6_*) and falls back to defaults: MINT_CAP = 0 (uncapped), 10 DIN MIN_STAKE, and so on. The guide's source .env.sepolia_op_devnet would pick them up, but nothing tells the operator to set them. With the testnet values still open in issue No. 155, one sentence linking DeployPlatform.md and the --- Effective tokenomics --- log block would prevent an accidental default-tokenomics deploy.
  • No. 4 (nit, roles/dinrep.md): "Unlike localhost.json, this file is committed". The file doesn't exist yet. Something like "not gitignored; commit it" says what the operator should do. In the same section, "A full deploy is roughly 30 transactions" should say 22. The local broadcast is 7 implementations + 7 proxies + 8 wiring calls, because OZ v5 creates each ProxyAdmin inside the proxy constructor.
  • No. 5 (nit, proxy-deployment-architecture.md): the banner is dated "Status update (2026-09-25)", but the change it describes (commands removed) lands with this PR.
  • No. 6 (observation, DinValidatorStake.md): the rewrite drops the seven worked scenarios and the onboarding/exit walkthroughs. What remains is accurate. Say if you want them back; they'd need the 50/50 slash disposition added.

Surfaced during review: not PR defects, and not tracked anywhere yet

These are in foundry/src, outside this PR's scope (no contract changes). Neither is in issues No. 192, No. 201 or No. 202, the foundry/src security review baseline, or develop's docs. I proved both with a throwaway forge test that extends EncryptedTestDataTest. The test is deleted and was not committed.

  • A. Anyone can drain a GI reward pool through test-data disputes, for free. DINTaskAuditor.md §13 No. 1 now records this as a caveat, but there is no issue for it. openTestDataDispute and resolveTestDataDispute are unauthenticated, disputeBondAmount defaults to 0, and any commitment mismatch upholds the dispute. The probe test_probe_outsiderGriefsRewardPool PASSED. An outsider with no DIN, not a batch auditor, opens a dispute and resolves it with a junk K. The owner reassigns, and the outsider repeats. After 3 rounds giRewardPool(1) is below half its starting value (0.75³ ≈ 42%): 25% per round is burned or forwarded to the treasury, and the outsider spends nothing. It also blocks the batch (pendingReassignment) each round. Suggest opening an issue.
  • B. registerDINaggregator has no onlyCurrentGI, unlike registerDINAuditor. During GI N's registration window a validator can register for GI N+k, which puts them in dinAggregators[N+k] before that GI exists and increments their active-registration count. The probe test_probe_aggregatorPreRegistersFutureGI PASSED: during GI 1, registerDINaggregator(2) succeeds and isDINAggregator(2, v) is true. DINTaskCoordinator.md §6.2 describes this behavior neutrally but doesn't list it in §10. Suggest an issue and a caveat line.

Out of repo: the public wiki's DIN-Representative page is still pre-refresh. It uses dincli dindao add-slasher / dindao registry … (the CLI group is dinrep), a set-admin command, a link to a roles/dindao.md that doesn't exist, "treasury withdrawals", and "Cannot mint DIN at will — bound to depositAndMint" (mintEmission exists now). It's worth a follow-up wiki edit after this merges.

Recommendation: mergeable as-is. No. 1–5 are small enough to fold into the merge. A and B should become issues rather than being fixed in this PR.

@umeradl

umeradl commented Sep 30, 2026 •

Copy link
Copy Markdown
Member

Files changed (23), as of 82abbcd (PR head)

Diffed against merge-base e373c8d, which is also the current develop tip. develop has not moved since the branch was cut, so none of these files differ from develop except through this PR. GitHub agrees: mergeable: MERGEABLE, mergeStateStatus: CLEAN. Confirmed with a local git merge-tree --write-tree dry run: clean, exit 0.

dincli/cli/dinrep.py

Field Value
Change Modified
Lines +14/-146
Diff (what exactly is in this PR) Deletes deploy_app and its three commands (din-coordinator, din-validator-stake, din-model-registry) and the time / load_din_info / save_din_info imports. add_slasher gains an else: that prints "No slasher contract given" and exits 1, and the get_deployed_din_coordinator_contract() call moves below target resolution. The add-slasher and registry help strings are rewritten.
Functionality — how & why How: dinrep now registers only the registry and coordinator sub-apps plus add-slasher. In add_slasher, --contract / --taskCoordinator / --taskAuditor resolve contract_address. With none of them it exits 1 before any contract load or tx. Otherwise it loads DinCoordinator and sends addSlasherContract. Why: the three deploy commands send constructors that don't exist on the proxy-era contracts. Two are rejected by web3. din-coordinator "succeeds", then overwrites din_info.json with an uninitialisable implementation and a zero token address. add-slasher with no flag hit UnboundLocalError at :243.
Diff vs current develop HEAD None: untouched on develop since the merge-base
Recommended merge proposal Merge as-is. Verified: break-then-fix (2 of the 3 new tests fail on develop's file, 3/3 pass on the PR's), real CLI under an isolated HOME (No such command 'deploy', exit 2), full pytest 385 passed. Optional No. 1 from the verification comment: check the flags before get_en_w3_account_console(), so an encrypted-wallet user isn't asked for a password before the usage error.
Actual merge proposal Applied the PR diff plus the No. 1 fix: the no-target check now runs before get_en_w3_account_console() (message via ctx.obj.console, exit 1), so an encrypted-wallet user gets the usage error with no password prompt. The PR's else: branch became unreachable and was removed; DinCoordinator still loads only after target resolution. Evidence on the final merged tree: forge clean && forge build (via_ir) clean, forge test 476 passed / 0 failed / 0 skipped; pytest -m "not integration" (empty HOME) 385 passed / 0 failed, 133 deselected; npx hardhat compile && npx hardhat test 32 passing; check_doc_links.py 258 (Documentation) + 237 (Developer) links resolve. No build/test fixes were needed. Uncommitted in the develop cwd: not committed or pushed.
Pending proposal Optional verification No. 1 (flag check before wallet unlock) Done: flag check moved ahead of the wallet unlock, pinned by the account_loads == 0 assertion in tests/test_dinrep_add_slasher.py
Local merge conflict No
GitHub merge conflict No

tests/test_dinrep_add_slasher.py

Field Value
Change New
Lines +81/-0
Diff (what exactly is in this PR) 3 tests with a DummyContextObj that counts coordinator loads and a monkeypatched build_and_send_tx: no target → typer.Exit(1), nothing sent, coordinator_loads == 0, message printed; --contract → one addSlasherContract(SLASHER) send; dinrep deploy --help via the real app → No such command.
Functionality — how & why How: calls dinrep.add_slasher directly with explicit keyword arguments, so it runs without Typer's option parsing or a real network, and runs main_app through CliRunner for the sub-app check. Why: pins both fixes (issue amendment 7 in particular: the coordinator must not load in the no-flag case), so a revert of either change fails CI.
Diff vs current develop HEAD None: file doesn't exist on develop
Recommended merge proposal Merge as-is. Break-then-fix confirmed: 2 failed / 1 passed against develop's dinrep.py, 3 passed against the PR's.
Actual merge proposal Copied from the PR, plus (approved) DummyContextObj now counts get_en_w3_account_console() calls: the no-target test asserts account_loads == 0 (fails against the PR's original dinrep.py, so it pins the No. 1 fix) and the --contract test asserts account_loads == 1. 3 passed. Evidence on the final merged tree: forge clean && forge build (via_ir) clean, forge test 476 passed / 0 failed / 0 skipped; pytest -m "not integration" (empty HOME) 385 passed / 0 failed, 133 deselected; npx hardhat compile && npx hardhat test 32 passing; check_doc_links.py 258 (Documentation) + 237 (Developer) links resolve. No build/test fixes were needed. Uncommitted in the develop cwd: not committed or pushed.
Pending proposal None
Local merge conflict No
GitHub merge conflict No

hardhat/contracts/DINModelRegistry.sol

Field Value
Change Modified
Lines +0/-19
Diff (what exactly is in this PR) Removes daoAdmin(), setDAOAdmin() and the DAOAdminUpdated event.
Functionality — how & why How: admin transfer is now only the inherited transferOwnership, matching the canonical foundry/src/DINModelRegistry.sol, which never had the shims. Why: the shims existed only for dincli's set-dao-admin, which PR No. 183 removed. There are no callers in dincli, tests, hardhat/test, hardhat/scripts or foundry.
Diff vs current develop HEAD None
Recommended merge proposal Merge as-is. npx hardhat compile (30 files) and npx hardhat test: 32 passing.
Actual merge proposal Applied as-is (daoAdmin(), setDAOAdmin(), DAOAdminUpdated removed; git grep finds no remaining callers). Evidence on the final merged tree: forge clean && forge build (via_ir) clean, forge test 476 passed / 0 failed / 0 skipped; pytest -m "not integration" (empty HOME) 385 passed / 0 failed, 133 deselected; npx hardhat compile && npx hardhat test 32 passing; check_doc_links.py 258 (Documentation) + 237 (Developer) links resolve. No build/test fixes were needed. Uncommitted in the develop cwd: not committed or pushed.
Pending proposal None
Local merge conflict No
GitHub merge conflict No

Contract docs: Documentation/technical/contracts/DinCoordinator.md, DinToken.md, DinValidatorStake.md, DINTaskCoordinator.md, DINTaskAuditor.md, DINShared.md

Field Value
Change Modified (×6)
Lines +148/-125, +60/-21, +170/-518, +220/-386, +139/-280, +122/-34
Diff (what exactly is in this PR) Rewrites all six against foundry/src. DinCoordinator: withdraw() removed; the fee-router sweep, mint cap, faucet retirement and emission minting added. DinToken: burn. DinValidatorStake: 50/50 slash disposition, slashPartial / S5, S6, jail, reactivate, per-model floors, caps, encryption keys. DINTaskCoordinator: restructured, keeps the seed lock and T1/T2 commit-reveal, adds disputes, weights and the state table. DINTaskAuditor: rewards, test-data disputes, S1/S3. DINShared: missing interface methods and errors.
Functionality — how & why How: each doc now follows the actual function bodies: guards, defaults, events, errors, and the deploy-script step numbers. Each ends with a caveats section listing real gaps (EIP-170 overage, selective non-reveal, per-model recidivism, blacklisted S5 revert, unauthenticated test-data dispute resolution, dincli constructor lag, and more). Why: the old docs described removed functions (withdraw), claimed "no burn" and "nothing sets Jailed", and omitted whole mechanisms, so anyone reading them had a wrong model of the contracts.
Diff vs current develop HEAD None
Recommended merge proposal Merge, with verification No. 2 (DINTaskAuditor.md §3.3: S1 bound "1 – 10 000") folded in. Verified by script: 140/140 DINShared.sol errors present, and every identifier resolves in foundry/src apart from the expected exceptions (OpenZeppelin names, test contracts, env keys, removed names in change logs). Verified by reading every body cited; details in the verification comment. No. 6 (dropped scenarios in DinValidatorStake.md) is your call.
Actual merge proposal Applied the PR diff for all six, plus: No. 2 (DINTaskAuditor.md §3.3 S1 bound → "1 – 10 000"); DINTaskAuditor.md §13 No. 1 now notes the zero default bond / repeatability and links issue No. 205; new DINTaskCoordinator.md §10 No. 11 for the missing onlyCurrentGI on registerDINaggregator, linking issue No. 206; No. 6 (approved): new DinValidatorStake.md §11 "Workflows & Scenarios" restoring onboarding/exit and Scenarios 1–7, updated for the current contract (50/50 burn/treasury split, slashable while blacklisted, per-call MIN_STAKE), plus a new Scenario 8 (S5 escalation, jail, reactivate). Later sections renumbered 12–15 and internal refs updated. Evidence on the final merged tree: forge clean && forge build (via_ir) clean, forge test 476 passed / 0 failed / 0 skipped; pytest -m "not integration" (empty HOME) 385 passed / 0 failed, 133 deselected; npx hardhat compile && npx hardhat test 32 passing; check_doc_links.py 258 (Documentation) + 237 (Developer) links resolve. No build/test fixes were needed. Uncommitted in the develop cwd: not committed or pushed.
Pending proposal No. 2 (one-line fix); No. 6 (keep or restore the scenarios); surfaced A/B: open issues, and consider a DINTaskCoordinator.md §10 caveat for B Done: No. 2 fixed; scenarios restored and updated (§11); A → issue No. 205 and B → issue No. 206, both referenced from the caveat sections (new §10 No. 11 for B)
Local merge conflict No
GitHub merge conflict No

Documentation/public/roles/dinrep.md

Field Value
Change Modified
Lines +110/-22
Diff (what exactly is in this PR) Replaces the three dinrep deploy sections with §1a (local anvil, --unlocked) and §1b (Optimism Sepolia, cast wallet import keystore, optional --verify), plus import options. Documents explore-request. Corrects the kill-switch text: task contracts don't read modelDisabled. Rewrites the workflow step 1.
Functionality — how & why How: it documents the real flow, DeployPlatform.s.sol → foundry/deployments/<network>.json → dincli system import-deployments, with the chain-id → file mapping from DeploymentsPath.sol. Why: the public DIN-Representative guide told operators to run commands that cannot deploy the proxied platform.
Diff vs current develop HEAD None
Recommended merge proposal Merge, with verification No. 3 (one sentence plus a link on the tokenomics env keys for §1b, relevant while issue No. 155 is open) and No. 4 ("not gitignored; commit it") folded in. Every flag, path and command was checked against DeploymentsPath.sol, foundry.toml, .gitignore, .env.example and the Typer signatures.
Actual merge proposal Applied the PR diff plus No. 3 (new §1b "Tokenomics parameters" prerequisite: the 11 env keys DeployPlatform.s.sol reads, their fallback to in-code defaults, where to set them, the --- Effective tokenomics --- check, and a link to DeployPlatform.md; no values stated while issue No. 155 is open) and No. 4 ("roughly 30" → "22 transactions: seven implementations, seven proxies, and eight wiring calls"; "committed" → "not gitignored: commit it"). Evidence on the final merged tree: forge clean && forge build (via_ir) clean, forge test 476 passed / 0 failed / 0 skipped; pytest -m "not integration" (empty HOME) 385 passed / 0 failed, 133 deselected; npx hardhat compile && npx hardhat test 32 passing; check_doc_links.py 258 (Documentation) + 237 (Developer) links resolve. No build/test fixes were needed. Uncommitted in the develop cwd: not committed or pushed.
Pending proposal No. 3, No. 4 Done: tokenomics-env prerequisite added; transaction count and commit wording corrected
Local merge conflict No
GitHub merge conflict No

Documentation/technical/mechanisms/staking-mechanism.md, Documentation/technical/storage_layout.md, Documentation/technical/testing/dincli-testing-guide.md

Field Value
Change Modified (×3)
Lines +31/-22, +20/-45, +35/-18
Diff (what exactly is in this PR) Staking: state diagram gains Jailed; the behavior table covers S1/S2/S5/S6, full-severity faults and the 50/50 disposition. Storage: one ERC-7201 note, bracket rows collapsed, DinToken.coordinator slot 0 / __gap 1–50. Testing guide: foundry is the default toolchain (Anvil + DeployPlatform.s.sol), plus an EIP-170 warning about anvil.sh's code-size override and the new log names.
Functionality — how & why How / why: each fixes a statement the issue listed as wrong on develop ("no function sets Jailed", "slashed DIN remains inside the contract", OZ bases as sequential slots, "Hardhat-only harness").
Diff vs current develop HEAD None
Recommended merge proposal Merge as-is. Slot numbers are unchanged apart from DinToken's new ones (forge inspect DinToken storageLayout: coordinator slot 0, __gap slots 1–50, as documented). Testing-guide claims match conftest.py / constants.py. On the amendments-comment question: keep the collapsed bracket rows.
Actual merge proposal Applied as-is (collapsed OZ bracket rows kept in storage_layout.md). Evidence on the final merged tree: forge clean && forge build (via_ir) clean, forge test 476 passed / 0 failed / 0 skipped; pytest -m "not integration" (empty HOME) 385 passed / 0 failed, 133 deselected; npx hardhat compile && npx hardhat test 32 passing; check_doc_links.py 258 (Documentation) + 237 (Developer) links resolve. No build/test fixes were needed. Uncommitted in the develop cwd: not committed or pushed.
Pending proposal None
Local merge conflict No
GitHub merge conflict No

Hardhat docs: Documentation/technical/contracts/hardhat/README.md, contracts/hardhat/tests/DINModelRegistry.upgrade.test.md, upgradable-contracts/hardhat/README.md, upgradable-contracts/hardhat/test/DINModelRegistry.upgrade.test.md

Field Value
Change Modified (×4)
Lines +2/-0, +1/-1, +3/-8, +1/-1
Diff (what exactly is in this PR) Adds "Secondary toolchain" banners to the two READMEs, and replaces the shim descriptions with "removed" notes (issue amendment 2, both paths).
Functionality — how & why How / why: keeps the hardhat docs consistent with the DINModelRegistry.sol removal, and marks hardhat/contracts/ as lagging foundry/src.
Diff vs current develop HEAD None
Recommended merge proposal Merge as-is. The link check passes.
Actual merge proposal Applied as-is. Evidence on the final merged tree: forge clean && forge build (via_ir) clean, forge test 476 passed / 0 failed / 0 skipped; pytest -m "not integration" (empty HOME) 385 passed / 0 failed, 133 deselected; npx hardhat compile && npx hardhat test 32 passing; check_doc_links.py 258 (Documentation) + 237 (Developer) links resolve. No build/test fixes were needed. Uncommitted in the develop cwd: not committed or pushed.
Pending proposal None
Local merge conflict No
GitHub merge conflict No

Documentation/technical/upgradable-contracts/proxy-deployment-architecture.md

Field Value
Change Modified
Lines +7/-0
Diff (what exactly is in this PR) Adds a status-update banner above the 2026-07 decision record: Foundry is the reference with seven proxies, hardhat is kept but lagging, the interim flow is DeployPlatform.s.sol + import-deployments, and the constructor-based dinrep deploy is removed.
Functionality — how & why How / why: a decision record should stay as written, so the banner flags which of its context statements are now stale without rewriting the decision.
Diff vs current develop HEAD None
Recommended merge proposal Merge, with verification No. 5 (banner date) folded in.
Actual merge proposal Applied the PR diff plus No. 5: banner dated "Status update (2026-09-30)", the day the dinrep deploy removal lands in develop (adjust at the real merge commit if that lands later). Evidence on the final merged tree: forge clean && forge build (via_ir) clean, forge test 476 passed / 0 failed / 0 skipped; pytest -m "not integration" (empty HOME) 385 passed / 0 failed, 133 deselected; npx hardhat compile && npx hardhat test 32 passing; check_doc_links.py 258 (Documentation) + 237 (Developer) links resolve. No build/test fixes were needed. Uncommitted in the develop cwd: not committed or pushed.
Pending proposal No. 5 Done: banner date corrected
Local merge conflict No
GitHub merge conflict No

Developer/issues/dincli-native-proxy-deployment.md, Developer/ROADMAP.md, Developer/BACK_LOG.md, Developer/design/din-architecture.md, Documentation/public/workflows/din-workflow.md

Field Value
Change Modified (×5)
Lines +19/-12, +1/-1, +1/-1, +5/-5, +1/-1
Diff (what exactly is in this PR) Issue doc: "Interim state" now shows the Foundry flow (with Hardhat as the secondary path), and the removed commands become "new" commands (amendment 6). ROADMAP P4-IDX2 names list_pending_requests instead of a line range (amendment 5). BL-26 points at DINTaskCoordinator.md §6.3, the section's new number. Architecture doc: "DIN DAO" → "DIN-DAO" (×5). din-workflow: "a entity" → "an entity".
Functionality — how & why How / why: cross-references that the code and doc changes above would otherwise leave dangling or wrong. The §6.3 anchor exists (DINTaskCoordinator.md:158).
Diff vs current develop HEAD None
Recommended merge proposal Merge as-is. The link check on Developer passes (237 links).
Actual merge proposal Applied as-is. Evidence on the final merged tree: forge clean && forge build (via_ir) clean, forge test 476 passed / 0 failed / 0 skipped; pytest -m "not integration" (empty HOME) 385 passed / 0 failed, 133 deselected; npx hardhat compile && npx hardhat test 32 passing; check_doc_links.py 258 (Documentation) + 237 (Developer) links resolve. No build/test fixes were needed. Uncommitted in the develop cwd: not committed or pushed.
Pending proposal None
Local merge conflict No
GitHub merge conflict No

Verification

Full details are in the verification comment above. In summary:

  • pytest -m "not integration": 385 passed.
  • forge build / forge test (via_ir, clean): 476 passed, 0 failed (39 suites), matching the PR's count.
  • npx hardhat compile / test: 32 passing.
  • Doc link check: 257 (Documentation) + 237 (Developer) resolve.
  • Break-then-fix on the new tests.
  • All 7 issue-No.-203 amendments met.

Local vs. GitHub agree: both clean, with no conflicts.

@umeradl

umeradl commented Sep 30, 2026

Copy link
Copy Markdown
Member

Actual outcome: PR No. 204 merged, review follow-ups applied (pushed)

This supersedes the predictions in the merge-proposal comment above with what actually landed.

Two commits, both on origin/develop:

  1. 13a13cc: a real --no-ff merge of this PR's head 82abbcd into develop @ e373c8d, authored by umermjd11, merge commit rather than squash. 0 conflicts, as predicted; develop hadn't moved since the branch was cut. The merge tree is byte-identical to the PR head. GitHub agrees: the PR shows MERGED.
  2. 00c66e0: the review follow-ups, in a separate commit (same pattern as PR No. 197).

Files unchanged from the PR (16 of 23)

hardhat/contracts/DINModelRegistry.sol, DinCoordinator.md, DinToken.md, DINShared.md, staking-mechanism.md, storage_layout.md, dincli-testing-guide.md, the four hardhat docs, Developer/issues/dincli-native-proxy-deployment.md, Developer/ROADMAP.md, Developer/BACK_LOG.md, Developer/design/din-architecture.md, Documentation/public/workflows/din-workflow.md.

Files changed by 00c66e0 (7 of 23)

File What changed vs. this PR's merged version
dincli/cli/dinrep.py No. 1: the no-target check runs before get_en_w3_account_console(), so the usage error comes without a wallet password prompt. The unreachable else: branch is removed.
tests/test_dinrep_add_slasher.py No. 1: DummyContextObj counts account loads. The no-target test asserts 0, which fails against the original ordering, and the --contract test asserts 1.
Documentation/technical/contracts/DINTaskAuditor.md No. 2: the S1 fraction bound is "1 – 10 000". §13 No. 1 now notes the zero default bond and links issue No. 205.
Documentation/technical/contracts/DINTaskCoordinator.md New §10 No. 11: registerDINaggregator lacks onlyCurrentGI, linking issue No. 206.
Documentation/technical/contracts/DinValidatorStake.md No. 6: new §11 Workflows & Scenarios. Onboarding/exit and Scenarios 1–7 are restored and updated for the current contract, plus a new Scenario 8 (S5 escalation, jail, reactivate). Later sections are renumbered 12–15.
Documentation/public/roles/dinrep.md No. 3: a tokenomics env prerequisite for the Optimism Sepolia deploy, with no values while issue No. 155 is open. No. 4: "22 transactions", and "not gitignored: commit it".
Documentation/technical/upgradable-contracts/proxy-deployment-architecture.md No. 5: banner dated 2026-09-30.

Why the follow-ups

These aren't develop-drift deviations; nothing moved underneath the branch. They are the review's own findings No. 1–6 (see the verification comment) plus doc pointers to the two contract issues surfaced during review: issue No. 205 (unauthenticated test-data dispute resolution) and issue No. 206 (registerDINaggregator has no onlyCurrentGI). The stale wiki DIN-Representative page is tracked separately in issue No. 207.

Verification

On the pushed tree (00c66e0):

  • forge clean && forge build (via_ir): clean.
  • forge test: 476 passed / 0 failed / 0 skipped.
  • pytest -m "not integration" (empty HOME): 385 passed.
  • npx hardhat compile && npx hardhat test: 32 passing.
  • Doc links: 258 (Documentation) + 237 (Developer) resolve.
  • Pre-push local CI mirror: PRECHECK OK on 00c66e0 (docs, Python, forge build/test on the real via_ir profile, hardhat compile/test).
  • GitHub push run: success (run 36737582116).

Local vs. GitHub agree: no conflicts, as predicted.

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.

2 participants