From 4d36f59f158e9df28caa9604e357b5aea859c83b Mon Sep 17 00:00:00 2001 From: Ashok Menon Date: Tue, 20 Aug 2024 14:25:02 +0100 Subject: [PATCH 1/2] fix(jsonrpc): Flaky fullnode JSON-RPC test ## Description `transaction_tests::test_get_fullnode_transaction` included a flaky assertion that if you fetch 5 transactions, then the rest of the transactions, and then fetched the latest 10 transactions, the latter would match up with the suffix of the former two queries concatenated together. This does not work in general (at least not with this test set-up), because of consensus commit transactions. I have updated the test to check a similar property: That if you fetch 5 transactions, then the rest, and then fetch the first 10 transactions, then the prefixes match (this tests that transaction order is stable). There are other existing tests for what happens when you query transactions in descending order. ## Test plan This seed previously exhibited the failure, and now it succeeds. ``` sui-json-rpc-tests$ MSIM_TEST_SEED=968774516445189525 cargo simtest -- test_get_fullnode_transaction ``` --- .../sui-json-rpc-tests/tests/transaction_tests.rs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/crates/sui-json-rpc-tests/tests/transaction_tests.rs b/crates/sui-json-rpc-tests/tests/transaction_tests.rs index f1036cbf00d7d..0ab96b4ac0210 100644 --- a/crates/sui-json-rpc-tests/tests/transaction_tests.rs +++ b/crates/sui-json-rpc-tests/tests/transaction_tests.rs @@ -259,24 +259,23 @@ async fn test_get_fullnode_transaction() -> Result<(), anyhow::Error> { assert!(second_page.data.len() > 5); assert!(!second_page.has_next_page); - let mut all_txs_rev = first_page.data.clone(); - all_txs_rev.extend(second_page.data); - all_txs_rev.reverse(); + let mut all_txs = first_page.data.clone(); + all_txs.extend(second_page.data); - // test get 10 latest transactions paged + // test get 10 transactions paged let latest = client .read_api() .query_transaction_blocks( SuiTransactionBlockResponseQuery::default(), None, Some(10), - true, + false, ) .await .unwrap(); assert_eq!(10, latest.data.len()); - assert_eq!(Some(all_txs_rev[9].digest), latest.next_cursor); - assert_eq!(all_txs_rev[0..10], latest.data); + assert_eq!(Some(all_txs[9].digest), latest.next_cursor); + assert_eq!(all_txs[0..10], latest.data); assert!(latest.has_next_page); // test get from address txs in ascending order From a671b1187f26496ca98a8a06bd2489e822bd0a78 Mon Sep 17 00:00:00 2001 From: Ashok Menon Date: Tue, 20 Aug 2024 13:21:19 +0100 Subject: [PATCH 2/2] fix(graphql): Docs fixes from #17543 ## Description Gathering all the suggested docs fixes across the stack in #17543 into one PR. ## Test plan :eyes: --- crates/sui-graphql-rpc/schema.graphql | 10 +++++----- crates/sui-graphql-rpc/src/commands.rs | 2 +- .../src/data/package_resolver.rs | 2 +- .../sui-graphql-rpc/src/types/checkpoint.rs | 4 ++-- crates/sui-graphql-rpc/src/types/epoch.rs | 2 +- crates/sui-graphql-rpc/src/types/event.rs | 11 +++++------ .../sui-graphql-rpc/src/types/move_package.rs | 4 ++-- crates/sui-graphql-rpc/src/types/object.rs | 10 +++++----- crates/sui-graphql-rpc/src/types/query.rs | 10 +++++----- .../src/types/transaction_block.rs | 2 +- .../snapshot_tests__schema_sdl_export.snap | 10 +++++----- crates/sui-package-dump/src/lib.rs | 19 ++++++++++--------- crates/sui-tool/src/commands.rs | 2 +- 13 files changed, 44 insertions(+), 44 deletions(-) diff --git a/crates/sui-graphql-rpc/schema.graphql b/crates/sui-graphql-rpc/schema.graphql index 9a134382a7a70..db2f967fa4085 100644 --- a/crates/sui-graphql-rpc/schema.graphql +++ b/crates/sui-graphql-rpc/schema.graphql @@ -3088,12 +3088,12 @@ type Query { """ object(address: SuiAddress!, version: UInt53): Object """ - The package corresponding to the given address at the (optionally) given version. + The package corresponding to the given address (at the optionally given version). When no version is given, the package is loaded directly from the address given. Otherwise, the address is translated before loading to point to the package whose original ID matches - the package at `address`, but whose version is `version`. For non-system packages, this may - result in a different address than `address` because different versions of a package, + the package at `address`, but whose version is `version`. For non-system packages, this + might result in a different address than `address` because different versions of a package, introduced by upgrades, exist at distinct addresses. Note that this interpretation of `version` is different from a historical object read (the @@ -3156,8 +3156,8 @@ type Query { The Move packages that exist in the network, optionally filtered to be strictly before `beforeCheckpoint` and/or strictly after `afterCheckpoint`. - This query will return all versions of a given user package that appear between the - specified checkpoints, but only records the latest versions of system packages. + This query returns all versions of a given user package that appear between the specified + checkpoints, but only records the latest versions of system packages. """ packages(first: Int, after: String, last: Int, before: String, filter: MovePackageCheckpointFilter): MovePackageConnection! """ diff --git a/crates/sui-graphql-rpc/src/commands.rs b/crates/sui-graphql-rpc/src/commands.rs index e5166def39f50..4b0eca46c11cb 100644 --- a/crates/sui-graphql-rpc/src/commands.rs +++ b/crates/sui-graphql-rpc/src/commands.rs @@ -16,7 +16,7 @@ pub enum Command { /// Output a TOML config (suitable for passing into the --config parameter of the start-server /// command) with all values set to their defaults. GenerateConfig { - /// Optional path to a file to output to. Prints to stdout if none is provided. + /// Optional path to an output file. Prints to `stdout` if not provided. output: Option, }, diff --git a/crates/sui-graphql-rpc/src/data/package_resolver.rs b/crates/sui-graphql-rpc/src/data/package_resolver.rs index 467911753d266..f10067fd007b9 100644 --- a/crates/sui-graphql-rpc/src/data/package_resolver.rs +++ b/crates/sui-graphql-rpc/src/data/package_resolver.rs @@ -26,7 +26,7 @@ pub(crate) type PackageResolver = Arc>; /// to `fetch` pub struct DbPackageStore(DataLoader); -/// DataLoader key for fetching the latest version of a `Package` by its ID. +/// `DataLoader` key for fetching the latest version of a `Package` by its ID. #[derive(Copy, Clone, Hash, Eq, PartialEq, Debug)] struct PackageKey(AccountAddress); diff --git a/crates/sui-graphql-rpc/src/types/checkpoint.rs b/crates/sui-graphql-rpc/src/types/checkpoint.rs index cbebb9935f491..f59ad8c46ae17 100644 --- a/crates/sui-graphql-rpc/src/types/checkpoint.rs +++ b/crates/sui-graphql-rpc/src/types/checkpoint.rs @@ -36,7 +36,7 @@ pub(crate) struct CheckpointId { pub sequence_number: Option, } -/// DataLoader key for fetching a `Checkpoint` by its sequence number, constrained by a consistency +/// `DataLoader` key for fetching a `Checkpoint` by its sequence number, constrained by a consistency /// cursor. #[derive(Copy, Clone, Hash, Eq, PartialEq, Debug)] struct SeqNumKey { @@ -47,7 +47,7 @@ struct SeqNumKey { pub checkpoint_viewed_at: u64, } -/// DataLoader key for fetching a `Checkpoint` by its digest, constrained by a consistency cursor. +/// `DataLoader` key for fetching a `Checkpoint` by its digest, constrained by a consistency cursor. #[derive(Copy, Clone, Hash, Eq, PartialEq, Debug)] struct DigestKey { pub digest: Digest, diff --git a/crates/sui-graphql-rpc/src/types/epoch.rs b/crates/sui-graphql-rpc/src/types/epoch.rs index 6ca312ca3ba37..7ecfa9c7d773e 100644 --- a/crates/sui-graphql-rpc/src/types/epoch.rs +++ b/crates/sui-graphql-rpc/src/types/epoch.rs @@ -32,7 +32,7 @@ pub(crate) struct Epoch { pub checkpoint_viewed_at: u64, } -/// DataLoader key for fetching an `Epoch` by its ID, optionally constrained by a consistency +/// `DataLoader` key for fetching an `Epoch` by its ID, optionally constrained by a consistency /// cursor. #[derive(Copy, Clone, Hash, Eq, PartialEq, Debug)] struct EpochKey { diff --git a/crates/sui-graphql-rpc/src/types/event.rs b/crates/sui-graphql-rpc/src/types/event.rs index 6b7ba8ee8b3c2..e195b10844c26 100644 --- a/crates/sui-graphql-rpc/src/types/event.rs +++ b/crates/sui-graphql-rpc/src/types/event.rs @@ -143,13 +143,12 @@ impl Event { /// checkpoint sequence numbers as the cursor to determine the correct page of results. The /// query can optionally be further `filter`-ed by the `EventFilter`. /// - /// The `checkpoint_viewed_at` parameter represents the checkpoint sequence number at which this - /// page was queried. Each entity returned in the connection will inherit this checkpoint, so - /// that when viewing that entity's state, it will be as if it is being viewed at this - /// checkpoint. + /// The `checkpoint_viewed_at` parameter represents the checkpoint sequence number at which + /// this page was queried. Each entity returned in the connection inherits this checkpoint, so + /// that when viewing that entity's state, it's as if it's being viewed at this checkpoint. /// - /// The cursors in `page` may also include checkpoint viewed at fields. If these are set, they - /// take precedence over the checkpoint that pagination is being conducted in. + /// The cursors in `page` might also include checkpoint viewed at fields. If these are set, + /// they take precedence over the checkpoint that pagination is being conducted in. pub(crate) async fn paginate( db: &Db, page: Page, diff --git a/crates/sui-graphql-rpc/src/types/move_package.rs b/crates/sui-graphql-rpc/src/types/move_package.rs index aba7259ff1a22..bb92ba1e599a3 100644 --- a/crates/sui-graphql-rpc/src/types/move_package.rs +++ b/crates/sui-graphql-rpc/src/types/move_package.rs @@ -142,7 +142,7 @@ pub(crate) struct PackageCursor { pub checkpoint_viewed_at: u64, } -/// DataLoader key for fetching the storage ID of the (user) package that shares an original (aka +/// `DataLoader` key for fetching the storage ID of the (user) package that shares an original (aka /// runtime) ID with the package stored at `package_id`, and whose version is `version`. /// /// Note that this is different from looking up the historical version of an object -- the query @@ -154,7 +154,7 @@ struct PackageVersionKey { version: u64, } -/// DataLoader key for fetching the latest version of a user package: The package with the largest +/// `DataLoader` key for fetching the latest version of a user package: The package with the largest /// version whose original ID matches the original ID of the package at `address`. #[derive(Copy, Clone, Hash, Eq, PartialEq, Debug)] struct LatestKey { diff --git a/crates/sui-graphql-rpc/src/types/object.rs b/crates/sui-graphql-rpc/src/types/object.rs index cd3e18709760c..66887aa7be54c 100644 --- a/crates/sui-graphql-rpc/src/types/object.rs +++ b/crates/sui-graphql-rpc/src/types/object.rs @@ -185,7 +185,7 @@ pub(crate) struct AddressOwner { /// Filter for a point query of an Object. pub(crate) enum ObjectLookup { LatestAt { - /// The checkpoint sequence number at which this was viewed at + /// The checkpoint sequence number at which this was viewed at. checkpoint_viewed_at: u64, }, @@ -193,7 +193,7 @@ pub(crate) enum ObjectLookup { /// The parent version to be used as an upper bound for the query. Look for the latest /// version of a child object whose version is less than or equal to this upper bound. parent_version: u64, - /// The checkpoint sequence number at which this was viewed at + /// The checkpoint sequence number at which this was viewed at. checkpoint_viewed_at: u64, }, @@ -280,7 +280,7 @@ pub(crate) enum IObject { SuinsRegistration(SuinsRegistration), } -/// DataLoader key for fetching an `Object` at a specific version, constrained by a consistency +/// `DataLoader` key for fetching an `Object` at a specific version, constrained by a consistency /// cursor (if that version was created after the checkpoint the query is viewing at, then it will /// fail). #[derive(Copy, Clone, Hash, Eq, PartialEq, Debug)] @@ -290,7 +290,7 @@ struct HistoricalKey { checkpoint_viewed_at: u64, } -/// DataLoader key for fetching the latest version of an object whose parent object has version +/// `DataLoader` key for fetching the latest version of an object whose parent object has version /// `parent_version`, as of `checkpoint_viewed_at`. This look-up can fail to find a valid object if /// the key is not self-consistent, for example if the `parent_version` is set to a higher version /// than the object's actual parent as of `checkpoint_viewed_at`. @@ -301,7 +301,7 @@ struct ParentVersionKey { checkpoint_viewed_at: u64, } -/// DataLoader key for fetching the latest version of an `Object` as of a consistency cursor. +/// `DataLoader` key for fetching the latest version of an object as of a given checkpoint. #[derive(Copy, Clone, Hash, Eq, PartialEq, Debug)] struct LatestAtKey { id: SuiAddress, diff --git a/crates/sui-graphql-rpc/src/types/query.rs b/crates/sui-graphql-rpc/src/types/query.rs index 1ff209d351a55..07b51ef649d88 100644 --- a/crates/sui-graphql-rpc/src/types/query.rs +++ b/crates/sui-graphql-rpc/src/types/query.rs @@ -225,12 +225,12 @@ impl Query { Object::query(ctx, address, key).await.extend() } - /// The package corresponding to the given address at the (optionally) given version. + /// The package corresponding to the given address (at the optionally given version). /// /// When no version is given, the package is loaded directly from the address given. Otherwise, /// the address is translated before loading to point to the package whose original ID matches - /// the package at `address`, but whose version is `version`. For non-system packages, this may - /// result in a different address than `address` because different versions of a package, + /// the package at `address`, but whose version is `version`. For non-system packages, this + /// might result in a different address than `address` because different versions of a package, /// introduced by upgrades, exist at distinct addresses. /// /// Note that this interpretation of `version` is different from a historical object read (the @@ -440,8 +440,8 @@ impl Query { /// The Move packages that exist in the network, optionally filtered to be strictly before /// `beforeCheckpoint` and/or strictly after `afterCheckpoint`. /// - /// This query will return all versions of a given user package that appear between the - /// specified checkpoints, but only records the latest versions of system packages. + /// This query returns all versions of a given user package that appear between the specified + /// checkpoints, but only records the latest versions of system packages. async fn packages( &self, ctx: &Context<'_>, diff --git a/crates/sui-graphql-rpc/src/types/transaction_block.rs b/crates/sui-graphql-rpc/src/types/transaction_block.rs index c75a34ee2fcda..c33da8133001a 100644 --- a/crates/sui-graphql-rpc/src/types/transaction_block.rs +++ b/crates/sui-graphql-rpc/src/types/transaction_block.rs @@ -131,7 +131,7 @@ pub(crate) struct TransactionBlockCursor { pub tx_checkpoint_number: u64, } -/// DataLoader key for fetching a `TransactionBlock` by its digest, optionally constrained by a +/// `DataLoader` key for fetching a `TransactionBlock` by its digest, optionally constrained by a /// consistency cursor. #[derive(Copy, Clone, Hash, Eq, PartialEq, Debug)] struct DigestKey { diff --git a/crates/sui-graphql-rpc/tests/snapshots/snapshot_tests__schema_sdl_export.snap b/crates/sui-graphql-rpc/tests/snapshots/snapshot_tests__schema_sdl_export.snap index 87f5a27061f06..93eadb6691d92 100644 --- a/crates/sui-graphql-rpc/tests/snapshots/snapshot_tests__schema_sdl_export.snap +++ b/crates/sui-graphql-rpc/tests/snapshots/snapshot_tests__schema_sdl_export.snap @@ -3092,12 +3092,12 @@ type Query { """ object(address: SuiAddress!, version: UInt53): Object """ - The package corresponding to the given address at the (optionally) given version. + The package corresponding to the given address (at the optionally given version). When no version is given, the package is loaded directly from the address given. Otherwise, the address is translated before loading to point to the package whose original ID matches - the package at `address`, but whose version is `version`. For non-system packages, this may - result in a different address than `address` because different versions of a package, + the package at `address`, but whose version is `version`. For non-system packages, this + might result in a different address than `address` because different versions of a package, introduced by upgrades, exist at distinct addresses. Note that this interpretation of `version` is different from a historical object read (the @@ -3160,8 +3160,8 @@ type Query { The Move packages that exist in the network, optionally filtered to be strictly before `beforeCheckpoint` and/or strictly after `afterCheckpoint`. - This query will return all versions of a given user package that appear between the - specified checkpoints, but only records the latest versions of system packages. + This query returns all versions of a given user package that appear between the specified + checkpoints, but only records the latest versions of system packages. """ packages(first: Int, after: String, last: Int, before: String, filter: MovePackageCheckpointFilter): MovePackageConnection! """ diff --git a/crates/sui-package-dump/src/lib.rs b/crates/sui-package-dump/src/lib.rs index 8438a8bec7f88..ea70db5fd927a 100644 --- a/crates/sui-package-dump/src/lib.rs +++ b/crates/sui-package-dump/src/lib.rs @@ -20,10 +20,11 @@ mod query; /// Ensure all packages created before `before_checkpoint` are written to the `output_dir`ectory, /// from the GraphQL service at `rpc_url`. /// -/// `output_dir` can be a path to a non-existent directory, in which case it will be created, or an -/// existing empty directory (in which case it will be filled), or an existing directory that has -/// been written to in the past (in which case this invocation will pick back up from where the -/// previous invocation left off). +/// `output_dir` can be a path to a non-existent directory, an existing empty directory, or an +/// existing directory written to in the past. If the path is non-existent, the invocation creates +/// it. If the path exists but is empty, the invocation writes to the directory. If the directory +/// has been written to in the past, the invocation picks back up where the previous invocation +/// left off. pub async fn dump( rpc_url: String, output_dir: PathBuf, @@ -116,8 +117,8 @@ async fn max_page_size(client: &Client) -> Result { /// Read all the packages between `after_checkpoint` and `before_checkpoint`, in batches of /// `page_size` from the `client` connected to a GraphQL service. /// -/// If `after_checkpoint` is not provided, packages will be read from genesis. If -/// `before_checkpoint` is not provided, packages will be read until the latest checkpoint. +/// If `after_checkpoint` is not provided, packages are read from genesis. If `before_checkpoint` +/// is not provided, packages are read until the latest checkpoint. /// /// Returns the latest checkpoint that was read from in this fetch, and a list of all the packages /// that were read. @@ -198,10 +199,10 @@ async fn fetch_packages( /// /// - `linkage.json` -- a JSON serialization of the package's linkage table, mapping dependency /// original IDs to the version of the dependency being depended on and the ID of the object -/// on-chain that contains that version. +/// on chain that contains that version. /// -/// - `origins.json` -- a JSON serialize of the type origin table, mapping type names contained in -/// this package to the version of the package that first introduced that type. +/// - `origins.json` -- a JSON serialization of the type origin table, mapping type names contained +/// in this package to the version of the package that first introduced that type. /// /// - `*.mv` -- a BCS serialization of each compiled module in the package. fn dump_package(output_dir: &Path, pkg: &packages::MovePackage) -> Result<()> { diff --git a/crates/sui-tool/src/commands.rs b/crates/sui-tool/src/commands.rs index 3871143671e3e..5c352104c8aaa 100644 --- a/crates/sui-tool/src/commands.rs +++ b/crates/sui-tool/src/commands.rs @@ -178,7 +178,7 @@ pub enum ToolCommand { }, /// Download all packages to the local filesystem from a GraphQL service. Each package gets its - /// own sub-directory, named for its ID on-chain and version containing two metadata files + /// own sub-directory, named for its ID on chain and version containing two metadata files /// (linkage.json and origins.json), a file containing the overall object and a file for every /// module it contains. Each module file is named for its module name, with a .mv suffix, and /// contains Move bytecode (suitable for passing into a disassembler).