Fix/p2p discovery - #144
Open
SIDDHANTCOOKIE wants to merge 2 commits into
Open
Conversation
Swarm.listen() waits on a nursery that only exists while the swarm runs as a background service, so a node calling host.get_network().listen() directly never finished starting up: no multiaddr printed, no inbound connections possible. Listening now goes through host.run(). The dialing side had the same problem in reverse — it borrowed a nursery attribute the swarm doesn't expose, so a peer that dialed out never registered the connection or read from it. Both now use the nursery this module already opens for itself. Two mining bugs made a fresh chain effectively unusable: mining refused to produce a block when the mempool was empty, but the mining reward is the only source of new coins, so a chain that had never received one could never mine its first coin either. And two blocks minted within the same millisecond were rejected, since consensus requires strictly increasing timestamps. Both are fixed at the source: an empty block still carries the reward and proof-of-work, and the timestamp is clamped to be newer than the parent's instead of just read off the clock. A node that accepted a transaction or block only kept it — it never told any other peer, so a network more than one hop wide couldn't stay in sync. Accepted content is now relayed onward, excluding whoever sent it, with the existing dedup set stopping the echo from circulating. That set was unbounded, and now that it's load-bearing for stopping gossip loops rather than just avoiding duplicate work, its size is capped with an LRU eviction. Smaller fixes bundled in because they're cheap and adjacent: `peers` now lists the peer IDs it's connected to, not just a count; the JSON-RPC server's bind address is a flag instead of hardcoded to loopack; two unused constants (TRUSTED_PEERS, LOCALHOST_PEERS) are removed; and the README's --connect example, which predates the multiaddr-based CLI, is corrected.
Stacked on fix/p2p-core. That branch makes P2P work at all; this makes it
work across networks and survive a restart.
A node was only dialable if someone had arranged for its address to be
reachable, which in practice meant running bore or ngrok alongside it. Three
mechanisms replace that, in increasing order of last resort: mDNS finds
peers on the same LAN with nothing typed at all; --upnp asks the router for
a port mapping; --relay/--relay-addr route through another MiniChain node
when neither works, via libp2p's circuit relay v2. --bootstrap dials a list
of peers on startup — discovery rather than reachability, but the same path.
That solved reaching a peer once. It did not solve staying reachable: the
node's own identity was regenerated every restart, which invalidated any
address a peer had saved for it, and it forgot every peer it had ever
connected to the moment it stopped. Three more pieces close that:
identity.py persists the libp2p keypair to <datadir>/nodekey, so the
peer ID a node presents is the same across restarts.
known_peers a table beside the existing banned_peers one
(persistence.py) (remember_peer / get_known_peers). A node dials it on
startup alongside --bootstrap, and redials with backoff
when a stream it dialed itself drops.
getaddr/addr after a handshake, a node asks its peer for more peers and
(main.py) remembers what it's told, so the network stays reachable
after whoever it was originally introduced to goes away.
Addresses are stored, never relayed onward, so this can't
become an amplification vector. Only the dialing side of a
connection ever has a real address for the peer to share —
an inbound connection reveals nothing but an ephemeral
remote port — which is the same asymmetry Bitcoin has via
service flags.
--announce lets a node advertise a different address than the one it binds
to, for when the bind address (0.0.0.0 under --upnp, a NAT'd LAN IP) isn't
itself dialable. --fund now defaults to 0: it credits a wallet outside of
block validation, so a nonzero value diverges a node's state from every
peer's the moment it mines, and the previous default of 100 broke multi-node
operation out of the box.
Verified live: two nodes reach each other only through a relay, with no
direct route ever used; a node's peer ID and its ability to redial its last
peer both survive a restart, even for a node that was only ever reachable
through that relay; a third node dialing only one existing peer discovers
and connects to the other via addr gossip alone.
Contributor
|
Warning Review limit reachedNext included review available in 58 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (10)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Zahnentferner
approved these changes
Aug 31, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Addressed Issues:
Adds mDNS, UPnP, and circuit relay (via --relay/--relay-addr) so a node is reachable without a tunnel, plus --bootstrap for startup discovery. On top of that: a persisted node identity (minichain/identity.py) so peer ID survives restarts, a peer book so a node redials who it knew before, and getaddr/addr peer exchange so the network outlives whoever introduced you to it. --announce fixes the advertised address under NAT; --fund now defaults to 0 since a nonzero value silently broke consensus between nodes.
Test plan: 89/89 pytest passing (new identity + peer-book tests). Live: relay-only connection with no direct route, identity + redial surviving a restart, zero-flag node rejoining from its peer book, peer discovered via addr gossip alone, --announce override confirmed.
Fixes #(TODO:issue number)
Screenshots/Recordings:
TODO: If applicable, add screenshots or recordings that demonstrate the interface before and after the changes.
Additional Notes:
AI Usage Disclosure:
We encourage contributors to use AI tools responsibly when creating Pull Requests. While AI can be a valuable aid, it is essential to ensure that your contributions meet the task requirements, build successfully, include relevant tests, and pass all linters. Submissions that do not meet these standards may be closed without warning to maintain the quality and integrity of the project. Please take the time to understand the changes you are proposing and their impact. AI slop is strongly discouraged and may lead to banning and blocking. Do not spam our repos with AI slop.
Check one of the checkboxes below:
I have used the following AI models and tools: TODO
Checklist