diff options
author | Russell Yanofsky <russ@yanofsky.org> | 2017-10-26 07:10:59 -0400 |
---|---|---|
committer | Wladimir J. van der Laan <laanwj@gmail.com> | 2017-11-01 14:15:59 +0100 |
commit | 7d4546f17dca84928bd2b6d1e2588b673c237321 (patch) | |
tree | f359f7d7ddcfac17083bcb3fb242801d50485433 | |
parent | cf18f4289911c657eb876d91dee055db807870ad (diff) | |
download | bitcoin-7d4546f17dca84928bd2b6d1e2588b673c237321.tar.xz |
Make listsinceblock refuse unknown block hash
Change suggested by Cory Fields <cory-nospam-@coryfields.com> who noticed
listsinceblock would ignore invalid block hashes causing it to return a
completely unfiltered list of transactions.
Github-Pull: #11565
Rebased-From: 659b2061c4329472a45e913c5d45e6ab180600a3
Tree-SHA512: 2091a830b730421b49c806cb83a16c7da2ec0a7adac2bac0585324aad12a32bb99a840264c3d346937ea84786fac56e44befb6641511a417977803875efe5a21
-rw-r--r-- | doc/release-notes.md | 4 | ||||
-rw-r--r-- | src/wallet/rpcwallet.cpp | 19 | ||||
-rwxr-xr-x | test/functional/listsinceblock.py | 35 |
3 files changed, 48 insertions, 10 deletions
diff --git a/doc/release-notes.md b/doc/release-notes.md index 0235e1c606..1bb02e3761 100644 --- a/doc/release-notes.md +++ b/doc/release-notes.md @@ -68,6 +68,10 @@ Notable changes - `dumpwallet` no longer allows overwriting files. This is a security measure as well as prevents dangerous user mistakes. +- `listsinceblock` will now throw an error if an unknown `blockhash` argument + value is passed, instead of returning a list of all wallet transactions since + the genesis block. + Credits ======= diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index a6176c3485..6efbc23281 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -1823,19 +1823,20 @@ UniValue listsinceblock(const JSONRPCRequest& request) int target_confirms = 1; isminefilter filter = ISMINE_SPENDABLE; - if (!request.params[0].isNull()) { + if (!request.params[0].isNull() && !request.params[0].get_str().empty()) { uint256 blockId; blockId.SetHex(request.params[0].get_str()); BlockMap::iterator it = mapBlockIndex.find(blockId); - if (it != mapBlockIndex.end()) { - paltindex = pindex = it->second; - if (chainActive[pindex->nHeight] != pindex) { - // the block being asked for is a part of a deactivated chain; - // we don't want to depend on its perceived height in the block - // chain, we want to instead use the last common ancestor - pindex = chainActive.FindFork(pindex); - } + if (it == mapBlockIndex.end()) { + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Block not found"); + } + paltindex = pindex = it->second; + if (chainActive[pindex->nHeight] != pindex) { + // the block being asked for is a part of a deactivated chain; + // we don't want to depend on its perceived height in the block + // chain, we want to instead use the last common ancestor + pindex = chainActive.FindFork(pindex); } } diff --git a/test/functional/listsinceblock.py b/test/functional/listsinceblock.py index 6f428388ec..67e7744bf8 100755 --- a/test/functional/listsinceblock.py +++ b/test/functional/listsinceblock.py @@ -5,7 +5,7 @@ """Test the listsincelast RPC.""" from test_framework.test_framework import BitcoinTestFramework -from test_framework.util import assert_equal +from test_framework.util import assert_equal, assert_array_result, assert_raises_rpc_error class ListSinceBlockTest (BitcoinTestFramework): def set_test_params(self): @@ -16,10 +16,43 @@ class ListSinceBlockTest (BitcoinTestFramework): self.nodes[2].generate(101) self.sync_all() + self.test_no_blockhash() + self.test_invalid_blockhash() self.test_reorg() self.test_double_spend() self.test_double_send() + def test_no_blockhash(self): + txid = self.nodes[2].sendtoaddress(self.nodes[0].getnewaddress(), 1) + blockhash, = self.nodes[2].generate(1) + self.sync_all() + + txs = self.nodes[0].listtransactions() + assert_array_result(txs, {"txid": txid}, { + "category": "receive", + "amount": 1, + "blockhash": blockhash, + "confirmations": 1, + }) + assert_equal( + self.nodes[0].listsinceblock(), + {"lastblock": blockhash, + "removed": [], + "transactions": txs}) + assert_equal( + self.nodes[0].listsinceblock(""), + {"lastblock": blockhash, + "removed": [], + "transactions": txs}) + + def test_invalid_blockhash(self): + assert_raises_rpc_error(-5, "Block not found", self.nodes[0].listsinceblock, + "42759cde25462784395a337460bde75f58e73d3f08bd31fdc3507cbac856a2c4") + assert_raises_rpc_error(-5, "Block not found", self.nodes[0].listsinceblock, + "0000000000000000000000000000000000000000000000000000000000000000") + assert_raises_rpc_error(-5, "Block not found", self.nodes[0].listsinceblock, + "invalid-hex") + def test_reorg(self): ''' `listsinceblock` did not behave correctly when handed a block that was |