This repository has been archived by the owner on Nov 6, 2020. It is now read-only.
Cleanup stratum a bit #11161
Merged
Cleanup stratum a bit #11161
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Salvage some code from #10884 + some cleanup and typos.
dvdplm
added
A0-pleasereview 🤓
seunlanlege
approved these changes
Oct 10, 2019
niklasad1
reviewed
Oct 10, 2019
niklasad1
reviewed
Oct 10, 2019
| @@ -55,7 +55,7 @@ pub trait JobDispatcher: Send + Sync { | |||
| /// Interface that can handle requests to push job for workers | |||
| pub trait PushWorkHandler: Send + Sync { | |||
| /// push the same work package for all workers (`payload`: json of pow-specific set of work specification) | |||
| fn push_work_all(&self, payload: String) -> Result<(), Error>; | |||
There was a problem hiding this comment.
The trait looks a bit weird after this change because now push_work_all is infallible and push_work can fail (based on the return types only)
I do realize it is because we don't want to short-circuit if one message fails when we need to send x more messages but still I guess it would be possible to keep all failed messages in a buffer and return Err if !buffer.is_empty()
There was a problem hiding this comment.
The trait looks a bit weird
It does, but it's better to get the weirdness "in your face" than hidden behind a comfy Result that is always Ok.
it would be possible to keep all failed messages in a buffer
Yeah, and while that would be more satisfying I still wonder what use calling code can make with such an error? Honest question: would it be useful?
There was a problem hiding this comment.
Yeah, and while that would be more satisfying I still wonder what use calling code can make with such an error? Honest question: would it be useful?
I don't think so currently it is only called from here AFAIU and would require more changes then.
Also, seems like push_work is not used except in the tests.
We can merge this and investigate my points above separately
niklasad1
approved these changes
Oct 11, 2019
niklasad1
added
A8-looksgood 🦄
niklasad1
reviewed
Oct 11, 2019
| Ok(()) | ||
| } | ||
|
|
||
| fn push_work(&self, payloads: Vec<String>, tcp_dispatcher: &Dispatcher) -> Result<(), Error> { |
niklasad1
pushed a commit
that referenced
this pull request
Nov 5, 2019
* Cleanup stratum a bit Salvage some code from #10884 + some cleanup and typos. * HashSet::new does not allocate before first insert * Remove unused method push_work()
dvdplm
added a commit
that referenced
this pull request
Nov 6, 2019
* Cleanup stratum a bit Salvage some code from #10884 + some cleanup and typos. * HashSet::new does not allocate before first insert * Remove unused method push_work()
s3krit
pushed a commit
that referenced
this pull request
Nov 11, 2019
* ropsten #6631425 foundation #8798209 (#11201) * [stable] builtin, istanbul and mordor testnet backports (#11234) * ethcore-builtin (#10850) * [builtin]: support `multiple prices and activations` in chain spec (#11039) * [chain specs]: activate `Istanbul` on mainnet (#11228) * ethcore/res: add mordor testnet configuration (#11200) * Update list of bootnodes for xDai chain (#11236) * ethcore: remove `test-helper feat` from build (#11047) * Secret store: fix Instant::now() related race in net_keep_alive (#11155) (#11159) * [stable]: backport #10691 and #10683 (#11143) * Fix compiler warning (that will become an error) (#10683) * Refactor Clique stepping (#10691) * Add Constantinople eips to the dev (instant_seal) config (#10809) * Add cargo-remote dir to .gitignore (?) * Insert explicit warning into the panic hook (#11225) * Fix docker centos build (#11226) * Update MIX bootnodes. (#11203) * Use provided usd-per-eth value if an endpoint is specified (#11209) * Add new line after writing block to hex file. (#10984) * Type annotation for next_key() matching of json filter options (#11192) (but no `FilterOption` in 2.5 so…) * Upgrade jsonrpc to latest (#11206) * [CI] check evmbin build (#11096) * Correct EIP-712 encoding (#11092) * [client]: Fix for incorrectly dropped consensus messages (#11086) * Fix block detail updating (#11015) * Switching sccache from local to Redis (#10971) * Made ecrecover implementation trait public (#11188) * [dependencies]: jsonrpc `14.0.1` (#11183) * [receipt]: add `sender` & `receiver` to `RichReceipts` (#11179) * [ethcore/builtin]: do not panic in blake2pricer on short input (#11180) * util Host: fix a double Read Lock bug in fn Host::session_readable() (#11175) * ethcore client: fix a double Read Lock bug in fn Client::logs() (#11172) * Change how RPCs eth_call and eth_estimateGas handle "Pending" (#11127) * Cleanup stratum a bit (#11161) * Upgrade to jsonrpc v14 (#11151) * SecretStore: expose restore_key_public in HTTP API (#10241)
s3krit
pushed a commit
that referenced
this pull request
Nov 11, 2019
* [CI] check evmbin build (#11096) * Correct EIP-712 encoding (#11092) * [client]: Fix for incorrectly dropped consensus messages (#11082) (#11086) * Update hardcoded headers (foundation, classic, kovan, xdai, ewc, ...) (#11053) * Add cargo-remote dir to .gitignore (?) * Update light client headers: ropsten 6631425 foundation 8798209 (#11201) * Update list of bootnodes for xDai chain (#11236) * ethcore/res: add mordor testnet configuration (#11200) * [chain specs]: activate Istanbul on mainnet (#11228) * [builtin]: support multiple prices and activations in chain spec (#11039) * [receipt]: add sender & receiver to RichReceipts (#11179) * [ethcore/builtin]: do not panic in blake2pricer on short input (#11180) * Made ecrecover implementation trait public (#11188) * Fix docker centos build (#11226) * Update MIX bootnodes. (#11203) * Insert explicit warning into the panic hook (#11225) * Use provided usd-per-eth value if an endpoint is specified (#11209) * Cleanup stratum a bit (#11161) * Add Constantinople EIPs to the dev (instant_seal) config (#10809) (already backported) * util Host: fix a double Read Lock bug in fn Host::session_readable() (#11175) * ethcore client: fix a double Read Lock bug in fn Client::logs() (#11172) * Type annotation for next_key() matching of json filter options (#11192) * Upgrade jsonrpc to latest (#11206) * [dependencies]: jsonrpc 14.0.1 (#11183) * Upgrade to jsonrpc v14 (#11151) * Switching sccache from local to Redis (#10971) * Snapshot restoration overhaul (#11219) * Add new line after writing block to hex file. (#10984) * Pause pruning while snapshotting (#11178) * Change how RPCs eth_call and eth_estimateGas handle "Pending" (#11127) * Fix block detail updating (#11015) * Make InstantSeal Instant again #11186 * Filter out some bad ropsten warp snapshots (#11247)
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Salvage some code from #10884 + some cleanup and typos.