From 3ac6dd5f5d3402d3738b6409f091f89c2490e640 Mon Sep 17 00:00:00 2001 From: Alfredo Garcia Date: Tue, 31 Dec 2019 09:16:29 -0300 Subject: [PATCH 1/6] add negative height to getblock --- qa/rpc-tests/merkle_blocks.py | 9 +++++++++ src/rpc/blockchain.cpp | 9 +++++++-- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/qa/rpc-tests/merkle_blocks.py b/qa/rpc-tests/merkle_blocks.py index d9fec26f2..53acfe6ed 100755 --- a/qa/rpc-tests/merkle_blocks.py +++ b/qa/rpc-tests/merkle_blocks.py @@ -97,5 +97,14 @@ class MerkleBlockTest(BitcoinTestFramework): result = self.nodes[0].getblock(blockhash, 0) assert(c in string.hexdigits for c in result) # verbosity 0 returns raw hex + # Test getblock heights including negatives relative to the head + assert_equal(self.nodes[0].getblock("0")["height"], 0) + assert_raises(JSONRPCException, self.nodes[0].getblock, ["108"]) + assert_equal(self.nodes[0].getblock("107")["height"], 107) + assert_equal(self.nodes[0].getblock("106")["height"], 106) + assert_equal(self.nodes[0].getblock("-20")["height"], 87) + assert_equal(self.nodes[0].getblock("-107")["height"], 0) + assert_raises(JSONRPCException, self.nodes[0].getblock, ["-108"]) + if __name__ == '__main__': MerkleBlockTest().main() diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 0f6c6c760..387f58153 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -660,7 +660,7 @@ UniValue getblock(const UniValue& params, bool fHelp) "If verbosity is 1, returns an Object with information about the block.\n" "If verbosity is 2, returns an Object with information about the block and information about each transaction. \n" "\nArguments:\n" - "1. \"hash|height\" (string, required) The block hash or height\n" + "1. \"hash|height\" (string, required) The block hash or height. Height can be negative, relative to the head.\n" "2. verbosity (numeric, optional, default=1) 0 for hex encoded data, 1 for a json object, and 2 for json object with transaction data\n" "\nResult (for verbosity = 0):\n" "\"data\" (string) A string that is serialized, hex-encoded data for the block.\n" @@ -706,7 +706,7 @@ UniValue getblock(const UniValue& params, bool fHelp) // If height is supplied, find the hash if (strHash.size() < (2 * sizeof(uint256))) { // std::stoi allows characters, whereas we want to be strict - regex r("[[:digit:]]+"); + regex r("-?[[:digit:]]+"); if (!regex_match(strHash, r)) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid block height parameter"); } @@ -719,9 +719,14 @@ UniValue getblock(const UniValue& params, bool fHelp) throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid block height parameter"); } + if(nHeight < 0) { + nHeight += chainActive.Height(); + } + if (nHeight < 0 || nHeight > chainActive.Height()) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Block height out of range"); } + strHash = chainActive[nHeight]->GetBlockHash().GetHex(); } From fce6000f76e5fdf70462eb18b37c103947be7b27 Mon Sep 17 00:00:00 2001 From: Alfredo Garcia Date: Tue, 31 Dec 2019 09:35:37 -0300 Subject: [PATCH 2/6] allow negative index to getblockhash --- qa/rpc-tests/merkle_blocks.py | 7 ++++++- src/rpc/blockchain.cpp | 7 ++++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/qa/rpc-tests/merkle_blocks.py b/qa/rpc-tests/merkle_blocks.py index 53acfe6ed..bf333ab1f 100755 --- a/qa/rpc-tests/merkle_blocks.py +++ b/qa/rpc-tests/merkle_blocks.py @@ -101,10 +101,15 @@ class MerkleBlockTest(BitcoinTestFramework): assert_equal(self.nodes[0].getblock("0")["height"], 0) assert_raises(JSONRPCException, self.nodes[0].getblock, ["108"]) assert_equal(self.nodes[0].getblock("107")["height"], 107) - assert_equal(self.nodes[0].getblock("106")["height"], 106) + assert_equal(self.nodes[0].getblock("-1")["height"], 106) + assert_equal(self.nodes[0].getblock("-2")["height"], 105) assert_equal(self.nodes[0].getblock("-20")["height"], 87) assert_equal(self.nodes[0].getblock("-107")["height"], 0) assert_raises(JSONRPCException, self.nodes[0].getblock, ["-108"]) + # Test getblockhash negative heights + hash = self.nodes[0].getblockhash(106) + assert_equal(self.nodes[0].getblockhash(-1), hash) + if __name__ == '__main__': MerkleBlockTest().main() diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 387f58153..7bcd66203 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -576,7 +576,7 @@ UniValue getblockhash(const UniValue& params, bool fHelp) "getblockhash index\n" "\nReturns hash of block in best-block-chain at index provided.\n" "\nArguments:\n" - "1. index (numeric, required) The block index\n" + "1. index (numeric, required) The block index. If negative then the index will be relative to the head.\n" "\nResult:\n" "\"hash\" (string) The block hash\n" "\nExamples:\n" @@ -587,6 +587,11 @@ UniValue getblockhash(const UniValue& params, bool fHelp) LOCK(cs_main); int nHeight = params[0].get_int(); + + if(nHeight < 0) { + nHeight += chainActive.Height(); + } + if (nHeight < 0 || nHeight > chainActive.Height()) throw JSONRPCError(RPC_INVALID_PARAMETER, "Block height out of range"); From 574963ef8724d43e3ab1b8b59fb75a799ff18eec Mon Sep 17 00:00:00 2001 From: Alfredo Garcia Date: Sun, 12 Jan 2020 11:06:46 -0300 Subject: [PATCH 3/6] update docs --- src/rpc/blockchain.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 7bcd66203..b3639a757 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -576,7 +576,7 @@ UniValue getblockhash(const UniValue& params, bool fHelp) "getblockhash index\n" "\nReturns hash of block in best-block-chain at index provided.\n" "\nArguments:\n" - "1. index (numeric, required) The block index. If negative then the index will be relative to the head.\n" + "1. index (numeric, required) The block index. If negative then the index will be: last known valid block - abs(index)\n" "\nResult:\n" "\"hash\" (string) The block hash\n" "\nExamples:\n" @@ -665,7 +665,7 @@ UniValue getblock(const UniValue& params, bool fHelp) "If verbosity is 1, returns an Object with information about the block.\n" "If verbosity is 2, returns an Object with information about the block and information about each transaction. \n" "\nArguments:\n" - "1. \"hash|height\" (string, required) The block hash or height. Height can be negative, relative to the head.\n" + "1. \"hash|height\" (string, required) The block hash or height. Height can be negative, if so: height = last known valid block - abs(height)\n" "2. verbosity (numeric, optional, default=1) 0 for hex encoded data, 1 for a json object, and 2 for json object with transaction data\n" "\nResult (for verbosity = 0):\n" "\"data\" (string) A string that is serialized, hex-encoded data for the block.\n" From 7cb45243fe47588e5182b12ede3fa09dc64e08dc Mon Sep 17 00:00:00 2001 From: Alfredo Garcia Date: Mon, 20 Jan 2020 12:23:52 -0300 Subject: [PATCH 4/6] change convention --- qa/rpc-tests/merkle_blocks.py | 15 ++++++++------- src/rpc/blockchain.cpp | 8 ++++---- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/qa/rpc-tests/merkle_blocks.py b/qa/rpc-tests/merkle_blocks.py index bf333ab1f..ca7af0018 100755 --- a/qa/rpc-tests/merkle_blocks.py +++ b/qa/rpc-tests/merkle_blocks.py @@ -101,15 +101,16 @@ class MerkleBlockTest(BitcoinTestFramework): assert_equal(self.nodes[0].getblock("0")["height"], 0) assert_raises(JSONRPCException, self.nodes[0].getblock, ["108"]) assert_equal(self.nodes[0].getblock("107")["height"], 107) - assert_equal(self.nodes[0].getblock("-1")["height"], 106) - assert_equal(self.nodes[0].getblock("-2")["height"], 105) - assert_equal(self.nodes[0].getblock("-20")["height"], 87) - assert_equal(self.nodes[0].getblock("-107")["height"], 0) - assert_raises(JSONRPCException, self.nodes[0].getblock, ["-108"]) + assert_equal(self.nodes[0].getblock("-1")["height"], 107) + assert_equal(self.nodes[0].getblock("-2")["height"], 106) + assert_equal(self.nodes[0].getblock("-20")["height"], 88) + assert_equal(self.nodes[0].getblock("-107")["height"], 1) + assert_equal(self.nodes[0].getblock("-108")["height"], 0) + assert_raises(JSONRPCException, self.nodes[0].getblock, ["-109"]) # Test getblockhash negative heights - hash = self.nodes[0].getblockhash(106) - assert_equal(self.nodes[0].getblockhash(-1), hash) + assert_equal(self.nodes[0].getblockhash(-1), self.nodes[0].getblockhash(107)) + assert_equal(self.nodes[0].getblockhash(-2), self.nodes[0].getblockhash(106)) if __name__ == '__main__': MerkleBlockTest().main() diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index b3639a757..83f2daeff 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -576,7 +576,7 @@ UniValue getblockhash(const UniValue& params, bool fHelp) "getblockhash index\n" "\nReturns hash of block in best-block-chain at index provided.\n" "\nArguments:\n" - "1. index (numeric, required) The block index. If negative then the index will be: last known valid block - abs(index)\n" + "1. index (numeric, required) The block index. If negative then -1 is the last known valid block\n" "\nResult:\n" "\"hash\" (string) The block hash\n" "\nExamples:\n" @@ -589,7 +589,7 @@ UniValue getblockhash(const UniValue& params, bool fHelp) int nHeight = params[0].get_int(); if(nHeight < 0) { - nHeight += chainActive.Height(); + nHeight += chainActive.Height() + 1; } if (nHeight < 0 || nHeight > chainActive.Height()) @@ -665,7 +665,7 @@ UniValue getblock(const UniValue& params, bool fHelp) "If verbosity is 1, returns an Object with information about the block.\n" "If verbosity is 2, returns an Object with information about the block and information about each transaction. \n" "\nArguments:\n" - "1. \"hash|height\" (string, required) The block hash or height. Height can be negative, if so: height = last known valid block - abs(height)\n" + "1. \"hash|height\" (string, required) The block hash or height. Height can be negative where -1 is the last known valid block\n" "2. verbosity (numeric, optional, default=1) 0 for hex encoded data, 1 for a json object, and 2 for json object with transaction data\n" "\nResult (for verbosity = 0):\n" "\"data\" (string) A string that is serialized, hex-encoded data for the block.\n" @@ -725,7 +725,7 @@ UniValue getblock(const UniValue& params, bool fHelp) } if(nHeight < 0) { - nHeight += chainActive.Height(); + nHeight += chainActive.Height() + 1; } if (nHeight < 0 || nHeight > chainActive.Height()) { From 3fb8af512c464140d6ca67c814313bd72bb17780 Mon Sep 17 00:00:00 2001 From: Alfredo Garcia Date: Tue, 21 Jan 2020 21:24:21 -0300 Subject: [PATCH 5/6] change regex --- qa/rpc-tests/merkle_blocks.py | 1 + src/rpc/blockchain.cpp | 6 +++--- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/qa/rpc-tests/merkle_blocks.py b/qa/rpc-tests/merkle_blocks.py index ca7af0018..9787b3c8c 100755 --- a/qa/rpc-tests/merkle_blocks.py +++ b/qa/rpc-tests/merkle_blocks.py @@ -107,6 +107,7 @@ class MerkleBlockTest(BitcoinTestFramework): assert_equal(self.nodes[0].getblock("-107")["height"], 1) assert_equal(self.nodes[0].getblock("-108")["height"], 0) assert_raises(JSONRPCException, self.nodes[0].getblock, ["-109"]) + assert_raises(JSONRPCException, self.nodes[0].getblock, ["-0"]) # Test getblockhash negative heights assert_equal(self.nodes[0].getblockhash(-1), self.nodes[0].getblockhash(107)) diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 83f2daeff..a7a08f049 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -588,7 +588,7 @@ UniValue getblockhash(const UniValue& params, bool fHelp) int nHeight = params[0].get_int(); - if(nHeight < 0) { + if (nHeight < 0) { nHeight += chainActive.Height() + 1; } @@ -711,7 +711,7 @@ UniValue getblock(const UniValue& params, bool fHelp) // If height is supplied, find the hash if (strHash.size() < (2 * sizeof(uint256))) { // std::stoi allows characters, whereas we want to be strict - regex r("-?[[:digit:]]+"); + regex r("(?:(-?)[1-9][0-9]*|[0-9]+)"); if (!regex_match(strHash, r)) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid block height parameter"); } @@ -724,7 +724,7 @@ UniValue getblock(const UniValue& params, bool fHelp) throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid block height parameter"); } - if(nHeight < 0) { + if (nHeight < 0) { nHeight += chainActive.Height() + 1; } From a8f6b8c2e51639fa403411854f7c3d9f4ae935da Mon Sep 17 00:00:00 2001 From: Alfredo Garcia Date: Mon, 17 Feb 2020 09:52:40 -0300 Subject: [PATCH 6/6] fix rpx_wallet_tests --- src/wallet/test/rpc_wallet_tests.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/wallet/test/rpc_wallet_tests.cpp b/src/wallet/test/rpc_wallet_tests.cpp index 48705f90a..dc3054623 100644 --- a/src/wallet/test/rpc_wallet_tests.cpp +++ b/src/wallet/test/rpc_wallet_tests.cpp @@ -310,7 +310,8 @@ BOOST_AUTO_TEST_CASE(rpc_wallet) * getblock */ BOOST_CHECK_THROW(CallRPC("getblock too many args"), runtime_error); - BOOST_CHECK_THROW(CallRPC("getblock -1"), runtime_error); + BOOST_CHECK_NO_THROW(CallRPC("getblock -1")); // negative heights relative are allowed + BOOST_CHECK_THROW(CallRPC("getblock -2147483647"), runtime_error); // allowed, but chain tip - height < 0 BOOST_CHECK_THROW(CallRPC("getblock 2147483647"), runtime_error); // allowed, but > height of active chain tip BOOST_CHECK_THROW(CallRPC("getblock 2147483648"), runtime_error); // not allowed, > int32 used for nHeight BOOST_CHECK_THROW(CallRPC("getblock 100badchars"), runtime_error);