Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Removed calls to deprecated utils function in tests #2845

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Removed calls to deprecated utils function in tests #2845

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f5a06d7
Removed deprecated calls to convertHexToString in favor of `@xrplf/is…
etienneoh Dec 9, 2024
b363830
Removed deprecated calls toconvertHexToString in favor of `@xrplf/iso…
etienneoh Dec 9, 2024
b7adadf
Revert "Removed deprecated calls to convertHexToString in favor of `@…
etienneoh Dec 12, 2024
0c84d5b
Merge branch 'XRPLF:main' into main
etienneoh Dec 12, 2024
0b73dc2
Merge branch 'main' of https://github.com/etienneoh/xrpl.js
etienneoh Dec 12, 2024
6de1de4
Update hexConversion.test.ts
etienneoh Dec 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions packages/xrpl/snippets/src/multisigning.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import { stringToHex } from '@xrplf/isomorphic/utils'

import {
multisign,
Client,
AccountSet,
convertStringToHex,
SignerListSet,
Wallet,
} from '../../src'
Expand Down Expand Up @@ -50,7 +51,7 @@ async function multisigning(): Promise<void> {
const accountSet: AccountSet = {
TransactionType: 'AccountSet',
Account: walletMaster.classicAddress,
Domain: convertStringToHex('example.com'),
Domain: stringToHex('example.com'),
}
const accountSetTx = await client.autofill(accountSet, 2)
console.log('AccountSet transaction is ready to be multisigned:')
Expand Down
2 changes: 1 addition & 1 deletion packages/xrpl/src/models/transactions/NFTokenMint.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ export interface NFTokenMint extends BaseTransaction {
* opaque issuer-specific encoding. The URI is NOT checked for validity, but
* the field is limited to a maximum length of 256 bytes.
*
* This field must be hex-encoded. You can use `convertStringToHex` to
* This field must be hex-encoded. You can use `@xrplf/isomorphic/utils`'s `stringToHex` to
* convert this field to the proper encoding.
*
* This field must not be an empty string. Omit it from the transaction or
Expand Down
4 changes: 2 additions & 2 deletions packages/xrpl/test/integration/integration.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { stringToHex } from '@xrplf/isomorphic/utils'
import { assert } from 'chai'

import { Client, SubmitResponse } from '../../src'
import { AccountSet, SignerListSet } from '../../src/models/transactions'
import { convertStringToHex } from '../../src/utils'
import { multisign } from '../../src/Wallet/signer'

import serverUrl from './serverUrl'
Expand Down Expand Up @@ -74,7 +74,7 @@ describe('integration tests', function () {
const accountSet: AccountSet = {
TransactionType: 'AccountSet',
Account: testContext.wallet.classicAddress,
Domain: convertStringToHex('example.com'),
Domain: stringToHex('example.com'),
}
const accountSetTx = await client.autofill(accountSet, 2)
const { tx_blob: tx_blob1 } = signerWallet1.sign(accountSetTx, true)
Expand Down
10 changes: 5 additions & 5 deletions packages/xrpl/test/integration/regularKey.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { stringToHex } from '@xrplf/isomorphic/utils'
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Migration is incomplete - convertStringToHex is still present in the codebase

The deprecated convertStringToHex function is still present in:

  • packages/xrpl/src/utils/stringConversion.ts (function definition)
  • packages/xrpl/src/utils/index.ts (import and re-export)

While the function internally uses stringToHex, its presence might confuse developers. Consider:

  1. Removing the deprecated function
  2. Updating any remaining imports to use stringToHex directly
🔗 Analysis chain

Verify complete migration from deprecated function

Let's ensure there are no remaining instances of the deprecated convertStringToHex function in the codebase.

Let me gather more information to understand if the old function is still being used or just defined.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Search for any remaining usage of the deprecated function
# Expected: No results, indicating complete migration

# Search for any remaining instances of convertStringToHex
rg "convertStringToHex" --type-add 'ts:*.{ts,tsx}' --type ts

# Search for new stringToHex usage to confirm migration
rg "stringToHex.*example\.com" --type-add 'ts:*.{ts,tsx}' --type ts

Length of output: 1937

Script:

#!/bin/bash
# Search for actual usage of convertStringToHex (not just definitions)
rg "convertStringToHex\(" --type-add 'ts:*.{ts,tsx}' --type ts -C 2

# Check if there are any imports of convertStringToHex in the codebase
rg "import.*convertStringToHex" --type-add 'ts:*.{ts,tsx}' --type ts

Length of output: 581

import { assert } from 'chai'

import {
Expand All @@ -10,7 +11,6 @@ import {
OfferCreate,
ECDSA,
} from '../../src'
import { convertStringToHex } from '../../src/utils'
import { multisign } from '../../src/Wallet/signer'

import serverUrl from './serverUrl'
Expand Down Expand Up @@ -85,7 +85,7 @@ describe('regular key', function () {
const accountSet: AccountSet = {
TransactionType: 'AccountSet',
Account: regularKeyWallet.classicAddress,
Domain: convertStringToHex('example.com'),
Domain: stringToHex('example.com'),
}

await testTransaction(testContext.client, accountSet, regularKeyWallet)
Expand All @@ -103,7 +103,7 @@ describe('regular key', function () {
const accountSet: AccountSet = {
TransactionType: 'AccountSet',
Account: masterWallet.classicAddress,
Domain: convertStringToHex('example.com'),
Domain: stringToHex('example.com'),
}

await testTransaction(testContext.client, accountSet, masterWallet)
Expand Down Expand Up @@ -259,7 +259,7 @@ describe('regular key', function () {
const accountSet: AccountSet = {
TransactionType: 'AccountSet',
Account: testContext.wallet.classicAddress,
Domain: convertStringToHex('example.com'),
Domain: stringToHex('example.com'),
}
const accountSetTx = await client.autofill(accountSet, 2)
const signed1 = regularKeyWallet.sign(accountSetTx, true)
Expand Down Expand Up @@ -319,7 +319,7 @@ describe('regular key', function () {
const accountSet: AccountSet = {
TransactionType: 'AccountSet',
Account: testContext.wallet.classicAddress,
Domain: convertStringToHex('example.com'),
Domain: stringToHex('example.com'),
}
const accountSetTx = await client.autofill(accountSet, 2)
const signed1 = sameKeyDefaultAddressWallet.sign(accountSetTx, true)
Expand Down
4 changes: 2 additions & 2 deletions packages/xrpl/test/integration/requests/submit.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { stringToHex } from '@xrplf/isomorphic/utils'
import { assert } from 'chai'
import { decode } from 'ripple-binary-codec'

Expand All @@ -8,7 +9,6 @@ import {
hashes,
SubmittableTransaction,
} from '../../../src'
import { convertStringToHex } from '../../../src/utils'
import serverUrl from '../serverUrl'
import {
setupClient,
Expand All @@ -35,7 +35,7 @@ describe('submit', function () {
const accountSet: AccountSet = {
TransactionType: 'AccountSet',
Account: testContext.wallet.classicAddress,
Domain: convertStringToHex('example.com'),
Domain: stringToHex('example.com'),
}

const autofilledTx = await testContext.client.autofill(accountSet)
Expand Down
6 changes: 3 additions & 3 deletions packages/xrpl/test/integration/requests/submitMultisigned.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { stringToHex } from '@xrplf/isomorphic/utils'
import { assert } from 'chai'
import { decode } from 'ripple-binary-codec'

Expand All @@ -11,7 +12,6 @@ import {
SubmitMultisignedRequest,
SubmitMultisignedV1Response,
} from '../../../src'
import { convertStringToHex } from '../../../src/utils'
import { multisign } from '../../../src/Wallet/signer'
import serverUrl from '../serverUrl'
import {
Expand Down Expand Up @@ -75,7 +75,7 @@ describe('submit_multisigned', function () {
const accountSet: AccountSet = {
TransactionType: 'AccountSet',
Account: testContext.wallet.classicAddress,
Domain: convertStringToHex('example.com'),
Domain: stringToHex('example.com'),
}
const accountSetTx = await client.autofill(accountSet, 2)
const signed1 = signerWallet1.sign(accountSetTx, true)
Expand Down Expand Up @@ -148,7 +148,7 @@ describe('submit_multisigned', function () {
const accountSet: AccountSet = {
TransactionType: 'AccountSet',
Account: testContext.wallet.classicAddress,
Domain: convertStringToHex('example.com'),
Domain: stringToHex('example.com'),
}
const accountSetTx = await client.autofill(accountSet, 2)
const signed1 = signerWallet1.sign(accountSetTx, true)
Expand Down
6 changes: 3 additions & 3 deletions packages/xrpl/test/integration/requests/tx.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { stringToHex } from '@xrplf/isomorphic/utils'
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Remove deprecated convertStringToHex function and its exports

The deprecated convertStringToHex function is still present in the codebase:

  • packages/xrpl/src/utils/stringConversion.ts: Function definition and export
  • packages/xrpl/src/utils/index.ts: Import and re-export

While the function internally uses the new stringToHex from @xrplf/isomorphic/utils, it should be removed to avoid confusion and maintain consistency. The PR's changes should include removing this deprecated function since direct usage of stringToHex from @xrplf/isomorphic/utils is now the standard.

🔗 Analysis chain

Verify complete removal of deprecated function

Let's ensure all instances of the deprecated convertStringToHex function have been removed from the codebase.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for any remaining usage of convertStringToHex
rg "convertStringToHex" --type ts

# Search for all string-to-hex conversion patterns to ensure consistency
rg "(?i)(convert|to).*hex.*string" --type ts

Length of output: 4665

import { assert } from 'chai'

import {
Expand All @@ -8,7 +9,6 @@ import {
TxResponse,
TxV1Response,
} from '../../../src'
import { convertStringToHex } from '../../../src/utils'
import serverUrl from '../serverUrl'
import {
setupClient,
Expand All @@ -35,7 +35,7 @@ describe('tx', function () {
const accountSet: AccountSet = {
TransactionType: 'AccountSet',
Account: account,
Domain: convertStringToHex('example.com'),
Domain: stringToHex('example.com'),
}

const response: SubmitResponse = await testContext.client.submit(
Expand Down Expand Up @@ -82,7 +82,7 @@ describe('tx', function () {
const accountSet: AccountSet = {
TransactionType: 'AccountSet',
Account: account,
Domain: convertStringToHex('example.com'),
Domain: stringToHex('example.com'),
}

const response: SubmitResponse = await testContext.client.submit(
Expand Down
11 changes: 6 additions & 5 deletions packages/xrpl/test/integration/submitAndWait.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { stringToHex } from '@xrplf/isomorphic/utils'
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Migration to stringToHex is incomplete - Found additional instances to update

The deprecated convertStringToHex function is still present in the codebase:

  • It's defined in packages/xrpl/src/utils/stringConversion.ts (though it's now just a wrapper around stringToHex)
  • It's exported in packages/xrpl/src/utils/index.ts

These instances should also be removed to complete the migration to stringToHex.

🔗 Analysis chain

LGTM! Consistent migration across all test cases

The changes correctly replace all instances of the deprecated convertStringToHex with stringToHex from the isomorphic utils package. The migration is consistent across all test scenarios:

  • Unsigned transaction test
  • Validation error test
  • Signed transaction test
  • Extended wait test

Let's verify that we haven't missed any instances of the deprecated function:

Also applies to: 39-39, 93-93, 111-111, 134-134

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for any remaining instances of convertStringToHex
rg "convertStringToHex" --type ts

Length of output: 389

Script:

#!/bin/bash
# Let's check if there are any remaining usages of convertStringToHex in the codebase
rg "convertStringToHex\(" --type ts -A 2

# Also check for any imports of convertStringToHex
rg "import.*convertStringToHex" --type ts

Length of output: 412

import { assert } from 'chai'

import { AccountSet, convertStringToHex, ValidationError } from '../../src'
import { AccountSet, ValidationError } from '../../src'
import { assertRejects } from '../testUtils'

import serverUrl from './serverUrl'
Expand Down Expand Up @@ -35,7 +36,7 @@ describe('client.submitAndWait', function () {
const accountSet: AccountSet = {
TransactionType: 'AccountSet',
Account: testContext.wallet.classicAddress,
Domain: convertStringToHex('example.com'),
Domain: stringToHex('example.com'),
}

let retries = 10
Expand Down Expand Up @@ -89,7 +90,7 @@ describe('client.submitAndWait', function () {
const accountSet: AccountSet = {
TransactionType: 'AccountSet',
Account: testContext.wallet.classicAddress,
Domain: convertStringToHex('example.com'),
Domain: stringToHex('example.com'),
}

await assertRejects(
Expand All @@ -107,7 +108,7 @@ describe('client.submitAndWait', function () {
const accountSet: AccountSet = {
TransactionType: 'AccountSet',
Account: testContext.wallet.classicAddress,
Domain: convertStringToHex('example.com'),
Domain: stringToHex('example.com'),
}
const { tx_blob: signedAccountSet } = testContext.wallet.sign(
await testContext.client.autofill(accountSet),
Expand All @@ -130,7 +131,7 @@ describe('client.submitAndWait', function () {
const accountSet: AccountSet = {
TransactionType: 'AccountSet',
Account: testContext.wallet.classicAddress,
Domain: convertStringToHex('example.com'),
Domain: stringToHex('example.com'),
}
const { tx_blob: signedAccountSet } = testContext.wallet.sign(
await testContext.client.autofill(accountSet),
Expand Down
4 changes: 2 additions & 2 deletions packages/xrpl/test/integration/transactions/nftokenMint.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { stringToHex } from '@xrplf/isomorphic/utils'
import { assert } from 'chai'

import {
convertStringToHex,
getNFTokenID,
NFTokenMint,
TransactionMetadata,
Expand Down Expand Up @@ -33,7 +33,7 @@ describe('NFTokenMint', function () {
const tx: NFTokenMint = {
TransactionType: 'NFTokenMint',
Account: testContext.wallet.address,
URI: convertStringToHex('https://www.google.com'),
URI: stringToHex('https://www.google.com'),
NFTokenTaxon: 0,
}
const response = await testTransaction(
Expand Down
8 changes: 4 additions & 4 deletions packages/xrpl/test/models/NFTokenMint.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { stringToHex } from '@xrplf/isomorphic/utils'
import { assert } from 'chai'

import {
convertStringToHex,
validate,
ValidationError,
NFTokenMintFlags,
Expand All @@ -25,7 +25,7 @@ describe('NFTokenMint', function () {
NFTokenTaxon: 0,
Issuer: 'r9LqNeG6qHxjeUocjvVki2XR35weJ9mZgQ',
TransferFee: 1,
URI: convertStringToHex('http://xrpl.org'),
URI: stringToHex('http://xrpl.org'),
} as any

assert.doesNotThrow(() => validate(validNFTokenMint))
Expand All @@ -40,7 +40,7 @@ describe('NFTokenMint', function () {
Flags: NFTokenMintFlags.tfTransferable,
Issuer: 'r9LqNeG6qHxjeUocjvVki2XR35weJ9mZgQ',
TransferFee: 1,
URI: convertStringToHex('http://xrpl.org'),
URI: stringToHex('http://xrpl.org'),
} as any

assert.throws(
Expand All @@ -60,7 +60,7 @@ describe('NFTokenMint', function () {
Issuer: 'rWYkbWkCeg8dP6rXALnjgZSjjLyih5NXm',
TransferFee: 1,
NFTokenTaxon: 0,
URI: convertStringToHex('http://xrpl.org'),
URI: stringToHex('http://xrpl.org'),
} as any

assert.throws(
Expand Down
13 changes: 6 additions & 7 deletions packages/xrpl/test/utils/hexConversion.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,16 @@
import { stringToHex, hexToString } from '@xrplf/isomorphic/utils'
import { assert } from 'chai'

import { convertHexToString, convertStringToHex } from '../../src/utils'

describe('convertHexToString and convertStringToHex', function () {
describe('hexToString and stringToHex', function () {
it('converts "example.com"', function () {
const str = 'example.com'
const hex = convertStringToHex(str)
const hex = stringToHex(str)
assert.strictEqual(
hex,
'6578616D706C652E636F6D',
'should convert to hex equivalent',
)
const result = convertHexToString(hex)
const result = hexToString(hex)
assert.strictEqual(
result,
'example.com',
Expand All @@ -21,9 +20,9 @@ describe('convertHexToString and convertStringToHex', function () {

it('converts "你好"', function () {
const str = '你好'
const hex = convertStringToHex(str)
const hex = stringToHex(str)
assert.strictEqual(hex, 'E4BDA0E5A5BD', 'should convert to hex equivalent')
const result = convertHexToString(hex)
const result = hexToString(hex)
assert.strictEqual(result, '你好', 'should convert back to 你好')
})
})
Loading