js-libp2p icon indicating copy to clipboard operation
js-libp2p copied to clipboard

Review BigInt usage

Open p-shahi opened this issue 2 years ago • 3 comments

Inspired by https://github.com/ChainSafe/lodestar/issues/5892 and https://github.com/ipfs/protons/issues/112 cc: @wemeetagain

p-shahi avatar Aug 22 '23 16:08 p-shahi

For the most part we only use BigInts where a protobuf definitions means we need to handle values encoded as 64 bit numbers - we cannot use the js number type here because it can only handle integers up to 53 bits.

If the incoming message specifies things as 64 numbers I think we need to support the full range otherwise we could open ourselves up to attack or introduce unpredictable behaviour since js nodes will act differently compared to other implementations based on the data they are sent.

achingbrain avatar Aug 29 '23 09:08 achingbrain

I think it depends on the field. For example, in https://github.com/ChainSafe/js-libp2p-gossipsub/pull/327 we are looking at decoding the backoff field in the prune control message as a number. The backoff field is the time in seconds to wait before re-grafting. If a huge backoff value is provided, it doesn't really matter if we wait 2 ** 53 + 5 or 2 ** 53 + 1 seconds because we'll all be dead anyways.

This is the impetus for this issue. Inspect which uses of bigint are needed in practical terms.

wemeetagain avatar Aug 29 '23 13:08 wemeetagain

Related #2000 Another place where BigInt is used where the range provided by number is beyond adequate.

justin0mcateer avatar Sep 12 '23 00:09 justin0mcateer