Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat(store): add field index to Store_SpliceDynamicData event #2279

Merged
merged 16 commits into from Feb 21, 2024
Merged

feat(store): add field index to Store_SpliceDynamicData event #2279

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
bf9f113
feat(store): emit field index in Store_SpliceDynamicData
yonadaaa Feb 19, 2024
b877f16
wip: add startWithinField to event
yonadaaa Feb 20, 2024
647fbe7
chore: docs
yonadaaa Feb 20, 2024
9a707e3
chore: add comments
yonadaaa Feb 20, 2024
ec81d27
chore: regenerate test data
yonadaaa Feb 20, 2024
8f4864e
test: update store-sync test
yonadaaa Feb 20, 2024
9743557
test: add back storeCore tests
yonadaaa Feb 20, 2024
7bccf9d
test: fix storecoredynamic test
yonadaaa Feb 20, 2024
70b35c6
docs: expand comment
yonadaaa Feb 21, 2024
40c33eb
chore: remove startWithinField from event
yonadaaa Feb 21, 2024
c22ce58
chore: changeset
yonadaaa Feb 21, 2024
9fcdbaf
chore: major bumps
yonadaaa Feb 21, 2024
ea035d0
Merge remote-tracking branch 'origin/main' into yonadaaa/emit-dynamic…
yonadaaa Feb 21, 2024
888bf76
chore: gas report
yonadaaa Feb 21, 2024
40950d7
test: fix story-sync tests
yonadaaa Feb 21, 2024
81be743
Update .changeset/rich-deers-doubt.md
holic Feb 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
20 changes: 12 additions & 8 deletions docs/pages/store/reference/store-core.mdx
View file
Open in desktop
Expand Up @@ -812,6 +812,8 @@ Emitted when dynamic data in the store is spliced.
event Store_SpliceDynamicData(
ResourceId indexed tableId,
bytes32[] keyTuple,
uint8 dynamicFieldIndex,
uint40 startWithinField,
uint48 start,
uint40 deleteCount,
PackedCounter encodedLengths,
Expand All @@ -821,14 +823,16 @@ event Store_SpliceDynamicData(

**Parameters**

| Name | Type | Description |
| ---------------- | --------------- | ------------------------------------------------------------------------- |
| `tableId` | `ResourceId` | The ID of the table where the data is spliced. |
| `keyTuple` | `bytes32[]` | An array representing the composite key for the record. |
| `start` | `uint48` | The start position in bytes for the splice operation. |
| `deleteCount` | `uint40` | The number of bytes to delete in the splice operation. |
| `encodedLengths` | `PackedCounter` | The encoded lengths of the dynamic data of the record. |
| `data` | `bytes` | The data to insert into the dynamic data of the record at the start byte. |
| Name | Type | Description |
| ------------------- | --------------- | ------------------------------------------------------------------------- |
| `tableId` | `ResourceId` | The ID of the table where the data is spliced. |
| `keyTuple` | `bytes32[]` | An array representing the composite key for the record. |
| `dynamicFieldIndex` | `uint8` | The index of the dynamic field. |
| `startWithinField` | `uint40` | The start byte position within the field for splicing. |
| `start` | `uint48` | The start position in bytes for the splice operation. |
| `deleteCount` | `uint40` | The number of bytes to delete in the splice operation. |
| `encodedLengths` | `PackedCounter` | The encoded lengths of the dynamic data of the record. |
| `data` | `bytes` | The data to insert into the dynamic data of the record at the start byte. |
Copy link
Member

@holic holic Feb 21, 2024 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seeing this all laid out, I am wondering:

  • Do we still like the Store_SpliceDynamicData name since splice events are now ~guaranteed to operate on one field at a time and it includes both information for splicing within the field and for all of the dynamic data? Wondering if this should be something like Store_SpliceDynamicField to clarify the intent.
  • With above, I am also wondering if it's worth clarifying some argument names like startWithinField to fieldStart and start -> dynamicDataStart.
  • "dynamic" in dynamicFieldIndex might be redundant since this is a Store_SpliceDynamicData event. Also worth clarifying if this is the field index in the list of fields for the whole schema or field index among dynamic fields (I think it's the former?)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need startWithinField? Couldn't that be computed from dynamicFieldIndex, start and encodedLengths? I'd prefer adding that one additional field to the event than refactoring the event name and other field names. The current naming feels very intuitive if you're thinking of the data as one byte blob, while the Store_SpliceDynamicField naming would be a little more intuitive if you're storing data in separate locations, but I don't there is a strong enough bias in either direction to change the name at this point.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

const fieldLengths = getFieldLengths(encodedLengths); // some sort of decoding function
const fieldStart = fieldLengths.slice(0, dynamicFieldIndex).reduce((sum, length) => sum + length, 0);
const startWithinField = start - fieldStart;

checks out ✅


### Store_DeleteRecord

Expand Down
20 changes: 12 additions & 8 deletions docs/pages/store/reference/store.mdx
View file
Open in desktop
Expand Up @@ -61,6 +61,8 @@ Emitted when dynamic data in the store is spliced.
event Store_SpliceDynamicData(
ResourceId indexed tableId,
bytes32[] keyTuple,
uint8 dynamicFieldIndex,
uint40 startWithinField,
uint48 start,
uint40 deleteCount,
PackedCounter encodedLengths,
Expand All @@ -70,14 +72,16 @@ event Store_SpliceDynamicData(

**Parameters**

| Name | Type | Description |
| ---------------- | --------------- | ------------------------------------------------------------------------- |
| `tableId` | `ResourceId` | The ID of the table where the data is spliced. |
| `keyTuple` | `bytes32[]` | An array representing the composite key for the record. |
| `start` | `uint48` | The start position in bytes for the splice operation. |
| `deleteCount` | `uint40` | The number of bytes to delete in the splice operation. |
| `encodedLengths` | `PackedCounter` | The encoded lengths of the dynamic data of the record. |
| `data` | `bytes` | The data to insert into the dynamic data of the record at the start byte. |
| Name | Type | Description |
| ------------------- | --------------- | ------------------------------------------------------------------------- |
| `tableId` | `ResourceId` | The ID of the table where the data is spliced. |
| `keyTuple` | `bytes32[]` | An array representing the composite key for the record. |
| `dynamicFieldIndex` | `uint8` | The index of the dynamic field. |
| `startWithinField` | `uint40` | The start byte position within the field for splicing. |
| `start` | `uint48` | The start position in bytes for the splice operation. |
| `deleteCount` | `uint40` | The number of bytes to delete in the splice operation. |
| `encodedLengths` | `PackedCounter` | The encoded lengths of the dynamic data of the record. |
| `data` | `bytes` | The data to insert into the dynamic data of the record at the start byte. |

### Store_DeleteRecord

Expand Down
10 changes: 5 additions & 5 deletions packages/store-sync/src/postgres-decoded/createStorageAdapter.test.ts
View file
Open in desktop
Expand Up @@ -50,7 +50,7 @@ describe("createStorageAdapter", async () => {
expect(await db.select().from(storageAdapter.tables.configTable)).toMatchInlineSnapshot(`
[
{
"blockNumber": 20n,
"blockNumber": 23n,
"chainId": 31337,
"version": "0.0.4",
},
Expand All @@ -70,8 +70,8 @@ describe("createStorageAdapter", async () => {
).toMatchInlineSnapshot(`
[
{
"address": "0x2964aF56c8aACdE425978a28b018956D21cF50f0",
"blockNumber": 20n,
"address": "0x30dd55194071DE5B8620A14d6A2890eE3227C68a",
"blockNumber": 23n,
"dynamicData": "0x000001a400000045",
"encodedLengths": "0x0000000000000000000000000000000000000000000000000800000000000008",
"isDeleted": false,
Expand All @@ -89,7 +89,7 @@ describe("createStorageAdapter", async () => {
expect(tables).toMatchInlineSnapshot(`
[
{
"address": "0x2964aF56c8aACdE425978a28b018956D21cF50f0",
"address": "0x30dd55194071DE5B8620A14d6A2890eE3227C68a",
"keySchema": {},
"name": "NumberList",
"namespace": "",
Expand All @@ -106,7 +106,7 @@ describe("createStorageAdapter", async () => {
[
{
"__keyBytes": "0x",
"__lastUpdatedBlockNumber": 20n,
"__lastUpdatedBlockNumber": 23n,
"value": [
420,
69,
Expand Down
6 changes: 3 additions & 3 deletions packages/store-sync/src/postgres/createStorageAdapter.test.ts
View file
Open in desktop
Expand Up @@ -48,7 +48,7 @@ describe("createStorageAdapter", async () => {
expect(await db.select().from(storageAdapter.tables.configTable)).toMatchInlineSnapshot(`
[
{
"blockNumber": 20n,
"blockNumber": 23n,
"chainId": 31337,
"version": "0.0.4",
},
Expand All @@ -68,8 +68,8 @@ describe("createStorageAdapter", async () => {
).toMatchInlineSnapshot(`
[
{
"address": "0x2964aF56c8aACdE425978a28b018956D21cF50f0",
"blockNumber": 20n,
"address": "0x30dd55194071DE5B8620A14d6A2890eE3227C68a",
"blockNumber": 23n,
"dynamicData": "0x000001a400000045",
"encodedLengths": "0x0000000000000000000000000000000000000000000000000800000000000008",
"isDeleted": false,
Expand Down
16 changes: 8 additions & 8 deletions packages/store-sync/src/sqlite/sqliteStorage.test.ts
View file
Open in desktop
Expand Up @@ -64,7 +64,7 @@ describe("sqliteStorage", async () => {
{
"chainId": 31337,
"lastError": null,
"lastUpdatedBlockNumber": 20n,
"lastUpdatedBlockNumber": 23n,
"schemaVersion": 1,
},
]
Expand All @@ -73,11 +73,11 @@ describe("sqliteStorage", async () => {
expect(db.select().from(mudStoreTables).where(eq(mudStoreTables.name, "NumberList")).all()).toMatchInlineSnapshot(`
[
{
"address": "0x2964aF56c8aACdE425978a28b018956D21cF50f0",
"id": "0x2964aF56c8aACdE425978a28b018956D21cF50f0____NumberList",
"address": "0x30dd55194071DE5B8620A14d6A2890eE3227C68a",
"id": "0x30dd55194071DE5B8620A14d6A2890eE3227C68a____NumberList",
"keySchema": {},
"lastError": null,
"lastUpdatedBlockNumber": 20n,
"lastUpdatedBlockNumber": 23n,
"name": "NumberList",
"namespace": "",
"schemaVersion": 1,
Expand All @@ -93,11 +93,11 @@ describe("sqliteStorage", async () => {
expect(tables).toMatchInlineSnapshot(`
[
{
"address": "0x2964aF56c8aACdE425978a28b018956D21cF50f0",
"id": "0x2964aF56c8aACdE425978a28b018956D21cF50f0____NumberList",
"address": "0x30dd55194071DE5B8620A14d6A2890eE3227C68a",
"id": "0x30dd55194071DE5B8620A14d6A2890eE3227C68a____NumberList",
"keySchema": {},
"lastError": null,
"lastUpdatedBlockNumber": 20n,
"lastUpdatedBlockNumber": 23n,
"name": "NumberList",
"namespace": "",
"schemaVersion": 1,
Expand All @@ -117,7 +117,7 @@ describe("sqliteStorage", async () => {
"__encodedLengths": "0x0000000000000000000000000000000000000000000000000800000000000008",
"__isDeleted": false,
"__key": "0x",
"__lastUpdatedBlockNumber": 20n,
"__lastUpdatedBlockNumber": 23n,
"__staticData": null,
"value": [
420,
Expand Down
52 changes: 26 additions & 26 deletions packages/store/gas-report.json
View file
Open in desktop
Expand Up @@ -15,7 +15,7 @@
"file": "test/Callbacks.t.sol",
"test": "testSetAndGet",
"name": "Callbacks: set field",
"gasUsed": 55829
"gasUsed": 56409
},
{
"file": "test/Callbacks.t.sol",
Expand All @@ -27,7 +27,7 @@
"file": "test/Callbacks.t.sol",
"test": "testSetAndGet",
"name": "Callbacks: push 1 element",
"gasUsed": 32114
"gasUsed": 32694
},
{
"file": "test/FieldLayout.t.sol",
Expand Down Expand Up @@ -621,25 +621,25 @@
"file": "test/StoreCoreDynamic.t.sol",
"test": "testPopFromSecondField",
"name": "pop from field (cold, 1 slot, 1 uint32 item)",
"gasUsed": 17819
"gasUsed": 18398
},
{
"file": "test/StoreCoreDynamic.t.sol",
"test": "testPopFromSecondField",
"name": "pop from field (warm, 1 slot, 1 uint32 item)",
"gasUsed": 11825
"gasUsed": 12406
},
{
"file": "test/StoreCoreDynamic.t.sol",
"test": "testPopFromThirdField",
"name": "pop from field (cold, 2 slots, 10 uint32 items)",
"gasUsed": 15588
"gasUsed": 16167
},
{
"file": "test/StoreCoreDynamic.t.sol",
"test": "testPopFromThirdField",
"name": "pop from field (warm, 2 slots, 10 uint32 items)",
"gasUsed": 11594
"gasUsed": 12174
},
{
"file": "test/StoreCoreGas.t.sol",
Expand Down Expand Up @@ -705,7 +705,7 @@
"file": "test/StoreCoreGas.t.sol",
"test": "testHooks",
"name": "register subscriber",
"gasUsed": 56304
"gasUsed": 56877
},
{
"file": "test/StoreCoreGas.t.sol",
Expand All @@ -729,7 +729,7 @@
"file": "test/StoreCoreGas.t.sol",
"test": "testHooksDynamicData",
"name": "register subscriber",
"gasUsed": 56304
"gasUsed": 56877
},
{
"file": "test/StoreCoreGas.t.sol",
Expand All @@ -741,7 +741,7 @@
"file": "test/StoreCoreGas.t.sol",
"test": "testHooksDynamicData",
"name": "set (dynamic) field on table with subscriber",
"gasUsed": 23856
"gasUsed": 25009
},
{
"file": "test/StoreCoreGas.t.sol",
Expand All @@ -753,13 +753,13 @@
"file": "test/StoreCoreGas.t.sol",
"test": "testPushToDynamicField",
"name": "push to field (1 slot, 1 uint32 item)",
"gasUsed": 9250
"gasUsed": 9831
},
{
"file": "test/StoreCoreGas.t.sol",
"test": "testPushToDynamicField",
"name": "push to field (2 slots, 10 uint32 items)",
"gasUsed": 31922
"gasUsed": 32503
},
{
"file": "test/StoreCoreGas.t.sol",
Expand Down Expand Up @@ -855,7 +855,7 @@
"file": "test/StoreCoreGas.t.sol",
"test": "testSetAndGetField",
"name": "set dynamic field (1 slot, first dynamic field)",
"gasUsed": 53732
"gasUsed": 54314
},
{
"file": "test/StoreCoreGas.t.sol",
Expand All @@ -867,7 +867,7 @@
"file": "test/StoreCoreGas.t.sol",
"test": "testSetAndGetField",
"name": "set dynamic field (1 slot, second dynamic field)",
"gasUsed": 31957
"gasUsed": 32539
},
{
"file": "test/StoreCoreGas.t.sol",
Expand Down Expand Up @@ -909,13 +909,13 @@
"file": "test/StoreCoreGas.t.sol",
"test": "testUpdateInDynamicField",
"name": "update in field (1 slot, 1 uint32 item)",
"gasUsed": 8610
"gasUsed": 9190
},
{
"file": "test/StoreCoreGas.t.sol",
"test": "testUpdateInDynamicField",
"name": "push to field (2 slots, 6 uint64 items)",
"gasUsed": 9053
"gasUsed": 9634
},
{
"file": "test/StoreHook.t.sol",
Expand Down Expand Up @@ -945,13 +945,13 @@
"file": "test/StoreHooks.t.sol",
"test": "testOneSlot",
"name": "StoreHooks: set field with one elements (cold)",
"gasUsed": 57824
"gasUsed": 58404
},
{
"file": "test/StoreHooks.t.sol",
"test": "testTable",
"name": "StoreHooks: set field (cold)",
"gasUsed": 57824
"gasUsed": 58403
},
{
"file": "test/StoreHooks.t.sol",
Expand All @@ -963,25 +963,25 @@
"file": "test/StoreHooks.t.sol",
"test": "testTable",
"name": "StoreHooks: push 1 element (cold)",
"gasUsed": 12202
"gasUsed": 12782
},
{
"file": "test/StoreHooks.t.sol",
"test": "testTable",
"name": "StoreHooks: pop 1 element (warm)",
"gasUsed": 9506
"gasUsed": 10087
},
{
"file": "test/StoreHooks.t.sol",
"test": "testTable",
"name": "StoreHooks: push 1 element (warm)",
"gasUsed": 10218
"gasUsed": 10799
},
{
"file": "test/StoreHooks.t.sol",
"test": "testTable",
"name": "StoreHooks: update 1 element (warm)",
"gasUsed": 29452
"gasUsed": 30033
},
{
"file": "test/StoreHooks.t.sol",
Expand All @@ -993,19 +993,19 @@
"file": "test/StoreHooks.t.sol",
"test": "testTable",
"name": "StoreHooks: set field (warm)",
"gasUsed": 29965
"gasUsed": 30548
},
{
"file": "test/StoreHooks.t.sol",
"test": "testThreeSlots",
"name": "StoreHooks: set field with three elements (cold)",
"gasUsed": 80515
"gasUsed": 81094
},
{
"file": "test/StoreHooks.t.sol",
"test": "testTwoSlots",
"name": "StoreHooks: set field with two elements (cold)",
"gasUsed": 80427
"gasUsed": 81006
},
{
"file": "test/StoreHooksColdLoad.t.sol",
Expand Down Expand Up @@ -1035,13 +1035,13 @@
"file": "test/StoreHooksColdLoad.t.sol",
"test": "testPop",
"name": "StoreHooks: pop 1 element (cold)",
"gasUsed": 17942
"gasUsed": 18522
},
{
"file": "test/StoreHooksColdLoad.t.sol",
"test": "testUpdate",
"name": "StoreHooks: update 1 element (cold)",
"gasUsed": 19906
"gasUsed": 20485
},
{
"file": "test/StoreSwitch.t.sol",
Expand Down