search

ChainAgnostic / namespaces

Chain-Agnostic Namespaces host informative specs and profiles of CAIPs (Chain-Agnostic Improvement Proposals) per blockchain ecosystem
https://namespaces.chainAgnostic.org/
44 stars 57 forks source link

solana/caip25.md initial draft #102

Closed bumblefudge closed 6 days ago

bumblefudge commented 4 months ago

Only in draft because I'm waiting on real-world examples for the ## Examples section from a prototyping team. Big thanks to the many contributors so far.

Can't assign for review without adding you to the org but @jnwng @arein @glitch review from any of you would be great.

bumblefudge commented 3 months ago

@0xproflupin anything jump out at you as patently wrong? @obstropolos said you might know a thing or two about Solana interop

vsakos commented 5 days ago

@bumblefudge If I understand the spec correctly, the SolanaTransactionVersion and SolanaTransactionCommitment fields are locked to a specific value for the whole dApp <-> wallet session.

I don't think that declaring a transaction version makes sense for the whole session, because most wallets support both versioned (only v0 for now, but we will probably have v1, v2, etc. in the future) and non-versioned (legacy) transactions, and the dApp might want to use both versions in the same session. So, limiting the session to a single version might not be ideal. That's the reason why the Wallet Standard exposes the supportedTransactionVersions field, so the dApp knows which versions are supported by the wallet, but is free to use any.

As for the commitment, unlike other chains, dApps on Solana are expected to use their own JSON-RPC endpoints for fetching data. Currently, the only method that can accept the commitment is the signAndSendTransaction method. Just like with the transaction version, the dApp might want to use different finality for different transactions. For example, they can use confirmed for low-risk transactions to have transaction confirmations faster and finalized for high-risk transactions (like transferring larger token amounts) where it's acceptable to wait longer for the transaction to be confirmed.

bumblefudge commented 5 days ago

@vaskos thank you very much for this review-- I wish it had come in earlier!

@bumblefudge If I understand the spec correctly, the SolanaTransactionVersion and SolanaTransactionCommitment fields are locked to a specific value for the whole dApp <-> wallet session.

I don't think that declaring a transaction version makes sense for the whole session, because most wallets support both versioned (only v0 for now, but we will probably have v1, v2, etc. in the future) and non-versioned (legacy) transactions, and the dApp might want to use both versions in the same session. So, limiting the session to a single version might not be ideal. That's the reason why the Wallet Standard exposes the supportedTransactionVersions field, so the dApp knows which versions are supported by the wallet, but is free to use any.

I might have made a terrible mistake, in that case-- the wallet should be declaring supportedTransactionVersions not SolanaTransactionVersion (must have misread the Solana specs here). The intention is not to force the dapp to always use one or the other version, but to tell the CAIP-25 relay provider (e.g., Wallet Connect, Snaps, or equivalent) upfront what value to expose to the dapp via Wallet Standard, since the CAIP-25 provider is trying to honor the WS interface loyally be representing/emulating a wallet.

As for the commitment, unlike other chains, dApps on Solana are expected to use their own JSON-RPC endpoints for fetching data. Currently, the only method that can accept the commitment is the signAndSendTransaction method. Just like with the transaction version, the dApp might want to use different finality for different transactions. For example, they can use confirmed for low-risk transactions to have transaction confirmations faster and finalized for high-risk transactions (like transferring larger token amounts) where it's acceptable to wait longer for the transaction to be confirmed.

Same-- I thought the wallet might have some way of constraining or suggesting only finalized, for example, but I might also have misread the Solana docs here as well on what the wallet has the option to request or constrain. Should this be dropped altogether from the things a wallet can declare, and is purely a dapp<>RPC concern?

vsakos commented 4 days ago

@bumblefudge If the idea is to be fully compatible with the Wallet Standard, I think SolanaTransactionVersion (or some alternative name) should be a list of available versions, just like in the WS itself, instead of a single transaction version.

From the dApp's standpoint, a Solana wallet is just a signer and nothing more - with the exception of signAndSendTransaction, which is the only method that makes use of the commitment param. However, most of the Solana dApps still choose to use signTransaction and relay the transaction to their own RPC instead of relying on the wallet to do it. But in any case, the practice is to fully honor the dApp's request (with default values and fallbacks for missing params etc.)

glitch-txs commented 4 days ago

Thanks so much @vsakos for bringing this up! Let's revisit this, could we open a new PR for modifications? @bumblefudge

We haven't started implementing this in the WalletConnect protocol yet, but it would be nice to start ASAP once this is ready to go to prod 🙏

bumblefudge commented 2 days ago

@bumblefudge If the idea is to be fully compatible with the Wallet Standard, I think SolanaTransactionVersion (or some alternative name) should be a list of available versions, just like in the WS itself, instead of a single transaction version.

you are correct, @vsakos, it seems I completely missed that the wallet exposes an array, not a maximum-value (with the assumption of all lower/older values being implicit).

what I find odd is that in the example linked above, where, as you say, commitment would in fact matter and a dapp might want to know what commitments it could send via the wallet, the example doesn't include that variable... is that declared elsewhere? or does the wallet standard not contemplate wallets supporting a subset of commitment modes in the first place? Is it not declarable because it's a transaction-level concern already encoded in each transaction?

i'll keep digging and tag you in a PR to fix, in any case. If I can't figure out the wallet-side commitment syntax I'll just leave it out for now.

vsakos commented 2 days ago

@bumblefudge The commitment definitely matters, but I would not limit it in any way. Wallets should support everything the blockchain itself supports. After all, it's just an enum/string that gets forwarded to the RPC. https://solana.com/docs/rpc#configuring-state-commitment.

The commitment itself is just an indicator of which state of the blockchain should be considered for reading data or relaying a transaction. For example, if you are receiving some tokens in transaction 1 and you want to send those tokens to another wallet with transaction 2, and if tx1 is confirmed but does not have enough confirmations to be considered finalized, if you relay tx2 with confirmed it will go through but it will fail with finalized commitment because you still don't have those tokens in the finalized state.

This is why, in my opinion, it makes more sense to make this parameter per request/operation and not limit the dApp or the wallet in any way.

bumblefudge commented 2 days ago

@vsakos -- eloquently explained, thanks so much. I came to more or less the same conclusion after re-reading the RPC docs and the wallet adapter repo. I tagged you in the update PR, but it's pretty much what you recommended here and nothing new/extra beyond that.