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

Conversation

@jeffwashington
Copy link
Contributor

@jeffwashington jeffwashington commented Jun 14, 2021

Problem

non-index hash calculation is being used in more places with more options, including replacing index scan versions.

Summary of Changes

Plumb through more verification compared to index-scan so tests and optional args to validator or ledger-tool verify can check hash calculations.
Fixes #

@jeffwashington jeffwashington force-pushed the d2 branch 7 times, most recently from 1fd3940 to a228b25 Compare June 14, 2021 21:28
@jeffwashington jeffwashington requested a review from sakridge June 14, 2021 21:32
@jeffwashington jeffwashington marked this pull request as ready for review June 14, 2021 21:32
@codecov
Copy link

codecov bot commented Jun 14, 2021

Codecov Report

Merging #17939 (cf9db2e) into master (471b341) will decrease coverage by 0.0%.
The diff coverage is 93.9%.

@@            Coverage Diff            @@
##           master   #17939     +/-   ##
=========================================
- Coverage    82.6%    82.6%   -0.1%     
=========================================
  Files         431      431             
  Lines      121341   121342      +1     
=========================================
- Hits       100262   100254      -8     
- Misses      21079    21088      +9     

process_options.accounts_db_caching_enabled,
process_options.limit_load_slot_count_from_snapshot,
process_options.shrink_ratio,
process_options.accounts_db_test_hash_calculation,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

here's where we pass true to verify hash calculations further down.

check_hash,
can_cached_slot_be_unflushed,
)?;
let (calculated_hash, calculated_lamports) = self
Copy link
Contributor Author

Choose a reason for hiding this comment

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

switch to a lower level function that lets us verify the hash calculation.

@jeffwashington jeffwashington merged commit f558b9b into solana-labs:master Jun 15, 2021
mergify bot pushed a commit that referenced this pull request Jun 16, 2021
(cherry picked from commit f558b9b)

# Conflicts:
#	core/tests/snapshots.rs
#	ledger/src/bank_forks_utils.rs
#	runtime/src/snapshot_utils.rs
mergify bot added a commit that referenced this pull request Jun 16, 2021
…#17996)

* verify bank hash on startup with ledger tool option (#17939)

(cherry picked from commit f558b9b)

# Conflicts:
#	core/tests/snapshots.rs
#	ledger/src/bank_forks_utils.rs
#	runtime/src/snapshot_utils.rs

* fix merge errors

Co-authored-by: Jeff Washington (jwash) <[email protected]>
Co-authored-by: Jeff Washington (jwash) <[email protected]>
@brooksprumo brooksprumo mentioned this pull request Aug 23, 2021
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.

2 participants