Gas should be charged for Next, rather than Key() and Value()

[jaekwon]

cosmos-sdk/store/gaskvstore.go

Line 139 in 63713c9

g.gasMeter.ConsumeGas(g.gasConfig.KeyCostFlat, "KeyFlat")

We should prob not be charging for each call of Key()/Value(), since they'd be cached once fetched anyways. I also don't think we need two separate key/value charges either.

I'd change our store gas system to charge a FlatRead upon iterator initialization, and a new FlatIterNext for calling Next, and no charge for Key or Value.

Maybe also a "UnsafeGetParent()" function that returns the parent so we can debug applications, and a TODO to add a conditional on Unsafe to only allow it when e.g. a "mode=develop" flag is set somewhere. I had to expose gasIterator to debug the 7003 crash... (see #1938)

[jaekwon]

(assigning to Chris to reassign)

[jaekwon]

Also TODO: consider creating a new issue for CosmosSDK "develop mode" to enable this and auto-enabling tracing etc.

[cwgoes]

The reason that the gas KV store charges gas costs this way is that calling .Key() or .Value() copies a new slice each time, at least in GoLevelDB:

• https://github.com/tendermint/tendermint/blob/master/libs/db/go_level_db.go#L284
• https://github.com/tendermint/tendermint/blob/master/libs/db/go_level_db.go#L293

Does the IAVL tree (which is what we're iterating over) do that or something else?

We can definitely charge for .Next() instead, not hard to change, but I want to understand the underlying disk I/O cost model.

[alexanderbez]

Would like to look into this more myself...subscribing

[cwgoes]

Per discussion with @jaekwon:

• Call .Key() and .Value() when .Next() is called to charge for size