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

Geyser: return real parent blockhash, or default #33873

Merged

Conversation

CriesofCarrots
Copy link
Contributor

Problem

Geyser's notify_block_metadata() calls Bank::parent_hash() to populate its parent_blockhash field. But Bank::parent_hash() returns a bank hash, not a blockhash (ie. latest blockhash in the bank).

Summary of Changes

Update code to get the correct parent_blockhash. This currently includes an unwrap_or_default, because Bank::parent() returns an Option. Before merging this, we need to understand in what situations the parent bank would be None when this notification is triggered.

Fixes #33829

@codecov
Copy link

codecov bot commented Oct 26, 2023

Codecov Report

Merging #33873 (9892266) into master (0c3cab7) will decrease coverage by 0.1%.
The diff coverage is 0.0%.

@@            Coverage Diff            @@
##           master   #33873     +/-   ##
=========================================
- Coverage    81.9%    81.9%   -0.1%     
=========================================
  Files         810      810             
  Lines      219291   219293      +2     
=========================================
- Hits       179691   179667     -24     
- Misses      39600    39626     +26     

@CriesofCarrots CriesofCarrots marked this pull request as ready for review November 6, 2023 16:55
Copy link
Contributor

@bw-solana bw-solana left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Contributor

@lijunwangs lijunwangs left a comment

Choose a reason for hiding this comment

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

Good catch!

@CriesofCarrots CriesofCarrots added the v1.17 PRs that should be backported to v1.17 label Nov 6, 2023
@CriesofCarrots
Copy link
Contributor Author

I am proposing a v1.17 backport of this one, since it is a bug fix to geyser data

@CriesofCarrots CriesofCarrots merged commit d6ac9be into solana-labs:master Nov 6, 2023
33 checks passed
mergify bot pushed a commit that referenced this pull request Nov 6, 2023
Return real parent blockhash, or default

(cherry picked from commit d6ac9be)
mergify bot added a commit that referenced this pull request Nov 6, 2023
…33873) (#33959)

Geyser: return real parent blockhash, or default (#33873)

Return real parent blockhash, or default

(cherry picked from commit d6ac9be)

Co-authored-by: Tyera <tyera@solana.com>
anwayde pushed a commit to firedancer-io/solana that referenced this pull request Nov 16, 2023
…olana-labs#33873) (solana-labs#33959)

Geyser: return real parent blockhash, or default (solana-labs#33873)

Return real parent blockhash, or default

(cherry picked from commit d6ac9be)

Co-authored-by: Tyera <tyera@solana.com>
anwayde pushed a commit to firedancer-io/solana that referenced this pull request Nov 16, 2023
…olana-labs#33873) (solana-labs#33959)

Geyser: return real parent blockhash, or default (solana-labs#33873)

Return real parent blockhash, or default

(cherry picked from commit d6ac9be)

Co-authored-by: Tyera <tyera@solana.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v1.17 PRs that should be backported to v1.17
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Invalid field in notify_block_metadata for geyser
3 participants