Skip to content

jsonrpc: make coin_api cursors opaque #21137

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

Merged
merged 3 commits into from
Feb 7, 2025
Merged

Conversation

bmwill
Copy link
Contributor

@bmwill bmwill commented Feb 7, 2025

Explicitly make the cursors for get_coins and get_all_coins to be an opaque string instead of an ObjectId encoded as a string.

Description

Describe the changes or additions included in this PR.

Test plan

How did you test the new or updated feature?


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:

Explicitly make the cursors for GetCoins and GetAllCoins to be an opaque string instead of an ObjectId encoded as a string.

  • GraphQL:
  • CLI:
  • Rust SDK:

Copy link

vercel bot commented Feb 7, 2025

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 Feb 7, 2025 10:51pm
2 Skipped Deployments
Name Status Preview Comments Updated (UTC)
multisig-toolkit ⬜️ Ignored (Inspect) Visit Preview Feb 7, 2025 10:51pm
sui-kiosk ⬜️ Ignored (Inspect) Visit Preview Feb 7, 2025 10:51pm

@bmwill bmwill temporarily deployed to sui-typescript-aws-kms-test-env February 7, 2025 16:29 — with GitHub Actions Inactive
Copy link
Contributor

@amnn amnn left a comment

Choose a reason for hiding this comment

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

LGTM, let's warn people proactively about this change so that they know to update their SDKs.

@@ -868,7 +867,7 @@ impl RpcExampleProvider {
.collect::<Vec<_>>();
let page = CoinPage {
data: coins,
next_cursor: Some(next),
next_cursor: Some("abcd".to_string()),
Copy link
Contributor

Choose a reason for hiding this comment

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

Do the values end up appearing in the examples at all? If so it would be good to use something that's recognisably Base64?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

while it may be base64 encoded, the fact that it is is not part of the api contract as it really does need to be "opaque".

Explicitly make the cursors for get_coins and get_all_coins to be an
opaque string instead of an ObjectId encoded as a string.
@bmwill bmwill force-pushed the jsonrpc-get-coins branch from e7c6fbc to 62420e7 Compare February 7, 2025 21:40
@bmwill bmwill temporarily deployed to sui-typescript-aws-kms-test-env February 7, 2025 21:40 — with GitHub Actions Inactive
@bmwill bmwill enabled auto-merge (rebase) February 7, 2025 21:42
@bmwill bmwill disabled auto-merge February 7, 2025 21:42
@bmwill bmwill enabled auto-merge (squash) February 7, 2025 21:42
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.

2 participants