-
Notifications
You must be signed in to change notification settings - Fork 22
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
V1 contract state #243
Merged
Merged
V1 contract state #243
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
Instead just pass a closure that raises a Haskell exception.
The V1 contract state tests do not yet pass since the contract examples need to be revised for new state.
The paired implementation is somewhat hacky. It uses basic contract state and serialization to convert from one type of contract state to another. This is not ideal, but a more comprehensive solution requires much more extensive FFI (i.e., paired state constructions, and a paired construction of state in Rust). This is a lot of complications for something that ultimately does not matter much. The current solution is useful for basic testing, which is all that the paired state is used for.
abizjak
force-pushed
the
v1-contract-state
branch
from
February 22, 2022 07:47
fac53f8
to
a3ca9f8
Compare
Tests for smart contract iterators.
…node into v1-contract-state
5 tasks
Checkpointing tests for smart contracts. Co-authored-by: Aleš Bizjak <[email protected]>
Fixes the broken link to `smart-contracts` repository
* Introduced tests for cross messaging and checkpointing.
This does the following - revise the format of state storage to optimize for the common case - changes costs of some operations to better reflect reality.
Previously the cost did not include storage costs.
…e persistent tree.
td202
approved these changes
Apr 13, 2022
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small issues, but mostly fine.
concordium-consensus/src/Concordium/GlobalState/ContractStateV1.hs
Outdated
Show resolved
Hide resolved
concordium-consensus/src/Concordium/GlobalState/ContractStateV1.hs
Outdated
Show resolved
Hide resolved
concordium-consensus/src/Concordium/Scheduler/WasmIntegration/V1.hs
Outdated
Show resolved
Hide resolved
concordium-consensus/src/Concordium/Scheduler/WasmIntegration/V1.hs
Outdated
Show resolved
Hide resolved
concordium-consensus/tests/scheduler/SchedulerTests/SmartContracts/V1/CrossMessaging.hs
Outdated
Show resolved
Hide resolved
Bargsteen
approved these changes
Apr 21, 2022
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, just minor comments 👍
concordium-consensus/src/Concordium/GlobalState/Persistent/BlobStore.hs
Outdated
Show resolved
Hide resolved
concordium-consensus/tests/scheduler/SchedulerTests/SmartContracts/V1/Checkpointing.hs
Outdated
Show resolved
Hide resolved
concordium-consensus/tests/scheduler/SchedulerTests/SmartContracts/V1/Checkpointing.hs
Outdated
Show resolved
Hide resolved
concordium-consensus/tests/scheduler/SchedulerTests/SmartContracts/V1/Checkpointing.hs
Outdated
Show resolved
Hide resolved
concordium-consensus/tests/scheduler/SchedulerTests/SmartContracts/V1/Checkpointing.hs
Outdated
Show resolved
Hide resolved
concordium-consensus/tests/scheduler/SchedulerTests/SmartContracts/V1/Checkpointing.hs
Outdated
Show resolved
Hide resolved
abizjak
force-pushed
the
v1-contract-state
branch
from
April 21, 2022 09:42
c6e6b5d
to
b7857f2
Compare
7 tasks
5 tasks
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.
Purpose
Resolves #237
Resolves #254
Changes
The main change is the introduction of the
Concordium.GlobalState.ContractStateV1
module and its integration into global state and contract execution (in moduleConcordium.Scheduler.WasmIntegration.V1
).The remaining changes are essentially boilerplate. The main data structure implementation is in Concordium/concordium-wasm-smart-contracts#25 Rust, and that is also where it is used and integrated into the execution engine.
The implementation there exposes the minimal API through FFI so that the necessary abstractions can be implemented in the node. This in particular means
The remaining changes are essentially boilerplate changes to make it possible to use two different versions of the contract state in the scheduler. This involved introducing a new abstraction
ContractStateOperations
, analogous toAccountOperations
.Fallback entrypoints handling is essentially just in the Scheduler and is very straightforward.
Ugliness
The basic state implementation is what I would call not clean. It works, and I believe it is reasonable given the constraints of FFI, but it is nevertheless ugly. A similar ugliness is particularly notable in the paired implementation which uses serialization of contract state to transform it from one form to another, so that contracts only need to be executed once. I don't think there is another reasonable choice since we would have to duplicate the paired "host" implementation on the execution engine side. This is a lot of effort for little gain.
Limitations
The general limitation of state management, and state transfer on a protocol update remains. Namely that the entire state of all contracts is loaded into memory and serialized, and then deserialized at protocol update time. This should be addressed before the next protocol since already now the memory requirements are excessive. This will only be made worse if the new contract state is used. However this is not part of this pull request and is a big project on its own.
There is no intelligent caching of the state. This is in-line with the rest of the state, e.g., accounts since the current design of the node state does not really support an intelligent implementation of caching. Either everything is in-memory, or nothing is. This should also be revised fairly quickly with some form of LRU cache or similar to prevent excessive memory use of the node.
However this will be a substantial revision of the state. For now contract state behaves as the rest of the state.
Depends on
Checklist
hard-to-understand areas.