-
Notifications
You must be signed in to change notification settings - Fork 1.6k
implement provisioner #1473
implement provisioner #1473
Conversation
coriolinus
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@montekki Here's the current state of work. The biggest remaining todo, aside from the unimplemented bit and getting answers to all the review questions, will be to write a bunch of tests to verify that this actually works as expected.
Another TODO item I've thought of, but which might be better off as its own issue, will be to abstract away the potential for runtime errors. It's not ideal that the util runtime functions all return Result<RuntimeApiReceiver<T>, Error>. Given that the universal response to a runtime failure appears to be to log the error and then proceed, we could rewrite them to write their own log warning, and return Result<Option<Receiver<T>>, Error>. That TODO might be better extracted to its own issue, though.
|
I believe that this now implements the complete candidate selection algorithm from the guide. I'm going to work on adding tests now. I'm not going to mark this as ready for review until the tests are in, but I think it's ready for a preliminary look. |
| [package] | ||
| name = "polkadot" | ||
| version = "0.8.22" | ||
| version = "0.8.19" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
was this version revert intentional? probably bad merge
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, that was an accident.
Closes #1436.