Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

graphql: ObjectOwner::Parent exposed as Owner #19785

Merged
merged 3 commits into from
Oct 10, 2024
Merged

graphql: ObjectOwner::Parent exposed as Owner #19785

merged 3 commits into from
Oct 10, 2024

Conversation

amnn
Copy link
Member

@amnn amnn commented Oct 10, 2024

Description

Now that we no longer expose wrapped objects, we need a way to expose the addresses of object parents when they are other objects that have been wrapped.

Test plan

Updated tests:

sui$ cargo nextest run -p sui-graphql-e2e-tests
sui$ cargo nextest run -p sui-graphql-rpc

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):
  • Indexer:
  • JSON-RPC:
  • GraphQL: Change Parent.parent from an Object to an Owner. Although it's guaranteed to be an object if it exists, it may be wrapped, in which case it will not exist. Exposing it as an Owner allows queries to extract its ID and also fetch other dynamic fields from it even if it is wrapped.
  • CLI:
  • Rust SDK:
  • REST API:

## Description

Now that we no longer expose wrapped objects, we need a way to expose
the addresses of object parents when they are other objects that have
been wrapped.

## Test plan

Updated tests:

```
sui$ cargo nextest run -p sui-graphql-e2e-tests
sui$ cargo nextest run -p sui-graphql-rpc
```
@amnn amnn requested review from lxfind, bmwill and gegaowp October 10, 2024 13:37
@amnn amnn self-assigned this Oct 10, 2024
Copy link

vercel bot commented Oct 10, 2024

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

Name Status Preview Comments Updated (UTC)
sui-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 10, 2024 5:24pm
sui-typescript-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 10, 2024 5:24pm
2 Skipped Deployments
Name Status Preview Comments Updated (UTC)
multisig-toolkit ⬜️ Ignored (Inspect) Visit Preview Oct 10, 2024 5:24pm
sui-kiosk ⬜️ Ignored (Inspect) Visit Preview Oct 10, 2024 5:24pm

@@ -147,6 +147,7 @@ pub(crate) fn build_objects_query(
// Similar to the snapshot query, construct the filtered inner query for the history table.
let mut history_objs_inner = query!("SELECT * FROM objects_history");
history_objs_inner = filter_fn(history_objs_inner);
history_objs_inner = filter!(history_objs_inner, "object_status = 0");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sry that i missed it here and thx for the fix, is there an easy way to generate a sample query here so that I can verify it on psql?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You didn't miss it in this case -- I've just moved the filter up into a central place, rather than applying at the end to each query, because it was ambiguous whether the object_status query would apply just to the historical candidates, the newer values or both (and we only want it to apply to the historical candidates).

If you run the GraphQL service with debug logs enabled, you can see every query that gets run. I ran the tests as follows for example:

sui$ RUST_LOG="info,sui_graphql_rpc::data::pg=debug" \
  cargo nextest run -p sui-graphql-e2e-tests --no-capture \
  -- call/dynamic_fields.move

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants