SC: Support read only sync call

[linh2931]

• Refactor to use host function precondition check to disallow state modifying host functions in read-only transactions, instead of scattering the checks in various host function implementation.
• Add is_read_only() to host_context class to support both read-only transactions and read-only sync calls.
• Make sure read-only request is honored throughout a transaction and all children sync calls.
• Add comprehensive tests.

Resoles #1217

[heifner]

Why is is_caller_read_only not enough?

[linh2931]

It is not enough. The caller itself can be a regular non-read-only action or sync call, but the flags for the callee can request the next call is a read-only call.

[heifner]

Thanks. I was not thinking of the case of a non-read-only making a read-only sync-call.

[spoonincode]

ah sorry maybe it's just Friday but I'm still not understanding this reasoning. It makes sense that a read only action can only execute a read only sync call, and that a read only sync call can only execute a read only sync call. But it seems like that would be enforced at the time of dispatching the sync call, not here?

[linh2931]

Now all the enforcement is done by the host function wrappers.

This is for the cases that an action is read only, far away from it in the sync call sequence, say call_n. call_n's flag is not set; this does not mean call_n will modify the state. We should let call_n run and use the host function wrapper to make sure the state is not modified during call_n's execution.

[linh2931]

The is_read_only() will return true if any of the ancestor sync calls' flag is true or if the transaction itself is a read-only transaction. We use is_read_only() to print out the read_only flag in call traces.

[linh2931]

I will make sure the call traces flags are consistent when the two PRs are merged together.

Thanks to both of you.

[linh2931]

Oh, on the current call_trace branch, the read_only function does not look at the ancestors. But on the read_only brach, it does that. When the two PRs are merged, it will work as Matt expects.

[spoonincode]

I don't think it works the way described because in #1333 the call trace is populated from the flags, not the is_read_only():

spring/libraries/chain/host_context.cpp

Lines 47 to 48 in 4fd5a8c

	const bool read_only = flags & static_cast<uint64_t>(sync_call_flags::read_only);
	trace.call_traces.emplace_back(get_sync_call_ordinal(), call_receiver, read_only, data);

imo this is an unintuitive way to deal with this vs doing in execute_sync_call()

if(is_read_only())
   EOS_ASSERT(read_only /*(from flags)*/, "only can do read only from read only");

But if this is how you want it, then should make an issue about this problem so it isn't forgotten when the PRs are eventually merged

[linh2931]

#1408

[linh2931]

I think to merge this PR as is. The renaming to force_read_only is to be done in #1408.