SC: Fix read_only flag in call traces and make code cleaner in read only processing

[linh2931]

• Fix read_only flag in call traces such that read_only flag is true for all subsequent call traces after the first call whose read_only flag is true
• Rename read_only enum to force_read_only and introduce read_only to call_context to make code cleaner
• Add more comments explaining read_only request by the user and read_only state of the call_context
• Add tests to verify read_only flag values are correct in call traces

Resolves #1408

[spoonincode]

It seems like don't really even need flags any more in the current implementation -- until some time in the future where maybe needing more flags.

[linh2931]

Do you mean we remove flags from the entry point function? That would remove the ability of a call to tell if it is read only or not. But not sure how useful it is as read-only is enforced at the protocol level.

An advantage of flags is it would be simpler to extend in the future without changing entry point signature.

[spoonincode]

No, I'm referring to flags in sync_call_context. afaict it's currently unnecessary and actually burdens your implementation a little since you mutate the passed flags,

spring/libraries/chain/host_context.cpp

Lines 47 to 51 in fd03667

	// Update next call's flags based on is_next_call_read_only
	uint64_t updated_flags = flags; // get the default values
	if (is_next_call_read_only) {
	updated_flags = set_field(flags, sync_call_flags::force_read_only);
	}

I approved because I don't think you need to get rid of it but I think it would slightly reduce quantity of code

[linh2931]

oh I see. I removed it as it is not needed, and we can add it back when needed as it is an implementation thing and won't break consensus.

[linh2931]

2356b8f

[spoonincode]

You can simplify more. For example the removal of updated_flags and then just pass a bool to sync_call_context's ctor

[linh2931]

Thanks! I was a little hesitant to remove the flags in the constructor but that makes code even simpler. Let's do it.

[linh2931]

d7ecbe5

[spoonincode]

imo this description is missing a key point that with the flag clear it inherits the "readonlyness" from the calling context

[linh2931]

Thanks. I will reword the comments.

[linh2931]

6c799e6