[rpc] Correct reconsiderblock help text, add test #15057

pull MarcoFalke wants to merge 1 commits into bitcoin:master from MarcoFalke:Mf1608-qaAssert changing 2 files +48 −23

MarcoFalke commented at 4:22 PM on December 29, 2018: member

Rework documentation and test to match the implementation
MarcoFalke added the label RPC/REST/ZMQ on Dec 29, 2018
MarcoFalke force-pushed on Dec 29, 2018

DrahtBot commented at 6:13 PM on December 29, 2018: member

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Coverage

Coverage	Change (pull 15057, a830d1c4d3eeac4c869c9a061d64e2722a1a252a)	Reference (master, a1fd876122938469cc3527f53211f976351989c1)
Lines	-0.0129 %	87.3755 %
Functions	-0.0452 %	84.5100 %
Branches	-0.0079 %	51.3984 %

<sup>Updated at: 2018-12-29T18:13:51.889322.</sup>

ch4ot1c commented at 6:34 PM on December 29, 2018: contributor

utACK
MarcoFalke commented at 6:51 PM on December 29, 2018: member
For reference:
- https://drahtbot.github.io/reports/coverage/bitcoin/bitcoin/master/total.coverage/src/validation.cpp.gcov.html#2855
- https://drahtbot.github.io/reports/coverage/bitcoin/bitcoin/15057/total.coverage/src/validation.cpp.gcov.html#2855
gmaxwell commented at 2:38 AM on January 1, 2019: contributor

IIRC if you have a chain A B C D E and invalidate D then invalidate B, then reconsider B you'll end back up on E.
[rpc] Correct reconsiderblock help text, add test fa38d3df69
MarcoFalke force-pushed on Jan 1, 2019
MarcoFalke commented at 5:04 PM on January 1, 2019: member

Added a test for this as well (and fixed up the help text). Maybe the method should have been called reconsiderchain with the description "Reconsiders all chains the given block is in"
laanwj commented at 2:22 PM on January 2, 2019: member

utACK fa38d3df69851212fea7544badadc1c3e5369bf5

in test/functional/rpc_invalidateblock.py:72 in fa38d3df69

  86 | +        assert_equal(self.nodes[1].getbestblockhash(), blocks[-1])
  87 | +        # Invalidate the two blocks at the tip
  88 | +        self.nodes[1].invalidateblock(blocks[-1])
  89 | +        self.nodes[1].invalidateblock(blocks[-2])
  90 | +        assert_equal(self.nodes[1].getbestblockhash(), blocks[-3])
  91 | +        # Reconsider only the previous tip

promag commented at 9:04 PM on January 2, 2019:

s/previous/initial/

in test/functional/rpc_invalidateblock.py:69 in fa38d3df69

  83 | +
  84 | +        self.log.info("Verify that we reconsider all ancestors as well")
  85 | +        blocks = self.nodes[1].generatetoaddress(10, ADDRESS_BCRT1_UNSPENDABLE)
  86 | +        assert_equal(self.nodes[1].getbestblockhash(), blocks[-1])
  87 | +        # Invalidate the two blocks at the tip
  88 | +        self.nodes[1].invalidateblock(blocks[-1])

promag commented at 9:06 PM on January 2, 2019:

remove?

in test/functional/rpc_invalidateblock.py:81 in fa38d3df69

  95 | +
  96 | +        self.log.info("Verify that we reconsider all descendants")
  97 | +        blocks = self.nodes[1].generatetoaddress(10, ADDRESS_BCRT1_UNSPENDABLE)
  98 | +        assert_equal(self.nodes[1].getbestblockhash(), blocks[-1])
  99 | +        # Invalidate the two blocks at the tip
 100 | +        self.nodes[1].invalidateblock(blocks[-2])

promag commented at 9:07 PM on January 2, 2019:

remove?

MarcoFalke commented at 10:57 AM on January 3, 2019:

Why? This is exactly what the test should be testing for. See #15057 (comment)

promag commented at 11:18 AM on January 3, 2019:

Yap sorry.

promag commented at 9:08 PM on January 2, 2019: member

Some comments/questions, other than that looks great.
promag commented at 4:00 PM on January 5, 2019: member

utACK fa38d3d, don't mind the above nits..

Candidate for backport?
MarcoFalke commented at 4:11 PM on January 5, 2019: member

I'd say no. User-facing it is only a minor documentation fixup of a method that should not casually be used in production.
laanwj merged this on Jan 7, 2019
laanwj closed this on Jan 7, 2019
laanwj referenced this in commit 1973257da0 on Jan 7, 2019
MarcoFalke deleted the branch on Jan 10, 2019
jasonbcox referenced this in commit 3345d011bb on Oct 19, 2020
DrahtBot locked this on Dec 16, 2021

Contributors

Labels