0xPolygonMiden / miden-client

Client library that facilitates interaction with the Miden rollup
MIT License
32 stars 29 forks source link

Ask users to confirm their transaction (as a default) #257

Closed Dominik1999 closed 4 months ago

Dominik1999 commented 5 months ago

Feature description

I as a user would want to be informed about the effect of a transaction before I prove or submit the transaction. I could then confirm and the transaction is being executed and proven.

As an example in the CLI:

miden-client tx new p2id 0x123 0x345 0x678 50

then the user would be prompted

miden-client tx new p2id 0x123 0x345 0x678 50
...
This transaction removes 50 POL from your account. Proceed? (Y/N)

Then the user can type Y or N.

We can start by only telling the user the financial implications of his actions. We might need to add more or even custom information to the prompt in the future.

Why is this feature needed?

We want to be more user-friendly.

This can actually be a good first issue.

bobbinth commented 5 months ago

We can start by only telling the user the financial implications of his actions. We might need to add more or even custom information to the prompt in the future.

I would probably display the full info - i.e., print out the full account delta. Would not be as user-friendly as printing out more contextual messages - but I think for the purposes of the CLI this is fine.

One note: there are two ways to implement this functionality:

  1. Print out the delta after the transaction has been executed but before it has been submitted to the node.
  2. Print out the delta before the transaction has been signed by the user.

The first approach is simpler - we basically just need to add the code to print out account delta and read the confirmation from the user. But it has one major drawback: the user signs the transaction before they see the impact - so, we are kind of trusting the CLI to do the right thing here and not to send the fully-ready transaction to the node without the confirmation.

So, I think the second approach better, but it would require an authenticator integrated into the transaction host (similar to what is described in https://github.com/0xPolygonMiden/miden-base/issues/594).

But we can still implement the simpler approach first and then migrate to the authenticator once that's implemented in miden-base.

mFragaBA commented 5 months ago

for now I'll go with the simpler approach. Is it reasonable to add a flag to the tx new command to proceed without asking for input?

igamigo commented 5 months ago

Yeah, I think a --force should be included.

bobbinth commented 5 months ago

Yeah - I think that's fine. We'll need to think how this flag will be handled in the longer-term approach - but I think we'll find ways to deal with it.

mFragaBA commented 5 months ago

290 got merged. I Guess in the next iteration we'll introduce the usage of the Authenticator. Should we close this issue and open a new one or we keep working referencing this one?

bobbinth commented 5 months ago

I'd create a new issue.