Skip to content
Merged
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
12 changes: 2 additions & 10 deletions core/src/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ use crate::client::{EngineInfo, TermInfo};
use crate::consensus::CodeChainEngine;
use crate::error::{BlockError, Error};
use crate::transaction::{SignedTransaction, UnverifiedTransaction};
use crate::BlockId;

/// A block, encoded as it is on the block chain.
#[derive(Debug, Clone, PartialEq)]
Expand Down Expand Up @@ -493,16 +494,7 @@ pub fn enact<C: ChainTimeInfo + EngineInfo + FindActionHandler + TermInfo>(
b.populate_from(header);
b.push_transactions(transactions, client, parent.number(), parent.timestamp())?;

let term_common_params = {
let block_number = client
.last_term_finished_block_num((*header.parent_hash()).into())
.expect("The block of the parent hash should exist");
if block_number == 0 {
None
} else {
Some(client.common_params((block_number).into()).expect("Common params should exist"))
}
};
let term_common_params = client.term_common_params(BlockId::Hash(*header.parent_hash()));
b.close_and_lock(parent, term_common_params.as_ref())
}

Expand Down
17 changes: 17 additions & 0 deletions core/src/client/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -826,6 +826,23 @@ impl TermInfo for Client {
.map(|state| state.metadata().unwrap().expect("Metadata always exist"))
.map(|metadata| metadata.current_term_id())
}

fn term_common_params(&self, id: BlockId) -> Option<CommonParams> {
let state = self.state_at(id)?;
let metadata = state.metadata().unwrap().expect("Metadata always exist");

if let Some(term_params) = metadata.term_params() {
Some(*term_params)
} else {
let block_number =
self.last_term_finished_block_num(id).expect("The block of the parent hash should exist");
if block_number == 0 {
None
} else {
Some(self.common_params((block_number).into()).expect("Common params should exist"))
}
}
}
}

impl AccountData for Client {
Expand Down
1 change: 1 addition & 0 deletions core/src/client/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ pub trait ConsensusClient: BlockChainClient + EngineClient + EngineInfo + TermIn
pub trait TermInfo {
fn last_term_finished_block_num(&self, id: BlockId) -> Option<BlockNumber>;
fn current_term_id(&self, id: BlockId) -> Option<u64>;
fn term_common_params(&self, id: BlockId) -> Option<CommonParams>;
}

/// Provides methods to access account info
Expand Down
4 changes: 4 additions & 0 deletions core/src/client/test_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -679,6 +679,10 @@ impl TermInfo for TestBlockChainClient {
fn current_term_id(&self, _id: BlockId) -> Option<u64> {
self.term_id
}

fn term_common_params(&self, _id: BlockId) -> Option<CommonParams> {
None
}
}

impl StateInfo for TestBlockChainClient {
Expand Down
10 changes: 9 additions & 1 deletion core/src/consensus/tendermint/engine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ use std::sync::{Arc, Weak};
use ckey::{public_to_address, Address};
use cnetwork::NetworkService;
use crossbeam_channel as crossbeam;
use cstate::{ActionHandler, TopStateView};
use cstate::{ActionHandler, TopState, TopStateView};
use ctypes::{BlockHash, CommonParams, Header};
use num_rational::Ratio;

Expand Down Expand Up @@ -235,6 +235,14 @@ impl ConsensusEngine for Tendermint {

stake::on_term_close(block.state_mut(), block_number, &inactive_validators)?;

match term {
0 => {}
_ => match term_common_params.expect("Term common params should exist").era() {
0 => {}
1 => block.state_mut().snapshot_term_params()?,
_ => unimplemented!("It is not decided how we handle this"),
},
}
Ok(())
}

Expand Down
11 changes: 1 addition & 10 deletions core/src/miner/miner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -603,16 +603,7 @@ impl Miner {
let parent_header = chain.block_header(&parent_hash.into()).expect("Parent header MUST exist");
(parent_header.decode(), parent_hash)
};
let term_common_params = {
let block_number = chain
.last_term_finished_block_num(parent_hash.into())
.expect("The block of the parent hash should exist");
if block_number == 0 {
None
} else {
Some(chain.common_params((block_number).into()).expect("Common params should exist"))
}
};
let term_common_params = chain.term_common_params(parent_hash.into());
let block = open_block.close(&parent_header, term_common_params.as_ref())?;

let fetch_seq = |p: &Public| {
Expand Down
6 changes: 6 additions & 0 deletions state/src/impls/top_level.rs
Original file line number Diff line number Diff line change
Expand Up @@ -999,6 +999,12 @@ impl TopState for TopLevelState {
metadata.increase_seq();
Ok(())
}

fn snapshot_term_params(&mut self) -> StateResult<()> {
let mut metadata = self.get_metadata_mut()?;
metadata.snapshot_term_params();
Ok(())
}
}

fn is_active_account(state: &dyn TopStateView, address: &Address) -> TrieResult<bool> {
Expand Down
136 changes: 94 additions & 42 deletions state/src/item/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ pub struct Metadata {
term: TermMetadata,
seq: u64,
params: Option<CommonParams>,
term_params: Option<CommonParams>,
}

impl Metadata {
Expand All @@ -45,6 +46,7 @@ impl Metadata {
term: Default::default(),
seq: 0,
params: None,
term_params: None,
}
}

Expand Down Expand Up @@ -93,6 +95,14 @@ impl Metadata {
self.params = Some(params);
}

pub fn term_params(&self) -> Option<&CommonParams> {
self.term_params.as_ref()
}

pub fn snapshot_term_params(&mut self) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there is a more proper name than snapshot_term_params.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know snapshot is a little bit confusing. It collides with the name of the snapshot sync feature and it is awkward to use it as a verb to me, but I can't think of a better one. Can you suggest a good one? How about freeze_?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you mind if we change the name later? @remagpie is dependant on this PR.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changing the name is not a big deal in my work. You can change it now if you want.

self.term_params = self.params;
}

pub fn increase_term_id(&mut self, last_term_finished_block_num: u64) {
assert!(self.term.last_term_finished_block_num < last_term_finished_block_num);
self.term.last_term_finished_block_num = last_term_finished_block_num;
Expand Down Expand Up @@ -124,25 +134,31 @@ impl CacheableItem for Metadata {

const PREFIX: u8 = super::METADATA_PREFIX;

const INITIAL_LEN: usize = 4;
const TERM_LEN: usize = INITIAL_LEN + 2;
const PARAMS_LEN: usize = TERM_LEN + 2;
const TERM_PARAMS_LEN: usize = PARAMS_LEN + 1;
const VALID_LEN: &[usize] = &[INITIAL_LEN, TERM_LEN, PARAMS_LEN, TERM_PARAMS_LEN];

impl Encodable for Metadata {
fn rlp_append(&self, s: &mut RlpStream) {
const INITIAL_LEN: usize = 4;
const TERM_LEN: usize = 2;
const PARAMS_LEN: usize = 2;
let mut len = INITIAL_LEN;

let term_changed = self.term != Default::default();
if term_changed {
len += TERM_LEN;
}

let params_changed = self.seq != 0;
if params_changed {
if !term_changed {
len += TERM_LEN;
let term_params_exist = self.term_params.is_some();

let len = if term_params_exist {
if !params_changed {
panic!("Term params only can be changed if params changed");
}
len += PARAMS_LEN;
}
TERM_PARAMS_LEN
} else if params_changed {
PARAMS_LEN
} else if term_changed {
TERM_LEN
} else {
INITIAL_LEN
};

s.begin_list(len)
.append(&PREFIX)
.append(&self.number_of_shards)
Expand All @@ -159,48 +175,63 @@ impl Encodable for Metadata {
}
s.append(&self.seq).append(self.params.as_ref().unwrap());
}
if term_params_exist {
if !params_changed {
unreachable!("Term params only can be changed if params changed");
}
s.append(self.term_params.as_ref().unwrap());
}
}
}

impl Decodable for Metadata {
fn decode(rlp: &Rlp) -> Result<Self, DecoderError> {
let (term, seq, params) = match rlp.item_count()? {
4 => (TermMetadata::default(), 0, None),
6 => (
TermMetadata {
last_term_finished_block_num: rlp.val_at(4)?,
current_term_id: rlp.val_at(5)?,
},
0,
None,
),
8 => (
TermMetadata {
last_term_finished_block_num: rlp.val_at(4)?,
current_term_id: rlp.val_at(5)?,
},
rlp.val_at(6)?,
Some(rlp.val_at(7)?),
),
item_count => {
return Err(DecoderError::RlpInvalidLength {
got: item_count,
expected: 4,
})
}
};
let item_count = rlp.item_count()?;
if !VALID_LEN.contains(&item_count) {
return Err(DecoderError::RlpInvalidLength {
got: item_count,
expected: 4,
})
}

let prefix = rlp.val_at::<u8>(0)?;
if PREFIX != prefix {
cdebug!(STATE, "{} is not an expected prefix for asset", prefix);
return Err(DecoderError::Custom("Unexpected prefix"))
}
let number_of_shards = rlp.val_at(1)?;
let number_of_initial_shards = rlp.val_at(2)?;
let hashes = rlp.list_at(3)?;

let term = if item_count >= TERM_LEN {
TermMetadata {
last_term_finished_block_num: rlp.val_at(4)?,
current_term_id: rlp.val_at(5)?,
}
} else {
TermMetadata::default()
};

let (seq, params) = if item_count >= PARAMS_LEN {
(rlp.val_at(6)?, Some(rlp.val_at(7)?))
} else {
Default::default()
};

let term_params = if item_count >= TERM_PARAMS_LEN {
Some(rlp.val_at(8)?)
} else {
Default::default()
};

Ok(Self {
number_of_shards: rlp.val_at(1)?,
number_of_initial_shards: rlp.val_at(2)?,
hashes: rlp.list_at(3)?,
number_of_shards,
number_of_initial_shards,
hashes,
term,
seq,
params,
term_params,
})
}
}
Expand Down Expand Up @@ -266,6 +297,7 @@ mod tests {
term: Default::default(),
seq: 0,
params: None,
term_params: None,
};
let mut rlp = RlpStream::new_list(4);
rlp.append(&PREFIX).append(&10u16).append(&1u16).append_list::<H256, H256>(&[]);
Expand All @@ -281,6 +313,7 @@ mod tests {
term: Default::default(),
seq: 3,
params: Some(CommonParams::default_for_test()),
term_params: Some(CommonParams::default_for_test()),
};
rlp_encode_and_decode_test!(metadata);
}
Expand All @@ -297,6 +330,7 @@ mod tests {
},
seq: 0,
params: None,
term_params: None,
};
rlp_encode_and_decode_test!(metadata);
}
Expand All @@ -313,6 +347,24 @@ mod tests {
},
seq: 3,
params: Some(CommonParams::default_for_test()),
term_params: Some(CommonParams::default_for_test()),
};
rlp_encode_and_decode_test!(metadata);
}

#[test]
fn metadata_with_term_and_seq_but_not_term_params() {
let metadata = Metadata {
number_of_shards: 10,
number_of_initial_shards: 1,
hashes: vec![],
term: TermMetadata {
last_term_finished_block_num: 1,
current_term_id: 100,
},
seq: 3,
params: Some(CommonParams::default_for_test()),
term_params: None,
};
rlp_encode_and_decode_test!(metadata);
}
Expand Down
1 change: 1 addition & 0 deletions state/src/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,7 @@ pub trait TopState {
fn remove_action_data(&mut self, key: &H256);

fn update_params(&mut self, metadata_seq: u64, params: CommonParams) -> StateResult<()>;
fn snapshot_term_params(&mut self) -> StateResult<()>;
}

pub trait StateWithCache {
Expand Down