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.

Sign up for GitHub

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

Jump to bottom

refactor: remove StorageDataSource::DbTrieOnly #10490

Merged
merged 1 commit into from
Jan 24, 2024
Merged

refactor: remove StorageDataSource::DbTrieOnly #10490

merged 1 commit into from
Jan 24, 2024

Conversation

pugachAG
Copy link
Contributor

It was introduced as a temporary measure which is no longer required.

@pugachAG pugachAG requested a review from Longarithm January 24, 2024 16:39
@pugachAG pugachAG requested a review from a team as a code owner January 24, 2024 16:39
shreyan-gupta
shreyan-gupta approved these changes Jan 24, 2024
View reviewed changes
Copy link
Contributor

@shreyan-gupta shreyan-gupta left a comment

Choose a reason for hiding this comment

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

Curious, is there ever a time we create a trie without flat storage? I can see we have KeyLookupMode::Trie and correspondingly StorageGetMode::Trie but do we even use it anywhere?

@pugachAG
Copy link
Contributor Author

@shreyan-gupta we need to create Trie without flat storage to support old protocol version before flat storage was introduced. And also for stateless validators when using recorded storage.

@pugachAG pugachAG enabled auto-merge January 24, 2024 16:53
@pugachAG pugachAG added this pull request to the merge queue Jan 24, 2024
@codecov Codecov
Copy link

codecov bot commented Jan 24, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (669df43) 71.91% compared to head (431c719) 71.92%.
Report is 1 commits behind head on master.

Additional details and impacted files 
@@           Coverage Diff           @@
##           master   #10490   +/-   ##
=======================================
  Coverage   71.91%   71.92%           
=======================================
  Files         720      720           
  Lines      145776   145766   -10     
  Branches   145776   145766   -10     
=======================================
+ Hits       104841   104842    +1     
- Misses      36102    36103    +1     
+ Partials     4833     4821   -12
Flag Coverage Δ
backward-compatibility 0.08% <ø> (+<0.01%) ⬆️
db-migration 0.08% <ø> (+<0.01%) ⬆️
genesis-check 1.26% <ø> (+<0.01%) ⬆️
integration-tests 36.86% <ø> (-0.05%) ⬇️
linux 71.35% <ø> (+0.03%) ⬆️
linux-nightly 71.46% <ø> (-0.02%) ⬇️
macos 53.45% <ø> (-1.77%) ⬇️
pytests 1.48% <ø> (+<0.01%) ⬆️
sanity-checks 1.27% <ø> (+<0.01%) ⬆️
unittests 67.89% <ø> (-0.02%) ⬇️
upgradability 0.13% <ø> (+<0.01%) ⬆️

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.

@pugachAG pugachAG added the C-housekeeping Category: Refactoring, cleanups, code quality label Jan 24, 2024
Merged via the queue into master with commit be9cb1a Jan 24, 2024
26 checks passed
@pugachAG pugachAG deleted the storage-data-source-cleanup branch January 24, 2024 17:15
Ekleog-NEAR pushed a commit to Ekleog-NEAR/nearcore that referenced this pull request Apr 2, 2024
Revert "refactor: remove StorageDataSource::DbTrieOnly (near#10490)"
baaaa04 
This reverts commit be9cb1a.
@Longarithm Longarithm mentioned this pull request Apr 12, 2024
Closed
@Trisfald Trisfald mentioned this pull request Jul 11, 2024
Merged
github-merge-queue bot pushed a commit that referenced this pull request Jul 12, 2024
@Trisfald
fix: block apply in the past (#11767)
837a29f 
This PR fixes commands such as `neard view-state apply` and `neard
view-state apply-range` when used on a block height that doesn't have
flat storage built for it.

It works by re enabling the feature of accessing trie nodes without
paying gas costs (removed in #10490).

Probably it also fixes #8741.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shreyan-gupta shreyan-gupta shreyan-gupta approved these changes

@Longarithm Longarithm Awaiting requested review from Longarithm

Assignees
No one assigned
Labels
C-housekeeping Category: Refactoring, cleanups, code quality
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@pugachAG @shreyan-gupta