Skip to content

Increase limit by power of two to avoid all cells/transactions were filtered out in light-client-wasm#237

Closed
Officeyutong wants to merge 3 commits intodevelopfrom
fix-236
Closed

Increase limit by power of two to avoid all cells/transactions were filtered out in light-client-wasm#237
Officeyutong wants to merge 3 commits intodevelopfrom
fix-236

Conversation

@Officeyutong
Copy link
Collaborator

This PR changes the logic of get_cells and get_transactions in light-client-wasm

  • Introduced internal_limit, which was initialized by limit from rpc call, it will increase by power of two if no cells/transactions were found after filtered logic.
  • internal_limit will be used for limit of storage.collect_iterator
  • When internal_limit reaches u32::MAX, we can assume that no data was found, since the maximum value of limit is u32::MAX

@Hanssen0
Copy link

How the original get_transactions (group_by_transaction) implements this seems more reasonable: iterating all cells page by page. The current implementation seems to introduce extra costs for both performance and memory, and has worse readability.

@Officeyutong
Copy link
Collaborator Author

How the original get_transactions (group_by_transaction) implements this seems more reasonable: iterating all cells page by page. The current implementation seems to introduce extra costs for both performance and memory, and has worse readability.

The original implementation is based on RocksDB, which returns an iterator over selected elements, we can implement such API on wasm but it will be much more difficult. In my opinion, light client usually won't involve too much data so I think the current implementation in this PR is acceptable

@Hanssen0
Copy link

The original implementation is based on RocksDB, which returns an iterator over selected elements, we can implement such API on wasm but it will be much more difficult.

Sorry for the misunderstanding. When I say the "original implementation", I mean the version of WASM light client.

let mut kvs: Vec<_> = storage.collect_iterator(
from_key,
direction,
Box::new(move |key| key.starts_with(&prefix_cloned)),
100,
skip,
);
let mut tx_with_cells: Vec<TxWithCells> = Vec::new();
let mut last_key = Vec::new();
'outer: while !kvs.is_empty() {

let prefix_cloned = prefix.clone();
kvs = storage.collect_iterator(
last_key.clone(),
direction,
Box::new(move |key| key.starts_with(&prefix_cloned)),
100,
1,
);

@Hanssen0
Copy link

In my opinion, light client usually won't involve too much data

This is somewhat controversial. It is not uncommon to have addresses with hundreds of cells/NFTs. If these NFTs also store image data, the data size may be larger than expected.

@Officeyutong
Copy link
Collaborator Author

Officeyutong commented Aug 25, 2025

In my opinion, light client usually won't involve too much data

This is somewhat controversial. It is not uncommon to have addresses with hundreds of cells/NFTs. If these NFTs also store image data, the data size may be larger than expected.

Seems increase by power of 2 and increase by a fixed size of page have the same expected database query times (measured by fetch of data field), which do you think is better @quake

1

Increase by fixed page size:

  • May call storage.collect_iterator more times, which is very slow
  • Sometimes can decrease data fetch count

Increase by power of 2:

  • Call storage.collect_iterator less times (at most log(2,n) times)
  • May cause large data fetch count when the number of filtered out records is near power of 2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants