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",