Skip to content

Commit

Permalink
fix chainstateservice and tests
Browse files Browse the repository at this point in the history
1. implement default get_block_number instead of panicing
2. update ChainStateService with new block number
  • Loading branch information
simonjiao committed Oct 26, 2023
1 parent cd7effc commit 7324cd3
Show file tree
Hide file tree
Showing 19 changed files with 87 additions and 26 deletions.
4 changes: 4 additions & 0 deletions abi/resolver/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -405,6 +405,10 @@ mod tests {
fn is_genesis(&self) -> bool {
todo!()
}

fn get_block_number(&self) -> Option<u64> {
None
}
}

#[test]
Expand Down
5 changes: 3 additions & 2 deletions chain/src/chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -453,12 +453,13 @@ impl BlockChain {
parents_hash: Option<Vec<HashValue>>,
) -> Result<ExecutedBlock> {
let header = block.header();
// Todo: add debug_assert for height_of_statedb
let height_of_state = statedb.get_block_number();
debug_assert!(header.is_genesis() || parent_status.is_some());
debug_assert!(!header.is_genesis() || parent_status.is_none());
let block_id = header.id();
let block_num = header.number();
debug_assert!(header.is_genesis() && height_of_state.is_none());
debug_assert!(!header.is_genesis() && height_of_state.is_some());
let transactions = {
// genesis block do not generate BlockMetadata transaction.
let mut t = match &parent_status {
Expand Down Expand Up @@ -1066,7 +1067,7 @@ impl ChainReader for BlockChain {
let state_proof = if let Some(access_path) = access_path {
let statedb = self
.statedb
.fork_at(transaction_info.txn_info().state_root_hash());
.fork_at(transaction_info.txn_info().state_root_hash(), None);
Some(statedb.get_with_proof(&access_path)?)
} else {
None
Expand Down
4 changes: 3 additions & 1 deletion contrib-contracts/src/starcoin_merkle_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use starcoin_types::identifier::Identifier;
use starcoin_types::language_storage::ModuleId;
use starcoin_vm_types::access_path::AccessPath;
use starcoin_vm_types::account_config::association_address;
use starcoin_vm_types::state_view::StateView;
use starcoin_vm_types::transaction::{Package, ScriptFunction, TransactionPayload};
use starcoin_vm_types::value::MoveValue;
use test_helper::executor::{
Expand Down Expand Up @@ -38,6 +39,7 @@ fn test_starcoin_merkle() -> Result<()> {
}

let state_root = chain_state.state_root();
let block_number = chain_state.get_block_number();

{
let script_function = ScriptFunction::new(
Expand All @@ -62,7 +64,7 @@ fn test_starcoin_merkle() -> Result<()> {

{
// change to previous state root.
let old_chain_state = chain_state.fork_at(state_root);
let old_chain_state = chain_state.fork_at(state_root, block_number);
// let state_root = chain_state.state_root();
let _expected_root = MoveValue::vector_u8(state_root.to_vec());

Expand Down
4 changes: 4 additions & 0 deletions executor/tests/executor_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,10 @@ impl StateView for NullStateView {
fn is_genesis(&self) -> bool {
false
}

fn get_block_number(&self) -> Option<u64> {
None
}
}

#[stest::test]
Expand Down
4 changes: 4 additions & 0 deletions network-rpc/api/src/remote_chain_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -154,4 +154,8 @@ impl StateView for RemoteChainStateReader {
fn is_genesis(&self) -> bool {
false
}

fn get_block_number(&self) -> Option<u64> {
None
}
}
4 changes: 4 additions & 0 deletions rpc/client/src/remote_state_reader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -140,4 +140,8 @@ impl<'a> StateView for RemoteStateReader<'a> {
fn is_genesis(&self) -> bool {
false
}

fn get_block_number(&self) -> Option<u64> {
None
}
}
8 changes: 6 additions & 2 deletions rpc/server/src/module/contract_rpc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,9 +128,11 @@ where
args,
} = call;
let f = async move {
let state_root = service.state_root().await?;
let state_root = service.clone().state_root().await?;
let block_number = service.get_block_number().await?;
let output = playground.call_contract(
state_root,
block_number,
function_id.0.module,
function_id.0.function,
type_args.into_iter().map(|v| v.0).collect(),
Expand Down Expand Up @@ -231,7 +233,9 @@ where
let service = self.chain_state.clone();
let storage = self.storage.clone();
let fut = async move {
let state = ChainStateDB::new(storage, Some(service.state_root().await?));
let state_root = service.clone().state_root().await?;
let block_number = service.get_block_number().await?;
let state = ChainStateDB::new_with_root(storage, Some(state_root), block_number);
ABIResolver::new(&state)
.resolve_function(&function_id.0.module, function_id.0.function.as_ident_str())
}
Expand Down
20 changes: 12 additions & 8 deletions state/service/src/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -140,8 +140,12 @@ impl EventHandler<Self, NewHeadBlock> for ChainStateService {
let NewHeadBlock(block, _dag_parents) = msg;

let state_root = block.header().state_root();
debug!("ChainStateActor change StateRoot to : {:?}", state_root);
self.service.change_root(state_root);
let block_number = block.header().number();
debug!(
"ChainStateActor change StateRoot to : {:?} {block_number}",
state_root
);
self.service.change_root(state_root, block_number);
}
}

Expand Down Expand Up @@ -171,7 +175,7 @@ impl Inner {
) -> Result<Option<AccountStateSet>> {
match state_root {
Some(root) => {
let reader = self.state_db.fork_at(root);
let reader = self.state_db.fork_at(root, None);
reader.get_account_state_set(&address)
}
None => self.get_account_state_set(&address),
Expand All @@ -183,7 +187,7 @@ impl Inner {
access_path: AccessPath,
state_root: HashValue,
) -> Result<StateWithProof> {
let reader = self.state_db.fork_at(state_root);
let reader = self.state_db.fork_at(state_root, None);
reader.get_with_proof(&access_path)
}

Expand All @@ -193,7 +197,7 @@ impl Inner {
key: Vec<u8>,
state_root: HashValue,
) -> Result<StateWithTableItemProof> {
let reader = self.state_db.fork_at(state_root);
let reader = self.state_db.fork_at(state_root, None);
reader.get_with_table_item_proof(&handle, &key)
}

Expand All @@ -202,12 +206,12 @@ impl Inner {
account: AccountAddress,
state_root: HashValue,
) -> Result<Option<AccountState>> {
let reader = self.state_db.fork_at(state_root);
let reader = self.state_db.fork_at(state_root, None);
reader.get_account_state(&account)
}

pub(crate) fn change_root(&mut self, state_root: HashValue) {
self.state_db = self.state_db.fork_at(state_root);
pub(crate) fn change_root(&mut self, state_root: HashValue, block_number: BlockNumber) {
self.state_db = self.state_db.fork_at(state_root, Some(block_number));
self.adjust_time();
}

Expand Down
4 changes: 2 additions & 2 deletions state/statedb/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -291,8 +291,8 @@ impl ChainStateDB {
}

/// Fork a new statedb at `root_hash`
pub fn fork_at(&self, state_root: HashValue) -> Self {
Self::new_with_root(self.store.clone(), Some(state_root), None)
pub fn fork_at(&self, state_root: HashValue, block_number: Option<BlockNumber>) -> Self {
Self::new_with_root(self.store.clone(), Some(state_root), block_number)
}

fn new_state_tree<K: RawKey>(&self, root_hash: HashValue) -> StateTree<K> {
Expand Down
2 changes: 1 addition & 1 deletion test-helper/src/starcoin_dao.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ fn get_with_proof_by_root(
access_path: AccessPath,
state_root: HashValue,
) -> Result<StateWithProof> {
let reader = state_db.fork_at(state_root);
let reader = state_db.fork_at(state_root, None);
reader.get_with_proof(&access_path)
}

Expand Down
8 changes: 6 additions & 2 deletions vm/dev/src/playground.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,21 +43,25 @@ impl PlaygroudService {
pub fn dry_run(
&self,
state_root: HashValue,
block_number: Option<u64>,
txn: DryRunTransaction,
) -> Result<(VMStatus, TransactionOutput)> {
let state_view = ChainStateDB::new(self.state.clone(), Some(state_root));
let state_view =
ChainStateDB::new_with_root(self.state.clone(), Some(state_root), block_number);
dry_run(&state_view, txn, self.metrics.clone())
}

pub fn call_contract(
&self,
state_root: HashValue,
block_number: Option<u64>,
module_id: ModuleId,
func: Identifier,
type_args: Vec<TypeTag>,
args: Vec<TransactionArgument>,
) -> Result<Vec<AnnotatedMoveValue>> {
let state_view = ChainStateDB::new(self.state.clone(), Some(state_root));
let state_view =
ChainStateDB::new_with_root(self.state.clone(), Some(state_root), block_number);
let rets = call_contract(
&state_view,
module_id,
Expand Down
4 changes: 4 additions & 0 deletions vm/e2e-tests/src/data_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,10 @@ impl StateView for FakeDataStore {
fn is_genesis(&self) -> bool {
self.inner().is_empty()
}

fn get_block_number(&self) -> Option<u64> {
None
}
}

impl ChainStateWriter for FakeDataStore {
Expand Down
4 changes: 4 additions & 0 deletions vm/resource-viewer/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -324,4 +324,8 @@ impl StateView for NullStateView {
fn is_genesis(&self) -> bool {
false
}

fn get_block_number(&self) -> Option<u64> {
None
}
}
8 changes: 4 additions & 4 deletions vm/starcoin-transactional-test-harness/src/fork_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ impl ChainStateAsyncService for MockChainStateAsyncService {
) -> Result<Option<AccountStateSet>> {
match state_root {
Some(root) => {
let reader = self.state_db().fork_at(root);
let reader = self.state_db().fork_at(root, None);
reader.get_account_state_set(&address)
}
None => self.state_db().get_account_state_set(&address),
Expand All @@ -132,7 +132,7 @@ impl ChainStateAsyncService for MockChainStateAsyncService {
access_path: AccessPath,
state_root: HashValue,
) -> Result<StateWithProof> {
let reader = self.state_db().fork_at(state_root);
let reader = self.state_db().fork_at(state_root, None);
reader.get_with_proof(&access_path)
}

Expand All @@ -141,7 +141,7 @@ impl ChainStateAsyncService for MockChainStateAsyncService {
account_address: AccountAddress,
state_root: HashValue,
) -> Result<Option<AccountState>> {
let reader = self.state_db().fork_at(state_root);
let reader = self.state_db().fork_at(state_root, None);
reader.get_account_state(&account_address)
}

Expand All @@ -160,7 +160,7 @@ impl ChainStateAsyncService for MockChainStateAsyncService {
key: Vec<u8>,
state_root: HashValue,
) -> Result<StateWithTableItemProof> {
let reader = self.state_db().fork_at(state_root);
let reader = self.state_db().fork_at(state_root, None);
reader.get_with_table_item_proof(&handle, &key)
}

Expand Down
12 changes: 12 additions & 0 deletions vm/starcoin-transactional-test-harness/src/remote_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,10 @@ where
fn is_genesis(&self) -> bool {
false
}

fn get_block_number(&self) -> Option<u64> {
None
}
}
impl<A, B> ChainStateWriter for SelectableStateView<A, B>
where
Expand Down Expand Up @@ -184,6 +188,10 @@ where
fn is_genesis(&self) -> bool {
false
}

fn get_block_number(&self) -> Option<u64> {
None
}
}

//TODO migrate this to rpc client crate.
Expand Down Expand Up @@ -405,4 +413,8 @@ impl StateView for RemoteViewer {
fn is_genesis(&self) -> bool {
false
}

fn get_block_number(&self) -> Option<u64> {
None
}
}
8 changes: 5 additions & 3 deletions vm/types/src/state_view.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,7 @@ pub trait StateView: Sync {
/// Currently TransactionPayload::WriteSet is only valid for genesis state creation.
fn is_genesis(&self) -> bool;

fn get_block_number(&self) -> Option<u64> {
panic!("Never call me and implement this function yourself");
}
fn get_block_number(&self) -> Option<u64>;
}

impl<R, S> StateView for R
Expand All @@ -60,6 +58,10 @@ where
fn is_genesis(&self) -> bool {
self.deref().is_genesis()
}

fn get_block_number(&self) -> Option<u64> {
self.deref().get_block_number()
}
}

impl<T: ?Sized> StateReaderExt for T where T: StateView {}
Expand Down
4 changes: 4 additions & 0 deletions vm/vm-runtime/src/data_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,10 @@ impl<'block, S: StateView> StateView for StateViewCache<'block, S> {
fn is_genesis(&self) -> bool {
self.data_view.is_genesis()
}

fn get_block_number(&self) -> Option<u64> {
self.data_view.get_block_number()
}
}

impl<'block, S: StateView> ModuleResolver for StateViewCache<'block, S> {
Expand Down
4 changes: 4 additions & 0 deletions vm/vm-runtime/src/parallel_executor/storage_wrapper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,4 +40,8 @@ impl<'a, S: StateView> StateView for VersionedView<'a, S> {
fn is_genesis(&self) -> bool {
self.base_view.is_genesis()
}

fn get_block_number(&self) -> Option<u64> {
self.base_view.get_block_number()
}
}
2 changes: 1 addition & 1 deletion vm/vm-runtime/src/starcoin_vm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,7 @@ impl StarcoinVM {
if height_of_state >= FLEXI_DAG_FORK_HEIGHT
&& stdlib_version >= StdlibVersion::Version(FLEXIDAG_UPGRADE_VERSION_MARK)
{
todo!()
debug!("flexidag stdlib");
}

(gas_schedule, "gas schedule from GasSchedule")
Expand Down

0 comments on commit 7324cd3

Please sign in to comment.