stellar / stellar-cli

CLI for Soroban contracts.
66 stars 55 forks source link

Fix --cost parameter on invoke command #889

Closed leighmcculloch closed 1 year ago

leighmcculloch commented 1 year ago

What version are you using?

❯ soroban version
soroban 0.9.4 (76cacc7cf885bd7d37756735fab332351d893406)
soroban-env 0.0.17 (400d806387140553e4e685d232deb3a807ec0e36)
soroban-env interface version 85899345971
stellar-xdr 0.0.17 (0f16673441898162c9996da6117be2280ef8fd84)
xdr next (e372df9f677961aac04c5a4cc80a3667f310b29f)

What did you do?

soroban contract invoke --id 1 --wasm file.wasm --cost -- hello --to me

What did you expect to see?

I expected to see the cost table outputted, that looks like this:

=====================================================================================================================================================================
Cpu limit: 40000000; used: 914627
Mem limit: 52428800; used: 141947
=====================================================================================================================================================================
CostType                 iterations     input          cpu_insns      mem_bytes      const_term_cpu      lin_term_cpu        const_term_mem      lin_term_mem        
WasmInsnExec             344            None           2408           0              7                   0                   0                   0                   
WasmMemAlloc             0              None           0              0              0                   0                   1                   0                   
HostMemAlloc             10             Some(1006)     23500          1086           2350                0                   8                   1                   
HostMemCpy               19             Some(1142)     437            0              23                  0                   0                   0                   
HostMemCmp               7              Some(118)      419            0              43                  1                   0                   0                   
InvokeHostFunction       1              None           928            0              928                 0                   0                   0                   
VisitObject              1              None           19             0              19                  0                   0                   0                   
ValXdrConv               5              None           670            0              134                 0                   0                   0                   
ValSer                   0              Some(0)        0              0              587                 1                   9                   3                   
ValDeser                 0              Some(0)        0              0              870                 0                   4                   1                   
ComputeSha256Hash        0              Some(0)        0              0              1725                33                  40                  0                   
ComputeEd25519PubKey     0              None           0              0              25551               0                   0                   0                   
MapEntry                 29             None           1537           0              53                  0                   0                   0                   
VecEntry                 0              None           0              0              5                   0                   0                   0                   
GuardFrame               2              None           8100           944            4050                0                   472                 0                   
VerifyEd25519Sig         0              Some(0)        0              0              369634              21                  0                   0                   
VmMemRead                1              Some(16)       0              0              0                   0                   0                   0                   
VmMemWrite               0              Some(0)        0              0              124                 0                   0                   0                   
VmInstantiation          1              Some(539)      861323         139431         600447              484                 117871              40                  
VmCachedInstantiation    0              Some(0)        0              0              600447              484                 117871              40                  
InvokeVmFunction         1              None           5926           486            5926                0                   486                 0                   
ChargeBudget             72             None           9360           0              130                 0                   0                   0                   
ComputeKeccak256Hash     0              Some(0)        0              0              3322                46                  40                  0                   
ComputeEcdsaSecp256k1Key 0              None           0              0              56525               0                   0                   0                   
ComputeEcdsaSecp256k1Sig 0              None           0              0              250                 0                   0                   0                   
RecoverEcdsaSecp256k1Key 0              None           0              0              2319640             0                   181                 0                   
Int256AddSub             0              None           0              0              735                 0                   119                 0                   
Int256Mul                0              None           0              0              1224                0                   119                 0                   
Int256Div                0              None           0              0              1347                0                   119                 0                   
Int256Pow                0              None           0              0              5350                0                   119                 0                   
Int256Shift              0              None           0              0              538                 0                   119                 0                   
=====================================================================================================================================================================

What did you see instead?

Nothing. Nothing is output.

Prior discussion

leighmcculloch commented 1 year ago

It appears the cost output does show up when verbose output is enabled, regardless of whether --cost is specified. Looking at the code I don't see any references to the cost parameter.

If the intention is to display cost data only in verbose mode, then I think it will be unintuitive to discover. Viewing cost information about blockchain transactions in general is a first-class concern. Verbose modes in tools are usually reserved for debugging, or when users need to more completely understand the internals of what is happening.

leighmcculloch commented 1 year ago

As part of, or as a follow up to, fixing this bug and reintroducing cost analysis as a first-class feature, it would be helpful to add docs for it, which other than being a necessity will also help us evaluate if the developer experience is good. The docs issue is:

jayz22 commented 1 year ago

Happy to hear suggestions on how the output format can be improved (@leighmcculloch mentioned on the discord).

paulbellamy commented 1 year ago

I don't think this ever worked against the real live network. It logs a host "Budget" object after execution (which is why you get very detailed output about wasm execution). But for the real network we don't have that. Maybe the closest we could do would be to log the SorobanResources for the txn, which would give us read_bytes, write_bytes, insns and footprint?

leighmcculloch commented 1 year ago

It used to work in sandbox.

If we were to make it available for networks, using SorobanResources available from preflight would make sense. 👍🏻

footprint

Speaking of the footprint, there also used to be a --footprint option, but that was removed from the invoke command as well. I see the footprint in the verbose output now.

It's not obvious to me that moving these both into verbose leads to intuitive access to that information. Both footprint and cost are more than diagnostic/debug information, they're information about a contract that I think should be very accessible.

willemneal commented 1 year ago

The idea at the time was to allow filters in lieu of flags for each type of log. e.g. -f soroban_cli::log::footprint=debug. I am also considering defaulting to output logs to a file so you wouldn't need to rerun the command again to see any transaction related info.

paulbellamy commented 1 year ago

I think the separate --cost and --footprint flags are going to be easier for users to discover (they'll be in the man page, etc.), than the subsystem-logging approach.

Second, I'm not a huge fan of logs default secretly going off to some file somewhere. But I'm pretty familiar with bash output redirection etc, so maybe that's not typical of our target devs. 🤷

willemneal commented 1 year ago

How about listing all the possible filters in the man page?

I've tried to avoid bash specific solutions since we want to support windows. But I do think networked transaction logs would be useful when users need to report errors (perhaps we could add a command to display the log file).

leighmcculloch commented 1 year ago

separate --cost and --footprint flags are going to be easier for users to discover

+1. Logs are great for debugging and getting into the internal workings of things and it's good we output those details there, but sometimes it's helpful to just get cost info, and I imagine in the future we might want to control the output format of the cost info and logs won't be a convenient way to do that. E.g. we might end up adding --output plain|json to invoke, and then cost, footprint, and return-value outputs can be streamed json objects. Regardless of all that though, the main sell I think is feature discovery. When it's in logs, it doesn't feel like a first-class feature.

leighmcculloch commented 1 year ago

Alternatively, we add the options back, and those options just enable the logs filtered for those specific thing? Discoverable, and all items are found in one place.