Solana subscription intent specification #270
Conversation
Co-authored-by: jo <17280917+dev-jodee@users.noreply.github.com>
brendanjryan
left a comment
brendanjryan
left a comment
There was a problem hiding this comment.
I reviewed this against the shared subscription intent draft in #230 and left the main consistency comments inline.
| | `methodDetails.mint` | string | REQUIRED | Mint address echoed from `currency`. MUST equal the on-chain `plan.mint` | | ||
| | `methodDetails.decimals` | number | REQUIRED | Decimal precision of the mint | | ||
| | `methodDetails.tokenProgram` | string | REQUIRED | Token program ID. The value MUST be the SPL Token program or the SPL Token-2022 program | | ||
| | `methodDetails.puller` | string | REQUIRED | Base58 of the server's puller pubkey. MUST be `plan.owner` or appear in `plan.pullers` | |
There was a problem hiding this comment.
puller is a slightly odd name -- is this consistent with how solana represents it in other places?
|
|
||
| After successful activation, the server MUST return a | ||
| `subscriptionId` in the `Payment-Receipt`. On Solana, the | ||
| `subscriptionId` is the base58-encoded `SubscriptionDelegation` |
There was a problem hiding this comment.
This conflicts with the shared subscription intent, which says subscriptionId MUST be a base64url string without padding and unique within the server subscription namespace. This draft currently makes it the base58 SubscriptionDelegation address, and later server-accounting text says base64url, so the profile is inconsistent both externally and internally.
Suggested fix: keep subscriptionId compliant with the shared intent, and add a Solana-specific receipt field for the chain address, e.g. subscriptionDelegation or delegationAddress = base58 SubscriptionDelegation account address. Also update the accounting bullet to use the same identifier format.
| `SubscriptionDelegation` account reflects | ||
| `amount_pulled_in_period == amount_per_period` for period 0. | ||
|
|
||
| Servers MUST NOT treat activation as successful if the activation |
There was a problem hiding this comment.
subscriptionExpires is optional above, but this sentence treats it as always present.
Suggested fix: make this conditional, e.g. Servers MUST NOT treat activation as successful if subscriptionExpires is present and the activation transaction settles at or after that timestamp.
| Servers MUST reject request objects where the mapped per-billing- | ||
| period interval is zero or exceeds 8760 hours. | ||
|
|
||
| The `externalId` value is the base58 address of the on-chain `Plan` |
There was a problem hiding this comment.
This redefines externalId from the shared subscription intent. The shared intent defines externalId as the merchant’s reference for the subscription; here it becomes the on-chain Plan PDA, while the text above says the shared field meanings are preserved.
Suggested fix: put the Plan PDA in methodDetails.planId (or methodDetails.plan) and leave externalId with the shared merchant-reference meaning. If the intent is to constrain/redefine externalId for Solana, please state that explicitly rather than saying the shared meaning is preserved.
| Servers MUST NOT submit more than one successful renewal charge for | ||
| the same billing period. | ||
|
|
||
| The on-chain `transfer_subscription` advances |
There was a problem hiding this comment.
This renewal paragraph appears duplicated/garbled, and it also blurs the shared subscription invariant. The shared intent is fixed amount, once per billing period; this says the on-chain program accepts any transfer up to the remaining per-period cap.
Suggested fix: clean up the duplicate prose and state that conforming Solana subscription renewals collect exactly amount once per billing period. If the on-chain primitive allows partial/multiple pulls up to a cap, describe that as broader program capability outside this HTTP subscription profile.
| | `periodCount` | string | Positive integer count of `periodUnit` values per billing period | | ||
| | `recipient` | string | Recipient address authorized for subscription charges. The activation transaction MUST bind the destination at sign time | | ||
| | `externalId` | string | Base58 address of the on-chain `Plan` | | ||
| | `description` | string | Human-readable subscription description | |
There was a problem hiding this comment.
description is elevated to REQUIRED here, but the request examples below do not include it.
Suggested fix: either add description to all Solana subscription request examples, or leave description optional as in the shared subscription intent.
| | `methodDetails.decimals` | number | REQUIRED | Decimal precision of the mint | | ||
| | `methodDetails.tokenProgram` | string | REQUIRED | Token program ID. The value MUST be the SPL Token program or the SPL Token-2022 program | | ||
| | `methodDetails.puller` | string | REQUIRED | Base58 of the server's puller pubkey. MUST be `plan.owner` or appear in `plan.pullers` | | ||
| | `methodDetails.programId` | string | OPTIONAL | Base58 of the subscriptions program ID. If omitted, the default value is the canonical mainnet deployment | |
There was a problem hiding this comment.
If programId is optional and defaults to a canonical deployment, the draft needs to define that canonical value. Otherwise clients cannot validate the default unambiguously.
Suggested fix: either make methodDetails.programId REQUIRED or add a canonical IDs table.
| | `status` | string | `"success"` | | ||
| | `subscriptionId` | string | Base58 of the `SubscriptionDelegation` account address | | ||
| | `periodIndex` | string | Decimal index of the billing period (`"0"` on activation) | | ||
| | `periodStartTs` | string | {{RFC3339}} start of the current period | |
There was a problem hiding this comment.
what is Ts? timestamp? why not just periodStart?
| | `reference` | string | Base58 of the settlement transaction signature | | ||
| | `status` | string | `"success"` | | ||
| | `subscriptionId` | string | Base58 of the `SubscriptionDelegation` account address | | ||
| | `periodIndex` | string | Decimal index of the billing period (`"0"` on activation) | |
There was a problem hiding this comment.
if this is a decimal why not an int?
Spec Preview
|
2 participants
No description provided.