Modularization refactoring synchronization plan
Since initial feedback on #546 are somewhat positive it's probably a good time to think about how would we implement it. There's no way around the fact that such refactoring is going to be a large scope "stop-the-world" somewhat disruptive change. The best we can hope for is getting over it quickly, minimizing overhead and wasted effort.
Here is the plan (in order):
-
[ ] Get a consensus if something like #546 is what we want to pursue.
- [ ] The faster you raise concern, the faster we can address it or even consider different approaches
-
[ ] Shuffle the code around to look more like real one, without actually changing any real code yet:
- [ ] Amend #546 with "output finalization" api. (@dpc needs design help here)
- [ ] Prepare the desired backend module types.
- [ ] Move types into roughly right crates.
- [ ] Implement any desired macros to eliminate boilerplate.
-
[ ] Warn the community that it is going to be a stop-the world change, so they don't start any important large scope work until it's done.
-
[ ] Get a final agreement that all the APIs are looking OK.
-
[ ] Land as many in progress PRs as possible into
master, so the author's don't need to redo them. -
[ ] Stop-the-world and make a big change:
- No landing anything while this is going on. Rebasic things here is going to be messy, time consuming and make continuous review harder.
- Gang on the work. Create a video call and work on refactoring together, as people are available.
- Land commits frequently, to help people keep up with reviewing them, and make handing over work simple.
- Maintain an append-only commit history, so it's easy to verify what was done, and continue working by just looking at the new commits.
- Commits don't have to compile. They just have to continuously bring us into the desired destination.
- End result: all the code is refactored, everything compiles, all tests are passing, and we have a PR with a lot of commits. Eric squashes it and pushes to master.
-
[ ] Resume normal operation.
If possible, I think an incremental approach would be best. There is a lot of code that needs to be refactored and reviewed plus a high risk of introducing regressions. An estimate of 2-3 days could easily turn into 2-3 weeks.
We could break up the PRs into tasks like this, regardless of whether we want to stop-the-world or not:
- Create the framework for modularization of existing
enumtypes andFederationModule - Move types one-by-one in separate PRs, making appropriate changes on the federation / client side
- Move the consensus logic / modules themselves last
- Break the final dependencies of the "core" code on the modules
- Refactor boilerplate into macros to make the writing of new modules as easy as possible for new devs
Current we have:
core --> wallet,mint,ln
server --> core,wallet
client --> core
We need to get to:
wallet,mint,ln --> core
server --> core
client --> core
Also I'd like to finish up DKG before the FederationModule and configs get completely refactored since I've already spent a lot of time on it.
After talking with @dpc I'm more on board with just forking the repo and doing it all at once after we have the traits and crate organization well-defined...probably the easiest way overall, but we have to be ready to freeze the main repo for a couple weeks.
After talking with @dpc I'm more on board with just forking the repo and doing it all at once after we have the traits and crate organization well-defined...probably the easiest way overall, but we have to be ready to freeze the main repo for a couple weeks.
ACK, I think the current proposal would be to get the Demo out for Oct 1st and land all necessary PRs for that and then do a fork + freeze to get the modularization done. PRs that are done while the freeze is in place will need to either be based off the modularized fork or be rewritten after completion of the modularization (MVP). It seems like incremental rewriting of the whole system will not be feasible, as nearly everything will need to be moved or refactored in some way
Did I get that correct @dpc?
I think we can accomplish this in 3 large refactors as outlined here: https://github.com/jkitman/minimint/blob/modularization-architecture/docs/modular-architecture.md#refactoring-plan
This gives us some milestones to work with and the ability to merge to master if unfreezing the world becomes necessary.
Did I get that correct @dpc?
Yes, though ...
I think we can accomplish this in 3 large refactors as outlined here:
... we will give it a try before attempting a single big refactor.
I definitely want to have a framework for modularization landed, even before we start to refactor any code to use it, with crates in roughly destination positions. Then 2 and 3 from https://github.com/jkitman/minimint/blob/modularization-architecture/docs/modular-architecture.md#refactoring-plan seem doable, so instead of having one big world-stop event, we would have two smaller ones: one for server, one for the client.
After talking with Eric, the plan is as follows:
-
Clean up the sketch PR and make it landable. Possibly land.
-
Move forward with the thing and convert server-side code to use dyn trait from
core/server. -
Refactor existing client code to use trait (not dynamic yet, but to iron out the API).
-
Convert client-side code to use dyn trait (based on step 2).
Some quick and messy notes on what we chatted about:
- Questions about
ModuleKeybeing a self-assignedu16. - Move finalize to be a method on the module. Rename it
poll_pending_outputor something. - Preapre static trait(s) to be implemented by module authors, and then either
impl DynModuleTrait for T where T: StaticModuleTrait(if possible) or a macro to generate the code to do. - The server side module trait can be mostly taken as is and there's some confidence it will work. One idea there is to rely on database transactions and rollback when checking double spends etc.
- The client side module architecture/interfaces still a bit unclear, but possibly the current model there might work. The model there is to have modules mostly pure, and leave persisting data to the core client. Client would also subscribe to events (websockets) and drive modules to react to them.
- Simulate linear types / use session types to force persisting data in the database before triggering side effects.
Close this? @dpc @elsirion