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

Update wallet.md #1174

Merged
merged 11 commits into from
Apr 5, 2022
Merged

Update wallet.md #1174

merged 11 commits into from
Apr 5, 2022

Conversation

Clay-Mysten
Copy link
Contributor

Committing first round of edits to Wallet
Making many, many small enhancements for clarity

Committing first round of edits to Wallet
Making many, many small enhancements for clarity
Add back dollar signs, split output from commands
Reinstate original genesis customization verbiage
Move client_db TODO up for visibility as we call for that file later
```

All of this is contained in configuration and keystore files, as well
as an `authorities_db` database file.
TODO: Why do I no longer see a `client_db` file created, only `authorities_db` now?
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't have an answer for this but wanted to make sure we do not land this change with a TODO in it (@patrickkuo ?)

Copy link
Contributor

Choose a reason for hiding this comment

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

client_db folder is created by the wallet when it's first started

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you mean when we run this?
wallet --no-shell new-address

Because simply starting wallet doesn't do it.

Copy link
Contributor Author

@Clay-Mysten Clay-Mysten Apr 4, 2022

Choose a reason for hiding this comment

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

Also, that command assumes the files live in my home dir:

Error: Cannot open wallet config file at "/Users/claymurphy/.sui/sui_config/wallet.conf"

So I have to run:

claymurphy@Clays-MacBook-Pro tmp % wallet --config wallet.conf --no-shell new-address

@awelc how is this easier than having someone manually mkdir the directory of their choosing for all of this?

Copy link
Contributor Author

@Clay-Mysten Clay-Mysten Apr 4, 2022

Choose a reason for hiding this comment

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

Ah, wallet --config wallet.conf --no-shell new-address run in tmp/ where I have the config files results in a new tmp/client_db path. See:

claymurphy@Clays-MacBook-Pro tmp % ls -lah
total 24
drwxr-xr-x   7 claymurphy  staff   224B Apr  4 08:31 .
drwxr-x---+ 29 claymurphy  staff   928B Mar 31 14:43 ..
drwxr-xr-x   6 claymurphy  staff   192B Mar 31 14:43 authorities_db
-rw-r--r--   1 claymurphy  staff   1.2K Mar 31 14:43 network.conf
drwxr-xr-x   3 claymurphy  staff    96B Apr  4 08:31 tmp
-rw-r--r--   1 claymurphy  staff   1.3K Apr  4 08:34 wallet.conf
-rw-r--r--   1 claymurphy  staff   472B Mar 31 14:43 wallet.key
claymurphy@Clays-MacBook-Pro tmp % ls -lah tmp 
total 0
drwxr-xr-x   3 claymurphy  staff    96B Apr  4 08:31 .
drwxr-xr-x   7 claymurphy  staff   224B Apr  4 08:31 ..
drwxr-xr-x  13 claymurphy  staff   416B Apr  4 08:34 client_db
claymurphy@Clays-MacBook-Pro tmp % cd tmp 
claymurphy@Clays-MacBook-Pro tmp % ls
client_db
claymurphy@Clays-MacBook-Pro tmp % pwd
/Users/claymurphy/tmp/tmp

Is this intended? Do we specify the wallet CLI must be run from a particular directory? I seem to recall that in earlier versions but not in the current version.

Copy link
Contributor

Choose a reason for hiding this comment

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

I was not aware of this. I would suggest having client_db's default location to be ~/.sui/sui_config/ as well exactly for the the reason @Clay-Mysten mentions - we don't want to force a specific directory creation, but we may want to allow it. We could also make the directory for wallet to be ~/.sui/wallet_config and have both wallet.conf, wallet.key and client_db to go there by default. @patrickkuo?

Copy link
Contributor

Choose a reason for hiding this comment

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

the wallet will write the client_db to the same dir as the wallet.conf , it will cause problem if user starts 2 instances of wallet connecting to 2 different network if we hardcode the client_db to ~/.sui/wallet_config

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@patrickkuo But in my case, I ran the wallet new-address command in my tmp' directory where wallet.conflives, specifying it with the--config` flag, only to have a path of:

tmp/tmp/client_db

That doesn't seem right.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See my input and output above.

claymurphy@Clays-MacBook-Pro tmp % ps -ef | grep -i sui
501 442 1 0 12:02PM ?? 0:00.22 /System/Library/CoreServices/CoreServicesUIAgent.app/Contents/MacOS/CoreServicesUIAgent
501 623 1 0 12:03PM ?? 0:01.10 /System/Library/PrivateFrameworks/AppleMediaServicesUI.framework/amsengagementd
501 1433 1 0 1:01PM ?? 0:00.21 /System/Library/PrivateFrameworks/PaperKit.framework/Contents/LinkedNotesUIService.app/Contents/MacOS/LinkedNotesUIService
Copy link
Contributor

Choose a reason for hiding this comment

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

I am fishing here - REST API server running locally?

Copy link
Contributor

Choose a reason for hiding this comment

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

you can check which command is using the port using lsof -i tcp:10000

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good to know. I blew everything away and started over to get this working. Can we generate a more useful error message here?

Clay-Mysten and others added 4 commits April 4, 2022 08:37
Co-authored-by: Adam Welc <adam@mystenlabs.com>
Co-authored-by: Adam Welc <adam@mystenlabs.com>
Co-authored-by: Adam Welc <adam@mystenlabs.com>
@Clay-Mysten Clay-Mysten requested a review from awelc April 4, 2022 15:45
Remove example directory per Adam
Remove TODOs, note `client_db` creation.
@Clay-Mysten Clay-Mysten merged commit 7c76bf0 into main Apr 5, 2022
@Clay-Mysten Clay-Mysten deleted the Clay-Mysten-patch-15 branch April 5, 2022 18:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants