Page MenuHomePhabricator
Differential D478

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 751
Build 751: 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
CCulianu updated this revision to Diff 1200.Aug 23 2017, 11:00

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).
CCulianu updated this revision to Diff 1201.Aug 23 2017, 11:05
CCulianu edited the summary of this revision. (Show Details)

Eh.. rebase to not pick up unwanted diffs

CCulianu updated this revision to Diff 1202.Aug 23 2017, 11:08

Struggling with arcanist. This should do it.

Harbormaster completed remote builds in B751: Diff 1202.Aug 23 2017, 11:08
Mengerian added a subscriber: Mengerian.Aug 23 2017, 17:52

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

CCulianu added a comment.Aug 23 2017, 18:02
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?

CCulianu abandoned this revision.Aug 23 2017, 18:27

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

PathSize
src/
6 lines
3 lines
6 lines

Diff 1202

View Options

src/net_processing.cpp

Loading...
View Options

src/validation.h

Loading...
View Options

src/validation.cpp

Loading...