From 0d49ee4b6c82454a20871c8c835cd89bb53ee221 Mon Sep 17 00:00:00 2001 From: AstryX Date: Mon, 24 Jun 2019 14:46:32 +0100 Subject: [PATCH 1/3] Added additional test checks for rpc errors in onboarding and replaced a boost variant type comparison check method --- qa/rpc-tests/onboard.py | 21 ++++++++++++++++--- qa/rpc-tests/onboardmanual.py | 38 +++++++++++++++++++++++++++++------ src/policy/kycfile.cpp | 2 +- src/policy/whitelist.cpp | 4 ++-- src/qt/addresstablemodel.cpp | 2 +- src/rpc/blockchain.cpp | 9 +++++++-- src/validation.cpp | 2 +- src/wallet/rpcdump.cpp | 2 +- src/wallet/rpcwallet.cpp | 2 +- src/wallet/wallet.cpp | 4 ++-- 10 files changed, 66 insertions(+), 20 deletions(-) diff --git a/qa/rpc-tests/onboard.py b/qa/rpc-tests/onboard.py index 29c93bd09e..cbbdfdc93b 100755 --- a/qa/rpc-tests/onboard.py +++ b/qa/rpc-tests/onboard.py @@ -168,9 +168,15 @@ def run_test (self): assert((balance_1-balance_2) == 0) node1addr=self.nodes[1].getnewaddress() - iswl=self.nodes[0].querywhitelist(node1addr) + + try: + iswl=self.nodes[0].querywhitelist(node1addr) + except JSONRPCException as e: + print(e.error['message']) + assert(False) assert(iswl) + keypool=100 nwhitelisted=keypool @@ -249,7 +255,11 @@ def run_test (self): nlines4=self.linecount(wl1file_4) assert_equal(nlines3+1, nlines4) - iswl=self.nodes[1].querywhitelist(multiAddress2['address']) + try: + iswl=self.nodes[1].querywhitelist(multiAddress2['address']) + except JSONRPCException as e: + print(e.error['message']) + assert(False) assert(iswl) multiAddress1=self.nodes[1].createmultisig(2,[clientAddress1['pubkey'],clientAddress2['pubkey'],clientAddress3['pubkey']]) @@ -263,7 +273,12 @@ def run_test (self): nlines1=self.linecount(wl1file) nlines2=self.linecount(wl1file_2) assert_equal(nlines1+1,nlines2) - iswl=self.nodes[1].querywhitelist(multiAddress1['address']) + + try: + iswl=self.nodes[1].querywhitelist(multiAddress1['address']) + except JSONRPCException as e: + print(e.error['message']) + assert(False) assert(iswl) if(clientAddress1['pubkey'] == clientAddress1['derivedpubkey']): diff --git a/qa/rpc-tests/onboardmanual.py b/qa/rpc-tests/onboardmanual.py index 4b0d054f39..7906c44cd4 100755 --- a/qa/rpc-tests/onboardmanual.py +++ b/qa/rpc-tests/onboardmanual.py @@ -112,29 +112,55 @@ def run_test (self): onboardAddress2=self.nodes[1].validateaddress(self.nodes[1].getnewaddress()) onboardAddress3=self.nodes[1].validateaddress(self.nodes[1].getnewaddress()) untweakedPubkeys=[onboardAddress1['derivedpubkey'],onboardAddress2['derivedpubkey'],onboardAddress3['derivedpubkey']] - userOnboardPubKey=self.nodes[1].createkycfile(kycfile, [{"address":onboardAddress1['address'],"pubkey":onboardAddress1['derivedpubkey']},{"address":onboardAddress2['address'],"pubkey":onboardAddress2['derivedpubkey']}], [{"nmultisig":2,"pubkeys":untweakedPubkeys}]); - + try: + userOnboardPubKey=self.nodes[1].createkycfile(kycfile, [{"address":onboardAddress1['address'],"pubkey":onboardAddress1['derivedpubkey']},{"address":onboardAddress2['address'],"pubkey":onboardAddress2['derivedpubkey']}], [{"nmultisig":2,"pubkeys":untweakedPubkeys}]); + except JSONRPCException as e: + print(e.error['message']) + assert(False) + self.nodes[0].generate(101) self.sync_all() balance_1=self.nodes[0].getwalletinfo()["balance"]["WHITELIST"] - self.nodes[0].onboarduser(kycfile) + try: + self.nodes[0].onboarduser(kycfile) + except JSONRPCException as e: + print(e.error['message']) + assert(False) os.remove(kycfile) + time.sleep(5) self.nodes[0].generate(101) self.sync_all() + time.sleep(1) + balance_2=self.nodes[0].getwalletinfo()["balance"]["WHITELIST"] #Make sure the onboard transaction fee was zero assert((balance_1-balance_2) == 0) node1addr=self.nodes[1].getnewaddress() - iswl=self.nodes[0].querywhitelist(onboardAddress1['address']) + try: + iswl=self.nodes[0].querywhitelist(onboardAddress1['address']) + except JSONRPCException as e: + print(e.error['message']) + assert(False) + assert(iswl) + + try: + iswl=self.nodes[0].querywhitelist(onboardAddress2['address']) + except JSONRPCException as e: + print(e.error['message']) + assert(False) assert(iswl) + multiAdr=self.nodes[1].createmultisig(2,[onboardAddress1['pubkey'],onboardAddress2['pubkey'],onboardAddress3['pubkey']]) - iswl2=self.nodes[0].querywhitelist(multiAdr['address']) + try: + iswl2=self.nodes[0].querywhitelist(multiAdr['address']) + except JSONRPCException as e: + print(e.error['message']) + assert(False) assert(iswl2) - return if __name__ == '__main__': diff --git a/src/policy/kycfile.cpp b/src/policy/kycfile.cpp index d24f2cd8ca..25919501a8 100644 --- a/src/policy/kycfile.cpp +++ b/src/policy/kycfile.cpp @@ -224,7 +224,7 @@ void CKYCFile::parseMultisig(const std::vector vstr, const std::str //Will throw an error if address is not a valid derived address. CTxDestination multiKeyId; multiKeyId = address.Get(); - if (!boost::get(&multiKeyId)){ + if (!(multiKeyId.which() == ((CTxDestination)CNoDestination()).which())){ if(!Consensus::CheckValidTweakedAddress(multiKeyId, pubKeys, nMultisig)){ _decryptedStream << line << ": invalid key tweaking\n"; return; diff --git a/src/policy/whitelist.cpp b/src/policy/whitelist.cpp index 814d3a33eb..f3a4cf3608 100644 --- a/src/policy/whitelist.cpp +++ b/src/policy/whitelist.cpp @@ -89,7 +89,7 @@ void CWhiteList::add_derived(const CBitcoinAddress& address, const CPubKey& pub //Will throw an error if address is not a valid derived address. CTxDestination keyId; keyId = address.Get(); - if (boost::get(&keyId)) + if (keyId.which() == ((CTxDestination)CNoDestination()).which()) throw std::invalid_argument(std::string(std::string(__func__) + ": invalid key id")); @@ -162,7 +162,7 @@ void CWhiteList::add_multisig_whitelist(const CBitcoinAddress& address, const st //Will throw an error if address is not a valid derived address. CTxDestination keyId; keyId = address.Get(); - if (boost::get(&keyId)) + if (keyId.which() == ((CTxDestination)CNoDestination()).which()) throw std::invalid_argument(std::string(std::string(__func__) + ": invalid key id")); diff --git a/src/qt/addresstablemodel.cpp b/src/qt/addresstablemodel.cpp index 05d6bb5c4e..7f248cc71a 100644 --- a/src/qt/addresstablemodel.cpp +++ b/src/qt/addresstablemodel.cpp @@ -260,7 +260,7 @@ bool AddressTableModel::setData(const QModelIndex &index, const QVariant &value, } else if(index.column() == Address) { CTxDestination newAddress = CBitcoinAddress(value.toString().toStdString()).Get(); // Refuse to set invalid address, set error status and return false - if(boost::get(&newAddress)) + if (newAddress.which() == ((CTxDestination)CNoDestination()).which()) { editStatus = INVALID_ADDRESS; return false; diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 5c807d7a7c..1325411a04 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -1871,8 +1871,8 @@ UniValue querywhitelist(const JSONRPCRequest& request) //Will throw an error if address is not a valid derived address. CTxDestination keyId; keyId = address.Get(); - if (boost::get(&keyId)) - throw std::invalid_argument(std::string(std::string(__func__) + + if (keyId.which() == ((CTxDestination)CNoDestination()).which()) + throw std::invalid_argument(std::string(std::string(__func__) + ": invalid key id")); return addressWhitelist.is_whitelisted(keyId); @@ -1899,8 +1899,13 @@ UniValue removefromwhitelist(const JSONRPCRequest& request) //Will throw an error if address is not a valid derived address. CTxDestination keyId; keyId = address.Get(); +<<<<<<< Updated upstream if (boost::get(&keyId)) throw std::invalid_argument(std::string(std::string(__func__) + +======= + if (keyId.which() == ((CTxDestination)CNoDestination()).which()) + throw std::invalid_argument(std::string(std::string(__func__) + +>>>>>>> Stashed changes ": invalid key id")); addressWhitelist.remove(keyId); diff --git a/src/validation.cpp b/src/validation.cpp index 3a15040043..4d499a53e5 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1855,7 +1855,7 @@ bool CheckValidTweakedAddress(const CTxDestination keyID, const std::vector(&multiKeyId)) + if (multiKeyId.which() == ((CTxDestination)CNoDestination()).which()) return false; if (!(multiKeyId == destCopy)) diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index c40a9ea0aa..ea4024466e 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -883,7 +883,7 @@ UniValue createkycfile(const JSONRPCRequest& request) //Will skip if address is not a valid tweaked address. CTxDestination multiKeyId; multiKeyId = address.Get(); - if (boost::get(&multiKeyId)) + if (multiKeyId.which() == ((CTxDestination)CNoDestination()).which()) continue; ss << strprintf("%d %s", diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 34abdc2ea9..69a0965e8a 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -699,7 +699,7 @@ static void SendAddNextMultiToWhitelistTx(const CAsset& feeAsset, const CPubKey& CRegisterAddressScript* raScript = new CRegisterAddressScript(RA_MULTISIG); CTxDestination keyid = address.Get(); - if (boost::get(&keyid)) + if (keyid.which() == ((CTxDestination)CNoDestination()).which()) throw JSONRPCError(RPC_INVALID_PARAMETER, std::string(std::string(__func__) + ": invalid key id")); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index fa5e02ea06..45c34077f6 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2956,7 +2956,7 @@ bool CWallet::CreateTransaction(const vector& vecSend, CWalletTx& wt CScript scriptChange; // coin control: send change to custom address - if (coinControl && !boost::get(&coinControl->destChange)) + if (coinControl && !((coinControl->destChange).which() == ((CTxDestination)CNoDestination()).which())) scriptChange = GetScriptForDestination(coinControl->destChange); // no coin control: send change to newly generated address @@ -4168,7 +4168,7 @@ void CWallet::GetKeyBirthTimes(std::map &mapKeyBirth) c bool CWallet::AddDestData(const CTxDestination &dest, const std::string &key, const std::string &value) { - if (boost::get(&dest)) + if (dest.which() == ((CTxDestination)CNoDestination()).which()) return false; mapAddressBook[dest].destdata.insert(std::make_pair(key, value)); From 1c90e3b9f21097e5d28688e4fae625b5a1de0225 Mon Sep 17 00:00:00 2001 From: AstryX Date: Mon, 24 Jun 2019 14:52:39 +0100 Subject: [PATCH 2/3] Finished merge conflict resolution --- src/rpc/blockchain.cpp | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 1325411a04..377e9172ef 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -1899,13 +1899,8 @@ UniValue removefromwhitelist(const JSONRPCRequest& request) //Will throw an error if address is not a valid derived address. CTxDestination keyId; keyId = address.Get(); -<<<<<<< Updated upstream - if (boost::get(&keyId)) - throw std::invalid_argument(std::string(std::string(__func__) + -======= if (keyId.which() == ((CTxDestination)CNoDestination()).which()) throw std::invalid_argument(std::string(std::string(__func__) + ->>>>>>> Stashed changes ": invalid key id")); addressWhitelist.remove(keyId); From deb75c55941fa572243457a59a48cac075ebcba6 Mon Sep 17 00:00:00 2001 From: AstryX Date: Mon, 24 Jun 2019 16:03:58 +0100 Subject: [PATCH 3/3] Minor comment change --- src/rpc/blockchain.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 6ffd3c3c88..20082cc4b7 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -1876,6 +1876,7 @@ UniValue querywhitelist(const JSONRPCRequest& request) //Will throw an error if address is not a valid derived address. CTxDestination keyId; keyId = address.Get(); + //The which function for boost variant returns an index which represents the type of variant in order. if (keyId.which() == ((CTxDestination)CNoDestination()).which()) throw std::invalid_argument(std::string(std::string(__func__) + ": invalid key id"));