Improve pickNextTask

[samholmes]

The goal of this PR is to improve the readablity and maintainability of the pickNextTask routine in UtxoEngineProcessor. This is done by removing unnecessary code decoupling and bringing routine code closer together while fixing any issues discovered during this audit. And last but not least, replacing Deferred promises with the more appropriate use of generators.

Note to reviewer

I'm not certainly happy with how the types are written for the generator implementation, so please scrutinize that aspect. Additionally, I haven't tested the BlockbookElectrum refactor, so that is an area to look closely at during review.

CHANGELOG

Does this branch warrant an entry to the CHANGELOG?

• Yes
• No

Dependencies

none

Description

none

---

• To see the specific tasks where the Asana app for GitHub is being used, see below:
  • https://app.asana.com/0/0/1207457172710620

[swansontec]

I am going to approve this and let QA pick up the pieces. These dynamic state machines are hard to reason about, so the best approach is to just run it.

Regarding the types, my general rule is to replace any with unknown if you don't know / don't care what something is. If you do this across the board, one of two things will happen. In the good case, there are no type errors, because nobody actually reads these unknown values. In the less-good case, you will have type errors in a handful of places that actually read the unknown values. For these places use an as any to cast the unknown value, and add a comment explaining why the conversion is safe. Or use something like a cleaner or type assertion to make it explicit (like (x: unknown): x is Foo => ...). The idea is to concentrate the type badness into one place, so the rest of the codebase is clean.

So basically, just replace WsTask<any> with WsTask<unknown> and see what happens.

[samholmes]

I am going to approve this and let QA pick up the pieces. These dynamic state machines are hard to reason about, so the best approach is to just run it.

Regarding the types, my general rule is to replace any with unknown if you don't know / don't care what something is. If you do this across the board, one of two things will happen. In the good case, there are no type errors, because nobody actually reads these unknown values. In the less-good case, you will have type errors in a handful of places that actually read the unknown values. For these places use an as any to cast the unknown value, and add a comment explaining why the conversion is safe. Or use something like a cleaner or type assertion to make it explicit (like (x: unknown): x is Foo => ...). The idea is to concentrate the type badness into one place, so the rest of the codebase is clean.

So basically, just replace WsTask<any> with WsTask<unknown> and see what happens.

I'll go ahead an poke around with the type checking by replacing any with unknown to see if anything rears its head.

As far as testing, I've tested sync over a normal circumstances. Though, the BlockbookElectrum changes haven't been tested and I'm not certain if I did those changes correctly. I'll have to look closer at that piece.

I'm also wondering if there is a type alias that I can define that would harden the type checking throughout the layers.

[swansontec]

That came out pretty nice.