Skip to content

task_290626_2.md

Latest commit

 

History

History
289 lines (204 loc) · 13.5 KB

task_290626_2.md

File metadata and controls

289 lines (204 loc) · 13.5 KB
Raw

Task: PR #13 Final Fixes + Test Harness Integration + NatSpec + Foundry Migration Start

ID: task_290626_2
Assigned to: Robbert Abimbola
Created: 2026-06-29
Status: Open
Roadmap ref: P3-PR13 · P3-6.3b · Foundry migration (PR #15)
Branch: feature/platform-upgradeable (PR #13 fixes)
Week: 1 (July 1–3, 2026) — short week, Wednesday to Friday

Context

Three days, four focused pieces in priority order. Stop at the end of each part if time runs short — the ordering matters.

Environment Setup (do this before Day 1)

All paths below are from the lead engineer's machine and will not match yours. Before running anything, set up two Python venvs and update the constants file.

Two required venvs

pydin venv — used for all deploy/registry/wallet commands (no torch):

python3 -m venv <your-pydin-venv>
source <your-pydin-venv>/bin/activate
cd <your-devnet-root>
pip install -e .          # installs dincli from your local clone — do NOT use the GitHub hash from the freeze below
pip install pytest        # if not pulled in by editable install

Reference package list (versions to match — the -e git+... line becomes pip install -e . from your local clone):

aiohappyeyeballs==2.6.2, aiohttp==3.14.1, aiosignal==1.4.0, annotated-types==0.7.0,
attrs==26.1.0, base58==2.1.1, bitarray==3.8.2, blake3==1.0.8, certifi==2026.6.17,
charset-normalizer==3.4.7, ckzg==2.1.7, click==8.4.1, cytoolz==1.1.0,
eth-account==0.13.7, eth-hash==0.8.0, eth-keyfile==0.8.1, eth-keys==0.7.0,
eth-rlp==2.2.0, eth-typing==6.0.0, eth-utils==6.0.0, eth_abi==5.2.0,
frozenlist==1.8.0, hexbytes==1.3.1, idna==3.18, markdown-it-py==4.2.0,
mdurl==0.1.2, mmh3==5.2.1, morphys==1.0, multidict==6.7.1, packaging==26.2,
parsimonious==0.10.0, platformdirs==4.5.0, pluggy==1.6.0, propcache==0.5.2,
py-cid==0.5.0, py-multibase==2.0.0, py-multicodec==1.0.0, py-multihash==3.0.0,
pycryptodome==3.23.0, pydantic==2.13.4, pydantic_core==2.46.4, Pygments==2.20.0,
pytest==9.1.1, python-baseconv==1.2.2, python-dotenv==1.0.1, pyunormalize==17.0.0,
regex==2026.5.9, requests==2.34.2, rich==15.0.0, rlp==4.1.0, shellingham==1.5.4,
six==1.17.0, toolz==1.1.0, typer==0.20.0, types-requests==2.33.0.20260518,
typing-inspection==0.4.2, typing_extensions==4.15.0, urllib3==2.7.0, varint==1.0.2,
web3==7.10.0, websockets==15.0.1, yarl==1.24.2

torchenv venv — used for commands that invoke model training, aggregation, or auditor evaluation:

python3 -m venv <your-torchenv-venv>
source <your-torchenv-venv>/bin/activate
pip install torch==2.6.0 torchvision==0.21.0
pip install -e <your-devnet-root>   # same dincli editable install

Reference additional packages (above the base pydin list):

filelock==3.29.4, fsspec==2026.6.0, Jinja2==3.1.6, MarkupSafe==3.0.3,
mpmath==1.3.0, networkx==3.6.1, numpy==2.4.6, pillow==12.2.0,
setuptools==82.0.1, sympy==1.13.1, torch==2.6.0, torchvision==0.21.0,
triton==3.2.0
(+ nvidia-* CUDA packages if running on GPU)

Update tests/dincli/constants.py

The constants file has hardcoded paths from the lead engineer's machine. Update these four lines to match your setup before running the harness:

# tests/dincli/constants.py  — set these to your own paths
PYDIN_PYTHON    = "<your-pydin-venv>/bin/python"
TORCHENV_PYTHON = "<your-torchenv-venv>/bin/python"
ARTIFACT_BASE   = Path("<your-devnet-root>/hardhat/artifacts/contracts")
DIN_INFO_PATH   = Path("<your-devnet-root>/dincli/config/din_info.json")

Also check and update in tests/dincli/conftest.py:

DIN_TEMP  = Path("<a writable temp dir on your machine>")   # e.g. Path.home() / "tempdir/dincli"
NPX_BIN   = "<path to npx>"    # find with: which npx
IPFS_BIN  = "<path to ipfs>"   # find with: which ipfs

Check prerequisites are installed and running

which npx          # Node / Hardhat
which ipfs         # IPFS daemon
docker info        # Docker daemon running

Part 1 — PR #13 Final Fixes

IF you believe that these potential fixes are already addressed, stale requirements, non-issue plese point out

Note: DinValidatorStake.test.ts (Fix 1 from the original review) is already addressed in your branch — 231 lines, 16 tests. The remaining items are:

Fix 1: daoAdmin / owner compatibility on DINModelRegistry

Add daoAdmin() → owner() and setDAOAdmin(address) → transferOwnership(address) wrappers (shim approach), OR explicitly document the clean-break decision in the PR description with a hard tracked prerequisite on the dincli side. Pick whichever is cleaner; flag the decision and rationale.

Fix 2: dincli ABI call-site list in PR description

Written list of every dincli call site that breaks under the ABI changes in this PR. Not fixes — enumeration only. At minimum:

  • daoAdmin() on DINModelRegistry — removed; dincli calls this today
  • DinCoordinator/DinToken two-step bootstrap — any path that assumed dinToken = coordinator.dinToken() after a single deploy needs the new two-step wire
  • DAOAdminUpdated event listeners — removed from DINModelRegistry

Fix 3: _disableInitializers() tests × 4

One test per contract proving that calling initialize() directly on the implementation contract (not the proxy) reverts. Add to the existing upgrade-safety suite.

Addition: uint256[50] private __gap × 4 platform contracts

Add storage gap to DinCoordinator, DinToken, DinValidatorStake, DINModelRegistry. Known omission from the current branch — fix it before merge, not after.

Outcome: PR #13 ready for merge gate review by end of July 1.

Part 2 — Test Harness Integration (Day 2: July 2)

The develop branch now has a full CLI-level integration test harness at tests/dincli/ (task_290626_1, merged). Your PR branch does not have this yet.

Step 1: Sync your branch with develop

git fetch origin
git rebase origin/develop
# or merge if rebase produces conflicts that are harder to resolve

Step 2: Run the harness against your upgradeable contracts

Prerequisites (from tests/dincli/conftest.py — these are auto-managed but Docker must be running):

# Ensure Docker daemon is running
docker info   # if not running: start Docker Desktop or sudo systemctl start docker

# Activate your pydin venv
source <your-pydin-venv>/bin/activate

# From your devnet root
cd <your-devnet-root>
pytest tests/dincli/ -v -x -m integration --tb=short \
    2>&1 | tee <your-din-temp>/results/pr13_run.txt

The harness resolves paths from tests/dincli/constants.py and conftest.py — update those first (see Environment Setup above). The conftest auto-manages Hardhat node startup, contract compilation, and IPFS daemon; Docker must already be running.

Step 3: Re-upload IPFS artifacts

Before running, ensure the IPFS daemon is running and re-upload manifests and service files so CIDs in din_info.json / manifest.json are current:

# From your devnet root, with pydin venv active and IPFS daemon running
python -m dincli.main ipfs upload -f ./cache_model_0/manifest.json
python -m dincli.main ipfs upload -f ./cache_model_0/requirements.compact.txt
python -m dincli.main ipfs upload -f ./cache_model_0/requirements.txt
# All 6 service files under cache_model_0/services/
python -m dincli.main ipfs upload -f ./cache_model_0/services/model.py
# (repeat for aggregator.py, auditor.py, client.py, modelowner.py, validator.py)

# Task-level contract ABIs
python -m dincli.main ipfs upload -f ./hardhat/artifacts/contracts/DINTaskCoordinator.sol/DINTaskCoordinator.json
python -m dincli.main ipfs upload -f ./hardhat/artifacts/contracts/DINTaskAuditor.sol/DINTaskAuditor.json

Step 4: Document failures in PR description

For each test that fails against your upgradeable contracts, add it to the call-site list from Part 1. This is the evidence Umer needs to update dincli/cli before the merge gate is cleared. You are not fixing Python — you are documenting what breaks.

Outcome: PR description has a complete, evidence-backed call-site list from a real harness run.

Part 3 — P3-6.3b NatSpec (Day 3 — first half: July 3)

Add NatSpec to all public and external functions across all 6 contracts and DINShared.sol. Do this while the code is fresh.

Scope:

File __gap needed Notes
DinCoordinator.sol ✅ Yes (upgradeable) Already added in Part 1
DinToken.sol ✅ Yes (upgradeable) Already added in Part 1
DinValidatorStake.sol ✅ Yes (upgradeable) Already added in Part 1
DINModelRegistry.sol ✅ Yes (upgradeable) Already added in Part 1
DINTaskCoordinator.sol ❌ No (redeployed per model) NatSpec only
DINTaskAuditor.sol ❌ No (redeployed per model) NatSpec only
DINShared.sol ❌ No (types/interfaces) @notice on interfaces + @dev on custom errors

NatSpec format:

/// @notice One-line description of what this function does for a caller.
/// @param paramName What this parameter controls.
/// @return What is returned and what it represents.
/// @dev Internal implementation note — invariants, edge cases, or non-obvious constraints.

Also write Documentation/technical/storage_layout.md documenting the append-only storage rules followed in the 4 upgradeable contracts — this is a required deliverable for P3-6.3b and feeds directly into audit prep.

Part 4 — Foundry Migration Start, per PR #15 (Day 3 — second half: July 3)

PR #15 logs a conditional Foundry-only recommendation. Two checklist items remain open before the final call can be made. Work through them.

Gate 1: openzeppelin-foundry-upgrades storage-layout validation

Install the plugin and run a dry-run upgrade-safety check against PR #13's Transparent Proxy contracts:

cd foundry
forge install OpenZeppelin/openzeppelin-foundry-upgrades
# or: add to foundry.toml [dependencies]

Then write a minimal test (.t.sol) that calls Upgrades.validateUpgrade(...) (or equivalent) against each of the 4 proxy contracts and confirm it passes. This mirrors what hardhat-upgrades validateUpgrade() does in the existing Hardhat suite. If it passes, the primary gate is cleared.

Gate 2: forge verify-contract on Optimism Sepolia Blockscout

If a deployed contract address is available (even a testnet one), verify:

forge verify-contract <address> <ContractName> \
    --verifier blockscout \
    --verifier-url https://optimism-sepolia.blockscout.com/api

Document the result (success or exact failure) in the PR #15 description update.

Update stale foundry/src/ contracts

foundry/src/ currently holds 7 pre-upgrade, non-proxy contracts that diverged from hardhat/contracts/ before PR #13. Update them to match the upgradeable versions from your branch (initialize(), OwnableUpgradeable, removed immutables, __gap). No new Foundry test files required this week — that is Week 2 scope.

Outcome: PR #15 checklist complete; conditional recommendation confirmed or revised with evidence; foundry/src/ contracts current.

Part 4 — Status: Done (merged 2026-07-04)

PR #15 (docs/foundry-migration-findings) merged into develop via merge commit 0f7a6e6, no conflicts (GitHub reported CLEAN/MERGEABLE).

  • Gate 1 (storage-layout validation): PASS — openzeppelin-foundry-upgrades v0.4.1 installed as a submodule; foundry/test/UpgradeValidation.t.sol calls Upgrades.validateImplementation() against all 4 Transparent Proxy contracts.
  • Gate 2 (Blockscout verification): endpoint confirmed reachable, blocked on a live deployment address — documented in Developer/discussion/migrate_to_foundy.md.
  • foundry/src/ — all 7 contracts synced to the upgradeable versions from feature/platform-upgradeable (Initializable, OwnableUpgradeable, _disableInitializers(), __gap on the 4 platform contracts).
  • Recommendation in Developer/discussion/migrate_to_foundy.md upgraded from conditional to Foundry-only (2026-07-03 decision log entry).

Deliverables

By end of July 1 (Part 1):

  • daoAdmin shim added OR clean-break decision documented in PR #13
  • dincli ABI call-site list written into PR #13 description
  • _disableInitializers() tests × 4 added
  • uint256[50] private __gap added to all 4 platform contracts

By end of July 2 (Part 2):

  • feature/platform-upgradeable rebased onto current develop
  • Test harness run against upgradeable contracts, results logged to ~/tempdir/dincli/results/pr13_run.txt
  • Harness failures documented in PR #13 description (complete call-site list)

By end of July 3 (Parts 3–4):

  • NatSpec on all public/external functions — 6 contracts + DINShared.sol
  • Developer/Documentation/technical/storage_layout.md written
  • openzeppelin-foundry-upgrades installed; storage-layout gate run and result documented (PR #15, merged)
  • forge verify-contract result documented (pass or failure with exact error) (PR #15, merged — blocked on live address)
  • foundry/src/ contracts updated to match PR #13 upgradeable versions (PR #15, merged)

Notes

  • All Solidity / Hardhat / Foundry. Python and dincli changes (CLI fixes for broken call sites) are Umer's scope — your job is to document what breaks, not fix it.
  • PR #15 is pending Umer's review. Proceed on the assumption Hardhat remains active alongside Foundry until the migration decision is final — do not remove Hardhat tests.
  • If Parts 3–4 spill into Monday, prioritize in order: NatSpec before Foundry gates.