Age | Commit message (Collapse) | Author |
|
b6ff9c530d83260049aa18452f4e77dd137b1a56 doc: point release-notes.md to the dev wiki (fanquake)
8f1b7e5cf9d2c51c9348b7354f763e1bb6491354 doc: generate example bitcoin.conf for v27.0rc1 (fanquake)
b4fd211d2c630a6c9cccc5dd5ea44ec2adfe8309 doc: generate manual pages for v27.0rc1 (fanquake)
7589ce39972ef6d07e92f5e42ee9ad46144e129f build: bump version to v27.0rc1 (fanquake)
Pull request description:
Bump the version number.
Generate the man pages.
Generate example bitcoin.conf.
Point release-notes.md to the wiki: https://github.com/bitcoin-core/bitcoin-devwiki/wiki/27.0-Release-Notes-Draft.
ACKs for top commit:
achow101:
ACK b6ff9c530d83260049aa18452f4e77dd137b1a56
willcl-ark:
ACK b6ff9c530d83260049aa18452f4e77dd137b1a56
Tree-SHA512: 914d95f18cb6393db17ed590f62e47f06d5baad0672f8674d6e71b38b9e54c8f9d9aec4631c66c9050e9da9c2a98d3120b6d7fb155ade7d4e5eb4a17fa7ef847
|
|
See
https://github.com/bitcoin-core/bitcoin-devwiki/wiki/27.0-Release-Notes-Draft.
|
|
|
|
|
|
|
|
7ab54397f8b818b0474d4e9d4f5db45abb6fc249 seeds: Update testnet seeds (Ava Chow)
34a233b6d875976f354094f67b363a4d7b63ad2f seeds: Update mainnet seeds (Ava Chow)
9701bc435fe16fb7c285c682d87c44972f1c23b9 makeseeds: Check i2p seeds too (Ava Chow)
a8ec9eede4c745c6b6fd76974816ffad8034129a makeseeds: Update PATTERN_AGENT (Ava Chow)
Pull request description:
The ipv4 and ipv6 seeds are updated from sipa's crawler, as outlined in contrib/seeds/README.md. The onion and i2p seeds are pulled from my node's addrman using `getrawaddrman` and then a connection was made to each node to retrieve the current service flags, block height, and user agent string before filtering through makeseeds.py. The CJDNS nodes were not updated as my node is not connected to that network.
makeseeds.py is also updated for more recent user agent strings as well as being able to handle i2p addresses.
Also updated the testnet seeds.
ACKs for top commit:
fanquake:
ACK 7ab54397f8b818b0474d4e9d4f5db45abb6fc249
Tree-SHA512: 5edba63d51116e5d9a8ae23561ba5a311f4df88c555c60b2d7a6066e63f8cdfd256be7dac9acea4b370879d0d3c3a4b55328c15de4284b5f0d86e6cac2e5ba9b
|
|
312f3381a2d3a7afb7c81b4662214d4d67b4e84a fuzz: restrict fopencookie usage to Linux & FreeBSD (fanquake)
Pull request description:
Should fix the GCC compilation portion of https://github.com/bitcoin/bitcoin/issues/29517#issuecomment-1973573314.
See also: https://www.gnu.org/software/gnulib/manual/html_node/fopencookie.html.
ACKs for top commit:
m3dwards:
ACK https://github.com/bitcoin/bitcoin/pull/29529/commits/312f3381a2d3a7afb7c81b4662214d4d67b4e84a
TheCharlatan:
utACK 312f3381a2d3a7afb7c81b4662214d4d67b4e84a
Tree-SHA512: aa8ff20c3fa735415d05a93b8855877035c300f4d2dfd82f65fd9ed5b5c96ab619b4d84eef114ed0013dc5ff0800cb628ed3801e1efde0cfb0d426930d1673d5
|
|
Should fix the GCC compilation portion of #29517:
https://github.com/bitcoin/bitcoin/issues/29517#issuecomment-1973573314.
See also:
https://www.gnu.org/software/gnulib/manual/html_node/fopencookie.html
but note that FreeBSD has supported this function since 11.x.
|
|
in Windows GHA job
57e6e2279ee5562fe31eb418d9bcd8b80634ec8b ci: Fix functional tests step for pull requests in Windows GHA job (Hennadii Stepanov)
Pull request description:
This functionality has been broken since the Windows runner image version `20240128.1.0`.
Fixes https://github.com/bitcoin/bitcoin/issues/29534.
ACKs for top commit:
fanquake:
I can ACK 57e6e2279ee5562fe31eb418d9bcd8b80634ec8b this only based on the fact that in this PR, the native Windows functional tests run: https://github.com/bitcoin/bitcoin/actions/runs/8119259315/job/22194887783#step:27:72, and that the native Windows functional tests are not currently running on master: https://github.com/bitcoin/bitcoin/actions/runs/8131828989/job/22239779585#step:27:63.
hebasto:
> I can ACK 57e6e22
m3dwards:
ACK https://github.com/bitcoin/bitcoin/pull/29535/commits/57e6e2279ee5562fe31eb418d9bcd8b80634ec8b as a way to get the tests running again quickly.
Tree-SHA512: 03e04fb96292e31ca0a8057e91b57f0812df92589f52f0602844e151ec5ec296badf5e549b1b606bab85607a3f9cd6abdfd328df80bf268501b537a596db0d96
|
|
suppression
217c0ce552a5d519b5cc702aba0c82514a1c449e test: remove file-wide interpreter.cpp ubsan suppression (fanquake)
Pull request description:
ACKs for top commit:
Sjors:
utACK 217c0ce552a5d519b5cc702aba0c82514a1c449e
hebasto:
ACK 217c0ce552a5d519b5cc702aba0c82514a1c449e.
dergoegge:
ACK 217c0ce552a5d519b5cc702aba0c82514a1c449e
Tree-SHA512: ae0c2ff4531fdb7b0296709f66b71d4065fe3f32cbd39a44e45934a975b5cf6cf01c2f136f110753efee8e301636f7700278aed1d995b463fc025c07d586a8fa
|
|
d9f30b021af5c4d4bc7fa687cdcc236ca7bc5df2 kernel: chainparams updates for 27.x (fanquake)
1611aa1789dc1dcb445bc1654ac4bec841345a20 kernel: update chainTxData for 27.x (fanquake)
af78d31e710f61601dc2b21014cb4250ddea85ad kernel: update nMinimumChainWork & defaultAssumeValid for 27.x (fanquake)
Pull request description:
Update chainparams pre `27.x` branch off.
Note: Remember that some variance is expected in the m_assumed_* sizes.
ACKs for top commit:
Sjors:
ACK d9f30b021af5c4d4bc7fa687cdcc236ca7bc5df2
glozow:
ACK d9f30b021af5c4d4bc7fa687cdcc236ca7bc5df2 (checked mainnet locally, checked testnet/signet on block explorers and sanity checked the numbers)
instagibbs:
ACK d9f30b021af5c4d4bc7fa687cdcc236ca7bc5df2
Tree-SHA512: 6ce65b964334b9d15fff4aa1af6d26fb3ef4eab50b8237fc2cda180230ae724a99d13c9f6b3c58105548d3520c0ca0810f354736132d11793d6c91ad3eeac4c7
|
|
offline-signing-tutorial.md
990b348912ac8cf107d7111632d3f6fb7298f36b doc: update signet faucet link in offline-signing-tutorial.md (Supachai Kheawjuy)
Pull request description:
https://signet.bc-2.jp is broken and https://signetfaucet.com is the same as before.
https://signet.bc-2.jp from archive.org
<img width="1258" alt="image" src="https://github.com/bitcoin/bitcoin/assets/73651621/36817aa6-95ea-427d-8d1d-93e21af86dce">
https://signetfaucet.com
<img width="1242" alt="image" src="https://github.com/bitcoin/bitcoin/assets/73651621/e3248fb0-8a6d-45b3-9268-d883d2385c8f">
reference: https://en.bitcoin.it/wiki/Signet#Faucets
ACKs for top commit:
fanquake:
ACK 990b348912ac8cf107d7111632d3f6fb7298f36b
Tree-SHA512: 482c931168a162cc666ecbe610e80d94ae433ebdc6bc52832bcc40c58592f9d9b8c7f1aea6faa2739873e80c6d4ea70c8a4f78d18067d1739e8070effce83062
|
|
test/lint/README.md
53ffd5a410186109b9a56c5922768905e168acb3 docs: Fix broken reference to CI setup in test/lint/README.md (naiyoma)
Pull request description:
The current [reference](https://github.com/bitcoin/bitcoin/blob/master/test/ci/lint/04_install.sh
) for CI setup in /test/lint#readme returns a 404.
ACKs for top commit:
fanquake:
ACK 53ffd5a410186109b9a56c5922768905e168acb3
Tree-SHA512: 813c19a145f09e7da11963598b70dc438acba784eb722e509d6af59dc3af8f5da97628c454bed2b03cc919689603e070796de2db8d784d9162ae34e7b85a77d9
|
|
|
|
e67ab174c9c04bba7a10724b7e694dd57f732177 test: fix flaky wallet_send functional test (Max Edwards)
3c49e6967050cfc367b3c826c9eac86a48528af5 test: fix weight estimates in functional tests (Max Edwards)
Pull request description:
Fixes: https://github.com/bitcoin/bitcoin/issues/25164
The wallet_send functional test has been flaky due to a slightly overestimated weight calculation. This PR makes the weight calculation more accurate, although occasionally, due to how ECDSA signatures can be different lengths it might slightly over estimate. The assertion in the test can handle this slight variation and so should continue passing.
Update:
Because the signature can be shorter that is used in the weight estimation or the final transaction the estimate could be both slightly smaller or slightly larger.
ACKs for top commit:
achow101:
ACK e67ab174c9c04bba7a10724b7e694dd57f732177
S3RK:
Code review ACK e67ab174c9c04bba7a10724b7e694dd57f732177
Tree-SHA512: 3bf73b355309dce860fa1520afb8461e94268e4bcf0e92a8273c279b41b058c44472cf59daafa15a515529b50bd665b5d498bbe4d934f2315dbe810a05bc73f9
|
|
|
|
|
|
|
|
|
|
6e5eda83ad59315b95cc5d3cb7ccfa36f0c6c881 doc: remove rel note fragments (fanquake)
Pull request description:
These have been added to https://github.com/bitcoin-core/bitcoin-devwiki/wiki/27.0-Release-Notes-Draft, where they can be improved further.
ACKs for top commit:
stickies-v:
ACK 6e5eda83ad59315b95cc5d3cb7ccfa36f0c6c881
Tree-SHA512: 66874fe9a64ac3a99a15a602ac68ae0a9e08f52a0fe732e48136b245c2127ed04e8217f86c44459696b03b01532a926ab8d41101c6e670902c1fc31e583b4dc9
|
|
632b69f79bb83d2313df7d76667763fbb590136b qt: 27.0 translations update (Hennadii Stepanov)
Pull request description:
This PR pulls the recent translations from the [Transifex.com](https://www.transifex.com/bitcoin/bitcoin) using the [`bitcoin-maintainer-tools/update-translations.py`](https://github.com/bitcoin-core/bitcoin-maintainer-tools/blob/main/update-translations.py) tool.
ACKs for top commit:
stickies-v:
ACK 632b69f79bb83d2313df7d76667763fbb590136b , getting a zero-diff when running `update-translations.py` on fce53f132e1b3f2c8bf1530dca18f3da136f08ab
Tree-SHA512: 1e2823451e9192e60dec9d50e801fca4cdc621e6acabdc15dbd88cab1624e05bd56de9ac818a1ff91002d62e24c0bab0ef1eaad3fd3cc6ef6cd044989d39734f
|
|
These have been added to
https://github.com/bitcoin-core/bitcoin-devwiki/wiki/27.0-Release-Notes-Draft,
where they can be improved further.
|
|
|
|
|
|
|
|
6962c66b4a9657fd2a6fcca8e9a31beb90d13924 build, msvc: Do not compile redundant sources (Hennadii Stepanov)
Pull request description:
The `test\util\setup_common.cpp` and `wallet\test\util.cpp` sources are already compiled and included in the `libtest_util` library, which is linked to the `test_bitcoin-qt.exe` binary. This PR follows the same logic as `Makefile.qttest.include`.
Useful for comparing project files across the master branch and the developing [cmake-staging](https://github.com/hebasto/bitcoin/tree/cmake-staging) branch.
ACKs for top commit:
sipsorcery:
utACK 6962c66b4a9657fd2a6fcca8e9a31beb90d13924.
Tree-SHA512: 5c40f52f3c7df3fff994fb136d4b2779ade3857fa14cf167d3f8600f28e821294e3779ebd4f4ab10ad57bdc8e952f99f6eb211e746a986ec24e26c7d1a74c04f
|
|
b7aa717cdd3f6af266c244fec6d775e917cf8d0c refactor: gui, simplify boost signals disconnection (furszy)
f3a612f9016fe1f59c73d6059274bea8025b8940 gui: guard accessing a nullptr 'clientModel' (furszy)
Pull request description:
Fixing #800.
During shutdown, already queue events dispatched from the backend such
'numConnectionsChanged' and 0networkActiveChanged' could try to access
the clientModel object, which might not exist because we manually delete
it inside 'BitcoinApplication::requestShutdown()'.
This happen because boost does not clears the queued events when they arise
concurrently with the signal disconnection (see https://www.boost.org/doc/libs/1_55_0/doc/html/signals2/thread-safety.html).
From the docs:
1) "Note that since we unlock the connection's mutex before executing its associated slot, it is possible a slot will still be executing after it has been disconnected by a [connection::disconnect](https://www.boost.org/doc/libs/1_55_0/doc/html/boost/signals2/connection.html#idp89761576-bb)(), if the disconnect was called concurrently with signal invocation."
2) "The fact that concurrent signal invocations use the same combiner object means you need to insure any custom combiner you write is thread-safe"
So, we need to guard `clientModel` before accessing it at the handler side.
ACKs for top commit:
hebasto:
re-ACK b7aa717cdd3f6af266c244fec6d775e917cf8d0c
Tree-SHA512: f1a21d69248628f6a13556a9438c9e4ea9f0a3678aab09ddfe836e78e4eee405a6730d37d39f1445068ada3a110b655b619cf0e090fc2d0cdf99bed061364aeb
|
|
don't connect to known prev block
a1fbde0ef7cf6c94d4a5181f8ceb327096713160 p2p: Don't consider blocks mutated if they don't connect to known prev block (Greg Sanders)
Pull request description:
Followup to https://github.com/bitcoin/bitcoin/pull/29412 to revert some of the behavior change that was likely unintentional.
Based on comments from https://github.com/bitcoin/bitcoin/pull/29412#discussion_r1507499192
ACKs for top commit:
0xB10C:
utACK a1fbde0ef7cf6c94d4a5181f8ceb327096713160
dergoegge:
Code review ACK a1fbde0ef7cf6c94d4a5181f8ceb327096713160
Sjors:
ACK a1fbde0ef7cf6c94d4a5181f8ceb327096713160
sr-gi:
tACK https://github.com/bitcoin/bitcoin/commit/a1fbde0ef7cf6c94d4a5181f8ceb327096713160
Tree-SHA512: be6204c8cc57b271d55c1d02a5c77d03a37738d91cb5ac164f483b0bab3991c24679c5ff02fbaa52bf37ee625874b63f4c9f7b39ad6fd5f3a25386567a0942e4
|
|
|
|
|
|
|
|
521693378b86aaae5af1646c3a18a902cc835c69 build: move sha256_sse4 into libbitcoin_crypto_base (fanquake)
Pull request description:
Followup to discussion in #29407.
Drops `LIBBITCOIN_CRYPTO_SSE4`.
ACKs for top commit:
theuni:
utACK 521693378b86aaae5af1646c3a18a902cc835c69.
hebasto:
ACK 521693378b86aaae5af1646c3a18a902cc835c69.
TheCharlatan:
ACK 521693378b86aaae5af1646c3a18a902cc835c69
Tree-SHA512: 44889340b7647409a439ebe97012f66383e0e5f3d99200ffd55c124e91d3ed164f02b736ff9dafca2d9ba7e365fcdc79aff054471d4bc240d035b58659407420
|
|
This functionality has been broken since the Windows runner image
version `20240128.1.0`.
|
|
The `test\util\setup_common.cpp` and `wallet\test\util.cpp` sources are
already compiled and included in the `libtest_util` library, which is
linked to the `test_bitcoin-qt.exe` binary. This change follows the same
logic as `Makefile.qttest.include`.
|
|
Followup to discussion in #29407.
Drops LIBBITCOIN_CRYPTO_SSE4.
|
|
modernization
86b7f28d6c507155a9d3a15487ee883989b88943 serialization: use internal endian conversion functions (Cory Fields)
432b18ca8d0654318a8d882b28b20af2cb2d2e5d serialization: detect byteswap builtins without autoconf tests (Cory Fields)
297367b3bb062c57142747719ac9bf2e12717ce9 crypto: replace CountBits with std::bit_width (Cory Fields)
52f9bba889fd9b50a0543fd9fedc389592cdc7e5 crypto: replace non-standard CLZ builtins with c++20's bit_width (Cory Fields)
Pull request description:
This replaces #28674, #29036, and #29057. Now ready for testing and review.
Replaces platform-specific endian and byteswap functions. This is especially useful for kernel, as it means that our deep serialization code no longer requires bitcoin-config.h.
I apologize for the size of the last commit, but it's hard to avoid making those changes at once.
All platforms now use our internal functions rather than libc or platform-specific ones, with the exception of MSVC.
Sadly, benchmarking showed that not all compilers are capable of detecting and optimizing byteswap functions, so compiler builtins are instead used where possible. However, they're now detected via macros rather than autoconf checks.
This[ matches how libc++ implements std::byteswap for c++23](https://github.com/llvm/llvm-project/blob/main/libcxx/include/__bit/byteswap.h#L26).
I suggest we move/rename `compat/endian.h`, but I left that out of this PR to avoid bikeshedding.
#29057 pointed out some irregularities in benchmarks. After messing with various compilers and configs for a few weeks with these changes, I'm of the opinion that we can't win on every platform every time, so we should take the code that makes sense going forward. That said, if any real-world slowdowns are caused here, we should obviously investigate.
ACKs for top commit:
maflcko:
ACK 86b7f28d6c507155a9d3a15487ee883989b88943 📘
fanquake:
ACK 86b7f28d6c507155a9d3a15487ee883989b88943 - we can finish pruning out the __builtin_clz* checks/usage once the minisketch code has been updated. This is more good cleanup pre-CMake & for the kernal.
Tree-SHA512: 715a32ec190c70505ffbce70bfe81fc7b6aa33e376b60292e801f60cf17025aabfcab4e8c53ebb2e28ffc5cf4c20b74fe3dd8548371ad772085c13aec8b7970e
|
|
25eab523897e790f4f4d7b49cdbf19d13e3b0fcc fuzz: add target for local addresses (brunoerg)
Pull request description:
This PR adds fuzz target for local address functions - (`AddLocal`, `RemoveLocal`, `SeenLocal`, `IsLocal`)
ACKs for top commit:
dergoegge:
ACK 25eab523897e790f4f4d7b49cdbf19d13e3b0fcc
vasild:
ACK 25eab523897e790f4f4d7b49cdbf19d13e3b0fcc
Tree-SHA512: 24faaab86dcd8835ba0e2d81fb6322a39a9266c7edf66415dbc4421754054f47efb6e0de4efdc7ea026b0686792658e86a526f7cf27cbc6cf9ed0c4aed376f97
|
|
docs
efb70cd6452ce1f0d9f5464bec837b09ed5c2a78 doc: correct function name in AssumeUTXO design docs (jrakibi)
Pull request description:
Corrected the function name from `CompleteSnapshotValidation()` to `MaybeCompleteSnapshotValidation()` in the assumeutxo design documentation.
This change ensures that the documentation accurately reflects the actual function name used in the code
ACKs for top commit:
Empact:
ACK https://github.com/bitcoin/bitcoin/pull/29518/commits/efb70cd6452ce1f0d9f5464bec837b09ed5c2a78
Tree-SHA512: 68b9be3ba710d91a2a955189e227f86b46ccb6a2a13c345d46f276cec6ff12b77ebf9814c4bcb00db7c17e221510e4a2e71175c78a6faf0e0e3159c761bc9b94
|
|
Rather than asserting that the exact fees are the same, check the fee rate rounded to nearest interger. This will account for small differences in fees caused by variability in ECDSA signature lengths.
|
|
Updates the weight estimate to be more accurate by removing byte buffers and calculating the length of the count of scriptWitnesses rather than just using the count itself.
|
|
|
|
disable-asm option
f8a06f7a02be83e9b76a1b31f1b66a965dbedfce doc: remove references to disable-asm option now that it's gone (Cory Fields)
376f0f6d0798c10f09266d609afea3ada1b99f9b build: remove confusing and inconsistent disable-asm option (Cory Fields)
Pull request description:
1. It didn't actually disable asm usage in our code. Regardless of the setting, asm is used in random.cpp and support/cleanse.cpp.
2. The value wasn't forwarded to libsecp as a user might have reasonably expected.
3. We now have the DISABLE_OPTIMIZED_SHA256 define which is what disable-asm actually did in practice.
If there is any desire, we can hook DISABLE_OPTIMIZED_SHA256 up to a new configure option that actually does what it says.
Additionally, this is one of the last (THE last?) remaining uses of autoconf defines in our crypto code. As such it seems like low-hanging fruit.
ACKs for top commit:
fanquake:
ACK f8a06f7a02be83e9b76a1b31f1b66a965dbedfce
Tree-SHA512: 4a99c2130225acbe9dc7399ed572a04ca155cbfa3eef8178a632ba533017d264691e6482cceb1d8f9c5d768619d99a2466dea4b82b27b18b872bceae91b92fbb
|
|
a8c3454ba137dfac20b3c89bc558374de0524114 test: speedup bip324_cipher.py unit test (Sebastian Falbesoner)
Pull request description:
Executing the unit tests for the bip324_cipher.py module currently takes quite long (>60 seconds on my older notebook). Most time here is spent in empty plaintext/ciphertext encryption/decryption loops in `test_fschacha20poly1305aead`:
https://github.com/bitcoin/bitcoin/blob/9eeee7caa3f95ee17a645e12d330261f8e3c2dbf/test/functional/test_framework/crypto/bip324_cipher.py#L193-L194
https://github.com/bitcoin/bitcoin/blob/9eeee7caa3f95ee17a645e12d330261f8e3c2dbf/test/functional/test_framework/crypto/bip324_cipher.py#L198-L199
Their sole purpose is increasing the FSChaCha20Poly1305 packet counter in order to trigger rekeying, i.e. the actual encryption/decryption is not relevant, as the result is thrown away. This commit speeds up the tests by supporting to pass "None" as plaintext/ciphertext, indicating to the routines that no actual encryption/decryption should be done.
The approach here is a bit hacky, a cleaner alternative would probably be to introduce a special `seek`/`skip_packets` method and not touch the encrypt/decrypt routines, but that seemed overkill to me only for speeding up a unit test. Open for suggestions.
master branch:
```
$ python3 -m unittest ./test/functional/test_framework/crypto/bip324_cipher.py
..
----------------------------------------------------------------------
Ran 2 tests in 64.658s
```
PR branch:
```
$ python3 -m unittest ./test/functional/test_framework/crypto/bip324_cipher.py
..
----------------------------------------------------------------------
Ran 2 tests in 0.822s
```
ACKs for top commit:
delta1:
Concept ACK a8c3454
epiccurious:
Tested ACK a8c3454ba137dfac20b3c89bc558374de0524114.
achow101:
ACK a8c3454ba137dfac20b3c89bc558374de0524114
marcofleon:
ACK a8c3454ba137dfac20b3c89bc558374de0524114. The comments at the top of `bip324_cipher.py` specify that this should only be used for testing, so I think this optimization makes sense in that context.
cbergqvist:
ACK a8c3454!
stratospher:
ACK a8c3454. I think it's worth it because of the significant speedup in the unit test.
Tree-SHA512: 737dd805a850be6e035aa3c6d9e2c5b5b5e89ddc564f84a045c37e0238fef6419912de7c902139b64914abdd647c649fe02a694f1a5e1741d7d4459c041caccc
|
|
|
|
validation_tests
6ee3997d03e456655e3c44abf1e15270c423ed41 test: removes unnecessary check from validation_tests (Sergi Delgado Segura)
Pull request description:
An unnecessary check was added to the block mutation tests in #29412 where IsBlockMutated is returning true for the invalid reasons: we try to check mutation via transaction duplication, but the merkle root is not updated before the check, therefore the check fails because the provided root and the computed root differ, but not because the block contains the same transaction twice.
Notice that a proper check to test the duplication case is added a few lines later, so this check is just meaningless and can be removed. Check https://github.com/bitcoin/bitcoin/pull/29412#discussion_r1506490281 for context.
ACKs for top commit:
maflcko:
ACK 6ee3997d03e456655e3c44abf1e15270c423ed41
dergoegge:
utACK 6ee3997d03e456655e3c44abf1e15270c423ed41
BrandonOdiwuor:
utACK 6ee3997d03e456655e3c44abf1e15270c423ed41
Tree-SHA512: e4627668091dda5f589e4c15edac39dc84aabc9b34b8f7fadbf512beb7111d5477e1b69567a34b4a657e48ba66dfb864db5ff37c9bbe3ff24cd32931b2dd89e6
|
|
The comment about sha256_sse4::Transform is believed to be old and stale.
|
|
1. It didn't actually disable asm usage in our code. Regardless of the setting,
asm is used in random.cpp and support/cleanse.cpp.
2. The value wasn't forwarded to libsecp as a user might have reasonably
expected.
3. We now have the DISABLE_OPTIMIZED_SHA256 define which is what disable-asm
actually did in practice.
If there is any desire, we can hook DISABLE_OPTIMIZED_SHA256 up to a new
configure option that actually does what it says.
|
|
`getnewaddress` failures should not affect keypools for descriptor wallets
e073f1dfda7a2a2cb2be9fe2a1d576f122596021 test: make sure keypool sizes do not change on `getrawchangeaddress`/`getnewaddress` failures (UdjinM6)
367bb7a80cc71130995672c853d4a6e0134721d6 wallet: Avoid updating `ReserveDestination::nIndex` when `GetReservedDestination` fails (UdjinM6)
Pull request description:
I think the expected behaviour of `getrawchangeaddress` and `getnewaddress` RPCs is that their failure should not affect keypool in any way. At least that's how legacy wallets work, you can confirm this behaviour by running `wallet_keypool.py --legacy-wallet` on master with e073f1dfda7a2a2cb2be9fe2a1d576f122596021 applied on top. However running `wallet_keypool.py --descriptors` on the same commit results in the following failure:
```
File "/path/to/bitcoin/test/functional/test_framework/test_framework.py", line 131, in main
self.run_test()
File "/path/to/bitcoin/test/functional/wallet_keypool.py", line 114, in run_test
assert_equal(kp_size_before, kp_size_after)
File "/path/to/bitcoin/test/functional/test_framework/util.py", line 57, in assert_equal
raise AssertionError("not(%s)" % " == ".join(str(arg) for arg in (thing1, thing2) + args))
AssertionError: not([18, 24] == [19, 24])
```
This happens because we pass `nIndex` (which is a class member) into `GetReservedDestination` and since it's passed by reference we get an updated value back, so `nIndex` won't be equal `-1` anymore, no matter if the function failed or succeeded. This means that `ReturnDestination` (called by dtor of `ReserveDestination`) will try to return something we did not actually reserve.
The fix is to simply use a temporary variable instead of a class member and only update `nIndex` when `op_address` actually has value, basically do it the same way we do for other class members (`address` and `fInternal`) already.
ACKs for top commit:
achow101:
ACK e073f1dfda7a2a2cb2be9fe2a1d576f122596021
josibake:
ACK https://github.com/bitcoin/bitcoin/pull/29510/commits/e073f1dfda7a2a2cb2be9fe2a1d576f122596021
Tree-SHA512: 1128288a60dd4d8f306ef6f7ac66cdfeae3c9cc35c66ecada2d78fa61ac759f2a757b70fc3976ba8b5081200942b58dfabc184c01ccf911af40ba8c145344651
|
|
--v2transport
0487f91a2046c0d6f91ccaedeb00fbefa635c66d test: Fix intermittent failure in rpc_net.py --v2transport (stratospher)
Pull request description:
Fixes #29508.
Make sure that v2 handshake is complete before comparing getpeerinfo outputs so that `transport_protocol_type` isn't stuck at 'detecting'.
This is done by adding a wait_until statement till `transport_protocol_type = v2` so that bitcoind waits until the v2 handshake is complete. (on the python side, this is ensured by default since `wait_for_handshake = True` inside `add_p2p_connection()`)
ACKs for top commit:
Sjors:
ACK 0487f91a2046c0d6f91ccaedeb00fbefa635c66d
mzumsande:
Code Review ACK 0487f91a2046c0d6f91ccaedeb00fbefa635c66d
achow101:
ACK 0487f91a2046c0d6f91ccaedeb00fbefa635c66d
vasild:
ACK 0487f91a2046c0d6f91ccaedeb00fbefa635c66d
Tree-SHA512: 44dd646a61cd38da243f527df7321e22d1821c2b090be43673027746098caf450c6671708ed731ba257952df6b5886e64c9c2f9686a82f6ef0f25780b7a87d3d
|
|
Preventing dangling signals.
|