-
Notifications
You must be signed in to change notification settings - Fork 689
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 1 commit
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. | ||
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,19 @@ impl AllEpochConfig { | |
config.chunk_producer_kickout_threshold = 80; | ||
config.validator_selection_config.num_chunk_only_producer_seats = 200; | ||
} | ||
|
||
if chain_id == "testnet" | ||
&& checked_feature!("stable", TestnetMoreChunkOnlyProducers, protocol_version) | ||
{ | ||
let num_shards = config.shard_layout.num_shards() as usize; | ||
// On testnet, genesis config set num_block_producer_seats to 200 | ||
// This is to bring it back to 100 to be the same as on mainnet | ||
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. not 💯anymore This change will decrease the total number of validators as well. Maybe increase the number of cop to keep it high enough so that more people can join in as validators? I don't feel strongly about it either way, just brainstorming. 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.
fixed
Indeed, but with the current number of testnet validators it doesn't seem to be an issue. |
||
config.num_block_producer_seats = 20; | ||
// Technically, after ChunkOnlyProducers is enabled, this field is no longer used | ||
// We still set it here just in case | ||
config.num_block_producer_seats_per_shard = | ||
vec![config.num_block_producer_seats; num_shards]; | ||
} | ||
} | ||
|
||
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.