feat(phase-a): SDK — network expansion, CCTP V2, token breadth, refund wrapper#64
feat(phase-a): SDK — network expansion, CCTP V2, token breadth, refund wrapper#64RBKunnela wants to merge 5 commits into
Conversation
- Add OP/Arb/Polygon mainnet + testnets to NETWORKS (8 total) with native USDC addresses (Polygon native 0x3c49.. not bridged USDC.e) - TOKENS.USDC.addressByNetwork for all 6 new chains; EURC stays Base-only - networks-parity.test.ts enumerates every network (resolvable EIP-712 domain, CAIP-2, chainId match) to catch half-added chains - getEip712Domain unchanged (data-driven, proven by test) - 385 tests green; boundary clean Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…D/DAI)
- TokenConfig gains signingMethod ('eip3009'|'eip2612') + eip712Version (PYUSD verified v1)
- Guard in buildPaymentPayload + resolveDomain: non-eip3009 -> UNSUPPORTED_SIGNING_METHOD
(PayBotUnsupportedSigningMethodError) BEFORE any signature
- PYUSD working (eip3009, 6 dec, domain 'PayPal USD' v1); RLUSD+DAI documented-rejection (eip2612)
- USDC/EURC unchanged (default eip3009/v2, byte-identical)
- 401 tests green; boundary clean
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- CctpBridge: initiateTransfer (burn) -> getAttestation (Iris poll) -> completeTransfer (mint) - CCTP_DOMAINS registry (Base=6/OP=2/Arb=3/Polygon=7); V2-native (V1 sunsets 2026-07-31) - Orthogonal to client.pay(); injectable fetch/walletClient seams (zero real I/O in tests) - Standard V2 contract addresses left operator-injectable (not invented; source from Circle before mainnet) - 36 tests; 437 total green; boundary clean Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- refund({originalTxHash, amount?, reason?, idempotencyKey?}) -> RefundResult
- Mirrors pay(): never throws, success:false on failure, X-Idempotency-Key + LRU dedupe,
paybot.client.refund telemetry span
- Thin wrapper over core POST /refund (mocked; core endpoint is a separate task)
- 14 tests; 451 total green; boundary clean
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 59 minutes and 57 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (13)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
FriendlyAI review unavailable — upstream errorThe review service returned an error, so no verdict was produced. This is a neutral result, not a block. If this PR needs to merge while review is unavailable, a maintainer can apply the |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces CCTP V2 cross-chain USDC transport via a new CctpBridge class, adds a refund method to PayBotClient with idempotency and telemetry support, expands network support to Optimism, Arbitrum, and Polygon, and adds a guard to reject permit-only tokens (RLUSD, DAI) before signing. Feedback on these changes highlights several critical improvements: adding testnet CAIP-2 IDs to CCTP_DOMAINS to enable testnet CCTP support, fixing a hardcoded source domain and adding a delay to the polling loop in getAttestation, wrapping waitForTransactionReceipt in a try-catch block to handle RPC/network errors gracefully, and removing a redundant inner try-catch block in the refund method.
| export const CCTP_DOMAINS: Readonly<Record<string, CctpDomainConfig>> = { | ||
| // Base Mainnet — CCTP domain 6. | ||
| 'eip155:8453': { | ||
| cctpDomain: 6, | ||
| tokenMessengerV2: null, | ||
| messageTransmitterV2: null, | ||
| }, | ||
| // Optimism — CCTP domain 2. | ||
| 'eip155:10': { | ||
| cctpDomain: 2, | ||
| tokenMessengerV2: null, | ||
| messageTransmitterV2: null, | ||
| }, | ||
| // Arbitrum One — CCTP domain 3. | ||
| 'eip155:42161': { | ||
| cctpDomain: 3, | ||
| tokenMessengerV2: null, | ||
| messageTransmitterV2: null, | ||
| }, | ||
| // Polygon PoS — CCTP domain 7. | ||
| 'eip155:137': { | ||
| cctpDomain: 7, | ||
| tokenMessengerV2: null, | ||
| messageTransmitterV2: null, | ||
| }, | ||
| } as const; |
There was a problem hiding this comment.
The CCTP_DOMAINS registry only contains the 4 mainnet CAIP-2 IDs. Because getCctpConfig immediately returns undefined if a network is not present in CCTP_DOMAINS, it is currently impossible to use CCTP on testnets (such as Base Sepolia, OP Sepolia, Arbitrum Sepolia, or Polygon Amoy), even if the operator attempts to inject the contract addresses via domainOverrides.
Since this Phase A release is explicitly "testnet-targeted", you must add the corresponding testnet CAIP-2 IDs to CCTP_DOMAINS with their respective Circle domain IDs and null contract addresses.
export const CCTP_DOMAINS: Readonly<Record<string, CctpDomainConfig>> = {
// Base Mainnet — CCTP domain 6.
'eip155:8453': {
cctpDomain: 6,
tokenMessengerV2: null,
messageTransmitterV2: null,
},
// Base Sepolia — CCTP domain 6.
'eip155:84532': {
cctpDomain: 6,
tokenMessengerV2: null,
messageTransmitterV2: null,
},
// Optimism — CCTP domain 2.
'eip155:10': {
cctpDomain: 2,
tokenMessengerV2: null,
messageTransmitterV2: null,
},
// OP Sepolia — CCTP domain 2.
'eip155:11155420': {
cctpDomain: 2,
tokenMessengerV2: null,
messageTransmitterV2: null,
},
// Arbitrum One — CCTP domain 3.
'eip155:42161': {
cctpDomain: 3,
tokenMessengerV2: null,
messageTransmitterV2: null,
},
// Arbitrum Sepolia — CCTP domain 3.
'eip155:421614': {
cctpDomain: 3,
tokenMessengerV2: null,
messageTransmitterV2: null,
},
// Polygon PoS — CCTP domain 7.
'eip155:137': {
cctpDomain: 7,
tokenMessengerV2: null,
messageTransmitterV2: null,
},
// Polygon Amoy — CCTP domain 7.
'eip155:80002': {
cctpDomain: 7,
tokenMessengerV2: null,
messageTransmitterV2: null,
},
} as const;| async getAttestation(messageHash: string): Promise<CctpAttestation> { | ||
| let lastPending: CctpAttestation = { status: 'pending' }; | ||
|
|
||
| for (let attempt = 0; attempt < this.maxAttestationPolls; attempt++) { | ||
| const url = `${this.irisApiUrl}/v2/messages/0?transactionHash=${messageHash}`; |
There was a problem hiding this comment.
This method has two critical issues:
- Hardcoded Source Domain: The Circle Iris API endpoint is hardcoded to
0(Ethereum), but Phase A supports Optimism (2), Arbitrum (3), Base (6), and Polygon (7). Querying with the wrong domain will fail to find the transaction. - Tight Polling Loop: The loop retries immediately without any delay, which will spam the Circle Iris API and likely trigger rate limits or high CPU usage.
To fix this, update the signature to accept an optional sourceDomain parameter, use it in the URL, and add a delay (e.g., 2 seconds) between retries.
| async getAttestation(messageHash: string): Promise<CctpAttestation> { | |
| let lastPending: CctpAttestation = { status: 'pending' }; | |
| for (let attempt = 0; attempt < this.maxAttestationPolls; attempt++) { | |
| const url = `${this.irisApiUrl}/v2/messages/0?transactionHash=${messageHash}`; | |
| async getAttestation(messageHash: string, sourceDomain: number = 0): Promise<CctpAttestation> { | |
| let lastPending: CctpAttestation = { status: 'pending' }; | |
| for (let attempt = 0; attempt < this.maxAttestationPolls; attempt++) { | |
| if (attempt > 0) { | |
| await new Promise((resolve) => setTimeout(resolve, 2000)); | |
| } | |
| const url = `${this.irisApiUrl}/v2/messages/${sourceDomain}?transactionHash=${messageHash}`; |
| const publicClient = this.publicClientFactory(req.sourceNetwork); | ||
| const receipt = await publicClient.waitForTransactionReceipt({ hash: burnTxHash }); |
There was a problem hiding this comment.
The call to publicClient.waitForTransactionReceipt is executed outside of any try-catch block. If the transaction receipt query fails (e.g., due to a network timeout, node RPC issue, or reverted transaction), a raw viem error will bubble up to the caller.
To ensure consistent error handling and preserve the SDK's contract of throwing typed PayBotApiErrors, wrap this call in a try-catch block and throw a PayBotApiError with a CCTP_BURN_FAILED code.
const publicClient = this.publicClientFactory(req.sourceNetwork);
let receipt;
try {
receipt = await publicClient.waitForTransactionReceipt({ hash: burnTxHash });
} catch (error) {
throw new PayBotApiError(
`Failed to fetch CCTP burn receipt: ${getErrorMessage(error)}`,
'CCTP_BURN_FAILED',
502,
{ burnTxHash, sourceNetwork: req.sourceNetwork },
);
}| let data: Record<string, unknown>; | ||
| try { | ||
| data = await this._request<Record<string, unknown>>('/refund', { | ||
| method: 'POST', | ||
| body: { | ||
| botId: this.config.botId, | ||
| originalTxHash: request.originalTxHash, | ||
| amount: request.amount, | ||
| reason: request.reason, | ||
| }, | ||
| headers: idempotencyKey !== undefined | ||
| ? { 'X-Idempotency-Key': idempotencyKey } | ||
| : undefined, | ||
| }); | ||
| } catch (error: unknown) { | ||
| // A typed PayBotApiError (e.g. core 4xx) becomes a returned failure | ||
| // RefundResult — never re-thrown — so callers can branch on the code. | ||
| if (error instanceof PayBotApiError) { | ||
| refundSpan?.setAttribute('success', false); | ||
| return { | ||
| success: false, | ||
| originalTxHash: request.originalTxHash, | ||
| status: 'failed', | ||
| error: error.message, | ||
| errorCode: error.code, | ||
| errorDetails: error.details, | ||
| }; | ||
| } | ||
| throw error; | ||
| } |
There was a problem hiding this comment.
The inner try-catch block wrapping the _request call is redundant. Any PayBotApiError caught and returned here is handled in the exact same manner by the outer catch block (lines 623-641), which also sets the span's success attribute to false and returns the identical failure RefundResult.
Removing this inner block simplifies the code, reduces nesting, and improves maintainability.
const data = await this._request<Record<string, unknown>>('/refund', {
method: 'POST',
body: {
botId: this.config.botId,
originalTxHash: request.originalTxHash,
amount: request.amount,
reason: request.reason,
},
headers: idempotencyKey !== undefined
? { 'X-Idempotency-Key': idempotencyKey }
: undefined,
});1 participant
Phase A SDK side, testnet-targeted ($0), boundary-clean throughout (the new
open-core boundarygate passes on every commit).What's in here
signingMethod+eip712Versiondiscriminators. PYUSD working (eip3009, verified domain v1); RLUSD/DAI documented-rejection (UNSUPPORTED_SIGNING_METHOD).CctpBridge(burn → Iris attestation → mint), V2-native (V1 sunsets 2026-07-31). Standard V2 contract addresses left operator-injectable (not invented).POST /refund(contract frozen with the core PR).451 tests green · 98%+ coverage · lint + type-check + open-core boundary all clean. Pairs with the paybot-core Phase A PR.
🤖 Generated with Claude Code