ddc7e42d600a0cb3e763cda0dc04a1f2f34e9440 build: Set minimum Automake version to 1.13 (Hennadii Stepanov)
Pull request description:
This PR suggests to set the required minimum Automake version to `1.13` explicitly for the following reasons:
- it guarantees that [CVE-2012-3386](https://lists.gnu.org/archive/html/automake/2012-07/msg00023.html) has been fixed
- `AC_CONFIG_MACRO_DIR` macro support, which we already use; from the [release notes](https://lists.gnu.org/archive/html/automake/2012-12/msg00038.html):
> Improvements to aclocal and related rebuilds rules:
> - Autoconf-provided macros AC_CONFIG_MACRO_DIR and AC_CONFIG_MACRO_DIRS are now traced by aclocal, and can be used to declare the local m4 include directories. Formerly, one had to specify it with an explicit '-I' option to the 'aclocal' invocation.
- `AM_SILENT_RULES` macro support (since version `1.11`)
Automake `1.13` requires Autoconf `2.65` or greater. We already have `2.69` since #17769.
---
For reference, Automake `1.13` was released in [December of 2012](https://lists.gnu.org/archive/html/automake/2012-12/msg00038.html).
CentOS 7 uses Automake [`1.13.4`](https://centos.pkgs.org/7/centos-x86_64/automake-1.13.4-3.el7.noarch.rpm.html)
See the Automake docs for more info:
- [`AM_INIT_AUTOMAKE`](https://www.gnu.org/software/automake/manual/automake.html#Public-Macros)
- [List of Automake options](https://www.gnu.org/software/automake/manual/automake.html#List-of-Automake-options)
ACKs for top commit:
laanwj:
so also ACK ddc7e42d600a0cb3e763cda0dc04a1f2f34e9440
fanquake:
ACK ddc7e42d600a0cb3e763cda0dc04a1f2f34e9440 - I think adding a minimum required version here is fine. I'd be surprised if someone who is currently building Bitcoin Core was unable to after this change.
Tree-SHA512: a1f97864bc3a513450c03d041498f28e823e6f8cd9710d81df081435d72bd4b6cd2f3deb997dbf902f950215a859e48a2ee7ca1f8ebf4271778dd951ab78abf4
fab7d14ea5a4305317d66f35beb3225a07823d42 test: Check that wait_until returns if time point is in the past (MarcoFalke)
Pull request description:
Add an explicit regression test for the condvar bug (#18227), so that this doesn't happen again
ACKs for top commit:
laanwj:
ACK fab7d14ea5a4305317d66f35beb3225a07823d42
Tree-SHA512: 6ec0d0b3945cae87a001e367af34cca1953a8082b4a0d9f8a20d30acd1f36363e98035d4eb173ff786cf6692d352d41f960633415c46394af042eb44e3b5ad71
aff2748f8aee72f03b5fb6f6f97f0d0f66391755 httpserver: use own HTTP status codes (Filip Gospodinov)
Pull request description:
Before, macros defined in `<event2/http.h>` have been used for some HTTP status codes. `<event2/http.h>` is included implicitly and the usage of its status code macros is inconsistent with the majority HTTP response implementations in this file.
Now, the `HTTPStatusCode` enum from `<rpc/protocol.h>` is consistently used for all HTTP response implementations.
ACKs for top commit:
practicalswift:
ACK aff2748f8aee72f03b5fb6f6f97f0d0f66391755 -- patch looks correct
laanwj:
ACK aff2748f8aee72f03b5fb6f6f97f0d0f66391755
Tree-SHA512: 6a7043488b88dcd584215d16b5f16f7bd668fe5553d31298d1beef134d2b0648aef81533014e34d1cd600baa36ee4e853f195352f4a00c866bd5ab1ff688bd50
fac52dafa013047b051ca7163cc30ac69ad35531 test: Set catch_system_errors=no on boost unit tests (MarcoFalke)
Pull request description:
Closes#16700
Can be tested by adding an `assert(0)` and then running either `make check` or `./src/test/test_bitcoin -t bla_tests --catch_system_errors=no/yes`
ACKs for top commit:
practicalswift:
ACK fac52dafa013047b051ca7163cc30ac69ad35531
Empact:
Tested ACK fac52dafa0
Tree-SHA512: ec00636951b2c1137aaf43610739d78d16f823f7da76a726d47f93b8b089766fb66b21504b3c5413bcf8b6b5c3db0ad74027d677db24a44487d6d79a6bdee2e0
d6d2602a32251c1017da88b47c801b7283c66ce3 add: test that transactions expire from mempool (0xb10c)
Pull request description:
This adds the functional test `mempool_expiry.py` covering mempool transaction expiry. Both the default `DEFAULT_MEMPOOL_EXPIRY` of 336 hours (two weeks, set in #9312) and the user definable mempool expiry via the `-mempoolexpiry=<n>` command line option are tested. The test checks that descendants of expired transactions are removed as well.
*Notes for reviewers*
- `LimitMempoolSize()` (which is the only caller of `CTxMemPool::Expire()`) is only called when a transaction is added to the mempool. In order to test expiry of a transaction-that-should-expire, the mocktime is set and a random transaction is broadcast to trigger `LimitMempoolSize()`. The transaction-that-should-expire is then checked for expiry. LMK if there is another way, but I don't think there is.
ACKs for top commit:
MarcoFalke:
ACK d6d2602a32251c1017da88b47c801b7283c66ce3
theStack:
ACK d6d2602a32
promag:
Code review ACK d6d2602a32251c1017da88b47c801b7283c66ce3.
Tree-SHA512: eb68cd9e2d870872b8e8e1522fed8954fb99cc9e4edda4b28bb2a4e41cddbc53fe6f7d9c090f1e0e98ab49beb24bf37ff3787a9e9801a95e8ae9ca9eb34fe6f0
31879345ee9a222b12014c8b2359e1737ff0d8c4 cli: Add "headers" and "verificationprogress" to -getinfo (Wladimir J. van der Laan)
Pull request description:
These values are useful to know the current progress of initial sync, or of catching up, which is arguably the use of a quick `-getinfo` command.
ACKs for top commit:
MarcoFalke:
unsigned ACK 31879345ee9a222b12014c8b2359e1737ff0d8c4
jonasschnelli:
utACK 31879345ee9a222b12014c8b2359e1737ff0d8c4
jonatack:
Tested ACK 31879345ee9a222b12014c8b2359e1737ff0d8c4 on Debian 4.19.37-5+deb10u2 (2019-08-08) x86_64 GNU/Linux
Tree-SHA512: 185180ab426b4db5d99eb208ee88d1606f585361875ba3a92b6c28a74fe181d72ed710c8859b969ba49b1ca7d2385695932b79ff621c7a2a7cedd0df717a99ed
4896bacc00549c14f3284f5a2b61fb848ac31be0 Add testcase to simulate bitcoin schema in leveldb (MapleLaker)
Pull request description:
Resurrecting #14125 with updates based on comments of closed PR
ACKs for top commit:
laanwj:
ACK 4896bacc00549c14f3284f5a2b61fb848ac31be0
dongcarl:
ACK 4896bacc00549c14f3284f5a2b61fb848ac31be0
Tree-SHA512: 3290ea7e1e998901d5ee8921d1d76cec399cae30ac1911a45b86826afed47cee1acf92bd6438f1fa11ed785a3b17abdcb1c169bc0419945eda9fe4c089d0b6eb
088a730fe6f893d43b55ec52e6b5080fd445dd9e static tooltip (JeremyCrookshank)
Pull request description:
I noticed that on Bitcoin sends the tooltip wasn't very clear for new users and I hope my PR is more concise. If it needs changing more will happily change too 👍
ACKs for top commit:
laanwj:
ACK 088a730fe6f893d43b55ec52e6b5080fd445dd9e
Tree-SHA512: 2b1103ac934d8f68d22333af3c0f5d4228b665b1e507378d4ae5b83cc2b6d6aeb46a3d68298cca93feb839db5caa560322c8df5261dc2f7db5abeed9f0dd9c69
f09ba060cacd42e4cb9a242c1d731deb1f6623c6 doc: Added instructions for how to add an upsteam to forked repo (dannmat)
Pull request description:
As a first time git developer, I struggled to understand whether to create a new fork for each pull request or not.
After asking the IRC chat, I have added this to the documentation to further help new developers using git.
ACKs for top commit:
fanquake:
ACK f09ba060cacd42e4cb9a242c1d731deb1f6623c6 - For such a simple change, I think we've bike-shed this enough already. The `bitcoin/bitcoin` repo isn't really where anyone should be learning how to use `git` etc, but I think linking out here is ok.
Tree-SHA512: e0e9d655d0725e0128673afedb81dc5ba9387968fcbb681de7e50155a2cfa1a7f39fad040b596f4de9ad6727a1a8a90fd3d36eaa5242bc12186c3b82abd23fb2
7005d6ab8f4393545ec27f528d8c03fc4516ddab gui: Add placeholder text to the sign message field (Danny-Scott)
Pull request description:
When using the sign message functionality I noticed the "message" field had no label or placeholder text to highlight what it's for.
I've added the placeholder text to match the tool tip to help it be more user friendly.
ACKs for top commit:
hebasto:
Re-ACK 7005d6ab8f4393545ec27f528d8c03fc4516ddab
fanquake:
ACK 7005d6ab8f4393545ec27f528d8c03fc4516ddab
Tree-SHA512: 17fe51c134f6373d8d5f9ca98b15bd936da4e61aa5258ceb5d318575d49b43cbfde6f4c3f720eb5928206902e6ba52811ba08737a03c95224e45dabc947d9d11
fa0467326ff004a0a20c6af9c8f6adcc74a0c8af chain: Set all CBlockIndex members to null, remove SetNull helper (MarcoFalke)
Pull request description:
The first commit removes the `SetNull` helper and inlines the member initialization (C++11). See https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#c-data-structures for rationale.
<strike>The second commit adds the `cs_main` lock annotation to `RaiseValidity`. See also #17161.</strike>
ACKs for top commit:
promag:
Code review ACK fa0467326ff004a0a20c6af9c8f6adcc74a0c8af.
practicalswift:
ACK fa0467326ff004a0a20c6af9c8f6adcc74a0c8af -- diff still looks correct :)
laanwj:
ACK fa0467326ff004a0a20c6af9c8f6adcc74a0c8af, this makes it easy to see that all fields are initialized.
Tree-SHA512: 1b2b9fb0951c03c75b9cce322b89d4ecc9a364ae78b94d91b0b4669437824394dfada820ab6f74dfac3193f602899abfdc244ae2d9351ad293f555488f03470e
b3b26e149c34fee9c7ae8548c6e547ec6254b441 rpc: fix -rpcclienttimeout 0 option (Fabian Jahr)
Pull request description:
fixes#17117
I understood the bug as the help string being wrong, rather than that this feature is missing and should be added. Let me know if it should be the other way around.
It is notable that if 0 is given as an argument, the fallback that is being used is the libevent default of 50 seconds, rather than `DEFAULT_HTTP_CLIENT_TIMEOUT` (900 seconds). This is not intuitive for the user. I could handle this in this PR but I am unsure which would be the better solution then: Actually adding the feature as described in the help string or falling back to `DEFAULT_HTTP_CLIENT_TIMEOUT`? Happy to hear opinions.
ACKs for top commit:
MarcoFalke:
unsigned ACK b3b26e149c34fee9c7ae8548c6e547ec6254b441
Tree-SHA512: 65e526a652c0adcdb4f895e8d78d60c7caa5904c9915b165a3ae95725c87d13af1f916359f80302452a2fcac1a80f4c58cd805ec8c28720fa4b91b3c8baa4155
610d9384de7f4de861d94c9b1af4fddc8aa57ad9 gui: Added label & tooltip for Verify Message labels (dannmat)
Pull request description:
When using the Verify Message functionality, I found the input boxes to be rather confusing as they had no guidance for their purpose.
I have added tooltips and labels to aid users when verifying messages in future
ACKs for top commit:
promag:
Code review ACK 610d9384de7f4de861d94c9b1af4fddc8aa57ad9. Nit, commit and title are a little weird. Suggestion: "gui: Add toolTip and placeholderText to sign message fields"
MarcoFalke:
ACK 610d9384de7f4de861d94c9b1af4fddc8aa57ad9 (looks good, didn't compile or tested the changes)
fanquake:
ACK - 610d9384de7f4de861d94c9b1af4fddc8aa57ad9
Tree-SHA512: d6a1bc872ad270dce440e96a163ce72cdd4708913d87a0fea749fc8cf2d8163b791cbb96a82030e0cb7d239920ceb0e3f05e0eec113f45a1a8e1309fbd92b4b0
44f7a8d7a774f82417106c452d793e6f091bc23e Disable _FORTIFY_SOURCE when enable-debug (Andrew Chow)
Pull request description:
The `_FORTIFY_SOURCE` macro is enabled by default when hardening is enabled, but it requires optimization in order to be used. Since we disable all optimization with `--enable-debug`, this macro doesn't actually do anything and instead just causes a lot of warnings to be printed. This PR explicitly disables `_FORTIFY_SOURCE` so that these useless warnings aren't printed.
ACKs for top commit:
laanwj:
Thanks. ACK 44f7a8d7a774f82417106c452d793e6f091bc23e
Tree-SHA512: e9302aef794dfd9ca9d0d032179ecc51d3212a9a0204454419f410011343b27c32e6be05f385051b5b594c607b91b8e0e588f644584d6684429a649a413077d9
facb9a1315f97489a20eb0e969fdb14b5128ed2f init: Change fallback locale to C.UTF-8 (Wladimir J. van der Laan)
Pull request description:
Much of our code assumes file system UTF-8 support, and this is a more realistic guess for modern systems anyway than the default character set (which would be ASCII only). So change the assumed fallback locale (if no locale is defined by the user or OS) to `C.UTF-8`.
related: https://github.com/bitcoin/bitcoin/issues/14948#issuecomment-488385462
ACKs for top commit:
MarcoFalke:
ACK facb9a1315f97489a20eb0e969fdb14b5128ed2f
Tree-SHA512: 5075f9fe6791572d76ec38c58cd56f04ed8086c06a7d7f446d062dffc313c62466ba81f1a7d6b8c7e95791fcff82e4f76871c3534478fbfe5beb456dd8eea340
181989f6c9427fc266dbdcc84cb60ac03e67cdb2 build: Add variable printing target to Makefiles (Carl Dong)
Pull request description:
```
I kept finding myself needing these to debug our build system, since
they are innocuous and are very helpful they probably belong in the
codebase.
Source: John Graham-Cumming
https://www.cmcrossroads.com/article/printing-value-makefile-variable
```
ACKs for top commit:
MarcoFalke:
ACK 181989f6c9427fc266dbdcc84cb60ac03e67cdb2
fanquake:
ACK 181989f6c9427fc266dbdcc84cb60ac03e67cdb2 - concise amount of useful code. Tested on macOS. Did not visit the link.
Tree-SHA512: 2139621e68a499c7347663ca9dc04e166ea6280e05986c27858df0156016ef2f9461262464d70c601419384f43a4ae3bcc67dfc0a05dbeef64f08386ab429cd8
30fc1a3f5470cb347eb4aed281242f120510466f build: Remove workaround for ancient libtool (Hennadii Stepanov)
6ca01b9a104ebadbe7e180cb2b9d390f7c09b4ab build: Ensure a minimal version of libtool (Hennadii Stepanov)
Pull request description:
Since libtool 1.5.2, on Linux libtool no longer sets RPATH for any directories in the dynamic linker search path, so there is no longer an issue.
This commit reverts a98356fee8.
Refs:
- https://wiki.debian.org/RpathIssue
- [Debian jessie has libtool 2.4.2](https://packages.debian.org/jessie/libtool)
ACKs for top commit:
laanwj:
ACK 30fc1a3f5470cb347eb4aed281242f120510466f
Tree-SHA512: fab56265d4d2c96216a353cc076c6f510e15748d8134f97bae2f67b6d8c0b6a1a9f362d2ab23b19ccc3a8bba8eac3bb1668fc3e42037590f63a7ab4819c9ee15
8acd58927a614e006641384f61f8e7fca1cc68fc Fix Python Docstring to include all Args. (John Bampton)
Pull request description:
Found a Python function that had incorrect and missing arguments in its Docstring.
ACKs for top commit:
laanwj:
ACK 8acd58927a614e006641384f61f8e7fca1cc68fc
Tree-SHA512: 936f275f29a700d630bb479b5283e47b66f2df76d8b8c053f594e6aedf783cc98a29c924c3a46613f112dfc884acb50f21a0b18f96d939e887b12b921ef2e10f
3f89e1eb237efcbd6415ca2cd0acddb6596153d7 Prevent processing duplicate payment requests (João Barbosa)
Pull request description:
Considering the following from Qt [src/plugins/platforms/cocoa/qcocoaapplicationdelegate.mm#L267](13e0a36626/src/plugins/platforms/cocoa/qcocoaapplicationdelegate.mm (L267))
```cpp
- (void)application:(NSApplication *)sender openFiles:(NSArray *)filenames
{
Q_UNUSED(filenames);
Q_UNUSED(sender);
for (NSString *fileName in filenames) {
QString qtFileName = QString::fromNSString(fileName);
if (inLaunch) {
// We need to be careful because Cocoa will be nice enough to take
// command line arguments and send them to us as events. Given the history
// of Qt Applications, this will result in behavior people don't want, as
// they might be doing the opening themselves with the command line parsing.
if (qApp->arguments().contains(qtFileName))
continue;
}
QWindowSystemInterface::handleFileOpenEvent(qtFileName);
}
```
And that a2714a5c69f0b0506689af04c3e785f71ee0915d was merged, now Qt isn't able to filter out the above notifications, and then a [QFileOpenEvent](https://doc.qt.io/qt-5/qfileopenevent.html) event is delivered to `PaymentServer::eventFilter`, which in turn (re)adds the payment request.
This change fixes#17025, but makes sense regardless of the issue.
ACKs for top commit:
laanwj:
Nah, this seems fine, utACK 3f89e1eb237efcbd6415ca2cd0acddb6596153d7
Sjors:
ACK 3f89e1e on macOS 10.14.6
achow101:
Code review ACK 3f89e1eb237efcbd6415ca2cd0acddb6596153d7
Tree-SHA512: dd1e0c73fd84953418173ca71f6f5a67ad74a5dc7e3b1d54915ef0545f513df6a24f27242a77bb094e2833a478e2f3bf30ecd50251f3c55b65e780097cb8ab4d
a649cc6a17b8d8d602c6b67037b0c926960f9cdb Change sendcoins dialogue Yes to Send (Gregory Sanders)
Pull request description:
It's more self-explanatory, matches "cancel" better, and makes future extensions such as https://github.com/bitcoin/bitcoin/pull/16944 more directly understandable to the user.
ACKs for top commit:
Sjors:
Trivial code review ACK a649cc6. I also used Send in #16966 (`ui - make send a wizard`)
laanwj:
ACK a649cc6a17b8d8d602c6b67037b0c926960f9cdb
jonatack:
Code review ACK a649cc6a17b8d8d602c6b67037b0c926960f9cdb
Tree-SHA512: fe4993bc7ac653d28f3d399ade046bcfd405511aec06ff041bb5aef47e0736faf3e3112a6db660cd761af56392dc6b97f2c2341ed3eff4490079c5eb8a0d465a
addaf8af8268d918973883a304025d40af5a33c1 make sure to update the UI when deleting a transaction (Jonas Schnelli)
Pull request description:
`CWallet::ZapSelectTx` removes transactions from the internal model, but leaves the UI in the dark.
Adding a `NotifyTransactionChanged()` should avoid having invalid transactions in the GUI.
Fixes#16950
ACKs for top commit:
fanquake:
ACK addaf8af8268d918973883a304025d40af5a33c1 - tested that this fixes#16950
Sjors:
tACK addaf8a: tested with an unpruned wallet by calling `removeprunedfunds` on an RBF-replaced transaction. It neatly disappears from the UI.
kristapsk:
ACK addaf8af8268d918973883a304025d40af5a33c1 (tested both with and without this change)
Tree-SHA512: 65e8c690847f7499e82c9fef67b60d9aaa63c853732fe7fa7281da33054fcdcd9d24f5b86de71b0827728c25bac8efb7db445863f990304ebfee6fc450620c47
8cf9898b53d74bd474cc5188e07e671e24a3791f qt: Change default size of intro frame (Emil Engler)
Pull request description:
Because of the new pruning feature in the intro frame, the size of the intro frame is too small.
Like you see, some text is not visible completely.
### Before
### After
Update: I changed it so it adjusts the size dynamically
ACKs for top commit:
fanquake:
ACK 8cf9898b53d74bd474cc5188e07e671e24a3791f - Before and after macOS screens below. Given that most users will only ever see this screen once, I think Qts best effort to dynamically size it is fine.
jonasschnelli:
utACK 8cf9898b53d74bd474cc5188e07e671e24a3791f
Sjors:
Tested ACK 8cf9898 on macOS. English already fit, so to reproduce the issue, launch in German with `-resetguisettings -lang=de`.
laanwj:
ACK 8cf9898b53d74bd474cc5188e07e671e24a3791f
Tree-SHA512: 568b0ae0d5feeda603c0ccf67b5bb3857becea8f22fb98695e1901e662cb1e76377589e39ec743258154d7f6c4a5e544bb003fcc73597400dd427db047392638
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
