KeeperHub / KeeperHub/keeperhub

simulate behaves uniformly across /api/execute/* (tracking)

Open
#2,004 4 comments 0 reactions 0 assignees View on GitHub
accepted
Dominant language
TypeScript
Stars
24
Forks
93
Avg merge
1d 4h
Merged PRs (30d)
253

Description

Tracking issue for one invariant rather than its instances:

> **`simulate` behaves uniformly across `/api/execute/*` — a route either honours a dry run or refuses one, and never accepts the flag and broadcasts.**

Three reports reached this from different directions, and fixing them separately is how a fourth route gets missed.

## Current state, verified on `staging`

| Route | Body `simulate` | Query `?simulate=` |
|---|---|---|
| `/api/execute/transfer` | honoured | **silently ignored** |
| `/api/execute/contract-call` | honoured | **silently ignored** |
| `/api/execute/check-and-execute` | honoured | **silently ignored** |
| `/api/execute/{protocol}/{action}` | **accepted and ignored - broadcasts** | silently ignored |

The protocol catch-all (`app/api/execute/[...slug]/route.ts`, 452 lines) contains no `simulate` reference at all and never calls `parseSimulateFlag`, so the flag falls through as an unknown body field.

## Why one issue

`parseSimulateFlag` already states the principle this cluster is about:

> Coercing strings or numbers to a boolean is intentionally not supported: an execute endpoint must have exactly one shape per input, and silently falling through to "spend real funds" because a caller mistyped `"true"` instead of `true` is unsafe.

A flag in the wrong *place* is the same hazard as a flag of the wrong *type*, and a route with no support at all is the same hazard again. All three deserve the same answer.

## Members

- #1929 — protocol action route accepts `simulate` and broadcasts (the severe one)
- #1959 — `?simulate=true` silently ignored on `transfer`, reframed to the query-param path
- #1933 — blocked in practice by #1929: iterating on aave-v3 payloads has real consequences while that route cannot dry-run

Reporters: @RichardReki, @subheeksh5599.

## What accepting this commits to

1. The protocol route gains dry-run support, or refuses `simulate` with a 400 naming the routes that support it.
2. A `simulate` query parameter is rejected with a 400 on every `/api/execute/*` route, rather than ignored.
3. A test asserting the invariant across all four routes, so a fifth route cannot be added without it.

Item 1 is the one that needs a maintainer decision: implementing dry-run for arbitrary protocol actions is materially more work than refusing the flag, and refusing it is the smaller change that removes the hazard today.

Contributor guide

Open the contributing guide

Research direction

Start with app/api/execute/[...slug]/route.ts and the existing parseSimulateFlag behavior, then compare the transfer, contract-call, check-and-execute, and protocol action routes. Decide whether the protocol route should support dry runs or reject simulate, reject query parameters consistently, and add a test covering the invariant across all four routes.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
api, blockchain, security
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.