Conversation
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Changes requested ❌
Reviewed everything up to eb6b233 in 2 minutes and 2 seconds. Click for details.
- Reviewed
455lines of code in1files - Skipped
0files when reviewing. - Skipped posting
2draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. SPEC1.2.md:275
- Draft comment:
Clarify the requirement for transaction serialization/deserialization. Recommend explicitly stating that wallet implementations should use the solana-web3.js serialization methods to ensure consistent account key ordering. - Reason this comment was not posted:
Comment looked like it was already resolved.
2. SPEC1.2.md:78
- Draft comment:
Typo: Missing preposition. Consider changing "can be used locate transactions" to "can be used to locate transactions". - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 0% vs. threshold = 50% While the comment identifies a real grammatical issue, the review rules emphasize focusing on code changes and logic issues, not minor text fixes. This is a new documentation file, but the rules still suggest avoiding purely informative comments that don't require clear code changes. The comment is technically correct, but is it important enough to warrant interrupting the PR author's workflow? The meaning is clear even with the missing word. While clarity in documentation is valuable, this type of minor grammatical fix could be handled in a subsequent cleanup PR or directly committed. It doesn't materially impact understanding. The comment should be removed as it addresses a minor grammatical issue rather than a substantive code or logic problem.
Workflow ID: wflow_q0ont1jPM8C327Z5
You can customize 
Co-authored-by: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com>
Co-authored-by: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com>
Co-authored-by: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com>
amilz
left a comment
amilz
left a comment
There was a problem hiding this comment.
Looks good--added a few comments wrt feepayer/relayer. Most are optional--we can discuss, but I do think we need to add fee-payment-destination based on recent changes to Kora.
| - If present, wallets should include this additional payment to the fee payer service as part of the composed flow. The mechanism to pay the relayer may require switching to the Transaction Request flow or composing multiple transfers; concrete implementation is wallet-defined. | ||
| - If multiple entries are provided, wallets should select the entry matching the chosen payment currency when possible; otherwise wallets may pick any supported entry. | ||
| - If unsupported, wallets may ignore `fee-payer-fee` and proceed. | ||
|
|
There was a problem hiding this comment.
I think we need to add an optional fee-payment-destination parameter--we recently added the ability for Kora Operators to configure an optional payment address that may differ from the signer address.
Alternatively, maybe we could add an optional destination to fee-payer-fee
fee-payer-fee=<currency>,<amount>[,<destination>][;...]| ### Fee Payment Destination | |
| A single `fee-payment-destination` field is allowed as an optional query parameter. The value must be the base58-encoded public key of the account that should receive the relayer | |
| fee payment. | |
| Rules: | |
| - When present with `fee-payer` and `fee-payer-fee`, wallets should send the fee payment to this address instead of the `fee-payer` address | |
| - This supports relayers that use separate payment collection addresses (e.g., Kora with payment address configuration) | |
| - If omitted, wallets should default to sending payment to the `fee-payer` address |
| Where each entry specifies the currency and amount to be paid to the relayer for facilitating the transaction. The currency rules match `payment-options` (`sol` or SPL mint address). Amounts are user units and must comply with currency precision. | ||
|
|
||
| Rules: | ||
| - If present, wallets should include this additional payment to the fee payer service as part of the composed flow. The mechanism to pay the relayer may require switching to the Transaction Request flow or composing multiple transfers; concrete implementation is wallet-defined. |
There was a problem hiding this comment.
Adding here that it should be a payment instruction (Kora must see payment in the transaction it is signing rather than a separate transaction).
I'm not 100% clear to me what's meant by "Transaction Request flow or composing multiple transfers" -- but if this is suggesting multiple transactions we need to nix/reword.
| - If present, wallets should include this additional payment to the fee payer service as part of the composed flow. The mechanism to pay the relayer may require switching to the Transaction Request flow or composing multiple transfers; concrete implementation is wallet-defined. | |
| - If present, wallets should include this additional payment instruction to the fee payer service as part of the composed flow. The mechanism to pay the relayer may require switching to the Transaction Request flow or composing multiple transfers; concrete implementation is wallet-defined. |
| A single `fee-payer-server` field is allowed as an optional query parameter. If this option is provided, then the `fee-payer` field is required. The value must be an absolute HTTPS URL designating the relayer service endpoint that will accept and process the fee-payer flow for this transaction. This relayer should adhere to the sRFC-34 specification like Kora. | ||
|
|
||
| Rules: | ||
| - When `fee-payer` is present, wallets that support relaying should use `fee-payer-server` to direct relay submission to the specified service. |
There was a problem hiding this comment.
I think best if we rely on signTransaction and signTransactionIfPaid rather than signAndSendTransaction (which doesn't include payment validation)
| - When `fee-payer` is present, wallets that support relaying should use `fee-payer-server` to direct relay submission to the specified service. | |
| - When `fee-payer` is present, wallets that support relaying should use `fee-payer-server` to retrieve a fully-signed based-64 transaction. | |
| - For non-payment flows (when `fee-payer-fee` is NOT present), wallets should use `signTransaction` sRFC-34 `signTransaction` RPC method. This will return a fully signed base-64 transaction. | |
| - For payment flows (when `fee-payer-fee` IS present), wallets should use `signTransactionIfPaid` sRFC-34 `signTransaction` RPC method. This will return a fully signed base-64 transaction. |
|
|
||
| Rules: | ||
| - If provided, wallets that support fee relaying should set the transaction `feePayer` to the specified account and must not attempt to sign as that account. | ||
| - Wallets may submit the partially signed transaction to a configured relayer service (e.g., a Kora server) to finalize and broadcast. Relayer configuration and discovery are wallet-defined and out of scope for this specification. |
There was a problem hiding this comment.
Happy to chat on this, but I think best if we rely on signTransaction and signTransactionIfPaid rather than signAndSendTransaction (which doesn't include payment validation). Both return fully signed base64 txns.
| - Wallets may submit the partially signed transaction to a configured relayer service (e.g., a Kora server) to finalize and broadcast. Relayer configuration and discovery are wallet-defined and out of scope for this specification. | |
| - Wallets may submit the partially signed transaction to a configured relayer service (e.g., a Kora server) to fetch a fully signed transaction. Relayer configuration and discovery are wallet-defined and out of scope for this specification. |
| - If `fee-payer-server` is present without `fee-payer`, wallets must ignore it. | ||
| - Best practice: keep `fee-payer` as a pure public key and specify the service endpoint via `fee-payer-server`. | ||
|
|
||
| ### Fee Payer Fee |
There was a problem hiding this comment.
Just an idea, but we could simplify this even further by just having a currency specified (e.g., fee-payer-fee=<currency>) and then expecting the server to use getPaymentInstruction
to fetch/assemble a valid payment. This would put fee estimation closer to transaction sending and limit oracle risk of price changes between estimate and Kora signing.
| If the field is provided, the wallet must include a `MemoProgram` instruction as the second to last instruction of the transaction, immediately before the SOL or SPL Token transfer instruction, to avoid ambiguity with other instructions in the transaction. | ||
|
|
||
| ### Fee Payer | ||
| A single `fee-payer` field is allowed as an optional query parameter. The value must be the base58-encoded public key to be used as the transaction fee payer. |
There was a problem hiding this comment.
This may be a bit overkill given the suggested adds below, but I think making the whole flow clear up front is probably helpful if even a little verbose.
| A single `fee-payer` field is allowed as an optional query parameter. The value must be the base58-encoded public key to be used as the transaction fee payer. | |
| A single `fee-payer` field is allowed as an optional query parameter. The value must be the base58-encoded public key to be used as the transaction fee payer. When `fee-payer` is specified: | |
| 1. Wallet constructs the initial transaction with: | |
| - All instructions for the user's intended action | |
| - If `fee-payer-fee` is present, include payment instruction to `fee-payment-destination` (or `fee-payer` if not specified) | |
| - Set `feePayer` to the specified `fee-payer` address | |
| 2. User partially signs the transaction | |
| 3. Relayer validates and signs: | |
| - For free relaying: Uses `signTransaction` method | |
| - For paid relaying: Uses `signTransactionIfPaid` method with payment verification | |
| - Returns fully signed transaction to wallet | |
| 4. Fully signed transaction is broadcast to the network and finalized |
| ### Token Program | ||
| A single `token-program` field is allowed as an optional query parameter. The value must be the base58-encoded public key of a token program. | ||
|
|
||
| - If `spl-token` is provided, the wallet must use the specified `token-program` to derive accounts and to include the corresponding transfer instruction (e.g. Token-2022). | ||
| - If `token-program` is not provided, wallets and libraries should attempt to determine the correct program automatically from on-chain mint data. If auto-detection fails, default to the legacy SPL Token Program (`TokenkegQfeZyiNwAJbNbGKPFXCWuBvf9Ss623VQ5DA`) for backwards compatibility. | ||
| - Compressed tokens typically require additional proofs and must be handled via the Transaction Request flow. Wallets may reject a non-interactive transfer request that implies a compressed token transfer as **unsupported**. | ||
|
|
||
| Examples: | ||
| - Token-2022: `token-program=TokenzQdBNbLqP5VEhdkAS6EPFLC1PHnBqCXEpPxuEb` |
There was a problem hiding this comment.
Is there any use case where we would want this to be different to the token program that owns the spl-token mint, or where a wallet wouldn't be able to know that?
I think it'd be better to require wallets to use the on-chain token program for the mint if possible. A robust implementation will likely choose to do that anyway and reject (outside of the spec) if it doesn't match to protect users.
I don't think this is data that should be passed to wallets and treated as trusted.
|
|
||
| If the values are provided, the wallet must include them in the order provided as read-only, non-signer keys to the `SystemProgram.Transfer` or `TokenProgram.Transfer`/`TokenProgram.TransferChecked` instruction in the payment transaction. The values may or may not be unique to the payment request, and may or may not correspond to an account on Solana. | ||
|
|
||
| Because Solana validators index transactions by these account keys, `reference` values can be used as client IDs (IDs usable before knowing the eventual payment transaction). The [`getSignaturesForAddress`](https://docs.solana.com/developing/clients/jsonrpc-api#getsignaturesforaddress) RPC method can be used locate transactions this way. |
There was a problem hiding this comment.
Nit:
| Because Solana validators index transactions by these account keys, `reference` values can be used as client IDs (IDs usable before knowing the eventual payment transaction). The [`getSignaturesForAddress`](https://docs.solana.com/developing/clients/jsonrpc-api#getsignaturesforaddress) RPC method can be used locate transactions this way. | |
| Because Solana validators index transactions by these account keys, `reference` values can be used as client IDs (IDs usable before knowing the eventual payment transaction). The [`getSignaturesForAddress`](https://docs.solana.com/developing/clients/jsonrpc-api#getsignaturesforaddress) RPC method can be used to locate transactions this way. |
| &redirect=<redirect> | ||
| ``` | ||
|
|
||
| The request is non-interactive because the parameters in the URL are used by a wallet to directly compose a transaction. |
There was a problem hiding this comment.
This is no longer true if we add payment-options, some requests will require interactivity
Edit: Actually I guess amount is optional already. Nitpicky, probably just ignore if this language is already used in the spec.
| ### Payment Options | ||
| A single `payment-options` field is allowed as an optional query parameter. The value must be a semicolon-separated list of payment choices. Each choice must be a comma-separated pair of `<currency>,<amount>` where: | ||
|
|
||
| - `<currency>` is either the string `sol` (case-insensitive) to indicate native SOL, or the base58-encoded public key of an SPL Token mint account. | ||
| - `<amount>` is a non-negative integer or decimal number in user units for the corresponding currency. Scientific notation is prohibited. If the number of decimal places exceeds what's supported for the selected currency, the wallet must reject the URL as **malformed**. | ||
|
|
||
| Examples: | ||
| - `payment-options=sol,1` (1 SOL) | ||
| - `payment-options=EPjFWdd5AufqSSqeM2qN1xzybapC8G4wEGGkZwyTDt1v,0.01` (0.01 USDC) | ||
| - `payment-options=sol,1;EPjFWdd5AufqSSqeM2qN1xzybapC8G4wEGGkZwyTDt1v,0.01` (user may choose) | ||
| - With a top-level `spl-token` and `amount`, it's required to mirror the same option here for compatibility. | ||
|
|
||
| Rules: | ||
| - If `spl-token` is provided and `payment-options` is also present, wallets may offer both the top-level option (defined by `spl-token` and the top-level `amount`, if present) and the parsed `payment-options` list. The top-level option should be treated as the default selection for backwards compatibility. It is required to include an equivalent entry in `payment-options` for the top-level choice. | ||
| - If `payment-options` is present and `spl-token` is not provided, wallets that support this parameter should prompt the user to choose a payment option and compose the transfer accordingly. | ||
| - When `payment-options` is present and `spl-token` is not provided, wallets should ignore the top-level `amount` and use the amount from the selected payment option. When `spl-token` is provided, the top-level `amount` applies to the top-level selection; amounts in `payment-options` apply when the user selects an entry from that list. | ||
| - Wallets that do not support `payment-options` must ignore it and proceed as if it were absent (defaulting to SOL unless `spl-token` is present). |
There was a problem hiding this comment.
From a wallet perspective I'm concerned that this adds complexity and interactivity. I also think the rules around how it combines with other fields to maintain compatibility are quite confusing.
IMO if this field is present then spl-token and amount should be ignored, they'd only be included to provide compatibility with wallets that don't support this field. You'd only include this field if you want to support >1 payment option and have the user choose. I don't think this would affect the data size, since the spec requires including the top-level option in the list anyway.
| Rules: | ||
| - If provided, wallets that support fee relaying should set the transaction `feePayer` to the specified account and must not attempt to sign as that account. | ||
| - Wallets may submit the partially signed transaction to a configured relayer service (e.g., a Kora server) to finalize and broadcast. Relayer configuration and discovery are wallet-defined and out of scope for this specification. | ||
| - If fee relaying is unsupported or unavailable, wallets should either ignore `fee-payer` and proceed with the sender as the fee payer, or surface a structured error as defined in the Error Handling section (`SP_FEE_PAYER_UNAVAILABLE`). Implementations should prefer not breaking payment flows; merchants relying on relayers should also offer a fallback. |
There was a problem hiding this comment.
How would you offer a fallback in this spec as the merchant?
| Where each entry specifies the currency and amount to be paid to the relayer for facilitating the transaction. The currency rules match `payment-options` (`sol` or SPL mint address). Amounts are user units and must comply with currency precision. | ||
|
|
||
| Rules: | ||
| - If present, wallets should include this additional payment to the fee payer service as part of the composed flow. The mechanism to pay the relayer may require switching to the Transaction Request flow or composing multiple transfers; concrete implementation is wallet-defined. |
There was a problem hiding this comment.
I think this is too under-defined. We should specify the situations where the wallet will be able to pay the fee in this transaction, and make only those valid for this spec. Switching to a transaction request or composing multiple transactions should be out of spec IMO.
If we add fee payer support for transfer requests it should be simple: the user can pay extra in the same currency they selected to cover the fees.
Eg I can pay 0.51 SOL instead of 0.5 SOL, or 105 USDC instead of 100 USDC
The fee-payer-fee must include any of the options allowed by payment-options, and allow being paid in any one of them, with the wallet automatically using whichever matches the payment option the user selects.
Anything more complex or requiring anyone else (eg merchant) to pay the fee, should be handled by transaction request IMO
energeticlee
left a comment
energeticlee
left a comment
There was a problem hiding this comment.
Context & rationale
The most common challenge during in-person payments is that users either don’t hold the specific asset the application expects, or they want to pay with a different asset. paymentOptions doesn’t fully address this because many users don’t hold stablecoins or liquid SOL (balances are often in staked SOL or meme coins).
The suggestions in my inline comments let wallets present the user’s holdings and let the application swap to its settlement asset, while remaining backward-compatible.
Changes proposed (see inline suggestions):
- GET: add acceptsAnyCurrency (boolean) and defaultCurrency ("sol" or mint).
- POST: allow currency even when paymentOptions weren’t provided, and clarify the token scan camera (select token → send → camera) behavior: wallets should preselect the scanned token if funded, else fall back to defaultCurrency when funded.
- Back-compat: unknown fields are ignored by older clients/servers.
- Open to: different field names or nesting under paymentOptions if maintainers prefer.
| {"label":"<label>","icon":"<icon>","paymentOptions":[{"currency":"sol","amount":"1"},{"currency":"EPjFWdd5AufqSSqeM2qN1xzybapC8G4wEGGkZwyTDt1v","amount":"0.01"}]} | ||
| ``` | ||
| Where each `currency` value is either `sol` (case-insensitive) or a base58 SPL Token mint address. Applications may additionally include display metadata such as `symbol`, `decimals`, and a fixed `amount` per option if pricing is static. | ||
|
|
There was a problem hiding this comment.
| Additionally, the application may include the following optional fields (wallets that do not recognize them must ignore them): | |
| - `acceptsAnyCurrency` (boolean): when `true`, the application can accept any user-selected input currency and may perform server-side routing/swaps to settle in its preferred currency. | |
| - `defaultCurrency` (string: `"sol"` or a base58-encoded SPL Token mint address): the currency the wallet should preselect if the user has sufficient balance. Ignored when `acceptsAnyCurrency` is not `true`. | |
| These fields do not change existing `paymentOptions` behavior; if `paymentOptions` are provided, wallets may continue to display and honor them as specified. |
| {"account":"<account>","currency":"sol","amount":"<amount>"} | ||
| ``` | ||
| `currency` must be either `sol` or a base58 SPL Token mint address. If `amount` is provided, it must be a non-negative integer or decimal number of user units for the selected currency. Applications that do not support these fields must ignore them. | ||
|
|
There was a problem hiding this comment.
| If the GET response included `acceptsAnyCurrency: true`, the wallet may include an optional field to convey the user's asset preference: | |
| {"account":"<account>","currency":"sol"} | |
| When the scan originated from a token-send flow (“token scan camera”) and `acceptsAnyCurrency` is `true`, the wallet should preselect and display the scanned token as the payment currency if the user's balance is sufficient. If not sufficient, the wallet should default to `defaultCurrency` when funded, or otherwise present a shortlist based on the user's holdings. |
4 participants
Summary
Adds
SPEC1.2.md, the specification for Solana Pay v1.2.What’s in this PR
SPEC1.2.md.Highlights from v1.2 (documented in the spec)
Rationale
Future follow-ups (if spec is agreed to)
schemas/for v1.2.tests/vectors/v1_2/.README.mdand docs site toSPEC1.2.md.Important
Adds
SPEC1.2.mdfor Solana Pay v1.2, introducing Token-2022 support, payment options, fee relaying, and structured errors, with backward compatibility.SPEC1.2.mdfor Solana Pay v1.2 specification.token-programparameter.payment-optionsfor multiple payment choices.fee-payer,fee-payer-server, andfee-payer-feefor fee relaying.SP2_*codes.This description was created by
for eb6b233. You can customize this summary. It will automatically update as commits are pushed.