Skip to content

fix: check credit pool amount accumulation #7816

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

Merged
PastaPastaPasta merged 1 commit into from
Oct 8, 2026
Merged

fix: check credit pool amount accumulation #7816

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
29 changes: 25 additions & 4 deletions src/evo/creditpool.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,14 @@
#include <masternode/payments.h>
#include <node/blockstorage.h>
#include <shutdown.h>
#include <util/overflow.h>
#include <validation.h>

#include <algorithm>
#include <exception>
#include <memory>
#include <stack>
#include <stdexcept>

using node::ReadBlockFromDisk;

Expand All @@ -41,7 +43,11 @@ static bool GetDataFromUnlockTx(const CTransaction& tx, CAmount& toUnlock, uint6
if (!MoneyRange(txout.nValue)) {
return state.Invalid(TxValidationResult::TX_CONSENSUS, "failed-creditpool-unlock-txout-outofrange");
}
toUnlock += txout.nValue;
const auto total = CheckedAdd(toUnlock, txout.nValue);
if (!total) {
return state.Invalid(TxValidationResult::TX_CONSENSUS, "failed-creditpool-unlock-overflow");
}
toUnlock = *total;
}
return true;
}
Expand Down Expand Up @@ -288,6 +294,16 @@ CCreditPoolDiff::CCreditPoolDiff(CCreditPool starter, const CBlockIndex* pindexP
}
}

CAmount CCreditPoolDiff::GetTotalLocked() const
{
// Subtract withdrawals first to preserve a representable net balance.
auto total = CheckedAdd(pool.locked, -sessionUnlocked);
if (total) total = CheckedAdd(*total, sessionLocked);
if (total) total = CheckedAdd(*total, platformReward);
if (!total) throw std::overflow_error("credit-pool balance overflow");
return *total;
}

bool CCreditPoolDiff::Lock(const CTransaction& tx, TxValidationState& state)
{
if (const auto opt_assetLockTx = GetTxPayload<CAssetLockPayload>(tx); !opt_assetLockTx) {
Expand All @@ -297,7 +313,11 @@ bool CCreditPoolDiff::Lock(const CTransaction& tx, TxValidationState& state)
for (const CTxOut& txout : tx.vout) {
if (const CScript& script = txout.scriptPubKey; script.empty() || script[0] != OP_RETURN) continue;

sessionLocked += txout.nValue;
const auto total = CheckedAdd(sessionLocked, txout.nValue);
if (!total) {
return state.Invalid(TxValidationResult::TX_CONSENSUS, "failed-creditpool-lock-overflow");
}
sessionLocked = *total;
return true;
}

Expand All @@ -313,7 +333,8 @@ bool CCreditPoolDiff::Unlock(const CTransaction& tx, TxValidationState& state, s
return false;
}

if (sessionUnlocked + toUnlock > pool.currentLimit) {
const auto total = CheckedAdd(sessionUnlocked, toUnlock);
if (!total || *total > pool.currentLimit) {
return state.Invalid(TxValidationResult::TX_CONSENSUS, "failed-creditpool-unlock-too-much");
}

Expand All @@ -323,7 +344,7 @@ bool CCreditPoolDiff::Unlock(const CTransaction& tx, TxValidationState& state, s

newIndexes.insert(index);
if (inserted_index) *inserted_index = index;
sessionUnlocked += toUnlock;
sessionUnlocked = *total;
return true;
}

Expand Down
5 changes: 2 additions & 3 deletions src/evo/creditpool.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,10 +100,9 @@ class CCreditPoolDiff {

/**
* this function returns total amount of credits for the next block
* Throws if the total cannot be represented as CAmount.
*/
CAmount GetTotalLocked() const {
return pool.locked + sessionLocked - sessionUnlocked + platformReward;
}
CAmount GetTotalLocked() const;

std::string ToString() const {
return strprintf("CCreditPoolDiff(sessionLocked=%lld, sessionUnlocked=%lld, platforomReward=%lld, newIndexes=%lld, pool=%s)", sessionLocked, sessionUnlocked, platformReward, newIndexes.size(), pool.ToString());
Expand Down
35 changes: 35 additions & 0 deletions src/test/evo_assetlocks_tests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@

#include <boost/test/unit_test.hpp>

#include <limits>

// Helpers:
static bool IsStandardTx(const CTransaction& tx, std::string& reason)
{
Expand Down Expand Up @@ -141,6 +143,17 @@ static CTransactionRef CreateCreditPoolUnlockTx(uint64_t index, CAmount amount,
return MakeTransactionRef(std::move(tx));
}

static CTransactionRef CreateCreditPoolLockTx(CAmount amount)
{
CMutableTransaction tx;
tx.nVersion = 3;
tx.nType = TRANSACTION_ASSET_LOCK;
tx.vout.emplace_back(amount, CScript{} << OP_RETURN << OP_0);
SetTxPayload(tx, CAssetLockPayload{std::vector<CTxOut>{{amount, GetScriptForDestination(PKHash{})}},
CAssetLockPayload::INITIAL_VERSION});
return MakeTransactionRef(std::move(tx));
}

BOOST_FIXTURE_TEST_SUITE(evo_assetlocks_tests, TestChain100Setup)

static void CheckAssetLockCommon(uint8_t version, bool is_v24_active)
Expand Down Expand Up @@ -885,6 +898,28 @@ BOOST_AUTO_TEST_CASE(credit_pool_unlock_limit_v24)
BOOST_CHECK_EQUAL(limit(MAX_MONEY, MAX_MONEY), MAX_MONEY * 20 / 100);
}

BOOST_FIXTURE_TEST_CASE(credit_pool_lock_arithmetic, TestChain100Setup)
{
LOCK(cs_main);
const auto* tip = m_node.chainman->ActiveChain().Tip();
CCreditPoolDiff diff{CCreditPool{}, tip, m_node.chainman->GetConsensus(), 0};
const CAmount maximum = std::numeric_limits<CAmount>::max();
const auto lock_max_money = CreateCreditPoolLockTx(MAX_MONEY);
for (CAmount i = 0; i < maximum / MAX_MONEY; ++i) {
TxValidationState state;
BOOST_REQUIRE(diff.ProcessLockUnlockTransaction(*lock_max_money, state));
if (i == 1) BOOST_CHECK_EQUAL(diff.GetTotalLocked(), 2 * MAX_MONEY);
}
TxValidationState remainder_state;
BOOST_REQUIRE(diff.ProcessLockUnlockTransaction(*CreateCreditPoolLockTx(maximum % MAX_MONEY), remainder_state));
BOOST_CHECK_EQUAL(diff.GetTotalLocked(), maximum);

TxValidationState overflow_state;
BOOST_CHECK(!diff.ProcessLockUnlockTransaction(*CreateCreditPoolLockTx(1), overflow_state));
BOOST_CHECK_EQUAL(overflow_state.GetRejectReason(), "failed-creditpool-lock-overflow");
BOOST_CHECK_EQUAL(diff.GetTotalLocked(), maximum);
Comment on lines +901 to +920

@thepastaclaw thepastaclaw Oct 7, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🟡 Suggestion: Cover final-balance overflow and withdrawal-first boundary arithmetic

This test starts with an empty pool and reaches the sessionLocked limit, so the rejected deposit exercises Lock's overflow check but never the new overflow exception in GetTotalLocked. It also does not distinguish withdrawal-first arithmetic from deposit-first arithmetic. Add public-interface scenarios with a starting balance at the CAmount maximum and zero Platform reward: a small deposit should process successfully but make GetTotalLocked throw; an equal deposit and withdrawal within currentLimit should instead return the maximum balance after both transactions, in either transaction order. These directly cover the newly introduced final-balance behavior without requiring a funded-block fixture.

source: gpt-6.1-sol (phase2-reviewer: general)

@PastaPastaPasta PastaPastaPasta Oct 7, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Declining: a pool already at the CAmount maximum is unreachable (pool.locked is bounded by real supply), so testing GetTotalLocked()'s throw from that state would pin the evaluation order of an internal sum rather than observable behavior. The reachable overflow, stacked unfunded deposits, is covered by credit_pool_lock_arithmetic.

🤖 Posted autonomously by Claude on behalf of pasta.

@thepastaclaw thepastaclaw Oct 7, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Still applies (re-reviewed at 3ef05555): You're right that the proposed maximum starting pool is unreachable, and that fixture request was incorrect. A modest nonzero pool still separates the final-balance overflow path from the session overflow covered here; checking the getter's exception and successful withdrawal cancellation would test observable public-interface behavior rather than mandate an internal evaluation order.

}

BOOST_FIXTURE_TEST_CASE(credit_pool_package_atomicity, TestChain100Setup)
{
LOCK(cs_main);
Expand Down
Loading