Skip to content

Commit

Permalink
Merge branch 'master' into header-version-check
Browse files Browse the repository at this point in the history
  • Loading branch information
jancionear committed Oct 28, 2024
2 parents f1335a7 + 54f53c8 commit 3a0cec8
Show file tree
Hide file tree
Showing 5 changed files with 24 additions and 33 deletions.
2 changes: 1 addition & 1 deletion chain/chain/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -570,7 +570,7 @@ mod tests {
let shard_ids: Vec<_> = (0..32).map(ShardId::new).collect();
let genesis_chunks = genesis_chunks(
vec![Trie::EMPTY_ROOT],
vec![Default::default(); shard_ids.len()],
vec![Some(Default::default()); shard_ids.len()],
&shard_ids,
1_000_000,
0,
Expand Down
11 changes: 0 additions & 11 deletions chain/chain/src/validate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -220,17 +220,6 @@ fn validate_bandwidth_requests(
extra_bandwidth_requests: Option<&BandwidthRequests>,
header_bandwidth_requests: Option<&BandwidthRequests>,
) -> Result<(), Error> {
if extra_bandwidth_requests.is_none()
&& header_bandwidth_requests == Some(&BandwidthRequests::empty())
{
// This corner case happens for the first chunk that has the BandwidthScheduler feature enabled.
// The previous chunk was applied with a protocol version which doesn't have bandwidth scheduler
// enabled and because of that the bandwidth requests in ChunkExtra are None.
// The header was produced in the new protocol version, and the newer version of header always
// has some bandwidth requests, it's not an `Option`. Because of that the header requests are `Some(BandwidthRequests::empty())`.
return Ok(());
}

if extra_bandwidth_requests != header_bandwidth_requests {
fn requests_len(requests_opt: Option<&BandwidthRequests>) -> usize {
match requests_opt {
Expand Down
2 changes: 1 addition & 1 deletion chain/network/src/network_protocol/testonly.rs
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ impl ChunkSet {
// Consider making this more realistic.
let chunks = genesis_chunks(
vec![StateRoot::new()],
vec![Default::default(); shard_ids.len()],
vec![Some(Default::default()); shard_ids.len()],
&shard_ids,
1000,
0,
Expand Down
2 changes: 1 addition & 1 deletion core/primitives/benches/serialization.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ fn create_block() -> Block {
let shard_ids = vec![ShardId::new(0)];
let genesis_chunks = genesis_chunks(
vec![StateRoot::new()],
vec![Default::default(); shard_ids.len()],
vec![Some(Default::default()); shard_ids.len()],
&shard_ids,
1_000,
0,
Expand Down
40 changes: 21 additions & 19 deletions core/primitives/src/sharding.rs
Original file line number Diff line number Diff line change
Expand Up @@ -306,9 +306,13 @@ impl ShardChunkHeaderV3 {
bandwidth_requests: Option<BandwidthRequests>,
signer: &ValidatorSigner,
) -> Self {
let Some(congestion_info) = congestion_info else {
// Old inner without congestion info
let inner = ShardChunkHeaderInner::V2(ShardChunkHeaderInnerV2 {
let inner = if let Some(bandwidth_requests) = bandwidth_requests {
// `bandwidth_requests` can only be `Some` when bandwidth scheduler is enabled.
assert!(ProtocolFeature::BandwidthScheduler.enabled(protocol_version));

// Congestion control has to be enabled before bandwidth scheduler
assert!(ProtocolFeature::CongestionControl.enabled(protocol_version));
ShardChunkHeaderInner::V4(ShardChunkHeaderInnerV4 {
prev_block_hash,
prev_state_root,
prev_outcome_root,
Expand All @@ -322,17 +326,14 @@ impl ShardChunkHeaderV3 {
prev_outgoing_receipts_root,
tx_root,
prev_validator_proposals,
});
return Self::from_inner(inner, signer);
};
// `congestion_info`` can only be `Some` when congestion control is enabled.
assert!(ProtocolFeature::CongestionControl.enabled(protocol_version));

let bandwidth_requests = bandwidth_requests.unwrap_or_else(BandwidthRequests::empty);

let inner = if ProtocolFeature::BandwidthScheduler.enabled(protocol_version) {
congestion_info: congestion_info
.expect("Congestion info must exist when bandwidth scheduler is enabled"),
bandwidth_requests,
})
} else if let Some(congestion_info) = congestion_info {
// `congestion_info`` can only be `Some` when congestion control is enabled.
assert!(ProtocolFeature::CongestionControl.enabled(protocol_version));
ShardChunkHeaderInner::V4(ShardChunkHeaderInnerV4 {
ShardChunkHeaderInner::V3(ShardChunkHeaderInnerV3 {
prev_block_hash,
prev_state_root,
prev_outcome_root,
Expand All @@ -347,10 +348,9 @@ impl ShardChunkHeaderV3 {
tx_root,
prev_validator_proposals,
congestion_info,
bandwidth_requests,
})
} else {
ShardChunkHeaderInner::V3(ShardChunkHeaderInnerV3 {
ShardChunkHeaderInner::V2(ShardChunkHeaderInnerV2 {
prev_block_hash,
prev_state_root,
prev_outcome_root,
Expand All @@ -364,7 +364,6 @@ impl ShardChunkHeaderV3 {
prev_outgoing_receipts_root,
tx_root,
prev_validator_proposals,
congestion_info,
})
};
Self::from_inner(inner, signer)
Expand Down Expand Up @@ -607,10 +606,13 @@ impl ShardChunkHeader {
// Note that we allow V2 in the congestion control version.
// That is because the first chunk where this feature is
// enabled does not have the congestion info.
ShardChunkHeaderInner::V2(_) => version >= BLOCK_HEADER_V3_VERSION,
ShardChunkHeaderInner::V3(_) => {
version >= CONGESTION_CONTROL_VERSION && version < BANDWIDTH_SCHEDULER_VERSION
// In bandwidth scheduler version v3 and v4 are allowed. The first chunk in
// the bandwidth scheduler version will be v3 because the chunk extra for the
// last chunk of previous version doesn't have bandwidth requests.
ShardChunkHeaderInner::V2(_) => {
version >= BLOCK_HEADER_V3_VERSION && version < BANDWIDTH_SCHEDULER_VERSION
}
ShardChunkHeaderInner::V3(_) => version >= CONGESTION_CONTROL_VERSION,
ShardChunkHeaderInner::V4(_) => version >= BANDWIDTH_SCHEDULER_VERSION,
},
};
Expand Down

0 comments on commit 3a0cec8

Please sign in to comment.