Skip to content

Commit

Permalink
jsonrpc: make coin_api cursors opaque (#21137)
Browse files Browse the repository at this point in the history
Explicitly make the cursors for get_coins and get_all_coins to be an
opaque string instead of an ObjectId encoded as a string.
  • Loading branch information
bmwill authored Feb 7, 2025
1 parent 503e33a commit b86a009
Show file tree
Hide file tree
Showing 11 changed files with 369 additions and 318 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

21 changes: 11 additions & 10 deletions crates/sui-cluster-test/src/test_case/coin_index_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -523,16 +523,12 @@ impl TestCaseImpl for CoinIndexTest {
sui_coins_with_managed_coin_1.data.len(),
sui_coins.len() + 1
);
assert_eq!(
sui_coins_with_managed_coin_1.next_cursor,
Some(first_managed_coin)
);
assert!(sui_coins_with_managed_coin_1.has_next_page);
let cursor = sui_coins_with_managed_coin_1.next_cursor;

let managed_coins_2_11 = client
.coin_read_api()
.get_all_coins(account, cursor, Some(10))
.get_all_coins(account, cursor.clone(), Some(10))
.await
.unwrap();
assert_eq!(
Expand All @@ -554,14 +550,14 @@ impl TestCaseImpl for CoinIndexTest {

let managed_coins_12_40 = client
.coin_read_api()
.get_all_coins(account, cursor, None)
.get_all_coins(account, cursor.clone(), None)
.await
.unwrap();
assert_eq!(
managed_coins_12_40,
client
.coin_read_api()
.get_coins(account, Some(coin_type_str.clone()), cursor, None)
.get_coins(account, Some(coin_type_str.clone()), cursor.clone(), None)
.await
.unwrap(),
);
Expand All @@ -574,14 +570,19 @@ impl TestCaseImpl for CoinIndexTest {

let managed_coins_12_40 = client
.coin_read_api()
.get_all_coins(account, cursor, Some(30))
.get_all_coins(account, cursor.clone(), Some(30))
.await
.unwrap();
assert_eq!(
managed_coins_12_40,
client
.coin_read_api()
.get_coins(account, Some(coin_type_str.clone()), cursor, Some(30))
.get_coins(
account,
Some(coin_type_str.clone()),
cursor.clone(),
Some(30)
)
.await
.unwrap(),
);
Expand All @@ -597,7 +598,7 @@ impl TestCaseImpl for CoinIndexTest {
let _ = add_to_envelope(ctx, package.0, envelope.0, removed_coin_id).await;
let managed_coins_12_39 = client
.coin_read_api()
.get_all_coins(account, cursor, Some(40))
.get_all_coins(account, cursor.clone(), Some(40))
.await
.unwrap();
assert_eq!(
Expand Down
17 changes: 11 additions & 6 deletions crates/sui-indexer/src/apis/coin_api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use async_trait::async_trait;
use jsonrpsee::core::RpcResult;
use jsonrpsee::RpcModule;
use sui_json_rpc::coin_api::{parse_to_struct_tag, parse_to_type_tag};
use sui_json_rpc::error::SuiRpcInputError;
use sui_json_rpc::SuiRpcModule;
use sui_json_rpc_api::{cap_page_limit, CoinReadApiServer};
use sui_json_rpc_types::{Balance, CoinPage, Page, SuiCoinMetadata};
Expand All @@ -30,7 +31,7 @@ impl CoinReadApiServer for CoinReadApi {
&self,
owner: SuiAddress,
coin_type: Option<String>,
cursor: Option<ObjectID>,
cursor: Option<String>,
limit: Option<usize>,
) -> RpcResult<CoinPage> {
let limit = cap_page_limit(limit);
Expand All @@ -43,7 +44,9 @@ impl CoinReadApiServer for CoinReadApi {
parse_to_type_tag(coin_type)?.to_canonical_string(/* with_prefix */ true);

let cursor = match cursor {
Some(c) => c,
Some(c) => c
.parse()
.map_err(|e| SuiRpcInputError::GenericInvalid(format!("invalid cursor: {e}")))?,
// If cursor is not specified, we need to start from the beginning of the coin type, which is the minimal possible ObjectID.
None => ObjectID::ZERO,
};
Expand All @@ -54,7 +57,7 @@ impl CoinReadApiServer for CoinReadApi {

let has_next_page = results.len() > limit;
results.truncate(limit);
let next_cursor = results.last().map(|o| o.coin_object_id);
let next_cursor = results.last().map(|o| o.coin_object_id.to_string());
Ok(Page {
data: results,
next_cursor,
Expand All @@ -65,7 +68,7 @@ impl CoinReadApiServer for CoinReadApi {
async fn get_all_coins(
&self,
owner: SuiAddress,
cursor: Option<ObjectID>,
cursor: Option<String>,
limit: Option<usize>,
) -> RpcResult<CoinPage> {
let limit = cap_page_limit(limit);
Expand All @@ -74,7 +77,9 @@ impl CoinReadApiServer for CoinReadApi {
}

let cursor = match cursor {
Some(c) => c,
Some(c) => c
.parse()
.map_err(|e| SuiRpcInputError::GenericInvalid(format!("invalid cursor: {e}")))?,
// If cursor is not specified, we need to start from the beginning of the coin type, which is the minimal possible ObjectID.
None => ObjectID::ZERO,
};
Expand All @@ -85,7 +90,7 @@ impl CoinReadApiServer for CoinReadApi {

let has_next_page = results.len() > limit;
results.truncate(limit);
let next_cursor = results.last().map(|o| o.coin_object_id);
let next_cursor = results.last().map(|o| o.coin_object_id.to_string());
Ok(Page {
data: results,
next_cursor,
Expand Down
6 changes: 3 additions & 3 deletions crates/sui-json-rpc-api/src/coin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use jsonrpsee::proc_macros::rpc;
use sui_json_rpc_types::{Balance, CoinPage, SuiCoinMetadata};
use sui_open_rpc_macros::open_rpc;
use sui_types::balance::Supply;
use sui_types::base_types::{ObjectID, SuiAddress};
use sui_types::base_types::SuiAddress;

#[open_rpc(namespace = "suix", tag = "Coin Query API")]
#[rpc(server, client, namespace = "suix")]
Expand All @@ -20,7 +20,7 @@ pub trait CoinReadApi {
/// optional type name for the coin (e.g., 0x168da5bf1f48dafc111b0a488fa454aca95e0b5e::usdc::USDC), default to 0x2::sui::SUI if not specified.
coin_type: Option<String>,
/// optional paging cursor
cursor: Option<ObjectID>,
cursor: Option<String>,
/// maximum number of items per page
limit: Option<usize>,
) -> RpcResult<CoinPage>;
Expand All @@ -32,7 +32,7 @@ pub trait CoinReadApi {
/// the owner's Sui address
owner: SuiAddress,
/// optional paging cursor
cursor: Option<ObjectID>,
cursor: Option<String>,
/// maximum number of items per page
limit: Option<usize>,
) -> RpcResult<CoinPage>;
Expand Down
11 changes: 0 additions & 11 deletions crates/sui-json-rpc-tests/tests/rpc_server_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -389,17 +389,6 @@ async fn test_get_coins() -> Result<(), anyhow::Error> {
assert_eq!(2, result.data.len(), "{:?}", result);
assert!(!result.has_next_page);

let result: CoinPage = http_client
.get_coins(
address,
Some("0x2::sui::SUI".into()),
result.next_cursor,
None,
)
.await?;
assert_eq!(0, result.data.len(), "{:?}", result);
assert!(!result.has_next_page);

Ok(())
}

Expand Down
2 changes: 1 addition & 1 deletion crates/sui-json-rpc-types/src/sui_coin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use sui_types::object::Object;
use sui_types::sui_serde::BigInt;
use sui_types::sui_serde::SequenceNumber as AsSequenceNumber;

pub type CoinPage = Page<Coin, ObjectID>;
pub type CoinPage = Page<Coin, String>;

#[serde_as]
#[derive(Serialize, Deserialize, Debug, JsonSchema, PartialEq, Eq, Clone)]
Expand Down
1 change: 1 addition & 0 deletions crates/sui-json-rpc/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ bcs.workspace = true
eyre.workspace = true
once_cell.workspace = true
serde_json.workspace = true
base64.workspace = true

tap.workspace = true

Expand Down
Loading

0 comments on commit b86a009

Please sign in to comment.