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

Jump to bottom

introduce DeliverMax alias in Payment transactions #2684

Closed
wants to merge 10 commits into from
Closed

introduce DeliverMax alias in Payment transactions #2684

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
bf23a74
generateSeed() update: If no algorithm is specified, use ed25519 algo…
ckeshava Mar 7, 2024
08d8b08
introduce DeliverMax alias in Payment transactions
ckeshava Apr 23, 2024
331828d
Update packages/xrpl/src/models/transactions/payment.ts
ckeshava Apr 29, 2024
adfd945
Update packages/xrpl/src/models/transactions/payment.ts
ckeshava Apr 29, 2024
545f71d
Update packages/xrpl/src/models/transactions/payment.ts
ckeshava Apr 29, 2024
0a2ae5c
address Omar's comments
ckeshava Apr 29, 2024
e2bc2d6
Merge https://github.com/ckeshava/xrpl.js into dMax
ckeshava Apr 29, 2024
526c83d
Merge branch 'dMax' of https://github.com/ckeshava/xrpl.js into dMax
ckeshava Apr 29, 2024
a8036ae
Revert "Merge https://github.com/ckeshava/xrpl.js into dMax"
ckeshava Apr 29, 2024
109a88d
address omar's comments
ckeshava May 2, 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
18 changes: 18 additions & 0 deletions packages/xrpl/src/models/transactions/payment.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,24 @@ export interface PaymentMetadata extends TransactionMetadataBase {
export function validatePayment(tx: Record<string, unknown>): void {
validateBaseTransaction(tx)

if (tx.Amount === undefined) {
ckeshava marked this conversation as resolved.
Show resolved Hide resolved
// If only DeliverMax is provided, use it to populate the Amount field
if (tx.DeliverMax !== undefined) {
ckeshava marked this conversation as resolved.
Show resolved Hide resolved
tx.Amount = tx.DeliverMax
}
}

// If Amount is not identical to DeliverMax, throw an error
if (
tx.DeliverMax !== undefined &&
tx.Amount !== undefined &&
tx.Amount !== tx.DeliverMax
ckeshava marked this conversation as resolved.
Show resolved Hide resolved
) {
throw new ValidationError(
'PaymentTransaction: Amount and DeliverMax fields must be identical',
)
}

if (tx.Amount === undefined) {
throw new ValidationError('PaymentTransaction: missing field Amount')
}
Expand Down
134 changes: 134 additions & 0 deletions packages/xrpl/test/models/payment.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { validatePayment } from '../../src/models/transactions/payment'
*/
describe('Payment', function () {
let paymentTransaction
let paytxn1, paytxn2, paytxn3, paytxn4, paytxn5

beforeEach(function () {
paymentTransaction = {
Expand All @@ -33,6 +34,122 @@ describe('Payment', function () {
],
SendMax: '100000000',
} as any

// JSON contains only Amount field
paytxn1 = {
TransactionType: 'Payment',
Account: 'rUn84CUYbNjRoTQ6mSW7BVJPSVJNLb1QLo',
Amount: '1234',
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',
} as any

// JSON contains only DeliverMax field
paytxn2 = {
TransactionType: 'Payment',
Account: 'rUn84CUYbNjRoTQ6mSW7BVJPSVJNLb1QLo',
DeliverMax: '1234',
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',
} as any

// JSON contains identical DeliverMax and Amount fields
paytxn3 = {
TransactionType: 'Payment',
Account: 'rUn84CUYbNjRoTQ6mSW7BVJPSVJNLb1QLo',
DeliverMax: '1234',
Amount: '1234',
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',
} as any

// JSON contains different DeliverMax and Amount fields
paytxn4 = {
TransactionType: 'Payment',
Account: 'rUn84CUYbNjRoTQ6mSW7BVJPSVJNLb1QLo',
DeliverMax: '1234',
Amount: '321',
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',
} as any

// JSON does not contain either DeliverMax or Amount field
paytxn5 = {
TransactionType: 'Payment',
Account: 'rUn84CUYbNjRoTQ6mSW7BVJPSVJNLb1QLo',
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',
} as any
Copy link
Collaborator

Choose a reason for hiding this comment

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

It's more appropriate to put these objects in their corresponding tests.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed in 109a88d

})

it(`verifies valid PaymentTransaction`, function () {
Expand Down Expand Up @@ -258,4 +375,21 @@ describe('Payment', function () {
'PaymentTransaction: tfPartialPayment flag required with DeliverMin',
)
})

it(`verifies deliver_max alias in PaymentTransactions`, function () {
assert.doesNotThrow(() => validatePayment(paytxn1))
assert.doesNotThrow(() => validate(paytxn1))

assert.doesNotThrow(() => validatePayment(paytxn2))
assert.doesNotThrow(() => validate(paytxn2))

assert.doesNotThrow(() => validatePayment(paytxn3))
assert.doesNotThrow(() => validate(paytxn3))

assert.throws(() => validatePayment(paytxn4))
assert.throws(() => validate(paytxn4))

assert.throws(() => validatePayment(paytxn5))
assert.throws(() => validate(paytxn5))
})
Copy link
Collaborator

Choose a reason for hiding this comment

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

Each different test should be separated into its own it() function.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ok, I've addressed this comment in a8036ae

})
Loading