Python & TS Crossmint Solana Smart Wallet #332
Conversation
…d-crossmint-wallet' into devin/1737473874-add-crossmint-wallet
…allet' Co-Authored-By: [email protected] <[email protected]>
…int plugin Co-Authored-By: [email protected] <[email protected]>
Co-Authored-By: [email protected] <[email protected]>
Co-Authored-By: [email protected] <[email protected]>
…d-crossmint-wallet' into devin/1737473874-add-crossmint-wallet
…rs.py Co-Authored-By: [email protected] <[email protected]>
…allet' into devin/1737473874-add-crossmint-wallet
python/src/wallets/crossmint/goat_wallets/crossmint/solana_smart_wallet.py
Show resolved
Hide resolved
| return { | ||
| "decimals": 18, | ||
| "symbol": "ETH", | ||
| "name": "Ethereum", |
There was a problem hiding this comment.
This matches the TypeScript implementation:
async balanceOf(address: string) {
const resolvedAddress = await this.resolveAddress(address);
const balance = await this.#viemClient.getBalance({
address: resolvedAddress,
});
return {
decimals: 18,
symbol: "ETH",
name: "Ethereum",
value: formatUnits(balance, 18),
inBaseUnits: balance.toString(),
};
}
|
|
||
| return {"value": result} | ||
|
|
||
| def balance_of(self, address: str) -> Balance: |
There was a problem hiding this comment.
Don't we need to know balances for other fungible tokens?
There was a problem hiding this comment.
This matches the TS implementation decision to use the RPC endpoint to check for native chain token balance. Do you think instead we should implement in both to obtain the balance from the CrossMint API (which supports [SOME] tokens)?
I wonder if the correct approach wuold be that plugins deal with this kind of contract-centric functions.
|
|
||
| @property | ||
| def has_custodial_signer(self) -> bool: | ||
| return isinstance(self._signer, str) |
There was a problem hiding this comment.
Not for this PR! But I think deviating from the structure of signers we use to create the wallet to a Signer type that only lives in this package:
CustodialSigner = str
KeyPairSigner = TypedDict('KeyPairSigner', {
'secretKey': str,
'address': str
})
Signer = Union[CustodialSigner, KeyPairSigner]is quite confusing
There was a problem hiding this comment.
Actually "address" shouldn't be there. In general I agree that Python's typing is a bit awkward here, it is the only way I came up with to constrain the signer definition though. I have changed a little now, to provide some more runtime detection.
python/examples/langchain/crossmint/solana_smart_wallet_example.py
Outdated
Show resolved
Hide resolved
| }) | ||
| print(f"Created wallet with address: {wallet.get_address()}") | ||
|
|
||
| signer_response = wallet.register_delegated_signer( |
There was a problem hiding this comment.
We cannot register a delegated signer without the approval of the admin signer which i thiiink we're not adding anywhere right?
There was a problem hiding this comment.
I hope I've understood and implemented the right process now!
python/examples/langchain/crossmint/examples/delegated_signer.py
Outdated
Show resolved
Hide resolved
…olana-smart-wallet
…olana-smart-wallet
| adminSigner: | ||
| | { | ||
| type: "solana-keypair"; | ||
| secretKey: string; |
There was a problem hiding this comment.
Wouldn't it be more natural to receive a Keypair?
| private static deriveAddressFromSecretKey(secretKey: string): string { | ||
| try { | ||
| const decoded = bs58.decode(secretKey); | ||
| const keyPair = nacl.sign.keyPair.fromSecretKey(decoded); | ||
| return bs58.encode(Buffer.from(keyPair.publicKey)); | ||
| } catch (error) { | ||
| throw new Error(`Invalid secret key: ${error}`); | ||
| } | ||
| } |
There was a problem hiding this comment.
If we switch to keypair it;d be just adminsigner.keypair.publicKey.toBase58()
| async sendRawTransaction(transaction: string): Promise<{ hash: string }> { | ||
| try { | ||
| const { id: transactionId } = await this.#api.createSolanaTransaction(this.#locator, transaction); | ||
| while (true) { | ||
| const latestTransaction = await this.#api.checkTransactionStatus(this.#locator, transactionId); | ||
| if (latestTransaction.status === "success") { | ||
| return { | ||
| hash: latestTransaction.onChain?.txId ?? "", | ||
| }; | ||
| } | ||
| if (latestTransaction.status === "failed") { | ||
| throw new Error(`Transaction failed: ${latestTransaction.error}`); | ||
| } | ||
| if (latestTransaction.status === "awaiting-approval") { | ||
| if (this.#adminSigner.type === "solana-keypair") { | ||
| const message = latestTransaction.approvals?.pending?.[0]?.message; | ||
| if (message) { | ||
| await this.sendApprovals(transactionId, message, bs58.decode(this.#adminSigner.secretKey)); | ||
| } | ||
| } | ||
| } | ||
| await new Promise((resolve) => setTimeout(resolve, 2000)); | ||
| } | ||
| } catch (error) { | ||
| throw new Error(`Failed to send raw transaction: ${error}`); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
All send transactions functions should receive a list of additional signers that, even not being the admin signer, will be needed for sending the transaction. (for example, the wallet could send a transaction to transfer a token out of other wallet, in which case we would need this other wallet signature). Or also like the sign from an authority to perform operations regarding an NFT. You could copy this logic from this helper https://github.com/Paella-Labs/crossbit-main/blob/29818933454fc41873db7ed7cd213efdd10dc563/apps/crossmint-nextjs/src/playwright/integration/wallets/api/2022-06-09/transactions/solana-smart-wallet/utils/crossmint-helpers.ts#L52
| async sendTransaction({ | ||
| instructions, | ||
| addressLookupTableAddresses = [], | ||
| }: SolanaTransaction): Promise<{ hash: string }> { |
There was a problem hiding this comment.
Regarding the comment below, it could be
| async sendTransaction({ | |
| instructions, | |
| addressLookupTableAddresses = [], | |
| }: SolanaTransaction): Promise<{ hash: string }> { | |
| async sendTransaction({ | |
| instructions, | |
| addressLookupTableAddresses = [], | |
| }: SolanaTransaction, additionalSigners: Array<Keypair>): Promise<{ hash: string }> { |
| @@ -616,7 +670,10 @@ describe("CrossmintWalletsAPI", () => { | |||
| ); | |||
|
|
|||
| await expect( | |||
| api.waitForTransaction("wallet123", "tx123", { interval: 100, maxAttempts: 2 }), | |||
| api.waitForTransaction("wallet123", "tx123", { | |||
| interval: 100, | |||
There was a problem hiding this comment.
Nit: 100 may be overkill. Non blocking!
| def send_approvals( | ||
| self, | ||
| transaction_id: str, | ||
| message: str, | ||
| signer_private_key: bytes | ||
| ) -> None: | ||
| """Send approval signatures for a pending transaction. | ||
|
|
||
| Args: | ||
| transaction_id: The ID of the transaction to approve | ||
| message: The message to sign (usually transaction data) | ||
| signer_private_key: The private key bytes of the signer | ||
|
|
||
| Raises: | ||
| ValueError: If signature generation or approval submission fails | ||
| """ | ||
| try: | ||
| # Generate detached signature using nacl | ||
| signing_key = nacl.signing.SigningKey(signer_private_key) | ||
| signature = signing_key.sign(base58.b58decode(message)).signature | ||
| encoded_signature = base58.b58encode(signature).decode() | ||
|
|
||
| # Send approval with signature | ||
| approvals: List[Dict[str, str]] = [{ | ||
| "signer": "solana-keypair:" + self.derive_address_from_secret_key(self._admin_signer["secretKey"]) if "secretKey" in self._admin_signer else "", | ||
| "signature": encoded_signature | ||
| }] | ||
|
|
||
| self._client.approve_transaction( | ||
| self._locator, | ||
| transaction_id, | ||
| approvals=approvals | ||
| ) | ||
| except Exception as e: | ||
| raise ValueError(f"Failed to send transaction approval: {str(e)}") | ||
|
|
There was a problem hiding this comment.
Same! We'd need optional additional signers
| Args: | ||
| signer: The locator of the delegated signer | ||
| expires_at: Optional expiry date in milliseconds since UNIX epoch | ||
| permissions: Optional list of ERC-7715 permission objects |
There was a problem hiding this comment.
Lets delete permissions from hereelete!
| return address; | ||
| } | ||
|
|
||
| export class SolanaSmartWalletClient extends SolanaWalletClient { |
There was a problem hiding this comment.
I think we're missing the delegated signer functionality here right?
There was a problem hiding this comment.
Still not live so not critical but we'll need it
No description provided.