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

Commit

Permalink
Problem (Fix #777): Order of client list address api is not stable
Browse files Browse the repository at this point in the history
Solution:
- change `staking_keys`/`root_hashes` from `BTreeSet` to `Vec`
  • Loading branch information
yihuang committed Jan 20, 2020
1 parent e9eb46b commit cb0a155
Show file tree
Hide file tree
Showing 6 changed files with 30 additions and 43 deletions.
5 changes: 4 additions & 1 deletion client-cli/src/command/address_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,10 @@ impl AddressCommand {

let pub_keys = match address_type {
AddressType::Staking => wallet_client.staking_keys(name, &enckey)?,
AddressType::Transfer => wallet_client.public_keys(name, &enckey)?,
AddressType::Transfer => wallet_client
.public_keys(name, &enckey)?
.into_iter()
.collect(),
};
for pubkey in pub_keys.iter() {
println!("{}", pubkey);
Expand Down
33 changes: 13 additions & 20 deletions client-core/src/service/wallet_service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,9 @@ pub struct Wallet {
/// public keys to construct transfer addresses
pub public_keys: BTreeSet<PublicKey>,
/// public keys of staking addresses
pub staking_keys: BTreeSet<PublicKey>,
pub staking_keys: Vec<PublicKey>,
/// root hashes of multi-sig transfer addresses
pub root_hashes: BTreeSet<H256>,
pub root_hashes: Vec<H256>,
}

impl Wallet {
Expand All @@ -39,15 +39,15 @@ impl Wallet {
}

/// Returns all staking addresses stored in a wallet
pub fn staking_addresses(&self) -> BTreeSet<StakedStateAddress> {
pub fn staking_addresses(&self) -> Vec<StakedStateAddress> {
self.staking_keys
.iter()
.map(|public_key| StakedStateAddress::BasicRedeem(RedeemAddress::from(public_key)))
.collect()
}

/// Returns all tree addresses stored in a wallet
pub fn transfer_addresses(&self) -> BTreeSet<ExtendedAddr> {
pub fn transfer_addresses(&self) -> Vec<ExtendedAddr> {
self.root_hashes
.iter()
.cloned()
Expand Down Expand Up @@ -170,37 +170,30 @@ where

/// Returns all public keys stored in a wallet
pub fn public_keys(&self, name: &str, enckey: &SecKey) -> Result<BTreeSet<PublicKey>> {
let wallet = self.get_wallet(name, enckey)?;
Ok(wallet.public_keys)
Ok(self.get_wallet(name, enckey)?.public_keys)
}

/// Returns all public keys corresponding to staking addresses stored in a wallet
pub fn staking_keys(&self, name: &str, enckey: &SecKey) -> Result<BTreeSet<PublicKey>> {
let wallet = self.get_wallet(name, enckey)?;
Ok(wallet.staking_keys)
pub fn staking_keys(&self, name: &str, enckey: &SecKey) -> Result<Vec<PublicKey>> {
Ok(self.get_wallet(name, enckey)?.staking_keys)
}

/// Returns all multi-sig addresses stored in a wallet
pub fn root_hashes(&self, name: &str, enckey: &SecKey) -> Result<BTreeSet<H256>> {
let wallet = self.get_wallet(name, enckey)?;
Ok(wallet.root_hashes)
pub fn root_hashes(&self, name: &str, enckey: &SecKey) -> Result<Vec<H256>> {
Ok(self.get_wallet(name, enckey)?.root_hashes)
}

/// Returns all staking addresses stored in a wallet
pub fn staking_addresses(
&self,
name: &str,
enckey: &SecKey,
) -> Result<BTreeSet<StakedStateAddress>> {
) -> Result<Vec<StakedStateAddress>> {
Ok(self.get_wallet(name, enckey)?.staking_addresses())
}

/// Returns all tree addresses stored in a wallet
pub fn transfer_addresses(
&self,
name: &str,
enckey: &SecKey,
) -> Result<BTreeSet<ExtendedAddr>> {
pub fn transfer_addresses(&self, name: &str, enckey: &SecKey) -> Result<Vec<ExtendedAddr>> {
Ok(self.get_wallet(name, enckey)?.transfer_addresses())
}

Expand All @@ -224,7 +217,7 @@ where
staking_key: &PublicKey,
) -> Result<()> {
self.modify_wallet(name, enckey, move |wallet| {
wallet.staking_keys.insert(staking_key.clone());
wallet.staking_keys.push(staking_key.clone());
})
}

Expand Down Expand Up @@ -255,7 +248,7 @@ where
/// Adds a multi-sig address to given wallet
pub fn add_root_hash(&self, name: &str, enckey: &SecKey, root_hash: H256) -> Result<()> {
self.modify_wallet(name, enckey, move |wallet| {
wallet.root_hashes.insert(root_hash);
wallet.root_hashes.push(root_hash);
})
}

Expand Down
12 changes: 4 additions & 8 deletions client-core/src/wallet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,20 +73,16 @@ pub trait WalletClient: Send + Sync {
fn public_keys(&self, name: &str, enckey: &SecKey) -> Result<BTreeSet<PublicKey>>;

/// Retrieves all public keys corresponding to staking addresses stored in given wallet
fn staking_keys(&self, name: &str, enckey: &SecKey) -> Result<BTreeSet<PublicKey>>;
fn staking_keys(&self, name: &str, enckey: &SecKey) -> Result<Vec<PublicKey>>;

/// Retrieves all root hashes corresponding to given wallet
fn root_hashes(&self, name: &str, enckey: &SecKey) -> Result<BTreeSet<H256>>;
fn root_hashes(&self, name: &str, enckey: &SecKey) -> Result<Vec<H256>>;

/// Returns all staking addresses in current wallet
fn staking_addresses(
&self,
name: &str,
enckey: &SecKey,
) -> Result<BTreeSet<StakedStateAddress>>;
fn staking_addresses(&self, name: &str, enckey: &SecKey) -> Result<Vec<StakedStateAddress>>;

/// Returns all the multi-sig transfer addresses in current wallet
fn transfer_addresses(&self, name: &str, enckey: &SecKey) -> Result<BTreeSet<ExtendedAddr>>;
fn transfer_addresses(&self, name: &str, enckey: &SecKey) -> Result<Vec<ExtendedAddr>>;

/// Finds staking key corresponding to given redeem address
fn find_staking_key(
Expand Down
12 changes: 4 additions & 8 deletions client-core/src/wallet/default_wallet_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -247,26 +247,22 @@ where
}

#[inline]
fn staking_keys(&self, name: &str, enckey: &SecKey) -> Result<BTreeSet<PublicKey>> {
fn staking_keys(&self, name: &str, enckey: &SecKey) -> Result<Vec<PublicKey>> {
self.wallet_service.staking_keys(name, enckey)
}

#[inline]
fn root_hashes(&self, name: &str, enckey: &SecKey) -> Result<BTreeSet<H256>> {
fn root_hashes(&self, name: &str, enckey: &SecKey) -> Result<Vec<H256>> {
self.wallet_service.root_hashes(name, enckey)
}

#[inline]
fn staking_addresses(
&self,
name: &str,
enckey: &SecKey,
) -> Result<BTreeSet<StakedStateAddress>> {
fn staking_addresses(&self, name: &str, enckey: &SecKey) -> Result<Vec<StakedStateAddress>> {
self.wallet_service.staking_addresses(name, enckey)
}

#[inline]
fn transfer_addresses(&self, name: &str, enckey: &SecKey) -> Result<BTreeSet<ExtendedAddr>> {
fn transfer_addresses(&self, name: &str, enckey: &SecKey) -> Result<Vec<ExtendedAddr>> {
self.wallet_service.transfer_addresses(name, enckey)
}

Expand Down
3 changes: 1 addition & 2 deletions client-core/src/wallet/syncer.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
#![allow(missing_docs)]
use itertools::{izip, Itertools};
use non_empty_vec::NonEmpty;
use std::collections::BTreeSet;
use std::sync::mpsc::Sender;

use chain_core::common::H256;
Expand Down Expand Up @@ -531,7 +530,7 @@ impl FilteredBlock {

fn filter_staking_transactions(
block_results: &BlockResults,
staking_addresses: &BTreeSet<StakedStateAddress>,
staking_addresses: &[StakedStateAddress],
block: &Block,
) -> Result<Vec<Transaction>> {
for staking_address in staking_addresses {
Expand Down
8 changes: 4 additions & 4 deletions client-rpc/src/rpc/wallet_rpc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,10 +74,10 @@ pub trait WalletRpc: Send + Sync {
fn list_public_keys(&self, request: WalletRequest) -> Result<BTreeSet<PublicKey>>;

#[rpc(name = "wallet_listStakingAddresses")]
fn list_staking_addresses(&self, request: WalletRequest) -> Result<BTreeSet<String>>;
fn list_staking_addresses(&self, request: WalletRequest) -> Result<Vec<String>>;

#[rpc(name = "wallet_listTransferAddresses")]
fn list_transfer_addresses(&self, request: WalletRequest) -> Result<BTreeSet<String>>;
fn list_transfer_addresses(&self, request: WalletRequest) -> Result<Vec<String>>;

#[rpc(name = "wallet_listUTxO")]
fn list_utxo(&self, request: WalletRequest) -> Result<UnspentTransactions>;
Expand Down Expand Up @@ -275,14 +275,14 @@ where
.map_err(to_rpc_error)
}

fn list_staking_addresses(&self, request: WalletRequest) -> Result<BTreeSet<String>> {
fn list_staking_addresses(&self, request: WalletRequest) -> Result<Vec<String>> {
self.client
.staking_addresses(&request.name, &request.enckey)
.map(|addresses| addresses.iter().map(ToString::to_string).collect())
.map_err(to_rpc_error)
}

fn list_transfer_addresses(&self, request: WalletRequest) -> Result<BTreeSet<String>> {
fn list_transfer_addresses(&self, request: WalletRequest) -> Result<Vec<String>> {
self.client
.transfer_addresses(&request.name, &request.enckey)
.map(|addresses| addresses.iter().map(ToString::to_string).collect())
Expand Down

0 comments on commit cb0a155

Please sign in to comment.