KeeperHub / KeeperHub/keeperhub

fix(protocols): 38 actions are declared on L2 chains where the contract does not implement them

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

Description

### Before filing

- [x] I searched open and closed issues for this proposal.
- [x] I checked the docs and the current behaviour on `staging`.
- [x] This is one change, not several.

### Reason: what happens

38 protocol actions are declared on L2 chains where the deployed contract does not implement them. Every one reverts on call. 10 of them are writes, so a user pays gas before finding out.

Verified against `staging` at `f8c8f18c7` over public RPC, minutes before filing:

**`lido` wstETH on Base (`8453`), `0xc1CBa3fCea344f92D9239c08C0568f6F2F0ee452`**. 6 of 8 declared actions fail:

```
get-steth-by-wsteth REVERT
get-wsteth-by-steth REVERT
steth-per-token REVERT
tokens-per-steth REVERT
wrap (write) REVERT
unwrap (write) REVERT
get-wsteth-balance OK 0
get-wsteth-total-supply OK 27219250972291143228143
```

**`sky` sUSDS on Base (`8453`) `0x5875eEE11Cf8398102FdAd704C9E96607675467a` and Arbitrum (`42161`) `0xdDb46999F8891663a8F2828d25298f70416d7610`**. 16 of 18 fail on each:

```
vault-asset, vault-total-assets, vault-convert-to-assets,
vault-convert-to-shares, vault-preview-{deposit,mint,withdraw,redeem},
vault-max-{deposit,mint,withdraw,redeem} all REVERT (reads)
vault-deposit, vault-mint, vault-withdraw, vault-redeem all REVERT (writes)
vault-balance, vault-total-supply OK
```

**This is not an RPC fault and not a wrong address.** On those exact addresses, on the same call in the same second, the ERC-20 surface answers normally:

```
Base wstETH name() = "Wrapped liquid staked Ether 2.0" totalSupply() = 27219250972291143228143
Base sUSDS name() = "Savings USDS" totalSupply() = 10829128842209047357151231
Arb sUSDS name() = "Savings USDS" totalSupply() = 326995432173787510251504877
```

Only the declared non-ERC-20 functions revert. The same function names succeed on mainnet, against the mainnet addresses in the same contract entries:

```
L1 wstETH 0x7f39C581... stEthPerToken() = 1243790495836331246
tokensPerStEth() = 803993922889396867
L1 sUSDS 0xa3931d71... asset() = 0xdC035D45d973E3EC169d2276DDab16f1e407384F
totalAssets() = 4606522745776546741910922524
```

### Reason: why, and what told me to expect otherwise

Both L2 addresses are bridged representations, not second deployments of the L1 contract. Base wstETH answers `bridge() = 0xac9D11cD4D7eF6e54F14643a393F68Ca014287AB`; it is an OP-stack bridged ERC-20 carrying balances and nothing else. The sUSDS addresses on Base and Arbitrum are the same shape: they hold the token, not the ERC-4626 vault logic. A bridged token has the ERC-20 surface, so the two actions that work are exactly the two that are pure ERC-20.

The contract entry is keyed by function set, not by token identity. Adding a chain to `addresses` asserts that **every action derived from that ABI** works there, because `defineAbiProtocol` derives one action set per contract entry and applies it to every chain in the map. `protocols/lido.ts:152-158` lists mainnet, Base and Sepolia under one `wsteth` entry whose ABI includes `stEthPerToken`, `wrap` and `unwrap`.

**What told me to expect otherwise** is this project's own reasoning on #2394, where the Base cbETH exclusion was settled on precisely this ground:

> it is an OP-stack bridged representation, not a second deployment [...] since `defineAbiProtocol` derives one action set per contract entry, adding Base later means a second contract key rather than a second address on this one.

That is the rule. `lido/wsteth` and `sky/sUsds` predate it and do not follow it.

### Reason: what it costs

A read returns a workflow error instead of a value. A write is worse: the action is offered in the builder, the user configures it, the transaction is submitted and it reverts on chain, so they pay gas to discover the action was never supported there. `vault-deposit` and `vault-withdraw` on two chains are the sharp end of that.

Nothing currently catches it. The calldata goldens assert encoding, never that the target implements the function. The tier-1 simulation only exercises chains present in `testData`, where neither protocol declares Base or Arbitrum fixtures. So a chain can be added to an address map and no test anywhere disagrees.

### Scope: what this touches, and what it does not

**In scope:** the chain lists on `protocols/lido.ts` `wsteth` and `protocols/sky.ts` `sUsds`, plus a coverage check that a declared `(action, chain)` is actually callable.

**Not in scope:**

- The mainnet entries, which are correct and verified above.
- `sky/usds`, whose 2 actions are pure ERC-20 and pass on both L2s.
- The `wrapped`, `chainlink`, `aave-v3`, `uniswap`, `superfluid` and other multi-chain entries: I checked every zero-argument read across 8 mainnets and only these two contracts failed.
- Adding the L2 surface back as its own contract key. That is a separate change needing its own issue, since it means new action slugs.

**Surfaces checked:** 141 zero-argument `(read, chain)` pairs across Ethereum, Base, Arbitrum, Optimism, Polygon, BSC, Avalanche and Gnosis: 116 OK, 6 failing, 0 inconclusive. Then all 48 `(action, chain)` pairs on the two suspect contracts including argument-taking reads and writes: 10 OK, 38 failing. Sepolia and other testnets were not swept, so the true figure may be higher on testnets. I would rather state that than imply a complete sweep.

### Plan

1. Remove `8453` from the `lido` `wsteth` entry, plus `8453` and `42161` from the `sky` `sUsds` entry. The two ERC-20 actions that do work on those chains are lost with them, which is the honest trade: one contract entry cannot advertise a function set the address does not implement. If you would rather keep the balance reads, the alternative is a second contract key per the #2394 precedent. I will do it that way instead.
2. A coverage check that every declared `(action, chain)` is callable at its declared address, so a chain cannot be added to an address map without the function set being verified there. This is the piece that does not exist. I would like your steer on where it belongs: a unit test over a recorded fixture keeps CI hermetic, while a live `eth_call` sweep is what actually caught this but puts network in the test path. `tests/integration/protocol-*-onchain.test.ts` is already ungated and network-backed, so that is the precedent I would follow unless you prefer otherwise.

I have the sweep script ready and will attach it to the PR either way.

### Plan: alternatives considered

- **Leave the chains and document the gaps.** Rejected. The builder offers the action; a comment in a protocol file does not reach the user configuring a deposit.
- **Narrow the ABI on the shared entry to the ERC-20 subset.** Rejected: it would strip `wrap`, `unwrap` and the whole vault surface from mainnet, where they work.
- **Add `skipped` entries in `testData`.** Rejected: that silences a fixture, it does not stop the action being offered on a chain where it reverts.

### AI assistance

AI assistance (Claude, Anthropic) was used to write the sweep script and draft this issue. Every call above was executed against live public RPC and the results, the addresses and the contrast with mainnet were verified by the author before filing.

Contributor guide

Open the contributing guide

Research direction

Start with defineAbiProtocol and the wsteth entry in protocols/lido.ts:152-158, then inspect the sUsds entry in protocols/sky.ts and the precedent in #2394. Review tests/integration/protocol-*-onchain.test.ts and the reported RPC results to decide how coverage should be recorded. Done means unsupported L2 action-chain pairs are no longer advertised and a coverage check prevents them from returning unnoticed.

Written by the indexing model from the issue text.

Assessment

Tech stack
blockchain, typescript
Domain
blockchain, testing-qa
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.