Skip to content

Commit

Permalink
Improvements to transaction queue and tests
Browse files Browse the repository at this point in the history
  • Loading branch information
marta-lokhova committed Oct 31, 2023
1 parent 488cbe1 commit 1866e49
Show file tree
Hide file tree
Showing 7 changed files with 506 additions and 379 deletions.
7 changes: 1 addition & 6 deletions src/herder/SurgePricingUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -349,12 +349,7 @@ SurgePricingPriorityQueue::canFitWithEviction(
auto txNewResources = mLaneConfig->getTxResources(tx);
if (txDiscount)
{
if (anyLessThan(txNewResources, *txDiscount))
{
throw std::invalid_argument("Discount shouldn't be larger than "
"transaction operations count");
}
txNewResources -= *txDiscount;
txNewResources = subtractNonNegative(txNewResources, *txDiscount);
}

if (anyGreater(txNewResources, mLaneLimits[GENERIC_LANE]) ||
Expand Down
9 changes: 4 additions & 5 deletions src/herder/TransactionQueue.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -245,8 +245,8 @@ TransactionQueue::canAdd(TransactionFrameBasePtr tx,
mApp.getConfig());
}

int64_t newInclusionFee = tx->getInclusionFee();
if (newInclusionFee < 0)
int64_t newFullFee = tx->getFullFee();
if (tx->getInclusionFee() < 0 || newFullFee < 0)
{
return TransactionQueue::AddResult::ADD_STATUS_ERROR;
}
Expand Down Expand Up @@ -312,10 +312,9 @@ TransactionQueue::canAdd(TransactionFrameBasePtr tx,
}

oldTx = txToReplaceIter->mTx;
int64_t oldInclusionFee = oldTx->getInclusionFee();
if (oldTx->getFeeSourceID() == tx->getFeeSourceID())
{
newInclusionFee -= oldInclusionFee;
newFullFee -= oldTx->getFullFee();
}
}

Expand Down Expand Up @@ -397,7 +396,7 @@ TransactionQueue::canAdd(TransactionFrameBasePtr tx,
int64_t totalFees = feeStateIter == mAccountStates.end()
? 0
: feeStateIter->second.mTotalFees;
if (getAvailableBalance(ltx.loadHeader(), feeSource) - newInclusionFee <
if (getAvailableBalance(ltx.loadHeader(), feeSource) - newFullFee <
totalFees)
{
tx->getResult().result.code(txINSUFFICIENT_BALANCE);
Expand Down
2 changes: 1 addition & 1 deletion src/herder/TransactionQueue.h
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,7 @@ class TransactionQueue

void shutdown();
bool sourceAccountPending(AccountID const& accountID) const;
virtual size_t getMaxQueueSizeOps() const = 0;

protected:
/**
Expand Down Expand Up @@ -201,7 +202,6 @@ class TransactionQueue
getMaxResourcesToFloodThisPeriod() const = 0;
virtual bool broadcastSome() = 0;
virtual int getFloodPeriod() const = 0;
virtual size_t getMaxQueueSizeOps() const = 0;

void broadcast(bool fromCallback);
// broadcasts a single transaction
Expand Down
16 changes: 5 additions & 11 deletions src/herder/TxQueueLimiter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@ TxQueueLimiter::~TxQueueLimiter()
size_t
TxQueueLimiter::size() const
{
releaseAssert(!mIsSoroban);
return mTxs->totalResources().getVal(Resource::Type::OPERATIONS);
}
#endif
Expand Down Expand Up @@ -171,24 +170,19 @@ TxQueueLimiter::canAddTx(TransactionFrameBasePtr const& newTx,
(newTx->getFullFee() - newTx->getInclusionFee()));
}

uint32_t txOpsDiscount = 0;
// For eviction purposes, treat old tx resources as a "discount", since it
// will be replaced by the new transaction
std::optional<Resource> oldTxDiscount = std::nullopt;
if (oldTx)
{
auto newTxOps = newTx->getNumOperations();
auto oldTxOps = oldTx->getNumOperations();
// Bump transactions must have at least the old amount of operations.
releaseAssert(oldTxOps <= newTxOps);
txOpsDiscount = newTxOps - oldTxOps;
oldTxDiscount = oldTx->getResources(false);
}

auto discount = newTx->isSoroban()
? std::nullopt
: std::make_optional<Resource>(txOpsDiscount);
// Update the operation limit in case upgrade happened. This is cheap
// enough to happen unconditionally without relying on upgrade triggers.
mSurgePricingLaneConfig->updateGenericLaneLimit(
Resource(maxScaledLedgerResources(newTx->isSoroban(), ltxOuter)));
return mTxs->canFitWithEviction(*newTx, discount, txsToEvict);
return mTxs->canFitWithEviction(*newTx, oldTxDiscount, txsToEvict);
}

void
Expand Down
Loading

5 comments on commit 1866e49

@latobarita
Copy link
Contributor

Choose a reason for hiding this comment

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

saw approval from dmkozh
at marta-lokhova@1866e49

@latobarita
Copy link
Contributor

Choose a reason for hiding this comment

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

merging marta-lokhova/stellar-core/improvements_and_tests = 1866e49 into auto

@latobarita
Copy link
Contributor

Choose a reason for hiding this comment

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

marta-lokhova/stellar-core/improvements_and_tests = 1866e49 merged ok, testing candidate = 9f8182a

@latobarita
Copy link
Contributor

Choose a reason for hiding this comment

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

fast-forwarding master to auto = 9f8182a

Please sign in to comment.