Skip to content
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.

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

Prevent transactions in blocks from going below balance. #136

Merged
merged 9 commits into from
Dec 14, 2020
5 changes: 4 additions & 1 deletion src/masternodes/accountshistory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
#include <masternodes/accounts.h>
#include <key_io.h>

#include <limits>

/// @attention make sure that it does not overlap with those in masternodes.cpp/tokens.cpp/undos.cpp/accounts.cpp !!!
const unsigned char CAccountsHistoryView::ByAccountHistoryKey::prefix = 'h'; // don't intersects with CMintedHeadersView::MintedHeaders::prefix due to different DB

Expand All @@ -32,7 +34,8 @@ Res CAccountsHistoryView::SetAccountHistory(const CScript & owner, uint32_t heig
}

bool CAccountsHistoryView::TrackAffectedAccounts(CStorageKV const & before, MapKV const & diff, uint32_t height, uint32_t txn, const uint256 & txid, unsigned char category) {
if (!gArgs.GetBoolArg("-acindex", false))
// txn set to max if called from CreateNewBlock to check account balances, do not track here.
if (!gArgs.GetBoolArg("-acindex", false) || txn == std::numeric_limits<uint32_t>::max())
return false;

std::map<CScript, TAmounts> balancesDiff;
Expand Down
14 changes: 7 additions & 7 deletions src/masternodes/mn_checks.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ bool HasFoundationAuth(CTransaction const & tx, CCoinsViewCache const & coins, C
return false;
}

Res ApplyCustomTx(CCustomCSView & base_mnview, CCoinsViewCache const & coins, CTransaction const & tx, Consensus::Params const & consensusParams, uint32_t height, uint32_t txn, bool isCheck)
Res ApplyCustomTx(CCustomCSView & base_mnview, CCoinsViewCache const & coins, CTransaction const & tx, Consensus::Params const & consensusParams, uint32_t height, uint32_t txn, bool isCheck, bool skipAuth)
{
Res res = Res::Ok();

Expand Down Expand Up @@ -145,7 +145,7 @@ Res ApplyCustomTx(CCustomCSView & base_mnview, CCoinsViewCache const & coins, CT
res = ApplyUpdateTokenAnyTx(mnview, coins, tx, height, metadata, consensusParams);
break;
case CustomTxType::MintToken:
res = ApplyMintTokenTx(mnview, coins, tx, height, metadata, consensusParams);
res = ApplyMintTokenTx(mnview, coins, tx, height, metadata, consensusParams, skipAuth);
break;
case CustomTxType::CreatePoolPair:
res = ApplyCreatePoolPairTx(mnview, coins, tx, height, metadata, consensusParams);
Expand All @@ -166,7 +166,7 @@ Res ApplyCustomTx(CCustomCSView & base_mnview, CCoinsViewCache const & coins, CT
res = ApplyUtxosToAccountTx(mnview, tx, height, metadata, consensusParams);
break;
case CustomTxType::AccountToUtxos:
res = ApplyAccountToUtxosTx(mnview, coins, tx, height, metadata, consensusParams);
res = ApplyAccountToUtxosTx(mnview, coins, tx, height, metadata, consensusParams, skipAuth);
break;
case CustomTxType::AccountToAccount:
res = ApplyAccountToAccountTx(mnview, coins, tx, height, metadata, consensusParams);
Expand Down Expand Up @@ -423,7 +423,7 @@ Res ApplyUpdateTokenAnyTx(CCustomCSView & mnview, CCoinsViewCache const & coins,
return Res::Ok(base);
}

Res ApplyMintTokenTx(CCustomCSView & mnview, CCoinsViewCache const & coins, CTransaction const & tx, uint32_t height, std::vector<unsigned char> const & metadata, Consensus::Params const & consensusParams)
Res ApplyMintTokenTx(CCustomCSView & mnview, CCoinsViewCache const & coins, CTransaction const & tx, uint32_t height, std::vector<unsigned char> const & metadata, Consensus::Params const & consensusParams, bool skipAuth)
{
if((int)height < consensusParams.AMKHeight) { return Res::Err("Token tx before AMK height (block %d)", consensusParams.AMKHeight); }

Expand Down Expand Up @@ -473,7 +473,7 @@ Res ApplyMintTokenTx(CCustomCSView & mnview, CCoinsViewCache const & coins, CTra
return Res::Err("%s: token not mintable!", tokenId.ToString());
}

if (!HasAuth(tx, coins, auth.out.scriptPubKey)) { // in the case of DAT, it's ok to do not check foundation auth cause exact DAT owner is foundation member himself
if (!skipAuth && !HasAuth(tx, coins, auth.out.scriptPubKey)) { // in the case of DAT, it's ok to do not check foundation auth cause exact DAT owner is foundation member himself
if (!tokenImpl.IsDAT())
return Res::Err("%s: %s", base, "tx must have at least one input from token owner");

Expand Down Expand Up @@ -696,7 +696,7 @@ Res ApplyUtxosToAccountTx(CCustomCSView & mnview, CTransaction const & tx, uint3
return Res::Ok(base);
}

Res ApplyAccountToUtxosTx(CCustomCSView & mnview, CCoinsViewCache const & coins, CTransaction const & tx, uint32_t height, std::vector<unsigned char> const & metadata, Consensus::Params const & consensusParams)
Res ApplyAccountToUtxosTx(CCustomCSView & mnview, CCoinsViewCache const & coins, CTransaction const & tx, uint32_t height, std::vector<unsigned char> const & metadata, Consensus::Params const & consensusParams, bool skipAuth)
{
if((int)height < consensusParams.AMKHeight) { return Res::Err("Token tx before AMK height (block %d)", consensusParams.AMKHeight); }

Expand All @@ -710,7 +710,7 @@ Res ApplyAccountToUtxosTx(CCustomCSView & mnview, CCoinsViewCache const & coins,
const auto base = strprintf("Transfer AccountToUtxos: %s", msg.ToString());

// check auth
if (!HasAuth(tx, coins, msg.from)) {
if (!skipAuth && !HasAuth(tx, coins, msg.from)) {
return Res::Err("%s: %s", base, "tx must have at least one input from account owner");
}
// check that all tokens are minted, and no excess tokens are minted
Expand Down
6 changes: 3 additions & 3 deletions src/masternodes/mn_checks.h
Original file line number Diff line number Diff line change
Expand Up @@ -113,15 +113,15 @@ bool HasAuth(CTransaction const & tx, CKeyID const & auth);
bool HasAuth(CTransaction const & tx, CCoinsViewCache const & coins, CScript const & auth);
bool HasCollateralAuth(CTransaction const & tx, CCoinsViewCache const & coins, uint256 const & collateralTx);

Res ApplyCustomTx(CCustomCSView & mnview, CCoinsViewCache const & coins, CTransaction const & tx, const Consensus::Params& consensusParams, uint32_t height, uint32_t txn, bool isCheck = true);
Res ApplyCustomTx(CCustomCSView & mnview, CCoinsViewCache const & coins, CTransaction const & tx, const Consensus::Params& consensusParams, uint32_t height, uint32_t txn, bool isCheck = true, bool skipAuth = false);
//! Deep check (and write)
Res ApplyCreateMasternodeTx(CCustomCSView & mnview, CTransaction const & tx, uint32_t height, std::vector<unsigned char> const & metadata);
Res ApplyResignMasternodeTx(CCustomCSView & mnview, CCoinsViewCache const & coins, CTransaction const & tx, uint32_t height, std::vector<unsigned char> const & metadata);

Res ApplyCreateTokenTx(CCustomCSView & mnview, CCoinsViewCache const & coins, CTransaction const & tx, uint32_t height, std::vector<unsigned char> const & metadata, Consensus::Params const & consensusParams);
Res ApplyUpdateTokenTx(CCustomCSView & mnview, CCoinsViewCache const & coins, CTransaction const & tx, uint32_t height, std::vector<unsigned char> const & metadata, Consensus::Params const & consensusParams);
Res ApplyUpdateTokenAnyTx(CCustomCSView & mnview, CCoinsViewCache const & coins, CTransaction const & tx, uint32_t height, std::vector<unsigned char> const & metadata, Consensus::Params const & consensusParams);
Res ApplyMintTokenTx(CCustomCSView & mnview, CCoinsViewCache const & coins, CTransaction const & tx, uint32_t height, std::vector<unsigned char> const & metadata, Consensus::Params const & consensusParams);
Res ApplyMintTokenTx(CCustomCSView & mnview, CCoinsViewCache const & coins, CTransaction const & tx, uint32_t height, std::vector<unsigned char> const & metadata, Consensus::Params const & consensusParams, bool skipAuth = false);

Res ApplyCreatePoolPairTx(CCustomCSView & mnview, CCoinsViewCache const & coins, CTransaction const & tx, uint32_t height, std::vector<unsigned char> const & metadata, Consensus::Params const & consensusParams);
Res ApplyUpdatePoolPairTx(CCustomCSView & mnview, CCoinsViewCache const & coins, CTransaction const & tx, uint32_t height, std::vector<unsigned char> const & metadata, Consensus::Params const & consensusParams);
Expand All @@ -130,7 +130,7 @@ Res ApplyAddPoolLiquidityTx(CCustomCSView & mnview, CCoinsViewCache const & coin
Res ApplyRemovePoolLiquidityTx(CCustomCSView & mnview, CCoinsViewCache const & coins, CTransaction const & tx, uint32_t height, std::vector<unsigned char> const & metadata, Consensus::Params const & consensusParams);

Res ApplyUtxosToAccountTx(CCustomCSView & mnview, CTransaction const & tx, uint32_t height, std::vector<unsigned char> const & metadata, Consensus::Params const & consensusParams);
Res ApplyAccountToUtxosTx(CCustomCSView & mnview, CCoinsViewCache const & coins, CTransaction const & tx, uint32_t height, std::vector<unsigned char> const & metadata, Consensus::Params const & consensusParams);
Res ApplyAccountToUtxosTx(CCustomCSView & mnview, CCoinsViewCache const & coins, CTransaction const & tx, uint32_t height, std::vector<unsigned char> const & metadata, Consensus::Params const & consensusParams, bool skipAuth = false);
Res ApplyAccountToAccountTx(CCustomCSView & mnview, CCoinsViewCache const & coins, CTransaction const & tx, uint32_t height, std::vector<unsigned char> const & metadata, Consensus::Params const & consensusParams);
Res ApplyAnyAccountsToAccountsTx(CCustomCSView & mnview, CCoinsViewCache const & coins, CTransaction const & tx, uint32_t height, std::vector<unsigned char> const & metadata, Consensus::Params const & consensusParams);

Expand Down
54 changes: 51 additions & 3 deletions src/miner.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ std::unique_ptr<CBlockTemplate> BlockAssembler::CreateNewBlock(const CScript& sc

int nPackagesSelected = 0;
int nDescendantsUpdated = 0;
addPackageTxs(nPackagesSelected, nDescendantsUpdated);
addPackageTxs(nPackagesSelected, nDescendantsUpdated, nHeight);

int64_t nTime1 = GetTimeMicros();

Expand Down Expand Up @@ -421,7 +421,7 @@ void BlockAssembler::SortForBlock(const CTxMemPool::setEntries& package, std::ve
// Each time through the loop, we compare the best transaction in
// mapModifiedTxs with the next transaction in the mempool to decide what
// transaction package to work on next.
void BlockAssembler::addPackageTxs(int &nPackagesSelected, int &nDescendantsUpdated)
void BlockAssembler::addPackageTxs(int &nPackagesSelected, int &nDescendantsUpdated, int nHeight)
{
// mapModifiedTx will store sorted packages after they are modified
// because some of their txs are already in the block
Expand All @@ -442,6 +442,12 @@ void BlockAssembler::addPackageTxs(int &nPackagesSelected, int &nDescendantsUpda
const int64_t MAX_CONSECUTIVE_FAILURES = 1000;
int64_t nConsecutiveFailed = 0;

// Custom TXs to undo at the end
std::set<uint256> checkedTX;

// Copy of the view
CCustomCSView view(*pcustomcsview);

while (mi != mempool.mapTx.get<ancestor_score>().end() || !mapModifiedTx.empty())
{
// First try to find a new transaction in mapTx to evaluate.
Expand Down Expand Up @@ -492,7 +498,7 @@ void BlockAssembler::addPackageTxs(int &nPackagesSelected, int &nDescendantsUpda

if (packageFees < blockMinFeeRate.GetFee(packageSize)) {
// Everything else we might consider has a lower fee rate
return;
break;
}

if (!TestPackage(packageSize, packageSigOpsCost)) {
Expand Down Expand Up @@ -538,6 +544,48 @@ void BlockAssembler::addPackageTxs(int &nPackagesSelected, int &nDescendantsUpda
std::vector<CTxMemPool::txiter> sortedEntries;
SortForBlock(ancestors, sortedEntries);

// Account check
bool customTxPassed{true};

// Apply and check custom TXs in order
for (size_t i = 0; i < sortedEntries.size(); ++i) {
const CTransaction& tx = sortedEntries[i]->GetTx();

// Do not double check already checked custom TX. This will be an ancestor of current TX.
if (checkedTX.find(tx.GetHash()) != checkedTX.end()) {
continue;
}

std::vector<unsigned char> metadata;
CustomTxType txType = GuessCustomTxType(tx, metadata);

// Only check custom TXs
if (txType != CustomTxType::None) {
auto res = ApplyCustomTx(view, ::ChainstateActive().CoinsTip(), tx, chainparams.GetConsensus(), nHeight, std::numeric_limits<uint32_t>::max(), false, true);

// Not okay invalidate, undo and skip
if (!res.ok && NotAllowedToFail(txType)) {
customTxPassed = false;

LogPrintf("%s: Failed %s TX %s: %s\n", __func__, ToString(txType), tx.GetHash().GetHex(), res.msg);

break;
}

// Track checked TXs to avoid double applying
checkedTX.insert(tx.GetHash());
}
}

// Failed, let's move on!
if (!customTxPassed) {
if (fUsingModified) {
mapModifiedTx.get<ancestor_score>().erase(modit);
failedTx.insert(iter);
}
continue;
}

for (size_t i=0; i<sortedEntries.size(); ++i) {
AddToBlock(sortedEntries[i]);
// Erase from the modified set, if present
Expand Down
2 changes: 1 addition & 1 deletion src/miner.h
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ class BlockAssembler
/** Add transactions based on feerate including unconfirmed ancestors
* Increments nPackagesSelected / nDescendantsUpdated with corresponding
* statistics from the package selection (for logging statistics). */
void addPackageTxs(int &nPackagesSelected, int &nDescendantsUpdated) EXCLUSIVE_LOCKS_REQUIRED(mempool.cs);
void addPackageTxs(int &nPackagesSelected, int &nDescendantsUpdated, int nHeight) EXCLUSIVE_LOCKS_REQUIRED(mempool.cs);

// helper functions for addPackageTxs()
/** Remove confirmed (inBlock) entries from given set */
Expand Down
44 changes: 44 additions & 0 deletions src/txmempool.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

#include <txmempool.h>

#include <chainparams.h>
#include <consensus/consensus.h>
#include <consensus/tx_verify.h>
#include <consensus/validation.h>
Expand Down Expand Up @@ -577,6 +578,49 @@ void CTxMemPool::removeForBlock(const std::vector<CTransactionRef>& vtx, unsigne
removeConflicts(*tx);
ClearPrioritisation(tx->GetHash());
}

bool accountConflict{false};

// Check if any custom TXs are in mempool with conflict
for (indexed_transaction_set::const_iterator it = mapTx.begin(); it != mapTx.end(); ++it) {
Copy link
Contributor

@ShengguangXiao ShengguangXiao Dec 13, 2020

Choose a reason for hiding this comment

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

Use auto instead of indexed_transaction_set::const_iterator

std::vector<unsigned char> metadata;
CustomTxType txType = GuessCustomTxType(it->GetTx(), metadata);
if (NotAllowedToFail(txType)) {
auto res = ApplyCustomTx(*pcustomcsview, g_chainstate->CoinsTip(), it->GetTx(), Params().GetConsensus(), nBlockHeight, 0, true);
if (!res.ok && (res.code & CustomTxErrCodes::Fatal)) {
accountConflict = true;
break;
}
}
}

// Account conflict, check entire mempool
if (accountConflict) {
std::set<CTransactionRef> txsToRemove;
CCoinsViewCache mempoolDuplicate(const_cast<CCoinsViewCache*>(&::ChainstateActive().CoinsTip()));
CAmount txfee = 0;

// Check custom TX consensus types are now not in conflict with account layer
for (indexed_transaction_set::const_iterator it = mapTx.begin(); it != mapTx.end(); ++it) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Use auto instead of indexed_transaction_set::const_iterator

CValidationState state;
if (!Consensus::CheckTxInputs(it->GetTx(), state, mempoolDuplicate, pcustomcsview.get(), nBlockHeight, txfee, Params())) {
LogPrintf("%s: Remove conflicting TX: %s\n", __func__, it->GetTx().GetHash().GetHex());
txsToRemove.insert(it->GetSharedTx());
}
}

for (auto& tx : txsToRemove) {
txiter it = mapTx.find(tx->GetHash());
if (it != mapTx.end()) {
setEntries stage;
stage.insert(it);
RemoveStaged(stage, true, MemPoolRemovalReason::CONFLICT);
}
removeConflicts(*tx);
ClearPrioritisation(tx->GetHash());
}
}

lastRollingFeeUpdate = GetTime();
blockSinceLastRollingFeeBump = true;
}
Expand Down
1 change: 0 additions & 1 deletion src/validation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1835,7 +1835,6 @@ Res ApplyGeneralCoinbaseTx(CCustomCSView & mnview, CTransaction const & tx, int
return Res::ErrDbg("bad-cb-wrong-tokens", "coinbase should pay only Defi coins");

if (height >= consensus.AMKHeight) {
LogPrintf("ApplyGeneralCoinbaseTx() post AMK logic\n");
// check classic UTXO foundation share:
if (!consensus.foundationShareScript.empty() && consensus.foundationShareDFIP1 != 0) {
CAmount foundationReward = blockReward * consensus.foundationShareDFIP1 / COIN;
Expand Down
47 changes: 47 additions & 0 deletions test/functional/feature_account_mining.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
#!/usr/bin/env python3
# Copyright (c) 2014-2019 The Bitcoin Core developers
# Copyright (c) DeFi Blockchain Developers
# Distributed under the MIT software license, see the accompanying
# file COPYING or http://www.opensource.org/licenses/mit-license.php.
"""Test account mining behaviour"""

from test_framework.test_framework import DefiTestFramework
from test_framework.util import assert_equal

class AccountMiningTest(DefiTestFramework):
def set_test_params(self):
self.num_nodes = 1
self.setup_clean_chain = True
self.extra_args = [['-txnotokens=0', '-amkheight=50']]

def run_test(self):
node = self.nodes[0]
node.generate(120)

# Get addresses and set up account
account = node.getnewaddress()
destination = node.getnewaddress()
node.utxostoaccount({account: "10@0"})
node.generate(1)

# Check we have expected balance
assert_equal(node.getaccount(account)[0], "10.00000000@DFI")

# Send double the amount we have in account
for _ in range(100):
node.accounttoutxos(account, {destination: "2@DFI"})

# Store block height
blockcount = node.getblockcount()

# Generate a block
node.generate(1)

# Check the blockchain has extended as expected
assert_equal(node.getblockcount(), blockcount + 1)

# Account should now be empty
assert_equal(node.getaccount(account), [])
Copy link
Contributor

Choose a reason for hiding this comment

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

Can also check the count of transactions?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll add it. We would expect five TXs in the block plus coinbase. This test will also be extended to include other account TXs.


if __name__ == '__main__':
AccountMiningTest().main ()
1 change: 1 addition & 0 deletions test/functional/test_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,7 @@
'p2p_permissions.py',
'feature_blocksdir.py',
'feature_config_args.py',
'feature_account_mining.py',
'rpc_help.py',
'feature_help.py',
'feature_shutdown.py',
Expand Down
1 change: 1 addition & 0 deletions test/lint/lint-circular-dependencies.sh
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ EXPECTED_CIRCULAR_DEPENDENCIES=(
"validation -> wallet/wallet -> validation"
"policy/fees -> txmempool -> validation -> wallet/wallet -> policy/fees"
"policy/fees -> txmempool -> validation -> wallet/wallet -> util/fees -> policy/fees"
"chainparams -> masternodes/mn_checks -> txmempool -> chainparams"
)

EXIT_CODE=0
Expand Down