Skip to content

Commit

Permalink
Cleanup
Browse files Browse the repository at this point in the history
  • Loading branch information
@SirTyson
SirTyson committed Jul 1, 2024
1 parent d028ed9 commit a9a97a8
Show file tree
Hide file tree
Showing 13 changed files with 112 additions and 94 deletions.
13 changes: 7 additions & 6 deletions src/herder/TransactionQueue.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ TransactionQueue::AddResult::AddResult(AddResultCode addCode,
TransactionQueue::AddResult::AddResult(AddResultCode addCode,
TransactionFrameBasePtr tx,
TransactionResultCode txErrorCode)
: code(addCode), txResult(tx->createTxResult())
: code(addCode), txResult(tx->createSuccessResult())
{
releaseAssert(txErrorCode != txSUCCESS);
txResult.value()->setResultCode(txErrorCode);
Expand Down Expand Up @@ -324,7 +324,7 @@ TransactionQueue::canAdd(TransactionFrameBasePtr tx,
// appropriate error message
if (tx->isSoroban())
{
auto txResult = tx->createTxResult();
auto txResult = tx->createSuccessResult();
if (!tx->checkSorobanResourceAndSetError(
mApp,
mApp.getLedgerManager()
Expand Down Expand Up @@ -389,8 +389,9 @@ TransactionQueue::canAdd(TransactionFrameBasePtr tx,
result.txResult.value()->getResult().feeCharged = canAddRes.second;
return result;
}
return {TransactionQueue::AddResultCode::ADD_STATUS_TRY_AGAIN_LATER,
nullptr};
return AddResult(
TransactionQueue::AddResultCode::ADD_STATUS_TRY_AGAIN_LATER,
nullptr);
}

auto closeTime = mApp.getLedgerManager()
Expand Down Expand Up @@ -582,8 +583,8 @@ TransactionQueue::tryAdd(TransactionFrameBasePtr tx, bool submittedFromSelf)
// fast fail when Soroban tx is malformed
if ((tx->isSoroban() != (c1 || c2)) || !tx->XDRProvidesValidFee())
{
return {TransactionQueue::AddResultCode::ADD_STATUS_ERROR, tx,
txMALFORMED};
return AddResult(TransactionQueue::AddResultCode::ADD_STATUS_ERROR, tx,
txMALFORMED);
}

AccountStates::iterator stateIter;
Expand Down
12 changes: 6 additions & 6 deletions src/herder/TransactionQueue.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,16 +78,16 @@ class TransactionQueue
std::optional<MutableTxResultPtr> txResult;

// AddResult with no txResult
AddResult(TransactionQueue::AddResultCode addCode);
explicit AddResult(TransactionQueue::AddResultCode addCode);

// AddResult from existing transaction result
AddResult(TransactionQueue::AddResultCode addCode,
MutableTxResultPtr payload);
explicit AddResult(TransactionQueue::AddResultCode addCode,
MutableTxResultPtr payload);

// AddResult with error txResult with the specified txErrorCode
AddResult(TransactionQueue::AddResultCode addCode,
TransactionFrameBasePtr tx,
TransactionResultCode txErrorCode);
explicit AddResult(TransactionQueue::AddResultCode addCode,
TransactionFrameBasePtr tx,
TransactionResultCode txErrorCode);
};

/**
Expand Down
5 changes: 3 additions & 2 deletions src/test/FuzzerImpl.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -901,7 +901,8 @@ class FuzzTransactionFrame : public TransactionFrame
public:
FuzzTransactionFrame(Hash const& networkID,
TransactionEnvelope const& envelope)
: TransactionFrame(networkID, envelope), mTxResult(createTxResult()){};
: TransactionFrame(networkID, envelope)
, mTxResult(createSuccessResult()){};

void
attemptApplication(Application& app, AbstractLedgerTxn& ltx)
Expand All @@ -915,7 +916,7 @@ class FuzzTransactionFrame : public TransactionFrame
}

// reset results of operations
mTxResult = createTxResultWithFeeCharged(ltx.getHeader(), 0, true);
mTxResult = createSuccessResultWithFeeCharged(ltx.getHeader(), 0, true);

// attempt application of transaction without processing the fee or
// committing the LedgerTxn
Expand Down
41 changes: 11 additions & 30 deletions src/transactions/FeeBumpTransactionFrame.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -134,25 +134,6 @@ FeeBumpTransactionFrame::processPostApply(Application& app,
// should also be reflected here.
int64_t refund =
mInnerTx->processRefund(app, ltx, meta, getFeeSourceID(), *txResult);

// The result codes and a feeCharged without the refund are set in
// updateResult in FeeBumpTransactionFrame::apply. At this point, feeCharged
// is set correctly on the inner transaction, so update the feeBump result.
if (protocolVersionStartsFrom(ltx.loadHeader().current().ledgerVersion,
ProtocolVersion::V_21) &&
isSoroban())
{
// First update feeCharged of the inner result on the feeBump using
// mInnerTx
{
auto& irp = txResult->getResult().result.innerResultPair();
auto& innerRes = irp.result;
innerRes.feeCharged = txResult->getInnermostResult().feeCharged;

// Now set the updated feeCharged on the fee bump.
txResult->getResult().feeCharged -= refund;
}
}
}

bool
Expand Down Expand Up @@ -181,15 +162,15 @@ FeeBumpTransactionFrame::checkValid(Application& app,
{
if (!XDRProvidesValidFee())
{
auto txResult = createTxResult();
auto txResult = createSuccessResult();
txResult->setResultCode(txMALFORMED);
return {false, txResult};
}

LedgerTxn ltx(ltxOuter);
int64_t minBaseFee = ltx.loadHeader().current().baseFee;
auto txResult = createTxResultWithFeeCharged(ltx.loadHeader().current(),
minBaseFee, false);
auto txResult = createSuccessResultWithFeeCharged(
ltx.loadHeader().current(), minBaseFee, false);

SignatureChecker signatureChecker{ltx.loadHeader().current().ledgerVersion,
getContentsHash(),
Expand All @@ -209,7 +190,7 @@ FeeBumpTransactionFrame::checkValid(Application& app,
auto [res, innerTxResult] = mInnerTx->checkValidWithOptionallyChargedFee(
app, ltx, current, false, lowerBoundCloseTimeOffset,
upperBoundCloseTimeOffset);
auto finalTxResult = createTxResultWithNewInnerTx(
auto finalTxResult = createSuccessResultWithNewInnerTx(
std::move(txResult), std::move(innerTxResult), mInnerTx);

return {res, finalTxResult};
Expand Down Expand Up @@ -505,8 +486,8 @@ MutableTxResultPtr
FeeBumpTransactionFrame::processFeeSeqNum(AbstractLedgerTxn& ltx,
std::optional<int64_t> baseFee) const
{
auto txResult =
createTxResultWithFeeCharged(ltx.loadHeader().current(), baseFee, true);
auto txResult = createSuccessResultWithFeeCharged(
ltx.loadHeader().current(), baseFee, true);
releaseAssert(txResult);

auto feeSource = stellar::loadAccount(ltx, getFeeSourceID());
Expand Down Expand Up @@ -554,12 +535,12 @@ FeeBumpTransactionFrame::removeOneTimeSignerKeyFromFeeSource(
}

MutableTxResultPtr
FeeBumpTransactionFrame::createTxResultWithFeeCharged(
FeeBumpTransactionFrame::createSuccessResultWithFeeCharged(
LedgerHeader const& header, std::optional<int64_t> baseFee,
bool applying) const
{
auto innerTxResult =
mInnerTx->createTxResultWithFeeCharged(header, baseFee, applying);
mInnerTx->createSuccessResultWithFeeCharged(header, baseFee, applying);

// feeCharged is updated accordingly to represent the cost of the
// transaction regardless of the failure modes.
Expand All @@ -573,14 +554,14 @@ FeeBumpTransactionFrame::createTxResultWithFeeCharged(
}

MutableTxResultPtr
FeeBumpTransactionFrame::createTxResult() const
FeeBumpTransactionFrame::createSuccessResult() const
{
return MutableTxResultPtr(
new FeeBumpMutableTransactionResult(mInnerTx->createTxResult()));
new FeeBumpMutableTransactionResult(mInnerTx->createSuccessResult()));
}

MutableTxResultPtr
FeeBumpTransactionFrame::createTxResultWithNewInnerTx(
FeeBumpTransactionFrame::createSuccessResultWithNewInnerTx(
MutableTxResultPtr&& outerResult, MutableTxResultPtr&& innerResult,
TransactionFrameBasePtr innerTx) const
{
Expand Down
14 changes: 7 additions & 7 deletions src/transactions/FeeBumpTransactionFrame.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,17 +84,17 @@ class FeeBumpTransactionFrame : public TransactionFrameBase
checkSorobanResourceAndSetError(Application& app, uint32_t ledgerVersion,
MutableTxResultPtr txResult) const override;

MutableTxResultPtr createTxResult() const override;
MutableTxResultPtr createSuccessResult() const override;

MutableTxResultPtr
createTxResultWithFeeCharged(LedgerHeader const& header,
std::optional<int64_t> baseFee,
bool applying) const override;
createSuccessResultWithFeeCharged(LedgerHeader const& header,
std::optional<int64_t> baseFee,
bool applying) const override;

MutableTxResultPtr
createTxResultWithNewInnerTx(MutableTxResultPtr&& outerResult,
MutableTxResultPtr&& innerResult,
TransactionFrameBasePtr innerTx) const;
createSuccessResultWithNewInnerTx(MutableTxResultPtr&& outerResult,
MutableTxResultPtr&& innerResult,
TransactionFrameBasePtr innerTx) const;

TransactionEnvelope const& getEnvelope() const override;

Expand Down
30 changes: 30 additions & 0 deletions src/transactions/MutableTransactionResult.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,14 @@ MutableTransactionResult::getDiagnosticEvents() const
}
}

void
MutableTransactionResult::refundSorobanFee(int64_t feeRefund,
uint32_t ledgerVersion)
{
releaseAssertOrThrow(mSorobanExtension);
mTxResult->feeCharged -= feeRefund;
}

FeeBumpMutableTransactionResult::FeeBumpMutableTransactionResult(
MutableTxResultPtr innerTxResult)
: MutableTransactionResultBase(), mInnerTxResult(innerTxResult)
Expand Down Expand Up @@ -402,4 +410,26 @@ FeeBumpMutableTransactionResult::getDiagnosticEvents() const
{
return mInnerTxResult->getDiagnosticEvents();
}

void
FeeBumpMutableTransactionResult::refundSorobanFee(int64_t feeRefund,
uint32_t ledgerVersion)
{
// First update feeCharged of the inner result
mInnerTxResult->refundSorobanFee(feeRefund, ledgerVersion);

// The result codes and a feeCharged without the refund should have been set
// already in updateResult in FeeBumpTransactionFrame::apply. At this point,
// feeCharged is set correctly on the inner transaction, so update the
// feeBump result.
if (protocolVersionStartsFrom(ledgerVersion, ProtocolVersion::V_21))
{
auto& irp = mTxResult->result.innerResultPair();
auto& innerRes = irp.result;
innerRes.feeCharged = mInnerTxResult->getResult().feeCharged;

// Now set the updated feeCharged on the fee bump.
mTxResult->feeCharged -= feeRefund;
}
}
}
25 changes: 19 additions & 6 deletions src/transactions/MutableTransactionResult.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,12 +84,19 @@ class MutableTransactionResultBase : public NonMovableOrCopyable
virtual TransactionResult& getResult() = 0;
virtual TransactionResult const& getResult() const = 0;
virtual TransactionResultCode getResultCode() const = 0;

// Note: changing "code" normally causes the XDR structure to be destructed,
// then a different XDR structure is constructed. However, txFAILED and
// txSUCCESS have the same underlying field number so this does not
// occur when changing between these two codes.
virtual void setResultCode(TransactionResultCode code) = 0;
virtual OperationResult& getOpResultAt(size_t index) = 0;
virtual std::shared_ptr<SorobanTxData> getSorobanData() = 0;

virtual xdr::xvector<DiagnosticEvent> const&
getDiagnosticEvents() const = 0;

virtual void refundSorobanFee(int64_t feeRefund,
uint32_t ledgerVersion) = 0;
};

class MutableTransactionResult : public MutableTransactionResultBase
Expand All @@ -99,9 +106,10 @@ class MutableTransactionResult : public MutableTransactionResultBase

MutableTransactionResult(TransactionFrame const& tx, int64_t feeCharged);

friend MutableTxResultPtr TransactionFrame::createTxResult() const;
friend MutableTxResultPtr TransactionFrame::createSuccessResult() const;

friend MutableTxResultPtr TransactionFrame::createTxResultWithFeeCharged(
friend MutableTxResultPtr
TransactionFrame::createSuccessResultWithFeeCharged(
LedgerHeader const& header, std::optional<int64_t> baseFee,
bool applying) const;

Expand All @@ -118,6 +126,8 @@ class MutableTransactionResult : public MutableTransactionResultBase
OperationResult& getOpResultAt(size_t index) override;
std::shared_ptr<SorobanTxData> getSorobanData() override;
xdr::xvector<DiagnosticEvent> const& getDiagnosticEvents() const override;

void refundSorobanFee(int64_t feeRefund, uint32_t ledgerVersion) override;
};

class FeeBumpMutableTransactionResult : public MutableTransactionResultBase
Expand All @@ -131,15 +141,16 @@ class FeeBumpMutableTransactionResult : public MutableTransactionResultBase
MutableTxResultPtr&& innerTxResult,
TransactionFrameBasePtr innerTx);

friend MutableTxResultPtr FeeBumpTransactionFrame::createTxResult() const;
friend MutableTxResultPtr
FeeBumpTransactionFrame::createSuccessResult() const;

friend MutableTxResultPtr
FeeBumpTransactionFrame::createTxResultWithFeeCharged(
FeeBumpTransactionFrame::createSuccessResultWithFeeCharged(
LedgerHeader const& header, std::optional<int64_t> baseFee,
bool applying) const;

friend MutableTxResultPtr
FeeBumpTransactionFrame::createTxResultWithNewInnerTx(
FeeBumpTransactionFrame::createSuccessResultWithNewInnerTx(
MutableTxResultPtr&& outerResult, MutableTxResultPtr&& innerResult,
TransactionFrameBasePtr innerTx) const;

Expand All @@ -160,5 +171,7 @@ class FeeBumpMutableTransactionResult : public MutableTransactionResultBase
OperationResult& getOpResultAt(size_t index) override;
std::shared_ptr<SorobanTxData> getSorobanData() override;
xdr::xvector<DiagnosticEvent> const& getDiagnosticEvents() const override;

void refundSorobanFee(int64_t feeRefund, uint32_t ledgerVersion) override;
};
}
4 changes: 2 additions & 2 deletions src/transactions/OperationFrame.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -141,9 +141,9 @@ OperationFrame::apply(Application& app, SignatureChecker& signatureChecker,
std::shared_ptr<SorobanTxData> sorobanData) const
{
ZoneScoped;
bool applyRes;
CLOG_TRACE(Tx, "{}", xdrToCerealString(mOperation, "Operation"));
applyRes = checkValid(app, signatureChecker, ltx, true, res, sorobanData);
bool applyRes =
checkValid(app, signatureChecker, ltx, true, res, sorobanData);
if (applyRes)
{
if (isSoroban())
Expand Down
Loading

0 comments on commit a9a97a8

Please sign in to comment.