-
Notifications
You must be signed in to change notification settings - Fork 86
feat: [2/4] integrate smtforest, avoid ser/de of full account/vault data in database #1394
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
base: next
Are you sure you want to change the base?
Changes from 35 commits
4ff0970
5ee1043
8eca359
e7f17ed
c8b43ab
19164be
8eb49af
6725461
ee65a88
741df6f
6a64077
9d5806e
2964a93
9416a63
ccc2d63
66ea831
1c4f8b1
80e0393
e7bf1aa
dad90e7
e441245
0a319d1
8897939
7d7fefc
0f53fa9
7400134
0e2d871
f78103e
ea05b01
3cd457a
c8f0eb1
ed7224e
6336f41
a1173f7
36470a5
928fdb4
5de3936
f5b4898
ca5ef9a
17fd95b
22f3ca9
3ee1884
eaf7242
72126e1
be9071b
a0f8fc9
88c058b
b84f25f
25b5550
31dacdd
55f4a46
bf67ce8
0c0e32b
ec4318e
4bfee30
b8d2e66
e453faa
f6d1ce1
b1f9cf6
d2d9e8c
2781db8
b0e537c
d6b31ef
9c859fc
b016495
579b9dc
3336edb
2aa8c8b
354d586
a96def0
e8cdad1
3009bf7
3110962
53cb5e8
0cc0c61
ac7b8f9
369db2f
6cd1033
7613624
e04ff10
9d8c220
5ed1a4f
c43a5a2
3346d9f
dbbc1eb
b3c91df
77443c2
1efa4f0
69ee5a5
c5a199a
8d33f66
635cb78
7ca6999
c712ba4
d9a666f
29b840c
1b22a34
1cab5e2
c42d5f5
f1655fe
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 |
|---|---|---|
|
|
@@ -69,6 +69,7 @@ impl From<AccountId> for proto::account::AccountId { | |
| // ACCOUNT UPDATE | ||
| // ================================================================================================ | ||
|
|
||
| // TODO should be called `AccountStateRef` or so | ||
| #[derive(Debug, PartialEq)] | ||
| pub struct AccountSummary { | ||
| pub account_id: AccountId, | ||
|
|
@@ -86,6 +87,7 @@ impl From<&AccountSummary> for proto::account::AccountSummary { | |
| } | ||
| } | ||
|
|
||
| // TODO #[deprecated(note = "avoid this type, details will be `None` always!")] | ||
| #[derive(Debug, PartialEq)] | ||
| pub struct AccountInfo { | ||
| pub summary: AccountSummary, | ||
|
|
@@ -345,12 +347,31 @@ impl From<AccountStorageHeader> for proto::account::AccountStorageHeader { | |
| } | ||
| } | ||
|
|
||
| /// Account vault assets | ||
| /// | ||
| /// Represents a list of assets, if the number of assets is reasonably small, which | ||
| /// is currently set to 1000 for no particular reason. | ||
| /// | ||
| /// When an account contains a large number of assets, including all assets | ||
| /// in a single RPC response would create performance issues on client and server as | ||
| /// and consume quite a bit of bandwidth, besides requiring additional memory on | ||
| /// possibly low powered clients. | ||
| /// | ||
| /// Hence `too_many_assets` is returned, which is indicating to the client to use the dedicated | ||
| /// `SyncAccountVault` RPC endpoint and do incremental retrieval | ||
| #[derive(Debug, Clone, PartialEq, Eq)] | ||
| pub struct AccountVaultDetails { | ||
| /// Flag indicating whether the vault has too many assets to return inline. | ||
| /// If `true`, clients must use `SyncAccountVault` endpoint instead. | ||
| pub too_many_assets: bool, | ||
|
|
||
| /// The assets in the vault. Empty if `too_many_assets` is `true`. | ||
| pub assets: Vec<Asset>, | ||
| } | ||
drahnr marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| impl AccountVaultDetails { | ||
| /// Maximum number of vault entries that can be returned in a single response. | ||
| /// Accounts with more assets will have `too_many_assets = true` and empty `assets`. | ||
| const MAX_RETURN_ENTRIES: usize = 1000; | ||
|
|
||
| pub fn new(vault: &AssetVault) -> Self { | ||
|
|
@@ -371,6 +392,27 @@ impl AccountVaultDetails { | |
| } | ||
| } | ||
|
|
||
| /// Creates `AccountVaultDetails` from vault entries (key-value pairs). | ||
| /// | ||
| /// This is useful when entries have been fetched directly from the database | ||
| /// rather than extracted from an `AssetVault`. | ||
| /// | ||
| /// The entries are `(vault_key, asset)` pairs where `asset` is a Word representation. | ||
sergerad marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| pub fn from_entries(entries: Vec<(Word, Word)>) -> Result<Self, miden_objects::AssetError> { | ||
| let too_many_assets = entries.len() > Self::MAX_RETURN_ENTRIES; | ||
|
|
||
| if too_many_assets { | ||
| return Ok(Self::too_many()); | ||
| } | ||
|
||
|
|
||
| let assets = entries | ||
| .into_iter() | ||
| .map(|(_key, asset_word)| Asset::try_from(asset_word)) | ||
| .collect::<Result<Vec<_>, _>>()?; | ||
|
|
||
| Ok(Self { too_many_assets: false, assets }) | ||
| } | ||
|
|
||
| fn too_many() -> Self { | ||
| Self { | ||
| too_many_assets: true, | ||
|
|
@@ -410,20 +452,42 @@ impl From<AccountVaultDetails> for proto::rpc::AccountVaultDetails { | |
| } | ||
| } | ||
|
|
||
| /// Details about an account storage map slot, including overflow handling. | ||
| /// | ||
| /// ## Rationale for "Too Many Entries" Flag | ||
| /// | ||
| /// Similar to `AccountVaultDetails`, when a storage map contains many entries (> 1000), | ||
| /// returning all entries in a single RPC response creates performance issues: | ||
| /// - Large serialization/deserialization costs | ||
| /// - Network bandwidth saturation | ||
| /// - Client memory pressure | ||
| /// | ||
| /// When `too_many_entries` is `true`: | ||
| /// - The `map_entries` field is empty (no data included) | ||
| /// - Clients should use the dedicated `SyncStorageMaps` RPC endpoint | ||
| /// - That endpoint supports pagination and block range filtering | ||
| #[derive(Debug, Clone, PartialEq, Eq)] | ||
| pub struct AccountStorageMapDetails { | ||
| pub slot_index: u8, | ||
|
|
||
| /// Flag indicating whether the storage map has too many entries to return inline. | ||
| /// If `true`, clients must use `SyncStorageMaps` endpoint instead. | ||
| pub too_many_entries: bool, | ||
|
|
||
| /// The storage map entries (key-value pairs). Empty if `too_many_entries` is `true`. | ||
| /// TODO: For partial responses, also include Merkle proofs and inner SMT nodes. | ||
| pub map_entries: Vec<(Word, Word)>, | ||
| } | ||
|
|
||
| impl AccountStorageMapDetails { | ||
| const MAX_RETURN_ENTRIES: usize = 1000; | ||
| /// Maximum number of storage map entries that can be returned in a single response. | ||
| /// Maps with more entries will have `too_many_entries = true` and empty `map_entries`. | ||
| pub const MAX_RETURN_ENTRIES: usize = 1000; | ||
|
|
||
| pub fn new(slot_index: u8, slot_data: SlotData, storage_map: &StorageMap) -> Self { | ||
| match slot_data { | ||
| SlotData::All => Self::from_all_entries(slot_index, storage_map), | ||
| SlotData::MapKeys(keys) => Self::from_specific_keys(slot_index, &keys[..], storage_map), | ||
| SlotData::MapKeys(_keys) => Self::from_all_entries(slot_index, storage_map), /* TODO use from_specific_keys */ | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -440,12 +504,17 @@ impl AccountStorageMapDetails { | |
| } | ||
| } | ||
|
|
||
| fn from_specific_keys(slot_index: u8, keys: &[Word], storage_map: &StorageMap) -> Self { | ||
| if keys.len() > Self::MAX_RETURN_ENTRIES { | ||
| Self::too_many_entries(slot_index) | ||
| } else { | ||
| // TODO For now, we return all entries instead of specific keys with proofs | ||
| Self::from_all_entries(slot_index, storage_map) | ||
| /// Creates an `AccountStorageMapDetails` from already-queried entries (e.g., from database). | ||
| /// This is useful when entries have been fetched directly rather than extracted from a | ||
| /// `StorageMap`. | ||
| pub fn from_entries(slot_index: u8, map_entries: Vec<(Word, Word)>) -> Self { | ||
| let too_many_entries = map_entries.len() > Self::MAX_RETURN_ENTRIES; | ||
| let map_entries = if too_many_entries { Vec::new() } else { map_entries }; | ||
|
||
|
|
||
| Self { | ||
| slot_index, | ||
| too_many_entries, | ||
| map_entries, | ||
| } | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently, every asset is 32 bytes - though, we are likely to increase this to 64 bytes in the future. So, 1K assets would be about 64KB of data. It feels a bit high - so, maybe we should reduce this, but I don't have good rationale to what exactly.