Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: quick workaround for flat storage memtrie comparison #12592

Merged
merged 3 commits into from
Dec 10, 2024

Conversation

Trisfald
Copy link
Contributor

Quick fix to allow state comparison between flat storage and memtries only when the view over key value pairs is at the same height, for both of them.

Copy link
Contributor

@wacban wacban left a comment

Choose a reason for hiding this comment

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

LGTM

Approving to unblock but it would also be nice to assert that we actually executed at least one check.

Comment on lines 661 to 664
if let FlatStorageStatus::Ready(status) = client
.chain
.runtime_adapter
.get_flat_storage_manager()
Copy link
Contributor

Choose a reason for hiding this comment

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

mini nit: you also need the manager a few lines below, maybe move to a variable

Copy link
Member

@Longarithm Longarithm left a comment

Choose a reason for hiding this comment

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

^ just found out that trie_sanity_check.check_epochs checks that at least one comparison happened

Copy link

codecov bot commented Dec 10, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 70.18%. Comparing base (0b1bcc9) to head (d4b02e1).
Report is 2 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #12592   +/-   ##
=======================================
  Coverage   70.18%   70.18%           
=======================================
  Files         840      840           
  Lines      169987   169987           
  Branches   169987   169987           
=======================================
+ Hits       119299   119304    +5     
+ Misses      45621    45616    -5     
  Partials     5067     5067           
Flag Coverage Δ
backward-compatibility 0.16% <ø> (ø)
db-migration 0.16% <ø> (ø)
genesis-check 1.29% <ø> (ø)
linux 69.41% <ø> (ø)
linux-nightly 69.77% <ø> (-0.01%) ⬇️
pytests 1.59% <ø> (ø)
sanity-checks 1.40% <ø> (ø)
unittests 70.00% <ø> (+<0.01%) ⬆️
upgradability 0.20% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Trisfald Trisfald added this pull request to the merge queue Dec 10, 2024
Merged via the queue into near:master with commit 6bc4f56 Dec 10, 2024
27 checks passed
@Trisfald Trisfald deleted the resharding-test-comparison-fix branch December 10, 2024 20:53
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.

3 participants