Skip to content

Commit

Permalink
fix: Add bounds to event collections
Browse files Browse the repository at this point in the history
  • Loading branch information
aidan46 committed Nov 27, 2024
1 parent 97a9170 commit b23660f
Show file tree
Hide file tree
Showing 6 changed files with 57 additions and 43 deletions.
31 changes: 16 additions & 15 deletions pallets/storage-provider/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,7 @@ pub mod pallet {

extern crate alloc;

use alloc::{
collections::{BTreeMap, BTreeSet},
vec,
vec::Vec,
};
use alloc::{collections::BTreeMap, vec, vec::Vec};
use core::fmt::Debug;

use cid::Cid;
Expand Down Expand Up @@ -75,7 +71,7 @@ pub mod pallet {
DeclareFaultsParams, DeclareFaultsRecoveredParams, FaultDeclaration,
RecoveryDeclaration,
},
partition::PartitionNumber,
partition::{PartitionNumber, MAX_PARTITIONS_PER_DEADLINE},
proofs::{assign_proving_period_offset, SubmitWindowedPoStParams},
sector::{
ProveCommitResult, ProveCommitSector, SectorOnChainInfo, SectorPreCommitInfo,
Expand Down Expand Up @@ -283,7 +279,7 @@ pub mod pallet {
SectorsSlashed {
owner: T::AccountId,
// No need for a bounded collection as we produce the output ourselves.
sector_numbers: Vec<SectorNumber>,
sector_numbers: BoundedVec<SectorNumber, ConstU32<MAX_SECTORS>>,
},
/// Emitted when an SP submits a valid PoSt
ValidPoStSubmitted { owner: T::AccountId },
Expand All @@ -300,8 +296,11 @@ pub mod pallet {
/// Emitted when an SP doesn't submit Windowed PoSt in time and PoSt hook marks partitions as faulty
PartitionsFaulty {
owner: T::AccountId,
// No need to be bounded as we are creating the sets ourselves
faulty_partitions: BTreeMap<PartitionNumber, BTreeSet<SectorNumber>>,
faulty_partitions: BoundedBTreeMap<
PartitionNumber,
BoundedBTreeSet<SectorNumber, ConstU32<MAX_SECTORS>>,
ConstU32<MAX_PARTITIONS_PER_DEADLINE>,
>,
},
/// Emitted when an SP terminates some sectors.
SectorsTerminated {
Expand Down Expand Up @@ -1105,12 +1104,12 @@ pub mod pallet {
return;
}

let mut removed_sectors = Vec::new();
let mut removed_sectors = BoundedVec::new();
log::info!(target: LOG_TARGET, "found {} expired pre committed sectors for {:?}", expired.len(), storage_provider);
for sector_number in expired {
// Expired sectors should be removed, because in other case they'd be processed twice in the next block.
if let Ok(()) = state.remove_pre_committed_sector(sector_number) {
removed_sectors.push(sector_number)
removed_sectors.force_push(sector_number)
} else {
log::error!(target: LOG_TARGET, "catastrophe, failed to remove sector {} for {:?}", sector_number, storage_provider);
continue;
Expand Down Expand Up @@ -1253,8 +1252,10 @@ pub mod pallet {

let mut faulty_partitions_amount = 0;
// Create collection for fault partitions, 1 event per SP
let mut faulty_partitions: BTreeMap<PartitionNumber, BTreeSet<SectorNumber>> =
BTreeMap::new();
let mut faulty_partitions: BTreeMap<
PartitionNumber,
BoundedBTreeSet<SectorNumber, ConstU32<MAX_SECTORS>>,
> = BTreeMap::new();
for (partition_number, partition) in deadline.partitions.iter_mut() {
if partition.sectors.len() == 0 {
continue;
Expand Down Expand Up @@ -1288,7 +1289,7 @@ pub mod pallet {
current_block, storage_provider, partition_number, new_faults.len());

if new_faults.len() > 0 {
faulty_partitions.insert(*partition_number, new_faults);
faulty_partitions.insert(*partition_number, new_faults.try_into().expect("should be able to create BoundedBTreeSet due to input being bounded"));
faulty_partitions_amount += 1;
}
}
Expand All @@ -1303,7 +1304,7 @@ pub mod pallet {
);
Self::deposit_event(Event::PartitionsFaulty {
owner: storage_provider.clone(),
faulty_partitions,
faulty_partitions: faulty_partitions.try_into().expect("should be able to create a BTreeMap with a MAX_PARTITIONS_PER_DEADLINE bound after iterating over a map with the same bound"),
})
} else {
log::info!(target: LOG_TARGET, "block: {:?}, sp: {:?}, deadline: {:?} - all proofs submitted on time.",
Expand Down
22 changes: 13 additions & 9 deletions pallets/storage-provider/src/tests/post_hook.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,7 @@
extern crate alloc;

use alloc::collections::{BTreeMap, BTreeSet};

use frame_support::{assert_ok, pallet_prelude::Get};
use primitives_proofs::{DealId, SectorNumber};
use sp_core::bounded_vec;
use sp_runtime::{BoundedBTreeMap, BoundedBTreeSet};

use super::new_test_ext;
use crate::{
Expand Down Expand Up @@ -70,18 +67,25 @@ fn marks_partitions_as_faulty() {
let partition = &deadline.partitions[&0];
let expected_sectors =
sector_set::<MAX_SECTORS>(&[first_sector_number, second_sector_number]);
let faulty_sectors = BTreeSet::from([
SectorNumber::new(first_sector_number).unwrap(),
SectorNumber::new(second_sector_number).unwrap(),
]);
let mut expected_faulty_sectors = BoundedBTreeSet::new();
expected_faulty_sectors
.try_insert(SectorNumber::new(first_sector_number).unwrap())
.unwrap();
expected_faulty_sectors
.try_insert(SectorNumber::new(second_sector_number).unwrap())
.unwrap();
assert_eq!(partition.faults.len(), 2);
assert_eq!(expected_sectors, partition.faults);
let mut expected_faulty_partitions = BoundedBTreeMap::new();
expected_faulty_partitions
.try_insert(0, expected_faulty_sectors)
.unwrap();
assert_eq!(
events(),
[RuntimeEvent::StorageProvider(
Event::<Test>::PartitionsFaulty {
owner: account(storage_provider),
faulty_partitions: BTreeMap::from([(0u32, faulty_sectors)]),
faulty_partitions: expected_faulty_partitions,
}
),]
);
Expand Down
20 changes: 11 additions & 9 deletions pallets/storage-provider/src/tests/pre_commit_sector_hook.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
extern crate alloc;

use alloc::collections::{BTreeMap, BTreeSet};

use primitives_proofs::SectorNumber;
use sp_core::bounded_vec;
use sp_runtime::{BoundedBTreeMap, BoundedBTreeSet};

use super::new_test_ext;
use crate::{
Expand Down Expand Up @@ -79,15 +76,20 @@ fn pre_commit_hook_slashed_deal() {
Balances::reserved_balance(account(storage_provider)),
deal_precommit_deposit
);
let mut expected_faulty_sectors = BoundedBTreeSet::new();
expected_faulty_sectors
.try_insert(SectorNumber::new(2).unwrap())
.unwrap();
let mut expected_faulty_partitions = BoundedBTreeMap::new();
expected_faulty_partitions
.try_insert(0, expected_faulty_sectors)
.unwrap();
assert_eq!(
events(),
[
RuntimeEvent::StorageProvider(Event::<Test>::PartitionsFaulty {
owner: account(storage_provider),
faulty_partitions: BTreeMap::from([(
0,
BTreeSet::from([SectorNumber::new(2).unwrap()])
)]),
faulty_partitions: expected_faulty_partitions,
}),
// the slash -> withdraw is related to the usage of slash_and_burn
// when slashing the SP for a failed pre_commit
Expand All @@ -108,7 +110,7 @@ fn pre_commit_hook_slashed_deal() {
}),
RuntimeEvent::StorageProvider(Event::<Test>::SectorsSlashed {
owner: account(storage_provider),
sector_numbers: vec![1.into()],
sector_numbers: bounded_vec![1.into()],
}),
]
);
Expand Down
Binary file modified storagext/lib/artifacts/metadata.scale
Binary file not shown.
23 changes: 13 additions & 10 deletions storagext/lib/src/runtime/display/storage_provider.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ impl std::fmt::Display for Event {
"Sectors Slashed: {{ owner: {}, sector_numbers: {} }}",
owner,
itertools::Itertools::intersperse(
sector_numbers.iter().map(ToString::to_string),
sector_numbers.0.iter().map(ToString::to_string),
", ".to_string()
)
.collect::<String>(),
Expand Down Expand Up @@ -127,15 +127,18 @@ impl std::fmt::Display for Event {
"Faulty Partitions: {{ owner: {}, faulty_partitions: [{}] }}",
owner,
itertools::Itertools::intersperse(
faulty_partitions.iter().map(|(partition, sectors)| format!(
"{{ partition: {}, sectors: {} }}",
partition,
itertools::Itertools::intersperse(
sectors.iter().map(ToString::to_string),
", ".to_string()
)
.collect::<String>()
)),
faulty_partitions
.0
.iter()
.map(|(partition, sectors)| format!(
"{{ partition: {}, sectors: {} }}",
partition,
itertools::Itertools::intersperse(
sectors.0.iter().map(ToString::to_string),
", ".to_string()
)
.collect::<String>()
)),
", ".to_string()
)
.collect::<String>()
Expand Down
4 changes: 4 additions & 0 deletions storagext/lib/src/runtime/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,10 @@ pub mod display;
path = "bounded_collections::bounded_btree_set::BoundedBTreeSet",
derive = "::serde::Serialize"
),
derive_for_type(
path = "bounded_collections::bounded_btree_map::BoundedBTreeMap",
derive = "::serde::Serialize"
),
derive_for_type(
path = "pallet_market::pallet::SettledDealData",
derive = "::serde::Serialize"
Expand Down

0 comments on commit b23660f

Please sign in to comment.