-
Notifications
You must be signed in to change notification settings - Fork 38.7k
rpc: Add generateblock to mine a custom set of transactions #17653
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
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:
|
bc0fd7f to
54d67c5
Compare
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsReviewers, this pull request conflicts with the following ones:
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. |
54d67c5 to
066f26a
Compare
@instagibbs Most of the duplicated code to create the block is in
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 |
066f26a to
de2a77f
Compare
|
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. |
promag
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Concept ACK.
| {"transactions", RPCArg::Type::ARR, RPCArg::Optional::NO, "An array of hex strings which are either txids or raw transactions.\n" | ||
| "Txids must reference transactions currently in the mempool.", | ||
| { | ||
| {"rawtx/txid", RPCArg::Type::STR_HEX, RPCArg::Optional::OMITTED, ""}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess it's fine supporting both here.
| CMutableTransaction mtx; | ||
| if (ParseHashStr(str, hash)) { | ||
|
|
||
| LOCK(mempool.cs); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can replace this code with CTxMemPool::get.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
|
Closed by mistake. Reopened new PR #17693 |
…ransactions 7524b64 Add tests for generateblock (Andrew Toth) dcc8332 Add generateblock rpc (Andrew Toth) Pull request description: 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. This reopens bitcoin#17653 since it was closed by mistake. Thanks to instagibbs for code suggestions that I used here. ACKs for top commit: MarcoFalke: re-ACK 7524b64 📁 Tree-SHA512: 857106007465b5b9b8a84b6d07c17cbf8378a33a72d32ff79abea1d5ab4babb4d53a11ddbb14595aa1fac9dfa1391e3a11403d742f69951beea2f683e8a01cd4
…ransactions 7524b64 Add tests for generateblock (Andrew Toth) dcc8332 Add generateblock rpc (Andrew Toth) Pull request description: 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. This reopens bitcoin#17653 since it was closed by mistake. Thanks to instagibbs for code suggestions that I used here. ACKs for top commit: MarcoFalke: re-ACK 7524b64 📁 Tree-SHA512: 857106007465b5b9b8a84b6d07c17cbf8378a33a72d32ff79abea1d5ab4babb4d53a11ddbb14595aa1fac9dfa1391e3a11403d742f69951beea2f683e8a01cd4
Bitcoin Core PR: bitcoin/bitcoin#17693 Pull request description: 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. This reopens bitcoin/bitcoin#17653 since it was closed by mistake. Thanks to instagibbs for code suggestions that I used here.
The existing block generation rpcs for regtest,
generatetoaddressandgeneratetodescriptor, 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: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.