From 526b4a4a6d4e3374412e8a39eaea12ac948bf482 Mon Sep 17 00:00:00 2001 From: JoelKatz Date: Sun, 31 Mar 2013 23:55:59 -0700 Subject: [PATCH 1/6] Don't permit XRP to be specified as an object. --- src/cpp/ripple/Amount.cpp | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/cpp/ripple/Amount.cpp b/src/cpp/ripple/Amount.cpp index 115fd9a367..df38bbee76 100644 --- a/src/cpp/ripple/Amount.cpp +++ b/src/cpp/ripple/Amount.cpp @@ -158,7 +158,13 @@ STAmount::STAmount(SField::ref n, const Json::Value& v) mIsNative = !currency.isString() || currency.asString().empty() || (currency.asString() == SYSTEM_CURRENCY_CODE); - if (!mIsNative) { + if (mIsNative) + { + if (v.isObject()) + throw std::runtime_error("XRP may not be specified as an object"); + } + else + { // non-XRP if (!currencyFromString(mCurrency, currency.asString())) throw std::runtime_error("invalid currency"); From f6b9e0ca80da438df09dd70cc56466cbea0aeeb6 Mon Sep 17 00:00:00 2001 From: JoelKatz Date: Sun, 31 Mar 2013 23:56:37 -0700 Subject: [PATCH 2/6] Should return 'false', not throw. --- src/cpp/ripple/SHAMap.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cpp/ripple/SHAMap.cpp b/src/cpp/ripple/SHAMap.cpp index 676290ccad..a08028047e 100644 --- a/src/cpp/ripple/SHAMap.cpp +++ b/src/cpp/ripple/SHAMap.cpp @@ -582,7 +582,7 @@ bool SHAMap::addGiveItem(SHAMapItem::ref item, bool isTransaction, bool hasMeta) stack.pop(); if (node->isLeaf() && (node->peekItem()->getTag() == tag)) - throw std::runtime_error("addGiveItem ends on leaf with same tag"); + return false; uint256 prevHash; returnNode(node, true); From 450fb5c0dbb070a595053bca699eaeecd1ca62fe Mon Sep 17 00:00:00 2001 From: JoelKatz Date: Sun, 31 Mar 2013 23:56:48 -0700 Subject: [PATCH 3/6] Cosmetic. --- src/cpp/ripple/Transactor.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/cpp/ripple/Transactor.cpp b/src/cpp/ripple/Transactor.cpp index 7534546d54..c7924b7a80 100644 --- a/src/cpp/ripple/Transactor.cpp +++ b/src/cpp/ripple/Transactor.cpp @@ -142,7 +142,8 @@ TER Transactor::checkSeq() cLog(lsWARNING) << "applyTransaction: past sequence number"; return tefPAST_SEQ; - }else + } + else { mTxnAccount->setFieldU32(sfSequence, t_seq + 1); } From d0009872c1c4cb7239bed3c036942ad5a0237b94 Mon Sep 17 00:00:00 2001 From: JoelKatz Date: Sun, 31 Mar 2013 23:57:03 -0700 Subject: [PATCH 4/6] Check. --- src/cpp/ripple/TransactionCheck.cpp | 5 +++++ src/cpp/ripple/TransactionEngine.cpp | 10 +++++++++- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/src/cpp/ripple/TransactionCheck.cpp b/src/cpp/ripple/TransactionCheck.cpp index 50b1a7f7cf..433bd6960d 100644 --- a/src/cpp/ripple/TransactionCheck.cpp +++ b/src/cpp/ripple/TransactionCheck.cpp @@ -7,5 +7,10 @@ bool TransactionEngine::checkInvariants(TER result, const SerializedTransaction& txn, TransactionEngineParams params) { + +// 1) Make sure transaction changed account sequence number to correct value + +// 2) Make sure transaction didn't create XRP + return true; } diff --git a/src/cpp/ripple/TransactionEngine.cpp b/src/cpp/ripple/TransactionEngine.cpp index 1c0049fb20..1747f114d1 100644 --- a/src/cpp/ripple/TransactionEngine.cpp +++ b/src/cpp/ripple/TransactionEngine.cpp @@ -176,12 +176,20 @@ TER TransactionEngine::applyTransaction(const SerializedTransaction& txn, Transa if (isSetBit(params, tapOPEN_LEDGER)) { if (!mLedger->addTransaction(txID, s)) + { + cLog(lsFATAL) << "Tried to add transaction to open ledger that already had it"; assert(false); + throw std::runtime_error("Duplicate transaction applied"); + } } else { if (!mLedger->addTransaction(txID, s, m)) - assert(false); + { + cLog(lsFATAL) << "Tried to add transaction to ledger that already had it"; + assert(false); + throw std::runtime_error("Duplicate transaction applied to closed ledger"); + } // Charge whatever fee they specified. STAmount saPaid = txn.getTransactionFee(); From 1725b5df48083310ffc6275f1b81b1e02e50d9a8 Mon Sep 17 00:00:00 2001 From: Vahe Hovhannisyan Date: Fri, 29 Mar 2013 14:54:43 +0400 Subject: [PATCH 5/6] JS: new account_tx --- src/js/remote.js | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/src/js/remote.js b/src/js/remote.js index 085a279858..ed7c83022a 100644 --- a/src/js/remote.js +++ b/src/js/remote.js @@ -821,20 +821,33 @@ Remote.prototype.request_account_offers = function (accountID, account_index, cu .ledger_choose(current); }; -Remote.prototype.request_account_tx = function (accountID, ledger_min, ledger_max) { +Remote.prototype.request_account_tx = function (accountID, ledger_index_min, ledger_index_max, descending, limit, offset) { // XXX Does this require the server to be trusted? //utils.assert(this.trusted); var request = new Request(this, 'account_tx'); - request.message.account = accountID; + request.message.account = accountID; + request.message.count = true; - if (ledger_min === ledger_max) { - request.message.ledger = ledger_min; + if (descending) { + request.message.descending = descending; + } + + if (limit) { + request.message.limit = limit; + } + + if (offset) { + request.message.offset = offset; + } + + if (ledger_index_min === ledger_index_max) { + request.message.ledger = ledger_index_min; } else { - request.message.ledger_min = ledger_min; - request.message.ledger_max = ledger_max; + request.message.ledger_index_min = ledger_index_min; + request.message.ledger_index_max = ledger_index_max; } return request; From 5659fb44b38ee64944be0374222379779ba6de65 Mon Sep 17 00:00:00 2001 From: jatchili Date: Mon, 1 Apr 2013 16:24:48 -0700 Subject: [PATCH 6/6] Added unit test: account_tx-test.js. Also, revised request_account_tx in remote.js to use the new argument list for account_tx. --- src/js/remote.js | 29 ++++-- test/account_tx-test.js | 190 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 213 insertions(+), 6 deletions(-) create mode 100644 test/account_tx-test.js diff --git a/src/js/remote.js b/src/js/remote.js index 085a279858..b363e6068b 100644 --- a/src/js/remote.js +++ b/src/js/remote.js @@ -821,20 +821,37 @@ Remote.prototype.request_account_offers = function (accountID, account_index, cu .ledger_choose(current); }; -Remote.prototype.request_account_tx = function (accountID, ledger_min, ledger_max) { + +/* + account: account, + ledger_index_min: ledger_index, // optional, defaults to -1 if ledger_index_max is specified. + ledger_index_max: ledger_index, // optional, defaults to -1 if ledger_index_min is specified. + binary: boolean, // optional, defaults to false + count: boolean, // optional, defaults to false + descending: boolean, // optional, defaults to false + offset: integer, // optional, defaults to 0 + limit: integer // optional +*/ + +Remote.prototype.request_account_tx = function (obj) { // XXX Does this require the server to be trusted? //utils.assert(this.trusted); var request = new Request(this, 'account_tx'); - request.message.account = accountID; + request.message.account = obj.account; - if (ledger_min === ledger_max) { - request.message.ledger = ledger_min; + if (false && ledger_min === ledger_max) { + //request.message.ledger = ledger_min; } else { - request.message.ledger_min = ledger_min; - request.message.ledger_max = ledger_max; + if (obj.ledger_index_min) {request.message.ledger_index_min = obj.ledger_index_min;} + if (obj.ledger_index_max) {request.message.ledger_index_max = obj.ledger_index_max;} + if (obj.binary) {request.message.binary = obj.binary;} + if (obj.count) {request.message.count = obj.count;} + if (obj.descending) {request.message.descending = obj.descending;} + if (obj.offset) {request.message.offset = obj.offset;} + if (obj.limit) {request.message.limit = obj.limit;} } return request; diff --git a/test/account_tx-test.js b/test/account_tx-test.js new file mode 100644 index 0000000000..cdc8b29f54 --- /dev/null +++ b/test/account_tx-test.js @@ -0,0 +1,190 @@ +var async = require("async"); +var buster = require("buster"); + +var Amount = require("../src/js/amount").Amount; +var Remote = require("../src/js/remote").Remote; +var Transaction = require("../src/js/transaction").Transaction; +var Server = require("./server").Server; + +var testutils = require("./testutils"); + +require('../src/js/config').load(require('./config')); + +buster.testRunner.timeout = 250000; //This is a very long test! + + +// Hard-coded limits we'll be testing: +var BINARY_LIMIT = 500; +var NONBINARY_LIMIT = 200; + +var ACCOUNT = "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh"; +var FIRST_BATCH = 199; // Within both limits +var OFFSET = 180; +var LIMIT = 170; +var SECOND_BATCH = 10; // Between NONBINARY_LIMIT and BINARY_LIMIT +var THIRD_BATCH = 295; // Exceeds both limits + +buster.testCase("Account_tx tests", { + 'setUp' : testutils.build_setup(), + 'tearDown' : testutils.build_teardown(), + + "make a lot of transactions and query using account_tx" : + function (done) { + var self = this; + var final_create; + + var transactionCounter = 0; + + var createOfferFunction = function (callback) { + self.remote.transaction() + .offer_create("root", "500", "100/USD/root") + .on('proposed', function (m) { + transactionCounter++; + console.log('Submitted transaction', transactionCounter); + callback(m.result !== 'tesSUCCESS'); + }) + .on('final', function (m) { + buster.assert.equals('tesSUCCESS', m.metadata.TransactionResult); + buster.assert(final_create); + }) + .submit(); + }; + + function lotsOfTransactions(number, whenDone) { + var bunchOfOffers = []; + for (var i=0; it2.inLedger || (t1.inLedger==t2.inLedger && t1.hash > t2.hash ), + "Transactions were not ordered correctly: "+t1.inLedger+"#"+t1.hash+" should not have come before "+t2.inLedger+"#"+t2.hash); + } + callback(false); + }) + .on('error', standardErrorHandler(callback)) + .request(); + }, + + + ], function (error) { + buster.refute(error); + finalCallback(); + } + ); + } + } +}); + + + +// TODO: +// Test the "count" feature. \ No newline at end of file