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

Speed limit / throttle network usage #273

Open
slothbag opened this Issue May 26, 2011 · 101 comments

Comments

Projects
None yet
@slothbag

slothbag commented May 26, 2011

I noticed the other day Bitcoin was maxing out my upload bandwidth on ADSL. Probably due to sending the block chain to fellow bitcoin users ( I had about 65 connections at the time )..

The ability to limit / throttle the network usage like most other p2p programs would be beneficial. Otherwise I have to ensure I close the Bitcoin application to keep it from killing my upload.

@m4rcelofs

This comment has been minimized.

m4rcelofs commented Jun 16, 2011

+1 to that!

Bitcoin is currently using all my upload speed, which actually means sometimes I can't even browse! Closing BitCoin solves the issue, but it's not really beneficial to the network...

@NvrBst

This comment has been minimized.

NvrBst commented Aug 6, 2011

+1 as well.

I have a 60KB/s upload limit on my ADSL, and bitcoin.exe is killing my upload so bad at times that I cannot even browse internet very well. Some extra information:

I usually have ~32 connections in bitcoin. Receive seems to be fine. After 10 mins the current bitcoin.exe connections have downloaded about 100kB each, but, download is usually not as limited as upload.

After about 10 mins the current bitcoin.exe connections use about 100kB upload each, but, there are ~3 which uses ~1MB+. This comes to over 10kB/s average (not much average wise). But I suspect when those 1MB+ guys are going it uses 100% of the upload and just drops down to 7.5kB/s average after that part is done.

Preferably I'd like to limit upload to 2kB/s to 5kB/s on my current connection.

EDIT: I'm using bitcoin 0.3.24-beta

@Trupik

This comment has been minimized.

Trupik commented Apr 10, 2012

I have hosted public bitcoin node 24/7 for over a year now on my home linux server. It is causing me a considerable packet loss on the line and it is getting worse over time, as the chain grows. Please, add bandwidth limiting to the bitcoind. Otherwise I will have to shut down the node, because my internet connection is getting unusable.

@luke-jr

This comment has been minimized.

Member

luke-jr commented Apr 26, 2012

QoS is really a router job, but I guess reliable routers aren't too common :/

@neofutur

This comment has been minimized.

neofutur commented Apr 26, 2012

one more +1, also needing this, or a way to have bitcoind use a shared blockchain ( see http://bitcoin.stackexchange.com/questions/3199/read-only-blockchain-in-bitcoind-patch-ideas and https://bitcointalk.org/index.php?topic=71542 ) , even better than just throttle imho

also,waiting foran option in bitcoind, you could have a look at 3 userspace bandwidth limiting tools : http://monkey.org/~marius/pages/?page=trickle , http://klicman.org/throttle/ and http://stromberg.dnsalias.org/~strombrg/slowdown/

@laanwj

This comment has been minimized.

Member

laanwj commented Apr 26, 2012

No +1s here please. I think we all agree it is a good idea for a P2P program to have configurable limits.

It's not realistic to require people to move this to a router. Especially on a VPS you don't really have much control over routing.

However, this feature has currently no priority for the core developers. If you want to speed up this issue, help with the implementation.

Edit: and I suggest trying out thin clients such as Electrum, which claim to use much less bandwidth and 'share' a block chain on a supernode.

@Diapolo

This comment has been minimized.

Diapolo commented Apr 26, 2012

Perhaps unrelated, but you guys could (for Windows) try out cFosSpeed if your connection get's laggy because of uploads.

@rebroad

This comment has been minimized.

Contributor

rebroad commented May 3, 2012

For MS Windows, I can suggest NetLimiter as a nice piece of software, albeit commercial. I'd have thought linux has various QoS options available, doesn't it?

@nadrimajstor

This comment has been minimized.

nadrimajstor commented May 18, 2012

When you use P2P application (e.g. torrent client) you are always presented with some form of information and/or customizing options regarding the nature of P2P communication and its bandwidth consumption.
With BitCoin client this is not so obvious and lead me to false belief that BitCoin is not heavy on network traffic.
Sporadic internet hogs was always attributed to flaky ISP until I hunted down that actually BitCoin client was the culprit (filling up all of upload bandwidth).
For the sake of less tech savvy users, please make obvious and easy option to, at least, add -nolisten argument when starting client.

@FernanDK

This comment has been minimized.

FernanDK commented Dec 11, 2012

issue was created 2 damn years ago. What is going on dev guys? Why this has not been patched yet? Don't you guys see the urgent necessity of such feature?

it's so stupid to not implement this ASAP cuz it does give a bad look to the whole app and community, since it fucks up people's internet, and a lot of them are not into technical stuff, which basically means, bad program -> uninstall..

@laanwj

This comment has been minimized.

Member

laanwj commented Dec 11, 2012

You misunderstand how open source development works. We work on this in our
spare time so we decide for ourselves what is important and want to spend
time on.

If you want something to be very badly implemented it is your
responsibility to make that happen, not ours.

Can you contribute to solving this issue?

Or are you willing to pay to have the feature implemented? You could offer
an bounty.

@gmaxwell

This comment has been minimized.

Member

gmaxwell commented Dec 11, 2012

I don't, fwiw, see an urgent need for such a feature— and until some extra components are developed such a feature would be harmful for the network: if you set a low throttle your peers need to switch to pulling blocks from someone else when you are slow. (Also an issue absent a throttle but many people setting a throttle would make it worse)

Right now it's better for traffic constrained nodes to just disable listening. This will largely have the desired effect, doesn't risk adding problems, and is an already available option.

@sipa

This comment has been minimized.

Member

sipa commented Dec 11, 2012

I think the issue here is much more that it is not obvious to a new user that by default your node will provide blocks to the network. Disable listening would indeed mostly fix that, but it's far from obvious.

@lucb1e

This comment has been minimized.

lucb1e commented Jan 23, 2013

If I don't forward my port, can't other clients download from me? The client makes outgoing connections in order to receive blocks. In BitTorrent outgoing connections are also used for uploading it seems, and it makes sense because this keeps the network much more alive. Does Bitcoin do the same, upload blocks to connected nodes if you didn't portforward?

@gmaxwell

This comment has been minimized.

Member

gmaxwell commented Jan 23, 2013

Nodes don't announce themselves as accepting connections until they are mostly caught up... so you'd only be forwarding on new blocks, which isn't a tremendous amount of data.

@lucb1e

This comment has been minimized.

lucb1e commented Jan 23, 2013

I am caught up with the block chain, that's not the issue here. But I would be forwarding new blocks to connected peers regardless of whether my port is forwarded? Because that might result in an upload job of 1.5MB for a new block, if all connected nodes don't have the new block yet.

@cjastram

This comment has been minimized.

cjastram commented Mar 3, 2013

"Nodes don't announce themselves as accepting connections until they are mostly caught up... so you'd only be forwarding on new blocks, which isn't a tremendous amount of data."

Nevertheless, it saturates my uplink. I am on DSL with no other options, and it is a solid 50-70 kilobyte (not kilobit) upload, making the connection completely unusable for anything else. Someday, Google Fiber will bring me a giant fat pipe and this won't be a concern, but since I live in a fairly rural area, it is not likely for a while.

This lack of upload control harms the network because I turn the whole node completely off unless I am actually engaging in a transaction. Wouldn't it be better to have a slow node than no node?

@FernanDK

This comment has been minimized.

FernanDK commented Mar 3, 2013

seems like the devs think everybody has googlefiber, this is a shame, really, there's X types of internet services around the world with Y upload/download limits. Forcing the user to do something that will harm its own internet is an outrageous decision.

Not to mention that, once the user finds out what is hammering its internet, the most common decision is kill the program who is causing the problem and abandon it, or use it as low as possible, it becomes a bad thing instead of a good thing. This is bad marketing. This disappoints me in so many ways.

How am i going to recommend this to a friend knowing that later the same friend will come to me and say "hey remember that program you told me to use? it was messing with my internet, making it very slow, i couldn't even see a utube video in piece, bad call dud". That's the least that could happen, and this is just a simple example.

Like cjastram just pointed out, better to HAVE a slow node than NO node at all. If the devs were intelligent enough to create such an futuristic and advanced code, i'm sure they already know everything we are pointing out here, no more words, i'm done.

@gmaxwell

This comment has been minimized.

Member

gmaxwell commented Mar 3, 2013

@XcaninoX so, you're saying you turned off listening as advised and it's using 70kbytes/sec outbound?

@sipa

This comment has been minimized.

Member

sipa commented Mar 3, 2013

Personally, I think upload throttling as-is is a bad idea, at least until we improve the block sync mechanism. If you happen to hit a throttled node when syncing, syncing will be slow.

Instead, I think we need a "non server" mode (some checkbox, perhaps asked on first startup), which disables serving (historic) blocks, disables listening (by default) and disables NODE_NETWORK (so nodes don't announce themself as full noes). This means people can still do the validation and relaying part of being a full node, but without the bandwidth implications of serving the full chain.

Later, such a non-server mode could be turned into a pruning mode, where historic blocks aren't even kept on disk.

@cjastram

This comment has been minimized.

cjastram commented Mar 4, 2013

Personally, I think upload throttling as-is is a bad idea, at least until we improve the block sync mechanism. If you happen to hit a throttled node when syncing, syncing will be slow.

Of course, but isn't the sync slow if you hit a node that has limited bandwidth? You don't magically get a fast sync just because you don't allow nodes the ability to throttle. The problem is that if you don't allow throttling, then you get no node because people just turn it off.

After a while of people just turning it off, then you start getting massive sync load on the system because people need to sync hundreds (or thousands) of blocks whenever they want to do a BTC transaction. Instead of the lightweight sync load that happens in realtime, you have a small (but significant) number of people that require loads of bandwidth all at once.

Hence the problem. If we could just leave BitCoin turned on at low bandwidth, then we wouldn't have the sync issue.

Instead, I think we need a "non server" mode (some checkbox, perhaps asked on first startup), which disables serving (historic) blocks, disables listening (by default) and disables NODE_NETWORK (so nodes don't announce themself as full noes). This means people can still do the validation and relaying part of being a full node, but without the bandwidth implications of serving the full chain.

Maybe. I don't know what my upload bandwidth saturation actually is, whether it is transaction confirmations or other peoples' block syncs.

Later, such a non-server mode could be turned into a pruning mode, where historic blocks aren't even kept on disk.

Let's try to not over-engineer a simpler solution?

@sipa

This comment has been minimized.

Member

sipa commented Mar 4, 2013

@cjastram That is actually my point: I think it's better for people who do cannot or don't want to serve historic blocks to not do that at all, and not even advertize on the network that they do. If we enable bandwidth throttling without that, the chance of accidentally hitting a slow node would increase, while if we remove those nodes from the pool altogether, the chance for that decreases.

People fully shutting down nodes because it saturates their data link is of course bad. Throttling or disabling serving would improve that, but you're close to the resources you're willing to spend, perhaps you shouldn't run a full node in the first place.

Just relay of new transactions and blocks is typically quite low bandwidth - it's just when you get hit by a node that is syncing from scratch that you suddenly get an upload burst.

About over-engineering: the ability to run a pruned node is an inevitable evolution somewhere in the future, in my opinion, and it would also imply a solution to your problem anyway.

@jgarzik

This comment has been minimized.

Contributor

jgarzik commented Mar 4, 2013

FWIW, I respectfully disagree, and think the easiest solution is to offer an optional upload (i.e. send or write syscall) throttle. A download throttle is less useful: You cannot really control the amount of incoming remote data; ceasing read(2) will cause good guys to throttle eventually, but bad guys always have a technique or three that will flood incoming anyway. And in the field, users care less about limiting their download speed than they do limiting their upload speed.

It is a common knob on other P2P data serving apps, so I would ACK a properly implemented upload throttle w/ knob.

@sipa

This comment has been minimized.

Member

sipa commented Mar 4, 2013

@jgarzik My point is that we're currently better off with less serving nodes, if that means those nodes are faster. Once we have headers-first, and parallel chain fetching, things are different, and probably anyone willing to contribute a bit of upload is useful.

@slothbag

This comment has been minimized.

slothbag commented Mar 6, 2013

OP here, its been a while since creating this issue and for the past 2 years my bitcoin-qt client has very rarely maxed out my upload so I didn't think too much about it.

But recently, in the last month or two I have noticed that my upload or sending data out is quite often completely maxed! I have a decent connection, something like 300-500KB/s upload. Luckily i've managed to catch it most times and I open up a network monitoring tool and kill the ports in question (always bitcoin-qt). I have to be careful because if it ran at that speed for a long period of time, not only would my net be slow but it will quickly consume my data quota.

So now instead of running a full node for ~8 hours a day I let it sync with the network and then shut it down.

If I had a throttle option, I would set it at something like 50KB/s upload.. this is faster than I ever use when downloading the latest blocks (CPU bound).

I think this is a problem, I would like to run a full node, but will not while it consumes 100% of my upload majority of the time.

@gmaxwell

This comment has been minimized.

Member

gmaxwell commented Mar 6, 2013

@slothbag Have you disabled listening?

@slothbag

This comment has been minimized.

slothbag commented Mar 6, 2013

No I haven't. I actually would like to contribute as a full listening node.
If I'm gonna disable listening then i might as well not bother and just let
it sync and then shut it down.

On Wednesday, March 6, 2013, Gregory Maxwell wrote:

@slothbag https://github.com/slothbag Have you disabled listening?


Reply to this email directly or view it on GitHubhttps://github.com//issues/273#issuecomment-14491575
.

@gmaxwell

This comment has been minimized.

Member

gmaxwell commented Mar 6, 2013

@slothbag As a non-listening node you are still a contributor to the network— validating and forwarding transactions and new blocks, preventing partitioning... but you won't use much bandwidth. A rate limit while serving historic blocks would be very damaging to the network right now, it is actually better that you do not run a node than serve historic blocks with a rate limit. ... but better still to run without listening.

TheBlueMatt pushed a commit to TheBlueMatt/bitcoin that referenced this issue Oct 20, 2015

Squashed 'src/secp256k1/' changes from 22f60a6..71ed475
71ed475 Pedersen commitments, borromean ring signatures, and ZK range proofs.
afd1379 Add 64-bit integer utilities
9d96e36 Merge pull request bitcoin#280
432e1ce Merge pull request bitcoin#283
14727fd Use correct name in gitignore
356b0e9 Actually test static precomputation in Travis
ff3a5df Merge pull request bitcoin#284
2587208 Merge pull request bitcoin#212
a5a66c7 Add support for custom EC-Schnorr-SHA256 signatures
d84a378 Merge pull request bitcoin#252
72ae443 Improve perf. of cmov-based table lookup
92e53fc Implement endomorphism optimization for secp256k1_ecmult_const
ed35d43 Make `secp256k1_scalar_add_bit` conditional; make `secp256k1_scalar_split_lambda_var` constant time
91c0ce9 Add benchmarks for ECDH and const-time multiplication
0739bbb Add ECDH module which works by hashing the output of ecmult_const
4401500 Add constant-time multiply `secp256k1_ecmult_const` for ECDH
e4ce393 build: fix hard-coded usage of "gen_context"
b8e39ac build: don't use BUILT_SOURCES for the static context header
baa75da tests: add a couple tests
ae4f0c6 Merge pull request bitcoin#278
995c548 Introduce callback functions for dealing with errors.
c333074 Merge pull request bitcoin#282
18c329c Remove the internal secp256k1_ecdsa_sig_t type
74a2acd Add a secp256k1_ecdsa_signature_t type
23cfa91 Introduce secp256k1_pubkey_t type
4c63780 Merge pull request bitcoin#269
3e6f1e2 Change rfc6979 implementation to be a generic PRNG
ed5334a Update configure.ac to make it build on OpenBSD
1b68366 Merge pull request bitcoin#274
a83bb48 Make ecmult static precomputation default
166b32f Merge pull request bitcoin#276
c37812f Add gen_context src/ecmult_static_context.h to CLEANFILES to fix distclean.
125c15d Merge pull request bitcoin#275
76f6769 Fix build with static ecmult altroot and make dist.
5133f78 Merge pull request bitcoin#254
b0a60e6 Merge pull request bitcoin#258
733c1e6 Add travis build to test the static context.
fbecc38 Add ability to use a statically generated ecmult context.
4fb174d Merge pull request bitcoin#263
4ab8990 Merge pull request bitcoin#270
bdf0e0c Merge pull request bitcoin#271
31d0c1f Merge pull request bitcoin#273
eb2c8ff Add missing casts to SECP256K1_FE_CONST_INNER
55399c2 Further performance improvements to _ecmult_wnaf
99fd963 Add secp256k1_ec_pubkey_compress(), with test similar to the related decompress() function.
145cc6e Improve performance of _ecmult_wnaf
36b305a Verify the result of GMP modular inverse using non-GMP code
0cbc860 Merge pull request bitcoin#266
06ff7fe Merge pull request bitcoin#267
5a43124 Save 1 _fe_negate since s1 == -s2
a5d796e Update code comments
3f3964e Add specific VERIFY tests for _fe_cmov
7d054cd Refactor to save a _fe_negate
b28d02a Refactor to remove a local var
55e7fc3 Perf. improvement in _gej_add_ge
a0601cd Fix VERIFY calculations in _fe_cmov methods
17f7148 Merge pull request bitcoin#261
7657420 Add tests for adding P+Q with P.x!=Q.x and P.y=-Q.y
8c5d5f7 tests: Add failing unit test for bitcoin#257 (bad addition formula)
5de4c5d gej_add_ge: fix degenerate case when computing P + (-lambda)P
bcf2fcf gej_add_ge: rearrange algebra
e2a07c7 Fix compilation with C++
873a453 Merge pull request bitcoin#250
91eb0da Merge pull request bitcoin#247
210ffed Use separate in and out pointers in `secp256k1_ec_pubkey_decompress`
a1d5ae1 Tiny optimization
729badf Merge pull request bitcoin#210
2d5a186 Apply effective-affine trick to precomp
4f9791a Effective affine addition in EC multiplication

git-subtree-dir: src/secp256k1
git-subtree-split: 71ed475ea53ff4576b7344762584b752a824c60f

sipa added a commit to sipa/bitcoin that referenced this issue Oct 27, 2015

Squashed 'src/secp256k1/' changes from 22f60a6..d7eb1ae
d7eb1ae Merge pull request bitcoin#342
7914a6e Make lax_der_privatekey_parsing.h not depend on internal code
73f64ff Merge pull request bitcoin#339
9234391 Overhaul flags handling
1a36898 Make flags more explicit, add runtime checks.
1a3e03a Merge pull request bitcoin#340
96be204 Add additional tests for eckey and arg-checks.
bb5aa4d Make the tweak function zeroize-output-on-fail behavior consistent.
4a243da Move secp256k1_ec_privkey_import/export to contrib.
1b3efc1 Move secp256k1_ecdsa_sig_recover into the recovery module.
e3cd679 Eliminate all side-effects from VERIFY_CHECK() usage.
b30fc85 Avoid nonce_function_rfc6979 algo16 argument emulation.
70d4640 Make secp256k1_ec_pubkey_create skip processing invalid secret keys.
6c476a8 Minor comment improvements.
131afe5 Merge pull request bitcoin#334
0c6ab2f Introduce explicit lower-S normalization
fea19e7 Add contrib/lax_der_parsing.h
3bb9c44 Rewrite ECDSA signature parsing code
fa57f1b Use secp256k1_rand_int and secp256k1_rand_bits more
49b3749 Add new tests for the extra testrand functions
f684d7d Faster secp256k1_rand_int implementation
251b1a6 Improve testrand: add extra random functions
31994c8 Merge pull request bitcoin#338
f79aa88 Bugfix: swap arguments to noncefp
c98df26 Merge pull request bitcoin#319
67f7da4 Extensive interface and operations tests for secp256k1_ec_pubkey_parse.
ee2cb40 Add ARG_CHECKs to secp256k1_ec_pubkey_parse/secp256k1_ec_pubkey_serialize
7450ef1 Merge pull request bitcoin#328
68a3c76 Merge pull request bitcoin#329
98135ee Merge pull request bitcoin#332
37100d7 improve ECDH header-doc
b13d749 Fix couple of typos in API comments
7c823e3 travis: fixup module configs
cc3141a Merge pull request bitcoin#325
ee58fae Merge pull request bitcoin#326
213aa67 Do not force benchmarks to be statically linked.
338fc8b Add API exports to secp256k1_nonce_function_default and secp256k1_nonce_function_rfc6979.
52fd03f Merge pull request bitcoin#320
9f6993f Remove some dead code.
357f8cd Merge pull request bitcoin#314
118cd82 Use explicit symbol visibility.
4e64608 Include public module headers when compiling modules.
1f41437 Merge pull request bitcoin#316
fe0d463 Merge pull request bitcoin#317
cfe0ed9 Fix miscellaneous style nits that irritate overactive static analysis.
2b199de Use the explicit NULL macro for pointer comparisons.
9e90516 Merge pull request bitcoin#294
dd891e0 Get rid of _t as it is POSIX reserved
201819b Merge pull request bitcoin#313
912f203 Eliminate a few unbraced statements that crept into the code.
eeab823 Merge pull request bitcoin#299
486b9bb Use a flags bitfield for compressed option to secp256k1_ec_pubkey_serialize and secp256k1_ec_privkey_export
05732c5 Callback data: Accept pointers to either const or non-const data
1973c73 Bugfix: Reinitialise buffer lengths that have been used as outputs
788038d Use size_t for lengths (at least in external API)
c9d7c2a secp256k1_context_set_{error,illegal}_callback: Restore default handler by passing NULL as function argument
9aac008 secp256k1_context_destroy: Allow NULL argument as a no-op
64b730b secp256k1_context_create: Use unsigned type for flags bitfield
cb04ab5 Merge pull request bitcoin#309
a551669 Merge pull request bitcoin#295
81e45ff Update group_impl.h
85e3a2c Merge pull request bitcoin#112
b2eb63b Merge pull request bitcoin#293
dc0ce9f [API BREAK] Change argument order to out/outin/in
6d947ca Merge pull request bitcoin#298
c822693 Merge pull request bitcoin#301
6d04350 Merge pull request bitcoin#303
7ab311c Merge pull request bitcoin#304
5fb3229 Fixes a bug where bench_sign would fail due to passing in too small a buffer.
263dcbc remove unused assignment
b183b41 bugfix: "ARG_CHECK(ctx != NULL)" makes no sense
6da1446 build: fix parallel build
5eb4356 Merge pull request bitcoin#291
c996d53 Print success
9f443be Move pubkey recovery code to separate module
d49abbd Separate ECDSA recovery tests
439d34a Separate recoverable and normal signatures
a7b046e Merge pull request bitcoin#289
f66907f Improve/reformat API documentation secp256k1.h
2f77487 Add context building benchmarks
cc623d5 Merge pull request bitcoin#287
de7e398 small typo fix
9d96e36 Merge pull request bitcoin#280
432e1ce Merge pull request bitcoin#283
14727fd Use correct name in gitignore
356b0e9 Actually test static precomputation in Travis
ff3a5df Merge pull request bitcoin#284
2587208 Merge pull request bitcoin#212
a5a66c7 Add support for custom EC-Schnorr-SHA256 signatures
d84a378 Merge pull request bitcoin#252
72ae443 Improve perf. of cmov-based table lookup
92e53fc Implement endomorphism optimization for secp256k1_ecmult_const
ed35d43 Make `secp256k1_scalar_add_bit` conditional; make `secp256k1_scalar_split_lambda_var` constant time
91c0ce9 Add benchmarks for ECDH and const-time multiplication
0739bbb Add ECDH module which works by hashing the output of ecmult_const
4401500 Add constant-time multiply `secp256k1_ecmult_const` for ECDH
e4ce393 build: fix hard-coded usage of "gen_context"
b8e39ac build: don't use BUILT_SOURCES for the static context header
baa75da tests: add a couple tests
ae4f0c6 Merge pull request bitcoin#278
995c548 Introduce callback functions for dealing with errors.
c333074 Merge pull request bitcoin#282
18c329c Remove the internal secp256k1_ecdsa_sig_t type
74a2acd Add a secp256k1_ecdsa_signature_t type
23cfa91 Introduce secp256k1_pubkey_t type
4c63780 Merge pull request bitcoin#269
3e6f1e2 Change rfc6979 implementation to be a generic PRNG
ed5334a Update configure.ac to make it build on OpenBSD
1b68366 Merge pull request bitcoin#274
a83bb48 Make ecmult static precomputation default
166b32f Merge pull request bitcoin#276
c37812f Add gen_context src/ecmult_static_context.h to CLEANFILES to fix distclean.
125c15d Merge pull request bitcoin#275
76f6769 Fix build with static ecmult altroot and make dist.
5133f78 Merge pull request bitcoin#254
b0a60e6 Merge pull request bitcoin#258
733c1e6 Add travis build to test the static context.
fbecc38 Add ability to use a statically generated ecmult context.
4fb174d Merge pull request bitcoin#263
4ab8990 Merge pull request bitcoin#270
bdf0e0c Merge pull request bitcoin#271
31d0c1f Merge pull request bitcoin#273
eb2c8ff Add missing casts to SECP256K1_FE_CONST_INNER
55399c2 Further performance improvements to _ecmult_wnaf
99fd963 Add secp256k1_ec_pubkey_compress(), with test similar to the related decompress() function.
145cc6e Improve performance of _ecmult_wnaf
36b305a Verify the result of GMP modular inverse using non-GMP code
0cbc860 Merge pull request bitcoin#266
06ff7fe Merge pull request bitcoin#267
5a43124 Save 1 _fe_negate since s1 == -s2
a5d796e Update code comments
3f3964e Add specific VERIFY tests for _fe_cmov
7d054cd Refactor to save a _fe_negate
b28d02a Refactor to remove a local var
55e7fc3 Perf. improvement in _gej_add_ge
a0601cd Fix VERIFY calculations in _fe_cmov methods
17f7148 Merge pull request bitcoin#261
7657420 Add tests for adding P+Q with P.x!=Q.x and P.y=-Q.y
8c5d5f7 tests: Add failing unit test for bitcoin#257 (bad addition formula)
5de4c5d gej_add_ge: fix degenerate case when computing P + (-lambda)P
bcf2fcf gej_add_ge: rearrange algebra
e2a07c7 Fix compilation with C++
873a453 Merge pull request bitcoin#250
91eb0da Merge pull request bitcoin#247
210ffed Use separate in and out pointers in `secp256k1_ec_pubkey_decompress`
a1d5ae1 Tiny optimization
729badf Merge pull request bitcoin#210
2d5a186 Apply effective-affine trick to precomp
4f9791a Effective affine addition in EC multiplication
2b4cf41 Use pkg-config always when possible, with failover to manual checks for libcrypto

git-subtree-dir: src/secp256k1
git-subtree-split: d7eb1ae

dexX7 added a commit to dexX7/bitcoin that referenced this issue Nov 1, 2015

Merge pull request bitcoin#273
6012227 Remove "Experimental UI" label GUI splash screen (dexX7)

sipa added a commit to sipa/bitcoin that referenced this issue Nov 12, 2015

Squashed 'src/secp256k1/' changes from 22f60a6..2bfb82b
2bfb82b Merge pull request bitcoin#351
06aeea5 Turn secp256k1_ec_pubkey_serialize outlen to in/out
970164d Merge pull request bitcoin#348
6466625 Improvements for coordinate decompression
e2100ad Merge pull request bitcoin#347
8e48787 Change secp256k1_ec_pubkey_combine's count argument to size_t.
c69dea0 Clear output in more cases for pubkey_combine, adds tests.
269d422 Comment copyediting.
b4d17da Merge pull request bitcoin#344
4709265 Merge pull request bitcoin#345
26abce7 Adds 32 static test vectors for scalar mul, sqr, inv.
5b71a3f Better error case handling for pubkey_create & pubkey_serialize, more tests.
3b7bc69 Merge pull request bitcoin#343
eed87af Change contrib/laxder from headers-only to files compilable as standalone C
d7eb1ae Merge pull request bitcoin#342
7914a6e Make lax_der_privatekey_parsing.h not depend on internal code
73f64ff Merge pull request bitcoin#339
9234391 Overhaul flags handling
1a36898 Make flags more explicit, add runtime checks.
1a3e03a Merge pull request bitcoin#340
96be204 Add additional tests for eckey and arg-checks.
bb5aa4d Make the tweak function zeroize-output-on-fail behavior consistent.
4a243da Move secp256k1_ec_privkey_import/export to contrib.
1b3efc1 Move secp256k1_ecdsa_sig_recover into the recovery module.
e3cd679 Eliminate all side-effects from VERIFY_CHECK() usage.
b30fc85 Avoid nonce_function_rfc6979 algo16 argument emulation.
70d4640 Make secp256k1_ec_pubkey_create skip processing invalid secret keys.
6c476a8 Minor comment improvements.
131afe5 Merge pull request bitcoin#334
0c6ab2f Introduce explicit lower-S normalization
fea19e7 Add contrib/lax_der_parsing.h
3bb9c44 Rewrite ECDSA signature parsing code
fa57f1b Use secp256k1_rand_int and secp256k1_rand_bits more
49b3749 Add new tests for the extra testrand functions
f684d7d Faster secp256k1_rand_int implementation
251b1a6 Improve testrand: add extra random functions
31994c8 Merge pull request bitcoin#338
f79aa88 Bugfix: swap arguments to noncefp
c98df26 Merge pull request bitcoin#319
67f7da4 Extensive interface and operations tests for secp256k1_ec_pubkey_parse.
ee2cb40 Add ARG_CHECKs to secp256k1_ec_pubkey_parse/secp256k1_ec_pubkey_serialize
7450ef1 Merge pull request bitcoin#328
68a3c76 Merge pull request bitcoin#329
98135ee Merge pull request bitcoin#332
37100d7 improve ECDH header-doc
b13d749 Fix couple of typos in API comments
7c823e3 travis: fixup module configs
cc3141a Merge pull request bitcoin#325
ee58fae Merge pull request bitcoin#326
213aa67 Do not force benchmarks to be statically linked.
338fc8b Add API exports to secp256k1_nonce_function_default and secp256k1_nonce_function_rfc6979.
52fd03f Merge pull request bitcoin#320
9f6993f Remove some dead code.
357f8cd Merge pull request bitcoin#314
118cd82 Use explicit symbol visibility.
4e64608 Include public module headers when compiling modules.
1f41437 Merge pull request bitcoin#316
fe0d463 Merge pull request bitcoin#317
cfe0ed9 Fix miscellaneous style nits that irritate overactive static analysis.
2b199de Use the explicit NULL macro for pointer comparisons.
9e90516 Merge pull request bitcoin#294
dd891e0 Get rid of _t as it is POSIX reserved
201819b Merge pull request bitcoin#313
912f203 Eliminate a few unbraced statements that crept into the code.
eeab823 Merge pull request bitcoin#299
486b9bb Use a flags bitfield for compressed option to secp256k1_ec_pubkey_serialize and secp256k1_ec_privkey_export
05732c5 Callback data: Accept pointers to either const or non-const data
1973c73 Bugfix: Reinitialise buffer lengths that have been used as outputs
788038d Use size_t for lengths (at least in external API)
c9d7c2a secp256k1_context_set_{error,illegal}_callback: Restore default handler by passing NULL as function argument
9aac008 secp256k1_context_destroy: Allow NULL argument as a no-op
64b730b secp256k1_context_create: Use unsigned type for flags bitfield
cb04ab5 Merge pull request bitcoin#309
a551669 Merge pull request bitcoin#295
81e45ff Update group_impl.h
85e3a2c Merge pull request bitcoin#112
b2eb63b Merge pull request bitcoin#293
dc0ce9f [API BREAK] Change argument order to out/outin/in
6d947ca Merge pull request bitcoin#298
c822693 Merge pull request bitcoin#301
6d04350 Merge pull request bitcoin#303
7ab311c Merge pull request bitcoin#304
5fb3229 Fixes a bug where bench_sign would fail due to passing in too small a buffer.
263dcbc remove unused assignment
b183b41 bugfix: "ARG_CHECK(ctx != NULL)" makes no sense
6da1446 build: fix parallel build
5eb4356 Merge pull request bitcoin#291
c996d53 Print success
9f443be Move pubkey recovery code to separate module
d49abbd Separate ECDSA recovery tests
439d34a Separate recoverable and normal signatures
a7b046e Merge pull request bitcoin#289
f66907f Improve/reformat API documentation secp256k1.h
2f77487 Add context building benchmarks
cc623d5 Merge pull request bitcoin#287
de7e398 small typo fix
9d96e36 Merge pull request bitcoin#280
432e1ce Merge pull request bitcoin#283
14727fd Use correct name in gitignore
356b0e9 Actually test static precomputation in Travis
ff3a5df Merge pull request bitcoin#284
2587208 Merge pull request bitcoin#212
a5a66c7 Add support for custom EC-Schnorr-SHA256 signatures
d84a378 Merge pull request bitcoin#252
72ae443 Improve perf. of cmov-based table lookup
92e53fc Implement endomorphism optimization for secp256k1_ecmult_const
ed35d43 Make `secp256k1_scalar_add_bit` conditional; make `secp256k1_scalar_split_lambda_var` constant time
91c0ce9 Add benchmarks for ECDH and const-time multiplication
0739bbb Add ECDH module which works by hashing the output of ecmult_const
4401500 Add constant-time multiply `secp256k1_ecmult_const` for ECDH
e4ce393 build: fix hard-coded usage of "gen_context"
b8e39ac build: don't use BUILT_SOURCES for the static context header
baa75da tests: add a couple tests
ae4f0c6 Merge pull request bitcoin#278
995c548 Introduce callback functions for dealing with errors.
c333074 Merge pull request bitcoin#282
18c329c Remove the internal secp256k1_ecdsa_sig_t type
74a2acd Add a secp256k1_ecdsa_signature_t type
23cfa91 Introduce secp256k1_pubkey_t type
4c63780 Merge pull request bitcoin#269
3e6f1e2 Change rfc6979 implementation to be a generic PRNG
ed5334a Update configure.ac to make it build on OpenBSD
1b68366 Merge pull request bitcoin#274
a83bb48 Make ecmult static precomputation default
166b32f Merge pull request bitcoin#276
c37812f Add gen_context src/ecmult_static_context.h to CLEANFILES to fix distclean.
125c15d Merge pull request bitcoin#275
76f6769 Fix build with static ecmult altroot and make dist.
5133f78 Merge pull request bitcoin#254
b0a60e6 Merge pull request bitcoin#258
733c1e6 Add travis build to test the static context.
fbecc38 Add ability to use a statically generated ecmult context.
4fb174d Merge pull request bitcoin#263
4ab8990 Merge pull request bitcoin#270
bdf0e0c Merge pull request bitcoin#271
31d0c1f Merge pull request bitcoin#273
eb2c8ff Add missing casts to SECP256K1_FE_CONST_INNER
55399c2 Further performance improvements to _ecmult_wnaf
99fd963 Add secp256k1_ec_pubkey_compress(), with test similar to the related decompress() function.
145cc6e Improve performance of _ecmult_wnaf
36b305a Verify the result of GMP modular inverse using non-GMP code
0cbc860 Merge pull request bitcoin#266
06ff7fe Merge pull request bitcoin#267
5a43124 Save 1 _fe_negate since s1 == -s2
a5d796e Update code comments
3f3964e Add specific VERIFY tests for _fe_cmov
7d054cd Refactor to save a _fe_negate
b28d02a Refactor to remove a local var
55e7fc3 Perf. improvement in _gej_add_ge
a0601cd Fix VERIFY calculations in _fe_cmov methods
17f7148 Merge pull request bitcoin#261
7657420 Add tests for adding P+Q with P.x!=Q.x and P.y=-Q.y
8c5d5f7 tests: Add failing unit test for bitcoin#257 (bad addition formula)
5de4c5d gej_add_ge: fix degenerate case when computing P + (-lambda)P
bcf2fcf gej_add_ge: rearrange algebra
e2a07c7 Fix compilation with C++
873a453 Merge pull request bitcoin#250
91eb0da Merge pull request bitcoin#247
210ffed Use separate in and out pointers in `secp256k1_ec_pubkey_decompress`
a1d5ae1 Tiny optimization
729badf Merge pull request bitcoin#210
2d5a186 Apply effective-affine trick to precomp
4f9791a Effective affine addition in EC multiplication
2b4cf41 Use pkg-config always when possible, with failover to manual checks for libcrypto

git-subtree-dir: src/secp256k1
git-subtree-split: 2bfb82b
@laanwj

This comment has been minimized.

Member

laanwj commented Feb 16, 2016

First step toward this was implemented by @jonasschnelli in #6622, which puts up a upload limit per time span.
Once the new P2P code by @theuni is finished, work toward throttling can be started.

@rebroad

This comment has been minimized.

Contributor

rebroad commented Nov 13, 2016

@laanwj An upload limit per ASN might also be a useful feature - would help distribute the blockchain more fairly rather than allowing one ASN to use up all of the allowance. Also, using ASNs to calculate eviction of peers would be a good step towards protecting against bogons.

@laanwj

This comment has been minimized.

Member

laanwj commented Nov 21, 2016

Yes using the ASNs makes sense for a few things, including evictions and throttling. Though the practical problem that came up last time is that the ASN database is too large to include as-is. Maybe it could be approximated/encoded in some efficient way for querying though. Or maybe leave downloading it up to the user and make it optional.

@rebroad

This comment has been minimized.

Contributor

rebroad commented Nov 21, 2016

@laanwj oh, I had thought the outbound node selection logic was already using ASNs, but perhaps not - there is something there to "spread" the net wider though - perhaps worth a closer look at the logic to see if it's suitable to be used in the absence of an actual ASN database.

rebroad pushed a commit to rebroad/bitcoin that referenced this issue Dec 7, 2016

Squashed 'src/secp256k1/' changes from 22f60a6..6c527ec
6c527ec Merge pull request bitcoin#357
445f7f1 Fix for Windows compile issue
2bfb82b Merge pull request bitcoin#351
06aeea5 Turn secp256k1_ec_pubkey_serialize outlen to in/out
970164d Merge pull request bitcoin#348
6466625 Improvements for coordinate decompression
e2100ad Merge pull request bitcoin#347
8e48787 Change secp256k1_ec_pubkey_combine's count argument to size_t.
c69dea0 Clear output in more cases for pubkey_combine, adds tests.
269d422 Comment copyediting.
b4d17da Merge pull request bitcoin#344
4709265 Merge pull request bitcoin#345
26abce7 Adds 32 static test vectors for scalar mul, sqr, inv.
5b71a3f Better error case handling for pubkey_create & pubkey_serialize, more tests.
3b7bc69 Merge pull request bitcoin#343
eed87af Change contrib/laxder from headers-only to files compilable as standalone C
d7eb1ae Merge pull request bitcoin#342
7914a6e Make lax_der_privatekey_parsing.h not depend on internal code
73f64ff Merge pull request bitcoin#339
9234391 Overhaul flags handling
1a36898 Make flags more explicit, add runtime checks.
1a3e03a Merge pull request bitcoin#340
96be204 Add additional tests for eckey and arg-checks.
bb5aa4d Make the tweak function zeroize-output-on-fail behavior consistent.
4a243da Move secp256k1_ec_privkey_import/export to contrib.
1b3efc1 Move secp256k1_ecdsa_sig_recover into the recovery module.
e3cd679 Eliminate all side-effects from VERIFY_CHECK() usage.
b30fc85 Avoid nonce_function_rfc6979 algo16 argument emulation.
70d4640 Make secp256k1_ec_pubkey_create skip processing invalid secret keys.
6c476a8 Minor comment improvements.
131afe5 Merge pull request bitcoin#334
0c6ab2f Introduce explicit lower-S normalization
fea19e7 Add contrib/lax_der_parsing.h
3bb9c44 Rewrite ECDSA signature parsing code
fa57f1b Use secp256k1_rand_int and secp256k1_rand_bits more
49b3749 Add new tests for the extra testrand functions
f684d7d Faster secp256k1_rand_int implementation
251b1a6 Improve testrand: add extra random functions
31994c8 Merge pull request bitcoin#338
f79aa88 Bugfix: swap arguments to noncefp
c98df26 Merge pull request bitcoin#319
67f7da4 Extensive interface and operations tests for secp256k1_ec_pubkey_parse.
ee2cb40 Add ARG_CHECKs to secp256k1_ec_pubkey_parse/secp256k1_ec_pubkey_serialize
7450ef1 Merge pull request bitcoin#328
68a3c76 Merge pull request bitcoin#329
98135ee Merge pull request bitcoin#332
37100d7 improve ECDH header-doc
b13d749 Fix couple of typos in API comments
7c823e3 travis: fixup module configs
cc3141a Merge pull request bitcoin#325
ee58fae Merge pull request bitcoin#326
213aa67 Do not force benchmarks to be statically linked.
338fc8b Add API exports to secp256k1_nonce_function_default and secp256k1_nonce_function_rfc6979.
52fd03f Merge pull request bitcoin#320
9f6993f Remove some dead code.
357f8cd Merge pull request bitcoin#314
118cd82 Use explicit symbol visibility.
4e64608 Include public module headers when compiling modules.
1f41437 Merge pull request bitcoin#316
fe0d463 Merge pull request bitcoin#317
cfe0ed9 Fix miscellaneous style nits that irritate overactive static analysis.
2b199de Use the explicit NULL macro for pointer comparisons.
9e90516 Merge pull request bitcoin#294
dd891e0 Get rid of _t as it is POSIX reserved
201819b Merge pull request bitcoin#313
912f203 Eliminate a few unbraced statements that crept into the code.
eeab823 Merge pull request bitcoin#299
486b9bb Use a flags bitfield for compressed option to secp256k1_ec_pubkey_serialize and secp256k1_ec_privkey_export
05732c5 Callback data: Accept pointers to either const or non-const data
1973c73 Bugfix: Reinitialise buffer lengths that have been used as outputs
788038d Use size_t for lengths (at least in external API)
c9d7c2a secp256k1_context_set_{error,illegal}_callback: Restore default handler by passing NULL as function argument
9aac008 secp256k1_context_destroy: Allow NULL argument as a no-op
64b730b secp256k1_context_create: Use unsigned type for flags bitfield
cb04ab5 Merge pull request bitcoin#309
a551669 Merge pull request bitcoin#295
81e45ff Update group_impl.h
85e3a2c Merge pull request bitcoin#112
b2eb63b Merge pull request bitcoin#293
dc0ce9f [API BREAK] Change argument order to out/outin/in
6d947ca Merge pull request bitcoin#298
c822693 Merge pull request bitcoin#301
6d04350 Merge pull request bitcoin#303
7ab311c Merge pull request bitcoin#304
5fb3229 Fixes a bug where bench_sign would fail due to passing in too small a buffer.
263dcbc remove unused assignment
b183b41 bugfix: "ARG_CHECK(ctx != NULL)" makes no sense
6da1446 build: fix parallel build
5eb4356 Merge pull request bitcoin#291
c996d53 Print success
9f443be Move pubkey recovery code to separate module
d49abbd Separate ECDSA recovery tests
439d34a Separate recoverable and normal signatures
a7b046e Merge pull request bitcoin#289
f66907f Improve/reformat API documentation secp256k1.h
2f77487 Add context building benchmarks
cc623d5 Merge pull request bitcoin#287
de7e398 small typo fix
9d96e36 Merge pull request bitcoin#280
432e1ce Merge pull request bitcoin#283
14727fd Use correct name in gitignore
356b0e9 Actually test static precomputation in Travis
ff3a5df Merge pull request bitcoin#284
2587208 Merge pull request bitcoin#212
a5a66c7 Add support for custom EC-Schnorr-SHA256 signatures
d84a378 Merge pull request bitcoin#252
72ae443 Improve perf. of cmov-based table lookup
92e53fc Implement endomorphism optimization for secp256k1_ecmult_const
ed35d43 Make `secp256k1_scalar_add_bit` conditional; make `secp256k1_scalar_split_lambda_var` constant time
91c0ce9 Add benchmarks for ECDH and const-time multiplication
0739bbb Add ECDH module which works by hashing the output of ecmult_const
4401500 Add constant-time multiply `secp256k1_ecmult_const` for ECDH
e4ce393 build: fix hard-coded usage of "gen_context"
b8e39ac build: don't use BUILT_SOURCES for the static context header
baa75da tests: add a couple tests
ae4f0c6 Merge pull request bitcoin#278
995c548 Introduce callback functions for dealing with errors.
c333074 Merge pull request bitcoin#282
18c329c Remove the internal secp256k1_ecdsa_sig_t type
74a2acd Add a secp256k1_ecdsa_signature_t type
23cfa91 Introduce secp256k1_pubkey_t type
4c63780 Merge pull request bitcoin#269
3e6f1e2 Change rfc6979 implementation to be a generic PRNG
ed5334a Update configure.ac to make it build on OpenBSD
1b68366 Merge pull request bitcoin#274
a83bb48 Make ecmult static precomputation default
166b32f Merge pull request bitcoin#276
c37812f Add gen_context src/ecmult_static_context.h to CLEANFILES to fix distclean.
125c15d Merge pull request bitcoin#275
76f6769 Fix build with static ecmult altroot and make dist.
5133f78 Merge pull request bitcoin#254
b0a60e6 Merge pull request bitcoin#258
733c1e6 Add travis build to test the static context.
fbecc38 Add ability to use a statically generated ecmult context.
4fb174d Merge pull request bitcoin#263
4ab8990 Merge pull request bitcoin#270
bdf0e0c Merge pull request bitcoin#271
31d0c1f Merge pull request bitcoin#273
eb2c8ff Add missing casts to SECP256K1_FE_CONST_INNER
55399c2 Further performance improvements to _ecmult_wnaf
99fd963 Add secp256k1_ec_pubkey_compress(), with test similar to the related decompress() function.
145cc6e Improve performance of _ecmult_wnaf
36b305a Verify the result of GMP modular inverse using non-GMP code
0cbc860 Merge pull request bitcoin#266
06ff7fe Merge pull request bitcoin#267
5a43124 Save 1 _fe_negate since s1 == -s2
a5d796e Update code comments
3f3964e Add specific VERIFY tests for _fe_cmov
7d054cd Refactor to save a _fe_negate
b28d02a Refactor to remove a local var
55e7fc3 Perf. improvement in _gej_add_ge
a0601cd Fix VERIFY calculations in _fe_cmov methods
17f7148 Merge pull request bitcoin#261
7657420 Add tests for adding P+Q with P.x!=Q.x and P.y=-Q.y
8c5d5f7 tests: Add failing unit test for bitcoin#257 (bad addition formula)
5de4c5d gej_add_ge: fix degenerate case when computing P + (-lambda)P
bcf2fcf gej_add_ge: rearrange algebra
e2a07c7 Fix compilation with C++
873a453 Merge pull request bitcoin#250
91eb0da Merge pull request bitcoin#247
210ffed Use separate in and out pointers in `secp256k1_ec_pubkey_decompress`
a1d5ae1 Tiny optimization
729badf Merge pull request bitcoin#210
2d5a186 Apply effective-affine trick to precomp
4f9791a Effective affine addition in EC multiplication
2b4cf41 Use pkg-config always when possible, with failover to manual checks for libcrypto

git-subtree-dir: src/secp256k1
git-subtree-split: 6c527ec

CodeShark pushed a commit to CodeShark/bitcoin that referenced this issue Jan 18, 2017

Added testnet DNS Seed (bitcoin#273)
* Added testnet DNS Seed

deadalnix pushed a commit to deadalnix/bitcoin that referenced this issue Jan 19, 2017

Merge pull request bitcoin#273
eb2c8ff Add missing casts to SECP256K1_FE_CONST_INNER (Andrew Poelstra)

ptschip pushed a commit to ptschip/bitcoin that referenced this issue Feb 17, 2017

Merge pull request bitcoin#273 from ptschip/dev_ischain
Use std::atomic for IsChainNearlySyncd()
@earonesty

This comment has been minimized.

earonesty commented May 19, 2017

I think the new sync code does a better job of distributing the load among peers. Throttling should be less of an issue now.

jcansdale pushed a commit to jcansdale/bitcoin that referenced this issue Oct 5, 2017

Merge pull request bitcoin#273 from 1337ninja/dev
Refactoring: removing using namespace std from src/wallet

classesjack pushed a commit to classesjack/bitcoin that referenced this issue Jan 2, 2018

@andrelam

This comment has been minimized.

andrelam commented Feb 1, 2018

@earonesty it seems it still is.

@jlopp

This comment has been minimized.

Contributor

jlopp commented Mar 4, 2018

Technically, this issue has been addressed with the addition of the "maxuploadtarget" config parameter. Not sure if that parameter has been exposed in the GUI, though.

@jonasschnelli

This comment has been minimized.

Member

jonasschnelli commented Mar 5, 2018

maxuploadtarget has not yet been exposed to the GUI.
Also, once we have libevent for the p2p layer, network throttling should be a low hanging fruit.
Though, I personally think, network throttling should be done a layer deeper then the application layer (router level), since, the application does not know what else – running in the same network – requires bandwidth.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment