fix(graphql): add root object version for dynamic field queries

crates/sui-graphql-rpc/schema/current_progress_schema.graphql

@@ -889,9 +889,10 @@ type DynamicField {
 	"""
 	name: MoveValue
 	"""
-	The actual data stored in the dynamic field.
-	The returned dynamic field is an object if its return type is MoveObject,
-	in which case it is also accessible off-chain via its address.
+	The returned dynamic field is an object if its return type is `MoveObject`,
+	and its contents will be from the latest version that is at most equal to
+	its parent object's version. Otherwise, it is also accessible off-chain
+	via its address.
 	"""
 	value: DynamicFieldValue
 }

crates/sui-graphql-rpc/src/types/coin.rs

@@ -244,7 +244,7 @@ impl Coin {
         name: DynamicFieldName,
     ) -> Result<Option<DynamicField>> {
         OwnerImpl::from(&self.super_.super_)
-            .dynamic_field(ctx, name, Some(self.super_.super_.version_impl()))
+            .dynamic_field(ctx, name, Some(self.super_.root_version()))
             .await
     }
 
@@ -261,7 +261,7 @@ impl Coin {
         name: DynamicFieldName,
     ) -> Result<Option<DynamicField>> {
         OwnerImpl::from(&self.super_.super_)
-            .dynamic_object_field(ctx, name, Some(self.super_.super_.version_impl()))
+            .dynamic_object_field(ctx, name, Some(self.super_.root_version()))
             .await
     }
 
@@ -284,7 +284,7 @@ impl Coin {
                 after,
                 last,
                 before,
-                Some(self.super_.super_.version_impl()),
+                Some(self.super_.root_version()),
             )
             .await
     }
@@ -336,7 +336,8 @@ impl Coin {
             // To maintain consistency, the returned cursor should have the same upper-bound as the
             // checkpoint found on the cursor.
             let cursor = stored.cursor(checkpoint_viewed_at).encode_cursor();
-            let object = Object::try_from_stored_history_object(stored, checkpoint_viewed_at)?;
+            let object =
+                Object::try_from_stored_history_object(stored, checkpoint_viewed_at, None)?;
 
             let move_ = MoveObject::try_from(&object).map_err(|_| {
                 Error::Internal(format!(

crates/sui-graphql-rpc/src/types/coin_metadata.rs

@@ -233,7 +233,7 @@ impl CoinMetadata {
         name: DynamicFieldName,
     ) -> Result<Option<DynamicField>> {
         OwnerImpl::from(&self.super_.super_)
-            .dynamic_field(ctx, name, Some(self.super_.super_.version_impl()))
+            .dynamic_field(ctx, name, Some(self.super_.root_version()))
             .await
     }
 
@@ -250,7 +250,7 @@ impl CoinMetadata {
         name: DynamicFieldName,
     ) -> Result<Option<DynamicField>> {
         OwnerImpl::from(&self.super_.super_)
-            .dynamic_object_field(ctx, name, Some(self.super_.super_.version_impl()))
+            .dynamic_object_field(ctx, name, Some(self.super_.root_version()))
             .await
     }
 
@@ -273,7 +273,7 @@ impl CoinMetadata {
                 after,
                 last,
                 before,
-                Some(self.super_.super_.version_impl()),
+                Some(self.super_.root_version()),
             )
             .await
     }

crates/sui-graphql-rpc/src/types/dynamic_field.rs

@@ -94,9 +94,10 @@ impl DynamicField {
         Ok(Some(MoveValue::new(type_tag, Base64::from(bcs))))
     }
 
-    /// The actual data stored in the dynamic field.
-    /// The returned dynamic field is an object if its return type is MoveObject,
-    /// in which case it is also accessible off-chain via its address.
+    /// The returned dynamic field is an object if its return type is `MoveObject`,
+    /// and its contents will be from the latest version that is at most equal to
+    /// its parent object's version. Otherwise, it is also accessible off-chain
+    /// via its address.
     async fn value(&self, ctx: &Context<'_>) -> Result<Option<DynamicFieldValue>> {
         if self.df_kind == DynamicFieldType::DynamicObject {
             // If `df_kind` is a DynamicObject, the object we are currently on is the field object,
@@ -107,12 +108,7 @@ impl DynamicField {
             let obj = MoveObject::query(
                 ctx,
                 self.df_object_id,
-                Object::under_parent(
-                    // TODO (RPC-131): The dynamic object field value's version should be bounded by
-                    // the field's parent version, not the version of the field object itself.
-                    self.super_.super_.version_impl(),
-                    self.super_.super_.checkpoint_viewed_at,
-                ),
+                Object::under_parent(self.root_version(), self.super_.super_.checkpoint_viewed_at),
             )
             .await
             .extend()?;
@@ -228,7 +224,11 @@ impl DynamicField {
             // checkpoint found on the cursor.
             let cursor = stored.cursor(checkpoint_viewed_at).encode_cursor();
 
-            let object = Object::try_from_stored_history_object(stored, checkpoint_viewed_at)?;
+            let object = Object::try_from_stored_history_object(
+                stored,
+                checkpoint_viewed_at,
+                parent_version,
+            )?;
 
             let move_ = MoveObject::try_from(&object).map_err(|_| {
                 Error::Internal(format!(
@@ -243,6 +243,10 @@ impl DynamicField {
 
         Ok(conn)
     }
+
+    pub(crate) fn root_version(&self) -> u64 {
+        self.super_.root_version()
+    }
 }
 
 impl TryFrom<MoveObject> for DynamicField {

crates/sui-graphql-rpc/src/types/move_object.rs

@@ -312,7 +312,7 @@ impl MoveObject {
         name: DynamicFieldName,
     ) -> Result<Option<DynamicField>> {
         OwnerImpl::from(&self.super_)
-            .dynamic_field(ctx, name, Some(self.super_.version_impl()))
+            .dynamic_field(ctx, name, Some(self.root_version()))
             .await
     }
 
@@ -329,7 +329,7 @@ impl MoveObject {
         name: DynamicFieldName,
     ) -> Result<Option<DynamicField>> {
         OwnerImpl::from(&self.super_)
-            .dynamic_object_field(ctx, name, Some(self.super_.version_impl()))
+            .dynamic_object_field(ctx, name, Some(self.root_version()))
             .await
     }
 
@@ -346,14 +346,7 @@ impl MoveObject {
         before: Option<object::Cursor>,
     ) -> Result<Connection<String, DynamicField>> {
         OwnerImpl::from(&self.super_)
-            .dynamic_fields(
-                ctx,
-                first,
-                after,
-                last,
-                before,
-                Some(self.super_.version_impl()),
-            )
+            .dynamic_fields(ctx, first, after, last, before, Some(self.root_version()))
             .await
     }
 
@@ -461,6 +454,13 @@ impl MoveObject {
         })
         .await
     }
+
+    /// Root parent object version for dynamic fields.
+    ///
+    /// Check [`Object::root_version`] for details.
+    pub(crate) fn root_version(&self) -> u64 {
+        self.super_.root_version()
+    }
 }
 
 impl TryFrom<&Object> for MoveObject {

crates/sui-graphql-rpc/src/types/object.rs

@@ -53,6 +53,18 @@ pub(crate) struct Object {
     pub kind: ObjectKind,
     /// The checkpoint sequence number at which this was viewed at.
     pub checkpoint_viewed_at: u64,
+    /// Root parent object version for dynamic fields.
+    ///
+    /// This enables consistent dynamic field reads in the case of chained dynamic object fields,
+    /// e.g., `Parent -> DOF1 -> DOF2`. In such cases, the object versions may end up like
+    /// `Parent >= DOF1, DOF2` but `DOF1 < DOF2`. Thus, database queries for dynamic fields must
+    /// bound the object versions by the version of the root object of the tree.
+    ///
+    /// Essentially, lamport timestamps of objects are updated for all top-level mutable objects
+    /// provided as inputs to a transaction as well as any mutated dynamic child objects. However,
+    /// any dynamic child objects that were loaded but not actually mutated don't end up having
+    /// their versions updated.
+    root_version: u64,
 }
 
 /// Type to implement GraphQL fields that are shared by all Objects.
@@ -462,7 +474,7 @@ impl Object {
         name: DynamicFieldName,
     ) -> Result<Option<DynamicField>> {
         OwnerImpl::from(self)
-            .dynamic_field(ctx, name, Some(self.version_impl()))
+            .dynamic_field(ctx, name, Some(self.root_version()))
             .await
     }
 
@@ -479,7 +491,7 @@ impl Object {
         name: DynamicFieldName,
     ) -> Result<Option<DynamicField>> {
         OwnerImpl::from(self)
-            .dynamic_object_field(ctx, name, Some(self.version_impl()))
+            .dynamic_object_field(ctx, name, Some(self.root_version()))
             .await
     }
 
@@ -496,7 +508,7 @@ impl Object {
         before: Option<Cursor>,
     ) -> Result<Connection<String, DynamicField>> {
         OwnerImpl::from(self)
-            .dynamic_fields(ctx, first, after, last, before, Some(self.version_impl()))
+            .dynamic_fields(ctx, first, after, last, before, Some(self.root_version()))
             .await
     }
 
@@ -672,15 +684,24 @@ impl Object {
     /// `checkpoint_viewed_at` represents the checkpoint sequence number at which this `Object` was
     /// constructed in. This is stored on `Object` so that when viewing that entity's state, it will
     /// be as if it was read at the same checkpoint.
+    ///
+    /// `root_version` represents the version of the root object in some nested chain of dynamic
+    /// fields. This should typically be left `None`, unless the object(s) being resolved is a
+    /// dynamic field, or if `root_version` has been explicitly set for this object. If None, then
+    /// we use [`version_for_dynamic_fields`] to infer a root version to then propagate from this
+    /// object down to its dynamic fields.
     pub(crate) fn from_native(
         address: SuiAddress,
         native: NativeObject,
         checkpoint_viewed_at: u64,
+        root_version: Option<u64>,
     ) -> Object {
+        let root_version = root_version.unwrap_or_else(|| version_for_dynamic_fields(&native));
         Object {
             address,
             kind: ObjectKind::NotIndexed(native),
             checkpoint_viewed_at,
+            root_version,
         }
     }
 
@@ -702,6 +723,13 @@ impl Object {
         }
     }
 
+    /// Root parent object version for dynamic fields.
+    ///
+    /// Check [`Object::root_version`] for details.
+    pub(crate) fn root_version(&self) -> u64 {
+        self.root_version
+    }
+
     /// Query the database for a `page` of objects, optionally `filter`-ed.
     ///
     /// `checkpoint_viewed_at` represents the checkpoint sequence number at which this page was
@@ -766,7 +794,8 @@ impl Object {
             // To maintain consistency, the returned cursor should have the same upper-bound as the
             // checkpoint found on the cursor.
             let cursor = stored.cursor(checkpoint_viewed_at).encode_cursor();
-            let object = Object::try_from_stored_history_object(stored, checkpoint_viewed_at)?;
+            let object =
+                Object::try_from_stored_history_object(stored, checkpoint_viewed_at, None)?;
             conn.edges.push(Edge::new(cursor, downcast(object)?));
         }
 
@@ -854,9 +883,16 @@ impl Object {
     /// `checkpoint_viewed_at` represents the checkpoint sequence number at which this `Object` was
     /// constructed in. This is stored on `Object` so that when viewing that entity's state, it will
     /// be as if it was read at the same checkpoint.
+    ///
+    /// `root_version` represents the version of the root object in some nested chain of dynamic
+    /// fields. This should typically be left `None`, unless the object(s) being resolved is a
+    /// dynamic field, or if `root_version` has been explicitly set for this object. If None, then
+    /// we use [`version_for_dynamic_fields`] to infer a root version to then propagate from this
+    /// object down to its dynamic fields.
     pub(crate) fn try_from_stored_history_object(
         history_object: StoredHistoryObject,
         checkpoint_viewed_at: u64,
+        root_version: Option<u64>,
     ) -> Result<Self, Error> {
         let address = addr(&history_object.object_id)?;
 
@@ -881,10 +917,13 @@ impl Object {
                     Error::Internal(format!("Failed to deserialize object {address}"))
                 })?;
 
+                let root_version =
+                    root_version.unwrap_or_else(|| version_for_dynamic_fields(&native_object));
                 Ok(Self {
                     address,
                     kind: ObjectKind::Indexed(native_object, history_object),
                     checkpoint_viewed_at,
+                    root_version,
                 })
             }
             NativeObjectStatus::WrappedOrDeleted => Ok(Self {
@@ -896,11 +935,23 @@ impl Object {
                     checkpoint_sequence_number: history_object.checkpoint_sequence_number,
                 }),
                 checkpoint_viewed_at,
+                root_version: history_object.object_version as u64,
             }),
         }
     }
 }
 
+/// We're deliberately choosing to use a child object's version as the root here, and letting the
+/// caller override it with the actual root object's version if it has access to it.
+///
+/// Using the child object's version as the root means that we're seeing the dynamic field tree
+/// under this object at the state resulting from the transaction that produced this version.
+///
+/// See [`Object::root_version`] for more details on parent/child object version mechanics.
+fn version_for_dynamic_fields(native: &NativeObject) -> u64 {
+    native.as_inner().version().into()
+}
+
 impl ObjectFilter {
     /// Try to create a filter whose results are the intersection of objects in `self`'s results and
     /// objects in `other`'s results. This may not be possible if the resulting filter is
@@ -1167,8 +1218,14 @@ impl Loader<HistoricalKey> for Db {
                 continue;
             }
 
-            let object =
-                Object::try_from_stored_history_object(stored.clone(), key.checkpoint_viewed_at)?;
+            let object = Object::try_from_stored_history_object(
+                stored.clone(),
+                key.checkpoint_viewed_at,
+                // This conversion will use the object's own version as the `Object::root_version`
+                // if it's not a child object, but if it is, it will be set to `None`, since we do
+                // not have any information on the `root_version` of the fetched dynamic object.
+                None,
+            )?;
             result.insert(*key, object);
         }
 
@@ -1255,8 +1312,13 @@ impl Loader<LatestAtKey> for Db {
             for (group_key, stored) in
                 group.map_err(|e| Error::Internal(format!("Failed to fetch objects: {e}")))?
             {
-                let object =
-                    Object::try_from_stored_history_object(stored, group_key.checkpoint_viewed_at)?;
+                let object = Object::try_from_stored_history_object(
+                    stored,
+                    group_key.checkpoint_viewed_at,
+                    // If `LatestAtKey::parent_version` is set, it must have been correctly
+                    // propagated from the `Object::root_version` of some object.
+                    group_key.parent_version,
+                )?;
 
                 let key = LatestAtKey {
                     id: object.address,

crates/sui-graphql-rpc/src/types/stake.rs

@@ -252,7 +252,7 @@ impl StakedSui {
         name: DynamicFieldName,
     ) -> Result<Option<DynamicField>> {
         OwnerImpl::from(&self.super_.super_)
-            .dynamic_field(ctx, name, Some(self.super_.super_.version_impl()))
+            .dynamic_field(ctx, name, Some(self.super_.root_version()))
             .await
     }
 
@@ -269,7 +269,7 @@ impl StakedSui {
         name: DynamicFieldName,
     ) -> Result<Option<DynamicField>> {
         OwnerImpl::from(&self.super_.super_)
-            .dynamic_object_field(ctx, name, Some(self.super_.super_.version_impl()))
+            .dynamic_object_field(ctx, name, Some(self.super_.root_version()))
             .await
     }
 
@@ -292,7 +292,7 @@ impl StakedSui {
                 after,
                 last,
                 before,
-                Some(self.super_.super_.version_impl()),
+                Some(self.super_.root_version()),
             )
             .await
     }