Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Problem: sending any Whisper message fails #7421

Merged
merged 1 commit into from
Jan 2, 2018

Conversation

yrashk
Copy link
Contributor

@yrashk yrashk commented Jan 2, 2018

The error is "PoW too low to compete with other messages"

This has been previously reported in #7144

Solution: prevent the move semantics

The source of the error is in PoolHandle.relay
implementation for NetPoolHandle.

Because of the move semantics, res variable is in fact
copied (as it implements Copy) into the closure and for
that reason, the returned result is always `false.


I am not sure what's the best way to cover this with a test. Any suggestions?

The error is "PoW too low to compete with other messages"

This has been previously reported in openethereum#7144

Solution: prevent the move semantics

The source of the error is in PoolHandle.relay
implementation for NetPoolHandle.

Because of the move semantics, `res` variable is in fact
copied (as it implements Copy) into the closure and for
that reason, the returned result is always `false.
@parity-cla-bot
Copy link

It looks like @yrashk signed our Contributor License Agreement. 👍

Many thanks,

Parity Technologies CLA Bot

@tomusdrw tomusdrw mentioned this pull request Jan 2, 2018
@tomusdrw tomusdrw requested a review from rphmeier January 2, 2018 09:02
@tomusdrw tomusdrw added A0-pleasereview 🤓 Pull request needs code review. B0-patch M4-core ⛓ Core client code / Rust. labels Jan 2, 2018
@debris
Copy link
Collaborator

debris commented Jan 2, 2018

cc @rphmeier

@@ -51,7 +51,7 @@ impl PoolHandle for NetPoolHandle {
fn relay(&self, message: Message) -> bool {
let mut res = false;
let mut message = Some(message);
self.net.with_proto_context(whisper_net::PROTOCOL_ID, &mut move |ctx| {
self.net.with_proto_context(whisper_net::PROTOCOL_ID, &mut |ctx| {
Copy link
Contributor

@rphmeier rphmeier Jan 2, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's a tricky one. nice catch!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am a little disappointed that rustc doesn't give you a warning for unused_mut when the variable is only copied into a closure.

Minimal playpen: https://play.rust-lang.org/?gist=c66bc6098aeacddb71550878c0d887f9&version=stable

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rphmeier rphmeier added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Jan 2, 2018
@debris debris merged commit 4ce643b into openethereum:master Jan 2, 2018
@5chdn 5chdn added this to the 1.9 milestone Jan 2, 2018
@rstormsf
Copy link

rstormsf commented Jan 2, 2018

Thank you @yrashk for fixing this critical issue for the whole ethereum community.
@ensrationis - you can use Whisper with Parity. I remember we talked about it on DevCon3

@debris debris mentioned this pull request Jan 3, 2018
@ensrationis
Copy link

@rstormsf Thanks for alert :) We will try.

@rstormsf
Copy link

@ensrationis actually we have found more issues with Whisper in parity. False alarm :-(

@c0gent c0gent deleted the whisper-relay branch July 27, 2018 19:03
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well. M4-core ⛓ Core client code / Rust.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants