Skip to content
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

Remove unused function in prevote event #352

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions core/src/reserved.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ impl ReservedState {
let validator_set = self
.members
.iter()
.filter(|member| member.expelled == false)
.map(|member| {
let name = member
.consensus_delegatee
Expand Down Expand Up @@ -55,6 +56,7 @@ impl ReservedState {
let governance_set = self
.members
.iter()
.filter(|member| member.expelled == false)
.map(|member| {
let name = member
.governance_delegatee
Expand Down Expand Up @@ -92,7 +94,7 @@ impl ReservedState {
return Err("delegation proof verification failed".to_string());
}
for delegator in &mut self.members {
if delegator.name == tx.data.delegator {
if delegator.name == tx.data.delegator && delegator.expelled == false {
if tx.data.governance {
delegator.governance_delegatee = Some(tx.data.delegatee.clone());
delegator.consensus_delegatee = Some(tx.data.delegatee.clone());
Expand All @@ -110,7 +112,7 @@ impl ReservedState {
return Err("delegation proof verification failed".to_string());
}
for delegator in &mut self.members {
if delegator.name == tx.data.delegator {
if delegator.name == tx.data.delegator && delegator.expelled == false {
if delegator.consensus_delegatee.is_some() {
delegator.consensus_delegatee = None;
delegator.governance_delegatee = None;
Expand Down Expand Up @@ -157,6 +159,7 @@ mod tests {
consensus_voting_power: 1,
governance_delegatee: None,
consensus_delegatee: None,
expelled: false,
}
}

Expand All @@ -172,6 +175,7 @@ mod tests {
consensus_voting_power: 1,
governance_delegatee: None,
consensus_delegatee: Some(format!("member-{delegatee_member_num:04}")),
expelled: false,
}
}

Expand All @@ -187,6 +191,7 @@ mod tests {
consensus_voting_power: 1,
governance_delegatee: Some(format!("member-{delegatee_member_num:04}")),
consensus_delegatee: None,
expelled: false,
}
}

Expand Down
2 changes: 2 additions & 0 deletions core/src/test_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ pub fn generate_standard_genesis(
consensus_voting_power: 1,
governance_delegatee: None,
consensus_delegatee: None,
expelled: false,
})
.collect::<Vec<_>>();
let genesis_header = BlockHeader {
Expand Down Expand Up @@ -100,6 +101,7 @@ pub fn generate_delegated_genesis(
} else {
None
},
expelled: false,
})
.collect::<Vec<_>>();
let genesis_header = BlockHeader {
Expand Down
4 changes: 4 additions & 0 deletions core/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,10 @@ pub struct Member {
// - Unlock-Automatically-After-T-Seconds
// - Unlock-If-The-Delegatee-Is-Not-Active
// - Unlock-If-The-Validator-Set-Changes
/// If true, all voting powers are ignored.
/// Note that once granted, Simperby keeps all members forever in the reserved state.
/// If you want to remove a member, you must set this to true instead of removing the member.
pub expelled: bool,
}

#[derive(Debug, Serialize, Deserialize, PartialEq, Eq, Clone)]
Expand Down
105 changes: 70 additions & 35 deletions core/src/verify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,9 +87,23 @@ pub fn verify_finalization_proof(
Ok(())
}

/// Verifies the version of input reserved state is valid.
pub fn verify_version_syntax(s: &str) -> bool {
let mut segments = s.split('.');

let is_valid_segment = |segment: &str| {
segment.parse::<u32>().map_or(false, |number| number < 10)
};

segments.next().map_or(false, is_valid_segment)
&& segments.next().map_or(false, is_valid_segment)
&& segments.next().map_or(false, is_valid_segment)
&& segments.next().is_none()
}

// Phases of the `CommitSequenceVerifier`.
//
// Note that `Phase::X` is agenda phase where `Commit::X` is the last commit.
// Note that `Phase::X` is the phase where `Commit::X` is the last commit.
#[derive(Debug, Clone, PartialEq, Eq)]
enum Phase {
// The transaction phase.
Expand Down Expand Up @@ -131,6 +145,9 @@ pub struct CommitSequenceVerifier {
impl CommitSequenceVerifier {
/// Creates a new `CommitSequenceVerifier` with the given block header.
pub fn new(start_header: BlockHeader, reserved_state: ReservedState) -> Result<Self, Error> {
// if verify_reserved_state(&self, reserved_state)? {
// return Err(Error::InvalidArgument(format!("Reserved state is not valid")));
// }
Ok(Self {
header: start_header.clone(),
phase: Phase::Block,
Expand Down Expand Up @@ -173,21 +190,43 @@ impl CommitSequenceVerifier {

/// Verifies whether the given reserved state is valid from the current state.
pub fn verify_reserved_state(&self, _rs: &ReservedState) -> Result<(), Error> {
// TODO:
// 1. Check that the number of members is at least 4.
// 2. Check that the version advances correctly.
if _rs.members.len() < 4 {
return Err(Error::InvalidArgument(format!("Number of members is not over 4")));
}
if self.reserved_state.version != _rs.version {
if !(self.reserved_state.version < _rs.version && verify_version_syntax(&_rs.version)) {
return Err(Error::InvalidArgument(format!("Version advances is incorrect")));
}
}
// 3. Check that `consensus_leader_order` is correct.
// 4. Check that `genesis_info` stays the same.
if self.reserved_state.consensus_leader_order != _rs.consensus_leader_order {
return Err(Error::InvalidArgument(format!("Consensus leader order is incorrect")));
}
if self.reserved_state.genesis_info != _rs.genesis_info {
return Err(Error::InvalidArgument(format!("Genesis_info is not stays the same")));
}
// 5. Check that the newly added (if exists) `Member::name` is unique.
// 6. Check that `member` monotonicaly increases (refer to `Member::expelled`).
// 7. Check that the delegation state doesn't change.
if !self.reserved_state.members.iter().all(|m1| _rs.members.iter().any(|m2| m1.public_key == m2.public_key)) {
return Err(Error::InvalidArgument(format!("New member set do not have all previous member")));
}
let public_key_set: HashSet<&PublicKey> = _rs.members.iter().map(|m| &m.public_key).collect();
if public_key_set.len() != _rs.members.len() {
return Err(Error::InvalidArgument(format!("Newly added member public keys are not unique")));
}
// 6. Check that `member` monotonic increases (refer to `Member::expelled`).
if _rs.members.iter().any(|member| member.expelled) {
return Err(Error::InvalidArgument(format!("Member expulsion time not monotonic increasing")));
}
Ok(())
}

/// Verifies the given commit and updates the internal reserved_state of CommitSequenceVerifier.
pub fn apply_commit(&mut self, commit: &Commit) -> Result<(), Error> {
match (commit, &mut self.phase) {
(Commit::Block(block_header), Phase::AgendaProof { agenda_proof: _ }) => {
if self.reserved_state.version != block_header.version {
return Err(Error::InvalidArgument(format!("Version of header is not match reserved_state")));
}
verify_header_to_header(&self.header, block_header)?;
// Verify commit merkle root
let commit_merkle_root =
Expand All @@ -202,12 +241,10 @@ impl CommitSequenceVerifier {
self.phase = Phase::Block;
self.commits_for_next_block = vec![];
}
(
Commit::Block(block_header),
Phase::ExtraAgendaTransaction {
last_extra_agenda_timestamp,
},
) => {
(Commit::Block(block_header), Phase::ExtraAgendaTransaction {last_extra_agenda_timestamp,}) => {
if self.reserved_state.version != block_header.version {
return Err(Error::InvalidArgument(format!("Version of header is not match reserved_state")));
}
verify_header_to_header(&self.header, block_header)?;
// Check if the block contains all the extra-agenda transactions.
if block_header.timestamp < *last_extra_agenda_timestamp {
Expand All @@ -232,20 +269,15 @@ impl CommitSequenceVerifier {
(Commit::Transaction(tx), Phase::Block) => {
// Update reserved_state for reserved-diff transactions.
if let Diff::Reserved(rs) = &tx.diff {
self.verify_reserved_state(rs)?;
self.reserved_state = *rs.clone();
}
self.phase = Phase::Transaction {
last_transaction: tx.clone(),
preceding_transactions: vec![],
};
}
(
Commit::Transaction(tx),
Phase::Transaction {
last_transaction,
preceding_transactions,
},
) => {
(Commit::Transaction(tx), Phase::Transaction {last_transaction, preceding_transactions}) => {
// Check if transactions are in chronological order
if tx.timestamp < last_transaction.timestamp {
return Err(Error::InvalidArgument(format!(
Expand All @@ -255,6 +287,7 @@ impl CommitSequenceVerifier {
}
// Update reserved_state for reserved-diff transactions.
if let Diff::Reserved(rs) = &tx.diff {
self.verify_reserved_state(rs)?;
self.reserved_state = *rs.clone();
}
preceding_transactions.push(last_transaction.clone());
Expand All @@ -281,13 +314,7 @@ impl CommitSequenceVerifier {
agenda: agenda.clone(),
};
}
(
Commit::Agenda(agenda),
Phase::Transaction {
last_transaction,
preceding_transactions,
},
) => {
(Commit::Agenda(agenda), Phase::Transaction {last_transaction, preceding_transactions}) => {
// Check if agenda is associated with the current block sequence.
if agenda.height != self.header.height + 1 {
return Err(Error::InvalidArgument(format!(
Expand Down Expand Up @@ -398,12 +425,7 @@ impl CommitSequenceVerifier {
ExtraAgendaTransaction::Report(_tx) => unimplemented!(),
}
}
(
Commit::ExtraAgendaTransaction(tx),
Phase::ExtraAgendaTransaction {
last_extra_agenda_timestamp,
},
) => {
(Commit::ExtraAgendaTransaction(tx), Phase::ExtraAgendaTransaction {last_extra_agenda_timestamp}) => {
match tx {
ExtraAgendaTransaction::Delegate(tx) => {
// Update reserved reserved_state by applying delegation
Expand Down Expand Up @@ -434,8 +456,7 @@ impl CommitSequenceVerifier {
ExtraAgendaTransaction::Report(_tx) => unimplemented!(),
}
}
(Commit::ChatLog(_chat_log), _) => unimplemented!(),
(commit, phase) => {
(Commit::ChatLog(_chat_log), _) => unimplemented!(), (commit, phase) => {
return Err(Error::PhaseMismatch(
format!("{commit:?}"),
format!("{phase:?}"),
Expand Down Expand Up @@ -498,6 +519,7 @@ mod test {
consensus_voting_power: *voting_power,
governance_delegatee: None,
consensus_delegatee: None,
expelled: false,
});
}
members
Expand Down Expand Up @@ -582,6 +604,7 @@ mod test {
consensus_voting_power: 1,
governance_delegatee: None,
consensus_delegatee: None,
expelled: false,
});
reserved_state
.consensus_leader_order
Expand Down Expand Up @@ -1567,4 +1590,16 @@ mod test {

// TODO: add test cases where the `Report` extra-agenda transactions are invalid.
// These test cases are TODO because the `Report` extra-agenda transaction is not implemented yet.

//#[test]
// Test the case where check verify_reserved_state function run well
// fn check_verify_reserved_state_run() {
// let (validator_keypair, reserved_state, mut csv) = setup_test(3);
// csv.verify_reserved_state(&reserved_state).unwrap_err();


// //make wrong reserved_state
// //chech vrs function

// }
}
3 changes: 0 additions & 3 deletions vetomint/src/progress.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,9 +76,6 @@ pub(crate) fn progress(
});
let mut response = Vec::new();
if let Some(proposal) = proposal {
response.extend(on_4f_non_nil_prevote_in_propose_step(
state, round, proposal,
));
response.extend(on_4f_non_nil_prevote_in_prevote_step(
state, round, proposal,
));
Expand Down
Loading