Merge #13983: rpc: Return more specific reject reason for submitblock
fa6ab8ada1
rpc: Return more specific reject reason for submitblock (MarcoFalke)
Pull request description:
The second commit in #13439 made the `TODO` in the first commit impossible to solve.
The meaning of `fNewBlock` changed from "This is the first time we process this block" to "We are about to write the new *valid* block".
So whenever `fNewBlock` is true, the block was valid. And whenever the `fNewBlock` is false, the block is either valid or invalid. If it was valid and not new, we know it is a `"duplicate"`. In all other cases, the `BIP22ValidationResult()` will return the reason why it is invalid.
Tree-SHA512: 4b6edf7a912339c3acb0fccfabbdd6d812a0321fb1639c244c2714e58dc119aa2b8c6bf8f7d61ea609a1b861bbc23f920370fcf989c48452721e259a8ce93d24
This commit is contained in:
commit
962c302710
2 changed files with 20 additions and 7 deletions
|
@ -750,11 +750,7 @@ static UniValue submitblock(const JSONRPCRequest& request)
|
||||||
RegisterValidationInterface(&sc);
|
RegisterValidationInterface(&sc);
|
||||||
bool accepted = ProcessNewBlock(Params(), blockptr, /* fForceProcessing */ true, /* fNewBlock */ &new_block);
|
bool accepted = ProcessNewBlock(Params(), blockptr, /* fForceProcessing */ true, /* fNewBlock */ &new_block);
|
||||||
UnregisterValidationInterface(&sc);
|
UnregisterValidationInterface(&sc);
|
||||||
if (!new_block) {
|
if (!new_block && accepted) {
|
||||||
if (!accepted) {
|
|
||||||
// TODO Maybe pass down fNewBlock to AcceptBlockHeader, so it is properly set to true in this case?
|
|
||||||
return "invalid";
|
|
||||||
}
|
|
||||||
return "duplicate";
|
return "duplicate";
|
||||||
}
|
}
|
||||||
if (!sc.found) {
|
if (!sc.found) {
|
||||||
|
|
|
@ -44,6 +44,12 @@ class MiningTest(BitcoinTestFramework):
|
||||||
def run_test(self):
|
def run_test(self):
|
||||||
node = self.nodes[0]
|
node = self.nodes[0]
|
||||||
|
|
||||||
|
def assert_submitblock(block, result_str_1, result_str_2=None):
|
||||||
|
block.solve()
|
||||||
|
result_str_2 = result_str_2 or 'duplicate-invalid'
|
||||||
|
assert_equal(result_str_1, node.submitblock(hexdata=b2x(block.serialize())))
|
||||||
|
assert_equal(result_str_2, node.submitblock(hexdata=b2x(block.serialize())))
|
||||||
|
|
||||||
self.log.info('getmininginfo')
|
self.log.info('getmininginfo')
|
||||||
mining_info = node.getmininginfo()
|
mining_info = node.getmininginfo()
|
||||||
assert_equal(mining_info['blocks'], 200)
|
assert_equal(mining_info['blocks'], 200)
|
||||||
|
@ -96,6 +102,7 @@ class MiningTest(BitcoinTestFramework):
|
||||||
bad_block = copy.deepcopy(block)
|
bad_block = copy.deepcopy(block)
|
||||||
bad_block.vtx.append(bad_block.vtx[0])
|
bad_block.vtx.append(bad_block.vtx[0])
|
||||||
assert_template(node, bad_block, 'bad-txns-duplicate')
|
assert_template(node, bad_block, 'bad-txns-duplicate')
|
||||||
|
assert_submitblock(bad_block, 'bad-txns-duplicate', 'bad-txns-duplicate')
|
||||||
|
|
||||||
self.log.info("getblocktemplate: Test invalid transaction")
|
self.log.info("getblocktemplate: Test invalid transaction")
|
||||||
bad_block = copy.deepcopy(block)
|
bad_block = copy.deepcopy(block)
|
||||||
|
@ -104,12 +111,14 @@ class MiningTest(BitcoinTestFramework):
|
||||||
bad_tx.rehash()
|
bad_tx.rehash()
|
||||||
bad_block.vtx.append(bad_tx)
|
bad_block.vtx.append(bad_tx)
|
||||||
assert_template(node, bad_block, 'bad-txns-inputs-missingorspent')
|
assert_template(node, bad_block, 'bad-txns-inputs-missingorspent')
|
||||||
|
assert_submitblock(bad_block, 'bad-txns-inputs-missingorspent')
|
||||||
|
|
||||||
self.log.info("getblocktemplate: Test nonfinal transaction")
|
self.log.info("getblocktemplate: Test nonfinal transaction")
|
||||||
bad_block = copy.deepcopy(block)
|
bad_block = copy.deepcopy(block)
|
||||||
bad_block.vtx[0].nLockTime = 2 ** 32 - 1
|
bad_block.vtx[0].nLockTime = 2 ** 32 - 1
|
||||||
bad_block.vtx[0].rehash()
|
bad_block.vtx[0].rehash()
|
||||||
assert_template(node, bad_block, 'bad-txns-nonfinal')
|
assert_template(node, bad_block, 'bad-txns-nonfinal')
|
||||||
|
assert_submitblock(bad_block, 'bad-txns-nonfinal')
|
||||||
|
|
||||||
self.log.info("getblocktemplate: Test bad tx count")
|
self.log.info("getblocktemplate: Test bad tx count")
|
||||||
# The tx count is immediately after the block header
|
# The tx count is immediately after the block header
|
||||||
|
@ -128,24 +137,29 @@ class MiningTest(BitcoinTestFramework):
|
||||||
bad_block = copy.deepcopy(block)
|
bad_block = copy.deepcopy(block)
|
||||||
bad_block.hashMerkleRoot += 1
|
bad_block.hashMerkleRoot += 1
|
||||||
assert_template(node, bad_block, 'bad-txnmrklroot', False)
|
assert_template(node, bad_block, 'bad-txnmrklroot', False)
|
||||||
|
assert_submitblock(bad_block, 'bad-txnmrklroot', 'bad-txnmrklroot')
|
||||||
|
|
||||||
self.log.info("getblocktemplate: Test bad timestamps")
|
self.log.info("getblocktemplate: Test bad timestamps")
|
||||||
bad_block = copy.deepcopy(block)
|
bad_block = copy.deepcopy(block)
|
||||||
bad_block.nTime = 2 ** 31 - 1
|
bad_block.nTime = 2 ** 31 - 1
|
||||||
assert_template(node, bad_block, 'time-too-new')
|
assert_template(node, bad_block, 'time-too-new')
|
||||||
|
assert_submitblock(bad_block, 'time-too-new', 'time-too-new')
|
||||||
bad_block.nTime = 0
|
bad_block.nTime = 0
|
||||||
assert_template(node, bad_block, 'time-too-old')
|
assert_template(node, bad_block, 'time-too-old')
|
||||||
|
assert_submitblock(bad_block, 'time-too-old', 'time-too-old')
|
||||||
|
|
||||||
self.log.info("getblocktemplate: Test not best block")
|
self.log.info("getblocktemplate: Test not best block")
|
||||||
bad_block = copy.deepcopy(block)
|
bad_block = copy.deepcopy(block)
|
||||||
bad_block.hashPrevBlock = 123
|
bad_block.hashPrevBlock = 123
|
||||||
assert_template(node, bad_block, 'inconclusive-not-best-prevblk')
|
assert_template(node, bad_block, 'inconclusive-not-best-prevblk')
|
||||||
|
assert_submitblock(bad_block, 'prev-blk-not-found', 'prev-blk-not-found')
|
||||||
|
|
||||||
self.log.info('submitheader tests')
|
self.log.info('submitheader tests')
|
||||||
assert_raises_rpc_error(-22, 'Block header decode failed', lambda: node.submitheader(hexdata='xx' * 80))
|
assert_raises_rpc_error(-22, 'Block header decode failed', lambda: node.submitheader(hexdata='xx' * 80))
|
||||||
assert_raises_rpc_error(-22, 'Block header decode failed', lambda: node.submitheader(hexdata='ff' * 78))
|
assert_raises_rpc_error(-22, 'Block header decode failed', lambda: node.submitheader(hexdata='ff' * 78))
|
||||||
assert_raises_rpc_error(-25, 'Must submit previous header', lambda: node.submitheader(hexdata='ff' * 80))
|
assert_raises_rpc_error(-25, 'Must submit previous header', lambda: node.submitheader(hexdata='ff' * 80))
|
||||||
|
|
||||||
|
block.nTime += 1
|
||||||
block.solve()
|
block.solve()
|
||||||
|
|
||||||
def chain_tip(b_hash, *, status='headers-only', branchlen=1):
|
def chain_tip(b_hash, *, status='headers-only', branchlen=1):
|
||||||
|
@ -164,7 +178,8 @@ class MiningTest(BitcoinTestFramework):
|
||||||
node.submitheader(hexdata=b2x(CBlockHeader(bad_block_root).serialize()))
|
node.submitheader(hexdata=b2x(CBlockHeader(bad_block_root).serialize()))
|
||||||
assert chain_tip(bad_block_root.hash) in node.getchaintips()
|
assert chain_tip(bad_block_root.hash) in node.getchaintips()
|
||||||
# Should still reject invalid blocks, even if we have the header:
|
# Should still reject invalid blocks, even if we have the header:
|
||||||
assert_equal(node.submitblock(hexdata=b2x(bad_block_root.serialize())), 'invalid')
|
assert_equal(node.submitblock(hexdata=b2x(bad_block_root.serialize())), 'bad-txnmrklroot')
|
||||||
|
assert_equal(node.submitblock(hexdata=b2x(bad_block_root.serialize())), 'bad-txnmrklroot')
|
||||||
assert chain_tip(bad_block_root.hash) in node.getchaintips()
|
assert chain_tip(bad_block_root.hash) in node.getchaintips()
|
||||||
# We know the header for this invalid block, so should just return early without error:
|
# We know the header for this invalid block, so should just return early without error:
|
||||||
node.submitheader(hexdata=b2x(CBlockHeader(bad_block_root).serialize()))
|
node.submitheader(hexdata=b2x(CBlockHeader(bad_block_root).serialize()))
|
||||||
|
@ -175,7 +190,8 @@ class MiningTest(BitcoinTestFramework):
|
||||||
bad_block_lock.vtx[0].rehash()
|
bad_block_lock.vtx[0].rehash()
|
||||||
bad_block_lock.hashMerkleRoot = bad_block_lock.calc_merkle_root()
|
bad_block_lock.hashMerkleRoot = bad_block_lock.calc_merkle_root()
|
||||||
bad_block_lock.solve()
|
bad_block_lock.solve()
|
||||||
assert_equal(node.submitblock(hexdata=b2x(bad_block_lock.serialize())), 'invalid')
|
assert_equal(node.submitblock(hexdata=b2x(bad_block_lock.serialize())), 'bad-txns-nonfinal')
|
||||||
|
assert_equal(node.submitblock(hexdata=b2x(bad_block_lock.serialize())), 'duplicate-invalid')
|
||||||
# Build a "good" block on top of the submitted bad block
|
# Build a "good" block on top of the submitted bad block
|
||||||
bad_block2 = copy.deepcopy(block)
|
bad_block2 = copy.deepcopy(block)
|
||||||
bad_block2.hashPrevBlock = bad_block_lock.sha256
|
bad_block2.hashPrevBlock = bad_block_lock.sha256
|
||||||
|
@ -201,6 +217,7 @@ class MiningTest(BitcoinTestFramework):
|
||||||
assert_raises_rpc_error(-25, 'bad-prevblk', lambda: node.submitheader(hexdata=b2x(CBlockHeader(bad_block2).serialize())))
|
assert_raises_rpc_error(-25, 'bad-prevblk', lambda: node.submitheader(hexdata=b2x(CBlockHeader(bad_block2).serialize())))
|
||||||
node.submitheader(hexdata=b2x(CBlockHeader(block).serialize()))
|
node.submitheader(hexdata=b2x(CBlockHeader(block).serialize()))
|
||||||
node.submitheader(hexdata=b2x(CBlockHeader(bad_block_root).serialize()))
|
node.submitheader(hexdata=b2x(CBlockHeader(bad_block_root).serialize()))
|
||||||
|
assert_equal(node.submitblock(hexdata=b2x(block.serialize())), 'duplicate') # valid
|
||||||
|
|
||||||
|
|
||||||
if __name__ == '__main__':
|
if __name__ == '__main__':
|
||||||
|
|
Loading…
Reference in a new issue