The Long Road to Refactoring

[average-gary]

From @plebhash (providing here for context later so you don't have to dig through GitHub

Refactoring is going to be a long road... we need to make sure our efforts are properly planned and executed, because otherwise we might end up creating more technical debt while the goal is to mitigate it.

The efforts are being structured into differet stages, where protocols (aka low-level libs) is the first stage.

The strategy for this stage was also broken down into sequential steps.

The first step consists of writing Rust Docs for each crate. We have #845, which is the tracker with multiple subissues tracking the progress on this effort.

For each protocols crate, the inputs of this task are:
Github Discussions are leveraged to ask Q&A and gather conceptual insight for this specific crate (e.g.: #858 on framing_sv2).
Issue tracking Rust Docs for this specific crate.

For each protocols crate, the outputs of this task are:
PRs adding comments for Rust Docs (at least one, perhaps more) (e.g.: #848 for framing_sv2)
Github issue outlining technical debt (e.g.: #903 for framing_sv2)
refactoring PRs (at least one, perhaps more) (e.g.: #1033 + #1049 + #976 + #982 for framing_sv2)

[pavlenex]

To be clear this effort is led by @rrybarczyk and @plebhash others shouldn't jump into this, until there's structure and more clarity.

[rrybarczyk]

Do we have a discussion regarding the protocols and roles refactor? I know we have issue #845 that we have been working off of as the kick off of the refactor effort. We also have #854 which generally just lets us know there are a lot of todos in the protocols.

@plebhash, if we do not have an official Discussion for this refactor effort, this can be it. Let me know and I will comment back with our full plan so we can all get on the same page here as this touches so many issues and a lot of code.

[plebhash]

that sounds good!

recently I started #1071 which is somewhat adjacent to this topic, but there's not so much content there yet, so we're still early we should probably migrate everything here

basically there's #1071 (comment) where I propose a basic layout for a repo split

and also #1071 (comment) where @Shourya742 shares some good resource for overall refactoring practices

[plebhash]

we should also consider doing some edits on the original Discussion description by @marathon-gary

right now the description scope is limited to protocols (low-level) refactor only, but if we're aiming to make this Discussion about both low-level plus application level, ideally we need to convey that clearly, while still being clear with regards to the context where each point of discussion applies

otherwise, we could risk blurring the lines between what's low-level vs application level refactor during the discussions, with potentially negative consequences on how tasks are structured

[average-gary]

we should also consider doing some edits on the original Discussion description by @marathon-gary

If this is the ultimate Discussion, then this makes a lot of sense.

[plebhash]

migrating from #1071 (comment)

this is a proposal for a potential new repository structure, along with some high level ideas of how things could be structured in terms of crates, bin/lib, and modules:

this would not affect so much how low-level libs (protocols) are currently structured, but it does imply some deeper changes into how the application layer is modularized (see #702 for more details)

[average-gary]

When you were researching how we could re-organize the crates/libs (that nice graphic I see), did you collaborate with other Rust projects in the space? Maybe a call with some of the B/LDK or Fedimint or ?? might make sense to just get perspective on this layout.

[plebhash]

For this proposed repo split, I took some loose inspiration from LDK, mainly because there's some parallels with SRI in the sense that we take different building blocks to create "node"-like executables (which we call "roles" under the current terminology).

On this context, given that BDK is for building wallets, so there's not so many conceptual parallels so I don't think there's a lot of inspiration to get from it.

And Fedimint follows a monorepo structure, which is what we already have in place with the SRI monorepo, and I hope we eventually move away from this.

---

So elaborating a bit more on the LDK inspiration for the repos:

stratum would contain what is currently under protocols, which consist of the "low-level" libraries. It would be analogous to the multiple lightning-* crates on the rust-lightning repo.

Then, stratum-node would be analogous to:

• the lightning crate of the rust-lightning repo, in the sense that it is a place where the low-level crates are composed together to provide "application" level primitives.
• ldk-node, in the sense that it provides a ready-to-go node implementation stratumd (plus the CPU miners, which make sense to bundle together here if we're making this repo a place to put the binaries to be used out-of-the-box).

---

One could argue: "why not both stratum + stratum-node in the same repo?

All crates under core/low-level/protocols are already going into production on the ecosystem (mainly by @Fi3 on Demand Pool), so this crate layout is somewhat ossified, and I would object against changing that.

Having "core" vs "application" logic living on a separate vs same repository is open to debate. I like the idea of having a clear delineation between "core" logic and "application" logic.

But if people have different ideas I'm also happy to edit the image above to update the proposal.

[average-gary]

Having "core" vs "application" logic living on a separate vs same repository is open to debate. I like the idea of having a clear delineation between "core" logic and "application" logic, so that's why I decided to slightly diverge from LDK with this regard on my proposal.

No strong opinions here but would having the binary reference implementation in the same repo make it easier to test and/or version releases?

[Fi3]

I think that after a certain "mature" point (eg after the ongiong refector) make more sense to have libraries in their own repo and consume them via crates.io as every other implementation is supposed to do. I agree that when we are in a prototyping phase is very handy having everything in the same crate.But in a later stage is more important having smaller and easy to handle/maintains repos.

[plebhash]

migrating from #1071 (comment)

originally proposed by @Shourya742 :

A few design patterns that can be considered during refactoring: https://refactoring.guru/design-patterns/rust

[plebhash]

I still haven't read this in much detail but I believe it could be a good reference when we start considering deeper refactors for the application layer (e.g.: #702)

I wouldn't necessarily try to bring those patterns too much into the low-level refactoring of #845 though, because that could imply more drastic changes and we don't really want to go that deep on the low-level layer

[rrybarczyk]

Here is a proposed outline for how to approach this large undertaking.

This is intended to kick off the plan of action conversation/feedback session and should not be taken as the only way to proceed. Also, note that some of these items may overlap (like the implementation of the protocols and roles architecture changes), and this outline is soley intended to give loose structure/starting point that will likely change when we begin to implement.

Setup

protocols

1. Rust Docs + refactor: protocols crates #845 Rust doc comments on protocols. This will get us familiar with the protocols crates and give an opportunity to note any places where refactoring may be needed.
2. Compile a list (either in a new tracker issue, discussion, or shared doc) that addresses the found areas in protocols that should be refactored. This list should be categorized into the following:

• Logic to refactor immediately, before any protocols API design is made (see @plebhash's diagram).
• Logic to refactor after the proposed protocols API design changes are finalized.

3. Come to consensus on finalizing the diagram/new protocols API design.

Breaking changes introduced in protocols should be limited to how APIs are structured. The relationship between the crates and how they are working together as modular pieces, is somewhat ossified and should only be changed for a critical reason. Otherwise we risk causing problems for current users.

roles

1. Compile a list that addresses the found areas in roles that should be refactored. This list should be categorized into the following:

• Logic to refactor immediately, before any major API design changes are implemented in protocols AND roles architecture is made.
• Logic to refactor after the protocols API design changes are implemented.
• Logic to refactor after the proposed roles architecture changes are finalized and implemented.

2. Come to consensus on finalizing the diagram/new roles architecture.

Implementation

1. Implement the immediate protocols refactor pre-API design changes (likely will be mainly addressing todos and panics).
2. Implement the immediate roles refactor and patches (pre-architecture changes).
3. Implement the protocols API design changes.
4. Implement the roles architecture changes.

[rrybarczyk]

This comment is designed to capture the roles issues that have been reported but should not be worked on until the protocols refactor effort is complete and the roles architecture changes are set.

[GitGab19]

#427 - pool & jdc

[GitGab19]

#951 - jdc

[GitGab19]

#1082 - jdc & pool

[GitGab19]

#778 - error handling in roles

[plebhash]

#997