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][RFC] Introduce UInt53 scalar #18552

Merged
merged 4 commits into from
Jul 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
104 changes: 55 additions & 49 deletions crates/sui-graphql-rpc/schema/current_progress_schema.graphql

Large diffs are not rendered by default.

27 changes: 13 additions & 14 deletions crates/sui-graphql-rpc/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
// SPDX-License-Identifier: Apache-2.0

use crate::functional_group::FunctionalGroup;
use crate::types::big_int::BigInt;
use async_graphql::*;
use fastcrypto_zkp::bn254::zk_login_api::ZkLoginEnv;
use serde::{Deserialize, Serialize};
Expand Down Expand Up @@ -67,23 +66,23 @@ pub struct Limits {
/// Maximum number of nodes in the requests.
pub max_query_nodes: u32,
/// Maximum number of output nodes allowed in the response.
pub max_output_nodes: u64,
pub max_output_nodes: u32,
/// Maximum size (in bytes) of a GraphQL request.
pub max_query_payload_size: u32,
/// Queries whose EXPLAIN cost are more than this will be logged. Given in the units used by the
/// database (where 1.0 is roughly the cost of a sequential page access).
pub max_db_query_cost: u64,
pub max_db_query_cost: u32,
/// Paginated queries will return this many elements if a page size is not provided.
pub default_page_size: u64,
pub default_page_size: u32,
/// Paginated queries can return at most this many elements.
pub max_page_size: u64,
pub max_page_size: u32,
/// Time (in milliseconds) to wait for a transaction to be executed and the results returned
/// from GraphQL. If the transaction takes longer than this time to execute, the request will
/// return a timeout error, but the transaction may continue executing.
pub mutation_timeout_ms: u64,
pub mutation_timeout_ms: u32,
/// Time (in milliseconds) to wait for a read request from the GraphQL service. Requests that
/// take longer than this time to return a result will return a timeout error.
pub request_timeout_ms: u64,
pub request_timeout_ms: u32,
/// Maximum amount of nesting among type arguments (type arguments nest when a type argument is
/// itself generic and has arguments).
pub max_type_argument_depth: u32,
Expand Down Expand Up @@ -223,36 +222,36 @@ impl ServiceConfig {
/// with a connection of first: 10 and has a field to a connection with last: 20, the count
/// at the second level would be 200 nodes. This is then summed to the count of 10 nodes
/// at the first level, for a total of 210 nodes.
pub async fn max_output_nodes(&self) -> u64 {
pub async fn max_output_nodes(&self) -> u32 {
self.limits.max_output_nodes
}

/// Maximum estimated cost of a database query used to serve a GraphQL request. This is
/// measured in the same units that the database uses in EXPLAIN queries.
async fn max_db_query_cost(&self) -> BigInt {
BigInt::from(self.limits.max_db_query_cost)
async fn max_db_query_cost(&self) -> u32 {
self.limits.max_db_query_cost
}

/// Default number of elements allowed on a single page of a connection.
async fn default_page_size(&self) -> u64 {
async fn default_page_size(&self) -> u32 {
self.limits.default_page_size
}

/// Maximum number of elements allowed on a single page of a connection.
async fn max_page_size(&self) -> u64 {
async fn max_page_size(&self) -> u32 {
self.limits.max_page_size
}

/// Maximum time in milliseconds spent waiting for a response from fullnode after issuing a
/// a transaction to execute. Note that the transaction may still succeed even in the case of a
/// timeout. Transactions are idempotent, so a transaction that times out should be resubmitted
/// until the network returns a definite response (success or failure, not timeout).
async fn mutation_timeout_ms(&self) -> u64 {
async fn mutation_timeout_ms(&self) -> u32 {
self.limits.mutation_timeout_ms
}

/// Maximum time in milliseconds that will be spent to serve one query request.
async fn request_timeout_ms(&self) -> u64 {
async fn request_timeout_ms(&self) -> u32 {
self.limits.request_timeout_ms
}

Expand Down
4 changes: 2 additions & 2 deletions crates/sui-graphql-rpc/src/data/pg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ pub(crate) struct PgExecutor {
}

pub(crate) struct PgConnection<'c> {
max_cost: u64,
max_cost: u32,
conn: &'c mut diesel::PgConnection,
}

Expand Down Expand Up @@ -147,7 +147,7 @@ mod query_cost {
}

/// Run `EXPLAIN` on the `query`, and log the estimated cost.
pub(crate) fn log<Q>(conn: &mut PgConnection, max_db_query_cost: u64, query: Q)
pub(crate) fn log<Q>(conn: &mut PgConnection, max_db_query_cost: u32, query: Q)
where
Q: Query + QueryId + QueryFragment<Pg> + RunQueryDsl<PgConnection>,
{
Expand Down
2 changes: 1 addition & 1 deletion crates/sui-graphql-rpc/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ pub enum Error {
#[error("'first' and 'last' must not be used together")]
CursorNoFirstLast,
#[error("Connection's page size of {0} exceeds max of {1}")]
PageTooLarge(u64, u64),
PageTooLarge(u64, u32),
// Catch-all for client-fault errors
#[error("{0}")]
Client(String),
Expand Down
18 changes: 9 additions & 9 deletions crates/sui-graphql-rpc/src/extensions/query_limits_checker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ pub(crate) struct ShowUsage;
#[derive(Clone, Debug, Default)]
struct ValidationRes {
input_nodes: u32,
output_nodes: u64,
output_nodes: u32,
depth: u32,
num_variables: u32,
num_fragments: u32,
Expand Down Expand Up @@ -73,7 +73,7 @@ impl ExtensionFactory for QueryLimitsChecker {
#[derive(Debug)]
struct ComponentCost {
pub input_nodes: u32,
pub output_nodes: u64,
pub output_nodes: u32,
pub depth: u32,
}

Expand Down Expand Up @@ -234,7 +234,7 @@ impl QueryLimitsChecker {
// Use BFS to analyze the query and count the number of nodes and the depth of the query
struct ToVisit<'s> {
selection: &'s Positioned<Selection>,
parent_node_count: u64,
parent_node_count: u32,
}

// Queue to store the nodes at each level
Expand Down Expand Up @@ -431,8 +431,8 @@ fn check_directives(directives: &[Positioned<Directive>]) -> ServerResult<()> {
fn estimate_output_nodes_for_curr_node(
f: &Positioned<Field>,
variables: &Variables,
default_page_size: u64,
) -> u64 {
default_page_size: u32,
) -> u32 {
if !is_connection(f) {
1
} else {
Expand All @@ -446,19 +446,19 @@ fn estimate_output_nodes_for_curr_node(
}
}

/// Try to extract a u64 value from the given argument, or return None on failure.
fn extract_limit(value: Option<&Positioned<GqlValue>>, variables: &Variables) -> Option<u64> {
/// Try to extract a u32 value from the given argument, or return None on failure.
fn extract_limit(value: Option<&Positioned<GqlValue>>, variables: &Variables) -> Option<u32> {
if let GqlValue::Variable(var) = &value?.node {
return match variables.get(var) {
Some(Value::Number(num)) => num.as_u64(),
Some(Value::Number(num)) => num.as_u64().map(|v| v as u32),
_ => None,
};
}

let GqlValue::Number(value) = &value?.node else {
return None;
};
value.as_u64()
value.as_u64().map(|v| v as u32)
Copy link
Contributor

Choose a reason for hiding this comment

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

Took me a second to wrap my head around this but I think it's fine because u64 maps to Int which on graphQL is capped at u32, so it shouldn't be possible for a graphQL client to provide > u32

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The query limits checker needs a deeper overhaul in any case so your confusion is warranted -- this should be clearer!

}

/// Checks if the given field is a connection field by whether it has 'edges' or 'nodes' selected.
Expand Down
4 changes: 2 additions & 2 deletions crates/sui-graphql-rpc/src/extensions/timeout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,9 @@ impl Extension for TimeoutExt {
// increase the timeout if the request is a mutation
let is_mutation = self.is_mutation.load(Ordering::Relaxed);
let request_timeout = if is_mutation {
Duration::from_millis(cfg.limits.mutation_timeout_ms)
Duration::from_millis(cfg.limits.mutation_timeout_ms.into())
} else {
Duration::from_millis(cfg.limits.request_timeout_ms)
Duration::from_millis(cfg.limits.request_timeout_ms.into())
};

timeout(request_timeout, next.run(ctx, operation_name))
Expand Down
8 changes: 4 additions & 4 deletions crates/sui-graphql-rpc/src/server/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -420,7 +420,7 @@ impl ServerBuilder {
// Bound each statement in a request with the overall request timeout, to bound DB
// utilisation (in the worst case we will use 2x the request timeout time in DB wall
// time).
config.service.limits.request_timeout_ms,
config.service.limits.request_timeout_ms.into(),
)
.map_err(|e| Error::Internal(format!("Failed to create pg connection pool: {}", e)))?;

Expand Down Expand Up @@ -688,7 +688,7 @@ pub mod tests {
let reader = PgManager::reader_with_config(
connection_config.db_url.clone(),
connection_config.db_pool_size,
service_config.limits.request_timeout_ms,
service_config.limits.request_timeout_ms.into(),
)
.expect("Failed to create pg connection pool");

Expand Down Expand Up @@ -771,8 +771,8 @@ pub mod tests {
sui_client: &SuiClient,
) -> Response {
let mut cfg = ServiceConfig::default();
cfg.limits.request_timeout_ms = timeout.as_millis() as u64;
cfg.limits.mutation_timeout_ms = timeout.as_millis() as u64;
cfg.limits.request_timeout_ms = timeout.as_millis() as u32;
cfg.limits.mutation_timeout_ms = timeout.as_millis() as u32;

let schema = prep_schema(None, Some(cfg))
.context_data(Some(sui_client.clone()))
Expand Down
5 changes: 3 additions & 2 deletions crates/sui-graphql-rpc/src/types/balance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

use super::available_range::AvailableRange;
use super::cursor::{self, Page, RawPaginated, Target};
use super::uint53::UInt53;
use super::{big_int::BigInt, move_type::MoveType, sui_address::SuiAddress};
use crate::consistency::Checkpointed;
use crate::data::{Db, DbConnection, QueryExecutor};
Expand All @@ -26,7 +27,7 @@ pub(crate) struct Balance {
/// Coin type for the balance, such as 0x2::sui::SUI
pub(crate) coin_type: MoveType,
/// How many coins of this type constitute the balance
pub(crate) coin_object_count: Option<u64>,
pub(crate) coin_object_count: Option<UInt53>,
/// Total balance across all coin objects of the coin type
pub(crate) total_balance: Option<BigInt>,
}
Expand Down Expand Up @@ -174,7 +175,7 @@ impl TryFrom<StoredBalance> for Balance {
.transpose()
.map_err(|_| Error::Internal("Failed to read balance.".to_string()))?;

let coin_object_count = count.map(|c| c as u64);
let coin_object_count = count.map(|c| UInt53::from(c as u64));

let coin_type = MoveType::new(
parse_sui_type_tag(&coin_type)
Expand Down
17 changes: 9 additions & 8 deletions crates/sui-graphql-rpc/src/types/checkpoint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ use super::{
epoch::Epoch,
gas::GasCostSummary,
transaction_block::{self, TransactionBlock, TransactionBlockFilter},
uint53::UInt53,
};
use crate::consistency::Checkpointed;
use crate::{
Expand All @@ -32,7 +33,7 @@ use sui_types::messages_checkpoint::CheckpointDigest;
#[derive(Default, InputObject)]
pub(crate) struct CheckpointId {
pub digest: Option<Digest>,
pub sequence_number: Option<u64>,
pub sequence_number: Option<UInt53>,
}

/// DataLoader key for fetching a `Checkpoint` by its sequence number, constrained by a consistency
Expand Down Expand Up @@ -90,8 +91,8 @@ impl Checkpoint {

/// This checkpoint's position in the total order of finalized checkpoints, agreed upon by
/// consensus.
async fn sequence_number(&self) -> u64 {
self.sequence_number_impl()
async fn sequence_number(&self) -> UInt53 {
self.sequence_number_impl().into()
}

/// The timestamp at which the checkpoint is agreed to have happened according to consensus.
Expand All @@ -115,8 +116,8 @@ impl Checkpoint {
}

/// The total number of transaction blocks in the network by the end of this checkpoint.
async fn network_total_transactions(&self) -> Option<u64> {
Some(self.network_total_transactions_impl())
async fn network_total_transactions(&self) -> Option<UInt53> {
Some(self.network_total_transactions_impl().into())
}

/// The computation cost, storage cost, storage rebate, and non-refundable storage fee
Expand Down Expand Up @@ -157,7 +158,7 @@ impl Checkpoint {
let Some(filter) = filter
.unwrap_or_default()
.intersect(TransactionBlockFilter {
at_checkpoint: Some(self.stored.sequence_number as u64),
at_checkpoint: Some(UInt53::from(self.stored.sequence_number as u64)),
..Default::default()
})
else {
Expand All @@ -178,7 +179,7 @@ impl Checkpoint {
impl CheckpointId {
pub(crate) fn by_seq_num(seq_num: u64) -> Self {
CheckpointId {
sequence_number: Some(seq_num),
sequence_number: Some(seq_num.into()),
digest: None,
}
}
Expand Down Expand Up @@ -213,7 +214,7 @@ impl Checkpoint {
} => {
let DataLoader(dl) = ctx.data_unchecked();
dl.load_one(SeqNumKey {
sequence_number,
sequence_number: sequence_number.into(),
digest,
checkpoint_viewed_at,
})
Expand Down
3 changes: 2 additions & 1 deletion crates/sui-graphql-rpc/src/types/coin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ use super::sui_address::SuiAddress;
use super::suins_registration::{DomainFormat, SuinsRegistration};
use super::transaction_block::{self, TransactionBlock, TransactionBlockFilter};
use super::type_filter::ExactTypeFilter;
use super::uint53::UInt53;
use async_graphql::*;

use async_graphql::connection::{Connection, CursorType, Edge};
Expand Down Expand Up @@ -150,7 +151,7 @@ impl Coin {
.await
}

pub(crate) async fn version(&self) -> u64 {
pub(crate) async fn version(&self) -> UInt53 {
ObjectImpl(&self.super_.super_).version().await
}

Expand Down
3 changes: 2 additions & 1 deletion crates/sui-graphql-rpc/src/types/coin_metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ use super::sui_address::SuiAddress;
use super::suins_registration::{DomainFormat, SuinsRegistration};
use super::transaction_block::{self, TransactionBlock, TransactionBlockFilter};
use super::type_filter::ExactTypeFilter;
use super::uint53::UInt53;
use crate::data::Db;
use crate::error::Error;
use async_graphql::connection::Connection;
Expand Down Expand Up @@ -139,7 +140,7 @@ impl CoinMetadata {
.await
}

pub(crate) async fn version(&self) -> u64 {
pub(crate) async fn version(&self) -> UInt53 {
ObjectImpl(&self.super_.super_).version().await
}

Expand Down
6 changes: 3 additions & 3 deletions crates/sui-graphql-rpc/src/types/cursor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ impl<C> Page<C> {
(limit, after, None, before) => Page {
after,
before,
limit: limit.unwrap_or(limits.default_page_size),
limit: limit.unwrap_or(limits.default_page_size as u64),
end: End::Front,
},

Expand All @@ -152,7 +152,7 @@ impl<C> Page<C> {
},
};

if page.limit > limits.max_page_size {
if page.limit > limits.max_page_size as u64 {
return Err(Error::PageTooLarge(page.limit, limits.max_page_size).extend());
}

Expand Down Expand Up @@ -797,7 +797,7 @@ mod tests {
#[test]
fn test_err_page_too_big() {
let config = ServiceConfig::default();
let too_big = config.limits.max_page_size + 1;
let too_big = config.limits.max_page_size as u64 + 1;
let err = Page::<JsonCursor<u64>>::from_params(&config, Some(too_big), None, None, None)
.unwrap_err();

Expand Down
Loading
Loading