Skip to content

Commit

Permalink
Refactored checkValid with ReadOnlyState
Browse files Browse the repository at this point in the history
  • Loading branch information
SirTyson committed Jul 10, 2024
1 parent 7623494 commit 23ef614
Show file tree
Hide file tree
Showing 5 changed files with 248 additions and 28 deletions.
92 changes: 92 additions & 0 deletions src/ledger/ReadOnlyState.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
// Copyright 2024 Stellar Development Foundation and contributors. Licensed
// under the Apache License, Version 2.0. See the COPYING file at the root
// of this distribution or at http://www.apache.org/licenses/LICENSE-2.0

#include "ledger/ReadOnlyState.h"
#include "bucket/BucketListSnapshot.h"
#include "ledger/LedgerTxn.h"
#include "util/GlobalChecks.h"

namespace stellar
{

LtxReadOnlyResult::LtxReadOnlyResult(LedgerTxnEntry&& entry)
: mEntry(std::move(entry))
{
}

ReadOnlyResultPtr
LtxReadOnlyResult::create(LedgerTxnEntry&& entry)
{
return ReadOnlyResultPtr(new LtxReadOnlyResult(std::move(entry)));
}

LedgerEntry const&
LtxReadOnlyResult::entry() const
{
return mEntry.current();
}

bool
LtxReadOnlyResult::isDead() const
{
return !static_cast<bool>(mEntry);
}

BucketListReadOnlyResult::BucketListReadOnlyResult(
std::shared_ptr<LedgerEntry> entry)
: mEntry(entry)
{
}

ReadOnlyResultPtr
BucketListReadOnlyResult::create(std::shared_ptr<LedgerEntry> entry)
{
return ReadOnlyResultPtr(new BucketListReadOnlyResult(entry));
}

LedgerEntry const&
BucketListReadOnlyResult::entry() const
{
releaseAssertOrThrow(mEntry);
return *mEntry;
}

bool
BucketListReadOnlyResult::isDead() const
{
return !static_cast<bool>(mEntry);
}

BucketListReadOnlyState::BucketListReadOnlyState(
SearchableBucketListSnapshot& snapshot)
: mSnapshot(snapshot)
{
}

ReadOnlyResultPtr
BucketListReadOnlyState::loadEntry(LedgerKey const& key)
{
auto result = mSnapshot.getLedgerEntry(key);
return BucketListReadOnlyResult::create(result);
}

LtxReadOnlyState::LtxReadOnlyState(AbstractLedgerTxn& ltx) : mLtx(ltx)
{
releaseAssert(threadIsMain());
}

ReadOnlyResultPtr
LtxReadOnlyState::loadEntry(LedgerKey const& key)
{
releaseAssert(threadIsMain());
auto result = mLtx.load(key);
return LtxReadOnlyResult::create(std::move(result));
}

AbstractLedgerTxn&
LtxReadOnlyState::getLedgerTxn()
{
return mLtx;
}
}
88 changes: 88 additions & 0 deletions src/ledger/ReadOnlyState.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
#pragma once

// Copyright 2024 Stellar Development Foundation and contributors. Licensed
// under the Apache License, Version 2.0. See the COPYING file at the root
// of this distribution or at http://www.apache.org/licenses/LICENSE-2.0

#include "ledger/LedgerTxnEntry.h"
#include "util/NonCopyable.h"
#include "util/types.h"

#include <memory>

namespace stellar
{

class AbstractLedgerTxn;
class SearchableBucketListSnapshot;
class ReadOnlyResult;
class ReadOnlyState;

typedef std::shared_ptr<ReadOnlyResult> ReadOnlyResultPtr;

// ReadOnlyResult is a generic wrapper for the result of loading a LedgerEntry
// loaded either from a LedgerTxn or from a BucketList snapshot.
class ReadOnlyResult
{
public:
virtual LedgerEntry const& entry() const = 0;
virtual bool isDead() const = 0;
};

class LtxReadOnlyResult : public ReadOnlyResult
{
private:
LedgerTxnEntry mEntry;
LtxReadOnlyResult(LedgerTxnEntry&& entry);

public:
static ReadOnlyResultPtr create(LedgerTxnEntry&& entry);

LedgerEntry const& entry() const override;
bool isDead() const override;
};

class BucketListReadOnlyResult : public ReadOnlyResult
{
private:
std::shared_ptr<LedgerEntry> mEntry;
BucketListReadOnlyResult(std::shared_ptr<LedgerEntry> entry);

public:
static ReadOnlyResultPtr create(std::shared_ptr<LedgerEntry> entry);

LedgerEntry const& entry() const override;
bool isDead() const override;
};

// ReadOnlyState is an interface for loading LedgerEntries from either a
// LedgerTxn object or a SearchableBucketListSnapshot
class ReadOnlyState
{
public:
virtual ReadOnlyResultPtr loadEntry(LedgerKey const& key) = 0;
};

class BucketListReadOnlyState : public ReadOnlyState
{
private:
SearchableBucketListSnapshot& mSnapshot;

public:
BucketListReadOnlyState(SearchableBucketListSnapshot& snapshot);

ReadOnlyResultPtr loadEntry(LedgerKey const& key) override;
};

class LtxReadOnlyState : public ReadOnlyState
{
private:
AbstractLedgerTxn& mLtx;

public:
LtxReadOnlyState(AbstractLedgerTxn& ledgerTxn);

ReadOnlyResultPtr loadEntry(LedgerKey const& key) override;
AbstractLedgerTxn& getLedgerTxn();
};
}
5 changes: 3 additions & 2 deletions src/transactions/FeeBumpTransactionFrame.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -191,10 +191,11 @@ FeeBumpTransactionFrame::checkValid(Application& app,
auto header = ltx.loadHeader().current();
auto sorobanCfg = app.getLedgerManager().maybeGetSorobanNetworkConfigPtr(
header.ledgerVersion);
LtxReadOnlyState roState(ltx);

auto innerTxResult = mInnerTx->checkValidWithOptionallyChargedFee(
ltx, cfg, sorobanCfg, header, current, false, lowerBoundCloseTimeOffset,
upperBoundCloseTimeOffset);
roState, cfg, sorobanCfg, header, current, false,
lowerBoundCloseTimeOffset, upperBoundCloseTimeOffset);
auto finalTxResult = createSuccessResultWithNewInnerTx(
std::move(txResult), std::move(innerTxResult), mInnerTx);

Expand Down
79 changes: 56 additions & 23 deletions src/transactions/TransactionFrame.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -318,6 +318,26 @@ TransactionFrame::checkExtraSigners(SignatureChecker& signatureChecker) const
return true;
}

ReadOnlyResultPtr
TransactionFrame::loadReadOnlySourceAccount(ReadOnlyState& roState,
LedgerHeader const& header) const
{
ZoneScoped;

// While this function should be truly read-only, we need to preserve a
// buggy cache from older versions of the protocol, resulting in this messy
// cast and potential write via loadSourceAccount.
if (protocolVersionIsBefore(header.ledgerVersion, ProtocolVersion::V_8))
{
releaseAssert(threadIsMain());
auto& ltx = static_cast<LtxReadOnlyState&>(roState).getLedgerTxn();
auto ltxe = loadSourceAccount(ltx, header);
return LtxReadOnlyResult::create(std::move(ltxe));
}

return roState.loadEntry(accountKey(getSourceID()));
}

LedgerTxnEntry
TransactionFrame::loadSourceAccount(AbstractLedgerTxn& ltx,
LedgerHeader const& header) const
Expand Down Expand Up @@ -867,7 +887,7 @@ TransactionFrame::isTooEarlyForAccount(LedgerHeader const& header,

bool
TransactionFrame::commonValidPreSeqNum(
AbstractLedgerTxn& ltx, Config const& cfg,
ReadOnlyResultPtr sourceAccount, Config const& cfg,
SorobanNetworkConfig const* const sorobanCfg, LedgerHeader const& header,
bool chargeFee, uint64_t lowerBoundCloseTimeOffset,
uint64_t upperBoundCloseTimeOffset,
Expand Down Expand Up @@ -1051,7 +1071,7 @@ TransactionFrame::commonValidPreSeqNum(
return false;
}

if (!loadSourceAccount(ltx, header))
if (sourceAccount->isDead())
{
txResult->setInnermostResultCode(txNO_ACCOUNT);
return false;
Expand Down Expand Up @@ -1173,15 +1193,14 @@ TransactionFrame::ValidationType
TransactionFrame::commonValid(
Config const& cfg, SorobanNetworkConfig const* const sorobanCfg,
LedgerHeader const& header, SignatureChecker& signatureChecker,
AbstractLedgerTxn& ltxOuter, SequenceNumber current, bool applying,
ReadOnlyResultPtr sourceAccount, SequenceNumber current, bool applying,
bool chargeFee, uint64_t lowerBoundCloseTimeOffset,
uint64_t upperBoundCloseTimeOffset,
std::optional<FeePair> sorobanResourceFee,
MutableTxResultPtr txResult) const
{
ZoneScoped;
releaseAssertOrThrow(txResult);
LedgerTxn ltx(ltxOuter);
ValidationType res = ValidationType::kInvalid;

if (applying &&
Expand All @@ -1191,14 +1210,15 @@ TransactionFrame::commonValid(
"Applying transaction with non-current closeTime");
}

if (!commonValidPreSeqNum(
ltx, cfg, sorobanCfg, header, chargeFee, lowerBoundCloseTimeOffset,
upperBoundCloseTimeOffset, sorobanResourceFee, txResult))
if (!commonValidPreSeqNum(sourceAccount, cfg, sorobanCfg, header, chargeFee,
lowerBoundCloseTimeOffset,
upperBoundCloseTimeOffset, sorobanResourceFee,
txResult))
{
return res;
}

auto sourceAccount = loadSourceAccount(ltx, header);
releaseAssertOrThrow(!sourceAccount->isDead());

// in older versions, the account's sequence number is updated when taking
// fees
Expand All @@ -1208,7 +1228,7 @@ TransactionFrame::commonValid(
{
if (current == 0)
{
current = sourceAccount.current().data.account().seqNum;
current = sourceAccount->entry().data.account().seqNum;
}
if (isBadSeq(header, current))
{
Expand All @@ -1219,16 +1239,16 @@ TransactionFrame::commonValid(

res = ValidationType::kInvalidUpdateSeqNum;

if (isTooEarlyForAccount(header, sourceAccount.current(),
if (isTooEarlyForAccount(header, sourceAccount->entry(),
lowerBoundCloseTimeOffset))
{
txResult->setInnermostResultCode(txBAD_MIN_SEQ_AGE_OR_GAP);
return res;
}

if (!checkSignature(
signatureChecker, sourceAccount.current(),
sourceAccount.current().data.account().thresholds[THRESHOLD_LOW]))
signatureChecker, sourceAccount->entry(),
sourceAccount->entry().data.account().thresholds[THRESHOLD_LOW]))
{
txResult->setInnermostResultCode(txBAD_AUTH);
return res;
Expand All @@ -1255,7 +1275,7 @@ TransactionFrame::commonValid(
// don't let the account go below the reserve after accounting for
// liabilities
if (chargeFee &&
getAvailableBalance(header, sourceAccount.current()) < feeToPay)
getAvailableBalance(header, sourceAccount->entry()) < feeToPay)
{
txResult->setInnermostResultCode(txINSUFFICIENT_BALANCE);
return res;
Expand Down Expand Up @@ -1379,7 +1399,7 @@ TransactionFrame::removeAccountSigner(AbstractLedgerTxn& ltxOuter,

MutableTxResultPtr
TransactionFrame::checkValidWithOptionallyChargedFee(
AbstractLedgerTxn& ltxOuter, Config const& cfg,
ReadOnlyState& roState, Config const& cfg,
SorobanNetworkConfig const* const sorobanCfg, LedgerHeader const& header,
SequenceNumber current, bool chargeFee, uint64_t lowerBoundCloseTimeOffset,
uint64_t upperBoundCloseTimeOffset) const
Expand All @@ -1394,7 +1414,6 @@ TransactionFrame::checkValidWithOptionallyChargedFee(
return txResult;
}

LedgerTxn ltx(ltxOuter);
int64_t minBaseFee = header.baseFee;
if (!chargeFee)
{
Expand All @@ -1416,12 +1435,17 @@ TransactionFrame::checkValidWithOptionallyChargedFee(
sorobanResourceFee = computePreApplySorobanResourceFee(
header.ledgerVersion, *sorobanCfg, cfg);
}
bool res = commonValid(cfg, sorobanCfg, header, signatureChecker, ltx,
current, false, chargeFee, lowerBoundCloseTimeOffset,
upperBoundCloseTimeOffset, sorobanResourceFee,
txResult) == ValidationType::kMaybeValid;

auto sourceAccount = loadReadOnlySourceAccount(roState, header);
bool res =
commonValid(cfg, sorobanCfg, header, signatureChecker, sourceAccount,
current, false, chargeFee, lowerBoundCloseTimeOffset,
upperBoundCloseTimeOffset, sorobanResourceFee,
txResult) == ValidationType::kMaybeValid;
if (res)
{
// TODO: Pipe sourceAccount directly to op
auto& ltx = static_cast<LtxReadOnlyState&>(roState).getLedgerTxn();
for (size_t i = 0; i < mOperations.size(); ++i)
{
auto const& op = mOperations[i];
Expand Down Expand Up @@ -1457,9 +1481,10 @@ TransactionFrame::checkValid(Application& app, AbstractLedgerTxn& ltxOuter,
auto& header = ltxOuter.loadHeader().current();
auto sorobanCfg = app.getLedgerManager().maybeGetSorobanNetworkConfigPtr(
header.ledgerVersion);
LtxReadOnlyState roState(ltxOuter);

return checkValidWithOptionallyChargedFee(
ltxOuter, app.getConfig(), sorobanCfg, header, current, true,
roState, app.getConfig(), sorobanCfg, header, current, true,
lowerBoundCloseTimeOffset, upperBoundCloseTimeOffset);
}

Expand Down Expand Up @@ -1751,10 +1776,18 @@ TransactionFrame::apply(Application& app, AbstractLedgerTxn& ltx,
declaredSorobanResourceFee() -
sorobanResourceFee->non_refundable_fee);
}

LedgerTxn ltxTx(ltx);
auto cv = commonValid(app.getConfig(), sorobanCfg, header,
signatureChecker, ltxTx, 0, true, chargeFee, 0, 0,
sorobanResourceFee, txResult);
TransactionFrame::ValidationType cv;
{
LedgerTxn cvLtx(ltxTx);
auto sourceAccount =
LtxReadOnlyResult::create(loadSourceAccount(cvLtx, header));

cv = commonValid(app.getConfig(), sorobanCfg, header,
signatureChecker, sourceAccount, 0, true,
chargeFee, 0, 0, sorobanResourceFee, txResult);
}
if (cv >= ValidationType::kInvalidUpdateSeqNum)
{
processSeqNum(ltxTx);
Expand Down
Loading

0 comments on commit 23ef614

Please sign in to comment.