Skip to content

Commit 8d8266f

Browse files
authored
Refactor TxSet validation logic. (#4627)
# Description Refactor TxSet validation logic. Also added the validation logic for the parallel Soroban phase, which has motivated the refactoring in the first place. The general idea is to move the phase-specific validation into phase frames. Also improved the test coverage for the TxSet validation: - Fixed the test for XDR structure validation - the whole 'valid' section has never been executed. - Added a test for Soroban resource validation - Added more coverage for parallel tx set phase validation # Checklist - [ ] Reviewed the [contributing](https://github.com/stellar/stellar-core/blob/master/CONTRIBUTING.md#submitting-changes) document - [ ] Rebased on top of master (no merge commits) - [ ] Ran `clang-format` v8.0.0 (via `make format` or the Visual Studio extension) - [ ] Compiles - [ ] 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)
2 parents c888dfb + fd97e01 commit 8d8266f

10 files changed

+1524
-708
lines changed

src/herder/TxSetFrame.cpp

+452-267
Large diffs are not rendered by default.

src/herder/TxSetFrame.h

+28-16
Original file line numberDiff line numberDiff line change
@@ -196,18 +196,18 @@ class TxSetXDRFrame : public NonMovableOrCopyable
196196
// - The whole phase (`TxStageFrameList`) consists of several sequential
197197
// 'stages' (`TxStageFrame`). A stage has to be executed after every
198198
// transaction in the previous stage has been applied.
199-
// - A 'stage' (`TxStageFrame`) consists of several parallel 'threads'
200-
// (`TxThreadFrame`). Transactions in different 'threads' are independent of
199+
// - A 'stage' (`TxStageFrame`) consists of several independent 'clusters'
200+
// (`TxClusterFrame`). Transactions in different 'clusters' are independent of
201201
// each other and can be applied in parallel.
202-
// - A 'thread' (`TxThreadFrame`) consists of transactions that should
202+
// - A 'cluster' (`TxClusterFrame`) consists of transactions that should
203203
// generally be applied sequentially. However, not all the transactions in
204-
// the thread are necessarily conflicting with each other; it is possible
205-
// that some, or even all transactions in the thread structure can be applied
204+
// the cluster are necessarily conflicting with each other; it is possible
205+
// that some, or even all transactions in the cluster structure can be applied
206206
// in parallel with each other (depending on their footprints).
207207
//
208208
// This structure mimics the XDR structure of the `ParallelTxsComponent`.
209-
using TxThreadFrame = TxFrameList;
210-
using TxStageFrame = std::vector<TxThreadFrame>;
209+
using TxClusterFrame = TxFrameList;
210+
using TxStageFrame = std::vector<TxClusterFrame>;
211211
using TxStageFrameList = std::vector<TxStageFrame>;
212212

213213
// Alias for the map from transaction to its inclusion fee as defined by the
@@ -276,19 +276,23 @@ class TxSetPhaseFrame
276276
Iterator(TxStageFrameList const& txs, size_t stageIndex);
277277
TxStageFrameList const& mStages;
278278
size_t mStageIndex = 0;
279-
size_t mThreadIndex = 0;
279+
size_t mClusterIndex = 0;
280280
size_t mTxIndex = 0;
281281
};
282282
Iterator begin() const;
283283
Iterator end() const;
284-
size_t size() const;
284+
size_t sizeTx() const;
285+
size_t sizeOp() const;
286+
size_t size(LedgerHeader const& lclHeader) const;
285287
bool empty() const;
286288

287289
// Get _inclusion_ fee map for this phase. The map contains lowest base
288290
// fee for each transaction (lowest base fee is identical for all
289291
// transactions in the same lane)
290292
InclusionFeeMap const& getInclusionFeeMap() const;
291293

294+
std::optional<Resource> getTotalResources() const;
295+
292296
private:
293297
friend class TxSetXDRFrame;
294298
friend class ApplicableTxSetFrame;
@@ -312,16 +316,16 @@ class TxSetPhaseFrame
312316
TxFrameList& invalidTxs,
313317
bool enforceTxsApplyOrder);
314318
#endif
315-
316-
TxSetPhaseFrame(TxFrameList const& txs,
319+
TxSetPhaseFrame(TxSetPhase phase, TxFrameList const& txs,
317320
std::shared_ptr<InclusionFeeMap> inclusionFeeMap);
318-
TxSetPhaseFrame(TxStageFrameList&& txs,
321+
TxSetPhaseFrame(TxSetPhase phase, TxStageFrameList&& txs,
319322
std::shared_ptr<InclusionFeeMap> inclusionFeeMap);
320323

321324
// Creates a new phase from `TransactionPhase` XDR coming from a
322325
// `GeneralizedTransactionSet`.
323326
static std::optional<TxSetPhaseFrame>
324-
makeFromWire(Hash const& networkID, TransactionPhase const& xdrPhase);
327+
makeFromWire(TxSetPhase phase, Hash const& networkID,
328+
TransactionPhase const& xdrPhase);
325329

326330
// Creates a new phase from all the transactions in the legacy
327331
// `TransactionSet` XDR.
@@ -330,10 +334,20 @@ class TxSetPhaseFrame
330334
xdr::xvector<TransactionEnvelope> const& xdrTxs);
331335

332336
// Creates a valid empty phase with given `isParallel` flag.
333-
static TxSetPhaseFrame makeEmpty(bool isParallel);
337+
static TxSetPhaseFrame makeEmpty(TxSetPhase phase, bool isParallel);
334338

335339
// Returns a copy of this phase with transactions sorted for apply.
336340
TxSetPhaseFrame sortedForApply(Hash const& txSetHash) const;
341+
bool checkValid(Application& app, uint64_t lowerBoundCloseTimeOffset,
342+
uint64_t upperBoundCloseTimeOffset) const;
343+
bool checkValidClassic(LedgerHeader const& lclHeader) const;
344+
bool checkValidSoroban(LedgerHeader const& lclHeader,
345+
SorobanNetworkConfig const& sorobanConfig) const;
346+
347+
bool txsAreValid(Application& app, uint64_t lowerBoundCloseTimeOffset,
348+
uint64_t upperBoundCloseTimeOffset) const;
349+
350+
TxSetPhase mPhase;
337351

338352
TxStageFrameList mStages;
339353
std::shared_ptr<InclusionFeeMap> mInclusionFeeMap;
@@ -471,8 +485,6 @@ class ApplicableTxSetFrame
471485
ApplicableTxSetFrame(ApplicableTxSetFrame const&) = default;
472486
ApplicableTxSetFrame(ApplicableTxSetFrame&&) = default;
473487

474-
std::optional<Resource> getTxSetSorobanResource() const;
475-
476488
void toXDR(TransactionSet& set) const;
477489
void toXDR(GeneralizedTransactionSet& generalizedTxSet) const;
478490

src/herder/test/TestTxSetUtils.cpp

+65-4
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ makeTxSetXDR(std::vector<TransactionFrameBasePtr> const& txs,
3131
}
3232

3333
GeneralizedTransactionSet
34-
makeGeneralizedTxSetXDR(std::vector<ComponentPhases> const& phases,
34+
makeGeneralizedTxSetXDR(std::vector<PhaseComponents> const& phases,
3535
Hash const& previousLedgerHash,
3636
bool useParallelSorobanPhase)
3737
{
@@ -76,11 +76,11 @@ makeGeneralizedTxSetXDR(std::vector<ComponentPhases> const& phases,
7676
}
7777
if (!txs.empty())
7878
{
79-
auto& thread =
79+
auto& cluster =
8080
component.executionStages.emplace_back().emplace_back();
8181
for (auto const& tx : txs)
8282
{
83-
thread.emplace_back(tx->getEnvelope());
83+
cluster.emplace_back(tx->getEnvelope());
8484
}
8585
}
8686
#else
@@ -120,7 +120,7 @@ makeNonValidatedTxSet(std::vector<TransactionFrameBasePtr> const& txs,
120120

121121
std::pair<TxSetXDRFrameConstPtr, ApplicableTxSetFrameConstPtr>
122122
makeNonValidatedGeneralizedTxSet(
123-
std::vector<ComponentPhases> const& txsPerBaseFee, Application& app,
123+
std::vector<PhaseComponents> const& txsPerBaseFee, Application& app,
124124
Hash const& previousLedgerHash, std::optional<bool> useParallelSorobanPhase)
125125
{
126126
if (!useParallelSorobanPhase.has_value())
@@ -157,5 +157,66 @@ makeNonValidatedTxSetBasedOnLedgerVersion(
157157
}
158158
}
159159

160+
#ifdef ENABLE_NEXT_PROTOCOL_VERSION_UNSAFE_FOR_PRODUCTION
161+
void
162+
normalizeParallelPhaseXDR(TransactionPhase& phase)
163+
{
164+
auto compareTxHash = [](TransactionEnvelope const& tx1,
165+
TransactionEnvelope const& tx2) -> bool {
166+
return xdrSha256(tx1) < xdrSha256(tx2);
167+
};
168+
for (auto& stage : phase.parallelTxsComponent().executionStages)
169+
{
170+
for (auto& cluster : stage)
171+
{
172+
std::sort(cluster.begin(), cluster.end(), compareTxHash);
173+
}
174+
std::sort(stage.begin(), stage.end(),
175+
[&](auto const& c1, auto const& c2) {
176+
return compareTxHash(c1.front(), c2.front());
177+
});
178+
}
179+
std::sort(phase.parallelTxsComponent().executionStages.begin(),
180+
phase.parallelTxsComponent().executionStages.end(),
181+
[&](auto const& s1, auto const& s2) {
182+
return compareTxHash(s1.front().front(), s2.front().front());
183+
});
184+
}
185+
186+
std::pair<TxSetXDRFrameConstPtr, ApplicableTxSetFrameConstPtr>
187+
makeNonValidatedGeneralizedTxSet(PhaseComponents const& classicTxsPerBaseFee,
188+
std::optional<int64_t> sorobanBaseFee,
189+
TxStageFrameList const& sorobanTxsPerStage,
190+
Application& app,
191+
Hash const& previousLedgerHash)
192+
{
193+
auto xdrTxSet = makeGeneralizedTxSetXDR({classicTxsPerBaseFee},
194+
previousLedgerHash, false);
195+
xdrTxSet.v1TxSet().phases.emplace_back(1);
196+
auto& phase = xdrTxSet.v1TxSet().phases.back();
197+
if (sorobanBaseFee)
198+
{
199+
phase.parallelTxsComponent().baseFee.activate() = *sorobanBaseFee;
200+
}
201+
202+
auto& stages = phase.parallelTxsComponent().executionStages;
203+
for (auto const& stage : sorobanTxsPerStage)
204+
{
205+
auto& xdrStage = stages.emplace_back();
206+
for (auto const& cluster : stage)
207+
{
208+
auto& xdrCluster = xdrStage.emplace_back();
209+
for (auto const& tx : cluster)
210+
{
211+
xdrCluster.emplace_back(tx->getEnvelope());
212+
}
213+
}
214+
}
215+
normalizeParallelPhaseXDR(phase);
216+
auto txSet = TxSetXDRFrame::makeFromWire(xdrTxSet);
217+
return std::make_pair(txSet, txSet->prepareForApply(app));
218+
}
219+
#endif
220+
160221
} // namespace testtxset
161222
} // namespace stellar

src/herder/test/TestTxSetUtils.h

+12-2
Original file line numberDiff line numberDiff line change
@@ -12,17 +12,27 @@ namespace stellar
1212
namespace testtxset
1313
{
1414

15-
using ComponentPhases = std::vector<
15+
using PhaseComponents = std::vector<
1616
std::pair<std::optional<int64_t>, std::vector<TransactionFrameBasePtr>>>;
1717
std::pair<TxSetXDRFrameConstPtr, ApplicableTxSetFrameConstPtr>
1818
makeNonValidatedGeneralizedTxSet(
19-
std::vector<ComponentPhases> const& txsPerBaseFee, Application& app,
19+
std::vector<PhaseComponents> const& txsPerBaseFee, Application& app,
2020
Hash const& previousLedgerHash,
2121
std::optional<bool> useParallelSorobanPhase = std::nullopt);
2222

2323
std::pair<TxSetXDRFrameConstPtr, ApplicableTxSetFrameConstPtr>
2424
makeNonValidatedTxSetBasedOnLedgerVersion(
2525
std::vector<TransactionFrameBasePtr> const& txs, Application& app,
2626
Hash const& previousLedgerHash);
27+
#ifdef ENABLE_NEXT_PROTOCOL_VERSION_UNSAFE_FOR_PRODUCTION
28+
void normalizeParallelPhaseXDR(TransactionPhase& phase);
29+
30+
std::pair<TxSetXDRFrameConstPtr, ApplicableTxSetFrameConstPtr>
31+
makeNonValidatedGeneralizedTxSet(PhaseComponents const& classicTxsPerBaseFee,
32+
std::optional<int64_t> sorobanBaseFee,
33+
TxStageFrameList const& sorobanTxsPerStage,
34+
Application& app,
35+
Hash const& previousLedgerHash);
36+
#endif
2737
} // namespace testtxset
2838
} // namespace stellar

0 commit comments

Comments
 (0)