decred / dcrwallet

A secure Decred wallet daemon written in Go (golang).
https://decred.org
ISC License
211 stars 153 forks source link

Add 100-500ms of per-peer inventory delay #2383

Closed jrick closed 2 weeks ago

jrick commented 2 weeks ago

Similar to a recent dcrd change, we want published inventory by SPV wallet peers be submitted with a small random amount of delay. In this change we use the same logic that dcrd now uses to pick, for each connected peer, a random delay between 100-500ms before the inventory is sent.

jrick commented 2 weeks ago

Something I noticed in https://github.com/decred/dcrd/pull/3363 was that the address list was shuffled.

Should you also shuffle the address list here?

dcrwallet does not respond to getaddr. It only requests them.

jrick commented 2 weeks ago

or did you mean this, shuffling the inventory before we publish?

diff /home/jrick/src/dcrwallet
commit - b226415203f66ccaaad9d5e70bdb5adfa2e2e565
path + /home/jrick/src/dcrwallet
blob - 6e4effdee7b8a6306d5b60586d3a498b605aa6a6
file + p2p/peering.go
--- p2p/peering.go
+++ p2p/peering.go
@@ -542,6 +542,7 @@ func (rp *RemotePeer) sendWaitingInventory(ctx context
    }

    s := rp.waitingInvs
+   rand.ShuffleSlice(s)
    for len(s) != 0 {
        l := len(s)
        if l > wire.MaxInvPerMsg {
matthawkins90 commented 2 weeks ago

Yes, sorry. Shuffling the inventory. Just wondering if it was worth doing.

jrick commented 2 weeks ago

it's a good idea. we shouldn't need to depend on the order being dependency order, as both mempool and mixpool handle orphans.

davecgh commented 2 weeks ago

it's a good idea. we shouldn't need to depend on the order being dependency order, as both mempool and mixpool handle orphans.

I actually disagree with this. In fact, it's a bad idea, imo. It's true that mempool handles orphans, but only a pretty limited number of them on purpose to prevent malicious actors being able to fill up memory without actually spending any real coins. It assumes that they are going to mostly be advertised in dependency order. Changing that would make it so malicious actors can hinder propagation by intentionally sending a bunch of junk to flush orphan buffers.