Skip to content

Commit b124567

Browse files
[sui-fork] Fix child object reads to support bounded versions (#26966)
## Description This PR adds support for reading child object bounded versions. Without this, a bunch of transactions do not work correctly and the backend complains with an invariant violation, because we're passing the latest object rather than the correct bounded version of that child object. ## Test plan Several tests added that cover loading the object at the right version. --- ## Release notes Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required. For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates. - [ ] Protocol: - [ ] Nodes (Validators and Full nodes): - [ ] gRPC: - [ ] JSON-RPC: - [ ] GraphQL: - [ ] CLI: - [ ] Rust SDK: - [ ] Indexing Framework:
1 parent 9b9e32f commit b124567

4 files changed

Lines changed: 310 additions & 8 deletions

File tree

crates/sui-fork/src/filesystem.rs

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,16 @@ struct ObjectLatestMetadata {
141141
state: ObjectLatestState,
142142
}
143143

144+
/// Result of a bounded local object lookup.
145+
pub(crate) enum BoundedObjectLookup {
146+
/// A live object version at or below the requested bound was found locally.
147+
Hit(Object),
148+
/// A local tombstone at or below the requested bound proves the object is absent.
149+
NegativeHit,
150+
/// Local storage has no authoritative answer for the requested bound.
151+
Miss,
152+
}
153+
144154
impl ObjectLatestMetadata {
145155
/// Construct numeric-only latest metadata for a live object version.
146156
fn live(version: u64) -> Self {
@@ -445,6 +455,40 @@ impl FilesystemStore {
445455
self.read_bcs_file(&version_file).map(Some)
446456
}
447457

458+
/// Get the highest locally persisted object version at or below `version_bound`.
459+
pub(crate) fn get_object_lt_or_eq_version(
460+
&self,
461+
object_id: &ObjectID,
462+
version_bound: u64,
463+
) -> anyhow::Result<BoundedObjectLookup> {
464+
let object_dir = self.objects_dir().join(object_id.to_string());
465+
if !object_dir.exists() {
466+
return Ok(BoundedObjectLookup::Miss);
467+
}
468+
469+
if let Some(latest) = self.read_object_latest_metadata_if_exists(&object_dir)?
470+
&& latest.version <= version_bound
471+
{
472+
if latest.state.is_removed() {
473+
return Ok(BoundedObjectLookup::NegativeHit);
474+
}
475+
476+
let version_file = object_dir.join(latest.version.to_string());
477+
return self
478+
.read_bcs_file(&version_file)
479+
.map(BoundedObjectLookup::Hit);
480+
}
481+
482+
let Some(version) =
483+
self.find_highest_object_version_lt_or_eq(&object_dir, version_bound)?
484+
else {
485+
return Ok(BoundedObjectLookup::Miss);
486+
};
487+
488+
self.read_bcs_file(&object_dir.join(version.to_string()))
489+
.map(BoundedObjectLookup::Hit)
490+
}
491+
448492
/// Write an object fetched from remote/cache paths. Existing deleted or wrapped current-state
449493
/// metadata is preserved so remote reads cannot resurrect local removals.
450494
pub(crate) fn write_object(&self, object: &Object) -> anyhow::Result<()> {
@@ -603,6 +647,54 @@ impl FilesystemStore {
603647
.with_context(|| format!("Failed to write latest file: {}", latest_file.display()))
604648
}
605649

650+
/// Scan the object directory for the highest version at or below the bound, ignoring
651+
/// non-version files.
652+
fn find_highest_object_version_lt_or_eq(
653+
&self,
654+
object_dir: &Path,
655+
version_bound: u64,
656+
) -> anyhow::Result<Option<u64>> {
657+
let mut best = None;
658+
for entry in fs::read_dir(object_dir)
659+
.with_context(|| format!("Failed to read object directory: {}", object_dir.display()))?
660+
{
661+
let entry = entry.with_context(|| {
662+
format!(
663+
"Failed to read object directory entry: {}",
664+
object_dir.display()
665+
)
666+
})?;
667+
668+
if !entry
669+
.file_type()
670+
.with_context(|| {
671+
format!(
672+
"Failed to read object file type: {}",
673+
entry.path().display()
674+
)
675+
})?
676+
.is_file()
677+
{
678+
continue;
679+
}
680+
681+
let file_name = entry.file_name();
682+
let Some(name) = file_name.to_str() else {
683+
continue;
684+
};
685+
let Ok(version) = name.parse::<u64>() else {
686+
continue;
687+
};
688+
689+
// Compare the current best version with the candidate version, and update it if the
690+
// candidate version is higher but still within bound.
691+
if version <= version_bound && best.is_none_or(|best| version > best) {
692+
best = Some(version);
693+
}
694+
}
695+
Ok(best)
696+
}
697+
606698
/// Read the owned-object index. Missing index files represent an empty index.
607699
pub(crate) fn get_owned_object_entries(&self) -> anyhow::Result<Vec<OwnedObjectEntry>> {
608700
let path = self.owned_objects_index_path();

crates/sui-fork/src/store.rs

Lines changed: 36 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@ use crate::ObjectRead;
7474
use crate::TransactionInfo;
7575
use crate::TransactionRead;
7676
use crate::VersionQuery;
77+
use crate::filesystem::BoundedObjectLookup;
7778
use crate::filesystem::FilesystemStore;
7879
use crate::filesystem::ObjectLatestState;
7980
use crate::filesystem::OwnedObjectEntry;
@@ -342,6 +343,36 @@ impl DataStore {
342343
Ok(object)
343344
}
344345

346+
/// Get the latest object version at or below the given root version.
347+
fn get_object_lt_or_eq_version(
348+
&self,
349+
object_id: &ObjectID,
350+
version_bound: SequenceNumber,
351+
) -> anyhow::Result<Option<Object>> {
352+
match self
353+
.inner
354+
.local
355+
.get_object_lt_or_eq_version(object_id, version_bound.value())?
356+
{
357+
BoundedObjectLookup::Hit(object) => return Ok(Some(object)),
358+
BoundedObjectLookup::NegativeHit => return Ok(None),
359+
BoundedObjectLookup::Miss => {}
360+
}
361+
362+
let mut objects = self.inner.gql.get_objects(&[ObjectKey {
363+
object_id: *object_id,
364+
version_query: VersionQuery::RootVersion(version_bound.value()),
365+
}])?;
366+
let object = objects.pop().flatten().map(|(object, _)| object);
367+
368+
if let Some(ref object) = object {
369+
let _local_snapshot_guard = self.write_local_snapshot()?;
370+
self.inner.local.write_object(object)?;
371+
}
372+
373+
Ok(object)
374+
}
375+
345376
/// Local-first lookup for the latest known version of an object. Falls back to a remote
346377
/// `AtCheckpoint(forked_at_checkpoint)` query and caches the result on disk.
347378
fn get_latest_object(&self, object_id: &ObjectID) -> anyhow::Result<Option<Object>> {
@@ -847,7 +878,11 @@ impl ChildObjectResolver for DataStore {
847878
child: &ObjectID,
848879
child_version_upper_bound: SequenceNumber,
849880
) -> SuiResult<Option<Object>> {
850-
let child_object = match self.get_object(child).ok().flatten() {
881+
let child_object = match self
882+
.get_object_lt_or_eq_version(child, child_version_upper_bound)
883+
.ok()
884+
.flatten()
885+
{
851886
None => return Ok(None),
852887
Some(obj) => obj,
853888
};
@@ -861,13 +896,6 @@ impl ChildObjectResolver for DataStore {
861896
.into());
862897
}
863898

864-
if child_object.version() > child_version_upper_bound {
865-
return Err(sui_types::error::SuiErrorKind::UnsupportedFeatureError {
866-
error: "DataStore::read_child_object does not yet support bounded reads".to_owned(),
867-
}
868-
.into());
869-
}
870-
871899
Ok(Some(child_object))
872900
}
873901

crates/sui-fork/src/tests/filesystem.rs

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -222,6 +222,72 @@ fn test_get_object_at_version_returns_none_for_unknown_version() {
222222
assert!(result.is_none());
223223
}
224224

225+
#[test]
226+
fn test_object_lt_or_eq_returns_highest_version_within_bound() {
227+
let (_dir, store) = test_store();
228+
let id = ObjectID::random();
229+
let v1 = make_object(id, 1);
230+
let v3 = make_object(id, 3);
231+
let v5 = make_object(id, 5);
232+
233+
store.write_object(&v1).unwrap();
234+
store.write_object(&v3).unwrap();
235+
store.write_object(&v5).unwrap();
236+
237+
let bounded = store.get_object_lt_or_eq_version(&id, 4).unwrap();
238+
let BoundedObjectLookup::Hit(object) = bounded else {
239+
panic!("expected bounded object hit");
240+
};
241+
assert_eq!(object, v3);
242+
243+
let exact = store.get_object_lt_or_eq_version(&id, 5).unwrap();
244+
let BoundedObjectLookup::Hit(object) = exact else {
245+
panic!("expected exact-bound object hit");
246+
};
247+
assert_eq!(object, v5);
248+
}
249+
250+
#[test]
251+
fn test_object_lt_or_eq_misses_when_no_local_version_within_bound() {
252+
let (_dir, store) = test_store();
253+
let id = ObjectID::random();
254+
let object = make_object(id, 5);
255+
256+
store.write_object(&object).unwrap();
257+
258+
assert!(matches!(
259+
store.get_object_lt_or_eq_version(&id, 4).unwrap(),
260+
BoundedObjectLookup::Miss,
261+
));
262+
}
263+
264+
#[test]
265+
fn test_removed_latest_at_or_below_bound_is_negative_hit() {
266+
let (_dir, store) = test_store();
267+
let id = ObjectID::random();
268+
let object = make_object(id, 3);
269+
270+
store.write_object(&object).unwrap();
271+
store
272+
.mark_object_as_wrapped(id, SequenceNumber::from_u64(5))
273+
.unwrap();
274+
275+
assert!(matches!(
276+
store.get_object_lt_or_eq_version(&id, 5).unwrap(),
277+
BoundedObjectLookup::NegativeHit,
278+
));
279+
assert!(matches!(
280+
store.get_object_lt_or_eq_version(&id, 6).unwrap(),
281+
BoundedObjectLookup::NegativeHit,
282+
));
283+
284+
let before_wrap = store.get_object_lt_or_eq_version(&id, 4).unwrap();
285+
let BoundedObjectLookup::Hit(before_wrap) = before_wrap else {
286+
panic!("expected object version before wrap");
287+
};
288+
assert_eq!(before_wrap, object);
289+
}
290+
225291
#[test]
226292
fn test_latest_tracks_highest_written_version() {
227293
let (_dir, store) = test_store();

crates/sui-fork/src/tests/store_execution.rs

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,28 @@ fn object_at_checkpoint_response(object: &Object) -> serde_json::Value {
136136
})
137137
}
138138

139+
fn objects_response(objects: &[Option<&Object>]) -> serde_json::Value {
140+
serde_json::json!({
141+
"data": {
142+
"multiGetObjects": objects
143+
.iter()
144+
.map(|object| {
145+
object.map(|object| {
146+
serde_json::json!({
147+
"address": object.id().to_string(),
148+
"version": object.version().value(),
149+
"objectBcs": FastCryptoBase64::from_bytes(
150+
&bcs::to_bytes(object).expect("object should serialize"),
151+
)
152+
.encoded(),
153+
})
154+
})
155+
})
156+
.collect::<Vec<_>>(),
157+
}
158+
})
159+
}
160+
139161
async fn mock_seed_object(server: &MockServer, checkpoint: u64, object: &Object) {
140162
Mock::given(method("POST"))
141163
.and(path("/"))
@@ -538,6 +560,100 @@ fn test_missing_owned_index_after_local_checkpoint_advancement_fails_closed() {
538560
);
539561
}
540562

563+
#[test]
564+
fn test_read_child_object_uses_highest_local_version_within_bound() {
565+
let (_temp, store) = test_data_store();
566+
let parent = ObjectID::random();
567+
let child_id = ObjectID::random();
568+
let child_v5 = make_gas_object(child_id, 5, Owner::ObjectOwner(parent.into()));
569+
let child_v7 = make_gas_object(child_id, 7, Owner::ObjectOwner(parent.into()));
570+
571+
store.local().write_object(&child_v5).unwrap();
572+
store.local().write_object(&child_v7).unwrap();
573+
574+
let child = sui_types::storage::ChildObjectResolver::read_child_object(
575+
&store,
576+
&parent,
577+
&child_id,
578+
SequenceNumber::from_u64(6),
579+
)
580+
.expect("bounded child read should not error")
581+
.expect("child object should be found");
582+
583+
assert_eq!(child, child_v5);
584+
}
585+
586+
#[tokio::test]
587+
async fn test_read_child_object_falls_back_to_remote_root_version() {
588+
let temp = tempfile::tempdir().expect("failed to create tempdir");
589+
let checkpoint = 42;
590+
let parent = ObjectID::random();
591+
let child_id = ObjectID::random();
592+
let child = make_gas_object(child_id, 5, Owner::ObjectOwner(parent.into()));
593+
594+
let server = MockServer::start().await;
595+
Mock::given(method("POST"))
596+
.and(path("/"))
597+
.and(body_partial_json(serde_json::json!({
598+
"variables": {
599+
"keys": [
600+
{
601+
"address": child_id.to_string(),
602+
"rootVersion": 6,
603+
},
604+
],
605+
}
606+
})))
607+
.respond_with(ResponseTemplate::new(200).set_body_json(objects_response(&[Some(&child)])))
608+
.mount(&server)
609+
.await;
610+
611+
let store =
612+
DataStore::new_for_testing_with_remote(temp.path().to_path_buf(), server.uri(), checkpoint);
613+
let read = sui_types::storage::ChildObjectResolver::read_child_object(
614+
&store,
615+
&parent,
616+
&child_id,
617+
SequenceNumber::from_u64(6),
618+
)
619+
.expect("remote bounded child read should not error")
620+
.expect("child object should be found");
621+
622+
assert_eq!(read, child);
623+
assert_eq!(
624+
store.local().get_object_at_version(&child_id, 5).unwrap(),
625+
Some(child),
626+
);
627+
}
628+
629+
#[test]
630+
fn test_read_child_object_rejects_wrong_owner_after_bounded_lookup() {
631+
let (_temp, store) = test_data_store();
632+
let parent = ObjectID::random();
633+
let other_parent = ObjectID::random();
634+
let child_id = ObjectID::random();
635+
let child = make_gas_object(child_id, 5, Owner::ObjectOwner(other_parent.into()));
636+
637+
store.local().write_object(&child).unwrap();
638+
639+
let err = sui_types::storage::ChildObjectResolver::read_child_object(
640+
&store,
641+
&parent,
642+
&child_id,
643+
SequenceNumber::from_u64(6),
644+
)
645+
.expect_err("wrong child owner should error");
646+
647+
assert!(matches!(
648+
err.as_inner(),
649+
sui_types::error::SuiErrorKind::InvalidChildObjectAccess {
650+
object,
651+
given_parent,
652+
actual_owner,
653+
} if *object == child_id && *given_parent == parent && actual_owner == &child.owner
654+
));
655+
}
656+
541657
#[test]
542658
fn test_local_deletion_removes_current_object_but_preserves_historical_lookup() {
543659
let (_temp, mut store) = test_data_store();

0 commit comments

Comments
 (0)