Skip to content

Conversation

spencerstock
Copy link
Collaborator

@spencerstock spencerstock commented Aug 21, 2025

Summary

Added subscription functionality to the payment interface using spend permissions. This enables applications to create recurring payment subscriptions on Base network with USDC.

Key changes:

  • New subscribe() function in payment interface that creates spend permissions for recurring payments
  • Support for configurable subscription periods (default 30 days)
  • Integration with wallet_sign method using mutable data fields for account replacement
  • Telemetry events for subscription lifecycle (started, completed, error)
  • Type definitions for SubscriptionOptions and SubscriptionResult
  • Exports added to browser entry and main index files
  • Updated error message in getHash method for clarity

How did you test your changes?

Unit test coverage includes:

  • Existing payment interface tests continue to pass
  • Integration with spend permission utilities (getHash, createSpendPermissionTypedData)
  • Error handling for invalid inputs and wallet responses
  • Telemetry event logging for subscription operations
  • Updated getHash test to match new error message

All CI checks pass:

Manual testing:

  • Subscription creation with testnet and mainnet configurations
  • Various subscription periods (7, 30, 90 days)
  • Amount validation with USDC decimals
  • Wallet signature flow with mutable data fields

@cb-heimdall
Copy link
Collaborator

cb-heimdall commented Aug 21, 2025

✅ Heimdall Review Status

Requirement Status More Info
Reviews 2/1
Denominator calculation
Show calculation
1 if user is bot 0
1 if user is external 0
2 if repo is sensitive 0
From .codeflow.yml 1
Additional review requirements
Show calculation
Max 0
0
From CODEOWNERS 0
Global minimum 0
Max 1
1
1 if commit is unverified 0
Sum 1

@spencerstock spencerstock marked this pull request as ready for review August 21, 2025 05:08
@cb-heimdall
Copy link
Collaborator

Review Error for Yzaik05 @ 2025-08-23 11:21:38 UTC
User must have write permissions to review

Comment on lines +28 to +30
* @param options.testnet - Whether to use Base Sepolia testnet (default: false)
* @param options.walletUrl - Optional wallet URL to use
* @param options.telemetry - Whether to enable telemetry logging (default: true)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you think this would make more sense in a nested param, since these are more sdk config focused rather than affecting the behavior of the subscribe functioanlity?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a valid consideration and I don't think there's a direct right answer - but I would be inclined to leave as is for now and add a nested param if a flat call starts to feel over-encumbered as we add more configurability down the road.

fan-zhang-sv
fan-zhang-sv previously approved these changes Sep 3, 2025
Copy link
Collaborator

@fan-zhang-sv fan-zhang-sv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice, left a comment

@@ -52,7 +52,7 @@ const getHashFn = async ({

if (!client) {
throw new Error(
`No client found for chain ID ${chainId}. Please ensure SDK is in connected state`
`No client found for chain ID ${chainId}. Chain not supported or RPC URL not available`
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: i think mentioning SDK connected state is also helpful, SDK connection gets the RPC URL from FE

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right - yeah maybe there's a way to phrase this like "Chain not supported or RPC URL not available - SDK may be in a disconnected state"

@spencerstock spencerstock force-pushed the spencer/subscribe-functionality branch from 5833141 to 1c322cb Compare September 4, 2025 15:39
@spencerstock spencerstock merged commit dc62400 into master Sep 4, 2025
8 checks passed
@spencerstock spencerstock deleted the spencer/subscribe-functionality branch September 4, 2025 18:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants