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

Support archival queries by modifying the WorkingSet #1184

Merged
merged 16 commits into from
Dec 6, 2023

Conversation

dubbelosix
Copy link
Contributor

Description

This accomplishes support for archival / versioned queries against state-db and native-db. This is similar to #1170 but instead of modifying the Storage level, this modification is at the WorkingSet level

Linked Issues

Copy link

codecov bot commented Nov 27, 2023

Codecov Report

Merging #1184 (171b302) into nightly (c6a012f) will increase coverage by 0.0%.
The diff coverage is 90.2%.

Additional details and impacted files
Files Coverage Δ
full-node/db/sov-db/src/schema/tables.rs 89.0% <100.0%> (+2.6%) ⬆️
...ystem/module-implementations/sov-bank/src/token.rs 81.7% <ø> (-0.2%) ⬇️
...odule-system/sov-modules-api/src/containers/map.rs 96.5% <ø> (ø)
...odule-system/sov-modules-api/src/containers/mod.rs 100.0% <100.0%> (ø)
module-system/sov-modules-api/src/lib.rs 97.6% <ø> (ø)
module-system/sov-state/src/prover_storage.rs 94.3% <100.0%> (+0.3%) ⬆️
full-node/db/sov-db/src/native_db.rs 98.6% <96.2%> (-1.4%) ⬇️
...odule-system/sov-modules-core/src/storage/cache.rs 90.0% <90.9%> (+<0.1%) ⬆️
module-system/sov-modules-core/src/storage/mod.rs 89.2% <92.3%> (-3.5%) ⬇️
module-system/sov-state/src/zk_storage.rs 55.2% <0.0%> (-3.9%) ⬇️
... and 1 more

... and 1 file with indirect coverage changes

@dubbelosix dubbelosix marked this pull request as ready for review November 28, 2023 14:47
Copy link
Member

@neysofu neysofu left a comment

Choose a reason for hiding this comment

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

The PR looks good – I'd just like to make sure that we're not accumulating too much tech debt with this change, since it doesn't replace all instances of WorkingSet with generics so it leaves things in a somewhat messy state. With the addition of ArchivalWorkingSet, we also have too many WorkingSet-like structs – although that's definitely not this PR's fault. Let's brainstorm at standup!

module-system/sov-modules-api/src/containers/map.rs Outdated Show resolved Hide resolved
module-system/sov-modules-core/src/storage/scratchpad.rs Outdated Show resolved Hide resolved
module-system/sov-modules-core/src/storage/scratchpad.rs Outdated Show resolved Hide resolved
module-system/sov-modules-core/src/storage/scratchpad.rs Outdated Show resolved Hide resolved
Copy link
Member

@citizen-stig citizen-stig left a comment

Choose a reason for hiding this comment

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

Look good to me. I don't see any blockers to integrating fork management into it.

Changes in sov-db are not entierly new, native db just gets same functionality as state_db already has.

Copy link
Member

@bkolad bkolad left a comment

Choose a reason for hiding this comment

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

LGTM, would it be possible to add more tests in sov-modules-code/tests ? (In a separate PR)

@dubbelosix dubbelosix added this pull request to the merge queue Dec 6, 2023
Merged via the queue into nightly with commit a099cb3 Dec 6, 2023
16 checks passed
@dubbelosix dubbelosix deleted the dub/archival_workingset branch December 6, 2023 11:06
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.

None yet

4 participants