Skip to content

fix(xrp): route MPT token enablement to enableMpt type via coin config [CGD-2013]#9167

Merged
0xPrabh merged 1 commit into
masterfrom
CGD-2013-fix-xrp-mpt-token-enablement
Jul 2, 2026
Merged

fix(xrp): route MPT token enablement to enableMpt type via coin config [CGD-2013]#9167
0xPrabh merged 1 commit into
masterfrom
CGD-2013-fix-xrp-mpt-token-enablement

Conversation

@0xPrabh

@0xPrabh 0xPrabh commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Add getEnableTokenType to TokenEnablementConfig so coins can return the correct tx type per token
  • XRP returns 'enableMpt' for XrpMptCoin tokens and 'enabletoken' for IOU — fixes the txrp:feesec is not an XRP token error caused by hardcoded 'enabletoken' routing all enablements to the IOU/TrustSet path
  • Add xrpl codec fallbacks in ripple.ts and xrp.ts for MPTokenAuthorize (unsupported by ripple-binary-codec)
  • Simplify handleV2EnableTokens in express — remove offline MPT/IOU split, sendTokenEnablements handles both

Root Cause

wallet.buildTokenEnablements() hardcoded type = 'enabletoken', routing to wallet-platform's TrustSet path which calls getXrpCurrencyFromTokenNameinstanceof XrpCoin → throws for XrpMptCoin.

@0xPrabh 0xPrabh requested review from a team as code owners July 2, 2026 08:27
@linear-code

linear-code Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

CGD-2013

@0xPrabh 0xPrabh force-pushed the CGD-2013-fix-xrp-mpt-token-enablement branch from 88d4575 to 42cc7e0 Compare July 2, 2026 08:33
@0xPrabh

0xPrabh commented Jul 2, 2026

Copy link
Copy Markdown
Contributor Author

@claude

@github-actions

github-actions Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Claude finished @0xPrabh's task —— View job


Code Review for XRP MPT Token Enablement Fix

Overview

This PR successfully addresses the root cause of the MPT token enablement routing issue by adding proper type detection via coin configuration. The changes are well-structured and solve the hardcoded 'enabletoken' problem that was causing XrpMptCoin tokens to fail.

✅ Positive Changes

1. Core Interface Enhancement (iBaseCoin.ts)

  • Line 611: Added optional getEnableTokenType?: (tokenName: string) => string to TokenEnablementConfig
  • Clean interface addition that maintains backward compatibility

2. Smart Type Routing (wallet.ts:3978-3979)

  • Key Fix: Replaced hardcoded 'enabletoken' with dynamic type detection:
const tokenName = params.enableTokens[0]?.name;
buildParams.type = (tokenName && teConfig.getEnableTokenType?.(tokenName)) ?? 'enabletoken';
  • Proper fallback to 'enabletoken' maintains backward compatibility
  • Fixes the core issue described in the PR summary

3. XRP Implementation (xrp.ts:135-136)

  • Excellent Logic: Uses statics to differentiate MPT vs IOU tokens:
getEnableTokenType: (tokenName: string) =>
  coins.has(tokenName) && coins.get(tokenName) instanceof XrpMptCoin ? 'enableMpt' : 'enabletoken'
  • Leverages existing XrpMptCoin type checking from @bitgo/statics

4. Enhanced Verification (xrp.ts:457-465)

  • Added proper MPT verification logic that checks for MPTokenAuthorize transaction type
  • Maintains existing TrustSet verification for IOU tokens

5. Codec Fallbacks (ripple.ts and xrp.ts)

  • Smart Approach: Falls back to xrpl.decode() when ripple-binary-codec fails
  • Handles newer transaction types (like MPTokenAuthorize) not supported by the older codec version
  • Preserves error handling chain with proper error context

🔍 Areas for Consideration

1. Error Handling Edge Cases

In xrp.ts:336-346, consider what happens if xrpl.decode() also fails but the transaction is actually valid JSON. The current logic may lose some error context:

} catch (e2) {
  try {
    transaction = JSON.parse(txHex);
  } catch (e3) {
    throw new Error('txHex needs to be either hex or JSON string for XRP');
  }
}

Suggestion: Consider logging the original decode errors for better debugging.

2. Type Safety

The getEnableTokenType method returns string, but the consuming code expects specific values ('enableMpt', 'enabletoken'). Consider using a union type for better type safety:

getEnableTokenType?: (tokenName: string) => 'enableMpt' | 'enabletoken';

3. Documentation

The new getEnableTokenType interface method could benefit from JSDoc comments explaining its purpose and expected return values.

🚀 Overall Assessment

This is a well-executed fix that:

  • ✅ Addresses the exact root cause described in the PR
  • ✅ Maintains backward compatibility
  • ✅ Uses proper abstraction via the coin configuration system
  • ✅ Includes appropriate verification logic
  • ✅ Handles codec limitations gracefully
  • ✅ Follows existing code patterns and conventions

The solution is elegant and avoids hardcoding transaction types while providing the flexibility needed for different XRP token types. The codec fallback strategy is particularly well thought out.

Recommendation: APPROVE - This PR effectively solves the stated problem with minimal risk and good engineering practices.

…oin config

Add getEnableTokenType to TokenEnablementConfig so coins can return the
correct transaction type. XRP uses this to return 'enableMpt' for XrpMptCoin
tokens instead of hardcoded 'enabletoken', avoiding the IOU TrustSet path
that throws for MPT tokens. Remove redundant xrpl.decode fallbacks in
ripple.ts and xrp.ts since binary/rippleBinaryCodec are aliased to xrpl
which supports all tx types including MPTokenAuthorize. Simplify express
handleV2EnableTokens by removing the offline MPT/IOU split. Add unit tests
for getTokenEnablementConfig and MPTokenAuthorize explainTransaction paths.

TICKET: CGD-2013

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@0xPrabh 0xPrabh force-pushed the CGD-2013-fix-xrp-mpt-token-enablement branch from 42cc7e0 to d274c8e Compare July 2, 2026 08:59
Comment thread modules/sdk-coin-xrp/src/ripple.ts
@0xPrabh 0xPrabh merged commit 4918ce9 into master Jul 2, 2026
25 checks passed
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.

2 participants