-
Notifications
You must be signed in to change notification settings - Fork 5
feat: implement approval gate for wallet commands and add SSE transpo… #16
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,6 +11,7 @@ import { formatChainId, formatChainIds } from "../lib/chains"; | |
| import { c } from "../lib/color"; | ||
| import { openBrowser } from "../lib/browser"; | ||
| import { selectOption, prompt } from "../lib/prompt"; | ||
| import { withApprovalGate } from "../lib/walletGate"; | ||
| import qrcode from "qrcode-terminal"; | ||
|
|
||
| export function registerWalletCommands(program: Command): void { | ||
|
|
@@ -37,10 +38,8 @@ export function registerWalletCommands(program: Command): void { | |
| .action(async (opts, cmd) => { | ||
| const json = isJson(cmd); | ||
| try { | ||
| const provider = await createProviderAdapter(); | ||
| const signature = await provider.signMessage( | ||
| Number(opts.chainId), | ||
| opts.message | ||
| const signature = await withApprovalGate((provider) => | ||
| provider.signMessage(Number(opts.chainId), opts.message) | ||
| ); | ||
| outputResult(json, { signature }); | ||
| } catch (err) { | ||
|
|
@@ -82,10 +81,8 @@ export function registerWalletCommands(program: Command): void { | |
| ); | ||
| } | ||
|
|
||
| const provider = await createProviderAdapter(); | ||
| const signature = await provider.signTypedData( | ||
| Number(opts.chainId), | ||
| typedData | ||
| const signature = await withApprovalGate((provider) => | ||
| provider.signTypedData(Number(opts.chainId), typedData) | ||
| ); | ||
| outputResult(json, { signature }); | ||
| } catch (err) { | ||
|
|
@@ -112,16 +109,6 @@ export function registerWalletCommands(program: Command): void { | |
| ); | ||
| } | ||
|
|
||
| const provider = await createProviderAdapter(); | ||
| const supportedChainIds = await provider.getSupportedChainIds(); | ||
| if (!supportedChainIds.includes(chainId)) { | ||
| throw new CliError( | ||
| `Unsupported chain ID: ${formatChainId(chainId)}`, | ||
| "VALIDATION_ERROR", | ||
| `Supported chains: ${formatChainIds(supportedChainIds)}` | ||
| ); | ||
| } | ||
|
|
||
| if (!isAddress(opts.to)) { | ||
| throw new CliError( | ||
| `Invalid --to address: ${opts.to}`, | ||
|
|
@@ -151,10 +138,20 @@ export function registerWalletCommands(program: Command): void { | |
| } | ||
| } | ||
|
|
||
| const transactionHash = await provider.sendTransaction(chainId, { | ||
| to: opts.to, | ||
| ...(opts.data !== undefined ? { data: opts.data } : {}), | ||
| ...(value !== undefined ? { value } : {}), | ||
| const transactionHash = await withApprovalGate(async (provider) => { | ||
| const supportedChainIds = await provider.getSupportedChainIds(); | ||
| if (!supportedChainIds.includes(chainId)) { | ||
| throw new CliError( | ||
| `Unsupported chain ID: ${formatChainId(chainId)}`, | ||
| "VALIDATION_ERROR", | ||
| `Supported chains: ${formatChainIds(supportedChainIds)}` | ||
| ); | ||
| } | ||
| return provider.sendTransaction(chainId, { | ||
| to: opts.to, | ||
| ...(opts.data !== undefined ? { data: opts.data } : {}), | ||
| ...(value !== undefined ? { value } : {}), | ||
| }); | ||
| }); | ||
| outputResult(json, { transactionHash }); | ||
| } catch (err) { | ||
|
|
@@ -368,9 +365,9 @@ export function registerWalletCommands(program: Command): void { | |
| if (!json && isTTY()) { | ||
| process.stdout.write(" Signing wallet verification..."); | ||
| } | ||
| signature = await provider.signMessage( | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Topup command creates redundant expensive provider adapterLow Severity In the Additional Locations (1)Reviewed by Cursor Bugbot for commit 14d8f65. Configure here. |
||
| chainId, | ||
| initResult.data.challenge | ||
| const challenge = initResult.data.challenge; | ||
| signature = await withApprovalGate((p) => | ||
| p.signMessage(chainId, challenge) | ||
| ); | ||
| if (!json && isTTY()) { | ||
| console.log(` ${c.green("✓")}`); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| import { | ||
| STREAMS, | ||
| type IEvmProviderAdapter, | ||
| } from "@virtuals-protocol/acp-node-v2"; | ||
| import { createProviderAdapter, createSseTransport } from "./agentFactory"; | ||
|
|
||
| export async function withApprovalGate<T>( | ||
| fn: (provider: IEvmProviderAdapter) => Promise<T> | ||
| ): Promise<T> { | ||
| const provider = await createProviderAdapter(); | ||
| const transport = await createSseTransport(provider, [STREAMS.WALLET]); | ||
| try { | ||
| return await fn(provider); | ||
| } finally { | ||
| void Promise.resolve(transport.disconnect()).catch(() => {}); | ||
| } | ||
| } |


There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Redundant getSupportedChainIds call in send-transaction path
Low Severity
In the
send-transactioncommand,provider.getSupportedChainIds()is called twice on the same provider instance within a singlewithApprovalGateinvocation — once insidecreateSseTransport(to populateproviderSupportedChainIdsin the transport context) and again in the callback at line 142 for chain validation. Since both calls happen on the same provider, the second call is redundant work (likely an additional network round-trip). ThewithApprovalGateAPI could expose the already-fetched chain IDs to avoid this duplication.Reviewed by Cursor Bugbot for commit 23a4560. Configure here.