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

Add -keepnode which attempts to -addnode and keep a connection open #454

Merged
merged 2 commits into from
Jan 16, 2012

Conversation

TheBlueMatt
Copy link
Contributor

Got tired of telling pool ops to not use hub mode and use -addnode and adding a little asterisk that it would be nice if addnode kept the node connected as a "Bitcoin backbone" should do, so I hacked an implementation together.

@luke-jr
Copy link
Member

luke-jr commented Dec 2, 2011

ACK

@TheBlueMatt
Copy link
Contributor Author

Updated version, now with less redundant code and less potential to segfault in very rare cases.

@TheBlueMatt
Copy link
Contributor Author

After discussion on #bitcoin-dev, no more exponential backoff and retry connections every 2 minutes.

@gavinandresen
Copy link
Contributor

Is there a good reason for this to be a separate switch? I think I'd prefer if we just changed the -addnode behavior to stay connected.

@TheBlueMatt
Copy link
Contributor Author

not really, I just always prefer to not change existing behavior, but if there is a consensus that -addnode in its current for is unnecessary, thats fine.

@TheBlueMatt
Copy link
Contributor Author

-addnode removed, s/keepnode/addnode/

@luke-jr
Copy link
Member

luke-jr commented Jan 12, 2012

This needs rebasing again. Not trivial this time. :(

@TheBlueMatt
Copy link
Contributor Author

Rebased, and it was trivial.

@luke-jr
Copy link
Member

luke-jr commented Jan 12, 2012

No, it isn't that trivial. This still does not build.

@TheBlueMatt
Copy link
Contributor Author

OK, theres an actually useful version.

@luke-jr
Copy link
Member

luke-jr commented Jan 13, 2012

The cleanup really doesn't belong here, but it looks fine to me... ACK on 10ba0a3 (Minor cleanup...)

@TheBlueMatt
Copy link
Contributor Author

Sorry, I wrote that patch while writing keepnode and its way too small to warrant its own pull request, so I just left it in here...

@gavinandresen
Copy link
Contributor

This doesn't work for me.

How I tested:

3 testnet-in-a-box nodes, a/b/c listening on ports 8900,8901,8902

b -connects to a
c -connects to a and also -addnode's b

Test: start them all up, verify c's connections.
Shutdown b.
Then restart b, and see how long it takes c to reconnect.
I waited a long time.

I think the culprit is:
Sleep(120000); // Retry every 2 minutes

... which I believe is 20 minutes, not two.

@TheBlueMatt
Copy link
Contributor Author

No, 120,000 is correct (http://www.wolframalpha.com/input/?i=2+minutes+to+milliseconds), also it worked fine for me...
I just ran two regular nodes with -connect=0.0.0.0 and one with -addnode=127.0.0.1:8334 (the second started after the first) and 2 minutes after it first tried its connection, it connected just fine.
Care to run in gdb?

@gavinandresen
Copy link
Contributor

ACK-- problem with my test is elsewhere in the networking code, I can't make more than one connection from daemons running on my machine on different ports.

@gavinandresen gavinandresen merged commit 10ba0a3 into bitcoin:master Jan 16, 2012
zathras-crypto pushed a commit to zathras-crypto/omnicore that referenced this pull request Feb 3, 2017
1773613 Fix two Omni Core related build warnings (dexX7)
ptschip pushed a commit to ptschip/bitcoin that referenced this pull request May 2, 2017
sipa added a commit to sipa/bitcoin that referenced this pull request May 5, 2017
84973d3 Merge bitcoin#454: Remove residual parts from the schnorr expirement.
5e95bf2 Remove residual parts from the schnorr expirement.
cbc20b8 Merge bitcoin#452: Minor optimizations to _scalar_inverse to save 4M
4cc8f52 Merge bitcoin#437: Unroll secp256k1_fe_(get|set)_b32 to make them much faster.
465159c Further shorten the addition chain for scalar inversion.
a2b6b19 Fix benchmark print_number infinite loop.
8b7680a Unroll secp256k1_fe_(get|set)_b32 for 10x26.
aa84990 Unroll secp256k1_fe_(get|set)_b32 for 5x52.
cf12fa1 Minor optimizations to _scalar_inverse to save 4M
1199492 Merge bitcoin#408: Add `secp256k1_ec_pubkey_negate` and `secp256k1_ec_privkey_negate`
6af0871 Merge bitcoin#441: secp256k1_context_randomize: document.
ab31a52 Merge bitcoin#444: test: Use checked_alloc
eda5c1a Merge bitcoin#449: Remove executable bit from secp256k1.c
51b77ae Remove executable bit from secp256k1.c
5eb030c test: Use checked_alloc
72d952c FIXUP: Missing "is"
70ff29b secp256k1_context_randomize: document.
9d560f9 Merge bitcoin#428: Exhaustive recovery
8e48aa6 Add `secp256k1_ec_pubkey_negate` and `secp256k1_ec_privkey_negate`
2cee5fd exhaustive tests: add recovery module
678b0e5 exhaustive tests: remove erroneous comment from ecdsa_sig_sign
03ff8c2 group_impl.h: remove unused `secp256k1_ge_set_infinity` function
a724d72 configure: add --enable-coverage to set options for coverage analysis
b595163 recovery: add tests to cover API misusage
6f8ae2f ecdh: test NULL-checking of arguments
25e3cfb ecdsa_impl: replace scalar if-checks with VERIFY_CHECKs in ecdsa_sig_sign

git-subtree-dir: src/secp256k1
git-subtree-split: 84973d3
protonn pushed a commit to argentumproject/argentum that referenced this pull request Nov 1, 2017
lateminer pushed a commit to lateminer/bitcoin that referenced this pull request Jan 22, 2019
1e6f1f5 Merge bitcoin#529: fix tests.c in the count == 0 case
95e99f1 fix tests.c in the count == 0 case
452d8e4 Merge bitcoin#523: scratch: add stack frame support
6fe5043 scratch: add stack frame support
9bc2e26 Merge bitcoin#522: parameterize ecmult_const over input size
7c1b91b parameterize ecmult_const over input size
dbc3ddd Merge bitcoin#513: Increase sparsity of pippenger fixed window naf representation
fb9271d Merge bitcoin#510: add a couple missing `const`s to ecmult_pippenger_wnaf
cd5f602 Merge bitcoin#515: Fix typo
09146ae Merge bitcoin#512: secp256k1_ec_privkey_negate - fix documentation
ec0a7b3 Don't touch leading zeros in wnaf_fixed.
9e36d1b Fix bug in wnaf_fixed where the wnaf array is not completely zeroed when given a 0 scalar.
96f68a0 Don't invert scalar in wnaf_fixed when it is even because a caller might intentionally give a scalar with many leading zeros.
9b7c47a Fix typo
6dbb007 Increase sparsity of pippenger fixed window naf representation
1646ace secp256k1_ec_privkey_negate - fix documentation
9b3ff03 add a couple missing `const`s to ecmult_pippenger_wnaf
cd329db Merge bitcoin#460: [build] Update ax_jni_include_dir.m4 macro
7f9c1a1 Merge bitcoin#498: tests: Avoid calling fclose(...) with an invalid argument
f99aa8d Merge bitcoin#499: tests: Make sure we get the requested number of bytes from /dev/urandom
b549d3d Merge bitcoin#472: [build] Set --enable-jni to no by default instead of auto.
d333521 Merge bitcoin#494: Support OpenSSL versions >= 1.1 for ENABLE_OPENSSL_TESTS
2ef8ea5 Merge bitcoin#495: Add bench_ecmult to .gitignore
82a96e4 tests: Make sure we get the requested number of bytes from /dev/urandom
5aae5b5 Avoid calling fclose(...) with an invalid argument
cb32940 Add bench_ecmult to .gitignore
31abd3a Support OpenSSL versions >= 1.1 for ENABLE_OPENSSL_TESTS
c95f6f1 Merge bitcoin#487: fix tests typo, s/changed/unchanged
fb46c83 Merge bitcoin#463: Reduce usage of hardcoded size constants
02f5001 Merge bitcoin#490: Disambiguate bench functions and types
1f46d60 Disambiguate bench functions and types
f54c6c5 Merge bitcoin#480: Enable benchmark building by default
c77fc08 Merge bitcoin#486: Add pippenger_wnaf for multi-multiplication
d2f9c6b Use more precise pippenger bucket windows
4c950bb Save some additions per window in _pippenger_wnaf
a58f543 Add flags for choosing algorithm in ecmult_multi benchmark
36b22c9 Use scratch space dependent batching in ecmult_multi
355a38f Add pippenger_wnaf ecmult_multi
bc65aa7 Add bench_ecmult
dba5471 Add ecmult_multi tests
8c1c831 Generalize Strauss to support multiple points
548de42 add resizeable scratch space API
0e96cdc fix typo, s/changed/unchanged
c7680e5 Reduce usage of hardcoded size constants
6ad5cdb Merge bitcoin#479: Get rid of reserved _t in type names
7a78f60 Print whether we're building benchmarks
4afec9f Build benchmarks by default
d1dc9df Get rid of reserved _t in type names
0b70241 Merge bitcoin#474: Fix header guards using reserved identifiers
ab1f89f Merge bitcoin#478: Fixed multiple typos
8c7ea22 Fixed multiple typos
abe2d3e Fix header guards using reserved identifiers
57752d2 [build] Set --enable-jni to no by default instead of auto.
f532bdc Merge bitcoin#459: Add pubkey prefix constants to include/secp256k1.h
cac7c55 Merge bitcoin#470: Fix wnaf_const documentation
768514b Fix wnaf_const documentation with respect to return value and number of words set
b8c26a3 Merge bitcoin#458: Fix typo in API documentation
817fb20 Merge bitcoin#440: Fix typos
12230f9 Merge bitcoin#468: Remove redundant conditional expression
2e1ccdc Remove redundant conditional expression
e7daa9b [build] Tweak JNI macro to warn instead of error for JNI not found.
5b22977 [build] Update ax_jni_include_dir.m4 macro to deal with recent versions of macOS
bc61b91 add pubkey prefix constants to include/secp256k1.h
b0452e6 Fix typo in API documentation
84973d3 Merge bitcoin#454: Remove residual parts from the schnorr expirement.
5e95bf2 Remove residual parts from the schnorr expirement.
cbc20b8 Merge bitcoin#452: Minor optimizations to _scalar_inverse to save 4M
4cc8f52 Merge bitcoin#437: Unroll secp256k1_fe_(get|set)_b32 to make them much faster.
465159c Further shorten the addition chain for scalar inversion.
a2b6b19 Fix benchmark print_number infinite loop.
8b7680a Unroll secp256k1_fe_(get|set)_b32 for 10x26.
aa84990 Unroll secp256k1_fe_(get|set)_b32 for 5x52.
cf12fa1 Minor optimizations to _scalar_inverse to save 4M
1199492 Merge bitcoin#408: Add `secp256k1_ec_pubkey_negate` and `secp256k1_ec_privkey_negate`
6af0871 Merge bitcoin#441: secp256k1_context_randomize: document.
ab31a52 Merge bitcoin#444: test: Use checked_alloc
eda5c1a Merge bitcoin#449: Remove executable bit from secp256k1.c
51b77ae Remove executable bit from secp256k1.c
5eb030c test: Use checked_alloc
72d952c FIXUP: Missing "is"
70ff29b secp256k1_context_randomize: document.
4c0f32e Fix typo: "Agressive" → "Aggressive"
73aca83 Fix typo: "exectured" → "executed"
8e48aa6 Add `secp256k1_ec_pubkey_negate` and `secp256k1_ec_privkey_negate`

git-subtree-dir: src/secp256k1
git-subtree-split: 1e6f1f5
fjahr pushed a commit to fjahr/bitcoin that referenced this pull request Jul 24, 2019
5e95bf2 Remove residual parts from the schnorr expirement. (Gregory Maxwell)

Tree-SHA512: de1e56cc54443e29a60787996a1b1381b0b84eacb87a8f1af06b5ba3900b1771c3a04fd547c65e21979e3c08c3a45d258d699eb951a956f8e617833c5396ecfe
@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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants