Fix Dynamic Field Cursor Index Bug

[oxade]

Description

Fixes: #7686

For Dynamic Object Fields, there are 3 different IDs at play:
parent_id: the ID of the parent object
dof_wrapper_id: the ID of the wrapper dynamic field which wraps the actual child object. This is an implementation detail of dynamic object fields.
child_object_id: the ID of the inner child object. This is the real object users are interested in.

The DB key is type DynamicFieldKey which is setup as (parent_id, dof_wrapper_id), and value DynamicFieldInfo which contains child_object_id
But the old code was returning child_object_id as cursor instead of dof_wrapper_id, which means when you try to find the next page, the cursor does not exist in the DB keys so the search lands on the first entry closest to (parent_id, child_object_id).

There were two potential fixes:

1. Change the key to (parent_id, child_object_id). This is more user friendly as the cursor is the direct object people care about. However this would break existing DB entries.
2. Change the cursor logic to return dof_wrapper_id so that subsequent queries are keyed properly. Although dof_wrapper_id is just an indirection on the child_object_id and does not show up in the response itself (only the child_object_id shows up), keeping this approach allows us to roll out this fix without breaking existing DB entries.

For example the structure in the DB is:
1: (parent_id: 3, dof_wrapper_id: 4) -> DynamicFieldInfo {child_object_id: 2}
2: (parent_id: 3, dof_wrapper_id: 7) -> DynamicFieldInfo {child_object_id: 3}
3: (parent_id: 3, dof_wrapper_id: 9) -> DynamicFieldInfo {child_object_id: 5}
4: (parent_id: 3, dof_wrapper_id: 12) -> DynamicFieldInfo {child_object_id: 19}

If the query is made with a limit of 3, the returned cursor will be 5 which is from child_object_id in the 3rd entry.
But when this cursor is used the next time, it will match the 2nd entry (parent_id: 3, dof_wrapper_id: 7) -> DynamicFieldInfo {child_object_id: 3} because the the DB implementation of skip_to which is used in the dynamic field iterator seeks to the nearest highest key from 5 which is 7.
This is an example of how one will see duplicates because the cursor can point backwards.

The solution in this code changes the cursor to return the dof_wrapper_id of 9 in this example, so the next query will return the 4th entry (parent_id: 3, dof_wrapper_id: 12) -> DynamicFieldInfo {child_object_id: 19} as expected.

Test Plan

Manual testing

---

If your changes are not user-facing and not a breaking change, you can skip the following section. Otherwise, please indicate what changed, and then add to the Release Notes section as highlighted during the release process.

Type of Change (Check all that apply)

• user-visible impact
• breaking change for a client SDKs
• breaking change for FNs (FN binary must upgrade)
• breaking change for validators or node operators (must upgrade binaries)
• breaking change for on-chain data layout
• necessitate either a data wipe or data migration

Release notes

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

4 Ignored Deployments

Name	Status	Preview	Updated (UTC)
explorer	⬜️ Ignored (Inspect)		May 7, 2023 1:16am
explorer-storybook	⬜️ Ignored (Inspect)		May 7, 2023 1:16am
sui-wallet-kit	⬜️ Ignored (Inspect)		May 7, 2023 1:16am
wallet-adapter	⬜️ Ignored (Inspect)		May 7, 2023 1:16am

[666lcz]

@oxade , thanks for the fix! Could you elaborate on what caused the bug and how this PR fixed it?

[oxade]

@oxade , thanks for the fix! Could you elaborate on what caused the bug and how this PR fixed it?

For Dynamic Object Fields, there are 3 different IDs at play:
parent_id: the ID of the parent object
dof_wrapper_id: the ID of the wrapper dynamic field which wraps the actual child object. This is an implementation detail of dynamic object fields.
child_object_id: the ID of the inner child object. This is the real object users are interested in.

The DB key is type DynamicFieldKey which is setup as (parent_id, dof_wrapper_id), and value DynamicFieldInfo which contains child_object_id
But the old code was returning child_object_id as cursor instead of dof_wrapper_id, which means when you try to find the next page, the cursor does not exist in the DB keys so the search lands on the first entry closest to (parent_id, child_object_id).

There were two potential fixes:

1. Change the key to (parent_id, child_object_id). This is more user friendly as the cursor is the direct object people care about. However this would break existing DB entries.
2. Change the cursor logic to return dof_wrapper_id so that subsequent queries are keyed properly. Although dof_wrapper_id is just an indirection on the child_object_id and does not show up in the response itself (only the child_object_id shows up), keeping this approach allows us to roll out this fix without breaking existing DB entries.

For example the structure in the DB is:
1: (parent_id: 3, dof_wrapper_id: 4) -> DynamicFieldInfo {child_object_id: 2}
2: (parent_id: 3, dof_wrapper_id: 7) -> DynamicFieldInfo {child_object_id: 3}
3: (parent_id: 3, dof_wrapper_id: 9) -> DynamicFieldInfo {child_object_id: 5}
4: (parent_id: 3, dof_wrapper_id: 12) -> DynamicFieldInfo {child_object_id: 19}

If the query is made with a limit of 3, the returned cursor will be 5 which is from child_object_id in the 3rd entry.
But when this cursor is used the next time, it will match the 2nd entry (parent_id: 3, dof_wrapper_id: 7) -> DynamicFieldInfo {child_object_id: 3} because the the DB implementation of skip_to which is used in the dynamic field iterator seeks to the nearest highest key from 5 which is 7.
This is an example of how one will see duplicates because the cursor can point backwards.

The solution in this code changes the cursor to return the dof_wrapper_id of 9 in this example, so the next query will return the 4th entry (parent_id: 3, dof_wrapper_id: 12) -> DynamicFieldInfo {child_object_id: 19} as expected.

[samlaf]

So this means that the cursors returned will no longer be the actual object ids, and we will have to keep this implementation detail in our code if we later need to continue retrieving from the last retrieved DF? 😢

malaise just to be sure, the ordering of the child objects (including dof_wrapper and child ids) can only ever increase right? Or can future dfs be inserted in between the ones we see here? (For eg where there is a gap between the wrapper_ids)

[oxade]

So this means that the cursors returned will no longer be the actual object ids, and we will have to keep this implementation detail in our code if we later need to continue retrieving from the last retrieved DF? 😢

Yes. Unfortunately its a choice between that, and having invalid data returned.
If you prefer the latter and maybe want think up some clever way to workaround it on the client side, we can discuss, but it likely wont be straightforward.
Note that for Dynamic Fields, there is no wrapper, so you'll get the actual object id. This wrapper only applies to Dynamic Object Fields.
But generally best to just assume the cursor is an abstract entity.

malaise just to be sure, the ordering of the child objects (including dof_wrapper and child ids) can only ever increase right? Or can future dfs be inserted in between the ones we see here? (For eg where there is a gap between the wrapper_ids)

Since object ids are randomly generated, anything can be inserted anywhere as our DB only cares about lexicographic key ordering. So yes gaps can be filled at any point.

[samlaf]

Note that for Dynamic Fields, there is no wrapper, so you'll get the actual object id. This wrapper only applies to Dynamic Object Fields.
But generally best to just assume the cursor is an abstract entity.

Ah I see! Unfortunate that both behave differently, since this will definitely cause confusion to people using both. But it is what it is.

Since object ids are randomly generated, anything can be inserted anywhere as our DB only cares about lexicographic key ordering. So yes gaps can be filled at any point.

I see. This is the biggest point we would like to see improved.

• pagination is already pretty limiting in terms of lots of reads ([rpc] The Page API is limiting #11725)
• but would also be nice to be able to query for the latest added DF/DoFs since some ts (maybe this is possible via the AND queries, but afaik they weren't implemented last time I tried them)

[oxade]

Ah I see! Unfortunate that both behave differently, since this will definitely cause confusion to people using both. But it is what it is.

If the cursor is used strictly as a cursor without assuming it holds any other meaning, there should be no confusion either way as it operates the same way in both cases. It just so happens to match the underlying object id for DF.

since some ts

what is "ts"?

[samlaf]

what is "ts"?

timestamp sorry. Like "retrieve all DFs added some timestampMS=..."