-
Notifications
You must be signed in to change notification settings - Fork 1
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: add support for --include-partial-seats #12
Changes from 1 commit
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 |
---|---|---|
|
@@ -3,7 +3,7 @@ use num_rational::Ratio; | |
use serde::Serialize; | ||
use std::path::PathBuf; | ||
|
||
use crate::seat::Seat; | ||
use crate::{partial_seat::PartialSeat, seat::Seat}; | ||
|
||
#[derive(Args, Serialize, Debug)] | ||
pub struct Config { | ||
|
@@ -30,18 +30,24 @@ pub struct Config { | |
/// data will be used in the simulation. | ||
#[arg(long)] | ||
pub validator_data: Option<PathBuf>, | ||
/// A validator's stake might not entirely cover seats given a particular `stake_per_seat`. This | ||
/// option controls whether remaining stake (not covering a full seat) should be assigned to a | ||
/// partial seat or ignored. | ||
#[arg(long, default_value_t = false)] | ||
pub include_partial_seats: bool, | ||
} | ||
|
||
impl Config { | ||
#[cfg(test)] | ||
pub fn new_mock() -> Self { | ||
pub fn new_mock(include_partial_seats: bool) -> Self { | ||
Self { | ||
num_blocks: 1_000, | ||
num_shards: 4, | ||
seats_per_shard: 2, | ||
stake_per_seat: 100, | ||
max_malicious_stake_per_shard: Ratio::new(1, 3), | ||
validator_data: None, | ||
include_partial_seats, | ||
} | ||
} | ||
|
||
|
@@ -93,31 +99,70 @@ impl Config { | |
|
||
Ok(shard_seats) | ||
} | ||
|
||
/// Collect partials seats for `shard_idx` by picking seats from positions with `position % | ||
/// num_shards == shard_idx`. | ||
/// | ||
/// # Motivation for assignment via modulo | ||
/// | ||
/// Every validator may hold at most 1 partial seat. If a validator's stake covers full seats | ||
/// without remainder, it holds 0 partial seats. Hence `0 <= num_partial_seats <= | ||
/// num_validators`. Assigning seats to shards with `%` allows distributing the existing number | ||
/// of partial seats to shards as evenly as possible. | ||
Comment on lines
+108
to
+111
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 choice is probably ok for the simulation (I don't think it will impact the results), but for production I think we need to be more careful about this assignment. While using the modulus is even, it is also biased to smaller shard ids (for example suppose there are 4 shards and 9 partial seats, then shard 0 gets 3 partial seats while all the others get 2). This means it will be systematically easier to corrupt higher shard ids than earlier ones. To even this out we probably need to randomly shuffle the shard order as well as the partial seats. By "shuffle the shard order" I mean we can still use this scheme to assign an index to each partial seat, but then that index may or may not equal the shard index, depending on how we have decided to permute the shards (for example [0, 1, 2, 3] might be shuffled to [1, 3, 2, 0]). Another idea could be to correlate partial and full seat assignment to try to even out stake as much as possible. For example, suppose there are 4 shards, 13 seats and 9 partial seats. With the current scheme shard 0 is getting the extra full seat and the extra partial seat. If there is a partial seat that is "close" to being a full seat it should be separated from the other partial seats and not in the same shard as the extra full seat because that would result in the most even stake distribution. Making that correlation rigorous could be challenging and may impact the security properties of the algorithm. Perhaps it is worth thinking about more carefully. 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.
The result only counts the number of corrupted shards, so I also think it’s fine for the simulation.
Good point! Shuffling the shard order sounds good to me since it is a relatively cheap operation. Especially since the number of shards should remain low in the proximate future.
A concern could be its computational and logical overhead. I think a high number of seats is preferred as it loweres the probability of shard corrruption. A high number of seats implies low A middle ground could be the shuffling of the shard order for both full and partial seats. This should at least remove the bias towards smaller shard ids. 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. Yeah, the shard order independently for shuffling both full and partial seats sounds like a good idea. |
||
/// | ||
/// # No minimum number of required _partial_ seats per shard | ||
/// | ||
/// It is not needed since partial seats are included only to distribute leftover stake (not | ||
/// covering full seats) to shards. | ||
pub fn collect_partial_seats_for_shard<'seats>( | ||
&self, | ||
shard_idx: usize, | ||
partial_seats: &'seats [PartialSeat], | ||
) -> anyhow::Result<Vec<&PartialSeat<'seats>>> { | ||
if shard_idx >= usize::from(self.num_shards) { | ||
anyhow::bail!( | ||
"shard_idx {} is an invalid index for {} shards", | ||
shard_idx, | ||
self.num_shards | ||
) | ||
} | ||
|
||
let mut shard_partial_seats = vec![]; | ||
for idx in (shard_idx..partial_seats.len()).step_by(self.num_shards.into()) { | ||
shard_partial_seats.push(&partial_seats[idx]); | ||
} | ||
|
||
Ok(shard_partial_seats) | ||
} | ||
} | ||
|
||
#[cfg(test)] | ||
mod tests { | ||
use std::collections::BTreeMap; | ||
|
||
use super::Config; | ||
use crate::validator::tests::new_test_raw_validator_data; | ||
use crate::validator::{new_ordered_seats, parse_raw_validator_data}; | ||
use crate::validator::{ | ||
new_ordered_partial_seats, new_ordered_seats, parse_raw_validator_data, | ||
}; | ||
|
||
#[test] | ||
pub fn test_seats_per_stake() { | ||
let config = Config::new_mock(); | ||
fn test_seats_per_stake() { | ||
let config = Config::new_mock(false); | ||
assert_eq!(config.seats_per_stake(20), 0); | ||
assert_eq!(config.seats_per_stake(100), 1); | ||
assert_eq!(config.seats_per_stake(530), 5); | ||
} | ||
|
||
#[test] | ||
pub fn test_total_seats() { | ||
let config = Config::new_mock(); | ||
fn test_total_seats() { | ||
let config = Config::new_mock(false); | ||
assert_eq!(config.total_seats(), 8); | ||
} | ||
|
||
#[test] | ||
pub fn test_collect_seats_for_shard() { | ||
let config = Config::new_mock(); | ||
fn test_collect_seats_for_shard() { | ||
let config = Config::new_mock(false); | ||
let (_, validators) = parse_raw_validator_data(&config, &new_test_raw_validator_data()); | ||
// Using ordered seats as input to have a deterministic result of `collect_seats_for_shard`. | ||
let seats = new_ordered_seats(&validators); | ||
|
@@ -137,12 +182,52 @@ mod tests { | |
} | ||
|
||
#[test] | ||
pub fn test_collect_seats_for_shard_errors() { | ||
let config = Config::new_mock(); | ||
fn test_collect_seats_for_shard_errors() { | ||
let config = Config::new_mock(false); | ||
let (_, validators) = parse_raw_validator_data(&config, &new_test_raw_validator_data()); | ||
let seats = new_ordered_seats(&validators); | ||
|
||
insta::assert_debug_snapshot!(config.collect_seats_for_shard(4, &seats)); | ||
insta::assert_debug_snapshot!(config.collect_seats_for_shard(0, &[])); | ||
} | ||
|
||
#[test] | ||
fn test_collect_partial_seats_for_shard() { | ||
let mut config = Config::new_mock(true); | ||
config.stake_per_seat = 90; | ||
let (_, validators) = parse_raw_validator_data(&config, &new_test_raw_validator_data()); | ||
// Using ordered partial seats as input to have a deterministic result of | ||
// `collect_partial_seats_for_shard`. | ||
let partial_seats = new_ordered_partial_seats(&validators, config.stake_per_seat); | ||
|
||
// Using `BTreeMap` for deterministic ordering of keys. | ||
let mut assignments = BTreeMap::new(); | ||
for shard_idx in 0..config.num_shards { | ||
let assignment = config | ||
.collect_partial_seats_for_shard(shard_idx.into(), &partial_seats) | ||
.unwrap(); | ||
assignments.insert(format!("shard_{shard_idx}"), assignment); | ||
} | ||
|
||
insta::with_settings!({ | ||
info => &( | ||
&config, | ||
"partial_seats:", | ||
&partial_seats | ||
) | ||
}, { | ||
insta::assert_yaml_snapshot!(assignments); | ||
}) | ||
} | ||
|
||
#[test] | ||
fn test_collect_partial_seats_for_shard_errors() { | ||
let config = Config::new_mock(true); | ||
let (_, validators) = parse_raw_validator_data(&config, &new_test_raw_validator_data()); | ||
let partial_seats = new_ordered_partial_seats(&validators, config.stake_per_seat); | ||
|
||
insta::assert_debug_snapshot!( | ||
config.collect_partial_seats_for_shard(config.num_shards.into(), &partial_seats) | ||
); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,49 @@ | ||
use serde::Serialize; | ||
|
||
use crate::validator::Validator; | ||
|
||
/// Represents a partial seat filled by a particular validator. | ||
/// | ||
/// A partial seat may not outlive the validator it is referrencing. | ||
#[derive(Serialize, PartialEq, Debug, Clone)] | ||
pub struct PartialSeat<'validator> { | ||
/// Reference to the validator holding this particular partial seat. | ||
validator: &'validator Validator, | ||
/// The stake attributed to the partial seat. Note that `0 < weight < stake_per_seat`. | ||
/// | ||
/// For example, let validator `V` have a stake of 12 and let `stake_per_seat = 5`. Then `V` | ||
/// holds 2 full seats and a partial seat with `weight = 2`. | ||
weight: u128, | ||
} | ||
|
||
impl<'validator> PartialSeat<'validator> { | ||
/// Constructs the partial seat filled by `validator`. | ||
pub fn new(validator: &'validator Validator, weight: u128) -> Self { | ||
Self { validator, weight } | ||
} | ||
|
||
pub fn get_is_malicious(&self) -> bool { | ||
self.validator.get_is_malicious() | ||
} | ||
|
||
pub fn get_weight(&self) -> u128 { | ||
self.weight | ||
} | ||
} | ||
|
||
pub struct ShuffledPartialSeats<'seats> { | ||
partial_seats: &'seats [PartialSeat<'seats>], | ||
} | ||
|
||
impl<'seats> ShuffledPartialSeats<'seats> { | ||
/// Shuffled the input `partial_seats`. | ||
// TODO(rand) do all shuffling operations in one generic fn | ||
pub fn new(partial_seats: &'seats mut [PartialSeat<'seats>]) -> Self { | ||
fastrand::shuffle(partial_seats); | ||
Self { partial_seats } | ||
} | ||
|
||
pub fn get_partial_seats(&self) -> &[PartialSeat] { | ||
self.partial_seats | ||
} | ||
} |
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.
Opened #13 to change
collect_seats_for_shard()
to be consistent with this.