KeeperHub / KeeperHub/keeperhub

execute_contract_call: a reused idempotency_key replays a cached failure, so a retry can never recover

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

Description

### Summary

Reusing an `idempotency_key` across retries of a failed `execute_contract_call` makes recovery impossible. KeeperHub caches the outcome including failures and replays it, so an agent that failed once keeps receiving that first failure while the chain has moved on.

I lost about twenty minutes to this and only caught it because I check chain state instead of trusting the response. An agent that trusts the response would have given up on distributing an estate that was ready to distribute.

### What I expected

I read `idempotency_key` the obvious way: one key per logical action, so a retry storm cannot double-submit. For a contract that moves someone's assets that seemed exactly right, and it is what most retry wrappers will do by default.

### What happens

My keeper fired `executeInheritance()` a few seconds before its grace period expired. It reverted with `LK: not yet due`, which is correct. It then retried three more times over four minutes, reusing the same key.

All three retries returned the same revert. Meanwhile the contract was verifiably past its deadline:

```
secondsSinceHeartbeat: 420 (needs 150)
timeoutExceeded: true
graceElapsed: true <- due, callable by anyone
```

```
[executor] attempt 1 failed (Contract call failed: Error(LK: not yet due)); retrying in 47s
[executor] attempt 2 failed (Contract call failed: Error(LK: not yet due)); retrying in 65s
[executor] attempt 3 failed (Contract call failed: Error(LK: not yet due)); retrying in 135s
```

No transaction was sent for attempts 2 through 4. The cached result came back each time.

### Repro

1. Call `execute_contract_call` with `idempotency_key: "k1"` against a function whose precondition is currently false. Let it revert.
2. Wait for the precondition to become true onchain. Verify with a read call.
3. Call again with the same `idempotency_key: "k1"`.
4. You get the original revert. The chain says it should have succeeded.

### Why this one bites harder than most caching bugs

It fails toward doing nothing, on an irreversible and time-critical action. And it is invisible. The response carries a plausible, contract-shaped error message rather than any hint that it came from cache. Nothing distinguishes a fresh revert from a replayed one, so the agent has no way to tell it is looking at stale news.

The parameter name also works against you here. `idempotency_key` reads like a safety feature, and it is, for the double-submit direction. In the retry-after-failure direction it introduces a liveness bug.

### What I changed

Scoped the key per attempt rather than per action:

```ts
idempotency_key: `${executionKey}-a${attempt}`
```

Transport-level duplicates of a single attempt are still deduplicated, which is the case I actually wanted protection for. Double execution is guarded by my contract's own executed flags, which is where that guarantee belongs anyway.

### Suggestions

Any one of these would have saved me the debugging:

- Document that the key caches failures too, not just successes.
- Return `cached: true` on a replayed response so a client can tell.
- Only cache successful executions. A failed call has no side effect worth deduplicating.

Happy to test a fix against the same repro if that helps.

### Environment

- MCP `https://app.keeperhub.com/mcp`, server `keeperhub v1.2.0`
- Sepolia (11155111), verified contract, `execute_contract_call`
- Hand-rolled TypeScript MCP client, node 22

Contributor guide

Open the contributing guide

Research direction

No repository file or test is named in the issue. Start at the execute_contract_call entry point and trace how idempotency_key results are cached and replayed, then reproduce the failed-call and later-successful-call sequence. Done means retries after the precondition changes are no longer trapped by the earlier cached failure, with coverage for the reported sequence.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
api, backend-api-design, blockchain
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.