adjust query to support ordering #107

45930 · 2024-10-21T21:20:59Z

Fixes: #106

This PR adds sequenceNumber, as well as zkappAccountUpdateIds to the graphQL schema, which will expose enough information for clients to verify the order in which actions were processed by consensus. It also attempts to return actions in that order, and exposes the ordering spec in a unit test.

We also overhaul the test contract and test suite to emit more random and different values so that sequential events and actions can be identified by their data.

I have tested this branch locally against the sibling branch in o1js.

deepthiskumar

I have to better understand the design of this API but my initial reaction is-

sequenceNumber makes more sense under transactionInfo since it is a property of the transaction. Also it needs block-height (assuming the API returns canonical blocks only) to disambiguate transactions with same sequence numbers but are part of different blocks. So the client can order by block height and sequence number
Same with zkappAccountUpdateIds since it describes the order of the account updates within a transaction. Also, I think we'll want to expose the index of the account-id that this action corresponds to. A client can then order by block-height + sequence-number + account-update-sequence-number
zkappEventElementIds: Similar to zkappAccountUpdateIds because events/actions are a list of field elements. The API returns an array of field elements for both actions and events but this array may not be in the same order as it was in the account update. So we might want to use the ordering in zkappEventElementIds from zke.element_ids for actions and events respectively. Finally the order for actions/events for an account across a few different blocks will be based on block-height + transaction-sequence-number + account-sequence-number + event-sequence-number.

45930 · 2024-10-23T15:28:23Z

Yes, I will have to work on the format. Here's what I can produce on the current branch:

{
    "data": {
        "actions": [
            {
                "blockInfo": {
                    "stateHash": "3NKZqA81s8iHMQc4vcJxrcx6epVB7C9vBD8oV7K3EpCZzvfebCtP",
                    "timestamp": "1729695587000",
                    "height": 69,
                    "parentHash": "3NLAneCAdNdj2u6DW5ycvUAvuJZP4mMain7eLWEU6QvL9ZTmZ53i",
                    "chainStatus": "canonical",
                    "distanceFromMaxBlockHeight": 36,
                    "globalSlotSinceGenesis": 132
                },
                "actionState": {
                    "actionStateOne": "25619375377485241455325506385026113155326956491463643967090964080969742671789",
                    "actionStateTwo": "25079927036070901246064867767436987657692091363973573142121686150614948079097",
                    "actionStateThree": "25079927036070901246064867767436987657692091363973573142121686150614948079097",
                    "actionStateFour": "25079927036070901246064867767436987657692091363973573142121686150614948079097",
                    "actionStateFive": "25079927036070901246064867767436987657692091363973573142121686150614948079097"
                },
                "actionData": [
                    {
                        "data": [
                            "24650",
                            "1",
                            "63073",
                            "4611450100234868895111341341933539196411650948711574062334330094836938200670",
                            "1"
                        ],
                        "accountUpdateId": "27",
                        "transactionInfo": {
                            "status": "applied",
                            "hash": "5JvAynd7NAHEiMqCpgZD85WCzLniBv18eSEJyuvNVK8pQL8Fi5kx",
                            "memo": "E4YM2vTHhWEg66xpj52JErHUBU4pZ1yageL4TVDDpTTSsv8mK6YaH"
                        },
                        "sequenceNumber": 2,
                        "zkappAccountUpdateIds": [
                            27,
                            28
                        ],
                        "zkappEventElementIds": [
                            23
                        ]
                    },
                    {
                        "data": [
                            "24651",
                            "1",
                            "63073",
                            "4611450100234868895111341341933539196411650948711574062334330094836938200670",
                            "1"
                        ],
                        "accountUpdateId": "28",
                        "transactionInfo": {
                            "status": "applied",
                            "hash": "5JvAynd7NAHEiMqCpgZD85WCzLniBv18eSEJyuvNVK8pQL8Fi5kx",
                            "memo": "E4YM2vTHhWEg66xpj52JErHUBU4pZ1yageL4TVDDpTTSsv8mK6YaH"
                        },
                        "sequenceNumber": 2,
                        "zkappAccountUpdateIds": [
                            27,
                            28
                        ],
                        "zkappEventElementIds": [
                            24
                        ]
                    },
                    {
                        "data": [
                            "73587",
                            "1",
                            "92297",
                            "8253926607854573680301530978054957179571865656842078211022112667228198387920",
                            "1"
                        ],
                        "accountUpdateId": "25",
                        "transactionInfo": {
                            "status": "applied",
                            "hash": "5JumftuGBSY8x4yHpqvpg4gDWUnEpuZ8335Nxr5FgsXxmUsqmX8J",
                            "memo": "E4YM2vTHhWEg66xpj52JErHUBU4pZ1yageL4TVDDpTTSsv8mK6YaH"
                        },
                        "sequenceNumber": 1,
                        "zkappAccountUpdateIds": [
                            25,
                            26
                        ],
                        "zkappEventElementIds": [
                            21
                        ]
                    },
                    {
                        "data": [
                            "73588",
                            "1",
                            "92297",
                            "8253926607854573680301530978054957179571865656842078211022112667228198387920",
                            "1"
                        ],
                        "accountUpdateId": "26",
                        "transactionInfo": {
                            "status": "applied",
                            "hash": "5JumftuGBSY8x4yHpqvpg4gDWUnEpuZ8335Nxr5FgsXxmUsqmX8J",
                            "memo": "E4YM2vTHhWEg66xpj52JErHUBU4pZ1yageL4TVDDpTTSsv8mK6YaH"
                        },
                        "sequenceNumber": 1,
                        "zkappAccountUpdateIds": [
                            25,
                            26
                        ],
                        "zkappEventElementIds": [
                            22
                        ]
                    },
                    {
                        "data": [
                            "55455",
                            "1",
                            "86171",
                            "476632679643188528985438056980162363837707337312210785567256027534077014652",
                            "0"
                        ],
                        "accountUpdateId": "23",
                        "transactionInfo": {
                            "status": "applied",
                            "hash": "5Ju8GoXZWFqBKTAv3b1iMbc77yUhZEoVc5RwD5aj3VQw5eCkHwU9",
                            "memo": "E4YM2vTHhWEg66xpj52JErHUBU4pZ1yageL4TVDDpTTSsv8mK6YaH"
                        },
                        "sequenceNumber": 0,
                        "zkappAccountUpdateIds": [
                            23,
                            24
                        ],
                        "zkappEventElementIds": [
                            19
                        ]
                    },
                    {
                        "data": [
                            "55456",
                            "1",
                            "86171",
                            "476632679643188528985438056980162363837707337312210785567256027534077014652",
                            "0"
                        ],
                        "accountUpdateId": "24",
                        "transactionInfo": {
                            "status": "applied",
                            "hash": "5Ju8GoXZWFqBKTAv3b1iMbc77yUhZEoVc5RwD5aj3VQw5eCkHwU9",
                            "memo": "E4YM2vTHhWEg66xpj52JErHUBU4pZ1yageL4TVDDpTTSsv8mK6YaH"
                        },
                        "sequenceNumber": 0,
                        "zkappAccountUpdateIds": [
                            23,
                            24
                        ],
                        "zkappEventElementIds": [
                            20
                        ]
                    }
                ]
            },
            {
                "blockInfo": {
                    "stateHash": "3NLEZvkc7Z1mRdFkh1bSppnKo8aZrB6VGe83J7UdKpL4PuLjtjqB",
                    "timestamp": "1729695707000",
                    "height": 70,
                    "parentHash": "3NKZqA81s8iHMQc4vcJxrcx6epVB7C9vBD8oV7K3EpCZzvfebCtP",
                    "chainStatus": "canonical",
                    "distanceFromMaxBlockHeight": 35,
                    "globalSlotSinceGenesis": 138
                },
                "actionState": {
                    "actionStateOne": "7365288285578028013264727183932035337741887015276045401228215954698411077529",
                    "actionStateTwo": "25619375377485241455325506385026113155326956491463643967090964080969742671789",
                    "actionStateThree": "25079927036070901246064867767436987657692091363973573142121686150614948079097",
                    "actionStateFour": "25079927036070901246064867767436987657692091363973573142121686150614948079097",
                    "actionStateFive": "25079927036070901246064867767436987657692091363973573142121686150614948079097"
                },
                "actionData": [
                    {
                        "data": [
                            "73850",
                            "1",
                            "11895",
                            "8253926607854573680301530978054957179571865656842078211022112667228198387920",
                            "1"
                        ],
                        "accountUpdateId": "33",
                        "transactionInfo": {
                            "status": "applied",
                            "hash": "5JuunjPt2xffChmddFDzdreN3TtTsADs1AiW4cgPXkYoFERyKPAi",
                            "memo": "E4YM2vTHhWEg66xpj52JErHUBU4pZ1yageL4TVDDpTTSsv8mK6YaH"
                        },
                        "sequenceNumber": 3,
                        "zkappAccountUpdateIds": [
                            33,
                            34
                        ],
                        "zkappEventElementIds": [
                            29
                        ]
                    },
                    {
                        "data": [
                            "73851",
                            "1",
                            "11895",
                            "8253926607854573680301530978054957179571865656842078211022112667228198387920",
                            "1"
                        ],
                        "accountUpdateId": "34",
                        "transactionInfo": {
                            "status": "applied",
                            "hash": "5JuunjPt2xffChmddFDzdreN3TtTsADs1AiW4cgPXkYoFERyKPAi",
                            "memo": "E4YM2vTHhWEg66xpj52JErHUBU4pZ1yageL4TVDDpTTSsv8mK6YaH"
                        },
                        "sequenceNumber": 3,
                        "zkappAccountUpdateIds": [
                            33,
                            34
                        ],
                        "zkappEventElementIds": [
                            30
                        ]
                    },
                    {
                        "data": [
                            "73237",
                            "1",
                            "52294",
                            "476632679643188528985438056980162363837707337312210785567256027534077014652",
                            "0"
                        ],
                        "accountUpdateId": "31",
                        "transactionInfo": {
                            "status": "applied",
                            "hash": "5JtuWFstMC6m3wgeoX8E5Nh1ZdwRdGXvhcrVbGB247Ph72KGpL23",
                            "memo": "E4YM2vTHhWEg66xpj52JErHUBU4pZ1yageL4TVDDpTTSsv8mK6YaH"
                        },
                        "sequenceNumber": 1,
                        "zkappAccountUpdateIds": [
                            31,
                            32
                        ],
                        "zkappEventElementIds": [
                            27
                        ]
                    },
                    {
                        "data": [
                            "73238",
                            "1",
                            "52294",
                            "476632679643188528985438056980162363837707337312210785567256027534077014652",
                            "0"
                        ],
                        "accountUpdateId": "32",
                        "transactionInfo": {
                            "status": "applied",
                            "hash": "5JtuWFstMC6m3wgeoX8E5Nh1ZdwRdGXvhcrVbGB247Ph72KGpL23",
                            "memo": "E4YM2vTHhWEg66xpj52JErHUBU4pZ1yageL4TVDDpTTSsv8mK6YaH"
                        },
                        "sequenceNumber": 1,
                        "zkappAccountUpdateIds": [
                            31,
                            32
                        ],
                        "zkappEventElementIds": [
                            28
                        ]
                    },
                    {
                        "data": [
                            "35093",
                            "1",
                            "39852",
                            "4611450100234868895111341341933539196411650948711574062334330094836938200670",
                            "1"
                        ],
                        "accountUpdateId": "29",
                        "transactionInfo": {
                            "status": "applied",
                            "hash": "5JtrkeMGmJFJQkVk27EtUsmzkLeiCinetkiwvVBfuHXrMqrwJvj2",
                            "memo": "E4YM2vTHhWEg66xpj52JErHUBU4pZ1yageL4TVDDpTTSsv8mK6YaH"
                        },
                        "sequenceNumber": 0,
                        "zkappAccountUpdateIds": [
                            29,
                            30
                        ],
                        "zkappEventElementIds": [
                            25
                        ]
                    },
                    {
                        "data": [
                            "35094",
                            "1",
                            "39852",
                            "4611450100234868895111341341933539196411650948711574062334330094836938200670",
                            "1"
                        ],
                        "accountUpdateId": "30",
                        "transactionInfo": {
                            "status": "applied",
                            "hash": "5JtrkeMGmJFJQkVk27EtUsmzkLeiCinetkiwvVBfuHXrMqrwJvj2",
                            "memo": "E4YM2vTHhWEg66xpj52JErHUBU4pZ1yageL4TVDDpTTSsv8mK6YaH"
                        },
                        "sequenceNumber": 0,
                        "zkappAccountUpdateIds": [
                            29,
                            30
                        ],
                        "zkappEventElementIds": [
                            26
                        ]
                    }
                ]
            }
        ]
    }
}

45930 · 2024-10-23T15:37:27Z

It looks like all the data is there, but it can be better-organized.

@deepthiskumar, do you know the relationship between events and actions? It seems like each transaction here has one event id, and 2 action ids. My test script does generate 2 actions per transaction, so I expected that, but I'm not sure why only one event would be sent.

45930 · 2024-10-23T18:12:31Z

Also it needs block-height (assuming the API returns canonical blocks only) to disambiguate transactions with same sequence numbers but are part of different blocks

This is already available, as blockInfo.height. In my example JSON, you can see the blocks 69 and 70 are present in the response. Within each block, there are transactions with sequence [0, 1, 2] on one, and [0, 1, 3] on the other. Then within each transaction, there are 2 events each. The accountUpdateId property is on the action itself, and the zkappAccountUpdateIds is an array showing the correct order on the transactionInfo.

I could add a property, accountUpdateIndex and remove the property accountUpdateId and handle that logic in the resolver, or we could send both the accountUpdateId of the action, and the zkappAccountUpdateIds of the transaction and allow clients to do the logic on their side.

45930 · 2024-10-23T18:21:54Z

So we could have a response like

{
    "data": [
        "24651",
        "1",
        "63073",
        "4611450100234868895111341341933539196411650948711574062334330094836938200670",
        "1"
    ],
    "accountUpdateId": "28",
    "transactionInfo": {
        "status": "applied",
        "hash": "5JvAynd7NAHEiMqCpgZD85WCzLniBv18eSEJyuvNVK8pQL8Fi5kx",
        "memo": "E4YM2vTHhWEg66xpj52JErHUBU4pZ1yageL4TVDDpTTSsv8mK6YaH",
        "sequenceNumber": 2,
        "zkappAccountUpdateIds": [
            27,
            28
        ],
        "zkappEventElementIds": [
            24
        ]
    }
},

We have enough info here to tell us within the block that it's in, it belongs to transaction 2, and it is the second update in that sequence. That seems good enough to me, as long as that is the correct way to order things.

45930 · 2024-10-23T18:24:32Z

For pending blocks, it looks like we follow the longest chain rule here:

SELECT
  id, state_hash, parent_hash, parent_id, height, global_slot_since_genesis, global_slot_since_hard_fork, timestamp, chain_status, ledger_hash, last_vrf_output
FROM
  blocks b
WHERE
  height = (SELECT max(height) FROM blocks)

We can probably handle filtering for canonical blocks in o1js. For reducing actions, I think we can ignore pending blocks by default, but for random exploratory queries, people might be interested to see pending data as well.

45930 · 2024-10-23T19:53:13Z

tests/makeActionsRequest.ts

@@ -0,0 +1,66 @@
+import { createYoga, createSchema } from 'graphql-yoga';


This whole file may be better to remove. It's just a script to make a graphql request against the local API.

45930 · 2024-10-23T20:28:31Z

src/services/actions-service/actions-service.ts

@@ -157,4 +157,26 @@ class ActionsService implements IActionsService {
    }
    return actions;
  }
+
+  sortActions(actions: Action[]): Action[] {
+    return actions.sort((a, b) => {


Since actions are returned grouped by block, this is focused on ordering the actions within a block.

I'm not sure the sort order is correct. Maybe it's reversed, but we should get it right, then we will have access to the unit test as a spec.

@deepthiskumar it's worth noting that you and gregor both mentioned a sub-order to the account update in o1-labs/o1js#1872

It is your point 4, and Gregor said "It's extremely important that when reordering the actions here, the order within the same account update is maintained".

This sort only looks at sequence number and account update id. But with my JSON comment on this PR and this code, we should be able to get on the same page more easily. If there's a third sorting lever we need here, then I can add it, but it's not clear to me what it is.

Yes, the third sorting lever is the element ID index from zkappEventElementIds

So as linked in my other comment, I think we only need to sort to the event field array level. Then all the fields from all the field arrays get put into data in the correct order.

45930 · 2024-10-23T20:31:33Z

tests/resolvers.test.ts

+        /* fundNewAccount = */ false
+      );
+    } catch (error) {
+      console.error(error);


This console was helpful in debugging. Errors were swallowed otherwise.

deepthiskumar · 2024-10-24T18:03:51Z

@deepthiskumar, do you know the relationship between events and actions? It seems like each transaction here has one event id, and 2 action ids. My test script does generate 2 actions per transaction, so I expected that, but I'm not sure why only one event would be sent.

Those are two separate fields in the transaction. Events are more for recording information corresponding to an account update. They don't change any on-chain state. Actions on the other hand are committed to the account (action_state field in an account). They

Can you generate at least two account updates (excluding the fee payer account update) each having two or more events and two or more actions? I think that should cover all the cases.

deepthiskumar · 2024-10-24T18:22:01Z

I could add a property, accountUpdateIndex and remove the property accountUpdateId and handle that logic in the resolver, or we could send both the accountUpdateId of the action, and the zkappAccountUpdateIds of the transaction and allow clients to do the logic on their side.

Adding the index or a sequence field makes more sense to me than exposing account ID that is not consistent across DBs. If one were to run the API on two different DBs (or existing DB was updated during migration) then they could get different values for the same field. But with a sequence number or index they should be consistent across all archive DBs. Imo none of the database ID fields should be exposed to the client to ensure the client side application doesn't get tightly coupled to the database instance serving the API

45930 · 2024-10-24T18:34:01Z

Adding the index or a sequence field makes more sense to me than exposing account ID that is not consistent across DBs. If one were to run the API on two different DBs (or existing DB was updated during migration) then they could get different values for the same field. But with a sequence number or index they should be consistent across all archive DBs.

This fundamentally makes sense to me, but just to confirm, there is not literally an account update index value in the DB, it's something that needs to be calculated right? e.g. as I do here? The only thing I worry about is the added compute cost to calculate this every time. It seems quite cheap, but perhaps at scale it's not acceptable.

If that's the right solution for the long term, it's also a possibility to do a quick fix now and a migration of the archive DB in the future to better serve this use case.

45930 · 2024-10-24T18:43:30Z

@boray is going to advance this PR while I'm on PTO, so to do a quick summary of the changes I suggest based on your feedback:

Add accountUpdateIndexNumber to the graphQL schema
- the index of the accountUpdateID in zkappAccountUpdateIds
Add eventElementIndexNumber to the graphQL schema
- the index of the eventElementId in zkappEventElementIds
Add eventElement as the third layer of sorting in the sortAcitons function
Generate a test case where an action has more than one event

Important

Don't remove any values from the graphQL schema, because it should continue to work with older o1js versions which will make the old query. We can consider the db identifier fields deprecated though.

deepthiskumar · 2024-10-24T18:55:09Z

This fundamentally makes sense to me, but just to confirm, there is not literally an account update index value in the DB, it's something that needs to be calculated right? e.g. as I do here? The only thing I worry about is the added compute cost to calculate this every time. It seems quite cheap, but perhaps at scale it's not acceptable.

If that's the right solution for the long term, it's also a possibility to do a quick fix now and a migration of the archive DB in the future to better serve this use case.

Yes, the index needs to be computed given the current schema. It is an option to do in the archive processor and have a new table to store the sequence number and the corresponding element ID. It is not clear to me if it is a better option (another huge table to join vs looking up in arrays)

deepthiskumar · 2024-10-24T19:08:31Z

src/blockchain/types.ts

@@ -41,6 +41,9 @@ export type TransactionInfo = {
  hash: string;
  memo: string;
  authorizationKind: string;
+  sequenceNumber: number;
+  zkappAccountUpdateIds: number[];
+  zkappEventElementIds: number[];


You'll want another similar field for Events (this is for Actions) to include as part of the Events data, no?

Why do we need that, assuming events will not be sorted?

assuming events will not be sorted

Not sure what you mean by this. Events are recorded in the same order as actions i.e., sequence of blocks, sequence of transactions within a block, sequence of account updates within a transaction and sequence of events within an account update. The only difference is that the actions are accumulated on chain in action state and so one will always want to know the order of actions to validate against action states. For events it is more of a utility where an app might want to process events in the order they were confirmed/included in the chain. As an AP,I it seems like it should be providing that utility

deepthiskumar · 2024-10-29T08:19:15Z

There's actually another order that I didn't account for in my previous messages. I've updated now o1-labs/o1js#1872 (comment)
So, a zkapp transaction is a list of account updates, each account update can have a list of action/events, and finally each event/action is an array of field elements.

45930 · 2024-11-15T14:41:00Z

Please note that the current branch has failing tests, but CI is green. I will try to resolve that in the next commit, but this is still WIP.

Another one to reduce the complexity of reviewing #107 This PR: - exposes port 8282 on docker in CI, which is used by lightnet - improves the resolver tests - properly annotates the type of the graphQL response object - exits the process with code `1` on error instead of always exiting with code `0` - makes use of additional describe blocks to clarify the test cases - explicitly test response data for correctness, rather than metadata like length only Since there are no implementation changes here, we can be more confident about changes to the test file, then in the other PR where I do change the implementation, I will be able to leave existing tests as-is.

45930 · 2024-11-27T16:52:46Z

zkapp/contract.ts

+    this.emitEvent('struct', struct);
+  }
+
+  @method async emitStructsEvent(structs: TestStructArray) {


Updating these methods give us more variance to test with - not just emitting the same thing over and over.

45930 · 2024-11-27T16:55:38Z

zkapp/contract.ts

    const x = this.x.getAndRequireEquals();
    const y = this.y.getAndRequireEquals();
    const z = this.z.getAndRequireEquals();
-    this.reducer.dispatch(new TestStruct({ x, y, z, address: this.address }));
+    const struct = new TestStruct({ x, y, z, address: this.address });
+    this.reducer.dispatch({ structs: [struct, struct, struct] });


One contract may have many types of events, but only one type of action. To keep the tests slightly simple and fast, I updated this action type to the most complex: One action with multiple field arrays with multiple fields.

The alternative was writing a new contract where one has a simple action type and one has a complex action type, and we would need to await compile, deploy, etc... on both.

45930 · 2024-11-27T17:11:40Z

Ok, tests are green and this is ready for review!

deepthiskumar · 2024-12-02T20:31:24Z

src/db/sql/events-actions/types.ts

  // List of `element_ids` that are used to construct the zkApp event.
  zkapp_event_element_ids: number[];

+  // List of `element_ids` that are used to construct the field array.
+  zkapp_field_array_element_ids: number[];
+


The naming is confusing and it is making it a bit difficult to review the code for me. May I suggest the following-

account_update_events_id : id of an array of events per account update

account_update_event_id : id of a single event in an account update. account_update_events_id (from 1) points to an array of account_update_event_id

event_field_elements_id : Id of an array of field elements. Each event (account_update_event_id from 2) corresponds to an array of field elements

event_field_element_id : Id of a field element. event_field_elements_id (from 3) points to an array of event_field_element_id

This type is the ArchiveNodeDatabaseRow, and I believe it should retain the column names of the query result (I'll take another pass and make sure it does, and see if the new columns I added can be named better).

But I will take this feedback and apply it to the user-facing types.

deepthiskumar · 2024-12-02T20:51:12Z

src/resolvers-types.ts

  status: Scalars['String']['output'];
+  zkappAccountUpdateIds: Array<Maybe<Scalars['Int']['output']>>;
+  zkappEventElementIds: Array<Maybe<Scalars['Int']['output']>>;


rename zkappEventElementIds to accountUpdateEventIds and zkappFieldArrayElementIds to eventFieldElementIds

I ended up just removing these. I don't think we need to expose them in graphQL after all

deepthiskumar · 2024-12-02T21:57:57Z

src/services/actions-service/actions-service.ts

+        );
+
+      return aAccountUpdateIndex - bAccountUpdateIndex;
+    });


Shouldn't this include action ids and field element ids also? The protocol supports multiple actions per account update

Also, are we not doing similar sorting for events?

Shouldn't this include action ids and field element ids also? The protocol supports multiple actions per account update

Within one account update, the actions are ordered within the data property in the response, see: https://github.com/o1-labs/Archive-Node-API/pull/107/files#diff-ce4b532132cbf96cc52ea84724fbfa50698c62ca9c690844068dafbfc687d3a8R341

const structData = eventData.data; assert.strictEqual(structData.length, 16); assert.strictEqual(structData[0], '2'); assert.deepStrictEqual( structData.slice(1, 6), structToAction(expectedS1) ); assert.deepStrictEqual( structData.slice(6, 11), structToAction(expectedS2) ); assert.deepStrictEqual( structData.slice(11, 16), structToAction(expectedS3) );

I haven't changed the intra-AU event order here, as it seems to be correct already.

Also, are we not doing similar sorting for events?

I've always been told as a zkapp developer that event order is not guaranteed, so I don't think we need to. The only reason we need to sort actions is because an actions hash is committed to, and that hash uses a particular order that needs to be reproduced. There is no such events hash on chain, so no ordering should be necessary.

The query here to fetch actions uses ANY and so the ordering is not guaranteed. I'm guessing the ordering here just happens to be correct

Ok I'll double check

@deepthiskumar we convert field arrays into data here. It seems like we take the element_ids for the correct order, and elementIdFieldValues (key-value map) to access the correct values.

Maybe I can add a test to make sure this order is explicitly respected, but I think it happens to work correctly already.

Could you please add such a test? I think that'll help a lot with order being preserved implicitly. Everything else looks good otherwise.

Yes I will add a test in a fast-follow!

45930

Ok, responded to some of the naming issues at the database level. I will also improve the tying here and make sure to propagate the naming upstream as well, where relevant.

I will comment again when the PR is explicitly ready for review.

45930 · 2024-12-12T05:56:27Z

src/services/data-adapters/database-row-adapters.ts

@@ -123,8 +123,8 @@ function removeRedundantEmittedFields(
  for (let i = 0; i < archiveNodeRow.length; i++) {
    const currentRow = archiveNodeRow[i];
    const {
-      zkapp_event_array_id, // The unique id for the event/action emitted
-      zkapp_event_element_ids, // The list of field ids that make up the event/action
+      event_field_elements_id, // The unique id for the field array in the current row


Looks like the comments here weren't quite right. Following the comments explicitly led to test failures, so I found the intended inputs and updated the comments. Will double check the logic here.

45930 · 2024-12-12T23:07:14Z

src/db/sql/events-actions/queries.ts

@@ -106,8 +106,23 @@ function emittedZkAppCommandsCTE(db_client: postgres.Sql) {
  return db_client`
  emitted_zkapp_commands AS (
    SELECT
-      blocks_accessed.*,
+      blocks_accessed.requesting_zkapp_account_identifier_id,


Expanding * queries is optional, but I think it will be more clear for the next person who has to dig here. I struggled to make sense of these queries at first.

45930 · 2024-12-12T23:08:27Z

src/db/sql/events-actions/types.ts

+/**
+ * A Field is the smallest unit of raw data in an Event
+ */
+namespace Field {


Namespaces are also optional. It gives the ArchiveNodeDatabaseRow columns some more clarity, since zkappEventElementFieldArrayElementValue is not obvious!

45930 · 2024-12-12T23:10:20Z

src/db/sql/events-actions/types.ts

 */
 export type ArchiveNodeDatabaseRow = {
-  // Unique block identifier.
+  /**


This comment styling makes intellisense render the comments on hover.

45930 · 2024-12-12T23:10:54Z

src/db/sql/events-actions/types.ts

+   * List of `element_ids` that are used to construct the field array.
+   * Each entry corresponds to a `Field.Id`, linking to a `Field`.
+   */
+  event_field_element_ids: EventFieldArray.FieldIds;


Example of how the namespace types are being used.

45930 · 2024-12-12T23:12:27Z

src/services/actions-service/actions-service.ts

@@ -157,4 +157,26 @@ class ActionsService implements IActionsService {
    }
    return actions;
  }
+
+  sortActions(actions: Action[]): Action[] {
+    return actions.sort((a, b) => {


So as linked in my other comment, I think we only need to sort to the event field array level. Then all the fields from all the field arrays get put into data in the correct order.

45930 · 2024-12-12T23:18:43Z

tests/utils.test.ts

@@ -12,7 +12,7 @@ import { ArchiveNodeDatabaseRow } from '../src/db/sql/events-actions/types.js';
 describe('utils', () => {
  describe('partitionBlocks', () => {
    test('should partition rows by block hash and transaction hash', () => {
-      const rows: any[] = [
+      const rows: Partial<ArchiveNodeDatabaseRow>[] = [


Using Partial<> over any so that invalid keys are flagged as an error

Hebilicious · 2024-12-13T17:46:21Z

Hello there, please excuse me in advance if this off-track.

I am not sure if this falls within #106 scope, but when doing the events query (or actions), it appears that there is no way to know what is the latest blockHeight currently in the database when the results set is empty.

This information is present within the SQL queries (height = (SELECT max(height) FROM blocks)) but it's not directly exposed by the graphql server, only distanceFromMaxBlockHeight.

It could be helpful to know what is the maxBlockHeight when the query return an empty set of results.

Would it be within the scope of this PR to add it ? Or should I open a different issue ?

45930 · 2024-12-13T18:05:34Z

@Hebilicious great feedback! Please open a different issue for this, but it sounds pretty straightforward to me!

45930 force-pushed the 2024-10-bugfix-actions-order branch from 045c9c8 to ddd8b27 Compare October 22, 2024 17:47

45930 changed the base branch from main to 2024-10-upgrade-o1js October 22, 2024 17:47

deepthiskumar reviewed Oct 22, 2024

View reviewed changes

Base automatically changed from 2024-10-upgrade-o1js to main October 23, 2024 03:54

45930 added 3 commits October 23, 2024 15:51

adjust query to support ordering

5efccc8

fix build

f01a38e

updating the schema, adding multi-emit test

030c60f

45930 force-pushed the 2024-10-bugfix-actions-order branch from d8f0b60 to 030c60f Compare October 23, 2024 19:51

45930 added 2 commits October 23, 2024 16:25

adding sort order and testing

01f0a70

another case

5bc4124

45930 commented Oct 23, 2024

View reviewed changes

45930 marked this pull request as ready for review October 23, 2024 20:33

more cases

6c1a253

deepthiskumar reviewed Oct 24, 2024

View reviewed changes

45930 mentioned this pull request Oct 24, 2024

Failed to derive correct actions hash o1-labs/o1js#1872

Closed

adding sorting levers

e1fc23d

45930 force-pushed the 2024-10-bugfix-actions-order branch 2 times, most recently from 4b3b73f to d453a59 Compare November 20, 2024 23:57

45930 mentioned this pull request Nov 22, 2024

Sort actions by tx sequence when available o1-labs/o1js#1917

Merged

45930 added 2 commits November 27, 2024 11:22

merge conflicts

71e84e7

add zkapp_account_update_id to test row

a6eb6f9

45930 commented Nov 27, 2024

View reviewed changes

rm test files

29c936b

45930 requested a review from deepthiskumar November 27, 2024 17:11

deepthiskumar reviewed Dec 2, 2024

View reviewed changes

45930 added 3 commits December 11, 2024 16:45

messing with query

49cb8b0

some tests fixed

d3a6b51

more tests fixed maybe\?

5e706f7

45930 commented Dec 12, 2024

View reviewed changes

45930 added 6 commits December 12, 2024 17:44

type annotations

43feb71

rm package-lock diff

231a9ca

diable namespace lint rule

f959060

cleanup graphql types

fb9be4b

fix comment

ac04af1

rm noisy diff

a007e8a

45930 commented Dec 12, 2024

View reviewed changes

45930 added 2 commits December 12, 2024 18:19

linting

0aeec84

cleaning up dummy action factory

335dffa

45930 requested a review from deepthiskumar December 12, 2024 23:52

Hebilicious mentioned this pull request Dec 15, 2024

[feature request] latest known blockHeight #111

Closed

deepthiskumar approved these changes Dec 16, 2024

View reviewed changes

45930 merged commit 44a437a into main Dec 17, 2024
4 checks passed

45930 deleted the 2024-10-bugfix-actions-order branch December 17, 2024 14:58

		@@ -0,0 +1,66 @@
		import { createYoga, createSchema } from 'graphql-yoga';

adjust query to support ordering #107

adjust query to support ordering #107

Conversation

45930 commented Oct 21, 2024 • edited Loading

deepthiskumar left a comment

Choose a reason for hiding this comment

45930 commented Oct 23, 2024

45930 commented Oct 23, 2024

45930 commented Oct 23, 2024

45930 commented Oct 23, 2024

45930 commented Oct 23, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

deepthiskumar commented Oct 24, 2024

deepthiskumar commented Oct 24, 2024 • edited Loading

45930 commented Oct 24, 2024

45930 commented Oct 24, 2024

deepthiskumar commented Oct 24, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

deepthiskumar commented Oct 29, 2024

45930 commented Nov 15, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

45930 commented Nov 27, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

45930 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Hebilicious commented Dec 13, 2024

45930 commented Dec 13, 2024

45930 commented Oct 21, 2024 •

edited

Loading

deepthiskumar commented Oct 24, 2024 •

edited

Loading