feat: Combined layout for transactions

[infrmtcs]

No description provided.

[codecov]

Codecov Report

❌ Patch coverage is 68.18182% with 21 lines in your changes missing coverage. Please review.
✅ Project coverage is 75.76%. Comparing base (f208739) to head (ec27349).

Files with missing lines	Patch %	Lines
core/accessors.go	63.04%	7 Missing and 10 partials ⚠️
blockchain/blockchain.go	78.94%	2 Missing and 2 partials ⚠️

Additional details and impacted files

@@                    Coverage Diff                     @@
##           dat/block-transactions    #3304      +/-   ##
==========================================================
+ Coverage                   75.60%   75.76%   +0.16%     
==========================================================
  Files                         352      352              
  Lines                       33350    33352       +2     
==========================================================
+ Hits                        25214    25270      +56     
+ Misses                       6338     6268      -70     
- Partials                     1798     1814      +16     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[rodrodros]

Looks very good! Couldn't find any problems.

I've just added some nitpicks that I think it can help with its reading and some questions of two small things that are not super clear to me

[rodrodros]

nitpick: I know this line is outside the scope of the PR but could you split the assignment from the if

[infrmtcs]

Is there any particular reason why we favor breaking it into 2 lines? IMO it's a good feature to scope the declaration of err, which avoid (possibly) wrong reuse of err.

[rodrodros]

Similarly here

[rodrodros]

nitpick: parentization would help a bit here in my opinion

As I understand it, here you want to start through the oldest non-migrated block transactions or receipts, in case the migration was interrupted (I assume). I don't understand very well the logic though.

If the first block with transactions to migrate is t = 5 and the first one with receipts to migrate is r = 10 with batch size b = 2 then:

1. The minimum is min = min(t, r) -> min = min(10, 5) -> min = 5
2. The result is res = min - (min % 2)
3. Which then becomes -> res = 5 - (5 % 2) -> res = 5 - 1 -> res = 4.
4. The block we should then start from is block 4 and not 5 🤔 – what am I missing.

Additionally, if either transactions and receipts is zero, min block will be zero as well 🤔

[infrmtcs]

It's mainly to "normalize" the batch start and end to be multiples of batchSize, which is currently 10. The main benefits are to guarantee that multiple runs use consistent batches. For example, we always run batch 0 -> 9, 10 -> 19, 20 -> 29,... but never get batch 5 -> 14. This is to avoid unexpected batch that isn't created during the experiments, which may create unexpected spike in resource usage.

[infrmtcs]

Now I remember one more strict reason why we need this. Imagine that the actual next block to migrate is block 20, but block 20 doesn't have any transaction nor receipt. The the first batch will be 21 -> 30 instead of 20 -> 29.
Before rounds of benchmark, I chose batch of 100, which may this approach okay-ish because it only fails in the case where there are 100 consecutive empty blocks. However, now the batch size is 10, which means probably I'll need a better approach, since 10 consecutive empty blocks will fail.

[rodrodros]

What's the reasoning behind b.Data[:end]? It looks similar to just passing b.Data.

[infrmtcs]

First half of the data is for transactions, second half is for receipts. We're utilizing end of data as the end for the last item, so for transactions case, we need to slice at the beginning of the first receipt