-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
[Coinbase Go/No-Go] Bond balances in genesis block #2308
Changes from 6 commits
74d29bf
9bfa3cc
bf03d25
d84e304
befcf83
8d1563f
dc96d3d
133c822
ed0abd0
b72f6d6
e05c5af
15999db
e12600c
285435b
3fca065
04fff0d
33c4af3
50c0673
9e83c91
7540ac7
c2ef7fb
d7e719b
19f93ce
6e93760
30baf60
caf3e38
39596bc
9bbf4fb
45113b9
74cf60c
1cb2e52
1f07554
c0e90e6
1b6163c
63ff34b
a4e6c0e
a01e333
48d4fab
cf32b3c
9f2ca27
02fcf3c
425db4d
3f42b53
6d4773a
6cf37c6
532b37b
073c949
d4aea86
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 |
---|---|---|
|
@@ -182,12 +182,12 @@ impl<N: Network, C: ConsensusStorage<N>> VM<N, C> { | |
|
||
// Initialize an iterator for ratifications before finalize. | ||
let pre_ratifications = ratifications.iter().filter(|r| match r { | ||
Ratify::Genesis(_, _) => true, | ||
Ratify::Genesis(_, _, _) => true, | ||
Ratify::BlockReward(..) | Ratify::PuzzleReward(..) => false, | ||
}); | ||
// Initialize an iterator for ratifications after finalize. | ||
let post_ratifications = ratifications.iter().filter(|r| match r { | ||
Ratify::Genesis(_, _) => false, | ||
Ratify::Genesis(_, _, _) => false, | ||
Ratify::BlockReward(..) | Ratify::PuzzleReward(..) => true, | ||
}); | ||
|
||
|
@@ -464,12 +464,12 @@ impl<N: Network, C: ConsensusStorage<N>> VM<N, C> { | |
atomic_finalize!(self.finalize_store(), FinalizeMode::RealRun, { | ||
// Initialize an iterator for ratifications before finalize. | ||
let pre_ratifications = ratifications.iter().filter(|r| match r { | ||
Ratify::Genesis(_, _) => true, | ||
Ratify::Genesis(_, _, _) => true, | ||
Ratify::BlockReward(..) | Ratify::PuzzleReward(..) => false, | ||
}); | ||
// Initialize an iterator for ratifications after finalize. | ||
let post_ratifications = ratifications.iter().filter(|r| match r { | ||
Ratify::Genesis(_, _) => false, | ||
Ratify::Genesis(_, _, _) => false, | ||
Ratify::BlockReward(..) | Ratify::PuzzleReward(..) => true, | ||
}); | ||
|
||
|
@@ -707,7 +707,7 @@ impl<N: Network, C: ConsensusStorage<N>> VM<N, C> { | |
// Iterate over the ratifications. | ||
for ratify in pre_ratifications { | ||
match ratify { | ||
Ratify::Genesis(committee, public_balances) => { | ||
Ratify::Genesis(committee, public_balances, bonded_balances) => { | ||
// Ensure this is the genesis block. | ||
ensure!(state.block_height() == 0, "Ratify::Genesis(..) expected a genesis block"); | ||
// Ensure the genesis committee round is 0. | ||
|
@@ -730,17 +730,60 @@ impl<N: Network, C: ConsensusStorage<N>> VM<N, C> { | |
// } | ||
// } | ||
|
||
// Initialize the stakers. | ||
let mut stakers = IndexMap::with_capacity(committee.members().len()); | ||
// Iterate over the committee members. | ||
for (validator, (microcredits, _)) in committee.members() { | ||
// Insert the validator into the stakers. | ||
stakers.insert(*validator, (*validator, *microcredits)); | ||
// Calculate the stake per validator using `bonded_balances`. | ||
let mut stake_per_validator = IndexMap::with_capacity(committee.members().len()); | ||
howardwu marked this conversation as resolved.
Show resolved
Hide resolved
|
||
for (address, (validator_address, amount)) in bonded_balances { | ||
// Check that the amount meets the minimum requirement, depending on whether the address is a validator. | ||
if *address == *validator_address { | ||
ensure!( | ||
*amount >= 1_000_000_000_000_u64, | ||
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. Is there a way to ensure that these values match the |
||
"Ratify::Genesis(..) the validator {address} must stake at least 1_000_000_000_000", | ||
); | ||
} else { | ||
ensure!( | ||
*amount >= 10_000_000_u64, | ||
"Ratify::Genesis(..) the delegator {address} must stake at least 10_000_000", | ||
); | ||
// If the address is a delegator, check that the corresponding validator is open. | ||
ensure!( | ||
committee.is_committee_member_open(*validator_address), | ||
"Ratify::Genesis(..) the delegator {address} is delegating to a closed validator {validator_address}", | ||
); | ||
} | ||
// Ensure that each staker has an entry in `public_balances`. | ||
ensure!( | ||
public_balances.contains_key(address), | ||
"Ratify::Genesis(..) the staker {address} is missing from the public balances", | ||
); | ||
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. Why is this check necessary? Unless you decrement from 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 check was introduced to be consistent with |
||
// Accumulate the staked amount per validator. | ||
let total = stake_per_validator.entry(validator_address).or_insert(0u64); | ||
*total = total.saturating_add(*amount); | ||
raychu86 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
// Ensure the stake per validator matches the committee. | ||
ensure!( | ||
stake_per_validator.len() == committee.members().len(), | ||
"Ratify::Genesis(..) the number of validators in the committee does not match the number of validators in the bonded balances", | ||
); | ||
|
||
// Check that `committee` is consistent with `stake_per_validator`. | ||
for (validator_address, amount) in stake_per_validator { | ||
// Retrieve the expected validator stake from the committee. | ||
let expected_amount = match committee.members().get(validator_address) { | ||
Some((amount, _)) => *amount, | ||
None => bail!( | ||
"Ratify::Genesis(..) found a validator in the bonded balances that is not in the committee" | ||
), | ||
}; | ||
howardwu marked this conversation as resolved.
Show resolved
Hide resolved
|
||
// Ensure the staked amount matches the committee. | ||
ensure!( | ||
expected_amount == amount, | ||
"Ratify::Genesis(..) inconsistent staked amount for validator {validator_address}", | ||
); | ||
} | ||
|
||
// Construct the next committee map and next bonded map. | ||
let (next_committee_map, next_bonded_map) = | ||
to_next_commitee_map_and_bonded_map(committee, &stakers); | ||
to_next_commitee_map_and_bonded_map(committee, bonded_balances); | ||
raychu86 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
// Insert the next committee into storage. | ||
store.committee_store().insert(state.block_height(), committee.clone())?; | ||
|
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.
I wonder if this has a non-trivial impact on object/storage size?
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.
It might have some impact, but the jump from
(Committee<N>, PublicBalances<N>)
to(Committee<N>, PublicBalances<N>, BondedBalances<N>)
won't be that big.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.
Considering every block contains this enum, and we will always pay the worst-case memory footprint, I don't understand why one chose to ignore this clippy warning.
You should follow the advise of
clippy
to resolve it (I assume its to useBox
). Otherwise, every enum variant will always pay the worst case bound.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.
@d0cd since you've been finishing off the PR, want to wrap the contents in Box? I also just recently noticed we're already using this pattern in snarkVM.
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.
Done! By boxing the elements of
Genesis::Ratify
the size reduces from 240 to 32 bytes.