Simplify staker sampling

[fjarri]

More logical weighted sampling for stakers, with the sampler extracted to a separate class for the ease of unit testing. additional_ursulas and attempts parameters removed from sample().

The second commit makes exposes the staker sampler as an iterator-like object. BlockchainPolicy.sample_essential() now works as follows:

• quantity stakers is needed
• Get the stakers reservoir based on the list of stakers returned by the contract (excluding handpicked_ursulas right away)
• Draw quantity of stakers, see which ones are already known, and send the rest to the learner
• Wait a little, check for newly known nodes. If it's still not enough (some nodes are being checked), draw more stakers from the reservoir and send them to the learner.
• Loop until time is expired or we have enough nodes

The nodes that were drawn first have the priority. The returned result is shuffled (or should it be shuffled in sample() instead, that is shuffle the known stakers as well?)

Advantages over the previous sampling method:

• no need to "pre-sample" 1.5x of Ursulas; we draw samples as we need them.
• simpler algorithm structure
• we give addresses to the learner in a batch instead of one by one
• do not run the loop again and again; sleep() and let the learner do its thing

There are more advanced sampling methods, e.g. "Weighted random sampling with a reservoir" by Efraimidis and Spirakis, but they require floats. I'll stick to integers for now, in case we ever want it back in a contract.

[KPrasch]

@fjarri - Can the integration test failure be fixed via rebase over master?

[fjarri]

Evidently, yes :)

[codecov]

Codecov Report

Merging #2056 into main will decrease coverage by 0.01%.
The diff coverage is 86.95%.

@@            Coverage Diff             @@
##             main    #2056      +/-   ##
==========================================
- Coverage   83.66%   83.64%   -0.02%     
==========================================
  Files         103      103              
  Lines       15039    15058      +19     
==========================================
+ Hits        12582    12595      +13     
- Misses       2457     2463       +6     

Impacted Files	Coverage Δ
nucypher/blockchain/eth/agents.py	92.50% <85.41%> (-0.38%)	⬇️
nucypher/policy/policies.py	90.87% <86.48%> (+0.35%)	⬆️
nucypher/blockchain/eth/actors.py	86.56% <100.00%> (-0.02%)	⬇️
nucypher/network/nodes.py	81.84% <100.00%> (+0.02%)	⬆️
nucypher/cli/commands/alice.py	85.51% <0.00%> (-0.94%)	⬇️
nucypher/characters/base.py	89.59% <0.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 76eea57...449139e. Read the comment docs.

[derekpierre]

Still digesting the PR - but the sampling algorithm is much easier to understand, and I like the reservoir abstraction.

[KPrasch]

👍 - Looking good - I invite reviews from @cygnusv, @GhadaAlmashaqbeh, and @michwill !

[cygnusv]

In general, looking very good. It definitely improves the codebase, getting rid of old and ugly code and replacing it with a much simpler yet elegant alternative. Great stuff!
I have some comments where I'd like your opinion, though.

[cygnusv]

If the output is a set, I don't know if the shuffling is necessary as the iteration order over sets is arbitrary.

[fjarri]

Hm, don't sets share the ordered behaviour with dicts since Py3.7?

[derekpierre]

🎸

[derekpierre]

Feels like the previous one-liner was simpler

[fjarri]

Fair enough, not sure how it ended up this way. Fixed.