cmd/devp2p: add eth cmd to send new block announcements #25212
Conversation
lightclient
force-pushed
the
devp2p-eth-cmd
branch
from
8806a3f to
55417e4
Compare
|
@fjl PTAL when you have a chance! |
| } | ||
|
|
||
| // runGeth creates and starts a geth node | ||
| func runGeth() (*node.Node, error) { |
There was a problem hiding this comment.
It's unfortunate this is mostly copied exactly from cmd/devp2p/internal/ethtest/suite_test.go. Not sure if there is anything better that could be done.
cmd/devp2p/ethcmd.go
Outdated
| code := uint64((ethtest.NewBlock{}).Code()) | ||
| if _, err = conn.Conn.Write(code, msg); err != nil { | ||
| exit(fmt.Errorf("failed to write to connection: %w", err)) | ||
| } |
There was a problem hiding this comment.
This is a pretty generic send already. You could basically do
| code := uint64((ethtest.NewBlock{}).Code()) | |
| if _, err = conn.Conn.Write(code, msg); err != nil { | |
| exit(fmt.Errorf("failed to write to connection: %w", err)) | |
| } | |
| func ethMessage(..)( | |
| ... | |
| msgCode, err := hex.DecodeString(ctx.Args().Get(1)) | |
| msg, err := hex.DecodeString(ctx.Args().Get(2)) | |
| ... | |
| code := uint64((msgCode) | |
| if _, err = conn.Conn.Write(code, msg); err != nil { | |
| exit(fmt.Errorf("failed to write to connection: %w", err)) | |
| } |
And then we'd increase the generality of this tool quite a lot. The UX-cost would be an additional uint argument to add the NewBlock msg code.
There was a problem hiding this comment.
@lightclient want to pursue this?
There was a problem hiding this comment.
I rebased + made it generic now :)
There was a problem hiding this comment.
The generic command is great, but a cli command with 3 args is not very ergonomic. We already have a command with 3 args, devp2p rlpx eth-test, and I could never remember the parameter order... It would be better to handle the args as flags. I suggest that it should be handled like this:
devp2p rlpx eth-msg -version 67 -node enr:... <code || msg>
i.e. only one positional argument and combined msg code + data.
| } | ||
| // Peer with node. | ||
| status := ðtest.Status{ | ||
| ProtocolVersion: 66, |
There was a problem hiding this comment.
This hard-coded version is going to be a problem. I think it should be passed as a flag.
|
Another note: the original purpose of this command is kind of lost since the merge, because block announcements are not accepted via |
|
Yeah that is a good point, I'll see if I can update that hive PR to use the engine API instead. |
This PR adds a new subcommand to
devp2p:eth. This subcommand has a single command of it's own currently,new-blockwhich sends the specified hex as a devp2p block announcement message. We can expandethto support more arbitrary message sending in the future.$ devp2p eth new-block <enode> <msg>