From ade05d3bb0ab252d5a8abb80055b489c3c769080 Mon Sep 17 00:00:00 2001 From: Chris Li Date: Tue, 30 Jun 2026 02:17:25 -0700 Subject: [PATCH 01/16] refactor(rest): address cosmetic review nits on partner auth MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Non-blocking follow-ups from the #654 re-review (no behavior change): - partnerPrincipal fields lowercased (partnerID/subject) — package-private type that never leaves the package. - Drop the no-op WithValidMethods on the ParseUnverified preview parse; add a comment that alg is enforced on the real jwt.Parse verify below. - Remove the now-unneeded `key := pk` loop copy (Go 1.22+ per-iteration binding). - Test: cover RawURLEncoding in the key-decode test (prod already accepts it). --- aggregator/rest/partner.go | 27 ++++++++++++++------------- aggregator/rest/partner_test.go | 3 ++- 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/aggregator/rest/partner.go b/aggregator/rest/partner.go index 3a7bdb03..74dcd48f 100644 --- a/aggregator/rest/partner.go +++ b/aggregator/rest/partner.go @@ -58,11 +58,12 @@ const ( ) // partnerPrincipal is the result of a verified partner assertion: which -// partner vouched (PartnerID) and the end-user it vouched for (Subject, -// possibly empty or a non-address identifier). +// partner vouched (partnerID) and the end-user it vouched for (subject, +// possibly empty or a non-address identifier). Package-private — fields stay +// lowercase since it never leaves this package. type partnerPrincipal struct { - PartnerID string - Subject string + partnerID string + subject string } // requireSimulateAuth authorizes a simulate-family request via EITHER an @@ -91,12 +92,12 @@ func (s *Server) requireSimulateAuth(ctx echo.Context) (*model.User, error) { // `sub` is attribution only. Use it as the acting address when it's // a real EOA; otherwise leave the zero address — simulate runs // against any address and never checks ownership. - if common.IsHexAddress(principal.Subject) { - user.Address = common.HexToAddress(principal.Subject) + if common.IsHexAddress(principal.subject) { + user.Address = common.HexToAddress(principal.subject) } s.logger.Info("partner-delegated simulate call", - "partner_id", principal.PartnerID, - "subject", principal.Subject, + "partner_id", principal.partnerID, + "subject", principal.subject, ) return user, nil } @@ -149,9 +150,10 @@ func (s *Server) verifyPartnerAssertion(ctx echo.Context, requiredScope string) } // Read the issuer without verifying so we can select the partner's keys. - parser := jwt.NewParser(jwt.WithValidMethods([]string{"EdDSA"})) + // No alg enforcement here — ParseUnverified validates nothing; the EdDSA + // requirement is enforced on the real verify (jwt.Parse) below. preview := jwt.MapClaims{} - if _, _, err := parser.ParseUnverified(raw, preview); err != nil { + if _, _, err := jwt.NewParser().ParseUnverified(raw, preview); err != nil { return nil, partnerError(http.StatusUnauthorized, "PARTNER_ASSERTION_MALFORMED", "Malformed partner assertion", err.Error()) } @@ -176,8 +178,7 @@ func (s *Server) verifyPartnerAssertion(ctx echo.Context, requiredScope string) // Verify the signature against each registered key (rotation support). var verified *jwt.Token for _, pk := range pubKeys { - key := pk - tok, verr := jwt.Parse(raw, func(*jwt.Token) (any, error) { return key, nil }, + tok, verr := jwt.Parse(raw, func(*jwt.Token) (any, error) { return pk, nil }, jwt.WithValidMethods([]string{"EdDSA"})) if verr == nil && tok.Valid { verified = tok @@ -227,7 +228,7 @@ func (s *Server) verifyPartnerAssertion(ctx echo.Context, requiredScope string) } subject, _ := claims["sub"].(string) - return &partnerPrincipal{PartnerID: issuer, Subject: strings.TrimSpace(subject)}, nil + return &partnerPrincipal{partnerID: issuer, subject: strings.TrimSpace(subject)}, nil } // lookupPartner returns the registered partner whose ID matches id, or nil. diff --git a/aggregator/rest/partner_test.go b/aggregator/rest/partner_test.go index 41a8b1e2..ccab7f5f 100644 --- a/aggregator/rest/partner_test.go +++ b/aggregator/rest/partner_test.go @@ -138,6 +138,7 @@ func TestDecodeEd25519Keys_TolerantAndMultiEncoding(t *testing.T) { std, base64.RawStdEncoding.EncodeToString(pub), base64.URLEncoding.EncodeToString(pub), + base64.RawURLEncoding.EncodeToString(pub), "ed25519:" + std, } { if _, err := decodeEd25519Key(enc); err != nil { @@ -270,7 +271,7 @@ func TestVerifyPartnerAssertion_Audience(t *testing.T) { if err != nil { t.Fatalf("expected success, got: %v", err) } - if principal == nil || principal.PartnerID != "studio" { + if principal == nil || principal.partnerID != "studio" { t.Fatalf("expected studio principal, got %+v", principal) } }) From b3ada41cf1df25a82d7c6658057cb24f880b4caa Mon Sep 17 00:00:00 2001 From: Chris Li Date: Tue, 30 Jun 2026 12:05:58 -0700 Subject: [PATCH 02/16] docs: fix stale make targets in CLAUDE.md dev-build/dev-agg/dev-op do not exist. The local gateway is run with make gateway (logs -> gateway.log) or make dev-gateway; operators via make dev-operator-sepolia; full local stack via make dev-stack. --- CLAUDE.md | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 5a5b7cb5..0089ea50 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -9,12 +9,14 @@ Ava Protocol Automation AVS (Actively Validated Service) on EigenLayer - a task ### Building and Running ```bash -make build # Build main binary -make dev-build # Build development version -make dev-agg # Run aggregator locally -make dev-op # Run operator locally -make up # Start docker compose stack -make dev-live # Live reload during development +make build # Build main binary (-> out/ap) +make build-prod # Build production binary +make gateway # Run local-dev gateway (config/gateway.yaml); logs -> gateway.log +make dev-gateway # Same, logs -> logs/gateway.log +make dev-operator-sepolia # Run a Sepolia operator locally +make dev-stack # gateway + workers + operator together (logs/) +make up # Start docker compose stack +make dev-live # Live reload during development ``` ### Testing From d1e488e27e61745ce302e2335f9428689aea995e Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 4 Jul 2026 06:55:15 +0000 Subject: [PATCH 03/16] ci: move unit test workflow back to GitHub-hosted runners The Run Unit Tests workflow's jobs (Check Changed Files, Lint, and the 13-package Unit Test matrix) mostly finish in under a minute, so Blacksmith's faster 4-vCPU runners have little wall-clock advantage there while still billing at the 4-vCPU rate on every push. Revert these jobs to ubuntu-latest, undoing #505 for this workflow only. The Docker image publish workflows (~4-5 min builds) stay on Blacksmith where the faster hardware genuinely pays off. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01Un7opV3NNydU4EEHD4pjAV --- .github/workflows/run-test-on-pr.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/run-test-on-pr.yml b/.github/workflows/run-test-on-pr.yml index 90e6ff0d..fe5a390a 100644 --- a/.github/workflows/run-test-on-pr.yml +++ b/.github/workflows/run-test-on-pr.yml @@ -36,7 +36,7 @@ on: jobs: check-changes: name: Check Changed Files - runs-on: blacksmith-4vcpu-ubuntu-2404 + runs-on: ubuntu-latest outputs: should_run: ${{ steps.check.outputs.should_run }} steps: @@ -78,7 +78,7 @@ jobs: name: Lint needs: check-changes if: needs.check-changes.outputs.should_run == 'true' - runs-on: blacksmith-4vcpu-ubuntu-2404 + runs-on: ubuntu-latest steps: - uses: actions/checkout@v5 with: @@ -122,7 +122,7 @@ jobs: name: Unit Test needs: check-changes if: needs.check-changes.outputs.should_run == 'true' - runs-on: blacksmith-4vcpu-ubuntu-2404 + runs-on: ubuntu-latest strategy: fail-fast: false matrix: From f9e215f041c25db7af20d545bd0f65bee1ef77ee Mon Sep 17 00:00:00 2001 From: Chris Li Date: Thu, 9 Jul 2026 23:52:08 -0700 Subject: [PATCH 04/16] docs: plan on-demand single-node action backend gaps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Analysis of the EigenLayer-AVS backend gaps for chat-driven, one-time single-node (contractWrite) execution — the preview→confirm→execute contract over nodes:run. Verified against the code with file:line refs: - G1: native ETH value dropped on the real path (PackExecute hardcodes 0) - G2: no typed tx identity / return value on real executes - G3: pending UserOp reported as failure; nodes:run is stateless - G4: approve+swap are non-atomic (2 UserOps), partial on-chain side effects - G5: execute is not idempotent (retried confirm re-broadcasts) - G6: runner salt not propagated on the nodes:run real path - G7: no execution fee on nodes:run (pricing decision) Scope: delegated execution (no per-tx user signature); server-side spend policy deferred; studio owns call construction + market-order helper + UX. --- PLAN_AGENT_ONDEMAND_ACTIONS.md | 167 +++++++++++++++++++++++++++++++++ 1 file changed, 167 insertions(+) create mode 100644 PLAN_AGENT_ONDEMAND_ACTIONS.md diff --git a/PLAN_AGENT_ONDEMAND_ACTIONS.md b/PLAN_AGENT_ONDEMAND_ACTIONS.md new file mode 100644 index 00000000..c68ec4be --- /dev/null +++ b/PLAN_AGENT_ONDEMAND_ACTIONS.md @@ -0,0 +1,167 @@ +# PLAN: On-Demand Single-Node Actions — EigenLayer-AVS backend gaps + +- **Date**: 2026-07-09 +- **Owner**: Chris +- **Status**: Proposed +- **Anchor repo**: `EigenLayer-AVS` (this doc's focus). `studio` owns the chat UX **and** constructs the `contractWrite` call (intent → node JSON, quotes, slippage, the market-order helper). `ava-sdk-js` only needs to surface the richer response this plan adds. +- **Motivating use case**: the studio LLM agent watches a user's portfolio and, on-demand through chat, **previews then executes a one-time Uniswap market order** by calling a **single node** (`nodes:run`), not by deploying a workflow. + +--- + +## 0. Scope & decisions (read first) + +**What's decided (so this plan can be concrete):** + +1. **Custody model: delegated execution.** Reuse the existing model — the runner smart wallet is controlled by the aggregator controller key; `nodes:run` with `is_simulated=false` executes for real with **no per-transaction user signature**. We are *not* building a non-custodial "return an unsigned tx for the user to sign" path. +2. **Defer the server-side spend/policy gate.** "Make the function work first." The confirmation gate is the studio UX (user clicks Confirm) plus the existing ownership check; a server-side spend-cap/fund-authorization policy is a **later** hardening pass, explicitly out of scope here. (Tracked in the old §policy notes; not v1.) +3. **Studio owns call construction and the market-order helper.** The backend does **not** grow a Uniswap/market-order helper or a new `uniswapSwap` node type. Studio assembles the `contractWrite` node (router address, `exactInputSingle` tuple, `amountOutMinimum`, approve) and drives preview→confirm→execute. The backend's job is to **run a single `contractWrite` node correctly in both modes and return a usable result.** + +**What this plan is, therefore:** a precise list of the **EigenLayer-AVS backend gaps** that stand between "studio can build the call" and "preview faithfully predicts execute, and execute returns something studio can act on." Everything below is verified against the code. + +--- + +## 1. The single-node path as it exists today (verified) + +`POST /api/v1/nodes:run` → `Server.RunNode` ([aggregator/rest/handlers_nodes.go:23](aggregator/rest/handlers_nodes.go#L23)) → `Engine.RunNodeImmediatelyRPCWithContext` ([core/taskengine/run_node_immediately.go:2996](core/taskengine/run_node_immediately.go#L2996)) → `runProcessingNodeWithInputs` → `VM.RunNodeWithInputs` ([core/taskengine/vm.go:2477](core/taskengine/vm.go#L2477)) → `ContractWriteProcessor.Execute` ([core/taskengine/vm_runner_contract_write.go:1450](core/taskengine/vm_runner_contract_write.go#L1450)). + +Confirmed behaviors that make this usable **today**: + +- **Real execution is wired and reachable.** `useSimulation` defaults to `true` but honors `is_simulated=false` from node config ([run_node_immediately.go:3076-3083](core/taskengine/run_node_immediately.go#L3076)); the REST mapping passes `isSimulated`/`value` through untouched (`jsonRetargetProto`, [aggregator/rest/mapping/node.go:54](aggregator/rest/mapping/node.go#L54)). There is **no gateway-level force-simulate** — the SDK's "always Tenderly" is only a test convention. +- **Preview is faithful for token-only calls.** The simulate branch ([vm_runner_contract_write.go:403-622](core/taskengine/vm_runner_contract_write.go#L403)) runs Tenderly with `senderAddress` = the runner smart wallet, honors `value` via `extractTransactionValue(node)` ([:482](core/taskengine/vm_runner_contract_write.go#L482)), seeds `erc20Overrides`, and **injects the ERC-20 allowance slot after an `approve()` sim** ([:519-548](core/taskengine/vm_runner_contract_write.go#L519)) so a following `swap` sees the approval — the approve→swap preview primitive. +- **Auth is correct for fund-moving.** `needsAASender` (true for `contractWrite`) requires a non-zero authenticated `TaskOwner` and a `settings.runner` the owner actually owns (DB list, else salt 0–4 derivation, [run_node_immediately.go:2618-2748](core/taskengine/run_node_immediately.go#L2618)). A partner-assertion-only caller (zero address) is rejected — real execution needs the user's Bearer JWT. +- **Overall step success is all-or-nothing across method calls** — `computeWriteStepSuccess` ([core/taskengine/node_utils.go:257](core/taskengine/node_utils.go#L257)) sets success=false if any method result fails or has a receipt failure. + +--- + +## 2. "preview→confirm→execute contract" — what it is, and where it's broken + +**The contract.** Studio drives one node through two backend calls with the **same node JSON**: + +``` + preview = nodes:run(node, is_simulated=true) → show the user the outcome + (user confirms in studio) + execute = nodes:run(node, is_simulated=false) → broadcast, return a result to reconcile +``` + +For this to be trustworthy, two properties must hold — and **each is where the backend gap lives**: + +- **(A) Fidelity** — the preview must predict the execute. It breaks wherever the *simulate* path and the *real* path diverge on the same node config. +- **(B) Reconcilable result** — the execute must return a result studio can render and match against the preview (tx identity + what the user got). + +The "gap of the preview→confirm→execute contract" is exactly the set of divergences (A) and result-shape holes (B) below. It is **not** a missing endpoint — the endpoint exists and studio already calls it twice. + +--- + +## 3. The backend gaps (verified, ranked) + +| # | Gap | Property | Severity | Where | +| --- | --- | --- | --- | --- | +| G1 | Native ETH `value` honored in preview, **hardcoded to 0 on execute** | Fidelity (A) | **P0 — breaks the example** | [vm_runner_contract_write.go:687](core/taskengine/vm_runner_contract_write.go#L687) | +| G2 | Real execute returns **no typed tx identity**, and **no return value** (`Value: nil`) | Reconcilable (B) | **P0** | [vm_runner_contract_write.go:1112](core/taskengine/vm_runner_contract_write.go#L1112), [handlers_nodes.go:139-148](aggregator/rest/handlers_nodes.go#L139) | +| G3 | Pending UserOp reports **success=false** (indistinguishable from failure); `nodes:run` is stateless (no re-poll) | Reconcilable (B) | **P0** | [vm_runner_contract_write.go:1022-1038, 1087](core/taskengine/vm_runner_contract_write.go#L1022) | +| G4 | approve+swap = **two non-atomic UserOps**; partial on-chain side effects on failure | Fidelity (A) | **P1 — reliability** | [vm_runner_contract_write.go:1526, 643](core/taskengine/vm_runner_contract_write.go#L1526) | +| G5 | Execute is **not idempotent** — a retried Confirm re-broadcasts | Safety | **P1** | `nodes:run` stateless (no key) | +| G6 | Runner **salt not propagated** on the `nodes:run` real path (validates salt 0–4, sets only `aa_sender`) | Correctness (edge) | **P2** | [run_node_immediately.go:2731](core/taskengine/run_node_immediately.go#L2731) | +| G7 | **No execution fee** wired on `nodes:run` (nil `executionFeeWei`) | Product/pricing | **P2 — decision** | (fee set only in `executor.go`) | + +### G1 — Native `value` is dropped on execute (P0, breaks the example) + +The real path packs the smart-wallet call with a **hardcoded zero value**: + +```go +smartWalletCallData, err := aa.PackExecute( + contractAddress, + big.NewInt(0), // ETH value (0 for contract calls) ← always 0 + callDataBytes, +) +``` +([vm_runner_contract_write.go:685-689](core/taskengine/vm_runner_contract_write.go#L685)) + +The simulate path, by contrast, passes `r.extractTransactionValue(node)` to Tenderly ([:482](core/taskengine/vm_runner_contract_write.go#L482)). So a **payable** call — wrapping ETH, or an **ETH-in** Uniswap swap — previews correctly (value applied) and then **executes with 0 wei** → reverts or no-ops. This directly breaks the "sell my ETH" market-order example on the real path. +**Fix**: replace `big.NewInt(0)` with the parsed `extractTransactionValue(node)` (guard parse errors; it already returns a decimal-wei string). Small, self-contained. Reconcile with the paymaster-reimbursement batch wrapper, which is the only place native value flows today. + +### G2 — Execute returns no typed tx identity and no return value (P0) + +A mined real result carries a `Receipt` **map** with `transactionHash` and the standard fields, but: +- there is **no first-class typed field** for `txHash` / `userOpHash` / status — studio must dig through the generic `output` bag that `runNodeRespToOpenAPI` flattens ([handlers_nodes.go:139-148](aggregator/rest/handlers_nodes.go#L139)); and +- **`Value` is nil for real transactions** — `createRealTransactionResult` sets `Value: nil // Real transactions don't return values directly` ([vm_runner_contract_write.go:1112](core/taskengine/vm_runner_contract_write.go#L1112)). So the **preview returns the swap `amountOut`** (decoded return value), but the **execute does not** — studio can't confirm "you received X USDC" from the execute response without decoding logs itself. + +`userOpHash` is only populated in the *pending* branch ([:1025](core/taskengine/vm_runner_contract_write.go#L1025)); once a receipt exists, only `transactionHash` is kept. +**Fix**: add a typed execution result on `RunNodeWithInputsResp` (or the `contractWrite` output) — `{ userOpHash, txHash, status, blockNumber, gasUsed }` populated in both mined and pending branches — and decode/attach the primary return value (e.g. Swap `amountOut` from the receipt logs) for real executes so preview and execute expose the same shape. + +### G3 — Pending ≠ failure (P0) + +`SendUserOp` blocks up to ~2 minutes polling for the receipt and can return a **UserOp with no receipt** (still pending). In that branch the result is `status:"pending"`, `transactionHash:"pending"` ([vm_runner_contract_write.go:1022-1030](core/taskengine/vm_runner_contract_write.go#L1022)), and success is computed as `receipt != nil && receipt.Status == 1 && userOpInnerSuccess` ([:1087](core/taskengine/vm_runner_contract_write.go#L1087)) → **`success=false`**. So a swap that *will* land but hasn't yet is reported identically to a swap that *failed*. And because `nodes:run` is stateless, there is **no way to re-poll** that UserOp's status afterward. +**Fix**: (a) surface a distinct `pending` status in the typed result (G2) so studio doesn't render pending as failure; (b) provide a way to resolve a pending UserOp — either a lightweight status lookup keyed by `userOpHash`, or have studio poll chain/bundler with the returned `userOpHash`. Minimum viable: make `pending` unambiguous in the response. + +### G4 — Non-atomic approve+swap (P1, reliability) + +`Execute` loops over `methodCalls` ([vm_runner_contract_write.go:1526](core/taskengine/vm_runner_contract_write.go#L1526)); each real method calls `executeRealUserOpTransaction` independently ([:643](core/taskengine/vm_runner_contract_write.go#L643)) → **one UserOp per method**, and the loop **does not stop on a failed method** ("Don't fail the entire execution for individual method failures", [:1729](core/taskengine/vm_runner_contract_write.go#L1729)). So approve+swap = two sequential UserOps; if approve lands and swap fails, `computeWriteStepSuccess` reports the *step* as failed while the **approve is already on-chain** — a partial side effect the preview (single atomic Tenderly sim with allowance injection) never showed. This is both a fidelity gap and a footgun. +**Fix**: when a node has multiple `methodCalls` and `is_simulated=false`, batch them into **one `executeBatch` UserOp** via the existing `aa.PackExecuteBatch` / `PackExecuteBatchWithValues` ([core/chainio/aa/aa.go:206](core/chainio/aa/aa.go#L206)) — atomic, one gas estimate, one signature. Keep per-method sequential as a fallback. Until this lands, restrict chat execution to single-call actions (or accept the approve-then-swap risk with a guard). + +### G5 — Idempotency (P1, safety even without spend caps) + +`nodes:run` execute has no idempotency key; a retried Confirm (network retry, double-click, studio re-send) **re-broadcasts** a second UserOp. Since we're moving real funds, add an **idempotency key** on the execute request that the aggregator dedupes within a short window (distinct from the on-chain nonce, which doesn't protect the API layer). This is the one "safety" item worth doing in v1 even though the broader policy gate is deferred. + +### G6 — Runner salt not propagated (P2, edge correctness) + +The run-node path validates the runner across **salts 0–4** and computes `matchedSalt`, but then sets only `aa_sender` and **discards the salt** ([run_node_immediately.go:2731](core/taskengine/run_node_immediately.go#L2731)) — `aa_salt` is never set on this path (it *is* set on the deployed-task path, [executor.go:491](core/taskengine/executor.go#L491), and in `runContractWrite` from `vm.task`, [vm.go:1658](core/taskengine/vm.go#L1658), but `vm.task` is nil for `nodes:run`). The real send reads `aa_salt` ([vm_runner_contract_write.go:756](core/taskengine/vm_runner_contract_write.go#L756)) and, absent it, defaults to salt 0. For the overwhelmingly common **salt:0** runner this is fine; for a runner at salt 1–4 the sender override and the auto-deploy salt/initCode diverge → wrong/failed deploy. +**Fix**: carry `matchedSalt` into `aa_salt` on the `nodes:run` path (mirror `executor.go`). Cheap; removes a latent multi-wallet bug. + +### G7 — Execution fee posture (P2, decision not bug) + +The deployed-task executor adds a platform `executionFeeWei` batch transfer ([executor.go:356-368]); the `nodes:run` path leaves it nil, so **on-demand actions are gas-sponsored but fee-free** today. Decide explicitly: charge a fee on on-demand executes (wire it like the executor) or document them as free. Pure product/pricing. + +--- + +## 4. What is explicitly NOT a backend gap + +- **Building the swap call** — router/quoter addresses, `exactInputSingle` tuple, fee tier, `amountOutMinimum`/slippage, the approve — studio + `@avaprotocol/protocols` own this. The backend takes a fully-formed `contractWrite` node. +- **The confirm gate / spend policy** — deferred (studio Confirm + ownership check suffice for v1). +- **A `uniswapSwap` node type** — unnecessary; a `contractWrite` with two method calls (after G4) covers it. +- **Chain resolution** — already correct: `chain_id` is required per-node post-G5 ([vm.go:354](core/taskengine/vm.go#L354)); `nodes:run` stamps the request chain onto the node if unset. + +--- + +## 5. SDK & studio touchpoints (high-level) + +- **`ava-sdk-js`**: consume the typed execution result from G2/G3 (add `RunNodeExecuteResult` fields: `userOpHash`, `txHash`, `status`, `amountOut`/return value); make `is_simulated=false` an explicit, obviously-fund-moving call that requires a Bearer JWT (not the partner-assertion simulate path). Add a `nodes:run` command to `examples/example.ts` for agent verification (none exists today). The market-order helper lives here or in `@avaprotocol/protocols`, not in EigenLayer-AVS. +- **`studio`** (kept high-level): a preview→confirm→execute action card (preview via `is_simulated=true` + `erc20Overrides`; execute via `is_simulated=false` bound to the previewed `amountOutMinimum`); a curated `uniswap-swap` verified action so the confidence scorer treats it as reviewed-not-blocked; and (later) the portfolio-watch → suggested-action loop. This is the execution capability the advisor plan and the idle-funds gap note deliberately deferred. + +--- + +## 6. Suggested backend sequencing + +1. **G1 (value passthrough)** + **G2 (typed result incl. return value)** + **G3 (pending status)** — the P0 set that makes a single real swap *work and be reconcilable*. Ship together; they're the whole "execute returns something studio can trust." +2. **G4 (atomic batch)** — makes approve+swap a reliable market order. +3. **G5 (idempotency)** — safety before chat drives real executes at volume. +4. **G6 (salt)**, **G7 (fee decision)** — cleanup/decisions. + +Each is independently shippable; **G1 alone is a small, standalone correctness fix worth landing regardless.** Deferred (later hardening): server-side spend/fund-authorization policy. + +--- + +## 7. Risks + +- **Non-atomic partial fills (pre-G4).** Mitigation: land G4 before enabling multi-call chat execution; until then restrict to single-call or add an approve-state guard. +- **`value=0` silent wrong execution (pre-G1).** Mitigation: G1 is a small early fix; gate native-value actions off until it lands. +- **Pending read as failure (pre-G3).** Mitigation: G3 status field; studio must not render `pending` as failed. +- **Double-execute on retry (pre-G5).** Mitigation: idempotency key. +- **Scope creep into autonomous trading.** This is *one-time, user-confirmed* actions; recurring/autonomous strategies remain workflows with their own review. + +--- + +## Appendix — verified source references + +- Single-node REST handler — [aggregator/rest/handlers_nodes.go:23](aggregator/rest/handlers_nodes.go#L23); response flattening [:117](aggregator/rest/handlers_nodes.go#L117) +- Engine single-node RPC (`is_simulated` default) — [run_node_immediately.go:2996](core/taskengine/run_node_immediately.go#L2996), [:3076](core/taskengine/run_node_immediately.go#L3076) +- `needsAASender` runner ownership + salt scan — [run_node_immediately.go:2618-2748](core/taskengine/run_node_immediately.go#L2618); `aa_sender` set (no salt) [:2731](core/taskengine/run_node_immediately.go#L2731) +- ContractWrite `Execute` loop (per-method) — [vm_runner_contract_write.go:1450](core/taskengine/vm_runner_contract_write.go#L1450), method loop [:1526](core/taskengine/vm_runner_contract_write.go#L1526) +- Simulate branch (value + allowance injection) — [vm_runner_contract_write.go:403-622](core/taskengine/vm_runner_contract_write.go#L403) +- Real path (`value=0` hardcode) — [vm_runner_contract_write.go:685-689](core/taskengine/vm_runner_contract_write.go#L685) +- Real result build (`Value: nil`, pending branch) — [vm_runner_contract_write.go:952-1113](core/taskengine/vm_runner_contract_write.go#L952) +- Step success (all-or-nothing) — [core/taskengine/node_utils.go:257](core/taskengine/node_utils.go#L257) +- Batch pack helpers (for G4) — [core/chainio/aa/aa.go:206](core/chainio/aa/aa.go#L206) +- `aa_salt` set on deployed path (for G6) — [core/taskengine/executor.go:491](core/taskengine/executor.go#L491) +- REST mapping passes config through — [aggregator/rest/mapping/node.go:54](aggregator/rest/mapping/node.go#L54) + From fb070884240d8d4867f30bc3d0d024928c22258c Mon Sep 17 00:00:00 2001 From: Chris Li Date: Mon, 13 Jul 2026 04:16:13 +0200 Subject: [PATCH 05/16] feat: correct + idempotent single-node contractWrite execution (#659) Co-authored-by: Chris Li --- PLAN_AGENT_ONDEMAND_ACTIONS.md | 42 +++- aggregator/rest/handlers_nodes.go | 6 +- core/taskengine/engine.go | 7 + core/taskengine/run_node_immediately.go | 135 ++++++++++++- .../run_node_immediately_idempotency_test.go | 115 +++++++++++ core/taskengine/vm_runner_contract_write.go | 74 ++++++- .../vm_runner_contract_write_ondemand_test.go | 185 ++++++++++++++++++ go.mod | 2 +- 8 files changed, 546 insertions(+), 20 deletions(-) create mode 100644 core/taskengine/run_node_immediately_idempotency_test.go create mode 100644 core/taskengine/vm_runner_contract_write_ondemand_test.go diff --git a/PLAN_AGENT_ONDEMAND_ACTIONS.md b/PLAN_AGENT_ONDEMAND_ACTIONS.md index c68ec4be..7271190f 100644 --- a/PLAN_AGENT_ONDEMAND_ACTIONS.md +++ b/PLAN_AGENT_ONDEMAND_ACTIONS.md @@ -64,6 +64,18 @@ The "gap of the preview→confirm→execute contract" is exactly the set of dive | G6 | Runner **salt not propagated** on the `nodes:run` real path (validates salt 0–4, sets only `aa_sender`) | Correctness (edge) | **P2** | [run_node_immediately.go:2731](core/taskengine/run_node_immediately.go#L2731) | | G7 | **No execution fee** wired on `nodes:run` (nil `executionFeeWei`) | Product/pricing | **P2 — decision** | (fee set only in `executor.go`) | +### Implementation status (2026-07-12, branch `feat/ondemand-single-node-execution`) + +| Gap | Status | Commit | +| --- | --- | --- | +| G1 native value passthrough | ✅ **done + tested** | `fix(taskengine): correct single-node contractWrite real execution` | +| G2 typed identity (userOpHash + normalized status) | ✅ **done + tested** (via receipt fields, no proto change) | same | +| G3 pending ≠ failure | ✅ **done + tested** | same | +| G6 runner salt propagation | ✅ **done** | same | +| G5 idempotency | ✅ **done + tested** (Idempotency-Key header, no proto change) | `feat(taskengine): idempotent nodes:run via Idempotency-Key header` | +| G4 atomic batching | ⛔ **deferred** — shares the Execute loop with deployed workflows and interleaves with the reimbursement wrapper; restrict chat execution to single-call actions until revisited | — | +| G7 fee posture | ✅ **decided: fee-free for v1** (confirmed 2026-07-12); monetization revisited separately (analysis below) | — | + ### G1 — Native `value` is dropped on execute (P0, breaks the example) The real path packs the smart-wallet call with a **hardcoded zero value**: @@ -94,7 +106,9 @@ A mined real result carries a `Receipt` **map** with `transactionHash` and the s `SendUserOp` blocks up to ~2 minutes polling for the receipt and can return a **UserOp with no receipt** (still pending). In that branch the result is `status:"pending"`, `transactionHash:"pending"` ([vm_runner_contract_write.go:1022-1030](core/taskengine/vm_runner_contract_write.go#L1022)), and success is computed as `receipt != nil && receipt.Status == 1 && userOpInnerSuccess` ([:1087](core/taskengine/vm_runner_contract_write.go#L1087)) → **`success=false`**. So a swap that *will* land but hasn't yet is reported identically to a swap that *failed*. And because `nodes:run` is stateless, there is **no way to re-poll** that UserOp's status afterward. **Fix**: (a) surface a distinct `pending` status in the typed result (G2) so studio doesn't render pending as failure; (b) provide a way to resolve a pending UserOp — either a lightweight status lookup keyed by `userOpHash`, or have studio poll chain/bundler with the returned `userOpHash`. Minimum viable: make `pending` unambiguous in the response. -### G4 — Non-atomic approve+swap (P1, reliability) +### G4 — Non-atomic approve+swap (P1, reliability) — DEFERRED + +> **Decision: deferred.** The `Execute` loop is shared with deployed workflows and the fix interleaves with the reimbursement wrapper (which decodes a *single* `execute`, not a batch), so this is a behavior change with real blast radius, not a local fix. Until it lands, **restrict chat execution to single-call actions** (do the `approve` as its own confirmed action, or use tokens already approved). Design notes retained below. `Execute` loops over `methodCalls` ([vm_runner_contract_write.go:1526](core/taskengine/vm_runner_contract_write.go#L1526)); each real method calls `executeRealUserOpTransaction` independently ([:643](core/taskengine/vm_runner_contract_write.go#L643)) → **one UserOp per method**, and the loop **does not stop on a failed method** ("Don't fail the entire execution for individual method failures", [:1729](core/taskengine/vm_runner_contract_write.go#L1729)). So approve+swap = two sequential UserOps; if approve lands and swap fails, `computeWriteStepSuccess` reports the *step* as failed while the **approve is already on-chain** — a partial side effect the preview (single atomic Tenderly sim with allowance injection) never showed. This is both a fidelity gap and a footgun. **Fix**: when a node has multiple `methodCalls` and `is_simulated=false`, batch them into **one `executeBatch` UserOp** via the existing `aa.PackExecuteBatch` / `PackExecuteBatchWithValues` ([core/chainio/aa/aa.go:206](core/chainio/aa/aa.go#L206)) — atomic, one gas estimate, one signature. Keep per-method sequential as a fallback. Until this lands, restrict chat execution to single-call actions (or accept the approve-then-swap risk with a guard). @@ -108,9 +122,19 @@ A mined real result carries a `Receipt` **map** with `transactionHash` and the s The run-node path validates the runner across **salts 0–4** and computes `matchedSalt`, but then sets only `aa_sender` and **discards the salt** ([run_node_immediately.go:2731](core/taskengine/run_node_immediately.go#L2731)) — `aa_salt` is never set on this path (it *is* set on the deployed-task path, [executor.go:491](core/taskengine/executor.go#L491), and in `runContractWrite` from `vm.task`, [vm.go:1658](core/taskengine/vm.go#L1658), but `vm.task` is nil for `nodes:run`). The real send reads `aa_salt` ([vm_runner_contract_write.go:756](core/taskengine/vm_runner_contract_write.go#L756)) and, absent it, defaults to salt 0. For the overwhelmingly common **salt:0** runner this is fine; for a runner at salt 1–4 the sender override and the auto-deploy salt/initCode diverge → wrong/failed deploy. **Fix**: carry `matchedSalt` into `aa_salt` on the `nodes:run` path (mirror `executor.go`). Cheap; removes a latent multi-wallet bug. -### G7 — Execution fee posture (P2, decision not bug) +### G7 — Execution fee posture (P2, decision — resolved: fee-free for v1) + +The deployed-task executor adds a platform `executionFeeWei` batch transfer ([executor.go:356-368]); the `nodes:run` path leaves it nil, so **on-demand actions are gas-sponsored but fee-free** today. + +**Decision for v1: keep on-demand executes fee-free.** Ship the capability; don't couple monetization to correctness. Monetization is a separate, deliberate design. The analysis that informs that later design: + +**Does batching a fee into the UserOp add significant gas?** No — *if batched*, which is already how `wrapWithReimbursement` works (it appends `executionFeeWei` as an extra `executeBatchWithValues` entry, `builder.go:499-503`). The marginal cost of a fee is one extra batch op = a value-transfer CALL to an existing recipient (~9,000 gas) + the extra batch entry's calldata (~3 words ≈ 1,500 gas) + minor loop/decode overhead ≈ **~10–15k gas**. Against a Uniswap `exactInputSingle` (~120–180k) inside a full UserOp (~250–400k with EntryPoint verification), that's **~3–6%**. The expensive alternative is a *separate* fee UserOp — a whole extra EntryPoint verification + preVerificationGas + base ≈ 60–100k+ gas. So **batching is the efficient path**; a separate fee tx is not. + +**Would charging on every tx raise user cost?** Yes, two components: (1) the fee amount itself (revenue, a direct cost), and (2) the ~10–15k marginal gas — borne by the user because the paymaster sponsors then the wallet reimburses. For **micro trades** a *flat* per-tx fee (+ the fixed gas overhead) is a large % of a small swap; a **bps fee** scales with size and is the better "charge on every tx" shape. Batched into the existing reimbursement op, a bps fee is the lowest-friction option and needs no new signature. -The deployed-task executor adds a platform `executionFeeWei` batch transfer ([executor.go:356-368]); the `nodes:run` path leaves it nil, so **on-demand actions are gas-sponsored but fee-free** today. Decide explicitly: charge a fee on on-demand executes (wire it like the executor) or document them as free. Pure product/pricing. +**Coinbase x402 as a micro-fee rail?** x402 revives HTTP 402: the client hits an endpoint, gets payment requirements, pays with stablecoin (USDC, typically Base) by signing an EIP-3009 `transferWithAuthorization`, retries with an `X-PAYMENT` header, and a *facilitator* verifies + settles on-chain. The **payer pays no gas directly** (the facilitator submits settlement; USDC transfer gas on Base is sub-cent), so it's effectively gasless-for-payer and purpose-built for **agent/API micropayments** — conceptually a good fit for "charge $X per chat action," stable-denominated and decoupled from the swap's gas. +- **Friction against our model:** x402 reintroduces a **user signature** (the EIP-3009 authorization), which conflicts with the delegated model where the aggregator signs and the user doesn't sign per-tx — unless the fund-authorization/session-key layer also authorizes the x402 payment. It's also Base/USDC-centric today and adds a facilitator dependency. +- **Recommendation:** for a wallet/ETH-denominated fee, **batch a bps fee into the existing reimbursement UserOp** (cheap, no new signature, works today). Treat **x402 as a separate, deliberate rail** specifically for the agent/chat surface if/when per-action USDC micropayments are desired — its own project, not part of this backend correctness work. --- @@ -130,14 +154,14 @@ The deployed-task executor adds a platform `executionFeeWei` batch transfer ([ex --- -## 6. Suggested backend sequencing +## 6. Backend sequencing (status) -1. **G1 (value passthrough)** + **G2 (typed result incl. return value)** + **G3 (pending status)** — the P0 set that makes a single real swap *work and be reconcilable*. Ship together; they're the whole "execute returns something studio can trust." -2. **G4 (atomic batch)** — makes approve+swap a reliable market order. -3. **G5 (idempotency)** — safety before chat drives real executes at volume. -4. **G6 (salt)**, **G7 (fee decision)** — cleanup/decisions. +1. ✅ **G1 + G2 + G3 + G6** — shipped: a single real execute is correct (value forwarded), reconcilable (userOpHash + normalized `confirmed`/`pending`/`failed`), pending is not failure, and non-zero-salt runners resolve. (commit `fix(taskengine): correct single-node contractWrite real execution`) +2. ✅ **G5 (idempotency)** — shipped: `Idempotency-Key` header dedupes retries/double-clicks. (commit `feat(taskengine): idempotent nodes:run via Idempotency-Key header`) +3. ⛔ **G4 (atomic batch)** — deferred (shared-loop blast radius). Chat execution restricted to single-call actions meanwhile. +4. ✅ **G7 (fee)** — decided fee-free for v1 (confirmed); monetization is a separate design (see G7 analysis). -Each is independently shippable; **G1 alone is a small, standalone correctness fix worth landing regardless.** Deferred (later hardening): server-side spend/fund-authorization policy. +Deferred (later hardening): server-side spend/fund-authorization policy; atomic multi-call batching (G4); fee monetization (G7). --- diff --git a/aggregator/rest/handlers_nodes.go b/aggregator/rest/handlers_nodes.go index ba4444c3..1160ce16 100644 --- a/aggregator/rest/handlers_nodes.go +++ b/aggregator/rest/handlers_nodes.go @@ -59,7 +59,11 @@ func (s *Server) RunNode(ctx echo.Context) error { req.Erc20Overrides = openAPIERC20OverridesToProto(*body.Erc20Overrides) } - resp, err := s.engine.RunNodeImmediatelyRPCWithContext(ctx.Request().Context(), user, req) + // A caller-supplied Idempotency-Key (Stripe-style header) makes a retried or + // double-submitted execute safe: the same key returns the first result instead + // of broadcasting a second UserOp. Absent the header, behavior is unchanged. + idempotencyKey := ctx.Request().Header.Get("Idempotency-Key") + resp, err := s.engine.RunNodeImmediatelyRPCIdempotent(ctx.Request().Context(), user, req, idempotencyKey) if err != nil { return err } diff --git a/core/taskengine/engine.go b/core/taskengine/engine.go index 35ced517..60a69b35 100644 --- a/core/taskengine/engine.go +++ b/core/taskengine/engine.go @@ -30,6 +30,7 @@ import ( "github.com/ethereum/go-ethereum/crypto" "github.com/ethereum/go-ethereum/ethclient" "github.com/oklog/ulid/v2" + "golang.org/x/sync/singleflight" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" "google.golang.org/protobuf/encoding/protojson" @@ -305,6 +306,12 @@ type Engine struct { // fixed set of mutexes bounds memory — the same executionID always maps to the // same shard, so concurrent resumes of one execution serialize. See executionMutex. executionMutexes [executionLockShards]sync.Mutex + + // Collapses concurrent nodes:run requests that carry the same Idempotency-Key + // so a retried/double-clicked Confirm can't broadcast a second UserOp. Works + // with a persistent TTL cache (see RunNodeImmediatelyRPCIdempotent) that also + // dedupes sequential retries after the first request has completed. + idempotencyGroup singleflight.Group } // executionLockShards bounds the per-execution resume locks to a fixed set of mutexes. diff --git a/core/taskengine/run_node_immediately.go b/core/taskengine/run_node_immediately.go index e7870f82..3d5cdb23 100644 --- a/core/taskengine/run_node_immediately.go +++ b/core/taskengine/run_node_immediately.go @@ -2,6 +2,9 @@ package taskengine import ( "context" + "crypto/sha256" + "encoding/binary" + "encoding/hex" "encoding/json" "fmt" "math" @@ -18,6 +21,7 @@ import ( "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/ethclient" + "google.golang.org/protobuf/proto" "google.golang.org/protobuf/types/known/structpb" ) @@ -2650,12 +2654,19 @@ func (n *Engine) runProcessingNodeWithInputs(ctx context.Context, user *model.Us return nil, fmt.Errorf("failed to list wallets for owner %s: %w", vm.TaskOwner.Hex(), err) } var chosenSender common.Address - var matchedSalt *big.Int // Track matched salt for debug logging + var matchedSalt *big.Int // Matched wallet salt; propagated to aa_salt so the real UserOp path derives/deploys the correct sender for non-zero salts walletExists := false for _, w := range resp.GetItems() { if strings.EqualFold(w.GetAddress(), runnerStr) { chosenSender = common.HexToAddress(w.GetAddress()) walletExists = true + // Capture the wallet's salt so a runner at salt 1-4 deploys/derives + // under its own salt rather than defaulting to salt 0. + if saltStr := w.GetSalt(); saltStr != "" { + if parsedSalt, ok := new(big.Int).SetString(saltStr, 10); ok { + matchedSalt = parsedSalt + } + } break } } @@ -2729,6 +2740,14 @@ func (n *Engine) runProcessingNodeWithInputs(ctx context.Context, user *model.Us } vm.AddVar("aa_sender", chosenSender.Hex()) + // Propagate the resolved salt so the real UserOp path (aa_salt) derives + // and auto-deploys the runner under its own salt. Absent this, a runner + // at salt 1-4 would fall back to salt 0 and mismatch its initCode/sender. + // For the common salt:0 runner this is a no-op (big.NewInt(0) resolves + // identically to the default). + if matchedSalt != nil { + vm.AddVar("aa_salt", matchedSalt) + } } else { return nil, fmt.Errorf("settings must be an object for %s", nodeType) } @@ -2990,6 +3009,120 @@ func (n *Engine) RunNodeImmediatelyRPC(user *model.User, req *avsproto.RunNodeWi return n.RunNodeImmediatelyRPCWithContext(context.Background(), user, req) } +// idempotencyTTL bounds how long a completed nodes:run result stays replayable +// under the same Idempotency-Key. Sized to comfortably cover a Confirm retry (a +// real UserOp send blocks up to ~2 min) without pinning results indefinitely. +const idempotencyTTL = 15 * time.Minute + +// idempotencyCachePrefix namespaces persisted nodes:run responses in storage. +const idempotencyCachePrefix = "idem:noderun:" + +// idempotencyCacheKey derives a bounded, collision-resistant storage key from the +// authenticated subject and the client-supplied key. Scoping by subject prevents +// one caller's idempotency key from colliding with another's. +func idempotencyCacheKey(subject, clientKey string) []byte { + sum := sha256.Sum256([]byte(subject + "\x00" + clientKey)) + return []byte(idempotencyCachePrefix + hex.EncodeToString(sum[:])) +} + +// RunNodeImmediatelyRPCIdempotent runs a single node, deduplicating by +// idempotencyKey so a retried or double-submitted request (e.g. a chat Confirm) +// does not execute — and broadcast — a second UserOp. An empty key preserves the +// plain non-idempotent behavior. +// +// Two layers cooperate: singleflight collapses requests that are in flight +// concurrently (a double-click), and a persistent TTL cache returns the prior +// result for a request that arrives after the first has completed (a retry). +// Only a definitive response (err == nil, which includes an on-chain failure or +// a still-pending UserOp) is cached; a transport/engine error caches nothing so +// the caller may safely retry. Polling a pending UserOp's final status is a +// separate concern — reuse of the same key returns the cached pending result. +func (n *Engine) RunNodeImmediatelyRPCIdempotent(ctx context.Context, user *model.User, req *avsproto.RunNodeWithInputsReq, idempotencyKey string) (*avsproto.RunNodeWithInputsResp, error) { + // Idempotency needs a distinct authenticated subject to scope the cache key. + // A partner-assertion (simulate-only) caller can resolve to the zero address; + // deduping those would let one caller's key cross-replay another's result, so + // skip idempotency when there is no real subject. Safe by construction: real + // executes (fund-moving) always carry a non-zero authenticated owner. + if idempotencyKey == "" || user == nil || user.Address == (common.Address{}) { + return n.RunNodeImmediatelyRPCWithContext(ctx, user, req) + } + cacheKey := idempotencyCacheKey(user.Address.Hex(), idempotencyKey) + + // Fast path: a prior completed result still within the TTL window. + if resp := n.readIdempotentResponse(cacheKey); resp != nil { + return resp, nil + } + + // Collapse concurrent duplicates: only the singleflight leader executes. + v, err, _ := n.idempotencyGroup.Do(string(cacheKey), func() (interface{}, error) { + // Re-check under the leader in case a racing request completed and cached + // between our fast-path miss and acquiring the singleflight slot. + if resp := n.readIdempotentResponse(cacheKey); resp != nil { + return resp, nil + } + resp, runErr := n.RunNodeImmediatelyRPCWithContext(ctx, user, req) + if runErr != nil { + return nil, runErr + } + n.writeIdempotentResponse(cacheKey, resp) + return resp, nil + }) + if err != nil { + return nil, err + } + resp, _ := v.(*avsproto.RunNodeWithInputsResp) + return resp, nil +} + +// readIdempotentResponse returns a cached response if one exists and is within +// the TTL; otherwise nil. Stored as [8-byte big-endian unix-nano ts][proto bytes]. +func (n *Engine) readIdempotentResponse(cacheKey []byte) *avsproto.RunNodeWithInputsResp { + if n.db == nil { + return nil + } + raw, err := n.db.GetKey(cacheKey) + if err != nil { + return nil // not found + } + if len(raw) < 8 { + _ = n.db.Delete(cacheKey) // corrupt/short entry — drop it + return nil + } + ts := int64(binary.BigEndian.Uint64(raw[:8])) + if time.Since(time.Unix(0, ts)) > idempotencyTTL { + // Expired: best-effort delete so stale keys don't accumulate unbounded, + // then let the fresh request re-execute. + _ = n.db.Delete(cacheKey) + return nil + } + resp := &avsproto.RunNodeWithInputsResp{} + if err := proto.Unmarshal(raw[8:], resp); err != nil { + // Corrupt payload: drop it so the next request re-executes and repopulates + // cleanly instead of failing to decode forever. + _ = n.db.Delete(cacheKey) + return nil + } + return resp +} + +// writeIdempotentResponse persists a completed response for TTL-bounded replay. +// Best-effort: a store failure only means a later retry re-executes. +func (n *Engine) writeIdempotentResponse(cacheKey []byte, resp *avsproto.RunNodeWithInputsResp) { + if n.db == nil || resp == nil { + return + } + payload, err := proto.Marshal(resp) + if err != nil { + return + } + buf := make([]byte, 8+len(payload)) + binary.BigEndian.PutUint64(buf[:8], uint64(time.Now().UnixNano())) + copy(buf[8:], payload) + if err := n.db.Set(cacheKey, buf); err != nil && n.logger != nil { + n.logger.Warn("RunNodeImmediately: failed to persist idempotent response", "error", err) + } +} + // RunNodeImmediatelyRPCWithContext is RunNodeImmediatelyRPC with a // caller-supplied context, propagated to the worker-routed block reads and // wallet-salt scan so a cancelled request interrupts them. diff --git a/core/taskengine/run_node_immediately_idempotency_test.go b/core/taskengine/run_node_immediately_idempotency_test.go new file mode 100644 index 00000000..d14694c4 --- /dev/null +++ b/core/taskengine/run_node_immediately_idempotency_test.go @@ -0,0 +1,115 @@ +package taskengine + +import ( + "context" + "encoding/binary" + "testing" + "time" + + "github.com/AvaProtocol/EigenLayer-AVS/core/testutil" + "github.com/AvaProtocol/EigenLayer-AVS/model" + avsproto "github.com/AvaProtocol/EigenLayer-AVS/protobuf" + "github.com/AvaProtocol/EigenLayer-AVS/storage" + "github.com/ethereum/go-ethereum/common" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "google.golang.org/protobuf/proto" +) + +// customCodeReq builds a single-node RunNodeWithInputs request whose customCode +// node returns the given constant, so distinct sources produce distinct results. +func customCodeReq(source string) *avsproto.RunNodeWithInputsReq { + return &avsproto.RunNodeWithInputsReq{ + Node: &avsproto.TaskNode{ + Id: "cc1", + Name: "cc1", + Type: avsproto.NodeType_NODE_TYPE_CUSTOM_CODE, + TaskType: &avsproto.TaskNode_CustomCode{ + CustomCode: &avsproto.CustomCodeNode{ + Config: &avsproto.CustomCodeNode_Config{ + Lang: avsproto.Lang_LANG_JAVASCRIPT, + Source: source, + }, + }, + }, + }, + } +} + +// G5: a retried or double-submitted nodes:run carrying the same Idempotency-Key +// must replay the first result instead of executing (and broadcasting) again. +func TestRunNodeImmediatelyIdempotency(t *testing.T) { + db := testutil.TestMustDB() + defer storage.Destroy(db.(*storage.BadgerStorage)) + + engine := New(db, testutil.GetAggregatorConfig(), nil, testutil.GetLogger()) + user := &model.User{Address: common.HexToAddress("0x1111111111111111111111111111111111111111")} + ctx := context.Background() + + // First call under key K executes node A ("first"). + resp1, err := engine.RunNodeImmediatelyRPCIdempotent(ctx, user, customCodeReq("return 'first'"), "confirm-key-1") + require.NoError(t, err) + require.True(t, resp1.GetSuccess(), "customCode node should execute successfully") + + // Same key, a DIFFERENT node ("second"): must return node A's cached result, + // proving the second execution was short-circuited by idempotency. + resp2, err := engine.RunNodeImmediatelyRPCIdempotent(ctx, user, customCodeReq("return 'second'"), "confirm-key-1") + require.NoError(t, err) + assert.True(t, proto.Equal(resp1, resp2), "same Idempotency-Key must replay the first result, not re-execute") + + // No key: never dedupes — executes node B, giving a result different from A. + resp3, err := engine.RunNodeImmediatelyRPCIdempotent(ctx, user, customCodeReq("return 'second'"), "") + require.NoError(t, err) + assert.False(t, proto.Equal(resp1, resp3), "an empty key must not dedupe") + + // Different key: executes node B (not a replay of key K's result). + resp4, err := engine.RunNodeImmediatelyRPCIdempotent(ctx, user, customCodeReq("return 'second'"), "confirm-key-2") + require.NoError(t, err) + assert.False(t, proto.Equal(resp1, resp4), "a different key must not replay another key's result") + + // Subject scoping: another user with the SAME key must not see the result. + otherUser := &model.User{Address: common.HexToAddress("0x2222222222222222222222222222222222222222")} + resp5, err := engine.RunNodeImmediatelyRPCIdempotent(ctx, otherUser, customCodeReq("return 'second'"), "confirm-key-1") + require.NoError(t, err) + assert.False(t, proto.Equal(resp1, resp5), "idempotency cache must be scoped per authenticated subject") + + // Zero-address subject (e.g. a partner-assertion simulate caller) must NOT + // dedupe — otherwise two indistinguishable callers sharing a key would + // cross-replay. Same key, different nodes → the second still executes. + zeroUser := &model.User{} // Address is the zero value + zresp1, err := engine.RunNodeImmediatelyRPCIdempotent(ctx, zeroUser, customCodeReq("return 'first'"), "shared-key") + require.NoError(t, err) + zresp2, err := engine.RunNodeImmediatelyRPCIdempotent(ctx, zeroUser, customCodeReq("return 'second'"), "shared-key") + require.NoError(t, err) + assert.False(t, proto.Equal(zresp1, zresp2), "a zero-address subject must not dedupe (avoids cross-caller replay)") +} + +// The cache key is deterministic and scoped, and entries older than the TTL are +// ignored so a stale key is allowed to re-execute. +func TestIdempotencyCacheKeyAndTTL(t *testing.T) { + db := testutil.TestMustDB() + defer storage.Destroy(db.(*storage.BadgerStorage)) + engine := New(db, testutil.GetAggregatorConfig(), nil, testutil.GetLogger()) + + kA := idempotencyCacheKey("0xA", "k") + assert.Equal(t, string(kA), string(idempotencyCacheKey("0xA", "k")), "key derivation must be deterministic") + assert.NotEqual(t, string(kA), string(idempotencyCacheKey("0xB", "k")), "different subject must yield a different key") + assert.NotEqual(t, string(kA), string(idempotencyCacheKey("0xA", "k2")), "different client key must yield a different key") + + // Fresh entry round-trips. + resp := &avsproto.RunNodeWithInputsResp{Success: true} + engine.writeIdempotentResponse(kA, resp) + got := engine.readIdempotentResponse(kA) + require.NotNil(t, got) + assert.True(t, got.GetSuccess()) + + // An entry older than the TTL is ignored (a fresh request may re-execute). + expiredKey := idempotencyCacheKey("0xA", "expired") + payload, err := proto.Marshal(resp) + require.NoError(t, err) + buf := make([]byte, 8+len(payload)) + binary.BigEndian.PutUint64(buf[:8], uint64(time.Now().Add(-idempotencyTTL-time.Minute).UnixNano())) + copy(buf[8:], payload) + require.NoError(t, db.Set(expiredKey, buf)) + assert.Nil(t, engine.readIdempotentResponse(expiredKey), "entry older than the TTL must be treated as absent") +} diff --git a/core/taskengine/vm_runner_contract_write.go b/core/taskengine/vm_runner_contract_write.go index 2d746d6c..b63dec8f 100644 --- a/core/taskengine/vm_runner_contract_write.go +++ b/core/taskengine/vm_runner_contract_write.go @@ -640,11 +640,11 @@ func (r *ContractWriteProcessor) executeMethodCall( "contract", contractAddress.Hex(), "method", methodName) - return r.executeRealUserOpTransaction(ctx, contractAddress, callData, methodName, parsedABI, t0) + return r.executeRealUserOpTransaction(ctx, contractAddress, callData, methodName, parsedABI, node, t0) } // executeRealUserOpTransaction executes a real UserOp transaction for contract writes -func (r *ContractWriteProcessor) executeRealUserOpTransaction(ctx context.Context, contractAddress common.Address, callData string, methodName string, parsedABI *abi.ABI, startTime time.Time) *avsproto.ContractWriteNode_MethodResult { +func (r *ContractWriteProcessor) executeRealUserOpTransaction(ctx context.Context, contractAddress common.Address, callData string, methodName string, parsedABI *abi.ABI, node *avsproto.ContractWriteNode, startTime time.Time) *avsproto.ContractWriteNode_MethodResult { r.vm.logger.Info("🔍 REAL USEROP DEBUG - Starting real UserOp transaction execution", "contract_address", contractAddress.Hex(), "method_name", methodName, @@ -661,6 +661,27 @@ func (r *ContractWriteProcessor) executeRealUserOpTransaction(ctx context.Contex // Convert hex calldata to bytes callDataBytes := common.FromHex(callData) + // Resolve the native ETH value to send with this call (payable methods / native-in + // swaps such as wrapping ETH or an ETH-in Uniswap swap). The simulation path already + // honors node.Config.Value via extractTransactionValue; the real path must match it, + // otherwise a call that previews correctly executes with 0 wei and reverts/no-ops. + transactionValue := big.NewInt(0) + if rawValue := r.extractTransactionValue(node); rawValue != "" && rawValue != "0" { + parsedValue, parseErr := bigint.Parse(rawValue) + if parseErr != nil || parsedValue.Sign() < 0 { + r.vm.logger.Error("🚨 DEPLOYED WORKFLOW ERROR: Invalid transaction value", + "raw_value", rawValue, + "method_name", methodName, + "error", parseErr) + return &avsproto.ContractWriteNode_MethodResult{ + Success: false, + Error: fmt.Sprintf("invalid transaction value %q: must be a non-negative wei integer", rawValue), + MethodName: methodName, + } + } + transactionValue = parsedValue + } + // 🔍 PRE-FLIGHT VALIDATION: Check for common failure scenarios before gas estimation if validationErr := r.validateTransactionBeforeGasEstimation(methodName, callData, callDataBytes, contractAddress); validationErr != nil { executionLogBuilder.WriteString(fmt.Sprintf("PRE-FLIGHT VALIDATION FAILED: %v\n", validationErr)) @@ -679,13 +700,13 @@ func (r *ContractWriteProcessor) executeRealUserOpTransaction(ctx context.Contex // Create smart wallet execute calldata: execute(target, value, data) executionLogBuilder.WriteString(fmt.Sprintf("Packing smart wallet execute calldata...\n")) executionLogBuilder.WriteString(fmt.Sprintf(" Target contract: %s\n", contractAddress.Hex())) - executionLogBuilder.WriteString(fmt.Sprintf(" ETH value: 0\n")) + executionLogBuilder.WriteString(fmt.Sprintf(" ETH value: %s wei\n", transactionValue.String())) executionLogBuilder.WriteString(fmt.Sprintf(" Method calldata: %d bytes\n", len(callDataBytes))) smartWalletCallData, err := aa.PackExecute( - contractAddress, // target contract - big.NewInt(0), // ETH value (0 for contract calls) - callDataBytes, // contract method calldata + contractAddress, // target contract + transactionValue, // ETH value to send with the call (0 for non-payable calls) + callDataBytes, // contract method calldata ) if err != nil { executionLogBuilder.WriteString(fmt.Sprintf("CALLDATA PACKING FAILED: %v\n", err)) @@ -1085,8 +1106,28 @@ func (r *ContractWriteProcessor) createRealTransactionResult(methodName, contrac // For AA transactions, check both receipt status AND UserOp inner success // For regular transactions, just check receipt status success := receipt != nil && receipt.Status == 1 && userOpInnerSuccess + + // A UserOp that the bundler accepted but that hasn't been mined within the + // send timeout is PENDING, not failed — the on-chain outcome is still + // undecided and can be resolved later via userOpHash. Distinguishing this + // from an outright failure is essential for the chat preview→confirm→execute + // flow, where rendering a slow-but-successful swap as an error is wrong. + pending := receipt == nil && userOp != nil + + // executionStatus is the unambiguous, machine-readable outcome clients should + // branch on (instead of interpreting the EVM hex `status`): "confirmed" + // (mined + inner success), "pending" (submitted, awaiting confirmation), or + // "failed" (submission or on-chain failure). + executionStatus := "failed" + switch { + case success: + executionStatus = "confirmed" + case pending: + executionStatus = "pending" + } + errorMsg := "" - if !success { + if !success && !pending { if receipt == nil { errorMsg = "No transaction receipt received" } else if receipt.Status != 1 { @@ -1097,9 +1138,23 @@ func (r *ContractWriteProcessor) createRealTransactionResult(methodName, contrac } } + // Stamp the normalized status onto the receipt the client reads, and ensure + // the UserOp hash is present on the mined branch too (the pending branch + // already sets it) so callers always have a handle to the submitted UserOp. + if receiptMap != nil { + receiptMap["executionStatus"] = executionStatus + if _, hasHash := receiptMap["userOpHash"]; !hasHash && userOp != nil && r.smartWalletConfig != nil { + receiptMap["userOpHash"] = userOp.GetUserOpHash(r.smartWalletConfig.EntrypointAddress, big.NewInt(r.smartWalletConfig.ChainID)).Hex() + } + if v, err := structpb.NewValue(receiptMap); err == nil { + receiptValue = v + } + } + r.vm.logger.Info("🎯 DEPLOYED WORKFLOW: Final transaction result", "method_name", methodName, "success", success, + "execution_status", executionStatus, "error_msg", errorMsg, "has_receipt_value", receiptValue != nil) @@ -1109,7 +1164,7 @@ func (r *ContractWriteProcessor) createRealTransactionResult(methodName, contrac Success: success, Error: errorMsg, Receipt: receiptValue, - Value: nil, // Real transactions don't return values directly + Value: nil, // Real transactions don't return values directly; decoded events carry outputs } } @@ -1225,8 +1280,10 @@ func (r *ContractWriteProcessor) convertTenderlyResultToFlexibleFormat(result *C // Note: blockNumber/blockHash default to placeholders here and can be // overridden by the caller with real chain context when available. receiptStatus := "0x1" // Default to success + simExecutionStatus := "confirmed" if !result.Success { receiptStatus = "0x0" // Set to failure if transaction failed + simExecutionStatus = "failed" } // Prepare logs from Tenderly simulation result @@ -1249,6 +1306,7 @@ func (r *ContractWriteProcessor) convertTenderlyResultToFlexibleFormat(result *C "cumulativeGasUsed": r.getGasUsedWithFallback(result, StandardGasCostHex), "effectiveGasPrice": r.getEffectiveGasPriceWithFallback(result), "status": receiptStatus, // Success/failure status based on actual result + "executionStatus": simExecutionStatus, // Normalized outcome, uniform with the real path ("confirmed"/"failed") "logsBloom": "0x00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000", // Empty logs bloom "logs": receiptLogs, // Logs from Tenderly simulation "type": "0x2", // EIP-1559 transaction type diff --git a/core/taskengine/vm_runner_contract_write_ondemand_test.go b/core/taskengine/vm_runner_contract_write_ondemand_test.go new file mode 100644 index 00000000..0f52891d --- /dev/null +++ b/core/taskengine/vm_runner_contract_write_ondemand_test.go @@ -0,0 +1,185 @@ +package taskengine + +import ( + "context" + "math/big" + "testing" + + "github.com/AvaProtocol/EigenLayer-AVS/core/config" + "github.com/AvaProtocol/EigenLayer-AVS/pkg/erc4337/preset" + "github.com/AvaProtocol/EigenLayer-AVS/pkg/erc4337/userop" + "github.com/AvaProtocol/EigenLayer-AVS/pkg/logger" + avsproto "github.com/AvaProtocol/EigenLayer-AVS/protobuf" + "github.com/ethereum/go-ethereum/common" + "github.com/ethereum/go-ethereum/core/types" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// wellFormedUserOp returns a UserOperation with every big.Int/byte field populated +// so GetUserOpHash / PackForSignature don't panic in unit tests. +func wellFormedUserOp(sender common.Address) *userop.UserOperation { + return &userop.UserOperation{ + Sender: sender, + Nonce: big.NewInt(0), + InitCode: []byte{}, + CallData: []byte{}, + CallGasLimit: big.NewInt(0), + VerificationGasLimit: big.NewInt(0), + PreVerificationGas: big.NewInt(0), + MaxFeePerGas: big.NewInt(0), + MaxPriorityFeePerGas: big.NewInt(0), + PaymasterAndData: []byte{}, + Signature: []byte{}, + } +} + +// ondemandTestConfig builds a smart-wallet config sufficient to drive the real +// UserOp path in-process. PaymasterAddress is left zero so shouldUsePaymaster() +// short-circuits to self-funded (no bundler gas estimation needed under the mock). +func ondemandTestConfig() *config.SmartWalletConfig { + return &config.SmartWalletConfig{ + EntrypointAddress: common.HexToAddress(config.DefaultEntrypointAddressHex), + FactoryAddress: common.HexToAddress("0x0000000000000000000000000000000000009999"), + ChainID: 11155111, + } +} + +func minedReceipt(status uint64) *types.Receipt { + return &types.Receipt{ + Status: status, + TxHash: common.HexToHash("0x00000000000000000000000000000000000000000000000000000000000000ab"), + BlockHash: common.HexToHash("0x00000000000000000000000000000000000000000000000000000000000000de"), + BlockNumber: big.NewInt(100), + TransactionIndex: 0, + GasUsed: 21000, + CumulativeGasUsed: 21000, + EffectiveGasPrice: big.NewInt(1_000_000_000), + Type: 2, + } +} + +// G2/G3: createRealTransactionResult must expose an unambiguous executionStatus +// (confirmed/failed/pending) plus a userOpHash, so the chat preview→confirm→execute +// flow can tell a mined success from a revert from a still-pending UserOp instead +// of collapsing pending into failure. +func TestCreateRealTransactionResultExecutionStatus(t *testing.T) { + owner := common.HexToAddress("0x1111111111111111111111111111111111111111") + runner := common.HexToAddress("0x2222222222222222222222222222222222222222") + contractAddr := common.HexToAddress("0x1c7d4b196cb0c7b01d743fbc6116a902379c7238") + swConfig := ondemandTestConfig() + + vm, err := NewVMWithData(nil, nil, swConfig, nil) + require.NoError(t, err) + vm.AddVar("aa_sender", runner.Hex()) + + processor := NewContractWriteProcessor(vm, nil, swConfig, owner) + userOp := wellFormedUserOp(runner) + + receiptMapOf := func(t *testing.T, mr *avsproto.ContractWriteNode_MethodResult) map[string]any { + require.NotNil(t, mr.Receipt) + m, ok := mr.Receipt.AsInterface().(map[string]any) + require.True(t, ok) + return m + } + + t.Run("mined success -> confirmed", func(t *testing.T) { + mr := processor.createRealTransactionResult("transfer", contractAddr.Hex(), "0x", nil, userOp, minedReceipt(1)) + assert.True(t, mr.Success) + assert.Empty(t, mr.Error) + rm := receiptMapOf(t, mr) + assert.Equal(t, "confirmed", rm["executionStatus"]) + assert.NotEmpty(t, rm["userOpHash"], "mined result must carry a userOpHash") + }) + + t.Run("mined revert -> failed", func(t *testing.T) { + mr := processor.createRealTransactionResult("transfer", contractAddr.Hex(), "0x", nil, userOp, minedReceipt(0)) + assert.False(t, mr.Success) + assert.NotEmpty(t, mr.Error) + rm := receiptMapOf(t, mr) + assert.Equal(t, "failed", rm["executionStatus"]) + }) + + t.Run("submitted but not mined -> pending, not failure", func(t *testing.T) { + mr := processor.createRealTransactionResult("transfer", contractAddr.Hex(), "0x", nil, userOp, nil) + assert.False(t, mr.Success, "pending is not confirmed-success") + assert.Empty(t, mr.Error, "pending must not be reported as an error") + rm := receiptMapOf(t, mr) + assert.Equal(t, "pending", rm["executionStatus"]) + assert.Equal(t, "pending", rm["transactionHash"]) + assert.NotEmpty(t, rm["userOpHash"], "pending result must carry a userOpHash to poll") + }) +} + +// G1: the real UserOp path must forward the node's native ETH value into the +// packed execute(target, value, data) calldata, matching the simulation path. +// It previously hardcoded 0, so a payable call (e.g. wrapping ETH or an ETH-in +// swap) previewed correctly then executed with 0 wei. +func TestContractWriteRealPathForwardsNativeValue(t *testing.T) { + owner := common.HexToAddress("0x1111111111111111111111111111111111111111") + runner := common.HexToAddress("0x2222222222222222222222222222222222222222") + contractAddr := common.HexToAddress("0x1c7d4b196cb0c7b01d743fbc6116a902379c7238") + swConfig := ondemandTestConfig() + + // A valid ERC20 transfer calldata (>= 4 bytes) — the inner call is irrelevant + // here; we only assert on the value word of the outer execute() wrapper. + transferCallData := "0xa9059cbb000000000000000000000000e0f7d11fd714674722d325cd86062a5f1882e13a00000000000000000000000000000000000000000000000000000000000003e8" + + // run executes a single real contractWrite method call with the given config + // value and returns the value word decoded from the packed execute() calldata. + run := func(t *testing.T, valueWei string) *big.Int { + vm, err := NewVMWithData(nil, nil, swConfig, nil) + require.NoError(t, err) + vm.AddVar("aa_sender", runner.Hex()) + + processor := NewContractWriteProcessor(vm, nil, swConfig, owner) + + var capturedCallData []byte + processor.sendUserOpFunc = func( + _ *config.SmartWalletConfig, + _ common.Address, + callData []byte, + _ *preset.VerifyingPaymasterRequest, + _ *common.Address, + _ *big.Int, + _ *big.Int, + _ logger.Logger, + ) (*userop.UserOperation, *types.Receipt, error) { + capturedCallData = callData + return wellFormedUserOp(runner), minedReceipt(1), nil + } + + cfg := &avsproto.ContractWriteNode_Config{ + ContractAddress: contractAddr.Hex(), + ChainId: swConfig.ChainID, + } + if valueWei != "" { + cfg.Value = &valueWei + } + node := &avsproto.ContractWriteNode{Config: cfg} + + callData := transferCallData + methodCall := &avsproto.ContractWriteNode_MethodCall{ + MethodName: "transfer", + CallData: &callData, + } + + mr := processor.executeMethodCall(context.Background(), nil, "", contractAddr, methodCall, false, node) + require.NotNil(t, mr) + + // execute(address dest, uint256 value, bytes data): after the 4-byte + // selector, word 0 is dest and word 1 (bytes 36:68) is the ETH value. + require.GreaterOrEqual(t, len(capturedCallData), 68, "expected packed execute(target,value,data) calldata") + return new(big.Int).SetBytes(capturedCallData[36:68]) + } + + t.Run("config value is forwarded", func(t *testing.T) { + got := run(t, "1230000000000000000") // 1.23 ETH + assert.Equal(t, "1230000000000000000", got.String()) + }) + + t.Run("unset value defaults to zero", func(t *testing.T) { + got := run(t, "") + assert.Equal(t, "0", got.String()) + }) +} diff --git a/go.mod b/go.mod index b208fec1..95caf27f 100644 --- a/go.mod +++ b/go.mod @@ -29,6 +29,7 @@ require ( github.com/stretchr/testify v1.11.1 go.uber.org/zap v1.27.0 golang.org/x/crypto v0.48.0 + golang.org/x/sync v0.19.0 google.golang.org/grpc v1.77.0 google.golang.org/protobuf v1.36.11 gopkg.in/yaml.v2 v2.4.0 @@ -144,7 +145,6 @@ require ( golang.org/x/exp v0.0.0-20240613232115-7f521ea00fb8 // indirect golang.org/x/mod v0.32.0 // indirect golang.org/x/net v0.50.0 // indirect - golang.org/x/sync v0.19.0 // indirect golang.org/x/sys v0.41.0 // indirect golang.org/x/text v0.34.0 // indirect golang.org/x/time v0.14.0 // indirect From 7a69721170e18cd95cec0f64239cd80463cfa428 Mon Sep 17 00:00:00 2001 From: Chris Li Date: Mon, 13 Jul 2026 19:56:23 +0200 Subject: [PATCH 06/16] fix(rest): surface contractWrite per-method receipt in nodes:run response (#660) Co-authored-by: Chris Li --- aggregator/rest/handlers_nodes.go | 17 +++++++- aggregator/rest/handlers_nodes_test.go | 57 ++++++++++++++++++++++++++ 2 files changed, 72 insertions(+), 2 deletions(-) create mode 100644 aggregator/rest/handlers_nodes_test.go diff --git a/aggregator/rest/handlers_nodes.go b/aggregator/rest/handlers_nodes.go index 1160ce16..d97e2c3b 100644 --- a/aggregator/rest/handlers_nodes.go +++ b/aggregator/rest/handlers_nodes.go @@ -127,8 +127,21 @@ func runNodeRespToOpenAPI(in *avsproto.RunNodeWithInputsResp) generated.RunNodeR out.ErrorCode = &code } if md := in.GetMetadata(); md != nil { - if v, ok := md.AsInterface().(map[string]interface{}); ok && v != nil { - out.Metadata = &v + switch v := md.AsInterface().(type) { + case map[string]interface{}: + if v != nil { + out.Metadata = &v + } + case []interface{}: + // Some node types (notably contractWrite) produce a per-method + // results array as metadata. RunNodeResponse.metadata is an object, + // so a bare array was previously dropped here — taking the per-method + // receipts (executionStatus, userOpHash, transactionHash) with it. + // Wrap it under "results" so those actually reach the client. + if len(v) > 0 { + wrapped := map[string]interface{}{"results": v} + out.Metadata = &wrapped + } } } if ec := in.GetExecutionContext(); ec != nil { diff --git a/aggregator/rest/handlers_nodes_test.go b/aggregator/rest/handlers_nodes_test.go new file mode 100644 index 00000000..3b904bd4 --- /dev/null +++ b/aggregator/rest/handlers_nodes_test.go @@ -0,0 +1,57 @@ +package rest + +import ( + "testing" + + avsproto "github.com/AvaProtocol/EigenLayer-AVS/protobuf" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "google.golang.org/protobuf/types/known/structpb" +) + +// contractWrite surfaces its per-method results (including the receipt with +// executionStatus / userOpHash / transactionHash) as an ARRAY-typed metadata. +// runNodeRespToOpenAPI must wrap that array under "results" so the client can +// read it — a bare array used to be silently dropped, taking the receipts with it. +func TestRunNodeRespToOpenAPISurfacesContractWriteReceiptArray(t *testing.T) { + results := []interface{}{ + map[string]interface{}{ + "methodName": "exactInputSingle", + "success": true, + "receipt": map[string]interface{}{ + "executionStatus": "confirmed", + "userOpHash": "0xuserop", + "transactionHash": "0xtx", + }, + }, + } + md, err := structpb.NewValue(results) + require.NoError(t, err) + + out := runNodeRespToOpenAPI(&avsproto.RunNodeWithInputsResp{Success: true, Metadata: md}) + + require.NotNil(t, out.Metadata, "array metadata must be surfaced, not dropped") + m := *out.Metadata + arr, ok := m["results"].([]interface{}) + require.True(t, ok, "array metadata must be wrapped under 'results'") + require.Len(t, arr, 1) + first := arr[0].(map[string]interface{}) + receipt := first["receipt"].(map[string]interface{}) + assert.Equal(t, "confirmed", receipt["executionStatus"]) + assert.Equal(t, "0xuserop", receipt["userOpHash"]) + assert.Equal(t, "0xtx", receipt["transactionHash"]) +} + +// Map-typed metadata (e.g. ethTransfer's gasUsed/transactionHash) must keep +// surfacing as-is, without the "results" wrapper. +func TestRunNodeRespToOpenAPIMapMetadataStillForwarded(t *testing.T) { + md, err := structpb.NewValue(map[string]interface{}{"transactionHash": "0xabc", "gasUsed": "21000"}) + require.NoError(t, err) + + out := runNodeRespToOpenAPI(&avsproto.RunNodeWithInputsResp{Success: true, Metadata: md}) + + require.NotNil(t, out.Metadata) + m := *out.Metadata + assert.Equal(t, "0xabc", m["transactionHash"]) + assert.Nil(t, m["results"], "map metadata is forwarded as-is, not wrapped") +} From 12be437957fe3298de0b0f01bcc05da5c8261444 Mon Sep 17 00:00:00 2001 From: Limer_Beast <153820464+Antrikshgwal@users.noreply.github.com> Date: Mon, 13 Jul 2026 23:30:19 +0530 Subject: [PATCH 07/16] fix: parseUnit macro implements ethers parseUnits scaling with fractional support (#657) --- core/taskengine/macros/exp.go | 49 +++++++++++++++++++++--- core/taskengine/macros/parseunit_test.go | 39 +++++++++++++++++++ 2 files changed, 82 insertions(+), 6 deletions(-) create mode 100644 core/taskengine/macros/parseunit_test.go diff --git a/core/taskengine/macros/exp.go b/core/taskengine/macros/exp.go index 5e273c66..f656424f 100644 --- a/core/taskengine/macros/exp.go +++ b/core/taskengine/macros/exp.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "math/big" + "regexp" "strings" "github.com/ethereum/go-ethereum/accounts/abi" @@ -59,7 +60,7 @@ func readContractData(contractAddress string, data string, method string, contra } // QueryContract -//const taskCondition = `cmp(chainlinkPrice("0x694AA1769357215DE4FAC081bf1f309aDC325306"), parseUnit("262199799820", 8)) > 1` +//const taskCondition = `cmp(chainlinkPrice("0x694AA1769357215DE4FAC081bf1f309aDC325306"), parseUnit("2621.99", 8)) > 1` func chainlinkLatestRoundData(tokenPair string) *big.Int { output, err := QueryContract( @@ -115,14 +116,50 @@ func BigLt(a *big.Int, b *big.Int) bool { return a.Cmp(b) < 0 } +// parseUnitRe matches a non-negative decimal number with an optional fractional +// part (e.g. "2621", "2621.99", "2621."). Validating the shape up front rejects +// signs, a leading "+" in the fraction, hex, and other malformed input that +// big.Int.SetString would otherwise silently accept. A bare ".5" (empty whole +// part) is intentionally rejected; task thresholds are written with a leading 0. +var parseUnitRe = regexp.MustCompile(`^[0-9]+(\.[0-9]*)?$`) + +// parseUnitMaxDecimals caps 10^decimals so a user-supplied `decimal` can't force +// the aggregator/operator to materialize a huge integer. 10^78 ≈ 2^259, beyond +// anything on-chain needs (uint256 tops out near 10^77). +const parseUnitMaxDecimals = 78 + +// ParseUnit converts a non-negative decimal string into a fixed-point integer +// scaled by 10^decimals, matching the ethers.js parseUnits(value, decimals) +// convention. It accepts a fractional component (e.g. ParseUnit("2621.99", 8) == +// 262199000000), which is what task-condition expressions need to build a price +// threshold to compare against chainlinkPrice(). Input that is not a non-negative +// decimal, a fraction with more digits than `decimals` (over-precision), or +// `decimals` beyond the on-chain range is rejected. func ParseUnit(val string, decimal uint) *big.Int { - b, ok := ethmath.ParseBig256(val) - if !ok { - panic(fmt.Errorf("Parse error: %s", val)) + if !parseUnitRe.MatchString(val) { + panic(fmt.Errorf("Parse error: %q is not a non-negative decimal number", val)) + } + if decimal > parseUnitMaxDecimals { + panic(fmt.Errorf("Parse error: decimals %d out of range (max %d)", decimal, parseUnitMaxDecimals)) + } + + parts := strings.SplitN(val, ".", 2) + whole, _ := new(big.Int).SetString(parts[0], 10) // regex guarantees a valid non-negative integer + + scale := new(big.Int).Exp(big.NewInt(10), new(big.Int).SetUint64(uint64(decimal)), nil) + result := new(big.Int).Mul(whole, scale) + + // Add the fractional component, right-padded to `decimals` digits. + if len(parts) == 2 && parts[1] != "" { + frac := parts[1] + if uint(len(frac)) > decimal { + panic(fmt.Errorf("Parse error: fractional part %q exceeds %d decimals in %q", frac, decimal, val)) + } + fracVal, _ := new(big.Int).SetString(frac+strings.Repeat("0", int(decimal)-len(frac)), 10) + result.Add(result, fracVal) } - r := big.NewInt(0) - return r.Div(b, big.NewInt(int64(decimal))) + return result } func ToBigInt(val string) *big.Int { diff --git a/core/taskengine/macros/parseunit_test.go b/core/taskengine/macros/parseunit_test.go new file mode 100644 index 00000000..633a1bda --- /dev/null +++ b/core/taskengine/macros/parseunit_test.go @@ -0,0 +1,39 @@ +package macros + +import ( + "math/big" + "testing" +) + +func TestParseUnit(t *testing.T) { + e8 := new(big.Int).Exp(big.NewInt(10), big.NewInt(8), nil) + + // The regression: divide-by-decimal gave 327; ethers parseUnits gives 262199000000. + if got, want := ParseUnit("2621.99", 8), big.NewInt(262199000000); got.Cmp(want) != 0 { + t.Fatalf("ParseUnit(2621.99, 8) = %s, want %s", got, want) + } + if got := ParseUnit("1", 8); got.Cmp(e8) != 0 { // integer scaling + t.Fatalf("ParseUnit(1, 8) = %s, want %s", got, e8) + } + + // Malformed or out-of-range input must be rejected, not silently coerced. + for _, bad := range []struct { + val string + decimal uint + }{ + {"1.999", 2}, // over-precision + {"-0.5", 8}, // negative with a "-0" whole part + {"-1.5", 8}, // negative + {"1.+5", 8}, // malformed fraction (leading +) + {"1", 1000}, // decimals out of range + } { + func() { + defer func() { + if recover() == nil { + t.Fatalf("ParseUnit(%q, %d) expected panic, got none", bad.val, bad.decimal) + } + }() + ParseUnit(bad.val, bad.decimal) + }() + } +} From 2bed566aab3f559763c8bd0ac33aa6c564ed99ab Mon Sep 17 00:00:00 2001 From: Chris Li Date: Wed, 15 Jul 2026 21:17:31 -0700 Subject: [PATCH 08/16] docs: plan per-workflow state + native guardian-scan node Gateway-side response to studio's guardian-monitoring hand-off. Specifies the one missing capability as a generic primitive: durable cross-run per-WORKFLOW state (wfstate::...), scoped to taskId not to a user, so a user can run any number of monitoring workflows each with isolated, cascade-deleted state. Covers the SetIfAbsent/DeleteByPrefix storage additions (reusing IncCounter's serializable txn), the native guardian-scan node (holds GoPlus/Moralis creds, mints the GoPlus token, diffs via wfstate, claim-once alerts), credential config, and answers to the studio plan's five open questions. Additive storage (new namespace + model struct), no migration. --- PLAN_WORKFLOW_STATE_GUARDIAN_MONITORING.md | 272 +++++++++++++++++++++ 1 file changed, 272 insertions(+) create mode 100644 PLAN_WORKFLOW_STATE_GUARDIAN_MONITORING.md diff --git a/PLAN_WORKFLOW_STATE_GUARDIAN_MONITORING.md b/PLAN_WORKFLOW_STATE_GUARDIAN_MONITORING.md new file mode 100644 index 00000000..87a99b43 --- /dev/null +++ b/PLAN_WORKFLOW_STATE_GUARDIAN_MONITORING.md @@ -0,0 +1,272 @@ +# PLAN — Per-Workflow State + Native Guardian-Scan (gateway design) + +> **Audience.** EigenLayer-AVS gateway/backend. This is the gateway-side response to the studio +> hand-off `PLAN_GUARDIAN_MONITORING_ARCHITECTURE.md` (in the `studio` repo). It specifies the one +> capability the guardian actually needs and doesn't have — **durable, cross-run, per-workflow +> state** — as a *generic* primitive, with the continuous guardian monitor as its first consumer. +> +> **Framing decision.** State is scoped to the **workflow (`taskId`)**, never to a user. There is +> **no "one monitoring workflow per user"** rule. A user may create workflow A to watch one EOA +> today, then workflow B to watch two other EOAs next week — two independent tasks, two independent +> state namespaces, zero user-level coupling. **The client (studio) owns setting each workflow up +> correctly** (which wallets, which cadence); the gateway just gives every workflow a private, +> cascade-scoped scratchpad and a native scan step. + +--- + +## 0. Why this doc exists + +The guardian needs to alert on **new** security flags **exactly once**. That requires remembering, run +to run, which flags a workflow has already seen and already alerted on. Today the gateway has **no +cross-run workflow state**: node runners get `vm.db` (Go side) but user-authored CustomCode JS gets no +DB handle, execution history is RPC-read-only (not readable mid-run), and the only "state-like" +things are a monotonic contract-write counter (`ct:cw:`), a 15-min idempotency TTL cache +(`idem:noderun:`), and per-execution checkpoints (`ckpt:`, deleted at terminal state). None +can hold arbitrary per-workflow values across runs. + +Everything else the guardian needs already exists: `CronTrigger` (per-task gocron jobs, +`core/taskengine/trigger/time.go`), `BranchNode`, `RestAPINode` + `CustomCodeNode`, durable execution +logs (`history:` keys), and `{{apContext.configVars.*}}` credential injection (`core/taskengine/vm.go`, +sourced from `macros.secrets`). So this doc scopes only the missing core. + +--- + +## 1. The generic capability: per-workflow state (`wfstate`) + +A workflow-scoped key/value store that survives across executions and is cleaned up with the workflow. + +### 1.1 Key schema (matches `core/taskengine/schema.go` conventions) + +``` +wfstate::seen: → JSON record (the diff-set; prefix-scanned each run) +wfstate::ntfy: → unix-ms (claim marker; presence = "already alerted") +``` + +- `taskId` — the ULID of the workflow. **This is the entire scoping key. No `owner`, no `wallet` in + the namespace** — the workflow already knows what it monitors (its config / input variables), and + two workflows of the same user get different `taskId`s and thus fully isolated state. +- `stateKey` — an opaque, consumer-defined string. The guardian uses `::` + so one workflow can monitor several EOAs on several chains without collision. +- `seen:` vs `ntfy:` split keeps the diff-set scan clean and lets the claim be a pure key-existence + CAS (no JSON coupling in the storage layer). + +### 1.2 Storage interface additions (`storage/db.go`) + +`Set`, `GetKey`, `Delete`, `BatchWrite`, `Exist`, and prefix scan (used today by `ListExecutions`) +already exist. Add two small primitives: + +```go +// SetIfAbsent atomically writes value iff key does not exist. Returns claimed=true only for the +// caller that performed the write. Reuses the serializable db.Update(txn) pattern already used by +// IncCounter (storage/db.go:~486): txn.Get(key) → ErrKeyNotFound ? txn.Set : no-op. +SetIfAbsent(key []byte, value []byte) (claimed bool, err error) + +// DeleteByPrefix removes every key under a prefix (workflow-teardown cascade). +DeleteByPrefix(prefix []byte) (int, error) +``` + +`SetIfAbsent` is the exactly-once primitive; nothing generic like it exists today (grep for +`CompareAndSet|SetIfAbsent|SetIfNotExist` is empty), but BadgerDB's `Update` gives the required +serializable isolation and the exact txn shape is copy-paste from `IncCounter`. + +### 1.3 Helper key builders (`core/taskengine/schema.go`) + +```go +func WorkflowStateSeenKey(taskID, stateKey string) []byte // "wfstate:%s:seen:%s" +func WorkflowStateSeenPrefix(taskID string) []byte // "wfstate:%s:seen:" (diff scan) +func WorkflowStateNtfyKey(taskID, stateKey string) []byte // "wfstate:%s:ntfy:%s" +func WorkflowStatePrefix(taskID string) []byte // "wfstate:%s:" (cascade delete) +``` + +`wfstate` is a fresh namespace (no existing `wfstate:` literal collides — verified against the +namespace list in `core/taskengine/doc.go`). + +### 1.4 Lifecycle + +- **Create:** implicit — the first `Set`/`SetIfAbsent` under a `taskId` creates its state. +- **Cascade delete:** the task-teardown path (engine `DeleteWorkflow` / task deletion) calls + `DeleteByPrefix(WorkflowStatePrefix(taskId))`. Delete the workflow → its state is gone. **No orphan + bookkeeping, no per-user cleanup.** +- **Re-deploy = fresh state (the one tradeoff).** A new workflow has a new `taskId`, so its state + starts empty and its first run re-alerts on all currently-active flags **once**. For the guardian + this is benign (surfacing real current risk to someone who just re-opted-in), and mitigable by + **keeping `taskId` stable across re-enroll** (update-in-place instead of delete+create) or seeding + new-workflow state at migration. Documented, not a blocker. + +### 1.5 Access boundary (v1: engine-internal, not CustomCode JS) + +Node **runners** (Go, `vm_runner_*.go`) already receive `vm.db` (`core/taskengine/vm.go`, `WithDb`) +and the executing task context (so they know `taskId`). The guardian-scan runner (§3) reads/writes +`wfstate` through `vm.db` directly. We **do not** expose `wfstate` to user-authored CustomCode JS in +v1 (goja gets no DB handle by design) — that would turn every workflow stateful and pull in size +limits, isolation, and abuse concerns. A future `state.get/set` JS binding can layer on this same +primitive if we decide to make it a public building block. + +--- + +## 2. The user scenario, modeled + +> User creates **Workflow A** to monitor **EOA1**; two weeks later creates **Workflow B** to monitor +> **EOA2 + EOA3**. + +``` +Workflow A (taskId = 01A…) Workflow B (taskId = 01B…) + wfstate:01A…:seen:1:0xEOA1:approval:0x…:0x… wfstate:01B…:seen:1:0xEOA2:token:0x… + wfstate:01A…:seen:1:0xEOA1:token:0x… wfstate:01B…:seen:8453:0xEOA3:approval:0x…:0x… +``` + +Two tasks, two `wfstate::…` namespaces, no shared "user" record. B monitoring two EOAs is +just two `stateKey` prefixes inside B's own namespace. Deleting A wipes `wfstate:01A…:*` and leaves B +untouched. This is exactly the "workflow does not attach a user" model. + +--- + +## 3. The native `guardian-scan` node (first consumer) + +A new engine-side node type — because the scan must (a) reach `wfstate` via `vm.db`, (b) hold the +GoPlus/Moralis credentials without leaking them into client-visible workflow JSON, and (c) mint the +GoPlus signed token — none of which a CustomCode JS node can do. + +### 3.1 Node shape (protobuf `avs.proto` → `make protoc-gen`) + +Add `NODE_TYPE_GUARDIAN_SCAN` to the node enum and a message to the `TaskNode` oneof: + +```proto +message GuardianScanNode { + message Config { + repeated string wallets = 1; // EOAs to scan (from the workflow's inputVariables) + repeated int64 chain_ids = 2; // mainnets only: [1, 8453] + } + message Output { google.protobuf.Value data = 1; } // { newFindings: [ … ] } + Config config = 1; +} +``` + +Runner `core/taskengine/vm_runner_guardian_scan.go`. Per run, for each `(wallet, chainId)`: + +1. **Scan** (port studio §5–§6 verbatim — this doc does not restate the verdict rules): + - approvals via Moralis `GET /wallets/{address}/approvals` → GoPlus + `GET /v2/token_approval_security` join on `(token, spender)`; `flagged` needs a strong signal + and a non-trust-listed spender (weak signals `honeypot_related_address`/`blacklist_doubt` never + flag alone). + - held tokens via balances → GoPlus `GET /v1/token_security` (top 25 by USD); `critical` per the + critical-flag set or `max(tax) ≥ 0.5`; `trust_list=="1"` overrides. + - alert set = **flagged approvals + critical tokens only** (never warnings/spam/unknown). +2. **Diff:** prefix-scan `WorkflowStateSeenPrefix(taskId)` → the set of already-seen `stateKey`s. + `new = currentFindings − seen`. For each current finding, upsert its `seen:` record + (`detail`, `firstSeenAt`, refresh `lastSeenAt`). +3. **Emit:** the node output is the **new findings** (`detail` payloads) — the downstream `BranchNode` + gates "any new findings? → notify", and the `RestAPINode` sends the Telegram DM. +4. **Degraded scans contribute nothing:** if a provider/chain errors, emit no finding for it and + **never delete** `seen:` rows — absence of data ≠ "flag cleared." + +### 3.2 Claim-once (exactly-once alert) + +The notify node (or the guardian-scan node, if it also delivers) claims per finding: + +``` +claimed, _ := db.SetIfAbsent(WorkflowStateNtfyKey(taskId, stateKey), nowMillisBytes) +if claimed { send alert; if send fails → db.Delete(ntfyKey) // release, retry next run } +else { already alerted — skip } +``` + +This is the `notifiedAt: null→now` claim from studio §7, implemented as key-existence CAS. + +### 3.3 Credentials (studio §3.3 / §5.1) + +- **Config only for the secrets:** add `goplus_app_key` / `goplus_app_secret` to `macros.secrets` in + `config/gateway.yaml` (+ `gateway.example.yaml`, `test.yaml`, `test.example.yaml`). `moralis_api_key` + is **already present**. No Go change to expose them — the loader reads `Macros["secrets"]` generically. +- **Server-side token mint (net-new helper):** GoPlus uses `sign = sha1_hex(app_key + time + app_secret)` + → `POST /api/v1/token` → `Bearer` token (~2h). The runner reads the secrets via + `GetMacroSecret("goplus_app_key"/"goplus_app_secret")` (engine.go), mints + module-caches the token + (refresh ~60s early), and calls GoPlus itself. **The `app_secret` is never templated into a node + and never leaves the gateway.** Keyless fallback: on app-level `code:4033`, retry once with no auth + header; accept `code ∈ {1,2}`. A first cut may run keyless entirely and add minting for the paid + CU tier at volume. + +--- + +## 4. The workflow shape studio emits + +Unchanged from the existing alert templates, with the scan step swapped to the native node: + +``` +[CronTrigger "0 * * * *"] → [GuardianScanNode wallets=[…] chains=[1,8453]] + → [BranchNode: newFindings.length > 0 ? "if" : "else"] + if → [RestAPINode → /api/notify OR api.telegram.org/bot{{apContext.configVars.ap_notify_bot_token}}/sendMessage] + else → (stop) +``` + +Cadence is cron (studio converts "hourly/6h" → cron; no separate `TimeInterval` type exists). The +scan `wallets`/`chains` come from the workflow's own config/`inputVariables` — **the client decides +what each workflow watches.** No signature is needed per run; deploy needs the owner's session JWT +(see §6.2). + +--- + +## 5. Worked state timeline (one workflow, one wallet) + +`notifiedAt` = presence of the `ntfy:` key. + +| Run | Scan finds | `seen:` before | Action | After | +|---|---|---|---|---| +| t0 | approval **A** | ∅ | A new → write `seen:A` → `SetIfAbsent(ntfy:A)`=claimed → **alert** ✅ | seen:{A}, ntfy:{A} | +| t1 | A again | {A} | A already seen; `ntfy:A` exists → **no alert** | unchanged | +| t2 | A + critical **B** | {A} | B new → write `seen:B` → claim B → **alert B** ✅ | seen:{A,B}, ntfy:{A,B} | +| t3 | *(Moralis down)* | {A,B} | degraded → emit nothing, **delete nothing** | unchanged | +| t4 | *(A revoked, clean)* | {A,B} | guardian alerts on **new** only; do **not** delete/alert on removal | unchanged | + +Send-failure variant at t0: `SetIfAbsent(ntfy:A)` claimed, Telegram POST fails → `Delete(ntfy:A)` → +next run re-claims and retries. Exactly-once preserved. + +--- + +## 6. Answers to studio §8 open questions + +1. **Per-workflow persistent state (the deciding capability):** **Being added** as generic `wfstate` + (§1). Not present today; additive, low-risk. +2. **No-fund deploy auth:** A read-only, notify-only scheduled workflow deploys with **no per-run + signature and no fund-authority check** (a task with no runner smart wallet skips the ownership + gate, `engine.go` `CreateWorkflow`). **But it is not signature-less:** `CreateWorkflow` requires a + Bearer **session JWT**, minted only after an EIP-191 wallet signature (per-48h session, not + per-run). `X-Partner-Assertion` is **forbidden for create** (partner.go) — it's simulate-only. So + enrollment needs the user to hold an AVS session, i.e. one wallet signature per 48h, not zero. + **Studio must design opt-in around a real session, not a partner assertion.** +3. **GoPlus token-mint injector:** creds in `macros.secrets`; token minted server-side via a net-new + sha1 helper reading `GetMacroSecret`; secret never exposed to a node (§3.3). Moralis key already + configured. +4. **Cost & cadence:** cron cadence is the lever (hourly vs 6h); pair with a paid GoPlus CU tier and a + brief per-`(chain,address)` verdict cache. Per-workflow executions are durably logged (`history:`) + — the observability win studio wanted. +5. **Migration:** re-deploy starts fresh state (§1.4). Handle existing enrolled users by seeding + `wfstate::seen:*` (mark current flags as already-seen, no `ntfy:` → no alert) at cutover, + or keep `taskId` stable. One-time, not a per-run concern. + +--- + +## 7. Build phases + +1. **Storage primitive** — `SetIfAbsent` + `DeleteByPrefix` on the `Storage` interface (reuse the + `IncCounter` serializable-txn shape); `wfstate` key builders in `schema.go`; cascade-delete hook in + the task-teardown path. Persist the guardian record struct under `model/` so `make storage-check` + tracks it (additive — new namespace + new struct, no migration). +2. **GoPlus client** — port studio `app/lib/goplus.ts` (auth mint + `token_security` + + `token_approval_security`), add secrets to `macros.secrets`. +3. **`guardian-scan` node** — `avs.proto` message + enum, `make protoc-gen`, runner + `vm_runner_guardian_scan.go` (scan → diff via `wfstate` → emit new findings), claim-once on notify. +4. **Template + wiring** — the cron→scan→branch→notify template studio emits; end-to-end test on + mainnet-only wallets (GoPlus has no testnet coverage). + +## 8. Non-goals / open choices + +- **Not** exposing `wfstate` to CustomCode JS in v1 (engine-internal only). Revisit as a public + `state.*` binding later. +- Claim-once granularity is per-`stateKey` (per flag); a coarser per-run claim is not needed. +- Whether the notify send lives in the guardian-scan node or a separate REST node is an + implementation detail — the claim-once logic is identical either way. + +--- + +*Planning only. No code changed by this document. Gateway-side response to the studio +`PLAN_GUARDIAN_MONITORING_ARCHITECTURE.md` hand-off.* From 67a21a0d88c1ff6409b0f3d335edf80a063a0dc2 Mon Sep 17 00:00:00 2001 From: Chris Li Date: Wed, 15 Jul 2026 21:29:42 -0700 Subject: [PATCH 09/16] docs: redesign guardian plan as composable extensions (drop bespoke node) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the native guardian-scan node with two generic, reusable gateway extensions, verified feasible against the code: 1) REST node server-side auth providers (GoPlus signed-token minted from macros.secrets, injected into processedHeaders; secret never in client JSON) — precedents: options.summarize reader, balance runner reading macroSecrets, the /api/notify summarizer's own outbound call. 2) client-defined per-workflow state via a state.* goja binding over wfstate: (client defines the JSON) — precedents: console.log Go-func binding, r.vm.db access, apContext AddVar injection. The guardian becomes a pure node composition (cron→REST→REST→customCode→ branch→notify) whose verdict/diff logic ports studio JS ~verbatim. Additive storage, no migration. --- PLAN_WORKFLOW_STATE_GUARDIAN_MONITORING.md | 386 ++++++++++----------- 1 file changed, 189 insertions(+), 197 deletions(-) diff --git a/PLAN_WORKFLOW_STATE_GUARDIAN_MONITORING.md b/PLAN_WORKFLOW_STATE_GUARDIAN_MONITORING.md index 87a99b43..035eae93 100644 --- a/PLAN_WORKFLOW_STATE_GUARDIAN_MONITORING.md +++ b/PLAN_WORKFLOW_STATE_GUARDIAN_MONITORING.md @@ -1,270 +1,262 @@ -# PLAN — Per-Workflow State + Native Guardian-Scan (gateway design) +# PLAN — Composable Workflow State + REST Auth Providers (guardian as a pure composition) -> **Audience.** EigenLayer-AVS gateway/backend. This is the gateway-side response to the studio -> hand-off `PLAN_GUARDIAN_MONITORING_ARCHITECTURE.md` (in the `studio` repo). It specifies the one -> capability the guardian actually needs and doesn't have — **durable, cross-run, per-workflow -> state** — as a *generic* primitive, with the continuous guardian monitor as its first consumer. +> **Audience.** EigenLayer-AVS gateway/backend. Gateway-side response to the studio hand-off +> `PLAN_GUARDIAN_MONITORING_ARCHITECTURE.md` (in the `studio` repo). > -> **Framing decision.** State is scoped to the **workflow (`taskId`)**, never to a user. There is -> **no "one monitoring workflow per user"** rule. A user may create workflow A to watch one EOA -> today, then workflow B to watch two other EOAs next week — two independent tasks, two independent -> state namespaces, zero user-level coupling. **The client (studio) owns setting each workflow up -> correctly** (which wallets, which cadence); the gateway just gives every workflow a private, -> cascade-scoped scratchpad and a native scan step. +> **Design decision (revised).** **No bespoke `guardian-scan` node.** Instead, add **two small, +> generic, reusable extensions** to existing primitives, and let the guardian be an ordinary workflow +> composed from them: +> 1. **REST node server-side auth providers** — the RestAPI node can mint/attach a provider's auth +> (GoPlus signed session token first) from gateway-held secrets, so a workflow calls GoPlus with a +> plain REST node and the secret never enters the (client-visible) workflow JSON. +> 2. **Client-defined per-workflow state** — a generic, cross-run key/value store scoped to the +> workflow (`taskId`), exposed to CustomCode as a `state.*` binding so the **client defines the JSON +> structure** and the diff/verdict logic. Useful far beyond the guardian. +> +> **Framing (unchanged).** State is scoped to the **workflow (`taskId`)**, never to a user. No "one +> workflow per user" rule — a user can create workflow A to watch one EOA today and workflow B to +> watch two other EOAs later; two tasks, two isolated state namespaces. **The client owns composing +> each workflow correctly.** --- -## 0. Why this doc exists - -The guardian needs to alert on **new** security flags **exactly once**. That requires remembering, run -to run, which flags a workflow has already seen and already alerted on. Today the gateway has **no -cross-run workflow state**: node runners get `vm.db` (Go side) but user-authored CustomCode JS gets no -DB handle, execution history is RPC-read-only (not readable mid-run), and the only "state-like" -things are a monotonic contract-write counter (`ct:cw:`), a 15-min idempotency TTL cache -(`idem:noderun:`), and per-execution checkpoints (`ckpt:`, deleted at terminal state). None -can hold arbitrary per-workflow values across runs. +## 0. Why this shape (composition, not a monolith) -Everything else the guardian needs already exists: `CronTrigger` (per-task gocron jobs, -`core/taskengine/trigger/time.go`), `BranchNode`, `RestAPINode` + `CustomCodeNode`, durable execution -logs (`history:` keys), and `{{apContext.configVars.*}}` credential injection (`core/taskengine/vm.go`, -sourced from `macros.secrets`). So this doc scopes only the missing core. +A bespoke node buries the guardian's logic in Go and helps nothing else. The two extensions below are +each independently useful (any workflow can call a signed-auth API; any workflow can be stateful), and +together they let the guardian be a **normal node graph** whose verdict/diff logic is **the studio JS +ported almost verbatim** (`app/lib/goplus.ts`, `app/lib/guardian/rescan.ts`). Everything else the +guardian needs already exists: `CronTrigger`, `BranchNode`, `RestAPINode`, `CustomCodeNode`, durable +`history:` logs, and `{{apContext.configVars.*}}` injection. --- -## 1. The generic capability: per-workflow state (`wfstate`) +## 1. Extension 1 — REST node server-side auth providers (GoPlus first) -A workflow-scoped key/value store that survives across executions and is cleaned up with the workflow. +Let the RestAPI node attach a provider-minted credential at execution time, resolved entirely +server-side. **Feasibility: EASY** (every primitive has a working precedent in the same file). -### 1.1 Key schema (matches `core/taskengine/schema.go` conventions) +### 1.1 How the client asks for it -``` -wfstate::seen: → JSON record (the diff-set; prefix-scanned each run) -wfstate::ntfy: → unix-ms (claim marker; presence = "already alerted") -``` +Reuse the already-generic `options` bag (`RestAPINode.Config.Options`, a `google.protobuf.Value`; +`options.summarize` is read today at `vm_runner_rest.go:832`). No proto change: -- `taskId` — the ULID of the workflow. **This is the entire scoping key. No `owner`, no `wallet` in - the namespace** — the workflow already knows what it monitors (its config / input variables), and - two workflows of the same user get different `taskId`s and thus fully isolated state. -- `stateKey` — an opaque, consumer-defined string. The guardian uses `::` - so one workflow can monitor several EOAs on several chains without collision. -- `seen:` vs `ntfy:` split keeps the diff-set scan clean and lets the claim be a pure key-existence - CAS (no JSON coupling in the storage layer). +```jsonc +// A GoPlus call in a plain REST node — no secret in the JSON: +{ "type": "restApi", + "config": { + "method": "GET", + "url": "https://api.gopluslabs.io/api/v1/token_security/1?contract_addresses=0x…", + "options": { "auth": { "provider": "goplus" } } + } } +``` -### 1.2 Storage interface additions (`storage/db.go`) +Optionally also **URL auto-detection** (`api.gopluslabs.io` → goplus), mirroring the existing +`detectNotificationProvider` (`vm_runner_rest.go:848`). Explicit `options.auth` is preferred (clear intent). -`Set`, `GetKey`, `Delete`, `BatchWrite`, `Exist`, and prefix scan (used today by `ListExecutions`) -already exist. Add two small primitives: +### 1.2 What the gateway does (server-side, secret never leaves) -```go -// SetIfAbsent atomically writes value iff key does not exist. Returns claimed=true only for the -// caller that performed the write. Reuses the serializable db.Update(txn) pattern already used by -// IncCounter (storage/db.go:~486): txn.Get(key) → ErrKeyNotFound ? txn.Set : no-op. -SetIfAbsent(key []byte, value []byte) (claimed bool, err error) +Precedents this reuses directly: +- **Read a gateway secret in the runner, not from client JSON:** `vm_runner_balance.go:272` reads + `macroSecrets["moralis_api_key"]`; accessor `GetMacroSecret(...)` at `engine.go:90`. GoPlus keys go + in `macros.secrets` beside Moralis (config-only; §4). +- **Runner makes its own outbound call with a gateway credential:** the `/api/notify` summarizer + (`vm_runner_rest.go:649-682`, its own `http.Client`). +- **Inject the header pre-flight:** `processedHeaders` is built at `vm_runner_rest.go:554-572` and + consumed by `ExecuteRequest` at `:732` — insert one block in between. -// DeleteByPrefix removes every key under a prefix (workflow-teardown cascade). -DeleteByPrefix(prefix []byte) (int, error) +Flow for `provider:"goplus"`: ``` - -`SetIfAbsent` is the exactly-once primitive; nothing generic like it exists today (grep for -`CompareAndSet|SetIfAbsent|SetIfNotExist` is empty), but BadgerDB's `Update` gives the required -serializable isolation and the exact txn shape is copy-paste from `IncCounter`. - -### 1.3 Helper key builders (`core/taskengine/schema.go`) - -```go -func WorkflowStateSeenKey(taskID, stateKey string) []byte // "wfstate:%s:seen:%s" -func WorkflowStateSeenPrefix(taskID string) []byte // "wfstate:%s:seen:" (diff scan) -func WorkflowStateNtfyKey(taskID, stateKey string) []byte // "wfstate:%s:ntfy:%s" -func WorkflowStatePrefix(taskID string) []byte // "wfstate:%s:" (cascade delete) +token := goplusToken() // module-cached; mint if missing/expiring +processedHeaders["Authorization"] = token // GoPlus token already includes "Bearer " ``` +`goplusToken()` (net-new, ~40 lines): `sign = sha1_hex(app_key + time + app_secret)` → +`POST /api/v1/token` → cache `{value, expiresAt}` behind an `sync.RWMutex`, refresh ~60s early. On +GoPlus app-level `code:4033`, drop the header and retry keyless (studio §5.1). New import `crypto/sha1`. -`wfstate` is a fresh namespace (no existing `wfstate:` literal collides — verified against the -namespace list in `core/taskengine/doc.go`). - -### 1.4 Lifecycle +### 1.3 Generic framing -- **Create:** implicit — the first `Set`/`SetIfAbsent` under a `taskId` creates its state. -- **Cascade delete:** the task-teardown path (engine `DeleteWorkflow` / task deletion) calls - `DeleteByPrefix(WorkflowStatePrefix(taskId))`. Delete the workflow → its state is gone. **No orphan - bookkeeping, no per-user cleanup.** -- **Re-deploy = fresh state (the one tradeoff).** A new workflow has a new `taskId`, so its state - starts empty and its first run re-alerts on all currently-active flags **once**. For the guardian - this is benign (surfacing real current risk to someone who just re-opted-in), and mitigable by - **keeping `taskId` stable across re-enroll** (update-in-place instead of delete+create) or seeding - new-workflow state at migration. Documented, not a blocker. - -### 1.5 Access boundary (v1: engine-internal, not CustomCode JS) - -Node **runners** (Go, `vm_runner_*.go`) already receive `vm.db` (`core/taskengine/vm.go`, `WithDb`) -and the executing task context (so they know `taskId`). The guardian-scan runner (§3) reads/writes -`wfstate` through `vm.db` directly. We **do not** expose `wfstate` to user-authored CustomCode JS in -v1 (goja gets no DB handle by design) — that would turn every workflow stateful and pull in size -limits, isolation, and abuse concerns. A future `state.get/set` JS binding can layer on this same -primitive if we decide to make it a public building block. +Model it as a tiny **auth-provider registry** (`provider → mintFn`) so future signed-token APIs plug in +without touching the runner core. GoPlus is provider #1. **Change surface: `vm_runner_rest.go` only** +(a `restAuthProvider(node)` reader like `shouldSummarize`, a `mintGoPlusToken()` + cache, one injection +block). No proto, no SDK type change. --- -## 2. The user scenario, modeled +## 2. Extension 2 — client-defined per-workflow state + +A generic cross-run key/value store scoped to `taskId`, with the **client defining the JSON** via a +CustomCode binding. **Feasibility: EASY→MODERATE.** -> User creates **Workflow A** to monitor **EOA1**; two weeks later creates **Workflow B** to monitor -> **EOA2 + EOA3**. +### 2.1 Storage layer (`storage/db.go` + `core/taskengine/schema.go`) ``` -Workflow A (taskId = 01A…) Workflow B (taskId = 01B…) - wfstate:01A…:seen:1:0xEOA1:approval:0x…:0x… wfstate:01B…:seen:1:0xEOA2:token:0x… - wfstate:01A…:seen:1:0xEOA1:token:0x… wfstate:01B…:seen:8453:0xEOA3:approval:0x…:0x… +wfstate:: → client-defined JSON blob ``` +- `taskId` (ULID) is the whole scope — **no owner/wallet in the key**; two workflows of one user get + isolated namespaces. `stateKey` is opaque/client-chosen. +- Fresh namespace (`wfstate:` doesn't collide — verified vs `core/taskengine/doc.go`). Key builders in + `schema.go` (`WorkflowStateKey`, `WorkflowStatePrefix`). +- `Storage` already has `Set`, `GetKey`, `Delete`, `GetByPrefix`, `ListKeys`, `BatchWrite` + (`storage/db.go:23`). Add two small primitives: + - `SetIfAbsent(key, value) (claimed bool, err error)` — atomic exactly-once (reuse the + `IncCounter` serializable `db.Update(txn)` shape at `storage/db.go:~486`). Optional; only needed + for strict claim-once under concurrent runs (§3.3). + - `DeleteByPrefix(prefix) (int, error)` — the workflow-teardown cascade. + +### 2.2 The `state.*` CustomCode binding (client defines the structure) + +goja binds Go closures as JS methods today (`console.log` at `utils.go:116`); runners hold `r.vm.db` +(`WithDb`, `vm.go:332`) and the task id (`r.vm.GetTaskId()`, `vm.go:2469`). So in `NewJSProcessor` / +`NewJSProcessorWithIsolatedVars` (`vm_runner_customcode.go:29, 80`) bind: + +```js +state.get(key) // → stored JSON (or undefined) +state.set(key, value) // persist arbitrary JSON under wfstate::key +state.list(prefix?) // → [key, …] for diffing +state.setIfAbsent(key, val) // → bool (optional, strict claim-once) +``` +The closures capture `r.vm.db` + `taskId`. The **client writes arbitrary JS over arbitrary JSON** — +this is the "broader use case" surface (rate-limiters, "last processed block", DCA "already-bought", +dedup, and the guardian's flag diff). -Two tasks, two `wfstate::…` namespaces, no shared "user" record. B monitoring two EOAs is -just two `stateKey` prefixes inside B's own namespace. Deleting A wipes `wfstate:01A…:*` and leaves B -untouched. This is exactly the "workflow does not attach a user" model. - ---- +Two caveats to honor: +- **Simulation must not mutate real state.** Gate writes on `vm.IsSimulation` (`vm.go:247`) — + `nodes:run` / `workflows:simulate` dry-runs get a scratch/no-op `state` so previews never persist. +- **Atomicity.** A direct `state.set` hits the db immediately (not atomic with the `history:` write at + `executor.go:784`). For the guardian that's fine (mark-after-send, §3.3). If atomic-with-execution + is wanted later, buffer writes and flush them into the executor's `updates` batch (`executor.go:754-787`), + exactly as durable checkpoints do (`durable.go:217/295`). -## 3. The native `guardian-scan` node (first consumer) +### 2.3 Optional read-side sugar -A new engine-side node type — because the scan must (a) reach `wfstate` via `vm.db`, (b) hold the -GoPlus/Moralis credentials without leaking them into client-visible workflow JSON, and (c) mint the -GoPlus signed token — none of which a CustomCode JS node can do. +One line in `NewVMWithData…` (`vm.go:424`, beside the apContext injection at `:457`): +`v.AddVar("state", loadWorkflowState(db, taskId))` — makes `{{state.*}}` template-readable in **every** +node (REST url/body, Branch expressions), not just CustomCode. Cheap, high-leverage; can follow v1. -### 3.1 Node shape (protobuf `avs.proto` → `make protoc-gen`) +### 2.4 Lifecycle -Add `NODE_TYPE_GUARDIAN_SCAN` to the node enum and a message to the `TaskNode` oneof: +Cascade-delete `wfstate::*` in the task-teardown path (engine `DeleteWorkflow`). Re-deploy = +new `taskId` = fresh state (first run re-alerts once — benign for the guardian; mitigate by keeping +`taskId` stable across re-enroll, or seed state at migration). -```proto -message GuardianScanNode { - message Config { - repeated string wallets = 1; // EOAs to scan (from the workflow's inputVariables) - repeated int64 chain_ids = 2; // mainnets only: [1, 8453] - } - message Output { google.protobuf.Value data = 1; } // { newFindings: [ … ] } - Config config = 1; -} -``` +--- -Runner `core/taskengine/vm_runner_guardian_scan.go`. Per run, for each `(wallet, chainId)`: +## 3. The guardian as a pure composition (no bespoke node) -1. **Scan** (port studio §5–§6 verbatim — this doc does not restate the verdict rules): - - approvals via Moralis `GET /wallets/{address}/approvals` → GoPlus - `GET /v2/token_approval_security` join on `(token, spender)`; `flagged` needs a strong signal - and a non-trust-listed spender (weak signals `honeypot_related_address`/`blacklist_doubt` never - flag alone). - - held tokens via balances → GoPlus `GET /v1/token_security` (top 25 by USD); `critical` per the - critical-flag set or `max(tax) ≥ 0.5`; `trust_list=="1"` overrides. - - alert set = **flagged approvals + critical tokens only** (never warnings/spam/unknown). -2. **Diff:** prefix-scan `WorkflowStateSeenPrefix(taskId)` → the set of already-seen `stateKey`s. - `new = currentFindings − seen`. For each current finding, upsert its `seen:` record - (`detail`, `firstSeenAt`, refresh `lastSeenAt`). -3. **Emit:** the node output is the **new findings** (`detail` payloads) — the downstream `BranchNode` - gates "any new findings? → notify", and the `RestAPINode` sends the Telegram DM. -4. **Degraded scans contribute nothing:** if a provider/chain errors, emit no finding for it and - **never delete** `seen:` rows — absence of data ≠ "flag cleared." +``` +[CronTrigger "0 * * * *"] + → [restApi Moralis: approvals + balances] (macros: moralis_api_key) + → [restApi GoPlus: token_security / token_approval_security] (options.auth: goplus ← Ext 1) + → [customCode verdict + diff] (state.* ← Ext 2) + → [branch newFindings.length > 0 ?] + if → [restApi notify → /api/notify | api.telegram.org/…] ({{apContext.configVars.ap_notify_bot_token}}) + → [customCode mark-notified on send success] (state.set ← Ext 2) +``` -### 3.2 Claim-once (exactly-once alert) +### 3.1 The verdict+diff CustomCode (ports studio JS ~verbatim) -The notify node (or the guardian-scan node, if it also delivers) claims per finding: +Reads the upstream REST outputs from `vm.vars`, applies the studio verdict rules (§6 of the studio +plan — `trust_list` override, critical/warning flag sets, tax thresholds; approvals `flagged` needs a +strong signal and a non-trust-listed spender, weak signals never flag alone), then diffs against state: +```js +const seen = new Set(state.list("seen:")); // flags already recorded +const current = computeFindings(moralis.data, goplus.data);// studio verdict logic, verbatim +const notified = new Set(state.list("ntfy:")); +const newFindings = current.filter(f => !notified.has("ntfy:" + f.flagKey)); +for (const f of current) state.set("seen:" + f.flagKey, f.detail); // record seen (never delete) +return { newFindings }; // → branch + notify ``` -claimed, _ := db.SetIfAbsent(WorkflowStateNtfyKey(taskId, stateKey), nowMillisBytes) -if claimed { send alert; if send fails → db.Delete(ntfyKey) // release, retry next run } -else { already alerted — skip } -``` +`flagKey` = `approval:{token}:{spender}` or `token:{tokenAddress}` (lowercased). Degraded scan (a REST +node errored) → `computeFindings` yields nothing for it and we **never delete** `seen:` rows (absence ≠ +cleared). + +### 3.2 Notify -This is the `notifiedAt: null→now` claim from studio §7, implemented as key-existence CAS. +Branch gates on `newFindings.length > 0`; the notify REST node sends the studio-formatted plaintext DM +(read-only, counterfactual voice — studio §7.1). -### 3.3 Credentials (studio §3.3 / §5.1) +### 3.3 Claim-once (exactly-once alerting) -- **Config only for the secrets:** add `goplus_app_key` / `goplus_app_secret` to `macros.secrets` in - `config/gateway.yaml` (+ `gateway.example.yaml`, `test.yaml`, `test.example.yaml`). `moralis_api_key` - is **already present**. No Go change to expose them — the loader reads `Macros["secrets"]` generically. -- **Server-side token mint (net-new helper):** GoPlus uses `sign = sha1_hex(app_key + time + app_secret)` - → `POST /api/v1/token` → `Bearer` token (~2h). The runner reads the secrets via - `GetMacroSecret("goplus_app_key"/"goplus_app_secret")` (engine.go), mints + module-caches the token - (refresh ~60s early), and calls GoPlus itself. **The `app_secret` is never templated into a node - and never leaves the gateway.** Keyless fallback: on app-level `code:4033`, retry once with no auth - header; accept `code ∈ {1,2}`. A first cut may run keyless entirely and add minting for the paid - CU tier at volume. +Mark **after** a successful send: the trailing CustomCode reads the notify node's response from +`vm.vars` and, only if it succeeded, `state.set("ntfy:"+flagKey, now)` for each finding just sent. On +send failure nothing is marked → next run retries (§3.1 diffs against `ntfy:`). This is +practically-exactly-once for a sequential per-workflow cron and self-healing on transient failures. +For strict exactly-once under overlapping runs, pre-claim with `state.setIfAbsent("ntfy:"+flagKey, now)` +before sending and release (`state.set(..., null)` / delete) on failure. --- -## 4. The workflow shape studio emits +## 4. The user scenario, modeled -Unchanged from the existing alert templates, with the scan step swapped to the native node: +> Workflow A watches EOA1; two weeks later Workflow B watches EOA2 + EOA3. ``` -[CronTrigger "0 * * * *"] → [GuardianScanNode wallets=[…] chains=[1,8453]] - → [BranchNode: newFindings.length > 0 ? "if" : "else"] - if → [RestAPINode → /api/notify OR api.telegram.org/bot{{apContext.configVars.ap_notify_bot_token}}/sendMessage] - else → (stop) +Workflow A (taskId 01A…) Workflow B (taskId 01B…) + wfstate:01A…:seen:approval:0x…:0x… wfstate:01B…:seen:token:0x… (EOA2) + wfstate:01A…:ntfy:approval:0x…:0x… wfstate:01B…:seen:approval:0x…:0x… (EOA3) ``` - -Cadence is cron (studio converts "hourly/6h" → cron; no separate `TimeInterval` type exists). The -scan `wallets`/`chains` come from the workflow's own config/`inputVariables` — **the client decides -what each workflow watches.** No signature is needed per run; deploy needs the owner's session JWT -(see §6.2). +Two tasks, two `wfstate::*` namespaces, no shared user record. B watching two EOAs just uses +`stateKey`s that encode the wallet (`::`). Delete A → `wfstate:01A…:*` wiped, +B untouched. --- -## 5. Worked state timeline (one workflow, one wallet) - -`notifiedAt` = presence of the `ntfy:` key. +## 5. Worked timeline (one workflow, one wallet) -| Run | Scan finds | `seen:` before | Action | After | +| Run | REST scan finds | state before | CustomCode + notify | state after | |---|---|---|---|---| -| t0 | approval **A** | ∅ | A new → write `seen:A` → `SetIfAbsent(ntfy:A)`=claimed → **alert** ✅ | seen:{A}, ntfy:{A} | -| t1 | A again | {A} | A already seen; `ntfy:A` exists → **no alert** | unchanged | -| t2 | A + critical **B** | {A} | B new → write `seen:B` → claim B → **alert B** ✅ | seen:{A,B}, ntfy:{A,B} | -| t3 | *(Moralis down)* | {A,B} | degraded → emit nothing, **delete nothing** | unchanged | -| t4 | *(A revoked, clean)* | {A,B} | guardian alerts on **new** only; do **not** delete/alert on removal | unchanged | +| t0 | approval **A** | ∅ | A not in `ntfy:` → newFindings=[A] → send ✅ → mark `ntfy:A` | seen:{A}, ntfy:{A} | +| t1 | A again | {A} | A in `ntfy:` → newFindings=[] → no send | unchanged | +| t2 | A + critical **B** | {A} | B new → send ✅ → mark `ntfy:B` | seen:{A,B}, ntfy:{A,B} | +| t3 | *(Moralis errors)* | {A,B} | degraded → no findings, delete nothing | unchanged | +| t4 | *(A revoked, clean)* | {A,B} | alert on **new** only; no delete/alert on removal | unchanged | -Send-failure variant at t0: `SetIfAbsent(ntfy:A)` claimed, Telegram POST fails → `Delete(ntfy:A)` → -next run re-claims and retries. Exactly-once preserved. +Send-failure at t0: notify REST node fails → trailing CustomCode does **not** mark `ntfy:A` → next run +re-sends. Exactly-once preserved without an explicit release. --- ## 6. Answers to studio §8 open questions -1. **Per-workflow persistent state (the deciding capability):** **Being added** as generic `wfstate` - (§1). Not present today; additive, low-risk. -2. **No-fund deploy auth:** A read-only, notify-only scheduled workflow deploys with **no per-run - signature and no fund-authority check** (a task with no runner smart wallet skips the ownership - gate, `engine.go` `CreateWorkflow`). **But it is not signature-less:** `CreateWorkflow` requires a - Bearer **session JWT**, minted only after an EIP-191 wallet signature (per-48h session, not - per-run). `X-Partner-Assertion` is **forbidden for create** (partner.go) — it's simulate-only. So - enrollment needs the user to hold an AVS session, i.e. one wallet signature per 48h, not zero. - **Studio must design opt-in around a real session, not a partner assertion.** -3. **GoPlus token-mint injector:** creds in `macros.secrets`; token minted server-side via a net-new - sha1 helper reading `GetMacroSecret`; secret never exposed to a node (§3.3). Moralis key already - configured. -4. **Cost & cadence:** cron cadence is the lever (hourly vs 6h); pair with a paid GoPlus CU tier and a - brief per-`(chain,address)` verdict cache. Per-workflow executions are durably logged (`history:`) - — the observability win studio wanted. -5. **Migration:** re-deploy starts fresh state (§1.4). Handle existing enrolled users by seeding - `wfstate::seen:*` (mark current flags as already-seen, no `ntfy:` → no alert) at cutover, - or keep `taskId` stable. One-time, not a per-run concern. +1. **Per-workflow persistent state (deciding capability):** added as generic `wfstate` + a `state.*` + CustomCode binding (§2). Client-defined JSON; reusable beyond the guardian. +2. **No-fund deploy auth:** a read-only, notify-only scheduled workflow deploys with **no per-run + signature and no fund-authority check** (a task with no runner wallet skips the ownership gate, + `engine.go` `CreateWorkflow`). **But not signature-less:** `CreateWorkflow` needs a Bearer session + JWT, minted only after an EIP-191 wallet signature (per-48h, not per-run). `X-Partner-Assertion` is + **forbidden for create** — simulate-only. Studio must design opt-in around a real session. +3. **GoPlus token-mint:** Extension 1 — creds in `macros.secrets`, token minted + cached server-side + in the REST runner, never exposed to the node. Moralis key already configured. +4. **Cost & cadence:** cron cadence is the lever (hourly vs 6h); pair with a paid GoPlus CU tier + a + brief per-`(chain,address)` verdict cache. Runs are durably logged (`history:`). +5. **Migration:** re-deploy starts fresh state (§2.4); seed `wfstate::ntfy:*` (mark current + flags notified, no alert) at cutover, or keep `taskId` stable. One-time. --- ## 7. Build phases -1. **Storage primitive** — `SetIfAbsent` + `DeleteByPrefix` on the `Storage` interface (reuse the - `IncCounter` serializable-txn shape); `wfstate` key builders in `schema.go`; cascade-delete hook in - the task-teardown path. Persist the guardian record struct under `model/` so `make storage-check` - tracks it (additive — new namespace + new struct, no migration). -2. **GoPlus client** — port studio `app/lib/goplus.ts` (auth mint + `token_security` + - `token_approval_security`), add secrets to `macros.secrets`. -3. **`guardian-scan` node** — `avs.proto` message + enum, `make protoc-gen`, runner - `vm_runner_guardian_scan.go` (scan → diff via `wfstate` → emit new findings), claim-once on notify. -4. **Template + wiring** — the cron→scan→branch→notify template studio emits; end-to-end test on +1. **Extension 1 (REST auth providers)** — `restAuthProvider(node)` reader + `mintGoPlusToken()` + + RWMutex token cache + header injection, all in `vm_runner_rest.go`; add `goplus_app_key`/ + `goplus_app_secret` to `macros.secrets` (`config/gateway.yaml` + examples/test). Keyless fallback + on `code:4033`. Independently shippable and testable (a REST node hitting GoPlus authed). +2. **Extension 2 (workflow state)** — `SetIfAbsent`/`DeleteByPrefix` on `Storage` (reuse `IncCounter` + txn); `wfstate` key builders; `state.*` goja binding in `vm_runner_customcode.go` (honor + `IsSimulation`); cascade-delete hook in task teardown; persist any tracked record struct under + `model/`. Additive storage (new namespace, no migration — `make storage-check` clean). +3. **Guardian composition** — port studio `goplus.ts`/`rescan.ts` verdict logic into the CustomCode + node; assemble the cron→REST→REST→customCode→branch→notify template studio emits; e2e on mainnet-only wallets (GoPlus has no testnet coverage). +4. **Optional** — `{{state.*}}` auto-load (§2.3) for template-read in any node. ## 8. Non-goals / open choices -- **Not** exposing `wfstate` to CustomCode JS in v1 (engine-internal only). Revisit as a public - `state.*` binding later. -- Claim-once granularity is per-`stateKey` (per flag); a coarser per-run claim is not needed. -- Whether the notify send lives in the guardian-scan node or a separate REST node is an - implementation detail — the claim-once logic is identical either way. +- Exposing `state.*` to CustomCode **is** the design (previously a non-goal). Guard it with + `IsSimulation` and, if needed, a per-workflow state-size cap. +- Claim-once is mark-after-send (§3.3); `setIfAbsent` pre-claim is the opt-in for strict concurrency. +- Auth-provider registry starts with GoPlus; add providers as needed without touching the runner core. --- From e94fd9e10b81c9b7ed48586ddeaa07d324e5553a Mon Sep 17 00:00:00 2001 From: Chris Li Date: Wed, 15 Jul 2026 21:50:36 -0700 Subject: [PATCH 10/16] docs: formalize {{state.*}} namespace for workflow custom storage Position state as the client-defined read/write persistent namespace parallel to {{settings.*}} (config) and {{context.*}} (read-only runtime): namespace table, snake_case sub-key conventions, and the reservedSystemVarNames registration requirement so it's re-injected fresh and collision-safe. --- PLAN_WORKFLOW_STATE_GUARDIAN_MONITORING.md | 26 +++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/PLAN_WORKFLOW_STATE_GUARDIAN_MONITORING.md b/PLAN_WORKFLOW_STATE_GUARDIAN_MONITORING.md index 035eae93..f37f7c22 100644 --- a/PLAN_WORKFLOW_STATE_GUARDIAN_MONITORING.md +++ b/PLAN_WORKFLOW_STATE_GUARDIAN_MONITORING.md @@ -83,11 +83,35 @@ block). No proto, no SDK type change. --- -## 2. Extension 2 — client-defined per-workflow state +## 2. Extension 2 — client-defined per-workflow state (`{{state.*}}`) A generic cross-run key/value store scoped to `taskId`, with the **client defining the JSON** via a CustomCode binding. **Feasibility: EASY→MODERATE.** +### 2.0 Namespace — `state`, parallel to `settings` + +The client already configures workflows with `{{settings.*}}` (immutable config: `runner`, +`chain_id`, `address_list`, …) and reads platform runtime via `{{context.*}}` (read-only). Neither is +mutable + persistent + client-defined. Add one namespace — **`state`** — for exactly that: + +| Namespace | Role | Mutability | Set by | +|---|---|---|---| +| `{{apContext.configVars.*}}` | platform secrets | immutable, read-only | gateway config | +| `{{settings.*}}` | workflow config | immutable, read-only | client at create | +| `{{context.*}}` | runtime metadata | read-only | gateway | +| **`{{state.*}}`** | **workflow memory** | **read + write, persists across runs** | **client (CustomCode)** | + +Rule of thumb: **`settings` in, `state` in/out, `context` read-only.** `state` is chosen (over +`store`/`storage`/`memory`) because it's the same word as the CustomCode binding and consistent with +the existing single-word lowercase namespaces. Verified: `state` is unused as a var namespace today. +**Register `state` in `reservedSystemVarNames` (`durable.go:204`)** alongside `settings`/`context` so +it's re-injected fresh each run (never snapshotted as a node output) and can't be clobbered by a node +named `state`. Client `state.` maps to storage `wfstate::`. + +Sub-key conventions (mirror `settings` snake_case): scalars/objects as top-level keys +(`{{state.last_processed_block}}`, `{{state.cursor}}`, template-readable anywhere); iterated +collections as prefixed keys via the JS binding (`state.set("seen:"+id, …)` / `state.list("seen:")`). + ### 2.1 Storage layer (`storage/db.go` + `core/taskengine/schema.go`) ``` From 4ee40eb06901ca4b3eba62eb72ee69b635114d44 Mon Sep 17 00:00:00 2001 From: Chris Li Date: Wed, 15 Jul 2026 22:39:39 -0700 Subject: [PATCH 11/16] =?UTF-8?q?docs:=20guardian=20plan=20=C2=A79=20?= =?UTF-8?q?=E2=80=94=20resolve=20client-review=20A-D=20with=20gateway=20fa?= =?UTF-8?q?cts?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add gateway-side commitments from the studio client review: (A) no in-place update confirmed → guardian_ruleset configVar for central rule updates + track an in-place UpdateWorkflow; (B) read-only create-auth relaxation via partner assertion as the Telegram-only enrollment fix, counterfactual runner accepted; (C) serialized + at-most-once + no-retry execution makes mark-after-send (at-least-once) the correct security bias, not a shortcut; (D) default cadence 6h, approvals-first v1. --- PLAN_WORKFLOW_STATE_GUARDIAN_MONITORING.md | 42 ++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/PLAN_WORKFLOW_STATE_GUARDIAN_MONITORING.md b/PLAN_WORKFLOW_STATE_GUARDIAN_MONITORING.md index f37f7c22..27521ffe 100644 --- a/PLAN_WORKFLOW_STATE_GUARDIAN_MONITORING.md +++ b/PLAN_WORKFLOW_STATE_GUARDIAN_MONITORING.md @@ -284,5 +284,47 @@ re-sends. Exactly-once preserved without an explicit release. --- +## 9. Client-review resolutions (A–D) — gateway-side commitments + +The studio client-side review of the composed workflow surfaced four items. Resolutions, and the +gateway facts behind them (all verified against current code): + +- **A — Logic-freeze / central update.** Confirmed: there is **no in-place workflow update** today + (`handlers_workflows.go` exposes only create/get/delete/pause/resume; editing a node's `customCode` + means delete+recreate → new `taskId` → lost `wfstate`). Two gateway actions: + 1. **Expose a `guardian_ruleset` (and, generally, feature-ruleset) configVar** so a workflow can read + volatile rules (flag sets, thresholds, trust overrides) at runtime and a single gateway-config + change propagates to **all** deployed instances on next run — no re-deploy, no state loss. (Note: + only `macros.secrets` currently reaches `apContext.configVars`; either place the ruleset there or + add `macros.vars` injection.) + 2. **Add an in-place `UpdateWorkflow`** (edit node config, **same `taskId`, preserve `wfstate`). + Without it, any change to the frozen interpreter shape is a fleet re-deploy + state migration. This + is now the strongest driver for the feature — track it as its own item. +- **B — Read-only deploy auth.** Confirmed: `CreateWorkflow` requires a user JWT (EIP-191 signature); + partner assertion is rejected for create; **no** fund-moving-vs-read-only distinction exists. The + ownership gate is a pure DB check, so a **counterfactual (undeployed) runner is accepted** for a + read-only workflow. **Gateway decision to make:** allow the **read-only / no-runner-fund class to be + created via `X-Partner-Assertion` (or a managed signer)** so studio can enroll social/Telegram-only + users with zero user signature. Low risk (moves no funds; worst case is unwanted notifications). Until + decided, studio uses a one-time web-sign hand-off (one signature ever). +- **C — Claim-once semantics.** Confirmed: executions are **serialized per task** (single FIFO worker, + no overlap) and **at-most-once with NO retry** (`apqueue/worker.go` marks failed and drops; + `Recover()` is a no-op). Therefore **mark-after-send (at-least-once) is the correct design for a + security monitor** — never miss a real flag; a rare duplicate (send ok → mark fails → re-send next + tick) is the safe direction. Pre-claim (`setIfAbsent`) would convert a send failure into a permanent + miss and is **not** the default. `state` binding for v1 = `get/set/list` (`setIfAbsent` optional). + One detail to verify: the notify node sends a **single pre-built JSON body template** + (`{{verdict.data.telegramBody}}`) — ensure `preprocessJSONWithVariableMapping` forwards it verbatim + rather than re-parsing/re-serializing (which could corrupt the escaped payload). +- **D — Cadence / cost.** **Default cadence is 6h** (`0 */6 * * *`), not hourly. Scans/day = + `users × wallets × chains × (24/cadenceHours)`; approvals-only = 2 external calls/scan. Pair with a + paid GoPlus CU tier; add a brief per-`(chain,address)` verdict cache. **Scope: approvals-only for v1**; + held-token scan is a fast-follow. + +`wfstate` growth is bounded by distinct-flags-ever per workflow (small in practice); no prune on clean +scans (absence ≠ cleared, studio parity). A time-based `wfstate` TTL is a future option if needed. + +--- + *Planning only. No code changed by this document. Gateway-side response to the studio `PLAN_GUARDIAN_MONITORING_ARCHITECTURE.md` hand-off.* From c46fd468aa35d77573aeb3183484173f4a0ef59f Mon Sep 17 00:00:00 2001 From: Chris Li Date: Thu, 16 Jul 2026 10:28:49 -0700 Subject: [PATCH 12/16] =?UTF-8?q?docs:=20fold=20studio=20A/B=20decisions?= =?UTF-8?q?=20into=20guardian=20plan=20=C2=A79?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Align gateway plan with the studio-side decisions (2026-07-15): A — accept the logic-freeze, no UpdateWorkflow ask; structural changes handled by re-create + silent-seed first run (no wfstate read API needed). B — web hand-off is the enrollment path (one EIP-191 sign ever, no create-auth change); partner- assertion relaxation parked as a future lever. Net gateway scope shrinks to the two extensions + guardian_ruleset configVar. --- PLAN_WORKFLOW_STATE_GUARDIAN_MONITORING.md | 39 ++++++++++++---------- 1 file changed, 22 insertions(+), 17 deletions(-) diff --git a/PLAN_WORKFLOW_STATE_GUARDIAN_MONITORING.md b/PLAN_WORKFLOW_STATE_GUARDIAN_MONITORING.md index 27521ffe..f31397ef 100644 --- a/PLAN_WORKFLOW_STATE_GUARDIAN_MONITORING.md +++ b/PLAN_WORKFLOW_STATE_GUARDIAN_MONITORING.md @@ -289,24 +289,29 @@ re-sends. Exactly-once preserved without an explicit release. The studio client-side review of the composed workflow surfaced four items. Resolutions, and the gateway facts behind them (all verified against current code): -- **A — Logic-freeze / central update.** Confirmed: there is **no in-place workflow update** today +- **A — Logic-freeze / central update. DECIDED (studio, 2026-07-15): accept the freeze; NO + `UpdateWorkflow` ask.** Confirmed there is no in-place workflow update today (`handlers_workflows.go` exposes only create/get/delete/pause/resume; editing a node's `customCode` - means delete+recreate → new `taskId` → lost `wfstate`). Two gateway actions: - 1. **Expose a `guardian_ruleset` (and, generally, feature-ruleset) configVar** so a workflow can read - volatile rules (flag sets, thresholds, trust overrides) at runtime and a single gateway-config - change propagates to **all** deployed instances on next run — no re-deploy, no state loss. (Note: - only `macros.secrets` currently reaches `apContext.configVars`; either place the ruleset there or - add `macros.vars` injection.) - 2. **Add an in-place `UpdateWorkflow`** (edit node config, **same `taskId`, preserve `wfstate`). - Without it, any change to the frozen interpreter shape is a fleet re-deploy + state migration. This - is now the strongest driver for the feature — track it as its own item. -- **B — Read-only deploy auth.** Confirmed: `CreateWorkflow` requires a user JWT (EIP-191 signature); - partner assertion is rejected for create; **no** fund-moving-vs-read-only distinction exists. The - ownership gate is a pure DB check, so a **counterfactual (undeployed) runner is accepted** for a - read-only workflow. **Gateway decision to make:** allow the **read-only / no-runner-fund class to be - created via `X-Partner-Assertion` (or a managed signer)** so studio can enroll social/Telegram-only - users with zero user signature. Low risk (moves no funds; worst case is unwanted notifications). Until - decided, studio uses a one-time web-sign hand-off (one signature ever). + means delete+recreate → new `taskId` → lost `wfstate`). The only gateway-side need is: + - **Expose a `guardian_ruleset` (and, generally, feature-ruleset) configVar** so a workflow reads + volatile rules (flag sets, thresholds, trust overrides) at runtime and a single gateway-config + change propagates to **all** deployed instances on next run — no re-deploy, no state loss. (Note: + only `macros.secrets` currently reaches `apContext.configVars`; either place the ruleset there or + add `macros.vars` injection.) + + A rare **structural** (interpreter-shape) change is handled by **re-creating** the workflow — the new + task's first run is a **silent seed** (compute findings, write all `ntfy:` markers, send nothing) so + the switch doesn't double-alert. This needs **no `wfstate` read API** and **no `UpdateWorkflow`** — a + fleet re-create on rare structural change is acceptable. (An in-place `UpdateWorkflow` remains a + possible future ergonomics win but is explicitly **not** required for the guardian.) +- **B — Read-only deploy auth. DECIDED (studio, 2026-07-15): web hand-off; NO create-auth change.** + Confirmed: `CreateWorkflow` requires a user JWT (EIP-191 signature); partner assertion is rejected for + create; **no** fund-moving-vs-read-only distinction exists; and a **counterfactual (undeployed) runner + is accepted** (the ownership gate is a pure DB check). Studio enrolls via its existing + conversation-resume-on-web hand-off — a one-time EIP-191 sign establishes the 48h session and deploys + (one signature ever). **No gateway change required.** Relaxing create-auth for the read-only/no-fund + class via `X-Partner-Assertion` (zero user signature) is parked as a **future lever**, not a + dependency. - **C — Claim-once semantics.** Confirmed: executions are **serialized per task** (single FIFO worker, no overlap) and **at-most-once with NO retry** (`apqueue/worker.go` marks failed and drops; `Recover()` is a no-op). Therefore **mark-after-send (at-least-once) is the correct design for a From 6dcb08f6a4f1c432b8d9793207babcfc76fb6133 Mon Sep 17 00:00:00 2001 From: Chris Li Date: Thu, 16 Jul 2026 11:14:43 -0700 Subject: [PATCH 13/16] docs: guardian_ruleset must always be set (unresolved source template hard-fails) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Live run against v4.2.0 showed the verdict node fails on an unset guardian_ruleset configVar (could not resolve variable ... in source) before the JS runs — so it is required guardian setup, not optional-with-JS-fallback. --- PLAN_WORKFLOW_STATE_GUARDIAN_MONITORING.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/PLAN_WORKFLOW_STATE_GUARDIAN_MONITORING.md b/PLAN_WORKFLOW_STATE_GUARDIAN_MONITORING.md index f31397ef..aece7fdf 100644 --- a/PLAN_WORKFLOW_STATE_GUARDIAN_MONITORING.md +++ b/PLAN_WORKFLOW_STATE_GUARDIAN_MONITORING.md @@ -297,7 +297,10 @@ gateway facts behind them (all verified against current code): volatile rules (flag sets, thresholds, trust overrides) at runtime and a single gateway-config change propagates to **all** deployed instances on next run — no re-deploy, no state loss. (Note: only `macros.secrets` currently reaches `apContext.configVars`; either place the ruleset there or - add `macros.vars` injection.) + add `macros.vars` injection.) **It must ALWAYS be set** (even to `{}`): verified live that an + unresolved `{{...}}` inside a `customCode` `source` hard-fails the node (`could not resolve + variable apContext.configVars.guardian_ruleset in source`), so this is required guardian setup, + not an optional value a JS fallback can cover. A rare **structural** (interpreter-shape) change is handled by **re-creating** the workflow — the new task's first run is a **silent seed** (compute findings, write all `ntfy:` markers, send nothing) so From bf42329b0fd0f838eb7eaf8759d475bab16e9414 Mon Sep 17 00:00:00 2001 From: Chris Li Date: Thu, 16 Jul 2026 23:47:14 +0200 Subject: [PATCH 14/16] feat: workflow {{state.*}} store + REST options.auth GoPlus provider (#662) Co-authored-by: Chris Li --- config/gateway.example.yaml | 9 + config/test.example.yaml | 7 + core/taskengine/engine.go | 13 ++ core/taskengine/schema.go | 17 ++ core/taskengine/vm.go | 36 ++++ core/taskengine/vm_runner_customcode.go | 94 +++++++++++ core/taskengine/vm_runner_rest.go | 134 +++++++++++++++ core/taskengine/vm_workflow_state_test.go | 194 ++++++++++++++++++++++ 8 files changed, 504 insertions(+) create mode 100644 core/taskengine/vm_workflow_state_test.go diff --git a/config/gateway.example.yaml b/config/gateway.example.yaml index 7e2248f9..354ff689 100644 --- a/config/gateway.example.yaml +++ b/config/gateway.example.yaml @@ -114,6 +114,15 @@ macros: sendgrid_key: "" thegraph_api_key: "" moralis_api_key: "" + # GoPlus signed-token auth for restApi nodes using `options.auth.provider: goplus`. + # Leave blank to fall back to keyless GoPlus (lower rate limits). + goplus_app_key: "" + goplus_app_secret: "" + # guardian_ruleset — the tunable verdict knobs the guardian customCode reads via + # {{apContext.configVars.guardian_ruleset}} (bare-injected as a JS object). MUST be + # a valid JSON object and MUST be set (an unresolved template in a customCode source + # hard-fails the node). Single-quote it so the inner double quotes survive YAML. + guardian_ruleset: '{"weak":["honeypot_related_address","blacklist_doubt"]}' # Optional: post-execution AI summarization. Disabled by default in dev. notifications: diff --git a/config/test.example.yaml b/config/test.example.yaml index eac3f623..8ad39ae8 100644 --- a/config/test.example.yaml +++ b/config/test.example.yaml @@ -91,6 +91,13 @@ macros: sendgrid_key: "" thegraph_api_key: "" moralis_api_key: "" + # GoPlus signed-token auth for restApi nodes using `options.auth.provider: goplus`. + # Leave blank to fall back to keyless GoPlus (lower rate limits). + goplus_app_key: "" + goplus_app_secret: "" + # guardian_ruleset — tunable verdict knobs read via {{apContext.configVars.guardian_ruleset}} + # (bare-injected as a JS object; must be valid JSON and must be set). + guardian_ruleset: '{"weak":["honeypot_related_address","blacklist_doubt"]}' # Optional: post-execution AI summarization. Disabled by default in tests. notifications: diff --git a/core/taskengine/engine.go b/core/taskengine/engine.go index 60a69b35..501a3253 100644 --- a/core/taskengine/engine.go +++ b/core/taskengine/engine.go @@ -4603,6 +4603,19 @@ func (n *Engine) DeleteWorkflowByUser(user *model.User, taskID string) (*avsprot // checkpoint/wake — and any operator internal trigger — don't outlive the workflow. n.gcDurableStateForTask(taskID) + // Cascade-delete this workflow's durable per-run state ({{state.*}} / + // wfstate:) so it doesn't outlive the task (scoped by taskID, so this + // touches only this workflow's namespace). + if stateKeys, listErr := n.db.ListKeys(string(WorkflowStatePrefix(taskID))); listErr == nil { + for _, k := range stateKeys { + if delErr := n.db.Delete([]byte(k)); delErr != nil { + n.logger.Warn("failed to delete workflow state key", "key", k, "error", delErr) + } + } + } else { + n.logger.Warn("failed to list workflow state keys for cascade delete; state may be orphaned", "task_id", taskID, "error", listErr) + } + n.logger.Info("📢 Starting operator notifications", "task_id", taskID) n.notifyOperatorsTaskOperation(taskID, avsproto.MessageOp_DeleteTask) n.logger.Info("✅ Delete task operation completed", "task_id", taskID) diff --git a/core/taskengine/schema.go b/core/taskengine/schema.go index 571ca384..f92c27a0 100644 --- a/core/taskengine/schema.go +++ b/core/taskengine/schema.go @@ -48,6 +48,23 @@ func WalletStorageKey(chainID int64, owner common.Address, smartWalletAddress st ) } +// WorkflowStateKey returns the key for one entry of a workflow's durable, +// cross-run mutable state — the `{{state.*}}` store exposed to customCode. +// +// Scoped ONLY by taskID (no owner, no chain): every workflow has an isolated +// namespace, so a user with several monitoring workflows never has their state +// collide, and deleting a workflow can wipe exactly its state (see +// WorkflowStatePrefix). `stateKey` is a client-defined opaque string. +func WorkflowStateKey(taskID, stateKey string) []byte { + return []byte(fmt.Sprintf("wfstate:%s:%s", taskID, stateKey)) +} + +// WorkflowStatePrefix returns the scan/cascade-delete prefix for all `{{state.*}}` +// entries of one workflow. +func WorkflowStatePrefix(taskID string) []byte { + return []byte(fmt.Sprintf("wfstate:%s:", taskID)) +} + // WalletBySaltKey returns the secondary index key that maps a // (chainID, owner, factory, salt) tuple to its current canonical wallet // address. diff --git a/core/taskengine/vm.go b/core/taskengine/vm.go index fbea81fb..b9bcac29 100644 --- a/core/taskengine/vm.go +++ b/core/taskengine/vm.go @@ -281,6 +281,42 @@ type VM struct { // the executor reads PendingSuspend() to checkpoint + register the wake instead // of finishing. nil for a normal run. suspend *SuspendRequest + + // stateScratch backs the {{state.*}} binding when it must NOT persist — in + // simulation, single-node runs (no task id), or when no DB is wired. It lives + // on the VM (not per-JSProcessor) so all customCode steps of one run share it, + // keeping read-after-write coherent across nodes just like the DB-backed path. + // Guarded by mu. Never flushed to storage. + stateScratch map[string][]byte +} + +// scratchGet/scratchSet/scratchList back the {{state.*}} binding's non-persistent +// mode (see stateScratch). All are mutex-guarded so parallel customCode steps are safe. +func (v *VM) scratchGet(key string) []byte { + v.mu.Lock() + defer v.mu.Unlock() + return v.stateScratch[key] +} + +func (v *VM) scratchSet(key string, value []byte) { + v.mu.Lock() + defer v.mu.Unlock() + if v.stateScratch == nil { + v.stateScratch = make(map[string][]byte) + } + v.stateScratch[key] = value +} + +func (v *VM) scratchList(prefix string) []string { + v.mu.Lock() + defer v.mu.Unlock() + out := make([]string, 0, len(v.stateScratch)) + for k := range v.stateScratch { + if strings.HasPrefix(k, prefix) { + out = append(out, k) + } + } + return out } func NewVM() *VM { diff --git a/core/taskengine/vm_runner_customcode.go b/core/taskengine/vm_runner_customcode.go index abb2febc..6152ea40 100644 --- a/core/taskengine/vm_runner_customcode.go +++ b/core/taskengine/vm_runner_customcode.go @@ -1,10 +1,12 @@ package taskengine import ( + "encoding/json" "fmt" "math/big" "reflect" "regexp" + "sort" "strings" "time" @@ -16,6 +18,90 @@ import ( avsproto "github.com/AvaProtocol/EigenLayer-AVS/protobuf" ) +// installStateBinding exposes `state.get/set/list` to customCode JS, backed by the +// per-workflow `wfstate:` namespace (WorkflowStateKey). This is the durable, +// cross-run, client-defined state store — the `{{state.*}}` capability. +// +// - state.get(key) -> the stored JSON value, or undefined +// - state.set(key, value) -> persists arbitrary JSON under this workflow's namespace +// - state.list(prefix) -> the stateKeys (prefix included) currently stored +// +// Writes never hit the DB in simulation, for a single-node run (no task id), or when +// no DB is wired: those use an in-memory scratch map so read-after-write still works +// within one run without mutating real state. Bind this AFTER the step vars so a node +// accidentally named "state" can't shadow it. +func installStateBinding(jsvm *goja.Runtime, vm *VM) { + if vm == nil { + return + } + obj := jsvm.NewObject() + + taskID := vm.GetTaskId() + // scratch-only when we must not (or cannot) persist. The scratch lives on the VM + // (vm.scratch*), so all customCode steps of one run share it. + scratchOnly := vm.db == nil || taskID == "" || vm.IsSimulation + + obj.Set("get", func(key string) interface{} { + if key == "" { + return goja.Undefined() + } + var raw []byte + if scratchOnly { + raw = vm.scratchGet(key) + } else if b, err := vm.db.GetKey(WorkflowStateKey(taskID, key)); err == nil { + raw = b + } + if raw == nil { + return goja.Undefined() + } + var value interface{} + if err := json.Unmarshal(raw, &value); err != nil { + return goja.Undefined() + } + return value + }) + + obj.Set("set", func(key string, value interface{}) { + if key == "" { + return + } + encoded, err := json.Marshal(value) + if err != nil { + return + } + if scratchOnly { + vm.scratchSet(key, encoded) + return + } + if setErr := vm.db.Set(WorkflowStateKey(taskID, key), encoded); setErr != nil && vm.logger != nil { + vm.logger.Warn("state.set failed", "key", key, "error", setErr) + } + }) + + obj.Set("list", func(prefix string) []string { + var out []string + if scratchOnly { + out = vm.scratchList(prefix) + } else { + full := string(WorkflowStatePrefix(taskID)) + keys, err := vm.db.ListKeys(full + prefix) + if err != nil { + return []string{} + } + out = make([]string, 0, len(keys)) + for _, k := range keys { + out = append(out, strings.TrimPrefix(k, full)) + } + } + // Deterministic order regardless of backing store (Badger iterates sorted; + // the scratch map does not) so simulation and real runs agree. + sort.Strings(out) + return out + }) + + jsvm.Set("state", obj) +} + type JSProcessor struct { *CommonProcessor jsvm *goja.Runtime @@ -73,6 +159,10 @@ func NewJSProcessor(vm *VM) *JSProcessor { } } + // Expose {{state.*}} — durable per-workflow state. Bound last so a step var + // accidentally named "state" cannot shadow it. + installStateBinding(r.jsvm, vm) + return &r } @@ -130,6 +220,10 @@ func NewJSProcessorWithIsolatedVars(vm *VM, isolatedVars map[string]any) *JSProc } } + // Expose {{state.*}} — durable per-workflow state. Bound last so an isolated var + // accidentally named "state" cannot shadow it. + installStateBinding(r.jsvm, vm) + return &r } diff --git a/core/taskengine/vm_runner_rest.go b/core/taskengine/vm_runner_rest.go index e8afdec1..5a0ee09d 100644 --- a/core/taskengine/vm_runner_rest.go +++ b/core/taskengine/vm_runner_rest.go @@ -1,11 +1,16 @@ package taskengine import ( + "bytes" + "crypto/sha1" + "encoding/hex" "encoding/json" "fmt" "net/http" "net/http/httptest" + "strconv" "strings" + "sync" "time" avsproto "github.com/AvaProtocol/EigenLayer-AVS/protobuf" @@ -571,6 +576,21 @@ func (r *RestProcessor) Execute(stepID string, node *avsproto.RestAPINode) (*avs processedHeaders[processedKey] = processedValue } + // Server-side auth providers (options.auth): mint + attach a provider token so the + // credential never lives in the workflow JSON. GoPlus first; on "" (keys unset or + // mint failed) we send no Authorization header — GoPlus still answers keyless. + if restAuthProvider(node) == "goplus" { + if token := goplusTokenProvider(); token != "" { + processedHeaders["Authorization"] = token + if r.vm.logger != nil { + r.vm.logger.Debug("REST: attached GoPlus session token (authed)", "stepID", stepID) + } + } else if r.vm.logger != nil { + // Keys unset or mint failed: GoPlus still answers keyless (lower limits). + r.vm.logger.Warn("REST: GoPlus auth requested but no token minted; falling back to keyless", "stepID", stepID) + } + } + // Only apply JSON preprocessing if content type is JSON contentType := "" for key, value := range processedHeaders { @@ -862,6 +882,120 @@ func detectNotificationProvider(u string) string { return "" } +// ── REST auth providers ───────────────────────────────────────────────────── +// A restApi node can request server-side credential injection with +// +// config.options.auth = { "provider": "goplus" } +// +// The gateway mints the provider's token from macros.secrets and attaches it as +// the Authorization header at execution time, so the secret never appears in the +// (client-visible) workflow JSON. GoPlus is the first provider; add more here. + +// restAuthProvider returns the lower-cased options.auth.provider (e.g. "goplus"), +// or "" when none is set. Mirrors shouldSummarize's options-bag read. +func restAuthProvider(node *avsproto.RestAPINode) string { + if node == nil || node.Config == nil || node.Config.Options == nil { + return "" + } + optsMap, ok := node.Config.Options.AsInterface().(map[string]interface{}) + if !ok { + return "" + } + auth, ok := optsMap["auth"].(map[string]interface{}) + if !ok { + return "" + } + provider, _ := auth["provider"].(string) + return strings.ToLower(strings.TrimSpace(provider)) +} + +// goplusTokenProvider is the seam the REST runner calls to obtain a GoPlus +// Authorization header value; overridable in tests. Returns "" for keyless. +var goplusTokenProvider = goplusAuthHeader + +// goplusHTTPClient bounds the token-mint call so a slow/hung GoPlus endpoint +// cannot block REST node execution indefinitely. +var goplusHTTPClient = &http.Client{Timeout: 15 * time.Second} + +// goplusTokenCache holds the module-cached GoPlus session token, refreshed shortly +// before expiry. Keyed by the app_key it was minted for, so a rotated key never +// reuses a stale token. The token value already includes the "Bearer " prefix. +var goplusTokenCache struct { + sync.RWMutex + token string + appKey string + expires time.Time +} + +// goplusAuthHeader returns the Authorization header value for GoPlus, minting + +// caching a signed session token from macros.secrets (goplus_app_key / +// goplus_app_secret): sign = sha1_hex(app_key + time + app_secret) → POST /v1/token. +// Returns "" to fall back to keyless GoPlus access (lower rate limits) when the +// keys are unset or minting fails — the caller then sends no Authorization header. +func goplusAuthHeader() string { + appKey := GetMacroSecret("goplus_app_key") + appSecret := GetMacroSecret("goplus_app_secret") + if appKey == "" || appSecret == "" { + return "" + } + + goplusTokenCache.RLock() + if goplusTokenCache.token != "" && goplusTokenCache.appKey == appKey && time.Until(goplusTokenCache.expires) > 60*time.Second { + token := goplusTokenCache.token + goplusTokenCache.RUnlock() + return token + } + goplusTokenCache.RUnlock() + + goplusTokenCache.Lock() + defer goplusTokenCache.Unlock() + // Re-check under the write lock (another goroutine may have refreshed for this key). + if goplusTokenCache.token != "" && goplusTokenCache.appKey == appKey && time.Until(goplusTokenCache.expires) > 60*time.Second { + return goplusTokenCache.token + } + + now := time.Now().Unix() + sum := sha1.Sum([]byte(appKey + strconv.FormatInt(now, 10) + appSecret)) + reqBody, _ := json.Marshal(map[string]interface{}{ + "app_key": appKey, + "sign": hex.EncodeToString(sum[:]), + "time": now, + }) + req, err := http.NewRequest(http.MethodPost, "https://api.gopluslabs.io/api/v1/token", bytes.NewReader(reqBody)) + if err != nil { + return "" + } + req.Header.Set("Content-Type", "application/json") + resp, err := goplusHTTPClient.Do(req) + if err != nil { + return "" + } + defer resp.Body.Close() + if resp.StatusCode < 200 || resp.StatusCode >= 300 { + return "" + } + var parsed struct { + Code int `json:"code"` + Result struct { + AccessToken string `json:"access_token"` + ExpiresIn int64 `json:"expires_in"` + } `json:"result"` + } + if decodeErr := json.NewDecoder(resp.Body).Decode(&parsed); decodeErr != nil || parsed.Code != 1 || parsed.Result.AccessToken == "" { + return "" + } + // GoPlus returns the token already prefixed with "Bearer "; normalize defensively + // so a change on their side can't produce an invalid Authorization header. + token := parsed.Result.AccessToken + if !strings.HasPrefix(token, "Bearer ") { + token = "Bearer " + token + } + goplusTokenCache.token = token + goplusTokenCache.appKey = appKey + goplusTokenCache.expires = time.Now().Add(time.Duration(parsed.Result.ExpiresIn) * time.Second) + return token +} + // escapeJSONString properly escapes a string for use within JSON func escapeJSONString(s string) string { // Use Go's built-in JSON marshaling to properly escape the string diff --git a/core/taskengine/vm_workflow_state_test.go b/core/taskengine/vm_workflow_state_test.go new file mode 100644 index 00000000..0b9266e2 --- /dev/null +++ b/core/taskengine/vm_workflow_state_test.go @@ -0,0 +1,194 @@ +package taskengine + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + + "github.com/AvaProtocol/EigenLayer-AVS/core/testutil" + "github.com/AvaProtocol/EigenLayer-AVS/model" + avsproto "github.com/AvaProtocol/EigenLayer-AVS/protobuf" + "github.com/AvaProtocol/EigenLayer-AVS/storage" + "github.com/stretchr/testify/require" + "google.golang.org/protobuf/types/known/structpb" +) + +// runCustomCodeForState executes `source` as a customCode node inside a task with +// the given taskID, wired to db. Returns after asserting the step succeeded. +func runCustomCodeForState(t *testing.T, db storage.Storage, taskID, source string, simulation bool) { + t.Helper() + taskNode := &avsproto.TaskNode{ + Id: "code1", + Name: "code1", + TaskType: &avsproto.TaskNode_CustomCode{ + CustomCode: &avsproto.CustomCodeNode{ + Config: &avsproto.CustomCodeNode_Config{ + Lang: avsproto.Lang_LANG_JAVASCRIPT, + Source: source, + }, + }, + }, + } + trigger := &avsproto.TaskTrigger{Id: "trigger", Name: "trigger"} + vm, err := NewVMWithData(&model.Workflow{ + Task: &avsproto.Task{ + Id: taskID, + Nodes: []*avsproto.TaskNode{taskNode}, + Edges: []*avsproto.TaskEdge{{Id: "e1", Source: trigger.Id, Target: "code1"}}, + Trigger: trigger, + }, + }, nil, testutil.GetTestSmartWalletConfig(), nil) + require.NoError(t, err) + vm.WithDb(db) + if simulation { + vm.SetSimulation(true) + } + + proc := NewJSProcessor(vm) + step, err := proc.Execute("code1", taskNode.GetCustomCode()) + require.NoError(t, err) + require.True(t, step.Success, "customCode step failed: %s", step.Error) +} + +func readStateMap(t *testing.T, db storage.Storage, taskID, stateKey string) map[string]interface{} { + t.Helper() + raw, err := db.GetKey(WorkflowStateKey(taskID, stateKey)) + require.NoError(t, err, "expected wfstate key %s to exist", stateKey) + var m map[string]interface{} + require.NoError(t, json.Unmarshal(raw, &m)) + return m +} + +// TestWorkflowStateBinding_SetGetListPersist covers the {{state.*}} binding: +// set/get/list within a run, real persistence under wfstate:, and — with a +// fresh VM/processor on the same db+taskId — cross-run persistence. +func TestWorkflowStateBinding_SetGetListPersist(t *testing.T) { + db := testutil.TestMustDB() + defer db.Close() + const taskID = "state-test-task" + + // Run 1: set two keys, then probe get + list by writing the results back so we + // can assert them via the DB (avoids parsing structpb output). + runCustomCodeForState(t, db, taskID, ` + state.set("ntfy:a", { x: 1 }); + state.set("seen:b", { y: 2 }); + var got = state.get("ntfy:a"); + var keys = state.list("ntfy:"); + state.set("_probe", { gotX: got && got.x, keyCount: keys.length, firstKey: keys[0] }); + return true; + `, false) + + // Values persisted under wfstate::. + require.Equal(t, float64(1), readStateMap(t, db, taskID, "ntfy:a")["x"]) + require.Equal(t, float64(2), readStateMap(t, db, taskID, "seen:b")["y"]) + + // The probe confirms get + list worked inside the JS: list("ntfy:") matched + // exactly the one ntfy: key (not seen:b). + probe := readStateMap(t, db, taskID, "_probe") + require.Equal(t, float64(1), probe["gotX"]) + require.Equal(t, float64(1), probe["keyCount"]) + require.Equal(t, "ntfy:a", probe["firstKey"]) + + // Run 2: a FRESH VM/processor on the same db + taskID still sees run-1 state. + runCustomCodeForState(t, db, taskID, ` + var a = state.get("ntfy:a"); + state.set("_probe2", { persisted: a && a.x }); + return true; + `, false) + require.Equal(t, float64(1), readStateMap(t, db, taskID, "_probe2")["persisted"]) +} + +// TestWorkflowStateBinding_SimulationDoesNotPersist: in simulation, writes go to an +// in-memory scratch, never the real DB — so a nodes:run / simulate preview can't +// mutate a live workflow's state. +func TestWorkflowStateBinding_SimulationDoesNotPersist(t *testing.T) { + db := testutil.TestMustDB() + defer db.Close() + const taskID = "state-sim-task" + + // Read-after-write still works within the run (scratch), but nothing hits the DB. + runCustomCodeForState(t, db, taskID, ` + state.set("ntfy:x", { v: 1 }); + if (!state.get("ntfy:x")) { throw new Error("scratch read-after-write should work"); } + return true; + `, true) + + _, err := db.GetKey(WorkflowStateKey(taskID, "ntfy:x")) + require.Error(t, err, "simulation writes must not reach the DB") +} + +// TestRestAuthProvider covers the options.auth.provider parsing that gates +// server-side GoPlus token injection. +func TestRestAuthProvider(t *testing.T) { + mk := func(opts map[string]interface{}) *avsproto.RestAPINode { + var o *structpb.Value + if opts != nil { + v, err := structpb.NewValue(opts) + require.NoError(t, err) + o = v + } + return &avsproto.RestAPINode{Config: &avsproto.RestAPINode_Config{Options: o}} + } + + require.Equal(t, "goplus", restAuthProvider(mk(map[string]interface{}{ + "auth": map[string]interface{}{"provider": "goplus"}, + }))) + // Trimmed + lower-cased. + require.Equal(t, "goplus", restAuthProvider(mk(map[string]interface{}{ + "auth": map[string]interface{}{"provider": " GoPlus "}, + }))) + // No auth key. + require.Equal(t, "", restAuthProvider(mk(map[string]interface{}{"summarize": true}))) + // No options / no config. + require.Equal(t, "", restAuthProvider(mk(nil))) + require.Equal(t, "", restAuthProvider(&avsproto.RestAPINode{})) + require.Equal(t, "", restAuthProvider(nil)) +} + +// TestRestGoPlusAuthInjection verifies the Authorization header is attached when +// options.auth.provider="goplus" and a token is available, and left unset for the +// keyless fallback. The token source (goplusTokenProvider) is stubbed so the test +// never hits the real GoPlus endpoint. +func TestRestGoPlusAuthInjection(t *testing.T) { + var gotAuth string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotAuth = r.Header.Get("Authorization") + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(`{"code":1,"message":"ok","result":[]}`)) + })) + defer srv.Close() + + orig := goplusTokenProvider + defer func() { goplusTokenProvider = orig }() + + authNode := func() *avsproto.RestAPINode { + opts, err := structpb.NewValue(map[string]interface{}{"auth": map[string]interface{}{"provider": "goplus"}}) + require.NoError(t, err) + return &avsproto.RestAPINode{Config: &avsproto.RestAPINode_Config{Url: srv.URL, Method: "GET", Options: opts}} + } + run := func(node *avsproto.RestAPINode) { + gotAuth = "" + taskNode := &avsproto.TaskNode{Id: "r", Name: "r", TaskType: &avsproto.TaskNode_RestApi{RestApi: node}} + vm, err := NewVMWithData(&model.Workflow{Task: &avsproto.Task{ + Id: "auth-test", + Nodes: []*avsproto.TaskNode{taskNode}, + Edges: []*avsproto.TaskEdge{{Id: "e1", Source: "trigger", Target: "r"}}, + Trigger: &avsproto.TaskTrigger{Id: "trigger", Name: "trigger"}, + }}, nil, testutil.GetTestSmartWalletConfig(), nil) + require.NoError(t, err) + proc := NewRestProcessor(vm) + _, err = proc.Execute("r", node) + require.NoError(t, err) + } + + // A token from the provider is attached as the Authorization header. + goplusTokenProvider = func() string { return "Bearer test-token" } + run(authNode()) + require.Equal(t, "Bearer test-token", gotAuth) + + // Keyless fallback (provider returns "") sends no Authorization header. + goplusTokenProvider = func() string { return "" } + run(authNode()) + require.Equal(t, "", gotAuth) +} From 5f567f786306455eec2419f66cab48ccb80b7963 Mon Sep 17 00:00:00 2001 From: Chris Li Date: Fri, 17 Jul 2026 12:04:43 -0700 Subject: [PATCH 15/16] docs: completion checklist for #662 + docs/changes record for state+auth extensions --- PLAN_WORKFLOW_STATE_GUARDIAN_MONITORING.md | 49 +++++++++++- .../20260717-workflow-state-and-rest-auth.md | 75 +++++++++++++++++++ 2 files changed, 122 insertions(+), 2 deletions(-) create mode 100644 docs/changes/20260717-workflow-state-and-rest-auth.md diff --git a/PLAN_WORKFLOW_STATE_GUARDIAN_MONITORING.md b/PLAN_WORKFLOW_STATE_GUARDIAN_MONITORING.md index aece7fdf..09dcb1dc 100644 --- a/PLAN_WORKFLOW_STATE_GUARDIAN_MONITORING.md +++ b/PLAN_WORKFLOW_STATE_GUARDIAN_MONITORING.md @@ -334,5 +334,50 @@ scans (absence ≠ cleared, studio parity). A time-based `wfstate` TTL is a futu --- -*Planning only. No code changed by this document. Gateway-side response to the studio -`PLAN_GUARDIAN_MONITORING_ARCHITECTURE.md` hand-off.* +## 10. Completion status + +The two gateway extensions shipped in **PR #662** (merged to `staging`, 2026-07-16). Shipped record: +`docs/changes/20260717-workflow-state-and-rest-auth.md`. + +**✅ Done — gateway (PR #662):** +- [x] **Extension 1 — REST `options.auth` providers.** `restAuthProvider` reads `options.auth.provider`; + `goplusAuthHeader` mints + caches a GoPlus signed token (sha1 sign → `/v1/token`), cache keyed by + `app_key`, 15 s HTTP timeout, 2xx check, `Bearer ` normalization; Authorization injected server-side + in `vm_runner_rest.go`. Keyless fallback when keys unset. (`vm_runner_rest.go`) +- [x] **Extension 2 — `{{state.*}}` per-workflow state.** `wfstate::` namespace + + `state.get/set/list` goja binding (VM-scoped scratch, `IsSimulation`-guarded, deterministic-sorted + `list`); cascade-delete on task teardown. Additive storage (`make storage-check` clean). + (`schema.go`, `vm_runner_customcode.go`, `vm.go`, `engine.go`) +- [x] **`guardian_ruleset` config wired** — `gateway.example.yaml` / `test.example.yaml`, the local + gateway config, and avs-infra `gateway-railway.yaml` (`${GOPLUS_APP_KEY}`/`${GOPLUS_APP_SECRET}` env + refs + inlined ruleset). +- [x] **Tests** — state roundtrip / cross-run persistence / simulation no-op; `restAuthProvider` + parsing; Authorization-injection (mockable seam). Live-verified: authed GoPlus + `state` against a + local gateway. + +**✅ Done — SDK (ava-sdk-js `staging`):** +- [x] Guardian wallet-risk monitor test + builder/verdict fixture (offline verdict parity + builder + shape + live deploy/trigger) — `tests/v4/templates/guardian-wallet-risk-monitor.test.ts`. + +**⏭️ Deferred / optional (not blocking v1):** +- [ ] `SetIfAbsent` storage primitive for **strict** exactly-once claim-once (mark-after-send is the v1 + default, §3.3). +- [ ] `{{state.*}}` template auto-load for non-CustomCode nodes (§2.3). +- [ ] GoPlus `code:4033` retry-keyless nuance (keyless-on-unset-keys **is** implemented). +- [ ] Held-token (critical-token) scan — approvals-only for v1; held-token is a fast-follow. +- [ ] Promote the `Guardian` builder from SDK test fixture to published `packages/sdk-js` surface. + +**🟨 Client-side (studio) — separate product work, tracked in the studio guardian docs:** +- [ ] Advisor tool `enable_guardian_monitoring` deploys the per-user workflow. +- [ ] Enrollment via the one-time web-sign hand-off (§9-B). +- [ ] Sync `guardian_ruleset` from `app/lib/goplus.ts`. + +**🔧 Ops follow-up:** +- [x] Railway `GOPLUS_APP_KEY` / `GOPLUS_APP_SECRET` set on the gateway service. +- [ ] Republish `avs-dev:latest` dev docker so the SDK E2E guardian **live** test asserts (it soft-skips + against the pre-merge image). + +--- + +*Gateway-side response to the studio `PLAN_GUARDIAN_MONITORING_ARCHITECTURE.md` hand-off. The two +extensions are now implemented (PR #662); this document is retained as the design record.* diff --git a/docs/changes/20260717-workflow-state-and-rest-auth.md b/docs/changes/20260717-workflow-state-and-rest-auth.md new file mode 100644 index 00000000..ad099fa1 --- /dev/null +++ b/docs/changes/20260717-workflow-state-and-rest-auth.md @@ -0,0 +1,75 @@ +# Per-Workflow State (`{{state.*}}`) + REST `options.auth` Providers + +## Summary + +Two generic, reusable gateway extensions (PR #662, merged to `staging` 2026-07-16). Design record: +`PLAN_WORKFLOW_STATE_GUARDIAN_MONITORING.md`. The first consumer is the studio "guardian" wallet-risk +monitor, but neither extension is guardian-specific. + +1. **`{{state.*}}` — durable, cross-run, per-workflow state.** A `customCode` node can call + `state.get/set/list` to persist arbitrary client-defined JSON that survives across scheduled runs, + scoped to the workflow (`taskId`) — not the user. This is what lets a monitor alert on *new* + findings only (diff current-vs-last-seen) without any studio callback in the hot path. +2. **REST `options.auth` providers.** A `restApi` node can set + `config.options.auth = { provider: "goplus" }`; the gateway mints/attaches the provider credential + server-side so the secret never lives in the (client-visible) workflow JSON. GoPlus is provider #1. + +## Extension 1 — `{{state.*}}` + +- **Storage**: new `wfstate::` BadgerDB namespace (`core/taskengine/schema.go` + `WorkflowStateKey` / `WorkflowStatePrefix`). Scoped by `taskId` only, so a user's multiple monitoring + workflows never collide and a workflow's state can be wiped exactly on deletion. Additive — no + migration (`make storage-check` clean). +- **Binding**: `state.get/set/list` bound into the goja runtime in both `NewJSProcessor` and + `NewJSProcessorWithIsolatedVars` (`core/taskengine/vm_runner_customcode.go` `installStateBinding`), + over `vm.db`. Bound after the step vars so a node named `state` can't shadow it. +- **Non-persistent mode**: in simulation, single-node runs (no `taskId`), or when no DB is wired, + reads/writes use a VM-scoped scratch map (`vm.stateScratch`, mutex-guarded, `core/taskengine/vm.go`) + so `nodes:run` / `workflows:simulate` previews never mutate live state, yet read-after-write and + cross-node coherence still hold within one run. +- **Deterministic `list`**: results are sorted in both the DB and scratch paths so simulation and real + execution agree on ordering. +- **Lifecycle**: cascade-deleted on task teardown (`DeleteWorkflowByUser`, `core/taskengine/engine.go`); + a `ListKeys` failure is logged rather than silently orphaning state. +- **Namespace convention**: `{{state.*}}` is the mutable, client-defined, read/write namespace parallel + to `{{settings.*}}` (immutable config) and `{{context.*}}` (read-only runtime). + +## Extension 2 — REST `options.auth` (GoPlus) + +- **`restAuthProvider(node)`** reads `options.auth.provider` from the node's `options` bag (mirrors + `shouldSummarize`). No proto/SDK type change — `options` is already an open `structpb.Value`. +- **`goplusAuthHeader()`** mints a GoPlus session token from `macros.secrets` + (`goplus_app_key`/`goplus_app_secret`): `sign = sha1_hex(app_key + time + app_secret)` → POST + `/api/v1/token`. Token is module-cached (RWMutex), **keyed by `app_key`** so a rotated key never + reuses a stale token, refreshed ~60 s early. Uses an `http.Client` with a 15 s timeout, checks for a + 2xx status, and normalizes the token to a `Bearer ` prefix defensively. +- **Injection**: if the provider mints a token, it's set as the `Authorization` header in + `processedHeaders` before the request; on `""` (keys unset or mint failed) no header is sent — GoPlus + answers keyless (lower rate limits). A `goplusTokenProvider` seam makes the injection unit-testable. +- **Config**: `goplus_app_key`/`goplus_app_secret` added to `macros.secrets` in the example configs; + real values via Railway env on the deployed `gateway-railway.yaml` (`${GOPLUS_APP_KEY}` / + `${GOPLUS_APP_SECRET}`). `guardian_ruleset` (the tunable verdict knobs) is inlined as JSON (not a + secret). + +## Files + +`core/taskengine/schema.go`, `vm.go`, `vm_runner_customcode.go`, `vm_runner_rest.go`, `engine.go`, +`vm_workflow_state_test.go`; `config/gateway.example.yaml`, `config/test.example.yaml`. + +## Testing + +- Go: state set/get/list roundtrip, cross-run persistence, simulation no-op guard; `restAuthProvider` + parsing; Authorization-injection via `httptest` + the `goplusTokenProvider` seam. +- Live: verified against a local gateway with real GoPlus keys — the guardian scan mints an authed + token (`attached GoPlus session token (authed)`) and the `state`-backed verdict runs; the ava-sdk-js + guardian template test passes 14/14 with the verdict step asserting. + +## Deferred / follow-ups + +- `SetIfAbsent` storage primitive for **strict** exactly-once claim-once (mark-after-send is the v1 + default); `{{state.*}}` template auto-load for non-CustomCode nodes; GoPlus `code:4033` retry-keyless + nuance; held-token (critical-token) scan (approvals-only for v1). +- Ops: republish `avs-dev:latest` so the SDK E2E guardian **live** test asserts against a gateway with + these extensions. +- Studio (separate): the `enable_guardian_monitoring` advisor tool, web-sign enrollment, and syncing + `guardian_ruleset` from `app/lib/goplus.ts`. From b2e1040ead4575a4f57d3c8b591535ed2cb8c7db Mon Sep 17 00:00:00 2001 From: Chris Li Date: Fri, 17 Jul 2026 12:30:05 -0700 Subject: [PATCH 16/16] =?UTF-8?q?fix:=20address=20review=20=E2=80=94=20GoP?= =?UTF-8?q?lus=20mint=20lock=20+=20state=20get/set=20error=20logging?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - goplusAuthHeader: mint the token WITHOUT holding the cache write lock so an in-flight HTTP call (up to the client timeout) no longer serializes unrelated REST executions; publish under a brief lock afterward (last-writer-wins, duplicate concurrent mints are cheap/harmless). - state.get / state.set: log a Warn on JSON decode/encode failure instead of silently returning undefined / skipping the write, so corrupt or non-encodable state is diagnosable. --- core/taskengine/vm_runner_customcode.go | 6 ++++++ core/taskengine/vm_runner_rest.go | 14 +++++++------- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/core/taskengine/vm_runner_customcode.go b/core/taskengine/vm_runner_customcode.go index 6152ea40..a110b41c 100644 --- a/core/taskengine/vm_runner_customcode.go +++ b/core/taskengine/vm_runner_customcode.go @@ -56,6 +56,9 @@ func installStateBinding(jsvm *goja.Runtime, vm *VM) { } var value interface{} if err := json.Unmarshal(raw, &value); err != nil { + if vm.logger != nil { + vm.logger.Warn("state.get: corrupt stored value, returning undefined", "key", key, "error", err) + } return goja.Undefined() } return value @@ -67,6 +70,9 @@ func installStateBinding(jsvm *goja.Runtime, vm *VM) { } encoded, err := json.Marshal(value) if err != nil { + if vm.logger != nil { + vm.logger.Warn("state.set: value not JSON-encodable, not persisted", "key", key, "error", err) + } return } if scratchOnly { diff --git a/core/taskengine/vm_runner_rest.go b/core/taskengine/vm_runner_rest.go index 5a0ee09d..ecc6f83b 100644 --- a/core/taskengine/vm_runner_rest.go +++ b/core/taskengine/vm_runner_rest.go @@ -947,13 +947,10 @@ func goplusAuthHeader() string { } goplusTokenCache.RUnlock() - goplusTokenCache.Lock() - defer goplusTokenCache.Unlock() - // Re-check under the write lock (another goroutine may have refreshed for this key). - if goplusTokenCache.token != "" && goplusTokenCache.appKey == appKey && time.Until(goplusTokenCache.expires) > 60*time.Second { - return goplusTokenCache.token - } - + // Mint the token WITHOUT holding the cache lock, so an in-flight HTTP call (up to + // the client timeout) never serializes unrelated REST executions that only need to + // RLock the cache. A few goroutines may race and mint duplicates on a cold/expiring + // cache — cheap and harmless (last writer wins). now := time.Now().Unix() sum := sha1.Sum([]byte(appKey + strconv.FormatInt(now, 10) + appSecret)) reqBody, _ := json.Marshal(map[string]interface{}{ @@ -990,9 +987,12 @@ func goplusAuthHeader() string { if !strings.HasPrefix(token, "Bearer ") { token = "Bearer " + token } + // Publish under the write lock — brief, no network held. + goplusTokenCache.Lock() goplusTokenCache.token = token goplusTokenCache.appKey = appKey goplusTokenCache.expires = time.Now().Add(time.Duration(parsed.Result.ExpiresIn) * time.Second) + goplusTokenCache.Unlock() return token }