cmd,core,eth,params,tests: enable 2565 for yolov3 #22213
Conversation
|
Haven't reviewed in-depth yet, but it looks more or less like what I had expected 👍 |
|
It seems like some blockchain and state tests are failing because the Berlin tests are not based on YoloV3. When I add the BLS precompiles and unset the modexp flag they pass. |
Yes, that makes sense. It's one reason why I tried to get everyone to implemenet Rant aside, we'll just have to skip these tests, which can be done in |
cmd/utils/flags.go
Outdated
| if !ctx.GlobalIsSet(NetworkIdFlag.Name) { | ||
| cfg.NetworkId = 133519467574834 // "yolov2" | ||
| cfg.NetworkId = 133519467574834 // "yolov3" |
There was a problem hiding this comment.
I also made this mistake when updating yolov1 to yolov2. But actually:
>>> import codecs
>>> codecs.decode(hex(133519467574834)[2:], "hex")
b'yolov2'
So this should be changed from 133519467574834 to 133519467574835
There was a problem hiding this comment.
Or, even better, you can fix it so we don't do the same mistake again, and change it t
new(big.Int).SetBytes([]byte("yolov3")).Uint64()
holiman
force-pushed
the
yolov3/enable-2565
branch
from
61a5e78
to
fbd2ea6
Compare
| var YoloV2Bootnodes = []string{ | ||
| // YoloV3Bootnodes are the enode URLs of the P2P bootstrap nodes running on the | ||
| // YOLOv3 ephemeral test network. | ||
| // TODO: Set Yolov3 bootnodes |
There was a problem hiding this comment.
I think we can leave this as is, and reuse both nodekey and IP.
There was a problem hiding this comment.
I think this is OK. The problem is that unless we also merge 2718/2930, we don't have a correct yolov3. So maybe we should disable --yolov3 here (just comment out the flag), and then un-comment it in 2718/2930.
That would make it possible to merge this, and then rebase 2718, and run yolov3 from that PR without
- merging that PR to master, and
- Having a confusing misconfigured
--yolov3mode onmaster
There were a few things that I think can only be done later, like setting bootnodes, genesis data, etc. In those cases I only renamed the variable but didn't touch the values.
Also the bls_fuzzer depended on those precompiles being exported so I created another set just for that.
I'm planning to tinker around with this a bit more make sure I haven't left anything out.