Skip to content

Commit

Permalink
WIP
Browse files Browse the repository at this point in the history
  • Loading branch information
@SirTyson
SirTyson committed Jul 10, 2024
1 parent 23ef614 commit 51379b0
Show file tree
Hide file tree
Showing 5 changed files with 107 additions and 42 deletions.
14 changes: 9 additions & 5 deletions src/test/FuzzerImpl.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -918,17 +918,21 @@ class FuzzTransactionFrame : public TransactionFrame
// reset results of operations
mTxResult = createSuccessResultWithFeeCharged(ltx.getHeader(), 0, true);

auto header = ltx.loadHeader();

// attempt application of transaction without processing the fee or
// committing the LedgerTxn
SignatureChecker signatureChecker{
ltx.loadHeader().current().ledgerVersion, getContentsHash(),
mEnvelope.v1().signatures};
SignatureChecker signatureChecker{header.current().ledgerVersion,
getContentsHash(),
mEnvelope.v1().signatures};
// if any ill-formed Operations, do not attempt transaction application
auto isInvalidOperation = [&](auto const& op, auto& opResult) {
auto ltxe = op->loadSourceAccount(ltx, header);
auto sourceAccount = LtxReadOnlyResult::create(std::move(ltxe));
return !op->checkValid(
&app.getLedgerManager().getSorobanNetworkConfig(),
app.getConfig(), signatureChecker, ltx, false, opResult,
mTxResult->getSorobanData());
app.getConfig(), signatureChecker, sourceAccount,
header.current(), false, opResult, mTxResult->getSorobanData());
};

auto const& ops = getOperations();
Expand Down
67 changes: 41 additions & 26 deletions src/transactions/OperationFrame.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,9 @@ namespace stellar
using namespace std;

static int32_t
getNeededThreshold(LedgerTxnEntry const& account, ThresholdLevel const level)
getNeededThreshold(LedgerEntry const& account, ThresholdLevel const level)
{
auto const& acc = account.current().data.account();
auto const& acc = account.data.account();
switch (level)
{
case ThresholdLevel::LOW:
Expand Down Expand Up @@ -149,8 +149,16 @@ OperationFrame::apply(Application& app, SignatureChecker& signatureChecker,
sorobanCfg = &app.getLedgerManager().getSorobanNetworkConfig();
}

bool applyRes = checkValid(sorobanCfg, app.getConfig(), signatureChecker,
ltx, true, res, sorobanData);
bool applyRes;
{
LedgerTxn cvLtx(ltx);
auto header = cvLtx.loadHeader();
auto sourceAccount =
LtxReadOnlyResult::create(loadSourceAccount(cvLtx, header));
applyRes =
checkValid(sorobanCfg, app.getConfig(), signatureChecker,
sourceAccount, header.current(), true, res, sorobanData);
}
if (applyRes)
{
if (isSoroban())
Expand Down Expand Up @@ -194,24 +202,12 @@ OperationFrame::isOpSupported(LedgerHeader const&) const

bool
OperationFrame::checkSignature(SignatureChecker& signatureChecker,
AbstractLedgerTxn& ltx, OperationResult& res,
ReadOnlyResultPtr sourceAccount,
LedgerHeader const& header, OperationResult& res,
bool forApply) const
{
ZoneScoped;
auto header = ltx.loadHeader();
auto sourceAccount = loadSourceAccount(ltx, header);
if (sourceAccount)
{
auto neededThreshold =
getNeededThreshold(sourceAccount, getThresholdLevel());
if (!mParentTx.checkSignature(signatureChecker, sourceAccount.current(),
neededThreshold))
{
res.code(opBAD_AUTH);
return false;
}
}
else
if (sourceAccount->isDead())
{
if (forApply || !mOperation.sourceAccount)
{
Expand All @@ -226,6 +222,17 @@ OperationFrame::checkSignature(SignatureChecker& signatureChecker,
return false;
}
}
else
{
auto neededThreshold =
getNeededThreshold(sourceAccount->entry(), getThresholdLevel());
if (!mParentTx.checkSignature(signatureChecker, sourceAccount->entry(),
neededThreshold))
{
res.code(opBAD_AUTH);
return false;
}
}

return true;
}
Expand All @@ -245,24 +252,24 @@ bool
OperationFrame::checkValid(SorobanNetworkConfig const* const sorobanCfg,
Config const& cfg,
SignatureChecker& signatureChecker,
AbstractLedgerTxn& ltxOuter, bool forApply,
ReadOnlyResultPtr sourceAccount,
LedgerHeader const& header, bool forApply,
OperationResult& res,
std::shared_ptr<SorobanTxData> sorobanData) const
{
ZoneScoped;
// Note: ltx is always rolled back so checkValid never modifies the ledger
LedgerTxn ltx(ltxOuter);
if (!isOpSupported(ltx.loadHeader().current()))
if (!isOpSupported(header))
{
res.code(opNOT_SUPPORTED);
return false;
}

auto ledgerVersion = ltx.loadHeader().current().ledgerVersion;
auto ledgerVersion = header.ledgerVersion;
if (!forApply ||
protocolVersionIsBefore(ledgerVersion, ProtocolVersion::V_10))
{
if (!checkSignature(signatureChecker, ltx, res, forApply))
if (!checkSignature(signatureChecker, sourceAccount, header, res,
forApply))
{
return false;
}
Expand All @@ -271,7 +278,7 @@ OperationFrame::checkValid(SorobanNetworkConfig const* const sorobanCfg,
{
// for ledger versions >= 10 we need to load account here, as for
// previous versions it is done in checkSignature call
if (!loadSourceAccount(ltx, ltx.loadHeader()))
if (sourceAccount->isDead())
{
res.code(opNO_ACCOUNT);
return false;
Expand Down Expand Up @@ -312,6 +319,14 @@ OperationFrame::loadSourceAccount(AbstractLedgerTxn& ltx,
return mParentTx.loadAccount(ltx, header.current(), getSourceID());
}

ReadOnlyResultPtr
OperationFrame::loadSourceAccount(ReadOnlyState& roState,
LedgerHeader const& header) const
{
ZoneScoped;
return mParentTx.loadReadOnlyAccount(roState, header, getSourceID());
}

void
OperationFrame::resetResultSuccess(OperationResult& res) const
{
Expand Down
15 changes: 9 additions & 6 deletions src/transactions/OperationFrame.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,14 +64,16 @@ class OperationFrame
// header flags
virtual bool isOpSupported(LedgerHeader const& header) const;

LedgerTxnEntry loadSourceAccount(AbstractLedgerTxn& ltx,
LedgerTxnHeader const& header) const;

public:
static std::shared_ptr<OperationFrame>
makeHelper(Operation const& op, TransactionFrame const& parentTx,
uint32_t index);

LedgerTxnEntry loadSourceAccount(AbstractLedgerTxn& ltx,
LedgerTxnHeader const& header) const;
ReadOnlyResultPtr loadSourceAccount(ReadOnlyState& roState,
LedgerHeader const& header) const;

OperationFrame(Operation const& op, TransactionFrame const& parentTx);
OperationFrame(OperationFrame const&) = delete;
virtual ~OperationFrame() = default;
Expand All @@ -80,15 +82,16 @@ class OperationFrame
void resetResultSuccess(OperationResult& res) const;

bool checkSignature(SignatureChecker& signatureChecker,
AbstractLedgerTxn& ltx, OperationResult& res,
ReadOnlyResultPtr sourceAccount,
LedgerHeader const& header, OperationResult& res,
bool forApply) const;

AccountID getSourceID() const;

bool checkValid(SorobanNetworkConfig const* const sorobanCfg,
Config const& cfg, SignatureChecker& signatureChecker,
AbstractLedgerTxn& ltxOuter, bool forApply,
OperationResult& res,
ReadOnlyResultPtr sourceAccount, LedgerHeader const& header,
bool forApply, OperationResult& res,
std::shared_ptr<SorobanTxData> sorobanData) const;

bool apply(Application& app, SignatureChecker& signatureChecker,
Expand Down
49 changes: 44 additions & 5 deletions src/transactions/TransactionFrame.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -392,6 +392,27 @@ TransactionFrame::loadAccount(AbstractLedgerTxn& ltx,
}
}

ReadOnlyResultPtr
TransactionFrame::loadReadOnlyAccount(ReadOnlyState& roState,
LedgerHeader const& header,
AccountID const& accountID) 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 = loadAccount(ltx, header, accountID);
return LtxReadOnlyResult::create(std::move(ltxe));
}

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

bool
TransactionFrame::hasDexOperations() const
{
Expand Down Expand Up @@ -1136,11 +1157,17 @@ TransactionFrame::processSignatures(
{
// scope here to avoid potential side effects of loading source accounts
LedgerTxn ltx(ltxOuter);
auto header = ltx.loadHeader();

for (size_t i = 0; i < mOperations.size(); ++i)
{
auto const& op = mOperations[i];
auto& opResult = txResult.getOpResultAt(i);
if (!op->checkSignature(signatureChecker, ltx, opResult, false))

auto sourceAccount =
LtxReadOnlyResult::create(op->loadSourceAccount(ltx, header));
if (!op->checkSignature(signatureChecker, sourceAccount,
header.current(), opResult, false))
{
allOpsValid = false;
}
Expand Down Expand Up @@ -1444,15 +1471,27 @@ TransactionFrame::checkValidWithOptionallyChargedFee(
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];
auto& opResult = txResult->getOpResultAt(i);

if (!op->checkValid(sorobanCfg, cfg, signatureChecker, ltx, false,
opResult, txResult->getSorobanData()))
bool cv;
if (getSourceID() == op->getSourceID())
{
cv = op->checkValid(sorobanCfg, cfg, signatureChecker,
sourceAccount, header, false, opResult,
txResult->getSorobanData());
}
else
{
auto opSourceAccount = op->loadSourceAccount(roState, header);
cv = op->checkValid(sorobanCfg, cfg, signatureChecker,
opSourceAccount, header, false, opResult,
txResult->getSorobanData());
}

if (!cv)
{
// it's OK to just fast fail here and not try to call
// checkValid on all operations as the resulting object
Expand Down
4 changes: 4 additions & 0 deletions src/transactions/TransactionFrame.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,10 @@ class TransactionFrame : public TransactionFrameBase
LedgerHeader const& header,
AccountID const& accountID) const;

ReadOnlyResultPtr loadReadOnlyAccount(ReadOnlyState& roState,
LedgerHeader const& header,
AccountID const& accountID) const;

std::optional<SequenceNumber const> const getMinSeqNum() const override;
Duration getMinSeqAge() const override;
uint32 getMinSeqLedgerGap() const override;
Expand Down

0 comments on commit 51379b0

Please sign in to comment.