add bitcoin support to nebula #84
Conversation
| type AddrInfo struct { | ||
| id string | ||
| Addr []ma.Multiaddr | ||
| } | ||
| type PeerInfo struct { | ||
| AddrInfo | ||
| } |
There was a problem hiding this comment.
You can move the fields into the PeerInfo struct. I don't think the nesting is necessary. You can also export the id field like so:
| type AddrInfo struct { | |
| id string | |
| Addr []ma.Multiaddr | |
| } | |
| type PeerInfo struct { | |
| AddrInfo | |
| } | |
| type PeerInfo struct { | |
| ID string | |
| Addr []ma.Multiaddr | |
| } |
bitcoin/driver_crawler.go
Outdated
| type CrawlDriverConfig struct { | ||
| Version string | ||
| TrackNeighbors bool | ||
| DialTimeout time.Duration | ||
| BootstrapPeers []ma.Multiaddr | ||
| AddrDialType config.AddrType | ||
| AddrTrackType config.AddrType | ||
| KeepENR bool | ||
| MeterProvider metric.MeterProvider | ||
| TracerProvider trace.TracerProvider | ||
| LogErrors bool | ||
| UDPBufferSize int | ||
| UDPRespTimeout time.Duration | ||
| } |
There was a problem hiding this comment.
You don't need the following fields.
| type CrawlDriverConfig struct { | |
| Version string | |
| TrackNeighbors bool | |
| DialTimeout time.Duration | |
| BootstrapPeers []ma.Multiaddr | |
| AddrDialType config.AddrType | |
| AddrTrackType config.AddrType | |
| KeepENR bool | |
| MeterProvider metric.MeterProvider | |
| TracerProvider trace.TracerProvider | |
| LogErrors bool | |
| UDPBufferSize int | |
| UDPRespTimeout time.Duration | |
| } | |
| type CrawlDriverConfig struct { | |
| Version string | |
| TrackNeighbors bool | |
| DialTimeout time.Duration | |
| BootstrapPeers []ma.Multiaddr | |
| MeterProvider metric.MeterProvider | |
| TracerProvider trace.TracerProvider | |
| LogErrors bool | |
| } |
maybe Version can also be removed.
| "dnsseed.bitcoin.dashjr.org", | ||
| "seed.bitcoinstats.com", | ||
| "seed.bitnodes.io", | ||
| "bitseed.xf2.org", |
There was a problem hiding this comment.
Consider using multi address format:
| "bitseed.xf2.org", | |
| "/dns/bitseed.xf2.org", |
thisHermit
changed the title
|
Could you also add the new network to the readme and perhaps mark it as alpha? |
| // there is no other reliable transport address like TCP or QUIC we use the UDP | ||
| // IP address + port and craft a TCP address out of it. The UDP address will | ||
| // still be removed and replaced with TCP. | ||
| func sanitizeAddrs(maddrs []ma.Multiaddr) ([]ma.Multiaddr, bool) { |
There was a problem hiding this comment.
this function is obsolete and can be removed
| if conn == nil { | ||
| log.Infoln("SOMETHING IS WRONG!!!!") | ||
| } | ||
| return wire.WriteMessage(conn, msg, wire.ProtocolVersion, wire.MainNet) |
There was a problem hiding this comment.
| if conn == nil { | |
| log.Infoln("SOMETHING IS WRONG!!!!") | |
| } | |
| return wire.WriteMessage(conn, msg, wire.ProtocolVersion, wire.MainNet) | |
| return wire.WriteMessage(conn, msg, wire.ProtocolVersion, wire.MainNet) |
| }) | ||
| logEntry.Debugln("Connecting to peer", pi.ID.ShortString()) | ||
|
|
||
| netAddr, _ := manet.ToNetAddr(pi.Addrs[0]) |
| // addrInfo := peer.AddrInfo{ | ||
| // ID: pi.ID(), | ||
| // Addrs: sanitizedAddrs, | ||
| // } | ||
|
|
||
| var conn net.Conn |
There was a problem hiding this comment.
| // addrInfo := peer.AddrInfo{ | |
| // ID: pi.ID(), | |
| // Addrs: sanitizedAddrs, | |
| // } | |
| var conn net.Conn | |
| var conn net.Conn |
| } | ||
| result.Error = err | ||
| resultCh <- result | ||
| close(resultCh) |
There was a problem hiding this comment.
rather do a defer close(resultCh) and also remove the close call at the end of the function
| close(resultCh) | ||
| return | ||
| } | ||
| // conn, result.ConnectError = c.connect(ctx, addrInfo) // use filtered addr list |
There was a problem hiding this comment.
same here, why the connect method below and this here? I would prefer to keep and use a dedicated connect method that abstracts away the error case handling and only returns an error if we really couldn't connect on any of the provided addresses.
| result.Agent = nodeRes.UserAgent | ||
| result.ProtocolVersion = nodeRes.ProtocolVersion | ||
| if err != nil { | ||
| log.Debugf("[%s] Handshake failed: %v", addrs, err) |
There was a problem hiding this comment.
rather use the WithError method on the logger. Also the log messages below.
| firstReceived := -1 | ||
| tolerateMessages := 5 | ||
| // The nodes send a lot of inv messages | ||
| tolerateInvMessages := 50 | ||
| tolerateVerAckMessages := 10 | ||
| toleratePingMessages := 3 |
There was a problem hiding this comment.
could you in a brief code comment explain the logic behind the message "toleration"
| if conn != nil { | ||
| if err := conn.Close(); err != nil { | ||
| log.WithError(err).WithField("remoteID", pi.ID().ShortString()).Warnln("Could not close connection on context cancel") | ||
| } | ||
| } |
There was a problem hiding this comment.
rather put this in a defer statement at the point in the code where you have received the connection object.
| Addr: []ma.Multiaddr{maddr}, | ||
| }, | ||
| } | ||
| } |
There was a problem hiding this comment.
any chance to remove the code duplication with above?
This PR introduces Bitcoin network support to the Nebula crawler by implementing necessary interfaces, integrating the
btcsuite/btcd/wirepackage, and adding a basic Bitcoin network crawler. Key changes include:1. Interface Implementations
PeerInfo:CrawlDriverConfig:CrawlDriver:2. Bitcoin Network Integration
--network bitcoin).CrawlDriver.3. Error Handling and Protocol Compliance
4.
btcsuite/btcd/wirePackage Integrationbtcsuite/btcd/wirefor Bitcoin protocol implementation.5. Basic Bitcoin Network Crawler Implementation