Avoid broadcasting transactions to peers that send them #3796
Conversation
Conflicts: ethcore/src/client/traits.rs
tomusdrw
added
A0-pleasereview 🤓
|
@tomusdrw Most likely can be done with an extension to the |
| @@ -344,7 +351,8 @@ struct TxRelay(Arc<BlockChainClient>); | |||
| impl LightHandler for TxRelay { | |||
| fn on_transactions(&self, ctx: &EventContext, relay: &[::ethcore::transaction::SignedTransaction]) { | |||
| trace!(target: "les", "Relaying {} transactions from peer {}", relay.len(), ctx.peer()); | |||
| self.0.queue_transactions(relay.iter().map(|tx| ::rlp::encode(tx).to_vec()).collect()) | |||
| // TODO [ToDr] Can we get a peer enode somehow? | |||
| self.0.queue_transactions(relay.iter().map(|tx| ::rlp::encode(tx).to_vec()).collect(), None) | |||
There was a problem hiding this comment.
ctx.persistent_peer_id(&ctx.peer()) once the method I mentioned has been added.
| @@ -75,6 +82,9 @@ pub trait EventContext { | |||
| /// disconnected/connected peer. | |||
| fn peer(&self) -> PeerId; | |||
|
|
|||
| /// Returns the relevant's peer persistent Id (aka NodeId). | |||
| fn persistent_peer_id(&self) -> Option<NodeId>; | |||
There was a problem hiding this comment.
would prefer for this to take arbitrary &PeerId ideally.
| let hashes: Vec<_> = txs.iter().map(|tx| tx.hash()).collect(); | ||
| let block_number = self.chain_info().best_block_number; | ||
| self.notify(|notify| { | ||
| notify.transactions_imported(hashes.clone(), peer_id.clone(), block_number); |
There was a problem hiding this comment.
Should not it happen only after successful import?
There was a problem hiding this comment.
The name of the event is misleading, even if the transaction is already in queue we want to note the the peer has propagated the transaction again, so we don't really care if it was imported or not.
Will rename to transactions_received.
|
|
||
| fn persistent_peer_id(&self) -> Option<NodeId> { | ||
| self.io.persistent_peer_id(self.peer) | ||
| } |
There was a problem hiding this comment.
Indentation looks strange around this block. Does the file use spaces?
There was a problem hiding this comment.
sorry, this is my mistake from earlier. Fixed in #3801
tomusdrw
added
A3-inprogress ⏳
tomusdrw
changed the title
Conflicts: ethcore/light/src/net/context.rs ethcore/light/src/net/tests/mod.rs
tomusdrw
added
A0-pleasereview 🤓
|
Changes Unknown when pulling 83d9bc1 on tx-broadcast into ** on master**. |
arkpar
added
A8-looksgood 🦄
arkpar
added
A7-looksgoodtestsfail 🤖
Closes #3757
It's looks a bit ugly to propagate the peer_id to the client and then back to sync, @arkpar any ideas how to improve this?
@rphmeier in light client implementation we cannot easily get
network.peer_session_info(peer_id)any ideas how to solve this?