feat(store-sync): rework blockLogsToStorage #1176
Conversation
🦋 Changeset detectedLatest commit: 43125df The changes in this PR will be included in the next version bump. This PR includes changesets to release 26 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
to double check my understanding, instead of passing in an object with handlers like like { setRecord, setField, deleteRecord } etc., users pass in a single handler storeOperations correct? for context why did you end up going this route instead of a handler per store operation?
Because the store operations are (and should be) grouped by block, so that you can execute them within the context of a database transaction. Otherwise it's hard to know when to begin/commit a database tx. It also helps us to avoid the work arounds we had before like |
| // I don't love carrying all these types through. Ideally this should be the shape of the thing we want, rather than the specific return type from a function. | ||
| export type StoreEventsLog = GetLogsResult<StoreEventsAbi>[number]; | ||
| export type BlockLogs = GroupLogsByBlockNumberResult<StoreEventsLog>[number]; |
There was a problem hiding this comment.
could you elaborate on this comment? having trouble following
There was a problem hiding this comment.
I was thinking that using the GetLogsResult import type ties it the shape defined in the other module (and its package version) and was worried it make it harder to pass in an object from another location (rather from this specific function in this specific module). But maybe that's not how these types actually work and they just compare shapes.
This worry comes from seeing weird type errors when certain package versions get out of alignment and is why we have most deps pinned to a specific version rather than a version range.
| // TODO: standardize on calling these "fields" or "values" or maybe "columns" | ||
| valueName: TValue; | ||
| value: Value<TConfig, TTable>[TValue]; |
There was a problem hiding this comment.
should we go with fieldName and fieldValue?
| [TTable in keyof TConfig["tables"]]: { | ||
| name: TTable; | ||
| key: Key<TConfig, TTable>; | ||
| record: Value<TConfig, TTable>; |
There was a problem hiding this comment.
should we go with value here based on the "consistent naming" doc?
| type TableKey = `${Address}:${TableNamespace}:${TableName}`; | ||
|
|
||
| // hacky fix for schema registration + metadata events spanning multiple blocks | ||
| // TODO: remove this once schema registration+metadata is one event or tx |
There was a problem hiding this comment.
| // TODO: remove this once schema registration+metadata is one event or tx | |
| // TODO: remove this once schema registration+metadata is one event or tx | |
| // (https://github.com/latticexyz/mud/pull/1182) |
| } | ||
| }); | ||
|
|
||
| // Then find all metadata events. These should follow schema registration events and be in the same block (since they're in the same tx). |
There was a problem hiding this comment.
seems like there is a comment missing above like "First find all schema events..." or something
| }); | ||
|
|
||
| // Then find all metadata events. These should follow schema registration events and be in the same block (since they're in the same tx). | ||
| // TODO: rework contracts so schemas+tables are combined and immutable |
There was a problem hiding this comment.
| // TODO: rework contracts so schemas+tables are combined and immutable | |
| // TODO: rework contracts so schemas+tables are combined and immutable | |
| // (https://github.com/latticexyz/mud/pull/1182) |
| log.args.data | ||
| ); | ||
| const valueNames = decodeAbiParameters(parseAbiParameters("string[]"), abiEncodedFieldNames as Hex)[0]; | ||
| // TODO: add key names to table registration when we refactor it |
There was a problem hiding this comment.
| // TODO: add key names to table registration when we refactor it | |
| // TODO: add key names to table registration when we refactor it | |
| // (https://github.com/latticexyz/mud/pull/1182) |
Pulled out of #1113