[Santiagocetran]

Addresses issue #224 based on the decisions in this discussion.

• Both startGI overloads reject unregistered or disabled models, including when their reward pools are already funded.
• Documents registration fees as non-refundable anti-spam costs.
• Documents S6 slashing as parked for testnet, leaving its behavior and storage unchanged.

The auditor checks the registry-assigned model ID through immutable registry wiring. CLI constructor support and bundled ABIs are updated. In-flight rounds can still complete, settle and pay rewards.

Protection requires a newly deployed compatible coordinator/auditor pair wired to the canonical registry. Existing deployments are unchanged. No GI-abort or pool-refund mechanism is added.

Validation:

• Clean full Foundry suite: 524 passed.
• Selected Python tests: 29 passed.
• Contract-size gate passed.
• Bundled ABIs match build artifacts; all 293 documentation links resolve.
• Full Docker/Kubo CLI integration was not run.

PR #261 should land first. The deployment CLI changes overlap, so this branch will need reconciliation after #261 lands. I’ll update it and rerun the relevant checks then.

[umeradl]

Reviewed against develop in an isolated worktree (branch applies cleanly on top of current tip e9424af, which is also the merge-base; 1 commit, no conflicts — confirmed by both git merge-tree --write-tree locally and GitHub's own mergeable: MERGEABLE / mergeStateStatus: BLOCKED, where BLOCKED is only the missing approvals).

CI on 2a9c546: CI OK green — Solidity (full forge test plus the size gate), Python and Docs. The non-required Integration (full GI) job failed at test_01_platform.py::test_deploy_platform_via_script (missing proxyAdmin in foundry/deployments/localhost.json). That's the same assertion it fails on develop itself (push run on e9424af), so it isn't caused by this PR. It does mean CI never reached this PR's deploy CLI, so I reproduced that locally on anvil. Also reproduced locally: a break-then-fix of the new kill-switch tests, the contract sizes, and the bundled-ABI parity.

Claimed: both startGI overloads reject unregistered or disabled models, including with a funded pool, before any mutation.

Verified — correct, and the tests are load-bearing: _startGI checks state and _GI == GI + 1, then calls dinTaskAuditorContract.validateGIStart(_GI) before updatePassScore, GI++ and _setGIstate. The interface declares it view, so it's a STATICCALL and the auditor can't re-enter. The auditor reverse-looks-up its own address in the registry (TA_ModelNotRegistered), checks modelDisabled(registeredId) (TA_ModelDisabled), then the pool (TC_GIRewardPoolNotFunded, same selector as before). Break-then-fix: the 8 new RewardEngine tests all pass on the PR. With the coordinator line swapped back to develop's if (dinTaskAuditorContract.giRewardPool(_GI) == 0) revert TC_GIRewardPoolNotFunded();, 5 fail ("next call did not revert as expected"): disabled-both-overloads, private-funding-after-disable, unregistered, registry-read-failure, and disabled-in-flight. The 3 that still pass test the auditor directly (constructor wiring, onlyTaskCoordinator, wrong-registry). Restored the file afterwards, and the tree is clean.

Claimed: in-flight rounds can still complete, settle and pay rewards.

Verified — correct: test_disabledInFlight_completesSettlesClaims_butCannotStartNext disables during GI 1, runs it to endGI, claimReward and claimRewards (the balance rises by claimable), then shows both overloads revert for GI 2 with a funded pool. The guard appears only in _startGI; no other entry point changed.

Claimed: the auditor checks the registry-assigned ID, not the constructor's anticipated one.

Verified — correct: the fixture deliberately deploys with constructor ID 1 while the registry assigns ID 0. The test asserts getModelIdByTaskAuditor(ta) == (true, 0), ta.modelId() == 1, and TA_ModelDisabled(0).

Claimed: contract-size gate passes.

Verified — locally: forge build --sizes: DINTaskCoordinator runtime 23,387 B (1,189 B margin), and DINTaskAuditor 21,885 B. Putting the registry reads in the auditor, as Abraham suggested, keeps the coordinator nearly flat. The Solidity CI job enforces the gate too.

Claimed: bundled ABIs match the build artifacts.

Verified — exact: comparing dincli/abis/DINTask{Auditor,Coordinator}.json with foundry/out/… as sorted canonical JSON gives equal ABIs (160/160 and 201/201 entries). The coordinator ABI loses TC_GIRewardPoolNotFunded because that error is now raised by the auditor. dincli prints raw revert text and decodes no custom errors, so nothing in the CLI depends on it.

Claimed: the CLI deploy supports the new constructor shapes.

Verified — on a live anvil, with the real Foundry artifacts: I drove deploy.task_coordinator and deploy.task_auditor themselves. Only the stake/registry lookup, .env writes and sleep were stubbed; transactions went to anvil for real.

Case	Result
coordinator, no --model-id	BadParameter: This artifact requires --model-id… (no tx)
coordinator --model-id 7	deployed, modelId() == 7
auditor --model-id 8 vs coordinator 7	BadParameter: --model-id must match the paired coordinator modelId() (no tx)
auditor --model-id 7	deployed; modelRegistry() = din_info registry, modelId() == 7, and the coordinator's dinTaskAuditorContract = auditor (setDINTaskAuditorContract ran)
no registry in din_info	BadParameter: Configure the canonical platform registry… (no tx)
--taskCoordinator pointing at a contract without modelId()	raw ContractLogicError: execution reverted — see observation 2

tests/test_model_disable_deployment.py: 13 passed locally, and green in CI.

Storage / upgrade safety: both task contracts are non-upgradeable. modelRegistry is immutable (bytecode, not storage), and DINModelRegistry.sol changes only a NatSpec comment, so there is no storage-layout impact on the upgradeable proxy. DINShared.sol adds an interface and two errors only.

Docs: the decisions are recorded with a link to Abraham's comment, which I checked is the real comment ID. The 9 doc files consistently say "future GIs only, no abort, redeploy required, review wiring". Links are green in CI Docs.

Observations:

1. The registry wiring is self-attested (the most important point here, but not a blocker). The only constructor check is modelRegistry_.code.length != 0, and DINModelRegistry.approveModel never reads the auditor's modelRegistry(). A model owner who deploys directly instead of through dincli can pass any contract that returns "registered, enabled". The EnabledModelRegistry test stub is exactly such a contract, and the live deploy above accepted it. That silently defeats the kill switch. The docs say this openly ("Immutable wiring … does not prove the supplied registry is canonical. The DIN-Representative must review…"), and arbitrary custom task code is out of scope anyway. Still, this one check is cheap to make mechanical, in either of two ways:

   • dincli (no contract change): have dinrep approve-registration read taskAuditor.modelRegistry() and refuse unless it equals the platform registry, and likewise in the slasher-authorization path.
   • On-chain: approveModel reverts unless IDINTaskAuditor(req.taskAuditor).modelRegistry() == address(this). That's a DINModelRegistry upgrade, and it would reject legacy pairs.

   I'd suggest a follow-up issue with the dincli check first.

2. deploy task-auditor paired with a coordinator that has no modelId() (a legacy one-argument coordinator, or a wrong --taskCoordinator) crashes with a raw ContractLogicError instead of a BadParameter. Wrapping the modelId().call() in try/except and raising BadParameter("--taskCoordinator does not expose modelId(); …") is a three-line fix at merge.

3. Integration harness: tests/dincli/test_02_task_contracts.py runs model-owner deploy task-coordinator/task-auditor without --model-id. It's already broken on develop, where the CLI passed one constructor argument to a two-argument contract. With this PR it fails earlier, with a clear BadParameter. It needs --model-id in the task_061026_21 Part D rework of the suite. Not this PR's job, but noting it so it isn't missed.

4. Merge order with PR No. 261: confirmed. git merge-tree of this branch against No. 261's head conflicts in dincli/cli/modelownerd/deploy.py, since both rewrite the constructor wiring. cache_model_0/abis/ (refreshed by No. 261) also isn't touched here, so the reference model's bundled task ABIs stay stale until No. 261 lands. As the author says: No. 261 first, then rebase this one and re-run.

Not independently re-verified: the full forge test (524 passed per the PR; covered by the green CI Solidity job) and a full GI through dincli + Docker/Kubo (the integration suite is red on develop for an unrelated reason).

---

Every checkable claim held up, and the kill-switch tests demonstrably guard the change. The contract side is mergeable as-is. Before merge: land PR No. 261 first and rebase (the deploy.py conflict), and ideally apply observation 2. Observation 1 should become a follow-up issue rather than block this PR, since the docs already scope the guarantee to reviewed wiring. This touches foundry/src/** and implements a mechanism decision, so per MAINTAINING No. 4.2 it also needs @abrahamnash's sign-off as the second key alongside mine. The PR says "Addresses issue No. 224" (no closing keyword). That's right for now: all three decisions in No. 224 are covered here, so No. 224 can go through din-close-issue after merge, with observation 1 as its follow-up.

[umeradl]

Files changed (31) — as of 2a9c546 (PR head)

Diffed against merge-base e9424af (develop). develop hasn't moved since the branch was cut. GitHub agrees: mergeable: MERGEABLE, mergeStateStatus: BLOCKED (approvals only). Confirmed with a local git merge-tree --write-tree dry run: clean. Against open PR No. 261's head it conflicts in dincli/cli/modelownerd/deploy.py (and in Developer/BACK_LOG.md, which comes from No. 261 alone). Clean against PRs No. 303 and No. 304.

foundry/src/DINTaskAuditor.sol

Field	Value
Change	Modified
Lines	+23/-3
Diff (what exactly is in this PR)	Adds ITaskModelRegistry public immutable modelRegistry. The constructor takes a fourth argument, modelRegistry_, and reverts TA_InvalidAddress if it has no code. Adds validateGIStart(uint256 gi) external view onlyTaskCoordinator. Updates the depositRewards NatSpec.
Functionality — how & why	How: validateGIStart calls modelRegistry.getModelIdByTaskAuditor(address(this)) → TA_ModelNotRegistered if absent, then modelDisabled(registeredId) → TA_ModelDisabled(id), then giRewardPool[gi] == 0 → TC_GIRewardPoolNotFunded. A registry revert bubbles up (fails closed). Why: issue No. 224, decision 1 (Abraham): disabling a model must stop new GIs, and the check lives in the auditor to protect the coordinator's size headroom.
Diff vs current develop HEAD	None — untouched since merge-base.
Recommended merge proposal	Merged as-is. Break-then-fix: 5 of the 8 new kill-switch tests fail when the coordinator stops calling this; all 8 pass with it. Size 21,885 B.
Actual merge proposal	Soon
Pending proposal	Follow-up issue (not this PR): the registry wiring is self-attested. Any contract with code passes the constructor, and approveModel doesn't check modelRegistry() == address(this). Suggested first step: a dincli dinrep pre-approval check.
Local merge conflict	No
GitHub merge conflict	No

foundry/src/DINTaskCoordinator.sol

Field	Value
Change	Modified
Lines	+3/-2
Diff (what exactly is in this PR)	In _startGI, replaces if (dinTaskAuditorContract.giRewardPool(_GI) == 0) revert TC_GIRewardPoolNotFunded(); with dinTaskAuditorContract.validateGIStart(_GI);, plus a comment.
Functionality — how & why	How: one STATICCALL after the state/index checks and before updatePassScore, GI++ and _setGIstate, shared by both startGI overloads. Why: routes the funded-pool check and the new registry checks through one auditor call, so the coordinator gains no registry wiring of its own (runtime 23,387 B, 1,189 B margin).
Diff vs current develop HEAD	None — untouched since merge-base.
Recommended merge proposal	Merged as-is. This line is what the break-then-fix toggled; see the verification comment.
Actual merge proposal	Soon
Pending proposal	None
Local merge conflict	No
GitHub merge conflict	No

foundry/src/DINShared.sol, foundry/src/DINModelRegistry.sol

Field	Value
Change	Modified
Lines	+13/-0, +2/-1
Diff (what exactly is in this PR)	DINShared: the new ITaskModelRegistry interface (two view functions), validateGIStart added to IDINTaskAuditor (as view), and the errors TA_ModelNotRegistered and TA_ModelDisabled(uint256). DINModelRegistry: disableModel NatSpec only.
Functionality — how & why	How: interfaces/errors only, no storage. The registry's existing getModelIdByTaskAuditor and public modelDisabled getter are what the auditor reads. Why: lets the auditor read the registry without importing it, and corrects the disableModel doc to the new scope. No storage-layout change to the upgradeable registry.
Diff vs current develop HEAD	None — untouched since merge-base.
Recommended merge proposal	Merged as-is.
Actual merge proposal	Soon
Pending proposal	None
Local merge conflict	No
GitHub merge conflict	No

dincli/cli/modelownerd/deploy.py

Field	Value
Change	Modified
Lines	+55/-8
Diff (what exactly is in this PR)	New _task_constructor_args maps the artifact's constructor input types to arguments. It accepts coordinator (address)/(address,uint256) and auditor (address,address)/(…,uint256)/(…,uint256,address), and rejects anything else. Adds --model-id to both commands. A missing stake address is now a BadParameter (previously an UnboundLocalError). The auditor checks the paired coordinator's modelId() against --model-id before deploying, and takes the registry from din_info[network]["registry"].
Functionality — how & why	How: constructor arguments are derived from the ABI, so Foundry and legacy Hardhat artifacts both deploy, and every mismatch fails before any tx. Why: develop's CLI passed one argument to the two-argument coordinator (unusable with current artifacts), and the new auditor needs the canonical registry.
Diff vs current develop HEAD	None — untouched since merge-base.
Recommended merge proposal	Needs a rebase after PR No. 261 (conflict in this file; both rewrite constructor wiring), then one small fix: wrap the paired modelId().call() in try/except and raise BadParameter. Today a coordinator without modelId() gives a raw ContractLogicError: execution reverted (reproduced on anvil). All other paths were verified live on anvil with the real Foundry artifacts.
Actual merge proposal	Soon
Pending proposal	Merge order: No. 261 first, then the author rebases and re-runs (as offered in the PR body). The try/except fix can go in that same rebase.
Local merge conflict	No (against develop); Yes against PR No. 261's head
GitHub merge conflict	No

dincli/abis/DINTaskAuditor.json, dincli/abis/DINTaskCoordinator.json

Field	Value
Change	Modified
Lines	+52/-0, +0/-5
Diff (what exactly is in this PR)	Regenerated: the auditor gains modelRegistry, validateGIStart, the fourth constructor input and the two new errors. The coordinator loses TC_GIRewardPoolNotFunded, which is now raised by the auditor.
Functionality — how & why	How: bundled ABIs are the fallback when a manifest gives no custom task_contracts ABI. Why: they must match what forge build emits.
Diff vs current develop HEAD	None — untouched since merge-base.
Recommended merge proposal	Merged as-is (or regenerated after the No. 261 rebase). Sorted-JSON comparison with foundry/out: identical (160/160, 201/201 entries).
Actual merge proposal	Soon
Pending proposal	cache_model_0/abis/ isn't refreshed here; PR No. 261 does that. Re-check after the rebase.
Local merge conflict	No
GitHub merge conflict	No

tests/test_model_disable_deployment.py

Field	Value
Change	New
Lines	+75/-0
Diff (what exactly is in this PR)	Parametrised tests of _task_constructor_args (supported shapes, --model-id range, missing registry, unknown signatures), plus one command-level test of task_auditor with mocks: canonical registry passed, two txs sent, and on mismatch no constructor call and no tx.
Functionality — how & why	How: no chain or wallet. Why: pins the ABI-shape mapping and the pair check.
Diff vs current develop HEAD	None — file doesn't exist on develop.
Recommended merge proposal	Merged as-is: 13 passed locally and green in CI Python. If the try/except fix is applied, add one case for a coordinator whose modelId() reverts.
Actual merge proposal	Soon
Pending proposal	None
Local merge conflict	No
GitHub merge conflict	No

foundry/test/RewardEngine.t.sol

Field	Value
Change	Modified
Lines	+143/-3
Diff (what exactly is in this PR)	The task-pair fixture now deploys against the real registry proxy, registers and approves the model (ID 0, against constructor ID 1), with optional skipRegistration / disableDuringGI flags. Adds 8 tests: both overloads disabled plus atomicity plus re-enable, private funding after disable, unregistered, registry-read failure (mockCallRevert), zero/EOA wiring, wrong registry contract, onlyTaskCoordinator, and in-flight GI completes/settles/claims but the next GI is blocked.
Functionality — how & why	How: integration against the real DINModelRegistry, not a stub. The atomicity test compares the params() bytes and the GI/GIstate before and after a failed start. Why: covers every branch of validateGIStart and the "no abort" in-flight guarantee.
Diff vs current develop HEAD	None — untouched since merge-base.
Recommended merge proposal	Merged as-is. 8/8 pass; break-then-fix turns 5 red without the coordinator call.
Actual merge proposal	Soon
Pending proposal	None
Local merge conflict	No
GitHub merge conflict	No

foundry/test/utils/EnabledModelRegistry.sol + constructor updates in 12 test files

AggregatorCommitReveal, AuditorCommitReveal, DeployPlatform, DisputeResolution, EncryptedTestData, GasSimulation, LifecycleEvents, PR146SlashingRegression, ScoringValidation, SecurityFindings, StakingEnforcement, TreasuryForwarding (.t.sol)

Field	Value
Change	New (1) / Modified (12)
Lines	+14/-0; +2/-1 each (SecurityFindings +6/-3, StakingEnforcement +3/-2)
Diff (what exactly is in this PR)	A stub registry that always returns (true, 0) and not-disabled. Each existing new DINTaskAuditor(…, id) gets a fourth argument, address(new EnabledModelRegistry()).
Functionality — how & why	How: keeps the isolated suites (which never register the model) startable under the new guard. Why: a mechanical update for the new constructor. The kill-switch semantics themselves are tested against the real registry in RewardEngine.
Diff vs current develop HEAD	None — untouched since merge-base.
Recommended merge proposal	Merged as-is. Full suite green in CI Solidity. Note that this stub is also a working example of the bypass described in observation 1 of the verification comment.
Actual merge proposal	Soon
Pending proposal	None
Local merge conflict	No
GitHub merge conflict	No

Documentation (9 files)

public/roles/dinrep.md, public/workflows/model-workflow.md, technical/audits/foundry-src-security-review.md, technical/contracts/{DINModelRegistry,DINShared,DINTaskAuditor,DINTaskCoordinator,DinValidatorStake}.md, technical/mechanisms/staking-mechanism.md

Field	Value
Change	Modified
Lines	+51/-28 in total
Diff (what exactly is in this PR)	Kill-switch scope (future GIs only, no abort or refund, redeploy required, the DIN-Representative reviews the wiring), the --model-id and din_info.registry deploy steps, the non-refundable registration fee (L-4 marked confirmed-intentional), S6 marked "parked for testnet", and M-2 in the audit doc marked resolved for new compatible pairs, with the original finding kept.
Functionality — how & why	How: each statement matches the code above. Decisions link to issue No. 224's decision comment. Why: decisions 2 and 3 of No. 224 are docs-only, and decision 1 changes documented behaviour.
Diff vs current develop HEAD	None — untouched since merge-base.
Recommended merge proposal	Merged as-is. Links are green in CI Docs.
Actual merge proposal	Soon
Pending proposal	If the dincli wiring check from observation 1 lands later, update the "DIN-Representative must review … registry address" sentences to point at it.
Local merge conflict	No
GitHub merge conflict	No

Verification

CI on 2a9c546: CI OK green. The non-required Integration (full GI) job is red on the same pre-existing missing proxyAdmin assertion as develop. Locally (real via_ir profile, after npm ci): forge build --sizes, the 8 kill-switch tests (8/8), break-then-fix (5 red without the coordinator call), the ABI parity check, pytest tests/test_model_disable_deployment.py (13 passed), and a live anvil deploy of the real Foundry task pair through the PR's deploy.py commands. Full detail is in the verification comment above.

Approvals needed (MAINTAINING No. 4.2): foundry/src/** plus a mechanism decision: the peer approval (@umeradl) and @abrahamnash as the second key.

Local vs. GitHub agree: both clean against develop.

[umeradl]

Approved at 2a9c546 (first key; foundry/src and a mechanism decision, so @abrahamnash's approval is also required). Kill-switch tests verified by break-then-fix (5/8 fail without the coordinator call); coordinator size margin 1,189 B; ABIs match the build; deploy CLI exercised on anvil with real artifacts. Before merge: land PR No. 261 first and rebase (deploy.py conflict), and turn the raw ContractLogicError for a paired coordinator without modelId() into a BadParameter. Self-attested registry wiring goes to a follow-up issue (observation 1).

[Santiagocetran]

Thanks for the detailed review and deployment checks, @umeradl!

Pushed the focused error handling in dfc5bd290644335ffd883cecec923735d45fcf16: a revert when reading the paired coordinator's modelId() now produces a clear BadParameter with guidance to check the address and compatibility. Unrelated errors still propagate.

All 24 targeted deployment tests passed in an isolated environment, covering the failure paths, absence of deployment side effects, and legacy compatibility. I haven't rerun live deployment or full-GI tests.

I'll keep this waiting for PR No. 261, then reconcile the deployment overlap and rerun the relevant checks. Abraham's requested sign-off remains outstanding, and I'll keep registry-wiring enforcement as a separate follow-up. Fresh CI and re-review of the new head are still needed.

[robertocarlous]

Reviewed against develop in an isolated worktree (pr-305-review on top of 90a72422, the PR head). Peer check: OK (Santiagocetran is the author; operator is not).

CI on 90a72422: CI OK ✅, Python ✅ (all non-integration tests pass), Solidity ✅, Docs ✅ — all green. Review is based on source inspection and CI.

---

Claimed — validateGIStart blocks both startGI overloads when the model is disabled. Verified: DINTaskCoordinator._startGI (L370–385) is the shared internal path called by both startGI(uint _GI, uint score) (L360) and startGI(uint _GI) (L366). Line 381 calls dinTaskAuditorContract.validateGIStart(_GI) unconditionally before any GI state change. Neither public overload touches state before that call.

Claimed — validateGIStart checks registry approval, disabled status, and reward pool in order. Verified by direct inspection of DINTaskAuditor.sol L446–451: getModelIdByTaskAuditor(address(this)) first; TA_ModelNotRegistered if !exists; TA_ModelDisabled(registeredId) if modelDisabled; TC_GIRewardPoolNotFunded if giRewardPool[gi] == 0. The implementation uses the registry-returned registeredId (the canonical ID from the registry), not the constructor's modelId (the stake-floor parameter). These two can differ if the registry assigns a different ID than the one passed to the constructor; the PR uses the authoritative registry value, which is correct.

Claimed — ITaskModelRegistry interface matches the real DINModelRegistry functions. Verified against DINModelRegistry.sol: getModelIdByTaskAuditor(address) is at L422–429, returns (bool exists, uint256 modelId) using a val - 1 sentinel to distinguish model ID 0 from "not registered" — exactly what the interface declares. modelDisabled(uint256) is the auto-getter for the public modelDisabled mapping at L119. Both signatures match the interface in DINShared.sol.

Claimed — modelRegistry is immutable and validated as a contract at construction. Verified: declared ITaskModelRegistry public immutable modelRegistry at L22; constructor checks modelRegistry_.code.length == 0 (L419) and reverts TA_InvalidAddress before assignment (L428). No setter exists; cannot be reassigned after deployment.

Claimed — all three errors are file-level selectors, not contract-scoped. Verified: TC_GIRewardPoolNotFunded (L359), TA_ModelNotRegistered (L488), and TA_ModelDisabled(uint256) (L490) are all in DINShared.sol. The pre-PR coordinator used TC_GIRewardPoolNotFunded as a file-level error; the selector is unchanged.

Claimed — 8 new kill-switch tests in RewardEngine.t.sol. Verified by CI (Solidity ✅) and by inspection: 8 new tests including test_disabledModel_blocksBothOverloadsAtomically_andReenable (both startGI overloads blocked, state unchanged, re-enable resumes) and test_disabledInFlight_completesSettlesClaims_butCannotStartNext (in-flight GI completes normally; next GI blocked after disable). EnabledModelRegistry.sol test stub always returns (true, 0) / false and is used in all non-kill-switch tests — this keeps existing test suites unaffected by the new auditor constructor arg.

Claimed — SecurityFindings.t.sol constructors updated. Confirmed: the existing security-finding regression tests now pass the 4-arg auditor constructor (CI Solidity ✅).

Claimed — dincli model-owner deploy handles the new 4-arg auditor constructor. Verified: _task_constructor_args in dincli/cli/modelownerd/deploy.py dispatches on constructor arity; 4-arg auditor passes [stake, coordinator, model_id, registry]. test_supported_constructor_shapes in test_model_disable_deployment.py exercises all five supported shapes including ("address", "address", "uint256", "address") → ["stake", "coordinator", 0, "registry"]; Python CI green.

Claimed — DINTaskAuditor.json ABI updated. Verified: new constructor input modelRegistry_, modelRegistry view function, validateGIStart function, TA_ModelNotRegistered and TA_ModelDisabled errors all present.

Claimed — security review M-2 updated with resolution. Verified in foundry-src-security-review.md: "Resolution" paragraph added above the original finding text; L-4 updated to "Confirmed intentional — [#224, Oct 26 decision] / BL-35." The original finding text is retained as audit context with the label "Original finding — ... superseded for new compatible pairs."

Claimed — full-gi.yml extracted from ci.yml. Verified: 178 lines removed from ci.yml (the integration-paths + integration jobs); equivalent job added as full-gi.yml with on: workflow_dispatch (manual only). The ci-ok job in ci.yml never waited on integration, so this is a no-op for required checks.

---

Finding — P2: Developer/BACK_LOG.md BL-34 status not updated to resolved. The PR changes BL-34 from "🆕 New — unaddressed" to "🆕 Decided, unimplemented — Abraham (Oct 26, [No. 224]): make the kill-switch real…" — but this PR IS that implementation. BL-34 should be "✅ Resolved — validateGIStart wired in DINTaskAuditor.sol; both startGI overloads check registry approval and disabled status before any state change. Compatible task pairs required; existing deployed pairs unaffected. PR No. 305."

Merge state: branch is BEHIND develop (GitHub mergeStateStatus: BEHIND), no conflict (local merge-tree exit 0). Needs update-branch before merge. foundry/src/** touched — a second sign-off from @umeradl or @abrahamnash is required per MAINTAINING No. 4.2 before this merges.

[robertocarlous]

Head at review: 90a72422
Merge base (upstream develop at PR creation): earlier develop tip
Current upstream develop: behind but clean — no conflict (merge-tree exit 0)

---

foundry/src/DINShared.sol

Key	Value
Change	+12 lines: ITaskModelRegistry interface (getModelIdByTaskAuditor, modelDisabled); IDINTaskAuditor.validateGIStart declaration; TA_ModelNotRegistered and TA_ModelDisabled(uint256) errors
Pending proposal	None — interface matches real DINModelRegistry functions; errors declared at file level; selectors correct
Recommended merge proposal	Merge as-is
Actual merge proposal	Soon

foundry/src/DINTaskAuditor.sol

Key	Value
Change	+26 lines: ITaskModelRegistry public immutable modelRegistry; constructor gains 4th arg modelRegistry_ (non-EOA check); validateGIStart(uint256 gi) external view onlyTaskCoordinator checks registry approval + disabled + reward pool in order
Pending proposal	None — implementation correct; uses registry-returned ID (not constructor modelId); all errors from DINShared.sol
Recommended merge proposal	Merge as-is
Actual merge proposal	Soon

foundry/src/DINTaskCoordinator.sol

Key	Value
Change	-2/+2 lines: _startGI replaces direct giRewardPool check with dinTaskAuditorContract.validateGIStart(_GI)
Pending proposal	None — both public overloads call _startGI; call is unconditional before any state mutation
Recommended merge proposal	Merge as-is
Actual merge proposal	Soon

foundry/test/utils/EnabledModelRegistry.sol (new)

Key	Value
Change	+19 lines: test stub always returns (true, 0) / false — satisfies new 4-arg auditor constructor in all non-kill-switch tests
Pending proposal	None
Recommended merge proposal	Merge as-is
Actual merge proposal	Soon

foundry/test/RewardEngine.t.sol

Key	Value
Change	+130 lines: 8 new kill-switch tests; covers both startGI overloads blocked atomically, state unchanged, re-enable resumes; in-flight GI completes normally after disable but next GI blocked
Pending proposal	None
Recommended merge proposal	Merge as-is
Actual merge proposal	Soon

foundry/test/SecurityFindings.t.sol

Key	Value
Change	Constructor updates to pass the 4-arg auditor form
Pending proposal	None
Recommended merge proposal	Merge as-is
Actual merge proposal	Soon

dincli/cli/modelownerd/deploy.py

Key	Value
Change	_task_constructor_args helper added; handles 2/3-arg coordinator and 2/3/4-arg auditor constructors; --model-id option; reads registry from din_info; cross-checks --model-id against modelId() on the paired coordinator
Pending proposal	None
Recommended merge proposal	Merge as-is
Actual merge proposal	Soon

dincli/abis/DINTaskAuditor.json

Key	Value
Change	modelRegistry_ added to constructor inputs; modelRegistry view function; validateGIStart function; TA_ModelNotRegistered and TA_ModelDisabled errors
Pending proposal	None
Recommended merge proposal	Merge as-is
Actual merge proposal	Soon

tests/test_model_disable_deployment.py (new)

Key	Value
Change	+184 lines: constructor-shape tests, model-ID validation, registry requirement, mismatch rejection, full deploy flow with/without registry arg
Pending proposal	None
Recommended merge proposal	Merge as-is
Actual merge proposal	Soon

Developer/BACK_LOG.md

Key	Value
Change	BL-24 no additional change; BL-27 status clarified (more detail on PR No. 261 scope); BL-34 updated from "New — unaddressed" to "Decided, unimplemented — Abraham (Oct 26, No. 224)"; BL-35 updated from "New — unaddressed" to "Decided — keep as intended (Abraham, Oct 26)"; BL-28 status updated with more detail on planned command
Pending proposal	P2 BL-34 status should be "✅ Resolved" — this PR implements the decision. Replace the current BL-34 status with: ✅ Resolved — \validateGIStart` wired in `DINTaskAuditor.sol`; both `startGI` overloads check registry approval and disabled status via `_startGI`. Compatible task pairs required; existing deployed pairs unaffected. PR No. 305`
Recommended merge proposal	Update BL-34 to "✅ Resolved", then merge
Actual merge proposal	Soon

Documentation/technical/audits/foundry-src-security-review.md

Key	Value
Change	M-2 resolution paragraph added; L-4 confirmed intentional (BL-35); original finding text retained as audit context
Pending proposal	None
Recommended merge proposal	Merge as-is
Actual merge proposal	Soon

Documentation/technical/contracts/DINTaskAuditor.md

Key	Value
Change	modelRegistry (immutable) added to dependencies table; validateGIStart added to coordinator-only caller table; new "Future-GI guard and deployment" subsection covering constructor, guard semantics, disabling/reenabling, non-upgradeable caveat
Pending proposal	None
Recommended merge proposal	Merge as-is
Actual merge proposal	Soon

Documentation/technical/contracts/DINTaskCoordinator.md, DINShared.md, DINModelRegistry.md, DinValidatorStake.md

Key	Value
Change	Cross-references updated to mention validateGIStart call in _startGI; DINModelRegistry.md notes getModelIdByTaskAuditor is called by compatible task auditors
Pending proposal	None
Recommended merge proposal	Merge as-is
Actual merge proposal	Soon

.github/workflows/ci.yml / .github/workflows/full-gi.yml (new)

Key	Value
Change	integration-paths + integration jobs removed from ci.yml (-178 lines); equivalent integration job added to new full-gi.yml as workflow_dispatch only (manual trigger)
Pending proposal	None — ci-ok never depended on the integration job; this is a non-breaking extraction
Recommended merge proposal	Merge as-is
Actual merge proposal	Soon

Remaining documentation files (skills, MAINTAINING.md, CONTRIBUTING.md, ROADMAP.md, role/workflow docs, README)

Key	Value
Change	Cross-references, task-contract deployment steps, and skill descriptions updated to reflect the 4-arg auditor constructor and validateGIStart behavior
Pending proposal	None
Recommended merge proposal	Merge as-is
Actual merge proposal	Soon

[robertocarlous]

P2-1: Developer/BACK_LOG.md BL-34 status should be updated to ✅ Resolved — this PR implements the fix. Please change the current "Decided, unimplemented" text to: "✅ Resolved — validateGIStart wired in DINTaskAuditor.sol; both startGI overloads check registry approval and disabled status via _startGI. Compatible task pairs required; existing deployed pairs unaffected. PR No. 305." Verified at 90a72422. Everything else — contracts, tests, CLI, ABIs, docs — is correct and merge-ready once this row is updated.

[Santiagocetran]

Thanks for the reviews and thorough checks!

Fixed BL-34 in 6970f5d to mark it as resolved and describe the implemented guard, including the compatibility caveat for existing deployments.

Only the backlog status changed. Diff checks and all 293 documentation-link checks passed.

I’ll reconcile the deployment changes once PR No. 261 lands, then rerun the relevant checks and request a fresh review.

[Santiagocetran]

Thanks for the thorough review! I’ve reconciled this branch with develop after PR No. 261 and pushed the updates through e8e7349.

The CLI now checks the coordinator’s protocol version before deploying or pairing an auditor, rejecting incompatible combinations while preserving matching current and legacy pairs. The artifacts and docs are updated too.

Local validation is green: 735 Python tests and 527 Foundry tests passed. Live CLI checks confirmed that all eight incompatible pairings reject without transactions or address writes. Astra also rechecked the fix and found no new issues.

Fresh CI is running. Persistent IPFS publication and independent retrieval, along with fresh CI results and the required reviews, are still pending before this is ready to merge. The current IPFS evidence covers local offline pinning and retrieval only.