feat: allow custom bloom production

[alarso16]

Why this should be merged

See ava-labs/strevm#120. To get bloom filters for a block when the header doesn't contain the correct bloom because of SAE's delayed settlement, you need a custom accessor to provide arbitrary bloom.

How this works

Introduces an optional interface which, if satisfied by filters.Backend implementations, is used to override the Bloom filter instead of retrieving it from the types.Header.

How this was tested

Existing tests unchanged. Integration test demonstrates calling of the override method when present.

[ARR4N]

To get bloom filters for a block when the header doesn't contain the correct bloom, you need a custom accessor to provide arbitrary bloom.

Because in SAE they correspond to the logs from the blocks being settled, while the RPC expects them to be from the transactions included by the block?

Existing test (do I need more?)

I think so. The existing tests only cover the backwards compatibility (i.e. when !ok). The new ones don't need to be extensive, only enough to demonstrate proper plumbing via an integration test. Basically, create a mini setup of how you plan to use everything you've introduced here and then assert correct behaviour. Additionally, it explains the motivation of the PR.

[JonathanOppenheimer]

I think integration test could be something like:

• Implement the BloomOverrider interface
• Verify that the custom blooms are properly indexed and retrieved
  • NewBloomIndexerService lifecycle
    • creation
    • indexing
    • querying
    • shutdown
    • (in that order)
• Verify bloom filtering works correctly with overridden blooms

[ARR4N]

I'm going to spend some time trying to refactor BloomIndexerService but wanted to share what I'm thinking thus far.

EDIT: decoupling things is working but I'll need a bit more time.

[alarso16]

@ARR4N I hate these tests, they're obstructive and test trivial implementation. Should I:
a. Not test this
b. Recreate rather complicated tests
c. Keep these rather useless tests
d. Any other ideas?

[ARR4N]

@ARR4N I hate these tests, they're obstructive and test trivial implementation. Should I: a. Not test this b. Recreate rather complicated tests c. Keep these rather useless tests d. Any other ideas?

This is where it's important to know the difference between the types of test doubles¹², and when it's appropriate to use each one. We could characterise what you've done as a spy or mock in that they expect a call to something.

Mocks are almost always the wrong solution because they test implementation, not behaviour. The rule of thumb I use is that they're suited to a state-changing call to an external system (i.e. when the "expect" is the behaviour).

All that said: rules are meant to be broken. You could instead use a stub that returns a canned Bloom filter and then test the behaviour from the outside. However the probabilistic nature of Bloom filters makes this imperfect, and the "expect" is actually a good alternative.

So, these aren't useless, they're just invasive. That's a very specific libevm problem (hence the extensive lesson) and in any other scenario wouldn't be an issue. I'll explore a bit more and see if we can find a balance.

Footnotes

1. https://testing.googleblog.com/2013/07/testing-on-toilet-know-your-test-doubles.html ↩

2. https://martinfowler.com/bliki/TestDouble.html ↩