Skip to content
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

Added max_weight parameter for mining #9234

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

SChernykh
Copy link
Contributor

@SChernykh SChernykh commented Mar 10, 2024

max_weight allows miners to limit the weight of the blocks they mine. Default is 0 (unlimited).

Added to get_block_template RPC, start_mining RPC and start_mining console command.

Disclaimer: this hasn't been tested yet, and I might have missed some places in the code that need changing.

@SChernykh
Copy link
Contributor Author

I did a quick test, it works as expected:

start_mining 44MnN1f3Eto8DZYUWuE5XZNUtE3vcRzt2j6PzqWpPau34e6Cf4fAxt6X2MBmrm6F9YMEiMNjN6W4Shn4pLcfNAja621jwyg 1 false true 100000
2024-03-11 19:39:54.147 W Block template filled with 54 txes, weight 99017/100000, coinbase 0.603093800000 (including 0.003093800000 in fees)
2024-03-11 19:39:54.163 I Miner thread was started [0]
2024-03-11 19:39:55.882 W Block template filled with 54 txes, weight 99017/100000, coinbase 0.603093800000 (including 0.003093800000 in fees)
2024-03-11 19:40:01.101 W Block template filled with 54 txes, weight 99017/100000, coinbase 0.603093800000 (including 0.003093800000 in fees)
2024-03-11 19:40:08.476 W Block template filled with 54 txes, weight 99017/100000, coinbase 0.603093800000 (including 0.003093800000 in fees)
2024-03-11 19:40:15.194 W Block template filled with 54 txes, weight 99020/100000, coinbase 0.603277880000 (including 0.003277880000 in fees)
2024-03-11 19:40:26.554 W Block template filled with 54 txes, weight 99020/100000, coinbase 0.603277880000 (including 0.003277880000 in fees)
stop_mining2024-03-11 19:40:31.741      W Block template filled with 54 txes, weight 99020/100000, coinbase 0.603277880000 (including 0.003277880000 in fees)

2024-03-11 19:40:31.960 I Miner thread stopped [0]
Mining stopped
start_mining 44MnN1f3Eto8DZYUWuE5XZNUtE3vcRzt2j6PzqWpPau34e6Cf4fAxt6X2MBmrm6F9YMEiMNjN6W4Shn4pLcfNAja621jwyg 1 false true 200000
2024-03-11 19:40:37.757 W Block template filled with 119 txes, weight 199407/200000, coinbase 0.605285620000 (including 0.005285620000 in fees)
2024-03-11 19:40:37.757 I Miner thread was started [0]
st2024-03-11 19:40:40.647       W Block template filled with 119 txes, weight 199407/200000, coinbase 0.605285620000 (including 0.005285620000 in fees)
op_mining
2024-03-11 19:40:42.818 I Miner thread stopped [0]
Mining stopped
start_mining 44MnN1f3Eto8DZYUWuE5XZNUtE3vcRzt2j6PzqWpPau34e6Cf4fAxt6X2MBmrm6F9YMEiMNjN6W4Shn4pLcfNAja621jwyg 1 false true 300000
2024-03-11 19:40:46.366 W Block template filled with 154 txes, weight 260840/300000, coinbase 0.606647700000 (including 0.006647700000 in fees)
2024-03-11 19:40:46.366 I Miner thread was started [0]
2024-03-11 19:40:47.554 W Block template filled with 156 txes, weight 263911/300000, coinbase 0.606709120000 (including 0.006709120000 in fees)
2024-03-11 19:40:57.772 W Block template filled with 168 txes, weight 288800/300000, coinbase 0.607207100000 (including 0.007207100000 in fees)
2024-03-11 19:41:06.444 W Block template filled with 175 txes, weight 299524/300000, coinbase 0.607421580000 (including 0.007421580000 in fees)
2024-03-11 19:41:14.554 W Block template filled with 175 txes, weight 299524/300000, coinbase 0.607421580000 (including 0.007421580000 in fees)
stop_mining
2024-03-11 19:41:29.413 I Miner thread stopped [0]
Mining stopped

@SChernykh SChernykh marked this pull request as ready for review March 11, 2024 19:43
@SyntheticBird45
Copy link

side note: I'll add it to the openrpc document

Copy link
Collaborator

@selsta selsta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bump CORE_RPC_VERSION_MINOR and WALLET_RPC_VERSION_MINOR.

`max_weight` allows miners to limit the weight of the blocks they mine. Default is 0 (unlimited).

Added to `get_block_template` RPC, `start_mining` RPC and `start_mining` console command.
@SChernykh
Copy link
Contributor Author

@selsta RPC versions bumped

@expiredhotdog
Copy link

Are we sure that the default should be no limit? It's not clear that relying on miners to set it themselves will result in any effective limit on block sizes. Unless that's not the goal in the first place, and rather only to give miners the option to, if they choose.

@SChernykh
Copy link
Contributor Author

@expiredhotdog There is still a limit enforced by the median block size.

@expiredhotdog
Copy link

@SChernykh Right, but I mean in the context of this PR.

@SChernykh
Copy link
Contributor Author

In the context of this PR, default behavior (0, no limit) is the same as without this PR. Right now there is no limit.

@nahuhh
Copy link

nahuhh commented Mar 26, 2024

im not sure why this is needed if you can set the max txpool weight. This seems to just enabke malicious or poorly equiped miners to mess with averages.

txpool shouldnt be allowed to be set smaller than 2x the median, and it should purge/replace tx via fees.

And miners shouldnt be allowed to produce blocks smaller than the calculated size of the block based on effect of the highest paying tx's in the txpool on the median block size. (if the next block would increase by a max of 2% [102%], nobody should be intentionally producing blocks are 98%..)

@SChernykh
Copy link
Contributor Author

im not sure why this is needed if you can set the max txpool weight

If you set max txpool weight, your node will remove transactions from txpool and will have to re-download them when a new block (containing them) arrives. This will put additional load on the network.

And miners shouldnt be allowed to produce blocks smaller than the calculated size of the block based on effect of the highest paying tx's in the txpool on the median block size. (if the next block would increase by a max of 2% [102%], nobody should be intentionally producing blocks are 98%..)

Impossible to implement as txpool contents are not a part of consensus, and it can't be coherent between different nodes.

@nahuhh
Copy link

nahuhh commented Mar 26, 2024

im not sure why this is needed if you can set the max txpool weight

If you set max txpool weight, your node will remove transactions from txpool and will have to re-download them when a new block (containing them) arrives. This will put additional load on the network.

as does mining less tx than are being paid to be mined. Constantly full tx pools arent effienient at all.
if you want to slow down the network, limiting your block size is, lol, bitcoin?

meanwhile, btc increases txpool and keeos a small blocksize, trying to force tx to be purged in a market that relies on bidding on limited blockspace.
In monero you bid for Inclusion. Not bid to exclude others.

And miners shouldnt be allowed to produce blocks smaller than the calculated size of the block based on effect of the highest paying tx's in the txpool on the median block size. (if the next block would increase by a max of 2% [102%], nobody should be intentionally producing blocks are 98%..)
Impossible to implement as txpool contents are not a part of consensus, and it can't be coherent between different nodes.

agreed. This just makes it possible for me/blockstream to coordinate with top pools to control the blocksize of monero?

let me ask this a different way:
why would this be a good feature?
Pros = edge case for mining on your car radio?

and cons =

  • slower tx confirmation for users
  • bigger txpool
  • creates tx fee environment to purge tx
  • if blocks grow to 1mb, and people dont update. Lol?

I dont see how this pr makes any sense

@SChernykh
Copy link
Contributor Author

This max_weight setting is meant for miners who would otherwise fall behind in syncing and not being able to mine blocks at all. No one is encouraging miners to limit their blocks (the setting is "unlimited" by default), plus they have an incentive (tx fees) to not do it as long as they can keep up with the network.

@nahuhh
Copy link

nahuhh commented Mar 26, 2024

This max_weight setting is meant for miners who would otherwise fall behind in syncing and not being able to mine blocks at all. No one is encouraging miners to limit their blocks (the setting is "unlimited" by default), plus they have an incentive (tx fees) to not do it as long as they can keep up with the network.

The incentive didnt work for mordinals or bitcoin.
miners dont want big blocks, they want bidding

why pay for storage, if you can get same the money without storing any extra tx?

@SChernykh
Copy link
Contributor Author

SChernykh commented Mar 26, 2024

You do realise that big pools who have devs on payroll, and even individual solo miners can recompile their nodes and introduce any restrictions they want? Even without this PR.

P.S. Maybe some already did...

@nahuhh
Copy link

nahuhh commented Mar 26, 2024

You do realise that big pools who have devs on payroll, and even individual solo miners can recompile their nodes and introduce any restrictions they want? Even without this PR.

P.S. Maybe some already did...

I do
and id like to make that harder, not easier

@SChernykh
Copy link
Contributor Author

It's already very easy with the command line parameters to limit the txpool. But these parameters hurt more than the specific limiter for the mining only.

@nahuhh
Copy link

nahuhh commented Mar 26, 2024

It's already very easy with the command line parameters to limit the txpool. But these parameters hurt more than the specific limiter for the mining only.

Yeah. I dont like that one can limit the txpool to unreasonably low sizes either.

But if there are 600mb of pending tx,maybe miners need to stop mining intentionally small blocks, yeah?

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.

None yet

7 participants