From 89240eae6267fdef39620b4a8444de23e11a9230 Mon Sep 17 00:00:00 2001 From: Jackson Mills Date: Fri, 17 Jun 2022 14:27:47 -0700 Subject: [PATCH 1/2] Handle edge cases for standard currency code signing (#2009) * Allow decoding symbols + lowercase standard codes * Standardize the treatment of standard codes and hex codes for verifying transaction equivalence --- packages/ripple-binary-codec/HISTORY.md | 1 + .../ripple-binary-codec/src/types/currency.ts | 2 +- .../ripple-binary-codec/test/hash.test.js | 18 +-- packages/xrpl/HISTORY.md | 3 + packages/xrpl/src/Wallet/index.ts | 55 ++++++++- packages/xrpl/test/wallet/index.ts | 111 +++++++++++++++++- 6 files changed, 180 insertions(+), 10 deletions(-) diff --git a/packages/ripple-binary-codec/HISTORY.md b/packages/ripple-binary-codec/HISTORY.md index b8155492..88e23dfc 100644 --- a/packages/ripple-binary-codec/HISTORY.md +++ b/packages/ripple-binary-codec/HISTORY.md @@ -1,6 +1,7 @@ # ripple-binary-codec Release History ## Unreleased +- Fixed standard currency codes with lowercase and allowed symbols not decoding into standard codes. ## 1.4.1 (2022-06-02) - Added a clearer error message for trying to encode an invalid transaction. (Ex. With an incorrect TransactionType) diff --git a/packages/ripple-binary-codec/src/types/currency.ts b/packages/ripple-binary-codec/src/types/currency.ts index 8172dd02..d9d46690 100644 --- a/packages/ripple-binary-codec/src/types/currency.ts +++ b/packages/ripple-binary-codec/src/types/currency.ts @@ -2,7 +2,7 @@ import { Hash160 } from './hash-160' import { Buffer } from 'buffer/' const XRP_HEX_REGEX = /^0{40}$/ -const ISO_REGEX = /^[A-Z0-9]{3}$/ +const ISO_REGEX = /^[A-Z0-9a-z?!@#$%^&*(){}[\]|]{3}$/ const HEX_REGEX = /^[A-F0-9]{40}$/ // eslint-disable-next-line no-control-regex const STANDARD_FORMAT_HEX_REGEX = /^0{24}[\x00-\x7F]{6}0{10}$/ diff --git a/packages/ripple-binary-codec/test/hash.test.js b/packages/ripple-binary-codec/test/hash.test.js index 177e918f..ea59df4d 100644 --- a/packages/ripple-binary-codec/test/hash.test.js +++ b/packages/ripple-binary-codec/test/hash.test.js @@ -54,15 +54,19 @@ describe('Currency', function () { const currencyCode = '0000000000000000000000005852500000000000' expect(Currency.from(currencyCode).toJSON()).toBe(currencyCode) }) - test('Currency with lowercase letters decode to hex', () => { - expect(Currency.from('xRp').toJSON()).toBe( - '0000000000000000000000007852700000000000', + test('Currency code with lowercase letters decodes to ISO code', () => { + expect(Currency.from('xRp').toJSON()).toBe('xRp') + }) + test('Currency codes with symbols decodes to ISO code', () => { + expect(Currency.from('x|p').toJSON()).toBe('x|p') + }) + test('Currency code with non-standard symbols decodes to hex', () => { + expect(Currency.from(':::').toJSON()).toBe( + '0000000000000000000000003A3A3A0000000000', ) }) - test('Currency codes with symbols decode to hex', () => { - expect(Currency.from('x|p').toJSON()).toBe( - '000000000000000000000000787C700000000000', - ) + test('Currency codes can be exclusively standard symbols', () => { + expect(Currency.from('![]').toJSON()).toBe('![]') }) test('Currency codes with uppercase and 0-9 decode to ISO codes', () => { expect(Currency.from('X8P').toJSON()).toBe('X8P') diff --git a/packages/xrpl/HISTORY.md b/packages/xrpl/HISTORY.md index 82a34db5..d0470339 100644 --- a/packages/xrpl/HISTORY.md +++ b/packages/xrpl/HISTORY.md @@ -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 +### Fixed +* Signing tx with standard currency codes with lowercase and allowed symbols causing an error on decode. + ### Added * When connected to nft-devnet, Client.fundWallet now defaults to using the nft-devnet faucet instead of requiring specification. diff --git a/packages/xrpl/src/Wallet/index.ts b/packages/xrpl/src/Wallet/index.ts index bf394e4f..c4f3251a 100644 --- a/packages/xrpl/src/Wallet/index.ts +++ b/packages/xrpl/src/Wallet/index.ts @@ -24,8 +24,10 @@ import { } from 'ripple-keypairs' import ECDSA from '../ECDSA' -import { ValidationError } from '../errors' +import { ValidationError, XrplError } from '../errors' +import { IssuedCurrencyAmount } from '../models/common' import { Transaction } from '../models/transactions' +import { isIssuedCurrency } from '../models/transactions/common' import { isHex } from '../models/utils' import { ensureClassicAddress } from '../sugar/utils' import { hashSignedTx } from '../utils/hashes/hashLedger' @@ -303,6 +305,7 @@ class Wallet { * @param multisign - Specify true/false to use multisign or actual address (classic/x-address) to make multisign tx request. * @returns A signed transaction. * @throws ValidationError if the transaction is already signed or does not encode/decode to same result. + * @throws XrplError if the issued currency being signed is XRP ignoring case. */ // eslint-disable-next-line max-lines-per-function -- introduced more checks to support both string and boolean inputs. public sign( @@ -351,6 +354,7 @@ class Wallet { this.privateKey, ) } + const serialized = encode(txToSignAndEncode) this.checkTxSerialization(serialized, tx) return { @@ -392,6 +396,7 @@ class Wallet { * @param tx - The transaction prior to signing. * @throws A ValidationError if the transaction does not have a TxnSignature/Signers property, or if * the serialized Transaction desn't match the original transaction. + * @throws XrplError if the transaction includes an issued currency which is equivalent to XRP ignoring case. */ // eslint-disable-next-line class-methods-use-this, max-lines-per-function -- Helper for organization purposes private checkTxSerialization(serialized: string, tx: Transaction): void { @@ -449,6 +454,38 @@ class Wallet { txCopy.URI = txCopy.URI.toUpperCase() } + /* eslint-disable @typescript-eslint/consistent-type-assertions -- We check at runtime that this is safe */ + Object.keys(txCopy).forEach((key) => { + const standard_currency_code_len = 3 + if (txCopy[key] && isIssuedCurrency(txCopy[key])) { + const decodedAmount = decoded[key] as unknown as IssuedCurrencyAmount + const decodedCurrency = decodedAmount.currency + const txCurrency = (txCopy[key] as IssuedCurrencyAmount).currency + + if ( + txCurrency.length === standard_currency_code_len && + txCurrency.toUpperCase() === 'XRP' + ) { + throw new XrplError( + `Trying to sign an issued currency with a similar standard code to XRP (received '${txCurrency}'). XRP is not an issued currency.`, + ) + } + + // Standardize the format of currency codes to the 40 byte hex string for comparison + const amount = txCopy[key] as IssuedCurrencyAmount + if (amount.currency.length !== decodedCurrency.length) { + /* eslint-disable-next-line max-depth -- Easier to read with two if-statements */ + if (decodedCurrency.length === standard_currency_code_len) { + decodedAmount.currency = isoToHex(decodedCurrency) + } else { + /* eslint-disable-next-line @typescript-eslint/no-unsafe-member-access -- We need to update txCopy directly */ + txCopy[key].currency = isoToHex(txCopy[key].currency) + } + } + } + }) + /* eslint-enable @typescript-eslint/consistent-type-assertions -- Done with dynamic checking */ + if (!_.isEqual(decoded, txCopy)) { const data = { decoded, @@ -509,4 +546,20 @@ function removeTrailingZeros(tx: Transaction): void { } } +/** + * Convert an ISO code to a hex string representation + * + * @param iso - A 3 letter standard currency code + */ +/* eslint-disable @typescript-eslint/no-magic-numbers -- Magic numbers are from rippleds of currency code encoding */ +function isoToHex(iso: string): string { + const bytes = Buffer.alloc(20) + if (iso !== 'XRP') { + const isoBytes = iso.split('').map((chr) => chr.charCodeAt(0)) + bytes.set(isoBytes, 12) + } + return bytes.toString('hex').toUpperCase() +} +/* eslint-enable @typescript-eslint/no-magic-numbers -- Only needed in this function */ + export default Wallet diff --git a/packages/xrpl/test/wallet/index.ts b/packages/xrpl/test/wallet/index.ts index 0b0ed31d..de0aa41e 100644 --- a/packages/xrpl/test/wallet/index.ts +++ b/packages/xrpl/test/wallet/index.ts @@ -1,6 +1,6 @@ import { assert } from 'chai' import { decode } from 'ripple-binary-codec/dist' -import { NFTokenMint, Transaction } from 'xrpl-local' +import { NFTokenMint, Payment, Transaction } from 'xrpl-local' import ECDSA from 'xrpl-local/ECDSA' import Wallet from 'xrpl-local/Wallet' @@ -301,6 +301,7 @@ describe('Wallet', function () { }) }) + // eslint-disable-next-line max-statements -- Required for test coverage. describe('sign', function () { let wallet: Wallet @@ -590,6 +591,114 @@ describe('Wallet', function () { }, /^1.1234567 is an illegal amount/u) }) + const issuedCurrencyPayment: Transaction = { + TransactionType: 'Payment', + Account: 'r9cZA1mLK5R5Am25ArfXFmqgNwjZgnfk59', + Destination: 'rQ3PTWGLCbPz8ZCicV5tCX3xuymojTng5r', + Amount: { + currency: 'foo', + issuer: 'rnURbz5HLbvqEq69b1B4TX6cUTNMmcrBqi', + value: '123.40', + }, + Flags: 2147483648, + Sequence: 23, + LastLedgerSequence: 8819954, + Fee: '12', + } + + it('lowercase standard currency code signs successfully', async function () { + const payment: Payment = { ...issuedCurrencyPayment } + payment.Amount = { + currency: 'foo', + issuer: 'rnURbz5HLbvqEq69b1B4TX6cUTNMmcrBqi', + value: '123.40', + } + + assert.deepEqual(wallet.sign(payment), { + tx_blob: + '12000022800000002400000017201B008694F261D504625103A72000000000000000000000000000666F6F00000000002E099DD75FDD96EB4A603037844F964832FED86B68400000000000000C732102A8A44DB3D4C73EEEE11DFE54D2029103B776AA8A8D293A91D645977C9DF5F54474473045022100D32EBD44F86FB6D0BE239A410B62A73A8B0C26CE3767321913D6FB7BE6FAC2410220430C011C25091DA9CD75E7C99BE406572FBB57B92132E39B4BF873863E744E2E81145E7B112523F68D2F5E879DB4EAC51C6698A693048314FDB08D07AAA0EB711793A3027304D688E10C3648', + hash: 'F822EA1D7B2A3026E4654A9152896652C3843B5690F8A56C4217CB4690C5C95A', + }) + }) + + it('issued currency in standard or hex format signs to the same transaction', async function () { + const payment: Payment = { ...issuedCurrencyPayment } + payment.Amount = { + currency: '***', + issuer: 'rnURbz5HLbvqEq69b1B4TX6cUTNMmcrBqi', + value: '123.40', + } + + const payment2: Payment = { ...issuedCurrencyPayment } + payment2.Amount = { + currency: '0000000000000000000000002A2A2A0000000000', + issuer: 'rnURbz5HLbvqEq69b1B4TX6cUTNMmcrBqi', + value: '123.40', + } + + assert.deepEqual(wallet.sign(payment), wallet.sign(payment2)) + }) + + it('sign throws when a payment contains an issued currency like XRP', async function () { + const payment: Payment = { ...issuedCurrencyPayment } + payment.Amount = { + currency: 'xrp', + issuer: 'rnURbz5HLbvqEq69b1B4TX6cUTNMmcrBqi', + value: '123.40', + } + assert.throws(() => { + wallet.sign(payment) + }, /^Trying to sign an issued currency with a similar standard code to XRP \(received 'xrp'\)\. XRP is not an issued currency\./u) + }) + + it('sign does NOT throw when a payment contains an issued currency like xrp in hex string format', async function () { + const payment: Payment = { ...issuedCurrencyPayment } + payment.Amount = { + currency: '0000000000000000000000007872700000000000', + issuer: 'rnURbz5HLbvqEq69b1B4TX6cUTNMmcrBqi', + value: '123.40', + } + assert.deepEqual(wallet.sign(payment), { + tx_blob: + '12000022800000002400000017201B008694F261D504625103A7200000000000000000000000000078727000000000002E099DD75FDD96EB4A603037844F964832FED86B68400000000000000C732102A8A44DB3D4C73EEEE11DFE54D2029103B776AA8A8D293A91D645977C9DF5F5447446304402202CD2BE27480860765B1B8DB6C499D299734C533F4FFA66317E46D1ADE5181EB7022066D2C65B975A6A9FEE56AB55211D5F2F65D6F988C8280019211874D11771A05D81145E7B112523F68D2F5E879DB4EAC51C6698A693048314FDB08D07AAA0EB711793A3027304D688E10C3648', + hash: '1FEAA7894E507E36D73F60DED89852CE28994366879BC7D3D806E4C50D10B1EE', + }) + }) + + it('sign succeeds with standard currency code with symbols', async function () { + const payment: Payment = { ...issuedCurrencyPayment } + payment.Amount = { + currency: '***', + issuer: 'rnURbz5HLbvqEq69b1B4TX6cUTNMmcrBqi', + value: '123.40', + } + const result = wallet.sign(payment) + const expectedResult = { + tx_blob: + '12000022800000002400000017201B008694F261D504625103A720000000000000000000000000002A2A2A00000000002E099DD75FDD96EB4A603037844F964832FED86B68400000000000000C732102A8A44DB3D4C73EEEE11DFE54D2029103B776AA8A8D293A91D645977C9DF5F54474463044022073E71588750C3D47D7D9A541F00FB897823DA67ED198D0A74404B6FE6D5E4AB5022021BE798D4159F375EBE13D0545F50EE864DF834D5A9F9A31504212156A57934C81145E7B112523F68D2F5E879DB4EAC51C6698A693048314FDB08D07AAA0EB711793A3027304D688E10C3648', + hash: '95BF9931C1EA164960FE13A504D5FBAEB1E072C1D291D75B85BA3F22A50346DF', + } + + assert.deepEqual(result, expectedResult) + }) + + it('sign succeeds with non-standard 3 digit currency code', async function () { + const payment: Payment = { ...issuedCurrencyPayment } + payment.Amount = { + currency: ':::', + issuer: 'rnURbz5HLbvqEq69b1B4TX6cUTNMmcrBqi', + value: '123.40', + } + const result = wallet.sign(payment) + const expectedResult = { + tx_blob: + '12000022800000002400000017201B008694F261D504625103A720000000000000000000000000003A3A3A00000000002E099DD75FDD96EB4A603037844F964832FED86B68400000000000000C732102A8A44DB3D4C73EEEE11DFE54D2029103B776AA8A8D293A91D645977C9DF5F5447446304402205952993DB235D3A6398E2CB5F91D7F0AD9067F02CB8E62FD335C516B64130F4702206777746CC516F95F39ADDD62CD395AF2F6BAFCCA355B5D23B9B4D9358474A11281145E7B112523F68D2F5E879DB4EAC51C6698A693048314FDB08D07AAA0EB711793A3027304D688E10C3648', + hash: 'CE80072E6D70932BC7AA698B931BCF97B6CC3DD3984E08DF284B74E8CB4E543A', + } + + assert.deepEqual(result, expectedResult) + }) + it('sign handles non-XRP amount with a trailing zero', async function () { const payment: Transaction = { TransactionType: 'Payment', From 9ff9fc77677b4a6cc4cefe7cd2ab38a57a95d91b Mon Sep 17 00:00:00 2001 From: Elliot Lee Date: Mon, 20 Jun 2022 08:18:31 -0500 Subject: [PATCH 2/2] fix: link to XRPL Rosetta (ThreeXRP.dev) (#2027) --- APPLICATIONS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/APPLICATIONS.md b/APPLICATIONS.md index d0bb378b..2a00be86 100644 --- a/APPLICATIONS.md +++ b/APPLICATIONS.md @@ -68,7 +68,7 @@ Warning: Use at your own risk. Attempts to detect RippleNet on-demand liquidity (ODL) transactions through known fiat corridors and report these transactions in real time. -- **[XRPL Rosetta](https://xrpl-rosetta-oepox.ondigitalocean.app)** +- **[XRPL Rosetta](https://threexrp.dev/)** 3D Globe written in three.js connected to a Node.js websocket server that is listening to exchanges and the XRPL. The visualization aims to show trading, ODL, and liquidity at exchanges, intra-exchange volume, and flows.