fix: Ensure pagination cursor updates for all cases in get_cells and get_transactions

[tea2x]

As the title goes, I notice pagination cursor in some corner cases does not get updated and results in infinite loops. The root cause is pagination cursor update is skipped when certain edge conditions are met simultaneously for example:

• When the query limit is small (e.g. 1 cell/transaction).
• And all cells/transactions are filtered out (e.g. 1 cell/transaction) - skipping pagination cursor update(the updater is below the loop continuer so it is skipped).

Please check this out!
BR,

[tea2x]

So if I'm not mistaken at this, full node rpc get_cells and get_transactions share the same issue.
ref to ckb full node rpc: https://github.com/nervosnetwork/ckb/blob/c24f833a1a1000370f16b493006246628a4c448a/util/indexer/src/service.rs#L315

Just correct me if i'm wrong.

[Hanssen0]

        let cells = iter
            .take_while(|(key, _value)| key.starts_with(&prefix))
            .filter_map(|(key, value)| {
               ...
            })
            .take(limit)
            .collect::<Vec<_>>();

The take happens after filter_map. Which means as long as there are still cells that meet the criteria, the cells should not be empty. The test results on full node RPC also support this.

Considering that we have only objects and last_cursor, I assume an empty objects indicates the end of the full paginated result, regardless of whether last_cursor is updated or not.

• When the query limit is small (e.g. 1 cell/transaction).
• And all cells/transactions are filtered out (e.g. 1 cell/transaction) - skipping pagination cursor update(the updater is below the loop continuer so it is skipped).

So I think the problem is that we should exit the loop because there are no more results, but we didn't. If you can share your client-side code / test cases to reproduce the problem, we may be able to identify potential problems better.

we may be able to identify potential problems better.

Because, as far as I can tell, I didn't see a logical problem with existing code. The last_cursor is updated to the last valid cell as long as we still have one.

However, this might lead to a performance improvement if we skip a lot of invalid cells after the last valid ones, and the client-side code only checks empty objects as the signal of the end. This means we can skip redundant checks for the last empty call.

Also, this might be regarded as a code style improvement. In the future, if we add more valid returns before the last one, early assignment can prevent us from forgetting to update the cursor.

[quake]

I’d like to clarify our design decision regarding pagination. In the current implementation, the last_cursor always represents the key of the last object in the returned result set. When the result set is empty, last_cursor is intentionally empty as well. This behavior is by design and not considered a bug in the pagination logic.

In other words, the cursor does not advance when no items are returned because there is no “last object” to anchor the pagination. This approach makes the behavior consistent and predictable for clients:

1. Non-empty results -> last_cursor points to the last object’s key.
2. Empty results -> last_cursor is empty.

If an application requires different semantics (e.g., skipping over filtered-out items even when a page is empty), this would fall into the category of client-side handling or an additional feature proposal rather than a bug in the core design.

I also added an unit test to ensure this behavior is explicitly verified: #235

[tea2x]

hi @Hanssen0 , @quake it's good to see you here.

I didn't see a logical problem with existing code. The last_cursor is updated to the last valid cell as long as we still have one. - Hanssen

When the result set is empty, last_cursor is intentionally empty as well. This behavior is by design and not considered a bug in the pagination logic - Quake

So it seems I am the only one who thinks this is an issue here - even if it is a rare one.

Here's my argument. Check the following scenario:

- My app finds plain cells to make a "transfer transaction"
- Search limit is 10
- The account has 18 cells in total
    + 12 DAO cells
    + 6 plain cells
- On the first query, unfortunately all 10 cells are DAO cells so they're filtered out. Now pagination cursor keeps unchanged.
- The search ends here without going for the other 8 cells left. As currently handled on the front-end in CCC. 
- And this ends up with things like "not enough capacity .... etc ..." but this is not True. The other 8 cells contain enough capacity.

I do see this is an issue in case all cells on the first page are filtered out. When get_cells responses with zero cells, there're 2 ways front-end handles and I see that none of these is ok if get_cells keep functioning as it is currently:

1. Follow the current assumption on the front end - assuming the search ends --> we're missing cells --> missing data. Again check the above scenario i described. This is the current behavior i observe.
2. FE keeps searching --> infinite loop. Because Cursor is not updated and front-end keeps searching and querying the same empty page over and over again.

The problem here is that get_cells API's response as currently implemented will not be able to convey to the FE that the search has ended IN THIS RARE CASE. This is the reason i opened this PR. What are your thoughts on this?

If you can share your client-side code / test cases to reproduce the problem, we may be able to identify potential problems better.

Please check ckb-devrel/ccc#283 (comment)

[Hanssen0]

So it seems I am the only one who thinks this is an issue here - even if it is a rare one.

Here's my argument. Check the following scenario:

- My app finds plain cells to make a "transfer transaction"
- Search limit is 10
- The account has 18 cells in total
    + 12 DAO cells
    + 6 plain cells
- On the first query, unfortunately all 10 cells are DAO cells so they're filtered out. Now pagination cursor keeps unchanged.
- The search ends here without going for the other 8 cells left. As currently handled on the front-end in CCC. 
- And this ends up with things like "not enough capacity .... etc ..." but this is not True. The other 8 cells contain enough capacity.

I do see this is an issue in case all cells on the first page are filtered out. When get_cells responses with zero cells, there're 2 ways front-end handles and I see that none of these is ok if get_cells keep functioning as it is currently:

Let me reiterate my point about this hypothesis:

        let cells = iter
            .take_while(|(key, _value)| key.starts_with(&prefix))
            .filter_map(|(key, value)| {
               ...
            })
            .take(limit)
            .collect::<Vec<_>>();

The take happens after filter_map. Which means as long as there are still cells that meet the criteria, the cells should not be empty. The test results on full node RPC also support this.

I don't think there is a situation where the first page is completely filtered out and empty, but the second page has data.

[tea2x]

I don't think there is a situation where the first page is completely filtered out and empty, but the second page has data.

I see.

It's clear I'm mistaken about #234 (comment)

The issue is in WASM module only.

[Hanssen0]

Update:
The real problem #236

[tea2x]

Thanks @Hanssen0 and @quake I understand it better now.
I'll be closing this PR because it's based on my misunderstanding about the design of get_cells and get_transactions APIs.

The real problem is at #236