Add enumerate() back into EnumerableSet

[thegostep]

Fixes #2166

[nventuro]

Thanks @thegostep! This would indeed make such an external function easier to implement. However, I worry about unbounded array access: even if these are view functions, eth_call is still gas-limited.

What do you think about doing pagination by providing a query size? It has the disadvantage that you need to query the set size beforehand, but I think this is one of the better ways to make this scale.

Using the high level getters the code might look something like:

function enumerate(Set set, uint256 startIndex, uint256 pageSize) internal view returns (uint256[] memory) {
    uint256[] memory result;
    require(startIndex + pageSize <= set.length(), "Read index out of bounds");

    for (uint256 i = 0; i < pageSize; ++i) {
        result[i] = set.at(i + startIndex);
    }

    return result;
}

[thegostep]

Why not all 3?

It's usually what I do in my projects:
https://github.com/erasureprotocol/erasure-protocol/blob/be41efc93cdc177b22f7b5a6dac0bb8d63aeb88c/contracts/modules/Registry.sol#L176-L223

[thegostep]

Removing functionality because there is a chance of running out of gas is not a good approach IMO.

Realistically, most use cases will not reach that edge case and the ones that do have all they need to mitigate it.

[frangio]

the ones that do have all they need to mitigate it.

How is this so? A smart contract that uses a function that returns an array has a vulnerability that potentially can't be mitigated. We don't want to provide a primitive that could put users in that position.

While you're free to include all 3 variants in your own contracts, because you know you won't misuse them, we think we have to be more careful and avoid providing interfaces that are easy to misuse.

[thegostep]

ERC20 is easy to misuse 😂

[stale]

Hi all!
This Pull Request has not had any recent activity, is it still relevant? If so, what is blocking it? Is there anything we can do to help move it forward?
Thanks!

[frangio]

@nventuro Should we close this issue in favor of a pagination or indexing approach?

It has the disadvantage that you need to query the set size beforehand, but I think this is one of the better ways to make this scale.

I don't think this is true by the way. It should be possible to query for pages until you get one that is smaller than the size you requested, then you know you've reached the end.

[nventuro]

Should we close this issue in favor of a pagination or indexing approach?

What do you mean by 'pagination or indexing'? Whether the index is the actual item index or the page number? Page number seems more sensible, and yes I'd go for that.

I don't think this is true by the way. It should be possible to query for pages until you get one that is smaller than the size you requested, then you know you've reached the end.

Good point.

[frangio]

Yes, I mean what you said.