bitcoin-dev-project / sim-ln

Payment activity generator for the lightning network
MIT License
63 stars 28 forks source link

refactor: Unify Dispatch of Random and Defined Activity #160

Closed carlaKC closed 10 months ago

carlaKC commented 1 year ago

This PR unifies dispatch of random and defined activities. I think that this is worthwhile doing:

  1. As a pathway to allowing users to specify both random and defined activity in future changes
  2. To reduce code duplication (functionality is similar in produce_events and produce_random_events)
  3. Possibly provide a different impl of things like #153 (though the existing one is quite simple so this is a weak motivation)

Opening up in draft for an early look (still needs some cleanup). This is my first foray into trait objects, so this PR only exists by the good graces of crust of rust / suggestions for more canonical rust ways of doing this are very welcome!

okjodom commented 1 year ago

Concept ack ✅ And yes the Rust looks crusty! Will take a closer look and test

sr-gi commented 1 year ago

I'll be reviewing the PR this week, sorry for taking so long

carlaKC commented 10 months ago

Ready for another review with all the proposed name changed, apologies for the long turnaround!

@enigbe this has quite a few renames / refactors in it, so will be worthwhile for you to take a pass while you're getting familiar with the codebase.

carlaKC commented 10 months ago

dyn-traits-patch.txt

Whoop, just saw this now. Added a commit that does the same for LightningNode + Send!

sr-gi commented 10 months ago

Just dropping by to acknowledge that I owe a review here, will do it ASAP (hopefully this week)