diff --git a/src/ripple/app/misc/TxQ.h b/src/ripple/app/misc/TxQ.h index 5f1d41c010..e375664aaa 100644 --- a/src/ripple/app/misc/TxQ.h +++ b/src/ripple/app/misc/TxQ.h @@ -613,17 +613,30 @@ private: } }; - /// Used for sorting @ref MaybeTx by `feeLevel` - class GreaterFee + /// Used for sorting @ref MaybeTx + class OrderCandidates { public: /// Default constructor - explicit GreaterFee() = default; + explicit OrderCandidates() = default; - /// Is the fee level of `lhs` greater than the fee level of `rhs`? + /** Sort @ref MaybeTx by `feeLevel` descending, then by + * transaction ID ascending + * + * The transaction queue is ordered such that transactions + * paying a higher fee are in front of transactions paying + * a lower fee, giving them an opportunity to be processed into + * the open ledger first. Within transactions paying the same + * fee, order by the arbitrary but consistent transaction ID. + * This allows validators to build similar queues in the same + * order, and thus have more similar initial proposals. + * + */ bool operator()(const MaybeTx& lhs, const MaybeTx& rhs) const { + if (lhs.feeLevel == rhs.feeLevel) + return lhs.txID < rhs.txID; return lhs.feeLevel > rhs.feeLevel; } }; @@ -722,7 +735,7 @@ private: &MaybeTx::byFeeListHook>; using FeeMultiSet = boost::intrusive:: - multiset>; + multiset>; using AccountMap = std::map; diff --git a/src/ripple/app/misc/impl/TxQ.cpp b/src/ripple/app/misc/impl/TxQ.cpp index 1315bb6e2e..04d06e04fe 100644 --- a/src/ripple/app/misc/impl/TxQ.cpp +++ b/src/ripple/app/misc/impl/TxQ.cpp @@ -466,30 +466,15 @@ TxQ::eraseAndAdvance(TxQ::FeeMultiSet::const_iterator_type candidateIter) assert(byFee_.iterator_to(accountIter->second) == candidateIter); auto const accountNextIter = std::next(accountIter); - // Check if the next transaction for this account has a greater - // SeqProxy, and a higher fee level, which means we skipped it - // earlier, and need to try it again. - // - // Edge cases: - // o If the next account tx has a lower fee level, it's going to be - // later in the fee queue, so we haven't skipped it yet. - // - // o If the next tx has an equal fee level, it was... - // - // * EITHER submitted later, so it's also going to be later in the - // fee queue, - // - // * OR the current was resubmitted to bump up the fee level, and - // we have skipped that next tx. - // - // In the latter case, continue through the fee queue anyway - // to head off potential ordering manipulation problems. + // Check if the next transaction for this account is earlier in the queue, + // which means we skipped it earlier, and need to try it again. + OrderCandidates o; auto const feeNextIter = std::next(candidateIter); bool const useAccountNext = accountNextIter != txQAccount.transactions.end() && accountNextIter->first > candidateIter->seqProxy && (feeNextIter == byFee_.end() || - accountNextIter->second.feeLevel > feeNextIter->feeLevel); + o(accountNextIter->second, *feeNextIter)); auto const candidateNextIter = byFee_.erase(candidateIter); txQAccount.transactions.erase(accountIter); @@ -1224,9 +1209,19 @@ TxQ::apply( if (!replacedTxIter && isFull()) { auto lastRIter = byFee_.rbegin(); - if (lastRIter->account == account) + while (lastRIter != byFee_.rend() && lastRIter->account == account) { - JLOG(j_.warn()) + ++lastRIter; + } + if (lastRIter == byFee_.rend()) + { + // The only way this condition can happen is if the entire + // queue is filled with transactions from this account. This + // is impossible with default settings - minimum queue size + // is 2000, and an account can only have 10 transactions + // queued. However, it can occur if settings are changed, + // and there is unit test coverage. + JLOG(j_.info()) << "Queue is full, and transaction " << transactionID << " would kick a transaction from the same account (" << account << ") out of the queue."; diff --git a/src/ripple/core/impl/JobQueue.cpp b/src/ripple/core/impl/JobQueue.cpp index fc50ff603e..6588a7078c 100644 --- a/src/ripple/core/impl/JobQueue.cpp +++ b/src/ripple/core/impl/JobQueue.cpp @@ -168,13 +168,9 @@ JobQueue::addLoadEvents(JobType t, int count, std::chrono::milliseconds elapsed) bool JobQueue::isOverloaded() { - for (auto& x : m_jobData) - { - if (x.second.load().isOver()) - return true; - } - - return false; + return std::any_of(m_jobData.begin(), m_jobData.end(), [](auto& entry) { + return entry.second.load().isOver(); + }); } Json::Value diff --git a/src/test/app/TxQ_test.cpp b/src/test/app/TxQ_test.cpp index 3fdf00a50f..2fcc4c8781 100644 --- a/src/test/app/TxQ_test.cpp +++ b/src/test/app/TxQ_test.cpp @@ -802,7 +802,7 @@ public: env(noop(charlie), fee(7000), queued); env(noop(daria), fee(7000), queued); env(noop(edgar), fee(7000), queued); - env(noop(felicia), fee(7000), queued); + env(noop(felicia), fee(6999), queued); checkMetrics(env, 6, 6, 4, 3, 257); env.close(); @@ -816,8 +816,8 @@ public: env(noop(daria), fee(8000), queued); env(noop(daria), fee(8000), seq(env.seq(daria) + 1), queued); env(noop(edgar), fee(8000), queued); - env(noop(felicia), fee(8000), queued); - env(noop(felicia), fee(8000), seq(env.seq(felicia) + 1), queued); + env(noop(felicia), fee(7999), queued); + env(noop(felicia), fee(7999), seq(env.seq(felicia) + 1), queued); checkMetrics(env, 8, 8, 5, 4, 257, 700 * 256); env.close(); @@ -1039,7 +1039,7 @@ public: checkMetrics(env, 0, initQueueMax, 4, 3, 256); // Alice - price starts exploding: held - env(noop(alice), queued); + env(noop(alice), fee(11), queued); checkMetrics(env, 1, initQueueMax, 4, 3, 256); auto aliceSeq = env.seq(alice); @@ -1088,7 +1088,7 @@ public: BEAST_EXPECT(env.seq(charlie) == charlieSeq + 1); // Alice - fill up the queue - std::int64_t aliceFee = 20; + std::int64_t aliceFee = 27; aliceSeq = env.seq(alice); auto lastLedgerSeq = env.current()->info().seq + 2; for (auto i = 0; i < 7; i++) @@ -1096,7 +1096,7 @@ public: env(noop(alice), seq(aliceSeq), json(jss::LastLedgerSequence, lastLedgerSeq + i), - fee(aliceFee), + fee(--aliceFee), queued); ++aliceSeq; } @@ -1104,17 +1104,18 @@ public: { auto& txQ = env.app().getTxQ(); auto aliceStat = txQ.getAccountTxs(alice.id(), *env.current()); - constexpr XRPAmount fee{20}; + aliceFee = 27; auto const& baseFee = env.current()->fees().base; auto seq = env.seq(alice); BEAST_EXPECT(aliceStat.size() == 7); for (auto const& tx : aliceStat) { BEAST_EXPECT(tx.seqProxy.isSeq() && tx.seqProxy.value() == seq); - BEAST_EXPECT(tx.feeLevel == toFeeLevel(fee, baseFee)); + BEAST_EXPECT( + tx.feeLevel == toFeeLevel(XRPAmount(--aliceFee), baseFee)); BEAST_EXPECT(tx.lastValid); BEAST_EXPECT( - (tx.consequences.fee() == drops(fee) && + (tx.consequences.fee() == drops(aliceFee) && tx.consequences.potentialSpend() == drops(0) && !tx.consequences.isBlocker()) || tx.seqProxy.value() == env.seq(alice) + 6); @@ -1141,13 +1142,13 @@ public: // Charlie - add another item to the queue, which // causes Alice's last txn to drop env(noop(charlie), fee(30), queued); - checkMetrics(env, 8, 8, 5, 4, 513); + checkMetrics(env, 8, 8, 5, 4, 538); // Alice - now attempt to add one more to the queue, // which fails because the last tx was dropped, so // there is no complete chain. env(noop(alice), seq(aliceSeq), fee(aliceFee), ter(telCAN_NOT_QUEUE)); - checkMetrics(env, 8, 8, 5, 4, 513); + checkMetrics(env, 8, 8, 5, 4, 538); // Alice wants this tx more than the dropped tx, // so resubmits with higher fee, but the queue @@ -1156,22 +1157,22 @@ public: seq(aliceSeq - 1), fee(aliceFee), ter(telCAN_NOT_QUEUE_FULL)); - checkMetrics(env, 8, 8, 5, 4, 513); + checkMetrics(env, 8, 8, 5, 4, 538); // Try to replace a middle item in the queue // without enough fee. aliceSeq = env.seq(alice) + 2; - aliceFee = 25; + aliceFee = 29; env(noop(alice), seq(aliceSeq), fee(aliceFee), ter(telCAN_NOT_QUEUE_FEE)); - checkMetrics(env, 8, 8, 5, 4, 513); + checkMetrics(env, 8, 8, 5, 4, 538); // Replace a middle item from the queue successfully ++aliceFee; env(noop(alice), seq(aliceSeq), fee(aliceFee), queued); - checkMetrics(env, 8, 8, 5, 4, 513); + checkMetrics(env, 8, 8, 5, 4, 538); env.close(); // Alice's transactions processed, along with @@ -1186,7 +1187,7 @@ public: // more than the minimum reserve in flight before the // last queued transaction aliceFee = - env.le(alice)->getFieldAmount(sfBalance).xrp().drops() - (59); + env.le(alice)->getFieldAmount(sfBalance).xrp().drops() - (62); env(noop(alice), seq(aliceSeq), fee(aliceFee), @@ -1334,6 +1335,9 @@ public: auto hankSeq = env.seq(hank); // This time, use identical fees. + + // This one gets into the queue, but gets dropped when the + // higher fee one is added later. env(noop(alice), fee(15), queued); env(noop(bob), fee(15), queued); env(noop(charlie), fee(15), queued); @@ -1341,8 +1345,6 @@ public: env(noop(elmo), fee(15), queued); env(noop(fred), fee(15), queued); env(noop(gwen), fee(15), queued); - // This one gets into the queue, but gets dropped when the - // higher fee one is added later. env(noop(hank), fee(15), queued); // Queue is full now. Minimum fee now reflects the @@ -1362,9 +1364,9 @@ public: // Queue is still full. checkMetrics(env, 8, 8, 5, 4, 385); - // alice, bob, charlie, daria, and elmo's txs + // bob, charlie, daria, elmo, and fred's txs // are processed out of the queue into the ledger, - // leaving fred and gwen's txs. hank's tx is + // leaving fred and hank's txs. alice's tx is // retried from localTxs, and put back into the // queue. env.close(); @@ -1372,45 +1374,46 @@ public: BEAST_EXPECT(aliceSeq + 1 == env.seq(alice)); BEAST_EXPECT(bobSeq + 1 == env.seq(bob)); - BEAST_EXPECT(charlieSeq + 2 == env.seq(charlie)); + BEAST_EXPECT(charlieSeq == env.seq(charlie)); BEAST_EXPECT(dariaSeq + 1 == env.seq(daria)); - BEAST_EXPECT(elmoSeq + 1 == env.seq(elmo)); - BEAST_EXPECT(fredSeq == env.seq(fred)); - BEAST_EXPECT(gwenSeq == env.seq(gwen)); - BEAST_EXPECT(hankSeq == env.seq(hank)); + BEAST_EXPECT(elmoSeq == env.seq(elmo)); + BEAST_EXPECT(fredSeq + 1 == env.seq(fred)); + BEAST_EXPECT(gwenSeq + 1 == env.seq(gwen)); + BEAST_EXPECT(hankSeq + 1 == env.seq(hank)); aliceSeq = env.seq(alice); bobSeq = env.seq(bob); charlieSeq = env.seq(charlie); dariaSeq = env.seq(daria); elmoSeq = env.seq(elmo); + fredSeq = env.seq(fred); // Fill up the queue again - env(noop(alice), fee(15), queued); - env(noop(alice), seq(aliceSeq + 1), fee(15), queued); - env(noop(alice), seq(aliceSeq + 2), fee(15), queued); + env(noop(fred), fee(15), queued); + env(noop(fred), seq(fredSeq + 1), fee(15), queued); + env(noop(fred), seq(fredSeq + 2), fee(15), queued); env(noop(bob), fee(15), queued); - env(noop(charlie), fee(15), queued); + env(noop(charlie), seq(charlieSeq + 2), fee(15), queued); env(noop(daria), fee(15), queued); // This one gets into the queue, but gets dropped when the // higher fee one is added later. - env(noop(elmo), fee(15), queued); + env(noop(elmo), seq(elmoSeq + 1), fee(15), queued); checkMetrics(env, 10, 10, 6, 5, 385); // Add another transaction, with a higher fee, // Not high enough to get into the ledger, but high // enough to get into the queue (and kick somebody out) - env(noop(alice), fee(100), seq(aliceSeq + 3), queued); + env(noop(fred), fee(100), seq(fredSeq + 3), queued); env.close(); checkMetrics(env, 4, 12, 7, 6, 256); - BEAST_EXPECT(fredSeq + 1 == env.seq(fred)); + BEAST_EXPECT(fredSeq + 4 == env.seq(fred)); BEAST_EXPECT(gwenSeq + 1 == env.seq(gwen)); BEAST_EXPECT(hankSeq + 1 == env.seq(hank)); - BEAST_EXPECT(aliceSeq + 4 == env.seq(alice)); - BEAST_EXPECT(bobSeq == env.seq(bob)); - BEAST_EXPECT(charlieSeq == env.seq(charlie)); + BEAST_EXPECT(aliceSeq == env.seq(alice)); + BEAST_EXPECT(bobSeq + 1 == env.seq(bob)); + BEAST_EXPECT(charlieSeq + 2 == env.seq(charlie)); BEAST_EXPECT(dariaSeq == env.seq(daria)); BEAST_EXPECT(elmoSeq == env.seq(elmo)); } @@ -2767,7 +2770,7 @@ public: // we only see a reduction by 5. env.close(); checkMetrics(env, 9, 50, 6, 5, 256); - BEAST_EXPECT(env.seq(alice) == aliceSeq + 16); + BEAST_EXPECT(env.seq(alice) == aliceSeq + 15); // Close ledger 7. That should remove 7 more of alice's transactions. env.close(); @@ -2775,7 +2778,7 @@ public: BEAST_EXPECT(env.seq(alice) == aliceSeq + 19); // Close one last ledger to see all of alice's transactions moved - // into the ledger. + // into the ledger, including the tickets env.close(); checkMetrics(env, 0, 70, 2, 7, 256); BEAST_EXPECT(env.seq(alice) == aliceSeq + 21); @@ -4130,7 +4133,7 @@ public: {{"minimum_txn_in_ledger_standalone", "1"}, {"ledgers_in_queue", "5"}, {"maximum_txn_per_account", "10"}}, - {{"account_reserve", "200"}, {"owner_reserve", "50"}}); + {{"account_reserve", "1000"}, {"owner_reserve", "50"}}); Env env(*this, std::move(cfg)); @@ -4184,14 +4187,16 @@ public: auto seqDaria = env.seq(daria); auto seqEllie = env.seq(ellie); auto seqFiona = env.seq(fiona); + // Use fees to guarantee order + int txFee{90}; for (int i = 0; i < 10; ++i) { - env(noop(alice), seq(seqAlice++), ter(terQUEUED)); - env(noop(bob), seq(seqBob++), ter(terQUEUED)); - env(noop(carol), seq(seqCarol++), ter(terQUEUED)); - env(noop(daria), seq(seqDaria++), ter(terQUEUED)); - env(noop(ellie), seq(seqEllie++), ter(terQUEUED)); - env(noop(fiona), seq(seqFiona++), ter(terQUEUED)); + env(noop(alice), seq(seqAlice++), fee(--txFee), ter(terQUEUED)); + env(noop(bob), seq(seqBob++), fee(--txFee), ter(terQUEUED)); + env(noop(carol), seq(seqCarol++), fee(--txFee), ter(terQUEUED)); + env(noop(daria), seq(seqDaria++), fee(--txFee), ter(terQUEUED)); + env(noop(ellie), seq(seqEllie++), fee(--txFee), ter(terQUEUED)); + env(noop(fiona), seq(seqFiona++), fee(--txFee), ter(terQUEUED)); } std::size_t expectedInQueue = 60; checkMetrics( @@ -4283,8 +4288,8 @@ public: // We'll be using fees to control which entries leave the queue in // which order. There's no "lowFee" -- that's the default fee from // the unit test. - auto const medFee = drops(15); - auto const hiFee = drops(1000); + int const medFee = 100; + int const hiFee = 1000; auto cfg = makeConfig( {{"minimum_txn_in_ledger_standalone", "5"}, @@ -4314,12 +4319,14 @@ public: // will expire out soon. auto seqAlice = env.seq(alice); auto const seqSaveAlice = seqAlice; + int feeDrops = 40; env(noop(alice), seq(seqAlice++), + fee(--feeDrops), json(R"({"LastLedgerSequence": 7})"), ter(terQUEUED)); - env(noop(alice), seq(seqAlice++), ter(terQUEUED)); - env(noop(alice), seq(seqAlice++), ter(terQUEUED)); + env(noop(alice), seq(seqAlice++), fee(--feeDrops), ter(terQUEUED)); + env(noop(alice), seq(seqAlice++), fee(--feeDrops), ter(terQUEUED)); BEAST_EXPECT(env.seq(alice) == seqSaveAlice); // Similarly for bob, but bob uses tickets in his transactions. @@ -4328,8 +4335,14 @@ public: ticket::use(bobTicketSeq + 0), json(R"({"LastLedgerSequence": 7})"), ter(terQUEUED)); - env(noop(bob), ticket::use(bobTicketSeq + 1), ter(terQUEUED)); - env(noop(bob), ticket::use(bobTicketSeq + 2), ter(terQUEUED)); + env(noop(bob), + ticket::use(bobTicketSeq + 1), + fee(--feeDrops), + ter(terQUEUED)); + env(noop(bob), + ticket::use(bobTicketSeq + 2), + fee(--feeDrops), + ter(terQUEUED)); // Fill the queue with higher fee transactions so alice's and // bob's transactions are stuck in the queue. @@ -4337,12 +4350,13 @@ public: auto seqDaria = env.seq(daria); auto seqEllie = env.seq(ellie); auto seqFiona = env.seq(fiona); + feeDrops = medFee; for (int i = 0; i < 7; ++i) { - env(noop(carol), seq(seqCarol++), fee(medFee), ter(terQUEUED)); - env(noop(daria), seq(seqDaria++), fee(medFee), ter(terQUEUED)); - env(noop(ellie), seq(seqEllie++), fee(medFee), ter(terQUEUED)); - env(noop(fiona), seq(seqFiona++), fee(medFee), ter(terQUEUED)); + env(noop(carol), seq(seqCarol++), fee(--feeDrops), ter(terQUEUED)); + env(noop(daria), seq(seqDaria++), fee(--feeDrops), ter(terQUEUED)); + env(noop(ellie), seq(seqEllie++), fee(--feeDrops), ter(terQUEUED)); + env(noop(fiona), seq(seqFiona++), fee(--feeDrops), ter(terQUEUED)); } checkMetrics(env, 34, 50, 7, 6, 256); @@ -4350,24 +4364,26 @@ public: checkMetrics(env, 26, 50, 8, 7, 256); // Re-fill the queue so alice and bob stay stuck. + feeDrops = medFee; for (int i = 0; i < 3; ++i) { - env(noop(carol), seq(seqCarol++), fee(medFee), ter(terQUEUED)); - env(noop(daria), seq(seqDaria++), fee(medFee), ter(terQUEUED)); - env(noop(ellie), seq(seqEllie++), fee(medFee), ter(terQUEUED)); - env(noop(fiona), seq(seqFiona++), fee(medFee), ter(terQUEUED)); + env(noop(carol), seq(seqCarol++), fee(--feeDrops), ter(terQUEUED)); + env(noop(daria), seq(seqDaria++), fee(--feeDrops), ter(terQUEUED)); + env(noop(ellie), seq(seqEllie++), fee(--feeDrops), ter(terQUEUED)); + env(noop(fiona), seq(seqFiona++), fee(--feeDrops), ter(terQUEUED)); } checkMetrics(env, 38, 50, 8, 7, 256); env.close(); checkMetrics(env, 29, 50, 9, 8, 256); // One more time... + feeDrops = medFee; for (int i = 0; i < 3; ++i) { - env(noop(carol), seq(seqCarol++), fee(medFee), ter(terQUEUED)); - env(noop(daria), seq(seqDaria++), fee(medFee), ter(terQUEUED)); - env(noop(ellie), seq(seqEllie++), fee(medFee), ter(terQUEUED)); - env(noop(fiona), seq(seqFiona++), fee(medFee), ter(terQUEUED)); + env(noop(carol), seq(seqCarol++), fee(--feeDrops), ter(terQUEUED)); + env(noop(daria), seq(seqDaria++), fee(--feeDrops), ter(terQUEUED)); + env(noop(ellie), seq(seqEllie++), fee(--feeDrops), ter(terQUEUED)); + env(noop(fiona), seq(seqFiona++), fee(--feeDrops), ter(terQUEUED)); } checkMetrics(env, 41, 50, 9, 8, 256); env.close(); @@ -4382,16 +4398,17 @@ public: env(noop(alice), seq(seqAlice), fee(hiFee), ter(telCAN_NOT_QUEUE)); // Once again, fill the queue almost to the brim. + feeDrops = medFee; for (int i = 0; i < 4; ++i) { - env(noop(carol), seq(seqCarol++), ter(terQUEUED)); - env(noop(daria), seq(seqDaria++), ter(terQUEUED)); - env(noop(ellie), seq(seqEllie++), ter(terQUEUED)); - env(noop(fiona), seq(seqFiona++), ter(terQUEUED)); + env(noop(carol), seq(seqCarol++), fee(--feeDrops), ter(terQUEUED)); + env(noop(daria), seq(seqDaria++), fee(--feeDrops), ter(terQUEUED)); + env(noop(ellie), seq(seqEllie++), fee(--feeDrops), ter(terQUEUED)); + env(noop(fiona), seq(seqFiona++), fee(--feeDrops), ter(terQUEUED)); } - env(noop(carol), seq(seqCarol++), ter(terQUEUED)); - env(noop(daria), seq(seqDaria++), ter(terQUEUED)); - env(noop(ellie), seq(seqEllie++), ter(terQUEUED)); + env(noop(carol), seq(seqCarol++), fee(--feeDrops), ter(terQUEUED)); + env(noop(daria), seq(seqDaria++), fee(--feeDrops), ter(terQUEUED)); + env(noop(ellie), seq(seqEllie++), fee(--feeDrops), ter(terQUEUED)); checkMetrics(env, 48, 50, 10, 9, 256); // Now induce a fee jump which should cause all the transactions @@ -4401,7 +4418,7 @@ public: // asynchronously lowered by LoadManager. Here we're just // pushing the local fee up really high and then hoping that we // outrace LoadManager undoing our work. - for (int i = 0; i < 10; ++i) + for (int i = 0; i < 30; ++i) env.app().getFeeTrack().raiseLocalFee(); // Now close the ledger, which will attempt to process alice's @@ -4442,7 +4459,7 @@ public: // Verify that there's a gap at the front of alice's queue by // queuing another low fee transaction into that spot. - env(noop(alice), seq(seqAlice++), ter(terQUEUED)); + env(noop(alice), seq(seqAlice++), fee(11), ter(terQUEUED)); // Verify that the first entry in alice's queue is still there // by trying to replace it and having that fail. @@ -4468,11 +4485,11 @@ public: // Verify that bob's first transaction was removed from the queue // by queueing another low fee transaction into that spot. - env(noop(bob), ticket::use(bobTicketSeq + 0), ter(terQUEUED)); + env(noop(bob), ticket::use(bobTicketSeq + 0), fee(12), ter(terQUEUED)); // Verify that bob's second transaction was removed from the queue // by queueing another low fee transaction into that spot. - env(noop(bob), ticket::use(bobTicketSeq + 1), ter(terQUEUED)); + env(noop(bob), ticket::use(bobTicketSeq + 1), fee(11), ter(terQUEUED)); // Verify that the last entry in bob's queue is still there // by trying to replace it and having that fail. diff --git a/src/test/rpc/LedgerRPC_test.cpp b/src/test/rpc/LedgerRPC_test.cpp index 13a2f9824f..df8bebface 100644 --- a/src/test/rpc/LedgerRPC_test.cpp +++ b/src/test/rpc/LedgerRPC_test.cpp @@ -1538,21 +1538,37 @@ class LedgerRPC_test : public beast::unit_test::suite env.close(); jrr = env.rpc("json", "ledger", to_string(jv))[jss::result]; - std::string txid1; - std::string txid2; - if (BEAST_EXPECT(jrr[jss::queue_data].size() == 2)) - { - auto const& txj = jrr[jss::queue_data][0u]; - BEAST_EXPECT(txj[jss::account] == alice.human()); - BEAST_EXPECT(txj[jss::fee_level] == "256"); - BEAST_EXPECT(txj["preflight_result"] == "tesSUCCESS"); - BEAST_EXPECT(txj["retries_remaining"] == 10); - BEAST_EXPECT(txj.isMember(jss::tx)); - auto const& tx = txj[jss::tx]; - BEAST_EXPECT(tx[jss::Account] == alice.human()); - BEAST_EXPECT(tx[jss::TransactionType] == jss::OfferCreate); - txid1 = tx[jss::hash].asString(); - } + const std::string txid1 = [&]() { + if (BEAST_EXPECT(jrr[jss::queue_data].size() == 2)) + { + const std::string txid0 = [&]() { + auto const& txj = jrr[jss::queue_data][0u]; + BEAST_EXPECT(txj[jss::account] == alice.human()); + BEAST_EXPECT(txj[jss::fee_level] == "256"); + BEAST_EXPECT(txj["preflight_result"] == "tesSUCCESS"); + BEAST_EXPECT(txj["retries_remaining"] == 10); + BEAST_EXPECT(txj.isMember(jss::tx)); + auto const& tx = txj[jss::tx]; + BEAST_EXPECT(tx[jss::Account] == alice.human()); + BEAST_EXPECT(tx[jss::TransactionType] == jss::AccountSet); + return tx[jss::hash].asString(); + }(); + + auto const& txj = jrr[jss::queue_data][1u]; + BEAST_EXPECT(txj[jss::account] == alice.human()); + BEAST_EXPECT(txj[jss::fee_level] == "256"); + BEAST_EXPECT(txj["preflight_result"] == "tesSUCCESS"); + BEAST_EXPECT(txj["retries_remaining"] == 10); + BEAST_EXPECT(txj.isMember(jss::tx)); + auto const& tx = txj[jss::tx]; + BEAST_EXPECT(tx[jss::Account] == alice.human()); + BEAST_EXPECT(tx[jss::TransactionType] == jss::OfferCreate); + const auto txid1 = tx[jss::hash].asString(); + BEAST_EXPECT(txid0 < txid1); + return txid1; + } + return std::string{}; + }(); env.close(); @@ -1561,7 +1577,15 @@ class LedgerRPC_test : public beast::unit_test::suite jrr = env.rpc("json", "ledger", to_string(jv))[jss::result]; if (BEAST_EXPECT(jrr[jss::queue_data].size() == 2)) { - auto const& txj = jrr[jss::queue_data][0u]; + auto const txid0 = [&]() { + auto const& txj = jrr[jss::queue_data][0u]; + BEAST_EXPECT(txj[jss::account] == alice.human()); + BEAST_EXPECT(txj[jss::fee_level] == "256"); + BEAST_EXPECT(txj["preflight_result"] == "tesSUCCESS"); + BEAST_EXPECT(txj.isMember(jss::tx)); + return txj[jss::tx].asString(); + }(); + auto const& txj = jrr[jss::queue_data][1u]; BEAST_EXPECT(txj[jss::account] == alice.human()); BEAST_EXPECT(txj[jss::fee_level] == "256"); BEAST_EXPECT(txj["preflight_result"] == "tesSUCCESS"); @@ -1569,6 +1593,7 @@ class LedgerRPC_test : public beast::unit_test::suite BEAST_EXPECT(txj["last_result"] == "terPRE_SEQ"); BEAST_EXPECT(txj.isMember(jss::tx)); BEAST_EXPECT(txj[jss::tx] == txid1); + BEAST_EXPECT(txid0 < txid1); } env.close(); @@ -1579,7 +1604,7 @@ class LedgerRPC_test : public beast::unit_test::suite jrr = env.rpc("json", "ledger", to_string(jv))[jss::result]; if (BEAST_EXPECT(jrr[jss::queue_data].size() == 2)) { - auto const& txj = jrr[jss::queue_data][0u]; + auto const& txj = jrr[jss::queue_data][1u]; BEAST_EXPECT(txj[jss::account] == alice.human()); BEAST_EXPECT(txj[jss::fee_level] == "256"); BEAST_EXPECT(txj["preflight_result"] == "tesSUCCESS"); @@ -1588,7 +1613,7 @@ class LedgerRPC_test : public beast::unit_test::suite BEAST_EXPECT(txj.isMember(jss::tx)); BEAST_EXPECT(txj[jss::tx].isMember(jss::tx_blob)); - auto const& txj2 = jrr[jss::queue_data][1u]; + auto const& txj2 = jrr[jss::queue_data][0u]; BEAST_EXPECT(txj2[jss::account] == alice.human()); BEAST_EXPECT(txj2[jss::fee_level] == "256"); BEAST_EXPECT(txj2["preflight_result"] == "tesSUCCESS"); @@ -1607,18 +1632,21 @@ class LedgerRPC_test : public beast::unit_test::suite jv[jss::binary] = false; jrr = env.rpc("json", "ledger", to_string(jv))[jss::result]; - if (BEAST_EXPECT(jrr[jss::queue_data].size() == 1)) - { - auto const& txj = jrr[jss::queue_data][0u]; - BEAST_EXPECT(txj[jss::account] == alice.human()); - BEAST_EXPECT(txj[jss::fee_level] == "256"); - BEAST_EXPECT(txj["preflight_result"] == "tesSUCCESS"); - BEAST_EXPECT(txj["retries_remaining"] == 1); - BEAST_EXPECT(txj["last_result"] == "terPRE_SEQ"); - BEAST_EXPECT(txj.isMember(jss::tx)); - BEAST_EXPECT(txj[jss::tx] != txid1); - txid2 = txj[jss::tx].asString(); - } + const std::string txid2 = [&]() { + if (BEAST_EXPECT(jrr[jss::queue_data].size() == 1)) + { + auto const& txj = jrr[jss::queue_data][0u]; + BEAST_EXPECT(txj[jss::account] == alice.human()); + BEAST_EXPECT(txj[jss::fee_level] == "256"); + BEAST_EXPECT(txj["preflight_result"] == "tesSUCCESS"); + BEAST_EXPECT(txj["retries_remaining"] == 1); + BEAST_EXPECT(txj["last_result"] == "terPRE_SEQ"); + BEAST_EXPECT(txj.isMember(jss::tx)); + BEAST_EXPECT(txj[jss::tx] != txid1); + return txj[jss::tx].asString(); + } + return std::string{}; + }(); jv[jss::full] = true;