Skip to content

[trivial] Cleanup maxuploadtarget (doc & log) #6958

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.

Sign up for GitHub

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

Jump to bottom
Merged
laanwj merged 2 commits into from
Nov 13, 2015
Merged

[trivial] Cleanup maxuploadtarget (doc & log) #6958

laanwj merged 2 commits into from
Nov 13, 2015
+27 −5

Conversation

@maflcko
Copy link
Member

@maflcko maflcko commented Nov 6, 2015

@jonasschnelli this fixes #6890.

jonasschnelli
jonasschnelli reviewed Nov 6, 2015
View reviewed changes
qa/rpc-tests/maxuploadtarget.py Outdated
Copy link
Contributor

@jonasschnelli jonasschnelli Nov 6, 2015

Choose a reason for hiding this comment

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

Hmm.... i think either we take in the MAX_BLOCK_SIZE constant (controversial) or we drop the comment.

Copy link
Member Author

@maflcko maflcko Nov 6, 2015

Choose a reason for hiding this comment

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

Is this exposed via rpc? I always like to do grep -r 'CONSTANT' bitcoin/ when touching a constant to see it's usage.

@jonasschnelli
Copy link
Contributor

jonasschnelli commented Nov 6, 2015

utACK. Thanks!

I think we should extend the documentation about -maxuploadtarget. How it actually works under the hood (short, 1-2 sentence). I can do this after this has been merged.

@maflcko maflcko force-pushed the MarcoFalke-2015-maxupload branch 2 times, most recently from 48876a8 to 9f61ffe Compare November 10, 2015 12:06
@jonasschnelli
Copy link
Contributor

jonasschnelli commented Nov 10, 2015

Two commits that could make sense for this PR (2nd one maybe independent):

  • db504eb ([doc] extend -maxuploadtarget release notes)
  • f6b96ce09ec827bb9be0fd926b199009dd08bfd4 ([doc] add documentation how to reduce traffic)

Both are written in my swenglish (swiss english). Feel free (or obligated) to correct and rewrite everything.

@laanwj laanwj added the Docs label Nov 10, 2015
@jonasschnelli jonasschnelli mentioned this pull request Nov 11, 2015
Merged
@maflcko maflcko force-pushed the MarcoFalke-2015-maxupload branch from 9a81fb8 to 3fb48ff Compare November 11, 2015 13:05
jonasschnelli
jonasschnelli reviewed Nov 11, 2015
View reviewed changes
src/net.cpp
Copy link
Contributor

@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.

Nice catch. Thanks!

@jonasschnelli
Copy link
Contributor

jonasschnelli commented Nov 11, 2015

utACK

@maflcko
Copy link
Member Author

maflcko commented Nov 11, 2015

Looks ready to merge!?

jtimon
jtimon reviewed Nov 11, 2015
View reviewed changes
doc/release-notes.md
Copy link
Contributor

@jtimon jtimon Nov 11, 2015

Choose a reason for hiding this comment

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

Maybe a link to https://github.com/jonasschnelli/bitcoin/blob/2015/11/doc_traffic/doc/reducetraffic.md (once it's merged) can be added here ? Something like "for more details on reducing traffic, see ...".

@jtimon
Copy link
Contributor

jtimon commented Nov 11, 2015

utACK besides my little nit.

[net] Cleanup maxuploadtarget
b27e81f 
* log: nMaxOutboundLimit is in bytes
* log: Hide misleading -maxuploadtarget=0 warning
* qa : Minor cleanup to maxuploadtarget rpc tests
* net: Use DEFAULT_MAX_UPLOAD_TARGET = 0
@maflcko maflcko force-pushed the MarcoFalke-2015-maxupload branch from 3fb48ff to 2bf3771 Compare November 12, 2015 19:49
@maflcko
Copy link
Member Author

maflcko commented Nov 12, 2015

@jtimon Done.

@maflcko maflcko force-pushed the MarcoFalke-2015-maxupload branch from cdfb9e0 to 9c3ee3b Compare November 12, 2015 20:13
@jtimon
Copy link
Contributor

jtimon commented Nov 12, 2015

Great, re-utACK

@laanwj laanwj merged commit 9c3ee3b into bitcoin:master Nov 13, 2015
laanwj added a commit that referenced this pull request Nov 13, 2015
@laanwj
Merge pull request #6958
d2e987a 
9c3ee3b [doc] Add -maxuploadtarget release notes (MarcoFalke)
b27e81f [net] Cleanup maxuploadtarget (MarcoFalke)
@maflcko maflcko deleted the MarcoFalke-2015-maxupload branch November 14, 2015 18:13
@str4d str4d mentioned this pull request Jul 14, 2017
Draft
@str4d str4d mentioned this pull request Dec 19, 2017
Merged
zkbot added a commit to zcash/zcash that referenced this pull request Feb 18, 2021
@zkbot
Auto merge of #2816 - str4d:2074-maxuploadtarget, r=str4d
1d378b1 
Introduce -maxuploadtarget

Cherry-picked from the following upstream PRs:

- bitcoin/bitcoin#6622
- bitcoin/bitcoin#6987
- bitcoin/bitcoin#6958
- bitcoin/bitcoin#6984
- bitcoin/bitcoin#6999

Part of #2074.
@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.

Reviewers

No reviews

Assignees

No one assigned

Labels

Docs

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add -maxuploadtarget to 0.12 release-notes

4 participants

@maflcko @jonasschnelli @jtimon @laanwj