Skip to content

Commit

Permalink
Fix for importaddress returning incorrect amounts (#479)
Browse files Browse the repository at this point in the history
* Fix for importaddress returning incorrect amounts

* Updated tests case to test cold staking address import as well

* Updated UI and functional tests to match discussed changes on PR #479

* Updated var names

* Updated test case

* Updated test case to test each address on different nodes

* Added check for spending/staking address against watched addresses on cold staking transactions
  • Loading branch information
francisjyap authored and alex v committed May 21, 2019
1 parent 926c49a commit b115c2d
Show file tree
Hide file tree
Showing 7 changed files with 181 additions and 3 deletions.
1 change: 1 addition & 0 deletions qa/pull-tester/rpc-tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,7 @@
'reject-version-bit.py',
'getcoldstakingaddress.py',
'getstakereport.py',
'importaddress.py',
'getstakinginfo.py',
'coldstaking_staking.py',
'coldstaking_spending.py',
Expand Down
88 changes: 88 additions & 0 deletions qa/rpc-tests/importaddress.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
#!/usr/bin/env python3
# Copyright (c) 2018 The Navcoin Core developers
# Distributed under the MIT software license, see the accompanying
# file COPYING or http://www.opensource.org/licenses/mit-license.php.

from test_framework.test_framework import NavCoinTestFramework
from test_framework.staticr_util import *

class ImportAddressTest(NavCoinTestFramework):

def __init__(self):
super().__init__()
self.setup_clean_chain = True
self.num_nodes = 6

def setup_network(self, split=False):
self.nodes = self.setup_nodes()
connect_nodes(self.nodes[0], 1)
connect_nodes(self.nodes[1], 2)
connect_nodes(self.nodes[2], 3)
connect_nodes(self.nodes[3], 4)
connect_nodes(self.nodes[4], 5)
connect_nodes(self.nodes[5], 0)
self.is_network_split = False

def run_test(self):
# Turn off staking until we need it
self.nodes[0].staking(False)
self.nodes[1].staking(False)
self.nodes[2].staking(False)
self.nodes[3].staking(False)
self.nodes[4].staking(False)
self.nodes[5].staking(False)

# Generate genesis block
activate_staticr(self.nodes[0])
self.sync_all()

# Create a normal address
address = self.nodes[0].getnewaddress()

# Create a spending address
spending_address = self.nodes[0].getnewaddress()

# Create a staking address
staking_address = self.nodes[1].getnewaddress()

# Create the cold address
coldstaking_address = self.nodes[0].getcoldstakingaddress(staking_address, spending_address)

# Send some nav to new addresses
self.nodes[0].sendtoaddress(address, 512)
self.nodes[0].sendtoaddress(coldstaking_address, 256)
self.nodes[0].sendtoaddress(coldstaking_address, 128)
self.nodes[0].generate(6)
self.sync_all()

# Import address with balances
self.nodes[2].importaddress(address)
self.nodes[3].importaddress(spending_address)
self.nodes[4].importaddress(staking_address)
self.nodes[5].importaddress(coldstaking_address)

# Assert transactions list has the old transactions with correct amounts
transactions = self.nodes[2].listtransactions("*", 1, 0, True)
assert_equal(512, transactions[0]['amount'])

transactions = self.nodes[3].listtransactions("*", 2, 0, True)
assert_equal(128, transactions[0]['amount'])
assert_equal(256, transactions[1]['amount'])

transactions = self.nodes[4].listtransactions("*", 2, 0, True)
assert_equal(128, transactions[0]['amount'])
assert_equal(256, transactions[1]['amount'])

transactions = self.nodes[5].listtransactions("*", 2, 0, True)
assert_equal(128, transactions[0]['amount'])
assert_equal(256, transactions[1]['amount'])

# Assert total balance
assert_equal(512, self.nodes[2].getbalance("*", 1, True))
assert_equal(384, self.nodes[3].getbalance("*", 1, True))
assert_equal(384, self.nodes[4].getbalance("*", 1, True))
assert_equal(384, self.nodes[5].getbalance("*", 1, True))


if __name__ == '__main__':
ImportAddressTest().main()
64 changes: 62 additions & 2 deletions src/qt/forms/overviewpage.ui
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,66 @@
</property>
</widget>
</item>
<item row="4" column="1">
<widget class="QLabel" name="labelWatchedBalance">
<property name="font">
<font>
<pointsize>10</pointsize>
<weight>50</weight>
<bold>false</bold>
<stylestrategy>PreferAntialias</stylestrategy>
</font>
</property>
<property name="styleSheet">
<string notr="true">color: #727272</string>
</property>
<property name="text">
<string>TextLabel</string>
</property>
<property name="alignment">
<set>Qt::AlignLeading|Qt::AlignLeft|Qt::AlignVCenter</set>
</property>
</widget>
</item>
<item row="4" column="0">
<widget class="QLabel" name="labelWatchedBalanceText">
<property name="sizePolicy">
<sizepolicy hsizetype="Minimum" vsizetype="Preferred">
<horstretch>0</horstretch>
<verstretch>0</verstretch>
</sizepolicy>
</property>
<property name="minimumSize">
<size>
<width>70</width>
<height>17</height>
</size>
</property>
<property name="maximumSize">
<size>
<width>70</width>
<height>17</height>
</size>
</property>
<property name="font">
<font>
<pointsize>10</pointsize>
<weight>50</weight>
<bold>false</bold>
<stylestrategy>PreferAntialias</stylestrategy>
</font>
</property>
<property name="styleSheet">
<string notr="true">color: #727272</string>
</property>
<property name="text">
<string>Watched</string>
</property>
<property name="alignment">
<set>Qt::AlignLeading|Qt::AlignLeft|Qt::AlignVCenter</set>
</property>
</widget>
</item>
<item row="2" column="1">
<widget class="QLabel" name="labelStaking">
<property name="font">
Expand Down Expand Up @@ -409,7 +469,7 @@
</property>
</widget>
</item>
<item row="4" column="1">
<item row="5" column="1">
<widget class="QLabel" name="labelTotal">
<property name="font">
<font>
Expand All @@ -430,7 +490,7 @@
</property>
</widget>
</item>
<item row="4" column="0">
<item row="5" column="0">
<widget class="QLabel" name="label_8">
<property name="sizePolicy">
<sizepolicy hsizetype="Minimum" vsizetype="Preferred">
Expand Down
12 changes: 11 additions & 1 deletion src/qt/overviewpage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -119,9 +119,11 @@ OverviewPage::OverviewPage(const PlatformStyle *platformStyle, QWidget *parent)
currentStakingBalance(-1),
currentColdStakingBalance(-1),
currentImmatureBalance(-1),
currentTotalBalance(-1),
currentWatchOnlyBalance(-1),
currentWatchUnconfBalance(-1),
currentWatchImmatureBalance(-1),
currentWatchOnlyTotalBalance(-1),
txdelegate(new TxViewDelegate(platformStyle)),
filter(0)
{
Expand Down Expand Up @@ -179,15 +181,18 @@ void OverviewPage::setBalance(const CAmount& balance, const CAmount& unconfirmed
currentStakingBalance = stakingBalance;
currentColdStakingBalance = coldStakingBalance;
currentImmatureBalance = immatureBalance;
currentTotalBalance = balance + unconfirmedBalance + immatureBalance;
currentWatchOnlyBalance = watchOnlyBalance;
currentWatchUnconfBalance = watchUnconfBalance;
currentWatchImmatureBalance = watchImmatureBalance;
currentWatchOnlyTotalBalance = watchOnlyBalance + watchUnconfBalance + watchImmatureBalance;
ui->labelBalance->setText(NavCoinUnits::formatWithUnit(unit, balance, false, NavCoinUnits::separatorAlways));
ui->labelUnconfirmed->setText(NavCoinUnits::formatWithUnit(unit, unconfirmedBalance, false, NavCoinUnits::separatorAlways));
ui->labelStaking->setText(NavCoinUnits::formatWithUnit(unit, stakingBalance, false, NavCoinUnits::separatorAlways));
ui->labelColdStaking->setText(NavCoinUnits::formatWithUnit(unit, coldStakingBalance, false, NavCoinUnits::separatorAlways));
ui->labelImmature->setText(NavCoinUnits::formatWithUnit(unit, immatureBalance, false, NavCoinUnits::separatorAlways));
ui->labelTotal->setText(NavCoinUnits::formatWithUnit(unit, balance + unconfirmedBalance + stakingBalance, false, NavCoinUnits::separatorAlways));
ui->labelWatchedBalance->setText(NavCoinUnits::formatWithUnit(unit, currentWatchOnlyTotalBalance, false, NavCoinUnits::separatorAlways));
ui->labelTotal->setText(NavCoinUnits::formatWithUnit(unit, currentTotalBalance + currentWatchOnlyTotalBalance, false, NavCoinUnits::separatorAlways));

updateStakeReportNow();

Expand All @@ -201,6 +206,11 @@ void OverviewPage::setBalance(const CAmount& balance, const CAmount& unconfirmed
ui->labelColdStaking->setVisible(showColdStaking);
ui->labelColdStakingText->setVisible(showColdStaking);

bool showWatchOnly = currentWatchOnlyTotalBalance != 0;

ui->labelWatchedBalance->setVisible(showWatchOnly);
ui->labelWatchedBalanceText->setVisible(showWatchOnly);

// only show immature (newly mined) balance if it's non-zero, so as not to complicate things
// for the non-mining users
bool showImmature = false;
Expand Down
2 changes: 2 additions & 0 deletions src/qt/overviewpage.h
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,11 @@ public Q_SLOTS:
CAmount currentStakingBalance;
CAmount currentColdStakingBalance;
CAmount currentImmatureBalance;
CAmount currentTotalBalance;
CAmount currentWatchOnlyBalance;
CAmount currentWatchUnconfBalance;
CAmount currentWatchImmatureBalance;
CAmount currentWatchOnlyTotalBalance;

TxViewDelegate *txdelegate;
TransactionFilterProxy *filter;
Expand Down
15 changes: 15 additions & 0 deletions src/script/ismine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

#include "ismine.h"

#include "base58.h"
#include "key.h"
#include "keystore.h"
#include "script/script.h"
Expand Down Expand Up @@ -79,6 +80,20 @@ isminetype IsMine(const CKeyStore &keystore, const CScript& scriptPubKey)
return ISMINE_SPENDABLE;
else if (fStakable)
return ISMINE_STAKABLE;

// if spending/staking address is being watched
CNavCoinAddress spendingAddress(keyID);
CNavCoinAddress stakingAddress(keyID2);
CScript spendingScript = GetScriptForDestination(spendingAddress.Get());
CScript stakingScript = GetScriptForDestination(stakingAddress.Get());
SignatureData sigs;

if (keystore.HaveWatchOnly(spendingScript)) {
return ProduceSignature(DummySignatureCreator(&keystore), spendingScript, sigs) ? ISMINE_WATCH_SOLVABLE : ISMINE_WATCH_UNSOLVABLE;
} else if (keystore.HaveWatchOnly(stakingScript)) {
return ProduceSignature(DummySignatureCreator(&keystore), stakingScript, sigs) ? ISMINE_WATCH_SOLVABLE : ISMINE_WATCH_UNSOLVABLE;
}

break;
}
case TX_SCRIPTHASH:
Expand Down
2 changes: 2 additions & 0 deletions src/wallet/rpcdump.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,8 @@ UniValue importaddress(const UniValue& params, bool fHelp)
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid NavCoin address or script");
}

pwalletMain->nTimeFirstKey = 1; // 0 would be considered 'no value'

if (fRescan)
{
pwalletMain->ScanForWalletTransactions(chainActive.Genesis(), true);
Expand Down

0 comments on commit b115c2d

Please sign in to comment.