-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
docs: ADR-049 state sync hooks #10976
Changes from 9 commits
805f653
8eb5147
3b52898
4d05187
6b34562
86bee96
5689001
7e46ff9
bf67fc3
7990098
50ef809
aea3ea2
bbc3c52
d3fe008
9d82396
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,152 @@ | ||
# ADR 049: State Sync Hooks | ||
|
||
## Changelog | ||
|
||
- Jan 19, 2022: Initial Draft | ||
|
||
## Status | ||
|
||
Draft, Under Implementation | ||
|
||
## Abstract | ||
|
||
This ADR outlines a hooks-based mechanism for application modules to provide additional state (outside of the IAVL tree) to be used | ||
during state sync. | ||
|
||
## Context | ||
|
||
New clients use state-sync to download snapshots of module state from peers. Currently, the snapshot consists of a | ||
stream of `SnapshotStoreItem` and `SnapshotIAVLItem`, which means that application modules that define their state outside of the IAVL | ||
tree cannot include their state as part of the state-sync process. | ||
|
||
Note, Even though the module state data is outside of the tree, for determinism we require that the hash of the external data should | ||
be posted in the IAVL tree. | ||
|
||
## Decision | ||
|
||
A simple proposal based on our existing implementation is that, we can add two new message types: `SnapshotExtensionMeta` | ||
and `SnapshotExtensionPayload`, and they are appended to the existing multi-store stream with `SnapshotExtensionMeta` | ||
acting as a delimiter between extensions. As the chunk hashes should be able to ensure data integrity, we don't need | ||
a delimiter to mark the end of the snapshot stream. | ||
|
||
Besides, we provide `Snapshotter` and `ExtensionSnapshotter` interface for modules to implement snapshotters, which will handle both taking | ||
snapshot and the restoration. Each module could have mutiple snapshotters, and for modules with additional state, they should | ||
implement `ExtensionSnapshotter` as extension snapshotters. When setting up the application, the snapshot `Manager` should call | ||
`RegisterExtensions([]ExtensionSnapshotter…)` to register all the extension snapshotters. | ||
|
||
```proto | ||
// SnapshotItem is an item contained in a rootmulti.Store snapshot. | ||
// Except the exsiting SnapshotStoreItem and SnapshotIAVLItem, we add two new options for the item. | ||
message SnapshotItem { | ||
// item is the specific type of snapshot item. | ||
oneof item { | ||
SnapshotStoreItem store = 1; | ||
SnapshotIAVLItem iavl = 2 [(gogoproto.customname) = "IAVL"]; | ||
SnapshotExtensionMeta extension = 3; | ||
SnapshotExtensionPayload extension_payload = 4; | ||
} | ||
} | ||
|
||
// SnapshotExtensionMeta contains metadata about an external snapshotter. | ||
// One module may need multiple snapshotters, so each module may have multiple SnapshotExtensionMeta. | ||
message SnapshotExtensionMeta { | ||
// the name of the ExtensionSnapshotter, and it is registered to snapshotter manager when setting up the application | ||
string name = 1; | ||
// this is used by each ExtensionSnapshotter to decide the format of payloads included in SnapshotExtensionPayload message | ||
// it is used within the snapshotter/namespace, not global one for all modules | ||
uint32 format = 2; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. format is globally unique? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It’s used within the snapshotter/namespace. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. They don't need to globally unique. They need to be unique in the state machine. So if you are 3 different modules that use different types of state sync packets you need formats 1, 2, 3. I don't see why they should be global There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes, they are not global, they can be 1 at the same time. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @zmanian and @yihuang you are saying different things.
This was more or less what I meant by globally unique (on the chain). That you cannot have 2 modules on the same chain both using format 1.
This is what I understood the answer to be, that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To clarify there are two formats at play in the snapshot:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
technically speaking we can have duplicated There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is somewhat confusing. I recommend updating the field comment to be very explicit. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Up. It would be good to clarify if There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
} | ||
|
||
// SnapshotExtensionPayload contains payloads of an external snapshotter. | ||
message SnapshotExtensionPayload { | ||
bytes payload = 1; | ||
} | ||
``` | ||
|
||
The snapshot stream would look like as follows: | ||
|
||
```go | ||
// multi-store snapshot | ||
{SnapshotStoreItem | SnapshotIAVLItem, ...} | ||
// extension1 snapshot | ||
SnapshotExtensionMeta | ||
{SnapshotExtensionPayload, ...} | ||
// extension2 snapshot | ||
SnapshotExtensionMeta | ||
{SnapshotExtensionPayload, ...} | ||
Comment on lines
+75
to
+80
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What's the ordering of these extensions in the stream if we register multiple in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's alphabetical by name in current implementation. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sounds good, would like to see some wording for this. i suppose There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
yes |
||
``` | ||
|
||
We add an `extensions` field to snapshot `Manager` for extension snapshotters. The `multistore` snapshotter is a special one and it doesn't need a name because it is always placed at the beginning of the binary stream. | ||
|
||
```go | ||
type Manager struct { | ||
store *Store | ||
multistore types.Snapshotter | ||
extensions map[string]types.ExtensionSnapshotter | ||
mtx sync.Mutex | ||
operation operation | ||
chRestore chan<- io.ReadCloser | ||
chRestoreDone <-chan restoreDone | ||
restoreChunkHashes [][]byte | ||
restoreChunkIndex uint32 | ||
} | ||
``` | ||
|
||
For extension snapshotters that implement the `ExtensionSnapshotter` interface, their names should be registered to the snapshot `Manager` by | ||
calling `RegisterExtensions` when setting up the application. The snapshotters will handle both taking snapshot and restoration. | ||
|
||
```go | ||
// RegisterExtensions register extension snapshotters to manager | ||
func (m *Manager) RegisterExtensions(extensions ...types.ExtensionSnapshotter) error | ||
``` | ||
|
||
Except the existing `Snapshotter` interface for the `multistore`, we add `ExtensionSnapshotter` interface for the extension snapshotters. Three more function signatures: `SnapshotFormat()`, `SupportedFormats()` and `SnapshotName()` are added to `ExtensionSnapshotter`. | ||
adu-web3 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
```go | ||
// ExtensionSnapshotter is an extension Snapshotter that is appended to the snapshot stream. | ||
// ExtensionSnapshotter has an unique name and manages it's own internal formats. | ||
type ExtensionSnapshotter interface { | ||
Snapshotter | ||
|
||
// SnapshotName returns the name of snapshotter, it should be unique in the manager. | ||
SnapshotName() string | ||
|
||
// SnapshotFormat returns the default format used to take a snapshot. | ||
SnapshotFormat() uint32 | ||
|
||
// SupportedFormats returns a list of formats it can restore from. | ||
SupportedFormats() []uint32 | ||
} | ||
``` | ||
|
||
## Consequences | ||
|
||
As a result of this implementation, we are able to create snapshots of binary chunk stream for the state that we maintain outside of the IAVL Tree, CosmWasm blobs for example. And new clients are able to fetch sanpshots of state for all modules that have implemented the corresponding interface from peer nodes. | ||
|
||
|
||
### Backwards Compatibility | ||
|
||
This ADR introduces new proto message types, add field for extension snapshotters to snapshot `Manager`, add new function signatures `SnapshotFormat()`, `SupportedFormats()` to `Snapshotter` interface and add new `ExtensionSnapshotter` interface, so this is not backwards compatible. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It might be worth mentioning that the wire format of the snapshot is backward compatible when there's no There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sure, I think this is significant for application chains that don't have additional state to take snapshots. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, let's add some wording that the stream is backwards-compatible if no extension is registered. |
||
|
||
### Positive | ||
|
||
- State maintained outside of IAVL tree like CosmWasm blobs can create snapshots by implementing extension snapshotters, and being fetched by new clients via state-sync. | ||
|
||
### Neutral | ||
|
||
- All modules that maintain state outside of IAVL tree need to implement `ExtensionSnapshotter` and the snapshot `Manager` need to call `RegisterExtensions` when setting up the application. | ||
|
||
## Further Discussions | ||
|
||
While an ADR is in the DRAFT or PROPOSED stage, this section should contain a summary of issues to be solved in future iterations (usually referencing comments from a pull-request discussion). | ||
Later, this section can optionally list ideas or improvements the author or reviewers found during the analysis of this ADR. | ||
|
||
## Test Cases [optional] | ||
|
||
Test cases for an implementation are mandatory for ADRs that are affecting consensus changes. Other ADRs can choose to include links to test cases if applicable. | ||
|
||
## References | ||
|
||
- https://github.com/cosmos/cosmos-sdk/pull/10961 | ||
- https://github.com/cosmos/cosmos-sdk/issues/7340 | ||
- https://hackmd.io/gJoyev6DSmqqkO667WQlGw |
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.
Here there would be one
SnapshotExtensionMeta
for x/wasm module, maybe a second one for Ethermint module, etc?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.
Yes, one for each extension snapshotter. You can have multiple snapshotters.
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.
add a comment and hope that can help clarify the question here