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

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, self.super_.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, self.super_.super_.root_version())
             .await
     }
 
@@ -284,7 +284,7 @@ impl Coin {
                 after,
                 last,
                 before,
-                Some(self.super_.super_.version_impl()),
+                self.super_.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, self.super_.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, self.super_.super_.root_version())
             .await
     }
 
@@ -273,7 +273,7 @@ impl CoinMetadata {
                 after,
                 last,
                 before,
-                Some(self.super_.super_.version_impl()),
+                self.super_.super_.root_version(),
             )
             .await
     }

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

@@ -26,6 +26,7 @@ pub(crate) struct DynamicField {
     pub super_: MoveObject,
     pub df_object_id: SuiAddress,
     pub df_kind: DynamicFieldType,
+    parent_version: Option<u64>,
 }
 
 #[derive(Union)]
@@ -107,12 +108,11 @@ 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,
-                ),
+                if let Some(parent_version) = self.parent_version {
+                    Object::under_parent(parent_version, self.super_.super_.checkpoint_viewed_at)
+                } else {
+                    Object::latest_at(self.super_.super_.checkpoint_viewed_at)
+                },
             )
             .await
             .extend()?;
@@ -181,7 +181,9 @@ impl DynamicField {
         )
         .await?;
 
-        super_.map(Self::try_from).transpose()
+        super_
+            .map(|mo| Self::try_from((mo, parent_version)))
+            .transpose()
     }
 
     /// Query the `db` for a `page` of dynamic fields attached to object with ID `parent`. The
@@ -228,7 +230,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!(
@@ -237,18 +243,18 @@ impl DynamicField {
                 ))
             })?;
 
-            let dynamic_field = DynamicField::try_from(move_)?;
+            let dynamic_field = DynamicField::try_from((move_, parent_version))?;
             conn.edges.push(Edge::new(cursor, dynamic_field));
         }
 
         Ok(conn)
     }
 }
 
-impl TryFrom<MoveObject> for DynamicField {
+impl TryFrom<(MoveObject, Option<u64>)> for DynamicField {
     type Error = Error;
 
-    fn try_from(stored: MoveObject) -> Result<Self, Error> {
+    fn try_from((stored, parent_version): (MoveObject, Option<u64>)) -> Result<Self, Error> {
         let super_ = &stored.super_;
 
         let (df_object_id, df_kind) = match &super_.kind {
@@ -284,6 +290,7 @@ impl TryFrom<MoveObject> for DynamicField {
             super_: stored,
             df_object_id,
             df_kind,
+            parent_version,
         })
     }
 }

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, self.super_.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, self.super_.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, self.super_.root_version())
             .await
     }
 

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,
+    /// Optional root parent object version if this is a dynamic field.
+    ///
+    /// 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: Option<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, 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, 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, self.root_version())
             .await
     }
 
@@ -676,11 +688,23 @@ impl Object {
         address: SuiAddress,
         native: NativeObject,
         checkpoint_viewed_at: u64,
+        root_version: Option<u64>,
     ) -> Object {
+        let root_version = root_version.or_else(|| Self::infer_root_version(&native));
         Object {
             address,
             kind: ObjectKind::NotIndexed(native),
             checkpoint_viewed_at,
+            root_version,
+        }
+    }
+
+    fn infer_root_version(native: &NativeObject) -> Option<u64> {
+        match native.as_inner().owner {
+            NativeOwner::AddressOwner(_) | NativeOwner::Shared { .. } => {
+                Some(native.as_inner().version().into())
+            }
+            _ => None,
         }
     }
 
@@ -702,6 +726,10 @@ impl Object {
         }
     }
 
+    pub(crate) fn root_version(&self) -> Option<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)?));
         }
 
@@ -857,6 +886,7 @@ impl Object {
     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 +911,13 @@ impl Object {
                     Error::Internal(format!("Failed to deserialize object {address}"))
                 })?;
 
+                let root_version =
+                    root_version.or_else(|| Self::infer_root_version(&native_object));
                 Ok(Self {
                     address,
                     kind: ObjectKind::Indexed(native_object, history_object),
                     checkpoint_viewed_at,
+                    root_version,
                 })
             }
             NativeObjectStatus::WrappedOrDeleted => Ok(Self {
@@ -896,6 +929,7 @@ impl Object {
                     checkpoint_sequence_number: history_object.checkpoint_sequence_number,
                 }),
                 checkpoint_viewed_at,
+                root_version: None,
             }),
         }
     }
@@ -1167,8 +1201,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 will infer the `Object::root_version` if it's not a child object, but if it
+                // is, it will be set to `None`, so dynamic object queries will be bounded by
+                // `checkpoint_viewed_at`.
+                None,
+            )?;
             result.insert(*key, object);
         }
 
@@ -1255,8 +1295,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, self.super_.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, self.super_.super_.root_version())
             .await
     }
 
@@ -292,7 +292,7 @@ impl StakedSui {
                 after,
                 last,
                 before,
-                Some(self.super_.super_.version_impl()),
+                self.super_.super_.root_version(),
             )
             .await
     }

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

@@ -289,7 +289,7 @@ impl SuinsRegistration {
         name: DynamicFieldName,
     ) -> Result<Option<DynamicField>> {
         OwnerImpl::from(&self.super_.super_)
-            .dynamic_field(ctx, name, Some(self.super_.super_.version_impl()))
+            .dynamic_field(ctx, name, self.super_.super_.root_version())
             .await
     }
 
@@ -306,7 +306,7 @@ impl SuinsRegistration {
         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, self.super_.super_.root_version())
             .await
     }
 
@@ -329,7 +329,7 @@ impl SuinsRegistration {
                 after,
                 last,
                 before,
-                Some(self.super_.super_.version_impl()),
+                self.super_.super_.root_version(),
             )
             .await
     }
@@ -509,7 +509,8 @@ impl NameService {
         // name_record. We then assign it to the correct field on `domain_expiration` based on the
         // address.
         for result in results {
-            let object = Object::try_from_stored_history_object(result, checkpoint_viewed_at)?;
+            let object =
+                Object::try_from_stored_history_object(result, checkpoint_viewed_at, None)?;
             let move_object = MoveObject::try_from(&object).map_err(|_| {
                 Error::Internal(format!(
                     "Expected {0} to be a NameRecord, but it's not a Move Object.",