Skip to content

Write default hash to account storage#469

Merged
HaoranYi merged 4 commits intoanza-xyz:masterfrom
HaoranYi:account_hash/default_hash
Apr 16, 2024
Merged

Write default hash to account storage#469
HaoranYi merged 4 commits intoanza-xyz:masterfrom
HaoranYi:account_hash/default_hash

Conversation

@HaoranYi
Copy link
Copy Markdown

@HaoranYi HaoranYi commented Mar 28, 2024

Problem

Storing default hash in account storage helps storage compression and saves snapshot storage.

Summary of Changes

Write default hash to account storage.

Note: This PR depends on #519 to be released. Once #519 is released. We can then merge this PR. For details, see the discussion here #476 (comment)

Fixes #

@HaoranYi HaoranYi force-pushed the account_hash/default_hash branch 2 times, most recently from 8ea3f0d to e1947f3 Compare March 29, 2024 15:04
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Mar 29, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.8%. Comparing base (cb2fd2b) to head (e5140e1).
Report is 8 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #469   +/-   ##
=======================================
  Coverage    81.8%    81.8%           
=======================================
  Files         851      851           
  Lines      231714   231712    -2     
=======================================
+ Hits       189649   189663   +14     
+ Misses      42065    42049   -16     

@HaoranYi HaoranYi force-pushed the account_hash/default_hash branch from e1947f3 to f60cafa Compare April 1, 2024 14:56
@HaoranYi HaoranYi changed the title write default hash to apendvec Write default hash to account storage Apr 1, 2024
@HaoranYi
Copy link
Copy Markdown
Author

Now #519 has been merged.
This PR should be ready to merge?

@HaoranYi HaoranYi marked this pull request as ready for review April 16, 2024 16:40
Comment thread runtime/tests/accounts.rs
);
} else {
assert_eq!(
db.load_account_hash(
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

should we ever be saving/loading a hash now? not sure about this test.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I think the intention of this test is to test load hash from cache vs storage.
What this test does is to make sure that when we load from the write cache, we should have the correct hash (pass=0), but when we load from the storage, we should have default hash (pass=1).

@HaoranYi HaoranYi force-pushed the account_hash/default_hash branch from f60cafa to d889fde Compare April 16, 2024 16:50
Comment thread accounts-db/src/append_vec.rs
@brooksprumo brooksprumo self-requested a review April 16, 2024 16:53
Copy link
Copy Markdown

@brooksprumo brooksprumo left a comment

Choose a reason for hiding this comment

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

:shipit:

Copy link
Copy Markdown

@jeffwashington jeffwashington left a comment

Choose a reason for hiding this comment

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

lgtm

@HaoranYi HaoranYi merged commit 03b5965 into anza-xyz:master Apr 16, 2024
@HaoranYi HaoranYi deleted the account_hash/default_hash branch April 16, 2024 21:04
michaelschem pushed a commit to michaelschem/agave that referenced this pull request Apr 20, 2024
* write default hash to apendvec

* fix get_pubkey_hash_for_slot

* fix tests

* fix rebase error

---------

Co-authored-by: HaoranYi <haoran.yi@solana.com>
OliverNChalk pushed a commit to OliverNChalk/agave that referenced this pull request Nov 11, 2025
* expose nonblocking clients from local-cluster

* fix typo
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants