qa: Add getrawtransaction in_active_chain=False test #11838
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Agree, |
promag
reviewed
Dec 6, 2017
| gottx = self.nodes[0].getrawtransaction(txid=tx, verbose=True, blockhash=block1) | ||
| assert_equal(gottx['in_active_chain'], False) | ||
| self.nodes[0].reconsiderblock(block1) | ||
| assert_equal(self.nodes[0].getbestblockhash(), block2) |
There was a problem hiding this comment.
Nit, IMO this check is not really necessary in the context of this test (it's testing reconsiderblockworked). The same could be done above after invalidateblock(block1).
But I guess you want to be sure the state is the same for the remaining script.
|
utACK fa4c16d |
|
utACK fa4c16d |
laanwj
added a commit
that referenced
this pull request
Dec 7, 2017
fa4c16d qa: Add getrawtransaction in_active_chain=False test (MarcoFalke) Pull request description: #10275 accidentally forgot to add a test for `in_active_chain==False`. This adds a test and also removes the special casing of `blockhash.IsNull()`, which makes no sense imo. Tree-SHA512: 6c51295820b3dcd53b0b48020ab2b8c8f5864cd5061ddab2b35d35d643eb3e60ef95ff20c06c985a2e47f7080e82f27f3e00ee61c85dce627776d5ea6febee8f
jnewbery
reviewed
Dec 7, 2017
| @@ -155,14 +155,12 @@ UniValue getrawtransaction(const JSONRPCRequest& request) | |||
|
|
|||
| if (!request.params[2].isNull()) { | |||
| uint256 blockhash = ParseHashV(request.params[2], "parameter 3"); | |||
There was a problem hiding this comment.
nit: suggest you add a comment here to say that ParseHashV() will throw if called with a string which isn't a 64 length hex strong.
| @@ -72,7 +72,13 @@ def run_test(self): | |||
| assert_raises_rpc_error(-8, "parameter 3 must be hexadecimal", self.nodes[0].getrawtransaction, tx, True, True) | |||
| assert_raises_rpc_error(-8, "parameter 3 must be hexadecimal", self.nodes[0].getrawtransaction, tx, True, "foobar") | |||
| assert_raises_rpc_error(-8, "parameter 3 must be of length 64", self.nodes[0].getrawtransaction, tx, True, "abcd1234") | |||
| assert_raises_rpc_error(-5, "Block hash not found", self.nodes[0].getrawtransaction, tx, True, "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa") | |||
| assert_raises_rpc_error(-5, "Block hash not found", self.nodes[0].getrawtransaction, tx, True, "0000000000000000000000000000000000000000000000000000000000000000") | |||
There was a problem hiding this comment.
nit: prefer assert_raises_rpc_error(-5, "Block hash not found", self.nodes[0].getrawtransaction, tx, True, "0" * 64) to make it clear that this is a valid length 64 hex string
| # Undo the blocks and check in_active_chain | ||
| self.nodes[0].invalidateblock(block1) | ||
| gottx = self.nodes[0].getrawtransaction(txid=tx, verbose=True, blockhash=block1) | ||
| assert_equal(gottx['in_active_chain'], False) |
There was a problem hiding this comment.
nit: slight preference for assert not gottx['in_active_chain']
it'd also be nice to test the positive case (call getrawtransaction with a blockhash in the current chain and assert gottx['in_active_chain']
PastaPastaPasta
pushed a commit
to PastaPastaPasta/dash
that referenced
this pull request
Feb 13, 2020
…test fa4c16d qa: Add getrawtransaction in_active_chain=False test (MarcoFalke) Pull request description: bitcoin#10275 accidentally forgot to add a test for `in_active_chain==False`. This adds a test and also removes the special casing of `blockhash.IsNull()`, which makes no sense imo. Tree-SHA512: 6c51295820b3dcd53b0b48020ab2b8c8f5864cd5061ddab2b35d35d643eb3e60ef95ff20c06c985a2e47f7080e82f27f3e00ee61c85dce627776d5ea6febee8f
PastaPastaPasta
pushed a commit
to PastaPastaPasta/dash
that referenced
this pull request
Feb 27, 2020
…test fa4c16d qa: Add getrawtransaction in_active_chain=False test (MarcoFalke) Pull request description: bitcoin#10275 accidentally forgot to add a test for `in_active_chain==False`. This adds a test and also removes the special casing of `blockhash.IsNull()`, which makes no sense imo. Tree-SHA512: 6c51295820b3dcd53b0b48020ab2b8c8f5864cd5061ddab2b35d35d643eb3e60ef95ff20c06c985a2e47f7080e82f27f3e00ee61c85dce627776d5ea6febee8f
PastaPastaPasta
pushed a commit
to PastaPastaPasta/dash
that referenced
this pull request
Feb 27, 2020
…test fa4c16d qa: Add getrawtransaction in_active_chain=False test (MarcoFalke) Pull request description: bitcoin#10275 accidentally forgot to add a test for `in_active_chain==False`. This adds a test and also removes the special casing of `blockhash.IsNull()`, which makes no sense imo. Tree-SHA512: 6c51295820b3dcd53b0b48020ab2b8c8f5864cd5061ddab2b35d35d643eb3e60ef95ff20c06c985a2e47f7080e82f27f3e00ee61c85dce627776d5ea6febee8f
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
#10275 accidentally forgot to add a test for
in_active_chain==False.This adds a test and also removes the special casing of
blockhash.IsNull(), which makes no sense imo.