rpc: Add generateblock to mine a custom set of transactions

andrewtoth commented at 7:43 pm on December 2, 2019: contributor

The existing block generation rpcs for regtest, generatetoaddress and generatetodescriptor, mine everything in the mempool up to the block weight limit. This makes it difficult to test a system for several scenarios where a different set of transactions are mined. For example:

Testing the common scenario where a transaction is replaced in the mempool but the replaced transaction is mined instead.
Testing for a double-spent transaction where a transaction that conflicts with the mempool is mined.
Testing for non-standard transactions that are mined.
Testing the scenario where several blocks are mined without a specific transaction in the mempool being included in a block.

This PR introduces a new rpc, generateblock, that takes an array of raw transactions and txids and mines only those and the coinbase. Any txids must be in the mempool, but the raw txs can be anything conforming to consensus rules. The coinbase can be specified as either an address or descriptor.

There is some relevant conversation here.

fanquake added the label RPC/REST/ZMQ on Dec 2, 2019

instagibbs commented at 7:52 pm on December 2, 2019: member

concept ACK, we want to test censorship type scenarios for Liquid, but don’t have a good way of doing that in our own test harness aside from manually constructing blocks ourselves. Previously we relied on premature witness data being non-standard to “censor” the transactions themselves but that is no longer a non-standard case due to buried deployment.

Thoughts:

Probably a bunch of code can be de-duplicated with some refactoring
This should probably only be available for test networks, and hidden?

andrewtoth force-pushed on Dec 2, 2019

DrahtBot commented at 9:02 pm on December 2, 2019: member

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

#16365 (Log RPC parameters (arguments) if -debug=rpcparams by LarryRuane)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

andrewtoth force-pushed on Dec 3, 2019

andrewtoth commented at 0:19 am on December 3, 2019: contributor

Probably a bunch of code can be de-duplicated with some refactoring @instagibbs Most of the duplicated code to create the block is in BlockAssembler, and it’s very tightly coupled with getting all txs from the mempool. I’m not sure it would be worth messing around with that code for this.

This should probably only be available for test networks, and hidden?

I don’t believe this would have any effect if trying to generate on a chain with high difficulty, so not sure if there’s any risk with letting it be used anywhere. The same could be said about generatetoaddress.

Add generatecustomblock rpc b0745f8dc5

Add tests for generatecustomblock de2a77fbc2

andrewtoth force-pushed on Dec 3, 2019

in test/functional/rpc_generatecustomblock.py:41 in de2a77fbc2

36+        utxos = node.listunspent(addresses=[address])
37+        raw = node.createrawtransaction([{"txid":utxos[0]["txid"], "vout":utxos[0]["vout"]}],[{address:1}])
38+        signed_raw = node.signrawtransactionwithwallet(raw)['hex']
39+        hash = node.generatecustomblock(address, [signed_raw])
40+        block = node.getblock(hash, 1)
41+        assert len(block['tx']) == 2

instagibbs commented at 3:31 pm on December 3, 2019:

assert_equal makes the error more clear when it doesn’t match

in test/functional/rpc_generatecustomblock.py:43 in de2a77fbc2

38+        signed_raw = node.signrawtransactionwithwallet(raw)['hex']
39+        hash = node.generatecustomblock(address, [signed_raw])
40+        block = node.getblock(hash, 1)
41+        assert len(block['tx']) == 2
42+        txid = block['tx'][1]
43+        assert node.gettransaction(txid)['hex'] == signed_raw

instagibbs commented at 3:31 pm on December 3, 2019:

assert_equal makes the error more clear when it doesn’t match

in test/functional/rpc_generatecustomblock.py:49 in de2a77fbc2

44+
45+        # Generate custom block with txid
46+        txid = node.sendtoaddress(address, 1)
47+        hash = node.generatecustomblock(address, [txid])
48+        block = node.getblock(hash, 1)
49+        assert len(block['tx']) == 2

instagibbs commented at 3:31 pm on December 3, 2019:

assert_equal makes the error more clear when it doesn’t match

in test/functional/rpc_generatecustomblock.py:50 in de2a77fbc2

45+        # Generate custom block with txid
46+        txid = node.sendtoaddress(address, 1)
47+        hash = node.generatecustomblock(address, [txid])
48+        block = node.getblock(hash, 1)
49+        assert len(block['tx']) == 2
50+        assert block['tx'][1] == txid

instagibbs commented at 3:31 pm on December 3, 2019:

assert_equal makes the error more clear when it doesn’t match

instagibbs commented at 3:43 pm on December 3, 2019: member

I did a bit of refactoring that makes this re-use the existing mining code: https://github.com/instagibbs/bitcoin/tree/generatecustomblock

Fewer added lines, and less redundancy imo.

Feel free to take this approach instead.

in test/functional/rpc_generatecustomblock.py:52 in de2a77fbc2

47+        hash = node.generatecustomblock(address, [txid])
48+        block = node.getblock(hash, 1)
49+        assert len(block['tx']) == 2
50+        assert block['tx'][1] == txid
51+
52+if __name__ == '__main__':

instagibbs commented at 3:45 pm on December 3, 2019:

I think there should also be a test making sure that invalid-ordering-of-txn block doesn’t cause something unexpected like a crash.

in src/rpc/client.cpp:35 in de2a77fbc2

31@@ -32,6 +32,7 @@ static const CRPCConvertParam vRPCConvertParams[] =
32     { "generatetoaddress", 2, "maxtries" },
33     { "generatetodescriptor", 0, "num_blocks" },
34     { "generatetodescriptor", 2, "maxtries" },
35+    { "generatecustomblock", 1, "transactions" },

promag commented at 3:49 pm on December 3, 2019:

Just generateblock?

andrewtoth commented at 4:05 pm on December 7, 2019:

Updated

in src/rpc/mining.cpp:301 in de2a77fbc2

296+        {
297+            {"address/descriptor", RPCArg::Type::STR, RPCArg::Optional::NO, "The address or descriptor to send the newly generated bitcoin to."},
298+            {"transactions", RPCArg::Type::ARR, RPCArg::Optional::NO, "An array of hex strings which are either txids or raw transactions.\n"
299+                "Txids must reference transactions currently in the mempool.",
300+                {
301+                    {"rawtx/txid", RPCArg::Type::STR_HEX, RPCArg::Optional::OMITTED, ""},

promag commented at 3:52 pm on December 3, 2019:

I guess it’s fine supporting both here.

in src/rpc/mining.cpp:352 in de2a77fbc2

347+
348+        uint256 hash;
349+        CMutableTransaction mtx;
350+        if (ParseHashStr(str, hash)) {
351+
352+            LOCK(mempool.cs);

promag commented at 3:54 pm on December 3, 2019:

You can replace this code with CTxMemPool::get.

andrewtoth commented at 5:35 pm on December 7, 2019:

Done

promag commented at 3:55 pm on December 3, 2019: member

Concept ACK.

andrewtoth closed this on Dec 7, 2019

andrewtoth deleted the branch on Dec 7, 2019

andrewtoth renamed this:
~~rpc: Add generatecustomblock to mine a custom set of transactions~~
rpc: Add generateblock to mine a custom set of transactions
on Dec 7, 2019

andrewtoth commented at 5:35 pm on December 7, 2019: contributor

Closed by mistake. Reopened new PR #17693

MarcoFalke referenced this in commit 51e2ce45d6 on Apr 10, 2020

sidhujag referenced this in commit 476e5c09c3 on Apr 13, 2020

PhotoshiNakamoto referenced this in commit f986fe023d on Dec 11, 2021

MarcoFalke locked this on Dec 16, 2021

rpc: Add generateblock to mine a custom set of transactions #17653

Conflicts