onflow / flow-cli

The Flow CLI is a command-line interface that provides useful utilities for building Flow applications
https://onflow.org
Apache License 2.0
206 stars 66 forks source link

Add common transaction templates, e.g. Flow Transfer #303

Closed Kay-Zee closed 1 year ago

Kay-Zee commented 3 years ago

Issue To Be Solved

There are currently some simple transactions that still require the user to provide the transaction CDC file, such as transfering flow.

It'd be much more user friendly to include these, (similar to how we've included the account creation tx text, as well as the staking info query script text)

(Optional): Suggest A Solution

My suggestion is probably either to have it as an extra flag for the transactions sub command:

flow transactions send --builtin=transfer

Or for something as big as transfer, maybe have it as a sub command, itself

flow transfer --amount=10 --recipient=0xdeadbeef

(Optional): Context

I'm trying to help some less technical users use the CLI to transfer FLOW funds, and having to always provide the transfer tx script separately is very troublesome

sideninja commented 3 years ago

I like this idea, some initial thoughts I have on this:

Maybe we can use the same API but try resolving the transaction name as a template if the specific file is not found, for example, we could do: flow transactions send transfer or flow transactions send withdrawal (if withdrawal would be another example of common txs). And if there isn't a file transfer it will try to resolve it with known transactions templates we have. This feels a bit like too much magic but it has an upside that it doesn't add complexity to the API for basic usage.

Reasons I would prefer arguments to flag: https://github.com/onflow/flow-cli/blob/master/CONTRIBUTING.md#arguments "Use an argument for the value that command requires in order to run."

Another good approach is like stated to add a new command, but maybe adding top command is not the best if there will be more templates it can get crowded at the top-level help screen, so having something like: flow transactions transfer could be better.

rheaplex commented 3 years ago

This is a great idea. I have a menagerie of tx files I currently use for this kind of thing.

I'm very wary of any potentially ambiguous behaviour. If something will be interpreted differently depending on intent that way lies unpleasant surprises. A file path and a builtin name are different until someone makes a mistake in their project directory, or someone places a malicious file there with the same name as a builtin.

Likewise, passing the transaction type as a flag seems like a major behaviour modification phrased as a minor configuration detail.

And I worry about subcommand proliferation as more and more important transactions are developed.

flow transactions builtin transfer, so making builtin an equivalent to send for builtins and specifying a name rather than a filepath as its argument, might combine the best features of the proposals?

sideninja commented 3 years ago

@rheaplex I agree, having the same send command for sending transactions from files or templates is not explicit enough and the security impact can be huge, you are right.

I was thinking of making another transaction command but couldn't think of a good name, I guess builtin can be good, much better than what I had. I would also like if the word would express action, not just property (send vs builtin), but then send-builtin is too long and weird.

Kay-Zee commented 3 years ago

having a separate key work works for me. I'm not opposed to send-builtin, since we already have send-signed. but just built-in works in my head as well.

sideninja commented 3 years ago

I would rather error on the length than clarity so I would say the send-builtin is better but then builtin is actually written built-in which makes the command send-built-in which I don't like and I would suggest send-template. This should become a new command under transaction category like so: flow transactions send-template and it should be implemented in the transactions command layer by invoking this functionality as part of the transactions service.

The templates to be included should be the obvious ones like transfer flow, but we could also check the network for most common transactions and get the data there.

sideninja commented 2 years ago

I feel like we should extend this to support transactions and scripts. We could have a more generic command something like flow send {command} and then support for all common transactions/scripts. So for example fungible token support could look like: flow send FT transfer, flow send FT get-balance, flow send FT get-supply basically anything in the https://github.com/onflow/flow-ft

nvdtf commented 2 years ago

Might be desirable to be able to run transactions from GitHub URIs:

flow transactions send github.com/onflow/flow-ft/transactions/setup_account.cdc --signer MyAccount
bjartek commented 2 years ago

This should be joined in with the interaction-template initiative I think.

nvdtf commented 1 year ago

FLIX offers a good solution to this. Here's the current interaction template for transferring FLOW: http://flix.flow.com/v1/templates?name=transfer-flow

We're looking into integrating FLIX into CLI so the above use case can be facilitated.

justinbarry commented 1 year ago

This needs to transition to an epic.

nvdtf commented 1 year ago

Now part of this epic https://github.com/onflow/flow-cli/issues/928

sideninja commented 1 year ago

Closing in favour of https://github.com/onflow/flow-cli/issues/928