Peer network upgrade

[igorsyl]

Given issues with the peer network such as #3753. Should we consider revamping the net stack to use another production-ready and robust p2p library such as:

https://docs.rs/libp2p/latest/libp2p/
https://github.com/libp2p/rust-libp2p

cc @obycode @Acaccia @jbencin

[kantai]

I would be a strong proponent of building a compatible network module using a p2p library (like libp2p).

The advantages to this are many, but the primary benefits are:

1. Freeing blockchain maintainer throughput. The blockchain networking stack works, but bugs in the stack are very difficult for contributors to diagnose and fix (and harder to test). It's also the part of the stack with the least shared knowledge among active contributors. It's certainly the part of the codebase I have the most trouble contributing to, and my guess is others agree. Employing a library for this allows maintainers to focus energy on other components which are necessarily more custom: the consensus data store, block selection, fork selection, the clarity VM, and special contracts.
2. Standardization of the stack. Using a networking library should lead to a stack that is more modular and extensible: a networking library will provide better documentation than our networking stack does, and it has a non-permeable interface. Our networking stack's boundaries between "networking code", "consensus code", and "clarity" are all quite fuzzy. Making this boundary sharp will make contributing to all of these components easier and safer. Finally, a more standard networking stack will make implementation in other languages much easier: implementing a simple stacks p2p client in TS or python should be easy if the interfaces with a portable library like libp2p are clear.

[jcnelson]

I agree with splitting out the networking stack into its own crate, but strongly disagree with using libp2p in particular (it has some pretty terrible design flaws and has rather anemic support in other languages besides Go), and somewhat disagree with using a 3rd party p2p library in general. The reasons for the latter are that (1) Stacks has somewhat bespoke requirements for block propagation and peer selection due to the fact that peers rely on the state of both Bitcoin's and Stacks' chain state to make peering and routing decisions, and (2) there just aren't that many well-designed alternatives today. Happy to discuss more in a call.

…

On Tue, Jun 20, 2023, 12:00 PM Aaron Blankstein ***@***.***> wrote: I would be a strong proponent of building a compatible network module using a p2p library (like libp2p). The advantages to this are many, but the primary benefits are: 1. *Freeing blockchain maintainer throughput*. Our networking stack works, but bugs in the stack are very difficult for contributors to diagnose and fix (and harder to test). It's also the part of the stack with the least shared knowledge among active contributors. It's certainly the part of the codebase I have the most trouble contributing to, and my guess is others agree. Employing a library for this allows maintainers to focus energy on other components which *are* necessarily more custom: the consensus data store, block selection, fork selection, the clarity VM, and special contracts. 2. *Standardization of the stack*. Using a networking library should lead to a stack that is more modular and extensible: a networking library will provide better documentation than our networking stack does, and it has a non-permeable interface. Our networking stack's boundaries between "networking code", "consensus code", and "clarity" are all quite fuzzy. Making this boundary sharp will make contributing to all of these components easier and safer. Finally, a more standard networking stack will make implementation in other languages much easier: implementing a simple stacks p2p client in TS or python should be easy if the interfaces with a portable library like libp2p are clear. — Reply to this email directly, view it on GitHub <#3761 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AADQJK34Q2CZK4PVSI7DXHDXMHCKVANCNFSM6AAAAAAZNF64RA> . You are receiving this because you are subscribed to this thread.Message ID: <stacks-network/stacks-blockchain/repo-discussions/3761/comments/6232486@ github.com>

[igorsyl]

Thanks for your thoughts! Could the first step towards this work be to define a clear network stack API and pull it into a new repository? Is stacks-net a good name?

[jcnelson]

On a scale of 1 to 10, where 10 is "drop everything and do this right now," where do you place the importance of figuring this out? Can it wait a couple weeks until I'm back?

…

On Tue, Jun 20, 2023, 6:42 PM Igor Sylvester ***@***.***> wrote: Thanks for your thoughts! Could the first step towards this work be to define a clear network stack API and pull it into a new repository? Is stacks-net a good name? — Reply to this email directly, view it on GitHub <#3761 (reply in thread)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AADQJKYS7HAAHL2YS3KUSZTXMIRL5ANCNFSM6AAAAAAZNF64RA> . You are receiving this because you commented.Message ID: <stacks-network/stacks-blockchain/repo-discussions/3761/comments/6235788@ github.com>

[fonty1]

Streamr Network could be an option for Stacks networking.

The SDK is currently in TypeScript however the Streamr Nodes have HTTP/MQTT/WS interfaces for any language.

It's based on WebRTC so peers can connect through difficult NATs, it has very low and predictable latency, it's decentralized, permissionless and so on.

Happy to answer any questions about the protocol. No tokens are needed.

Whitepaper

[jcnelson]

This discussion makes me sad. We're talking about replacing a very large subsystem (that 1-2 engineers are intimately familiar with) with a different very large subsystem that none of us are nearly as familiar with, but has known design deficiencies that the current system fixes. Furthermore, the impetus for this is an issue for which there is no PoC or smoking gun evidence that implicates the p2p network in the first place (and, neither is there any discussion about what the fix for that issue would be). It's like going to the doctor's office with a runny nose, and the doctor immediately saying "YOU NEED A LIVER TRANSPLANT!" without any sort of testing or consultation. And then other doctors walking through the hallway overhear this, poke their heads in, and say "NO YOU NEED A LUNG TRANSPLANT" or "YOU HAVE CANCER." If I were the patient, I would be running, not walking, to the nearest exit. What we ought to be doing is figuring out the root cause of the issue at hand, and having a discussion about how we can regularly test block propagation in large-scale deployments of the blockchain so we can catch these bugs earlier.

…

On Fri, Jun 30, 2023, 8:30 AM Matthew ***@***.***> wrote: Streamr Network <https://streamr.network/network> could be an option for Stacks networking. The SDK is currently in TypeScript however the Streamr Nodes have HTTP/MQTT/WS interfaces for any language. The main advantage over LibP2P is scalability beside low and predictable latency of the message transport. Happy to answer any questions about the protocol. No tokens are needed. — Reply to this email directly, view it on GitHub <#3761 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AADQJK5MKTBYKIEERSYPNHTXN3BIHANCNFSM6AAAAAAZNF64RA> . You are receiving this because you were mentioned.Message ID: <stacks-network/stacks-blockchain/repo-discussions/3761/comments/6325524@ github.com>

[jcnelson]

The impetus is something you mention above. The blockchain codebase has an important and large subsystem -- the networking stack -- that 1-2 people understand. And, if I'm included in that number, the networking stack realistically has only 1 person actually comfortable with working with it.

Actually, I was including @pavitthrap. But yes, I feel confident that there are at least 3 people available to debug networking issues (you may not feel comfortable working on it, but I'm confident that if push came to shove you would be effective at doing so). There are certainly more 3 people across the ecosystem who could also be effective; we just haven't had the pleasure of working with them yet.

Diagnosing issues: is a particular bad behavior seen in the network due to the p2p stack? Or is it due to the chainstate? The Clarity VM? If expertise is lacking in the p2p stack, that makes debugging all issues more difficult.

The difficulty of diagnosing an issue increases as the degree of familiarity decreases, so I'm not sure how this is specifically relevant to the p2p network? Would I be justified in outsourcing the chainstate DB and the Clarity VM if I didn't feel like I understood them well enough? Because, personally, I don't think I would. My impulse in that hypothetical situation would be to familiarize myself with those subsystems so I could get better at diagnosing issues over time. Whether or not the system was built in-house or by a 3rd party is irrelevant to my own need for knowledge, but keeping subsystem expertise in-house at least increases the chances that someone here will be able to effectively assist the diagnosis and help me improve my own understanding.

Implementation speed: the fact that networking code can realistically only be worked on by 1 person means that implementation is necessarily bottlenecked, and it constrains what engineers can work on what problems.

It's definitely not "only [be] worked on by 1 person" anymore. @pavitthrap has implemented a microblock anti-entropy system, and @Acaccia has recently merged a PR for a new RPC method. So, I think it's imminently possible for other people to work on. Now, could the system be factored to make it easier? Yes, 100%, but that's not the topic we're discussing.

Implementation verification: how many engineers even know whether a test for new functionality or changes is needed, and what that test should do?
Bug fixes: if there's an issue in the networking stack, how many people could diagnose it, develop and test a fix? Without potentially breaking other parts of the network stack?

I could replace "networking stack" with any other subsystem in the codebase and the statement would still be true. Why is the networking stack being singled out here?

Why is a given RPC request so slow? I regularly see log data from nodes that service client requests where 95th percentile RPC requests take > 10 seconds. Some kind of queueing is at work here -- the nodes own logs perceive these same requests as being processed in under a second. But figuring out where and why this queueing is occurring is basically impossible.

What if libp2p exhibited the same behavior? How would we be in a better place if no one here had as good of an understanding of libp2p as we do of the current p2p system? (Replace libp2p with a 3rd party black-box p2p stack).

In particular, I do not think it necessary to the stacks-blockchain to:
Maintain its own HTTP parsing library
Maintain its own HTTP server
Maintain its own network work thread

None of the problems we have encountered so far were in any of these code paths.

---

I want to make sure we're on the same page when discussing the "networking stack." To me, the networking stack has several layers -- some of which really cannot be outsourced. They layers in my mind, from lowest-level to highest-level, are:

1. The TCP socket (we use the mio crate for this)
2. The epoll system for opening, binding, and accepting sockets (we use mio for this as well).
3. The network message wire formats, implemented in src/net/codec.rs
4. The HTTP message parsing and decoding logic, implemented in src/net/http.rs and src/deps/httparse/*.rs.
5. The ProtocolFamily implementations for both the HTTP and p2p message types, in src/net/http.rs and src/net/codec.rs respectively. This provides a unified API for dealing with different categories of messages.
6. The NetworkConnection<P: ProtocolFamily> struct, implemented in src/net/connection.rs. This wraps a TCP socket with the following API
   1. An inbox/outbox API for sending and receiving whole messages in a buffered manner (the particulars of the messages live in the ProtocolFamily type implementation)
   2. A futures API for providing a thread a handle it can poll or block on in order to receive a reply for a message that had been previously sent (I strongly prefer this concurrency synchronization paradigm for blocking I/O over async/await, btw).
7. The ConversationP2P and ConversationHTTP structs, in src/net/chat.rs and src/net/rpc.rs respectively. These wrap a NetworkConnection with a particular protocol family (our p2p protocol or HTTP) and implement the requisite logic for establishing sessions and driving message receipt and transmission within that session from the main event loop. A ton of business logic happens in these two files, since they encode the logic to decide how to handle inbound messages based on the state of the chain and peer set.
8. The various state-machines, which drive the node's behavior.
   1. The HTTP state-machine, in src/net/server.rs. This simply wraps up the set of open ConversationHTTPs, feeds them messages and reads messages from them, and manages their lifecycles.
   2. The neighbor discovery state-machine, in src/net/neighbors.rs and src/net/db.rs. This crawls the peer network for new Stacks nodes and records session-level metadata to the frontier table in the PeerDB, and maintains set of open connections to a random subset of discovered nodes (the "neighbors").
   3. The inventory synchronization state-machine, in src/net/inv.rs. This queries block and reward cycle inventories from the node's neighbors.
   4. The block downloader state-machine, in src/net/download.rs and src/net/dns.rs. This uses state from the inventory state machine to determine which neighbors have which block and microblock streams available for download, and proceeds to fetch them.
   5. The pruner state-machine, in src/net/prune.rs. This audits the set of open neighbors to make sure that the node maintains a diverse set of healthy peers.
   6. The mempool sync state-machine, in src/net/p2p.rs. This queries other nodes' mempools for transactions that are not locally available, and fetches and stores them.
   7. The anti-entropy state-machine, in src/net/p2p.rs. This looks at block inventory state from the inventory state-machine, and pushes block and microblock data to neighbors that lack locally-available data. This works as a fallback mechanism to the downloader state-machine to help unhealthy nodes recover from missing blocks (this subsystem has saved our collective butts on at least one occasion, where a bug in the chainstate DB accidentally caused all nodes except for one NAT'ed node to delete the same valid microblock stream).
   8. The microblock sync state machine, that @pavitthrap built. This queries other nodes' latest unconfirmed microblock streams and fetches any missing microblocks, so the node (if it is a miner) can have more microblocks to confirm when they mine.
   9. The Atlas state-machine, which lives in src/net/atlas/*.rs. This queries other nodes for their transaction attachments for any subscribed smart contracts -- namely, BNS zone files -- and serve them to requesting nodes.
9. The PeerNetwork struct, which lives in src/net/p2p.rs. This drives all of the state machines and manages socket lifecycles.
10. The Relayer struct, in src/net/relay.rs. This consumes newly-arrived data from the PeerNetwork, stores it, and decides which state the PeerNetwork should forward to other nodes.

Layers 1-2 are already outsourced -- we use mio to do all the heavy lifting of establishing TCP connections and TCP server sockets.

Layer 3 cannot be outsourced, since we necessarily have our own message types as part of reasoning about how to interpret the bytes on the wire in a structured, correct manner.

Layers 4-6 might be outsourced one day, since they mainly deal with the nitty-gritty of creating a message-oriented session atop a TCP stream. However, I think there's a lot of value in keeping layer 6 in-house, because it provides synchronization primitives that do not require us to annotate each and every single function and function call with async/awaits (and moreover, we don't need to spawn a whole separate async/await execution engine, which really de-clutters gdb stack traces). If we do outsource these layers, it probably won't be to a libp2p -- it will probably be to some kind of stream adapter library.

BY FAR the most complex layers in the system are layers 7-10. They are the largest pieces of code in the networking system, and their specifications and implementations are highly dependent on the business needs of the Stacks blockchain. These cannot be outsourced -- libp2p does not implement them. Also, the bugs you have mentioned so far occur in these layers.

By giving you this layered overview of the networking stack, the point I hope you take away here is that most of the complexity of the networking stack lives in areas of the code that we're going to have to write ourselves, regardless of whether or not we find a drop-in p2p replacement. Replacing layers that libp2p (or whatever) could handle does not significantly reduce the cognitive burden of diagnosing network-related problems, since experience has shown that when problems arise, they're usually in the "higher" layers that are more closely-tied to the business logic. Moreover, should problems arise in a 3rd party library that implements layers 4-6 for us, they are still our problems to fix. It would therefore behoove us to ensure that we can tap as much domain expertise as possible for this subsystem (or any complex subsystem) from local sources, who are more than willing to drop everything and fix them if need be.

[kantai]

Would I be justified in outsourcing the chainstate DB and the Clarity VM if I didn't feel like I understood them well enough?

Maybe! If there were compelling places to apply a library in those modules, then yes definitely. After all, those modules use sqlite and rusqlite.

I could replace "networking stack" with any other subsystem in the codebase and the statement would still be true. Why is the networking stack being singled out here?

First, that maybe because it is the only module being discussed in this discussion thread. However, I'd also advocate using libraries or standardized tooling in other modules as well. In #3766 I advocate for applying similar logic to serialization (which impacts Clarity value serialization), and in #3772 I suggest using a standardized bytecode for Clarity execution. I am confident that I've been a pretty consistent proponent of standardization and use of libraries: it's why I feel like we should use an OpenAPI yaml file for our RPC documentation, prometheus for exposing node metrics, standardized logging libraries, etc.

Second, I think that the network stack implements several things where mature libraries do exactly what the codebase wants. I listed them above, but I'll repeat -- I believe that tokio can provide a TCP server and either warp or hyper can provide an HTTP server. I think that adding async and await decorations to the codebase would improve legibility in the places impacted (which is definitely not "everywhere", just in the places where we want to take advantage of asynchrony) because everywhere impacted is currently instead custom-implementing async/await.

The separate runtime is also, in my mind, an improvement over the current system (for the reasons mentioned above) where we essentially implement a similar system ourselves. The neon_node implementation could also likely benefit from async/await primitives--- many of the issues related to miners not waking up at appropriate times, receiving events in a timely fashion, spending too much time on some tasks but not others are all essentially scheduling problems, and async/await are just better primitives for managing those kinds of problems than what takes place in neon_node today.

In particular, I do not think it necessary to the stacks-blockchain to:
Maintain its own HTTP parsing library
Maintain its own HTTP server
Maintain its own network work thread

None of the problems we have encountered so far were in any of these code paths.

I'm not convinced that's true -- the relayer and peer network implemenations themselves have created issues (quick issues searches yield #3227 or #2552) and the interactions between them have caused other issues (various locking issues related to handling of atlas data). That the "relayer thread" is also responsible for invoking the miner is also problematic (see #3610). I don't really want to get into an accounting of how many issues there have been, but I don't think it's true to say that no problems have been encountered.

While it is true that many problems can be assigned to other parts of the system and that much of the complexity lies in things like network state machines, using tokio to manage the TCP server and hyper to manage the HTTP server would allow blockchain contributors to focus on the actual complex portions of the system rather than the details of custom-built async primitives, HTTP request parsers, handlers, etc.

What if libp2p exhibited the same behavior? How would we be in a better place if no one here had as good of an understanding of libp2p as we do of the current p2p system? (Replace libp2p with a 3rd party black-box p2p stack).

If the RPC system was using hyper or warp, we could drop in (feature-gated) tokio-metrics and collect real time metrics about idle times (https://github.com/tokio-rs/tokio-metrics#base-metrics) with a handful of lines of code. Node operators could report those statistics when opening issues.

By giving you this layered overview of the networking stack, the point I hope you take away here is that most of the complexity of the networking stack lives in areas of the code that we're going to have to write ourselves, regardless of whether or not we find a drop-in p2p replacement.

I mostly agree! But I think that everything that can be stripped away from the codebase should be. I do not believe the stacks-blockchain should be in the business of implementing an http server, request parser, or tcp server. I don't believe that it requires a custom async model. I think that maintaining, updating, and managing those items continuously saps resources away from the (still complex!) portions of the codebase that absolutely cannot be outsourced.

[jcnelson]

Second, I think that the network stack implements several things where mature libraries do exactly what the codebase wants

I think "mature" is being generous. "Widely-used" I'll grant, but "mature" to me means that great care has been taken by the developers to provide as high of an assurance as possible that the code works as intended.

Regarding maturity, one proxy measure of this is how much a given create uses gratuitous unsafe code. One of Rust's main selling points is zero-cost memory safety. I'm very, very skeptical of the correctness claims of Rust crates -- especially ones that have to deal with untrusted input! -- that can't be bothered to take advantage of this feature, especially given that humans are so very predictably bad at implementing memory safety by hand. I can understand the need to use unsafe to deal with FFI or OS-specific things that necessarily operate outside of Rust's safety guarantees, but by no means does it excuse the rest.

Let's look at hyper:

$ git clone https://github.com/hyperium/hyper
$ fgrep 'unsafe' -r hyper/src | wc -l
62

And tokio:

$ git clone https://github.com/tokio-rs/tokio
$ fgrep 'unsafe' -r tokio/tokio/ tokio/tokio-macros tokio/tokio-stream/ tokio/tokio-util/ | wc -l
805

These are both big, honking WATs to me. Like, why would you use a language that offers strong memory safety guarantees and then completely forego them?! Furthermore, why would you do so on libraries that handle untrusted input from the public Internet?!

To its credit, warp does not use unsafe code. But, it depends on httparse and hyper, which definitely do. Moreover, warp, hyper, httparse, and several other dependencies are all written by the same author; without a deeper audit of these packages, I'm currently inclined to believe that his apparent cavalier attitude towards memory safety in httparse is likely also present to a disturbing degree in hyper. When we vendored httparse, we removed all of his unsafe code.

For comparison, the Stacks blockchain itself has 20 instances of unsafe code. Of those, 12 of them have to do with dealing with POSIX signals and signal handlers. Seven of them are present in the vendored rust-bitcoin library or code derived from it, and can be removed (they all wrap std::mem::transmute()). The remaining instance is to wrap libc::isatty() in the logging system. There is no unsafe code under src/net at all.

many of the issues related to miners not waking up at appropriate times, receiving events in a timely fashion, spending too much time on some tasks but not others are all essentially scheduling problems, and async/await are just better primitives for managing those kinds of problems than what takes place in neon_node today.

I think that's a pretty extraordinary claim. What makes you so sure that these concerns suddenly go away by using async/await? The miner code has to do the same I/O-bound work either way, atop the same lock-contended resources. Like, if the miner creates an orphan because it was excessively blocked on an .await due to the sqlite DB being locked by the chains coordinator thread (a common occurrence until the introduction of the preemptable miner that runs in a separate thread from the relayer), then async/await isn't actually helping us at all! It's just obscuring the problem by adding an extra abstraction layer that we have to debug.

IMHO, the question of whether or not async/await is a "better" paradigm for managing program concurrency is more of a question of taste than substance. I personally can't stand it. I think the fact that I have to pepper the whole codebase with async/await keywords and have to suddenly be very aware of my dependencies' adherence to async/await is a huge turn-off. I also think the supposed "benefit" that async/await "helps" by adding type annotations to program effects is also bunk, because the same can be said about a future type (which the system currently uses) or a channel type; neither of these require me to restructure all of my codebase to leverage them.

While it is true that many problems can be assigned to other parts of the system and that much of the complexity lies in things like network state machines, using tokio to manage the TCP server and hyper to manage the HTTP server would allow blockchain contributors to focus on the actual complex portions of the system rather than the details of custom-built async primitives, HTTP request parsers, handlers, etc.

These really aren't the maintenance burdens you seem to think they are.

• The "custom-built async primitives" are thin wrappers around Rust's mpsc primitives, and the last time they were touched was in 2020. They haven't needed subsequent updates.

• The bulk of the HTTP request parser and codec, which validates, decodes, and parses a byte stream into an HTTP message, also hasn't been touched since 2020 because it works without issue. I think the only prod-discovered bug was that the system's limit on the size of all the HTTP headers was too small to accommodate all the stuff that Cloudflare puts into its proxied requests.

• We only add new code for defining new routes and how to encode/decode the HTTP query and body to/from its byte representation. Most of that heavy-lifting is already handled by regex, json, serde, BoundReader, and so on. Moreover, using hyper or warp does not free us from that responsibility!

• You're going to need to write an HTTP request handler for a new route with or without hyper or warp, so that one's a moot point.

So, I don't think they're "sapping" anything. No one's working on them, because they seem to work already.

If the RPC system was using hyper or warp, we could drop in (feature-gated) tokio-metrics and collect real time metrics about idle times (https://github.com/tokio-rs/tokio-metrics#base-metrics) with a handful of lines of code. Node operators could report those statistics when opening issues.

And what happens when there's a bug in their monitoring code? ;)

The point I was trying to make there is that we are ultimately responsible for what the Stacks blockchain does, regardless of who wrote the code. A flaw in, say, warp that leads to a DoS attack does not somehow free us from having to drop everything and ship a new version of the Stacks node with an updated warp, nor does it free us from going so far as to patch warp ourselves in the event that there is no upstream fix. I'd like for us to be in a position where we can both quickly add new features to the networking stack and quickly react to problems discovered in the wild. Outsourcing large swaths of the p2p system achieves the former at the expense of the latter. A better factoring of the current system, plus perhaps a guide on how the subsystem is organized, allows us to achieve both.

[kantai]

Regarding maturity, one proxy measure of this is how much a given create uses gratuitous unsafe code.

I do not believe that this a measure of code quality or maturity. These projects all have rigorous security policies, taking full advantage of RustSec, CVE databases, and Github's security reporting. They're widely used and depended on by multiple other blockchain projects. Polkadot, near, solana, and diem all use tokio and hyper--- nearly all rust blockchains I've examined do. I do not believe that the stacks-blockchain has wildly different trust requirements for dependencies than other blockchains, and I am very skeptical of the idea that we are getting any safety benefit at all from this kind of policy.

These really aren't the maintenance burdens you seem to think they are.

As someone who has had to add RPC endpoints to the HTTP server, I think it is a huge pain and expenditure of energy that I have to crawl across multiple files, modifying an enum, add multiple boilerplate cases to switch statements, make sure that the parser handler is doing the correct thing, and then add a boilerplate invocation of my handler which itself needs to be careful to send any data (error or otherwise) to a response file descriptor -- mixing application logic and transport layer logic throughout. Error types "HttpResponseType::NotFound" are the same type as non-error responses, meaning that any error response requires the same modification of enums, adding of boilerplate cases, etc. as a new response type. Error handling is not enforced through the type system, meaning that generic error responses from the RPC are the norm, rather than an exception (including 500 errors returned from perfectly functioning nodes), all of which lead to a hostile end user interface. If I wanted to stream a new type of data, the burden is greater.

No one's working on them, because they seem to work already.

I disagree -- interactions between the "peer thread" and the "relayer thread" have led to the problems I mentioned and linked in my prior post. Interactions between these threads and the "neon_node" module also have a history of problems, requiring multiple refactors, each of which solved some problems but created others. Long standing issues exist with these threads and they're long-standing precisely because its unclear what is even going on with any given issue. Just look at some network-related issues:

#1830 -- this one was almost certainly resolved, but the issue was never diagnosed.

#3671 -- very difficult to diagnose this kind of report.

#3145 -- this issue was addressed by changing the node's key (allowing it to circumvent blocklists), but the issue wasn't resolved

#2762 -- HTTP handling causes nodes to be marked as unresponsive. Discussions about resolving this propose implementing a RPC handler thread pool. I'd prefer to use an existing HTTP server and task manager instead.

I'd suggest that no one is working on improving the RPC handler or the P2P thread because either no one has time to do so or because the behaviors being reported are sufficiently difficult to diagnose that there is no identifiable bug.

And what happens when there's a bug in their monitoring code? ;)

I prefer a hypothetical bug in a monitoring library to an undiagnosable issue.

The point I was trying to make there is that we are ultimately responsible for what the Stacks blockchain does, regardless of who wrote the code.

I believe that this is as true for the stacks-blockchain as it is for any other blockchain.

[jcnelson]

I do not believe that this a measure of code quality or maturity. These projects all have rigorous security policies, taking full advantage of RustSec, CVE databases, and Github's security reporting.

None of these are preventative measures for avoiding memory safety violations.

They're widely used and depended on by multiple other blockchain projects. Polkadot, near, solana, and diem all use tokio and hyper--- nearly all rust blockchains I've examined do.

Since when does widespread use imply memory safety? I could use the same reasoning to argue switching to OpenSSL on the eve of the Heartbleed disclosure because at the time, it was the world's preeminent highly-reliable and mature TLS project.

I do not believe that the stacks-blockchain has wildly different trust requirements for dependencies than other blockchains, and I am very skeptical of the idea that we are getting any safety benefit at all from this kind of policy.

Given the choice between two equivalent crates, wouldn't you want the one that has a greater degree of machine-checked assurance of memory safety?

As someone who has had to add RPC endpoints to the HTTP server, I think it is a huge pain and expenditure of energy ...

I agree that we could reduce the boilerplate code for creating new RPC endpoints. To speak to your specific concerns:

• "crawl across multiple files"

I very much doubt that an HTTP server based on hyper or warp will be contained in a reasonable-length single Rust file. Look at the size of all the business logic in rpc.rs for example. As it is today, I'd like to factor each RPC endpoint implementation into its own file.

• "modifying an enum"

This comes with the territory of representing each kind of HTTP request and response as a distinct type. We could of course switch to using traits and impls if you think that would make things easier, but at the end of the day, we will have distinct types for these regardless of what the transport layer does. Doing so would address your following point:

• "add multiple boilerplate cases to switch statements"

If we switch to traits and impls, then at least we don't have to do this (which I agree is annoying).

• "add a boilerplate invocation of my handler which itself needs to be careful to send any data (error or otherwise) to a response file descriptor -- mixing application logic and transport layer logic throughout."

We could require that an RPC method instead returns an HTTPResponseType that implements a trait which makes it amenable to consumption by the transport layer. Then, it's not the RPC implementer's problem in most cases, with one exception (streaming). This dovetails with my desire to ultimately put each RPC method into its own file -- each RPC method would define a struct for its HTTP request and response types, and implement the "request" and "response" traits for them in order to bolt on the boilerplate. The trait definition itself would take care of most of the heavy lifting.

• "Error handling is not enforced through the type system, meaning that generic error responses from the RPC are the norm, rather than an exception (including 500 errors returned from perfectly functioning nodes), all of which lead to a hostile end user interface."

Nothing stops us from adding more specific HTTP error messages.

• "If I wanted to stream a new type of data, the burden is greater."

This wouldn't be different with async/await. Even if we did use async/await, we would still need to implement the equivalent of the StreamCursor system in order to multiplex access to on-disk data in a fair way. For example, async-std does not know how to fill the socket buffer of a client that wants to receive mempool transactions. We would still need to implement the streaming logic that pulls mempool rows out of the database in an incremental fashion so they can be written into the socket buffer one-by-one until the buffer is full (and, we'd still be on the hook for implementing some internal buffering bookkeeping in case only part of a mempool row can be sent).

All of these points would be great refactoring issues, btw. Also, I'd wager that addressing all of these would be less painful than replacing large swaths of the p2p network.

#1830 -- this one was almost certainly resolved, but the issue was never diagnosed.
#3671 -- very difficult to diagnose this kind of report.

Would these be any better if your hypothetical 3rd party library inexplicably degraded performance?

#3145 -- this issue was addressed by changing the node's key (allowing it to circumvent blocklists), but the issue wasn't resolved

This issue likely originates in layer 7 (the ban logic), so it'd still be our problem either way.

#2762 -- HTTP handling causes nodes to be marked as unresponsive. Discussions about resolving this propose implementing a RPC handler thread pool. I'd prefer to use an existing HTTP server and task manager instead.

What do you think happens inside these systems? POSIX I/O isn't asynchronous; these systems all use thread pools under the hood to implement concurrent I/O dispatches. And, in the case of lock contention on our underlying sqlite databases, all of these I/O requests are serialized anyway.

The big lift here isn't implementing a threadpool, be it 3rd party or in-house (also, threadpools aren't exactly rocket science in Rust -- this one manages to exist in under 800 lines of code, including comments). The big lift here is in removing the lock contention so concurrent I/O is even possible.

---

Anyway, I don't see much of a point of continuing this thread. Regardless of my feelings on the matter, I'd rather just enhance and refactor the p2p system to make it easier / better to use, since that wouldn't take as much developer time to do and could be done incrementally.

[jcnelson]

I think this is a discussion of specialization @obycode

(Sorry for not replying in the thread; that's not really an option for me right now) The p2p network is a core competency of a blockchain that should not be outsourced. Business requirements like block size, block latency, and block production all have a profound impact on how the system needs to store and propagate blocks and transactions. For example: * blockchains with large-ish blocks should use TCP instead of UDP for transmitting data, lest the system accidentally reinvent a shitty TCP on top of UDP to deal with congestion collapse and multi-framed messages * blockchains that support block production from regular users should permit node operation behind NATs in order to support users running nodes at home. This has a large impact on peer discovery and next-hop message forwarding -- for example, a peer needs to maintain stateful connections to NAT'ed peers in order to keep the translation table entries alive (and thus keep the NAT'ed peers connected) * blockchains with open-membership block production should employ a routing strategy that maximizes the number of nodes who can be reached by the next hop * blockchains with low latency requirements ought to employ a "happy path" propagation strategy on top of message flooding that minimizes the time it takes for block producers to receive the message. For example, block producers could self-organize into a minimum spanning tree for optimistic fast replication and fall back to flooding on timeouts. * blockchains that run across the public untrusted internet should employ a peer discovery strategy that resists node eclipsing, including via BPG prefix hijacking. I can think of more, but you get the idea: the common theme of all of these design points is that they impact the inner design and implementation of the p2p subsystem.

…

On Fri, Jun 30, 2023, 9:11 AM Brice Dobry ***@***.***> wrote: I think this is a discussion of specialization. A better metaphor is that Alice is a baker with the goal of making the world's best bread, but she also builds her own delivery vehicles. These vehicles seem to be having some issues, and she's not quite sure what's wrong, so Bob suggests buying a delivery truck from Acme Truck Co. to replace her custom built delivery vehicles so that she can focus on the bread. So the question that needs to be answered is, is the p2p networking code a vital component of making our bread, or is it something that can be outsourced so that our focus can be on other things. I don't have a clear answer to that, but I just wanted to explain better where I think this discussion is coming from. — Reply to this email directly, view it on GitHub <#3761 (reply in thread)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AADQJK6QZJF2HJWE4UPA44LXN3GBRANCNFSM6AAAAAAZNF64RA> . You are receiving this because you were mentioned.Message ID: <stacks-network/stacks-blockchain/repo-discussions/3761/comments/6325846@ github.com>

[igorsyl]

These are all important considerations and optimizations. However, would Ethereum's network stack have similar requirements to those you describe? What is the delta in network stack requirements between Ethereum and Stacks? Could we use the network stacks from https://github.com/paradigmxyz/reth or perhaps solana validators?

[jcnelson]

s/BPG/BGP

…

On Fri, Jun 30, 2023, 9:33 AM Jude Nelson ***@***.***> wrote: > I think this is a discussion of specialization @obycode (Sorry for not replying in the thread; that's not really an option for me right now) The p2p network is a core competency of a blockchain that should not be outsourced. Business requirements like block size, block latency, and block production all have a profound impact on how the system needs to store and propagate blocks and transactions. For example: * blockchains with large-ish blocks should use TCP instead of UDP for transmitting data, lest the system accidentally reinvent a shitty TCP on top of UDP to deal with congestion collapse and multi-framed messages * blockchains that support block production from regular users should permit node operation behind NATs in order to support users running nodes at home. This has a large impact on peer discovery and next-hop message forwarding -- for example, a peer needs to maintain stateful connections to NAT'ed peers in order to keep the translation table entries alive (and thus keep the NAT'ed peers connected) * blockchains with open-membership block production should employ a routing strategy that maximizes the number of nodes who can be reached by the next hop * blockchains with low latency requirements ought to employ a "happy path" propagation strategy on top of message flooding that minimizes the time it takes for block producers to receive the message. For example, block producers could self-organize into a minimum spanning tree for optimistic fast replication and fall back to flooding on timeouts. * blockchains that run across the public untrusted internet should employ a peer discovery strategy that resists node eclipsing, including via BPG prefix hijacking. I can think of more, but you get the idea: the common theme of all of these design points is that they impact the inner design and implementation of the p2p subsystem. On Fri, Jun 30, 2023, 9:11 AM Brice Dobry ***@***.***> wrote: > I think this is a discussion of specialization. A better metaphor is that > Alice is a baker with the goal of making the world's best bread, but she > also builds her own delivery vehicles. These vehicles seem to be having > some issues, and she's not quite sure what's wrong, so Bob suggests buying > a delivery truck from Acme Truck Co. to replace her custom built delivery > vehicles so that she can focus on the bread. > > So the question that needs to be answered is, is the p2p networking code > a vital component of making our bread, or is it something that can be > outsourced so that our focus can be on other things. > > I don't have a clear answer to that, but I just wanted to explain better > where I think this discussion is coming from. > > — > Reply to this email directly, view it on GitHub > <#3761 (reply in thread)>, > or unsubscribe > <https://github.com/notifications/unsubscribe-auth/AADQJK6QZJF2HJWE4UPA44LXN3GBRANCNFSM6AAAAAAZNF64RA> > . > You are receiving this because you were mentioned.Message ID: > <stacks-network/stacks-blockchain/repo-discussions/3761/comments/6325846@ > github.com> >

[jcnelson]

would Ethereum's network stack have similar requirements to those you

describe? @igorsyl No, they are pretty different. And not even solely due to business requirements; Ethereum's p2p system is just _sloppy_ in places. For example, up until 2018 it was trivial to eclipse Ethereum nodes [1]. As another example, Ethereum's RLP wire format makes it hard to build a parallelizable codec. Furthermore, Solana and Ethereum 2.0 are PoS networks, which place a different set of p2p constraints on their respective networks. For example, they prioritize all-to-all connectivity for block producers by staked weight, and they don't need to care about forks. We need to prioritize all-to-all connectivity for _everyone_ (since anyone can mine, even from behind a NAT), and we do care about forks (so we need the p2p network to help identify nodes on forks that are of interest to us). There are more subtleties, but I'm on mobile and don't have time to list them here. Point is, trying to drop-in replace our p2p stack with someone else's is probably not going to happen. It would be painful in many hard-to-anticipate ways, including dealing with mismatch between our chain's business needs and theirs. [1] https://eprint.iacr.org/2018/236.pdf [2]

…

On Fri, Jun 30, 2023, 1:40 PM Igor Sylvester ***@***.***> wrote: These are all important considerations and optimizations. However, would Ethereum's network stack have similar requirements to those you describe? What is the delta in network stack requirements between Ethereum and Stacks? Could we use the network stacks from https://github.com/paradigmxyz/reth? — Reply to this email directly, view it on GitHub <#3761 (reply in thread)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AADQJKZQJP34UEWUFJQEIT3XN4FRPANCNFSM6AAAAAAZNF64RA> . You are receiving this because you were mentioned.Message ID: <stacks-network/stacks-blockchain/repo-discussions/3761/comments/6328128@ github.com>

[diwakergupta]

@jcnelson this whole discussion is starting to feel a lot like NIH.

We should be very skeptical that we can build, for instance, a correct and performant HTTP parser, server etc better than projects like hyper, warp etc.

Likewise, I understand your stance on unsafe code and dependencies in general and am sympathetic to the idea that dependencies should be pulled in with caution. However, using that as an almost blanket ban on dependencies - many of which are arguably much more battle tested, better documented, easier to use than the current codebase -- is a real impediment not just to developer productivity, but also makes for an unfriendly code base for onboarding new developers.

Finally, if there are implicit rules or constraints on the codebase ("thou shalt not pull in any dependency with unsafe code" or "thou shalt not use tokio" or whatever), let's at least agree upon a process to arrive at those rules and make them explicit. You are just one of the many contributors and granted your opinions have an outsized influence, it might nevertheless be a minority viewpoint. How do you want to reconcile the input of other core contributors?

[friedger]

Point is, trying to drop-in replace our p2p stack with someone else's is
probably not going to happen. It would be painful in many
hard-to-anticipate ways, including dealing with mismatch between our
chain's business needs and theirs.

Can we estimate the effort to replace the current p2p implementation? I feel supportive of Jude's concerns that it is not that easy as it might seem.