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: add parent slot/blockhash to block #29855

Merged
merged 1 commit into from
Jan 25, 2023

Conversation

fanatid
Copy link
Contributor

@fanatid fanatid commented Jan 24, 2023

Problem

Ability to construct full block with geyser interface.

https://docs.rs/solana-storage-proto/1.14.13/solana_storage_proto/convert/generated/struct.ConfirmedBlock.html
Currently we can construct all transactions from notify_transaction but we still need previous_blockhash + parent_slot.

There also a question, do I understand correctly that number of transactions in the block should be equal to executed_transaction_count?

Summary of Changes

Add previous_blockhash + parent_slot to ReplicaBlockInfoV2

@mergify mergify bot added the community Community contribution label Jan 24, 2023
@mergify mergify bot requested a review from a team January 24, 2023 14:30
@steviez
Copy link
Contributor

steviez commented Jan 25, 2023

@lijunwangs - Can you take this one?

@lijunwangs lijunwangs self-requested a review January 25, 2023 04:14
@lijunwangs
Copy link
Contributor

@fanatid There is a workaround, you can obtain the parent slot and its block hash from the slot notification.

With block, you can get the slot, from the slot notification interface, you can know the parent, and hence the parent hash as well.

There also a question, do I understand correctly that number of transactions in the block should be equal to executed_transaction_count?

Yes, with this number you can definitely know when all transaction for that block have been notified and received.

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.

Looks good. This can be piggy backed with the executed transaction count change and released together without introducing another version

@lijunwangs lijunwangs added the CI Pull Request is ready to enter CI label Jan 25, 2023
@solana-grimes solana-grimes removed the CI Pull Request is ready to enter CI label Jan 25, 2023
@fanatid
Copy link
Contributor Author

fanatid commented Jan 25, 2023

There is a workaround, you can obtain the parent slot and its block hash from the slot notification.

Yes, we can but we already collect transactions with a workaround. Block version will be changed anyway, so to have parent slot/blockhash would be good 🙂

Should I change something for green CI?

@steviez
Copy link
Contributor

steviez commented Jan 25, 2023

Should I change something for green CI?

Nope, flaky test and I just restart. However, I will defer merging the commit (once all greens) to @lijunwangs

@lijunwangs lijunwangs merged commit b4d1769 into solana-labs:master Jan 25, 2023
@fanatid fanatid deleted the geyser-block-parent branch January 25, 2023 22:21
@fanatid
Copy link
Contributor Author

fanatid commented Jan 25, 2023

Awesome, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community Community contribution
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants