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

Bank hash xor#5573

Merged
sakridge merged 1 commit intosolana-labs:masterfrom
sakridge:bank-hash-xor
Sep 20, 2019
Merged

Bank hash xor#5573
sakridge merged 1 commit intosolana-labs:masterfrom
sakridge:bank-hash-xor

Conversation

@sakridge
Copy link
Copy Markdown
Contributor

Problem

hash_internal_state is slow and doesn't allow for incremental state checking.

Summary of Changes

Hash account state on load/store and then xor with an accumulator to create the bank hash.

Fixes #

Comment thread runtime/src/accounts_db.rs Outdated
Comment thread sdk/src/hash.rs Outdated
Comment thread sdk/src/hash.rs
@rob-solana
Copy link
Copy Markdown
Contributor

yeah, what you're touching is exactly what I'm touching :-( should be fun.

@sakridge
Copy link
Copy Markdown
Contributor Author

yeah, what you're touching is exactly what I'm touching :-( should be fun.

Would it help you think if we just merged a smaller PR with the BankHash and SlotHash types where BankHash is typedef'ed to just Hash for now?

Comment thread runtime/src/accounts_db.rs Outdated
@sakridge sakridge force-pushed the bank-hash-xor branch 3 times, most recently from edd20da to bc6b287 Compare August 22, 2019 17:53
@rob-solana
Copy link
Copy Markdown
Contributor

the conflict we'll have is with load()'s signature

no need to avoid each other, I could use the merge practice

@sakridge sakridge force-pushed the bank-hash-xor branch 8 times, most recently from 66363e0 to 308859d Compare August 25, 2019 17:52
@codecov
Copy link
Copy Markdown

codecov Bot commented Aug 26, 2019

Codecov Report

Merging #5573 into master will decrease coverage by 1.5%.
The diff coverage is 73%.

@@           Coverage Diff            @@
##           master   #5573     +/-   ##
========================================
- Coverage    71.7%   70.1%   -1.6%     
========================================
  Files         236     237      +1     
  Lines       43274   44571   +1297     
========================================
+ Hits        31033   31253    +220     
- Misses      12241   13318   +1077

@sakridge sakridge force-pushed the bank-hash-xor branch 7 times, most recently from 5727e90 to 9271902 Compare September 9, 2019 18:51
@sakridge sakridge force-pushed the bank-hash-xor branch 3 times, most recently from 15e93b0 to cdee022 Compare September 17, 2019 20:45
Comment thread runtime/src/accounts_db.rs
Comment thread runtime/src/accounts_db.rs Outdated
@sakridge sakridge marked this pull request as ready for review September 18, 2019 20:46
@sakridge sakridge force-pushed the bank-hash-xor branch 2 times, most recently from c2eeba5 to a6058f7 Compare September 19, 2019 21:49
@sakridge sakridge requested review from rob-solana and t-nelson and removed request for carllin September 19, 2019 22:07
Comment thread sdk/src/bank_hash.rs
let rvs = storage.accounts.append_accounts(&with_meta[infos.len()..]);
let rvs = storage
.accounts
.append_accounts(&with_meta[infos.len()..], &hashes);
Copy link
Copy Markdown
Contributor

@ryoqun ryoqun Jan 14, 2020

Choose a reason for hiding this comment

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

This is not correct; see here: #7797

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.

4 participants