Add bridge-then-deploy action provider - #3

Closed
zaryab2000 wants to merge 3 commits into
feat/bridgeThenDeployfrom
claude/tender-mccarthy-ROhys
Closed

Add bridge-then-deploy action provider#3
zaryab2000 wants to merge 3 commits into
feat/bridgeThenDeployfrom
claude/tender-mccarthy-ROhys

Conversation

@zaryab2000

Copy link
Copy Markdown
Owner

Summary

Adds a new bridge-then-deploy ActionProvider (bridgeDeploy) that composes an Across bridge with a destination lending/vault supply, letting an agent express "move this capital to chain X and put it to work" as a single intent.

Central design constraint (honest by design)

Across destination-side execution only delivers to a deployed handler contract that implements handleV3AcrossMessage(...)never to a plain EOA. Agent wallets are EOAs, so this flow is deliberately two-step and non-atomic: bridge → poll status → supply on arrival. The provider never claims the supply has happened before the bridge fills.

Actions

  • bridge_and_deploy — initiates the Across deposit and records the pending destination supply; returns a depositId and status: "bridging". Does not supply yet.
  • bridge_deploy_status — polls the Across deposit-status API; once filled, auto-runs the recorded destination supply and returns the deploy tx hash. Reports pending/refunded otherwise.
  • deploy_on_destination — supplies an already-bridged token into Compound (Comet supply) or Morpho (vault deposit) on Base. Performs a balance preflight, so it is safe to retry to recover from a partial failure.

Scope

  • Destination deploy gated to Base (8453/84532); origin can be any EVM chain Across supports.
  • v1 ships Flow A (EOA, two-step) only. Flow B (handler-contract atomic deposit) is documented in the README as the upgrade path.

Conventions

  • All actions return Promise<string>; errors are returned, not thrown.
  • Zod v4 schemas with .describe() on every field; no .strip().
  • Reuses the shared approve util and mirrors the in-tree across provider's SDK wiring (privateKey constructor config, fully mocked in tests).
  • Factory re-exported from src/action-providers/index.ts.

Checks

  • pnpm test — full agentkit suite green (875 tests, incl. 11 new for this provider, mocked wallet + Across SDK).
  • pnpm run lint, pnpm run format, pnpm build — all clean.
  • Changeset added (@coinbase/agentkit, patch, past tense).

Note: per-provider README.md prominently documents the two-step / non-atomic / EOA-can't-receive-callback semantics and the partial-failure recovery path.

https://claude.ai/code/session_013rnmDYrKDMctusY33uneiy


Generated by Claude Code

Composes an Across bridge with a destination lending/vault supply so an
agent can express "move this capital to chain X and put it to work" as one
intent. Honest about the EOA constraint: Across destination execution only
reaches a handler contract, never an EOA, so the flow is two-step and
non-atomic (bridge -> poll status -> supply on arrival).
Actions:
- bridge_and_deploy: initiates the Across deposit and records the pending
destination supply; never claims the supply has happened.
- bridge_deploy_status: polls the Across deposit-status API and auto-runs
the recorded supply once the bridge is filled.
- deploy_on_destination: supplies an already-bridged token into Compound or
Morpho on Base, with a balance preflight so it is safe to retry.
Destination deploy is gated to Base; origin can be any EVM chain Across
supports. Includes unit tests (mocked wallet + Across SDK), README
documenting the non-atomic semantics, and a changeset.
https://claude.ai/code/session_013rnmDYrKDMctusY33uneiy
@github-actionsgithub-actionsBot added documentation Improvements or additions to documentation typescript action provider labels Jun 6, 2026
@zaryab2000

Copy link
Copy Markdown
OwnerAuthor

@claude start review

@claude

claudeBot commented Jun 6, 2026

Copy link
Copy Markdown

Claude finished @zaryab2000's task in 2m 59s —— View job


Code Review — bridgeDeploy ActionProvider

Tasks

  • Load tools
  • Read all changed files (bridgeDeployActionProvider.ts, utils.ts, schemas.ts, constants.ts, index.ts, test, README, changeset)
  • Analyze against AgentKit conventions (CLAUDE.md)
  • Review for bugs, security, correctness
  • Post final review

Overall the code is well-structured and closely follows AgentKit conventions. All three major checks from CLAUDE.md pass (Promise, Zod v4 with .describe(), arrow-function supportsNetwork). Below are the findings in priority order.


🔴 High — Correctness Bugs

1. In-memory #pendingDeploys is lost on provider re-instantiation

bridgeDeployActionProvider.ts:46

#pendingDeploys: Map<string,PendingDeploy>=newMap();

#pendingDeploys is instance-level state. The factory bridgeDeployActionProvider(config) creates a new instance each time it is called. In most agent patterns the provider is re-created between sessions (or even between turns). If the provider is re-instantiated while a bridge is in flight — which can take minutes to an hour — the pending deploy record is gone. bridge_deploy_status will then return the "no pending supply was recorded" fallback instead of auto-supplying.

This either needs to be documented prominently as a hard constraint ("the same provider instance must be reused across calls") or the pending state must be serialized to somewhere durable (a file, a passed-in store, etc.). At minimum the README should warn that a new instance forgets all pending deploys.

Fix this →


2. Compound supply() silently ignores recipient

utils.ts:339-344

if(params.protocol==="compound"){data=encodeFunctionData({abi: COMET_SUPPLY_ABI,functionName: "supply",args: [token,atomicAmount],// ← no recipient});}

The supply(address asset, uint amount) Comet function deposits from msg.sender into msg.sender's own position. There is no recipient argument here. If the caller passes recipient ≠ walletProvider.getAddress(), for Morpho the deposit goes to the intended recipient, but for Compound it silently goes to the caller's position instead. This is a silent semantic divergence.

Comet provides supplyTo(address dst, address asset, uint amount) for this purpose. The ABI in constants.ts and the encodeFunctionData call should use supplyTo when recipient != walletProvider.getAddress(), or the COMET_SUPPLY_ABI should be updated to include supplyTo and always use it.

Fix this →


3. Quoted outputAmount stored as supply target — mismatches actual fill

bridgeDeployActionProvider.ts:112-117

this.#pendingDeploys.set(this.#pendingKey(...),{
...
amount: deposit.outputAmount,// ← from quote, not actual fill
...
});

deposit.outputAmount is the quoted output. The actual fill amount may differ. When deployToProtocol later runs the balance preflight (balance < atomicAmount), it uses the quoted amount, which can either:

  • Block the supply if the fill was slightly less than quoted (e.g., fee changed between quote and fill)
  • Over-supply if the fill was more than quoted (e.g., the relayer undercut the fee)

The safer approach is to supply Math.min(balance, atomicAmount) — i.e., use whatever landed — or to drop the preflight comparison and let the protocol revert if underfunded.


🟡 Medium — Reliability / Security

4. depositId interpolated into URL without encoding

utils.ts:255-258

constresponse=awaitfetch(`${ACROSS_DEPOSIT_STATUS_API}?originChainId=${originChainId}&depositId=${depositId}`,

depositId (and originChainId) are user-supplied strings concatenated directly into the URL. If either contains &, =, or #, the query string is silently malformed. Use URLSearchParams:

consturl=newURL(ACROSS_DEPOSIT_STATUS_API);url.searchParams.set("originChainId",String(originChainId));url.searchParams.set("depositId",depositId);constresponse=awaitfetch(url.toString(),{method: "GET"});

Fix this →


5. Two test paths not covered

bridgeDeployActionProvider.test.ts

  • filled with no pending deploybridgeDeployStatus returns a "call deploy_on_destination manually" note when the map has no entry for the deposit. bridgeDeployActionProvider.ts:178-187 handles this but it is never exercised.
  • Supply fails on filledbridgeDeployActionProvider.ts:191-198 keeps the pending deploy for retry when deployResult.startsWith("Error"). This retry path has no test.

6. Floating-point slippage arithmetic

utils.ts:183-185

constslippageBps=((Number(formattedInput)-Number(formattedOutput))/Number(formattedInput))*10000;

Converting bigintformatUnitsNumber → arithmetic introduces floating-point rounding. For basis-point comparisons, prefer bigint arithmetic directly:

constslippageBps=Number(((quote.deposit.inputAmount-quote.deposit.outputAmount)*10000n)/quote.deposit.inputAmount);

(The existing across provider has the same issue — it is a pre-existing pattern, but worth fixing here.)


🟢 Low — Minor / Style

7. Dead validation in constructor (bridgeDeployActionProvider.ts:56-57)

constaccount=privateKeyToAccount(this.#privateKey asHex);if(!account)thrownewError("Invalid private key");

privateKeyToAccount from viem throws on an invalid key — it never returns null or undefined. The if (!account) branch is unreachable. (Same pattern in the upstream across provider.)


8. http() with no RPC URL (utils.ts:107)

transport: http(),

http() with no argument uses the chain's built-in public RPC, which is rate-limited. This is fine for occasional test runs but can fail under production load. (Same pattern in across provider — pre-existing.)


9. Token identifier inconsistency between actions

bridge_and_deploy accepts token as a symbol ("USDC"); deploy_on_destination accepts token as an address ("0x833..."). This is noted in the README, but the inconsistency creates a real risk that an LLM agent passes a symbol to the standalone deploy action and hits a silent failure (the Zod regex rejects non-address strings, so at least it fails loudly).

Consider adding a note to the deploy_on_destination action description explicitly warning: "token must be the destination-chain ERC-20 address, not the symbol".


Conventions Checklist

RuleStatus
All actions return Promise<string>; errors returned not thrown
Zod v4 schemas with .describe() on every field; no .strip()
supportsNetwork is an arrow-function property
Factory bridgeDeployActionProvider exported and re-exported from index.ts
README.md present
pnpm run changeset — patch, past tense, correct package
Tests present and mocking wallet provider
super("bridgeDeploy", []) constructor call
Network.chainId compared as string

Summary: The implementation is clean and well-documented. The two blocking issues before upstream merge are the Compound supplyTo vs supply recipient bug (#2) and the in-memory state loss documentation (#1). The URL encoding (#4) and slippage arithmetic (#6) are simple one-liners to fix. The missing test coverage (#5) should be added before the PR goes out.

- Compound: use supplyTo(dst, asset, amount) so the position is credited to
the recipient instead of msg.sender (silent divergence from Morpho).
- Auto-deploy: supply the amount that actually landed when a fill comes in
under the quoted output, instead of failing the stale-quote preflight;
deploy_on_destination stays strict for explicit calls.
- getDepositStatus: build the status URL with URLSearchParams.
- Slippage check now uses bigint arithmetic to avoid float rounding.
- Document the in-memory pending-deploy state (lost on re-instantiation) in
the README and on the field; clarify token is an address (not symbol) for
deploy_on_destination.
- Add tests for the filled-without-pending-record and supply-fails-retry paths.
https://claude.ai/code/session_013rnmDYrKDMctusY33uneiy
@zaryab2000Claude

Copy link
Copy Markdown
OwnerAuthor

Thanks for the review — addressed in 8633b59. Summary of what changed and why:

Fixed

Intentionally deferred (with rationale)


Generated by Claude Code

@zaryab2000

Copy link
Copy Markdown
OwnerAuthor

@claude conduct final review and confirm if the PR is accurate now?

@claude

claudeBot commented Jun 11, 2026

Copy link
Copy Markdown

Claude finished @zaryab2000's task in 1m 38s —— View job


Final Review — bridgeDeploy ActionProvider

Tasks


All seven addressed issues are correctly fixed. Here is the per-item confirmation:

Verified fixes

#IssueStatusEvidence
#2Compound supplyTo recipient✅ FixedCOMET_SUPPLY_ABI in constants.ts now includes supplyTo(address dst, address asset, uint256 amount); utils.ts:358–362 uses supplyTo with [params.recipient, token, atomicAmount]
#3Quoted vs actual fill✅ FixeddeployToProtocol takes allowPartial = false; auto-deploy path (bridge_deploy_status) passes true; supplies balance when partial; standalone deploy_on_destination stays strict (false)
#4URL encoding✅ Fixedutils.ts:256–259 uses new URL(ACROSS_DEPOSIT_STATUS_API) + url.searchParams.set(...)
#6Bigint slippage arithmetic✅ Fixedutils.ts:183–185: ((inputAmount - outputAmount) * 10000n) / inputAmount — no float conversion
#1In-memory state documented✅ FixedJSDoc on #pendingDeploys (bridgeDeployActionProvider.ts:47–56) and a dedicated "⚠️ Pending-supply state is in-memory only" section in README.md:23–40 both clearly state the instance-local constraint
coinbase#9Token address vs symbol warning✅ Fixeddeploy_on_destination action description says "NOT the token symbol"; README.md:74–76 also distinguishes bridge_and_deploy (symbol) from deploy_on_destination (address)
#5Missing test coverage✅ Fixedtest.ts:216–236 covers filled with no pending record; test.ts:238–270 covers supply-fails-on-filled retry retention

Intentionally deferred (confirmed reasonable)

One minor observation (non-blocking)

COMET_SUPPLY_ABI in constants.ts still includes the supply(address asset, uint256 amount) entry, but the code now always calls supplyTo. The supply entry is dead code in the execution path. The comment justifies its presence for documentation purposes, and since it cannot cause a runtime error, this is harmless — just worth noting for cleanliness.

Conventions checklist

RuleStatus
All actions return Promise<string>; errors returned not thrown
Zod v4 schemas with .describe() on every field; no .strip()
supportsNetwork is an arrow-function property
Factory bridgeDeployActionProvider exported and re-exported from index.ts
README.md present and documents EOA/two-step semantics
Changeset — @coinbase/agentkit, patch, past tense
Tests present, mocked wallet, 13 provider tests
super("bridgeDeploy", []) constructor call
Network.chainId compared as string

The PR is accurate. All blocking issues from the previous review have been correctly addressed. The one remaining item (supply ABI entry being unused) is non-blocking cosmetic cleanup. The implementation is clean, well-documented, and matches AgentKit conventions.

@zaryab2000Claude

Copy link
Copy Markdown
OwnerAuthor

Final review — confirmed accurate ✅

Re-reviewed the full provider on head 8633b59 and verified all the review fixes landed correctly.

Correctness — verified in code

  • Compound recipient:deployToProtocol now encodes supplyTo(recipient, asset, amount) (utils.ts:357-362), so the position is credited to the intended owner, matching Morpho's deposit(assets, receiver). ✅
  • Partial-fill handling: auto-deploy passes allowPartial=true and supplies min(requested, balance) when a fill lands under the quote; deploy_on_destination stays strict (exact amount or error). ✅
  • URL building:getDepositStatus uses URLSearchParams. ✅
  • Slippage: computed with bigint arithmetic. ✅
  • State + token docs: in-memory #pendingDeploys warning on the field and in the README; address-not-symbol note on deploy_on_destination. ✅
  • Error-return semantics, Promise<string>, Zod .describe() (no .strip()), arrow supportsNetwork, factory re-export — all intact.

CI — green on head 8633b59

  • 25/25 checks passing (lint-typescript, format-typescript, test-agentkit-typescript (18 & 20), check-package-lock, plus the python jobs).
  • mergeable_state: clean — no conflicts with feat/bridgeThenDeploy.

Local

  • 13 provider tests + 877 full agentkit suite green; lint / format / build clean.

One honest limitation worth recording (not a blocker, by design):
The auto-deploy can only supply funds the wallet custodies. If you set recipient on bridge_and_deploy to a third party, the bridge sends there and the wallet holds nothing, so the auto-supply returns a graceful "insufficient balance" (pending kept for retry) rather than silently doing nothing. With the default (recipient = sender) the full bridge→supply flow works as intended. This is consistent and non-lossy, and the README documents that recipient defaults to the sender.

Minor / optional: the PR description is slightly stale vs the latest commits — it still says "875 tests" and "Compound (Comet supply)" (now 877 tests and supplyTo). Happy to refresh the description if you'd like; the code and changeset are accurate.

Still intentionally deferred:#7 (dead if (!account)) and coinbase#8 (http() default RPC) — verbatim parity with the in-tree across provider; will change if you prefer divergence.

Net: the PR is accurate and merge-ready into feat/bridgeThenDeploy. Reminder that upstream (coinbase/agentkit) still requires the human-only steps from our checklist — commit signing, the docs-internal//fork-tooling strip, and a rebase on the latest main — before it goes out as the public contribution.


Generated by Claude Code

The Compound path always uses supplyTo to honor the recipient, so the
supply(asset, amount) entry was dead code. Drop it from COMET_SUPPLY_ABI.
https://claude.ai/code/session_013rnmDYrKDMctusY33uneiy
@zaryab2000Claude

Copy link
Copy Markdown
OwnerAuthor

Addressed the one new observation from the final review in 5a6c1fa: removed the now-unused supply(asset, amount) entry from COMET_SUPPLY_ABI — the Compound path always calls supplyTo, so it was dead code. ABI now contains only supplyTo.

No functional change; the dead-code path is gone. Verified: 13 provider tests + full agentkit suite green, lint / format / build clean.

No other new bugs were reported in the latest review (it confirmed all 7 prior fixes as correct). PR remains accurate and merge-ready into feat/bridgeThenDeploy.


Generated by Claude Code

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

Labels

action providerdocumentationImprovements or additions to documentationtypescript

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@zaryab2000@claude
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

Add bridge-then-deploy action provider - #3

Closed
zaryab2000 wants to merge 3 commits into
feat/bridgeThenDeployfrom
claude/tender-mccarthy-ROhys
Closed

Add bridge-then-deploy action provider#3
zaryab2000 wants to merge 3 commits into
feat/bridgeThenDeployfrom
claude/tender-mccarthy-ROhys

Conversation

@zaryab2000

Copy link
Copy Markdown
Owner

Summary

Adds a new bridge-then-deploy ActionProvider (bridgeDeploy) that composes an Across bridge with a destination lending/vault supply, letting an agent express "move this capital to chain X and put it to work" as a single intent.

Central design constraint (honest by design)

Across destination-side execution only delivers to a deployed handler contract that implements handleV3AcrossMessage(...)never to a plain EOA. Agent wallets are EOAs, so this flow is deliberately two-step and non-atomic: bridge → poll status → supply on arrival. The provider never claims the supply has happened before the bridge fills.

Actions

  • bridge_and_deploy — initiates the Across deposit and records the pending destination supply; returns a depositId and status: "bridging". Does not supply yet.
  • bridge_deploy_status — polls the Across deposit-status API; once filled, auto-runs the recorded destination supply and returns the deploy tx hash. Reports pending/refunded otherwise.
  • deploy_on_destination — supplies an already-bridged token into Compound (Comet supply) or Morpho (vault deposit) on Base. Performs a balance preflight, so it is safe to retry to recover from a partial failure.

Scope

  • Destination deploy gated to Base (8453/84532); origin can be any EVM chain Across supports.
  • v1 ships Flow A (EOA, two-step) only. Flow B (handler-contract atomic deposit) is documented in the README as the upgrade path.

Conventions

  • All actions return Promise<string>; errors are returned, not thrown.
  • Zod v4 schemas with .describe() on every field; no .strip().
  • Reuses the shared approve util and mirrors the in-tree across provider's SDK wiring (privateKey constructor config, fully mocked in tests).
  • Factory re-exported from src/action-providers/index.ts.

Checks

  • pnpm test — full agentkit suite green (875 tests, incl. 11 new for this provider, mocked wallet + Across SDK).
  • pnpm run lint, pnpm run format, pnpm build — all clean.
  • Changeset added (@coinbase/agentkit, patch, past tense).

Note: per-provider README.md prominently documents the two-step / non-atomic / EOA-can't-receive-callback semantics and the partial-failure recovery path.

https://claude.ai/code/session_013rnmDYrKDMctusY33uneiy


Generated by Claude Code

Composes an Across bridge with a destination lending/vault supply so an
agent can express "move this capital to chain X and put it to work" as one
intent. Honest about the EOA constraint: Across destination execution only
reaches a handler contract, never an EOA, so the flow is two-step and
non-atomic (bridge -> poll status -> supply on arrival).
Actions:
- bridge_and_deploy: initiates the Across deposit and records the pending
destination supply; never claims the supply has happened.
- bridge_deploy_status: polls the Across deposit-status API and auto-runs
the recorded supply once the bridge is filled.
- deploy_on_destination: supplies an already-bridged token into Compound or
Morpho on Base, with a balance preflight so it is safe to retry.
Destination deploy is gated to Base; origin can be any EVM chain Across
supports. Includes unit tests (mocked wallet + Across SDK), README
documenting the non-atomic semantics, and a changeset.
https://claude.ai/code/session_013rnmDYrKDMctusY33uneiy
@github-actionsgithub-actionsBot added documentation Improvements or additions to documentation typescript action provider labels Jun 6, 2026
@zaryab2000

Copy link
Copy Markdown
OwnerAuthor

@claude start review

@claude

claudeBot commented Jun 6, 2026

Copy link
Copy Markdown

Claude finished @zaryab2000's task in 2m 59s —— View job


Code Review — bridgeDeploy ActionProvider

Tasks

  • Load tools
  • Read all changed files (bridgeDeployActionProvider.ts, utils.ts, schemas.ts, constants.ts, index.ts, test, README, changeset)
  • Analyze against AgentKit conventions (CLAUDE.md)
  • Review for bugs, security, correctness
  • Post final review

Overall the code is well-structured and closely follows AgentKit conventions. All three major checks from CLAUDE.md pass (Promise, Zod v4 with .describe(), arrow-function supportsNetwork). Below are the findings in priority order.


🔴 High — Correctness Bugs

1. In-memory #pendingDeploys is lost on provider re-instantiation

bridgeDeployActionProvider.ts:46

#pendingDeploys: Map<string,PendingDeploy>=newMap();

#pendingDeploys is instance-level state. The factory bridgeDeployActionProvider(config) creates a new instance each time it is called. In most agent patterns the provider is re-created between sessions (or even between turns). If the provider is re-instantiated while a bridge is in flight — which can take minutes to an hour — the pending deploy record is gone. bridge_deploy_status will then return the "no pending supply was recorded" fallback instead of auto-supplying.

This either needs to be documented prominently as a hard constraint ("the same provider instance must be reused across calls") or the pending state must be serialized to somewhere durable (a file, a passed-in store, etc.). At minimum the README should warn that a new instance forgets all pending deploys.

Fix this →


2. Compound supply() silently ignores recipient

utils.ts:339-344

if(params.protocol==="compound"){data=encodeFunctionData({abi: COMET_SUPPLY_ABI,functionName: "supply",args: [token,atomicAmount],// ← no recipient});}

The supply(address asset, uint amount) Comet function deposits from msg.sender into msg.sender's own position. There is no recipient argument here. If the caller passes recipient ≠ walletProvider.getAddress(), for Morpho the deposit goes to the intended recipient, but for Compound it silently goes to the caller's position instead. This is a silent semantic divergence.

Comet provides supplyTo(address dst, address asset, uint amount) for this purpose. The ABI in constants.ts and the encodeFunctionData call should use supplyTo when recipient != walletProvider.getAddress(), or the COMET_SUPPLY_ABI should be updated to include supplyTo and always use it.

Fix this →


3. Quoted outputAmount stored as supply target — mismatches actual fill

bridgeDeployActionProvider.ts:112-117

this.#pendingDeploys.set(this.#pendingKey(...),{
...
amount: deposit.outputAmount,// ← from quote, not actual fill
...
});

deposit.outputAmount is the quoted output. The actual fill amount may differ. When deployToProtocol later runs the balance preflight (balance < atomicAmount), it uses the quoted amount, which can either:

  • Block the supply if the fill was slightly less than quoted (e.g., fee changed between quote and fill)
  • Over-supply if the fill was more than quoted (e.g., the relayer undercut the fee)

The safer approach is to supply Math.min(balance, atomicAmount) — i.e., use whatever landed — or to drop the preflight comparison and let the protocol revert if underfunded.


🟡 Medium — Reliability / Security

4. depositId interpolated into URL without encoding

utils.ts:255-258

constresponse=awaitfetch(`${ACROSS_DEPOSIT_STATUS_API}?originChainId=${originChainId}&depositId=${depositId}`,

depositId (and originChainId) are user-supplied strings concatenated directly into the URL. If either contains &, =, or #, the query string is silently malformed. Use URLSearchParams:

consturl=newURL(ACROSS_DEPOSIT_STATUS_API);url.searchParams.set("originChainId",String(originChainId));url.searchParams.set("depositId",depositId);constresponse=awaitfetch(url.toString(),{method: "GET"});

Fix this →


5. Two test paths not covered

bridgeDeployActionProvider.test.ts

  • filled with no pending deploybridgeDeployStatus returns a "call deploy_on_destination manually" note when the map has no entry for the deposit. bridgeDeployActionProvider.ts:178-187 handles this but it is never exercised.
  • Supply fails on filledbridgeDeployActionProvider.ts:191-198 keeps the pending deploy for retry when deployResult.startsWith("Error"). This retry path has no test.

6. Floating-point slippage arithmetic

utils.ts:183-185

constslippageBps=((Number(formattedInput)-Number(formattedOutput))/Number(formattedInput))*10000;

Converting bigintformatUnitsNumber → arithmetic introduces floating-point rounding. For basis-point comparisons, prefer bigint arithmetic directly:

constslippageBps=Number(((quote.deposit.inputAmount-quote.deposit.outputAmount)*10000n)/quote.deposit.inputAmount);

(The existing across provider has the same issue — it is a pre-existing pattern, but worth fixing here.)


🟢 Low — Minor / Style

7. Dead validation in constructor (bridgeDeployActionProvider.ts:56-57)

constaccount=privateKeyToAccount(this.#privateKey asHex);if(!account)thrownewError("Invalid private key");

privateKeyToAccount from viem throws on an invalid key — it never returns null or undefined. The if (!account) branch is unreachable. (Same pattern in the upstream across provider.)


8. http() with no RPC URL (utils.ts:107)

transport: http(),

http() with no argument uses the chain's built-in public RPC, which is rate-limited. This is fine for occasional test runs but can fail under production load. (Same pattern in across provider — pre-existing.)


9. Token identifier inconsistency between actions

bridge_and_deploy accepts token as a symbol ("USDC"); deploy_on_destination accepts token as an address ("0x833..."). This is noted in the README, but the inconsistency creates a real risk that an LLM agent passes a symbol to the standalone deploy action and hits a silent failure (the Zod regex rejects non-address strings, so at least it fails loudly).

Consider adding a note to the deploy_on_destination action description explicitly warning: "token must be the destination-chain ERC-20 address, not the symbol".


Conventions Checklist

RuleStatus
All actions return Promise<string>; errors returned not thrown
Zod v4 schemas with .describe() on every field; no .strip()
supportsNetwork is an arrow-function property
Factory bridgeDeployActionProvider exported and re-exported from index.ts
README.md present
pnpm run changeset — patch, past tense, correct package
Tests present and mocking wallet provider
super("bridgeDeploy", []) constructor call
Network.chainId compared as string

Summary: The implementation is clean and well-documented. The two blocking issues before upstream merge are the Compound supplyTo vs supply recipient bug (#2) and the in-memory state loss documentation (#1). The URL encoding (#4) and slippage arithmetic (#6) are simple one-liners to fix. The missing test coverage (#5) should be added before the PR goes out.

- Compound: use supplyTo(dst, asset, amount) so the position is credited to
the recipient instead of msg.sender (silent divergence from Morpho).
- Auto-deploy: supply the amount that actually landed when a fill comes in
under the quoted output, instead of failing the stale-quote preflight;
deploy_on_destination stays strict for explicit calls.
- getDepositStatus: build the status URL with URLSearchParams.
- Slippage check now uses bigint arithmetic to avoid float rounding.
- Document the in-memory pending-deploy state (lost on re-instantiation) in
the README and on the field; clarify token is an address (not symbol) for
deploy_on_destination.
- Add tests for the filled-without-pending-record and supply-fails-retry paths.
https://claude.ai/code/session_013rnmDYrKDMctusY33uneiy
@zaryab2000Claude

Copy link
Copy Markdown
OwnerAuthor

Thanks for the review — addressed in 8633b59. Summary of what changed and why:

Fixed

Intentionally deferred (with rationale)


Generated by Claude Code

@zaryab2000

Copy link
Copy Markdown
OwnerAuthor

@claude conduct final review and confirm if the PR is accurate now?

@claude

claudeBot commented Jun 11, 2026

Copy link
Copy Markdown

Claude finished @zaryab2000's task in 1m 38s —— View job


Final Review — bridgeDeploy ActionProvider

Tasks


All seven addressed issues are correctly fixed. Here is the per-item confirmation:

Verified fixes

#IssueStatusEvidence
#2Compound supplyTo recipient✅ FixedCOMET_SUPPLY_ABI in constants.ts now includes supplyTo(address dst, address asset, uint256 amount); utils.ts:358–362 uses supplyTo with [params.recipient, token, atomicAmount]
#3Quoted vs actual fill✅ FixeddeployToProtocol takes allowPartial = false; auto-deploy path (bridge_deploy_status) passes true; supplies balance when partial; standalone deploy_on_destination stays strict (false)
#4URL encoding✅ Fixedutils.ts:256–259 uses new URL(ACROSS_DEPOSIT_STATUS_API) + url.searchParams.set(...)
#6Bigint slippage arithmetic✅ Fixedutils.ts:183–185: ((inputAmount - outputAmount) * 10000n) / inputAmount — no float conversion
#1In-memory state documented✅ FixedJSDoc on #pendingDeploys (bridgeDeployActionProvider.ts:47–56) and a dedicated "⚠️ Pending-supply state is in-memory only" section in README.md:23–40 both clearly state the instance-local constraint
coinbase#9Token address vs symbol warning✅ Fixeddeploy_on_destination action description says "NOT the token symbol"; README.md:74–76 also distinguishes bridge_and_deploy (symbol) from deploy_on_destination (address)
#5Missing test coverage✅ Fixedtest.ts:216–236 covers filled with no pending record; test.ts:238–270 covers supply-fails-on-filled retry retention

Intentionally deferred (confirmed reasonable)

One minor observation (non-blocking)

COMET_SUPPLY_ABI in constants.ts still includes the supply(address asset, uint256 amount) entry, but the code now always calls supplyTo. The supply entry is dead code in the execution path. The comment justifies its presence for documentation purposes, and since it cannot cause a runtime error, this is harmless — just worth noting for cleanliness.

Conventions checklist

RuleStatus
All actions return Promise<string>; errors returned not thrown
Zod v4 schemas with .describe() on every field; no .strip()
supportsNetwork is an arrow-function property
Factory bridgeDeployActionProvider exported and re-exported from index.ts
README.md present and documents EOA/two-step semantics
Changeset — @coinbase/agentkit, patch, past tense
Tests present, mocked wallet, 13 provider tests
super("bridgeDeploy", []) constructor call
Network.chainId compared as string

The PR is accurate. All blocking issues from the previous review have been correctly addressed. The one remaining item (supply ABI entry being unused) is non-blocking cosmetic cleanup. The implementation is clean, well-documented, and matches AgentKit conventions.

@zaryab2000Claude

Copy link
Copy Markdown
OwnerAuthor

Final review — confirmed accurate ✅

Re-reviewed the full provider on head 8633b59 and verified all the review fixes landed correctly.

Correctness — verified in code

  • Compound recipient:deployToProtocol now encodes supplyTo(recipient, asset, amount) (utils.ts:357-362), so the position is credited to the intended owner, matching Morpho's deposit(assets, receiver). ✅
  • Partial-fill handling: auto-deploy passes allowPartial=true and supplies min(requested, balance) when a fill lands under the quote; deploy_on_destination stays strict (exact amount or error). ✅
  • URL building:getDepositStatus uses URLSearchParams. ✅
  • Slippage: computed with bigint arithmetic. ✅
  • State + token docs: in-memory #pendingDeploys warning on the field and in the README; address-not-symbol note on deploy_on_destination. ✅
  • Error-return semantics, Promise<string>, Zod .describe() (no .strip()), arrow supportsNetwork, factory re-export — all intact.

CI — green on head 8633b59

  • 25/25 checks passing (lint-typescript, format-typescript, test-agentkit-typescript (18 & 20), check-package-lock, plus the python jobs).
  • mergeable_state: clean — no conflicts with feat/bridgeThenDeploy.

Local

  • 13 provider tests + 877 full agentkit suite green; lint / format / build clean.

One honest limitation worth recording (not a blocker, by design):
The auto-deploy can only supply funds the wallet custodies. If you set recipient on bridge_and_deploy to a third party, the bridge sends there and the wallet holds nothing, so the auto-supply returns a graceful "insufficient balance" (pending kept for retry) rather than silently doing nothing. With the default (recipient = sender) the full bridge→supply flow works as intended. This is consistent and non-lossy, and the README documents that recipient defaults to the sender.

Minor / optional: the PR description is slightly stale vs the latest commits — it still says "875 tests" and "Compound (Comet supply)" (now 877 tests and supplyTo). Happy to refresh the description if you'd like; the code and changeset are accurate.

Still intentionally deferred:#7 (dead if (!account)) and coinbase#8 (http() default RPC) — verbatim parity with the in-tree across provider; will change if you prefer divergence.

Net: the PR is accurate and merge-ready into feat/bridgeThenDeploy. Reminder that upstream (coinbase/agentkit) still requires the human-only steps from our checklist — commit signing, the docs-internal//fork-tooling strip, and a rebase on the latest main — before it goes out as the public contribution.


Generated by Claude Code

The Compound path always uses supplyTo to honor the recipient, so the
supply(asset, amount) entry was dead code. Drop it from COMET_SUPPLY_ABI.
https://claude.ai/code/session_013rnmDYrKDMctusY33uneiy
@zaryab2000Claude

Copy link
Copy Markdown
OwnerAuthor

Addressed the one new observation from the final review in 5a6c1fa: removed the now-unused supply(asset, amount) entry from COMET_SUPPLY_ABI — the Compound path always calls supplyTo, so it was dead code. ABI now contains only supplyTo.

No functional change; the dead-code path is gone. Verified: 13 provider tests + full agentkit suite green, lint / format / build clean.

No other new bugs were reported in the latest review (it confirmed all 7 prior fixes as correct). PR remains accurate and merge-ready into feat/bridgeThenDeploy.


Generated by Claude Code

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

Labels

action providerdocumentationImprovements or additions to documentationtypescript

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@zaryab2000@claude
, 'i'); if (__m === '*' || __re.test(location.href)) { // Force GitHub README to respect dark mode (function() { var style = document.createElement('style'); style.textContent = ' .markdown-body { color-scheme: dark light; } .markdown-body pre { background: #161b22 !important; } .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; } .markdown-body table th, .markdown-body table td { border-color: #30363d !important; } .markdown-body img { background: #0d1117; } .markdown-body blockquote { border-left-color: #8b949e; } .markdown-body hr { border-color: #30363d; } '; document.head.appendChild(style); })(); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Add bridge-then-deploy action provider - #3

Closed
zaryab2000 wants to merge 3 commits into
feat/bridgeThenDeployfrom
claude/tender-mccarthy-ROhys
Closed

Add bridge-then-deploy action provider#3
zaryab2000 wants to merge 3 commits into
feat/bridgeThenDeployfrom
claude/tender-mccarthy-ROhys

Conversation

@zaryab2000

Copy link
Copy Markdown
Owner

Summary

Adds a new bridge-then-deploy ActionProvider (bridgeDeploy) that composes an Across bridge with a destination lending/vault supply, letting an agent express "move this capital to chain X and put it to work" as a single intent.

Central design constraint (honest by design)

Across destination-side execution only delivers to a deployed handler contract that implements handleV3AcrossMessage(...)never to a plain EOA. Agent wallets are EOAs, so this flow is deliberately two-step and non-atomic: bridge → poll status → supply on arrival. The provider never claims the supply has happened before the bridge fills.

Actions

  • bridge_and_deploy — initiates the Across deposit and records the pending destination supply; returns a depositId and status: "bridging". Does not supply yet.
  • bridge_deploy_status — polls the Across deposit-status API; once filled, auto-runs the recorded destination supply and returns the deploy tx hash. Reports pending/refunded otherwise.
  • deploy_on_destination — supplies an already-bridged token into Compound (Comet supply) or Morpho (vault deposit) on Base. Performs a balance preflight, so it is safe to retry to recover from a partial failure.

Scope

  • Destination deploy gated to Base (8453/84532); origin can be any EVM chain Across supports.
  • v1 ships Flow A (EOA, two-step) only. Flow B (handler-contract atomic deposit) is documented in the README as the upgrade path.

Conventions

  • All actions return Promise<string>; errors are returned, not thrown.
  • Zod v4 schemas with .describe() on every field; no .strip().
  • Reuses the shared approve util and mirrors the in-tree across provider's SDK wiring (privateKey constructor config, fully mocked in tests).
  • Factory re-exported from src/action-providers/index.ts.

Checks

  • pnpm test — full agentkit suite green (875 tests, incl. 11 new for this provider, mocked wallet + Across SDK).
  • pnpm run lint, pnpm run format, pnpm build — all clean.
  • Changeset added (@coinbase/agentkit, patch, past tense).

Note: per-provider README.md prominently documents the two-step / non-atomic / EOA-can't-receive-callback semantics and the partial-failure recovery path.

https://claude.ai/code/session_013rnmDYrKDMctusY33uneiy


Generated by Claude Code

Composes an Across bridge with a destination lending/vault supply so an
agent can express "move this capital to chain X and put it to work" as one
intent. Honest about the EOA constraint: Across destination execution only
reaches a handler contract, never an EOA, so the flow is two-step and
non-atomic (bridge -> poll status -> supply on arrival).
Actions:
- bridge_and_deploy: initiates the Across deposit and records the pending
destination supply; never claims the supply has happened.
- bridge_deploy_status: polls the Across deposit-status API and auto-runs
the recorded supply once the bridge is filled.
- deploy_on_destination: supplies an already-bridged token into Compound or
Morpho on Base, with a balance preflight so it is safe to retry.
Destination deploy is gated to Base; origin can be any EVM chain Across
supports. Includes unit tests (mocked wallet + Across SDK), README
documenting the non-atomic semantics, and a changeset.
https://claude.ai/code/session_013rnmDYrKDMctusY33uneiy
@github-actionsgithub-actionsBot added documentation Improvements or additions to documentation typescript action provider labels Jun 6, 2026
@zaryab2000

Copy link
Copy Markdown
OwnerAuthor

@claude start review

@claude

claudeBot commented Jun 6, 2026

Copy link
Copy Markdown

Claude finished @zaryab2000's task in 2m 59s —— View job


Code Review — bridgeDeploy ActionProvider

Tasks

  • Load tools
  • Read all changed files (bridgeDeployActionProvider.ts, utils.ts, schemas.ts, constants.ts, index.ts, test, README, changeset)
  • Analyze against AgentKit conventions (CLAUDE.md)
  • Review for bugs, security, correctness
  • Post final review

Overall the code is well-structured and closely follows AgentKit conventions. All three major checks from CLAUDE.md pass (Promise, Zod v4 with .describe(), arrow-function supportsNetwork). Below are the findings in priority order.


🔴 High — Correctness Bugs

1. In-memory #pendingDeploys is lost on provider re-instantiation

bridgeDeployActionProvider.ts:46

#pendingDeploys: Map<string,PendingDeploy>=newMap();

#pendingDeploys is instance-level state. The factory bridgeDeployActionProvider(config) creates a new instance each time it is called. In most agent patterns the provider is re-created between sessions (or even between turns). If the provider is re-instantiated while a bridge is in flight — which can take minutes to an hour — the pending deploy record is gone. bridge_deploy_status will then return the "no pending supply was recorded" fallback instead of auto-supplying.

This either needs to be documented prominently as a hard constraint ("the same provider instance must be reused across calls") or the pending state must be serialized to somewhere durable (a file, a passed-in store, etc.). At minimum the README should warn that a new instance forgets all pending deploys.

Fix this →


2. Compound supply() silently ignores recipient

utils.ts:339-344

if(params.protocol==="compound"){data=encodeFunctionData({abi: COMET_SUPPLY_ABI,functionName: "supply",args: [token,atomicAmount],// ← no recipient});}

The supply(address asset, uint amount) Comet function deposits from msg.sender into msg.sender's own position. There is no recipient argument here. If the caller passes recipient ≠ walletProvider.getAddress(), for Morpho the deposit goes to the intended recipient, but for Compound it silently goes to the caller's position instead. This is a silent semantic divergence.

Comet provides supplyTo(address dst, address asset, uint amount) for this purpose. The ABI in constants.ts and the encodeFunctionData call should use supplyTo when recipient != walletProvider.getAddress(), or the COMET_SUPPLY_ABI should be updated to include supplyTo and always use it.

Fix this →


3. Quoted outputAmount stored as supply target — mismatches actual fill

bridgeDeployActionProvider.ts:112-117

this.#pendingDeploys.set(this.#pendingKey(...),{
...
amount: deposit.outputAmount,// ← from quote, not actual fill
...
});

deposit.outputAmount is the quoted output. The actual fill amount may differ. When deployToProtocol later runs the balance preflight (balance < atomicAmount), it uses the quoted amount, which can either:

  • Block the supply if the fill was slightly less than quoted (e.g., fee changed between quote and fill)
  • Over-supply if the fill was more than quoted (e.g., the relayer undercut the fee)

The safer approach is to supply Math.min(balance, atomicAmount) — i.e., use whatever landed — or to drop the preflight comparison and let the protocol revert if underfunded.


🟡 Medium — Reliability / Security

4. depositId interpolated into URL without encoding

utils.ts:255-258

constresponse=awaitfetch(`${ACROSS_DEPOSIT_STATUS_API}?originChainId=${originChainId}&depositId=${depositId}`,

depositId (and originChainId) are user-supplied strings concatenated directly into the URL. If either contains &, =, or #, the query string is silently malformed. Use URLSearchParams:

consturl=newURL(ACROSS_DEPOSIT_STATUS_API);url.searchParams.set("originChainId",String(originChainId));url.searchParams.set("depositId",depositId);constresponse=awaitfetch(url.toString(),{method: "GET"});

Fix this →


5. Two test paths not covered

bridgeDeployActionProvider.test.ts

  • filled with no pending deploybridgeDeployStatus returns a "call deploy_on_destination manually" note when the map has no entry for the deposit. bridgeDeployActionProvider.ts:178-187 handles this but it is never exercised.
  • Supply fails on filledbridgeDeployActionProvider.ts:191-198 keeps the pending deploy for retry when deployResult.startsWith("Error"). This retry path has no test.

6. Floating-point slippage arithmetic

utils.ts:183-185

constslippageBps=((Number(formattedInput)-Number(formattedOutput))/Number(formattedInput))*10000;

Converting bigintformatUnitsNumber → arithmetic introduces floating-point rounding. For basis-point comparisons, prefer bigint arithmetic directly:

constslippageBps=Number(((quote.deposit.inputAmount-quote.deposit.outputAmount)*10000n)/quote.deposit.inputAmount);

(The existing across provider has the same issue — it is a pre-existing pattern, but worth fixing here.)


🟢 Low — Minor / Style

7. Dead validation in constructor (bridgeDeployActionProvider.ts:56-57)

constaccount=privateKeyToAccount(this.#privateKey asHex);if(!account)thrownewError("Invalid private key");

privateKeyToAccount from viem throws on an invalid key — it never returns null or undefined. The if (!account) branch is unreachable. (Same pattern in the upstream across provider.)


8. http() with no RPC URL (utils.ts:107)

transport: http(),

http() with no argument uses the chain's built-in public RPC, which is rate-limited. This is fine for occasional test runs but can fail under production load. (Same pattern in across provider — pre-existing.)


9. Token identifier inconsistency between actions

bridge_and_deploy accepts token as a symbol ("USDC"); deploy_on_destination accepts token as an address ("0x833..."). This is noted in the README, but the inconsistency creates a real risk that an LLM agent passes a symbol to the standalone deploy action and hits a silent failure (the Zod regex rejects non-address strings, so at least it fails loudly).

Consider adding a note to the deploy_on_destination action description explicitly warning: "token must be the destination-chain ERC-20 address, not the symbol".


Conventions Checklist

RuleStatus
All actions return Promise<string>; errors returned not thrown
Zod v4 schemas with .describe() on every field; no .strip()
supportsNetwork is an arrow-function property
Factory bridgeDeployActionProvider exported and re-exported from index.ts
README.md present
pnpm run changeset — patch, past tense, correct package
Tests present and mocking wallet provider
super("bridgeDeploy", []) constructor call
Network.chainId compared as string

Summary: The implementation is clean and well-documented. The two blocking issues before upstream merge are the Compound supplyTo vs supply recipient bug (#2) and the in-memory state loss documentation (#1). The URL encoding (#4) and slippage arithmetic (#6) are simple one-liners to fix. The missing test coverage (#5) should be added before the PR goes out.

- Compound: use supplyTo(dst, asset, amount) so the position is credited to
the recipient instead of msg.sender (silent divergence from Morpho).
- Auto-deploy: supply the amount that actually landed when a fill comes in
under the quoted output, instead of failing the stale-quote preflight;
deploy_on_destination stays strict for explicit calls.
- getDepositStatus: build the status URL with URLSearchParams.
- Slippage check now uses bigint arithmetic to avoid float rounding.
- Document the in-memory pending-deploy state (lost on re-instantiation) in
the README and on the field; clarify token is an address (not symbol) for
deploy_on_destination.
- Add tests for the filled-without-pending-record and supply-fails-retry paths.
https://claude.ai/code/session_013rnmDYrKDMctusY33uneiy
@zaryab2000Claude

Copy link
Copy Markdown
OwnerAuthor

Thanks for the review — addressed in 8633b59. Summary of what changed and why:

Fixed

Intentionally deferred (with rationale)


Generated by Claude Code

@zaryab2000

Copy link
Copy Markdown
OwnerAuthor

@claude conduct final review and confirm if the PR is accurate now?

@claude

claudeBot commented Jun 11, 2026

Copy link
Copy Markdown

Claude finished @zaryab2000's task in 1m 38s —— View job


Final Review — bridgeDeploy ActionProvider

Tasks


All seven addressed issues are correctly fixed. Here is the per-item confirmation:

Verified fixes

#IssueStatusEvidence
#2Compound supplyTo recipient✅ FixedCOMET_SUPPLY_ABI in constants.ts now includes supplyTo(address dst, address asset, uint256 amount); utils.ts:358–362 uses supplyTo with [params.recipient, token, atomicAmount]
#3Quoted vs actual fill✅ FixeddeployToProtocol takes allowPartial = false; auto-deploy path (bridge_deploy_status) passes true; supplies balance when partial; standalone deploy_on_destination stays strict (false)
#4URL encoding✅ Fixedutils.ts:256–259 uses new URL(ACROSS_DEPOSIT_STATUS_API) + url.searchParams.set(...)
#6Bigint slippage arithmetic✅ Fixedutils.ts:183–185: ((inputAmount - outputAmount) * 10000n) / inputAmount — no float conversion
#1In-memory state documented✅ FixedJSDoc on #pendingDeploys (bridgeDeployActionProvider.ts:47–56) and a dedicated "⚠️ Pending-supply state is in-memory only" section in README.md:23–40 both clearly state the instance-local constraint
coinbase#9Token address vs symbol warning✅ Fixeddeploy_on_destination action description says "NOT the token symbol"; README.md:74–76 also distinguishes bridge_and_deploy (symbol) from deploy_on_destination (address)
#5Missing test coverage✅ Fixedtest.ts:216–236 covers filled with no pending record; test.ts:238–270 covers supply-fails-on-filled retry retention

Intentionally deferred (confirmed reasonable)

One minor observation (non-blocking)

COMET_SUPPLY_ABI in constants.ts still includes the supply(address asset, uint256 amount) entry, but the code now always calls supplyTo. The supply entry is dead code in the execution path. The comment justifies its presence for documentation purposes, and since it cannot cause a runtime error, this is harmless — just worth noting for cleanliness.

Conventions checklist

RuleStatus
All actions return Promise<string>; errors returned not thrown
Zod v4 schemas with .describe() on every field; no .strip()
supportsNetwork is an arrow-function property
Factory bridgeDeployActionProvider exported and re-exported from index.ts
README.md present and documents EOA/two-step semantics
Changeset — @coinbase/agentkit, patch, past tense
Tests present, mocked wallet, 13 provider tests
super("bridgeDeploy", []) constructor call
Network.chainId compared as string

The PR is accurate. All blocking issues from the previous review have been correctly addressed. The one remaining item (supply ABI entry being unused) is non-blocking cosmetic cleanup. The implementation is clean, well-documented, and matches AgentKit conventions.

@zaryab2000Claude

Copy link
Copy Markdown
OwnerAuthor

Final review — confirmed accurate ✅

Re-reviewed the full provider on head 8633b59 and verified all the review fixes landed correctly.

Correctness — verified in code

  • Compound recipient:deployToProtocol now encodes supplyTo(recipient, asset, amount) (utils.ts:357-362), so the position is credited to the intended owner, matching Morpho's deposit(assets, receiver). ✅
  • Partial-fill handling: auto-deploy passes allowPartial=true and supplies min(requested, balance) when a fill lands under the quote; deploy_on_destination stays strict (exact amount or error). ✅
  • URL building:getDepositStatus uses URLSearchParams. ✅
  • Slippage: computed with bigint arithmetic. ✅
  • State + token docs: in-memory #pendingDeploys warning on the field and in the README; address-not-symbol note on deploy_on_destination. ✅
  • Error-return semantics, Promise<string>, Zod .describe() (no .strip()), arrow supportsNetwork, factory re-export — all intact.

CI — green on head 8633b59

  • 25/25 checks passing (lint-typescript, format-typescript, test-agentkit-typescript (18 & 20), check-package-lock, plus the python jobs).
  • mergeable_state: clean — no conflicts with feat/bridgeThenDeploy.

Local

  • 13 provider tests + 877 full agentkit suite green; lint / format / build clean.

One honest limitation worth recording (not a blocker, by design):
The auto-deploy can only supply funds the wallet custodies. If you set recipient on bridge_and_deploy to a third party, the bridge sends there and the wallet holds nothing, so the auto-supply returns a graceful "insufficient balance" (pending kept for retry) rather than silently doing nothing. With the default (recipient = sender) the full bridge→supply flow works as intended. This is consistent and non-lossy, and the README documents that recipient defaults to the sender.

Minor / optional: the PR description is slightly stale vs the latest commits — it still says "875 tests" and "Compound (Comet supply)" (now 877 tests and supplyTo). Happy to refresh the description if you'd like; the code and changeset are accurate.

Still intentionally deferred:#7 (dead if (!account)) and coinbase#8 (http() default RPC) — verbatim parity with the in-tree across provider; will change if you prefer divergence.

Net: the PR is accurate and merge-ready into feat/bridgeThenDeploy. Reminder that upstream (coinbase/agentkit) still requires the human-only steps from our checklist — commit signing, the docs-internal//fork-tooling strip, and a rebase on the latest main — before it goes out as the public contribution.


Generated by Claude Code

The Compound path always uses supplyTo to honor the recipient, so the
supply(asset, amount) entry was dead code. Drop it from COMET_SUPPLY_ABI.
https://claude.ai/code/session_013rnmDYrKDMctusY33uneiy
@zaryab2000Claude

Copy link
Copy Markdown
OwnerAuthor

Addressed the one new observation from the final review in 5a6c1fa: removed the now-unused supply(asset, amount) entry from COMET_SUPPLY_ABI — the Compound path always calls supplyTo, so it was dead code. ABI now contains only supplyTo.

No functional change; the dead-code path is gone. Verified: 13 provider tests + full agentkit suite green, lint / format / build clean.

No other new bugs were reported in the latest review (it confirmed all 7 prior fixes as correct). PR remains accurate and merge-ready into feat/bridgeThenDeploy.


Generated by Claude Code

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

Labels

action providerdocumentationImprovements or additions to documentationtypescript

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@zaryab2000@claude
, 'i'); if (__m === '*' || __re.test(location.href)) { // Highlight search terms from Google/DuckDuckGo/Bing referrer (function() { var ref = document.referrer; var terms = []; if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) { var url = new URL(ref); var q = url.searchParams.get('q') || url.searchParams.get('p'); if (q) { terms = q.split(/\s+/).filter(function(t) { return t.length > 2; }); } } if (terms.length === 0) return; var style = document.createElement('style'); style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }'; document.head.appendChild(style); function highlight(node) { if (node.nodeType === 3) { // text node var text = node.textContent; var found = false; terms.forEach(function(term) { var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\]\\]/g, '\\') + ')', 'gi'); if (regex.test(text)) { found = true; var frag = document.createDocumentFragment(); var parts = text.split(regex); parts.forEach(function(part, i) { if (i % 2 === 0) { frag.appendChild(document.createTextNode(part)); } else { var span = document.createElement('span'); span.className = 'userscript-highlight'; span.textContent = part; frag.appendChild(span); } }); node.parentNode.replaceChild(frag, node); } }); } else if (node.nodeType === 1 && node.childNodes) { // element var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT']; if (!skipTags.includes(node.tagName)) { Array.from(node.childNodes).forEach(highlight); } } } highlight(document.body); // Re-highlight on dynamic content var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1 || node.nodeType === 3) highlight(node); }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Add bridge-then-deploy action provider - #3

Closed
zaryab2000 wants to merge 3 commits into
feat/bridgeThenDeployfrom
claude/tender-mccarthy-ROhys
Closed

Add bridge-then-deploy action provider#3
zaryab2000 wants to merge 3 commits into
feat/bridgeThenDeployfrom
claude/tender-mccarthy-ROhys

Conversation

@zaryab2000

Copy link
Copy Markdown
Owner

Summary

Adds a new bridge-then-deploy ActionProvider (bridgeDeploy) that composes an Across bridge with a destination lending/vault supply, letting an agent express "move this capital to chain X and put it to work" as a single intent.

Central design constraint (honest by design)

Across destination-side execution only delivers to a deployed handler contract that implements handleV3AcrossMessage(...)never to a plain EOA. Agent wallets are EOAs, so this flow is deliberately two-step and non-atomic: bridge → poll status → supply on arrival. The provider never claims the supply has happened before the bridge fills.

Actions

  • bridge_and_deploy — initiates the Across deposit and records the pending destination supply; returns a depositId and status: "bridging". Does not supply yet.
  • bridge_deploy_status — polls the Across deposit-status API; once filled, auto-runs the recorded destination supply and returns the deploy tx hash. Reports pending/refunded otherwise.
  • deploy_on_destination — supplies an already-bridged token into Compound (Comet supply) or Morpho (vault deposit) on Base. Performs a balance preflight, so it is safe to retry to recover from a partial failure.

Scope

  • Destination deploy gated to Base (8453/84532); origin can be any EVM chain Across supports.
  • v1 ships Flow A (EOA, two-step) only. Flow B (handler-contract atomic deposit) is documented in the README as the upgrade path.

Conventions

  • All actions return Promise<string>; errors are returned, not thrown.
  • Zod v4 schemas with .describe() on every field; no .strip().
  • Reuses the shared approve util and mirrors the in-tree across provider's SDK wiring (privateKey constructor config, fully mocked in tests).
  • Factory re-exported from src/action-providers/index.ts.

Checks

  • pnpm test — full agentkit suite green (875 tests, incl. 11 new for this provider, mocked wallet + Across SDK).
  • pnpm run lint, pnpm run format, pnpm build — all clean.
  • Changeset added (@coinbase/agentkit, patch, past tense).

Note: per-provider README.md prominently documents the two-step / non-atomic / EOA-can't-receive-callback semantics and the partial-failure recovery path.

https://claude.ai/code/session_013rnmDYrKDMctusY33uneiy


Generated by Claude Code

Composes an Across bridge with a destination lending/vault supply so an
agent can express "move this capital to chain X and put it to work" as one
intent. Honest about the EOA constraint: Across destination execution only
reaches a handler contract, never an EOA, so the flow is two-step and
non-atomic (bridge -> poll status -> supply on arrival).
Actions:
- bridge_and_deploy: initiates the Across deposit and records the pending
destination supply; never claims the supply has happened.
- bridge_deploy_status: polls the Across deposit-status API and auto-runs
the recorded supply once the bridge is filled.
- deploy_on_destination: supplies an already-bridged token into Compound or
Morpho on Base, with a balance preflight so it is safe to retry.
Destination deploy is gated to Base; origin can be any EVM chain Across
supports. Includes unit tests (mocked wallet + Across SDK), README
documenting the non-atomic semantics, and a changeset.
https://claude.ai/code/session_013rnmDYrKDMctusY33uneiy
@github-actionsgithub-actionsBot added documentation Improvements or additions to documentation typescript action provider labels Jun 6, 2026
@zaryab2000

Copy link
Copy Markdown
OwnerAuthor

@claude start review

@claude

claudeBot commented Jun 6, 2026

Copy link
Copy Markdown

Claude finished @zaryab2000's task in 2m 59s —— View job


Code Review — bridgeDeploy ActionProvider

Tasks

  • Load tools
  • Read all changed files (bridgeDeployActionProvider.ts, utils.ts, schemas.ts, constants.ts, index.ts, test, README, changeset)
  • Analyze against AgentKit conventions (CLAUDE.md)
  • Review for bugs, security, correctness
  • Post final review

Overall the code is well-structured and closely follows AgentKit conventions. All three major checks from CLAUDE.md pass (Promise, Zod v4 with .describe(), arrow-function supportsNetwork). Below are the findings in priority order.


🔴 High — Correctness Bugs

1. In-memory #pendingDeploys is lost on provider re-instantiation

bridgeDeployActionProvider.ts:46

#pendingDeploys: Map<string,PendingDeploy>=newMap();

#pendingDeploys is instance-level state. The factory bridgeDeployActionProvider(config) creates a new instance each time it is called. In most agent patterns the provider is re-created between sessions (or even between turns). If the provider is re-instantiated while a bridge is in flight — which can take minutes to an hour — the pending deploy record is gone. bridge_deploy_status will then return the "no pending supply was recorded" fallback instead of auto-supplying.

This either needs to be documented prominently as a hard constraint ("the same provider instance must be reused across calls") or the pending state must be serialized to somewhere durable (a file, a passed-in store, etc.). At minimum the README should warn that a new instance forgets all pending deploys.

Fix this →


2. Compound supply() silently ignores recipient

utils.ts:339-344

if(params.protocol==="compound"){data=encodeFunctionData({abi: COMET_SUPPLY_ABI,functionName: "supply",args: [token,atomicAmount],// ← no recipient});}

The supply(address asset, uint amount) Comet function deposits from msg.sender into msg.sender's own position. There is no recipient argument here. If the caller passes recipient ≠ walletProvider.getAddress(), for Morpho the deposit goes to the intended recipient, but for Compound it silently goes to the caller's position instead. This is a silent semantic divergence.

Comet provides supplyTo(address dst, address asset, uint amount) for this purpose. The ABI in constants.ts and the encodeFunctionData call should use supplyTo when recipient != walletProvider.getAddress(), or the COMET_SUPPLY_ABI should be updated to include supplyTo and always use it.

Fix this →


3. Quoted outputAmount stored as supply target — mismatches actual fill

bridgeDeployActionProvider.ts:112-117

this.#pendingDeploys.set(this.#pendingKey(...),{
...
amount: deposit.outputAmount,// ← from quote, not actual fill
...
});

deposit.outputAmount is the quoted output. The actual fill amount may differ. When deployToProtocol later runs the balance preflight (balance < atomicAmount), it uses the quoted amount, which can either:

  • Block the supply if the fill was slightly less than quoted (e.g., fee changed between quote and fill)
  • Over-supply if the fill was more than quoted (e.g., the relayer undercut the fee)

The safer approach is to supply Math.min(balance, atomicAmount) — i.e., use whatever landed — or to drop the preflight comparison and let the protocol revert if underfunded.


🟡 Medium — Reliability / Security

4. depositId interpolated into URL without encoding

utils.ts:255-258

constresponse=awaitfetch(`${ACROSS_DEPOSIT_STATUS_API}?originChainId=${originChainId}&depositId=${depositId}`,

depositId (and originChainId) are user-supplied strings concatenated directly into the URL. If either contains &, =, or #, the query string is silently malformed. Use URLSearchParams:

consturl=newURL(ACROSS_DEPOSIT_STATUS_API);url.searchParams.set("originChainId",String(originChainId));url.searchParams.set("depositId",depositId);constresponse=awaitfetch(url.toString(),{method: "GET"});

Fix this →


5. Two test paths not covered

bridgeDeployActionProvider.test.ts

  • filled with no pending deploybridgeDeployStatus returns a "call deploy_on_destination manually" note when the map has no entry for the deposit. bridgeDeployActionProvider.ts:178-187 handles this but it is never exercised.
  • Supply fails on filledbridgeDeployActionProvider.ts:191-198 keeps the pending deploy for retry when deployResult.startsWith("Error"). This retry path has no test.

6. Floating-point slippage arithmetic

utils.ts:183-185

constslippageBps=((Number(formattedInput)-Number(formattedOutput))/Number(formattedInput))*10000;

Converting bigintformatUnitsNumber → arithmetic introduces floating-point rounding. For basis-point comparisons, prefer bigint arithmetic directly:

constslippageBps=Number(((quote.deposit.inputAmount-quote.deposit.outputAmount)*10000n)/quote.deposit.inputAmount);

(The existing across provider has the same issue — it is a pre-existing pattern, but worth fixing here.)


🟢 Low — Minor / Style

7. Dead validation in constructor (bridgeDeployActionProvider.ts:56-57)

constaccount=privateKeyToAccount(this.#privateKey asHex);if(!account)thrownewError("Invalid private key");

privateKeyToAccount from viem throws on an invalid key — it never returns null or undefined. The if (!account) branch is unreachable. (Same pattern in the upstream across provider.)


8. http() with no RPC URL (utils.ts:107)

transport: http(),

http() with no argument uses the chain's built-in public RPC, which is rate-limited. This is fine for occasional test runs but can fail under production load. (Same pattern in across provider — pre-existing.)


9. Token identifier inconsistency between actions

bridge_and_deploy accepts token as a symbol ("USDC"); deploy_on_destination accepts token as an address ("0x833..."). This is noted in the README, but the inconsistency creates a real risk that an LLM agent passes a symbol to the standalone deploy action and hits a silent failure (the Zod regex rejects non-address strings, so at least it fails loudly).

Consider adding a note to the deploy_on_destination action description explicitly warning: "token must be the destination-chain ERC-20 address, not the symbol".


Conventions Checklist

RuleStatus
All actions return Promise<string>; errors returned not thrown
Zod v4 schemas with .describe() on every field; no .strip()
supportsNetwork is an arrow-function property
Factory bridgeDeployActionProvider exported and re-exported from index.ts
README.md present
pnpm run changeset — patch, past tense, correct package
Tests present and mocking wallet provider
super("bridgeDeploy", []) constructor call
Network.chainId compared as string

Summary: The implementation is clean and well-documented. The two blocking issues before upstream merge are the Compound supplyTo vs supply recipient bug (#2) and the in-memory state loss documentation (#1). The URL encoding (#4) and slippage arithmetic (#6) are simple one-liners to fix. The missing test coverage (#5) should be added before the PR goes out.

- Compound: use supplyTo(dst, asset, amount) so the position is credited to
the recipient instead of msg.sender (silent divergence from Morpho).
- Auto-deploy: supply the amount that actually landed when a fill comes in
under the quoted output, instead of failing the stale-quote preflight;
deploy_on_destination stays strict for explicit calls.
- getDepositStatus: build the status URL with URLSearchParams.
- Slippage check now uses bigint arithmetic to avoid float rounding.
- Document the in-memory pending-deploy state (lost on re-instantiation) in
the README and on the field; clarify token is an address (not symbol) for
deploy_on_destination.
- Add tests for the filled-without-pending-record and supply-fails-retry paths.
https://claude.ai/code/session_013rnmDYrKDMctusY33uneiy
@zaryab2000Claude

Copy link
Copy Markdown
OwnerAuthor

Thanks for the review — addressed in 8633b59. Summary of what changed and why:

Fixed

Intentionally deferred (with rationale)


Generated by Claude Code

@zaryab2000

Copy link
Copy Markdown
OwnerAuthor

@claude conduct final review and confirm if the PR is accurate now?

@claude

claudeBot commented Jun 11, 2026

Copy link
Copy Markdown

Claude finished @zaryab2000's task in 1m 38s —— View job


Final Review — bridgeDeploy ActionProvider

Tasks


All seven addressed issues are correctly fixed. Here is the per-item confirmation:

Verified fixes

#IssueStatusEvidence
#2Compound supplyTo recipient✅ FixedCOMET_SUPPLY_ABI in constants.ts now includes supplyTo(address dst, address asset, uint256 amount); utils.ts:358–362 uses supplyTo with [params.recipient, token, atomicAmount]
#3Quoted vs actual fill✅ FixeddeployToProtocol takes allowPartial = false; auto-deploy path (bridge_deploy_status) passes true; supplies balance when partial; standalone deploy_on_destination stays strict (false)
#4URL encoding✅ Fixedutils.ts:256–259 uses new URL(ACROSS_DEPOSIT_STATUS_API) + url.searchParams.set(...)
#6Bigint slippage arithmetic✅ Fixedutils.ts:183–185: ((inputAmount - outputAmount) * 10000n) / inputAmount — no float conversion
#1In-memory state documented✅ FixedJSDoc on #pendingDeploys (bridgeDeployActionProvider.ts:47–56) and a dedicated "⚠️ Pending-supply state is in-memory only" section in README.md:23–40 both clearly state the instance-local constraint
coinbase#9Token address vs symbol warning✅ Fixeddeploy_on_destination action description says "NOT the token symbol"; README.md:74–76 also distinguishes bridge_and_deploy (symbol) from deploy_on_destination (address)
#5Missing test coverage✅ Fixedtest.ts:216–236 covers filled with no pending record; test.ts:238–270 covers supply-fails-on-filled retry retention

Intentionally deferred (confirmed reasonable)

One minor observation (non-blocking)

COMET_SUPPLY_ABI in constants.ts still includes the supply(address asset, uint256 amount) entry, but the code now always calls supplyTo. The supply entry is dead code in the execution path. The comment justifies its presence for documentation purposes, and since it cannot cause a runtime error, this is harmless — just worth noting for cleanliness.

Conventions checklist

RuleStatus
All actions return Promise<string>; errors returned not thrown
Zod v4 schemas with .describe() on every field; no .strip()
supportsNetwork is an arrow-function property
Factory bridgeDeployActionProvider exported and re-exported from index.ts
README.md present and documents EOA/two-step semantics
Changeset — @coinbase/agentkit, patch, past tense
Tests present, mocked wallet, 13 provider tests
super("bridgeDeploy", []) constructor call
Network.chainId compared as string

The PR is accurate. All blocking issues from the previous review have been correctly addressed. The one remaining item (supply ABI entry being unused) is non-blocking cosmetic cleanup. The implementation is clean, well-documented, and matches AgentKit conventions.

@zaryab2000Claude

Copy link
Copy Markdown
OwnerAuthor

Final review — confirmed accurate ✅

Re-reviewed the full provider on head 8633b59 and verified all the review fixes landed correctly.

Correctness — verified in code

  • Compound recipient:deployToProtocol now encodes supplyTo(recipient, asset, amount) (utils.ts:357-362), so the position is credited to the intended owner, matching Morpho's deposit(assets, receiver). ✅
  • Partial-fill handling: auto-deploy passes allowPartial=true and supplies min(requested, balance) when a fill lands under the quote; deploy_on_destination stays strict (exact amount or error). ✅
  • URL building:getDepositStatus uses URLSearchParams. ✅
  • Slippage: computed with bigint arithmetic. ✅
  • State + token docs: in-memory #pendingDeploys warning on the field and in the README; address-not-symbol note on deploy_on_destination. ✅
  • Error-return semantics, Promise<string>, Zod .describe() (no .strip()), arrow supportsNetwork, factory re-export — all intact.

CI — green on head 8633b59

  • 25/25 checks passing (lint-typescript, format-typescript, test-agentkit-typescript (18 & 20), check-package-lock, plus the python jobs).
  • mergeable_state: clean — no conflicts with feat/bridgeThenDeploy.

Local

  • 13 provider tests + 877 full agentkit suite green; lint / format / build clean.

One honest limitation worth recording (not a blocker, by design):
The auto-deploy can only supply funds the wallet custodies. If you set recipient on bridge_and_deploy to a third party, the bridge sends there and the wallet holds nothing, so the auto-supply returns a graceful "insufficient balance" (pending kept for retry) rather than silently doing nothing. With the default (recipient = sender) the full bridge→supply flow works as intended. This is consistent and non-lossy, and the README documents that recipient defaults to the sender.

Minor / optional: the PR description is slightly stale vs the latest commits — it still says "875 tests" and "Compound (Comet supply)" (now 877 tests and supplyTo). Happy to refresh the description if you'd like; the code and changeset are accurate.

Still intentionally deferred:#7 (dead if (!account)) and coinbase#8 (http() default RPC) — verbatim parity with the in-tree across provider; will change if you prefer divergence.

Net: the PR is accurate and merge-ready into feat/bridgeThenDeploy. Reminder that upstream (coinbase/agentkit) still requires the human-only steps from our checklist — commit signing, the docs-internal//fork-tooling strip, and a rebase on the latest main — before it goes out as the public contribution.


Generated by Claude Code

The Compound path always uses supplyTo to honor the recipient, so the
supply(asset, amount) entry was dead code. Drop it from COMET_SUPPLY_ABI.
https://claude.ai/code/session_013rnmDYrKDMctusY33uneiy
@zaryab2000Claude

Copy link
Copy Markdown
OwnerAuthor

Addressed the one new observation from the final review in 5a6c1fa: removed the now-unused supply(asset, amount) entry from COMET_SUPPLY_ABI — the Compound path always calls supplyTo, so it was dead code. ABI now contains only supplyTo.

No functional change; the dead-code path is gone. Verified: 13 provider tests + full agentkit suite green, lint / format / build clean.

No other new bugs were reported in the latest review (it confirmed all 7 prior fixes as correct). PR remains accurate and merge-ready into feat/bridgeThenDeploy.


Generated by Claude Code

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

Labels

action providerdocumentationImprovements or additions to documentationtypescript

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@zaryab2000@claude
, 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

Add bridge-then-deploy action provider - #3

Closed
zaryab2000 wants to merge 3 commits into
feat/bridgeThenDeployfrom
claude/tender-mccarthy-ROhys
Closed

Add bridge-then-deploy action provider#3
zaryab2000 wants to merge 3 commits into
feat/bridgeThenDeployfrom
claude/tender-mccarthy-ROhys

Conversation

@zaryab2000

Copy link
Copy Markdown
Owner

Summary

Adds a new bridge-then-deploy ActionProvider (bridgeDeploy) that composes an Across bridge with a destination lending/vault supply, letting an agent express "move this capital to chain X and put it to work" as a single intent.

Central design constraint (honest by design)

Across destination-side execution only delivers to a deployed handler contract that implements handleV3AcrossMessage(...)never to a plain EOA. Agent wallets are EOAs, so this flow is deliberately two-step and non-atomic: bridge → poll status → supply on arrival. The provider never claims the supply has happened before the bridge fills.

Actions

  • bridge_and_deploy — initiates the Across deposit and records the pending destination supply; returns a depositId and status: "bridging". Does not supply yet.
  • bridge_deploy_status — polls the Across deposit-status API; once filled, auto-runs the recorded destination supply and returns the deploy tx hash. Reports pending/refunded otherwise.
  • deploy_on_destination — supplies an already-bridged token into Compound (Comet supply) or Morpho (vault deposit) on Base. Performs a balance preflight, so it is safe to retry to recover from a partial failure.

Scope

  • Destination deploy gated to Base (8453/84532); origin can be any EVM chain Across supports.
  • v1 ships Flow A (EOA, two-step) only. Flow B (handler-contract atomic deposit) is documented in the README as the upgrade path.

Conventions

  • All actions return Promise<string>; errors are returned, not thrown.
  • Zod v4 schemas with .describe() on every field; no .strip().
  • Reuses the shared approve util and mirrors the in-tree across provider's SDK wiring (privateKey constructor config, fully mocked in tests).
  • Factory re-exported from src/action-providers/index.ts.

Checks

  • pnpm test — full agentkit suite green (875 tests, incl. 11 new for this provider, mocked wallet + Across SDK).
  • pnpm run lint, pnpm run format, pnpm build — all clean.
  • Changeset added (@coinbase/agentkit, patch, past tense).

Note: per-provider README.md prominently documents the two-step / non-atomic / EOA-can't-receive-callback semantics and the partial-failure recovery path.

https://claude.ai/code/session_013rnmDYrKDMctusY33uneiy


Generated by Claude Code

Composes an Across bridge with a destination lending/vault supply so an
agent can express "move this capital to chain X and put it to work" as one
intent. Honest about the EOA constraint: Across destination execution only
reaches a handler contract, never an EOA, so the flow is two-step and
non-atomic (bridge -> poll status -> supply on arrival).
Actions:
- bridge_and_deploy: initiates the Across deposit and records the pending
destination supply; never claims the supply has happened.
- bridge_deploy_status: polls the Across deposit-status API and auto-runs
the recorded supply once the bridge is filled.
- deploy_on_destination: supplies an already-bridged token into Compound or
Morpho on Base, with a balance preflight so it is safe to retry.
Destination deploy is gated to Base; origin can be any EVM chain Across
supports. Includes unit tests (mocked wallet + Across SDK), README
documenting the non-atomic semantics, and a changeset.
https://claude.ai/code/session_013rnmDYrKDMctusY33uneiy
@github-actionsgithub-actionsBot added documentation Improvements or additions to documentation typescript action provider labels Jun 6, 2026
@zaryab2000

Copy link
Copy Markdown
OwnerAuthor

@claude start review

@claude

claudeBot commented Jun 6, 2026

Copy link
Copy Markdown

Claude finished @zaryab2000's task in 2m 59s —— View job


Code Review — bridgeDeploy ActionProvider

Tasks

  • Load tools
  • Read all changed files (bridgeDeployActionProvider.ts, utils.ts, schemas.ts, constants.ts, index.ts, test, README, changeset)
  • Analyze against AgentKit conventions (CLAUDE.md)
  • Review for bugs, security, correctness
  • Post final review

Overall the code is well-structured and closely follows AgentKit conventions. All three major checks from CLAUDE.md pass (Promise, Zod v4 with .describe(), arrow-function supportsNetwork). Below are the findings in priority order.


🔴 High — Correctness Bugs

1. In-memory #pendingDeploys is lost on provider re-instantiation

bridgeDeployActionProvider.ts:46

#pendingDeploys: Map<string,PendingDeploy>=newMap();

#pendingDeploys is instance-level state. The factory bridgeDeployActionProvider(config) creates a new instance each time it is called. In most agent patterns the provider is re-created between sessions (or even between turns). If the provider is re-instantiated while a bridge is in flight — which can take minutes to an hour — the pending deploy record is gone. bridge_deploy_status will then return the "no pending supply was recorded" fallback instead of auto-supplying.

This either needs to be documented prominently as a hard constraint ("the same provider instance must be reused across calls") or the pending state must be serialized to somewhere durable (a file, a passed-in store, etc.). At minimum the README should warn that a new instance forgets all pending deploys.

Fix this →


2. Compound supply() silently ignores recipient

utils.ts:339-344

if(params.protocol==="compound"){data=encodeFunctionData({abi: COMET_SUPPLY_ABI,functionName: "supply",args: [token,atomicAmount],// ← no recipient});}

The supply(address asset, uint amount) Comet function deposits from msg.sender into msg.sender's own position. There is no recipient argument here. If the caller passes recipient ≠ walletProvider.getAddress(), for Morpho the deposit goes to the intended recipient, but for Compound it silently goes to the caller's position instead. This is a silent semantic divergence.

Comet provides supplyTo(address dst, address asset, uint amount) for this purpose. The ABI in constants.ts and the encodeFunctionData call should use supplyTo when recipient != walletProvider.getAddress(), or the COMET_SUPPLY_ABI should be updated to include supplyTo and always use it.

Fix this →


3. Quoted outputAmount stored as supply target — mismatches actual fill

bridgeDeployActionProvider.ts:112-117

this.#pendingDeploys.set(this.#pendingKey(...),{
...
amount: deposit.outputAmount,// ← from quote, not actual fill
...
});

deposit.outputAmount is the quoted output. The actual fill amount may differ. When deployToProtocol later runs the balance preflight (balance < atomicAmount), it uses the quoted amount, which can either:

  • Block the supply if the fill was slightly less than quoted (e.g., fee changed between quote and fill)
  • Over-supply if the fill was more than quoted (e.g., the relayer undercut the fee)

The safer approach is to supply Math.min(balance, atomicAmount) — i.e., use whatever landed — or to drop the preflight comparison and let the protocol revert if underfunded.


🟡 Medium — Reliability / Security

4. depositId interpolated into URL without encoding

utils.ts:255-258

constresponse=awaitfetch(`${ACROSS_DEPOSIT_STATUS_API}?originChainId=${originChainId}&depositId=${depositId}`,

depositId (and originChainId) are user-supplied strings concatenated directly into the URL. If either contains &, =, or #, the query string is silently malformed. Use URLSearchParams:

consturl=newURL(ACROSS_DEPOSIT_STATUS_API);url.searchParams.set("originChainId",String(originChainId));url.searchParams.set("depositId",depositId);constresponse=awaitfetch(url.toString(),{method: "GET"});

Fix this →


5. Two test paths not covered

bridgeDeployActionProvider.test.ts

  • filled with no pending deploybridgeDeployStatus returns a "call deploy_on_destination manually" note when the map has no entry for the deposit. bridgeDeployActionProvider.ts:178-187 handles this but it is never exercised.
  • Supply fails on filledbridgeDeployActionProvider.ts:191-198 keeps the pending deploy for retry when deployResult.startsWith("Error"). This retry path has no test.

6. Floating-point slippage arithmetic

utils.ts:183-185

constslippageBps=((Number(formattedInput)-Number(formattedOutput))/Number(formattedInput))*10000;

Converting bigintformatUnitsNumber → arithmetic introduces floating-point rounding. For basis-point comparisons, prefer bigint arithmetic directly:

constslippageBps=Number(((quote.deposit.inputAmount-quote.deposit.outputAmount)*10000n)/quote.deposit.inputAmount);

(The existing across provider has the same issue — it is a pre-existing pattern, but worth fixing here.)


🟢 Low — Minor / Style

7. Dead validation in constructor (bridgeDeployActionProvider.ts:56-57)

constaccount=privateKeyToAccount(this.#privateKey asHex);if(!account)thrownewError("Invalid private key");

privateKeyToAccount from viem throws on an invalid key — it never returns null or undefined. The if (!account) branch is unreachable. (Same pattern in the upstream across provider.)


8. http() with no RPC URL (utils.ts:107)

transport: http(),

http() with no argument uses the chain's built-in public RPC, which is rate-limited. This is fine for occasional test runs but can fail under production load. (Same pattern in across provider — pre-existing.)


9. Token identifier inconsistency between actions

bridge_and_deploy accepts token as a symbol ("USDC"); deploy_on_destination accepts token as an address ("0x833..."). This is noted in the README, but the inconsistency creates a real risk that an LLM agent passes a symbol to the standalone deploy action and hits a silent failure (the Zod regex rejects non-address strings, so at least it fails loudly).

Consider adding a note to the deploy_on_destination action description explicitly warning: "token must be the destination-chain ERC-20 address, not the symbol".


Conventions Checklist

RuleStatus
All actions return Promise<string>; errors returned not thrown
Zod v4 schemas with .describe() on every field; no .strip()
supportsNetwork is an arrow-function property
Factory bridgeDeployActionProvider exported and re-exported from index.ts
README.md present
pnpm run changeset — patch, past tense, correct package
Tests present and mocking wallet provider
super("bridgeDeploy", []) constructor call
Network.chainId compared as string

Summary: The implementation is clean and well-documented. The two blocking issues before upstream merge are the Compound supplyTo vs supply recipient bug (#2) and the in-memory state loss documentation (#1). The URL encoding (#4) and slippage arithmetic (#6) are simple one-liners to fix. The missing test coverage (#5) should be added before the PR goes out.

- Compound: use supplyTo(dst, asset, amount) so the position is credited to
the recipient instead of msg.sender (silent divergence from Morpho).
- Auto-deploy: supply the amount that actually landed when a fill comes in
under the quoted output, instead of failing the stale-quote preflight;
deploy_on_destination stays strict for explicit calls.
- getDepositStatus: build the status URL with URLSearchParams.
- Slippage check now uses bigint arithmetic to avoid float rounding.
- Document the in-memory pending-deploy state (lost on re-instantiation) in
the README and on the field; clarify token is an address (not symbol) for
deploy_on_destination.
- Add tests for the filled-without-pending-record and supply-fails-retry paths.
https://claude.ai/code/session_013rnmDYrKDMctusY33uneiy
@zaryab2000Claude

Copy link
Copy Markdown
OwnerAuthor

Thanks for the review — addressed in 8633b59. Summary of what changed and why:

Fixed

Intentionally deferred (with rationale)


Generated by Claude Code

@zaryab2000

Copy link
Copy Markdown
OwnerAuthor

@claude conduct final review and confirm if the PR is accurate now?

@claude

claudeBot commented Jun 11, 2026

Copy link
Copy Markdown

Claude finished @zaryab2000's task in 1m 38s —— View job


Final Review — bridgeDeploy ActionProvider

Tasks


All seven addressed issues are correctly fixed. Here is the per-item confirmation:

Verified fixes

#IssueStatusEvidence
#2Compound supplyTo recipient✅ FixedCOMET_SUPPLY_ABI in constants.ts now includes supplyTo(address dst, address asset, uint256 amount); utils.ts:358–362 uses supplyTo with [params.recipient, token, atomicAmount]
#3Quoted vs actual fill✅ FixeddeployToProtocol takes allowPartial = false; auto-deploy path (bridge_deploy_status) passes true; supplies balance when partial; standalone deploy_on_destination stays strict (false)
#4URL encoding✅ Fixedutils.ts:256–259 uses new URL(ACROSS_DEPOSIT_STATUS_API) + url.searchParams.set(...)
#6Bigint slippage arithmetic✅ Fixedutils.ts:183–185: ((inputAmount - outputAmount) * 10000n) / inputAmount — no float conversion
#1In-memory state documented✅ FixedJSDoc on #pendingDeploys (bridgeDeployActionProvider.ts:47–56) and a dedicated "⚠️ Pending-supply state is in-memory only" section in README.md:23–40 both clearly state the instance-local constraint
coinbase#9Token address vs symbol warning✅ Fixeddeploy_on_destination action description says "NOT the token symbol"; README.md:74–76 also distinguishes bridge_and_deploy (symbol) from deploy_on_destination (address)
#5Missing test coverage✅ Fixedtest.ts:216–236 covers filled with no pending record; test.ts:238–270 covers supply-fails-on-filled retry retention

Intentionally deferred (confirmed reasonable)

One minor observation (non-blocking)

COMET_SUPPLY_ABI in constants.ts still includes the supply(address asset, uint256 amount) entry, but the code now always calls supplyTo. The supply entry is dead code in the execution path. The comment justifies its presence for documentation purposes, and since it cannot cause a runtime error, this is harmless — just worth noting for cleanliness.

Conventions checklist

RuleStatus
All actions return Promise<string>; errors returned not thrown
Zod v4 schemas with .describe() on every field; no .strip()
supportsNetwork is an arrow-function property
Factory bridgeDeployActionProvider exported and re-exported from index.ts
README.md present and documents EOA/two-step semantics
Changeset — @coinbase/agentkit, patch, past tense
Tests present, mocked wallet, 13 provider tests
super("bridgeDeploy", []) constructor call
Network.chainId compared as string

The PR is accurate. All blocking issues from the previous review have been correctly addressed. The one remaining item (supply ABI entry being unused) is non-blocking cosmetic cleanup. The implementation is clean, well-documented, and matches AgentKit conventions.

@zaryab2000Claude

Copy link
Copy Markdown
OwnerAuthor

Final review — confirmed accurate ✅

Re-reviewed the full provider on head 8633b59 and verified all the review fixes landed correctly.

Correctness — verified in code

  • Compound recipient:deployToProtocol now encodes supplyTo(recipient, asset, amount) (utils.ts:357-362), so the position is credited to the intended owner, matching Morpho's deposit(assets, receiver). ✅
  • Partial-fill handling: auto-deploy passes allowPartial=true and supplies min(requested, balance) when a fill lands under the quote; deploy_on_destination stays strict (exact amount or error). ✅
  • URL building:getDepositStatus uses URLSearchParams. ✅
  • Slippage: computed with bigint arithmetic. ✅
  • State + token docs: in-memory #pendingDeploys warning on the field and in the README; address-not-symbol note on deploy_on_destination. ✅
  • Error-return semantics, Promise<string>, Zod .describe() (no .strip()), arrow supportsNetwork, factory re-export — all intact.

CI — green on head 8633b59

  • 25/25 checks passing (lint-typescript, format-typescript, test-agentkit-typescript (18 & 20), check-package-lock, plus the python jobs).
  • mergeable_state: clean — no conflicts with feat/bridgeThenDeploy.

Local

  • 13 provider tests + 877 full agentkit suite green; lint / format / build clean.

One honest limitation worth recording (not a blocker, by design):
The auto-deploy can only supply funds the wallet custodies. If you set recipient on bridge_and_deploy to a third party, the bridge sends there and the wallet holds nothing, so the auto-supply returns a graceful "insufficient balance" (pending kept for retry) rather than silently doing nothing. With the default (recipient = sender) the full bridge→supply flow works as intended. This is consistent and non-lossy, and the README documents that recipient defaults to the sender.

Minor / optional: the PR description is slightly stale vs the latest commits — it still says "875 tests" and "Compound (Comet supply)" (now 877 tests and supplyTo). Happy to refresh the description if you'd like; the code and changeset are accurate.

Still intentionally deferred:#7 (dead if (!account)) and coinbase#8 (http() default RPC) — verbatim parity with the in-tree across provider; will change if you prefer divergence.

Net: the PR is accurate and merge-ready into feat/bridgeThenDeploy. Reminder that upstream (coinbase/agentkit) still requires the human-only steps from our checklist — commit signing, the docs-internal//fork-tooling strip, and a rebase on the latest main — before it goes out as the public contribution.


Generated by Claude Code

The Compound path always uses supplyTo to honor the recipient, so the
supply(asset, amount) entry was dead code. Drop it from COMET_SUPPLY_ABI.
https://claude.ai/code/session_013rnmDYrKDMctusY33uneiy
@zaryab2000Claude

Copy link
Copy Markdown
OwnerAuthor

Addressed the one new observation from the final review in 5a6c1fa: removed the now-unused supply(asset, amount) entry from COMET_SUPPLY_ABI — the Compound path always calls supplyTo, so it was dead code. ABI now contains only supplyTo.

No functional change; the dead-code path is gone. Verified: 13 provider tests + full agentkit suite green, lint / format / build clean.

No other new bugs were reported in the latest review (it confirmed all 7 prior fixes as correct). PR remains accurate and merge-ready into feat/bridgeThenDeploy.


Generated by Claude Code

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

Labels

action providerdocumentationImprovements or additions to documentationtypescript

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@zaryab2000@claude
, 'i'); if (__m === '*' || __re.test(location.href)) { // Auto-enable theater mode on YouTube (function() { function tryTheater() { var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]'); if (btn && !btn.classList.contains('activated')) { btn.click(); } } // Try immediately tryTheater(); // Try after navigation (SPA) var lastUrl = location.href; setInterval(function() { if (location.href !== lastUrl) { lastUrl = location.href; setTimeout(tryTheater, 500); } }, 1000); // Also try on player load var observer = new MutationObserver(tryTheater); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Add bridge-then-deploy action provider - #3

Closed
zaryab2000 wants to merge 3 commits into
feat/bridgeThenDeployfrom
claude/tender-mccarthy-ROhys
Closed

Add bridge-then-deploy action provider#3
zaryab2000 wants to merge 3 commits into
feat/bridgeThenDeployfrom
claude/tender-mccarthy-ROhys

Conversation

@zaryab2000

Copy link
Copy Markdown
Owner

Summary

Adds a new bridge-then-deploy ActionProvider (bridgeDeploy) that composes an Across bridge with a destination lending/vault supply, letting an agent express "move this capital to chain X and put it to work" as a single intent.

Central design constraint (honest by design)

Across destination-side execution only delivers to a deployed handler contract that implements handleV3AcrossMessage(...)never to a plain EOA. Agent wallets are EOAs, so this flow is deliberately two-step and non-atomic: bridge → poll status → supply on arrival. The provider never claims the supply has happened before the bridge fills.

Actions

  • bridge_and_deploy — initiates the Across deposit and records the pending destination supply; returns a depositId and status: "bridging". Does not supply yet.
  • bridge_deploy_status — polls the Across deposit-status API; once filled, auto-runs the recorded destination supply and returns the deploy tx hash. Reports pending/refunded otherwise.
  • deploy_on_destination — supplies an already-bridged token into Compound (Comet supply) or Morpho (vault deposit) on Base. Performs a balance preflight, so it is safe to retry to recover from a partial failure.

Scope

  • Destination deploy gated to Base (8453/84532); origin can be any EVM chain Across supports.
  • v1 ships Flow A (EOA, two-step) only. Flow B (handler-contract atomic deposit) is documented in the README as the upgrade path.

Conventions

  • All actions return Promise<string>; errors are returned, not thrown.
  • Zod v4 schemas with .describe() on every field; no .strip().
  • Reuses the shared approve util and mirrors the in-tree across provider's SDK wiring (privateKey constructor config, fully mocked in tests).
  • Factory re-exported from src/action-providers/index.ts.

Checks

  • pnpm test — full agentkit suite green (875 tests, incl. 11 new for this provider, mocked wallet + Across SDK).
  • pnpm run lint, pnpm run format, pnpm build — all clean.
  • Changeset added (@coinbase/agentkit, patch, past tense).

Note: per-provider README.md prominently documents the two-step / non-atomic / EOA-can't-receive-callback semantics and the partial-failure recovery path.

https://claude.ai/code/session_013rnmDYrKDMctusY33uneiy


Generated by Claude Code

Composes an Across bridge with a destination lending/vault supply so an
agent can express "move this capital to chain X and put it to work" as one
intent. Honest about the EOA constraint: Across destination execution only
reaches a handler contract, never an EOA, so the flow is two-step and
non-atomic (bridge -> poll status -> supply on arrival).
Actions:
- bridge_and_deploy: initiates the Across deposit and records the pending
destination supply; never claims the supply has happened.
- bridge_deploy_status: polls the Across deposit-status API and auto-runs
the recorded supply once the bridge is filled.
- deploy_on_destination: supplies an already-bridged token into Compound or
Morpho on Base, with a balance preflight so it is safe to retry.
Destination deploy is gated to Base; origin can be any EVM chain Across
supports. Includes unit tests (mocked wallet + Across SDK), README
documenting the non-atomic semantics, and a changeset.
https://claude.ai/code/session_013rnmDYrKDMctusY33uneiy
@github-actionsgithub-actionsBot added documentation Improvements or additions to documentation typescript action provider labels Jun 6, 2026
@zaryab2000

Copy link
Copy Markdown
OwnerAuthor

@claude start review

@claude

claudeBot commented Jun 6, 2026

Copy link
Copy Markdown

Claude finished @zaryab2000's task in 2m 59s —— View job


Code Review — bridgeDeploy ActionProvider

Tasks

  • Load tools
  • Read all changed files (bridgeDeployActionProvider.ts, utils.ts, schemas.ts, constants.ts, index.ts, test, README, changeset)
  • Analyze against AgentKit conventions (CLAUDE.md)
  • Review for bugs, security, correctness
  • Post final review

Overall the code is well-structured and closely follows AgentKit conventions. All three major checks from CLAUDE.md pass (Promise, Zod v4 with .describe(), arrow-function supportsNetwork). Below are the findings in priority order.


🔴 High — Correctness Bugs

1. In-memory #pendingDeploys is lost on provider re-instantiation

bridgeDeployActionProvider.ts:46

#pendingDeploys: Map<string,PendingDeploy>=newMap();

#pendingDeploys is instance-level state. The factory bridgeDeployActionProvider(config) creates a new instance each time it is called. In most agent patterns the provider is re-created between sessions (or even between turns). If the provider is re-instantiated while a bridge is in flight — which can take minutes to an hour — the pending deploy record is gone. bridge_deploy_status will then return the "no pending supply was recorded" fallback instead of auto-supplying.

This either needs to be documented prominently as a hard constraint ("the same provider instance must be reused across calls") or the pending state must be serialized to somewhere durable (a file, a passed-in store, etc.). At minimum the README should warn that a new instance forgets all pending deploys.

Fix this →


2. Compound supply() silently ignores recipient

utils.ts:339-344

if(params.protocol==="compound"){data=encodeFunctionData({abi: COMET_SUPPLY_ABI,functionName: "supply",args: [token,atomicAmount],// ← no recipient});}

The supply(address asset, uint amount) Comet function deposits from msg.sender into msg.sender's own position. There is no recipient argument here. If the caller passes recipient ≠ walletProvider.getAddress(), for Morpho the deposit goes to the intended recipient, but for Compound it silently goes to the caller's position instead. This is a silent semantic divergence.

Comet provides supplyTo(address dst, address asset, uint amount) for this purpose. The ABI in constants.ts and the encodeFunctionData call should use supplyTo when recipient != walletProvider.getAddress(), or the COMET_SUPPLY_ABI should be updated to include supplyTo and always use it.

Fix this →


3. Quoted outputAmount stored as supply target — mismatches actual fill

bridgeDeployActionProvider.ts:112-117

this.#pendingDeploys.set(this.#pendingKey(...),{
...
amount: deposit.outputAmount,// ← from quote, not actual fill
...
});

deposit.outputAmount is the quoted output. The actual fill amount may differ. When deployToProtocol later runs the balance preflight (balance < atomicAmount), it uses the quoted amount, which can either:

  • Block the supply if the fill was slightly less than quoted (e.g., fee changed between quote and fill)
  • Over-supply if the fill was more than quoted (e.g., the relayer undercut the fee)

The safer approach is to supply Math.min(balance, atomicAmount) — i.e., use whatever landed — or to drop the preflight comparison and let the protocol revert if underfunded.


🟡 Medium — Reliability / Security

4. depositId interpolated into URL without encoding

utils.ts:255-258

constresponse=awaitfetch(`${ACROSS_DEPOSIT_STATUS_API}?originChainId=${originChainId}&depositId=${depositId}`,

depositId (and originChainId) are user-supplied strings concatenated directly into the URL. If either contains &, =, or #, the query string is silently malformed. Use URLSearchParams:

consturl=newURL(ACROSS_DEPOSIT_STATUS_API);url.searchParams.set("originChainId",String(originChainId));url.searchParams.set("depositId",depositId);constresponse=awaitfetch(url.toString(),{method: "GET"});

Fix this →


5. Two test paths not covered

bridgeDeployActionProvider.test.ts

  • filled with no pending deploybridgeDeployStatus returns a "call deploy_on_destination manually" note when the map has no entry for the deposit. bridgeDeployActionProvider.ts:178-187 handles this but it is never exercised.
  • Supply fails on filledbridgeDeployActionProvider.ts:191-198 keeps the pending deploy for retry when deployResult.startsWith("Error"). This retry path has no test.

6. Floating-point slippage arithmetic

utils.ts:183-185

constslippageBps=((Number(formattedInput)-Number(formattedOutput))/Number(formattedInput))*10000;

Converting bigintformatUnitsNumber → arithmetic introduces floating-point rounding. For basis-point comparisons, prefer bigint arithmetic directly:

constslippageBps=Number(((quote.deposit.inputAmount-quote.deposit.outputAmount)*10000n)/quote.deposit.inputAmount);

(The existing across provider has the same issue — it is a pre-existing pattern, but worth fixing here.)


🟢 Low — Minor / Style

7. Dead validation in constructor (bridgeDeployActionProvider.ts:56-57)

constaccount=privateKeyToAccount(this.#privateKey asHex);if(!account)thrownewError("Invalid private key");

privateKeyToAccount from viem throws on an invalid key — it never returns null or undefined. The if (!account) branch is unreachable. (Same pattern in the upstream across provider.)


8. http() with no RPC URL (utils.ts:107)

transport: http(),

http() with no argument uses the chain's built-in public RPC, which is rate-limited. This is fine for occasional test runs but can fail under production load. (Same pattern in across provider — pre-existing.)


9. Token identifier inconsistency between actions

bridge_and_deploy accepts token as a symbol ("USDC"); deploy_on_destination accepts token as an address ("0x833..."). This is noted in the README, but the inconsistency creates a real risk that an LLM agent passes a symbol to the standalone deploy action and hits a silent failure (the Zod regex rejects non-address strings, so at least it fails loudly).

Consider adding a note to the deploy_on_destination action description explicitly warning: "token must be the destination-chain ERC-20 address, not the symbol".


Conventions Checklist

RuleStatus
All actions return Promise<string>; errors returned not thrown
Zod v4 schemas with .describe() on every field; no .strip()
supportsNetwork is an arrow-function property
Factory bridgeDeployActionProvider exported and re-exported from index.ts
README.md present
pnpm run changeset — patch, past tense, correct package
Tests present and mocking wallet provider
super("bridgeDeploy", []) constructor call
Network.chainId compared as string

Summary: The implementation is clean and well-documented. The two blocking issues before upstream merge are the Compound supplyTo vs supply recipient bug (#2) and the in-memory state loss documentation (#1). The URL encoding (#4) and slippage arithmetic (#6) are simple one-liners to fix. The missing test coverage (#5) should be added before the PR goes out.

- Compound: use supplyTo(dst, asset, amount) so the position is credited to
the recipient instead of msg.sender (silent divergence from Morpho).
- Auto-deploy: supply the amount that actually landed when a fill comes in
under the quoted output, instead of failing the stale-quote preflight;
deploy_on_destination stays strict for explicit calls.
- getDepositStatus: build the status URL with URLSearchParams.
- Slippage check now uses bigint arithmetic to avoid float rounding.
- Document the in-memory pending-deploy state (lost on re-instantiation) in
the README and on the field; clarify token is an address (not symbol) for
deploy_on_destination.
- Add tests for the filled-without-pending-record and supply-fails-retry paths.
https://claude.ai/code/session_013rnmDYrKDMctusY33uneiy
@zaryab2000Claude

Copy link
Copy Markdown
OwnerAuthor

Thanks for the review — addressed in 8633b59. Summary of what changed and why:

Fixed

Intentionally deferred (with rationale)


Generated by Claude Code

@zaryab2000

Copy link
Copy Markdown
OwnerAuthor

@claude conduct final review and confirm if the PR is accurate now?

@claude

claudeBot commented Jun 11, 2026

Copy link
Copy Markdown

Claude finished @zaryab2000's task in 1m 38s —— View job


Final Review — bridgeDeploy ActionProvider

Tasks


All seven addressed issues are correctly fixed. Here is the per-item confirmation:

Verified fixes

#IssueStatusEvidence
#2Compound supplyTo recipient✅ FixedCOMET_SUPPLY_ABI in constants.ts now includes supplyTo(address dst, address asset, uint256 amount); utils.ts:358–362 uses supplyTo with [params.recipient, token, atomicAmount]
#3Quoted vs actual fill✅ FixeddeployToProtocol takes allowPartial = false; auto-deploy path (bridge_deploy_status) passes true; supplies balance when partial; standalone deploy_on_destination stays strict (false)
#4URL encoding✅ Fixedutils.ts:256–259 uses new URL(ACROSS_DEPOSIT_STATUS_API) + url.searchParams.set(...)
#6Bigint slippage arithmetic✅ Fixedutils.ts:183–185: ((inputAmount - outputAmount) * 10000n) / inputAmount — no float conversion
#1In-memory state documented✅ FixedJSDoc on #pendingDeploys (bridgeDeployActionProvider.ts:47–56) and a dedicated "⚠️ Pending-supply state is in-memory only" section in README.md:23–40 both clearly state the instance-local constraint
coinbase#9Token address vs symbol warning✅ Fixeddeploy_on_destination action description says "NOT the token symbol"; README.md:74–76 also distinguishes bridge_and_deploy (symbol) from deploy_on_destination (address)
#5Missing test coverage✅ Fixedtest.ts:216–236 covers filled with no pending record; test.ts:238–270 covers supply-fails-on-filled retry retention

Intentionally deferred (confirmed reasonable)

One minor observation (non-blocking)

COMET_SUPPLY_ABI in constants.ts still includes the supply(address asset, uint256 amount) entry, but the code now always calls supplyTo. The supply entry is dead code in the execution path. The comment justifies its presence for documentation purposes, and since it cannot cause a runtime error, this is harmless — just worth noting for cleanliness.

Conventions checklist

RuleStatus
All actions return Promise<string>; errors returned not thrown
Zod v4 schemas with .describe() on every field; no .strip()
supportsNetwork is an arrow-function property
Factory bridgeDeployActionProvider exported and re-exported from index.ts
README.md present and documents EOA/two-step semantics
Changeset — @coinbase/agentkit, patch, past tense
Tests present, mocked wallet, 13 provider tests
super("bridgeDeploy", []) constructor call
Network.chainId compared as string

The PR is accurate. All blocking issues from the previous review have been correctly addressed. The one remaining item (supply ABI entry being unused) is non-blocking cosmetic cleanup. The implementation is clean, well-documented, and matches AgentKit conventions.

@zaryab2000Claude

Copy link
Copy Markdown
OwnerAuthor

Final review — confirmed accurate ✅

Re-reviewed the full provider on head 8633b59 and verified all the review fixes landed correctly.

Correctness — verified in code

  • Compound recipient:deployToProtocol now encodes supplyTo(recipient, asset, amount) (utils.ts:357-362), so the position is credited to the intended owner, matching Morpho's deposit(assets, receiver). ✅
  • Partial-fill handling: auto-deploy passes allowPartial=true and supplies min(requested, balance) when a fill lands under the quote; deploy_on_destination stays strict (exact amount or error). ✅
  • URL building:getDepositStatus uses URLSearchParams. ✅
  • Slippage: computed with bigint arithmetic. ✅
  • State + token docs: in-memory #pendingDeploys warning on the field and in the README; address-not-symbol note on deploy_on_destination. ✅
  • Error-return semantics, Promise<string>, Zod .describe() (no .strip()), arrow supportsNetwork, factory re-export — all intact.

CI — green on head 8633b59

  • 25/25 checks passing (lint-typescript, format-typescript, test-agentkit-typescript (18 & 20), check-package-lock, plus the python jobs).
  • mergeable_state: clean — no conflicts with feat/bridgeThenDeploy.

Local

  • 13 provider tests + 877 full agentkit suite green; lint / format / build clean.

One honest limitation worth recording (not a blocker, by design):
The auto-deploy can only supply funds the wallet custodies. If you set recipient on bridge_and_deploy to a third party, the bridge sends there and the wallet holds nothing, so the auto-supply returns a graceful "insufficient balance" (pending kept for retry) rather than silently doing nothing. With the default (recipient = sender) the full bridge→supply flow works as intended. This is consistent and non-lossy, and the README documents that recipient defaults to the sender.

Minor / optional: the PR description is slightly stale vs the latest commits — it still says "875 tests" and "Compound (Comet supply)" (now 877 tests and supplyTo). Happy to refresh the description if you'd like; the code and changeset are accurate.

Still intentionally deferred:#7 (dead if (!account)) and coinbase#8 (http() default RPC) — verbatim parity with the in-tree across provider; will change if you prefer divergence.

Net: the PR is accurate and merge-ready into feat/bridgeThenDeploy. Reminder that upstream (coinbase/agentkit) still requires the human-only steps from our checklist — commit signing, the docs-internal//fork-tooling strip, and a rebase on the latest main — before it goes out as the public contribution.


Generated by Claude Code

The Compound path always uses supplyTo to honor the recipient, so the
supply(asset, amount) entry was dead code. Drop it from COMET_SUPPLY_ABI.
https://claude.ai/code/session_013rnmDYrKDMctusY33uneiy
@zaryab2000Claude

Copy link
Copy Markdown
OwnerAuthor

Addressed the one new observation from the final review in 5a6c1fa: removed the now-unused supply(asset, amount) entry from COMET_SUPPLY_ABI — the Compound path always calls supplyTo, so it was dead code. ABI now contains only supplyTo.

No functional change; the dead-code path is gone. Verified: 13 provider tests + full agentkit suite green, lint / format / build clean.

No other new bugs were reported in the latest review (it confirmed all 7 prior fixes as correct). PR remains accurate and merge-ready into feat/bridgeThenDeploy.


Generated by Claude Code

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

Labels

action providerdocumentationImprovements or additions to documentationtypescript

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@zaryab2000@claude
, 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Add bridge-then-deploy action provider - #3

Closed
zaryab2000 wants to merge 3 commits into
feat/bridgeThenDeployfrom
claude/tender-mccarthy-ROhys
Closed

Add bridge-then-deploy action provider#3
zaryab2000 wants to merge 3 commits into
feat/bridgeThenDeployfrom
claude/tender-mccarthy-ROhys

Conversation

@zaryab2000

Copy link
Copy Markdown
Owner

Summary

Adds a new bridge-then-deploy ActionProvider (bridgeDeploy) that composes an Across bridge with a destination lending/vault supply, letting an agent express "move this capital to chain X and put it to work" as a single intent.

Central design constraint (honest by design)

Across destination-side execution only delivers to a deployed handler contract that implements handleV3AcrossMessage(...)never to a plain EOA. Agent wallets are EOAs, so this flow is deliberately two-step and non-atomic: bridge → poll status → supply on arrival. The provider never claims the supply has happened before the bridge fills.

Actions

  • bridge_and_deploy — initiates the Across deposit and records the pending destination supply; returns a depositId and status: "bridging". Does not supply yet.
  • bridge_deploy_status — polls the Across deposit-status API; once filled, auto-runs the recorded destination supply and returns the deploy tx hash. Reports pending/refunded otherwise.
  • deploy_on_destination — supplies an already-bridged token into Compound (Comet supply) or Morpho (vault deposit) on Base. Performs a balance preflight, so it is safe to retry to recover from a partial failure.

Scope

  • Destination deploy gated to Base (8453/84532); origin can be any EVM chain Across supports.
  • v1 ships Flow A (EOA, two-step) only. Flow B (handler-contract atomic deposit) is documented in the README as the upgrade path.

Conventions

  • All actions return Promise<string>; errors are returned, not thrown.
  • Zod v4 schemas with .describe() on every field; no .strip().
  • Reuses the shared approve util and mirrors the in-tree across provider's SDK wiring (privateKey constructor config, fully mocked in tests).
  • Factory re-exported from src/action-providers/index.ts.

Checks

  • pnpm test — full agentkit suite green (875 tests, incl. 11 new for this provider, mocked wallet + Across SDK).
  • pnpm run lint, pnpm run format, pnpm build — all clean.
  • Changeset added (@coinbase/agentkit, patch, past tense).

Note: per-provider README.md prominently documents the two-step / non-atomic / EOA-can't-receive-callback semantics and the partial-failure recovery path.

https://claude.ai/code/session_013rnmDYrKDMctusY33uneiy


Generated by Claude Code

Composes an Across bridge with a destination lending/vault supply so an
agent can express "move this capital to chain X and put it to work" as one
intent. Honest about the EOA constraint: Across destination execution only
reaches a handler contract, never an EOA, so the flow is two-step and
non-atomic (bridge -> poll status -> supply on arrival).
Actions:
- bridge_and_deploy: initiates the Across deposit and records the pending
destination supply; never claims the supply has happened.
- bridge_deploy_status: polls the Across deposit-status API and auto-runs
the recorded supply once the bridge is filled.
- deploy_on_destination: supplies an already-bridged token into Compound or
Morpho on Base, with a balance preflight so it is safe to retry.
Destination deploy is gated to Base; origin can be any EVM chain Across
supports. Includes unit tests (mocked wallet + Across SDK), README
documenting the non-atomic semantics, and a changeset.
https://claude.ai/code/session_013rnmDYrKDMctusY33uneiy
@github-actionsgithub-actionsBot added documentation Improvements or additions to documentation typescript action provider labels Jun 6, 2026
@zaryab2000

Copy link
Copy Markdown
OwnerAuthor

@claude start review

@claude

claudeBot commented Jun 6, 2026

Copy link
Copy Markdown

Claude finished @zaryab2000's task in 2m 59s —— View job


Code Review — bridgeDeploy ActionProvider

Tasks

  • Load tools
  • Read all changed files (bridgeDeployActionProvider.ts, utils.ts, schemas.ts, constants.ts, index.ts, test, README, changeset)
  • Analyze against AgentKit conventions (CLAUDE.md)
  • Review for bugs, security, correctness
  • Post final review

Overall the code is well-structured and closely follows AgentKit conventions. All three major checks from CLAUDE.md pass (Promise, Zod v4 with .describe(), arrow-function supportsNetwork). Below are the findings in priority order.


🔴 High — Correctness Bugs

1. In-memory #pendingDeploys is lost on provider re-instantiation

bridgeDeployActionProvider.ts:46

#pendingDeploys: Map<string,PendingDeploy>=newMap();

#pendingDeploys is instance-level state. The factory bridgeDeployActionProvider(config) creates a new instance each time it is called. In most agent patterns the provider is re-created between sessions (or even between turns). If the provider is re-instantiated while a bridge is in flight — which can take minutes to an hour — the pending deploy record is gone. bridge_deploy_status will then return the "no pending supply was recorded" fallback instead of auto-supplying.

This either needs to be documented prominently as a hard constraint ("the same provider instance must be reused across calls") or the pending state must be serialized to somewhere durable (a file, a passed-in store, etc.). At minimum the README should warn that a new instance forgets all pending deploys.

Fix this →


2. Compound supply() silently ignores recipient

utils.ts:339-344

if(params.protocol==="compound"){data=encodeFunctionData({abi: COMET_SUPPLY_ABI,functionName: "supply",args: [token,atomicAmount],// ← no recipient});}

The supply(address asset, uint amount) Comet function deposits from msg.sender into msg.sender's own position. There is no recipient argument here. If the caller passes recipient ≠ walletProvider.getAddress(), for Morpho the deposit goes to the intended recipient, but for Compound it silently goes to the caller's position instead. This is a silent semantic divergence.

Comet provides supplyTo(address dst, address asset, uint amount) for this purpose. The ABI in constants.ts and the encodeFunctionData call should use supplyTo when recipient != walletProvider.getAddress(), or the COMET_SUPPLY_ABI should be updated to include supplyTo and always use it.

Fix this →


3. Quoted outputAmount stored as supply target — mismatches actual fill

bridgeDeployActionProvider.ts:112-117

this.#pendingDeploys.set(this.#pendingKey(...),{
...
amount: deposit.outputAmount,// ← from quote, not actual fill
...
});

deposit.outputAmount is the quoted output. The actual fill amount may differ. When deployToProtocol later runs the balance preflight (balance < atomicAmount), it uses the quoted amount, which can either:

  • Block the supply if the fill was slightly less than quoted (e.g., fee changed between quote and fill)
  • Over-supply if the fill was more than quoted (e.g., the relayer undercut the fee)

The safer approach is to supply Math.min(balance, atomicAmount) — i.e., use whatever landed — or to drop the preflight comparison and let the protocol revert if underfunded.


🟡 Medium — Reliability / Security

4. depositId interpolated into URL without encoding

utils.ts:255-258

constresponse=awaitfetch(`${ACROSS_DEPOSIT_STATUS_API}?originChainId=${originChainId}&depositId=${depositId}`,

depositId (and originChainId) are user-supplied strings concatenated directly into the URL. If either contains &, =, or #, the query string is silently malformed. Use URLSearchParams:

consturl=newURL(ACROSS_DEPOSIT_STATUS_API);url.searchParams.set("originChainId",String(originChainId));url.searchParams.set("depositId",depositId);constresponse=awaitfetch(url.toString(),{method: "GET"});

Fix this →


5. Two test paths not covered

bridgeDeployActionProvider.test.ts

  • filled with no pending deploybridgeDeployStatus returns a "call deploy_on_destination manually" note when the map has no entry for the deposit. bridgeDeployActionProvider.ts:178-187 handles this but it is never exercised.
  • Supply fails on filledbridgeDeployActionProvider.ts:191-198 keeps the pending deploy for retry when deployResult.startsWith("Error"). This retry path has no test.

6. Floating-point slippage arithmetic

utils.ts:183-185

constslippageBps=((Number(formattedInput)-Number(formattedOutput))/Number(formattedInput))*10000;

Converting bigintformatUnitsNumber → arithmetic introduces floating-point rounding. For basis-point comparisons, prefer bigint arithmetic directly:

constslippageBps=Number(((quote.deposit.inputAmount-quote.deposit.outputAmount)*10000n)/quote.deposit.inputAmount);

(The existing across provider has the same issue — it is a pre-existing pattern, but worth fixing here.)


🟢 Low — Minor / Style

7. Dead validation in constructor (bridgeDeployActionProvider.ts:56-57)

constaccount=privateKeyToAccount(this.#privateKey asHex);if(!account)thrownewError("Invalid private key");

privateKeyToAccount from viem throws on an invalid key — it never returns null or undefined. The if (!account) branch is unreachable. (Same pattern in the upstream across provider.)


8. http() with no RPC URL (utils.ts:107)

transport: http(),

http() with no argument uses the chain's built-in public RPC, which is rate-limited. This is fine for occasional test runs but can fail under production load. (Same pattern in across provider — pre-existing.)


9. Token identifier inconsistency between actions

bridge_and_deploy accepts token as a symbol ("USDC"); deploy_on_destination accepts token as an address ("0x833..."). This is noted in the README, but the inconsistency creates a real risk that an LLM agent passes a symbol to the standalone deploy action and hits a silent failure (the Zod regex rejects non-address strings, so at least it fails loudly).

Consider adding a note to the deploy_on_destination action description explicitly warning: "token must be the destination-chain ERC-20 address, not the symbol".


Conventions Checklist

RuleStatus
All actions return Promise<string>; errors returned not thrown
Zod v4 schemas with .describe() on every field; no .strip()
supportsNetwork is an arrow-function property
Factory bridgeDeployActionProvider exported and re-exported from index.ts
README.md present
pnpm run changeset — patch, past tense, correct package
Tests present and mocking wallet provider
super("bridgeDeploy", []) constructor call
Network.chainId compared as string

Summary: The implementation is clean and well-documented. The two blocking issues before upstream merge are the Compound supplyTo vs supply recipient bug (#2) and the in-memory state loss documentation (#1). The URL encoding (#4) and slippage arithmetic (#6) are simple one-liners to fix. The missing test coverage (#5) should be added before the PR goes out.

- Compound: use supplyTo(dst, asset, amount) so the position is credited to
the recipient instead of msg.sender (silent divergence from Morpho).
- Auto-deploy: supply the amount that actually landed when a fill comes in
under the quoted output, instead of failing the stale-quote preflight;
deploy_on_destination stays strict for explicit calls.
- getDepositStatus: build the status URL with URLSearchParams.
- Slippage check now uses bigint arithmetic to avoid float rounding.
- Document the in-memory pending-deploy state (lost on re-instantiation) in
the README and on the field; clarify token is an address (not symbol) for
deploy_on_destination.
- Add tests for the filled-without-pending-record and supply-fails-retry paths.
https://claude.ai/code/session_013rnmDYrKDMctusY33uneiy
@zaryab2000Claude

Copy link
Copy Markdown
OwnerAuthor

Thanks for the review — addressed in 8633b59. Summary of what changed and why:

Fixed

Intentionally deferred (with rationale)


Generated by Claude Code

@zaryab2000

Copy link
Copy Markdown
OwnerAuthor

@claude conduct final review and confirm if the PR is accurate now?

@claude

claudeBot commented Jun 11, 2026

Copy link
Copy Markdown

Claude finished @zaryab2000's task in 1m 38s —— View job


Final Review — bridgeDeploy ActionProvider

Tasks


All seven addressed issues are correctly fixed. Here is the per-item confirmation:

Verified fixes

#IssueStatusEvidence
#2Compound supplyTo recipient✅ FixedCOMET_SUPPLY_ABI in constants.ts now includes supplyTo(address dst, address asset, uint256 amount); utils.ts:358–362 uses supplyTo with [params.recipient, token, atomicAmount]
#3Quoted vs actual fill✅ FixeddeployToProtocol takes allowPartial = false; auto-deploy path (bridge_deploy_status) passes true; supplies balance when partial; standalone deploy_on_destination stays strict (false)
#4URL encoding✅ Fixedutils.ts:256–259 uses new URL(ACROSS_DEPOSIT_STATUS_API) + url.searchParams.set(...)
#6Bigint slippage arithmetic✅ Fixedutils.ts:183–185: ((inputAmount - outputAmount) * 10000n) / inputAmount — no float conversion
#1In-memory state documented✅ FixedJSDoc on #pendingDeploys (bridgeDeployActionProvider.ts:47–56) and a dedicated "⚠️ Pending-supply state is in-memory only" section in README.md:23–40 both clearly state the instance-local constraint
coinbase#9Token address vs symbol warning✅ Fixeddeploy_on_destination action description says "NOT the token symbol"; README.md:74–76 also distinguishes bridge_and_deploy (symbol) from deploy_on_destination (address)
#5Missing test coverage✅ Fixedtest.ts:216–236 covers filled with no pending record; test.ts:238–270 covers supply-fails-on-filled retry retention

Intentionally deferred (confirmed reasonable)

One minor observation (non-blocking)

COMET_SUPPLY_ABI in constants.ts still includes the supply(address asset, uint256 amount) entry, but the code now always calls supplyTo. The supply entry is dead code in the execution path. The comment justifies its presence for documentation purposes, and since it cannot cause a runtime error, this is harmless — just worth noting for cleanliness.

Conventions checklist

RuleStatus
All actions return Promise<string>; errors returned not thrown
Zod v4 schemas with .describe() on every field; no .strip()
supportsNetwork is an arrow-function property
Factory bridgeDeployActionProvider exported and re-exported from index.ts
README.md present and documents EOA/two-step semantics
Changeset — @coinbase/agentkit, patch, past tense
Tests present, mocked wallet, 13 provider tests
super("bridgeDeploy", []) constructor call
Network.chainId compared as string

The PR is accurate. All blocking issues from the previous review have been correctly addressed. The one remaining item (supply ABI entry being unused) is non-blocking cosmetic cleanup. The implementation is clean, well-documented, and matches AgentKit conventions.

@zaryab2000Claude

Copy link
Copy Markdown
OwnerAuthor

Final review — confirmed accurate ✅

Re-reviewed the full provider on head 8633b59 and verified all the review fixes landed correctly.

Correctness — verified in code

  • Compound recipient:deployToProtocol now encodes supplyTo(recipient, asset, amount) (utils.ts:357-362), so the position is credited to the intended owner, matching Morpho's deposit(assets, receiver). ✅
  • Partial-fill handling: auto-deploy passes allowPartial=true and supplies min(requested, balance) when a fill lands under the quote; deploy_on_destination stays strict (exact amount or error). ✅
  • URL building:getDepositStatus uses URLSearchParams. ✅
  • Slippage: computed with bigint arithmetic. ✅
  • State + token docs: in-memory #pendingDeploys warning on the field and in the README; address-not-symbol note on deploy_on_destination. ✅
  • Error-return semantics, Promise<string>, Zod .describe() (no .strip()), arrow supportsNetwork, factory re-export — all intact.

CI — green on head 8633b59

  • 25/25 checks passing (lint-typescript, format-typescript, test-agentkit-typescript (18 & 20), check-package-lock, plus the python jobs).
  • mergeable_state: clean — no conflicts with feat/bridgeThenDeploy.

Local

  • 13 provider tests + 877 full agentkit suite green; lint / format / build clean.

One honest limitation worth recording (not a blocker, by design):
The auto-deploy can only supply funds the wallet custodies. If you set recipient on bridge_and_deploy to a third party, the bridge sends there and the wallet holds nothing, so the auto-supply returns a graceful "insufficient balance" (pending kept for retry) rather than silently doing nothing. With the default (recipient = sender) the full bridge→supply flow works as intended. This is consistent and non-lossy, and the README documents that recipient defaults to the sender.

Minor / optional: the PR description is slightly stale vs the latest commits — it still says "875 tests" and "Compound (Comet supply)" (now 877 tests and supplyTo). Happy to refresh the description if you'd like; the code and changeset are accurate.

Still intentionally deferred:#7 (dead if (!account)) and coinbase#8 (http() default RPC) — verbatim parity with the in-tree across provider; will change if you prefer divergence.

Net: the PR is accurate and merge-ready into feat/bridgeThenDeploy. Reminder that upstream (coinbase/agentkit) still requires the human-only steps from our checklist — commit signing, the docs-internal//fork-tooling strip, and a rebase on the latest main — before it goes out as the public contribution.


Generated by Claude Code

The Compound path always uses supplyTo to honor the recipient, so the
supply(asset, amount) entry was dead code. Drop it from COMET_SUPPLY_ABI.
https://claude.ai/code/session_013rnmDYrKDMctusY33uneiy
@zaryab2000Claude

Copy link
Copy Markdown
OwnerAuthor

Addressed the one new observation from the final review in 5a6c1fa: removed the now-unused supply(asset, amount) entry from COMET_SUPPLY_ABI — the Compound path always calls supplyTo, so it was dead code. ABI now contains only supplyTo.

No functional change; the dead-code path is gone. Verified: 13 provider tests + full agentkit suite green, lint / format / build clean.

No other new bugs were reported in the latest review (it confirmed all 7 prior fixes as correct). PR remains accurate and merge-ready into feat/bridgeThenDeploy.


Generated by Claude Code

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

Labels

action providerdocumentationImprovements or additions to documentationtypescript

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@zaryab2000@claude
, 'i'); if (__m === '*' || __re.test(location.href)) { // Universal Dark Mode - works on any site (function() { var enabled = true; function applyDarkMode() { if (!enabled) return; // Create style element if it doesn't exist var style = document.getElementById('universal-dark-mode-style'); if (!style) { style = document.createElement('style'); style.id = 'universal-dark-mode-style'; document.head.appendChild(style); } // Dark mode CSS - inverts colors but preserves images/video style.textContent = ' /* Invert everything except media */ html { filter: invert(1) hue-rotate(180deg) !important; background: #1a1a2e !important; } /* Restore images, videos, iframes, canvas */ img, video, iframe, canvas, svg, picture, [style*="background-image"] { filter: invert(1) hue-rotate(180deg) !important; } /* Preserve specific elements that should not be inverted */ .no-dark-mode, .no-dark-mode *, [data-theme="light"], [data-theme="light"], .ace_editor, .ace_editor *, .CodeMirror, .CodeMirror *, .monaco-editor, .monaco-editor *, .markdown-body pre, .markdown-body pre *, .highlight, .highlight *, pre code, pre code * { filter: none !important; } /* Fix common UI elements */ .modal, .popup, .dropdown-menu, .tooltip, .popover { filter: invert(1) hue-rotate(180deg) !important; background: #2d2d44 !important; border-color: #444 !important; } /* Scrollbars */ ::-webkit-scrollbar { background: #1a1a2e !important; } ::-webkit-scrollbar-thumb { background: #444 !important; } ::-webkit-scrollbar-thumb:hover { background: #555 !important; } /* Selection */ ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; } ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; } '; } function removeDarkMode() { var style = document.getElementById('universal-dark-mode-style'); if (style) style.remove(); } // Toggle with Alt+Shift+D document.addEventListener('keydown', function(e) { if (e.altKey && e.shiftKey && e.key === 'D') { e.preventDefault(); enabled = !enabled; if (enabled) { applyDarkMode(); console.log('[Universal Dark Mode] Enabled'); } else { removeDarkMode(); console.log('[Universal Dark Mode] Disabled'); } } }); // Apply on load applyDarkMode(); // Re-apply on dynamic content var observer = new MutationObserver(function(mutations) { if (enabled && !document.getElementById('universal-dark-mode-style')) { applyDarkMode(); } }); observer.observe(document.head, { childList: true }); console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle'); })(); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

Add bridge-then-deploy action provider - #3

Closed
zaryab2000 wants to merge 3 commits into
feat/bridgeThenDeployfrom
claude/tender-mccarthy-ROhys
Closed

Add bridge-then-deploy action provider#3
zaryab2000 wants to merge 3 commits into
feat/bridgeThenDeployfrom
claude/tender-mccarthy-ROhys

Conversation

@zaryab2000

Copy link
Copy Markdown
Owner

Summary

Adds a new bridge-then-deploy ActionProvider (bridgeDeploy) that composes an Across bridge with a destination lending/vault supply, letting an agent express "move this capital to chain X and put it to work" as a single intent.

Central design constraint (honest by design)

Across destination-side execution only delivers to a deployed handler contract that implements handleV3AcrossMessage(...)never to a plain EOA. Agent wallets are EOAs, so this flow is deliberately two-step and non-atomic: bridge → poll status → supply on arrival. The provider never claims the supply has happened before the bridge fills.

Actions

  • bridge_and_deploy — initiates the Across deposit and records the pending destination supply; returns a depositId and status: "bridging". Does not supply yet.
  • bridge_deploy_status — polls the Across deposit-status API; once filled, auto-runs the recorded destination supply and returns the deploy tx hash. Reports pending/refunded otherwise.
  • deploy_on_destination — supplies an already-bridged token into Compound (Comet supply) or Morpho (vault deposit) on Base. Performs a balance preflight, so it is safe to retry to recover from a partial failure.

Scope

  • Destination deploy gated to Base (8453/84532); origin can be any EVM chain Across supports.
  • v1 ships Flow A (EOA, two-step) only. Flow B (handler-contract atomic deposit) is documented in the README as the upgrade path.

Conventions

  • All actions return Promise<string>; errors are returned, not thrown.
  • Zod v4 schemas with .describe() on every field; no .strip().
  • Reuses the shared approve util and mirrors the in-tree across provider's SDK wiring (privateKey constructor config, fully mocked in tests).
  • Factory re-exported from src/action-providers/index.ts.

Checks

  • pnpm test — full agentkit suite green (875 tests, incl. 11 new for this provider, mocked wallet + Across SDK).
  • pnpm run lint, pnpm run format, pnpm build — all clean.
  • Changeset added (@coinbase/agentkit, patch, past tense).

Note: per-provider README.md prominently documents the two-step / non-atomic / EOA-can't-receive-callback semantics and the partial-failure recovery path.

https://claude.ai/code/session_013rnmDYrKDMctusY33uneiy


Generated by Claude Code

Composes an Across bridge with a destination lending/vault supply so an
agent can express "move this capital to chain X and put it to work" as one
intent. Honest about the EOA constraint: Across destination execution only
reaches a handler contract, never an EOA, so the flow is two-step and
non-atomic (bridge -> poll status -> supply on arrival).
Actions:
- bridge_and_deploy: initiates the Across deposit and records the pending
destination supply; never claims the supply has happened.
- bridge_deploy_status: polls the Across deposit-status API and auto-runs
the recorded supply once the bridge is filled.
- deploy_on_destination: supplies an already-bridged token into Compound or
Morpho on Base, with a balance preflight so it is safe to retry.
Destination deploy is gated to Base; origin can be any EVM chain Across
supports. Includes unit tests (mocked wallet + Across SDK), README
documenting the non-atomic semantics, and a changeset.
https://claude.ai/code/session_013rnmDYrKDMctusY33uneiy
@github-actionsgithub-actionsBot added documentation Improvements or additions to documentation typescript action provider labels Jun 6, 2026
@zaryab2000

Copy link
Copy Markdown
OwnerAuthor

@claude start review

@claude

claudeBot commented Jun 6, 2026

Copy link
Copy Markdown

Claude finished @zaryab2000's task in 2m 59s —— View job


Code Review — bridgeDeploy ActionProvider

Tasks

  • Load tools
  • Read all changed files (bridgeDeployActionProvider.ts, utils.ts, schemas.ts, constants.ts, index.ts, test, README, changeset)
  • Analyze against AgentKit conventions (CLAUDE.md)
  • Review for bugs, security, correctness
  • Post final review

Overall the code is well-structured and closely follows AgentKit conventions. All three major checks from CLAUDE.md pass (Promise, Zod v4 with .describe(), arrow-function supportsNetwork). Below are the findings in priority order.


🔴 High — Correctness Bugs

1. In-memory #pendingDeploys is lost on provider re-instantiation

bridgeDeployActionProvider.ts:46

#pendingDeploys: Map<string,PendingDeploy>=newMap();

#pendingDeploys is instance-level state. The factory bridgeDeployActionProvider(config) creates a new instance each time it is called. In most agent patterns the provider is re-created between sessions (or even between turns). If the provider is re-instantiated while a bridge is in flight — which can take minutes to an hour — the pending deploy record is gone. bridge_deploy_status will then return the "no pending supply was recorded" fallback instead of auto-supplying.

This either needs to be documented prominently as a hard constraint ("the same provider instance must be reused across calls") or the pending state must be serialized to somewhere durable (a file, a passed-in store, etc.). At minimum the README should warn that a new instance forgets all pending deploys.

Fix this →


2. Compound supply() silently ignores recipient

utils.ts:339-344

if(params.protocol==="compound"){data=encodeFunctionData({abi: COMET_SUPPLY_ABI,functionName: "supply",args: [token,atomicAmount],// ← no recipient});}

The supply(address asset, uint amount) Comet function deposits from msg.sender into msg.sender's own position. There is no recipient argument here. If the caller passes recipient ≠ walletProvider.getAddress(), for Morpho the deposit goes to the intended recipient, but for Compound it silently goes to the caller's position instead. This is a silent semantic divergence.

Comet provides supplyTo(address dst, address asset, uint amount) for this purpose. The ABI in constants.ts and the encodeFunctionData call should use supplyTo when recipient != walletProvider.getAddress(), or the COMET_SUPPLY_ABI should be updated to include supplyTo and always use it.

Fix this →


3. Quoted outputAmount stored as supply target — mismatches actual fill

bridgeDeployActionProvider.ts:112-117

this.#pendingDeploys.set(this.#pendingKey(...),{
...
amount: deposit.outputAmount,// ← from quote, not actual fill
...
});

deposit.outputAmount is the quoted output. The actual fill amount may differ. When deployToProtocol later runs the balance preflight (balance < atomicAmount), it uses the quoted amount, which can either:

  • Block the supply if the fill was slightly less than quoted (e.g., fee changed between quote and fill)
  • Over-supply if the fill was more than quoted (e.g., the relayer undercut the fee)

The safer approach is to supply Math.min(balance, atomicAmount) — i.e., use whatever landed — or to drop the preflight comparison and let the protocol revert if underfunded.


🟡 Medium — Reliability / Security

4. depositId interpolated into URL without encoding

utils.ts:255-258

constresponse=awaitfetch(`${ACROSS_DEPOSIT_STATUS_API}?originChainId=${originChainId}&depositId=${depositId}`,

depositId (and originChainId) are user-supplied strings concatenated directly into the URL. If either contains &, =, or #, the query string is silently malformed. Use URLSearchParams:

consturl=newURL(ACROSS_DEPOSIT_STATUS_API);url.searchParams.set("originChainId",String(originChainId));url.searchParams.set("depositId",depositId);constresponse=awaitfetch(url.toString(),{method: "GET"});

Fix this →


5. Two test paths not covered

bridgeDeployActionProvider.test.ts

  • filled with no pending deploybridgeDeployStatus returns a "call deploy_on_destination manually" note when the map has no entry for the deposit. bridgeDeployActionProvider.ts:178-187 handles this but it is never exercised.
  • Supply fails on filledbridgeDeployActionProvider.ts:191-198 keeps the pending deploy for retry when deployResult.startsWith("Error"). This retry path has no test.

6. Floating-point slippage arithmetic

utils.ts:183-185

constslippageBps=((Number(formattedInput)-Number(formattedOutput))/Number(formattedInput))*10000;

Converting bigintformatUnitsNumber → arithmetic introduces floating-point rounding. For basis-point comparisons, prefer bigint arithmetic directly:

constslippageBps=Number(((quote.deposit.inputAmount-quote.deposit.outputAmount)*10000n)/quote.deposit.inputAmount);

(The existing across provider has the same issue — it is a pre-existing pattern, but worth fixing here.)


🟢 Low — Minor / Style

7. Dead validation in constructor (bridgeDeployActionProvider.ts:56-57)

constaccount=privateKeyToAccount(this.#privateKey asHex);if(!account)thrownewError("Invalid private key");

privateKeyToAccount from viem throws on an invalid key — it never returns null or undefined. The if (!account) branch is unreachable. (Same pattern in the upstream across provider.)


8. http() with no RPC URL (utils.ts:107)

transport: http(),

http() with no argument uses the chain's built-in public RPC, which is rate-limited. This is fine for occasional test runs but can fail under production load. (Same pattern in across provider — pre-existing.)


9. Token identifier inconsistency between actions

bridge_and_deploy accepts token as a symbol ("USDC"); deploy_on_destination accepts token as an address ("0x833..."). This is noted in the README, but the inconsistency creates a real risk that an LLM agent passes a symbol to the standalone deploy action and hits a silent failure (the Zod regex rejects non-address strings, so at least it fails loudly).

Consider adding a note to the deploy_on_destination action description explicitly warning: "token must be the destination-chain ERC-20 address, not the symbol".


Conventions Checklist

RuleStatus
All actions return Promise<string>; errors returned not thrown
Zod v4 schemas with .describe() on every field; no .strip()
supportsNetwork is an arrow-function property
Factory bridgeDeployActionProvider exported and re-exported from index.ts
README.md present
pnpm run changeset — patch, past tense, correct package
Tests present and mocking wallet provider
super("bridgeDeploy", []) constructor call
Network.chainId compared as string

Summary: The implementation is clean and well-documented. The two blocking issues before upstream merge are the Compound supplyTo vs supply recipient bug (#2) and the in-memory state loss documentation (#1). The URL encoding (#4) and slippage arithmetic (#6) are simple one-liners to fix. The missing test coverage (#5) should be added before the PR goes out.

- Compound: use supplyTo(dst, asset, amount) so the position is credited to
the recipient instead of msg.sender (silent divergence from Morpho).
- Auto-deploy: supply the amount that actually landed when a fill comes in
under the quoted output, instead of failing the stale-quote preflight;
deploy_on_destination stays strict for explicit calls.
- getDepositStatus: build the status URL with URLSearchParams.
- Slippage check now uses bigint arithmetic to avoid float rounding.
- Document the in-memory pending-deploy state (lost on re-instantiation) in
the README and on the field; clarify token is an address (not symbol) for
deploy_on_destination.
- Add tests for the filled-without-pending-record and supply-fails-retry paths.
https://claude.ai/code/session_013rnmDYrKDMctusY33uneiy
@zaryab2000Claude

Copy link
Copy Markdown
OwnerAuthor

Thanks for the review — addressed in 8633b59. Summary of what changed and why:

Fixed

Intentionally deferred (with rationale)


Generated by Claude Code

@zaryab2000

Copy link
Copy Markdown
OwnerAuthor

@claude conduct final review and confirm if the PR is accurate now?

@claude

claudeBot commented Jun 11, 2026

Copy link
Copy Markdown

Claude finished @zaryab2000's task in 1m 38s —— View job


Final Review — bridgeDeploy ActionProvider

Tasks


All seven addressed issues are correctly fixed. Here is the per-item confirmation:

Verified fixes

#IssueStatusEvidence
#2Compound supplyTo recipient✅ FixedCOMET_SUPPLY_ABI in constants.ts now includes supplyTo(address dst, address asset, uint256 amount); utils.ts:358–362 uses supplyTo with [params.recipient, token, atomicAmount]
#3Quoted vs actual fill✅ FixeddeployToProtocol takes allowPartial = false; auto-deploy path (bridge_deploy_status) passes true; supplies balance when partial; standalone deploy_on_destination stays strict (false)
#4URL encoding✅ Fixedutils.ts:256–259 uses new URL(ACROSS_DEPOSIT_STATUS_API) + url.searchParams.set(...)
#6Bigint slippage arithmetic✅ Fixedutils.ts:183–185: ((inputAmount - outputAmount) * 10000n) / inputAmount — no float conversion
#1In-memory state documented✅ FixedJSDoc on #pendingDeploys (bridgeDeployActionProvider.ts:47–56) and a dedicated "⚠️ Pending-supply state is in-memory only" section in README.md:23–40 both clearly state the instance-local constraint
coinbase#9Token address vs symbol warning✅ Fixeddeploy_on_destination action description says "NOT the token symbol"; README.md:74–76 also distinguishes bridge_and_deploy (symbol) from deploy_on_destination (address)
#5Missing test coverage✅ Fixedtest.ts:216–236 covers filled with no pending record; test.ts:238–270 covers supply-fails-on-filled retry retention

Intentionally deferred (confirmed reasonable)

One minor observation (non-blocking)

COMET_SUPPLY_ABI in constants.ts still includes the supply(address asset, uint256 amount) entry, but the code now always calls supplyTo. The supply entry is dead code in the execution path. The comment justifies its presence for documentation purposes, and since it cannot cause a runtime error, this is harmless — just worth noting for cleanliness.

Conventions checklist

RuleStatus
All actions return Promise<string>; errors returned not thrown
Zod v4 schemas with .describe() on every field; no .strip()
supportsNetwork is an arrow-function property
Factory bridgeDeployActionProvider exported and re-exported from index.ts
README.md present and documents EOA/two-step semantics
Changeset — @coinbase/agentkit, patch, past tense
Tests present, mocked wallet, 13 provider tests
super("bridgeDeploy", []) constructor call
Network.chainId compared as string

The PR is accurate. All blocking issues from the previous review have been correctly addressed. The one remaining item (supply ABI entry being unused) is non-blocking cosmetic cleanup. The implementation is clean, well-documented, and matches AgentKit conventions.

@zaryab2000Claude

Copy link
Copy Markdown
OwnerAuthor

Final review — confirmed accurate ✅

Re-reviewed the full provider on head 8633b59 and verified all the review fixes landed correctly.

Correctness — verified in code

  • Compound recipient:deployToProtocol now encodes supplyTo(recipient, asset, amount) (utils.ts:357-362), so the position is credited to the intended owner, matching Morpho's deposit(assets, receiver). ✅
  • Partial-fill handling: auto-deploy passes allowPartial=true and supplies min(requested, balance) when a fill lands under the quote; deploy_on_destination stays strict (exact amount or error). ✅
  • URL building:getDepositStatus uses URLSearchParams. ✅
  • Slippage: computed with bigint arithmetic. ✅
  • State + token docs: in-memory #pendingDeploys warning on the field and in the README; address-not-symbol note on deploy_on_destination. ✅
  • Error-return semantics, Promise<string>, Zod .describe() (no .strip()), arrow supportsNetwork, factory re-export — all intact.

CI — green on head 8633b59

  • 25/25 checks passing (lint-typescript, format-typescript, test-agentkit-typescript (18 & 20), check-package-lock, plus the python jobs).
  • mergeable_state: clean — no conflicts with feat/bridgeThenDeploy.

Local

  • 13 provider tests + 877 full agentkit suite green; lint / format / build clean.

One honest limitation worth recording (not a blocker, by design):
The auto-deploy can only supply funds the wallet custodies. If you set recipient on bridge_and_deploy to a third party, the bridge sends there and the wallet holds nothing, so the auto-supply returns a graceful "insufficient balance" (pending kept for retry) rather than silently doing nothing. With the default (recipient = sender) the full bridge→supply flow works as intended. This is consistent and non-lossy, and the README documents that recipient defaults to the sender.

Minor / optional: the PR description is slightly stale vs the latest commits — it still says "875 tests" and "Compound (Comet supply)" (now 877 tests and supplyTo). Happy to refresh the description if you'd like; the code and changeset are accurate.

Still intentionally deferred:#7 (dead if (!account)) and coinbase#8 (http() default RPC) — verbatim parity with the in-tree across provider; will change if you prefer divergence.

Net: the PR is accurate and merge-ready into feat/bridgeThenDeploy. Reminder that upstream (coinbase/agentkit) still requires the human-only steps from our checklist — commit signing, the docs-internal//fork-tooling strip, and a rebase on the latest main — before it goes out as the public contribution.


Generated by Claude Code

The Compound path always uses supplyTo to honor the recipient, so the
supply(asset, amount) entry was dead code. Drop it from COMET_SUPPLY_ABI.
https://claude.ai/code/session_013rnmDYrKDMctusY33uneiy
@zaryab2000Claude

Copy link
Copy Markdown
OwnerAuthor

Addressed the one new observation from the final review in 5a6c1fa: removed the now-unused supply(asset, amount) entry from COMET_SUPPLY_ABI — the Compound path always calls supplyTo, so it was dead code. ABI now contains only supplyTo.

No functional change; the dead-code path is gone. Verified: 13 provider tests + full agentkit suite green, lint / format / build clean.

No other new bugs were reported in the latest review (it confirmed all 7 prior fixes as correct). PR remains accurate and merge-ready into feat/bridgeThenDeploy.


Generated by Claude Code

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

Labels

action providerdocumentationImprovements or additions to documentationtypescript

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@zaryab2000@claude