203a67d21f566634165531a7a75c3f8c9f9c9d6a doc: Put PR template in comments (Wladimir J. van der Laan)
Pull request description:
This prevents the common annoyance of the text being included into PRs
accidentally.
ACKs for top commit:
sdaftuar:
utACK 203a67d21f566634165531a7a75c3f8c9f9c9d6a
fanquake:
ACK 203a67d21f566634165531a7a75c3f8c9f9c9d6a - I make an effort to remove it whenever I see it in a PR.
Tree-SHA512: 3514d285488b7930d7f3d7f8823198d7325d8b7de57a6d8f13e559c0c23b30d58916b15782cbbdc347a375b418e9d0f7a5b99b34d26f3b957d7d5a03a3d83dfd
67d99900b0d770038c9c5708553143137b124a6c make SaltedOutpointHasher noexcept (Martin Ankerl)
Pull request description:
If the hash is not `noexcept`, `unorderd_map` has to assume that it can throw an exception. Thus when rehashing care needs to be taken. libstdc++ solves this by simply caching the hash value, which increases memory of each node by 8 bytes. Adding `noexcept` prevents this caching. In my experiments with `-reindex-chainstate -stopatheight=594000`, memory usage (maximum resident set size) has decreased by 9.4% while runtime has increased by 1.6% due to additional hashing. Additionally, memusage::DynamicUsage() is now more accurate and does not underestimate.
| | runtime h:mm:ss | max RSS kbyte |
|---------------------------------------|-----------------|--------------|
| master | 4:13:59 | 7696728 |
| 2019-09-SaltedOutpointHasher-noexcept | 4:18:11 | 6971412 |
| change | +1.65% | -9,42% |
Comparison of progress masters vs. 2019-09-SaltedOutpointHasher-noexcept
ACKs for top commit:
jamesob:
Tested ACK 67d99900b0
Tree-SHA512: 9c44e3cca993b5a564dd61ebd2926b9c4a238609ea4d283514c018236f977d935e35a384dd4696486fd3d78781dd2ba190bb72596e20a5e931042fa465872a0b
efd2474d17098c754367b844ec646ebececc7c74 util: CBufferedFile fixes (Larry Ruane)
Pull request description:
The `CBufferedFile` object guarantees its user is able to "rewind" the data stream (that's being read from a file) up to a certain number of bytes, as specified by the user in the constructor. This guarantee is not honored due to a bug in the `SetPos` method.
Such rewinding is done in `LoadExternalBlockFile()` (currently the only user of this object), which deserializes a series of `CBlock` objects. If that function encounters something unexpected in the data stream, which is coming from a `blocks/blk00???.dat` file, it "rewinds" to an earlier position in the stream to try to get in sync again. The `CBufferedFile` object does not actually rewind its file offset; it simply repositions its internal offset, `nReadPos`, to an earlier position within the object's private buffer; this is why there's a limit to how far the user may rewind.
If `LoadExternalBlockFile()` needs to rewind (call `blkdat.SetPos()`), the stream may not be positioned as it should be, causing errors in deserialization. This need to rewind is probably rare, which is likely why this bug hasn't been noticed already. But if this object is used elsewhere in the future, this could be a serious problem, especially as, due to the nature of the bug, the `SetPos()` _sometimes_ works.
This PR adds a unit test for `CBufferedFile` that fails due to this bug. (Until now it has had no unit tests.) The unit test provides good documentation and examples for developers trying to understand `LoadExternalBlockFile()` and for future users of this object.
This PR also adds code to throw an exception from the constructor if the rewind argument is not less than the buffer size (since that doesn't make any sense).
Finally, I discovered that the object is too restrictive in one respect: When the deserialization methods call this object's `read` method, a check ensures that the number of bytes being requested is less than the size of the buffer (adjusting for the rewind size), else it throws an exception. This restriction is unnecessary; the object being deserialized can be larger than the buffer because multiple reads from disk can satisfy the request.
ACKs for top commit:
laanwj:
ACK ~after squash.~ efd2474d17098c754367b844ec646ebececc7c74
mzumsande:
I had intended to follow up earlier on my last comment, ACK efd2474d17098c754367b844ec646ebececc7c74. I reviewed the code, ran tests and did a successful reindex on testnet with this branch.
Tree-SHA512: 695529e0af38bae2af4e0cc2895dda56a71b9059c3de04d32e09c0165a50f6aacee499f2042156ab5eaa6f0349bab6bcca4ef9f6f9ded4e60d4483beab7e4554
2c3eade704 Make fs::path::string() always return utf-8 string (Chun Kuan Lee)
Pull request description:
Imbue `fs::path` with `std::codecvt_utf8_utf16` at `SetupEnvironment()`, so that default string encoding will be utf-8 inside `fs::path`.
Tree-SHA512: 0cb59464d777278decbf24771fc5ff0cb2caa7bc2fe8ee5cd36c97a2324873a3caad131f08f050393b488316ee7f4ab0b28b7fa4699e41839f8e51b9867d5118
# Conflicts:
# src/qt/guiutil.cpp
bb6ca65f9890e8280ace32de5a37774e14705859 gui: get special folder in unicode (Chun Kuan Lee)
1c5d22585384c8bb05a27a04eab5c57b31d623fb Drop boost::scoped_array (Chun Kuan Lee)
Pull request description:
Drop boost::scoped_array and simplify the code.
`TCHAR` should be defined as `wchar_t` if `UNICODE` is defined. So we can use `.toStdWString().c_str()` to get wchar_t C-style string.
Fix#13819
Tree-SHA512: 3fd4aa784129c9d1576b01e6ee27faa42d793e152d132f2dde504d917dad3a8e95e065fcbc54a3895d74fb6b2a9ed4f5ec67d893395552f585e225486a84a454
# Conflicts:
# src/qt/guiutil.cpp
bb99c4e684bbd3053ecf7a789049b11b29260189 build: update boost macros to latest upstream (fanquake)
Pull request description:
Fixes: #16803
I opened an [upstream PR](https://github.com/autoconf-archive/autoconf-archive/pull/197) to improve the Boost error reporting, so pull the latest macros.
ACKs for top commit:
laanwj:
Code review ACK bb99c4e684bbd3053ecf7a789049b11b29260189
jonatack:
Sanity check ACK bb99c4e684bbd3053ecf7a789049b11b29260189, light code read, built and ran tests on Debian 4.19.37-5+deb10u2 (2019-08-08) x86_64 GNU/Linux. Only tested the happy path.
Tree-SHA512: 34704ed623ac0085215fd874a23fde8f6e39a69fa20d78472b0c4d2306dc101c0571fa26c4c8821600746b94daaaf05faf6d15546899d588081c26357d29ec46
14c6a2de1a4cf8cc17116d418242709ba2519b9e [doc] update brew instruction for doxygen (Sjors Provoost)
Pull request description:
I noticed while testing #16912 that `brew install doxygen --with-graphviz` no long works. Instead you need to use `brew install graphviz doxygen`.
ACKs for top commit:
fanquake:
ACK 14c6a2de1a4cf8cc17116d418242709ba2519b9e - tested a `make docs` on macOS with and without `graphviz` (`dot`) available.
Tree-SHA512: 2682568e558c16e9e0a657421c449b74cc14a89771844c1c88623fb75b07b89afb63c45a919eb7b9c3dba9bdfaef21489b5f7ea45a08d8d5da18614657c19e47
f0636d34185d235f51eebaa2ad14c1e6fcaed6c2 depends: zlib: Move toolchain options to configure (Carl Dong)
Pull request description:
```
zlib has its own custom configure script, see comment in zlib.mk for
more details
```
Performed Guix cross-builds locally and everything worked as expected.
ACKs for top commit:
laanwj:
ACK f0636d34185d235f51eebaa2ad14c1e6fcaed6c2
Tree-SHA512: 7ff6114e52a9c49941da31cb0ebd8918b056bf23343790d758e107003d856f3b1f16ebf4ce0ce22e1216a37a610b4c106def3f869d128bfffa61280d45ed6b38
2457aea83c1f9fba708e2335bb197950bf0b6244 Assert that the HRP is lowercase in Bech32::Encode (Samuel Dobson)
Pull request description:
From BIP-173:
> The lowercase form is used when determining a character's value for checksum purposes.
> Encoders MUST always output an all lowercase Bech32 string. If an uppercase version of the encoding result is desired, (e.g.- for presentation purposes, or QR code use), then an uppercasing procedure can be performed external to the encoding process.
Currently if HRP contains uppercase characters, the checksum will be generated over these uppercase characters resulting in mixed-case output that will always be invalid even if the case is changed manually after encoding. This shouldn't happen because both prefix's `bc` and `tb` are lowercase currently, but we assert this condition anyway.
This is consistent also with the [C reference implementation](2b0aac650c/ref/c/segwit_addr.c (L59))
ACKs for top commit:
laanwj:
ACK 2457aea83c1f9fba708e2335bb197950bf0b6244
Tree-SHA512: 24fcbbc2f315c72c550cc3d82b4332443eea6378fc73d571f98b87492604d023378dd102377c9e05467192cae6049606dee98e4c5688c8d5e4caac50c970284b
490da639cbd48ce0dc438abbfc89ab796391cb2a Make lint-includes.sh work from any directory (Kristaps Kaupe)
Pull request description:
Before this change it works from root folder of bitcoin git repo, but if you do `cd test/lint; ./test-includes.sh`, you will have a lot of false positive messages like this:
```
Good job! The circular dependency "chainparamsbase -> util/system -> chainparamsbase" is no longer present.
Please remove it from EXPECTED_CIRCULAR_DEPENDENCIES in ./lint-circular-dependencies.sh
to make sure this circular dependency is not accidentally reintroduced.
Good job! The circular dependency "index/txindex -> validation -> index/txindex" is no longer present.
Please remove it from EXPECTED_CIRCULAR_DEPENDENCIES in ./lint-circular-dependencies.sh
to make sure this circular dependency is not accidentally reintroduced.
```
Top commit has no ACKs.
Tree-SHA512: 07fa69cb2883181dcee922191acac4b242722eeb2916cdffdc7163421302b22f3c9525aaf4c754a9dba1c307032c05285e38191d5c6aabc894321f8a27bbceaa
635e9154da223ca760713d7b3b66c0a9321d1277 [qt] Change uninstall icon on Windows (GChuf)
Pull request description:
Change uninstall icon in Windows by changing a registry value.
Original uninstall.exe icon remains the same
Reason: almost no other modern program uses that uninstall icon in Windows.
before:
after:
ACKs for top commit:
promag:
ACK 635e9154da223ca760713d7b3b66c0a9321d1277.
l2a5b1:
ACK 635e915
practicalswift:
ACK 635e9154da223ca760713d7b3b66c0a9321d1277
jonasschnelli:
utACK 635e9154da223ca760713d7b3b66c0a9321d1277
fanquake:
ACK 635e9154da223ca760713d7b3b66c0a9321d1277 - Tested building and installing using WSL on Windows 10.
Tree-SHA512: a0f435c79e726896cc93db058f1712dcf37daefbcacbf213b340ef2d5f4ff387148b8d659302f3e84be8a9f3df23e72b0ca5937f5d77499b808081bd40bfbbac
fa0b910486e9aada077fe47e1201dcb3bd523c87 [doc] chain: Declare BLOCK_VALID_HEADER reserved (MarcoFalke)
Pull request description:
`BLOCK_VALID_HEADER` was never used and the comment is confusing to me in several ways:
* It claims "version ok". However, without the previous header, it is not possible to check the validity of the version since the height needs to be known (c.f. BIP 90)
* It claims "hash satisfies claimed PoW". While it is possible to check against the claimed PoW, it is not possible without the previous header to check that the claimed PoW is itself valid.
* It claims "1 <= vtx count <= max". However, with the header alone and current consensus rules, the number of transactions is unknown.
ACKs for top commit:
sipa:
ACK fa0b910486e9aada077fe47e1201dcb3bd523c87
ryanofsky:
ACK fa0b910486e9aada077fe47e1201dcb3bd523c87
Tree-SHA512: 3972995a0a2f83aa55767bf8982af1fcb9493483f62aee6df27e58be9181a48d5968ae718b390cecc8be3ed4f26495683b1cffde8ef272dea0bd610ec169ef8b
d48c1e837ae1bd08e0f18ad1b57ff72675c3d6ad Add window final block height to getchaintxstats (Jonathan "Duke" Leto)
Pull request description:
This patch is motivated by the desire to make the output of `getchaintxstats` more useful and optimized for applications to consume and render the data.
Firstly, this data is already available to the RPC, no additional work is done. Currently additional RPC calls will be needed to look up the height of the final block in the window or the block height that began the window.
By adding the block height of the final block in the window, the JSON is "self-contained" and applications can calculate the exact block height range of the window with no additional RPC requests.
For example, a web application which wants to render historical information for `getchaintxstats` RPC on various window sizes might call the RPC with various window lengths, once per day, and store the JSON results somewhere. Because the final block height of each dataset is included, it's no extra work to determine the exact block window range of each JSON response.
ACKs for top commit:
promag:
ACK d48c1e837ae1bd08e0f18ad1b57ff72675c3d6ad.
Tree-SHA512: fd4952c125f81a4ad18f7c78498c6b3e265b93cb574832166ac25596321ce84957f971f3f78f37d7e42638dc65f2a5d4d760f289873c9c2f2a82eb00a0f87c3f
41d484d5c88f057e75cfdd8b88587b9a12a61744 doc: Delete stale URL in test README (Michael Folkson)
Pull request description:
The resource on the Boost unit test framework previously linked to in src/test/README.md was a stale URL.
Instead of deleting it, I've replaced it with an alternative resource on the framework on [boost.org](https://www.boost.org/doc/libs/1_45_0/libs/test/doc/html/utf/tutorials.html).
ACKs for top commit:
promag:
ACK 41d484d5c88f057e75cfdd8b88587b9a12a61744.
hebasto:
ACK 41d484d5c88f057e75cfdd8b88587b9a12a61744, the removed link is really obsolete.
fanquake:
ACK 41d484d5c88f057e75cfdd8b88587b9a12a61744 - Thanks.
Tree-SHA512: 764f12548441bde615f77b7a2ca7c5188b4ab936972d16b84960fbd8604d4cbd224415bc59ce839e7e63293aa84fd97f31a69e38734e531231cdb0e148d2e1bd
1373fa7e3d3f04ce6938cdcd2124cba71ff82ca0 doc: add default bitcoin.conf locations (Chuf)
Pull request description:
Added default bitcoin.conf data directories and paths
ACKs for top commit:
practicalswift:
ACK 1373fa7e3d3f04ce6938cdcd2124cba71ff82ca0
ryanofsky:
ACK 1373fa7e3d3f04ce6938cdcd2124cba71ff82ca0
fanquake:
ACK 1373fa7e3d3f04ce6938cdcd2124cba71ff82ca0 - Already three ACKs and lots of discussion here, so I'm going to merge, and the other comment
Tree-SHA512: 8bb1ed9868c5d171b6791bd6dc9598eddfdf64977d327ff4f333323cef8e3e76b1a67da21e4199f008a12f5610ac6dc6f34f4a13235e8846754eb6d6e5075da4
9452802480bd154e23771230bbdfebde1dbaa941 doc: Tidy up shadowing section (João Barbosa)
Pull request description:
Removes the example because it violates the code format.
ACKs for top commit:
MarcoFalke:
unsigned ACK 9452802480bd154e23771230bbdfebde1dbaa941
ryanofsky:
ACK 9452802480bd154e23771230bbdfebde1dbaa941
fanquake:
ACK 9452802480bd154e23771230bbdfebde1dbaa941 - Thanks for following up.
Tree-SHA512: 1fc31355d368225713298da7803e39e99014fbfcd229f2d3b56c082de95ab2965e51c80b172a5abce4646c53f845fa62a6d94d5df714e7835cac07a8ec7d5da7
c295cba5a2f934e51a7c8610ab4c58b8e9d56619 depends: zeromq: disable draft classes and methods (fanquake)
0072237b9e33e0b89f6c9f51dd0b946fa89a6134 depends: xproto: configure flags cleanup (fanquake)
6a8ada3a4f67affcf0ef7452e206083d7b58b2bc depends: qrencode: configure flags cleanup (fanquake)
86beb8cdc4e312bd0bed2cbb273aebb792be2747 depends: fontconfig: configure flags cleanup (fanquake)
e656d95ec74336c2bd93bd387f67aeb6aed4dc40 depends: libxcb: configure flags cleanup (fanquake)
e439388b352b7dfbf2e00c6ba2970fed0a4a5554 depends: libXau: configure flags cleanup (fanquake)
Pull request description:
Related to #16354.
This PR adds additional configure flags to packages in depends to explicitly disable features we aren't using; similar to #16183. It also fixes passing `--without-tools` to `qrencode`.
I've added `--disable-drafts` to `zeromq`:
```bash
Build and install draft classes and methods [default=yes]
```
I'm not entirely sure how far we want to take this. i.e in the `zeromq` package we explicitly pass `--without-libsodium`, even though it's disabled by default.
Do we also want to explicitly pass all the other `--without` flags? :
```bash
--with-libgssapi_krb5 require libzmq build with libgssapi_krb5
[default=no]
--with-libsodium use libsodium instead of built-in tweetnacl
[default=no]
--with-pgm build libzmq with PGM extension. Requires pkg-config
[default=no]
--with-norm build libzmq with NORM protocol extension,
optionally specifying norm path [default=no]
--with-vmci build libzmq with VMCI transport [default=no]
```
ACKs for top commit:
dongcarl:
ACK c295cba5a2f934e51a7c8610ab4c58b8e9d56619
Tree-SHA512: df6d38b863b4008ed2cb06c97eb0e21eaa4b5fde552876065ba7f3c87bf6e372e5b954a51bf3fde2151cfb6d2c022227d34337fc6e50ce0caa1d518abbd2412a
a4765bd77f9bc5c4d5c866ee6940807522741941 gui: Ensure tx send error highlight is visible (bpay)
Pull request description:
Rebased and squashed #14956.
> If sending to multiple recipients and one of the recipient fields is malformed, the highlighted field may not be visible due to being scrolled out of view. This results in a confusing lack of error feedback.
> Avoid this problem by ensuring the first field containing an error is scrolled into view when Send is clicked.
You can see the behavior here: https://imgur.com/a/QZG5TQc
How to test:
Add a few recipients and give any of them an invalid address or amount. Scroll the invalid recipient out of view and hit Send. With this change, the GUI will scroll to show the invalid recipient, with master it will not, "hiding" the error.
ACKs for top commit:
jonatack:
Tested ACK a4765bd77f9bc5c4d5c866ee6940807522741941 on Linux Debian with Qt 5.11.3. Change is that I had added an inadvertent typo in my make bash alias; fixed.
hebasto:
ACK a4765bd77f9bc5c4d5c866ee6940807522741941, tested on Debian 9.9 with system Qt 5.7.1.
Tree-SHA512: a5653ca44d6d540214bdb424b0b75a06a5872cff41b0cd8cffd9cef99ebf04a17a3652e561139ac75315b39c3347e5f7ae304fa35e14b48bdae4768a416df9b0
153d9dd9acffa95bb97a4b1579bd20237fdc9c52 refactor: replace qLowerBound & qUpperBound with std:: upper_bound & lower_bound (fanquake)
59373e3e94015316bcaa03a7b9c2e6f442641720 refactor: replace qSort with std::sort (fanquake)
fea33cbbdfb4673033f3414bf1613591ff654aac refactor: replace qStableSort with std::stable_sort (fanquake)
Pull request description:
`qStablesort`, `qSort`, `qLowerBound` and `qUpperBound` have been marked as obsolete since at least Qt 5.9: [Obsolete Members for QtAlgorithms](https://doc.qt.io/qt-5.9/qtalgorithms-obsolete.html).
This pull request replaces their usage with the suggested `std::` replacements.
This also removes some warning spam when compiling against newer Qt (5.13.0 via brew):
```bash
CXX qt/libbitcoinqt_a-walletcontroller.o
qt/transactiontablemodel.cpp:96:52: warning: 'qLowerBound<QList<TransactionRecord>::iterator, uint256, TxLessThan>' is deprecated: Use std::lower_bound [-Wdeprecated-declarations]
QList<TransactionRecord>::iterator lower = qLowerBound(
qt/transactiontablemodel.cpp:98:52: warning: 'qUpperBound<QList<TransactionRecord>::iterator, uint256, TxLessThan>' is deprecated: Use std::upper_bound [-Wdeprecated-declarations]
QList<TransactionRecord>::iterator upper = qUpperBound(
```
```bash
CXX qt/libbitcoinqt_a-moc_walletcontroller.o
qt/bantablemodel.cpp:64:13: warning: 'qStableSort<QList<CCombinedBan>::iterator, BannedNodeLessThan>' is deprecated: Use std::stable_sort [-Wdeprecated-declarations]
qStableSort(cachedBanlist.begin(), cachedBanlist.end(), BannedNodeLessThan(sortColumn, sortOrder));
```
```bash
CXX qt/libbitcoinqt_a-sendcoinsentry.o
qt/recentrequeststablemodel.cpp:205:5: warning: 'qSort<QList<RecentRequestEntry>::iterator, RecentRequestEntryLessThan>' is deprecated: Use std::sort [-Wdeprecated-declarations]
qSort(list.begin(), list.end(), RecentRequestEntryLessThan(column, order));
```
ACKs for top commit:
hebasto:
ACK 153d9dd9acffa95bb97a4b1579bd20237fdc9c52
promag:
ACK 153d9dd9acffa95bb97a4b1579bd20237fdc9c52.
jonasschnelli:
utACK 153d9dd9acffa95bb97a4b1579bd20237fdc9c52
Tree-SHA512: 22f7290ed798ce8b0f5f313405377845d4c8e48dc8687be7464e27fff53363b451a40e9e18910a8c3b4b9d4dcc236a366c92e7d171fcb8576c48f149a1886c26
b6e9ff899677770741d94f1cf4f61ebb13fc453f build: Remove src/obj directory from repository (Wladimir J. van der Laan)
Pull request description:
This directory is automatically created by the build process (in the build target directory, see #16588) and doesn't need to be in the repository nor in the tarballs.
Move associated ignore directive to top-level `.gitignore` file.
ACKs for top commit:
hebasto:
Concept ACK b6e9ff899677770741d94f1cf4f61ebb13fc453f
Tree-SHA512: 5f3f5a0e8f19ecf925eb16cab327c3023b8512731bbaad5875828da7a25fdda1b77f6fbd06c002a383913627dc9b552f09ad27c57bcf0cb020ed3b1f506e5fef
a47df13471e3168e2e02023fb20cdf2414141b36 [qa] Test disconnect block failure -> shutdown (Suhas Daftuar)
4433ed0f730cfd60eeba3694ff3c283ce2c0c8ee [validation] Crash if disconnecting a block fails (Suhas Daftuar)
Pull request description:
If we're unable to disconnect a block during normal operation, then that is a
failure of our local system (such as disk failure) or the chain that we are on
(eg CVE-2018-17144), but cannot be due to failure of the (more work) chain that
we're trying to validate.
We should abort rather than stay on a less work chain.
Fixes#14341.
ACKs for top commit:
practicalswift:
utACK a47df13471e3168e2e02023fb20cdf2414141b36
TheBlueMatt:
utACK a47df13471e3168e2e02023fb20cdf2414141b36. Didn't bother to review the test in detail, it looked fine. Debated whether invalidateblock should ever crash the node, but *not* crashing in the case of hitting a pruned block (which is the only change here) is clearly better, even if there are other cases I'd argue we should crash in.
ryanofsky:
utACK a47df13471e3168e2e02023fb20cdf2414141b36. Only change since last review is new comment.
promag:
ACK a47df1347, it takes awhile to quit (RPC connection timeouts) but that's unrelated - hope to fix that soon.
fanquake:
ACK a47df13471e3168e2e02023fb20cdf2414141b36
Tree-SHA512: 4dec8cef6e7dbbe513c138fc5821a7ceab855e603ece3c16185b51a3830ab7ebbc844a28827bf64e75326f45325991dcb672f13bd7baede53304f27289c4af8d
a52818cc5633494992da7d1dc8fdb04b4a1b7c29 net: Make poll in InterruptibleRecv only filter for POLLIN events. poll should block until there is data to be read or the timeout expires. (tecnovert)
Pull request description:
poll should block until there is data to be read or the timeout expires.
Filtering for the POLLOUT event causes poll to return immediately which leads to high CPU usage when trying to connect to non-responding peers through tor.
When USE_POLL is not defined select is used with the writefds parameter set to nullptr.
Removing POLLOUT causes the behavior of poll to match that of select.
Fixes: #16004.
ACKs for top commit:
laanwj:
code review ACK a52818cc5633494992da7d1dc8fdb04b4a1b7c29
jonasschnelli:
utACK a52818cc5633494992da7d1dc8fdb04b4a1b7c29
Tree-SHA512: 69934cc14e3327c7ff7f6c5942af8761e865220b2540d74ea1e176adad326307a73860417dddfd32d601b5c0e9e2ada1848bd7e3d27b0b7a9b42f11129af8eb1
5c3c24cf9eab7bf19a9201599e93955cace5c154 test: remove redundant setup in addrman_tests (zenosage)
Pull request description:
#10765 make this default behavior. No reason to keep these line.
Top commit has no ACKs.
Tree-SHA512: 545eea9c2d0741a75708f288f2c8752534ecaa6d54a9d014ef9afa295b0d075007704b64809eec090023703f47753e8ec755d22c9ccecf57b75f6898f6b708dd
a981e749e6553487cd48eda28e590f769e81c85c fix: tor: Call event_base_loopbreak from the event's callback (João Barbosa)
Pull request description:
Calling `event_base_loopbreak` before `event_base_dispatch` has no effect. Fix this by calling `event_base_loopbreak` from the event's callback. From the [documentation](http://www.wangafu.net/~nickm/libevent-2.0/doxygen/html/event_8h.html#a07a7599e478e4031fa8cf52e26d8aa1e):
> event_base_loop() will abort the loop after the next event is completed; event_base_loopbreak() is typically invoked from this event's callback. This behavior is analogous to the "break;" statement.
This can be tested by running the following with and without this change:
```sh
bitcoind -- -regtest -proxy=127.0.0.1:9050 -listen=1 -bind=127.0.0.1 -whitebind=127.0.0.1:0
```
Fixes#16376.
ACKs for top commit:
laanwj:
code review ACK a981e749e6553487cd48eda28e590f769e81c85c
fanquake:
ACK a981e749e6553487cd48eda28e590f769e81c85c
Tree-SHA512: 328fe71366404d5be8177d7081d5b4868cee73412df631a1865d24fb1c153427210762738109e06b737f037f4c68966812fba041831bb9e8129861f19ce61a63
e142ee03e7a139168aa1dbf5910c616f60d25042 doc: describe how to pass wildcard names to test runner (Jon Atack)
6a7a70b8cf05a82737c72020fd2b0eebc97cb5e4 test: enable passing wildcards with path to test runner (Jon Atack)
Pull request description:
Currently, passing wildcard testname args to the test runner from outside the test/functional/ directory does not work, even though developers expect it to. See these recent IRC discussions for more background: http://www.erisian.com.au/bitcoin-core-dev/log-2019-07-10.html#l-262 (lines 262 to 323) and http://www.erisian.com.au/bitcoin-core-dev/log-2019-07-11.html#l-134.
1. [BUGFIX] Enable passing wildcards with paths. Examples:
- `test/functional/test_runner.py test/functional/wallet*`
- `functional/test_runner.py functional/wallet*`
- `test/functional/test_runner.py ./test/functional/tool* test/functional/mempool*`
- A current limitation this PR does not change: 9 test files with arguments in their filename are not picked up by wildcard search.
2. [Docs] Describe how to pass wildcard names (multiple and with paths) to the test runner in test/README.md.
ACKs for top commit:
jnewbery:
tested ACK e142ee03e7a139168aa1dbf5910c616f60d25042
jachiang:
Tested ACK e142ee03e7. Thanks a lot for this fix!
MarcoFalke:
ACK e142ee03e7a139168aa1dbf5910c616f60d25042, fine with me
Tree-SHA512: cb3d994880cdc9b8918546b573a25faa5b4c7339826ac7cfe20f076aac6e731a34271609c0cf5a7ee5e4a2d5ae205298319d24bf36ef5b5d569a1a0c57883e54
8f250ab7882a852f1b1947cef4837d2de5ca6913 TEST: Replace hard-coded hex tx with classes (Steven Roose)
Pull request description:
Came across these breaking Elements.
ACKs for top commit:
MarcoFalke:
ACK 8f250ab7882a852f1b1947cef4837d2de5ca6913
instagibbs:
utACK 8f250ab788
Tree-SHA512: e8615dad4cda0beea4b0c7d4951a467fb9882a0a64d49c9b5ecf167369ea62a3fe5348e2401153162b0ccadecdb128492c94be36ebb881c3c42659626d86eda8
e1a55690e66ca962179bc8170695b92af8a3caa8 Delete error-prone CScript constructor (Gregory Sanders)
Pull request description:
The behavior of this constructor is not the expected behavior compared to the other constructors which directly interpret the vector as a CScript, rather than serialize it into a new CScript. It has only four uses in the entire codebase. Delete this constructor and replace its four uses with the more clear serialization construction.
ACKs for top commit:
Empact:
ACK e1a55690e6
sipa:
Concept and code review ACK e1a55690e66ca962179bc8170695b92af8a3caa8, but I'd like to make sure we have tests covering the FindAndDelete usage.
Tree-SHA512: b6721e343c867ca401a80ec87c25939d7f1fc798f3bf7e5feb0ea6f8280eecb6bd65afc8286912c76ff8119ccea50ad7726b1a4137cae70c9d4fed7d960e10d3
64b27c46e4c21fc7902a69d8ddb6791ef417c4af docs: add reduce-memory.md (fanquake)
Pull request description:
Following some discussion in https://github.com/bitcoin-core/docs/issues/50, this adds Wladimir's [reducing bitcoind memory usage gist](https://gist.github.com/laanwj/efe29c7661ce9b6620a7) to `/doc`.
The conclusion seemed to be that if the main repo already has [reduce-traffic.md](https://github.com/bitcoin/bitcoin/blob/master/doc/reduce-traffic.md), then we could also add `reduce-memory.md`.
ACKs for top commit:
practicalswift:
ACK 64b27c46e4c21fc7902a69d8ddb6791ef417c4af
hebasto:
ACK 64b27c46e4c21fc7902a69d8ddb6791ef417c4af, I have reviewed the changes and they look OK, I agree they can be merged. Also a link from `/doc/README.md` has been tested.
jonasschnelli:
ACK 64b27c46e4c21fc7902a69d8ddb6791ef417c4af
Tree-SHA512: 0ab3035403e5145cfe33c29990a8d082df834ac6602b4ad6bfa821523d57e8451f0cde3017fbf3c2c4e0b34941b6374909d11d27f9598e211bbc14accd487be1
88fd556a969d1120a0ff633bc384336388a11709 Use placeholder instead of key expiration date (Hennadii Stepanov)
Pull request description:
Use a placeholder instead of the actual expiration date, so that the documentation doesn't require updating every time an expiry date changes.
ACKs for top commit:
fanquake:
ACK 88fd556a969d1120a0ff633bc384336388a11709
Tree-SHA512: 391707833cc0e701cf560ec82fd91368468c90a95f85e4ce2a211b20d12463c85775142f28a3536b57c5f6950b9e6e0785632f6f071fa2180bc8aab53008603b
f53a70ce95231d34bb14cd6c58503927e8d7ff59 Improve documentation of memory_cleanse() (Tim Ruffing)
cac30a436cab3641bba3b774d3d3ddbc426e7908 Clean up logic in memory_cleanse() for MSVC (Tim Ruffing)
Pull request description:
When working on https://github.com/bitcoin-core/secp256k1/issues/185, I noticed that the logic in memory_cleanse(), which is supposed to clear memory securely, is weird on MSVC. While it's correct, it's at least a code smell because the code clears the memory twice on MSVC. This weirdness was introduced by #11558.
This PR fixes the logic on MSVC and also improves the docs around this function. Best reviewed in individual commits, see the commit messages for more rationale. The second commit touches only comments.
ACKs for top commit:
practicalswift:
utACK f53a70ce95231d34bb14cd6c58503927e8d7ff59 :-)
laanwj:
code review ACK f53a70ce95231d34bb14cd6c58503927e8d7ff59
Tree-SHA512: 1c2fd98ae62b34b3e6e59d1178b293af969a9e06cbb7df02a699ce8802f145a336f72edb178c520e3ecec81f7e8083828f90a5ba6367d966a2c7d7c0dd6c0475
3b9bf0eb0e0d69f112ce905078018d8351c73e26 rpc: Allow shutdown while in generateblocks (Patrick Strateman)
Pull request description:
By checking the shutdown flag every loop we can use the entire 32 bit nonce space instead of breaking every 16 bits to check the flag.
This is possible now because the shutdown flag is an atomic where before it was controlled by a condition variable and lock.
ACKs for top commit:
kallewoof:
Re-ACK 3b9bf0e
Tree-SHA512: d0664201a55215130c2e9199a31fb81361daf4102a65cb3418984fd61cb98bfb9136d9ee8d23a85d57e50051f9bb0059bd71fe0488a17f63c38ea5caa6004504
d9753383b9e1b61d19d98bcd1d66607f398c7e9f addrdb: Remove temporary files created in SerializeFileDB. Fixes non-determinism in unit tests. (practicalswift)
Pull request description:
Remove temporary files created in `SerializeFileDB` in case of errors.
_Edit: Previously this was hit non-deterministically from the tests: that is no longer the case but the cleanup issue remains :-)_
ACKs for top commit:
laanwj:
code-review ACK d9753383b9e1b61d19d98bcd1d66607f398c7e9f
Tree-SHA512: e72b74b8de411f433bd8bb354cacae07ab75a240db6232bc6a37802ccd8086bff5275ce3d196ddde033d8ab9e2794bb8f60eb83554af7ec2e9f91d6186cb4647
dddd9270f85bd2e71fd281a0c6b4053e02fce93c net: Document what happens to getdata of unknonw type (MarcoFalke)
Pull request description:
Any getdata of unknown type will never be processed and blocks all future messages from a peer. This isn't obviously clear from reading the code, so document it.
Top commit has no ACKs.
Tree-SHA512: 4f8e43bbe6534242facfcfffae28b7a6aa2d228841fa2146a87d494e69f614b0da23cf7a5f3d4367358a7c1981fe2ec196a21c437ae1653f1c7e0351be22598a
