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.

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

Aliter: Implement DeliverMax alias in Payment transactions, through autofill method #2689

Merged
merged 28 commits into from
Jun 27, 2024
Merged
Show file tree
Hide file tree
Changes from 27 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
829de40
autofill function: populates and validates Payment transactions in ri…
ckeshava May 2, 2024
1312556
unit tests for Payment txn
ckeshava May 2, 2024
8347cb0
implementation of DeliverMax alias in Payment transactions
ckeshava May 3, 2024
7213301
use const, instead of 'let' in variable declarations
ckeshava May 3, 2024
9ed4834
ignnore linter errors on `any` type-erasure
ckeshava May 9, 2024
778d3a2
[FIX] rectify linter warnings
ckeshava May 9, 2024
b16a322
Merge branch 'main' into autofill-temp
ckeshava May 9, 2024
9632c55
Merge branch 'main' into autofill-temp
ckeshava Jun 4, 2024
0895d05
address PR comments
ckeshava Jun 6, 2024
5f218b2
simplify if-conditions
ckeshava Jun 7, 2024
4fabd79
Merge remote-tracking branch 'upstream/main' into autofill-temp
ckeshava Jun 7, 2024
e27d23e
- remove extraneous comment
ckeshava Jun 7, 2024
60f3e6f
disable linter errors for longer functions
ckeshava Jun 10, 2024
71bc89c
Update packages/xrpl/src/client/index.ts
ckeshava Jun 10, 2024
aedab81
integration tests pertaining to DeliverMax
ckeshava Jun 10, 2024
b0b5c4b
fix browser test failures: avoid throwing ValidationError inside paym…
ckeshava Jun 11, 2024
c8c2072
remove commented code
ckeshava Jun 11, 2024
9708845
Merge branch 'main' into autofill-temp
ckeshava Jun 11, 2024
b854a2d
[DRAFT] negative integration test -- fails jasmine browser test
ckeshava Jun 12, 2024
dfb2f9a
remove the integration test with differing DeliverMax and Amount fiel…
ckeshava Jun 13, 2024
c49957c
address PR comments: remove underscores in variable names
ckeshava Jun 14, 2024
28d1ef4
address omar PR comments -- simplify the structure of the tests
ckeshava Jun 14, 2024
0d3d409
Update packages/xrpl/test/integration/transactions/payment.test.ts
ckeshava Jun 14, 2024
d661fe6
Update packages/xrpl/test/integration/transactions/payment.test.ts
ckeshava Jun 14, 2024
4b69021
Update packages/xrpl/test/integration/transactions/payment.test.ts
ckeshava Jun 14, 2024
270567d
Update packages/xrpl/src/client/index.ts
ckeshava Jun 14, 2024
bc803a8
address PR comments -- use beforeAll() construct
ckeshava Jun 14, 2024
07804f2
Merge branch 'main' into autofill-temp
ckeshava Jun 27, 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
38 changes: 35 additions & 3 deletions packages/xrpl/src/client/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ class Client extends EventEmitter<EventTypes> {
* const client = new Client('wss://s.altnet.rippletest.net:51233')
* ```
*/
// eslint-disable-next-line max-lines-per-function -- okay because we have to set up all the connection handlers
/* eslint-disable max-lines-per-function -- the constructor requires more lines to implement the logic */
public constructor(server: string, options: ClientOptions = {}) {
super()
if (typeof server !== 'string' || !/wss?(?:\+unix)?:\/\//u.exec(server)) {
Expand Down Expand Up @@ -290,6 +290,7 @@ class Client extends EventEmitter<EventTypes> {
this.emit('path_find', path)
})
}
/* eslint-enable max-lines-per-function */

/**
* Get the url that the client is connected to.
Expand Down Expand Up @@ -638,15 +639,17 @@ class Client extends EventEmitter<EventTypes> {
* @param signersCount - The expected number of signers for this transaction.
* Only used for multisigned transactions.
* @returns The autofilled transaction.
* @throws ValidationError If Amount and DeliverMax fields are not identical in a Payment Transaction
*/

// eslint-disable-next-line complexity -- handling Payment transaction API v2 requires more logic
public async autofill<T extends SubmittableTransaction>(
transaction: T,
signersCount?: number,
): Promise<T> {
const tx = { ...transaction }

setValidAddresses(tx)

setTransactionFlagsToNumber(tx)

const promises: Array<Promise<void>> = []
Expand All @@ -666,6 +669,34 @@ class Client extends EventEmitter<EventTypes> {
promises.push(checkAccountDeleteBlockers(this, tx))
}

// eslint-disable-next-line @typescript-eslint/ban-ts-comment -- ignore type-assertions on the DeliverMax property
// @ts-expect-error -- DeliverMax property exists only at the RPC level, not at the protocol level
if (tx.TransactionType === 'Payment' && tx.DeliverMax != null) {
// eslint-disable-next-line @typescript-eslint/no-unnecessary-condition -- This is a valid null check for Amount
if (tx.Amount == null) {
// If only DeliverMax is provided, use it to populate the Amount field
// eslint-disable-next-line @typescript-eslint/ban-ts-comment -- ignore type-assertions on the DeliverMax property
// @ts-expect-error -- DeliverMax property exists only at the RPC level, not at the protocol level
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment -- DeliverMax is a known RPC-level property
tx.Amount = tx.DeliverMax
ckeshava marked this conversation as resolved.
Show resolved Hide resolved
}

// eslint-disable-next-line @typescript-eslint/ban-ts-comment -- ignore type-assertions on the DeliverMax property
// @ts-expect-error -- DeliverMax property exists only at the RPC level, not at the protocol level
// eslint-disable-next-line @typescript-eslint/no-unnecessary-condition -- This is a valid null check for Amount
if (tx.Amount != null && tx.Amount !== tx.DeliverMax) {
return Promise.reject(
new ValidationError(
'PaymentTransaction: Amount and DeliverMax fields must be identical when both are provided',
),
)
}

// eslint-disable-next-line @typescript-eslint/ban-ts-comment -- ignore type-assertions on the DeliverMax property
// @ts-expect-error -- DeliverMax property exists only at the RPC level, not at the protocol level
delete tx.DeliverMax
}

return Promise.all(promises).then(() => tx)
}

Expand Down Expand Up @@ -909,7 +940,7 @@ class Client extends EventEmitter<EventTypes> {
* @param options.limit - Limit number of balances to return.
* @returns An array of XRP/non-XRP balances for the given account.
*/
// eslint-disable-next-line max-lines-per-function -- Longer definition is required for end users to see the definition.
/* eslint-disable max-lines-per-function -- getBalances requires more lines to implement logic */
public async getBalances(
address: string,
options: {
Expand Down Expand Up @@ -957,6 +988,7 @@ class Client extends EventEmitter<EventTypes> {
)
return balances.slice(0, options.limit)
}
/* eslint-enable max-lines-per-function */

/**
* Fetch orderbook (buy/sell orders) between two currency pairs. This checks both sides of the orderbook
Expand Down
65 changes: 63 additions & 2 deletions packages/xrpl/test/client/autofill.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
Payment,
Transaction,
} from '../../src'
import { ValidationError } from '../../src/errors'
import rippled from '../fixtures/rippled'
import {
setupClient,
Expand All @@ -23,6 +24,8 @@ const HOOKS_TESTNET_ID = 21338

describe('client.autofill', function () {
let testContext: XrplTestContext
const AMOUNT = '1234'
let paymentTx: Payment

async function setupMockRippledVersionAndID(
buildVersion: string,
Expand All @@ -41,10 +44,68 @@ describe('client.autofill', function () {
await testContext.client.connect()
}

beforeEach(async () => {
beforeAll(async () => {
testContext = await setupClient()
})
afterEach(async () => teardownClient(testContext))
afterAll(async () => teardownClient(testContext))

beforeEach(async () => {
paymentTx = {
TransactionType: 'Payment',
Account: 'rUn84CUYbNjRoTQ6mSW7BVJPSVJNLb1QLo',
Amount: AMOUNT,
Destination: 'rfkE1aSy9G8Upk4JssnwBxhEv5p4mn2KTy',
DestinationTag: 1,
Fee: '12',
Flags: 2147483648,
LastLedgerSequence: 65953073,
Sequence: 65923914,
SigningPubKey:
'02F9E33F16DF9507705EC954E3F94EB5F10D1FC4A354606DBE6297DBB1096FE654',
TxnSignature:
'3045022100E3FAE0EDEC3D6A8FF6D81BC9CF8288A61B7EEDE8071E90FF9314CB4621058D10022043545CF631706D700CEE65A1DB83EFDD185413808292D9D90F14D87D3DC2D8CB',
InvoiceID:
'6F1DFD1D0FE8A32E40E1F2C05CF1C15545BAB56B617F9C6C2D63A6B704BEF59B',
Paths: [
[{ currency: 'BTC', issuer: 'r9vbV3EHvXWjSkeQ6CAcYVPGeq7TuiXY2X' }],
],
SendMax: '100000000',
}
})

it('Validate Payment transaction API v2: Payment Transaction: Specify Only Amount field', async function () {
const txResult = await testContext.client.autofill(paymentTx)

assert.strictEqual(txResult.Amount, AMOUNT)
})

it('Validate Payment transaction API v2: Payment Transaction: Specify Only DeliverMax field', async function () {
// @ts-expect-error -- DeliverMax is a non-protocol, RPC level field in Payment transactions
paymentTx.DeliverMax = paymentTx.Amount
// @ts-expect-error -- DeliverMax is a non-protocol, RPC level field in Payment transactions
delete paymentTx.Amount
const txResult = await testContext.client.autofill(paymentTx)

assert.strictEqual(txResult.Amount, AMOUNT)
})

it('Validate Payment transaction API v2: Payment Transaction: identical DeliverMax and Amount fields', async function () {
// @ts-expect-error -- DeliverMax is a non-protocol, RPC level field in Payment transactions
paymentTx.DeliverMax = paymentTx.Amount

const txResult = await testContext.client.autofill(paymentTx)

assert.strictEqual(txResult.Amount, AMOUNT)
assert.strictEqual('DeliverMax' in txResult, false)
})

it('Validate Payment transaction API v2: Payment Transaction: differing DeliverMax and Amount fields', async function () {
// @ts-expect-error -- DeliverMax is a non-protocol, RPC level field in Payment transactions
paymentTx.DeliverMax = '6789'
paymentTx.Amount = '1234'

await assertRejects(testContext.client.autofill(paymentTx), ValidationError)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@justinr1234 here is the test case where the ValidationError is caught as expected -- there is not websocket connection to a rippled instance, but it traverses the new code path.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this is not an integration test, hence it does not involve any codepaths through connection.ts or other websocket related functions

})

it('should not autofill if fields are present', async function () {
const tx: Transaction = {
Expand Down
78 changes: 74 additions & 4 deletions packages/xrpl/test/integration/transactions/payment.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import { Payment } from '../../../src'
import { assert } from 'chai'

import { Payment, Wallet } from '../../../src'
import serverUrl from '../serverUrl'
import {
setupClient,
Expand All @@ -12,24 +14,92 @@ const TIMEOUT = 20000

describe('Payment', function () {
let testContext: XrplIntegrationTestContext
let paymentTx: Payment
const AMOUNT = '10000000'
// This wallet is used for DeliverMax related tests
let senderWallet: Wallet

beforeEach(async () => {
// this payment transaction JSON needs to be refreshed before every test.
// Because, we tinker with Amount and DeliverMax fields in the API v2 tests
paymentTx = {
TransactionType: 'Payment',
Account: senderWallet.classicAddress,
Amount: AMOUNT,
Destination: 'rfkE1aSy9G8Upk4JssnwBxhEv5p4mn2KTy',
}
})

beforeAll(async () => {
testContext = await setupClient(serverUrl)
senderWallet = await generateFundedWallet(testContext.client)
})
afterEach(async () => teardownClient(testContext))
afterAll(async () => teardownClient(testContext))

it(
'base',
async () => {
const wallet2 = await generateFundedWallet(testContext.client)
const tx: Payment = {
TransactionType: 'Payment',
Account: testContext.wallet.classicAddress,
Destination: wallet2.classicAddress,
Destination: senderWallet.classicAddress,
Amount: '1000',
}
await testTransaction(testContext.client, tx, testContext.wallet)
},
TIMEOUT,
)

it(
'Validate Payment transaction API v2: Payment Transaction: Specify Only Amount field',
async () => {
const result = await testTransaction(
testContext.client,
paymentTx,
senderWallet,
)

assert.equal(result.result.engine_result_code, 0)
assert.equal((result.result.tx_json as Payment).Amount, AMOUNT)
},
TIMEOUT,
)

it(
'Validate Payment transaction API v2: Payment Transaction: Specify Only DeliverMax field',
async () => {
// @ts-expect-error -- DeliverMax is a non-protocol, RPC level field in Payment transactions
paymentTx.DeliverMax = paymentTx.Amount
// @ts-expect-error -- DeliverMax is a non-protocol, RPC level field in Payment transactions
delete paymentTx.Amount

const result = await testTransaction(
testContext.client,
paymentTx,
senderWallet,
)

assert.equal(result.result.engine_result_code, 0)
assert.equal((result.result.tx_json as Payment).Amount, AMOUNT)
},
TIMEOUT,
)

it(
'Validate Payment transaction API v2: Payment Transaction: identical DeliverMax and Amount fields',
async () => {
// @ts-expect-error -- DeliverMax is a non-protocol, RPC level field in Payment transactions
paymentTx.DeliverMax = paymentTx.Amount

const result = await testTransaction(
testContext.client,
paymentTx,
senderWallet,
)

assert.equal(result.result.engine_result_code, 0)
assert.equal((result.result.tx_json as Payment).Amount, AMOUNT)
},
TIMEOUT,
)
})
Loading