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

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/32468.

Reviews

See the guideline for information on the review process.
A summary of reviews will appear here.

Conflicts

Reviewers, this pull request conflicts with the following ones:

• #32317 (kernel: Separate UTXO set access from validation functions by sedited)

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.

LLM Linter (✨ experimental)

Possible typos and grammar issues:

• The reward is split in equal parts through all addresses and descriptors provided. -> The reward is split equally among all addresses and descriptors provided. [original phrasing ("through ...") is ungrammatical and can confuse which items are being divided among]
• The addresses or descriptors to split, in equal parts, the coinbase reward among. -> The addresses or descriptors among which to split the coinbase reward in equal parts. [word order is incorrect and makes the sentence hard to parse]
• Generate an empty block without address and fallback to OP_RETURN -> Generate an empty block without an address and fall back to OP_RETURN. ["without address" is missing an article; "fallback" should be the verb phrase "fall back" for clarity]
• Generate block with multiple txid -> Generate block with multiple txids. ["txid" should be plural here to match "multiple"]

^{drahtbot_id_5_m}

I would like to discuss how to implement reward distribution. Do we ask the user for the exact number of bitcoins for each address or do we opt to use percentages of the reward?

IMHO it is better to use percentages, it makes things easier for the user.

🚧 At least one of the CI tasks failed.
_{Task tidy: https://github.com/bitcoin/bitcoin/runs/41993075774
LLM reason (✨ experimental): The CI failure is due to a build error related to missing members in node::BlockAssembler::Options.}

The generateblock command takes either an address or descriptor as its first argument. Perhaps it would be easier to extend that RPC to also take an array of address or descriptor and value pairs? That way we won't need a new RPC.

Perhaps it would be easier to extend that RPC to also take an array of address or descriptor and value pairs?

That would be easy as the code is already done on the new RPC. But this change would not be backwards compatible, idk if we want to keep it that way.

There are some softwares like Polar that would be affected by this.

But this change would not be backwards compatible

Sure it would be. Just parse an array first and if that fails parse a string.

Just parse an array first and if that fails parse a string.

Oh right, that's an option. I will let it as it is for now to see other contributors' opinions.

May take your approach later.

Can a PSBT be constructed with only outputs, no inputs? If so, then maybe generateblock could accept a psbt or a string as an argument. If it is a psbt, it must contain outputs but no inputs.

But also, due to fees being variable, it may be useful to have some kind of "remainder" option so that you can say, for example, "give 1 btc apiece to the first 3 addresses, and give the remainder to the last address"

Can a PSBT be constructed with only outputs, no inputs?

I'm not sure using a PSBT is the best approach. If we check the normal workflow for a PSBT we see it is intended to:

1. Create the PSBT with inputs and outputs but no metadata
2. Updated with the data needed to spend the UTXOs in the inputs
3. Signed
4. Finalized

IMO using PSBTs here does not make sense, we don't need any of that, we don't need to share with other users or software to sign or add information to inputs. It's easier to just provide a JSON with the addresses and amounts (it's the only info we need), also using a JSON it's easier to implement with less changes.

But also, due to fees being variable, it may be useful to have some kind of "remainder" option so that you can say, for example, "give 1 btc apiece to the first 3 addresses, and give the remainder to the last address"

Do you have any idea on how that could look like?
Maybe something like this; where amount_fixed takes a number and share_reminder is a boolean:
generatetomany 1 [{addr1, amount_fixed, share_remainder}, ...]

As per your example would be:

generatetomany 1 [{bcrt1qal6p633hvwz2yp5mav0qy7u2az8gkn2xywnj6v, 1, 0}, {bcrt1q3me8uxjzw2egq76hjw6zcgzmlhlzzztfwqzjfv, 1, 0}, {bcrt1q57yu04dn4l0zxv6ul4prm7favr630d3lmwvjh0, 1, 0}, {bcrt1qal6p633hvwz2yp5mav0qy7u2az8gkn2xywnj6v, 0, 1}]

Do you have any idea on how that could look like?

Here is how the syntax looks for the sendmany command which I am using as inspiration:

bitcoin-cli sendmany "" "{\"bc1q09vm5lfy0j5reeulh4x5752q25uqqvz34hufdl\":0.01,\"bc1q02ad21edsxd23d32dfgqqsz4vv4nmtfzuklhy3\":0.02}"

So maybe we could do something similar:

bitcoin-cli generatetomany "{\"bcrt1qal6p633hvwz2yp5mav0qy7u2az8gkn2xywnj6v\":1,\"bcrt1q3me8uxjzw2egq76hjw6zcgzmlhlzzztfwqzjfv\":1,\"bcrt1q57yu04dn4l0zxv6ul4prm7favr630d3lmwvjh0\":1,\"bcrt1qal6p633hvwz2yp5mav0qy7u2az8gkn2xywnj6v\":\"remainder\"}" <-- note the remainder in the last btc address

note the remainder in the last btc address

what if you want to split the reminder between multiple addresses?

note the remainder in the last btc address

what if you want to split the reminder between multiple addresses?

bitcoin-cli generatetomany 5 "{\"bcrt1qal6p633hvwz2yp5mav0qy7u2az8gkn2xywnj6v\":1,\"bcrt1q3me8uxjzw2egq76hjw6zcgzmlhlzzztfwqzjfv\":1,\"bcrt1q57yu04dn4l0zxv6ul4prm7favr630d3lmwvjh0\":1,\"bcrt1qal6p633hvwz2yp5mav0qy7u2az8gkn2xywnj6v\":\"remainder\", \"bcrt1qz07rr2j8r699c9r55rt44q9vm3faxgprlv62xx\":\"remainder\", \"bcrt1q9vur8vqhfndr8r76eveuu2wr46f2rs8fgv2kvh\":\"remainder\"}"

All fields with "remainder" to have the remainder split equally amongst them?

@polespinasa @supertestnet Please check out polespinasa#1 with the discussed changes and let me know what you think.

Also maybe we would want to follow @andrewtoth advice and not create a new rpc command in the first place.

All fields with "remainder" to have the remainder split equally amongst them?

I don't see duplicating entries as a good option. Could lead to errors. I think is better to have two camps for each address entry, one for the amount (can be 0) and one for reminder (can be done with 0 or 1 or by setting "remainder" or an empty string)

@polespinasa @supertestnet Please check out polespinasa#1 with the discussed changes and let me know what you think.

left some comments on your proposal PR

Also maybe we would want to follow @andrewtoth advice and not create a new rpc command in the first place.

Still not sure about this, would like to know more opinions, for other things like send we have multiple RPCs. Anyway, it's an easy change, so it's something we can do at the end if we want.

duplicating entries

How is it duplicating entries? In the example there is a different address for each entry.

How is it duplicating entries?

First address is duplicated. Anyway it is duplicated in case you want to assign an amount to an address + remainder.

Rebased on top of master (after #33230 was merged).

The diff is simple (plus some changes in test file):

diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp
index 320823b1c1..acbdc14db8 100644
--- a/src/rpc/client.cpp
+++ b/src/rpc/client.cpp
@@ -37,7 +37,7 @@ static const CRPCConvertParam vRPCConvertParams[] =
     { "generatetoaddress", 2, "maxtries" },
     { "generatetodescriptor", 0, "num_blocks" },
     { "generatetodescriptor", 2, "maxtries" },
-    //{ "generateblock", 0, "outputs"},
+    { "generateblock", 0, "outputs", /*also_string=*/true},
     { "generateblock", 1, "transactions" },
     { "generateblock", 2, "submit" },
     { "getnetworkhashps", 0, "nblocks" },

--- a/src/rpc/mining.cpp
+++ b/src/rpc/mining.cpp
@@ -343,13 +343,14 @@ static RPCHelpMan generateblock()
 {
     UniValue address_or_descriptor = UniValue(UniValue::VARR);
     UniValue parsed;
-    if (!request.params[0].isNull() && parsed.read(request.params[0].get_str())) {
-        if (!parsed.isArray()) {
+    if (!request.params[0].isNull()) {
+        if (request.params[0].isArray()) {
+            address_or_descriptor = request.params[0].get_array();
+        } else if (request.params[0].isStr()){
+            address_or_descriptor.push_back(request.params[0]);
+        } else {
             throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Error: Invalid address or descriptor");
         }
-        address_or_descriptor = parsed.get_array();
-    } else if (!request.params[0].isNull()) {
-        address_or_descriptor.push_back(request.params[0]);
     }