-
Notifications
You must be signed in to change notification settings - Fork 37.7k
rpc: generateblock to allow multiple outputs #32468
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
base: master
Are you sure you want to change the base?
Conversation
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. Code Coverage & BenchmarksFor details see: https://corecheck.dev/bitcoin/bitcoin/pulls/32468. ReviewsSee the guideline for information on the review process. 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. |
cdcb630
to
a689525
Compare
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. HintsTry to run the tests locally, according to the documentation. However, a CI failure may still
Leave a comment here, if you need help tracking down a confusing failure. |
a689525
to
b98da78
Compare
The |
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. |
b98da78
to
fb28aea
Compare
Sure it would be. 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 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" |
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:
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.
Do you have any idea on how that could look like? As per your example would be:
|
Here is how the syntax looks for the
So maybe we could do something similar:
|
what if you want to split the reminder between multiple addresses? |
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. |
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) |
left some comments on your proposal PR
Still not sure about this, would like to know more opinions, for other things like |
How is it duplicating entries? In the example there is a different address for each entry. |
First address is duplicated. Anyway it is duplicated in case you want to assign an amount to an address + remainder. |
db5f8fe
to
9143274
Compare
9143274
to
5a5ff2a
Compare
🚧 At least one of the CI tasks failed. HintsTry to run the tests locally, according to the documentation. However, a CI failure may still
Leave a comment here, if you need help tracking down a confusing failure. |
7e8893c
to
cdd0826
Compare
I think this is ready for review. Custom rewards split can be done in a followup. IMO, this PR by itself is ok. |
cdd0826
to
f7a0929
Compare
Rebased on top of master e5f9218 |
5224306
to
f5b31fa
Compare
7f7abe3
to
ea5c4f7
Compare
912d9cb
to
847fd39
Compare
adc76f7
to
ae33a56
Compare
ae33a56
to
0d55bc4
Compare
Generatetomany
First approach was to create a
generatetomany
rpc call. That approach can be seen here polespinasa#3Generateblock
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.Motivation
https://x.com/SuperTestnet/status/1921220086645342550
Citating @supertestnet
Notes
Needs release noteChanges ongenerateblock
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.