| Age | Commit message (Collapse) | Author |
|---|
|
|
|
|
|
`nLocalServices`
1b5bec78e942473b6ffac4004b1cf6d535fd2892 init: avoid unsetting service bits from `nLocalServices` (Sebastian Falbesoner)
Pull request description:
This PR is a late follow-up to the [review club session about the PR "Default to NODE_WITNESS in nLocalServices" ](https://bitcoincore.reviews/21090#l-90) (#21090):
```
17:32 <lightlike> hmm, if we are in pruned mode, we first set NODE_NETWORK and then unset it later in init.cpp. that seems a bit strange.
...
17:33 <jnewbery> lightlike: ah yes, you're right. That does seem a bit messy.
```
Rather than setting the service bit `NODE_NETWORK` first and then unset it (if in `fPruneMode`), start with the bare minimum flags that we always serve and only add `NODE_NETWORK` if we are running as a non-pruned node. This seems to be a more logical approach than currently on master.
ACKs for top commit:
naumenkogs:
ACK 1b5bec78e942473b6ffac4004b1cf6d535fd2892
stickies-v:
ACK 1b5bec78e942473b6ffac4004b1cf6d535fd2892
LarryRuane:
ACK 1b5bec78e942473b6ffac4004b1cf6d535fd2892
Tree-SHA512: 2e82d66c4298ffacff41d9e0458b74b83bc156a1fa49e3f3471e942878e5dd2b253b5597ee5ec1d9c8726b432751d05e40f0c580f3976a9e00a7d1f417921ab0
|
|
Google Code to Github)
2c05dc7811db4fe61d7f30fc9b46c374f75226c5 Fix link to MurmurHash3.cpp from Austin Appleby (dontbyte)
Pull request description:
Google Code repo doesn't exist anymore
ACKs for top commit:
Zero-1729:
crACK 2c05dc7811db4fe61d7f30fc9b46c374f75226c5
Tree-SHA512: 3e095255757b536f382ffb63e4292413592246c2446d486acbb71c52e4a3ece519d7cfae941685d9e25fd62de5c783510b3d076cd990a3d391496dc3076a0385
|
|
where needed
89576ccc572fcaf9fb7117ad6124482cc95fbd9f refactor: add LIFETIMEBOUND to blockfilter where needed (stickies-v)
Pull request description:
Noticed from https://github.com/bitcoin/bitcoin/pull/25637#issuecomment-1231860822 that [`BlockFilter::GetFilter()`](https://github.com/bitcoin/bitcoin/blob/01e1627e25bc5477c40f51da03c3c31b609a85c9/src/blockfilter.h#L132) returns a reference to a member variable. Added LIFETIMEBOUND to all blockfilter-related code to ensure that the return values do not have a lifetime that exceeds the lifetime of what it is bound to. See https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#lifetimebound or https://github.com/bitcoin/bitcoin/pull/25060 for a similar example.
I used `grep -E '[a-zA-Z>0-9][&*] ([a-zA-Z]*)\((.*)\)' src/**/blockfilter*` to grep all possible occurrences (not all of them require LIFETIMEBOUND)
ACKs for top commit:
brunoerg:
crACK 89576ccc572fcaf9fb7117ad6124482cc95fbd9f
Tree-SHA512: 6fe61fc0c1ed9e446edce083d1b093e1a5e2ef8c39ff74125bb12a24e514d45711845809817fbd4a04d7a9c23c8b362203771c17b6d831d2560b1af268453019
|
|
94af3e43e20aa00b18e7a3f6d0f5fe3ad9494d97 Fix typo from PR25717 (Suhas Daftuar)
e5982ecdc4650e9b6de38f190f3a97d792499e2a Bypass headers anti-DoS checks for NoBan peers (Suhas Daftuar)
132ed7eaaa4a47ab94db72ebfab0ef0e03caa488 Move headerssync logging to BCLog::NET (Suhas Daftuar)
Pull request description:
Remove BCLog::HEADERSSYNC and move all headerssync logging to BCLog::NET.
Bypass headers anti-DoS checks for NoBan peers
Also fix a typo that was introduced in PR25717.
ACKs for top commit:
Sjors:
tACK 94af3e43e20aa00b18e7a3f6d0f5fe3ad9494d97
ajtowns:
ACK 94af3e43e20aa00b18e7a3f6d0f5fe3ad9494d97
sipa:
ACK 94af3e43e20aa00b18e7a3f6d0f5fe3ad9494d97
naumenkogs:
ACK 94af3e43e20aa00b18e7a3f6d0f5fe3ad9494d97
w0xlt:
ACK https://github.com/bitcoin/bitcoin/pull/25960/commits/94af3e43e20aa00b18e7a3f6d0f5fe3ad9494d97
Tree-SHA512: 612d594eddace977359bcc8234b2093d273fd50662f4ac70cb90903d28fb831f6e1aecff51a4ef6c0bb0f6fb5d1aa7ff1eb8798fac5ac142783788f3080717dc
|
|
Ensure that the return values do not have a lifetime that exceeds
the lifetime of what it is bound to.
See https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#lifetimebound
|
|
tidy fixup
6b24dfe24d2ed50ea0b06ce1919db3dc0e871a03 CBlockLocator: performance-move-const-arg Clang tidy fixups (Jon Atack)
Pull request description:
Fix Clang-tidy CI errors on master. See https://cirrus-ci.com/task/4806752200818688?logs=ci#L4696 for an example.
ACKs for top commit:
MarcoFalke:
review ACK 6b24dfe24d2ed50ea0b06ce1919db3dc0e871a03
vasild:
ACK 6b24dfe24d2ed50ea0b06ce1919db3dc0e871a03
Tree-SHA512: 7a67acf7b42da07b63fbb392236e9a7be8cf35c36e37ca980c4467fe8295c2eda8aef10f41a1e3036cd9ebece47fa957fc3256033f853bd6a97ce2ca42799a0a
|
|
Co-authored-by: "Pieter Wuille <pieter@wuille.net>"
Co-authored-by: "Vasil Dimov <vd@FreeBSD.org>"
Co-authored-by: "MarcoFalke <falke.marco@gmail.com>"
|
|
|
|
|
|
fa875349e22f2f0f9c2c98ee991372d08ff90318 Fix iwyu (MacroFake)
faad673716cfbad1e715f1bdf8ac00938a055aea Fix issues when calling std::move(const&) (MacroFake)
Pull request description:
Passing a symbol to `std::move` that is marked `const` is a no-op, which can be fixed in two ways:
* Remove the `const`, or
* Remove the `std::move`
ACKs for top commit:
ryanofsky:
Code review ACK fa875349e22f2f0f9c2c98ee991372d08ff90318. Looks good. Good for univalue to support c++11 move optimizations
Tree-SHA512: 3dc5cad55b93cfa311abedfb811f35fc1b7f30a1c68561f15942438916c7de25e179c364be11881e01f844f9c2ccd71a3be55967ad5abd2f35b10bb7a882edea
|
|
|
|
|
|
f345dc3960c2cf4d69ebbcc011e4e836205f0361 tidy: enable bugprone-use-after-move (fanquake)
94f2235f858bc4fdaf0ab0882599f6a228401cf5 test: work around bugprone-use-after-move warnings in util tests (fanquake)
Pull request description:
Would have caught #25640.
Currently `// NOLINT`s around:
```bash
test/util_tests.cpp:2513:34: error: 't2' used after it was moved [bugprone-use-after-move,-warnings-as-errors]
BOOST_CHECK(v2[0].origin == &t2);
^
test/util_tests.cpp:2511:15: note: move occurred here
auto v2 = Vector(std::move(t2));
^
test/util_tests.cpp:2519:34: error: 't2' used after it was moved [bugprone-use-after-move,-warnings-as-errors]
BOOST_CHECK(v3[1].origin == &t2);
^
test/util_tests.cpp:2516:15: note: move occurred here
auto v3 = Vector(t1, std::move(t2));
^
test/util_tests.cpp:2527:34: error: 't3' used after it was moved [bugprone-use-after-move,-warnings-as-errors]
BOOST_CHECK(v4[2].origin == &t3);
^
test/util_tests.cpp:2523:15: note: move occurred here
auto v4 = Vector(std::move(v3[0]), v3[1], std::move(t3));
```
See: https://releases.llvm.org/14.0.0/tools/clang/tools/extra/docs/clang-tidy/checks/bugprone-use-after-move.html
ACKs for top commit:
ryanofsky:
Code review ACK f345dc3960c2cf4d69ebbcc011e4e836205f0361. Only change since last review is switching to NOLINT directives
Tree-SHA512: afadecbaf1069653f4be5d6e66a5800ffd975c0b1a960057abc6367b616c181cd518897a874a8f3fd5e5e1f45fcc165f7a9a3171136cd4deee641214c4b765b8
|
|
|
|
3add23454624c4c79c9eebc060b6fbed4e3131a7 ui: show header pre-synchronization progress (Pieter Wuille)
738421c50f2dbd7395b50a5dbdf6168b07435e62 Emit NotifyHeaderTip signals for pre-synchronization progress (Pieter Wuille)
376086fc5a187f5b2ab3a0d1202ed4e6c22bdb50 Make validation interface capable of signalling header presync (Pieter Wuille)
93eae27031a65b4156df49015ae45b2b541b4e5a Test large reorgs with headerssync logic (Suhas Daftuar)
355547334f7d08640ee1fa291227356d61145d1a Track headers presync progress and log it (Pieter Wuille)
03712dddfbb9fe0dc7a2ead53c65106189f5c803 Expose HeadersSyncState::m_current_height in getpeerinfo() (Suhas Daftuar)
150a5486db50ff77c91765392149000029c8a309 Test headers sync using minchainwork threshold (Suhas Daftuar)
0b6aa826b53470c9cc8ef4a153fa710dce80882f Add unit test for HeadersSyncState (Suhas Daftuar)
83c6a0c5249c4ecbd11f7828c84a50fb473faba3 Reduce spurious messages during headers sync (Suhas Daftuar)
ed6cddd98e32263fc116a4380af6d66da20da990 Require callers of AcceptBlockHeader() to perform anti-dos checks (Suhas Daftuar)
551a8d957c4c44afbd0d608fcdf7c6a4352babce Utilize anti-DoS headers download strategy (Suhas Daftuar)
ed470940cddbeb40425960d51cefeec4948febe4 Add functions to construct locators without CChain (Pieter Wuille)
84852bb6bb3579e475ce78fe729fd125ddbc715f Add bitdeque, an std::deque<bool> analogue that does bit packing. (Pieter Wuille)
1d4cfa4272cf2c8b980cc8762c1ff2220d3e8d51 Add function to validate difficulty changes (Suhas Daftuar)
Pull request description:
New nodes starting up for the first time lack protection against DoS from low-difficulty headers. While checkpoints serve as our protection against headers that fork from the main chain below the known checkpointed values, this protection only applies to nodes that have been able to download the honest chain to the checkpointed heights.
We can protect all nodes from DoS from low-difficulty headers by adopting a different strategy: before we commit to storing a header in permanent storage, first verify that the header is part of a chain that has sufficiently high work (either `nMinimumChainWork`, or something comparable to our tip). This means that we will download headers from a given peer twice: once to verify the work on the chain, and a second time when permanently storing the headers.
The p2p protocol doesn't provide an easy way for us to ensure that we receive the same headers during the second download of peer's headers chain. To ensure that a peer doesn't (say) give us the main chain in phase 1 to trick us into permanently storing an alternate, low-work chain in phase 2, we store commitments to the headers during our first download, which we validate in the second download.
Some parameters must be chosen for commitment size/frequency in phase 1, and validation of commitments in phase 2. In this PR, those parameters are chosen to both (a) minimize the per-peer memory usage that an attacker could utilize, and (b) bound the expected amount of permanent memory that an attacker could get us to use to be well-below the memory growth that we'd get from the honest chain (where we expect 1 new block header every 10 minutes).
After this PR, we should be able to remove checkpoints from our code, which is a nice philosophical change for us to make as well, as there has been confusion over the years about the role checkpoints play in Bitcoin's consensus algorithm.
Thanks to Pieter Wuille for collaborating on this design.
ACKs for top commit:
Sjors:
re-tACK 3add23454624c4c79c9eebc060b6fbed4e3131a7
mzumsande:
re-ACK 3add23454624c4c79c9eebc060b6fbed4e3131a7
sipa:
re-ACK 3add23454624c4c79c9eebc060b6fbed4e3131a7
glozow:
ACK 3add234546
Tree-SHA512: e7789d65f62f72141b8899eb4a2fb3d0621278394d2d7adaa004675250118f89a4e4cb42777fe56649d744ec445ad95141e10f6def65f0a58b7b35b2e654a875
|
|
Will error with:
```bash
coins.cpp:102:22: error: 'coin' used after it was moved [bugprone-use-after-move,-warnings-as-errors]
(uint32_t)coin.nHeight,
^
coins.cpp:96:21: note: move occurred here
it->second.coin = std::move(coin);
```
until #25663 is merged.
See:
https://releases.llvm.org/14.0.0/tools/clang/tools/extra/docs/clang-tidy/checks/bugprone-use-after-move.html
|
|
```bash
test/util_tests.cpp:2513:34: error: 't2' used after it was moved [bugprone-use-after-move,-warnings-as-errors]
BOOST_CHECK(v2[0].origin == &t2);
^
test/util_tests.cpp:2511:15: note: move occurred here
auto v2 = Vector(std::move(t2));
^
test/util_tests.cpp:2519:34: error: 't2' used after it was moved [bugprone-use-after-move,-warnings-as-errors]
BOOST_CHECK(v3[1].origin == &t2);
^
test/util_tests.cpp:2516:15: note: move occurred here
auto v3 = Vector(t1, std::move(t2));
^
test/util_tests.cpp:2527:34: error: 't3' used after it was moved [bugprone-use-after-move,-warnings-as-errors]
BOOST_CHECK(v4[2].origin == &t3);
^
test/util_tests.cpp:2523:15: note: move occurred here
auto v4 = Vector(std::move(v3[0]), v3[1], std::move(t3));
```
|
|
Google Code repo doesn't exist anymore
|
|
They don't have any private data and they can't be nested so they
should return false for ToPrivateString.
|
|
|
|
|
|
|
|
Both of these functions do almost the exact same thing. They can be
deduplicated so that their behavior matches except for the filtering
aspect. As this function will now always be called on wallet loading,
nNextResend will also always be initialized, so
wallet_resendwallettransactions.py is updated to account for that.
This also resolves a bug where ResendWalletTransactions would fail to
rebroadcast txs in insertion order thereby potentially rebroadcasting a
child transaction before its parent and causing the child to not
actually get rebroadcast.
Also names the combined function to ResubmitWalletTransactions as the
function just submits the transactions to the mempool rather than doing
any sending by itself.
|
|
m_permissionFlags and m_prefer_evict are treated as const -- they're
only set immediately after construction before any other thread has
access to the object, and not changed again afterwards. As such they
don't need to be marked atomic or guarded by a mutex; though it would
probably be better to actually mark them as const...
|
|
Dereferencing a unique_ptr is not necessarily thread safe. The reason
these are safe is because their values are set at construction and do
not change later; so mark them as const and set them via the initializer
list to guarantee that.
|
|
The (V1)TransportSerializer instance CNode::m_serializer is used from
multiple threads via PushMessage without protection by a mutex. This
is only thread safe because the class does not have any mutable state,
so document that by marking the methods and the object as "const".
|
|
|
|
|
|
|
|
This makes a number of changes:
- Get rid of the verification_progress argument in the node interface
NotifyHeaderTip (it was always 0.0).
- Instead of passing a CBlockIndex* in the UI interface's NotifyHeaderTip,
send separate height, timestamp fields. This is becuase in headers presync,
no actual CBlockIndex object is available.
- Add a bool presync argument to both of the above, to identify signals
pertaining to the first headers sync phase.
|
|
|
|
|
|
|
|
Delay sending SENDHEADERS (BIP 130) message until we know our peer's best
header's chain has more than nMinimumChainWork. This reduces inadvertent
headers messages received during initial headers sync due to block
announcements, which throw off our sync algorithm.
|
|
In order to prevent memory DoS, we must ensure that we don't accept a new
header into memory until we've performed anti-DoS checks, such as verifying
that the header is part of a sufficiently high work chain. This commit adds a
new argument to AcceptBlockHeader() so that we can ensure that all call-sites
which might cause a new header to be accepted into memory have to grapple with
the question of whether the header is safe to accept, or needs further
validation.
This patch also fixes two places where low-difficulty-headers could have been
processed without such validation (processing an unrequested block from the
network, and processing a compact block).
Credit to Niklas Gögge for noticing this issue, and thanks to Sjors Provoost
for test code.
|
|
Avoid permanently storing headers from a peer, unless the headers are part of a
chain with sufficiently high work. This prevents memory attacks using low-work
headers.
Designed and co-authored with Pieter Wuille.
|
|
|
|
- remove unreachable seeds
- add reachable seeds having service bit 1
- update chainparamsseeds.h
Co-authored-by: "Stacie <staciewaleyko@gmail.com>"
|
|
5ef8c2c9fc4ebce6cbfea6a55a89a0ab7ee98a1a test: fix typo for MaybeResendWalletTxs (stickies-v)
fbba4a131647c991afc53b6a3dfb9721f5c430b2 wallet: trigger MaybeResendWalletTxs() every minute (stickies-v)
Pull request description:
ResendWalletTransactions() only executes every [12-36h (24h average)](https://github.com/bitcoin/bitcoin/blob/1420547ec30a24fc82ba3ae5ac18374e8e5af5e5/src/wallet/wallet.cpp#L1947). Triggering it every second is excessive, once per minute should be plenty.
The goal of this PR is to reduce the amount of (unnecessary) schedule executions by ~60x without meaningfully altering transaction rebroadcast logic/assumptions which would require more significant review.
ACKs for top commit:
achow101:
ACK 5ef8c2c9fc4ebce6cbfea6a55a89a0ab7ee98a1a
1440000bytes:
ACK https://github.com/bitcoin/bitcoin/pull/25922/commits/5ef8c2c9fc4ebce6cbfea6a55a89a0ab7ee98a1a
Tree-SHA512: 4a077e3579b289c11c347eaa0d3601ef2dbb9fee66ab918d56b4a0c2e08222560a0e6be295297a74831836e001a997ecc143adb0c132faaba96a669dac1cd9e6
|
|
different minversion
835bd27e9a0dd627f266e3dc0a7422d8d0612eff Wallet::SetMinVersion - Log the new minversion (Ali Sherief)
Pull request description:
This change prints a single additional line in the debug.log when bitcoin-cli loads a wallet using `loadwallet` (*not* `createwallet`).
When Bitcoin Core creates a wallet, it's `minversion` is set to `FEATURE_BASE`, which is 10500. However, once the wallet is unloaded using `unloadwallet` or through program termination, and subsequently loaded again, `loadwallet` updates the `minversion` in the wallet.dat file to `FEATURE_LATEST`, currently 169900.
The current logging format prints the very old wallet version during `createwallet`, and then the actual version in calls to `loadwallet`. This has confused at least one person ([reference](https://bitcointalk.org/index.php?topic=5410650.0) - I was the one who asked there if there were plans to change that behavior, and was subsequently redirected here by achow), so it will be very helpful to users to explicitly specify in the logs what the walletdb is doing.
ACKs for top commit:
achow101:
ACK 835bd27e9a0dd627f266e3dc0a7422d8d0612eff
Tree-SHA512: 967c8c617e06a84915ddb147378ec3c8b0343e45f43145ec78df9cbc0201867f49c8e11cd068c403eb5ec06e07d38c3c0d3864dad8edc5efbb134a3fb30be41f
|
|
outbound connections
59aa54f7312f3441692c89feed86b8756d9d6b7a i2p: log "SAM session" instead of "session" (Vasil Dimov)
d7ec30b648721133b5a5ac3f52275f779c54310f doc: add release notes about the I2P transient addresses (Vasil Dimov)
47c0d02f126c73755288c3084402098567964329 doc: document I2P transient addresses usage in doc/i2p.md (Vasil Dimov)
3914e472f5685c29aa3d1c6dc5af9a758313d6c1 test: add a test that -i2pacceptincoming=0 creates a transient session (Vasil Dimov)
ae1e97ce863609e06be44a2632fb9d1fbb8e5698 net: use transient I2P session for outbound if -i2pacceptincoming=0 (Vasil Dimov)
a1580a04f5d7c9ecb30ee0d3bfdae519843a67ac net: store an optional I2P session in CNode (Vasil Dimov)
2b781ad66e34000037f589c71366c203255ed058 i2p: add support for creating transient sessions (Vasil Dimov)
Pull request description:
Add support for generating a transient, one-time I2P address for ourselves when making I2P outbound connection and discard it once the connection is closed.
Background
---
In I2P connections, the host that receives the connection knows the I2P address of the connection initiator. This is unlike the Tor network where the recipient does not know who is connecting to them, not even the initiator's Tor address.
Persistent vs transient I2P addresses
---
Even if an I2P node is not accepting incoming connections, they are known to other nodes by their outgoing I2P address. This creates an opportunity to white-list given nodes or treat them differently based on their I2P address. However, this also creates an opportunity to fingerprint or analyze a given node because it always uses the same I2P address when it connects to other nodes. If this is undesirable, then a node operator can use the newly introduced `-i2ptransientout` to generate a transient (disposable), one-time I2P address for each new outgoing connection. That address is never going to be reused again, not even if reconnecting to the same peer later.
ACKs for top commit:
mzumsande:
ACK 59aa54f7312f3441692c89feed86b8756d9d6b7a (verified via range-diff that just a typo / `unique_ptr` initialisation were fixed)
achow101:
re-ACK 59aa54f7312f3441692c89feed86b8756d9d6b7a
jonatack:
utACK 59aa54f7312f3441692c89feed86b8756d9d6b7a reviewed range diff, rebased to master, debug build + relevant tests + review at each commit
Tree-SHA512: 2be9b9dd7502b2d44a75e095aaece61700766bff9af0a2846c29ca4e152b0a92bdfa30f61e8e32b6edb1225f74f1a78d19b7bf069f00b8f8173e69705414a93e
|
|
Refactors SetupDescSPKMs so that the DescSPKM loops are in their own
function. This allows us to call it later during migration with a key
that was already generated.
|
|
|
|
(s/rescanwallet/rescanblockchain/)
e90a445d7e84df4bfdd689c6593f6688cbace746 scripted-diff: rpc: fix rescan RPC name (s/rescanwallet/rescanblockchain/) (Sebastian Falbesoner)
Pull request description:
There is no RPC call named `rescanwallet`, i.e. fix this by renaming to the actual RPC called `rescanblockchain`.
ACKs for top commit:
achow101:
ACK e90a445d7e84df4bfdd689c6593f6688cbace746
aureleoules:
ACK e90a445d7e84df4bfdd689c6593f6688cbace746.
promag:
ACK e90a445d7e84df4bfdd689c6593f6688cbace746
Tree-SHA512: abf1d1c18de32d87c29e4ff2b782dfb0e4a46dc2c2cc51ab616d12674a0f4a5d22214e00955663ae897cbb88f4f6ced913850f28ea3f5c1b3a54577a25fbf399
|
|
|
|
|
|
|
|
|
