Skip to content

Commit d18af91

Browse files
committed
Legacy tx sets: avoid using read-only LCL meant for main thread only
1 parent 69cb5cb commit d18af91

6 files changed

+41
-16
lines changed

src/herder/HerderSCPDriver.cpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -709,7 +709,7 @@ HerderSCPDriver::combineCandidates(uint64_t slotIndex,
709709
auto cTxSet = mPendingEnvelopes.getTxSet(sv.txSetHash);
710710
releaseAssert(cTxSet);
711711
// Only valid applicable tx sets should be combined.
712-
auto cApplicableTxSet = cTxSet->prepareForApply(mApp);
712+
auto cApplicableTxSet = cTxSet->prepareForApply(mApp, lcl.header);
713713
releaseAssert(cApplicableTxSet);
714714
if (cTxSet->previousLedgerHash() == lcl.hash)
715715
{
@@ -1246,7 +1246,7 @@ HerderSCPDriver::checkAndCacheTxSetValid(TxSetXDRFrame const& txSet,
12461246
ApplicableTxSetFrameConstPtr applicableTxSet;
12471247
if (txSet.previousLedgerHash() == lcl.hash)
12481248
{
1249-
applicableTxSet = txSet.prepareForApply(mApp);
1249+
applicableTxSet = txSet.prepareForApply(mApp, lcl.header);
12501250
}
12511251

12521252
bool res = true;

src/herder/TxSetFrame.cpp

+4-4
Original file line numberDiff line numberDiff line change
@@ -783,7 +783,7 @@ makeTxSetFromTransactions(PerPhaseTransactionList const& txPhases,
783783
#endif
784784

785785
ApplicableTxSetFrameConstPtr outputApplicableTxSet =
786-
outputTxSet->prepareForApply(app);
786+
outputTxSet->prepareForApply(app, lclHeader.header);
787787

788788
if (!outputApplicableTxSet)
789789
{
@@ -934,7 +934,8 @@ TxSetXDRFrame::toStellarMessage() const
934934
#endif
935935

936936
ApplicableTxSetFrameConstPtr
937-
TxSetXDRFrame::prepareForApply(Application& app) const
937+
TxSetXDRFrame::prepareForApply(Application& app,
938+
LedgerHeader const& currHeader) const
938939
{
939940
#ifdef BUILD_TESTS
940941
if (mApplicableTxSetOverride)
@@ -972,8 +973,7 @@ TxSetXDRFrame::prepareForApply(Application& app) const
972973
{
973974
auto const& xdrTxSet = std::get<TransactionSet>(mXDRTxSet);
974975
auto maybePhase = TxSetPhaseFrame::makeFromWireLegacy(
975-
app.getLedgerManager().getLastClosedLedgerHeader().header,
976-
app.getNetworkID(), xdrTxSet.txs);
976+
currHeader, app.getNetworkID(), xdrTxSet.txs);
977977
if (!maybePhase)
978978
{
979979
return nullptr;

src/herder/TxSetFrame.h

+2-1
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,8 @@ class TxSetXDRFrame : public NonMovableOrCopyable
143143
// This may *only* be called when LCL hash matches the `previousLedgerHash`
144144
// of this `TxSetFrame` - tx sets with a wrong ledger hash shouldn't even
145145
// be attempted to be interpreted.
146-
ApplicableTxSetFrameConstPtr prepareForApply(Application& app) const;
146+
ApplicableTxSetFrameConstPtr
147+
prepareForApply(Application& app, LedgerHeader const& currHeader) const;
147148

148149
bool isGeneralizedTxSet() const;
149150

src/herder/test/TestTxSetUtils.cpp

+6-2
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,8 @@ makeNonValidatedTxSet(std::vector<TransactionFrameBasePtr> const& txs,
113113
{
114114
auto xdrTxSet = makeTxSetXDR(txs, previousLedgerHash);
115115
auto txSet = TxSetXDRFrame::makeFromWire(xdrTxSet);
116-
auto applicableTxSet = txSet->prepareForApply(app);
116+
auto applicableTxSet = txSet->prepareForApply(
117+
app, app.getLedgerManager().getLastClosedLedgerHeader().header);
117118
return std::make_pair(txSet, std::move(applicableTxSet));
118119
}
119120
} // namespace
@@ -135,7 +136,10 @@ makeNonValidatedGeneralizedTxSet(
135136
auto xdrTxSet = makeGeneralizedTxSetXDR(txsPerBaseFee, previousLedgerHash,
136137
*useParallelSorobanPhase);
137138
auto txSet = TxSetXDRFrame::makeFromWire(xdrTxSet);
138-
return std::make_pair(txSet, txSet->prepareForApply(app));
139+
return std::make_pair(
140+
txSet,
141+
txSet->prepareForApply(
142+
app, app.getLedgerManager().getLastClosedLedgerHeader().header));
139143
}
140144

141145
std::pair<TxSetXDRFrameConstPtr, ApplicableTxSetFrameConstPtr>

src/herder/test/TxSetTests.cpp

+26-6
Original file line numberDiff line numberDiff line change
@@ -41,21 +41,33 @@ TEST_CASE("generalized tx set XDR validation", "[txset]")
4141
SECTION("no phases")
4242
{
4343
auto txSet = TxSetXDRFrame::makeFromWire(xdrTxSet);
44-
REQUIRE(txSet->prepareForApply(*app) == nullptr);
44+
REQUIRE(
45+
txSet->prepareForApply(
46+
*app,
47+
app->getLedgerManager().getLastClosedLedgerHeader().header) ==
48+
nullptr);
4549
}
4650
SECTION("one phase")
4751
{
4852
auto txSet = TxSetXDRFrame::makeFromWire(xdrTxSet);
4953
xdrTxSet.v1TxSet().phases.emplace_back();
50-
REQUIRE(txSet->prepareForApply(*app) == nullptr);
54+
REQUIRE(
55+
txSet->prepareForApply(
56+
*app,
57+
app->getLedgerManager().getLastClosedLedgerHeader().header) ==
58+
nullptr);
5159
}
5260
SECTION("too many phases")
5361
{
5462
xdrTxSet.v1TxSet().phases.emplace_back();
5563
xdrTxSet.v1TxSet().phases.emplace_back();
5664
xdrTxSet.v1TxSet().phases.emplace_back();
5765
auto txSet = TxSetXDRFrame::makeFromWire(xdrTxSet);
58-
REQUIRE(txSet->prepareForApply(*app) == nullptr);
66+
REQUIRE(
67+
txSet->prepareForApply(
68+
*app,
69+
app->getLedgerManager().getLastClosedLedgerHeader().header) ==
70+
nullptr);
5971
}
6072

6173
SECTION("two phase scenarios")
@@ -442,11 +454,17 @@ TEST_CASE("generalized tx set XDR validation", "[txset]")
442454
bool valid = classicIsValid && sorobanIsValid;
443455
if (valid)
444456
{
445-
REQUIRE(txSet->prepareForApply(*app) != nullptr);
457+
REQUIRE(txSet->prepareForApply(
458+
*app, app->getLedgerManager()
459+
.getLastClosedLedgerHeader()
460+
.header) != nullptr);
446461
}
447462
else
448463
{
449-
REQUIRE(txSet->prepareForApply(*app) == nullptr);
464+
REQUIRE(txSet->prepareForApply(
465+
*app, app->getLedgerManager()
466+
.getLastClosedLedgerHeader()
467+
.header) == nullptr);
450468
}
451469
}
452470
}
@@ -502,7 +520,9 @@ testGeneralizedTxSetXDRConversion(ProtocolVersion protocolVersion)
502520
auto checkXdrRoundtrip = [&](GeneralizedTransactionSet const& txSetXdr) {
503521
auto txSetFrame = TxSetXDRFrame::makeFromWire(txSetXdr);
504522
ApplicableTxSetFrameConstPtr applicableFrame =
505-
txSetFrame->prepareForApply(*app);
523+
txSetFrame->prepareForApply(
524+
*app,
525+
app->getLedgerManager().getLastClosedLedgerHeader().header);
506526
REQUIRE(applicableFrame->checkValid(*app, 0, 0));
507527
GeneralizedTransactionSet newXdr;
508528
applicableFrame->toWireTxSetFrame()->toXDR(newXdr);

src/ledger/LedgerManagerImpl.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -877,7 +877,7 @@ LedgerManagerImpl::applyLedger(LedgerCloseData const& ledgerData,
877877
header.current().scpValue = sv;
878878

879879
maybeResetLedgerCloseMetaDebugStream(header.current().ledgerSeq);
880-
auto applicableTxSet = txSet->prepareForApply(mApp);
880+
auto applicableTxSet = txSet->prepareForApply(mApp, prevHeader);
881881

882882
if (applicableTxSet == nullptr)
883883
{

0 commit comments

Comments
 (0)