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

Always serialize block with logs #4635

Closed
benesjan opened this issue Feb 16, 2024 · 0 comments · Fixed by #4541
Closed

Always serialize block with logs #4635

benesjan opened this issue Feb 16, 2024 · 0 comments · Fixed by #4541
Assignees

Comments

@benesjan
Copy link
Contributor

Currently we have 2 ways to serialize a block: with and without logs. This is violating the toBuffer, fromBuffer API which we use all around the codebase as you now need to pass a separate param (withLogs: bool).

Originally this was setup this way because the idea was that nodes would fetch blocks without logs and then use some form of note discovery scheme to get the relevant encrypted logs. This is turning out to be an example of premature optimization and currently it's making the code ugly while it is questionable whether the non-log serving mode would ever be used and needed - end users will most likely just run a "super light client" (get archive root from Rollup contract on L1, fetch header with merkle proof from nodes, verify the merkle proof and then get the notes and note sibling paths from some note discovery service).

The objective of this task is to remove the 2 ways of serializing a block (I am of an opinion that in general when we have 2 ways of serializing an object we are most likely doing something wrong (maybe have 2 different types) and the code is turning out to be brittle (what kind of buffer have I received? with or without logs?)).

@github-project-automation github-project-automation bot moved this to Todo in A3 Feb 16, 2024
@sklppy88 sklppy88 self-assigned this Feb 16, 2024
@sklppy88 sklppy88 linked a pull request Feb 16, 2024 that will close this issue
sklppy88 added a commit that referenced this issue Feb 20, 2024
@github-project-automation github-project-automation bot moved this from Todo to Done in A3 Feb 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants