-
Notifications
You must be signed in to change notification settings - Fork 660
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
feat(COP): only in testnet: reduce the number of block producers #9563
Changes from 48 commits
fa266eb
ce230e1
3db13dc
41a772e
6b46020
0a3c16d
96a8181
97d5859
cb6a1f0
7dd8b32
c18ebd9
2103b39
cf4589a
d2d28b4
9c25249
475bd2b
b39ca93
e2865be
a18c96f
a611048
2595c0a
00ac8ec
e15802d
9192147
2a7d676
bc6acc0
5692811
50572d5
0993de3
c0e5de7
c265eb2
da939ed
889fd84
7a15a73
db153fa
c208642
2b7de2f
0b55942
54f0f7c
b9d2635
0d1f9ed
d11fa4f
89a8ae4
28b6e89
5506502
6f1d917
429391b
5bcd9b5
f696e63
f94e2dc
877f113
09f7c6a
5682fd5
e403572
dee2cc6
ae12179
0f706d2
f9d7c8a
f419514
54d775d
f92c9a5
f37ce4f
c37e310
dfd0420
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -83,11 +83,17 @@ pub struct AllEpochConfig { | |
use_production_config: bool, | ||
/// EpochConfig from genesis | ||
genesis_epoch_config: EpochConfig, | ||
/// Chain Id. Some parameters are specific to certain chains. | ||
chain_id: String, | ||
} | ||
|
||
impl AllEpochConfig { | ||
pub fn new(use_production_config: bool, genesis_epoch_config: EpochConfig) -> Self { | ||
Self { use_production_config, genesis_epoch_config } | ||
pub fn new( | ||
use_production_config: bool, | ||
genesis_epoch_config: EpochConfig, | ||
chain_id: &str, | ||
) -> Self { | ||
Self { use_production_config, genesis_epoch_config, chain_id: chain_id.to_string() } | ||
} | ||
|
||
pub fn for_protocol_version(&self, protocol_version: ProtocolVersion) -> EpochConfig { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When testing on mocknet please remember to set There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Above you suggested to remove There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure, as long as you do that first, you'll be fine. It will make my life easier as well when testing resharding. |
||
|
@@ -98,7 +104,7 @@ impl AllEpochConfig { | |
|
||
Self::config_nightshade(&mut config, protocol_version); | ||
|
||
Self::config_chunk_only_producers(&mut config, protocol_version); | ||
Self::config_chunk_only_producers(&mut config, &self.chain_id, protocol_version); | ||
|
||
Self::config_max_kickout_stake(&mut config, protocol_version); | ||
|
||
|
@@ -126,7 +132,11 @@ impl AllEpochConfig { | |
config.avg_hidden_validator_seats_per_shard = vec![0; num_shards]; | ||
} | ||
|
||
fn config_chunk_only_producers(config: &mut EpochConfig, protocol_version: u32) { | ||
fn config_chunk_only_producers( | ||
config: &mut EpochConfig, | ||
chain_id: &str, | ||
protocol_version: u32, | ||
) { | ||
if checked_feature!("stable", ChunkOnlyProducers, protocol_version) { | ||
let num_shards = config.shard_layout.num_shards() as usize; | ||
// On testnet, genesis config set num_block_producer_seats to 200 | ||
|
@@ -139,6 +149,20 @@ impl AllEpochConfig { | |
config.chunk_producer_kickout_threshold = 80; | ||
config.validator_selection_config.num_chunk_only_producer_seats = 200; | ||
} | ||
|
||
// Adjust the number of block and chunk producers for all chains except | ||
// mainnet, to make it easier to test the change. | ||
if chain_id != "mainnet" | ||
&& checked_feature!("stable", TestnetFewerBlockProducers, protocol_version) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This will include mocknet. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, and that is intentional. I'd like to test this change in mocknet. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, okay but I am a bit weary about diverging mocknet and mainnet. |
||
{ | ||
let num_shards = config.shard_layout.num_shards() as usize; | ||
// Decrease the number of block producers from 100 to 20. | ||
config.num_block_producer_seats = 20; | ||
config.num_block_producer_seats_per_shard = | ||
vec![config.num_block_producer_seats; num_shards]; | ||
// Decrease the number of chunk producers. | ||
config.validator_selection_config.num_chunk_only_producer_seats = 100; | ||
} | ||
} | ||
|
||
fn config_max_kickout_stake(config: &mut EpochConfig, protocol_version: u32) { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not related but it would be cool to remove
use_production_config
as a follow up. I don't remember why but it really annoyed me.Generally I love the idea of customizing EpochConfigs based on the chain. I don't know if the chain name is the best way to go about it but it is an improvement.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea, but better to do it in a separate PR.