Skip to content

task_300726_8.md

Latest commit

 

History

History
152 lines (113 loc) · 15.7 KB

task_300726_8.md

File metadata and controls

152 lines (113 loc) · 15.7 KB
Raw

Task: SDK Wallet/Session/Tx Signing Keystone (Issue #20 continued)

ID: task_300726_8 Assigned to: Santiago Rodriguez Cetran (@Santiagocetran) Created: 2026-07-30 Status: Open Repo: https://github.com/InfiniteZeroFoundation/DevNet Base branch: feat/din-sdk (PR #31, draft) — feat/din-daemon (PR #32) stays stacked on top, no changes expected there this task Roadmap ref: P4-1.2 (remaining slice); resolves BL-2, addresses BL-1 as part of tx.py Dates: Jul 31–Aug 6, 2026 (5 working days; deadline Thursday, Aug 6)

Context

This is the task flagged as "next" in three places: task_220726_7.md's scope boundary #1 and closing note, Santiago's own progress updates on Discussion #50 ("deferred next task, flagged for @umeradl — the wallet/session/tx keystone"), and Umer's proposal review on issue #20 ("§2, §5b's core design... good to build on"). Both Part A and Part B of task_220726_7 landed clean (PR #31 @ 6eac109, PR #32 @ 99f060b) with no scope adjustments needed, and their code reviews (pr31_code_review.md, pr32_code_review.md) found no correctness blockers — see those docs for the full independent verification if useful background.

What this task builds, per Developer/design/sdk-interface-proposal.md §2, §5b (approved, with the open questions below to resolve as you go — not blockers to starting):

  • dincli/sdk/session.py — DinSession + the SignerProvider protocol (§2)
  • dincli/sdk/wallet.py — non-interactive keystore/account resolution (§8's package layout: "from utils.py + system.connect_wallet")
  • dincli/sdk/tx.py — send() + decode_events() + tx-param building, replacing utils.py::build_and_send_tx (§5b)

Deliberately not in this task: the sdk/operations/ role-command layer (registry.py, gi.py, aggregation.py, auditor.py, client.py, model.py, dao.py) and the plan/apply confirmation pattern (§6) that sits on top of it. Those consume DinSession/send() but are a substantial, separately-scoped follow-on — flag as the next task rather than starting early, same convention as this task itself was deferred out of task_220726_7. dind's JOB_HANDLERS registry stays a no-op "demo" entry until operations/ lands (see BL-9).

Scope boundaries — read before starting:

  1. This is the security-sensitive slice — private key handling, signing, transaction submission. Take the same care task_220726_7 explicitly deferred this for. If anything about the keystore/signing flow feels underspecified once you're in the code, stop and ask rather than guessing at a security-relevant default.
  2. No CLI command behavior change. cli/utils.py::build_and_send_tx, system.py::connect_wallet, and every caller of load_account keep their current exit codes and printed output — same re-export-shim pattern as the prior three extraction waves. dincli/sdk/tx.py/wallet.py/session.py are additive; cli/* becomes a thin wrapper over them.
  3. operations/ and plan/apply (§6) are out of scope, per above — don't start them even if the week's budget allows.
  4. dind gets no new job handlers. dincli/dind/jobs.py's JOB_HANDLERS registry stays untouched — daemon automation that actually signs/submits is downstream of operations/, not this task.
  5. Daemon-side config/reload commands are out of scope for implementation, but see item 1 below — the doc gap Umer flagged needs fixing regardless of whether dind config/dind load config get built this week.

1. dincli/sdk/session.py — DinSession + SignerProvider (§2)

Per proposal §2 (already approved):

# dincli/sdk/session.py
class DinSession:
    """Lazily resolves network, web3, account, config. No printing, no exit, no prompts."""
    def __init__(self, network: str | None = None, wallet: str | None = None,
                 signer: "SignerProvider | None" = None): ...

    @property
    def network(self) -> str: ...
    @property
    def w3(self) -> Web3: ...            # raises NetworkError if RPC unreachable

    @property
    def account(self) -> LocalAccount:
        """Resolve the signing account via the configured SignerProvider.
        NEVER calls getpass or any interactive prompt. Raises SignerUnavailable
        if no non-interactive credential is available, WalletError if the
        keystore is malformed or missing."""

    @property
    def config(self) -> dict: ...

@runtime_checkable
class SignerProvider(Protocol):
    def address(self) -> str: ...          # raises SignerUnavailable if none resolvable
    def can_decrypt(self) -> bool: ...      # True if usable key resolvable without further input
    def sign_transaction(self, tx: dict) -> SignedTransaction: ...
        # raises SignerUnavailable (no credential), WalletError (bad keystore/decrypt failure)

This replaces the runtime-resolution parts of DinContext (cli/context.py) minus console/exit/prompting — read DinContext end-to-end first to confirm exactly which resolution logic (network, w3, account, config) moves here vs. what's already SDK-side (sdk/web3.py::get_w3, sdk/config.py).

Two adapters, not implemented in the SDK itself:

  • CLI adapter (lives in cli/, wraps SignerProvider) — wraps the existing keystore flow in cli/utils.py::load_account; it is allowed to prompt via getpass (that's the interactive boundary), then hands the resolved signer into DinSession.
  • Daemon adapter (lives in dincli/dind/) — backed by a configured password source, must never prompt; any missing credential raises SignerUnavailable, which dind turns into a job error rather than a hang.

Doc/design gap to close as part of this work (from Umer's §2 review on issue #20, not yet reflected in the proposal doc): dind start prompts interactively for wallet/password once at process start, and the resolved signer stays in memory for that daemon's lifetime. Config can be added/changed later while the daemon is running, but only through a separate dind config command (interactive there is fine, same pattern as dincli system connect-wallet) plus a non-interactive dind load config trigger that just points the running daemon at already-resolved state — the daemon process itself never blocks on stdin mid-run. Document the daemon SignerProvider adapter against that session-held, interactively-bootstrapped state, not a raw env-var lookup, in whatever module/docstring ends up describing the adapter. You don't have to build dind config/dind load config this task (scope boundary #5) — just make sure the adapter's contract and docstring don't imply raw env-var-only resolution, since that's the gap Umer flagged.

2. dincli/sdk/wallet.py — non-interactive keystore/account resolution

Per §8's package layout: "keystore/account/demo-key, non-interactive signing (from utils.py + system.connect_wallet)". Grep cli/utils.py and cli/system.py for the current keystore/account/demo-key logic (load_account and anything connect-wallet depends on) and extract the non-interactive parts here — same verbatim-extraction discipline as state.py in task_220726_7 (snapshot current behavior before moving, diff after). The interactive getpass prompt itself stays in the CLI adapter (item 1), not in wallet.py.

Concretely implement the SignerProvider contract from item 1 against the existing keystore format (§7's backward-compat guarantee: no changes to on-disk keystore format/location).

3. dincli/sdk/tx.py — send() keystone (§5b)

# dincli/sdk/tx.py
@dataclass
class TxReceiptInfo:
    tx_hash: str                   # 0x hex
    status: int                    # 1 success, 0 revert
    block_number: int
    gas_used: int
    nonce: int
    contract_address: str | None   # set for deployment txs (deploy flows read this)
    logs: list[dict]               # raw, JSON-serializable log entries
    _raw: "TxReceipt" = field(repr=False, metadata={"json": "omit"})
        # web3 AttributeDict receipt, kept for in-SDK event decoding; NOT serialized

def send(session: DinSession, contract_function, *, tx_params: dict | None = None,
         on_event: Callable[[str, dict], None] | None = None) -> TxReceiptInfo:
    """Estimate -> build -> sign -> send -> wait. Returns receipt info.
    Raises TransactionError with subcode tx_estimation_failed | tx_reverted | tx_timeout |
    tx_nonce_conflict | tx_replacement_underpriced | receipt_missing (sdk/errors.py)."""

def decode_events(receipt: TxReceiptInfo, contract_event) -> list[dict]:
    """Decode a specific event from a receipt, e.g. registry.events.ModelRegistrationRequested().
    Wraps web3's process_receipt so a future operations layer can build typed results
    without exposing raw web3 objects to consumers."""

Replaces cli/utils.py::build_and_send_tx — that function becomes a thin wrapper calling send() and mapping TransactionError to today's console.print + typer.Exit(1) behavior (scope boundary #2: identical CLI exit codes/output).

Retry-safety shape (§10, already resolved): on failure, TransactionError.details must carry tx_hash if broadcast, nonce if known, and a broadcast: bool flag distinguishing pre-broadcast (safe to rebuild) from post-broadcast (must confirm the existing tx, don't resend) failures. On success, TxReceiptInfo carries tx_hash/nonce/status — that's the full retry-decision surface a future daemon job-retry policy needs.

Four open design questions from Umer's §5b review — resolve these as you build, not upfront in a separate design doc:

  1. Nonce management (BL-1, weigh this one heaviest). context.py::get_tx_params() currently calls get_transaction_count(address) with no block param (defaults to "latest", confirmed-only) — two transactions fired back-to-back before the first confirms collide on the same nonce today. Decide and document: does send() itself prevent this for concurrent submission (e.g. track an in-process nonce counter, or use "pending" block param), or is that a layer above send()'s responsibility? Whichever you pick, fix the live gap in get_tx_params()'s nonce fetch as part of this task — don't leave it as a documented-but-unfixed known issue.
  2. on_event callback vocabulary. The proposal only shows "submitted" as an example event name. Define the full set send() actually emits (e.g. submitted, confirmed, estimation_failed, reverted — your call on exact names) and document it in the send() docstring so CLI and daemon consumers both know what to listen for.
  3. Timeout/polling parameterization. send()'s signature above has no timeout/poll-interval params, but CLI (interactive, human waiting) and daemon (background job, different patience) likely want different wait behavior. Decide whether to add explicit params (e.g. timeout_s, poll_interval_s) with sensible defaults, or handle this a different way — document the choice.
  4. Replacement transactions. tx_replacement_underpriced is a defined error subcode but the replacement flow itself isn't designed. Decide: is replacing a stuck tx a fresh send() call with a manually bumped nonce/gas (caller's responsibility), or does tx.py need its own replace() function? A fresh-send()-with-bumped-params approach is probably enough for this task — don't over-build a dedicated replacement flow unless the four-question review implies otherwise once you're in the code.

4. Tests

  • tests/test_sdk_session.py (new): DinSession never prompts (assert no getpass/stdin access — same style as the existing import-boundary tests); SignerProvider protocol conformance; SignerUnavailable/WalletError raised on the documented conditions, not silently swallowed.
  • tests/test_sdk_wallet.py (new): keystore load/decrypt round-trip against the existing on-disk format (no format change, per scope boundary #2); non-interactive resolution never calls getpass.
  • tests/test_sdk_tx.py (new): send()'s happy path (mock web3, assert TxReceiptInfo fields populate correctly, including contract_address on a deployment-shaped mock and logs on an event-emitting one); each TransactionError subcode raised on its corresponding failure mode (estimation failure, revert, timeout, nonce conflict); TransactionError.details carries the right broadcast: bool/tx_hash/nonce shape per §10 on at least the pre-broadcast and post-broadcast failure cases; decode_events() against a mock receipt.
  • Extend tests/test_sdk_boundary.py's fresh-subprocess import check to include session, wallet, tx in the module list asserted clean of typer/rich/dincli.cli.*.
  • Golden CLI-compatibility check (per proposal §7): existing build_and_send_tx callers (dinrep.py, modelownerd/deploy.py, task.py, etc.) keep identical exit codes and printed output once wrapped over send() — spot-check at least the deploy and one registry-write call site by hand, don't just trust the existing test suite catches every regression here given how security-sensitive this path is.
  • Full suite green on both feat/din-sdk and feat/din-daemon before opening/updating PRs, same convention as task_220726_7.

Deliverables

  • dincli/sdk/session.py — DinSession + SignerProvider per §2, with the daemon-adapter doc gap (session-held bootstrapped state, not raw env lookup) fixed in the adapter's docstring/contract
  • dincli/sdk/wallet.py — non-interactive keystore/account resolution extracted from cli/utils.py + cli/system.py, old call sites re-exported as a shim
  • dincli/sdk/tx.py — send() + decode_events() + tx-param building per §5b, replacing build_and_send_tx's internals; cli/utils.py::build_and_send_tx becomes a thin wrapper with identical exit codes/output
  • Nonce-fetch gap (BL-1) fixed as part of send()/get_tx_params(), with the chosen approach documented
  • The three remaining §5b open questions (event vocabulary, timeout/polling, replacement flow) resolved and documented in code/docstrings, not left as open questions in a follow-up doc
  • New SDK tests per section 4; import-boundary test extended to cover session, wallet, tx
  • Existing test suite still green on both PRs — no CLI behavior change
  • PR #31 updated with the new commits; PR body notes this slice covers §2/§5b of the SDK proposal
  • Discussion posted on #50, referencing this task file, flagging sdk/operations/ + plan/apply (§6) as the explicitly-deferred next slice

Notes

  • Release valve if the 5-day budget runs tight: the replacement-transaction question (item 3, #4) is the most deferrable — a documented "not yet implemented, caller must construct a fresh send() call with bumped params" is an acceptable placeholder if time runs out, since nothing calls it yet. Do not cut the nonce-management fix (BL-1) or the core send()/SignerProvider contract — those are load-bearing for everything downstream, including the operations/ layer's next task.
  • The next task after this one is sdk/operations/ + plan/apply (§6). Per Umer's §6 review, the priority order there is aggregator/auditor flows first (least configuration, dind's primary automation surface), then gi.start, with register_request/client/model-owner tooling explicitly lower priority (separate tooling, not dind's job — see BL-4/BL-5). Flag it in the closing discussion post rather than starting early, same as this task was flagged rather than started under task_220726_7.
  • Re-pull and re-run the full suite on feat/din-sdk right before opening/updating the PR, per the existing convention — develop's tip may have moved.