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

v2.0: replay: extend last fec set check for 32+ retransmitter signed shreds (backport of #2101) #2192

Merged
merged 1 commit into from
Aug 28, 2024

Conversation

mergify[bot]
Copy link

@mergify mergify bot commented Jul 18, 2024

Problem

When the feature flag is turned on, we need to verify that the last fec set uses the new shred format.

Because the last fec set is not known when receiving shreds, and the new shred format is only used for the last set, we cannot perform this check until replay has completed.

Summary of Changes

Extend the check which compares the merkle roots of the last fec set to additionally check that they are all of the retransmitter shred variant.


This is an automatic backport of pull request #2101 done by [Mergify](https://mergify.com).

@mergify mergify bot requested a review from a team as a code owner July 18, 2024 20:42
@mergify mergify bot added the conflicts label Jul 18, 2024
Copy link
Author

mergify bot commented Jul 18, 2024

Cherry-pick of 93edb65 has failed:

On branch mergify/bp/v2.0/pr-2101
Your branch is up to date with 'origin/v2.0'.

You are currently cherry-picking commit 93edb65f7d.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	modified:   core/src/replay_stage.rs
	modified:   ledger/src/blockstore.rs
	modified:   ledger/src/blockstore_db.rs
	modified:   ledger/src/blockstore_processor.rs

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   ledger/src/shred.rs
	both modified:   sdk/src/feature_set.rs

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

@AshwinSekar
Copy link

This is the last piece of the duplicate block detection effort, which we hope to rollout in 2.x.
This PR ensures that any frozen bank must be comprised of shreds using the new shred format in the last FEC set when the feature flag is turned on.
The new shred format guarantees that the block has undergone checks to ensure that it is not a duplicate block.

@behzadnouri
Copy link

I think we need to first backport #1735 and #1840

@t-nelson
Copy link

are we planning to bp #1840

@behzadnouri
Copy link

are we planning to bp #1840

yes, but need to backport #1735 first

@behzadnouri behzadnouri force-pushed the mergify/bp/v2.0/pr-2101 branch from ccb0a09 to 2075492 Compare August 3, 2024 17:04
behzadnouri
behzadnouri previously approved these changes Aug 3, 2024
@behzadnouri
Copy link

I think this PR is ready for approval/merge.

bw-solana
bw-solana previously approved these changes Aug 5, 2024
Copy link

@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

@t-nelson
Copy link

@AshwinSekar can you resolve the merge conflict?

@AshwinSekar AshwinSekar dismissed stale reviews from bw-solana and behzadnouri via e37ff07 August 16, 2024 23:32
@AshwinSekar AshwinSekar force-pushed the mergify/bp/v2.0/pr-2101 branch from 2075492 to e37ff07 Compare August 16, 2024 23:32
@t-nelson
Copy link

f now downstream is busted

behzadnouri
behzadnouri previously approved these changes Aug 19, 2024
bw-solana
bw-solana previously approved these changes Aug 20, 2024
…#2101)

* replay: extend last fec set check for 32+ retransmitter signed shreds

* pr feedback: use separate feature flag

* pr feedback: is_retransmitter_signed -> is_retransmitter_signed_variant, false for legacy

* pr feedback: update doc comment fail -> error

* pr feedback: hash -> bank_hash for report metrics

* refactor metrics inside blockstore fn, return block_id for future use

* pr feedback: gate metrics reporting

* pr feedback: do not distinguish impossible combos, simplify check code

* pr feedback: remove report_metrics helper fn

* pr feedback: remove metric

* pr feedback: block_id -> last_fec_set_merkle_root
@AshwinSekar AshwinSekar dismissed stale reviews from bw-solana and behzadnouri via 72293fc August 23, 2024 20:02
@AshwinSekar AshwinSekar force-pushed the mergify/bp/v2.0/pr-2101 branch from fe4c2b3 to 72293fc Compare August 23, 2024 20:02
@AshwinSekar AshwinSekar merged commit 2088c58 into v2.0 Aug 28, 2024
39 checks passed
@AshwinSekar AshwinSekar deleted the mergify/bp/v2.0/pr-2101 branch August 28, 2024 04:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants