Execute FLIX scripts/transactions

[chasefleming]

Closes #984

Execute FLIX by id, name, or path (detected by CLI):

flow flix multiply 2 3 --network testnet
flow flix bd10ab0bf472e6b58ecc0398e9b3d1bd58a4205f14a7099c52c0640d9589295f 2 3 --network testnet
flow flix ./multiply.template.json 2 3 --network testnet

Description

TODO:

• Execute FLIX scripts by name
• Execute FLIX scripts by ID
• Execute local FLIX scripts
• Execute FLIX transactions by name
• Execute FLIX transactions by ID
• Execute local FLIX transactions
• Fix tests
• Use logger

---

For contributor use:

• Targeted PR against master branch
• Linked to Github issue with discussion and accepted design OR link to spec that describes this work
• Code follows the standards mentioned here
• Updated relevant documentation
• Re-reviewed Files changed in the Github PR explorer
• Added appropriate labels

[bjartek]

I think using cobra and commands and flags would make this api better. It is also a lot easier to generate good help that way. flow-cli is modeled that way, any reason to change that?

[chasefleming]

@bjartek fair point. I'm still getting familiar with Cobra. Is it possible to handle commands like the flix:id:123 or flix:name:transfer-flow in a command like flow transactions send flix:name:transfer-flow ... or flow scripts execute flix:id:123 ...?

[bjartek]

subcommands in cobra are seperate by space, so you could do
flow transactions send flix name transfer-flow or have name be a flag to the flix command, or just have the argument to flix command be name or id.
so
flow transactions send flix transfer-flow

i would personally also love to se a 'smart' command here
flow flix transfer-flow

The flix know if it is a transaction or a script.

[chasefleming]

The first too are a little verbose. I like the simplicity of option three, flow flix transfer-flow since -- as you mentioned -- you can determine if it's a script or tx from the flix type field. Good suggestions. Thanks. I'll implement it.

[chasefleming]

@bjartek one more note: yeah we'll actually need to probably add id, name, or path in there as well -- like flow flix name transfer-flow -- otherwise it won't know how to query flix...unless you have some really reliable way to identify an ID and then we could do it programmatically.

[sideninja]

I think defining this command to use flags would be better. Example: flow flix --name hello or flow flix --id 123. The usage of flow flix id 123 or flow flix name hello is nonconsistent with other commands. CLI never uses arguments to define the type of the next argument value. Also https://github.com/onflow/flow-cli/blob/master/CONTRIBUTING.md#arguments

[bjartek]

I like the new command syntax.

[bjartek]

What should happen if you try to execute a flix that do not have updated pins? So say your flix relies on an old version of a contract pinned and you try to run it after it has been updated.

should there be a prompt that tells the user "hey this flix is outdated" or "this flix is not verified"

The same can be said if you want your flix to be verified by a validator.

[chasefleming]

UPDATE: I wound up detecting if it's a path, name, or id and removed requirement to provided it as a command or as a flag (cc @sideninja )

Also, @bjartek I really like the prompt idea for the out of sync pins. Do you think that's fine to do in a separate issue? Doesn't seem required for this PR, right? Would be nice to close this one and start that separately since it would also require us adding the pinning logic to flixkit. Is Overflow's pin compute functions working fully or it requires flix to be updated to the new version that is more reliable for hashing with the array format?

[sideninja]

Left comments, looks good but some things to address, also can you link a documentation PR to this one.

[chasefleming]

@sideninja Here is the docs PR: onflow/docs#182

[codecov-commenter]

Codecov Report

Merging #1125 (972c23e) into master (1d93e79) will decrease coverage by 1.17%.
The diff coverage is 18.42%.

@@            Coverage Diff             @@
##           master    #1125      +/-   ##
==========================================
- Coverage   39.47%   38.31%   -1.17%     
==========================================
  Files          37       38       +1     
  Lines        1882     1947      +65     
==========================================
+ Hits          743      746       +3     
- Misses       1051     1113      +62     
  Partials       88       88              

Flag	Coverage Δ
unittests	38.31% <18.42%> (-1.17%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed	Coverage Δ
internal/super/flix.go	0.00% <0.00%> (ø)
internal/scripts/execute.go	78.12% <100.00%> (+1.45%)	⬆️
internal/transactions/send.go	86.66% <100.00%> (+0.22%)	⬆️

[sideninja]

@chasefleming let me know when you want another review

[chasefleming]

@sideninja Ready for another review

[sideninja]

Looks good, left couple of small comments