From 782f19b0688cde9ee46a8fc0a0e96b7c95eaf850 Mon Sep 17 00:00:00 2001 From: Chanaka Sameera Date: Tue, 15 Feb 2022 09:30:01 +0530 Subject: [PATCH 1/8] Add txn-no as a pegination into listaccounthistory rpc --- src/masternodes/rpc_accounts.cpp | 15 +++++++++++++++ test/functional/rpc_listaccounthistory.py | 7 +++++++ 2 files changed, 22 insertions(+) diff --git a/src/masternodes/rpc_accounts.cpp b/src/masternodes/rpc_accounts.cpp index 74992b52cf5..679d31de469 100644 --- a/src/masternodes/rpc_accounts.cpp +++ b/src/masternodes/rpc_accounts.cpp @@ -964,6 +964,8 @@ UniValue listaccounthistory(const JSONRPCRequest& request) { "Filter by transaction type, supported letter from {CustomTxType}"}, {"limit", RPCArg::Type::NUM, RPCArg::Optional::OMITTED, "Maximum number of records to return, 100 by default"}, + {"txn", RPCArg::Type::NUM, RPCArg::Optional::OMITTED, + "Order in block, unlimited by default"}, }, }, }, @@ -991,6 +993,7 @@ UniValue listaccounthistory(const JSONRPCRequest& request) { std::string tokenFilter; uint32_t limit = 100; auto txType = CustomTxType::None; + uint32_t txn = std::numeric_limits::max(); if (request.params.size() > 1) { UniValue optionsObj = request.params[1].get_obj(); @@ -1002,6 +1005,7 @@ UniValue listaccounthistory(const JSONRPCRequest& request) { {"token", UniValueType(UniValue::VSTR)}, {"txtype", UniValueType(UniValue::VSTR)}, {"limit", UniValueType(UniValue::VNUM)}, + {"txn", UniValueType(UniValue::VNUM)}, }, true, true); if (!optionsObj["maxBlockHeight"].isNull()) { @@ -1031,6 +1035,10 @@ UniValue listaccounthistory(const JSONRPCRequest& request) { if (limit == 0) { limit = std::numeric_limits::max(); } + + if (!optionsObj["txn"].isNull()) { + txn = (uint32_t) optionsObj["txn"].get_int64(); + } } pwallet->BlockUntilSyncedToCurrentChain(); @@ -1089,6 +1097,10 @@ UniValue listaccounthistory(const JSONRPCRequest& request) { return false; } + if(txn != std::numeric_limits::max() && txn != key.txn) { + return true; + } + std::unique_ptr reverter; if (!noRewards) { reverter = MakeUnique(view, key.owner, valueLazy.get().diff); @@ -1184,6 +1196,9 @@ UniValue listaccounthistory(const JSONRPCRequest& request) { return txs.count(pwtx->GetHash()) || startBlock > index->nHeight || index->nHeight > maxBlockHeight; }, [&](COutputEntry const & entry, CBlockIndex const * index, CWalletTx const * pwtx) { + if(txn != std::numeric_limits::max() && txn != entry.vout) { + return true; + } auto& array = ret.emplace(index->nHeight, UniValue::VARR).first->second; array.push_back(outputEntryToJSON(entry, index, pwtx)); return --count != 0; diff --git a/test/functional/rpc_listaccounthistory.py b/test/functional/rpc_listaccounthistory.py index cc0fa8a2115..b1805e07c61 100755 --- a/test/functional/rpc_listaccounthistory.py +++ b/test/functional/rpc_listaccounthistory.py @@ -72,6 +72,13 @@ def run_test(self): for txs in results: assert(hasattr(txs['amounts'], '__len__') and (not isinstance(txs['amounts'], str))) + txn = results[0]['txn'] + results = self.nodes[0].listaccounthistory(collateral_a, {"txn":txn}) + for txs in results: + assert_equal(txs['owner'], collateral_a) + assert_equal(txs['txn'], txn) + self.log.info("txn is %d", txs['txn']) + # Get node 1 results results = self.nodes[1].listaccounthistory(collateral_a) From 806a06bcb95c85c411d3a311b0743075e092bf00 Mon Sep 17 00:00:00 2001 From: Chanaka Sameera Date: Thu, 17 Feb 2022 12:06:34 +0530 Subject: [PATCH 2/8] Added a review comment suggesion --- src/masternodes/rpc_accounts.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/masternodes/rpc_accounts.cpp b/src/masternodes/rpc_accounts.cpp index 679d31de469..bd01c06e3d7 100644 --- a/src/masternodes/rpc_accounts.cpp +++ b/src/masternodes/rpc_accounts.cpp @@ -1097,10 +1097,6 @@ UniValue listaccounthistory(const JSONRPCRequest& request) { return false; } - if(txn != std::numeric_limits::max() && txn != key.txn) { - return true; - } - std::unique_ptr reverter; if (!noRewards) { reverter = MakeUnique(view, key.owner, valueLazy.get().diff); @@ -1129,6 +1125,10 @@ UniValue listaccounthistory(const JSONRPCRequest& request) { return true; } + if(txn != std::numeric_limits::max() && txn != key.txn) { + return true; + } + if (isMine) { // starts new account owned by the wallet if (lastOwner != key.owner) { From 9649fdea09a1007ccadd946e5765ff776a7c079e Mon Sep 17 00:00:00 2001 From: Chanaka Sameera Date: Mon, 21 Feb 2022 10:48:39 +0530 Subject: [PATCH 3/8] txn is used as a pagination key to search in account history DB --- src/masternodes/rpc_accounts.cpp | 6 +----- test/functional/rpc_listaccounthistory.py | 11 +++++++++-- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/masternodes/rpc_accounts.cpp b/src/masternodes/rpc_accounts.cpp index e83342de2ff..81a020e718f 100644 --- a/src/masternodes/rpc_accounts.cpp +++ b/src/masternodes/rpc_accounts.cpp @@ -1125,10 +1125,6 @@ UniValue listaccounthistory(const JSONRPCRequest& request) { return true; } - if(txn != std::numeric_limits::max() && txn != key.txn) { - return true; - } - if (isMine) { // starts new account owned by the wallet if (lastOwner != key.owner) { @@ -1171,7 +1167,7 @@ UniValue listaccounthistory(const JSONRPCRequest& request) { return count != 0 || isMine; }; - AccountHistoryKey startKey{account, maxBlockHeight, std::numeric_limits::max()}; + AccountHistoryKey startKey{account, maxBlockHeight, txn}; if (!noRewards && !account.empty()) { // revert previous tx to restore account balances to maxBlockHeight diff --git a/test/functional/rpc_listaccounthistory.py b/test/functional/rpc_listaccounthistory.py index b1805e07c61..a8a7dc66e61 100755 --- a/test/functional/rpc_listaccounthistory.py +++ b/test/functional/rpc_listaccounthistory.py @@ -72,12 +72,19 @@ def run_test(self): for txs in results: assert(hasattr(txs['amounts'], '__len__') and (not isinstance(txs['amounts'], str))) - txn = results[0]['txn'] + txn = 1 results = self.nodes[0].listaccounthistory(collateral_a, {"txn":txn}) for txs in results: assert_equal(txs['owner'], collateral_a) assert_equal(txs['txn'], txn) - self.log.info("txn is %d", txs['txn']) + self.log.info("test1: txn is %d", txs['txn']) + + txn = 2 + results = self.nodes[0].listaccounthistory(collateral_a, {"txn":txn}) + for txs in results: + assert_equal(txs['owner'], collateral_a) + assert_equal(txs['txn'], txn) + self.log.info("test2: txn is %d", txs['txn']) # Get node 1 results results = self.nodes[1].listaccounthistory(collateral_a) From aca5f0d1d56f650a1bddeb8fbe8655a54e689c0c Mon Sep 17 00:00:00 2001 From: Chanaka Sameera Date: Mon, 21 Feb 2022 23:09:25 +0530 Subject: [PATCH 4/8] block height and txn are used as keys to search in wallet --- src/masternodes/rpc_accounts.cpp | 7 ++++++- test/functional/rpc_listaccounthistory.py | 7 +++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/src/masternodes/rpc_accounts.cpp b/src/masternodes/rpc_accounts.cpp index 81a020e718f..527e7d3c347 100644 --- a/src/masternodes/rpc_accounts.cpp +++ b/src/masternodes/rpc_accounts.cpp @@ -1081,7 +1081,12 @@ UniValue listaccounthistory(const JSONRPCRequest& request) { std::map> ret; maxBlockHeight = std::min(maxBlockHeight, uint32_t(::ChainActive().Height())); - depth = std::min(depth, maxBlockHeight); + if(txn != std::numeric_limits::max()) { + depth = 0; // if txn is set, only one block is considered + } + else { + depth = std::min(depth, maxBlockHeight); + } const auto startBlock = maxBlockHeight - depth; auto shouldSkipBlock = [startBlock, maxBlockHeight](uint32_t blockHeight) { diff --git a/test/functional/rpc_listaccounthistory.py b/test/functional/rpc_listaccounthistory.py index a8a7dc66e61..57038c37bd9 100755 --- a/test/functional/rpc_listaccounthistory.py +++ b/test/functional/rpc_listaccounthistory.py @@ -86,6 +86,13 @@ def run_test(self): assert_equal(txs['txn'], txn) self.log.info("test2: txn is %d", txs['txn']) + txn = 1 + results = self.nodes[0].listaccounthistory(collateral_a, {"maxBlockHeight":102, "txn":txn}) + for txs in results: + assert_equal(txs['owner'], collateral_a) + assert_equal(txs['txn'], txn) + self.log.info("test3: txn is %d", txs['txn']) + # Get node 1 results results = self.nodes[1].listaccounthistory(collateral_a) From e0051d870e844e7b5092a74da8aca3f0296025f5 Mon Sep 17 00:00:00 2001 From: Chanaka Sameera Date: Tue, 22 Feb 2022 11:43:19 +0530 Subject: [PATCH 5/8] txn is used as pagination key to search in wallet --- src/masternodes/rpc_accounts.cpp | 2 +- test/functional/rpc_listaccounthistory.py | 15 ++++++++------- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/masternodes/rpc_accounts.cpp b/src/masternodes/rpc_accounts.cpp index 86a96f736db..4cd76498db1 100644 --- a/src/masternodes/rpc_accounts.cpp +++ b/src/masternodes/rpc_accounts.cpp @@ -1197,7 +1197,7 @@ UniValue listaccounthistory(const JSONRPCRequest& request) { return txs.count(pwtx->GetHash()) || startBlock > index->nHeight || index->nHeight > maxBlockHeight; }, [&](COutputEntry const & entry, CBlockIndex const * index, CWalletTx const * pwtx) { - if(txn != std::numeric_limits::max() && txn != entry.vout) { + if(txn != std::numeric_limits::max() && txn > entry.vout) { return true; } auto& array = ret.emplace(index->nHeight, UniValue::VARR).first->second; diff --git a/test/functional/rpc_listaccounthistory.py b/test/functional/rpc_listaccounthistory.py index 57038c37bd9..c92cbcda534 100755 --- a/test/functional/rpc_listaccounthistory.py +++ b/test/functional/rpc_listaccounthistory.py @@ -64,6 +64,7 @@ def run_test(self): found = False for txs in results: assert_equal(txs['owner'], collateral_a) + self.log.info("test 0: block %d, txn is %d", txs['blockHeight'], txs['txn']) if txs['type'] == 'MintToken': found = True assert_equal(found, True) @@ -73,25 +74,25 @@ def run_test(self): assert(hasattr(txs['amounts'], '__len__') and (not isinstance(txs['amounts'], str))) txn = 1 - results = self.nodes[0].listaccounthistory(collateral_a, {"txn":txn}) + results = self.nodes[0].listaccounthistory(collateral_a, {"maxBlockHeight":103, "txn":txn}) for txs in results: assert_equal(txs['owner'], collateral_a) - assert_equal(txs['txn'], txn) - self.log.info("test1: txn is %d", txs['txn']) + assert_equal(txs['txn'] >= txn, True) + self.log.info("test 1: block %d, txn is %d", txs['blockHeight'], txs['txn']) txn = 2 results = self.nodes[0].listaccounthistory(collateral_a, {"txn":txn}) for txs in results: assert_equal(txs['owner'], collateral_a) - assert_equal(txs['txn'], txn) - self.log.info("test2: txn is %d", txs['txn']) + assert_equal(txs['txn'] >= txn, True) + self.log.info("test 2: block %d, txn is %d", txs['blockHeight'], txs['txn']) txn = 1 results = self.nodes[0].listaccounthistory(collateral_a, {"maxBlockHeight":102, "txn":txn}) for txs in results: assert_equal(txs['owner'], collateral_a) - assert_equal(txs['txn'], txn) - self.log.info("test3: txn is %d", txs['txn']) + assert_equal(txs['txn'] >= txn, True) + self.log.info("test 3: block %d, txn is %d", txs['blockHeight'], txs['txn']) # Get node 1 results results = self.nodes[1].listaccounthistory(collateral_a) From a35af6cb2b586ed4d0d3e71e2b44672f63d1067a Mon Sep 17 00:00:00 2001 From: surangap Date: Tue, 22 Feb 2022 17:20:50 +0800 Subject: [PATCH 6/8] Updated txn logic and tests. --- src/masternodes/rpc_accounts.cpp | 9 ++----- test/functional/rpc_listaccounthistory.py | 29 ++++++++++++----------- 2 files changed, 17 insertions(+), 21 deletions(-) diff --git a/src/masternodes/rpc_accounts.cpp b/src/masternodes/rpc_accounts.cpp index 4cd76498db1..97100458c55 100644 --- a/src/masternodes/rpc_accounts.cpp +++ b/src/masternodes/rpc_accounts.cpp @@ -1081,12 +1081,7 @@ UniValue listaccounthistory(const JSONRPCRequest& request) { std::map> ret; maxBlockHeight = std::min(maxBlockHeight, uint32_t(::ChainActive().Height())); - if(txn != std::numeric_limits::max()) { - depth = 0; // if txn is set, only one block is considered - } - else { - depth = std::min(depth, maxBlockHeight); - } + depth = std::min(depth, maxBlockHeight); const auto startBlock = maxBlockHeight - depth; auto shouldSkipBlock = [startBlock, maxBlockHeight](uint32_t blockHeight) { @@ -1197,7 +1192,7 @@ UniValue listaccounthistory(const JSONRPCRequest& request) { return txs.count(pwtx->GetHash()) || startBlock > index->nHeight || index->nHeight > maxBlockHeight; }, [&](COutputEntry const & entry, CBlockIndex const * index, CWalletTx const * pwtx) { - if(txn != std::numeric_limits::max() && txn > entry.vout) { + if (txn != std::numeric_limits::max() && index->nHeight == maxBlockHeight && entry.vout > txn ) { return true; } auto& array = ret.emplace(index->nHeight, UniValue::VARR).first->second; diff --git a/test/functional/rpc_listaccounthistory.py b/test/functional/rpc_listaccounthistory.py index c92cbcda534..968f8816ec6 100755 --- a/test/functional/rpc_listaccounthistory.py +++ b/test/functional/rpc_listaccounthistory.py @@ -73,26 +73,27 @@ def run_test(self): for txs in results: assert(hasattr(txs['amounts'], '__len__') and (not isinstance(txs['amounts'], str))) - txn = 1 - results = self.nodes[0].listaccounthistory(collateral_a, {"maxBlockHeight":103, "txn":txn}) + # list {"maxBlockHeight":103, "txn":1}, should list without blockheight = 103, txn=2. i.e without MintToken + results = self.nodes[0].listaccounthistory(collateral_a, {"maxBlockHeight":103, "txn":1}) + #Note(surangap): the results here has 2 extra send receive entries with different txids. not sure where they come from. for txs in results: - assert_equal(txs['owner'], collateral_a) - assert_equal(txs['txn'] >= txn, True) self.log.info("test 1: block %d, txn is %d", txs['blockHeight'], txs['txn']) - - txn = 2 - results = self.nodes[0].listaccounthistory(collateral_a, {"txn":txn}) - for txs in results: assert_equal(txs['owner'], collateral_a) - assert_equal(txs['txn'] >= txn, True) - self.log.info("test 2: block %d, txn is %d", txs['blockHeight'], txs['txn']) + assert_equal(txs['blockHeight'] <= 103, True) + if txs['blockHeight'] == 103: + assert_equal(txs['txn'] <= 1 , True) # for block 103 txn:1 applies. + + # list {"maxBlockHeight":103, "txn":0}, should list without blockheight = 103, txn=1,2. i.e without any txs from 103 block + results = self.nodes[0].listaccounthistory(collateral_a, {"maxBlockHeight":103, "txn":0}) - txn = 1 - results = self.nodes[0].listaccounthistory(collateral_a, {"maxBlockHeight":102, "txn":txn}) for txs in results: + self.log.info("test 2: block %d, txn is %d", txs['blockHeight'], txs['txn']) assert_equal(txs['owner'], collateral_a) - assert_equal(txs['txn'] >= txn, True) - self.log.info("test 3: block %d, txn is %d", txs['blockHeight'], txs['txn']) + assert_equal(txs['blockHeight'] <= 103, True) + if txs['blockHeight'] == 103: + assert_equal(txs['txn'] <= 0 , True) + else: + assert_equal(txs['txn'] >= 0 , True) # means txn:0 only applicable to block 103 only # Get node 1 results results = self.nodes[1].listaccounthistory(collateral_a) From 9b32a5ec031c40e34e3f50bd68bccd9d2f26f371 Mon Sep 17 00:00:00 2001 From: Chanaka Sameera Date: Fri, 25 Feb 2022 16:40:51 +0530 Subject: [PATCH 7/8] CWalletTx.nIndex is used as txn instead of entry.vout --- src/masternodes/rpc_accounts.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/masternodes/rpc_accounts.cpp b/src/masternodes/rpc_accounts.cpp index 97100458c55..4081456c243 100644 --- a/src/masternodes/rpc_accounts.cpp +++ b/src/masternodes/rpc_accounts.cpp @@ -94,7 +94,7 @@ UniValue outputEntryToJSON(COutputEntry const & entry, CBlockIndex const * index } else { obj.pushKV("type", "receive"); } - obj.pushKV("txn", (uint64_t) entry.vout); + obj.pushKV("txn", (uint64_t) pwtx->nIndex); obj.pushKV("txid", pwtx->GetHash().ToString()); TAmounts amounts({{DCT_ID{0},entry.amount}}); obj.pushKV("amounts", AmountsToJSON(amounts)); @@ -1192,7 +1192,7 @@ UniValue listaccounthistory(const JSONRPCRequest& request) { return txs.count(pwtx->GetHash()) || startBlock > index->nHeight || index->nHeight > maxBlockHeight; }, [&](COutputEntry const & entry, CBlockIndex const * index, CWalletTx const * pwtx) { - if (txn != std::numeric_limits::max() && index->nHeight == maxBlockHeight && entry.vout > txn ) { + if (txn != std::numeric_limits::max() && index->nHeight == maxBlockHeight && pwtx->nIndex > txn ) { return true; } auto& array = ret.emplace(index->nHeight, UniValue::VARR).first->second; From 22749fe10c9014b1277f2424f6782aee16fc1bf7 Mon Sep 17 00:00:00 2001 From: surangap Date: Mon, 28 Feb 2022 15:59:44 +0800 Subject: [PATCH 8/8] Update test/functional/rpc_listaccounthistory.py --- test/functional/rpc_listaccounthistory.py | 1 - 1 file changed, 1 deletion(-) diff --git a/test/functional/rpc_listaccounthistory.py b/test/functional/rpc_listaccounthistory.py index 968f8816ec6..6b792a6b55b 100755 --- a/test/functional/rpc_listaccounthistory.py +++ b/test/functional/rpc_listaccounthistory.py @@ -75,7 +75,6 @@ def run_test(self): # list {"maxBlockHeight":103, "txn":1}, should list without blockheight = 103, txn=2. i.e without MintToken results = self.nodes[0].listaccounthistory(collateral_a, {"maxBlockHeight":103, "txn":1}) - #Note(surangap): the results here has 2 extra send receive entries with different txids. not sure where they come from. for txs in results: self.log.info("test 1: block %d, txn is %d", txs['blockHeight'], txs['txn']) assert_equal(txs['owner'], collateral_a)