Add third-party fee payer during deployment

jasongitmail commented 2 years ago

To discuss further.

E.g.

As a property for a given network in config.json -- key type & path to key files if on disk?
When adding a new network to config.json (interactively &/or via CLI argument)
As an optional argument during deployment, to override any such setting within config.json?

Use case

For easier initial deployment to mainnet. It's not critical for testnet b/c it's easy to get tMINA from a faucet for testing.

jasongitmail commented 1 year ago

Needs mini-RFC

nicc commented 1 year ago

Indirectly Irresponsible Individual: @jasongitmail

Please remember to get @shimkiv's input on a testing plan for all RFCs :)

mitschabaude commented 1 year ago

RFC: Third-party fee payer

Motivation

Currently, the CLI creates a keypair for the zkApp account as part of zk config, and displays a link to the faucet to get tMINA from. When running zk deploy, the CLI assumes that this zkApp account has funds to pay fees, and uses it as fee payer. Also, the interact.ts script that is shipped as an example with the default project template assumes it can use the zkApp account as fee payer.

There are two reasons why the flow above is insufficient:

Incompatible with permissions By default, a zkApp is deployed with send: proof permissions. That means you need to provide a proof (of running a smart contract method) as authorization to send MINA away from the zkApp account. On the other hand, a fee payer tries to pay the fee using signature authorization (proofs are not supported). In other words, using a zkApp as fee payer, after deployment, violates the permissions of the account, and the transaction will get rejected. This currently breaks all re-deployments and the interact.ts script.
Unnecessary work to fund zkApp Having to fund each zkApp account before deploying, just for using it to pay fees, is cumbersome. On testnet, there is a faucet which at least allows you to fund the zkApp with a few clicks (but still makes you wait a few minutes until the transaction is included). On mainnet, it gets worse because you'd have to manually send a funding transaction from your own account. In both cases, it would save time to just have one developer account, which is funded and can always pay fees immediately.

We propose to replace fee-paying zkApps with a new default: A developer account whose keypair is cached globally on the machine and used to pay fees. The old flow of using the zkApp account still remains an option.

An important consideration is ease of use even on mainnet. The developer should be able to use an account created with one of the existing browser wallets, to reduce setup time before being able to develop zkApps.

Proposal

During zk config, we add a new step:picking the fee-paying account.

$ zk config
Add a new deploy alias:
✔ Choose a name (can be anything): · mainnet
✔ Set the Mina GraphQL API URL to deploy to: · https://berkeley.minascan.io/graphql
✔ Set transaction fee to use when deploying (in MINA): · 0.201
? Pick an account to pay transaction fees:

If this is the first time a user runs this flow, there will be 3 options:

❯ Recover fee-payer account from a base58 private key
  Create a new fee-payer key pair
  Use zkApp account as fee payer

When creating a new account, or recovering the account from a private key, there will be an additional step where we pick a name / alias for that account:

? Pick an alias for this account: _

The account (keypair + alias) will be stored globally inside an OS-specific standard cache directory such as ~/.cache/zkapp-cli on Linux.

In the step where the user can paste their base58 private key, we will add a disclaimer:

? Account private key (base58): _
NOTE: the private key will be stored in plain text on this computer.
Do NOT use an account which holds a substantial amount of MINA.

Note here that the base58 private key can be exported from Auro wallet, so the developer will be able to use an existing account:

Let's say we pick "dev" as our alias. When running zk config a second time, there will now be a new first option (plus the 3 from before):

❯ Use stored account "dev" (public key:  B62qjR6VeV4UhAnEhuhpkprovmfrYdmqZYCK72urF1FJs8XHeV6dtTq)
  Recover fee-payer account from a base58 private key
  Create a new fee-payer key pair
  Use zkApp account as fee payer

Depending on which option the user picks, we will store an entry in config.json. The current "keyPath" entry will be replaced with two different entries: "zkAppKeyPath" and "feePayerKeyPath".

"deployAliases": {
    "mainnet": {
      "url": "https://berkeley.minascan.io/graphql",
      "zkAppKeyPath": "keys/mainnet.json",
      "feePayerKeyPath": "/home/gregor/.cache/zkapp-cli/keys/dev.json"
      "fee": "0.201"
    }
  }

Out of scope

It would be nice to be able to use your Ledger device to sign transactions from the CLI. That would create the ultimate secure signing flow. However, Ledger currently doesn't support zkApp transactions so this can't be implemented now.

jasongitmail commented 1 year ago

Thanks @mitschabaude!

Let's keep the existing keyPath property name for conciseness:

"deployAliases": {
"mainnet": {
  "url": "https://berkeley.minascan.io/graphql",
  "keyPath": "...",  
  "fee": "0.201",
  "feePayerKeyPath": "..."
}
}

We can skip asking them name the fee payer key pair and default it to <deployAlias>FeePayer.json to save them a step. This is also consistent with current behavior because we do not ask them a name for their key, we simply use <deployAlias>.json.

Question:

Will we ever want to use the zkApp account as the fee payer? What's the use case? Because if the answer is 'no' or 'no need that a 3rd party fee payer can't solve', then we could just always use a 3rd-party fee payer and we simplify how to explain what's going on and we gain perfect clarity in how to describe it to them and which address is shown to them when they see the faucet step. Would be nice to simplify this for ourselves if possible and reduce options shown to the user and maybe we do that by always using a 3rd party fee payer now. Thoughts, pros/cons? (Edit: maybe there are cons are hardware wallet usage because 2 keys would be involved...hmm)

I'll double back on the UX steps in a couple days.

mitschabaude commented 1 year ago

@jasongitmail thanks for the feedback!

Keeping the existing keyPath name makes sense and makes it easier to maintain backwards-compatibility.

I'm not sold on removing the fee payer alias. The idea was that this alias was, after set once, globally usable in every zkapp project. It wouldn't be scoped to the zkapp project or the particular deployAlias. That's why we need a alias that's recongnizable to the user -- so they can pick the same fee payer account again in their next zkapp project, without having to add their private key again

ymekuria commented 1 year ago

This is great @mitschabaude! I have some questions and suggestions.

Let's keep the existing keyPath property name for conciseness:

Strong +1 to keep the existing keyPath for backwards maintainability.

@Jason while it would be great to eliminate a step and not ask a user for a key alias, I agree with @mitschabaude 's approach of having a fee payer alias that is recognizable and not scoped to any project.

We still might want to include flows for linking to the faucet from the cli to fund accounts(on testnet) for the these paths

  Recover fee-payer account from a base58 private key
  Create a new fee-payer key pair
  Use zkApp account as fee payer

and possibly even for the use the Use stored account path.

Questions

At what steps will the key alias and keys in the cache be updated? Would cache be updated in both the Recover fee-payer account from a base58 private key and Create a new fee-payer key pair?
What is the default path we are pushing? Would it be zk config => Create a new fee-payer key pair or if cache Use stored account "dev"? It's nice that we are giving the users choices, but we should either make it clear in the UX/messaging the default path or eliminate/de-emphasize uncommon paths.
Would theUse zkApp account as fee payer behave the same as the existing default path in the cli?

Naming suggestions:

Assuming we keep the key alias. I propose this prompt for naming consistency. Pick an alias for this account: => Choose an alias for this account:
Add the word existing in this prompt for a little clarity.
Recover fee-payer account from a base58 private key => Recover fee-payer account from an existing base58 private key

mitschabaude commented 1 year ago

Thanks @ymekuria!

Naming suggestions:

👍🏻

At what steps will the key alias and keys in the cache be updated? Would cache be updated in both the Recover fee-payer account from a base58 private key and Create a new fee-payer key pair?

Yes that's what I was thinking!

What is the default path we are pushing? Would it be zk config => Create a new fee-payer key pair or if cache Use stored account "dev"? It's nice that we are giving the users choices, but we should either make it clear in the UX/messaging the default path or eliminate/de-emphasize uncommon paths.

Removing choice dilemma is a good point and maybe another point in favor of removing the zkApp account option.

I think the default after caching is to use the cached account. A user might never switch fee payer accounts after storing the account the first time, as long as they stay on the same network. (And it might even be useful to use the same keypair on different networks!)

So, if an account is stored I want to de-emphasize all other options. A solution could be to hide the other options behind an additional step:

❯ Use stored account "dev" (public key:  B62qjR6VeV4UhAnEhuhpkprovmfrYdmqZYCK72urF1FJs8XHeV6dtTq)
  Use a different account (select to see options)

And only when picking "Use a different account (select to see options)", they would be presented with the 3 options from before (recover keypair, create keypair, use zkapp account).

In the case the user does this the first time and no account is stored, I think both creating a new keypair and recovering from a base58 key are equally good options, so I would present them with the same emphasis. Would put "create keypair" on top because in a sense it is the "simpler" option (fewer steps in the CLI).

Would theUse zkApp account as fee payer behave the same as the existing default path in the cli?

I think in that case we should just store the zkApp keyPath also under "feePayerKeyPath", and elsewhere it would not treat the zkapp key differently than any other fee payer key.

We still might want to include flows for linking to the faucet from the cli to fund accounts(on testnet) for the these paths

I agree. Simple solution would be to always show the faucet link regardless of fee payer account. A fancier solution could be to query the graphql endpoint for the public key and check if it has enough balance to cover the fee at least once. In case it does, we don't show the faucet link.

ymekuria commented 1 year ago

Removing choice dilemma is a good point and maybe another point in favor of removing the zkApp account option.

+1 on removing the zkApp account option unless we can think of a good use case where it is necessary.

In the case the user does this the first time and no account is stored, I think both creating a new keypair and recovering from a base58 key are equally good options, so I would present them with the same emphasis. Would put "create keypair" on top because in a sense it is the "simpler" option (fewer steps in the CLI).

+1

So, if an account is stored I want to de-emphasize all other options. A solution could be to hide the other options behind an additional step:

❯ Use stored account "dev" (public key: B62qjR6VeV4UhAnEhuhpkprovmfrYdmqZYCK72urF1FJs8XHeV6dtTq) Use a different account (select to see options)

This is a good compromise between emphasizing a default path and giving users options.

I agree. Simple solution would be to always show the faucet link regardless of fee payer account. A fancier solution could be to query the graphql endpoint for the public key and check if it has enough balance to cover the fee at least once. In case it does, we don't show the faucet link.

Only showing the faucet links when an account needs funds would be a great dx. We can also start with the simple solution and add the account query check later if adding this check is out of scope.

shimkiv commented 1 year ago

Even if one of the steps imply this, I wanted to mention that it will be good if we will explicitly show the destination "cache storage" path with the warning to make sure it is secured.

mitschabaude commented 1 year ago

Even if one of the steps imply this, I wanted to mention that it will be good if we will explicitly show the destination "cache storage" path with the warning to make sure it is secured.

Good idea!

nicc commented 1 year ago

I like where this is going! +1 for removing the zkApp account option. Only showing the faucet link when the account balance is too low would be lovely, assuming it's not too much extra work.

o1-labs / zkapp-cli