Add tests for uncle timestamps and refactor timestamp type#1711
Add tests for uncle timestamps and refactor timestamp type#1711fjl merged 1 commit intoethereum:developfrom
Conversation
0659fb1 to
df8688f
Compare
There was a problem hiding this comment.
2nd arg why not parent.Time().Uint64()?
There was a problem hiding this comment.
Some tests do not initialise parent in the first call to this function, and I wanted to avoid changing them just for this patch.
df8688f to
a5f4bce
Compare
There was a problem hiding this comment.
Although valid, comparing against 1 is a bit odd. Usually big int comparison is done as:
< 0if the first is smaller== 0if they are equal> 0if the first one is greater
This scheme makes it a bit more visible as the comparison operator already "symbolizes" the actual result vs. having to interpret -1, or 1. It is used like that throughout the entire standard lib too, so better to stick with the canonical use.
There was a problem hiding this comment.
@karalabe I'd wait with this since the comparisons for extra data, difficulty and gas uses the "odd" convention - better we change this everywhere in a follow-up PR so we avoid touching unrelated code in this patch.
There was a problem hiding this comment.
Ok with me then, but it would be nice to update them at a certain point. :)
|
👍 modulo comments |
|
👎 Until the immutability is fixed. Also a question. The verify header does not check against negative values. Previously since we unmarshalled into an uint64, this was not an issue. Now however a timestamp big.Int may become negative. We should make sure that such cases are handled correctly. |
a5f4bce to
08c5f10
Compare
|
@karalabe RLP cannot contain negative big ints. |
There was a problem hiding this comment.
@Gustav-Simonsson please also update copyHeader
No description provided.