Remove block producer's Store trait

[hackaugusto]

The Store trait abstracts over the gRPC server, the main utility of this trait is to implement tests. However, this is not necessary, since tonic already has a strategy to mock the servers for testing, here is an example:

https://github.com/hyperium/tonic/blob/master/examples/src/mock/mock.rs

[bobbinth]

Would we basically just replace Store trait with the current DefaultStore struct? (i.e., Store trait goes away and DefaultStore struct becomes Store struct).

If we can do this while still mocking store for test purposes, I'm in favor or that (upon a quick look at the example, I didn't quite understand how it would work, but I also didn't look too long).

[hackaugusto]

Implementation wise it would something like this:

• Store trait is removed
  • This issue is not about ApplyBlock, that would stay there as far as this issue is concerned
• The existing impl Store for DefaultStore would become just an impl DefaultStore, as this code is doing some extra object translation.
• The MockStoreSuccess and MockStoreFailure would be rewritten, and implemented in terms of trait for the gRPC store interface
  • The linked example shows how this can be used. Basically the test spawns a task to run the test server, and uses a stream instead of a socket to do the communication.
• And if you want, the DefaultStore struct can be renamed to Store

[bobbinth]

Makes sense. Yes, I think this works. @plafer what do you think?

[plafer]

The one downside is that it makes it harder to move away from gRPC in the future. But if that's not much of a concern, then yes this approach is simpler and thus better.

[plafer]

I'm just remembering previous conversations we had about how when all components are running in the same process (including the database and rpc), we shouldn't need to use gRPC. This change would force us to keep using gRPC in that situation.

[hackaugusto]

The one downside is that it makes it harder to move away from gRPC in the future. But if that's not much of a concern, then yes this approach is simpler and thus better.

This is a fair point. But I think changes like this happen once or twice in a project's lifetime. So personally I would optimize for easy of readability here.

I'm just remembering previous conversations we had about how when all components are running in the same process (including the database and rpc), we shouldn't need to use gRPC. This change would force us to keep using gRPC in that situation.

This is true, we probably don't need the socket api, but we would need to do something similar to what is done in the tests, and communicate over a channel.