Skip to content

Commit 79f2b5f

Browse files
marta-lokhovamarta-lokhova
authoredAug 7, 2024
Fixed assert failure when banning txs (#4422)
# Description Resolves #4421 Fixes an assert failure that occurred when a TX that exceeds ledger limits is banned. # Checklist - [x] Reviewed the [contributing](https://github.com/stellar/stellar-core/blob/master/CONTRIBUTING.md#submitting-changes) document - [x] Rebased on top of master (no merge commits) - [x] Ran `clang-format` v8.0.0 (via `make format` or the Visual Studio extension) - [x] Compiles - [x] Ran all tests - [ ] If change impacts performance, include supporting evidence per the [performance document](https://github.com/stellar/stellar-core/blob/master/performance-eval/performance-eval.md)
·
v22.2.0v21.3.0rc1
2 parents 3089d1c + caf860a commit 79f2b5f

File tree

5 files changed

+33
-31
lines changed

5 files changed

+33
-31
lines changed
 

‎src/herder/TransactionQueue.cpp

100644100755
+4-5
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ TransactionQueue::AddResult::AddResult(AddResultCode addCode,
7070
: code(addCode), txResult(tx->createSuccessResult())
7171
{
7272
releaseAssert(txErrorCode != txSUCCESS);
73-
txResult.value()->setResultCode(txErrorCode);
73+
txResult->setResultCode(txErrorCode);
7474
}
7575

7676
TransactionQueue::TransactionQueue(Application& app, uint32 pendingDepth,
@@ -358,7 +358,7 @@ TransactionQueue::canAdd(
358358
AddResult result(
359359
TransactionQueue::AddResultCode::ADD_STATUS_ERROR, tx,
360360
txINSUFFICIENT_FEE);
361-
result.txResult.value()->getResult().feeCharged = minFee;
361+
result.txResult->getResult().feeCharged = minFee;
362362
return result;
363363
}
364364

@@ -386,12 +386,11 @@ TransactionQueue::canAdd(
386386
{
387387
AddResult result(TransactionQueue::AddResultCode::ADD_STATUS_ERROR,
388388
tx, txINSUFFICIENT_FEE);
389-
result.txResult.value()->getResult().feeCharged = canAddRes.second;
389+
result.txResult->getResult().feeCharged = canAddRes.second;
390390
return result;
391391
}
392392
return AddResult(
393-
TransactionQueue::AddResultCode::ADD_STATUS_TRY_AGAIN_LATER,
394-
nullptr);
393+
TransactionQueue::AddResultCode::ADD_STATUS_TRY_AGAIN_LATER);
395394
}
396395

397396
auto closeTime = mApp.getLedgerManager()

‎src/herder/TransactionQueue.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ class TransactionQueue
7575
struct AddResult
7676
{
7777
TransactionQueue::AddResultCode code;
78-
std::optional<MutableTxResultPtr> txResult;
78+
MutableTxResultPtr txResult;
7979

8080
// AddResult with no txResult
8181
explicit AddResult(TransactionQueue::AddResultCode addCode);

‎src/herder/test/TransactionQueueTests.cpp

+23-19
Original file line numberDiff line numberDiff line change
@@ -798,10 +798,15 @@ TEST_CASE("TransactionQueue hitting the rate limit",
798798
auto tx = transaction(*app, account5, 1, 1, 300 * 3, 3);
799799
auto addResult = testQueue.add(
800800
tx, TransactionQueue::AddResultCode::ADD_STATUS_ERROR);
801-
REQUIRE(addResult.txResult.value()->getResultCode() ==
802-
txINSUFFICIENT_FEE);
803-
REQUIRE(addResult.txResult.value()->getResult().feeCharged ==
804-
300 * 3 + 1);
801+
REQUIRE(addResult.txResult->getResultCode() == txINSUFFICIENT_FEE);
802+
REQUIRE(addResult.txResult->getResult().feeCharged == 300 * 3 + 1);
803+
}
804+
SECTION("cannot add - tx outside of limits")
805+
{
806+
auto tx = transaction(*app, account5, 1, 1, 100 * 1'000, 100);
807+
auto addResult = testQueue.add(
808+
tx, TransactionQueue::AddResultCode::ADD_STATUS_TRY_AGAIN_LATER);
809+
REQUIRE(!addResult.txResult);
805810
}
806811
SECTION("add high fee tx with eviction")
807812
{
@@ -817,9 +822,8 @@ TEST_CASE("TransactionQueue hitting the rate limit",
817822
auto nextTx = transaction(*app, account6, 1, 1, 300, 1);
818823
auto addResult = testQueue.add(
819824
nextTx, TransactionQueue::AddResultCode::ADD_STATUS_ERROR);
820-
REQUIRE(addResult.txResult.value()->getResultCode() ==
821-
txINSUFFICIENT_FEE);
822-
REQUIRE(addResult.txResult.value()->getResult().feeCharged == 301);
825+
REQUIRE(addResult.txResult->getResultCode() == txINSUFFICIENT_FEE);
826+
REQUIRE(addResult.txResult->getResult().feeCharged == 301);
823827
}
824828
SECTION("then add tx with higher fee than evicted")
825829
{
@@ -1239,7 +1243,7 @@ TEST_CASE("Soroban TransactionQueue limits",
12391243
REQUIRE(addResult.code ==
12401244
TransactionQueue::AddResultCode::ADD_STATUS_ERROR);
12411245
REQUIRE(addResult.txResult);
1242-
REQUIRE(addResult.txResult.value()->getResultCode() == txMALFORMED);
1246+
REQUIRE(addResult.txResult->getResultCode() == txMALFORMED);
12431247
}
12441248
SECTION("source account limit, soroban and classic tx queue")
12451249
{
@@ -1351,9 +1355,9 @@ TEST_CASE("Soroban TransactionQueue limits",
13511355
REQUIRE(!app->getHerder().isBannedTx(tx->getFullHash()));
13521356

13531357
REQUIRE(addResult.txResult);
1354-
REQUIRE(addResult.txResult.value()->getResultCode() ==
1358+
REQUIRE(addResult.txResult->getResultCode() ==
13551359
TransactionResultCode::txINSUFFICIENT_FEE);
1356-
REQUIRE(addResult.txResult.value()->getResult().feeCharged ==
1360+
REQUIRE(addResult.txResult->getResult().feeCharged ==
13571361
expectedFeeCharged);
13581362
}
13591363
SECTION("insufficient account balance")
@@ -1368,7 +1372,7 @@ TEST_CASE("Soroban TransactionQueue limits",
13681372
TransactionQueue::AddResultCode::ADD_STATUS_ERROR);
13691373
REQUIRE(!app->getHerder().isBannedTx(tx->getFullHash()));
13701374
REQUIRE(addResult.txResult);
1371-
REQUIRE(addResult.txResult.value()->getResultCode() ==
1375+
REQUIRE(addResult.txResult->getResultCode() ==
13721376
TransactionResultCode::txINSUFFICIENT_BALANCE);
13731377
}
13741378
SECTION("invalid resources")
@@ -1397,7 +1401,7 @@ TEST_CASE("Soroban TransactionQueue limits",
13971401
TransactionQueue::AddResultCode::ADD_STATUS_ERROR);
13981402
REQUIRE(!app->getHerder().isBannedTx(tx->getFullHash()));
13991403
REQUIRE(addResult.txResult);
1400-
REQUIRE(addResult.txResult.value()->getResultCode() ==
1404+
REQUIRE(addResult.txResult->getResultCode() ==
14011405
TransactionResultCode::txSOROBAN_INVALID);
14021406
}
14031407
SECTION("too many ops")
@@ -1411,7 +1415,7 @@ TEST_CASE("Soroban TransactionQueue limits",
14111415
TransactionQueue::AddResultCode::ADD_STATUS_ERROR);
14121416
REQUIRE(!app->getHerder().isBannedTx(tx->getFullHash()));
14131417
REQUIRE(addResult.txResult);
1414-
REQUIRE(addResult.txResult.value()->getResultCode() ==
1418+
REQUIRE(addResult.txResult->getResultCode() ==
14151419
TransactionResultCode::txMALFORMED);
14161420
}
14171421
}
@@ -2314,7 +2318,7 @@ TEST_CASE("transaction queue with fee-bump", "[herder][transactionqueue]")
23142318
1, isSoroban);
23152319
auto addResult = test.add(
23162320
tx2, TransactionQueue::AddResultCode::ADD_STATUS_ERROR);
2317-
REQUIRE(addResult.txResult.value()->getResultCode() ==
2321+
REQUIRE(addResult.txResult->getResultCode() ==
23182322
txINSUFFICIENT_BALANCE);
23192323
test.check(
23202324
{{{account1, 0, {fb1}}, {account2}, {account3, 0}}, {}});
@@ -2326,7 +2330,7 @@ TEST_CASE("transaction queue with fee-bump", "[herder][transactionqueue]")
23262330
auto fb2 = feeBump(*app, account3, tx2, newInclusionToPay);
23272331
auto addResult = test.add(
23282332
fb2, TransactionQueue::AddResultCode::ADD_STATUS_ERROR);
2329-
REQUIRE(addResult.txResult.value()->getResultCode() ==
2333+
REQUIRE(addResult.txResult->getResultCode() ==
23302334
txINSUFFICIENT_BALANCE);
23312335
test.check(
23322336
{{{account1, 0, {fb1}}, {account2}, {account3, 0}}, {}});
@@ -2339,7 +2343,7 @@ TEST_CASE("transaction queue with fee-bump", "[herder][transactionqueue]")
23392343
REQUIRE(account3.getAvailableBalance() >= fb2->getFullFee());
23402344
auto addResult = test.add(
23412345
fb2, TransactionQueue::AddResultCode::ADD_STATUS_ERROR);
2342-
REQUIRE(addResult.txResult.value()->getResultCode() ==
2346+
REQUIRE(addResult.txResult->getResultCode() ==
23432347
txINSUFFICIENT_BALANCE);
23442348
test.check(
23452349
{{{account1, 0, {fb1}}, {account2}, {account3, 0}}, {}});
@@ -2383,7 +2387,7 @@ TEST_CASE("transaction queue with fee-bump", "[herder][transactionqueue]")
23832387
auto addResult = test.add(
23842388
fb2, TransactionQueue::AddResultCode::ADD_STATUS_ERROR);
23852389

2386-
REQUIRE(addResult.txResult.value()->getResultCode() ==
2390+
REQUIRE(addResult.txResult->getResultCode() ==
23872391
txINSUFFICIENT_BALANCE);
23882392
test.check(
23892393
{{{account1, 0, {fb1}}, {account2}, {account3, 0}},
@@ -2492,7 +2496,7 @@ TEST_CASE("replace by fee", "[herder][transactionqueue]")
24922496
auto fb = feeBump(*app, feeSource, tx, 399);
24932497
auto addResult = test.add(
24942498
fb, TransactionQueue::AddResultCode::ADD_STATUS_ERROR);
2495-
auto& txResult = addResult.txResult.value();
2499+
auto& txResult = addResult.txResult;
24962500
REQUIRE(txResult->getResultCode() == txINSUFFICIENT_FEE);
24972501
REQUIRE(txResult->getResult().feeCharged ==
24982502
4000 + (tx->getFullFee() - tx->getInclusionFee()));
@@ -2509,7 +2513,7 @@ TEST_CASE("replace by fee", "[herder][transactionqueue]")
25092513
auto fb = feeBump(*app, feeSource, tx, 3999);
25102514
auto addResult = test.add(
25112515
fb, TransactionQueue::AddResultCode::ADD_STATUS_ERROR);
2512-
auto& txResult = addResult.txResult.value();
2516+
auto& txResult = addResult.txResult;
25132517
REQUIRE(txResult->getResultCode() == txINSUFFICIENT_FEE);
25142518
REQUIRE(txResult->getResult().feeCharged ==
25152519
4000 + (tx->getFullFee() - tx->getInclusionFee()));

‎src/main/CommandHandler.cpp

+3-4
Original file line numberDiff line numberDiff line change
@@ -1026,7 +1026,7 @@ CommandHandler::tx(std::string const& params, std::string& retStr)
10261026
std::string resultBase64;
10271027
releaseAssertOrThrow(addResult.txResult);
10281028

1029-
auto const& payload = addResult.txResult.value();
1029+
auto const& payload = addResult.txResult;
10301030
auto resultBin = xdr::xdr_to_opaque(payload->getResult());
10311031
resultBase64.reserve(decoder::encoded_size64(resultBin.size()) +
10321032
1);
@@ -1550,9 +1550,8 @@ CommandHandler::testTx(std::string const& params, std::string& retStr)
15501550
if (addResult.code == TransactionQueue::AddResultCode::ADD_STATUS_ERROR)
15511551
{
15521552
releaseAssert(addResult.txResult);
1553-
root["detail"] =
1554-
xdrToCerealString(addResult.txResult.value()->getResultCode(),
1555-
"TransactionResultCode");
1553+
root["detail"] = xdrToCerealString(
1554+
addResult.txResult->getResultCode(), "TransactionResultCode");
15561555
}
15571556
}
15581557
else

‎src/simulation/LoadGenerator.cpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -2181,7 +2181,7 @@ LoadGenerator::execute(TransactionTestFramePtr& txf, LoadGenMode mode,
21812181

21822182
auto resultStr =
21832183
addResult.txResult
2184-
? xdrToCerealString(addResult.txResult.value()->getResult(),
2184+
? xdrToCerealString(addResult.txResult->getResult(),
21852185
"TransactionResult")
21862186
: "";
21872187
CLOG_INFO(LoadGen, "tx rejected '{}': ===> {}, {}",
@@ -2193,7 +2193,7 @@ LoadGenerator::execute(TransactionTestFramePtr& txf, LoadGenMode mode,
21932193
if (addResult.code == TransactionQueue::AddResultCode::ADD_STATUS_ERROR)
21942194
{
21952195
releaseAssert(addResult.txResult);
2196-
code = addResult.txResult.value()->getResultCode();
2196+
code = addResult.txResult->getResultCode();
21972197
}
21982198
txm.mTxnRejected.Mark();
21992199
}

0 commit comments

Comments
 (0)