From 0c2f9d0e62d151d0e3945e3793a1c293fd942d6e Mon Sep 17 00:00:00 2001 From: Alan Cohen Date: Tue, 9 Feb 2016 20:10:34 -0800 Subject: [PATCH 1/3] Add deliveredAmount to payment outcome It is impossible to reliably compute the delivered amount from the metadata due to fixed precision. If the partial payment flag is not set and the transaction succeeded, the delivered amount should always be considered to be the amount specified in the transaction. --- docs/index.md | 16 ++++++++++ src/common/schemas/objects/amount.json | 2 +- src/common/schemas/output/outcome.json | 4 +++ src/ledger/parse/amount.js | 4 +-- src/ledger/parse/payment.js | 6 +--- src/ledger/parse/utils.js | 29 +++++++++++++++++-- test/fixtures/responses/get-ledger-full.json | 4 +++ .../responses/get-transaction-payment.json | 5 ++++ test/fixtures/responses/get-transactions.json | 10 +++++++ 9 files changed, 69 insertions(+), 11 deletions(-) diff --git a/docs/index.md b/docs/index.md index bc4354ff..0af4a3cf 100644 --- a/docs/index.md +++ b/docs/index.md @@ -828,6 +828,7 @@ outcome | object | The outcome of the transaction (what effects it had). *outcome.orderbookChanges.\*[].* makerExchangeRate | [value](#value) | *Optional* The exchange rate between the `quantity` currency and the `totalPrice` currency from the point of view of the maker. *outcome.* ledgerVersion | integer | The ledger version that the transaction was validated in. *outcome.* indexInLedger | integer | The ordering index of the transaction in the ledger. +*outcome.* deliveredAmount | [amount](#amount) | *Optional* For payment transactions, it is impossible to reliably compute the actual delivered amount from the balanceChanges due to fixed precision. If the payment is not a partial payment and the transaction succeeded, the deliveredAmount should always be considered to be the amount specified in the transaction. *outcome.* timestamp | date-time string | *Optional* The timestamp when the transaction was validated. (May be missing when requesting transactions in binary mode.) ### Example @@ -867,6 +868,11 @@ return api.getTransaction(id).then(transaction => { "result": "tesSUCCESS", "timestamp": "2013-03-12T23:56:50.000Z", "fee": "0.00001", + "deliveredAmount": { + "currency": "USD", + "value": "0.001", + "counterparty": "rMH4UxPrbuMa1spCBR98hLLyNJp4d8p4tM" + }, "balanceChanges": { "rpZc4mVfWUif9CRoHRKKcmhu1nx2xktxBo": [ { @@ -1001,6 +1007,11 @@ return api.getTransactions(address).then(transaction => { "outcome": { "result": "tesSUCCESS", "fee": "0.00001", + "deliveredAmount": { + "currency": "USD", + "value": "0.001", + "counterparty": "rMH4UxPrbuMa1spCBR98hLLyNJp4d8p4tM" + }, "balanceChanges": { "rpZc4mVfWUif9CRoHRKKcmhu1nx2xktxBo": [ { @@ -1093,6 +1104,11 @@ return api.getTransactions(address).then(transaction => { "outcome": { "result": "tesSUCCESS", "fee": "0.00001", + "deliveredAmount": { + "currency": "USD", + "value": "0.001", + "counterparty": "rMH4UxPrbuMa1spCBR98hLLyNJp4d8p4tM" + }, "balanceChanges": { "rpZc4mVfWUif9CRoHRKKcmhu1nx2xktxBo": [ { diff --git a/src/common/schemas/objects/amount.json b/src/common/schemas/objects/amount.json index de91eae5..633d25d7 100644 --- a/src/common/schemas/objects/amount.json +++ b/src/common/schemas/objects/amount.json @@ -2,7 +2,7 @@ "$schema": "http://json-schema.org/draft-04/schema#", "title": "amount", "link": "amount", - "description": "An Amount on the Ripple Protocol, used also for XRP in the ripple-rest API", + "description": "An Amount on the Ripple Protocol", "allOf": [ {"$ref": "amountbase"}, {"required": ["value"]} diff --git a/src/common/schemas/output/outcome.json b/src/common/schemas/output/outcome.json index 01ada268..bb45749b 100644 --- a/src/common/schemas/output/outcome.json +++ b/src/common/schemas/output/outcome.json @@ -17,6 +17,10 @@ "$ref": "value", "description": "The XRP fee that was charged for the transaction." }, + "deliveredAmount": { + "$ref": "amount", + "description": "For payment transactions, it is impossible to reliably compute the actual delivered amount from the balanceChanges due to fixed precision. If the payment is not a partial payment and the transaction succeeded, the deliveredAmount should always be considered to be the amount specified in the transaction." + }, "balanceChanges": { "type": "object", "additionalProperties": { diff --git a/src/ledger/parse/amount.js b/src/ledger/parse/amount.js index d6b65971..3461a870 100644 --- a/src/ledger/parse/amount.js +++ b/src/ledger/parse/amount.js @@ -1,6 +1,6 @@ /* @flow */ 'use strict'; -const utils = require('./utils'); +const utils = require('../utils'); import type {Amount, RippledAmount} from '../../common/types.js'; @@ -8,7 +8,7 @@ function parseAmount(amount: RippledAmount): Amount { if (typeof amount === 'string') { return { currency: 'XRP', - value: utils.dropsToXrp(amount) + value: utils.common.dropsToXrp(amount) }; } return { diff --git a/src/ledger/parse/payment.js b/src/ledger/parse/payment.js index d3e221e0..f1eb39cf 100644 --- a/src/ledger/parse/payment.js +++ b/src/ledger/parse/payment.js @@ -6,10 +6,6 @@ const utils = require('./utils'); const parseAmount = require('./amount'); const txFlags = utils.txFlags; -function isPartialPayment(tx) { - return (tx.Flags & txFlags.Payment.PartialPayment) !== 0; -} - function isNoDirectRipple(tx) { return (tx.Flags & txFlags.Payment.NoRippleDirect) !== 0; } @@ -45,7 +41,7 @@ function parsePayment(tx: Object): Object { memos: utils.parseMemos(tx), invoiceID: tx.InvoiceID, paths: tx.Paths ? JSON.stringify(tx.Paths) : undefined, - allowPartialPayment: isPartialPayment(tx) || undefined, + allowPartialPayment: utils.isPartialPayment(tx) || undefined, noDirectRipple: isNoDirectRipple(tx) || undefined, limitQuality: isQualityLimited(tx) || undefined }); diff --git a/src/ledger/parse/utils.js b/src/ledger/parse/utils.js index 8177d94a..68a65e69 100644 --- a/src/ledger/parse/utils.js +++ b/src/ledger/parse/utils.js @@ -4,6 +4,9 @@ const _ = require('lodash'); const transactionParser = require('ripple-lib-transactionparser'); const utils = require('../utils'); const BigNumber = require('bignumber.js'); +const parseAmount = require('./amount'); + +import type {Amount} from '../common/types.js'; function adjustQualityForXRP( quality: string, takerGetsCurrency: string, takerPaysCurrency: string @@ -48,6 +51,24 @@ function removeEmptyCounterpartyInOrderbookChanges(orderbookChanges) { }); } +function isPartialPayment(tx) { + return (tx.Flags & utils.common.txFlags.Payment.PartialPayment) !== 0; +} + +function parseDeliveredAmount(tx: Object): Amount | void { + let deliveredAmount; + + if (tx.TransactionType === 'Payment') { + if (tx.meta.delivered_amount) { + deliveredAmount = parseAmount(tx.meta.delivered_amount); + } else if (tx.Amount && !isPartialPayment(tx)) { + deliveredAmount = parseAmount(tx.Amount); + } + } + + return deliveredAmount; +} + function parseOutcome(tx: Object): ?Object { const metadata = tx.meta || tx.metaData; if (!metadata) { @@ -58,15 +79,16 @@ function parseOutcome(tx: Object): ?Object { removeEmptyCounterpartyInBalanceChanges(balanceChanges); removeEmptyCounterpartyInOrderbookChanges(orderbookChanges); - return { + return utils.common.removeUndefined({ result: tx.meta.TransactionResult, timestamp: parseTimestamp(tx.date), fee: utils.common.dropsToXrp(tx.Fee), balanceChanges: balanceChanges, orderbookChanges: orderbookChanges, ledgerVersion: tx.ledger_index, - indexInLedger: tx.meta.TransactionIndex - }; + indexInLedger: tx.meta.TransactionIndex, + deliveredAmount: parseDeliveredAmount(tx) + }); } function hexToString(hex: string): ?string { @@ -93,6 +115,7 @@ module.exports = { hexToString, parseTimestamp, adjustQualityForXRP, + isPartialPayment, dropsToXrp: utils.common.dropsToXrp, constants: utils.common.constants, txFlags: utils.common.txFlags, diff --git a/test/fixtures/responses/get-ledger-full.json b/test/fixtures/responses/get-ledger-full.json index e552b52d..f55c9346 100644 --- a/test/fixtures/responses/get-ledger-full.json +++ b/test/fixtures/responses/get-ledger-full.json @@ -34,6 +34,10 @@ "outcome": { "result": "tesSUCCESS", "fee": "0.00001", + "deliveredAmount": { + "currency": "XRP", + "value": "10000" + }, "balanceChanges": { "rLQBHVhFnaC5gLEkgr6HgBJJ3bgeZHg9cj": [ { diff --git a/test/fixtures/responses/get-transaction-payment.json b/test/fixtures/responses/get-transaction-payment.json index a1f65cd2..c2776595 100644 --- a/test/fixtures/responses/get-transaction-payment.json +++ b/test/fixtures/responses/get-transaction-payment.json @@ -24,6 +24,11 @@ "result": "tesSUCCESS", "timestamp": "2013-03-12T23:56:50.000Z", "fee": "0.00001", + "deliveredAmount": { + "currency": "USD", + "value": "0.001", + "counterparty": "rMH4UxPrbuMa1spCBR98hLLyNJp4d8p4tM" + }, "balanceChanges": { "rpZc4mVfWUif9CRoHRKKcmhu1nx2xktxBo": [ { diff --git a/test/fixtures/responses/get-transactions.json b/test/fixtures/responses/get-transactions.json index 4effe849..971be899 100644 --- a/test/fixtures/responses/get-transactions.json +++ b/test/fixtures/responses/get-transactions.json @@ -30,6 +30,11 @@ "outcome": { "result": "tesSUCCESS", "fee": "0.00001", + "deliveredAmount": { + "currency": "USD", + "value": "0.001", + "counterparty": "rMH4UxPrbuMa1spCBR98hLLyNJp4d8p4tM" + }, "balanceChanges": { "rpZc4mVfWUif9CRoHRKKcmhu1nx2xktxBo": [ { @@ -122,6 +127,11 @@ "outcome": { "result": "tesSUCCESS", "fee": "0.00001", + "deliveredAmount": { + "currency": "USD", + "value": "0.001", + "counterparty": "rMH4UxPrbuMa1spCBR98hLLyNJp4d8p4tM" + }, "balanceChanges": { "rpZc4mVfWUif9CRoHRKKcmhu1nx2xktxBo": [ { From 06f847c2d08ed945d5b3610236b059fc41e717ee Mon Sep 17 00:00:00 2001 From: Alan Cohen Date: Wed, 10 Feb 2016 19:45:51 -0800 Subject: [PATCH 2/3] Fix lint errors --- src/ledger/parse/utils.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/ledger/parse/utils.js b/src/ledger/parse/utils.js index 68a65e69..453cac2c 100644 --- a/src/ledger/parse/utils.js +++ b/src/ledger/parse/utils.js @@ -38,14 +38,14 @@ function removeEmptyCounterparty(amount) { } function removeEmptyCounterpartyInBalanceChanges(balanceChanges) { - _.forEach(balanceChanges, (changes) => { + _.forEach(balanceChanges, changes => { _.forEach(changes, removeEmptyCounterparty); }); } function removeEmptyCounterpartyInOrderbookChanges(orderbookChanges) { - _.forEach(orderbookChanges, (changes) => { - _.forEach(changes, (change) => { + _.forEach(orderbookChanges, changes => { + _.forEach(changes, change => { _.forEach(change, removeEmptyCounterparty); }); }); @@ -99,7 +99,7 @@ function parseMemos(tx: Object): ?Array { if (!Array.isArray(tx.Memos) || tx.Memos.length === 0) { return undefined; } - return tx.Memos.map((m) => { + return tx.Memos.map(m => { return utils.common.removeUndefined({ type: m.Memo.parsed_memo_type || hexToString(m.Memo.MemoType), format: m.Memo.parsed_memo_format || hexToString(m.Memo.MemoFormat), From 353637a0c0d4164b1698ef5286ffe55ff36fd304 Mon Sep 17 00:00:00 2001 From: Alan Cohen Date: Fri, 12 Feb 2016 10:56:35 -0800 Subject: [PATCH 3/3] Add TODO comment for fixing workaround for rippled bug --- src/ledger/parse/utils.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/ledger/parse/utils.js b/src/ledger/parse/utils.js index 453cac2c..b9178296 100644 --- a/src/ledger/parse/utils.js +++ b/src/ledger/parse/utils.js @@ -58,6 +58,8 @@ function isPartialPayment(tx) { function parseDeliveredAmount(tx: Object): Amount | void { let deliveredAmount; + // TODO: Workaround for existing rippled bug where delivered_amount may not be + // provided for account_tx if (tx.TransactionType === 'Payment') { if (tx.meta.delivered_amount) { deliveredAmount = parseAmount(tx.meta.delivered_amount);