fix: Resolve bugs introduced from new flashblocks timing logic

[sieniven]

📝 Summary

This PR resolves the bugs introduced in #380 from the refactoring of the flashblocks timing scheduler logic.

1. Refactors the scheduler to directly calculate the target flashblocks using the remaining time with a div_ceil instead. On computing the scheduler intervals, we will use the computed target flashblocks instead to populate the trigger timings.
2. Add option on the scheduled timings to handle late FCU case. We will directly skip building any flashblocks payload on the build_payload pipeline directly by handling None
3. Zero div protection on the scheduler::target_flashblocks function, which ensures non-zero target count is returned if intervals are present
4. Restore old behaviour by triggering the first flashblock building immediately once the scheduler worker thread is started, which will build the correct number of target flashblocks and prevent waste sequencing time
5. Correctly applies interval offsets on every flashblock intervals (previously only applied on first and last flashblock timing)

✅ I have completed the following steps:

• Run make lint
• Run make test
• Added tests (if applicable)

[akundaz]

I addressed all of your comments on #380. This PR is moving against the architectural decisions made in that PR and reintroduces the strong coupling between payload building, flashblock timing calculations, and the flashblock timer loop. One side effect is that the unit tests for timing calculations would no longer be testing the actual times when flashblocks would be triggered and (at least one that I can think of right now) log statements would become misleading.

If you're running into problems running the latest version of op-rbuilder we definitely want to know about them since they may be affecting others or us too. Details like your configuration and the logs you see would be very helpful so we can work together to fix things.

I want to share our product requirements here, since they may not be obvious from the code, and I wasn't too clear about it in my PR. For us it is very important that flashblocks are spaced evenly apart so that dapps and searchers can rely on a predictable service. If we're seeing that we're not getting evenly spaced flashblocks in production, then we want the appropriate logs to help us why, so we can figure out if we can mitigate the issue in the builder or correlate it to another issue upstream in the op stack.

If you build flashblocks at the start of each interval you might end up sending an extra flashblock per slot than if you chose to build at the end of the interval, but this just improves the reported metric for flashblocks sent without improving the underlying reliability. It introduces a tradeoff of increasing the time between the last flashblock of a block and the first flashblock of the next block, and this is more difficult for flashblock consumers to adapt to.

I do appreciate you digging into this since the timing logic is pretty hairy and it's good to have multiple people looking at it. I'm going to close this PR for now but I think the best path forward would be to start with a github issue or to reach out to us directly so we can align on changes that would be good for both of us.

[sieniven]

@akundaz thanks for sharing this and i understant better the intent now. Quick questions:

1. may i understand how flahsblocks may be uneven spaced out, since flashblock intervals (or flashblock time) is constant?
2. wouldnt this issue of an extra flashblock being built be easily resolved with a div_ceil?

It is also found, as detailed in the PR comment previously, the new fixes produces 1 less flashblock than what would be expected. This makes sense now since it seems like it is intended on your end for the builder to wait on your end up till the first interval offset, before triggering the first block building.

Our biggest concern with the fix is that it reduces the throughput of the chain, since we spend idling time waiting for the first flashblock offset, which could potentially be used to mint the first flashblock itself.

[akundaz]

@sieniven

I also want to add that lowering tx inclusion latency is important for us. A user should be able to send a tx late in the slot and still have it be included.

re uneven spacing. We have an offset for the last flashblock because we don't know exactly when getPayload will come so we need to finish block building early because of that. When combined with building at the start this skews more flashblocks to the front of the slot.

re extra flashblock. I'm not too sure what you're referring to with div_ceil.

Can you elaborate on what you mean by idle time? Most of the time is idle anyway since we're waiting between each tick to build flashblocks.

Also can you share your configuration that result in one less flashblock and share logs? We're not seeing and reduction in number of flashblocks being produced on our end from this.