Skip to content
This repository was archived by the owner on Jan 22, 2025. It is now read-only.

Avoid unsorted recent_blockhashes for determinism#7918

Merged
ryoqun merged 2 commits intosolana-labs:masterfrom
ryoqun:deterministic-recent-blockhashes
Jan 23, 2020
Merged

Avoid unsorted recent_blockhashes for determinism#7918
ryoqun merged 2 commits intosolana-labs:masterfrom
ryoqun:deterministic-recent-blockhashes

Conversation

@ryoqun
Copy link
Copy Markdown
Contributor

@ryoqun ryoqun commented Jan 22, 2020

Problem

BinaryHeap::into_iter is not sorted, resulting in not-deterministic recent_blockhashes sysvar.

This is surprising as a part of the Rust community voiced.

From the API reference (Emphasis mine):

fn into_iter(self) -> IntoIter [src] [−]

Creates a consuming iterator, that is, one that moves each value out of the binary heap in arbitrary order. The binary heap cannot be used after calling this.

Summary of Changes

  • Use a sorted iterator which is originally intended. Sadly, that API is still experimental so I cherry-picked from the rust GitHub repository.
    • As for licenses both (us, solana and they, rust) use Apache Lisence 2.0, so there should no problem in that regard.

Background

Found while working on #7892

Fixes #6663

Comment thread sdk/src/sysvar/recent_blockhashes.rs
@codecov
Copy link
Copy Markdown

codecov Bot commented Jan 22, 2020

Codecov Report

Merging #7918 into master will decrease coverage by <.1%.
The diff coverage is 86.9%.

@@           Coverage Diff            @@
##           master   #7918     +/-   ##
========================================
- Coverage    81.8%   81.8%   -0.1%     
========================================
  Files         241     244      +3     
  Lines       52121   52170     +49     
========================================
+ Hits        42678   42716     +38     
- Misses       9443    9454     +11

@ryoqun ryoqun requested a review from t-nelson January 22, 2020 16:50
@t-nelson
Copy link
Copy Markdown
Contributor

Oof! How'd I miss that... 🤔

Nice catch! This is absolutely the intention

t-nelson
t-nelson previously approved these changes Jan 22, 2020
Copy link
Copy Markdown
Contributor

@t-nelson t-nelson left a comment

Choose a reason for hiding this comment

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

lgtm

@mergify mergify Bot dismissed t-nelson’s stale review January 23, 2020 00:55

Pull request has been modified.

@ryoqun ryoqun merged commit e54bf56 into solana-labs:master Jan 23, 2020
mergify Bot pushed a commit that referenced this pull request Jan 23, 2020
* Avoid unsorted recent_blockhashes for determinism

* Add a test: test_create_account_unsorted

(cherry picked from commit e54bf56)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants