Skip to content

Add claimRestake action provider#5

Open
zaryab2000 wants to merge 1 commit into
feat/claimAndRestakefrom
claude/jolly-franklin-KxXTy
Open

Add claimRestake action provider#5
zaryab2000 wants to merge 1 commit into
feat/claimAndRestakefrom
claude/jolly-franklin-KxXTy

Conversation

@zaryab2000

Copy link
Copy Markdown
Owner

Summary

Adds a claim-and-restake middleware ActionProvider for Base mainnet that automates the yield-compounding loop: harvest lending rewards → (optionally) swap → restake. Built per the branch PRD.

  • get_claimable_rewards — reads claimable reward token + amount; no state change.
  • claim_rewards — claims rewards from the protocol (Morpho fetches the merkle proof first).
  • claim_and_restake — reads the reward, applies a gas-vs-reward threshold gate, claims, optionally swaps via the in-tree 0x action, and restakes into the same protocol or a generic ERC-4626 vault. Per-leg tx hashes; partial-failure reporting so funds are recoverable.

Protocols

Protocol Reward Preview Claim
Compound III COMP getRewardOwed via static simulation (non-view) CometRewards.claim
Morpho varies off-chain rewards API (claimable + proof) URD claim(account, reward, claimable, proof[])
Moonwell WELL not available on-chain (call claim_rewards) Comptroller claimReward

Restake targets (collision-aware)

Avoids the already-open Lido/Beefy staking PRs:

  • same → re-supply into Compound III (Comet.supply).
  • erc4626 → deposit into any generic ERC-4626 vault after validating asset() matches.

Conventions

  • class ClaimRestakeActionProvider extends ActionProvider<EvmWalletProvider>, super("claimRestake", []), factory claimRestakeActionProvider(), re-exported from action-providers/index.ts.
  • All actions return Promise<string>; errors returned, never thrown.
  • Zod schemas use plain .describe() on every field; no .strip().
  • supportsNetwork scoped to Base mainnet (chainId === "8453") for v1.

Checks

  • pnpm build ✅ · pnpm test ✅ (877 tests, incl. 13 new) · pnpm run lint ✅ · pnpm run format
  • Per-provider README.md ✅ · changeset (patch, past tense) ✅

Notes

  • The swap leg composes the in-tree 0x provider and requires ZEROX_API_KEY; without it it returns a clean non-throwing error and the reward stays in the wallet.
  • A few addresses/endpoints carry TODO(verify-at-build) markers (CometRewards address, Moonwell comptroller/claimReward signature, Morpho rewards API shape) per the PRD's STALE/confirm-at-build gradings — to be re-verified before any upstream PR.

https://claude.ai/code/session_011YXeQLQdEcCRBDhs9HXX5V


Generated by Claude Code

Add a claim-and-restake middleware action provider for Base mainnet that
harvests lending rewards (Compound III, Moonwell, Morpho), applies a
gas-vs-reward threshold gate, optionally swaps the reward via the in-tree
0x action, and restakes into the same protocol or a generic ERC-4626 vault.

- get_claimable_rewards, claim_rewards, claim_and_restake actions
- Compound getRewardOwed read via static simulation (non-view)
- Morpho claimable + merkle proof fetched from the off-chain rewards API
- ERC-4626 asset() validation and per-leg partial-failure reporting
- Unit tests with mocked wallet provider and fetch (13 tests)
- Changeset added
@github-actions github-actions Bot added documentation Improvements or additions to documentation typescript action provider labels Jun 6, 2026
@zaryab2000

Copy link
Copy Markdown
Owner Author

@claude start review

@claude

claude Bot commented Jun 6, 2026

Copy link
Copy Markdown

Claude finished @zaryab2000's task in 3m 16s —— View job


Code Review: claimRestake Action Provider

Tasks:

  • Gather context and understand the request
  • Read all changed files
  • Review main action provider implementation
  • Review schemas and types
  • Review adapters (Compound, Morpho, Moonwell)
  • Review utils and constants
  • Review tests
  • Review README and changeset
  • Compile and post final review

Overall the implementation is well-structured, follows AgentKit conventions (errors returned not thrown, Promise<string> actions, proper factory export, Zod v4 plain .describe()), and passes all checks. Below are findings ranked by severity.


Critical

1. restakeTarget: "same" is semantically broken for Compound

restakeIntoCompound (utils.ts:235) calls Comet.supply(COMP, amount) — it re-supplies the reward token (COMP) into the Compound III USDC market. But Compound III's Base USDC market does not accept COMP as its base asset; supply(COMP, ...) will revert. The "compound back" loop should supply USDC (the base asset), not COMP. Either:

  • Gate this path behind a swap to the base asset first, or
  • Disallow restakeTarget: "same" when the reward token is not the market's base asset, with a clear error.

Fix this →


High

2. Morpho distributor address is trusted from an unverified API response

morphoRewards.ts:112wallet.sendTransaction({ to: top.distributor.address, ... }) sends an on-chain transaction to an address returned by the off-chain Morpho API without any on-chain verification. A compromised or spoofed API response could redirect the transaction to an arbitrary contract. Consider maintaining a constant allowlist of known URD addresses for Base mainnet, or at minimum checking the distributor address against the known on-chain URD registry before submitting.

Fix this →


Medium

3. Only the first Morpho distribution is claimed

getMorphoClaimable and claimMorpho both operate on distributions[0] only. A user with multiple Morpho reward distributions (different reward tokens from different markets) will silently leave all others unclaimed. This is not surfaced in the action description. Either claim all distributions, or document this limitation explicitly in the claim_rewards action description.

Fix this →

4. claimCompound sends a claim transaction even when nothing is owed

compoundRewards.ts:44claimCompound always sends the claim tx. If reward.amount === 0n, this wastes gas and emits a misleading result. Add an early return:

if (reward.amount === 0n) {
  throw new Error("nothing to claim");
}

Fix this →

5. Double RPC call for Compound in claim_and_restake

claimAndRestake calls previewRewardgetCompoundClaimable (which calls simulateContract) and then dispatchClaimclaimCompoundgetCompoundClaimable again (another simulateContract). Pass the preview through to claimCompound so the reward info doesn't need to be re-fetched.

6. No address format validation in schemas

schemas.ts — The user, restakeVault, and swapToAsset fields accept any string. The in-tree ZeroX schemas use .regex(/^0x[a-fA-F0-9]{40}$/) for address fields. Applying the same pattern here would catch invalid inputs before any on-chain call:

user: z.string().regex(/^0x[a-fA-F0-9]{40}$/).optional().describe(...)

Fix this →


Low / Style

7. void wallet / void account suppression pattern

morphoRewards.ts:64 (void wallet) and moonwellRewards.ts:26 (void account) suppress unused-variable warnings with a JS expression instead of the TypeScript convention of prefixing with _. Prefer _wallet / _account parameter names; they're more idiomatic and lint-friendly.

Fix this →

8. claimMoonwell returns amount: 0n after a successful claim

moonwellRewards.ts:62 — the returned reward always has amount: 0n, which then appears as undefined in the claim_rewards JSON response (since reward.amount > 0n is false). A comment in the JSDoc explaining this is intentional (the actual amount is unknowable without inspecting the receipt logs) would prevent future confusion.

9. Missing test coverage for several branches

The test file (claimRestakeActionProvider.test.ts) covers the happy path for claim_and_restake with ERC-4626 only. Notable gaps:

  • restakeTarget: "same" success path
  • claim_and_restake with swapToAsset (the swap leg)
  • claim_and_restake with Morpho or Moonwell protocol
  • claimMorpho when distributions.length === 0 (throws "nothing to claim")

10. Three TODO(verify-at-build) markers need resolution before upstream PR

  • COMET_REWARDS_ADDRESS (0x1239...) — verify against Compound docs/registry
  • MOONWELL_COMPTROLLER_ADDRESS — verify against live Base deployment
  • Morpho rewards API endpoint/response shape — verify against docs.morpho.org

These are acknowledged in the PRD and the PR description; just confirming they must be resolved before opening upstream.


Summary

Severity Count
Critical 1 (Compound same restake reverts on-chain)
High 1 (Morpho distributor trust)
Medium 4
Low/Style 4

The structure, conventions, changeset, and README are all solid. The critical issue with restakeTarget: "same" needs to be resolved before this can be considered merge-ready — it would result in a transaction that always reverts. The Morpho distributor trust issue is also worth addressing before going upstream. Everything else is polish.

@zaryab2000 zaryab2000 left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code review of the claimRestake AP (CI is fully green; this is a correctness/quality pass).

Highest priority — one active correctness bug:

  1. Dead swap-failure guardswapOutcome.startsWith("Error") never matches a real 0x failure because executeSwap always returns JSON. Failed swaps are treated as success and can restake unrelated funds. (inline)

Other correctness/robustness:
2. Morpho claimable is cumulative — overstates the reward for the gate and the reported amount.
3. Moonwell claim always reports amount: 0n (→ undefined) even when WELL was transferred.
4. Gate is silently bypassed when the reward token can't be priced — a user-set minRewardUsd floor is ignored.
5. Latent decimals assumption in the swap sellAmount for non-18-decimal / null-preview reward tokens.
6. Morpho distributions fetched twice per claim_and_restake (preview vs claim can diverge) + duplicated mapping.

Cleanup:
7. Re-implements existing ERC-20 read helpers (erc20/getTokenDetails, compound/utils).

#1 is the load-bearing one and worth fixing before merge. Happy to push fixes for #1, #3, #4, and coinbase#6 if you'd like — #2 and #5 depend on the TODO(verify-at-build) API/decimals details.

🤖 Generated with Claude Code


Generated by Claude Code

});
steps.swap = { from: rewardToken, to: targetToken, outcome: swapOutcome };

if (swapOutcome.startsWith("Error")) {

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug (correctness): swap-failure guard is dead code. swapReward delegates to ZeroXActionProvider.executeSwap, which always returns JSON.stringify({ success: false, error: ... }) on every failure path (zeroXActionProvider.ts lines 248–417) — a string starting with {, never "Error". So swapOutcome.startsWith("Error") is never true for a real 0x failure; the only string that ever starts with "Error" is the missing-API-key sentinel from swapReward itself.

Consequence: a reverted/failed swap (no liquidity, insufficient balance, tx error) is treated as success. Execution falls through and computes restakeAmount = balanceAfterSwap - balanceBeforeSwap. If the target token already had a balance, that delta can be > 0 from unrelated funds → the loop restakes pre-existing user tokens and reports success while the reward swap actually failed.

Fix: parse the JSON and check success, e.g.

let parsed: { success?: boolean };
try { parsed = JSON.parse(swapOutcome); } catch { parsed = { success: false }; }
if (swapOutcome.startsWith("Error") || parsed.success === false) {
  // recoverable: reward claimed but swap failed
}

Generated by Claude Code

return {
token: top.asset.address,
symbol: top.asset.symbol ?? "UNKNOWN",
amount: BigInt(top.claimable),

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correctness/semantics: Morpho claimable is cumulative, not the newly-accrued delta. The URD/Merkl claimable field is the lifetime cumulative entitlement; the on-chain claim only transfers claimable - alreadyClaimed. Here BigInt(top.claimable) is used (a) as the reward amount feeding evaluateGate, and (b) as the reported claim amount.

Consequence: a user who has already claimed most of the distribution still shows the full cumulative figure, so the gas-vs-reward gate passes on what is actually dust, and get_claimable_rewards/claim_rewards report an amount that disagrees with the real transfer (and with the balance delta in claim_and_restake).

Since the exact API shape is already flagged TODO(verify-at-build), please also confirm whether the endpoint exposes an already-claimed / pending-delta field and gate/report on the delta rather than the cumulative total.


Generated by Claude Code

await wallet.waitForTransactionReceipt(txHash);

return {
reward: { token: MOONWELL_WELL_TOKEN, symbol, amount: 0n, decimals },

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correctness (reporting): Moonwell claim always reports amount: 0n. claimMoonwell hardcodes amount: 0n, so claim_rewards for Moonwell reports amount: undefined (provider line 143) even when the claimReward tx actually transferred WELL. The user is told nothing was claimed despite a successful on-chain transfer.

Consider reading the WELL balance delta around the claim tx (as claim_and_restake already does) and reporting the realized amount, or document explicitly that the amount is unavailable for Moonwell.


Generated by Claude Code

};
}

if (rewardUsd !== null && gasUsd !== null && rewardUsd < gasUsd * DEFAULT_GAS_MULTIPLE) {

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correctness: gate is silently bypassed when the reward can't be priced. Both the minRewardUsd floor (line 181) and the gas-multiple gate (line 190) are guarded by rewardUsd !== null. If getTokenUsdPrice returns null (token not on DefiLlama, or a transient !response.ok), rewardUsd is null, both checks are skipped, and evaluateGate returns skip: false.

Consequence: a user who set minRewardUsd = 50 to avoid dust will have the full claim→swap→restake loop run on a sub-cent reward whenever pricing is unavailable — the explicit floor they set is ignored. Safer default is to skip (or surface) when the reward is unpriceable, especially when minRewardUsd was provided.


Generated by Claude Code

const swapOutcome = await swapReward(walletProvider, {
sellToken: rewardToken,
buyToken: targetToken,
sellAmount: formatUnits(received, rewardDecimals),

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Latent correctness: swap sellAmount is formatted with assumed decimals. rewardDecimals = preview?.decimals ?? 18 (line 221), but received is atomic units of the reward token and 0x re-parses this human string with the token's actual on-chain decimals. For the null-preview path (Moonwell) the ?? 18 is a guess; any reward token whose real decimals ≠ 18 yields an order-of-magnitude-wrong sellAmount (round-trip atomic → human@wrong-decimals → atomic).

Currently latent (only Moonwell is preview-null and WELL is 18-decimals), but it's fragile. Prefer reading the reward token's real decimals (e.g. getErc20Decimals(walletProvider, rewardToken)) before formatting received, rather than relying on the preview default.


Generated by Claude Code

wallet: EvmWalletProvider,
account: Address,
): Promise<{ reward: ClaimableReward; txHash: string }> {
const distributions = await fetchMorphoDistributions(account);

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Robustness + duplication: Morpho distributions are fetched twice per claim_and_restake, and the mapping is copy-pasted. claim_and_restake calls getMorphoClaimable (preview) and then claimMorpho independently calls fetchMorphoDistributions again. The off-chain claimable/proof can change between the two fetches (new merkle root published), so the previewed/gated amount and the claimed amount may diverge. The distribution→ClaimableReward mapping is also duplicated verbatim in getMorphoClaimable (lines 75–82) and claimMorpho (lines 113–123).

Consider fetching once and threading the result through, and extracting the mapping into a single helper.


Generated by Claude Code

* @param token - The token address.
* @returns The token decimals.
*/
export async function getErc20Decimals(wallet: EvmWalletProvider, token: Address): Promise<number> {

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cleanup (reuse): re-implements existing ERC-20 read helpers. getErc20Decimals/getErc20Symbol/getErc20Balance duplicate helpers already in the tree — compound/utils.ts (getTokenDecimals/getTokenSymbol/getTokenBalance, near-identical bodies) and erc20/utils.ts getTokenDetails, which batches name+decimals+balance into a single multicall. The reward path needs decimals+symbol+balance, so getTokenDetails is the natural reuse and also collapses ~3 sequential RPCs into one. Upstream reviewers are likely to flag the duplication.


Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

action provider documentation Improvements or additions to documentation typescript

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants