Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(hd-wallet): enable/withdraw using any account'/change/address_index #1933

Merged
merged 27 commits into from
Aug 31, 2023
Merged
Show file tree
Hide file tree
Changes from 25 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
8963ff3
choose path from account to address when enabling a coin in hd mode
shamardy Jul 19, 2023
6dfe271
fix eth_coin_from_conf_and_request path_to_address
shamardy Jul 19, 2023
9e5d92e
Merge remote-tracking branch 'origin/dev' into fix-hd-acc-activation
shamardy Jul 19, 2023
8ac79d7
add more test cases for /account'/change/address_index, use enable_hd…
shamardy Jul 19, 2023
5581805
remove unneeded todos after checking them
shamardy Jul 19, 2023
abaf8a5
wip: withdraw from any hd /account'/change/address_index for utxo done
shamardy Jul 21, 2023
77f32b1
test withdraw from any hd address for segwit
shamardy Jul 21, 2023
b66cf69
withdraw from any hd /account'/change/address_index for EVM
shamardy Jul 27, 2023
40b5baa
fix wasm-lint
shamardy Jul 27, 2023
28e0fc4
fix wasm build
shamardy Jul 27, 2023
e94dcb5
wip: withdraw from any hd /account'/change/address_index for tendermint
shamardy Jul 28, 2023
0db8ba4
refactor: add bip39_secp_priv_key_or_err where it was missing and use it
shamardy Jul 28, 2023
7a95512
refactor: make derivation_path part of priv_key_policy
shamardy Jul 28, 2023
b7d4e44
refactor: use a common PrivKeyPolicy enum for all coins
shamardy Jul 28, 2023
5143cfc
refactor: create hd_wallet_derived_priv_key_or_err method for PrivKey…
shamardy Jul 28, 2023
80a0b6d
refactor: remove duplicated tendermint methods that I created
shamardy Jul 29, 2023
53d5e74
remove todos unrelated to this PR
shamardy Aug 4, 2023
9265cfa
Merge remote-tracking branch 'origin/dev' into fix-hd-acc-activation
shamardy Aug 4, 2023
8016af1
HD withdraw for tendermint token and IBC, add some tendermint coin wi…
shamardy Aug 4, 2023
afd268e
refactor: remove path_to_address from UtxoCoinConf
shamardy Aug 4, 2023
b6a465b
refactor: StandardUtxoWithdraw::new()
shamardy Aug 4, 2023
2346095
fix get_my_address for hd addresses
shamardy Aug 11, 2023
e34f75f
review fixes: use account only for zcoin, return error if from is use…
shamardy Aug 14, 2023
30b237a
review fixes: fix wasm WalletDbShared::new by adding account
shamardy Aug 15, 2023
fc444c6
review fixes: fix visibility issues, other refactors
shamardy Aug 15, 2023
5324bca
review fixes: add display for internal error variants
shamardy Aug 16, 2023
afe2e08
Merge remote-tracking branch 'origin/dev' into fix-hd-acc-activation
shamardy Aug 23, 2023
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
19 changes: 11 additions & 8 deletions mm2src/adex_cli/src/scenarios/init_mm2_cfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ struct Mm2Cfg {
#[serde(skip_serializing_if = "Vec::<Ipv4Addr>::is_empty")]
seednodes: Vec<Ipv4Addr>,
#[serde(skip_serializing_if = "Option::is_none")]
hd_account_id: Option<u64>,
enable_hd: Option<bool>,
rozhkovdmitrii marked this conversation as resolved.
Show resolved Hide resolved
}

impl Mm2Cfg {
Expand All @@ -68,7 +68,7 @@ impl Mm2Cfg {
rpc_local_only: None,
i_am_seed: None,
seednodes: Vec::<Ipv4Addr>::new(),
hd_account_id: None,
enable_hd: None,
}
}

Expand All @@ -84,7 +84,7 @@ impl Mm2Cfg {
self.inquire_rpc_local_only()?;
self.inquire_i_am_a_seed()?;
self.inquire_seednodes()?;
self.inquire_hd_account_id()?;
self.inquire_enable_hd()?;
Ok(())
}

Expand Down Expand Up @@ -311,13 +311,16 @@ impl Mm2Cfg {
}

#[inline]
fn inquire_hd_account_id(&mut self) -> Result<()> {
self.hd_account_id = CustomType::<InquireOption<u64>>::new("What is hd_account_id:")
.with_help_message(r#"Optional. If this value is set, the AtomicDEX-API will work in only the HD derivation mode, coins will need to have a coin derivation path entry in the coins file for activation. The hd_account_id value effectively takes its place in the full derivation as follows: m/44'/COIN_ID'/<hd_account_id>'/CHAIN/ADDRESS_ID"#)
.with_placeholder(DEFAULT_OPTION_PLACEHOLDER)
fn inquire_enable_hd(&mut self) -> Result<()> {
self.enable_hd = CustomType::<InquireOption<bool>>::new("What is enable_hd:")
.with_parser(OPTION_BOOL_PARSER)
.with_formatter(DEFAULT_OPTION_BOOL_FORMATTER)
.with_default_value_formatter(DEFAULT_DEFAULT_OPTION_BOOL_FORMATTER)
.with_default(InquireOption::None)
.with_help_message(r#"Optional. If this value is set, the Komodo DeFi API will work in HD wallet mode only, coins will need to have a coin derivation path entry in the coins file for activation. path_to_address `/account'/change/address_index` will have to be set in coins activation to change the default HD wallet address that is used in swaps for a coin in the full derivation path as follows: m/purpose'/coin_type/account'/change/address_index"#)
.prompt()
.map_err(|error|
error_anyhow!("Failed to get hd_account_id: {}", error)
error_anyhow!("Failed to get enable_hd: {}", error)
)?
.into();
Ok(())
Expand Down
144 changes: 86 additions & 58 deletions mm2src/coins/eth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ use common::{get_utc_timestamp, now_sec, small_rng, DEX_FEE_ADDR_RAW_PUBKEY};
#[cfg(target_arch = "wasm32")]
use common::{now_ms, wait_until_ms};
use crypto::privkey::key_pair_from_secret;
use crypto::{CryptoCtx, CryptoCtxError, GlobalHDAccountArc, KeyPairPolicy};
use crypto::{CryptoCtx, CryptoCtxError, GlobalHDAccountArc, KeyPairPolicy, StandardHDCoinAddress};
use derive_more::Display;
use enum_from::EnumFromStringify;
use ethabi::{Contract, Function, Token};
Expand Down Expand Up @@ -110,6 +110,7 @@ use crate::nft::{find_wallet_nft_amount, WithdrawNftResult};
use v2_activation::{build_address_and_priv_key_policy, EthActivationV2Error};

mod nonce;
use crate::{PrivKeyPolicy, WithdrawFrom};
use nonce::ParityNonce;

/// https://github.com/artemii235/etomic-swap/blob/master/contracts/EtomicSwap.sol
Expand Down Expand Up @@ -170,6 +171,7 @@ lazy_static! {
pub type Web3RpcFut<T> = Box<dyn Future<Item = T, Error = MmError<Web3RpcError>> + Send>;
pub type Web3RpcResult<T> = Result<T, MmError<Web3RpcError>>;
pub type GasStationResult = Result<GasStationData, MmError<GasStationReqErr>>;
type EthPrivKeyPolicy = PrivKeyPolicy<KeyPair>;
type GasDetails = (U256, U256);

#[derive(Debug, Display)]
Expand Down Expand Up @@ -409,35 +411,6 @@ impl TryFrom<PrivKeyBuildPolicy> for EthPrivKeyBuildPolicy {
}
}

/// An alternative to `crate::PrivKeyPolicy`, typical only for ETH coin.
#[derive(Clone)]
pub enum EthPrivKeyPolicy {
KeyPair(KeyPair),
#[cfg(target_arch = "wasm32")]
Metamask(EthMetamaskPolicy),
}

#[cfg(target_arch = "wasm32")]
#[derive(Clone)]
pub struct EthMetamaskPolicy {
pub(crate) public_key: H264,
pub(crate) public_key_uncompressed: H520,
}

impl From<KeyPair> for EthPrivKeyPolicy {
fn from(key_pair: KeyPair) -> Self { EthPrivKeyPolicy::KeyPair(key_pair) }
}

impl EthPrivKeyPolicy {
pub fn key_pair_or_err(&self) -> MmResult<&KeyPair, PrivKeyPolicyNotAllowed> {
match self {
EthPrivKeyPolicy::KeyPair(key_pair) => Ok(key_pair),
#[cfg(target_arch = "wasm32")]
EthPrivKeyPolicy::Metamask(_) => MmError::err(PrivKeyPolicyNotAllowed::HardwareWalletNotSupported),
}
}
}

/// pImpl idiom.
pub struct EthCoinImpl {
rozhkovdmitrii marked this conversation as resolved.
Show resolved Hide resolved
ticker: String,
Expand Down Expand Up @@ -735,7 +708,28 @@ async fn withdraw_impl(coin: EthCoin, req: WithdrawRequest) -> WithdrawResult {
let to_addr = coin
.address_from_str(&req.to)
.map_to_mm(WithdrawError::InvalidAddress)?;
let my_balance = coin.my_balance().compat().await?;
let (my_balance, my_address, key_pair) = match req.from {
Some(WithdrawFrom::HDWalletAddress(ref path_to_address)) => {
let raw_priv_key = coin
.priv_key_policy
.hd_wallet_derived_priv_key_or_err(path_to_address)?;
let key_pair = KeyPair::from_secret_slice(raw_priv_key.as_slice())
.map_to_mm(|e| WithdrawError::InternalError(e.to_string()))?;
let address = key_pair.address();
let balance = coin.address_balance(address).compat().await?;
(balance, address, key_pair)
},
Some(WithdrawFrom::AddressId(_)) | Some(WithdrawFrom::DerivationPath { .. }) => {
return MmError::err(WithdrawError::UnexpectedFromAddress(
"Withdraw from 'AddressId' or 'DerivationPath' is not supported yet for EVM!".to_string(),
))
},
None => (
coin.my_balance().compat().await?,
coin.my_address,
coin.priv_key_policy.activated_key_or_err()?.clone(),
),
};
let my_balance_dec = u256_to_big_decimal(my_balance, coin.decimals)?;

let (mut wei_amount, dec_amount) = if req.max {
Expand Down Expand Up @@ -778,9 +772,10 @@ async fn withdraw_impl(coin: EthCoin, req: WithdrawRequest) -> WithdrawResult {
};

let (tx_hash, tx_hex) = match coin.priv_key_policy {
EthPrivKeyPolicy::KeyPair(ref key_pair) => {
EthPrivKeyPolicy::Iguana(_) | EthPrivKeyPolicy::HDWallet { .. } => {
// Todo: nonce_lock is still global for all addresses but this needs to be per address
rozhkovdmitrii marked this conversation as resolved.
Show resolved Hide resolved
let _nonce_lock = coin.nonce_lock.lock().await;
let (nonce, _) = get_addr_nonce(coin.my_address, coin.web3_instances.clone())
let (nonce, _) = get_addr_nonce(my_address, coin.web3_instances.clone())
.compat()
.timeout_secs(30.)
.await?
Expand All @@ -800,6 +795,11 @@ async fn withdraw_impl(coin: EthCoin, req: WithdrawRequest) -> WithdrawResult {

(signed.hash, BytesJson::from(bytes.to_vec()))
},
EthPrivKeyPolicy::Trezor => {
return MmError::err(WithdrawError::UnsupportedError(
"Trezor is not supported for EVM yet!".to_string(),
))
},
#[cfg(target_arch = "wasm32")]
EthPrivKeyPolicy::Metamask(_) => {
if !req.broadcast {
Expand Down Expand Up @@ -842,7 +842,7 @@ async fn withdraw_impl(coin: EthCoin, req: WithdrawRequest) -> WithdrawResult {

let amount_decimal = u256_to_big_decimal(wei_amount, coin.decimals)?;
let mut spent_by_me = amount_decimal.clone();
let received_by_me = if to_addr == coin.my_address {
let received_by_me = if to_addr == my_address {
amount_decimal.clone()
} else {
0.into()
Expand All @@ -851,10 +851,9 @@ async fn withdraw_impl(coin: EthCoin, req: WithdrawRequest) -> WithdrawResult {
if coin.coin_type == EthCoinType::Eth {
spent_by_me += &fee_details.total_fee;
}
let my_address = coin.my_address()?;
Ok(TransactionDetails {
to: vec![checksum_address(&format!("{:#02x}", to_addr))],
from: vec![my_address],
from: vec![checksum_address(&format!("{:#02x}", my_address))],
total_amount: amount_decimal,
my_balance_change: &received_by_me - &spent_by_me,
spent_by_me,
Expand Down Expand Up @@ -951,7 +950,7 @@ pub async fn withdraw_erc1155(ctx: MmArc, withdraw_type: WithdrawErc1155) -> Wit
gas_price,
};

let secret = eth_coin.priv_key_policy.key_pair_or_err()?.secret();
let secret = eth_coin.priv_key_policy.activated_key_or_err()?.secret();
let signed = tx.sign(secret, eth_coin.chain_id);
let signed_bytes = rlp::encode(&signed);
let fee_details = EthTxFeeDetails::new(gas, gas_price, fee_coin)?;
Expand Down Expand Up @@ -1026,7 +1025,7 @@ pub async fn withdraw_erc721(ctx: MmArc, withdraw_type: WithdrawErc721) -> Withd
gas_price,
};

let secret = eth_coin.priv_key_policy.key_pair_or_err()?.secret();
let secret = eth_coin.priv_key_policy.activated_key_or_err()?.secret();
let signed = tx.sign(secret, eth_coin.chain_id);
let signed_bytes = rlp::encode(&signed);
let fee_details = EthTxFeeDetails::new(gas, gas_price, fee_coin)?;
Expand Down Expand Up @@ -1306,9 +1305,12 @@ impl SwapOps for EthCoin {
#[inline]
fn derive_htlc_key_pair(&self, _swap_unique_data: &[u8]) -> keys::KeyPair {
match self.priv_key_policy {
EthPrivKeyPolicy::KeyPair(ref key_pair) => {
key_pair_from_secret(key_pair.secret().as_bytes()).expect("valid key")
},
EthPrivKeyPolicy::Iguana(ref key_pair)
| EthPrivKeyPolicy::HDWallet {
activated_key: ref key_pair,
..
} => key_pair_from_secret(key_pair.secret().as_bytes()).expect("valid key"),
EthPrivKeyPolicy::Trezor => todo!(),
rozhkovdmitrii marked this conversation as resolved.
Show resolved Hide resolved
#[cfg(target_arch = "wasm32")]
EthPrivKeyPolicy::Metamask(_) => todo!(),
}
Expand All @@ -1317,10 +1319,15 @@ impl SwapOps for EthCoin {
#[inline]
fn derive_htlc_pubkey(&self, _swap_unique_data: &[u8]) -> Vec<u8> {
match self.priv_key_policy {
EthPrivKeyPolicy::KeyPair(ref key_pair) => key_pair_from_secret(key_pair.secret().as_bytes())
EthPrivKeyPolicy::Iguana(ref key_pair)
| EthPrivKeyPolicy::HDWallet {
activated_key: ref key_pair,
..
} => key_pair_from_secret(key_pair.secret().as_bytes())
.expect("valid key")
.public_slice()
.to_vec(),
EthPrivKeyPolicy::Trezor => todo!(),
rozhkovdmitrii marked this conversation as resolved.
Show resolved Hide resolved
#[cfg(target_arch = "wasm32")]
EthPrivKeyPolicy::Metamask(ref metamask_policy) => metamask_policy.public_key.as_bytes().to_vec(),
}
Expand Down Expand Up @@ -1810,10 +1817,15 @@ impl MarketCoinOps for EthCoin {

fn get_public_key(&self) -> Result<String, MmError<UnexpectedDerivationMethod>> {
match self.priv_key_policy {
EthPrivKeyPolicy::KeyPair(ref key_pair) => {
EthPrivKeyPolicy::Iguana(ref key_pair)
| EthPrivKeyPolicy::HDWallet {
activated_key: ref key_pair,
..
} => {
let uncompressed_without_prefix = hex::encode(key_pair.public());
Ok(format!("04{}", uncompressed_without_prefix))
},
EthPrivKeyPolicy::Trezor => MmError::err(UnexpectedDerivationMethod::Trezor),
#[cfg(target_arch = "wasm32")]
EthPrivKeyPolicy::Metamask(ref metamask_policy) => {
Ok(format!("{:02x}", metamask_policy.public_key_uncompressed))
Expand All @@ -1837,7 +1849,7 @@ impl MarketCoinOps for EthCoin {

fn sign_message(&self, message: &str) -> SignatureResult<String> {
let message_hash = self.sign_message_hash(message).ok_or(SignatureError::PrefixNotFound)?;
let privkey = &self.priv_key_policy.key_pair_or_err()?.secret();
let privkey = &self.priv_key_policy.activated_key_or_err()?.secret();
let signature = sign(privkey, &H256::from(message_hash))?;
Ok(format!("0x{}", signature))
}
Expand Down Expand Up @@ -2108,7 +2120,12 @@ impl MarketCoinOps for EthCoin {

fn display_priv_key(&self) -> Result<String, String> {
match self.priv_key_policy {
EthPrivKeyPolicy::KeyPair(ref key_pair) => Ok(format!("{:#02x}", key_pair.secret())),
EthPrivKeyPolicy::Iguana(ref key_pair)
Copy link
Member

Choose a reason for hiding this comment

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

Just an idea for the future: it might be worth adding derivation_path/address argument to the display_priv_key RPC.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks for the idea, a way to import/export private keys is planned for the future so this is the way to achieve the exporting part. Added it to a checklist #1838 (comment)

It should be allowed to specify which level of the derivation path to export/display a private key for too. Exporting public key/s should be added too.

| EthPrivKeyPolicy::HDWallet {
activated_key: ref key_pair,
..
} => Ok(format!("{:#02x}", key_pair.secret())),
EthPrivKeyPolicy::Trezor => ERR!("'display_priv_key' doesn't support Trezor yet!"),
#[cfg(target_arch = "wasm32")]
EthPrivKeyPolicy::Metamask(_) => ERR!("'display_priv_key' doesn't support MetaMask"),
}
Expand Down Expand Up @@ -2973,9 +2990,12 @@ impl EthCoin {
let coin = self.clone();
let fut = async move {
match coin.priv_key_policy {
EthPrivKeyPolicy::KeyPair(ref key_pair) => {
sign_and_send_transaction_with_keypair(ctx, &coin, key_pair, value, action, data, gas).await
},
EthPrivKeyPolicy::Iguana(ref key_pair)
| EthPrivKeyPolicy::HDWallet {
activated_key: ref key_pair,
..
} => sign_and_send_transaction_with_keypair(ctx, &coin, key_pair, value, action, data, gas).await,
EthPrivKeyPolicy::Trezor => Err(TransactionErr::Plain(ERRL!("Trezor is not supported for EVM yet!"))),
#[cfg(target_arch = "wasm32")]
EthPrivKeyPolicy::Metamask(_) => {
sign_and_send_transaction_with_metamask(coin, value, action, data, gas).await
Expand Down Expand Up @@ -3612,18 +3632,14 @@ impl EthCoin {
}
}

fn my_balance(&self) -> BalanceFut<U256> {
fn address_balance(&self, address: Address) -> BalanceFut<U256> {
let coin = self.clone();
let fut = async move {
match coin.coin_type {
EthCoinType::Eth => Ok(coin
.web3
.eth()
.balance(coin.my_address, Some(BlockNumber::Latest))
.await?),
EthCoinType::Eth => Ok(coin.web3.eth().balance(address, Some(BlockNumber::Latest)).await?),
EthCoinType::Erc20 { ref token_addr, .. } => {
let function = ERC20_CONTRACT.function("balanceOf")?;
let data = function.encode_input(&[Token::Address(coin.my_address)])?;
let data = function.encode_input(&[Token::Address(address)])?;

let res = coin.call_request(*token_addr, None, Some(data.into())).await?;
let decoded = function.decode_output(&res.0)?;
Expand All @@ -3640,6 +3656,8 @@ impl EthCoin {
Box::new(fut.boxed().compat())
}

fn my_balance(&self) -> BalanceFut<U256> { self.address_balance(self.my_address) }

pub async fn get_tokens_balance_list(&self) -> Result<HashMap<String, CoinBalance>, MmError<BalanceError>> {
let coin = || self;
let mut requests = Vec::new();
Expand Down Expand Up @@ -5139,7 +5157,12 @@ pub async fn eth_coin_from_conf_and_request(
}
let contract_supports_watchers = req["contract_supports_watchers"].as_bool().unwrap_or_default();

let (my_address, key_pair) = try_s!(build_address_and_priv_key_policy(conf, priv_key_policy).await);
let path_to_address = try_s!(json::from_value::<Option<StandardHDCoinAddress>>(
req["path_to_address"].clone()
))
.unwrap_or_default();
rozhkovdmitrii marked this conversation as resolved.
Show resolved Hide resolved
let (my_address, key_pair) =
try_s!(build_address_and_priv_key_policy(conf, priv_key_policy, &path_to_address).await);

let mut web3_instances = vec![];
let event_handlers = rpc_event_handlers_for_eth_transport(ctx, ticker.to_string());
Expand Down Expand Up @@ -5399,12 +5422,17 @@ impl From<CryptoCtxError> for GetEthAddressError {

/// `get_eth_address` returns wallet address for coin with `ETH` protocol type.
/// Note: result address has mixed-case checksum form.
pub async fn get_eth_address(ctx: &MmArc, ticker: &str) -> MmResult<MyWalletAddress, GetEthAddressError> {
pub async fn get_eth_address(
ctx: &MmArc,
conf: &Json,
ticker: &str,
path_to_address: &StandardHDCoinAddress,
) -> MmResult<MyWalletAddress, GetEthAddressError> {
let priv_key_policy = PrivKeyBuildPolicy::detect_priv_key_policy(ctx)?;
// Convert `PrivKeyBuildPolicy` to `EthPrivKeyBuildPolicy` if it's possible.
let priv_key_policy = EthPrivKeyBuildPolicy::try_from(priv_key_policy)?;

let (my_address, ..) = build_address_and_priv_key_policy(&ctx.conf, priv_key_policy).await?;
let (my_address, ..) = build_address_and_priv_key_policy(conf, priv_key_policy, path_to_address).await?;
let wallet_address = checksum_address(&format!("{:#02x}", my_address));

Ok(MyWalletAddress {
Expand Down
Loading
Loading