fa3a7331160d1a460b1c15fca1810e98070d629c chainparams: Bump assumed chain params (MarcoFalke)
Pull request description:
As every year, reviewers get extra point when their node is running:
* `assumevalid=0`
* `checkpoints=0`
* on non-x86_64 hardware
See https://github.com/bitcoin/bitcoin/blob/master/doc/release-process.md#before-every-major-and-minor-release for the process.
ACKs for top commit:
laanwj:
ACK fa3a7331160d1a460b1c15fca1810e98070d629c
Sjors:
ACK fa3a7331160d1a460b1c15fca1810e98070d629c for mainnet on macOS 10.14.6.
jamesob:
ACK fa3a733116
fanquake:
ACK fa3a7331160d1a460b1c15fca1810e98070d629c - checked the mainnet values. I have notes on reviewing `assumevalid` updates in [core-review](https://github.com/fanquake/core-review/blob/master/update-assumevalid.md).
Tree-SHA512: fc545ba0a7056908040b47076b393d028c1c022967c25a2074752f76f0386ef099a64445da6125117a04418bd7eb0655121bfc94e6f60b7bc2666947491b5228
fa06bb607da2d3e35723661156d873c8eac1fa50 qa: Do not force overwrite of QT_QPA_PLATFORM on windows for gui tests (MarcoFalke)
faccf5f9c899c40d4da5792629d0714249a4616b doc: Explain QT_QPA_PLATFORM for gui tests (MarcoFalke)
Pull request description:
Closes#17013
ACKs for top commit:
promag:
ACK fa06bb607da2d3e35723661156d873c8eac1fa50.
jonasschnelli:
ACK fa06bb60
ryanofsky:
utACK fa06bb607da2d3e35723661156d873c8eac1fa50
fanquake:
ACK fa06bb607da2d3e35723661156d873c8eac1fa50 - tested on macOS using `QT_QPA_PLATFORM=cocoa src/qt/test/test_bitcoin-qt`.
Tree-SHA512: f257159f6e66b2df7e870ac832ae9ef09eea173c8b7cd766458f87cf22f94681c81dcc54dea030dbc97eab5e3ae5132a4ffe8a343431a4e40f7ee29dc808dcb1
fadd6e0d2a6a5104aa215f456987ad7374bcc6ce doc: Remove mention of renamed mapBlocksUnlinked (MarcoFalke)
Pull request description:
This has been renamed to `m_blocks_unlinked`. Instead of adjusting the internal variable name in the help text, explain the debug flag with more general terms.
ACKs for top commit:
practicalswift:
ACK fadd6e0d2a6a5104aa215f456987ad7374bcc6ce -- diff looks correct
promag:
ACK fadd6e0d2a6a5104aa215f456987ad7374bcc6ce.
laanwj:
ACK fadd6e0d2a6a5104aa215f456987ad7374bcc6ce (as argument help is not translated this doesn't have to wait for the split-off)
Tree-SHA512: 8ad64965ab5bbba4b92933a5adcb0c9eda5bdb0cc080840a4a97b12c67f41f9b789fd289df4932d748f5a7eebc7305a000f03ceb968a78c9b5d9f34af61f0b15
85973bcc44f60fe3bbc952557ebf578dd4c475d2 When BIP70 is disabled, get PaymentRequest merchant using string search (Andrew Chow)
Pull request description:
The merchant name is stored in the X.509 certificate embedded in a PaymentRequest. Use some string searching to locate it so that it can be shown to the user in the transaction details when BIP70 support was not configured.
An additional notice is added to the merchant string that indicates the certificate was not verified. When BIP70 is enabled, the certificate would be verified and the merchant name not shown if the certificate was invalid.
ACKs for top commit:
laanwj:
ACK 85973bcc44f60fe3bbc952557ebf578dd4c475d2
Tree-SHA512: 50fdb60d418e2f9eb65a4b52477be16189f00bfc30493adb27d9fb62100fd5bca33b98b8db6caa8485db424838d3b7a1da802c14ff4917943464401f47391616
60e855f5c50c4578bbab93282226f236853491dc doc: Bump version in bips.md, mention bumping in release process (Wladimir J. van der Laan)
82c11773dc7493e1863b3b8277958f744a321eb9 doc: Add mention of BIP158 indexing since v0.19.0 (Wladimir J. van der Laan)
226700602b23b09d0da64bf67346ffab37bd9e1b doc: Add mention of BIP125 used by wallet GUI by default since v0.18.1 (Wladimir J. van der Laan)
b11514d4e5fa4dd079ef84479f0d63835730ca22 doc: Add mention of BIP70 disabling by default in bips.md (Wladimir J. van der Laan)
Pull request description:
- Add mention of BIP70 disabling by default at build time.
Any others?
E.g. does the burying of deployments of #16060 need to be mentioned? If so, where and how? For all of BIPs 34, 65 and 66?
ACKs for top commit:
hebasto:
ACK 60e855f5c50c4578bbab93282226f236853491dc
Tree-SHA512: 76aac3118bb9b56eeea75d046a55d8678a4c5c43004bec98a653f285ef59c34e67af01b0af3ddcefe4e92d37eea89f4f6627e4d056194f54e2e6168c79b4865c
3eea6a8f2686352a0fd868fee6af42ef1283bdda refactor: Remove Qt function to disable menu icons on macOS (Emil Engler)
Pull request description:
As menu icons were removed in #16612, this removes an unnecessary function for macOS
Could this get into v0.19.0?
ACKs for top commit:
jonasschnelli:
utACK 3eea6a8f2686352a0fd868fee6af42ef1283bdda
promag:
ACK 3eea6a8f2686352a0fd868fee6af42ef1283bdda.
fanquake:
ACK 3eea6a8f2686352a0fd868fee6af42ef1283bdda
Tree-SHA512: b3f2f5ed1141f546351433160e27d95dad914739e89dd3438d11756ca5aa41501f0f08345f2b50415717d88517894d73c1065b17f1bda38132374cc58c08df54
The merchant name is stored in the X.509 certificate embedded in a
PaymentRequest. Use some string searching to locate it so that it
can be shown to the user in the transaction details when BIP70 support
was not configured.
9c23ebd6b18fb1058a8d3e8aae9e0595d3a57ad5 qa: Fix service flag comparison check in rpc_net test (Luke Dashjr)
Pull request description:
Rebase of #16936
ACKs for top commit:
darosior:
ACK 9c23ebd6b18fb1058a8d3e8aae9e0595d3a57ad5
Tree-SHA512: 74f287740403da1040ab1e235ef6eba4e304f3ee5d57a3b25d1e2e1f2f982d256528d398a4d6cb24ba393798e680a8f46cd7dae54ed84ab2c747e96288f1f884
e2ce392aec664c0b729b1890f7fdf1ff2dd23d90 test: Avoid whitespace linting in qt translations (Wladimir J. van der Laan)
977dd23e4023ac2f6cbbe86eb769db079b8018be qt: Periodic translations update (Wladimir J. van der Laan)
Pull request description:
Pull new translations from Transifex (using bitcoin-core/bitcoin-maintainer-tools#36) and run `make translate`.
(maybe the last one before the split-off)
Also added a commit to add `src/qt/locale` to the exclusions for the whitespace linter. I don't think automatically generated files should be linted.
Top commit has no ACKs.
Tree-SHA512: 53aee46d44eceb18f78034febe76ac4d346c643dfc5a16878193433f85db1642977a7028bb2cf99c2c10d972d833c742f7f873991691b5d9f81b2df7b2679bf9
eb4c43e49f625895670866b89bb56ca641c4eeb7 doc: documents how to calculate m_assumed_blockchain_size and m_assumed_chain_state_size on the release process. (marcoagner)
Pull request description:
Regarding [this](https://github.com/bitcoin/bitcoin/pull/15183#issuecomment-463133734) on https://github.com/bitcoin/bitcoin/pull/15183.
Added an "Additional information" section for this which seems reasonable to me but may not be the best place for this. Also, let me know if anything else should be documented here (like more details).
ACKs for top commit:
laanwj:
ACK eb4c43e49f625895670866b89bb56ca641c4eeb7
Tree-SHA512: 7e6fc46740daa01dd9be5a8da7846e7a9f7fa866bf31fdc2cb252f90c698cfd6ef954f9588f7abcebda2355ec2b2a380635e14a164e53e77d38abefa3e2cc698
80031045fc6435ef4eb5dd1aee773d938c57a0fd Clarify includeWatching for fundrawtransaction (Steven Roose)
Pull request description:
Might be sufficient to solve https://github.com/bitcoin/bitcoin/issues/16396, https://github.com/bitcoin/bitcoin/issues/7879 and https://github.com/bitcoin/bitcoin/issues/14405.
ACKs for top commit:
Sjors:
ACK 8003104. This will always be confusing, but at least it gives a bunch more clues for the user to google.
Tree-SHA512: 9b8002c259c50f93d89fc5574105aae6152858d8d45c07b4c3d5b7023adafe73c7a98a290874ff3fbbb7dfad2ac1bdf4acb8769a2a1c14e38484922f44e84e54
463a1d5244337915de36db36de573eb8782bc430 Refresh ZeroMQ 4.3.1 patch (Nathan Marley)
Pull request description:
Currently in Alpine Linux (latest, 3.10) in the depends system, one of the ZeroMQ patches won't apply cleanly because the context around the patch has changed and Alpine's `patch` implementation can't handle the diff.
Some patch implementations can't handle fuzz / too much divergence from the original code.
This PR just tweaks the context code around the patch so that less-sophisticated patch implementations (such as on Alpine Linux) can apply the patch without errors.
This partially fixes#16925
ACKs for top commit:
fanquake:
ACK 463a1d5244337915de36db36de573eb8782bc430 - Tested building zeromq in depends inside an [Alpine container](https://github.com/fanquake/core-review/blob/master/docker/alpine.dockerfile) as well as on macOS.
Tree-SHA512: d6e3cb60835cdd090b9b864ca9cb33961687606bc9184fbbeb7a54ec23db4057b9317b65c5c276fb8c5492cb3cfcc4a7f3369f049551f4eb0915db971f2290ce
568aa0cf8329293692e4249dabd94052533c050d Add OpenSSL termios fix for musl libc (Nathan Marley)
Pull request description:
Currently the version of OpenSSL included in the depends system won't build on musl based systems because `termio.h` does not exist. The proper header named `termios.h` does exist.
This PR adds a patch for OpenSSL to replace the `termio.h` header with `termios.h`, which is the proper POSIX header as I understand it.
This is a known issue as `TERMIOS` (not `TERMIO`) should be the default, and is fixed in later versions of OpenSSL. There is discussion on the OpenSSL repo here: openssl/openssl#163
This has been [fixed in OpenSSL](64e6bf64b3).
This partly fixes#16925 and allows building Bitcoin on Alpine using the depends system.
ACKs for top commit:
laanwj:
ACK 568aa0cf8329293692e4249dabd94052533c050d
Tree-SHA512: d0aac116b7a1133bdecb34a9fb6c63db0336a3547585c07ed31ac9c5edb97e9570dcbf931e7fbc7172ce0735b6bfc11fb204e015532fcd90496a233e8fc17081
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
4320bfc0c0d88633c84146f8d640f5b6e4596244 build: Factor out qt translations from build system (Wladimir J. van der Laan)
Pull request description:
Move qt translations to a separate make include file. This makes it easier to auto-generate this list from tooling (see bitcoin-core/bitcoin-maintainer-tools#36).
ACKs for top commit:
promag:
ACK 4320bfc0c0d88633c84146f8d640f5b6e4596244.
Tree-SHA512: 7133d0103bcf97672ae5aa40ba35d4b81331a8c179190031bbc887da6a5ccc929428e522938db43d87dbcbf9ad3b121dac1e6faf1daa5ae81d0b5fed7f053b5f
43e7d576f590e90ad7d1ba3d550671a7958f1188 doc: Improve test READMEs (Fabian Jahr)
Pull request description:
General improvements on READMEs for unit tests and functional tests:
- Give unit test readme a headline
- Move general information on `src/test` folder to the top
- Add information on logging and debugging unit tests
- Improve debugging and logging information in functional testing
- Include all available log levels in functional tests
ACKs for top commit:
laanwj:
ACK 43e7d576f590e90ad7d1ba3d550671a7958f1188
Tree-SHA512: 22b27644992ba5d99a885cd51b7a474806714396fcea1fd2d6285e41bdf3b28835ad8c81449099e3ee15a63d57b3ab9acb89c425d9855ed1d9b4af21db35ab03
Move qt translations to a separate make include file.
This makes it easier to auto-generate this list from tooling
(see bitcoin-core/bitcoin-maintainer-tools#36).
fdb3e8f8b27e3b0b2f88c32915975c6e4c299b1e Ignore old versionbit activations (Anthony Towns)
Pull request description:
PR 16060 removed the CSV and Segwit BIP9 softfork definitions and hard-coded ('buried') the activation heights. The versionbits code will warn users if an undefined softfork has been signalled in block header versions, and removing the CSV/Segwit definitions caused those warnings to be triggered.
Change the BIP 9 warning code to only check for unknown softforks after the segwit activation height.
ACKs for top commit:
MarcoFalke:
ACK fdb3e8f8b2
ajtowns:
ACK fdb3e8f8b27e3b0b2f88c32915975c6e4c299b1e for what it's worth
achow101:
ACK fdb3e8f8b27e3b0b2f88c32915975c6e4c299b1e
Sjors:
ACK fdb3e8f8b27e3b0b2f88c32915975c6e4c299b1e. It makes the bit 0 warning go away in mainnet and testnet QT when a new block arrives. I think the code is clear enough.
jonatack:
ACK fdb3e8f8b27e3b0b2f88c32915975c6e4c299b1e
Tree-SHA512: e6fd34e8902f8c7affb28e8951803e47d542710d5f1229000746656a37ee59d754439fc33e36b7eef87544262e5aac374645db91b74cb507e73514003ca7a67f
1a02edb3f2803b6f82f06a31acf0b0e5fc19bd1c [RPC] Fix casing in getblockchaininfo to be inline with the rest of the response (Dan Gershony)
Pull request description:
The response in the RPC result `startTime` is camel cased while the rest of the response seems to be lower cased.
If this was intentional please ignore and close this PR.
Note: RPC field case changes might break existing callers
ACKs for top commit:
laanwj:
ACK 1a02edb3f2803b6f82f06a31acf0b0e5fc19bd1c
Tree-SHA512: 6f0eaf2b4aaf73c9a9bf1fbd4af59af5f95fc012fa88f94e050e6ae273b3ad647f5729df53bfce91e1a925fe4fd7b14818908bb6131a81413a555137d1007d7c
8d841ad4928b303465fa0b740d91985ff00b822a doc: Remove MSVC update step from translation process (Wladimir J. van der Laan)
Pull request description:
This part of the build system has been removed in #15529 and thus no longer needs to be updated.
ACKs for top commit:
MarcoFalke:
ACK 8d841ad4928b303465fa0b740d91985ff00b822a
sipsorcery:
ACK 8d841ad
Tree-SHA512: f561a6b1da806e8868a265c77725b94fabef60bc7b9d401e3f70c3d859323adc2e204e3d6fbfea4f1ff86e70667f8bd01157411106ea93974921c02d874e0083
The response in the RPC result `starttime` is camel cased while the rest of the response seems to be lower cased.
If this was intentional please ignore this PR.
Note: case might break existing callers
Reflect the change in the test data
Change to snake case
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
fa607c229295e0f0b89d5177b94d3381ab5e37d8 validation: Make GetWitnessCommitmentIndex public (MarcoFalke)
Pull request description:
`GenerateCoinbaseCommitment` is public and can be used in unit tests to update the witness commitment after the list of txs in a block has been changed. However, for it to work, the existing commitment (added by default in `CreateNewBlock`) must be removed (and thus its index must be known).
Make that possible by exposing the `GetWitnessCommitmentIndex` helper function in the header.
ACKs for top commit:
jb55:
ACK fa607c229295e0f0b89d5177b94d3381ab5e37d8
jamesob:
ACK fa607c2292
promag:
ACK fa607c229295e0f0b89d5177b94d3381ab5e37d8.
fanquake:
ACK fa607c229295e0f0b89d5177b94d3381ab5e37d8 - This unblocks work in #15845.
Tree-SHA512: d563aa2c201d5fb4874e506a28f468c37e457cc8a20229c377178af08c22d3be44e19ee6e8e524b6de99236cd5f2c9e39b8009d88c26854aa774737912bd5889
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 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.
