Skip to content

Conversation

polespinasa
Copy link
Contributor

@polespinasa polespinasa commented May 10, 2025

Generatetomany

First approach was to create a generatetomany rpc call. That approach can be seen here polespinasa#3

Generateblock

Modifies generateblock to allow a user to set multiple outputs in the coinbase transaction. If an empty set is provided, the block will be empty. If no set of transactions is provided, the block will mine the mempool. If a set of transactions is provided only that set of txs will be mined.

$./bitcoin-cli -rpcport=18443 generateblock '"bcrt1qw6qkj8wtw5hraxwxjp08m4aymkntms3yhqyacj"'
{
  "hash": "7359a3ed7af0f18f4016e6cb29193bd787a2452c584781135c585d5204b9ec5b"
}

$ ./bitcoin-cli -rpcport=18443 generateblock '"[\"bcrt1qal6p633hvwz2yp5mav0qy7u2az8gkn2xywnj6v\", \"bcrt1qvr3qgyhw6y0e0zj97v0j5yc40xtpea4wqj0g43\"]"'
{
  "hash": "6db028454b26d6667dd6df45be2772844121fb44f86433f6bbfbafa429974684"
}

$ ./bitcoin-cli -rpcport=18443 generateblock '"[\"bcrt1qal6p633hvwz2yp5mav0qy7u2az8gkn2xywnj6v\", \"bcrt1qvr3qgyhw6y0e0zj97v0j5yc40xtpea4wqj0g43\"]"' []
{
  "hash": "33e2ddc52546ddb0edf28bef5b295ee8fbd487b8aca19c97018467e2477877df"
}

Motivation

https://x.com/SuperTestnet/status/1921220086645342550

Citating @supertestnet

When mining a block on regtest, the command "generatetoaddress" is available if you want to send the entire coinbase to 1 address. Let's add generatetomany in case you want to split up the coinbase among multiple addresses, similar to how the "sendtoaddress" and "sendmany" commands both exist and let you send money to (1) one address or (2) multiple addresses. The generatetomany command would be particularly useful for simulating protocols that send money to multiple recipients directly from a coinbase, as several pools do now, like ocean and braidpool.

Notes

  • Needs release note
  • Changes on generateblock are not backwards compatible as now it takes an array of outputs instead a single output. This should not be critical as it is a test RPC.

@DrahtBot
Copy link
Contributor

DrahtBot commented May 10, 2025

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:

  • #33214 (rpc: require integer verbosity; remove boolean 'verbose' by fqlx)
  • #32317 (kernel: Separate UTXO set access from validation functions by TheCharlatan)

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.

@polespinasa
Copy link
Contributor Author

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.

@DrahtBot
Copy link
Contributor

🚧 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.

Hints

Try to run the tests locally, according to the documentation. However, a CI failure may still
happen due to a number of reasons, for example:

  • Possibly due to a silent merge conflict (the changes in this pull request being
    incompatible with the current code in the target branch). If so, make sure to rebase on the latest
    commit of the target branch.

  • A sanitizer issue, which can only be found by compiling with the sanitizer and running the
    affected test.

  • An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

@andrewtoth
Copy link
Contributor

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.

@polespinasa
Copy link
Contributor Author

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.

@andrewtoth
Copy link
Contributor

But this change would not be backwards compatible

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

@polespinasa
Copy link
Contributor Author

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.

@supertestnet
Copy link

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"

@polespinasa
Copy link
Contributor Author

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}]

@supertestnet
Copy link

supertestnet commented May 11, 2025

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

@polespinasa
Copy link
Contributor Author

note the remainder in the last btc address

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

@lollerfirst
Copy link

lollerfirst commented May 11, 2025

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?

@lollerfirst
Copy link

lollerfirst commented May 11, 2025

@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.

@polespinasa
Copy link
Contributor Author

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
Copy link
Contributor Author

polespinasa commented May 11, 2025

@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.

@lollerfirst
Copy link

duplicating entries

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

@polespinasa
Copy link
Contributor Author

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.

@polespinasa polespinasa force-pushed the generatetomany branch 3 times, most recently from db5f8fe to 9143274 Compare May 12, 2025 08:55
@DrahtBot
Copy link
Contributor

🚧 At least one of the CI tasks failed.
Task lint: https://github.com/bitcoin/bitcoin/runs/43951458497
LLM reason (✨ experimental): The CI failure is caused by a lint check failure due to release notes snippets being placed in the wrong folder.

Hints

Try to run the tests locally, according to the documentation. However, a CI failure may still
happen due to a number of reasons, for example:

  • Possibly due to a silent merge conflict (the changes in this pull request being
    incompatible with the current code in the target branch). If so, make sure to rebase on the latest
    commit of the target branch.

  • A sanitizer issue, which can only be found by compiling with the sanitizer and running the
    affected test.

  • An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

@polespinasa
Copy link
Contributor Author

I think this is ready for review.

Custom rewards split can be done in a followup. IMO, this PR by itself is ok.
@lollerfirst is working on it here polespinasa#1

@polespinasa
Copy link
Contributor Author

Rebased on top of master e5f9218

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants