Ignore ADDR messages from non-Cash peers if on the Cash chain already
AbandonedPublic
Actions

Authored by CCulianu on Aug 23 2017, 10:01.

Details

Reviewers

deadalnix
freetrader
zander
schancel
sickpig

Group Reviewers

Restricted Project

Summary

Note: this depends on D479!

This is a port of Bitcoin Classic's strategy to ignore non-Cash peer
ADDR messages if already on the Cash chain.

Since deadalnix seemed to approve of this for Bitcoin ABC based on slack
conversations in #dev, I went ahead and implemented it.

Note that Classic has a global "Chain state is CASH" type of variable for
quick and cheap determinations if we're in the bitcoin cash chain state.

UPDATE: I made this code use a a global flag to determine if on CASH chain (requires D479)

Test Plan

make check

run bitcoin with 'addrman' logging and see if you get any addr messages from non-cash peers when on a cash chain. You shouldn't.

Diff Detail

Repository

rABC Bitcoin ABC

Branch

addr_cash_only

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 750
Build 750: arc lint + arc unit

Event Timeline

CCulianu created this revision.Aug 23 2017, 10:01

Herald added a reviewer: Restricted Project. · View Herald TranscriptAug 23 2017, 10:01

zander accepted this revision.Aug 23 2017, 10:13

sickpig accepted this revision as: sickpig.Aug 23 2017, 10:30

CCulianu mentioned this in D479: Added fActiveChainTipIsUAHF as a global flag to indicate active chain is UAHF (potentially net_processing code needs this for NODE_CASH stuff).Aug 23 2017, 10:41

Use new flag from D479

CCulianu edited the summary of this revision. (Show Details)Aug 23 2017, 11:02

CCulianu added a parent revision: D479: Added fActiveChainTipIsUAHF as a global flag to indicate active chain is UAHF (potentially net_processing code needs this for NODE_CASH stuff).

Eh.. rebase to not pick up unwanted diffs

Struggling with arcanist. This should do it.

Harbormaster completed remote builds in B751: Diff 1202.Aug 23 2017, 11:08

For networking code, we will always be past UAHF fork. Unless we invent a time machine.

So the fActiveChainTipIsUAHF check is not needed

Classic needs to check this, since they have both a Cash and a Bitcoin version.

but ABC is always on the Cash chain

In D478#8417, @Mengerian wrote:

For networking code, we will always be past UAHF fork. Unless we invent a time machine.

No, we might be before the fork block (eg on legacy chain) when in the initial synch or if rescanning/reindexing the entire blocksdb.

For synching the blocks -- the question then becomes do we accept ADDR messages from non-Cash nodes when we are in that synching state?

Abandoned because it turns out we throw away non-cash ADDR's anyway a few lines later. So it doesn't hurt to accept ADDRs from non-cash nodes in the off chance they are telliing us about a Cash peer.

Revision Contents
Changeset List

Path

Size

src/

validation.h

3 lines

validation.cpp

6 lines

Diff 1201

View Options

src/validation.h