Merge #19655: rpc: Catch listsinceblock target_confirmations exceeding block count

c133cdcdc3397a734d57e05494682bf9bf6f4c15 Cap listsinceblock target_confirmations param (Adam Stein)

Pull request description:

  This addresses an issue brought up in #19587.

  Currently, the `target_confirmations` parameter to `listsinceblock` is not checked for being too large. When `target_confirmations` is greater than one more than the current number of blocks, `listsinceblock` fails with error code -1. In comparison, when `target_confirmations` is less than 1,  a -8 "Invalid parameter" error code is thrown.

  This PR fixes the issue by returning a -8 "Invalid parameter" error if the `target_confirmations` value corresponds to a block with more confirmations than the genesis block. This happens if `target_confirmations` exceeds one more than the number of blocks.

ACKs for top commit:
  laanwj:
    Code review ACK c133cdcdc3397a734d57e05494682bf9bf6f4c15
  ryanofsky:
    Code review ACK c133cdcdc3397a734d57e05494682bf9bf6f4c15. Just suggested changes since last review. Thanks!

Tree-SHA512: 02680f4cb937d2c24d5019abd0ebfa188b8a50679a1e64e9c26bfe5c17eef6aea906832e6e2d492ba8a2ea160041bf185d66795ee691e340f6793db03c21b89a
This commit is contained in:
Wladimir J. van der Laan 2020-08-13 11:03:47 +02:00
commit 6757b3ac8f
No known key found for this signature in database
GPG Key ID: 1E4AED62986CD25D
2 changed files with 24 additions and 1 deletions

View File

@ -1484,7 +1484,7 @@ static UniValue listsinceblock(const JSONRPCRequest& request)
{RPCResult::Type::ARR, "removed", "<structure is the same as \"transactions\" above, only present if include_removed=true>\n"
"Note: transactions that were re-added in the active chain will appear as-is in this array, and may thus have a positive confirmation count."
, {{RPCResult::Type::ELISION, "", ""},}},
{RPCResult::Type::STR_HEX, "lastblock", "The hash of the block (target_confirmations-1) from the best block on the main chain. This is typically used to feed back into listsinceblock the next time you call it. So you would generally use a target_confirmations of say 6, so you will be continually re-notified of transactions until they've reached 6 confirmations plus any new ones"},
{RPCResult::Type::STR_HEX, "lastblock", "The hash of the block (target_confirmations-1) from the best block on the main chain, or the genesis hash if the referenced block does not exist yet. This is typically used to feed back into listsinceblock the next time you call it. So you would generally use a target_confirmations of say 6, so you will be continually re-notified of transactions until they've reached 6 confirmations plus any new ones"},
}
},
RPCExamples{
@ -1567,6 +1567,7 @@ static UniValue listsinceblock(const JSONRPCRequest& request)
}
uint256 lastblock;
target_confirms = std::min(target_confirms, wallet.GetLastBlockHeight() + 1);
CHECK_NONFATAL(wallet.chain().findAncestorByHeight(wallet.GetLastBlockHash(), wallet.GetLastBlockHeight() + 1 - target_confirms, FoundBlock().hash(lastblock)));
UniValue ret(UniValue::VOBJ);

View File

@ -36,6 +36,7 @@ class ListSinceBlockTest(BitcoinTestFramework):
self.test_double_spend()
self.test_double_send()
self.double_spends_filtered()
self.test_targetconfirmations()
def test_no_blockhash(self):
self.log.info("Test no blockhash")
@ -74,6 +75,27 @@ class ListSinceBlockTest(BitcoinTestFramework):
assert_raises_rpc_error(-8, "blockhash must be hexadecimal string (not 'Z000000000000000000000000000000000000000000000000000000000000000')", self.nodes[0].listsinceblock,
"Z000000000000000000000000000000000000000000000000000000000000000")
def test_targetconfirmations(self):
'''
This tests when the value of target_confirmations exceeds the number of
blocks in the main chain. In this case, the genesis block hash should be
given for the `lastblock` property. If target_confirmations is < 1, then
a -8 invalid parameter error is thrown.
'''
self.log.info("Test target_confirmations")
blockhash, = self.nodes[2].generate(1)
blockheight = self.nodes[2].getblockheader(blockhash)['height']
self.sync_all()
assert_equal(
self.nodes[0].getblockhash(0),
self.nodes[0].listsinceblock(blockhash, blockheight + 1)['lastblock'])
assert_equal(
self.nodes[0].getblockhash(0),
self.nodes[0].listsinceblock(blockhash, blockheight + 1000)['lastblock'])
assert_raises_rpc_error(-8, "Invalid parameter",
self.nodes[0].listsinceblock, blockhash, 0)
def test_reorg(self):
'''
`listsinceblock` did not behave correctly when handed a block that was