mirror of
https://github.com/XRPLF/rippled.git
synced 2025-12-06 17:27:55 +00:00
fix: Skip processing transaction batch if the batch is empty (#5670)
Avoids an assertion failure in NetworkOPsImp::apply in the unlikely event that all incoming transactions are invalid.
This commit is contained in:
80
src/test/app/NetworkOPs_test.cpp
Normal file
80
src/test/app/NetworkOPs_test.cpp
Normal file
@@ -0,0 +1,80 @@
|
||||
//------------------------------------------------------------------------------
|
||||
/*
|
||||
This file is part of rippled: https://github.com/ripple/rippled
|
||||
Copyright (c) 2020 Dev Null Productions
|
||||
|
||||
Permission to use, copy, modify, and/or distribute this software for any
|
||||
purpose with or without fee is hereby granted, provided that the above
|
||||
copyright notice and this permission notice appear in all copies.
|
||||
|
||||
THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
|
||||
WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
|
||||
MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
|
||||
ANY SPECIAL , DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
|
||||
WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
|
||||
ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
|
||||
OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
|
||||
*/
|
||||
//==============================================================================
|
||||
|
||||
#include <test/jtx.h>
|
||||
#include <test/jtx/CaptureLogs.h>
|
||||
#include <test/jtx/Env.h>
|
||||
|
||||
#include <xrpld/app/misc/HashRouter.h>
|
||||
|
||||
namespace ripple {
|
||||
namespace test {
|
||||
|
||||
class NetworkOPs_test : public beast::unit_test::suite
|
||||
{
|
||||
public:
|
||||
void
|
||||
run() override
|
||||
{
|
||||
testAllBadHeldTransactions();
|
||||
}
|
||||
|
||||
void
|
||||
testAllBadHeldTransactions()
|
||||
{
|
||||
// All trasactions are already marked as SF_BAD, and we should be able
|
||||
// to handle the case properly without an assertion failure
|
||||
testcase("No valid transactions in batch");
|
||||
|
||||
std::string logs;
|
||||
|
||||
{
|
||||
using namespace jtx;
|
||||
auto const alice = Account{"alice"};
|
||||
Env env{
|
||||
*this,
|
||||
envconfig(),
|
||||
std::make_unique<CaptureLogs>(&logs),
|
||||
beast::severities::kAll};
|
||||
env.memoize(env.master);
|
||||
env.memoize(alice);
|
||||
|
||||
auto const jtx = env.jt(ticket::create(alice, 1), seq(1), fee(10));
|
||||
|
||||
auto transacionId = jtx.stx->getTransactionID();
|
||||
env.app().getHashRouter().setFlags(
|
||||
transacionId, HashRouterFlags::HELD);
|
||||
|
||||
env(jtx, json(jss::Sequence, 1), ter(terNO_ACCOUNT));
|
||||
|
||||
env.app().getHashRouter().setFlags(
|
||||
transacionId, HashRouterFlags::BAD);
|
||||
|
||||
env.close();
|
||||
}
|
||||
|
||||
BEAST_EXPECT(
|
||||
logs.find("No transaction to process!") != std::string::npos);
|
||||
}
|
||||
};
|
||||
|
||||
BEAST_DEFINE_TESTSUITE(NetworkOPs, app, ripple);
|
||||
|
||||
} // namespace test
|
||||
} // namespace ripple
|
||||
@@ -1452,6 +1452,11 @@ NetworkOPsImp::processTransactionSet(CanonicalTXSet const& set)
|
||||
for (auto& t : transactions)
|
||||
mTransactions.push_back(std::move(t));
|
||||
}
|
||||
if (mTransactions.empty())
|
||||
{
|
||||
JLOG(m_journal.debug()) << "No transaction to process!";
|
||||
return;
|
||||
}
|
||||
|
||||
doTransactionSyncBatch(lock, [&](std::unique_lock<std::mutex> const&) {
|
||||
XRPL_ASSERT(
|
||||
|
||||
@@ -2880,6 +2880,9 @@ PeerImp::checkTransaction(
|
||||
(stx->getFieldU32(sfLastLedgerSequence) <
|
||||
app_.getLedgerMaster().getValidLedgerIndex()))
|
||||
{
|
||||
JLOG(p_journal_.info())
|
||||
<< "Marking transaction " << stx->getTransactionID()
|
||||
<< "as BAD because it's expired";
|
||||
app_.getHashRouter().setFlags(
|
||||
stx->getTransactionID(), HashRouterFlags::BAD);
|
||||
charge(Resource::feeUselessData, "expired tx");
|
||||
@@ -2936,7 +2939,7 @@ PeerImp::checkTransaction(
|
||||
{
|
||||
if (!validReason.empty())
|
||||
{
|
||||
JLOG(p_journal_.trace())
|
||||
JLOG(p_journal_.debug())
|
||||
<< "Exception checking transaction: " << validReason;
|
||||
}
|
||||
|
||||
@@ -2963,7 +2966,7 @@ PeerImp::checkTransaction(
|
||||
{
|
||||
if (!reason.empty())
|
||||
{
|
||||
JLOG(p_journal_.trace())
|
||||
JLOG(p_journal_.debug())
|
||||
<< "Exception checking transaction: " << reason;
|
||||
}
|
||||
app_.getHashRouter().setFlags(
|
||||
|
||||
Reference in New Issue
Block a user