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

miner actor indexes slice by an int64 #521

Closed
phritz opened this issue May 31, 2018 · 8 comments
Closed

miner actor indexes slice by an int64 #521

phritz opened this issue May 31, 2018 · 8 comments
Labels
help wanted Call for participation: More complex than good-first-issue

Comments

@phritz
Copy link
Contributor

phritz commented May 31, 2018

As part of looking into #340 I noticed that the miner actor indexes a slice using an int64 https://github.com/filecoin-project/go-filecoin/blob/master/actor/builtin/miner/miner.go#L219. My understanding is that this is going to fail on 32-bit platforms as soon as the value is greater than int max. For example https://play.golang.org/p/lzNI3tQyMA7.

See also discussion in #340

@dignifiedquire
Copy link
Contributor

Yes it will. My main input here is, please don't use int or uint, as we need and want to be precise with the limits independent of your platform.

@aboodman
Copy link
Contributor

aboodman commented Jun 5, 2018

Trying to understand ... is there a problem here right now? This code won't work as-is when the length of the slice gets longer than 2^32, but at that point (probably way before that point) we will be using something fancier in-memory.

@aboodman
Copy link
Contributor

aboodman commented Jun 5, 2018

Put another way: is there a protocol-breaking change that needs to get made here? (honest question).

@phritz
Copy link
Contributor Author

phritz commented Jun 5, 2018

There is a problem in the sense that what we are doing is a bug. There is not a problem in the sense that it is not going to break any time soon.

@aboodman
Copy link
Contributor

aboodman commented Jun 5, 2018

OK snarkasaurus. Yeah there are so many things that fit that description.

Before we get anywhere close to this slice index, presumably the node will fall over trying to deserialize an XGB array into memory.

@phritz
Copy link
Contributor Author

phritz commented Jun 5, 2018

As it likely falls out of data model work I just moved this issue from "ready" to "backlog".

@phritz
Copy link
Contributor Author

phritz commented Sep 11, 2018

Think this is fixed by moving to champ, so let's make the scope of this issue to scan for any places where we're doing something like this to an on-chain data structure and then close the issue if none are found.

@phritz phritz added the help wanted Call for participation: More complex than good-first-issue label Sep 11, 2018
@ZenGround0
Copy link
Contributor

@phritz @acruikshank cool to close?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Call for participation: More complex than good-first-issue
Projects
None yet
Development

No branches or pull requests

4 participants