fix(sdk/vm): re-load iavl store from backup if empty #2568
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
the
📦 ⛰️ gno.land
|
Confirming it's working 👍 |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2568 +/- ##
==========================================
- Coverage 55.03% 55.03% -0.01%
==========================================
Files 595 595
Lines 79645 79662 +17
==========================================
+ Hits 43835 43841 +6
- Misses 32492 32505 +13
+ Partials 3318 3316 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
zivkovicmilos
approved these changes
Jul 10, 2024
ajnavarro
approved these changes
Jul 10, 2024
Leon-Africa
pushed a commit
to Leon-Africa/gno
that referenced
this pull request
Jul 10, 2024
This is a hack; during initialization, the iavlStore is backed up to the baseStore, and it is then recovered in Initialize if the iavlStore is found to be empty (but there's supposed to be packages inside). This is a hotfix for test4 non-validator nodes. It still requires to start up the non-validator node once, with the new changes applied, with a fresh db
gfanton
pushed a commit
to gfanton/gno
that referenced
this pull request
Jul 23, 2024
This is a hack; during initialization, the iavlStore is backed up to the baseStore, and it is then recovered in Initialize if the iavlStore is found to be empty (but there's supposed to be packages inside). This is a hotfix for test4 non-validator nodes. It still requires to start up the non-validator node once, with the new changes applied, with a fresh db
thehowl
added a commit
that referenced
this pull request
Sep 5, 2024
This pull request modifies the current `defaultStore` to support transactionality in the same way as our tm2 stores. A few new concepts are introduced: - A `TransactionStore` interface is added, which extends the `gnolang.Store` interface to support a Write() method. - Together with corresponding implementations allowing for transactionality on its cache maps, this means that the Gno store retained by the VM keeper is only modified atomically after a transaction has completed. - The tm2 `BaseApp` has the new "hooks" `BeginTxHook` and `EndTxHook`. The former is called right before starting a transaction, and the latter is called right after finishing it, together with the transaction result. - This allows us to plug in the Gno `TransactionalStore` in the `sdk.Context` through the `BeginTxHook`; and commit the result, if successful, in the `EndTxHook`. ## Overview of changes - `gno.land` - `pkg/gnoland/app.go`: the InitChainer is now additionally responsible of loading standard libraries. To separate the options related to app startup globally, and those to genesis, the InitChainer is now a method of its config struct, `InitChainerConfig`, embedded into the `AppOptions`. - `pkg/gnoland/app.go`: `NewAppOptions` is only used in `NewApp`, where most of its fields were modified, anyway. I replaced it by changing the `validate()` method to write default values. - `pkg/gnoland/node_inmemory.go`, `pkg/integration/testing_integration.go`: these changes were made necessary to support `gnoland restart` in our txtars, and supporting fast reloading of standard libraries (`LoadStdlibCached`). - `pkg/sdk/vm/keeper.go`: removed all the code to load standard libraries on Initialize, as it's now done in the InitChainer. The hack introduced in #2568 is no longer necessary as well. Other changes show how the Gno Store is injected and retrieved from the `sdk.Context`. - `gnovm/pkg/gnolang/store.go` - Fork and SwapStores have been removed, in favour of BeginTransaction. BeginTransaction creates a `TransactionalStore`; the "transaction-scoped" fields of the defaultStore are swapped with "transactional" versions (including the allocator, cacheObjects and cacheTypes/cacheNodes - the latter write to a `txLogMap`). - ClearObjectCache is still necessary for the case of a transaction with multiple messages. - The `Map` interface in `txlog` allows us to have a `txLog` data type stacked on top of another. This is useful for the cases where we use `BeginTransaction` in preprocess. (We previously used `Fork`.) See later in the "open questions" section. - I added an Iterator method on the `txlog.Map` interface - this will be compatible with [RangeFunc](https://go.dev/wiki/RangefuncExperiment), once we switch over to [go1.23](https://go.dev/doc/go1.23). - `tm2/pkg/sdk` - As previously mentioned, two hooks were added on the BaseApp to support injecting application code right before starting a transaction and then right after ending it; allowing us to inject the code relating to the Gno.land store while maintaining the modules decoupled - Other - `gnovm/pkg/gnolang/machine.go` has a one-line fix for a bug printing the machine, whereby it would panic if len(blocks) == 0. ## Open questions / notes - TransactionalStore should be a different implementation altogether; but decoupling the logic which is used within the stores without doing a massive copy-and-paste of the entire defaultStore implementation is non-trivial. See [this comment](#2319 (comment)), and [this PR](#2655) for a draft proposed store refactor, which would render the store more modular and testable. - There is an alternative implementation, which in micro-benchmarks would be somewhat faster, in place of the `txLog`; it's still in `gnolang/internal/txlog/txlog_test.go` where it also has benchmarks. See [1347c5f](1347c5f) for the solution which uses `bufferedTxMap`, without using the `txlog.Map` interface. The main problem with this approach is that it does not support stacking bufferedTxMaps, thus there is a different method to be used in preprocess - `preprocessFork()`.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a hack; during initialization, the iavlStore is backed up to the baseStore, and it is then recovered in Initialize if the iavlStore is found to be empty (but there's supposed to be packages inside).
This is a hotfix for test4 non-validator nodes.
It still requires to start up the non-validator node once, with the new changes applied, with a fresh db