Less constrained ordered task prep

[carver]

What was wrong?

While sketching out a skeleton header sync, I found myself wanting OrderedTaskPreparation but with some constraints removed:

1. The ability to register tasks out of order (eg~ register a header when we don't have its parent yet)
2. The ability to register tasks without any prereqs (eg~ once we have a header registered, we don't need anything else besides all its ancestors)

How was it fixed?

• Reworked how pruning was done, which didn't support out-of-order tasks registration.
• Fairly simple to support tasks without prereqs, just had to check at register time if the task was already done
• More tests to cover these cases (and the combination of these cases)

I decided that often we don't want to permit out-of-order tasks. It's useful to get the warnings in the downstream sync steps. So permitting out-of-order tasks is an opt-in flag.

This comes at some cost to pruning time, which could be reduced with some more effort, if it seems to be a bottleneck.

Cute Animal Picture

[pipermerriam]

I honestly still have trouble digesting this API you've created. That isn't to say there is anything wrong with it, more that I haven't actually worked directly with it in any way.

What I don't see in this PR is an update to the docstrings. Can you do a quick audit to see if they need to be updated for these changes?

[pipermerriam]

Returning the task_id here seems non-standard. Most of our validation functions either raise exceptions or return None.

[pipermerriam]

Looks like this is the reason you're returning the task_id from the validate function. You can use cytoolz.do to accomplish that.

from cytoolz.curried import do
get_dependency_of_id = compose(
    do(self._validate_has_task),
    ...
)

[carver]

Hah, I just ran across that last week actually. Pretty handy. Until ethereum/eth-utils#136 is done, this is my workaround:

curry(do)(self._validate_has_task)

[pipermerriam]

Just functional programming nitpicking. I think you can do the same thing in a less readable manner as follows. Code may have an off-by-one error.

from cytoolz import iterate, nth
return nth(self._max_depth, iterate(get_dependency_of_id, finished_task_id))

Fancy!

[carver]

Hah, nice, yeah I like it better! I was actually looking for something like iterate the other day when I found do. 👍

[pipermerriam]

Can define an upper bound on this loop? The infiniteness of it seems unnecessary and it seems like it would be easier to debug in that case if it were to explicitly break and throw an exception at some reasonable upper bound.

[carver]

I honestly still have trouble digesting this API you've created. That isn't to say there is anything wrong with it, more that I haven't actually worked directly with it in any way.

Yeah, the API is completely designed around being handy from the point of view of the syncer. But still, it's worth iterating on if it's not pretty immediately grok-able. Just maybe not right now: I don't have any better ideas, and it's doing its job of making the syncer easier to write/comprehend.