Support balance pagination

[moisses89]

Description

Enable pagination to support balances with max_limit = 200 and default pagination 100.
This PR is not affecting the database query, because we are getting the token balance from the ERC20 transfers, it is just reducing the number of calls to the RPC node.

Close #1432

[moisses89]

Not sure if this functions is a little bit confusing, what do you think?

[moisses89]

Maybe better return just the list and pass the list as parameter?

[Uxio0]

I don't have a strong opinion here, but indeed sounds confusing

[falvaradorodriguez]

In my opinion I would do a method that directly gets the erc20_addresses_paginated. That is to say something like "get_erc20_addresses_paginated_list" and inside add all the logic to retrieve the list and paginate it. Additionally, it would return the total count of the addresses list.

Something like this:

    def _get_erc20_addresses_paginated_list(
        self,
        safe_address: ChecksumAddress,
        limit: int,
        offset: int,
        only_trusted: bool = False,
        exclude_spam: bool = False,
    ) -> Tuple[List[ChecksumAddress], int]:

        all_erc20_addresses = ERC20Transfer.objects.tokens_used_by_address(safe_address)
        erc20_addresses = self._filter_addresses(
            all_erc20_addresses, only_trusted, exclude_spam
        )

        count = len(erc20_addresses)
        if not limit:
            # No pagination no limits
            return erc20_addresses[0:None], count

        if offset == 0:
            # First page will include also native token balance
            return erc20_addresses[offset : (limit - 1)], count
        else:
            # Include previous ERC20 after first page
            previous_offset = offset - 1
            return erc20_addresses[previous_offset : (previous_offset + limit)], count

This way, at least for me, it is easier to understand what it does inside, and the logic of the main method remains clearer.

WDYT?

[moisses89]

Done 0d1a9ab

[falvaradorodriguez]

Here we should update the :return: in method docstring

[moisses89]

Done 0d1a9ab

[falvaradorodriguez]

In my opinion I would do a method that directly gets the erc20_addresses_paginated. That is to say something like "get_erc20_addresses_paginated_list" and inside add all the logic to retrieve the list and paginate it. Additionally, it would return the total count of the addresses list.

Something like this:

    def _get_erc20_addresses_paginated_list(
        self,
        safe_address: ChecksumAddress,
        limit: int,
        offset: int,
        only_trusted: bool = False,
        exclude_spam: bool = False,
    ) -> Tuple[List[ChecksumAddress], int]:

        all_erc20_addresses = ERC20Transfer.objects.tokens_used_by_address(safe_address)
        erc20_addresses = self._filter_addresses(
            all_erc20_addresses, only_trusted, exclude_spam
        )

        count = len(erc20_addresses)
        if not limit:
            # No pagination no limits
            return erc20_addresses[0:None], count

        if offset == 0:
            # First page will include also native token balance
            return erc20_addresses[offset : (limit - 1)], count
        else:
            # Include previous ERC20 after first page
            previous_offset = offset - 1
            return erc20_addresses[previous_offset : (previous_offset + limit)], count

This way, at least for me, it is easier to understand what it does inside, and the logic of the main method remains clearer.

WDYT?

[falvaradorodriguez]

Should we keep the same criteria for max_limit in all endpoints? In any case, we should document it in the swagger. WDYT?

[moisses89]

I don´t think that we should have max_limit 10 for all the endpoints, we are setting just 10 for collectibles endpoint because have several outgoing requests to third parties endpoints to fill the metadata information of each collectible.

[falvaradorodriguez]

I didn't require changes, because all the comments are free of interpretation. We can discuss it :)

[falvaradorodriguez]

LGTM!