functions that use .call() should be not allowed in view functions

[seanyoung]

Functions that modify the state, like the SPL-token mint_to, are not view functions, but the compiler warns that they can be declared as view.

Example:

	function mint_to(address mint, address account, address authority, uint64 amount) internal {
		bytes instr = new bytes(9);

		instr[0] = uint8(TokenInstruction.MintTo);
		instr.writeUint64LE(amount, 1);

		AccountMeta[3] metas = [
			AccountMeta({pubkey: mint, is_writable: true, is_signer: false}),
			AccountMeta({pubkey: account, is_writable: true, is_signer: false}),
			AccountMeta({pubkey: authority, is_writable: true, is_signer: true})
		];

		tokenProgramId.call{accounts: metas}(instr);
	}

Warning:

warning: function can be declared 'view'
   ┌─ /solang/solana-library/spl_token.sol:46:2
   │
46 │     function mint_to(address mint, address account, address authority, uint64 amount) internal {
   │

Fixes #997

Another PR will implement staticcall for Solana, which is allowed in view functions.

[seanyoung]

@xermicus I don't know why there was a special case for Substrate, it may have been wrong or maybe there is a good reason. Do you know if seal_call is permitted in pure or view functions, i.e. non-mutable messages?

[xermicus]

No I don't know why there was a special case. I'm not aware on why it should not be permitted in pure or view functions, as long as it is guaranteed that the callee is also pure or view respectively. For raw calls where we don't know, assuming that the function might write to state (as your PR does) looks correct to me.

[LucasSte]

Does this PR solve #997 ?

Edit: I've just read the PR summary. Can you add the example from #997?

[LucasSte]

You used the question mark on the other questions, but not here. Please, adopt a single notation.

[seanyoung]

I've added question marks but I think it questionable whether this improves things.

[LucasSte]

You've only used may_write_to_state in a single place.
Why didn't you just reuse the does_write_to_state? You could have renamed it to may_write_to_state, as we are doing our best effort in this analysis.

[seanyoung]

We need to distinguish between does_write_state and may_write_state. If we have does_write_state then the function cannot be view or pure. If we only have may_write_state and !does_write_state then view is allowed.

[xermicus]

If we don't know, why are we not assume for "the worst" and flag it as a state writing function?

[seanyoung]

The problem with that is that you cannot mark functions as view which means you need pay for a transaction in order to call it. You don't want that in many cases. In fact, non-view functions cannot have return values Solana/Anchor so it would not work at all.

[xermicus]

Isn't it the point that you can't mark them as view, if you don't know (e.g. you can't prove to the compiler) whether they are actually view?

[seanyoung]

Hopefully the comment I've added makes sense

[xermicus]

Sorry for this nitpick, but I think using an enum Mutability { Write, Read, None } instead a bunch of bools would make for more idiomatic code overall. The struct could then just have two fields of this enum, like explicit_mutability (explicit is what the contract defines in the code) and implicit_mutability.

I mean its unrelated to this bugfix; I'll just leave it as suggestion if you feel like it

[seanyoung]

Great idea, I've re-written it with an enum.

[xermicus]

Looks a lot cleanier now, thanks!