Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

don't enforce maxuploadtarget's disconnect for whitelisted peers #6984

Merged
merged 3 commits into from Nov 14, 2015

Conversation

jonasschnelli
Copy link
Contributor

@jonasschnelli jonasschnelli commented Nov 11, 2015

No description provided.

@jonasschnelli jonasschnelli force-pushed the 2015/11/maxupload_whitebind branch 2 times, most recently from bba177b to 3fbe493 Compare Nov 11, 2015
static const int nOneWeek = 7 * 24 * 60 * 60; // assume > 1 week = historical
if (send && CNode::OutboundTargetReached(true) && ( ((pindexBestHeader != NULL) && (pindexBestHeader->GetBlockTime() - mi->second->GetBlockTime() > nOneWeek)) || inv.type == MSG_FILTERED_BLOCK) )
if (send && CNode::OutboundTargetReached(true) && ( ((pindexBestHeader != NULL) && (pindexBestHeader->GetBlockTime() - mi->second->GetBlockTime() > nOneWeek)) || inv.type == MSG_FILTERED_BLOCK) && !pfrom->fWhitelisted )
Copy link
Member

@sipa sipa Nov 11, 2015

Choose a reason for hiding this comment

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

Unnecessary space before )

Copy link
Contributor Author

@jonasschnelli jonasschnelli Nov 11, 2015

Choose a reason for hiding this comment

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

True. Force-push removed.

@sipa
Copy link
Member

sipa commented Nov 11, 2015

utACK

@jonasschnelli jonasschnelli force-pushed the 2015/11/maxupload_whitebind branch from 3fbe493 to 4566e5b Compare Nov 11, 2015
@gmaxwell
Copy link
Contributor

gmaxwell commented Nov 11, 2015

utACK

@fanquake
Copy link
Member

fanquake commented Nov 11, 2015

utACK @jonasschnelli has documentation written, it just needs to be included here.

@laanwj laanwj added the P2P label Nov 11, 2015
@laanwj
Copy link
Member

laanwj commented Nov 11, 2015

Concept ACK

@instagibbs
Copy link
Member

instagibbs commented Nov 12, 2015

utACK

NetworkThread().start() # Start up network handling in another thread
[x.wait_for_verack() for x in test_nodes]

#retrieve 20 blocks which should be enought to break the 1MB limit
Copy link
Member

@sipa sipa Nov 12, 2015

Choose a reason for hiding this comment

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

nit: enough

Copy link
Contributor Author

@jonasschnelli jonasschnelli Nov 12, 2015

Choose a reason for hiding this comment

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

Thanks. Fixed.

@sipa
Copy link
Member

sipa commented Nov 12, 2015

utACK

@@ -3856,8 +3856,9 @@ void static ProcessGetData(CNode* pfrom, const Consensus::Params& consensusParam
}
}
// disconnect node in case we have reached the outbound limit for serving historical blocks
// never disconnect whitelisted nodes
Copy link
Member

@MarcoFalke MarcoFalke Nov 13, 2015

Choose a reason for hiding this comment

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

Is this already mentioned in the release notes and/or documentation?

Copy link
Contributor

@gmaxwell gmaxwell Nov 13, 2015

Choose a reason for hiding this comment

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

I think it's more or less adequately explained by the documentation for whitelist.

Though the release note for maxupload target could mention it too.

@gmaxwell
Copy link
Contributor

gmaxwell commented Nov 13, 2015

Also tested, works good. (I gave a utACK above). Please update docs/release notes as you see fit.

@jonasschnelli jonasschnelli force-pushed the 2015/11/maxupload_whitebind branch from 5a2984f to 1aa57ac Compare Nov 13, 2015
@jonasschnelli
Copy link
Contributor Author

jonasschnelli commented Nov 13, 2015

I have added a short documentation in the release-notes.md and in the reduce-traffic.md doc (feel free to overhaul). Also i smuggled in a rename from reducetraffic.md to reduce-traffic.md (which would be silly to do in a separated PR). All other doc filename use the same pattern.

@jonasschnelli jonasschnelli force-pushed the 2015/11/maxupload_whitebind branch from 1aa57ac to e495ed5 Compare Nov 14, 2015
@jonasschnelli
Copy link
Contributor Author

jonasschnelli commented Nov 14, 2015

Thanks. Fixed.

@gmaxwell gmaxwell merged commit e495ed5 into bitcoin:master Nov 14, 2015
gmaxwell added a commit that referenced this pull request Nov 14, 2015
e495ed5 add documentation for exluding whitelistes peer from maxuploadtarget (Jonas Schnelli)
5760749 [docs] rename reducetraffic.md to reduce-traffic.md (Jonas Schnelli)
d61fcff don't enforce maxuploadtargets disconnect for whitelisted peers (Jonas Schnelli)
zkbot added a commit to zcash/zcash that referenced this pull request Feb 18, 2021
zkbot added a commit to zcash/zcash that referenced this pull request Feb 18, 2021
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants