-
Notifications
You must be signed in to change notification settings - Fork 87
feat: add pagination to GetNetworkAccountIds #1452
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
Changes from all commits
a8f3c0b
d356cb4
c1ca1e9
28ce096
c4b33cb
e3bf418
c1561c6
cf67a55
88b139e
d1b7060
a4a7fea
83b0bb4
cd46c29
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -410,28 +410,79 @@ pub(crate) fn select_all_accounts( | |
| Ok(account_infos) | ||
| } | ||
|
|
||
| /// Returns all network account IDs. | ||
| /// Returns network account IDs within the specified block range (based on account creation | ||
| /// block). | ||
| /// | ||
| /// The function may return fewer accounts than exist in the range if the result would exceed | ||
| /// `MAX_RESPONSE_PAYLOAD_BYTES / AccountId::SERIALIZED_SIZE` rows. In this case, the result is | ||
| /// truncated at a block boundary to ensure all accounts from included blocks are returned. | ||
| /// | ||
| /// # Returns | ||
| /// | ||
| /// A vector with network account IDs, or an error. | ||
| /// A tuple containing: | ||
| /// - A vector of network account IDs. | ||
| /// - The last block number that was fully included in the result. When truncated, this will be less | ||
| /// than the requested range end. | ||
| pub(crate) fn select_all_network_account_ids( | ||
| conn: &mut SqliteConnection, | ||
| ) -> Result<Vec<AccountId>, DatabaseError> { | ||
| let account_ids_raw: Vec<Vec<u8>> = QueryDsl::select( | ||
| schema::accounts::table.filter(schema::accounts::network_account_id_prefix.is_not_null()), | ||
| schema::accounts::account_id, | ||
| block_range: RangeInclusive<BlockNumber>, | ||
| ) -> Result<(Vec<AccountId>, BlockNumber), DatabaseError> { | ||
| const ROW_OVERHEAD_BYTES: usize = AccountId::SERIALIZED_SIZE; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. future work/probably too much: we might want to use
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should I create an issue for this one?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd like @Mirko-von-Leipzig 's input on it first :)
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is an internal only endpoint, so I think we can get away with simplifying this as much as possible such that we don't need a derived With a small bit of refactoring in the ntx builder (briefly touched upon in #1478 (comment)), we can simplify this function signature to pub(crate) fn select_all_network_account_ids(
conn: &mut SqliteConnection,
token: ContinuationToken,
) -> Result<(Vec<AccountId>, ContinuationToken), DatabaseError> This is because the ntx builder won't need to care about block-delimited pages, and instead only needs a mechanism to get all currently committed network account IDs. So all we need is an iteration key that retains the order, which could be ORDER BY rowid -- if we don't delete, or
ORDER BY created_at, account_id -- for an agnostic oneand then struct ContinuationToken(rowid);
// or
struct ContinuationToken(BlockNumber, AccountId);If we do decide to go the stream route, then |
||
| const MAX_ROWS: usize = MAX_RESPONSE_PAYLOAD_BYTES / ROW_OVERHEAD_BYTES; | ||
|
|
||
| const _: () = assert!( | ||
| MAX_ROWS > miden_protocol::MAX_ACCOUNTS_PER_BLOCK, | ||
| "Block pagination limit must exceed maximum block capacity to uphold assumed logic invariant" | ||
| ); | ||
|
|
||
| if block_range.is_empty() { | ||
| return Err(DatabaseError::InvalidBlockRange { | ||
| from: *block_range.start(), | ||
| to: *block_range.end(), | ||
| }); | ||
| } | ||
|
|
||
| let account_ids_raw: Vec<(Vec<u8>, i64)> = Box::new( | ||
| QueryDsl::select( | ||
| schema::accounts::table | ||
| .filter(schema::accounts::network_account_id_prefix.is_not_null()), | ||
| (schema::accounts::account_id, schema::accounts::created_at_block), | ||
| ) | ||
| .filter( | ||
| schema::accounts::block_num | ||
| .between(block_range.start().to_raw_sql(), block_range.end().to_raw_sql()), | ||
| ) | ||
| .order(schema::accounts::created_at_block.asc()) | ||
| .limit(i64::try_from(MAX_ROWS + 1).expect("limit fits within i64")), | ||
| ) | ||
| .load::<Vec<u8>>(conn)?; | ||
| .load::<(Vec<u8>, i64)>(conn)?; | ||
|
|
||
| let account_ids = account_ids_raw | ||
| .into_iter() | ||
| .map(|id_bytes| { | ||
| AccountId::read_from_bytes(&id_bytes).map_err(DatabaseError::DeserializationError) | ||
| }) | ||
| .collect::<Result<Vec<AccountId>, DatabaseError>>()?; | ||
| if account_ids_raw.len() > MAX_ROWS { | ||
| // SAFETY: We just checked that len > MAX_ROWS, so the vec is not empty. | ||
| let last_created_at_block = account_ids_raw.last().expect("vec is not empty").1; | ||
|
|
||
| let account_ids = account_ids_raw | ||
| .into_iter() | ||
| .take_while(|(_, created_at_block)| *created_at_block != last_created_at_block) | ||
Mirko-von-Leipzig marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| .map(|(id_bytes, _)| { | ||
| AccountId::read_from_bytes(&id_bytes).map_err(DatabaseError::DeserializationError) | ||
| }) | ||
| .collect::<Result<Vec<AccountId>, DatabaseError>>()?; | ||
|
|
||
| let last_block_included = | ||
| BlockNumber::from_raw_sql(last_created_at_block.saturating_sub(1))?; | ||
|
|
||
| Ok((account_ids, last_block_included)) | ||
| } else { | ||
| let account_ids = account_ids_raw | ||
| .into_iter() | ||
| .map(|(id_bytes, _)| { | ||
| AccountId::read_from_bytes(&id_bytes).map_err(DatabaseError::DeserializationError) | ||
| }) | ||
| .collect::<Result<Vec<AccountId>, DatabaseError>>()?; | ||
|
|
||
| Ok(account_ids) | ||
| Ok((account_ids, *block_range.end())) | ||
| } | ||
| } | ||
|
|
||
| #[derive(Debug, Clone, PartialEq, Eq)] | ||
|
|
@@ -803,13 +854,29 @@ pub(crate) fn upsert_accounts( | |
| let mut count = 0; | ||
| for update in accounts { | ||
| let account_id = update.account_id(); | ||
| let account_id_bytes = account_id.to_bytes(); | ||
Mirko-von-Leipzig marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| let block_num_raw = block_num.to_raw_sql(); | ||
|
|
||
| let network_account_id_prefix = if account_id.is_network() { | ||
| Some(NetworkAccountPrefix::try_from(account_id)?) | ||
| } else { | ||
| None | ||
| }; | ||
|
|
||
| // Preserve the original creation block when updating existing accounts. | ||
| let created_at_block = QueryDsl::select( | ||
| schema::accounts::table.filter( | ||
| schema::accounts::account_id | ||
| .eq(&account_id_bytes) | ||
| .and(schema::accounts::is_latest.eq(true)), | ||
| ), | ||
| schema::accounts::created_at_block, | ||
| ) | ||
| .first::<i64>(conn) | ||
| .optional() | ||
| .map_err(DatabaseError::Diesel)? | ||
| .unwrap_or(block_num_raw); | ||
|
|
||
| // NOTE: we collect storage / asset inserts to apply them only after the account row is | ||
| // written. The storage and vault tables have FKs pointing to `accounts (account_id, | ||
| // block_num)`, so inserting them earlier would violate those constraints when inserting a | ||
|
|
@@ -916,25 +983,26 @@ pub(crate) fn upsert_accounts( | |
| diesel::update(schema::accounts::table) | ||
| .filter( | ||
| schema::accounts::account_id | ||
| .eq(&account_id.to_bytes()) | ||
| .eq(&account_id_bytes) | ||
| .and(schema::accounts::is_latest.eq(true)), | ||
| ) | ||
| .set(schema::accounts::is_latest.eq(false)) | ||
| .execute(conn)?; | ||
|
|
||
| let account_value = AccountRowInsert { | ||
| account_id: account_id.to_bytes(), | ||
| account_id: account_id_bytes, | ||
| network_account_id_prefix: network_account_id_prefix | ||
| .map(NetworkAccountPrefix::to_raw_sql), | ||
| account_commitment: update.final_state_commitment().to_bytes(), | ||
| block_num: block_num.to_raw_sql(), | ||
| block_num: block_num_raw, | ||
| nonce: full_account.as_ref().map(|account| nonce_to_raw_sql(account.nonce())), | ||
| storage: full_account.as_ref().map(|account| account.storage().to_bytes()), | ||
| vault: full_account.as_ref().map(|account| account.vault().to_bytes()), | ||
| code_commitment: full_account | ||
| .as_ref() | ||
| .map(|account| account.code().commitment().to_bytes()), | ||
| is_latest: true, | ||
| created_at_block, | ||
| }; | ||
|
|
||
| diesel::insert_into(schema::accounts::table) | ||
|
|
@@ -995,6 +1063,7 @@ pub(crate) struct AccountRowInsert { | |
| pub(crate) vault: Option<Vec<u8>>, | ||
| pub(crate) nonce: Option<i64>, | ||
| pub(crate) is_latest: bool, | ||
| pub(crate) created_at_block: i64, | ||
| } | ||
|
|
||
| #[derive(Insertable, AsChangeset, Debug, Clone)] | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.