Skip to content

Skip building invariant delta when no invariants are enabled. #5289

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
dmkozh wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Skip building invariant delta when no invariants are enabled. #5289

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 13 additions & 10 deletions src/ledger/LedgerManagerImpl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2500,7 +2500,6 @@ LedgerManagerImpl::checkAllTxBundleInvariants(
{
for (auto const& txBundle : stage)
{
// First check the invariants
if (txBundle.getResPayload().isSuccess())
{
try
Expand All @@ -2513,7 +2512,7 @@ LedgerManagerImpl::checkAllTxBundleInvariants(
app.checkOnOperationApply(
txBundle.getTx()->getRawOperations().at(0),
txBundle.getResPayload().getOpResultAt(0),
txBundle.getEffects().getDelta(),
txBundle.getEffects().getDeltaForInvariants(),
txBundle.getEffects()
.getMeta()
.getOperationMetaBuilderAt(0)
Expand All @@ -2526,13 +2525,6 @@ LedgerManagerImpl::checkAllTxBundleInvariants(
"Invariant failure while applying operations: ", e.what());
}
}

// We don't call processPostApply for post v23 transactions at the
// moment because processPostApply is currently a no-op for those
// transactions.

txBundle.getEffects().getMeta().maybeSetRefundableFeeMeta(
txBundle.getResPayload().getRefundableFeeTracker());
Comment on lines -2529 to -2535
Copy link
Copy Markdown
Contributor

@graydon graydon May 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand why this call moved to the other loop

Copy link
Copy Markdown
Contributor Author

@dmkozh dmkozh May 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybeSetRefundableFeeMeta was incorrectly placed into checkAllTxBundleInvariants which makes no sense logically. I just cleaned up the code and separated invariant checks and refundable fee meta population. The logic remains the same otherwise (it's the same loop over all txs in stage).

}
}

Expand All @@ -2549,7 +2541,18 @@ LedgerManagerImpl::applySorobanStage(
auto threadStates = applySorobanStageClustersInParallel(
app, stage, globalParState, sorobanBasePrngSeed, config, ledgerInfo);

checkAllTxBundleInvariants(app, stage, config, ledgerInfo, header);
if (config.invariantsEnabled())
{
checkAllTxBundleInvariants(app, stage, config, ledgerInfo, header);
}
// We don't call processPostApply for post v23 transactions at the
// moment because processPostApply is currently a no-op for those
// transactions, so just set refundable fee meta here.
for (auto const& txBundle : stage)
{
Comment thread
dmkozh marked this conversation as resolved.
txBundle.getEffects().getMeta().maybeSetRefundableFeeMeta(
txBundle.getResPayload().getRefundableFeeTracker());
}

globalParState.commitChangesFromThreads(app, threadStates, stage);
}
Expand Down
6 changes: 6 additions & 0 deletions src/main/Config.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2704,6 +2704,12 @@ Config::toString(SCPQuorumSet const& qset)
return fw.write(json);
}

bool
Config::invariantsEnabled() const
{
return !INVARIANT_CHECKS.empty();
}

size_t
Config::getSorobanByteAllowance() const
{
Expand Down
2 changes: 2 additions & 0 deletions src/main/Config.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -956,6 +956,8 @@ class Config : public std::enable_shared_from_this<Config>
// function to stringify a quorum set
std::string toString(SCPQuorumSet const& qset);

bool invariantsEnabled() const;

// A special name to be used for stdin in stead of a file name in command
// line arguments.
static std::string const STDIN_SPECIAL_NAME;
Expand Down
16 changes: 9 additions & 7 deletions src/transactions/ParallelApplyStage.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,29 +30,31 @@ class TxEffects
{
return mMeta;
}

LedgerTxnDelta const&
getDelta()
getDeltaForInvariants()
{
return mDelta;
return mDeltaForInvariants;
}

void
setDeltaEntry(LedgerKey const& key, LedgerTxnDelta::EntryDelta const& delta)
setDeltaEntryForInvariants(LedgerKey const& key,
LedgerTxnDelta::EntryDelta const& delta)
{
auto [_, inserted] = mDelta.entry.emplace(key, delta);
auto [_, inserted] = mDeltaForInvariants.entry.emplace(key, delta);
releaseAssertOrThrow(inserted);
}

void
setDeltaHeader(LedgerHeader const& header)
{
mDelta.header.current = header;
mDelta.header.previous = header;
mDeltaForInvariants.header.current = header;
mDeltaForInvariants.header.previous = header;
}

private:
TransactionMetaBuilder mMeta;
LedgerTxnDelta mDelta;
LedgerTxnDelta mDeltaForInvariants;
};

// TxBundle contains a transaction, its associated result payload, and its
Expand Down
7 changes: 3 additions & 4 deletions src/transactions/ParallelApplyUtils.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -787,9 +787,8 @@ ThreadParallelApplyLedgerState::commitChangeFromSuccessfulTx(
}

void
ThreadParallelApplyLedgerState::setEffectsDeltaFromSuccessfulTx(
ParallelTxSuccessVal const& res, ParallelLedgerInfo const& ledgerInfo,
TxEffects& effects) const
ThreadParallelApplyLedgerState::setDeltaForInvariantsFromSuccessfulTx(
ParallelTxSuccessVal const& res, TxEffects& effects) const
{
ZoneScoped;
for (auto const& [lk, scopedEntryOpt] : res.getModifiedEntryMap())
Expand Down Expand Up @@ -824,7 +823,7 @@ ThreadParallelApplyLedgerState::setEffectsDeltaFromSuccessfulTx(
std::make_shared<InternalLedgerEntry>(entryOpt.value());
}
releaseAssertOrThrow(entryDelta.current || entryDelta.previous);
effects.setDeltaEntry(lk, entryDelta);
effects.setDeltaEntryForInvariants(lk, entryDelta);
}
}

Expand Down
5 changes: 2 additions & 3 deletions src/transactions/ParallelApplyUtils.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,9 +161,8 @@ class ThreadParallelApplyLedgerState
OptionalEntryT getLiveEntryOpt(LedgerKey const& key) const;
bool entryWasRestored(LedgerKey const& key) const;

void setEffectsDeltaFromSuccessfulTx(ParallelTxSuccessVal const& res,
ParallelLedgerInfo const& ledgerInfo,
TxEffects& effects) const;
void setDeltaForInvariantsFromSuccessfulTx(ParallelTxSuccessVal const& res,
TxEffects& effects) const;

void commitChangesFromSuccessfulTx(ParallelTxSuccessVal const& res,
TxBundle const& txBundle);
Expand Down
7 changes: 5 additions & 2 deletions src/transactions/TransactionFrame.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2277,8 +2277,11 @@ TransactionFrame::parallelApply(

if (res)
{
threadState.setEffectsDeltaFromSuccessfulTx(*res, ledgerInfo,
effects);
if (config.invariantsEnabled())
{
threadState.setDeltaForInvariantsFromSuccessfulTx(*res,
effects);
}
opMeta.setLedgerChangesFromSuccessfulOp(threadState, *res,
ledgerInfo.getLedgerSeq());
}
Expand Down
Loading