Skip to content

Commit

Permalink
Problem (Fix crypto-com#1313): light client doesn't verify the fetche…
Browse files Browse the repository at this point in the history
…d staking state

Solution:
- Add staking_root in sync state
- Verify staking and proof against the trusted staking_root in sync state
  • Loading branch information
yihuang committed Apr 29, 2020
1 parent 1cf0bf6 commit 7f28e20
Show file tree
Hide file tree
Showing 21 changed files with 195 additions and 111 deletions.
2 changes: 2 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

20 changes: 0 additions & 20 deletions chain-abci/src/app/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -170,26 +170,6 @@ impl<T: EnclaveProxy> ChainNodeApp<T> {
"app state not found",
);
}
"account" => {
let account_address = StakedStateAddress::try_from(_req.data.as_slice());
if let (Some(state), Ok(address)) = (&self.last_state, account_address) {
let (account, _proof) =
get_with_proof(&self.storage, state.staking_version, &address);
match account {
Some(a) => {
resp.value = a.encode();
// TODO: inclusion proof
}
None => {
resp.log += "account lookup failed: account not exists";
resp.code = 1;
}
}
} else {
resp.log += "account lookup failed (either invalid address or node not correctly restored / initialized)";
resp.code = 3;
}
}
"staking" => {
let mversion = if let Ok(height) = _req.height.try_into() {
self.storage.get_historical_staking_version(height)
Expand Down
12 changes: 0 additions & 12 deletions chain-abci/tests/abci_app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -751,18 +751,6 @@ fn no_delivered_tx_commit_should_keep_apphash() {
assert_eq!(&old_app_hash[..], &cresp.data[..]);
}

#[test]
fn query_should_return_an_account() {
let addr = "fe7c045110b8dbf29765047380898919c5cb56f9";
let mut app = init_chain_for(addr.parse().unwrap());
let mut qreq = RequestQuery::new();
qreq.data = hex::decode(&addr).unwrap();
qreq.path = "account".into();
let qresp = app.query(&qreq);
let account = StakedState::decode(&mut qresp.value.as_slice()).unwrap();
assert_eq!(account.address, StakedStateAddress::from_str(addr).unwrap());
}

#[test]
fn staking_query_should_return_an_account() {
let addr = "fe7c045110b8dbf29765047380898919c5cb56f9";
Expand Down
2 changes: 1 addition & 1 deletion chain-core/src/init/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ impl InitConfig {

/// returns the initial accounts
/// assumes one called [validate_config_get_genesis], otherwise it may panic
fn get_account(&self, genesis_time: Timespec) -> Vec<StakedState> {
pub fn get_account(&self, genesis_time: Timespec) -> Vec<StakedState> {
self.distribution
.iter()
.map(|(address, (destination, amount))| {
Expand Down
8 changes: 8 additions & 0 deletions chain-core/src/state/account/address.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,3 +121,11 @@ impl FromStr for StakedStateAddress {
Ok(StakedStateAddress::BasicRedeem(RedeemAddress::from_str(s)?))
}
}

impl AsRef<[u8]> for StakedStateAddress {
fn as_ref(&self) -> &[u8] {
match self {
StakedStateAddress::BasicRedeem(a) => &a,
}
}
}
13 changes: 10 additions & 3 deletions client-cli/src/command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,8 @@ pub enum Command {
},
#[structopt(name = "state", about = "Get staked state of an address")]
StakedState {
#[structopt(name = "wallet name", short = "n", long = "name", help = "Wallet name")]
name: String,
#[structopt(
name = "staking address",
short = "a",
Expand Down Expand Up @@ -316,7 +318,11 @@ impl Command {
transaction_command
.execute(network_ops_client.get_wallet_client(), &network_ops_client)
}
Command::StakedState { address, hardware } => {
Command::StakedState {
name,
address,
hardware,
} => {
let hw_key_service = match hardware {
None => HwKeyService::default(),
#[cfg(feature = "mock-hardware-wallet")]
Expand Down Expand Up @@ -350,7 +356,7 @@ impl Command {
fee_algorithm,
transaction_obfuscation,
);
Self::get_staked_stake(&network_ops_client, address)
Self::get_staked_stake(&network_ops_client, &name, address)
}
Command::Sync {
name,
Expand Down Expand Up @@ -382,9 +388,10 @@ impl Command {

fn get_staked_stake<N: NetworkOpsClient>(
network_ops_client: &N,
name: &str,
address: &StakedStateAddress,
) -> Result<()> {
let staked_state = network_ops_client.get_staked_state(address)?;
let staked_state = network_ops_client.get_staked_state(name, address, true)?;

let bold = CellFormat::builder().bold(true).build();
let justify_right = CellFormat::builder().justify(Justify::Right).build();
Expand Down
1 change: 1 addition & 0 deletions client-core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ client-common = { path = "../client-common" }
chain-tx-filter = { path = "../chain-tx-filter" }
enclave-protocol = { path = "../enclave-protocol" }
chain-tx-validation = { path = "../chain-tx-validation" }
chain-storage = { path = "../chain-storage" }
mock-utils = { path = "../chain-tx-enclave/mock-utils" }
secp256k1zkp = { git = "https://github.com/crypto-com/rust-secp256k1-zkp.git", rev = "745bc8d8dc80cb921d5788e863a3536d3b6498a1", features = ["serde", "zeroize", "rand", "recovery", "endomorphism", "musig"] }
parity-scale-codec = { features = ["derive"], version = "1.3" }
Expand Down
9 changes: 7 additions & 2 deletions client-core/src/service/sync_state_service.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use chain_core::common::H256;
use client_common::tendermint::lite;
use client_common::{ErrorKind, Result, ResultExt, Storage};
use parity_scale_codec::{Decode, Encode};
Expand All @@ -15,15 +16,18 @@ pub struct SyncState {
pub last_app_hash: String,
/// current trusted state for lite client verification
pub trusted_state: lite::TrustedState,
/// current trusted staking_root
pub staking_root: H256,
}

impl SyncState {
/// construct genesis global state
pub fn genesis(genesis_validators: Vec<validator::Info>) -> SyncState {
pub fn genesis(genesis_validators: Vec<validator::Info>, staking_root: H256) -> SyncState {
SyncState {
last_block_height: 0,
last_app_hash: "".to_owned(),
trusted_state: lite::TrustedState::genesis(genesis_validators),
staking_root,
}
}
}
Expand Down Expand Up @@ -131,6 +135,7 @@ mod tests {
"3891040F29C6A56A5E36B17DCA6992D8F91D1EAAB4439D008D19A9D703271D3C"
.to_string(),
trusted_state: TrustedState::genesis(vec![]),
staking_root: [0u8; 32],
}
)
.is_ok());
Expand Down Expand Up @@ -174,7 +179,7 @@ mod tests {
gen.validators.clone(),
)
.into();
let mut state = SyncState::genesis(vec![]);
let mut state = SyncState::genesis(vec![], [0u8; 32]);
state.last_block_height = 1;
state.last_app_hash =
"0F46E113C21F9EACB26D752F9523746CF8D47ECBEA492736D176005911F973A5".to_owned();
Expand Down
5 changes: 4 additions & 1 deletion client-core/src/wallet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ use client_common::{
use serde::{Deserialize, Serialize};

use crate::hd_wallet::HardwareKind;
use crate::service::WalletInfo;
use crate::service::{SyncState, WalletInfo};
use crate::transaction_builder::{SignedTransferTransaction, UnsignedTransferTransaction};
use crate::types::{AddressType, TransactionChange, TransactionPending, WalletBalance, WalletKind};
use crate::{InputSelectionStrategy, Mnemonic, UnspentTransactions};
Expand Down Expand Up @@ -379,6 +379,9 @@ pub trait WalletClient: Send + Sync {
enckey: &SecKey,
signed_tx: SignedTransferTransaction,
) -> Result<TxId>;

/// Get current sync state of wallet, return genesis one if not exists.
fn get_sync_state(&self, name: &str) -> Result<SyncState>;
}

/// Interface for a generic wallet for multi-signature transactions
Expand Down
11 changes: 11 additions & 0 deletions client-core/src/wallet/default_wallet_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use crate::transaction_builder::{SignedTransferTransaction, UnsignedTransferTran
use crate::types::{
AddressType, BalanceChange, TransactionChange, TransactionPending, WalletBalance, WalletKind,
};
use crate::wallet::syncer::get_genesis_sync_state;
use crate::wallet::syncer_logic::create_transaction_change;
use crate::{
InputSelectionStrategy, Mnemonic, MultiSigWalletClient, UnspentTransactions, WalletClient,
Expand Down Expand Up @@ -1026,6 +1027,16 @@ where
))
}
}

fn get_sync_state(&self, name: &str) -> Result<SyncState> {
let mstate = self.sync_state_service.get_global_state(name)?;
let sync_state = if let Some(sync_state) = mstate {
sync_state
} else {
get_genesis_sync_state(&self.tendermint_client)?
};
Ok(sync_state)
}
}

impl<S, C, T> MultiSigWalletClient for DefaultWalletClient<S, C, T>
Expand Down
44 changes: 39 additions & 5 deletions client-core/src/wallet/syncer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,14 @@
use indexmap::IndexMap;
use itertools::{izip, Itertools};
use non_empty_vec::NonEmpty;
use std::iter;

use chain_core::common::H256;
use chain_core::state::account::StakedStateAddress;
use chain_core::state::ChainState;
use chain_core::tx::data::TxId;
use chain_core::tx::fee::Fee;
use chain_storage::jellyfish::compute_staking_root;
use chain_tx_filter::BlockFilter;
use client_common::tendermint::types::{
Block, BlockExt, BlockResults, BlockResultsResponse, StatusResponse, Time,
Expand Down Expand Up @@ -227,11 +230,11 @@ impl<'a, S: SecureStorage, C: Client, D: TxDecryptor, F: FnMut(ProgressReport) -
format!("wallet not found: {}", env.name)
})?;

let sync_state = service::load_sync_state(&env.storage, &env.name)?;
let sync_state = if let Some(sync_state) = sync_state {
let mstate = service::load_sync_state(&env.storage, &env.name)?;
let sync_state = if let Some(sync_state) = mstate {
sync_state
} else {
SyncState::genesis(env.client.genesis()?.validators)
get_genesis_sync_state(&env.client)?
};

let wallet_state =
Expand Down Expand Up @@ -290,6 +293,7 @@ impl<'a, S: SecureStorage, C: Client, D: TxDecryptor, F: FnMut(ProgressReport) -
let block = blocks.last();
self.sync_state.last_block_height = block.block_height;
self.sync_state.last_app_hash = block.app_hash.clone();
self.sync_state.staking_root = block.staking_root.clone();
self.save(&memento)?;

if !self.update_progress(block.block_height) {
Expand Down Expand Up @@ -372,7 +376,7 @@ impl<'a, S: SecureStorage, C: Client, D: TxDecryptor, F: FnMut(ProgressReport) -
),
);

let block = FilteredBlock::from_block(&self.wallet, &block, &block_result)?;
let block = FilteredBlock::from_block(&self.wallet, &block, &block_result, &state)?;
self.update_progress(block.block_height);
batch.push(block);
}
Expand Down Expand Up @@ -410,11 +414,15 @@ impl<'a, S: SecureStorage, C: Client, D: TxDecryptor, F: FnMut(ProgressReport) -

let block = self.env.client.block(current_block_height)?;
let block_result = self.env.client.block_results(current_block_height)?;

let states = self
.env
.client
.query_state_batch(iter::once(current_block_height))?;
Ok(Some(FilteredBlock::from_block(
&self.wallet,
&block,
&block_result,
&states[0],
)?))
} else {
Ok(None)
Expand All @@ -428,17 +436,38 @@ impl<'a, S: SecureStorage, C: Client, D: TxDecryptor, F: FnMut(ProgressReport) -
if current_app_hash == self.sync_state.last_app_hash {
let current_block_height = block.header.height.value();
let block_result = self.env.client.block_results(current_block_height)?;
let states = self
.env
.client
.query_state_batch(iter::once(current_block_height))?;
Ok(Some(FilteredBlock::from_block(
&self.wallet,
&block,
&block_result,
&states[0],
)?))
} else {
Ok(None)
}
}
}

pub fn get_genesis_sync_state<C: Client>(client: &C) -> Result<SyncState> {
// FIXME verify against trusted genesis hash
let genesis = client.genesis()?;
let accounts = genesis.app_state.unwrap().get_account(
genesis
.genesis_time
.duration_since(Time::unix_epoch())
.expect("invalid genesis time")
.as_secs(),
);
Ok(SyncState::genesis(
genesis.validators,
compute_staking_root(&accounts),
))
}

/// A struct for providing progress report for synchronization
#[derive(Debug, Clone)]
pub enum ProgressReport {
Expand Down Expand Up @@ -478,6 +507,8 @@ pub(crate) struct FilteredBlock {
pub enclave_transaction_ids: Vec<TxId>,
/// List of un-encrypted transactions (only contains transactions of type `DepositStake` and `UnbondStake`)
pub staking_transactions: Vec<Transaction>,
/// staking root after this block
pub staking_root: H256,
}

impl FilteredBlock {
Expand All @@ -486,6 +517,7 @@ impl FilteredBlock {
wallet: &Wallet,
block: &Block,
block_result: &BlockResultsResponse,
state: &ChainState,
) -> Result<FilteredBlock> {
let app_hash = hex::encode(&block.header.app_hash);
let block_height = block.header.height.value();
Expand Down Expand Up @@ -513,6 +545,7 @@ impl FilteredBlock {
enclave_transaction_ids,
block_filter,
staking_transactions,
staking_root: state.account_root.clone(),
})
}
}
Expand Down Expand Up @@ -680,6 +713,7 @@ mod tests {
last_block_height: 1745,
last_app_hash: "3fe291fd64f1140acfe38988a9f8c5b0cb5da43a0214bbd4000035509ce34205"
.to_string(),
staking_root: [0u8; 32],
trusted_state,
},
)
Expand Down
Loading

0 comments on commit 7f28e20

Please sign in to comment.