feat: cast ls + cast accounts #656

tynes · 2022-02-02T05:44:40Z

Motivation

I'd like to use cast for sending transactions with a key on my hardware wallet.
Implementing cast ls helps with making sure that the correct hardware wallet
is plugged in.

cast ls should list off the available wallets that can be used for signing. This
includes the local keystore (geth account list), plugged in ledgers and plugged
in trezors. seth can also support the RPC endpoint eth_accounts
when the env var ETH_RPC_ACCOUNTS is set. I don't see an ethers remote signer
that uses RPC for signing so its probably early for that
functionality.

This PR doesn't yet support trezor as I don't have a trezor wallet to test out, but it
should be easy to add. It also needs sane default keystore locations based on the
OS.

Should the config options for ethsign ls be made available to cast ls? They aren't with seth ls,
unless you use env vars
I was looking into using https://github.com/roynalnaruto/eth-keystore-rs for parsing the keystores
but it doesn't support pulling the address field out of the keystore JSON. This field was removed
from the keystore spec as to preserve privacy

Solution

Implement cast ls which prints out a list of
addresses and their balances. The seth implementation
is based on ethsign, see:

mattsse

thanks for this,
looking good, but a couple of nits and suggestions

mattsse · 2022-02-02T11:22:11Z

cast/src/lib.rs

+        let chain_id = self.chain_id().await?;
+
+        // for each file in $HOME/.ethereum/keystore
+        if let Ok(home_dir) = std::env::var("HOME") {


there's the dirs-next crate that provides a helper for finding the home dir
https://crates.io/crates/dirs-next

We use that in the foundry_config crate as well

mattsse · 2022-02-02T11:23:38Z

cast/src/lib.rs

+        // for each file in $HOME/.ethereum/keystore
+        if let Ok(home_dir) = std::env::var("HOME") {
+             let path = std::path::Path::new(&home_dir).join(".ethereum").join("keystore");
+             let entries = fs::read_dir(path)?;


we want a better error message here as io errors don't provide info about the dir path of a dir that does not exist.

mattsse · 2022-02-02T11:29:46Z

cast/src/lib.rs

+                 let map = value.as_object_mut().unwrap();
+                 let addr = map.get_mut("address").unwrap().as_str().unwrap();
+                 let t = H160::from_str(addr).unwrap();


while it is (perhaps) unlikely that this fails, these unwraps here are a bit invasive.
alternatively, you can simply define a helper struct directly in this function like:

#[derive(Deserialize) struct Entry { address: Address }

mattsse · 2022-02-02T11:30:30Z

cast/src/lib.rs

+                }
+
+            }
+            Err(_) => {}


should we print something here?

seth doesn't print anything in this case, if its assumed that the output is safe to be consumed from a script then I don't think printing anything makes sense. I do think this command could use a -v flag

mattsse · 2022-02-02T11:32:25Z

cast/src/lib.rs

+        let mut s = String::new();
+        for (i, balance) in balances.iter().enumerate() {
+            let addr = addrs[i];
+            let result = match balance {
+                Ok(balance) => format!("{:?}\t{}\n", addr, balance),
+                Err(_) => format!("{:?}\t{}\n", addr, 0),
+            };
+            s.push_str(&result);
+        }


I would rather like the function to return the pairs of (Address, Result)
and do we do this formatting part outside, when printing to stdout, basically in the Subcommand::Ls

joshieDo · 2022-02-03T18:20:36Z

Glancing through, trezor should work pretty much the same as ledger here. I can test it out after the PR is closer to done.

What do you think of showing the derivation path if dealing with hardware wallets?

Edit: plus some labels to know what comes from where ( keystore vs ledger vs trezor)

tynes · 2022-02-04T02:20:15Z

Glancing through, trezor should work pretty much the same as ledger here. I can test it out after the PR is closer to done.

What do you think of showing the derivation path if dealing with hardware wallets?

Edit: plus some labels to know what comes from where ( keystore vs ledger vs trezor)

I'm into this idea, thinking of breaking out the code that finds accounts into another command cast accounts (seth supports this, see below) and reusing that command from within cast ls

$ seth accounts
0x27770a9694e4B4b1E130Ab91Bc327C36855f612E keystore
0x47baC1Afc3Ef96aAb8209c63b38B9AaAAe4EBDC4 keystore
0x00000398232E2064F896018496b4b44b3D62751F keystore
0xB79f76EF2c5F0286176833E7B2eEe103b1CC3244 ledger-m/44'/60'/0'/0/0
0x422F94147B13e7fd435981c003a83ba4a7c5B20D ledger-m/44'/60'/1'/0/0
0xde768894c145871746BAd479c133403a379780e7 ledger-m/44'/60'/2'/0/0
0x508885e9ce33A83784c5569f6a1BF09c66e4deD5 ledger-m/44'/60'/3'/0/0
0x9cc889680F5fBC0Ee0FbA8A68866C27acC599b35 ledger-m/44'/60'/4'/0/0
0x0245eDdb15eA32D89f04C564da9aa31A29502e65 ledger-m/44'/60'/5'/0/0
0xc8f19745d6301D21dfA0eeEA8006a1fBf993837b ledger-m/44'/60'/0'/0
0x84f70449f90300997840eCb0918873745Ede7aE6 ledger-m/44'/60'/0'/1
0x27770a9694e4B4b1E130Ab91Bc327C36855f612E ledger-m/44'/60'/0'/2
0x9DD1Ec758E49b07DbAb6ef52BBb61979fEfF831A ledger-m/44'/60'/0'/3
0x911537a2da8fE765350383b35118758F37047776 ledger-m/44'/60'/0'/4
0x7C9DbFC58af7b5538Ef30601d219D9f79c050625 ledger-m/44'/60'/0'/5

gakonst · 2022-02-04T08:36:09Z

ETH_RPC_ACCOUNTS works by querying the eth_accounts endpoint, ethersrs supports that, so I'd add that.

As a follow up:Wdyt about also making this work with a mnemonic, and also showing the eth/erc20 balances for the accounts? Almost like a quick overview/summary of all accs

tynes · 2022-02-05T05:07:36Z

cast/src/lib.rs

+            }
+        }
+
+        // Check the keystore


This should be able to take an argument similar to geth --datadir and fall back to the default keystore if it is not passed. The default keystore is OS dependent

tynes · 2022-02-05T05:09:47Z

utils/src/lib.rs

+
+    Some(path)
+}
+


This get_account function should be renamed or refactored when trezor support is added

tynes · 2022-02-05T05:11:26Z

utils/src/lib.rs

+        "windows" => {
+            if let Ok(local_app_data) = std::env::var("LOCALAPPDATA") {
+                path = std::path::Path::new(&local_app_data).join("Ethereum");
+            }


geth panics if LOCALAPPDATA is not defined, meaning that its running on windows xp. This needs to be updated with logic for when its not defined so that the default keystore path isn't the windows equivalent of $HOME/keystore

tynes · 2022-02-05T05:20:56Z

ETH_RPC_ACCOUNTS works by querying the eth_accounts endpoint, ethersrs supports that, so I'd add that.

As a follow up:Wdyt about also making this work with a mnemonic, and also showing the eth/erc20 balances for the accounts? Almost like a quick overview/summary of all accs

I've added the ability to query remote accounts using eth_accounts, they will show up as being remote accounts when doing cast accounts

I can definitely add flags to make this work with a mnemonic, thinking --mnemonic-path for sure and then either --mnemonic-index/--hd-path or just derive the first 6 accounts. Could also add a flag that lets you choose the number of accounts to derive, ethsign has something similar to that.

Regarding eth/erc20 balances, it currently shows eth balance when doing cast ls. I'm happy to either break the seth compatibility or add additional functionality behind a -v flag. Personally leaning towards the -v flag so that the user can opt into the command being slower.

To print erc20 balances, I think the functionality would be to look at the chainid, then grab addresses from the address book. Any opinions on which erc20 balances are queried? Some potentials:

DAI
USDC
WETH9

gakonst · 2022-02-18T15:06:02Z

@tynes any plans to add new things in this PR, or should we mark as ready for review?

Implement `cast ls` and `cast accounts` `cast ls` prints out a list of accounts and their balances. `cast accounts` prints out a lits of accounts and their type of account. These are based on `seth`. The `seth` implementation is based on `ethsign`, see: - https://github.com/dapphub/dapptools/blob/1be7d796a468f52a5eb5b6830591d76a3b4b1c49/src/seth/libexec/seth/seth-accounts#L4 - https://github.com/dapphub/dapptools/blob/1be7d796a468f52a5eb5b6830591d76a3b4b1c49/src/ethsign/ethsign.go#L210

tynes · 2022-02-24T17:35:26Z

utils/src/lib.rs

+}
+
+#[derive(Debug)]
+pub enum CastAccountType {


This feels like its a bit redundant, I've seen similar types in ethers elsewehere

gakonst · 2022-04-18T16:37:31Z

Closing as stale, let's revisit if we prioritize #926

lightclient · 2024-01-30T14:15:57Z

I mostly use cast with hardware wallets and something like cast wallet ls for them would be extremely useful.

tynes changed the title ~~Feat/cast ls~~ feat: cast ls Feb 2, 2022

mattsse requested changes Feb 2, 2022

View reviewed changes

onbjerg added the T-feature Type: feature label Feb 2, 2022

tynes force-pushed the feat/cast-ls branch from e0fc3a8 to 936b31b Compare February 5, 2022 05:04

tynes commented Feb 5, 2022

View reviewed changes

utils/src/lib.rs

Some(path)

}

Copy link

Contributor Author

tynes Feb 5, 2022

Choose a reason for hiding this comment

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

This get_account function should be renamed or refactored when trezor support is added

tynes commented Feb 5, 2022

View reviewed changes

tynes changed the title ~~feat: cast ls~~ feat: cast ls + cast accounts Feb 5, 2022

gakonst force-pushed the master branch 2 times, most recently from dd690ab to bc613e1 Compare February 9, 2022 11:48

tynes force-pushed the feat/cast-ls branch from 936b31b to 876a893 Compare February 24, 2022 17:33

tynes commented Feb 24, 2022

View reviewed changes

fmt: cargo +nightly fmt

12aa55a

tynes marked this pull request as ready for review February 24, 2022 17:37

gakonst mentioned this pull request Apr 18, 2022

Keystore #926

Closed

gakonst closed this Apr 18, 2022

mattsse mentioned this pull request Jan 30, 2024

Include HW wallets in cast wallet ls #6958

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: cast ls + cast accounts #656

feat: cast ls + cast accounts #656

tynes commented Feb 2, 2022

mattsse left a comment

mattsse Feb 2, 2022

mattsse Feb 2, 2022 •

edited

Loading

mattsse Feb 2, 2022 •

edited

Loading

mattsse Feb 2, 2022

mattsse Feb 2, 2022

tynes Feb 4, 2022

mattsse Feb 2, 2022

joshieDo commented Feb 3, 2022 •

edited

Loading

tynes commented Feb 4, 2022

gakonst commented Feb 4, 2022 •

edited

Loading

tynes Feb 5, 2022

tynes Feb 5, 2022

tynes Feb 5, 2022

tynes commented Feb 5, 2022

gakonst commented Feb 18, 2022

tynes Feb 24, 2022

gakonst commented Apr 18, 2022

lightclient commented Jan 30, 2024

feat: cast ls + cast accounts #656

feat: cast ls + cast accounts #656

Conversation

tynes commented Feb 2, 2022

Motivation

Solution

mattsse left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mattsse Feb 2, 2022 • edited Loading

Choose a reason for hiding this comment

mattsse Feb 2, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joshieDo commented Feb 3, 2022 • edited Loading

tynes commented Feb 4, 2022

gakonst commented Feb 4, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tynes commented Feb 5, 2022

gakonst commented Feb 18, 2022

Choose a reason for hiding this comment

gakonst commented Apr 18, 2022

lightclient commented Jan 30, 2024

mattsse Feb 2, 2022 •

edited

Loading

mattsse Feb 2, 2022 •

edited

Loading

joshieDo commented Feb 3, 2022 •

edited

Loading

gakonst commented Feb 4, 2022 •

edited

Loading