/bitcoin

[tests] mining functional tests (including regression test for submitblock) #10190

Merged
merged 9 commits into from Jul 11, 2017
+125 −158

Conversation

Reviewers

@jimmysong jimmysong

Assignees

@MarcoFalke MarcoFalke

Labels
Mining Tests
Projects
None yet
Milestone
  0.15.0
6 participants
@jnewbery @gmaxwell @laanwj @jimmysong @fanquake @MarcoFalke
@jnewbery
Member

jnewbery commented Apr 11, 2017 • edited

This PR refactors the getblocktemplate_proposals.py functional test into a more general mining test, which covers the submitblock RPC as well as the getblocktemplate propose mode.

Motivation was to add a regression test for #10146

This test removes all the manual block/transaction construction from the test case and uses the mininode classes to achieve the same tests. Result is less lines of code, and hopefully clearer and easier for people to extend/maintain since we're no longer hacking bits in a serialised block.

@gmaxwell since he wrote the fix for #10146

EDIT: there are lots of commits in this PR to aid reviewers. They can be squashed down into larger commits before merging.

jnewbery referenced this pull request Apr 11, 2017

Merged

Better error handling for submitblock #10146

@fanquake fanquake added Mining Tests labels Apr 12, 2017

@jimmysong

jimmysong reviewed Apr 18, 2017
View changes

utACK df93f6c

Gets rid of a lot of duplicate logic. A few nits.

test/functional/mining.py
+# Copyright (c) 2014-2016 The Bitcoin Core developers
+# Distributed under the MIT software license, see the accompanying
+# file COPYING or http://www.opensource.org/licenses/mit-license.php.
+"""Test block proposals with getblocktemplate."""
@jimmysong

jimmysong Apr 18, 2017

Contributor

Guessing you'll want to change this comment.

@jnewbery

jnewbery Apr 18, 2017

Member

Yes, thanks.

test/functional/mining.py
+ rsp = node.getblocktemplate({'data': b2x(block.serialize()), 'mode': 'proposal'})
+ assert_equal(rsp, expect)
+
+class GetBlockTemplateProposalTest(BitcoinTestFramework):
@jimmysong

jimmysong Apr 18, 2017

Contributor

rename the test to MiningTest?

@jnewbery

jnewbery Apr 18, 2017

Member

agreed

test/functional/mining.py
+ assert_template(node, bad_block, 'inconclusive-not-best-prevblk')
+
+if __name__ == '__main__':
+ GetBlockTemplateProposalTest().main()
@jimmysong

jimmysong Apr 18, 2017

Contributor

MiningTest?

@jnewbery

jnewbery Apr 18, 2017

Member

yep

test/functional/getblocktemplate_proposals.py
+ # sequence numbers must not be max for nLockTime to have effect
+ coinbase_tx.vin[0].nSequence = 2 ** 32 - 2
+ tmpl['coinbasetxn'] = {'data': coinbase_tx.serialize()}
+ txlist = [bytearray(coinbase_tx.serialize())]
@jimmysong

jimmysong Apr 18, 2017

Contributor

is tmpl['transactions'] expected to be empty?

@jnewbery

jnewbery Apr 18, 2017

Member

yes, I believe so

@jnewbery
Member

jnewbery commented May 9, 2017

@gmaxwell can I convince you to take a look at this? It adds a regression test for your fix in #10146
@gmaxwell
Member

gmaxwell commented May 26, 2017

Concept ACK. I am testing the test.
@jnewbery
Member

jnewbery commented Jun 18, 2017

No great rush for this but it'd be good to get it in soonish. It includes a regression test for #10146, which was backported to 0.14.1. Other benefits:

  • tidies up test code
  • increases coverage of mining RPCs

MarcoFalke added this to the 0.15.0 milestone Jun 18, 2017

MarcoFalke was assigned by laanwj Jun 26, 2017

@jnewbery
Member

jnewbery commented Jun 27, 2017

rebased

jnewbery changed the title from Mining functional tests (including regression test for submitblock) to [tests] mining functional tests (including regression test for submitblock) Jun 30, 2017

@laanwj
Owner

laanwj commented Jul 11, 2017

utACK 11ba8e9

@laanwj laanwj merged commit 11ba8e9 into bitcoin:master Jul 11, 2017
1 check passed

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@laanwj laanwj added a commit that referenced this pull request Jul 11, 2017

@laanwj laanwj Merge #10190: [tests] mining functional tests (including regression t… 
…est for submitblock)


11ba8e9 [tests] rename getblocktemplate_proposals.py to mining.py (John Newbery)
b29dd41 [tests] add test for submit block (John Newbery)
9bf0d80 [tests] run successful test in getblocktemplate first (John Newbery)
82dc597 [tests] don't build blocks manually in getblocktemplate test (John Newbery)
f82c709 [tests] clarify assertions in getblocktemplate test (John Newbery)
66c570a [tests] Don't build the coinbase manually in getblocktemplate test (John Newbery)
38b38cd [tests] getblocktemplate_proposals.py: add logging (John Newbery)
0a3a5ff [tests] Fix flake8 warnings in getblocktemplate tests (John Newbery)
32cffe6 [tests] Fix import order in getblocktemplate test (John Newbery)

Tree-SHA512: a51a57314fa1c4c4b8a7896492ec6e677b6bed12387060def34a62e9dfbee7961f71bb5553fbd70028be61ae32eccf13fd255fa9651f908e9a5e64c28f43f00e
This commit was signed with a verified signature.
@laanwj laanwj Wladimir J. van der Laan
GPG key ID: 1E4AED62986CD25D Learn about signing commits
e4f226a
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment