Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fixed PushBatchStoreStateReq message #156

Merged
merged 5 commits into from
Jul 22, 2024
Merged

fixed PushBatchStoreStateReq message #156

merged 5 commits into from
Jul 22, 2024

Conversation

pompon0
Copy link
Collaborator

@pompon0 pompon0 commented Jul 22, 2024

it was pushing the last batch, instead of just a batch number.
Additionally I've:

  • tweaked names a bit
  • replaced context() with wrap() to propagate cancellation
  • fixed gossipnet msg size limits
  • added Debug for the public type

@pompon0 pompon0 requested a review from aakoshh July 22, 2024 14:44
@aakoshh
Copy link
Contributor

aakoshh commented Jul 22, 2024

Great, changing BatchStoreState::last from a SyncBatch to a BatchNumber makes a whole lot more sense then unconditionally pushing all the L2 blocks to gossip peers after each L1 batch whether they need it or not 👍 We discussed this with @brunoffranca and @RomanBrodetski on standups but none of us were sure whether it's intentional, although the fact that the .proto file said it was a "QC" made it suspicious that it was a copy paste error from the similar block related artifacts.

Perhaps the title can be updated to reflect this change as it will also have to be fixed on the era side, ie. a bit more significant than a nit.

@pompon0 pompon0 changed the title fixed a bunch of nits fixed PushBatchStoreStateReq message Jul 22, 2024
@pompon0 pompon0 merged commit 99df300 into main Jul 22, 2024
5 checks passed
@pompon0 pompon0 deleted the gprusak-nits branch July 22, 2024 17:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants