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

Fix nil pointer in the block header logs #2267

Merged
merged 2 commits into from
Feb 16, 2022

Conversation

leszko
Copy link
Contributor

@leszko leszko commented Feb 15, 2022

What does this pull request do? Explain your changes. (required)
Fixes a bug introduced in #2222, block header logs were not preserved while enriching block with L1 Number.

Specific updates (required)

How did you test each of these updates (required)

Unit test + checked in the debug that events returned from L2 API may not contain logs.

Does this pull request close any open issues?

fix #2265

Checklist:

@leszko leszko requested a review from yondonfu February 15, 2022 14:20
Copy link
Member

@yondonfu yondonfu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - just one clarifying question

if err != nil {
return header, err
}
header.L1BlockNumber = fetchedBlock.L1BlockNumber
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah so the problem was that if header contained logs that were previously added to it we could end up returning a MiniHeader returned by HeaderByHash that does not have those logs. And the fix here is to make sure we copy over the L1BlockNumber from the MiniHeader returned by HeaderByHash to the header that is passed in to ensure that if the header passed in contains logs that they are preserved?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, exactly as you described. Before, I thought that what we get from the RPC call already include in logs, but apparently that's not the case.

@leszko leszko merged commit 17a93a6 into livepeer:master Feb 16, 2022
@leszko leszko deleted the 2265-fix-nil-pointer-in-watchers branch February 16, 2022 15:39
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.

Nil Pointer in watchers while running in Arbitrum
2 participants