Skip to content

dincli model-owner: deploy sends stale constructor args (broken); no command for releaseGIRegistrationSlots (BL-27, BL-28) #223

New issue
New issue
Open
Open
dincli model-owner: deploy sends stale constructor args (broken); no command for releaseGIRegistrationSlots (BL-27, BL-28)#223
Labels
bugSomething isn't workinggood first issueGood for newcomers

Description

@Abidoyesimze
Abidoyesimze
opened on Oct 5, 2026

Two separate, both-confirmed gaps in the model-owner tooling, batched here since they're the same area of the codebase (dincli/cli/modelownerd/) and the same root-cause category (dincli not kept in sync with a foundry/src contract change).

Part 1 (BL-27): dincli model-owner deploy is broken against current contracts

dincli/cli/modelownerd/deploy.py calls:

DINTaskCoordinator_contract.constructor(din_validator_stake_address)                         # 1 arg
DINTaskAuditor_contract.constructor(din_validator_stake_address, task_coordinator_address)    # 2 args

but the actual constructors (foundry/src/DINTaskCoordinator.sol:290, foundry/src/DINTaskAuditor.sol:406-410, verified against develop @ 740a613) are:

constructor(address dinvalidatorStakeContract_address, uint256 modelId_)
constructor(address _dinvalidatorStakeContract_address, address _dintaskcoordinator_contract_address, uint256 modelId_)

Both calls are missing modelId_. Verified by inspection against the current ABI — dincli model-owner deploy task-coordinator/task-auditor will fail at ABI encoding today. This is hidden in testing because tests/ deploys task contracts from stale hardhat artifacts, not the bundled foundry-derived ABIs.

This isn't a one-line fix — DINModelRegistry only assigns modelId at approveModel() time, after the task contracts must already exist (the registration flow requires already-deployed, already-owned taskCoordinator/taskAuditor addresses as input). So deploy-time modelId_ doesn't exist yet when these constructors run. Needs a decision: a placeholder/sentinel modelId at deploy time later reconciled at approval, a registry-side pre-assignment step before deploy, or a constructor change removing the dependency — out of scope for me to prescribe here, flagging for whoever picks this up.

Part 2 (BL-28): no dincli command for releaseGIRegistrationSlots

DINTaskCoordinator.releaseGIRegistrationSlots(gi) (L1287-1299) is the only function that decrements validators' activeRegistrationCount after a GI ends — endGI deliberately doesn't, to stay O(1) (BL-10). It's present in the bundled ABI (dincli/abis/DINTaskCoordinator.json) but grepping all of dincli/ turns up zero callers.

Currently harmless: maxConcurrentRegistrationsPerStakeUnit (DinValidatorStake.sol:115) defaults to 0, so the cap it would otherwise enforce (TC_ConcurrentRegistrationCapReached/TA_ConcurrentRegistrationCapReached) never fires. But the moment any network sets that cap above 0, every ended GI permanently consumes its validators' concurrent-registration slots until they hit the cap — with no dincli path to call the one function that frees them.

Suggested approach

  • Part 1: fix the constructor args once the modelId sourcing decision is made; add a dincli-level smoke test that actually deploys against the real foundry artifact (not hardhat) to catch this class of drift going forward.
  • Part 2: add dincli model-owner release-gi-slots <gi> (or fold into an existing model-owner gi ... subcommand), callable any time after the target GI reaches GIended.

References

Activity

  1. Abidoyesimze
    added
    bugSomething isn't working
    on Oct 5, 2026

  2. umeradl commented on Oct 6, 2026

    @umeradl
    umeradl
    on Oct 6, 2026
    Member

    Status update: confirmed; covered by task-plan-051026-2 TP-3

    Checked against develop @ a1fcce2. Both halves hold:

    • dincli/cli/modelownerd/deploy.py:35 sends DINTaskCoordinator.constructor(stake) and :77 sends DINTaskAuditor.constructor(stake, coordinator). foundry/src takes (stake, modelId_) (DINTaskCoordinator.sol:290) and (stake, coordinator, modelId_) (DINTaskAuditor.sol:406), so ABI encoding fails against the bundled foundry ABIs. There's also a latent NameError when din_info has no stake entry (:27-28, :61-62).
    • No dincli command calls releaseGIRegistrationSlots.

    Can be done now: yes, but it's already planned. Both are in umermjd11's task-plan-051026-2 (PR No. 229) TP-3:

    • --model-id on both deploys, defaulting to DINModelRegistry.totalModels() with a warning;
    • the auditor deploy checks the coordinator's modelId();
    • both deploys call setDinToken;
    • the NameError is fixed;
    • a dincli model-owner gi release-slots command is added;
    • the DIN-Rep approval refuses a modelId mismatch.

    The PR No. 229 review asks the plan to name this issue as closed by TP-3. It stays open until that PR lands, and isn't part of task_061026_22.

  3. umeradl
    added a commit that references this issue on Oct 6, 2026

    docs(backlog,audit): link BL-27..31 to new issues, mark stale finding…

    fa796ae
  4. abrahamnash
    added
    good first issueGood for newcomers
    on Oct 25, 2026

  5. abrahamnash commented on Oct 26, 2026

    @abrahamnash
    abrahamnash
    on Oct 26, 2026
    Member

    Note for when this lands: Documentation/public/workflows/model-workflow.md (lines 43 and 52) currently points model owners to hardhat/artifacts/... task contracts, presumably because of this bug, and to match the live DevNet 2.0 contracts. Once #223 is fixed and develop's Foundry contracts are deployed (DevNet 3.0), that guide should switch to the Foundry artifacts, and the Foundry-only decision in Developer/discussion/migrate_to_foundy.md can be closed out.

  6. abrahamnash commented on Oct 26, 2026

    @abrahamnash
    abrahamnash
    on Oct 26, 2026
    Member

    This overlaps with #246, which PR #261 fixes. Once #261 merges, both can be closed.

  7. umeradl commented on Oct 29, 2026

    @umeradl
    umeradl
    on Oct 29, 2026
    Member

    @abrahamnash checked against develop @ 6e73dca and PR No. 261's head (8dd6311). PR No. 261 closes issue No. 246 and Part 1 of this issue, but not all of it:

    Item On develop now PR No. 261
    Part 1 (BL-27): deploy sends stale constructor args ❌ dincli/cli/modelownerd/deploy.py:35 / :77 still omit modelId_ ✅ adds --model-id, passes it to both constructors, and checks the auditor's against the coordinator's modelId()
    Issue No. 246 (BL-25): stale cache_model_0/abis/ ❌ ✅ regenerates both ABIs and updates cache_model_0/manifest.json
    Part 2 (BL-28): no dincli command for releaseGIRegistrationSlots ❌ no caller in dincli/cli ❌ not in this PR. It's planned as dincli model-owner gi release-slots <model_id> --gi N in task-plan-051026-2 TP-3 (task_061026_21)
    Your note: model-workflow.md L43/L52 point to hardhat/artifacts/... ❌ unchanged ❌ PR No. 261 doesn't touch Documentation/

    So once PR No. 261 merges (Santiago's changes-requested review is still open on it), issue No. 246 can close. This issue stays open for Part 2 and the model-workflow.md artifact paths, which I'll add to TP-3. (PR No. 232's "Closes" on this issue was the task-spec PR, so it never delivered code.)

  8. umeradl commented on Nov 2, 2026

    @umeradl
    umeradl
    on Nov 2, 2026
    Member

    @Santiagocetran, could you run the closing check on this issue (din-close-issue 223)? I can't do it: din_peer_check.py refuses me because I wrote PRs No. 229 and No. 232, which implement part of it (MAINTAINING No. 5.2). Robbert wrote PR No. 261 and Similoluwa wrote PR No. 225, so you're the eligible peer.

    Pointers for the check:

    • BL-27 (stale constructor arguments in model-owner deploy): PR No. 261 (merged as d6f537bc today) fixed the deploy constructors, and Developer/BACK_LOG.md marks BL-27 "✅ Fixed — PR No. 261".
    • BL-28 (no command for releaseGIRegistrationSlots): I don't think this is on develop yet. My open PR No. 302 adds the release-slots command. If that's right, this needs a status update rather than a close.

  9. Santiagocetran commented on Nov 2, 2026

    @Santiagocetran
    Santiagocetran
    on Nov 2, 2026 – with ChatGPT Codex Connector
    Collaborator

    Closing check: not ready to close

    Checked the latest upstream develop at cf802168.

    Item Status Evidence
    Part 1 / BL-27: stale deploy constructor arguments ✅ Fixed PR No. 261 merged as d6f537bc. Both deploy calls pass model_id and match the bundled constructor ABIs. Auditor deployment also rejects a coordinator/model-ID mismatch.
    Part 2 / BL-28: release GI registration slots ❌ Missing releaseGIRegistrationSlots exists in the Foundry contract, but there is no caller under dincli/cli on develop. PR No. 302 adds model-owner gi release-slots but remains open and unmerged.
    Workflow documentation follow-up ❌ Outstanding Documentation/public/workflows/model-workflow.md:43,52 still uses Hardhat artifact paths and omits the now-required --model-id.

    Verification: the peer eligibility check passed for Santiagocetran. Static checks confirmed constructor argument counts against both bundled ABI sets and the missing release-slots caller. Deployment smoke tests were not run because the required tools and Python dependencies were unavailable.

    Verdict: leave this issue open. Recheck once the release-slots implementation lands and the workflow documentation is updated. PR No. 261 resolves Part 1 only.

  10. Santiagocetran
    added a commit that references this issue on Nov 4, 2026

    feat(dincli): rewards, encryption-key, release-slots and dispute comm…

    01c26e0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workinggood first issueGood for newcomers

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions