Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Review BigInt usage #1986

Open
p-shahi opened this issue Aug 22, 2023 · 3 comments
Open

Review BigInt usage #1986

p-shahi opened this issue Aug 22, 2023 · 3 comments
Labels
kind/discussion Topical discussion; usually not changes to codebase

Comments

@p-shahi
Copy link
Member

p-shahi commented Aug 22, 2023

Inspired by ChainSafe/lodestar#5892 and ipfs/protons#112
cc: @wemeetagain

@p-shahi p-shahi added the need/triage Needs initial labeling and prioritization label Aug 22, 2023
@achingbrain
Copy link
Member

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 achingbrain added kind/discussion Topical discussion; usually not changes to codebase and removed need/triage Needs initial labeling and prioritization labels Aug 29, 2023
@wemeetagain
Copy link
Member

I think it depends on the field. For example, in ChainSafe/js-libp2p-gossipsub#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.

@justin0mcateer
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/discussion Topical discussion; usually not changes to codebase
Projects
None yet
Development

No branches or pull requests

4 participants