From 24be72b4ca9b1bdab429839342d6cae6cd84a0a2 Mon Sep 17 00:00:00 2001 From: umermjd11 Date: Thu, 5 Nov 2026 21:05:21 +0500 Subject: [PATCH] feat(dincli): deploy sets DinToken and defaults --model-id (task_061026_21 Part C, Deploy) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The deploy follow-up PR #302 deferred until #261 merged: - `model-owner deploy task-coordinator` and `task-auditor` call setDinToken(din_info token) on the new contract. Without it, `rewards deposit` refuses and no GI can be funded or started. - `--model-id` is optional. The coordinator defaults to DINModelRegistry.totalModels() (the ID the next approval assigns) and warns that this only holds if no other request is approved first. The auditor defaults to the coordinator's modelId(); the mismatch guard from #261 is unchanged. - A missing `stake` or `token` din_info entry exits with a clear message before any transaction, instead of a NameError. DINTaskCoordinator.md §10 No. 10 and DINTaskAuditor.md §13 No. 8 now describe the fixed deploy. Co-Authored-By: Claude Opus 5.5 --- .../technical/contracts/DINTaskAuditor.md | 2 +- .../technical/contracts/DINTaskCoordinator.md | 4 +- dincli/cli/modelownerd/deploy.py | 74 +++++++-- tests/test_deploy_task_contracts.py | 154 ++++++++++++++++++ tests/test_task_auditor_model_id_guard.py | 10 +- 5 files changed, 226 insertions(+), 18 deletions(-) create mode 100644 tests/test_deploy_task_contracts.py diff --git a/Documentation/technical/contracts/DINTaskAuditor.md b/Documentation/technical/contracts/DINTaskAuditor.md index e32bbfe..57e0913 100644 --- a/Documentation/technical/contracts/DINTaskAuditor.md +++ b/Documentation/technical/contracts/DINTaskAuditor.md @@ -207,7 +207,7 @@ Read alongside the [foundry/src security review](../audits/foundry-src-security- - **No. 5 — Fixed: scoring can't open before test data is assigned.** `setTestDataAssignedFlag` now checks every batch has a commitment and moves the coordinator to `AuditTestDataAssigned`, which `startLMsubmissionsEvaluation` requires (task_061026_21 Part B). - **No. 6 — Stale NatSpec and reused errors:** `slashAuditors` says S1 and S3 are both `minStake()` (S1 is now a fraction). Several comments call parameters "DAO-settable"; they are `onlyOwner`, i.e. set by the model owner. `setDisputePenaltyBps` reuses `TA_InvalidDisputeBond`, and `closeExpiredDispute` reuses `TA_DisputeWindowClosed` for "window still open". - **No. 7 — `modelId` is fixed at construction**, before the registry assigns it (see [DINTaskCoordinator §10 No. 4](DINTaskCoordinator.md#10-review-notes--open-caveats)). -- **No. 8 — dincli lags this contract:** `dincli model-owner deploy task-auditor` still calls the older two-argument constructor (no `modelId`). +- **No. 8 — dincli deploy (fixed):** `dincli model-owner deploy task-auditor` calls the three-argument constructor. `--model-id` defaults to the coordinator's `modelId()`, and the command refuses a mismatch. It then calls `setDinToken` with the platform `DinToken` before linking the auditor to the coordinator. - **No. 9 — Fixed: a dincli auditor commit retry no longer loses the committed salt.** Rerunning `dincli auditor lms-evaluation evaluate --submit` used to generate a new salt and overwrite the local commit cache even when the on-chain commit already existed. The later reveal then failed the hash check, and the auditor was slashed for a missed vote. dincli now skips any LM whose `hasCommittedLM` is already set, leaving its cache untouched, and writes the cache before sending the commit tx (issue No. 202 Part 1). A rerun is safe. --- diff --git a/Documentation/technical/contracts/DINTaskCoordinator.md b/Documentation/technical/contracts/DINTaskCoordinator.md index 1ded10f..eb295ba 100644 --- a/Documentation/technical/contracts/DINTaskCoordinator.md +++ b/Documentation/technical/contracts/DINTaskCoordinator.md @@ -116,7 +116,7 @@ setDINTaskCoordinatorAsSlasher() → AwaitingDINTaskAuditorAsSlash [DIN-Representative: DinCoordinator.addSlasherContract(auditor)] setDINTaskAuditorAsSlasher() → AwaitingGenesisModel setGenesisModelIpfsHash(cid) → GenesisModelCreated -setDinToken(dinToken) (needed for dispute bonds) +setDinToken(dinToken) (needed for dispute bonds; dincli's deploy sends it) ``` --- @@ -282,7 +282,7 @@ Earlier findings from the [foundry/src security review](../audits/foundry-src-se - **No. 7 — Stale NatSpec:** comments around the dispute scaffold still say `DinTreasury` "doesn't exist on develop yet" (forfeitures are already forwarded). Several parameters are described as "DAO-settable"; they are `onlyOwner`, i.e. set by the model owner. - **No. 8 — Leftovers:** `networkFeeFloor` is stored but not enforced. - **No. 9 — Not upgradeable:** a bug in a model's task contracts requires redeploying them and re-registering the model. -- **No. 10 — dincli lags this contract:** `dincli model-owner deploy task-coordinator` still calls the older one-argument constructor (no `modelId`). (`dincli aggregator aggregate-t2` used to name its working directory, worker job and container after the last T1 batch id; fixed in issue No. 202 Part 2.) +- **No. 10 — dincli deploy (fixed):** `dincli model-owner deploy task-coordinator` calls the two-argument constructor. `--model-id` defaults to `DINModelRegistry.totalModels()`, the ID the next approval assigns, with a warning that this only holds if no other request is approved first. It then calls `setDinToken` with the platform `DinToken`. (`dincli aggregator aggregate-t2` used to name its working directory, worker job and container after the last T1 batch id; fixed in issue No. 202 Part 2.) - **No. 11 — Fixed: `registerDINaggregator` now checks the GI.** It used to have no `onlyCurrentGI`, so during GI N's window a validator could register for GI N+1. Up to 300 addresses could fill that list, which locked out honest registrants and handed the attacker every T1/T2 batch. Registering for a released past GI also leaked the caller's own slot. Fixed by adding `onlyCurrentGI` (issue #206); it cost 9 bytes inside the No. 1 budget. --- diff --git a/dincli/cli/modelownerd/deploy.py b/dincli/cli/modelownerd/deploy.py index bd75282..e8da08e 100644 --- a/dincli/cli/modelownerd/deploy.py +++ b/dincli/cli/modelownerd/deploy.py @@ -1,5 +1,7 @@ import os import time +from typing import Optional + import typer from dincli.cli.contract_utils import get_contract_instance @@ -14,24 +16,61 @@ # Deploy sub-app (for 'dincli modelowner deploy ...') deploy_app = typer.Typer(help="Deploy task-level smart contracts") + +def _require_din_info_address(effective_network, key, label, console): + """Return din_info[network][key], or exit with a clear message if it's missing.""" + address = load_din_info().get(effective_network, {}).get(key) + if not address: + console.print( + f"[bold red]Error:[/bold red] no {label} address ('{key}') in din_info for network " + f"'{effective_network}'. Run `dincli system import-deployments` (or configure the network) first." + ) + raise typer.Exit(1) + return address + + +def _default_model_id_from_registry(ctx, console): + """The ID the registry's next approval will assign: DINModelRegistry.totalModels() (0-based).""" + registry = ctx.obj.get_deployed_din_registry_contract(verbose=False) + model_id = registry.functions.totalModels().call() + console.print( + f"[yellow]⚠ --model-id not given; using DINModelRegistry.totalModels() = {model_id}, the ID the next " + "approval assigns. This only holds if no other registration request is approved before yours.[/yellow]" + ) + return model_id + + +def _set_din_token(ctx, contract, din_token_address, label): + """Point a freshly deployed task contract at the platform DinToken (rewards and bonds need it).""" + build_and_send_tx( + ctx, + contract.functions.setDinToken(din_token_address), + f"Setting DinToken {din_token_address} on {label}", + f"DinToken set on {label}", + f"Failed to set DinToken on {label}", + ) + @deploy_app.command() def task_coordinator( ctx: typer.Context, artifact_path: str = typer.Option(..., "--artifact", help="Path to DINTaskCoordinator contract artifact JSON (Foundry format)."), - model_id: int = typer.Option(..., "--model-id", help="Model ID assigned by DinModelRegistry for this task contract."), + model_id: Optional[int] = typer.Option( + None, "--model-id", + help="Model ID DinModelRegistry will assign to this model (default: DINModelRegistry.totalModels()).", + ), ): """ - Deploy the DINTaskCoordinator contract. + Deploy the DINTaskCoordinator contract and set its DinToken. """ effective_network, w3, account, console = ctx.obj.get_en_w3_account_console() # Load contract instance DINTaskCoordinator_contract = get_contract_instance(artifact_path, effective_network) - # Resolve DINValidatorStake address - din_info = load_din_info() - if effective_network in din_info and "stake" in din_info[effective_network]: - din_validator_stake_address = din_info[effective_network]["stake"] + din_validator_stake_address = _require_din_info_address(effective_network, "stake", "DinValidatorStake", console) + din_token_address = _require_din_info_address(effective_network, "token", "DinToken", console) + if model_id is None: + model_id = _default_model_id_from_registry(ctx, console) console.print(f"[bold green]Deploying DINTaskCoordinator on network:[/bold green] {effective_network}") console.print(f"[cyan]Using DINValidatorStake:[/cyan] {din_validator_stake_address}") @@ -52,24 +91,29 @@ def task_coordinator( set_env_key(effective_network.upper()+"_DINTaskCoordinator_Contract_Address", dintaskcoordinator_contract_address) console.print(f"[green]✓ Saved DINTaskCoordinator address to {os.getcwd()}/.env as {effective_network.upper()}_DINTaskCoordinator_Contract_Address[/green]") + deployed_coordinator = get_contract_instance(artifact_path, effective_network, dintaskcoordinator_contract_address) + _set_din_token(ctx, deployed_coordinator, din_token_address, "DINTaskCoordinator") + @deploy_app.command() def task_auditor( ctx: typer.Context, artifact_path: str = typer.Option(..., "--artifact", help="Path to DINTaskAuditor contract artifact JSON (Foundry format)."), task_coordinator: str = typer.Option(None, "--taskCoordinator", help="DINTaskCoordinator contract address"), - model_id: int = typer.Option(..., "--model-id", help="Model ID assigned by DinModelRegistry for this task contract."), + model_id: Optional[int] = typer.Option( + None, "--model-id", + help="Model ID DinModelRegistry will assign to this model (default: the coordinator's modelId()).", + ), ): """ - Deploy the DINTaskAuditor contract. + Deploy the DINTaskAuditor contract, link it to the coordinator and set its DinToken. """ effective_network, w3, account, console = ctx.obj.get_en_w3_account_console() # Load contract instance DINTaskAuditor_contract = get_contract_instance(artifact_path, effective_network) - din_info = load_din_info() - if effective_network in din_info and "stake" in din_info[effective_network]: - din_validator_stake_address = din_info[effective_network]["stake"] + din_validator_stake_address = _require_din_info_address(effective_network, "stake", "DinValidatorStake", console) + din_token_address = _require_din_info_address(effective_network, "token", "DinToken", console) # Resolve DINTaskCoordinator address task_coordinator_address = resolve_task_coordinator_address( @@ -80,6 +124,9 @@ def task_auditor( # Both IDs select per-model stake floors; pairing is one-shot and irreversible. coord_view = w3.eth.contract(address=task_coordinator_address, abi=_MODEL_ID_ABI) coord_model_id = coord_view.functions.modelId().call() + if model_id is None: + model_id = coord_model_id + console.print(f"[cyan]--model-id not given; using the coordinator's modelId() = {model_id}[/cyan]") if coord_model_id != model_id: console.print( f"[bold red]Error:[/bold red] --model-id {model_id} does not match " @@ -107,7 +154,10 @@ def task_auditor( set_env_key(effective_network.upper() + "_"+task_coordinator_address+"_DINTaskAuditor_Contract_Address", dintaskauditor_contract_address) console.print(f"[green]✓ Saved DINTaskAuditor address to {os.getcwd()}/.env as {effective_network.upper()}_"+task_coordinator_address+"_DINTaskAuditor_Contract_Address[/green]") - + + deployed_auditor = get_contract_instance(artifact_path, effective_network, dintaskauditor_contract_address) + _set_din_token(ctx, deployed_auditor, din_token_address, "DINTaskAuditor") + # Set DINTaskAuditor in DINTaskCoordinator console.print(f"[cyan]Setting DINTaskAuditor in DINTaskCoordinator...[/cyan]") diff --git a/tests/test_deploy_task_contracts.py b/tests/test_deploy_task_contracts.py new file mode 100644 index 0000000..14d04d1 --- /dev/null +++ b/tests/test_deploy_task_contracts.py @@ -0,0 +1,154 @@ +""" +`model-owner deploy task-coordinator` / `task-auditor` (task_061026_21 Part C, Deploy): + +- `--model-id` defaults to DINModelRegistry.totalModels() for the coordinator, + and to the coordinator's modelId() for the auditor; +- both deploys call setDinToken(din_info token) on the new contract; +- a missing `stake` (or `token`) din_info entry exits cleanly instead of + raising NameError (or KeyError), before any transaction is sent. +""" +import time +from types import SimpleNamespace + +import pytest +import typer + +from dincli.cli.modelownerd import deploy + +COORD_ADDR = "0x000000000000000000000000000000000000000C" +AUDITOR_ADDR = "0x000000000000000000000000000000000000000A" +STAKE_ADDR = "0x0000000000000000000000000000000000000001" +TOKEN_ADDR = "0x0000000000000000000000000000000000000002" + + +class DummyConsole: + def __init__(self): + self.messages = [] + + def print(self, *args, **kwargs): + self.messages.append(" ".join(str(a) for a in args)) + + def text(self): + return "\n".join(self.messages) + + +class Call(tuple): + """A contract call as (name, *args); doubles as the tx passed to build_and_send_tx.""" + + +def _contract(address=None): + return SimpleNamespace( + address=address, + constructor=lambda *a: Call(("constructor", *a)), + functions=SimpleNamespace( + setDinToken=lambda addr: Call(("setDinToken", address, addr)), + setDINTaskAuditorContract=lambda addr: Call(("setDINTaskAuditorContract", addr)), + ), + ) + + +class DummyCtxObj: + def __init__(self, total_models=3, coord_model_id=3): + self.console = DummyConsole() + self._total_models = total_models + self.w3 = SimpleNamespace(eth=SimpleNamespace( + contract=lambda address, abi: SimpleNamespace(functions=SimpleNamespace( + modelId=lambda: SimpleNamespace(call=lambda: coord_model_id), + )), + )) + + def get_en_w3_account_console(self, model_id=None): + return "local", self.w3, SimpleNamespace(address="0xowner"), self.console + + def get_deployed_din_registry_contract(self, verbose=True): + return SimpleNamespace(functions=SimpleNamespace( + totalModels=lambda: SimpleNamespace(call=lambda: self._total_models), + )) + + def get_deployed_din_task_coordinator_contract(self, verbose=False, model_id=None, taskCoordinator_address=None): + return _contract(taskCoordinator_address) + + +@pytest.fixture +def din_info(monkeypatch): + info = {"local": {"stake": STAKE_ADDR, "token": TOKEN_ADDR}} + monkeypatch.setattr(deploy, "load_din_info", lambda: info) + return info["local"] + + +@pytest.fixture +def sent(monkeypatch, din_info): + calls = [] + + def fake_build_and_send_tx(ctx, fn, *args, **kwargs): + calls.append(fn) + if fn[0] == "constructor": + # coordinator ctor has 2 args, auditor ctor 3 + return SimpleNamespace(contractAddress=COORD_ADDR if len(fn) == 3 else AUDITOR_ADDR) + return SimpleNamespace(status=1) + + monkeypatch.setattr(deploy, "build_and_send_tx", fake_build_and_send_tx) + monkeypatch.setattr(deploy, "get_contract_instance", lambda path, net, address=None: _contract(address)) + monkeypatch.setattr(deploy, "resolve_task_coordinator_address", lambda net, addr, console: COORD_ADDR) + monkeypatch.setattr(deploy, "set_env_key", lambda *a, **kw: None) + monkeypatch.setattr(time, "sleep", lambda _: None) + return calls + + +def _ctx(**kw): + return SimpleNamespace(obj=DummyCtxObj(**kw)) + + +class TestCoordinatorDeploy: + def test_model_id_defaults_to_total_models_with_warning(self, sent): + ctx = _ctx(total_models=5) + deploy.task_coordinator(ctx, artifact_path="c.json", model_id=None) + assert sent[0] == ("constructor", STAKE_ADDR, 5) + assert "totalModels() = 5" in ctx.obj.console.text() + + def test_explicit_model_id_skips_registry(self, sent): + ctx = _ctx(total_models=5) + deploy.task_coordinator(ctx, artifact_path="c.json", model_id=2) + assert sent[0] == ("constructor", STAKE_ADDR, 2) + assert "totalModels()" not in ctx.obj.console.text() + + def test_sets_din_token_on_new_coordinator(self, sent): + deploy.task_coordinator(_ctx(), artifact_path="c.json", model_id=0) + assert sent == [("constructor", STAKE_ADDR, 0), ("setDinToken", COORD_ADDR, TOKEN_ADDR)] + + def test_missing_stake_exits_before_any_tx(self, sent, din_info): + del din_info["stake"] + ctx = _ctx() + with pytest.raises(typer.Exit) as exc: + deploy.task_coordinator(ctx, artifact_path="c.json", model_id=0) + assert exc.value.exit_code == 1 + assert sent == [] + assert "DinValidatorStake" in ctx.obj.console.text() + + +class TestAuditorDeploy: + def test_model_id_defaults_to_coordinator_model_id(self, sent): + deploy.task_auditor(_ctx(coord_model_id=4), artifact_path="a.json", task_coordinator=COORD_ADDR, model_id=None) + assert sent[0] == ("constructor", STAKE_ADDR, COORD_ADDR, 4) + + def test_sets_din_token_then_links_coordinator(self, sent): + deploy.task_auditor(_ctx(coord_model_id=0), artifact_path="a.json", task_coordinator=COORD_ADDR, model_id=0) + assert sent == [ + ("constructor", STAKE_ADDR, COORD_ADDR, 0), + ("setDinToken", AUDITOR_ADDR, TOKEN_ADDR), + ("setDINTaskAuditorContract", AUDITOR_ADDR), + ] + + def test_missing_token_exits_before_any_tx(self, sent, din_info): + del din_info["token"] + ctx = _ctx(coord_model_id=0) + with pytest.raises(typer.Exit): + deploy.task_auditor(ctx, artifact_path="a.json", task_coordinator=COORD_ADDR, model_id=0) + assert sent == [] + assert "DinToken" in ctx.obj.console.text() + + def test_missing_stake_exits_before_any_tx(self, sent, din_info): + del din_info["stake"] + with pytest.raises(typer.Exit): + deploy.task_auditor(_ctx(coord_model_id=0), artifact_path="a.json", task_coordinator=COORD_ADDR, model_id=0) + assert sent == [] diff --git a/tests/test_task_auditor_model_id_guard.py b/tests/test_task_auditor_model_id_guard.py index f8fcb7b..7d1e5cb 100644 --- a/tests/test_task_auditor_model_id_guard.py +++ b/tests/test_task_auditor_model_id_guard.py @@ -17,6 +17,7 @@ COORD_ADDR = "0x000000000000000000000000000000000000000C" AUDITOR_ADDR = "0x000000000000000000000000000000000000000A" STAKE_ADDR = "0x0000000000000000000000000000000000000001" +TOKEN_ADDR = "0x0000000000000000000000000000000000000002" class DummyConsole: @@ -46,7 +47,7 @@ def __init__(self, coord_model_id: int): self.console = DummyConsole() self._w3 = _make_w3(coord_model_id) - def get_en_w3_account_console(self): + def get_en_w3_account_console(self, model_id=None): return "local", self._w3, SimpleNamespace(address=COORD_ADDR), self.console def get_deployed_din_task_coordinator_contract(self, verbose=False, model_id=None, taskCoordinator_address=None): @@ -62,11 +63,14 @@ def _patch_deploy(monkeypatch): """Stub out I/O so tests run without real artifacts, chain, or filesystem.""" monkeypatch.setattr( deploy, "get_contract_instance", - lambda path, net: SimpleNamespace(constructor=lambda *a: "ctor"), + lambda path, net, address=None: SimpleNamespace( + constructor=lambda *a: "ctor", + functions=SimpleNamespace(setDinToken=lambda addr: ("setDinToken", addr)), + ), ) monkeypatch.setattr( deploy, "load_din_info", - lambda: {"local": {"stake": STAKE_ADDR}}, + lambda: {"local": {"stake": STAKE_ADDR, "token": TOKEN_ADDR}}, ) monkeypatch.setattr( deploy, "resolve_task_coordinator_address",