[DEMO: don't merge] Trial kvstore interface cleanup

[ethanfrey]

Closes #823

Modify the interfaces as discussed in issue. And update ./store to use the new interface.

All other code, like Buckets, is not updated yet. Waiting for feedback if to continue with this or change course

[iov-bot-danger]

	1 Error
🚫	Please include a CHANGELOG entry. You can find it at CHANGELOG.md.

Generated by 🚫 Danger

[husio]

I think this is a good change. Makes even more sense when we merge #835

All database methods are accepting context, but I don't think they fully support it as they do not check the cancellation status. Usually the database driver should do this work. In our case the database is not context aware.

Should we add at the beginning of each or the core methods check if the context was cancelled?

switch {
case <-ctx.Done():
    return nil, ctx.Err()
default:
    // Context not cancelled, proceed.
}

[ethanfrey]

All database methods are accepting context, but I don't think they fully support it as they do not check the cancellation status. Usually the database driver should do this work. In our case the database is not context aware.

This makes sense. I think there is no cancel in iavl, except for the long-running processes, which we do handle with a context. That said, I like your idea of manually checking cancel in the beginning, so it does have some effect (even if it doesn't abort in middle of the I/O, it will break out long-running processes, like with a timeout)

[ruseinov]

Change seems good to me, and it would indeed be cool to have methods exit on context cancellation, effectively making them a noop if they were just called.

[husio]

I would love to merge this. There are a lot of conflicts by now. I can resolve all conflicts and bring the code to mergable state.
Do we want to apply this change? If yes, I can take care of finishing this pull request.

[ruseinov]

It does not seem like a good idea to do merge this now as we have decided to hold off on any interface breaking changes that touch multiple packages.

We either have to give up on the idea of keeping master launch-compatible, which in my opinion is an okay approach, or keep this open till we have a way forward.

To make this decision we'll need to have some direction from business.

[ethanfrey]

I think this should be done as a post-launch overhaul. It changes code, but doesn't break any external guarantees (how blockchain txs execute). So easy enough to add in an iov chain upgrade, say 1.0 -> 1.1