stellar / stellar-cli

CLI for Soroban contracts.
64 stars 55 forks source link

`keys generate`: don't overwrite existing account #1380

Open chadoh opened 2 months ago

chadoh commented 2 months ago

What problem does your feature solve?

Right now if you have an existing account named alice and you run keys generate again, it will generate a new account, overwrite the old alice.toml file with the new seed phrase, and fund the new account.

$ cat .soroban/identity/alice.toml
───────┬────────────────────────────────────
       │ File: .soroban/identity/alice.toml
───────┼────────────────────────────────────
   1   │ seed_phrase = "zoo solid..."
───────┴────────────────────────────────────
$ soroban keys generate alice --network local
$ cat .soroban/identity/alice.toml
───────┬────────────────────────────────────
       │ File: .soroban/identity/alice.toml
───────┼────────────────────────────────────
   1   │ seed_phrase = "camp endorse..."
───────┴────────────────────────────────────

What would you like to see?

By default

Here's an example of what this could look like:

$ soroban keys generate alice --network local
alice already exists!
    seed phrase found at: /path/to/.soroban/identity/alice.toml
    current XLM balance: 231
Balance >100 XLM, nothing to do. 
To generate new keys for alice, you can `keys rm alice` first.

Or, if the account doesn't have sufficient balance:

$ soroban keys generate alice --network local
alice already exists!
    seed phrase found at: /path/to/.soroban/identity/alice.toml
    account XLM balance: 9
Topping off... Balance is now 1009 XLM.
To generate new keys for alice, you can `keys rm alice` first.

What alternatives are there?

With #1389, we're considering adding a --minimum-balance option. I think for the sake of clarity & simplicity, we do NOT add that to keys generate. But should we mention the existence of keys fund in the output here?

leighmcculloch commented 2 months ago

Friendbot doesn't support topping off. Friendbot only creates new accounts and doesn't currently have the ability to send funds to existing accounts.

Instead of adding force, we can error and require the explicit removal instead of adding new options. So the way to do a force is just a remove and generate. Wdyt?

willemneal commented 2 months ago

You can top off with friendbot. Generate a temporary account then transfer all funds to existing account. But after some reflection, since we have a keys rm it's not that hard for users to delete without needing to add another option.

chadoh commented 2 months ago

I like it, @leighmcculloch @willemneal. I've added this:

And I updated the description here to mention it and to get rid of the --force idea.

chadoh commented 2 months ago

But there's a new question: With #1389, we're considering adding a --minimum-balance option. I think for the sake of clarity & simplicity, we do NOT add that to keys generate. But should we mention the existence of keys fund --minimum-balance in the output of keys generate?

fnando commented 1 month ago

the way i usually handle topoffs is by creating a new account + funding with friendbot, then merging the account. That means the existing account will be refilled with 10K XLM every time, which is good enough. I personally wouldn't add a --minimum-balance, and instead would refill the account every time if keys fund is called on a existing account.

leighmcculloch commented 1 month ago

But should we mention the existence of keys fund --minimum-balance in the output of keys generate?

Wouldn't hurt the UX to output to stderr a message saying that the account has been funded, and then how to refund it using #1389.

Today the command and its output looks like this:

$ stellar keys generate me --network testnet

We could change it to:

$ stellar keys generate me --network testnet
✅ Generated new key for 'me'
ℹ️ Public key: GDVLD7NRK4FSZ2QXPR4Z5QOM4U2KLBUP5J5RB7DDNBSHR6RASKR6APGE
ℹ️ Secret key: hidden (use 'stellar keys show me' to view)
🌎 Funding account with public key as address on testnet...
✅ Funded (use 'stellar keys fund me' to fund again)