fix: tx fields for accounts allowing empty string (#2525)

Fields that were encoded as STAccounts were turning '' into rrrrrrrrrrrrrrrrrrrrrhoLvTp the ACCOUNT_ZERO account. They will now throw a ValidationError.

Invalid addresses on a transaction now throws a ValidationError when submitting a transaction instead of Error('checksum_invalid').

This change updates some error messages for fields a few fields that aren't accounts, DestinationTag or NFTokenID. They will still be of type ValidationError.

Fixes: #2517 and #2475

---------

Co-authored-by: Elliot Lee <github.public@intelliot.com>

packages/xrpl/HISTORY.md

@@ -9,10 +9,14 @@ Subscribe to [the **xrpl-announce** mailing list](https://groups.google.com/g/xr
 * Add pseudo transaction types to `tx` and `ledger` method responses.
 * Add missing `type` param to `ledger_data` and `ledger` requests
 * Type assertions around `PreviousTxnID` and `PreviousTxnLgrSeq` missing on some ledger objects
+* Transaction fields that represent an address no longer allow an empty string (`''`). If you want to specify [ACCOUNT_ZERO](https://xrpl.org/addresses.html#special-addresses), you can specify `rrrrrrrrrrrrrrrrrrrrrhoLvTp`. ⚠️ **WARNING:** `rrrrrrrrrrrrrrrrrrrrrhoLvTp` is a black hole address, with no corresponding private key. Accounts/funds controlled by this address are not accessible.
+* Invalid addresses on a transaction now throws a `ValidationError` when submitting a transaction instead of `Error('checksum_invalid')`
 
 ### Updated
 * Make `LedgerEntryResponse` a generic so it can be used like `LedgerEntryResponse<Escrow>`
 * Clean up typing of `type` param and the response property `account_objects` of the `account_objects` request.
+* Error messages for fields that equate to an address, `DestinationTag`, or `NFTokenID`.  They will still be of type `ValidationError`.
+* Add alias type of `Account` to improve intellisense for Transaction fields that equate to an address.
 
 ### Changed
 * Removed sidechain-devnet faucet support as it is being moved to Devnet

packages/xrpl/src/models/transactions/NFTokenBurn.ts

@@ -1,6 +1,12 @@
-import { ValidationError } from '../../errors'
+import {
-
+  Account,
-import { BaseTransaction, validateBaseTransaction } from './common'
+  BaseTransaction,
+  isAccount,
+  isString,
+  validateBaseTransaction,
+  validateOptionalField,
+  validateRequiredField,
+} from './common'
 
 /**
  * The NFTokenBurn transaction is used to remove an NFToken object from the
@@ -20,7 +26,7 @@ export interface NFTokenBurn extends BaseTransaction {
    * in the NFToken, either the issuer account or an account authorized by the
    * issuer, i.e. MintAccount.
    */
-  Account: string
+  Account: Account
   /**
    * Identifies the NFToken object to be removed by the transaction.
    */
@@ -30,7 +36,7 @@ export interface NFTokenBurn extends BaseTransaction {
    * Account. Only used to burn tokens which have the lsfBurnable flag enabled
    * and are not owned by the signing account.
    */
-  Owner?: string
+  Owner?: Account
 }
 
 /**
@@ -41,8 +47,6 @@ export interface NFTokenBurn extends BaseTransaction {
  */
 export function validateNFTokenBurn(tx: Record<string, unknown>): void {
   validateBaseTransaction(tx)
-
+  validateRequiredField(tx, 'NFTokenID', isString)
-  if (tx.NFTokenID == null) {
+  validateOptionalField(tx, 'Owner', isAccount)
-    throw new ValidationError('NFTokenBurn: missing field NFTokenID')
-  }
 }

packages/xrpl/src/models/transactions/NFTokenCreateOffer.ts

@@ -8,6 +8,9 @@ import {
   validateBaseTransaction,
   isAmount,
   parseAmountValue,
+  isAccount,
+  validateOptionalField,
+  Account,
 } from './common'
 
 /**
@@ -67,7 +70,7 @@ export interface NFTokenCreateOffer extends BaseTransaction {
    * (since an offer to sell a token one doesn't already hold
    * is meaningless).
    */
-  Owner?: string
+  Owner?: Account
   /**
    * Indicates the time after which the offer will no longer
    * be valid. The value is the number of seconds since the
@@ -79,7 +82,7 @@ export interface NFTokenCreateOffer extends BaseTransaction {
    * accepted by the specified account. Attempts by other
    * accounts to accept this offer MUST fail.
    */
-  Destination?: string
+  Destination?: Account
   Flags?: number | NFTokenCreateOfferFlagsInterface
 }
 
@@ -126,6 +129,9 @@ export function validateNFTokenCreateOffer(tx: Record<string, unknown>): void {
     )
   }
 
+  validateOptionalField(tx, 'Destination', isAccount)
+  validateOptionalField(tx, 'Owner', isAccount)
+
   if (tx.NFTokenID == null) {
     throw new ValidationError('NFTokenCreateOffer: missing field NFTokenID')
   }

packages/xrpl/src/models/transactions/NFTokenMint.ts

@@ -1,7 +1,14 @@
 import { ValidationError } from '../../errors'
 import { isHex } from '../utils'
 
-import { BaseTransaction, GlobalFlags, validateBaseTransaction } from './common'
+import {
+  Account,
+  BaseTransaction,
+  GlobalFlags,
+  isAccount,
+  validateBaseTransaction,
+  validateOptionalField,
+} from './common'
 
 /**
  * Transaction Flags for an NFTokenMint Transaction.
@@ -68,7 +75,7 @@ export interface NFTokenMint extends BaseTransaction {
    * present, the `MintAccount` field in the `AccountRoot` of the `Issuer`
    * field must match the `Account`, otherwise the transaction will fail.
    */
-  Issuer?: string
+  Issuer?: Account
   /**
    * Specifies the fee charged by the issuer for secondary sales of the Token,
    * if such sales are allowed. Valid values for this field are between 0 and
@@ -109,6 +116,8 @@ export function validateNFTokenMint(tx: Record<string, unknown>): void {
     )
   }
 
+  validateOptionalField(tx, 'Issuer', isAccount)
+
   if (typeof tx.URI === 'string' && tx.URI === '') {
     throw new ValidationError('NFTokenMint: URI must not be empty string')
   }

packages/xrpl/src/models/transactions/XChainAccountCreateCommit.ts

@@ -4,9 +4,10 @@ import {
   BaseTransaction,
   isAmount,
   isXChainBridge,
-  isString,
   validateBaseTransaction,
   validateRequiredField,
+  isAccount,
+  Account,
 } from './common'
 
 /**
@@ -38,7 +39,7 @@ export interface XChainAccountCreateCommit extends BaseTransaction {
   /**
    * The destination account on the destination chain.
    */
-  Destination: string
+  Destination: Account
 
   /**
    * The amount, in XRP, to use for account creation. This must be greater than or
@@ -62,7 +63,7 @@ export function validateXChainAccountCreateCommit(
 
   validateRequiredField(tx, 'SignatureReward', isAmount)
 
-  validateRequiredField(tx, 'Destination', isString)
+  validateRequiredField(tx, 'Destination', isAccount)
 
   validateRequiredField(tx, 'Amount', isAmount)
 }

packages/xrpl/src/models/transactions/XChainAddAccountCreateAttestation.ts

@@ -1,7 +1,9 @@
 import { Amount, XChainBridge } from '../common'
 
 import {
+  Account,
   BaseTransaction,
+  isAccount,
   isAmount,
   isNumber,
   isString,
@@ -29,23 +31,23 @@ export interface XChainAddAccountCreateAttestation extends BaseTransaction {
   /**
    * The account that should receive this signer's share of the SignatureReward.
    */
-  AttestationRewardAccount: string
+  AttestationRewardAccount: Account
 
   /**
    * The account on the door account's signer list that is signing the transaction.
    */
-  AttestationSignerAccount: string
+  AttestationSignerAccount: Account
 
   /**
    * The destination account for the funds on the destination chain.
    */
-  Destination: string
+  Destination: Account
 
   /**
    * The account on the source chain that submitted the {@link XChainAccountCreateCommit}
    * transaction that triggered the event associated with the attestation.
    */
-  OtherChainSource: string
+  OtherChainSource: Account
 
   /**
    * The public key used to verify the signature.
@@ -91,13 +93,13 @@ export function validateXChainAddAccountCreateAttestation(
 
   validateRequiredField(tx, 'Amount', isAmount)
 
-  validateRequiredField(tx, 'AttestationRewardAccount', isString)
+  validateRequiredField(tx, 'AttestationRewardAccount', isAccount)
 
-  validateRequiredField(tx, 'AttestationSignerAccount', isString)
+  validateRequiredField(tx, 'AttestationSignerAccount', isAccount)
 
-  validateRequiredField(tx, 'Destination', isString)
+  validateRequiredField(tx, 'Destination', isAccount)
 
-  validateRequiredField(tx, 'OtherChainSource', isString)
+  validateRequiredField(tx, 'OtherChainSource', isAccount)
 
   validateRequiredField(tx, 'PublicKey', isString)
 

packages/xrpl/src/models/transactions/XChainAddClaimAttestation.ts

@@ -1,7 +1,9 @@
 import { Amount, XChainBridge } from '../common'
 
 import {
+  Account,
   BaseTransaction,
+  isAccount,
   isAmount,
   isNumber,
   isString,
@@ -28,24 +30,24 @@ export interface XChainAddClaimAttestation extends BaseTransaction {
   /**
    * The account that should receive this signer's share of the SignatureReward.
    */
-  AttestationRewardAccount: string
+  AttestationRewardAccount: Account
 
   /**
    * The account on the door account's signer list that is signing the transaction.
    */
-  AttestationSignerAccount: string
+  AttestationSignerAccount: Account
 
   /**
    * The destination account for the funds on the destination chain (taken from
    * the {@link XChainCommit} transaction).
    */
-  Destination?: string
+  Destination?: Account
 
   /**
    * The account on the source chain that submitted the {@link XChainCommit}
    * transaction that triggered the event associated with the attestation.
    */
-  OtherChainSource: string
+  OtherChainSource: Account
 
   /**
    * The public key used to verify the attestation signature.
@@ -87,13 +89,13 @@ export function validateXChainAddClaimAttestation(
 
   validateRequiredField(tx, 'Amount', isAmount)
 
-  validateRequiredField(tx, 'AttestationRewardAccount', isString)
+  validateRequiredField(tx, 'AttestationRewardAccount', isAccount)
 
-  validateRequiredField(tx, 'AttestationSignerAccount', isString)
+  validateRequiredField(tx, 'AttestationSignerAccount', isAccount)
 
-  validateOptionalField(tx, 'Destination', isString)
+  validateOptionalField(tx, 'Destination', isAccount)
 
-  validateRequiredField(tx, 'OtherChainSource', isString)
+  validateRequiredField(tx, 'OtherChainSource', isAccount)
 
   validateRequiredField(tx, 'PublicKey', isString)
 

packages/xrpl/src/models/transactions/XChainClaim.ts

@@ -1,7 +1,9 @@
 import { Amount, XChainBridge } from '../common'
 
 import {
+  Account,
   BaseTransaction,
+  isAccount,
   isAmount,
   isNumber,
   isString,
@@ -38,7 +40,7 @@ export interface XChainClaim extends BaseTransaction {
    * sequence number and collected signatures won't be destroyed, and the
    * transaction can be rerun with a different destination.
    */
-  Destination: string
+  Destination: Account
 
   /**
    * An integer destination tag.
@@ -69,7 +71,7 @@ export function validateXChainClaim(tx: Record<string, unknown>): void {
     (inp) => isNumber(inp) || isString(inp),
   )
 
-  validateRequiredField(tx, 'Destination', isString)
+  validateRequiredField(tx, 'Destination', isAccount)
 
   validateOptionalField(tx, 'DestinationTag', isNumber)
 

packages/xrpl/src/models/transactions/XChainCommit.ts

@@ -1,7 +1,9 @@
 import { Amount, XChainBridge } from '../common'
 
 import {
+  Account,
   BaseTransaction,
+  isAccount,
   isAmount,
   isNumber,
   isString,
@@ -41,7 +43,7 @@ export interface XChainCommit extends BaseTransaction {
    * destination chain will need to submit a {@link XChainClaim} transaction to
    * claim the funds.
    */
-  OtherChainDestination?: string
+  OtherChainDestination?: Account
 
   /**
    * The asset to commit, and the quantity. This must match the door account's
@@ -68,7 +70,7 @@ export function validateXChainCommit(tx: Record<string, unknown>): void {
     (inp) => isNumber(inp) || isString(inp),
   )
 
-  validateOptionalField(tx, 'OtherChainDestination', isString)
+  validateOptionalField(tx, 'OtherChainDestination', isAccount)
 
   validateRequiredField(tx, 'Amount', isAmount)
 }

packages/xrpl/src/models/transactions/XChainCreateClaimID.ts

@@ -1,9 +1,10 @@
 import { Amount, XChainBridge } from '../common'
 
 import {
+  Account,
   BaseTransaction,
+  isAccount,
   isAmount,
-  isString,
   isXChainBridge,
   validateBaseTransaction,
   validateRequiredField,
@@ -33,7 +34,7 @@ export interface XChainCreateClaimID extends BaseTransaction {
   /**
    * The account that must send the {@link XChainCommit} transaction on the source chain.
    */
-  OtherChainSource: string
+  OtherChainSource: Account
 }
 
 /**
@@ -49,5 +50,5 @@ export function validateXChainCreateClaimID(tx: Record<string, unknown>): void {
 
   validateRequiredField(tx, 'SignatureReward', isAmount)
 
-  validateRequiredField(tx, 'OtherChainSource', isString)
+  validateRequiredField(tx, 'OtherChainSource', isAccount)
 }

packages/xrpl/src/models/transactions/accountDelete.ts

@@ -1,6 +1,12 @@
-import { ValidationError } from '../../errors'
+import {
-
+  Account,
-import { BaseTransaction, validateBaseTransaction } from './common'
+  BaseTransaction,
+  isAccount,
+  isNumber,
+  validateBaseTransaction,
+  validateOptionalField,
+  validateRequiredField,
+} from './common'
 
 /**
  * An AccountDelete transaction deletes an account and any objects it owns in
@@ -16,7 +22,7 @@ export interface AccountDelete extends BaseTransaction {
    * sending account. Must be a funded account in the ledger, and must not be.
    * the sending account.
    */
-  Destination: string
+  Destination: Account
   /**
    * Arbitrary destination tag that identifies a hosted recipient or other.
    * information for the recipient of the deleted account's leftover XRP.
@@ -33,18 +39,6 @@ export interface AccountDelete extends BaseTransaction {
 export function validateAccountDelete(tx: Record<string, unknown>): void {
   validateBaseTransaction(tx)
 
-  if (tx.Destination === undefined) {
+  validateRequiredField(tx, 'Destination', isAccount)
-    throw new ValidationError('AccountDelete: missing field Destination')
+  validateOptionalField(tx, 'DestinationTag', isNumber)
-  }
-
-  if (typeof tx.Destination !== 'string') {
-    throw new ValidationError('AccountDelete: invalid Destination')
-  }
-
-  if (
-    tx.DestinationTag !== undefined &&
-    typeof tx.DestinationTag !== 'number'
-  ) {
-    throw new ValidationError('AccountDelete: invalid DestinationTag')
-  }
 }

packages/xrpl/src/models/transactions/accountSet.ts

@@ -1,8 +1,12 @@
-import { isValidClassicAddress } from 'ripple-address-codec'
-
 import { ValidationError } from '../../errors'
 
-import { BaseTransaction, validateBaseTransaction } from './common'
+import {
+  Account,
+  BaseTransaction,
+  isAccount,
+  validateBaseTransaction,
+  validateOptionalField,
+} from './common'
 
 /**
  * Enum for AccountSet Flags.
@@ -155,7 +159,7 @@ export interface AccountSet extends BaseTransaction {
    * Sets an alternate account that is allowed to mint NFTokens on this
    * account's behalf using NFTokenMint's `Issuer` field.
    */
-  NFTokenMinter?: string
+  NFTokenMinter?: Account
 }
 
 const MIN_TICK_SIZE = 3
@@ -167,16 +171,11 @@ const MAX_TICK_SIZE = 15
  * @param tx - An AccountSet Transaction.
  * @throws When the AccountSet is Malformed.
  */
-// eslint-disable-next-line max-lines-per-function, max-statements -- okay for this method, only a little over
+// eslint-disable-next-line max-lines-per-function -- okay for this method, only a little over
 export function validateAccountSet(tx: Record<string, unknown>): void {
   validateBaseTransaction(tx)
 
-  if (
+  validateOptionalField(tx, 'NFTokenMinter', isAccount)
-    tx.NFTokenMinter !== undefined &&
-    !isValidClassicAddress(String(tx.NFTokenMinter))
-  ) {
-    throw new ValidationError('AccountSet: invalid NFTokenMinter')
-  }
 
   if (tx.ClearFlag !== undefined) {
     if (typeof tx.ClearFlag !== 'number') {

packages/xrpl/src/models/transactions/checkCreate.ts

@@ -5,6 +5,11 @@ import {
   BaseTransaction,
   validateBaseTransaction,
   isIssuedCurrency,
+  isAccount,
+  validateRequiredField,
+  validateOptionalField,
+  isNumber,
+  Account,
 } from './common'
 
 /**
@@ -17,7 +22,7 @@ import {
 export interface CheckCreate extends BaseTransaction {
   TransactionType: 'CheckCreate'
   /** The unique address of the account that can cash the Check. */
-  Destination: string
+  Destination: Account
   /**
    * Maximum amount of source currency the Check is allowed to debit the
    * sender, including transfer fees on non-XRP currencies. The Check can only
@@ -56,9 +61,8 @@ export function validateCheckCreate(tx: Record<string, unknown>): void {
     throw new ValidationError('CheckCreate: missing field SendMax')
   }
 
-  if (tx.Destination === undefined) {
+  validateRequiredField(tx, 'Destination', isAccount)
-    throw new ValidationError('CheckCreate: missing field Destination')
+  validateOptionalField(tx, 'DestinationTag', isNumber)
-  }
 
   if (
     typeof tx.SendMax !== 'string' &&
@@ -68,17 +72,6 @@ export function validateCheckCreate(tx: Record<string, unknown>): void {
     throw new ValidationError('CheckCreate: invalid SendMax')
   }
 
-  if (typeof tx.Destination !== 'string') {
-    throw new ValidationError('CheckCreate: invalid Destination')
-  }
-
-  if (
-    tx.DestinationTag !== undefined &&
-    typeof tx.DestinationTag !== 'number'
-  ) {
-    throw new ValidationError('CheckCreate: invalid DestinationTag')
-  }
-
   if (tx.Expiration !== undefined && typeof tx.Expiration !== 'number') {
     throw new ValidationError('CheckCreate: invalid Expiration')
   }

packages/xrpl/src/models/transactions/common.ts

@@ -1,3 +1,4 @@
+import { isValidClassicAddress, isValidXAddress } from 'ripple-address-codec'
 import { TRANSACTION_TYPES } from 'ripple-binary-codec'
 
 import { ValidationError } from '../../errors'
@@ -118,6 +119,24 @@ export function isIssuedCurrency(
   )
 }
 
+/**
+ * Must be a valid account address
+ */
+export type Account = string
+
+/**
+ * Verify a string is in fact a valid account address.
+ *
+ * @param account - The object to check the form and type of.
+ * @returns Whether the account is properly formed account for a transaction.
+ */
+export function isAccount(account: unknown): account is Account {
+  return (
+    typeof account === 'string' &&
+    (isValidClassicAddress(account) || isValidXAddress(account))
+  )
+}
+
 /**
  * Verify the form and type of an Amount at runtime.
  *
@@ -203,7 +222,7 @@ export interface GlobalFlags {}
  */
 export interface BaseTransaction {
   /** The unique address of the transaction sender. */
-  Account: string
+  Account: Account
   /**
    * The type of transaction. Valid types include: `Payment`, `OfferCreate`,
    * `TrustSet`, and many others.

packages/xrpl/src/models/transactions/escrowCancel.ts

@@ -1,6 +1,12 @@
 import { ValidationError } from '../../errors'
 
-import { BaseTransaction, validateBaseTransaction } from './common'
+import {
+  Account,
+  BaseTransaction,
+  isAccount,
+  validateBaseTransaction,
+  validateRequiredField,
+} from './common'
 
 /**
  * Return escrowed XRP to the sender.
@@ -10,7 +16,7 @@ import { BaseTransaction, validateBaseTransaction } from './common'
 export interface EscrowCancel extends BaseTransaction {
   TransactionType: 'EscrowCancel'
   /** Address of the source account that funded the escrow payment. */
-  Owner: string
+  Owner: Account
   /**
    * Transaction sequence (or Ticket  number) of EscrowCreate transaction that.
    * created the escrow to cancel.
@@ -27,13 +33,7 @@ export interface EscrowCancel extends BaseTransaction {
 export function validateEscrowCancel(tx: Record<string, unknown>): void {
   validateBaseTransaction(tx)
 
-  if (tx.Owner == null) {
+  validateRequiredField(tx, 'Owner', isAccount)
-    throw new ValidationError('EscrowCancel: missing Owner')
-  }
-
-  if (typeof tx.Owner !== 'string') {
-    throw new ValidationError('EscrowCancel: Owner must be a string')
-  }
 
   if (tx.OfferSequence == null) {
     throw new ValidationError('EscrowCancel: missing OfferSequence')

packages/xrpl/src/models/transactions/escrowCreate.ts

@@ -1,6 +1,14 @@
 import { ValidationError } from '../../errors'
 
-import { BaseTransaction, validateBaseTransaction } from './common'
+import {
+  Account,
+  BaseTransaction,
+  isAccount,
+  isNumber,
+  validateBaseTransaction,
+  validateOptionalField,
+  validateRequiredField,
+} from './common'
 
 /**
  * Sequester XRP until the escrow process either finishes or is canceled.
@@ -16,7 +24,7 @@ export interface EscrowCreate extends BaseTransaction {
    */
   Amount: string
   /** Address to receive escrowed XRP. */
-  Destination: string
+  Destination: Account
   /**
    * The time, in seconds since the Ripple Epoch, when this escrow expires.
    * This value is immutable; the funds can only be returned the sender after.
@@ -58,13 +66,8 @@ export function validateEscrowCreate(tx: Record<string, unknown>): void {
     throw new ValidationError('EscrowCreate: Amount must be a string')
   }
 
-  if (tx.Destination === undefined) {
+  validateRequiredField(tx, 'Destination', isAccount)
-    throw new ValidationError('EscrowCreate: missing field Destination')
+  validateOptionalField(tx, 'DestinationTag', isNumber)
-  }
-
-  if (typeof tx.Destination !== 'string') {
-    throw new ValidationError('EscrowCreate: Destination must be a string')
-  }
 
   if (tx.CancelAfter === undefined && tx.FinishAfter === undefined) {
     throw new ValidationError(
@@ -89,11 +92,4 @@ export function validateEscrowCreate(tx: Record<string, unknown>): void {
   if (tx.Condition !== undefined && typeof tx.Condition !== 'string') {
     throw new ValidationError('EscrowCreate: Condition must be a string')
   }
-
-  if (
-    tx.DestinationTag !== undefined &&
-    typeof tx.DestinationTag !== 'number'
-  ) {
-    throw new ValidationError('EscrowCreate: DestinationTag must be a number')
-  }
 }

packages/xrpl/src/models/transactions/escrowFinish.ts

@@ -1,6 +1,12 @@
 import { ValidationError } from '../../errors'
 
-import { BaseTransaction, validateBaseTransaction } from './common'
+import {
+  Account,
+  BaseTransaction,
+  isAccount,
+  validateBaseTransaction,
+  validateRequiredField,
+} from './common'
 
 /**
  * Deliver XRP from a held payment to the recipient.
@@ -10,7 +16,7 @@ import { BaseTransaction, validateBaseTransaction } from './common'
 export interface EscrowFinish extends BaseTransaction {
   TransactionType: 'EscrowFinish'
   /** Address of the source account that funded the held payment. */
-  Owner: string
+  Owner: Account
   /**
    * Transaction sequence of EscrowCreate transaction that created the held.
    * payment to finish.
@@ -37,13 +43,7 @@ export interface EscrowFinish extends BaseTransaction {
 export function validateEscrowFinish(tx: Record<string, unknown>): void {
   validateBaseTransaction(tx)
 
-  if (tx.Owner == null) {
+  validateRequiredField(tx, 'Owner', isAccount)
-    throw new ValidationError('EscrowFinish: missing field Owner')
-  }
-
-  if (typeof tx.Owner !== 'string') {
-    throw new ValidationError('EscrowFinish: Owner must be a string')
-  }
 
   if (tx.OfferSequence == null) {
     throw new ValidationError('EscrowFinish: missing field OfferSequence')

packages/xrpl/src/models/transactions/payment.ts

@@ -7,6 +7,11 @@ import {
   isAmount,
   GlobalFlags,
   validateBaseTransaction,
+  isAccount,
+  validateRequiredField,
+  validateOptionalField,
+  isNumber,
+  Account,
 } from './common'
 
 /**
@@ -112,7 +117,7 @@ export interface Payment extends BaseTransaction {
    */
   Amount: Amount
   /** The unique address of the account receiving the payment. */
-  Destination: string
+  Destination: Account
   /**
    * Arbitrary tag that identifies the reason for the payment to the
    * destination, or a hosted recipient to pay.
@@ -163,19 +168,8 @@ export function validatePayment(tx: Record<string, unknown>): void {
     throw new ValidationError('PaymentTransaction: invalid Amount')
   }
 
-  if (tx.Destination === undefined) {
+  validateRequiredField(tx, 'Destination', isAccount)
-    throw new ValidationError('PaymentTransaction: missing field Destination')
+  validateOptionalField(tx, 'DestinationTag', isNumber)
-  }
-
-  if (!isAmount(tx.Destination)) {
-    throw new ValidationError('PaymentTransaction: invalid Destination')
-  }
-
-  if (tx.DestinationTag != null && typeof tx.DestinationTag !== 'number') {
-    throw new ValidationError(
-      'PaymentTransaction: DestinationTag must be a number',
-    )
-  }
 
   if (tx.InvoiceID !== undefined && typeof tx.InvoiceID !== 'string') {
     throw new ValidationError('PaymentTransaction: InvoiceID must be a string')

packages/xrpl/src/models/transactions/paymentChannelCreate.ts

@@ -1,6 +1,14 @@
 import { ValidationError } from '../../errors'
 
-import { BaseTransaction, validateBaseTransaction } from './common'
+import {
+  Account,
+  BaseTransaction,
+  isAccount,
+  isNumber,
+  validateBaseTransaction,
+  validateOptionalField,
+  validateRequiredField,
+} from './common'
 
 /**
  * Create a unidirectional channel and fund it with XRP. The address sending
@@ -21,7 +29,7 @@ export interface PaymentChannelCreate extends BaseTransaction {
    * Address to receive XRP claims against this channel. This is also known as
    * the "destination address" for the channel.
    */
-  Destination: string
+  Destination: Account
   /**
    * Amount of time the source address must wait before closing the channel if
    * it has unclaimed XRP.
@@ -54,7 +62,6 @@ export interface PaymentChannelCreate extends BaseTransaction {
  * @param tx - An PaymentChannelCreate Transaction.
  * @throws When the PaymentChannelCreate is Malformed.
  */
-// eslint-disable-next-line max-lines-per-function -- okay for this function, there's a lot of things to check
 export function validatePaymentChannelCreate(
   tx: Record<string, unknown>,
 ): void {
@@ -68,15 +75,8 @@ export function validatePaymentChannelCreate(
     throw new ValidationError('PaymentChannelCreate: Amount must be a string')
   }
 
-  if (tx.Destination === undefined) {
+  validateRequiredField(tx, 'Destination', isAccount)
-    throw new ValidationError('PaymentChannelCreate: missing Destination')
+  validateOptionalField(tx, 'DestinationTag', isNumber)
-  }
-
-  if (typeof tx.Destination !== 'string') {
-    throw new ValidationError(
-      'PaymentChannelCreate: Destination must be a string',
-    )
-  }
 
   if (tx.SettleDelay === undefined) {
     throw new ValidationError('PaymentChannelCreate: missing SettleDelay')
@@ -103,13 +103,4 @@ export function validatePaymentChannelCreate(
       'PaymentChannelCreate: CancelAfter must be a number',
     )
   }
-
-  if (
-    tx.DestinationTag !== undefined &&
-    typeof tx.DestinationTag !== 'number'
-  ) {
-    throw new ValidationError(
-      'PaymentChannelCreate: DestinationTag must be a number',
-    )
-  }
 }

packages/xrpl/test/models/NFTokenCreateOffer.test.ts

@@ -16,7 +16,7 @@ describe('NFTokenCreateOffer', function () {
       TransactionType: 'NFTokenCreateOffer',
       NFTokenID: NFTOKEN_ID,
       Amount: '1',
-      Owner: 'r9LqNeG6qHxjeUocjvVki2XR35weJ9mZgQ',
+      Owner: 'rcXY84C4g14iFp6taFXjjQGVeHqSCh9RX',
       Expiration: 1000,
       Account: 'rWYkbWkCeg8dP6rXALnjgZSjjLyih5NXm',
       Destination: 'r9LqNeG6qHxjeUocjvVki2XR35weJ9mZgQ',
@@ -104,7 +104,7 @@ describe('NFTokenCreateOffer', function () {
     const invalid = {
       TransactionType: 'NFTokenCreateOffer',
       Amount: '1',
-      Owner: 'rWYkbWkCeg8dP6rXALnjgZSjjLyih5NXe',
+      Owner: 'rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn',
       Expiration: 1000,
       Destination: 'r9LqNeG6qHxjeUocjvVki2XR35weJ9mZgQ',
       Account: 'rWYkbWkCeg8dP6rXALnjgZSjjLyih5NXm',
@@ -124,7 +124,7 @@ describe('NFTokenCreateOffer', function () {
       TransactionType: 'NFTokenCreateOffer',
       NFTokenID: NFTOKEN_ID,
       Amount: 1,
-      Owner: 'rWYkbWkCeg8dP6rXALnjgZSjjLyih5NXe',
+      Owner: 'rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn',
       Expiration: 1000,
       Destination: 'r9LqNeG6qHxjeUocjvVki2XR35weJ9mZgQ',
       Account: 'rWYkbWkCeg8dP6rXALnjgZSjjLyih5NXm',
@@ -142,7 +142,7 @@ describe('NFTokenCreateOffer', function () {
   it(`throws w/ missing Amount`, function () {
     const invalid = {
       TransactionType: 'NFTokenCreateOffer',
-      Owner: 'rWYkbWkCeg8dP6rXALnjgZSjjLyih5NXe',
+      Owner: 'rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn',
       Expiration: 1000,
       NFTokenID: NFTOKEN_ID,
       Destination: 'r9LqNeG6qHxjeUocjvVki2XR35weJ9mZgQ',
@@ -162,7 +162,7 @@ describe('NFTokenCreateOffer', function () {
     const invalid = {
       TransactionType: 'NFTokenCreateOffer',
       Expiration: 1000,
-      Owner: 'r9LqNeG6qHxjeUocjvVki2XR35weJ9mZgQ',
+      Owner: 'rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn',
       Account: 'rWYkbWkCeg8dP6rXALnjgZSjjLyih5NXm',
       NFTokenID: NFTOKEN_ID,
       Flags: NFTokenCreateOfferFlags.tfSellNFToken,

packages/xrpl/test/models/accountDelete.test.ts

@@ -58,12 +58,12 @@ describe('AccountDelete', function () {
     assert.throws(
       () => validateAccountDelete(invalidDestination),
       ValidationError,
-      'AccountDelete: invalid Destination',
+      'AccountDelete: invalid field Destination',
     )
     assert.throws(
       () => validate(invalidDestination),
       ValidationError,
-      'AccountDelete: invalid Destination',
+      'AccountDelete: invalid field Destination',
     )
   })
 
@@ -81,13 +81,13 @@ describe('AccountDelete', function () {
     assert.throws(
       () => validateAccountDelete(invalidDestinationTag),
       ValidationError,
-      'AccountDelete: invalid DestinationTag',
+      'AccountDelete: invalid field DestinationTag',
     )
 
     assert.throws(
       () => validate(invalidDestinationTag),
       ValidationError,
-      'AccountDelete: invalid DestinationTag',
+      'AccountDelete: invalid field DestinationTag',
     )
   })
 })

packages/xrpl/test/models/accountSet.test.ts

@@ -155,12 +155,12 @@ describe('AccountSet', function () {
     assert.throws(
       () => validateAccountSet(account),
       ValidationError,
-      'AccountSet: invalid NFTokenMinter',
+      'AccountSet: invalid field NFTokenMinter',
     )
     assert.throws(
       () => validate(account),
       ValidationError,
-      'AccountSet: invalid NFTokenMinter',
+      'AccountSet: invalid field NFTokenMinter',
     )
   })
 })

packages/xrpl/test/models/checkCreate.test.ts

@@ -42,12 +42,12 @@ describe('CheckCreate', function () {
     assert.throws(
       () => validateCheckCreate(invalidDestination),
       ValidationError,
-      'CheckCreate: invalid Destination',
+      'CheckCreate: invalid field Destination',
     )
     assert.throws(
       () => validate(invalidDestination),
       ValidationError,
-      'CheckCreate: invalid Destination',
+      'CheckCreate: invalid field Destination',
     )
   })
 
@@ -92,12 +92,12 @@ describe('CheckCreate', function () {
     assert.throws(
       () => validateCheckCreate(invalidDestinationTag),
       ValidationError,
-      'CheckCreate: invalid DestinationTag',
+      'CheckCreate: invalid field DestinationTag',
     )
     assert.throws(
       () => validate(invalidDestinationTag),
       ValidationError,
-      'CheckCreate: invalid DestinationTag',
+      'CheckCreate: invalid field DestinationTag',
     )
   })
 

packages/xrpl/test/models/escrowCancel.test.ts

@@ -38,12 +38,12 @@ describe('EscrowCancel', function () {
     assert.throws(
       () => validateEscrowCancel(cancel),
       ValidationError,
-      'EscrowCancel: missing Owner',
+      'EscrowCancel: missing field Owner',
     )
     assert.throws(
       () => validate(cancel),
       ValidationError,
-      'EscrowCancel: missing Owner',
+      'EscrowCancel: missing field Owner',
     )
   })
 
@@ -68,12 +68,12 @@ describe('EscrowCancel', function () {
     assert.throws(
       () => validateEscrowCancel(cancel),
       ValidationError,
-      'EscrowCancel: Owner must be a string',
+      'EscrowCancel: invalid field Owner',
     )
     assert.throws(
       () => validate(cancel),
       ValidationError,
-      'EscrowCancel: Owner must be a string',
+      'EscrowCancel: invalid field Owner',
     )
   })
 

packages/xrpl/test/models/escrowCreate.test.ts

@@ -67,12 +67,12 @@ describe('EscrowCreate', function () {
     assert.throws(
       () => validateEscrowCreate(escrow),
       ValidationError,
-      'EscrowCreate: Destination must be a string',
+      'EscrowCreate: invalid field Destination',
     )
     assert.throws(
       () => validate(escrow),
       ValidationError,
-      'EscrowCreate: Destination must be a string',
+      'EscrowCreate: invalid field Destination',
     )
   })
 
@@ -137,12 +137,12 @@ describe('EscrowCreate', function () {
     assert.throws(
       () => validateEscrowCreate(escrow),
       ValidationError,
-      'EscrowCreate: DestinationTag must be a number',
+      'EscrowCreate: invalid field DestinationTag',
     )
     assert.throws(
       () => validate(escrow),
       ValidationError,
-      'EscrowCreate: DestinationTag must be a number',
+      'EscrowCreate: invalid field DestinationTag',
     )
   })
 

packages/xrpl/test/models/escrowFinish.test.ts

@@ -48,12 +48,12 @@ describe('EscrowFinish', function () {
     assert.throws(
       () => validateEscrowFinish(escrow),
       ValidationError,
-      'EscrowFinish: Owner must be a string',
+      'EscrowFinish: invalid field Owner',
     )
     assert.throws(
       () => validate(escrow),
       ValidationError,
-      'EscrowFinish: Owner must be a string',
+      'EscrowFinish: invalid field Owner',
     )
   })
 

packages/xrpl/test/models/payment.test.ts

@@ -102,12 +102,12 @@ describe('Payment', function () {
     assert.throws(
       () => validatePayment(paymentTransaction),
       ValidationError,
-      'PaymentTransaction: missing field Destination',
+      'Payment: missing field Destination',
     )
     assert.throws(
       () => validate(paymentTransaction),
       ValidationError,
-      'PaymentTransaction: missing field Destination',
+      'Payment: missing field Destination',
     )
   })
 
@@ -116,12 +116,47 @@ describe('Payment', function () {
     assert.throws(
       () => validatePayment(paymentTransaction),
       ValidationError,
-      'PaymentTransaction: invalid Destination',
+      'Payment: invalid field Destination',
     )
     assert.throws(
       () => validate(paymentTransaction),
       ValidationError,
-      'PaymentTransaction: invalid Destination',
+      'Payment: invalid field Destination',
+    )
+  })
+
+  it(`throws when Destination is invalid classic address`, function () {
+    paymentTransaction.Destination = 'rABCD'
+    assert.throws(
+      () => validatePayment(paymentTransaction),
+      ValidationError,
+      'Payment: invalid field Destination',
+    )
+    assert.throws(
+      () => validate(paymentTransaction),
+      ValidationError,
+      'Payment: invalid field Destination',
+    )
+  })
+
+  it(`does not throw when Destination is a valid x-address`, function () {
+    paymentTransaction.Destination =
+      'X7WZKEeNVS2p9Tire9DtNFkzWBZbFtSiS2eDBib7svZXuc2'
+    assert.doesNotThrow(() => validatePayment(paymentTransaction))
+    assert.doesNotThrow(() => validate(paymentTransaction))
+  })
+
+  it(`throws when Destination is an empty string`, function () {
+    paymentTransaction.Destination = ''
+    assert.throws(
+      () => validatePayment(paymentTransaction),
+      ValidationError,
+      'Payment: invalid field Destination',
+    )
+    assert.throws(
+      () => validate(paymentTransaction),
+      ValidationError,
+      'Payment: invalid field Destination',
     )
   })
 
@@ -130,12 +165,12 @@ describe('Payment', function () {
     assert.throws(
       () => validatePayment(paymentTransaction),
       ValidationError,
-      'PaymentTransaction: DestinationTag must be a number',
+      'Payment: invalid field DestinationTag',
     )
     assert.throws(
       () => validate(paymentTransaction),
       ValidationError,
-      'PaymentTransaction: DestinationTag must be a number',
+      'Payment: invalid field DestinationTag',
     )
   })
 

packages/xrpl/test/models/paymentChannelCreate.test.ts

@@ -61,12 +61,12 @@ describe('PaymentChannelCreate', function () {
     assert.throws(
       () => validatePaymentChannelCreate(channel),
       ValidationError,
-      'PaymentChannelCreate: missing Destination',
+      'PaymentChannelCreate: missing field Destination',
     )
     assert.throws(
       () => validate(channel),
       ValidationError,
-      'PaymentChannelCreate: missing Destination',
+      'PaymentChannelCreate: missing field Destination',
     )
   })
 
@@ -121,12 +121,12 @@ describe('PaymentChannelCreate', function () {
     assert.throws(
       () => validatePaymentChannelCreate(channel),
       ValidationError,
-      'PaymentChannelCreate: Destination must be a string',
+      'PaymentChannelCreate: invalid field Destination',
     )
     assert.throws(
       () => validate(channel),
       ValidationError,
-      'PaymentChannelCreate: Destination must be a string',
+      'PaymentChannelCreate: invalid field Destination',
     )
   })
 
@@ -166,12 +166,12 @@ describe('PaymentChannelCreate', function () {
     assert.throws(
       () => validatePaymentChannelCreate(channel),
       ValidationError,
-      'PaymentChannelCreate: DestinationTag must be a number',
+      'PaymentChannelCreate: invalid field DestinationTag',
     )
     assert.throws(
       () => validate(channel),
       ValidationError,
-      'PaymentChannelCreate: DestinationTag must be a number',
+      'PaymentChannelCreate: invalid field DestinationTag',
     )
   })