libplanet icon indicating copy to clipboard operation
libplanet copied to clipboard

Change `offset` and `limit` parameters in various places to `long`

Open greymistcube opened this issue 4 years ago • 9 comments

Notably for methods in BlockChain<T> and IStore, offset and limit parameters are of type int or int?. Since Block<T>.Index is of type long, this discrepancy results in unnecessary casting. Assuming Block<T>.Index is of type long for a reason, that is, the maximum value of int may not be large enough, offset and limit should follow suit.

greymistcube avatar Jun 26 '21 08:06 greymistcube

This issue has been automatically marked as stale because it has not had recent activity. Thank you for your contributions.

stale[bot] avatar Aug 25 '21 19:08 stale[bot]

I started to work on this change, and I discovered that LiteDBStore is what needs offset and limit to be ints. E.g. when I changed them to long, I found I would have to make this change in DefaultStore.cs:

        public override IEnumerable<BlockHash> IterateIndexes(Guid chainId, long offset, long? limit)
        {
            return IndexCollection(chainId)
                .Find(Query.All(), (int) offset, (int?) limit ?? int.MaxValue)
                .Select(i => i.Hash);
        }

I doubt you want to cast longs to ints like this, due to the possibility of data loss.

forrcaho avatar Oct 31 '21 21:10 forrcaho

This issue has been automatically marked as stale because it has not had recent activity. Thank you for your contributions.

stale[bot] avatar Apr 16 '22 01:04 stale[bot]

hyennin avatar Jul 21 '22 11:07 hyennin

akaz00 avatar Jul 21 '22 11:07 akaz00

@Hyemin-12 as like @forrcaho said LiteDBCollection.Find() (or other similar APIs) doesn't seem to compatible with long. (e.g., 1) So I suggest throwing exception as like below when we facing non-compatible value. (I guess it's okay because LiteDB implementation isn't used in production and we'd like to obsolete it. 😅 )

/// <inheritdoc cref="BaseStore.IterateIndexes(Guid, long, long?)"/>
public override IEnumerable<BlockHash> IterateIndexes(Guid chainId, long offset, long? limit)
{
    int iOffSet = (offset < int.MinValue || offset > int.MaxValue)
        ? throw new ArgumentOutOfRangeException(
            nameof(offset),
            offset,
            "The given value isn't supported on the storage backend.")
        : (int)offset;
    int? iLimit = (limit < int.MinValue || limit > int.MaxValue)
       ? throw new ArgumentOutOfRangeException(
           nameof(limit),
           limit,
           "The given value isn't supported on the storage backend.")
       : (int?)limit;

    return IndexCollection(chainId)
        .Find(Query.All(), iOffSet, iLimit ?? int.MaxValue)
        .Select(i => i.Hash);
}

longfin avatar Jul 25 '22 13:07 longfin