Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
fix(gno.land): make gno store cache respect rollbacks (#2319)
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()`.
- Loading branch information