Conversation
… ETH, and we can't give spend permission for that)
| // In Node.js or other environments, use fetch as-is | ||
| return fetch; | ||
| }; | ||
|
|
There was a problem hiding this comment.
Fixes the bug in the browser environment outlined in the comments above
| protected logger: Logger; | ||
|
|
||
| constructor(baseRPCUrl: string, sourceSecretKey: Hex, logger?: Logger) { | ||
| constructor(baseRPCUrl: string, walletClient: WalletClient, logger?: Logger) { |
There was a problem hiding this comment.
This is a slightly worse devex, since we're now requiring the caller to do privateKeyToAccount(sourceSecretKey), but the upside is that this now supports non-EOA wallets, so it seems worth the tradeoff. Generally speaking, external devs aren't going to instantiate this class directly anyways - they're going to create a BaseAccount, which still does take a secret key as the constructor arg.
| } | ||
| }); | ||
| const provider = sdk.getProvider(); | ||
| await sdk.getProvider().request({ method: 'wallet_connect' }); |
There was a problem hiding this comment.
Should we be using the provider above or do we need to get it again?
There was a problem hiding this comment.
Ah, good catch - will update
|
|
||
| const DEFAULT_ALLOWANCE = 10n; | ||
| const DEFAULT_PERIOD_IN_DAYS = 7; | ||
| const PAYMASTER_URL = 'https://api.developer.coinbase.com/rpc/v1/base/snPdXqIzOGhRkGNJvEHM5bl9Hm3yRO3m'; |
There was a problem hiding this comment.
Should this be configurable via Env Vars? I'm wondering if we might want different dev / staging / production values?
There was a problem hiding this comment.
They're settable via params - eg miniapp-example sets them here. I think that's probably adequate?
There was a problem hiding this comment.
The PAYMASTER_URL is not settable. Maybe that's ok?
There was a problem hiding this comment.
That should be ok, at least for now. The paymaster is US, and we don't want devs to bring their own paymasters at this point.
| calls: [{ | ||
| to: smartWallet.address, | ||
| value: 0n, | ||
| data: '0x' as `0x${string}` |
There was a problem hiding this comment.
FYI, packages/atxp-client/src/types.ts has a Hex type. Maybe we should use it?
There was a problem hiding this comment.
Fair - viem also defines a Hex type with the same format. Updated
| import { prepareSpendCallData } from '@base-org/account/spend-permission'; | ||
|
|
||
| // Helper function to convert to base64url that works in both Node.js and browsers | ||
| function toBase64Url(data: string): string { |
There was a problem hiding this comment.
Nit, this isn't producing a URL, it's just the base64 string. Not-blocking...
There was a problem hiding this comment.
It produces a 'base64url' encoded string; I thought the naming was evocative of toBase64
Introduces a new
BaseAppAccount, which:Usage: