Skip to content

blockstore: scaffolding for chained merkle root conflict detection#719

Merged
mergify[bot] merged 2 commits intoanza-xyz:masterfrom
AshwinSekar:chain-dup-proof-scaffolding
Apr 10, 2024
Merged

blockstore: scaffolding for chained merkle root conflict detection#719
mergify[bot] merged 2 commits intoanza-xyz:masterfrom
AshwinSekar:chain-dup-proof-scaffolding

Conversation

@AshwinSekar
Copy link
Copy Markdown

Split from #102
The scaffolding needed to detect chained merkle root conflicts for sending duplicate proofs:

  • feature gate
  • window service processing
  • find previous consecutive erasure set
  • check forward / backwards chaining

Contributes to solana-labs#34897

Comment thread ledger/src/blockstore.rs
let candidate_erasure_set = ErasureSetId::new(slot, candidate_fec_set_index);
let candidate_erasure_meta: ErasureMeta = deserialize(candidate_erasure_meta.as_ref())?;

// Add this candidate to erasure metas to avoid blockstore lookup in future
Copy link
Copy Markdown
Author

@AshwinSekar AshwinSekar Apr 10, 2024

Choose a reason for hiding this comment

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

This is the only diff from the copy/paste in order to address this comment: #102 (comment)

@AshwinSekar AshwinSekar requested a review from behzadnouri April 10, 2024 18:39
@codecov-commenter
Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 61.21495% with 83 lines in your changes are missing coverage. Please review.

Project coverage is 81.9%. Comparing base (51f9972) to head (407d9b1).

Additional details and impacted files
@@            Coverage Diff            @@
##           master     #719     +/-   ##
=========================================
- Coverage    81.9%    81.9%   -0.1%     
=========================================
  Files         851      851             
  Lines      230722   230929    +207     
=========================================
+ Hits       189103   189159     +56     
- Misses      41619    41770    +151     

behzadnouri
behzadnouri previously approved these changes Apr 10, 2024
Comment thread sdk/src/feature_set.rs Outdated
}

pub mod chained_merkle_conflict_duplicate_proofs {
solana_sdk::declare_id!("chaie9S2zVfuxJKNRGkyTDokLwWxx6kD2ZLsqQHaDD8");
Copy link
Copy Markdown

@behzadnouri behzadnouri Apr 10, 2024

Choose a reason for hiding this comment

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

The risk of including the feature key here is that it will be misleading what minimum version is needed when deciding to activate the feature. ie. someone needs to recall what latest commit changed the associated logic.

Might be better to use a phony key for now and rekey once all the logic is in.

Copy link
Copy Markdown
Author

@AshwinSekar AshwinSekar Apr 10, 2024

Choose a reason for hiding this comment

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

good point, I put in a phoney key for now.
Will also create the feature gate issue on the final PR to make it clear.

@AshwinSekar AshwinSekar added automerge automerge Merge this Pull Request automatically once CI passes v1.18 labels Apr 10, 2024
@mergify
Copy link
Copy Markdown

mergify Bot commented Apr 10, 2024

Backports to the beta branch are to be avoided unless absolutely necessary for fixing bugs, security issues, and perf regressions. Changes intended for backport should be structured such that a minimum effective diff can be committed separately from any refactoring, plumbing, cleanup, etc that are not strictly necessary to achieve the goal. Any of the latter should go only into master and ride the normal stabilization schedule. Exceptions include CI/metrics changes, CLI improvements and documentation updates on a case by case basis.

@mergify mergify Bot merged commit 411fdc9 into anza-xyz:master Apr 10, 2024
mergify Bot pushed a commit that referenced this pull request Apr 10, 2024
)

* blockstore: scaffolding for chained merkle root conflict detection

* pr feedback: use dummy feature key until final plumbing

(cherry picked from commit 411fdc9)

# Conflicts:
#	sdk/src/feature_set.rs
AshwinSekar added a commit that referenced this pull request Apr 22, 2024
)

* blockstore: scaffolding for chained merkle root conflict detection

* pr feedback: use dummy feature key until final plumbing

(cherry picked from commit 411fdc9)

# Conflicts:
#	sdk/src/feature_set.rs
AshwinSekar added a commit that referenced this pull request Apr 23, 2024
)

* blockstore: scaffolding for chained merkle root conflict detection

* pr feedback: use dummy feature key until final plumbing

(cherry picked from commit 411fdc9)

# Conflicts:
#	sdk/src/feature_set.rs
AshwinSekar added a commit that referenced this pull request Apr 23, 2024
)

* blockstore: scaffolding for chained merkle root conflict detection

* pr feedback: use dummy feature key until final plumbing

(cherry picked from commit 411fdc9)

# Conflicts:
#	sdk/src/feature_set.rs
AshwinSekar added a commit that referenced this pull request Apr 23, 2024
…tion (backport of #719) (#729)

* blockstore: scaffolding for chained merkle root conflict detection (#719)

* blockstore: scaffolding for chained merkle root conflict detection

* pr feedback: use dummy feature key until final plumbing

(cherry picked from commit 411fdc9)

# Conflicts:
#	sdk/src/feature_set.rs

* fix feature_set conflicts

---------

Co-authored-by: Ashwin Sekar <ashwin@anza.xyz>
Co-authored-by: Ashwin Sekar <ashwin@solana.com>
anwayde pushed a commit to firedancer-io/agave that referenced this pull request Jul 23, 2024
…tion (backport of anza-xyz#719) (anza-xyz#729)

* blockstore: scaffolding for chained merkle root conflict detection (anza-xyz#719)

* blockstore: scaffolding for chained merkle root conflict detection

* pr feedback: use dummy feature key until final plumbing

(cherry picked from commit 411fdc9)

# Conflicts:
#	sdk/src/feature_set.rs

* fix feature_set conflicts

---------

Co-authored-by: Ashwin Sekar <ashwin@anza.xyz>
Co-authored-by: Ashwin Sekar <ashwin@solana.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

automerge automerge Merge this Pull Request automatically once CI passes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants