Issue 2212. Improve NFToken.URI handling for empty and undefined values (#2218)

* Handle undefined and null values in transactions better.
This commit is contained in:
Alexey Novikov
2023-03-10 10:34:28 +13:00
committed by GitHub
parent b3d3bc2f99
commit 97ff2aa104
6 changed files with 211 additions and 20 deletions

View File

@@ -3,6 +3,9 @@
Subscribe to [the **xrpl-announce** mailing list](https://groups.google.com/g/xrpl-announce) for release announcements. We recommend that xrpl.js (ripple-lib) users stay up-to-date with the latest stable release.
## Unreleased
### Added
* Null and undefined values in transactions are now treated as though the field was not passed in.
## 2.7.0 (2023-03-08)
### Fixed

View File

@@ -3,6 +3,7 @@ import BigNumber from 'bignumber.js'
import { fromSeed } from 'bip32'
import { mnemonicToSeedSync, validateMnemonic } from 'bip39'
import isEqual from 'lodash/isEqual'
import omitBy from 'lodash/omitBy'
import {
classicAddressToXAddress,
isValidXAddress,
@@ -26,7 +27,7 @@ import {
import ECDSA from '../ECDSA'
import { ValidationError, XrplError } from '../errors'
import { IssuedCurrencyAmount } from '../models/common'
import { Transaction } from '../models/transactions'
import { Transaction, validate } from '../models/transactions'
import { isIssuedCurrency } from '../models/transactions/common'
import { isHex } from '../models/utils'
import { ensureClassicAddress } from '../sugar/utils'
@@ -323,7 +324,12 @@ class Wallet {
multisignAddress = this.classicAddress
}
const tx = { ...transaction }
// clean null & undefined valued tx properties
// eslint-disable-next-line @typescript-eslint/consistent-type-assertions -- ensure Transaction flows through
const tx = omitBy(
{ ...transaction },
(value) => value == null,
) as unknown as Transaction
if (tx.TxnSignature || tx.Signers) {
throw new ValidationError(
@@ -333,6 +339,12 @@ class Wallet {
removeTrailingZeros(tx)
/*
* This will throw a more clear error for JS users if the supplied transaction has incorrect formatting
*/
// eslint-disable-next-line @typescript-eslint/consistent-type-assertions -- validate does not accept Transaction type
validate(tx as unknown as Record<string, unknown>)
const txToSignAndEncode = { ...tx }
txToSignAndEncode.SigningPubKey = multisignAddress ? '' : this.publicKey
@@ -460,9 +472,6 @@ class Wallet {
})
if (txCopy.TransactionType === 'NFTokenMint' && txCopy.URI) {
if (!isHex(txCopy.URI)) {
throw new ValidationError('URI must be a hex value')
}
txCopy.URI = txCopy.URI.toUpperCase()
}

View File

@@ -86,8 +86,11 @@ export interface NFTokenMint extends BaseTransaction {
*
* This field must be hex-encoded. You can use `convertStringToHex` to
* convert this field to the proper encoding.
*
* This field must not be an empty string. Omit it from the transaction or
* set to `undefined` value if you do not use it.
*/
URI?: string
URI?: string | null
Flags?: number | NFTokenMintFlagsInterface
}
@@ -106,6 +109,10 @@ export function validateNFTokenMint(tx: Record<string, unknown>): void {
)
}
if (typeof tx.URI === 'string' && tx.URI === '') {
throw new ValidationError('NFTokenMint: URI must not be empty string')
}
if (typeof tx.URI === 'string' && !isHex(tx.URI)) {
throw new ValidationError('NFTokenMint: URI must be in hex format')
}

View File

@@ -1,10 +1,6 @@
/* eslint-disable complexity -- verifies 19 tx types hence a lot of checks needed */
/* eslint-disable max-lines-per-function -- need to work with a lot of Tx verifications */
import isEqual from 'lodash/isEqual'
import omitBy from 'lodash/omitBy'
import { encode, decode } from 'ripple-binary-codec'
import { ValidationError } from '../../errors'
import { setTransactionFlagsToNumber } from '../utils/flags'
@@ -209,13 +205,4 @@ export function validate(transaction: Record<string, unknown>): void {
`Invalid field TransactionType: ${tx.TransactionType}`,
)
}
if (
!isEqual(
decode(encode(tx)),
omitBy(tx, (value) => value == null),
)
) {
throw new ValidationError(`Invalid Transaction: ${tx.TransactionType}`)
}
}

View File

@@ -68,6 +68,26 @@ describe('NFTokenMint', function () {
)
})
it(`throws w/ URI being an empty string`, function () {
const invalid = {
TransactionType: 'NFTokenMint',
Account: 'rWYkbWkCeg8dP6rXALnjgZSjjLyih5NXm',
Fee: '5000000',
Sequence: 2470665,
Flags: NFTokenMintFlags.tfTransferable,
NFTokenTaxon: 0,
Issuer: 'r9LqNeG6qHxjeUocjvVki2XR35weJ9mZgQ',
TransferFee: 1,
URI: '',
} as any
assert.throws(
() => validate(invalid),
ValidationError,
'NFTokenMint: URI must not be empty string',
)
})
it(`throws w/ URI not in hex format`, function () {
const invalid = {
TransactionType: 'NFTokenMint',

View File

@@ -864,6 +864,171 @@ describe('Wallet', function () {
assert.deepEqual(result, expectedResult)
})
it('sign throws when NFTokenMint.URI is empty string', async function () {
const tx: NFTokenMint = {
TransactionType: 'NFTokenMint',
Account: wallet.address,
TransferFee: 314,
NFTokenTaxon: 0,
Flags: 8,
Fee: '10',
URI: '',
Memos: [
{
Memo: {
MemoType:
'687474703a2f2f6578616d706c652e636f6d2f6d656d6f2f67656e65726963',
MemoData: '72656e74',
},
},
],
}
assert.throws(() => {
wallet.sign(tx)
}, /URI must not be empty string/u)
})
it('sign removes undefined NFTokenMint.URI property from transaction blob', async function () {
const tx: NFTokenMint = {
TransactionType: 'NFTokenMint',
Account: wallet.address,
TransferFee: 314,
NFTokenTaxon: 0,
Flags: 8,
Fee: '10',
URI: undefined,
Memos: [
{
Memo: {
MemoType:
'687474703a2f2f6578616d706c652e636f6d2f6d656d6f2f67656e65726963',
MemoData: '72656e74',
},
},
],
}
const result = wallet.sign(tx)
const decodedTx = decode(result.tx_blob) as unknown as NFTokenMint
assert.notExists(decodedTx.URI)
})
it('sign removes nulled NFTokenMint.URI property from transaction blob', async function () {
const tx: NFTokenMint = {
TransactionType: 'NFTokenMint',
Account: wallet.address,
TransferFee: 314,
NFTokenTaxon: 0,
Flags: 8,
Fee: '10',
URI: null,
Memos: [
{
Memo: {
MemoType:
'687474703a2f2f6578616d706c652e636f6d2f6d656d6f2f67656e65726963',
MemoData: '72656e74',
},
},
],
}
const result = wallet.sign(tx)
const decodedTx = decode(result.tx_blob) as unknown as NFTokenMint
assert.notExists(decodedTx.URI)
})
it('sign allows undefined value for NFTokenMint.URI', async function () {
// transaction with explicit URI: undefined
const tx_1: NFTokenMint = {
TransactionType: 'NFTokenMint',
Account: wallet.address,
TransferFee: 314,
NFTokenTaxon: 0,
Flags: 8,
Fee: '10',
URI: undefined,
Memos: [
{
Memo: {
MemoType:
'687474703a2f2f6578616d706c652e636f6d2f6d656d6f2f67656e65726963',
MemoData: '72656e74',
},
},
],
}
const result_1 = wallet.sign(tx_1)
// transaction with no URI
const tx_2: NFTokenMint = {
TransactionType: 'NFTokenMint',
Account: wallet.address,
TransferFee: 314,
NFTokenTaxon: 0,
Flags: 8,
Fee: '10',
Memos: [
{
Memo: {
MemoType:
'687474703a2f2f6578616d706c652e636f6d2f6d656d6f2f67656e65726963',
MemoData: '72656e74',
},
},
],
}
const result_2 = wallet.sign(tx_2)
assert.deepEqual(result_1, result_2)
})
it('sign allows nulled value for NFTokenMint.URI', async function () {
// transaction with explicit URI: null
const tx_1: NFTokenMint = {
TransactionType: 'NFTokenMint',
Account: wallet.address,
TransferFee: 314,
NFTokenTaxon: 0,
Flags: 8,
Fee: '10',
URI: null,
Memos: [
{
Memo: {
MemoType:
'687474703a2f2f6578616d706c652e636f6d2f6d656d6f2f67656e65726963',
MemoData: '72656e74',
},
},
],
}
const result_1 = wallet.sign(tx_1)
// transaction with no URI
const tx_2: NFTokenMint = {
TransactionType: 'NFTokenMint',
Account: wallet.address,
TransferFee: 314,
NFTokenTaxon: 0,
Flags: 8,
Fee: '10',
Memos: [
{
Memo: {
MemoType:
'687474703a2f2f6578616d706c652e636f6d2f6d656d6f2f67656e65726963',
MemoData: '72656e74',
},
},
],
}
const result_2 = wallet.sign(tx_2)
assert.deepEqual(result_1, result_2)
})
it('sign allows lowercase hex value for NFTokenMint.URI', async function () {
const tx: NFTokenMint = {
TransactionType: 'NFTokenMint',
@@ -915,7 +1080,7 @@ describe('Wallet', function () {
assert.throws(() => {
wallet.sign(tx)
}, /URI must be a hex value/u)
}, /URI must be in hex format/u)
})
})