eth: fix the block range assignment for log filter #17284
Conversation
eth/filters/api.go
Outdated
| } | ||
| if crit.ToBlock == nil { | ||
| end = int64(rpc.LatestBlockNumber) | ||
| crit.ToBlock = big.NewInt(rpc.LatestBlockNumber.Int64()) | ||
| } |
There was a problem hiding this comment.
Please keep the variables and update those instead of the criteria. I don't think in general it's a good idea to modify input parameters:
begin := rpc.LatestBlockNumber.Int64()
if crit.FromBlock != nil {
begin = crit.FromBlock.Int64()
}
end := rpc.LatestBlockNumber.Int64()
if crit.ToBlock != nil {
end = crit.ToBlock.Int64()
}
| ) | ||
| if crit.FromBlock == nil { | ||
| begin = int64(rpc.LatestBlockNumber) | ||
| begin := rpc.LatestBlockNumber.Int64() |
There was a problem hiding this comment.
nit: seems a bit wasteful to first get the value and then don't use it in some cases. How about if crit.FromBlock != nil begin = crit.FromBlock.Int64() else begin = rpc.LatestBlockNumber.Int64()
SCNR: would be even nicer in a language that supports this: begin = (if (crit.FromBlock != nil) crit.FromBlock else rpc.LatestBlockNumber).Int64() or even begin = (crit.FromBlock?:rpc.LatestBlockNumber).Int64
There was a problem hiding this comment.
LatestBlockNumber is just a big.Int constant, so the cost of this op is a cast from big.Int to int64. The more optimal version isn't really worth the more complex code.
There was a problem hiding this comment.
I do not think the code is more complex this way. I think this way it is more expressive and tells the developer what it is doing instantly - saving a few cycles in the process. But might also just be my language bias as I would have this in mind but it is not possible in go AFAIK: begin = (crit.FromBlock?:rpc.LatestBlockNumber).Int64
As I prefixed - just a nit & just something that went through my mind when reading the code - no real need to change something.
No description provided.