soroban-cli
soroban-cli copied to clipboard
stellar
Bail on submitting if the simulate transaction shows a readonly footprint
What problem does your feature solve?
Read-only contract functions shouldn't be able to be submitted. Or maybe we allow some function flags at the contract layer like Solidity does for things like pure, view, payable, etc. Being able to submit a transaction that makes no changes and just returns data seems weird.
What would you like to see?
At the very least there should be a flag that allows the CLI to only call and return the preflight.
See thread on Discord https://discord.com/channels/897514728459468821/966788672164855829/1032397970911526993
At the very least there should be a flag that allows the CLI to only call and return the preflight.
I think this is a duplicate of https://github.com/stellar/soroban-cli/issues/221.
Read-only contract functions shouldn't be able to be submitted.
I think it's difficult for the CLI to know for sure if a function is always read-only. It's also difficult for the SDK to guarantee a function will be read-only, so unfortunately if we did add a marker to the function it'd likely be more symbolic, so I'm hesitant to agree we should add that.
As @dmkozh mentioned on Discord the CLI can detect that the footprint in the simulation is read-only, which means that specific invocation was read-only. The CLI could bail early if it detects this.
This behavior might be inconsistent. A function might be read-only 90% of the time, then write sometimes. This could be surprising if a CLI invoke command works 90% of the time then actually submits 10% of the time. I don't think there's anything for us to do about this though. If the simulation is read-only, then the footprint will be read-only, and so even if the tx is submitting to network it will never have any impact, either that or it will fail if it tried to write. So it's probably fine to have it work like this:
The following command would exit early after simulation if the footprint is read-only:
$ soroban invoke --id ... --fn sometimes_read_only
stdout: <return value>
The following command would only ever do simulation (this is #221):
$ soroban invoke --id ... --fn sometimes_read_only --simulate
stdout: <return value>
If we discover a need where we want invocation to always submit to network we could add a new option at that time. We wouldn't need to add this until the need presented itself though.
$ soroban invoke --id ... --fn sometimes_read_only --force-submit
stdout: <return value>
The following command would exit early after simulation if the footprint is read-only:
Is this behavior already implemented? I was making calls yesterday to futurenet for functions that definitely didn't do any writing and it was behaving as if it was submitting a transactions. Which now that I think about it I'm not entirely sure it was submitting a transaction vs just not returning until after the next ledger close. (~5 second wait times for the read only fn) Which I wouldn't have expected simulation calls to only return after the current ledger closed. Thus I assumed the current CLI was in fact submitting the transaction and not exiting early.
The CLI doesn't exit early for simulation, but we could change it to do this. The area of code that needs changing is: https://github.com/stellar/soroban-cli/blob/125e2512bd91d5d34a28087fa4780e9e2953ed10/src/invoke.rs#L324-L341
Something not mentioned here already is that whether a function writes events or not should play into the decision over simulation only or submit, because events are writing state even if the footprint is read-only.
I don't think we want to encourage writing events only. The contract invocation will still be a function of the current ledger state, which might as well be executed off-chain. I'm not sure if there is a good reason for running a function without potentially updating he ledger (not that it's hard to work around that check, but at least that should be the intention).
I don't think we need to have an opinion on whether a contract should generate events without state. It does sound dubious. But regardless other blockchain contracts, for example Solidity, consider an event a write and prevent their use in read-only functions because data ends up in the ledger.
I'd like to suggest a different UI than what I proposed a few comments above:
I think having a new flag that operates on a single concept "sending" will be easier to understand that a couple new options, mostly because it will make the interactions with --build-only and --sim-only clearer:
Default behavior if the flag --send isn't passed, it does this by default:
soroban invoke --id ... --fn sometimes_read_only --submit=if-write
Never submit, even if call looks like it needs to be submitted:
soroban invoke --id ... --fn sometimes_read_only --submit=never
Always submit:
soroban invoke --id ... --fn sometimes_read_only --submit=always
Related:
- In v22 we remove
--sim-onlyto reduce potential confusion with simulating only vs not sending. - In v22 we remove
--is-viewwhich is an alias for--send=never.