Skip to content

Commit

Permalink
Add optional format flag for listaccounthistory (#1330)
Browse files Browse the repository at this point in the history 
* Add optional flag includeTokenId for listaccounthistory

* Fix listaccounts pagination

* Modifies flag for formating amounts in listaccounthistory

* Fix circular dependency error lint

* getaccount to maintain previous behaviour

* Return to default behaviour not to break other rpcs

* Fix typo on default description

* Refactor amountFormat to format

* Simplify enum

Co-authored-by: Prasanna Loganathar <[email protected]>
  • Loading branch information
@dcorral @prasannavl
dcorral and prasannavl authored Jun 25, 2022
1 parent a550863 commit 735849f
Show file tree
Hide file tree
Showing 7 changed files with 92 additions and 29 deletions.
3 changes: 2 additions & 1 deletion src/masternodes/govvariables/attributes.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Distributed under the MIT software license, see the accompanying
// file LICENSE or http://www.opensource.org/licenses/mit-license.php.

#include <masternodes/mn_rpc.h>
#include <masternodes/govvariables/attributes.h>

#include <masternodes/accountshistory.h> /// CAccountsHistoryWriter
Expand All @@ -13,7 +14,7 @@
#include <core_io.h> /// ValueFromAmount
#include <util/strencodings.h>

extern UniValue AmountsToJSON(TAmounts const & diffs);
extern UniValue AmountsToJSON(TAmounts const & diffs, AmountFormat format = AmountFormat::Symbol);

static inline std::string trim_all_ws(std::string s) {
s.erase(s.begin(), std::find_if(s.begin(), s.end(), [](unsigned char ch) {
Expand Down
10 changes: 10 additions & 0 deletions src/masternodes/mn_rpc.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,16 @@ typedef enum {
SelectionPie,
} AccountSelectionMode;

enum class AmountFormat : uint8_t {
Unknown,
// amount@0
Id,
// amount@DFI
Symbol,
// amount@0#DFI
Combined,
};

class CWalletCoinsUnlocker {
std::shared_ptr<CWallet> pwallet;
std::vector<COutPoint> coins;
Expand Down
70 changes: 52 additions & 18 deletions src/masternodes/rpc_accounts.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,22 +2,38 @@
#include <masternodes/govvariables/attributes.h>
#include <masternodes/mn_rpc.h>

std::string tokenAmountString(CTokenAmount const& amount) {
std::string tokenAmountString(CTokenAmount const& amount, AmountFormat format = AmountFormat::Symbol) {
const auto token = pcustomcsview->GetToken(amount.nTokenId);
const auto valueString = ValueFromAmount(amount.nValue).getValStr();
return valueString + "@" + token->CreateSymbolKey(amount.nTokenId);
const auto amountString = ValueFromAmount(amount.nValue).getValStr();

std::string tokenStr = {};
switch (format) {
case AmountFormat::Id:
tokenStr = amount.nTokenId.ToString();
break;
case AmountFormat::Symbol:
tokenStr = token->CreateSymbolKey(amount.nTokenId);
break;
case AmountFormat::Combined:
tokenStr = amount.nTokenId.ToString() + "#" + token->CreateSymbolKey(amount.nTokenId);
break;
case AmountFormat::Unknown:
tokenStr = "unknown";
break;
}
return amountString + "@" + tokenStr;
}

UniValue AmountsToJSON(TAmounts const & diffs) {
UniValue AmountsToJSON(TAmounts const & diffs, AmountFormat format = AmountFormat::Symbol) {
UniValue obj(UniValue::VARR);

for (auto const & diff : diffs) {
obj.push_back(tokenAmountString({diff.first, diff.second}));
obj.push_back(tokenAmountString({diff.first, diff.second}, format));
}
return obj;
}

UniValue accountToJSON(CScript const& owner, CTokenAmount const& amount, bool verbose, bool indexed_amounts) {
UniValue accountToJSON(CScript const& owner, CTokenAmount const& amount, bool verbose, bool indexed_amounts,AmountFormat format = AmountFormat::Symbol) {
// encode CScript into JSON
UniValue ownerObj(UniValue::VOBJ);
ScriptPubKeyToUniv(owner, ownerObj, true);
Expand All @@ -40,13 +56,13 @@ UniValue accountToJSON(CScript const& owner, CTokenAmount const& amount, bool ve
obj.pushKV("amount", amountObj);
}
else {
obj.pushKV("amount", tokenAmountString(amount));
obj.pushKV("amount", tokenAmountString(amount, format));
}

return obj;
}

UniValue accounthistoryToJSON(AccountHistoryKey const & key, AccountHistoryValue const & value) {
UniValue accounthistoryToJSON(AccountHistoryKey const & key, AccountHistoryValue const & value, AmountFormat format = AmountFormat::Symbol) {
UniValue obj(UniValue::VOBJ);

obj.pushKV("owner", ScriptToString(key.owner));
Expand All @@ -58,11 +74,11 @@ UniValue accounthistoryToJSON(AccountHistoryKey const & key, AccountHistoryValue
obj.pushKV("type", ToString(CustomTxCodeToType(value.category)));
obj.pushKV("txn", (uint64_t) key.txn);
obj.pushKV("txid", value.txid.ToString());
obj.pushKV("amounts", AmountsToJSON(value.diff));
obj.pushKV("amounts", AmountsToJSON(value.diff, format));
return obj;
}

UniValue rewardhistoryToJSON(CScript const & owner, uint32_t height, DCT_ID const & poolId, RewardType type, CTokenAmount amount) {
UniValue rewardhistoryToJSON(CScript const & owner, uint32_t height, DCT_ID const & poolId, RewardType type, CTokenAmount amount, AmountFormat format = AmountFormat::Id) {
UniValue obj(UniValue::VOBJ);
obj.pushKV("owner", ScriptToString(owner));
obj.pushKV("blockHeight", (uint64_t) height);
Expand All @@ -76,11 +92,11 @@ UniValue rewardhistoryToJSON(CScript const & owner, uint32_t height, DCT_ID cons
}
obj.pushKV("poolID", poolId.ToString());
TAmounts amounts({{amount.nTokenId,amount.nValue}});
obj.pushKV("amounts", AmountsToJSON(amounts));
obj.pushKV("amounts", AmountsToJSON(amounts, format));
return obj;
}

UniValue outputEntryToJSON(COutputEntry const & entry, CBlockIndex const * index, CWalletTx const * pwtx) {
UniValue outputEntryToJSON(COutputEntry const & entry, CBlockIndex const * index, CWalletTx const * pwtx, AmountFormat format = AmountFormat::Symbol) {
UniValue obj(UniValue::VOBJ);

obj.pushKV("owner", EncodeDestination(entry.destination));
Expand All @@ -97,7 +113,7 @@ UniValue outputEntryToJSON(COutputEntry const & entry, CBlockIndex const * index
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));
obj.pushKV("amounts", AmountsToJSON(amounts, format));
return obj;
}

Expand Down Expand Up @@ -968,6 +984,9 @@ UniValue listaccounthistory(const JSONRPCRequest& request) {
"Maximum number of records to return, 100 by default"},
{"txn", RPCArg::Type::NUM, RPCArg::Optional::OMITTED,
"Order in block, unlimited by default"},
{"format", RPCArg::Type::STR, RPCArg::Optional::OMITTED,
"Return amounts with the following: 'id' -> <amount>@id; (default)'symbol' -> <amount>@symbol"},

},
},
},
Expand Down Expand Up @@ -998,6 +1017,7 @@ UniValue listaccounthistory(const JSONRPCRequest& request) {
uint32_t limit = 100;
auto txType = CustomTxType::None;
uint32_t txn = std::numeric_limits<uint32_t>::max();
AmountFormat format = AmountFormat::Symbol;

if (request.params.size() > 1) {
UniValue optionsObj = request.params[1].get_obj();
Expand All @@ -1010,6 +1030,7 @@ UniValue listaccounthistory(const JSONRPCRequest& request) {
{"txtype", UniValueType(UniValue::VSTR)},
{"limit", UniValueType(UniValue::VNUM)},
{"txn", UniValueType(UniValue::VNUM)},
{"format", UniValueType(UniValue::VSTR)}
}, true, true);

if (!optionsObj["maxBlockHeight"].isNull()) {
Expand Down Expand Up @@ -1045,6 +1066,19 @@ UniValue listaccounthistory(const JSONRPCRequest& request) {
if (!optionsObj["txn"].isNull()) {
txn = (uint32_t) optionsObj["txn"].get_int64();
}

if (!optionsObj["format"].isNull()) {
const auto formatStr = optionsObj["format"].getValStr();
if (formatStr == "symbol"){
format = AmountFormat::Symbol;
}
else if (formatStr == "id") {
format = AmountFormat::Id;
}
else {
throw JSONRPCError(RPC_INVALID_REQUEST, "format must be one of the following: \"id\", \"symbol\"");
}
}
}

std::function<bool(CScript const &)> isMatchOwner = [](CScript const &) {
Expand Down Expand Up @@ -1145,7 +1179,7 @@ UniValue listaccounthistory(const JSONRPCRequest& request) {

if (accountRecord && (tokenFilter.empty() || hasToken(value.diff))) {
auto& array = ret.emplace(workingHeight, UniValue::VARR).first->second;
array.push_back(accounthistoryToJSON(key, value));
array.push_back(accounthistoryToJSON(key, value, format));
if (shouldSearchInWallet) {
txs.insert(value.txid);
}
Expand All @@ -1157,7 +1191,7 @@ UniValue listaccounthistory(const JSONRPCRequest& request) {
[&](int32_t height, DCT_ID poolId, RewardType type, CTokenAmount amount) {
if (tokenFilter.empty() || hasToken({{amount.nTokenId, amount.nValue}})) {
auto& array = ret.emplace(height, UniValue::VARR).first->second;
array.push_back(rewardhistoryToJSON(key.owner, height, poolId, type, amount));
array.push_back(rewardhistoryToJSON(key.owner, height, poolId, type, amount, format));
count ? --count : 0;
}
}
Expand Down Expand Up @@ -1199,7 +1233,7 @@ UniValue listaccounthistory(const JSONRPCRequest& request) {
return true;
}
auto& array = ret.emplace(index->nHeight, UniValue::VARR).first->second;
array.push_back(outputEntryToJSON(entry, index, pwtx));
array.push_back(outputEntryToJSON(entry, index, pwtx, format));
return --count != 0;
}
);
Expand Down Expand Up @@ -1811,7 +1845,7 @@ UniValue getburninfo(const JSONRPCRequest& request) {
}

for (const auto& kv : Params().GetConsensus().newNonUTXOSubsidies) {
if (kv.first == CommunityAccountType::Unallocated ||
if (kv.first == CommunityAccountType::Unallocated ||
kv.first == CommunityAccountType::IncentiveFunding ||
(height >= fortCanningHeight && kv.first == CommunityAccountType::Loan)) {
burnt += view.GetCommunityBalance(kv.first);
Expand Down Expand Up @@ -1866,7 +1900,7 @@ UniValue getburninfo(const JSONRPCRequest& request) {
};

burnView->ForEachAccountHistory(calculateBurnAmounts);

UniValue result(UniValue::VOBJ);
result.pushKV("address", ScriptToString(burnAddress));
result.pushKV("amount", ValueFromAmount(burntDFI));
Expand Down
4 changes: 2 additions & 2 deletions src/masternodes/rpc_vault.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
#include <masternodes/mn_rpc.h>
#include <masternodes/vaulthistory.h>

extern UniValue AmountsToJSON(TAmounts const & diffs);
extern std::string tokenAmountString(CTokenAmount const& amount);
extern UniValue AmountsToJSON(TAmounts const & diffs, AmountFormat format = AmountFormat::Symbol);
extern std::string tokenAmountString(CTokenAmount const& amount, AmountFormat format = AmountFormat::Symbol);

namespace {

Expand Down
4 changes: 0 additions & 4 deletions test/functional/feature_token_split_usd_value.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,6 @@ def setup_accounts(self):
self.generate_and_fill_accounts()

def add_total_account_to_liquidity_pool(self):
print(f'Adding liquidity with {len(self.accounts)} accounts...')
size = 1000000
for account in self.accounts:
totalAmount = Decimal(self.get_amount_from_account(account, self.symbolDUSD))
Expand All @@ -128,7 +127,6 @@ def add_total_account_to_liquidity_pool(self):
self.nodes[0].addpoolliquidity({account: [str(finalAmount)+"@T1", str(finalAmount)+"@DUSD"]}, account)
self.nodes[0].generate(1)
totalAmount -= finalAmount
print(f'account {account} finished')

def setup_pools(self):
self.nodes[0].createpoolpair({
Expand Down Expand Up @@ -212,14 +210,12 @@ def accounts_usd_values(self):

def compare_value_list(self, pre, post):
for index, amount in enumerate(pre):
print(f'Comparing values in valut {amount["account"]}')
if index != 0:
almost_equal(amount["DUSD"], post[index]["DUSD"])
almost_equal(amount["T1"], post[index]["T1"])

def compare_vaults_list(self, pre, post):
for index, vault in enumerate(pre):
print(f'Comparing valuts {vault["vaultId"]}')
if index != 0:
almost_equal(vault["collateralValue"], post[index]["collateralValue"])
almost_equal(vault["loanValue"], post[index]["loanValue"])
Expand Down
28 changes: 24 additions & 4 deletions test/functional/rpc_getaccounthistory.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,9 @@
"""Test getaccounthistory RPC."""

from test_framework.test_framework import DefiTestFramework
from test_framework.authproxy import JSONRPCException
from test_framework.util import assert_equal

from test_framework.util import (
assert_equal
)

class TokensRPCGetAccountHistory(DefiTestFramework):
def set_test_params(self):
Expand Down Expand Up @@ -53,7 +52,28 @@ def run_test(self):
self.nodes[0].generate(1)

# Get node 0 results
results = self.nodes[0].listaccounthistory(collateral_a)
# test amount@id format
results = self.nodes[0].listaccounthistory(collateral_a, {"format": "id"} )
# test token ids match token symbol
for result in results:
for amount in result["amounts"]:
symbol = amount.split('@')[1]
assert(symbol.isnumeric())

# test amount@symbol format
results = self.nodes[0].listaccounthistory(collateral_a, {"format": "symbol"} )
# test token ids match token symbol
for result in results:
for amount in result["amounts"]:
symbol = amount.split('@')[1]
assert(symbol.isnumeric() == False)

# test amount@symbol format
try:
results = self.nodes[0].listaccounthistory(collateral_a, {"format": "combined"} )
except JSONRPCException as e:
errorString = e.error['message']
assert("format must be one of the following: \"id\", \"symbol\"" in errorString)

# An account history from listaccounthistory and gettaccounthistory must be matched
expected = results[0]
Expand Down
2 changes: 2 additions & 0 deletions test/lint/lint-circular-dependencies.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ EXPECTED_CIRCULAR_DEPENDENCIES=(
"consensus/tx_verify -> masternodes/masternodes -> validation -> consensus/tx_verify"
"consensus/tx_verify -> masternodes/mn_checks -> txmempool -> consensus/tx_verify"
"index/txindex -> validation -> index/txindex"
"init -> miner -> masternodes/govvariables/attributes -> masternodes/mn_rpc -> wallet/rpcwallet -> init"
"masternodes/accountshistory -> masternodes/masternodes -> masternodes/accountshistory"
"masternodes/accountshistory -> masternodes/masternodes -> masternodes/mn_checks -> masternodes/accountshistory"
"masternodes/accountshistory -> masternodes/masternodes -> validation -> masternodes/accountshistory"
Expand All @@ -40,6 +41,7 @@ EXPECTED_CIRCULAR_DEPENDENCIES=(
"masternodes/govvariables/attributes -> masternodes/masternodes -> masternodes/govvariables/attributes"
"masternodes/govvariables/attributes -> masternodes/masternodes -> masternodes/poolpairs -> masternodes/govvariables/attributes"
"masternodes/govvariables/attributes -> masternodes/mn_checks -> masternodes/govvariables/attributes"
"masternodes/govvariables/attributes -> masternodes/mn_rpc -> masternodes/govvariables/attributes"
"masternodes/govvariables/attributes -> validation -> masternodes/govvariables/attributes"
"masternodes/govvariables/icx_takerfee_per_btc -> masternodes/gv -> masternodes/govvariables/icx_takerfee_per_btc"
"masternodes/govvariables/loan_daily_reward -> masternodes/gv -> masternodes/govvariables/loan_daily_reward"
Expand Down

0 comments on commit 735849f

Please sign in to comment.