-
Notifications
You must be signed in to change notification settings - Fork 20
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
Init Wallet SDK #956
base: main
Are you sure you want to change the base?
Init Wallet SDK #956
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
packages/wallets/src/sdk.ts
Outdated
...args: WalletTypeToArgs["solana-smart-wallet"] | ||
): Promise<SolanaSmartWallet>; | ||
public async getOrCreateWallet( | ||
type: "solana-mpc-wallet", |
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.
Looks awkward, and a bit harder to maintain, but this will give the consumers great DX:
const wallet = await sdk.getOrCreateWallet("evm-smart-wallet", adminSigner: …);
// ^ this will be typed as `EVMSmartWallet` ^ this will have full autocomplete
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.
i think u can just make another type map like WalletTypeToWallet
to use as the return type on the actual implementation func, and it should do the same thing without needing to define all these
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.
addressed below
packages/wallets/src/sdk.ts
Outdated
public async getOrCreateWallet<WalletType extends keyof WalletTypeToArgs>( | ||
_type: WalletType, | ||
..._args: WalletTypeToArgs[WalletType] | ||
): Promise<EVMSmartWallet | EVMMPCWallet> { |
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.
like this
public async getOrCreateWallet<WalletType extends keyof WalletTypeToArgs>( | |
_type: WalletType, | |
..._args: WalletTypeToArgs[WalletType] | |
): Promise<EVMSmartWallet | EVMMPCWallet> { | |
public async getOrCreateWallet<WalletType extends keyof WalletTypeToArgs>( | |
_type: WalletType, | |
..._args: WalletTypeToArgs[WalletType] | |
): Promise<WalletTypeToWallet[WalletType]> { |
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.
nice, this simplifies things a lot! fixed in 9abda4d
}) => Promise<Hex>; | ||
} | ||
|
||
export class EVMSmartWallet implements ViemWallet { |
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.
you should check the goat repo to see if there is overlap/ can we use this wallet client within goat
we have too many wallet client definitions already...
https://github.com/goat-sdk/goat/tree/main/typescript/packages/wallets
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.
Looked into it. Some things are similar, others are different (e.g. we don't need resolveAddress
here).
Probably best to keep separate to not have a tight dependency between two different projects.
}) => Promise<string>; // Returns transaction signature | ||
} | ||
|
||
export class SolanaSmartWallet implements SolanaWallet { |
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.
same about checking goat repo
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.
addressed above
packages/wallets/src/evm/chains.ts
Outdated
const SmartWalletTestnet = { | ||
BASE_SEPOLIA: Blockchain.BASE_SEPOLIA, | ||
POLYGON_AMOY: Blockchain.POLYGON_AMOY, | ||
OPTIMISM_SEPOLIA: Blockchain.OPTIMISM_SEPOLIA, | ||
ARBITRUM_SEPOLIA: Blockchain.ARBITRUM_SEPOLIA, | ||
} as const; | ||
type SmartWalletTestnet = ObjectValues<typeof SmartWalletTestnet>; | ||
const SMART_WALLET_TESTNETS: readonly SmartWalletChain[] = objectValues(SmartWalletTestnet); | ||
|
||
const SmartWalletMainnet = { | ||
BASE: Blockchain.BASE, | ||
POLYGON: Blockchain.POLYGON, | ||
OPTIMISM: Blockchain.OPTIMISM, | ||
ARBITRUM: Blockchain.ARBITRUM, | ||
} as const; | ||
type SmartWalletMainnet = ObjectValues<typeof SmartWalletMainnet>; | ||
const SMART_WALLET_MAINNETS: readonly SmartWalletChain[] = objectValues(SmartWalletMainnet); |
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.
i think i would just define this as like SMART_WALLET_SUPPORTED_TESTNETS = [Blockchain.BASE_SEPOLIA, ...]
rather than an object and having to re-define all the names of the blockchains
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.
agree, fixed in 4e3de34
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.
There are plenty of tooling to generate the code based on OpenAPI spec. Went with HeyAPI:
- low-level API (easy to build a high-level SDK around it)
- can set base URL dynamically
- has a plugin for zod (not used currently)
fetch
-based (no runtime dependencies likeaxios
)- generates both runtime code and the typings
type CreateWalletParams = CreateWalletDto; | ||
type CreateWalletResponse = WalletV1Alpha2ResponseDto; | ||
type GetWalletResponse = WalletV1Alpha2ResponseDto; | ||
|
||
type CreateTransactionParams = CreateTransactionDto; | ||
type CreateTransactionResponse = WalletsV1Alpha2TransactionResponseDto; | ||
type ApproveTransactionParams = SubmitApprovalDto; | ||
type ApproveTransactionResponse = WalletsV1ControllerGetTransaction4Response; | ||
type GetTransactionResponse = WalletsV1Alpha2TransactionResponseDto; | ||
|
||
type CreateSignatureParams = CreateSignatureRequestDto; | ||
type CreateSignatureResponse = WalletsV1Alpha2SignatureResponseDto; | ||
type ApproveSignatureParams = SubmitApprovalDto; | ||
type ApproveSignatureResponse = WalletsV1Alpha2SignatureResponseDto; | ||
type GetSignatureResponse = WalletsV1Alpha2SignatureResponseDto; | ||
|
||
type GetTransactionsResponse = WalletsV1Alpha2TransactionsResponseDto; | ||
type GetNftsResponse = Nftevm | Nftsol; | ||
type GetBalanceResponse = WalletBalanceResponseDto; |
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.
Renaming the types before the export to keep them aligned with ApiClient
methods
headers: { | ||
"X-API-KEY": this.apiKey, | ||
}, |
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.
there seems to be some redundancy here, as we will need to pass this on every call
Any way to abstract that out s e.g. the client takes care of adding this header?
|
||
constructor( | ||
private readonly apiKey: string, | ||
private readonly jwt?: string |
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.
the jwt can change over time (e.g. when the user refreshes the token). im not sure this ap client supports jwt mutations?
type GetNftsResponse = Nftevm | Nftsol; | ||
type GetBalanceResponse = WalletBalanceResponseDto; | ||
|
||
class ApiClient { |
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.
I thought we had already something like a api client class in base SDK. Did you check? Would it be possible to reuse parts of that?
throw new Error("Invalid API key"); | ||
} | ||
if (validationResult.usageOrigin === APIKeyUsageOrigin.CLIENT && !jwt) { | ||
throw new Error("JWT is required for client API key"); |
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.
Im nt sure jwt is always needed - why did you reach that conclusion?
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.
I think it depends on the specific API you;re calling
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.
Beyond the discussions happening on Slack around using CrossmintApiClient
, looking good!
import { defineConfig } from "@hey-api/openapi-ts"; | ||
|
||
export default defineConfig({ | ||
input: "src/openapi.json", |
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.
can we not fetch this from a url so it's always up to date?
Blockchain.ARBITRUM_SEPOLIA, | ||
]; | ||
|
||
const SMART_WALLET_MAINNET_CHAINS = [Blockchain.BASE, Blockchain.POLYGON, Blockchain.OPTIMISM, Blockchain.ARBITRUM]; |
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.
don't we support more of them?
|
||
import type { SmartWalletChain } from "./chains"; | ||
|
||
export interface ViemWallet { |
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.
interesting that viem doesnt export smth like this haha
Description
Initiates a new Wallet SDK (
@crossmint/wallets-sdk
) and defines the high-level project structure.Also introduces code generation based on the OpenAPI schema using HeyAPI and a high-level API client.
Example (to be used internally):
Test plan