feat: update gateway client api #634
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis update refactors the Gateway SDK and its example to use the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant GatewaySDK
participant ViemClient
participant WalletClient
participant XverseConnector
User->>GatewaySDK: swapBtcForToken(evmAddress)
GatewaySDK->>ViemClient: Create publicClient/walletClient
GatewaySDK->>XverseConnector: Create btcSigner
GatewaySDK->>GatewaySDK: getQuote({type: 'onramp', ...})
GatewaySDK->>GatewaySDK: executeQuote(quote, params, walletClient, publicClient, btcSigner)
GatewaySDK->>XverseConnector: btcSigner.signAllInputs(psbt)
GatewaySDK->>GatewaySDK: Process signed PSBT, broadcast transaction
GatewaySDK-->>User: Return transaction ID
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate Unit Tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
sdk/test/gateway.test.ts (1)
845-846: Consider tracking the staking strategy availability.The test is skipped because signet doesn't have staking strategies. This limitation should be documented or tracked to ensure it's addressed when strategies become available.
Would you like me to create an issue to track the implementation of staking strategies for the signet network?
sdk/src/gateway/types.ts (1)
110-111: Remove or uncomment the 'type' field.The
typefield is commented out. Either remove it entirely if it's no longer needed, or uncomment it if it's still required for the API.- // /** @description Optionally filter the type of routes returned */ - // type?: 'swap' | 'deposit' | 'withdraw' | 'claim';sdk/src/gateway/client.ts (1)
606-722: Consider refactoring executeQuote for better maintainability.While the unified
executeQuotemethod improves API consistency, it's becoming quite complex with three different execution paths. Consider extracting each quote type's logic into separate private methods.async executeQuote( executeQuoteParams: ExecuteQuoteParams, walletClient: WalletClient<Transport, ViemChain, Account>, publicClient: PublicClient<Transport>, btcSigner: { signAllInputs: (psbtBase64: string) => Promise<string> } ): Promise<string> { if (executeQuoteParams.type === 'onramp') { - const { params, quote } = executeQuoteParams; - const { uuid, psbtBase64 } = await this.startOrder(quote, params); - // ... rest of onramp logic + return this.executeOnrampQuote(executeQuoteParams, btcSigner); } else if (executeQuoteParams.type === 'offramp') { - const { params } = executeQuoteParams; - // ... rest of offramp logic + return this.executeOfframpQuote(executeQuoteParams, walletClient, publicClient); } else { - const { params } = executeQuoteParams; - // ... rest of stake logic + return this.executeStakeQuote(executeQuoteParams, walletClient, publicClient); } } + +private async executeOnrampQuote( + executeQuoteParams: Extract<ExecuteQuoteParams, { type: 'onramp' }>, + btcSigner: { signAllInputs: (psbtBase64: string) => Promise<string> } +): Promise<string> { + // Extract onramp logic here +} + +private async executeOfframpQuote( + executeQuoteParams: Extract<ExecuteQuoteParams, { type: 'offramp' }>, + walletClient: WalletClient<Transport, ViemChain, Account>, + publicClient: PublicClient<Transport> +): Promise<string> { + // Extract offramp logic here +} + +private async executeStakeQuote( + executeQuoteParams: Extract<ExecuteQuoteParams, { type: 'stake' }>, + walletClient: WalletClient<Transport, ViemChain, Account>, + publicClient: PublicClient<Transport> +): Promise<string> { + // Extract stake logic here +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
sdk/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (7)
sdk/examples/gateway.ts(1 hunks)sdk/package.json(2 hunks)sdk/src/gateway/client.ts(20 hunks)sdk/src/gateway/types.ts(9 hunks)sdk/src/gateway/utils.ts(1 hunks)sdk/src/mempool.ts(2 hunks)sdk/test/gateway.test.ts(21 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
sdk/src/gateway/utils.ts (2)
sdk/src/gateway/types.ts (2)
GatewayCreateOrderRequest(238-247)OfframpOrderStatus(354-354)crates/utils/src/bitcoin_client.rs (1)
network(275-278)
🪛 Gitleaks (8.26.0)
sdk/examples/gateway.ts
7-7: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🔇 Additional comments (21)
sdk/src/gateway/utils.ts (1)
1-69: Excellent modularization of utility functions!The extraction of these utility functions into a dedicated module improves code organization and reusability. All implementations are correct and follow best practices:
calculateOpReturnHashproperly uses ethers ABI encoding for data integritytoHexScriptPubKeycorrectly handles Bitcoin address conversion- Hex utilities provide clean string manipulation
parseOrderStatusincludes proper error handling for invalid inputsviemClientuses the standard viem client creation patternsdk/package.json (3)
3-3: Version bump is appropriate for the refactoring scope.The increment to 3.3.3 aligns with the feature additions and type safety improvements in this release.
44-44: Good addition of Bitcoin signing capability.The
@gobob/sats-wagmidependency enables the Bitcoin signing functionality demonstrated in the updated example.
52-52: Pinning viem version ensures compatibility.Fixing the viem version to 2.30.6 prevents potential breaking changes and ensures compatibility with the new typed
Addressusage patterns.sdk/src/mempool.ts (2)
263-263:⚠️ Potential issueFix incorrect promise handling that creates double-wrapped promises.
The current implementation returns
Promise<Promise<T>>instead ofPromise<T>. Sinceresponse.json()already returns a Promise, casting it asPromise<T>and returning from an async function creates a double-wrapped promise.Apply this fix:
- return response.json() as Promise<T>; + return response.json() as T;Likely an incorrect or invalid review comment.
274-274:⚠️ Potential issueFix incorrect promise handling that creates double-wrapped promises.
Same issue as in
getJson- this createsPromise<Promise<T>>instead of the intendedPromise<T>.Apply this fix:
- return response.text() as Promise<T>; + return response.text() as T;Likely an incorrect or invalid review comment.
sdk/examples/gateway.ts (4)
1-6: Excellent integration of new dependencies and typed imports.The imports properly leverage the new dependencies (
@gobob/sats-wagmi,viem) and demonstrate the enhanced type safety with typedAddressand client interfaces.
9-23: Well-structured client setup using the new patterns.The viem client creation and XverseConnector setup properly demonstrate the updated SDK patterns. The typed
Addressparameter and proper client configuration align with the refactoring objectives.
24-49: Streamlined quote execution flow.The refactored example effectively demonstrates the simplified quote execution using the new
executeQuotemethod, replacing the previous manual PSBT handling with a cleaner API.
7-7: Static analysis false positive - this is a legitimate contract address.The detected "Generic API Key" is actually the BOB tBTC v2 token contract address on the BOB network, which is a legitimate constant for the example.
🧰 Tools
🪛 Gitleaks (8.26.0)
7-7: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
sdk/test/gateway.test.ts (6)
4-4: LGTM! Good migration to viem types.The replacement of ethers'
ZeroAddresswith viem'szeroAddressis consistent with the broader migration to viem types across the SDK.
9-9: LGTM! Correct addition of signet base URL.The addition of
SIGNET_GATEWAY_BASE_URLimport aligns with the removal of 'testnet' support and introduction of 'signet' network.
143-143: LGTM! Consistent network name update.The change from 'testnet' to 'signet' is consistent with the SDK's new network naming convention.
550-563: LGTM! Enhanced fee transparency with detailed breakdown.The new
feeBreakdownstructure provides better visibility into fee components (inclusion, protocol, affiliate fees). This aligns with the PR objective of enhancing fee breakdowns for off-ramp operations.
772-774: LGTM! Correct parameter order for unified executeQuote.The updated parameter order (walletClient, publicClient, btcSigner) aligns with the refactored
executeQuotemethod signature in the client.
622-622:✅ Verification successful
Verify the response format change.
The mock response changed from a quoted string to a plain value. This appears to be fixing a previous inconsistency where the response was double-encoded as a JSON string.
🏁 Script executed:
#!/bin/bash # Description: Check if other tests or actual API responses use quoted strings for addresses # Search for similar nock mocks returning addresses to verify consistency rg -A 2 -B 2 "get.*address.*reply.*200.*['\"]0x" --type ts # Check for any other instances of registry address responses rg "offramp-registry-address.*reply" --type ts -A 3Length of output: 392
Mock response format is consistent across tests
Verified that the only other address mock in sdk/test/gateway.test.ts (
/offramp-registry-address) uses a template‐literal string (.reply(201, \${zeroAddress}`)`), and no tests return a double-encoded JSON string. The change at line 622 to return a plain hex string is aligned with existing tests.sdk/src/gateway/types.ts (3)
356-368: LGTM! Excellent fee transparency design.The
OfframpFeeBreakdowninterface provides granular fee visibility, breaking down total fees into inclusion, protocol, and affiliate components. This design enhances transparency for users.
542-578: LGTM! Well-designed discriminated union for quote execution.The
ExecuteQuoteParamsdiscriminated union provides excellent type safety by enforcing specific constraints for each quote type (onramp, offramp, stake). The chain and address type constraints ensure compile-time safety.
373-374:✅ Verification successful
Verify precision requirements for satoshi amounts.
The change from
biginttonumberforamountLockInSatis safe for typical Bitcoin amounts (up to ~90 million BTC), but consider if there are any edge cases that might require bigint precision.
🏁 Script executed:
#!/bin/bash # Description: Check if there are any large satoshi amount calculations that might overflow # Search for satoshi amount calculations and conversions rg "amountLockInSat|satoshis?" --type ts -B 2 -A 2 | grep -E "(BigInt|Number|parseInt|parseFloat|\* 10\*\*)" # Check for any existing bigint satoshi handling ast-grep --pattern 'BigInt($_)'Length of output: 5181
No precision concerns for OfframpQuote numeric type change
We’ve verified that both
amountLockInSatanddeadline(nownumber) are immediately converted back toBigIntin the gateway client, and typical satoshi values (≤ 9×10^15) and Unix timestamps stay well within the 2^53−1 safe-integer limit. No precision loss will occur.• sdk/src/gateway/client.ts (≈lines 330–332):
satAmountToLock: BigInt(offrampQuote.amountLockInSat), satFeesMax: BigInt(offrampQuote.feeBreakdown.overallFeeSats), orderCreationDeadline: BigInt(offrampQuote.deadline),→ No changes required.
sdk/src/gateway/client.ts (2)
95-112: LGTM! Clear network support with proper error handling.The constructor now clearly supports only 'mainnet', 'signet', and 'bob' networks, with appropriate error handling for invalid chains.
734-742: LGTM! Improved Bitcoin transaction handling.The addition of hex prefix stripping and the ability to fetch transaction hex from Esplora when only a txid is provided improves the API's flexibility.
…hore/merge-interfaces
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
sdk/test/gateway.test.ts (1)
869-869: Minor: Mock response string update.The change from
'🎉'to'request'makes the mock more realistic and professional.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
sdk/src/gateway/client.ts(16 hunks)sdk/test/gateway.test.ts(27 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
sdk/test/gateway.test.ts (2)
sdk/src/gateway/client.ts (1)
MAINNET_GATEWAY_BASE_URL(68-68)sdk/src/gateway/types.ts (3)
StakeParams(26-39)GatewayQuote(218-235)GatewayTokensInfo(334-339)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test
🔇 Additional comments (18)
sdk/test/gateway.test.ts (7)
8-8: LGTM: Import update aligns with refactoring.The import change correctly reflects the extraction of utility constants to a centralized location.
20-20: LGTM: Utility import moved to appropriate module.Good refactoring -
toHexScriptPubKeyhas been properly moved from client to the utils module, improving code organization.
48-55: LGTM: Consistent migration from ethers to viem addresses.The replacement of
ZeroAddressfrom ethers withzeroAddressfrom viem throughout the test suite ensures consistency with the new type system using viem'sAddresstype.Also applies to: 158-165, 208-224, 273-273, 285-285
142-142: LGTM: Network naming standardization.The change from 'testnet' to 'signet' aligns with the client.ts changes that removed 'testnet' support and standardized on 'signet' for the test network.
Also applies to: 363-363, 449-449, 856-856, 888-888
63-69: LGTM: Parameter structure simplification.The removal of nested
paramswrappers ingetQuotecalls reflects the streamlined API design mentioned in the AI summary.Also applies to: 73-79, 83-89, 93-99, 103-111
555-561: LGTM: Fee breakdown structure update.The mock response now includes detailed
feeBreakdownobjects instead of flat fee fields, correctly matching the newOfframpFeeBreakdownstructure mentioned in the AI summary.
845-846: Good: Test properly skipped with clear reasoning.The skip with FIXME comment clearly indicates that staking strategies are not available on signet, which aligns with the network changes.
sdk/src/gateway/client.ts (11)
1-62: LGTM: Comprehensive import reorganization improves type safety.The import restructuring correctly integrates viem types (
Address,Hex,isAddress,isAddressEqual) for better type safety and moves utility functions to a centralized utils module, improving code organization.
95-112: LGTM: Network configuration standardization.The removal of 'testnet' support and mapping 'signet' to
Chain.BOB_SEPOLIAcreates cleaner network handling. This aligns with the test changes and provides better consistency.
131-148: LGTM: Enhanced method overloading with proper error handling.The updated
getQuotemethod with clear overloads for onramp/offramp and proper BigInt conversion improves type safety. The error message is also correctly spelled (addressing a past review comment).
150-193: LGTM: Type-safe parameter handling.The
getOnrampQuotemethod now uses proper typing withAddresstypes and maintains backward compatibility with multiple input formats (chain ID, chain name, token address/symbol).
213-213: LGTM: Consistent token address resolution.Using
getTokenAddressfor consistent token address resolution ensures proper handling of both token symbols and addresses.Also applies to: 245-245
307-307: LGTM: Proper BigInt usage for token amounts.The explicit
BigIntconversion for amounts ensures type consistency throughout the offramp flow.Also applies to: 466-466
540-542: LGTM: Enhanced address validation.Using viem's
isAddressfunction provides more robust EVM address validation compared to manual string checks.
552-552: LGTM: Proper type annotation for encoded data.The explicit
as Hexcast ensures the ABI encoded data meets viem's type requirements.
609-714: LGTM: Unified executeQuote method with improved type safety.The refactored
executeQuotemethod provides a clean, unified interface for onramp, offramp, and stake operations. The discriminated union types ensure type safety, and the BigInt conversions are consistent throughout.
758-758: LGTM: Consistent Address type usage.The method signatures now properly use viem's
Addresstype for all address parameters, improving type safety across the API.Also applies to: 419-419, 887-887, 1055-1055
1017-1017: LGTM: Enhanced address validation in buildStake.The use of
isAddressEqualfor address comparison andisAddressfor validation provides more robust checking compared to string comparisons.Also applies to: 1024-1024, 1030-1030
…hore/merge-interfaces
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (3)
sdk/src/gateway/client.ts (3)
144-150: Fix typo in error message (duplicate issue).The error message contains a typo that was previously flagged.
379-379: BigInt usage inconsistency (duplicate issue).This was previously flagged - the fee value is converted to BigInt but type system expects a number.
461-463: 8-decimal minimum constraint documentation (duplicate issue).This limitation was previously flagged and should be documented.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.gitmodules(1 hunks)sdk/src/gateway/client.ts(15 hunks)sdk/src/gateway/types.ts(7 hunks)sdk/test/gateway.test.ts(27 hunks)
✅ Files skipped from review due to trivial changes (1)
- .gitmodules
🚧 Files skipped from review as they are similar to previous changes (2)
- sdk/test/gateway.test.ts
- sdk/src/gateway/types.ts
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test
🔇 Additional comments (14)
sdk/src/gateway/client.ts (14)
1-65: LGTM! Well-organized import restructuring.The import reorganization properly consolidates Bitcoin and Viem-related utilities, and the extraction of utility functions to a separate module improves modularity and maintainability.
216-218: LGTM! Improved token address validation.Using
getTokenAddressfunction provides better centralized token validation compared to manual address checks.
422-422: LGTM! Improved type safety with typed Address.The parameter type change from generic string to
Addressimproves type safety and prevents invalid address inputs.
543-545: LGTM! Added proper address validation.Using
isAddressvalidation improves security by preventing invalid addresses from being processed.
555-555: LGTM! Improved type safety with Hex type.Using the
Hextype forstrategyExtraDataprovides better type safety for hexadecimal strings.
608-637: Excellent refactoring of executeQuote method signatures.The unified method signature using
ExecuteQuoteParamsimproves maintainability and type safety. The overloaded signatures properly handle different parameter combinations for onramp vs offramp operations.
638-653: LGTM! Improved error handling and parameter destructuring.The destructuring of
executeQuoteParamsand explicit error handling for missingbtcSignerimproves code clarity and prevents runtime errors.
654-694: LGTM! Consistent BigInt usage in offramp flow.The offramp execution properly uses BigInt for amount comparisons and maintains type consistency throughout the flow.
696-733: LGTM! Improved staking flow implementation.The staking execution uses proper parameter destructuring and maintains consistent BigInt usage for amounts.
778-778: LGTM! Consistent with other method parameter types.The parameter type change to
Addressmaintains consistency with other methods likegetOfframpOrders.
907-907: LGTM! Improved return type specificity.Using
Address[]instead of generic EVM addresses provides better type safety for consumers of this method.
1037-1052: LGTM! Comprehensive address validation in buildStake.The use of
isAddressEqualfor strategy matching andisAddressfor address validation, combined with proper error messages, significantly improves the robustness of this method.
1075-1075: LGTM! Consistent typed Address parameter.The parameter type change maintains consistency with the other order-related methods.
310-310:✅ Verification successful
Verify BigInt usage consistency.
Using
BigInt(params.amount)here butparams.amounttype may already be bigint. Ensure type consistency across the codebase.
🏁 Script executed:
#!/bin/bash # Description: Check the type definition of amount parameter in OfframpQuoteParams # Expected: Consistent bigint usage ast-grep --pattern 'interface OfframpQuoteParams { $$$ amount: $_ $$$ }'Length of output: 80
🏁 Script executed:
#!/bin/bash # Search for the OfframpQuoteParams interface and its amount type rg 'interface .*OfframpQuoteParams' -n rg 'OfframpQuoteParams' -C3 -n # Locate the fetchOfframpQuote method signature to inspect its parameters rg 'fetchOfframpQuote' -n -A5Length of output: 3816
🏁 Script executed:
#!/bin/bash # Locate the definition of GatewayQuoteParams to inspect the type of `amount` rg 'export (interface|type) GatewayQuoteParams' -n -A5 sdk/src/gateway/types.tsLength of output: 352
🏁 Script executed:
#!/bin/bash # Expand GatewayQuoteParams definition to include the `amount` field rg 'export interface GatewayQuoteParams' -n -A20 sdk/src/gateway/types.tsLength of output: 1025
OfframpQuoteParams.amount is number|string; BigInt conversion is correct and consistent
The
OfframpQuoteParamstype picks upamount: number | stringfromGatewayQuoteParams, andfetchOfframpQuote’s signature expects abigint. CallingBigInt(params.amount)ingetQuoteis appropriate—other call sites already pass abigintdirectly. No changes are needed.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
sdk/examples/gateway.ts (2)
24-33: Magic numbers & network mismatch hide intentThe example hard-codes:
amount: 10000000(sats)gasRefill: 10000(sats)Network.mainnetfor the BTC signer while the swap uses'bitcoin'/ mainnet but targets the Bob test chain.Consider:
- Adding inline comments (
// 0.1 BTC in sats) or helper constants for clarity.- Clarifying why a mainnet signer is used when the EVM side is on bob (testnet?) – this is easy to misread.
No code change required, but a brief comment prevents copy-paste errors.
35-35: Riskyas Parameters<…>cast masks API driftForcing the result of
getQuoteto the input type ofexecuteQuoterelies on both staying structurally compatible.
When either signature evolves this line will silently compile and break at runtime.If
GatewaySDK#getQuotealready returns the correctQuotetype, widenexecuteQuoteto accept that type instead of casting here:-const quote = (await gatewaySDK.getQuote(quoteParams)) as Parameters<(typeof gatewaySDK)['executeQuote']>[0]; +const quote = await gatewaySDK.getQuote(quoteParams); // Quoteand update
executeQuote’s signature if needed.This preserves type-safety end-to-end.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
sdk/examples/gateway.ts(1 hunks)
🧰 Additional context used
🪛 Gitleaks (8.26.0)
sdk/examples/gateway.ts
7-7: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test
🔇 Additional comments (2)
sdk/examples/gateway.ts (2)
7-7: False positive from Gitleaks – token address is publicThe constant value flagged by Gitleaks is an on-chain ERC-20 address, not a secret.
Nothing to do here.
15-19: ```shell
#!/bin/bashLocate the gateway example and display its initial lines
rg "gateway.ts" -l
sed -n '1,200p' sdk/examples/gateway.ts</details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (3)
sdk/src/gateway/client.ts (3)
614-617: Typo + missing typings in overloads still unresolved
This was flagged earlier and remains:async executeQuote(exectueQuoteParams: … ❌ typo async executeQuote(exectueQuoteParams: … ❌ typoProvide correct spelling and strong types for
clients.
665-667: Same in off-ramp branch – clone before writingRepeat the cloning pattern here to keep the API side-effect-free.
647-649: Mutates caller-suppliedparams– surprising side-effect
params.toChainis reassigned in-place. Clone before normalising:- if (typeof params.toChain === 'number') { - params.toChain = chainIdMapping[params.toChain]; - } + const localParams = { ...params }; + if (typeof localParams.toChain === 'number') { + localParams.toChain = chainIdMapping[localParams.toChain]; + }Use
localParamsdownstream to preserve referential transparency.
🧹 Nitpick comments (2)
sdk/src/gateway/client.ts (2)
74-79: Out-of-date JSDoc reference to “testnet”The docstring still says
@default "https://gateway-api-testnet.gobob.xyz"while the constant below points to the signet endpoint.
Update the comment to avoid confusion.- * @default "https://gateway-api-testnet.gobob.xyz" + * @default "https://gateway-api-signet.gobob.xyz"
476-481:erris typedunknown– accessingerr.messageis unsafeEither narrow the error type or fall back gracefully:
- } catch (err) { - // Return false and 0n with an error message if fetching the quote fails - throw new Error(`Error fetching offramp quote: ${err.message || err}`); + } catch (err) { + const message = + typeof err === 'object' && err && 'message' in err ? (err as Error).message : String(err); + throw new Error(`Error fetching offramp quote: ${message}`);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
sdk/examples/gateway.ts(1 hunks)sdk/src/gateway/client.ts(16 hunks)
🧰 Additional context used
🪛 Gitleaks (8.26.0)
sdk/examples/gateway.ts
7-7: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test
🔇 Additional comments (2)
sdk/examples/gateway.ts (2)
20-21: Bitcoin signer instantiation is incomplete
XverseConnectortypically needs a.connect()/.getSigner()flow before it can sign PSBTs.
Document or include that extra step, otherwisebtcSigner.signAllInputsmay beundefinedat runtime.
24-33: Hard-coded token address flagged by gitleaks is benignThe constant
BOB_TBTC_V2_TOKEN_ADDRESSis an on-chain contract address, not a secret key.
No remediation needed – consider suppressing the false-positive in your gitleaks config.
Signed-off-by: Gregory Hill <gregorydhill@outlook.com>
gregdhill
force-pushed
the
chore/merge-interfaces
branch
from
c9f2315 to
ffcdfef
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (4)
sdk/examples/gateway.ts (1)
15-19:zeroAddressplaceholder breaks signing – still not fixed
The wallet client is instantiated withaccount: zeroAddress, so any later transaction signing (e.g. insideexecuteQuote) will revert. Replace with the connected EOA.- const walletClient = createWalletClient({ - chain: bob, - transport: http(), - account: zeroAddress, // Use connected address here - }); + const walletClient = createWalletClient({ + chain: bob, + transport: http(), + account: evmAddress, // or the connected signer address + });sdk/src/gateway/client.ts (3)
659-661: Same in-place mutation ofparams.fromChain
See comment above: clone before rewriting.
605-607: Overload signatures still use implicitanyforclients
Previous review pointed this out; the overload declarations leakany, undermining type-safety.-async executeQuote(executeQuoteParams: OnrampExecuteQuoteParams, clients): Promise<string>; -async executeQuote(executeQuoteParams: OfframpExecuteQuoteParams, clients): Promise<string>; +async executeQuote( + executeQuoteParams: OnrampExecuteQuoteParams, + clients: { walletClient: WalletClient<Transport, ViemChain, Account>; publicClient: PublicClient<Transport>; btcSigner: { signAllInputs(psbt: string): Promise<string> } } +): Promise<string>; +async executeQuote( + executeQuoteParams: OfframpExecuteQuoteParams, + clients: { walletClient: WalletClient<Transport, ViemChain, Account>; publicClient: PublicClient<Transport> } +): Promise<string>;
149-156:params.toChainmay beundefined, will throw on.toString()
WhenfromChain !== 'bitcoin', nothing guaranteestoChainis present. Callingparams.toChain.toString()onundefinedthrows before you reach the “Invalid quote arguments” check.- } else if (params.toChain.toString().toLowerCase() === 'bitcoin') { + } else if (params.toChain && params.toChain.toString().toLowerCase() === 'bitcoin') { ... - if (!params.fromToken) { + if (!params.toChain) { + throw new Error('`toChain` must be specified for offramp'); + } + if (!params.fromToken) { throw new Error('`fromToken` must be specified for offramp'); }
🧹 Nitpick comments (4)
sdk/examples/gateway.ts (2)
24-32: Unused_quoteMaxOnrampvariable
_quoteMaxOnrampis fetched but never referenced. Dead code hurts readability and confuses IDE/linters.- const _quoteMaxOnramp = await gatewaySDK.getQuote({ + // Drop this call unless you intend to use the result + /* const _quoteMaxOnramp = await gatewaySDK.getQuote({ ... - }); + }); */
1-70: Formatting fails Prettier check
CI reports a Prettier violation for this file. Runpnpm prettier --write sdk/examples/gateway.tsto avoid blocking merges.sdk/src/gateway/client.ts (2)
639-641: Mutates caller-suppliedparams(toChain) in-place
Directly rewritingparams.toChainsurprises callers who reuse the object.- if (typeof params.toChain === 'number') { - params.toChain = chainIdMapping[params.toChain]; - } + const localParams = { ...params }; + if (typeof localParams.toChain === 'number') { + localParams.toChain = chainIdMapping[localParams.toChain]; + }Apply the same pattern for the
fromChainbranch below (lines 659-661).
1-1: Prettier formatting check failed for this file
Runprettier --write sdk/src/gateway/client.tsto satisfy CI.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
sdk/examples/gateway.ts(1 hunks)sdk/src/gateway/client.ts(19 hunks)sdk/src/gateway/types.ts(7 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- sdk/src/gateway/types.ts
🧰 Additional context used
🪛 GitHub Actions: SDK
sdk/src/gateway/client.ts
[warning] 1-1: Prettier formatting check failed. Run 'prettier --write' to fix code style issues in this file.
sdk/examples/gateway.ts
[warning] 1-1: Prettier formatting check failed. Run 'prettier --write' to fix code style issues in this file.
🪛 Gitleaks (8.26.0)
sdk/examples/gateway.ts
7-7: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🔇 Additional comments (1)
sdk/src/gateway/client.ts (1)
675-677: ```shell
#!/bin/bashDisplay context around the allowance comparison to inspect how 'allowance' is obtained and its units.
sed -n '650,740p' sdk/src/gateway/client.ts
</details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
sdk/src/gateway/client.ts (2)
146-147: Guard against missingtoChainbefore calling.toString()
params.toChainis accessed unconditionally. When the caller omits this field (e.g. in an on-ramp flow) the expression throws before your own validation kicks in.- } else if (params.toChain.toString().toLowerCase() === 'bitcoin') { + } else if (params.toChain && params.toChain.toString().toLowerCase() === 'bitcoin') {
596-598: Overload signatures leakany– type theclientsparameterBoth overloads declare
clientswithout a type, undoing the benefit of the strongly-typed implementation that follows.-async executeQuote(executeQuoteParams: OnrampExecuteQuoteParams, clients): Promise<string>; -async executeQuote(executeQuoteParams: OfframpExecuteQuoteParams, clients): Promise<string>; +async executeQuote( + executeQuoteParams: OnrampExecuteQuoteParams, + clients: { walletClient: WalletClient<Transport, ViemChain, Account>; publicClient: PublicClient<Transport>; btcSigner: { signAllInputs(psbt: string): Promise<string> } } +): Promise<string>; +async executeQuote( + executeQuoteParams: OfframpExecuteQuoteParams, + clients: { walletClient: WalletClient<Transport, ViemChain, Account>; publicClient: PublicClient<Transport>; } +): Promise<string>;
🧹 Nitpick comments (3)
sdk/src/gateway/client.ts (1)
666-668: Clarify TODO – fee comparison mixes unitsThe comment hints at possible unit confusion (
satvs token amount). Confirm the business rule and replace the TODO with explicit validation or conversion to prevent incorrect allowance checks.sdk/test/gateway.test.ts (2)
648-657: Hard-coded private key-like strings flagged by scannersStatic analysis tagged these hex literals as “generic API keys”. They appear to be test addresses, but consider replacing with clearly dummy constants (e.g.
'0xdead...beef') to avoid audit noise.
839-843: Simulated contract response uses placeholder'🎉'
simulateContractstubs return{ request: '🎉' }. A non-hex string risks failing stricter type guards. Returning a deterministic hex (e.g.'0x1234') keeps the intent while remaining valid.Also applies to: 899-903
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
sdk/examples/gateway.ts(1 hunks)sdk/src/gateway/client.ts(16 hunks)sdk/src/gateway/types.ts(6 hunks)sdk/test/gateway.test.ts(27 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- sdk/examples/gateway.ts
- sdk/src/gateway/types.ts
🧰 Additional context used
🪛 Gitleaks (8.26.0)
sdk/test/gateway.test.ts
646-646: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
649-649: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
764-764: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
sdk/examples/gateway.ts (1)
15-20:zeroAddressplaceholder still blocks transaction signingThe wallet client is instantiated with
account: zeroAddress, so any subsequentwalletClient.writeContractorexecuteQuotecall will revert because there is no private key associated with the all-zero account. Replace the placeholder with the connected EOA (or remove the field and letwalletClientinfer the account from the connector).sdk/src/gateway/client.ts (2)
146-147:params.toChainmay be undefined – add a null-check before.toString()If callers omit
toChain(only supplyingfromChain), the expressionparams.toChain.toString()will throw. Guard early:if (!params.toChain) { throw new Error('`toChain` must be specified'); } if (params.toChain.toString().toLowerCase() === 'bitcoin') …
596-599: Overload signatures still leakany– type theclientsparameterThe two overloads keep
clientsuntyped, so every call site falls back toany, nullifying the benefit of strong typings added in the implementation. Re-use the object-literal type used below:-async executeQuote(executeQuoteParams: OnrampExecuteQuoteParams, clients): Promise<string>; -async executeQuote(executeQuoteParams: OfframpExecuteQuoteParams, clients): Promise<string>; +async executeQuote( + executeQuoteParams: OnrampExecuteQuoteParams, + clients: { walletClient: WalletClient<Transport, ViemChain, Account>; publicClient: PublicClient<Transport>; btcSigner: { signAllInputs(psbtBase64: string): Promise<string> } } +): Promise<string>; +async executeQuote( + executeQuoteParams: OfframpExecuteQuoteParams, + clients: { walletClient: WalletClient<Transport, ViemChain, Account>; publicClient: PublicClient<Transport>; } +): Promise<string>;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
sdk/examples/gateway.ts(1 hunks)sdk/src/gateway/client.ts(17 hunks)
🧰 Additional context used
🧠 Learnings (1)
sdk/src/gateway/client.ts (1)
Learnt from: slavastartsev
PR: bob-collective/bob#634
File: sdk/src/gateway/client.ts:136-140
Timestamp: 2025-06-17T11:18:53.153Z
Learning: The codebase has a requirement not to introduce discriminated unions in the Gateway SDK client methods, including the getQuote method return types in sdk/src/gateway/client.ts.
🧬 Code Graph Analysis (1)
sdk/src/gateway/client.ts (5)
sdk/src/gateway/strategy.ts (1)
StrategyClient(163-358)sdk/src/gateway/types.ts (11)
GetQuoteParams(544-544)GatewayQuote(223-240)GatewayTokensInfo(339-344)OfframpQuote(376-387)OfframpCreateOrderParams(400-421)OfframpOrder(448-469)GatewayStartOrder(353-357)OnrampExecuteQuoteParams(546-549)OfframpExecuteQuoteParams(551-554)ExecuteQuoteParams(558-558)chainIdMapping(23-26)sdk/src/gateway/tokens.ts (1)
getTokenAddress(372-386)sdk/src/wallet/utxo.ts (1)
getAddressInfo(22-24)sdk/src/gateway/utils.ts (1)
toHexScriptPubKey(29-33)
🪛 Gitleaks (8.26.0)
sdk/examples/gateway.ts
7-7: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
| const offrampQuote = await gatewaySDK.getQuote({ | ||
| fromChain: 'bob', | ||
| fromToken: BOB_TBTC_V2_TOKEN_ADDRESS, // or "tBTC" | ||
| toChain: 'bitcoin', | ||
| toUserAddress: 'bc1qafk4yhqvj4wep57m62dgrmutldusqde8adh20d', | ||
| toToken: 'BTC', | ||
| amount: 10000000, // 0.1 BTC | ||
| }); |
There was a problem hiding this comment.
Possible unit mismatch – amount for off-ramp quote must be in token decimals, not satoshis
getQuote interprets amount as wrapped-token units (tBTC, 18 dec by default), but the example passes 10_000_000, i.e. 0.00000001 tBTC, while the comment says “0.1 BTC”.
Convert the desired BTC value to wrapped-token decimals before calling:
- amount: 10000000, // 0.1 BTC
+ // 0.1 BTC in 18-dec wrapped token units
+ amount: 0.1 * 10 ** 18,(or compute it programmatically to avoid FP error).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const offrampQuote = await gatewaySDK.getQuote({ | |
| fromChain: 'bob', | |
| fromToken: BOB_TBTC_V2_TOKEN_ADDRESS, // or "tBTC" | |
| toChain: 'bitcoin', | |
| toUserAddress: 'bc1qafk4yhqvj4wep57m62dgrmutldusqde8adh20d', | |
| toToken: 'BTC', | |
| amount: 10000000, // 0.1 BTC | |
| }); | |
| const offrampQuote = await gatewaySDK.getQuote({ | |
| fromChain: 'bob', | |
| fromToken: BOB_TBTC_V2_TOKEN_ADDRESS, // or "tBTC" | |
| toChain: 'bitcoin', | |
| toUserAddress: 'bc1qafk4yhqvj4wep57m62dgrmutldusqde8adh20d', | |
| // 0.1 BTC in 18-decimal wrapped-token units | |
| amount: 0.1 * 10 ** 18, | |
| }); |
🤖 Prompt for AI Agents
In sdk/examples/gateway.ts around lines 44 to 51, the amount passed to getQuote
is incorrectly specified in satoshis instead of wrapped-token decimals. To fix
this, convert the BTC amount (0.1 BTC) to the equivalent value in wrapped-token
decimals (18 decimals) before passing it as the amount parameter. This can be
done by multiplying 0.1 by 10^18 or computing it programmatically to avoid
floating point errors.
| // TODO: is this sat or token amount? | ||
| if (BigInt(quote.amountLockInSat) > allowance) { | ||
| const { request } = await publicClient.simulateContract({ |
There was a problem hiding this comment.
Allowance check compares satoshis to ERC-20 units – always fails for real amounts
quote.amountLockInSat is in satoshis (8 dec) while allowance is in the token’s own decimals (usually 18). The current comparison will typically think the allowance is sufficient when it is not (or vice-versa). Convert the satoshi amount to token units first:
- // TODO: is this sat or token amount?
- if (BigInt(quote.amountLockInSat) > allowance) {
+ const decimals = getTokenDecimals(tokenAddress) ?? 18;
+ const amountInToken = BigInt(quote.amountLockInSat) * BigInt(10 ** (decimals - 8));
+
+ if (amountInToken > allowance) {This mirrors the conversion done in getBumpFeeRequirement.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // TODO: is this sat or token amount? | |
| if (BigInt(quote.amountLockInSat) > allowance) { | |
| const { request } = await publicClient.simulateContract({ | |
| const decimals = getTokenDecimals(tokenAddress) ?? 18; | |
| const amountInToken = BigInt(quote.amountLockInSat) * BigInt(10 ** (decimals - 8)); | |
| if (amountInToken > allowance) { | |
| const { request } = await publicClient.simulateContract({ | |
| // … |
🤖 Prompt for AI Agents
In sdk/src/gateway/client.ts around lines 667 to 669, the allowance check
incorrectly compares satoshis (quote.amountLockInSat) directly to the token
allowance, which uses different decimals (usually 18). To fix this, convert
quote.amountLockInSat from satoshis to the token's decimal units before
comparing it to allowance, following the same conversion approach used in
getBumpFeeRequirement.
Summary by CodeRabbit
New Features
Refactor
Addresstype from the Viem library throughout the SDK.Bug Fixes
Chores
@gobob/sats-wagmito support Bitcoin signing integration.