Skip to content

Three dormant/no-op mechanisms awaiting a product decision: disableModel kill-switch, rejectModel fee handling, S6 slashing (M-2, L-4, BL-31) #224

Open

Description

@Abidoyesimze

Three separate findings, batched because they share the same shape: each is a mechanism that exists in some form (an admin function, a fee flow, a slashing path) but nobody has made — or at least documented — the decision on whether/how it's actually meant to be used. None are exploitable bugs; all three just need someone with product/mechanism-design authority to decide, so the code (or the docs) can be made to match.

M-2: DINModelRegistry.disableModel() doesn't reach the live task contracts

modelDisabled[modelId] (DINModelRegistry.sol:119) only gates requestModelRegistration/requestManifestUpdate inside the registry itself (L151, L330). Confirmed by inspection against develop @ 740a613: DINTaskCoordinator.sol has zero references to DINModelRegistry, and DINTaskAuditor.sol's only two references are doc comments, not functional checks. A model the DIN-Representative has disabled — e.g., because its task contracts have a bug wrongfully slashing honest validators — keeps running full GIs completely unaffected. The task contracts' own doc table calls this a "kill-switch," which it currently isn't.

Decision needed: wire the task contracts to check DINModelRegistry.modelDisabled(modelId) before state-changing calls (real fix, requires threading the model ID + registry address into contracts that don't currently know about the registry at all), or explicitly document disableModel as registry-metadata-only so operators stop treating it as a circuit breaker.

L-4: rejectModel() never refunds the registration fee

Checked the full flow: feePaid is recorded at requestModelRegistration() (DINModelRegistry.sol:197) and is never refunded on either outcome — not on rejectModel() (L261-271) and not on approveModel() either (L217-258 doesn't touch feePaid at all). This consistency suggests the fee is intentionally a pay-to-apply cost regardless of outcome (anti-spam), not an oversight specific to rejection — but this was never written down anywhere as a deliberate choice.

Decision needed: confirm non-refundable-regardless-of-outcome is the intended design and document it (in DINModelRegistry.md and/or the dinrep role docs), so a future reader doesn't "fix" it into an inconsistent partial-refund state.

BL-31: S6 no-participation slashing is implemented, tested, but never called

DinValidatorStake.recordNoParticipation() (S6: escalating 10%-per-breach slash past a threshold, L333-354) is fully implemented and has its own tests. Every call site in both task contracts is a comment explaining why it's deliberately not called (DINTaskAuditor.sol:1391, DINTaskCoordinator.sol:1185) — slashPartial (S1/S2) already penalizes the same missed-work behavior, and calling both would double-penalize past MIN_STAKE. Confirmed: zero functional call sites anywhere in foundry/src/.

Decision needed: either S6 is meant to cover a gap S1/S2 don't (e.g., a validator registered but never assigned to any batch, so S1/S2 never trigger on them at all) and needs wiring to that specific case, or it's superseded dead code that should be removed along with its threshold parameter and event, per the slashing taxonomy (#38).

References

Activity

  1. umeradl commented on Oct 6, 2026

    @umeradl
    Member

    Status update: confirmed; decisions pending

    Checked against develop @ a1fcce2. All three hold as described. These are maintainer decisions; options and a recommendation for each are below. Nothing here is in task_061026_22.

    M-2: disableModel() doesn't reach the task contracts

    modelDisabled (foundry/src/DINModelRegistry.sol:119) gates only requestModelRegistration (:151) and requestManifestUpdate (:330). Neither task contract reads the registry.

    Option Cost
    A. Document disableModel as registry-only (it blocks manifest changes), and stop calling it a kill switch Docs only
    B. Wire the task contracts to check modelDisabled(modelId) before state-changing calls Each task contract needs the registry address plus a call per entry point. That's bytes on a coordinator with a 1,079 B margin, and the task contracts aren't upgradeable

    Recommended: A now. Revisit B with the GI-abort design (issue No. 222), since "stop a misbehaving model" needs the same "what happens to the in-flight GI" answer.

    L-4: rejectModel() keeps the registration fee

    feePaid is set at request time (:197, :307) and refunded on neither outcome (rejectModel at :261).

    Option Cost
    A. Confirm it's non-refundable on any outcome (anti-spam) and document it in DINModelRegistry.md and roles/dinrep.md Docs only
    B. Refund on reject A contract change, and it weakens the anti-spam cost

    Recommended: A.

    BL-31: S6 is implemented but never called

    DinValidatorStake.recordNoParticipation (:333) has no caller. Both task contracts skip it on purpose where slashPartial (S1/S2) already penalises the miss (DINTaskAuditor.sol:1391, DINTaskCoordinator.sol:1185).

    Option Cost
    A. Wire S6 only for what S1/S2 can't see (e.g. registered but never active) Needs a definition of "no participation" that doesn't double-count
    B. Remove S6 and its parameters Simplest. DinValidatorStake is upgradeable, so the storage has to stay or be gapped
    C. Leave it dormant and document why No change

    Recommended: decide with the slashing taxonomy (issue No. 38). Until then, C.

  2. abrahamnash commented on Oct 26, 2026

    @abrahamnash
    Member

    Decisions (Abraham):

    1. Model kill-switch (BL-34): make it real before the public testnet. Disabling a model should at minimum stop new GIs from starting on its task contracts (emission funding already checks modelDisabled; the task contracts don't). Given DINTaskCoordinator's size headroom (see Contract size headroom for all contracts (DINTaskCoordinator −9 B over EIP-170 with PR No. 197) + slash reason for committed-but-unrevealed across commit-reveal flows #201), consider putting the check in DINTaskAuditor or a small external guard rather than the coordinator.
    2. Fee on rejected models (BL-35): keep as intended. The registration fee is a pay-to-apply anti-spam cost, not refunded on approval or rejection. Please document this in DINModelRegistry.md and the dinrep role docs.
    3. S6 no-participation slashing (BL-31): park for testnet. S1/S2 plus S5 already cover missed work. Leave S6 unwired (or remove it and its parameters), and revisit after testnet if a real gap appears.

    Suggest splitting (1) into its own implementation issue; (2) is docs-only; (3) needs either a removal PR or a "parked" note in the docs.

  3. Santiagocetran commented on Oct 30, 2026

    @Santiagocetran
    Collaborator

    Based on this conversation, I’ve implemented the agreed changes in PR #305: preventing disabled models from starting new GIs, documenting the non-refundable registration fee, and marking S6 as parked for testnet.

    The PR includes tests and deployment compatibility notes. It overlaps with #261, which should land first; I’ll reconcile the branch and rerun the relevant checks afterward.

    Ready for review—feedback is welcome!

  4. umeradl commented on Nov 7, 2026

    @umeradl
    Member

    @robertocarlous, could you run the closing check on this issue (din-close-issue 224)? The peer check refuses me: I authored PR No. 229, a task-plan doc that references this issue. Santiago is the implementer, so he can't run it either.

    PR No. 305 merged into develop as 3def72f and covers M-2 (disableModel as a real kill-switch, BL-34). Two things to check against origin/develop:

    • L-4 (rejectModel fee is non-refundable) and BL-31 (wire in or remove S6 no-participation slashing): is a decision recorded for each, in code, in docs or in a comment? If either is still open, this gets a status update, not a close.
    • The four non-blocking follow-ups from PR No. 305 are listed in its outcome comment (artifact-gate message and CID check, get_code ordering in deploy task-auditor, CID pinning). Please note them in the result so they're tracked.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

questionFurther information is requested

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions