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

remove the block fill rate limit of 70% when farming a block #19005

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions chia/_tests/core/mempool/test_mempool_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -1081,7 +1081,7 @@ async def make_and_send_big_cost_sb(coin: Coin) -> None:
g1 = sk.get_g1()
sig = AugSchemeMPL.sign(sk, IDENTITY_PUZZLE_HASH, g1)
aggsig = G2Element()
for _ in range(169):
for _ in range(242):
Copy link
Contributor

Choose a reason for hiding this comment

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

do you want to comment what this magic number is

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, it's not great. it's the number of transactions we need to fill the mempool sufficiently to trigger the expected number of rejections. going from fill rate 70% -> 100%, I simply took 169 / 0.7 to increase this number proportionally. I'll try to improve this a bit.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sent #19026 as an attempt to clarify this.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think I might prefer the simpler construct with a comment like Amine has above. The latest version here Arvid has more magic numbers :-)

conditions.append([ConditionOpcode.AGG_SIG_UNSAFE, g1, IDENTITY_PUZZLE_HASH])
aggsig += sig
conditions.append([ConditionOpcode.CREATE_COIN, IDENTITY_PUZZLE_HASH, coin.amount - 10_000_000])
Expand Down Expand Up @@ -2045,9 +2045,7 @@ async def fill_mempool_with_test_sbs(
# and without them we won't be able to get the test bundle in.
# This defaults to `MAX_BLOCK_COST_CLVM // 2`
full_node_api.full_node._mempool_manager.max_tx_clvm_cost = max_block_clvm_cost
# This defaults to `MAX_BLOCK_COST_CLVM * BLOCK_SIZE_LIMIT_FACTOR`
# TODO: Revisit this when we eventually raise the fille rate to 100%
# and `BLOCK_SIZE_LIMIT_FACTOR` is no longer relevant.
# This defaults to `MAX_BLOCK_COST_CLVM - BLOCK_OVERHEAD`
full_node_api.full_node._mempool_manager.mempool.mempool_info = dataclasses.replace(
full_node_api.full_node._mempool_manager.mempool.mempool_info,
max_block_clvm_cost=CLVMCost(max_block_clvm_cost),
Expand Down
3 changes: 1 addition & 2 deletions chia/full_node/mempool_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -162,12 +162,11 @@ def __init__(
# spends.
self.nonzero_fee_minimum_fpc = 5

BLOCK_SIZE_LIMIT_FACTOR = 0.7
# We need to deduct the block overhead, which consists of the wrapping
# quote opcode's bytes cost as well as its execution cost.
BLOCK_OVERHEAD = QUOTE_BYTES * self.constants.COST_PER_BYTE + QUOTE_EXECUTION_COST

self.max_block_clvm_cost = uint64(self.constants.MAX_BLOCK_COST_CLVM * BLOCK_SIZE_LIMIT_FACTOR - BLOCK_OVERHEAD)
self.max_block_clvm_cost = uint64(self.constants.MAX_BLOCK_COST_CLVM - BLOCK_OVERHEAD)
self.max_tx_clvm_cost = (
max_tx_clvm_cost if max_tx_clvm_cost is not None else uint64(self.constants.MAX_BLOCK_COST_CLVM // 2)
)
Expand Down
Loading