Gas Cost for Cached Reads

[alexanderbez]

Summary

Through the GasMeter charges gas costs per read regardless if that read hits the cache or not of the subseqent KVStore.

func (gs *Store) Get(key []byte) (value []byte) {
	gs.gasMeter.ConsumeGas(gs.gasConfig.ReadCostFlat, types.GasReadCostFlatDesc)
	value = gs.parent.Get(key)

	// TODO overflow-safe math?
	gs.gasMeter.ConsumeGas(gs.gasConfig.ReadCostPerByte*types.Gas(len(value)), types.GasReadPerByteDesc)

	return value
}

Problem Definition

It would be ideal and more economical to either not charge at all for cache hits or have parameterized costs for cache hits (i.e. charge much less).

Proposal

To not charge at all would be to simply update the KVStore interface to have Get return ([]byte, ok) where the boolean signifies if there is a cache hit or not. This is just an idea as we can instead only alter the CacheKVStore interface.

If we want to charge based on parameterization, this will be slightly more difficult but perhaps one solution could be to have Stores return their own respective gas params via GetGasCosts() GasParams.

/cc @AdityaSripal

---

For Admin Use

• Not duplicate issue
• Appropriate labels applied
• Appropriate contributors tagged
• Contributor assigned/self-assigned

[AdityaSripal]

This is a particularly pertinent issue given #5006 since each decorator must read and write potentially the same value from the context, whereas with a single AnteHandler function there could be a single read, multiple updates, and then a final write which will consume much less gas.

I think we should accept this proposal since it would greatly reduce gas cost of #5006, and generally improve how closely out gas costs align with the actual computation/IO cost. It also has a relatively simple (albeit breaking) implementation:

func (gs *Store) Get(key []byte) (value []byte) {

	value, cached = gs.parent.Get(key)
        if !cached {
           gs.gasMeter.ConsumeGas(gs.gasConfig.ReadCostFlat, types.GasReadCostFlatDesc)

            // TODO overflow-safe math?
 	   gs.gasMeter.ConsumeGas(gs.gasConfig.ReadCostPerByte*types.Gas(len(value)), types.GasReadPerByteDesc)

        }

	return value
}

Pros:

1. Better gas metering, don't charge nearly as much for cached values

Cons:

1. Will have to break KV Store interface so that gasKv can know whether something has been cached. Get(key []byte) (value []byte, cached bool)

2. If we change gas consumption based on whether a value has been cached or not, then we must enforce that everyone on a given blockchain run the exact same cache configuration. Otherwise, 2 nodes running different cache policies will charge different gas and have different AppHash results. This, in my opinion, is the main concern with moving forward on this proposal. Do we want to allow users the flexibility of choosing their own cache policy or not

[alexanderbez]

Good point on (2) but this is no different really from the current implementation. Since these are not on-chain params, they're more-or-less hard-coded per app. Would like to get other's opinions on if we should charge gas for cached reads. (1) is pretty simple.

/cc @zmanian @ValarDragon @sunnya97

[ValarDragon]

I think gas should absolutely be charged for cached reads. I think it makes sense to maintain a canonical "expected cache", which assumes some amount of caching. e.g. Reading the same variable twice in a row should be much cheaper given any reasonable cache policy.

This is reminiscent to how you write optimized code expecting cachelines, and how ETH charges gas based on "commodity laptop".

This likely doesn't matter much for gaia, but becomes very important when creating a smart contracting language.

[alexanderbez]

Ok, questions I have are (1) what do we charge for cached-reads (% of non-cached cost) and (2) how to do this cleanly (e.g. KVStore return cost config)?

[sunnya97]

I feel charging differently for cached-hits vs not would make gas-estimation pretty difficult because you won't know whats in the cache when the tx actually runs.

[ValarDragon]

This feels untrue for param store entries which usually can be assumed to be cached under high load.

Though you make a good point that this is pointless without improved gas estimation, since unlike Ethereum we have to fill blocks based on the estimate.

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[ValarDragon]

Ethereum is handling this at the level of a single tx, by raising the gas of each location queried in state for the first time: https://github.com/ethereum/EIPs/blob/master/EIPS/eip-2929.md

Maybe this is the approach that ought to be taken to achieve this here? (Espec. since we now have multiple VMs nearing completion)

Cache arguments across txs get more complex once one considers optimizations for parallelizing their execution.