Add XRPL TypeScript vault depositor#6
Conversation
ihsraham
force-pushed
the
feat/xrpl-ts-depositor
branch
from
c37fc66 to
ed1283a
Compare
nksazonov
left a comment
nksazonov
left a comment
There was a problem hiding this comment.
Nice work overall — the PR delivers exactly what the description promises: XrplVaultDepositor, native and issued-currency paths, a full unit test suite, Docker-backed integration tests, and the browser demo with both local-signer and GemWallet paths. The code is clean and consistent with the existing depositor patterns. There are a few small things to address before landing.
| minConfirmations: bigint | number, | ||
| ): Promise<DepositStatus> { | ||
| const normalized = requireTxRef(ref); | ||
| normalizeMinConfirmations(minConfirmations); |
There was a problem hiding this comment.
The return value of normalizeMinConfirmations(minConfirmations) is discarded. Both EvmVaultDepositor (evm/depositor.ts:85) and SolanaVaultDepositor (sol/depositor.ts:128) assign it as const minConf = normalizeMinConfirmations(...) and use it to gate the "confirmed" return. Here the result is thrown away, so verifyDeposit(ref, 0) and verifyDeposit(ref, 100) behave identically — if the tx is in a validated ledger, it returns "confirmed" immediately.
XRPL finality is binary (a transaction in a validated ledger is final; there's no block-depth equivalent), so the parameter genuinely cannot be honoured the way EVM/Solana honour it. Callers who copy the EVM/Solana pattern expecting a minimum threshold to be enforced are silently wrong.
Suggestion: add a code comment on this line explaining that XRPL finality is binary so the parameter is validated for interface consistency but has no effect on the returned status.
| private readonly signer: XrplSigner; | ||
| private readonly vaultAddress: string; | ||
| private readonly maxFeeDrops: bigint | undefined; | ||
| private readonly client: Client; |
There was a problem hiding this comment.
XrplVaultDepositor is the first depositor in this SDK backed by a persistent WebSocket (EVM and Solana use HTTP). The connection is opened lazily by ensureConnected() but there is no disconnect() method — callers have no way to close it.
The demo at main.ts compounds this: it creates a new depositor instance at lines 182 and 233 on every button click, opening a fresh connection each time without cleaning up the prior one. Server-side code that creates depositors per request will accumulate open WebSocket connections.
Suggestion: expose async disconnect(): Promise<void> { if (this.client.isConnected()) await this.client.disconnect(); } and reuse a single depositor instance across actions in the demo rather than constructing one per call.
| if (!destination || typeof destination !== "object") { | ||
| throw new ClearnetSdkError( | ||
| "INVALID_ADDRESS", | ||
| "destination.account must be a 20-byte hex address", |
There was a problem hiding this comment.
The error message "destination.account must be a 20-byte hex address" fires at this line when destination is null or not an object — before destination.account is ever accessed. Lines 90 and 98 use the same message for the actual account validation, which is correct, but the message at line 80 blames a nested field for a missing parent.
Suggestion: use "destination is required and must be an object" at this line.
| function requireSubmitDepositOptions(options: unknown): SubmitDepositOptions { | ||
| if (options === null || typeof options !== "object") { | ||
| throw new ClearnetSdkError( | ||
| "RPC_ERROR", |
There was a problem hiding this comment.
RPC_ERROR is used throughout the codebase for XRPL node failures (network errors, rejected transactions). A null options argument is a caller mistake, not a node event. Callers who catch RPC_ERROR to handle connectivity problems will accidentally swallow this validation error.
Suggestion: add "INVALID_INPUT" to the ClearnetSdkErrorCode union in core/errors.ts and use it here, or at minimum use one of the existing INVALID_* codes.
dimast-x
left a comment
dimast-x
left a comment
There was a problem hiding this comment.
Good to merge once Nikita's comments are resolved
3 participants
Summary
Adds XRPL support to the TypeScript SDK vault depositor surface.
This includes:
XrplVaultDepositorexported from@yellow-org/clearnet-sdkbigintCUR.rIssuer/CUR:rIssuerasset keys and decimal string amountsPaymentmemo encoding for the Clearnet destination account and optional referenceDecisions Incorporated
XrplSignerinterface.maxFeeDropsceiling for callers that want one.destination.account/destination.refdeposit shape across chains.Demo Recordings
Local Signer Demo
GemWallet Demo
XRPL Demo Setup Notes
The XRPL demo now has two paths:
ledger_accept, and verifies the last transaction.wss://endpoint.The local rippled devnet uses
network_id: 31337. This intentionally avoids21337and21338, which are Xahau mainnet/testnet IDs. Reusing those IDs made the local chain look like Xahau to wallet tooling and caused signing/submission failures that were hard to distinguish from SDK issues.For the GemWallet path, the local rippled WebSocket (
ws://127.0.0.1:6006) needs a WSS tunnel or TLS proxy because GemWallet custom networks requirewss://. The working setup was:127.0.0.1:6006through a WSS endpoint, for example with ngrok.WebSocket URLto the same WSS endpoint.Admin HTTP URLas/xrpl-adminso the local demo can call standaloneledger_accept.The full runbook is documented in
sdk/ts/examples/xrpl-deposit/README.md.Issue Found During Demo Validation
The GemWallet path had two different failure modes:
server_info.network_idwith the demo RPC before opening the signing request.NetworkID: 31337. The demo now removesNetworkIDonly from the transaction object passed to GemWallet after the endpoint/network check. GemWallet autofillsNetworkIDfrom its selected custom endpoint before signing; tx lookup confirmed the submitted transaction carriedNetworkID: 31337and validated withtesSUCCESS.Validation
make devnetmake integrationnpm --prefix sdk/ts run typechecknpm --prefix sdk/ts testnpm --prefix sdk/ts run buildnpm --prefix sdk/ts --workspace @yellow-org/xrpl-deposit-demo run buildnpm --prefix sdk/ts audit --omit=dev --audit-level=moderategit diff --checkLive XRPL demo checks completed locally:
confirmedconfirmedtxlookup for the GemWallet-submitted hash returnedvalidated: true,TransactionResult: tesSUCCESS,NetworkID: 31337, anddelivered_amount: 1000000