Skip to content
This repository has been archived by the owner on Jul 27, 2022. It is now read-only.

Commit

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

Solution:
- Add staking_root in sync state
- Remove the "account" path in favor of the new "staking" path
- Client staking state command add wallet name parameter,
  and verify staking state and proof against the trusted staking_root in sync state
- Need to sync wallet before fetching the new staking state now.
  • Loading branch information
yihuang committed May 3, 2020
1 parent 94fe85a commit 6322057
Show file tree
Hide file tree
Showing 24 changed files with 266 additions and 125 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 @@ -779,18 +779,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 @@ -149,7 +149,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,
}
}
}
17 changes: 14 additions & 3 deletions client-cli/src/command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,11 @@ use client_core::cipher::mock::MockAbciTransactionObfuscation;
#[cfg(not(feature = "mock-enclave"))]
use client_core::cipher::DefaultTransactionObfuscation;

#[cfg(not(feature = "mock-enclave"))]
type AppTransactionCipher = DefaultTransactionObfuscation;
#[cfg(feature = "mock-enclave")]
type AppTransactionCipher = MockAbciTransactionObfuscation<WebsocketRpcClient>;

type AppTxBuilder = DefaultWalletTransactionBuilder<SledStorage, LinearFee, AppTransactionCipher>;
type AppWalletClient = DefaultWalletClient<SledStorage, WebsocketRpcClient, AppTxBuilder>;

Expand Down Expand Up @@ -152,6 +156,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 @@ -327,7 +333,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 @@ -361,7 +371,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 @@ -395,9 +405,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
9 changes: 7 additions & 2 deletions client-cli/src/command/transaction_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -602,6 +602,7 @@ fn new_withdraw_transaction<T: WalletClient, N: NetworkOpsClient>(
&from_address,
to_address,
attributes,
true,
)
}

Expand All @@ -613,7 +614,8 @@ fn new_unbond_transaction<N: NetworkOpsClient>(
let attributes = StakedStateOpAttributes::new(get_network_id());
let address = ask_staking_address()?;
let value = ask_cro()?;
network_ops_client.create_unbond_stake_transaction(name, enckey, address, value, attributes)
network_ops_client
.create_unbond_stake_transaction(name, enckey, address, value, attributes, true)
}

fn new_deposit_transaction<T: WalletClient, N: NetworkOpsClient>(
Expand Down Expand Up @@ -644,6 +646,7 @@ fn new_deposit_transaction<T: WalletClient, N: NetworkOpsClient>(
transactions,
to_address,
attributes,
true,
)
}

Expand Down Expand Up @@ -701,6 +704,7 @@ fn new_deposit_amount_transaction<T: WalletClient, N: NetworkOpsClient>(
transactions,
to_staking_address,
attr,
true,
)?;
let tx_id = transaction.tx_id();
success(&format!(
Expand Down Expand Up @@ -758,7 +762,7 @@ fn new_unjail_transaction<N: NetworkOpsClient>(
let attributes = StakedStateOpAttributes::new(get_network_id());
let address = ask_staking_address()?;

network_ops_client.create_unjail_transaction(name, enckey, address, attributes)
network_ops_client.create_unjail_transaction(name, enckey, address, attributes, true)
}

fn new_node_join_transaction<N: NetworkOpsClient>(
Expand All @@ -776,6 +780,7 @@ fn new_node_join_transaction<N: NetworkOpsClient>(
staking_account_address,
attributes,
node_metadata,
true,
)
}

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 = "f8759809f6e3fed793b37166f7cd91c57cdb2eab", 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
12 changes: 11 additions & 1 deletion client-core/src/wallet/default_wallet_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use crate::transaction_builder::{SignedTransferTransaction, UnsignedTransferTran
use crate::types::{
AddressType, BalanceChange, TransactionChange, TransactionPending, WalletBalance, WalletKind,
};
use crate::wallet::syncer::AddressRecovery;
use crate::wallet::syncer::{get_genesis_sync_state, AddressRecovery};
use crate::wallet::syncer_logic::create_transaction_change;
use crate::{
InputSelectionStrategy, Mnemonic, MultiSigWalletClient, UnspentTransactions, WalletClient,
Expand Down Expand Up @@ -1084,6 +1084,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
Loading

0 comments on commit 6322057

Please sign in to comment.