fetch past objects #4367

longbowlu · 2022-08-30T01:36:26Z

as title. Add for debugging purposes but should be useful for explorer and other use cases too.

meat is in authority.rs .

ExistsButPastNotFound is awkward, so plz suggest a better name if you have one.

lxfind

I recommend naming all the seq_num in rpc interfaces to something like seq_num_debug and document that this is for short-term debugging only, so that people don't end up depending on historic objects to be always available.

lxfind · 2022-08-30T03:19:49Z

crates/sui-types/src/object.rs

@@ -640,6 +640,14 @@ pub enum ObjectRead {
    NotExists(ObjectID),
    Exists(ObjectRef, Object, Option<MoveStructLayout>),
    Deleted(ObjectRef),
+    /// The object exists but not found in this node (could be pruned)
+    ExistsButPastNotFound(ObjectID, SequenceNumber),


I think ExistsButVersionNotFound is probably a better name.

indeed, thanks for the suggestion

longbowlu · 2022-08-30T03:51:00Z

I recommend naming all the seq_num in rpc interfaces to something like seq_num_debug and document that this is for short-term debugging only, so that people don't end up depending on historic objects to be always available.

Even in archival/partial node, we can't assume every object can be found in a node. I think we should educate callers about the potential unavailability in general by having better documents/comments. The term debug daunts people from using it even though it's helpful

lxfind · 2022-08-30T04:56:53Z

hmm do you actually want people to use this API? I was under the impression that only we developers would ever use it for debugging purposes.
Is there a scenario you have in mind that applications/clients might want to use it? I am worried that people might think this is a feature they could ever depend on.

666lcz · 2022-08-30T05:04:44Z

@lxfind @sblackshear mentioned in #3992 (comment) this might be something required by exchanges. It could also power some interesting features on the Explorer side to show object history. If we have consistent pruning policy(e.g., only store up to 5 most recent versions) and make it clear to the users, does it resolve your concern?

patrickkuo · 2022-08-30T09:47:20Z

@lxfind @sblackshear mentioned in #3992 (comment) this might be something required by exchanges. It could also power some interesting features on the Explorer side to show object history. If we have consistent pruning policy(e.g., only store up to 5 most recent versions) and make it clear to the users, does it resolve your concern?

This is great and definitely useful for the exchanges! But I think we will need a bit more than this to satisfy exchanges requirement.... what the exchange need (at lease for Rosetta) is a view of accounts at a particular slice of time/ block height/ checkpoint. Our seq numbers are local to the object, to get a global view at a certain block height/ checkpoint we will need to work out the seq number for all object at that time, which we don't store at the moment (can we store this information with checkpoint?) and it's hard to compute.

mystenmark · 2022-08-30T14:46:32Z

to get a global view at a certain block height/ checkpoint we will need to work out the seq number for all object at that time

Wow, do we really need to do this? its going to be a bit tricky

mystenmark · 2022-08-30T14:56:21Z

crates/sui-json-rpc/src/read_api.rs

@@ -74,10 +75,14 @@ impl RpcReadApiServer for ReadApi {
            .collect())
    }

-    async fn get_object(&self, object_id: ObjectID) -> RpcResult<GetObjectDataResponse> {
+    async fn get_object(


I think it might make a lot more sense to leave this API as is, and add a new API called get_archived_object_version or something similar. This would:

Keep this API simpler - it can continue returning the exists/not-exists/deleted enum.

Simplify the implementation of get_object_read which is now very confusing.

Make it clearer to the user that they are trying to access an archive, not live state, which may be pruned.

Simplify the error return values.

However, If you choose to continue with the current approach, may I then suggest the following:

Merge SequenceNumberTooHigh into NotExists.

Rename ExistsButPastNotFound to ObjectVersionPruned

@mystenmark Split into two functions. ExistsButPastNotFound renamed to VersionNotFound. I kept VersionTooHigh as it's trivial to tell and may be helpful to callers

longbowlu · 2022-08-30T18:34:21Z

If we have consistent pruning policy(e.g., only store up to 5 most recent versions) and make it clear to the users, does it resolve your concern?

IMO the node should have the flexibility to configure their pruning policy to fit their own business needs. What matters here is the caller of these apis should NOT be surprised to see a past object does not reside in this node. If they are, then they should talk to the node operators.

longbowlu · 2022-08-30T18:37:20Z

what the exchange need (at lease for Rosetta) is a view of accounts at a particular slice of time/ block height/ checkpoint. Our seq numbers are local to the object, to get a global view at a certain block height/ checkpoint we will need to work out the seq number for all object at that time, which we don't store at the moment (can we store this information with checkpoint?) and it's hard to compute.

How much coarseness can Resetta tolerate? Seq num is not the right identifier here. Checkpoint is much more suitable but it's a big interval.

patrickkuo · 2022-09-03T14:41:16Z

what the exchange need (at lease for Rosetta) is a view of accounts at a particular slice of time/ block height/ checkpoint. Our seq numbers are local to the object, to get a global view at a certain block height/ checkpoint we will need to work out the seq number for all object at that time, which we don't store at the moment (can we store this information with checkpoint?) and it's hard to compute.

How much coarseness can Resetta tolerate? Seq num is not the right identifier here. Checkpoint is much more suitable but it's a big interval.

The issue is not the coarseness, in Sui we are object based, we can easily work out the history of an object, however Rosetta is interested in account/ ownership of object at a past checkpoint, which we don't currently store....

lxfind · 2022-09-03T15:20:52Z

I think what we will eventually end up will be a specialized implementation of indexer/fullnode to support Rosetta API. For instance, we can maintain an account address balance history table, and update it at checkpoint boundaries.

patrickkuo

2 minor comments, otherwise LGTM

crates/sui-json-rpc/src/api.rs

crates/sui-json-rpc/src/gateway_api.rs

crates/sui-json-rpc-types/src/lib.rs

crates/sui-types/src/error.rs

longbowlu requested review from lxfind, patrickkuo, 666lcz, mystenmark and Jordan-Mysten August 30, 2022 01:36

lxfind reviewed Aug 30, 2022

View reviewed changes

mystenmark requested changes Aug 30, 2022

View reviewed changes

longbowlu force-pushed the fetch_past_object branch 2 times, most recently from c45b03a to 9adcc13 Compare September 2, 2022 22:20

longbowlu requested review from mystenmark and lxfind September 2, 2022 22:21

patrickkuo approved these changes Sep 6, 2022

View reviewed changes

crates/sui-json-rpc/src/api.rs Outdated Show resolved Hide resolved

crates/sui-json-rpc/src/gateway_api.rs Outdated Show resolved Hide resolved

longbowlu force-pushed the fetch_past_object branch from 9adcc13 to 50e6f1f Compare September 7, 2022 17:13

mystenmark approved these changes Sep 7, 2022

View reviewed changes

crates/sui-json-rpc-types/src/lib.rs Show resolved Hide resolved

crates/sui-json-rpc-types/src/lib.rs Show resolved Hide resolved

crates/sui-types/src/error.rs Outdated Show resolved Hide resolved

longbowlu added 3 commits September 9, 2022 00:36

fetch past objects

a8fa2a5

example + cleanup

3479642

move to fullnode api and rename maybe -> try

d1fb2db

longbowlu force-pushed the fetch_past_object branch from 50e6f1f to a7fb2f0 Compare September 9, 2022 07:56

longbowlu enabled auto-merge (squash) September 9, 2022 07:56

rebase

3e3f339

longbowlu force-pushed the fetch_past_object branch from a7fb2f0 to 3e3f339 Compare September 9, 2022 08:01

longbowlu merged commit 2bf1304 into main Sep 9, 2022

longbowlu deleted the fetch_past_object branch September 9, 2022 08:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fetch past objects #4367

fetch past objects #4367

longbowlu commented Aug 30, 2022

lxfind left a comment

lxfind Aug 30, 2022

longbowlu Aug 30, 2022

longbowlu commented Aug 30, 2022

lxfind commented Aug 30, 2022

666lcz commented Aug 30, 2022 •

edited

Loading

patrickkuo commented Aug 30, 2022 •

edited

Loading

mystenmark commented Aug 30, 2022

mystenmark Aug 30, 2022

longbowlu Sep 2, 2022

longbowlu commented Aug 30, 2022

longbowlu commented Aug 30, 2022 •

edited

Loading

patrickkuo commented Sep 3, 2022

lxfind commented Sep 3, 2022

patrickkuo left a comment

fetch past objects #4367

fetch past objects #4367

Conversation

longbowlu commented Aug 30, 2022

lxfind left a comment

Choose a reason for hiding this comment

lxfind Aug 30, 2022

Choose a reason for hiding this comment

longbowlu Aug 30, 2022

Choose a reason for hiding this comment

longbowlu commented Aug 30, 2022

lxfind commented Aug 30, 2022

666lcz commented Aug 30, 2022 • edited Loading

patrickkuo commented Aug 30, 2022 • edited Loading

mystenmark commented Aug 30, 2022

mystenmark Aug 30, 2022

Choose a reason for hiding this comment

longbowlu Sep 2, 2022

Choose a reason for hiding this comment

longbowlu commented Aug 30, 2022

longbowlu commented Aug 30, 2022 • edited Loading

patrickkuo commented Sep 3, 2022

lxfind commented Sep 3, 2022

patrickkuo left a comment

Choose a reason for hiding this comment

666lcz commented Aug 30, 2022 •

edited

Loading

patrickkuo commented Aug 30, 2022 •

edited

Loading

longbowlu commented Aug 30, 2022 •

edited

Loading