From 8d805e94226bc8e2e6424b6c0a8f1198c88c7558 Mon Sep 17 00:00:00 2001 From: Shoham Chakraborty Date: Mon, 16 Jan 2023 20:03:07 +0800 Subject: [PATCH] Add multi-account and multi-TX filters to `accounthistorycount` (#1666) * Support multiple TX type filtering * Support multiple account filtering * Fix errors * Add test for accounthistorycount * Add more tests for accounthistorycount * Add early return in shouldContinueToNextAccountHistory * Remove outer accountSet loop * Revert "Remove outer accountSet loop" This reverts commit 051fe241f4bf70309caab60a4d6bcd3d146fa01c. * Revert "Add early return in shouldContinueToNextAccountHistory" This reverts commit 33f35523c6605b47b095cc91b784473ba5710a9e. * Add total count test Co-authored-by: Peter John Bushnell --- src/masternodes/rpc_accounts.cpp | 158 +++++++++++------- ...re_listaccounthistory_multiaccountquery.py | 2 + test/functional/rpc_listaccounthistory.py | 9 + 3 files changed, 112 insertions(+), 57 deletions(-) diff --git a/src/masternodes/rpc_accounts.cpp b/src/masternodes/rpc_accounts.cpp index 08cf854e77f..5cdfacc49e6 100644 --- a/src/masternodes/rpc_accounts.cpp +++ b/src/masternodes/rpc_accounts.cpp @@ -1548,6 +1548,16 @@ UniValue accounthistorycount(const JSONRPCRequest& request) { {"no_rewards", RPCArg::Type::BOOL, RPCArg::Optional::OMITTED, "Filter out rewards"}, {"token", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "Filter by token"}, {"txtype", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "Filter by transaction type, supported letter from {CustomTxType}"}, + {"txtypes", RPCArg::Type::ARR, RPCArg::Optional::OMITTED, "Filter multiple transaction types, supported letter from {CustomTxType}", + { + { + "Transaction Type", + RPCArg::Type::STR, + RPCArg::Optional::OMITTED, + "letter from {CustomTxType}", + }, + } + }, }, }, }, @@ -1573,7 +1583,8 @@ UniValue accounthistorycount(const JSONRPCRequest& request) { bool noRewards = false; std::string tokenFilter; - auto txType = CustomTxType::None; + std::set txTypes{}; + bool hasTxFilter = false; if (request.params.size() > 1) { UniValue optionsObj = request.params[1].get_obj(); @@ -1582,6 +1593,7 @@ UniValue accounthistorycount(const JSONRPCRequest& request) { {"no_rewards", UniValueType(UniValue::VBOOL)}, {"token", UniValueType(UniValue::VSTR)}, {"txtype", UniValueType(UniValue::VSTR)}, + {"txtypes", UniValueType(UniValue::VARR)}, }, true, true); noRewards = optionsObj["no_rewards"].getBool(); @@ -1590,15 +1602,32 @@ UniValue accounthistorycount(const JSONRPCRequest& request) { tokenFilter = optionsObj["token"].get_str(); } - if (!optionsObj["txtype"].isNull()) { + if (!optionsObj["txtypes"].isNull()) { + hasTxFilter = true; + const auto types = optionsObj["txtypes"].get_array().getValues(); + if (!types.empty()) { + for (const auto& type : types) { + auto str = type.get_str(); + if (str.size() == 1) { + txTypes.insert(CustomTxCodeToType(str[0])); + } else { + txTypes.insert(FromString(str)); + } + } + } + } + else if (!optionsObj["txtype"].isNull()) { + hasTxFilter = true; const auto str = optionsObj["txtype"].get_str(); if (str.size() == 1) { - txType = CustomTxCodeToType(str[0]); + txTypes.insert(CustomTxCodeToType(str[0])); + } else { + txTypes.insert(FromString(str)); } } } - CScript owner; + std::set accountSet{CScript{}}; bool isMine = false; isminetype filter = ISMINE_ALL; @@ -1606,12 +1635,19 @@ UniValue accounthistorycount(const JSONRPCRequest& request) { isMine = true; filter = ISMINE_SPENDABLE; } else if (accounts != "all") { - owner = DecodeScript(accounts); - isMine = IsMineCached(*pwallet, owner) & ISMINE_ALL; + accountSet.clear(); + if (request.params[0].isArray()) { + for (const auto& acc: request.params[0].get_array().getValues()) + accountSet.emplace(DecodeScript(acc.get_str())); + } else { + const auto owner = DecodeScript(accounts); + accountSet.emplace(owner); + isMine = IsMineCached(*pwallet, owner) & ISMINE_ALL; + } } std::set txs; - const bool shouldSearchInWallet = (tokenFilter.empty() || tokenFilter == "DFI") && CustomTxType::None == txType; + const bool shouldSearchInWallet = (tokenFilter.empty() || tokenFilter == "DFI") && !hasTxFilter; auto hasToken = [&tokenFilter](TAmounts const & diffs) { for (auto const & diff : diffs) { @@ -1627,69 +1663,77 @@ UniValue accounthistorycount(const JSONRPCRequest& request) { LOCK(cs_main); CCustomCSView view(*pcustomcsview); CCoinsViewCache coins(&::ChainstateActive().CoinsTip()); - - CScript lastOwner; uint64_t count = 0; - auto lastHeight = uint32_t(::ChainActive().Height()); - const auto currentHeight = lastHeight; - auto shouldContinueToNextAccountHistory = [&](AccountHistoryKey const & key, AccountHistoryValue value) -> bool { - if (!owner.empty() && owner != key.owner) { - return false; - } + for (const auto &owner : accountSet) { + CScript lastOwner; + auto lastHeight = uint32_t(::ChainActive().Height()); + const auto currentHeight = lastHeight; - if (isMine && !(IsMineCached(*pwallet, key.owner) & filter)) { - return true; - } + auto shouldContinueToNextAccountHistory = [&](AccountHistoryKey const &key, AccountHistoryValue value) -> bool { + if (!owner.empty() && owner != key.owner) { + return false; + } - std::unique_ptr reverter; - if (!noRewards) { - reverter = std::make_unique(view, key.owner, value.diff); - } + if (isMine && !(IsMineCached(*pwallet, key.owner) & filter)) { + return true; + } - if (CustomTxType::None != txType && value.category != uint8_t(txType)) { - return true; - } + std::unique_ptr reverter; + if (!noRewards) { + reverter = std::make_unique(view, key.owner, value.diff); + } - if (tokenFilter.empty() || hasToken(value.diff)) { - if (shouldSearchInWallet) { - txs.insert(value.txid); + if (hasTxFilter && txTypes.find(CustomTxCodeToType(value.category)) == txTypes.end()) { + return true; } - ++count; - } - if (!noRewards) { - // starting new account - if (lastOwner != key.owner) { - view.Discard(); - lastOwner = key.owner; - lastHeight = currentHeight; + if (tokenFilter.empty() || hasToken(value.diff)) { + if (shouldSearchInWallet) { + txs.insert(value.txid); + } + ++count; } - onPoolRewards(view, key.owner, key.blockHeight, lastHeight, - [&](int32_t, DCT_ID, RewardType, CTokenAmount amount) { - if (tokenFilter.empty() || hasToken({{amount.nTokenId, amount.nValue}})) { - ++count; - } + + if (!noRewards) { + // starting new account + if (lastOwner != key.owner) { + view.Discard(); + lastOwner = key.owner; + lastHeight = currentHeight; } - ); - lastHeight = key.blockHeight; - } + onPoolRewards(view, + key.owner, + key.blockHeight, + lastHeight, + [&](int32_t, DCT_ID, RewardType, CTokenAmount amount) { + if (tokenFilter.empty() || hasToken({ + {amount.nTokenId, amount.nValue} + })) { + ++count; + } + }); + lastHeight = key.blockHeight; + } - return true; - }; + return true; + }; - paccountHistoryDB->ForEachAccountHistory(shouldContinueToNextAccountHistory, owner, currentHeight); + paccountHistoryDB->ForEachAccountHistory(shouldContinueToNextAccountHistory, owner, currentHeight); - if (shouldSearchInWallet) { - searchInWallet(pwallet, owner, filter, - [&](CBlockIndex const * index, CWalletTx const * pwtx) { - return txs.count(pwtx->GetHash()) || static_cast(index->nHeight) > currentHeight; - }, - [&count](COutputEntry const &, CBlockIndex const *, CWalletTx const *) { - ++count; - return true; - } - ); + if (shouldSearchInWallet) { + searchInWallet( + pwallet, + owner, + filter, + [&](CBlockIndex const *index, CWalletTx const *pwtx) { + return txs.count(pwtx->GetHash()) || static_cast(index->nHeight) > currentHeight; + }, + [&count](COutputEntry const &, CBlockIndex const *, CWalletTx const *) { + ++count; + return true; + }); + } } return GetRPCResultCache().Set(request, count); diff --git a/test/functional/feature_listaccounthistory_multiaccountquery.py b/test/functional/feature_listaccounthistory_multiaccountquery.py index dd9ec493b02..7feb67a39ab 100644 --- a/test/functional/feature_listaccounthistory_multiaccountquery.py +++ b/test/functional/feature_listaccounthistory_multiaccountquery.py @@ -63,9 +63,11 @@ def run_test(self): self.sync_blocks() combined = self.nodes[0].listaccounthistory([collateral_a, collateral_b]) + combined_count = self.nodes[0].accounthistorycount([collateral_a, collateral_b]) a = self.nodes[0].listaccounthistory([collateral_a]) b = self.nodes[0].listaccounthistory([collateral_b]) + assert_equal(len(combined), combined_count) assert_equal(len(combined), len(a) + len(b)) diff --git a/test/functional/rpc_listaccounthistory.py b/test/functional/rpc_listaccounthistory.py index f0c66ce3ab4..60af58c64a9 100755 --- a/test/functional/rpc_listaccounthistory.py +++ b/test/functional/rpc_listaccounthistory.py @@ -138,10 +138,16 @@ def run_test(self): self.nodes[0].listaccounthistory(collateral_a, {"txtype": "BurnToken"}) + self.nodes[0].listaccounthistory(collateral_a, {"txtype": "MintToken"})) + assert_equal(len(self.nodes[0].listaccounthistory(collateral_a, {"txtypes": ["MintToken", "BurnToken"]})), + self.nodes[0].accounthistorycount(collateral_a, {"txtypes": ["MintToken", "BurnToken"]})) + # txtype should be ignored if txtypes is passed assert_equal(self.nodes[0].listaccounthistory(collateral_a, {"txtypes": ["MintToken", "BurnToken"]}), self.nodes[0].listaccounthistory(collateral_a, {"txtype": "BurnToken", "txtypes": ["MintToken", "BurnToken"]})) + assert_equal(self.nodes[0].accounthistorycount(collateral_a, {"txtypes": ["MintToken", "BurnToken"]}), + self.nodes[0].accounthistorycount(collateral_a, {"txtype": "BurnToken", "txtypes": ["MintToken", + "BurnToken"]})) # test pagination res0 = self.nodes[0].listaccounthistory(collateral_a, {"start": 0, "including_start": True}) @@ -159,6 +165,9 @@ def run_test(self): assert_equal(len(res0), len(res2) + 2) assert_equal(len(res0), len(res3) + 3) + # accounthistorycount should return total count + assert_equal(self.nodes[0].accounthistorycount(), 112) + # REVERTING: # ======================== self.start_node(2)