Skip to content

Commit 0c79a1d

Browse files
remagpiesgkim126
authored andcommitted
Fix tendermint to use CurrentValidators
CurrentValidators was introduced to tendermint, but it wasn't used by some part of our implementation. This patch doesn't fix all of them, but important parts are filled in.
1 parent 9c95c78 commit 0c79a1d

File tree

6 files changed

+106
-15
lines changed

6 files changed

+106
-15
lines changed

core/src/consensus/stake/action_data.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -433,6 +433,10 @@ impl CurrentValidators {
433433
pub fn update(&mut self, validators: Vec<Validator>) {
434434
self.0 = validators;
435435
}
436+
437+
pub fn addresses(&self) -> Vec<Address> {
438+
self.0.iter().rev().map(|v| public_to_address(&v.pubkey)).collect()
439+
}
436440
}
437441

438442
impl Deref for CurrentValidators {
@@ -482,6 +486,12 @@ impl Deref for PreviousValidators {
482486
}
483487
}
484488

489+
impl From<PreviousValidators> for Vec<Validator> {
490+
fn from(val: PreviousValidators) -> Self {
491+
val.0
492+
}
493+
}
494+
485495
#[derive(Default, Debug, PartialEq)]
486496
pub struct IntermediateRewards {
487497
current: BTreeMap<Address, u64>,

core/src/consensus/tendermint/engine.rs

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -400,20 +400,21 @@ fn aggregate_work_info(
400400
end_of_the_last_term + 1
401401
};
402402
let mut header = start_of_the_next_term_header;
403-
let mut parent_validators = validators.current_addresses(&header.parent_hash());
404403
while start_of_the_current_term != header.number() {
404+
let parent = chain.block_header(&header.parent_hash().into()).unwrap();
405+
let current_validators = validators.current_addresses(&parent.hash());
406+
let previous_validators = validators.previous_addresses(&parent.hash());
405407
for index in TendermintSealView::new(&header.seal()).bitset()?.true_index_iter() {
406-
let signer = *parent_validators.get(index).expect("The seal must be the signature of the validator");
408+
let signer = *current_validators.get(index).expect("The seal must be the signature of the validator");
407409
work_info.entry(signer).or_default().signed += 1;
408410
}
409411

410-
header = chain.block_header(&header.parent_hash().into()).unwrap();
411-
parent_validators = validators.current_addresses(&header.parent_hash());
412-
413-
let author = header.author();
412+
let author = parent.author();
414413
let info = work_info.entry(author).or_default();
415414
info.proposed += 1;
416-
info.missed += parent_validators.len() - TendermintSealView::new(&header.seal()).bitset()?.count();
415+
info.missed += previous_validators.len() - TendermintSealView::new(&header.seal()).bitset()?.count();
416+
417+
header = parent;
417418
}
418419

419420
Ok(work_info)

core/src/consensus/tendermint/worker.rs

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1248,13 +1248,14 @@ impl Worker {
12481248
};
12491249

12501250
let mut voted_validators = BitSet::new();
1251-
let grand_parent_hash = self
1252-
.client()
1253-
.block_header(&(*header.parent_hash()).into())
1254-
.expect("The parent block must exist")
1255-
.parent_hash();
1251+
let parent_hash = header.parent_hash();
1252+
let grand_parent_hash =
1253+
self.client().block_header(&(*parent_hash).into()).expect("The parent block must exist").parent_hash();
12561254
for (bitset_index, signature) in seal_view.signatures()? {
1257-
let public = self.validators.get(&grand_parent_hash, bitset_index);
1255+
let public = match self.validators.get_current(header.parent_hash(), bitset_index) {
1256+
Some(p) => p,
1257+
None => self.validators.get(&grand_parent_hash, bitset_index),
1258+
};
12581259
if !verify_schnorr(&public, &signature, &precommit_vote_on.hash())? {
12591260
let address = public_to_address(&public);
12601261
return Err(EngineError::BlockNotAuthorized(address.to_owned()).into())
@@ -1267,7 +1268,7 @@ impl Worker {
12671268
if header.number() == 1 {
12681269
return Ok(())
12691270
}
1270-
self.validators.check_enough_votes(&grand_parent_hash, &voted_validators)?;
1271+
self.validators.check_enough_votes_with_current(&parent_hash, &voted_validators)?;
12711272
Ok(())
12721273
}
12731274

core/src/consensus/validator_set/dynamic_validator.rs

Lines changed: 74 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ use parking_lot::RwLock;
2424
use super::{RoundRobinValidator, ValidatorSet};
2525
use crate::client::ConsensusClient;
2626
use crate::consensus::bit_set::BitSet;
27-
use crate::consensus::stake::{CurrentValidators, NextValidators, Validator};
27+
use crate::consensus::stake::{CurrentValidators, NextValidators, PreviousValidators, Validator};
2828
use crate::consensus::EngineError;
2929

3030
/// Validator set containing a known set of public keys.
@@ -87,6 +87,29 @@ impl DynamicValidator {
8787
}
8888
}
8989

90+
fn previous_validators(&self, hash: BlockHash) -> Option<Vec<Validator>> {
91+
let client: Arc<dyn ConsensusClient> =
92+
self.client.read().as_ref().and_then(Weak::upgrade).expect("Client is not initialized");
93+
let block_id = hash.into();
94+
let term_id = client.current_term_id(block_id).expect(
95+
"valdators() is called when creating a block or verifying a block.
96+
Minor creates a block only when the parent block is imported.
97+
The n'th block is verified only when the parent block is imported.",
98+
);
99+
if term_id == 0 {
100+
return None
101+
}
102+
let state = client.state_at(block_id)?;
103+
let validators = PreviousValidators::load_from_state(&state).unwrap();
104+
if validators.is_empty() {
105+
None
106+
} else {
107+
let mut validators: Vec<_> = validators.into();
108+
validators.reverse();
109+
Some(validators)
110+
}
111+
}
112+
90113
fn validators_pubkey(&self, hash: BlockHash) -> Option<Vec<Public>> {
91114
self.next_validators(hash).map(|validators| validators.into_iter().map(|val| *val.pubkey()).collect())
92115
}
@@ -95,6 +118,10 @@ impl DynamicValidator {
95118
self.current_validators(hash).map(|validators| validators.into_iter().map(|val| *val.pubkey()).collect())
96119
}
97120

121+
fn previous_validators_pubkey(&self, hash: BlockHash) -> Option<Vec<Public>> {
122+
self.previous_validators(hash).map(|validators| validators.into_iter().map(|val| *val.pubkey()).collect())
123+
}
124+
98125
pub fn proposer_index(&self, parent: BlockHash, prev_proposer_index: usize, proposed_view: usize) -> usize {
99126
if let Some(validators) = self.next_validators(parent) {
100127
let num_validators = validators.len();
@@ -104,6 +131,44 @@ impl DynamicValidator {
104131
(prev_proposer_index + proposed_view + 1) % num_validators
105132
}
106133
}
134+
135+
pub fn get_current(&self, hash: &BlockHash, index: usize) -> Option<Public> {
136+
let validators = self.current_validators_pubkey(*hash)?;
137+
let n_validators = validators.len();
138+
Some(*validators.get(index % n_validators).unwrap())
139+
}
140+
141+
pub fn check_enough_votes_with_current(&self, hash: &BlockHash, votes: &BitSet) -> Result<(), EngineError> {
142+
if let Some(validators) = self.current_validators(*hash) {
143+
let mut voted_delegation = 0u64;
144+
let n_validators = validators.len();
145+
for index in votes.true_index_iter() {
146+
assert!(index < n_validators);
147+
let validator = validators.get(index).ok_or_else(|| {
148+
EngineError::ValidatorNotExist {
149+
height: 0, // FIXME
150+
index,
151+
}
152+
})?;
153+
voted_delegation += validator.delegation();
154+
}
155+
let total_delegation: u64 = validators.iter().map(Validator::delegation).sum();
156+
if voted_delegation * 3 > total_delegation * 2 {
157+
Ok(())
158+
} else {
159+
let threshold = total_delegation as usize * 2 / 3;
160+
Err(EngineError::BadSealFieldSize(OutOfBounds {
161+
min: Some(threshold),
162+
max: Some(total_delegation as usize),
163+
found: voted_delegation as usize,
164+
}))
165+
}
166+
} else {
167+
let client = self.client.read().as_ref().and_then(Weak::upgrade).expect("Client is not initialized");
168+
let header = client.block_header(&(*hash).into()).unwrap();
169+
self.check_enough_votes(&header.parent_hash(), votes)
170+
}
171+
}
107172
}
108173

109174
impl ValidatorSet for DynamicValidator {
@@ -208,6 +273,14 @@ impl ValidatorSet for DynamicValidator {
208273
*client_lock = Some(client);
209274
}
210275

276+
fn previous_addresses(&self, hash: &BlockHash) -> Vec<Address> {
277+
if let Some(validators) = self.previous_validators_pubkey(*hash) {
278+
validators.iter().map(public_to_address).collect()
279+
} else {
280+
self.initial_list.previous_addresses(hash)
281+
}
282+
}
283+
211284
fn current_addresses(&self, hash: &BlockHash) -> Vec<Address> {
212285
if let Some(validators) = self.current_validators_pubkey(*hash) {
213286
validators.iter().map(public_to_address).collect()

core/src/consensus/validator_set/mod.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,8 @@ pub trait ValidatorSet: Send + Sync {
5757
/// Allows blockchain state access.
5858
fn register_client(&self, _client: Weak<dyn ConsensusClient>) {}
5959

60+
fn previous_addresses(&self, _hash: &BlockHash) -> Vec<Address>;
61+
6062
fn current_addresses(&self, _hash: &BlockHash) -> Vec<Address>;
6163

6264
fn next_addresses(&self, _hash: &BlockHash) -> Vec<Address>;

core/src/consensus/validator_set/validator_list.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,10 @@ impl ValidatorSet for RoundRobinValidator {
105105
*self.client.write() = Some(client);
106106
}
107107

108+
fn previous_addresses(&self, _hash: &BlockHash) -> Vec<Address> {
109+
self.validators.iter().map(public_to_address).collect()
110+
}
111+
108112
fn current_addresses(&self, _hash: &BlockHash) -> Vec<Address> {
109113
self.validators.iter().map(public_to_address).collect()
110114
}

0 commit comments

Comments
 (0)