Age | Commit message (Collapse) | Author |
|
386ae0f6916d11d72022cc6f6b62bb2b82594f24 util: Make thread names shorter (Hennadii Stepanov)
Pull request description:
Thread names at the process level are limited by 15 characters:
https://github.com/bitcoin/bitcoin/blob/6b2210f1016c9ed96ce470e588e4cb12fa36a900/src/util/threadnames.cpp#L28-L29
This commit ensures that name `b-httpworker.42` will not be truncated.
On master (6b2210f1016c9ed96ce470e588e4cb12fa36a900):
```bash
hebasto@redcat:~$ ps -T -p $(cat /home/hebasto/.bitcoin/testnet3/bitcoind.pid)
PID SPID TTY TIME CMD
32647 32647 pts/6 00:00:00 bitcoin-main
32647 32648 pts/6 00:00:00 QXcbEventReader
32647 32649 pts/6 00:00:00 bitcoin:disk$0
32647 32650 pts/6 00:00:00 QDBusConnection
32647 32651 pts/6 00:00:00 gmain
32647 32652 pts/6 00:00:00 gdbus
32647 32653 pts/6 00:00:07 bitcoin-qt-init
32647 32656 pts/6 00:00:00 bitcoin-scriptc
32647 32657 pts/6 00:00:00 bitcoin-scriptc
32647 32658 pts/6 00:00:00 bitcoin-scriptc
32647 32659 pts/6 00:00:00 bitcoin-schedul
32647 32660 pts/6 00:00:00 bitcoin-http
32647 32661 pts/6 00:00:00 bitcoin-httpwor
32647 32662 pts/6 00:00:00 bitcoin-httpwor
32647 32663 pts/6 00:00:00 bitcoin-httpwor
32647 32664 pts/6 00:00:00 bitcoin-httpwor
32647 32665 pts/6 00:00:00 bitcoin-qt-init
32647 32668 pts/6 00:00:00 bitcoin-torcont
32647 32669 pts/6 00:00:00 bitcoin-upnp
32647 32670 pts/6 00:00:00 bitcoin-net
32647 32671 pts/6 00:00:00 bitcoin-dnsseed
32647 32672 pts/6 00:00:00 bitcoin-addcon
32647 32673 pts/6 00:00:00 bitcoin-opencon
32647 32674 pts/6 00:00:00 bitcoin-msghand
32647 32675 pts/6 00:00:00 QThread
32647 32676 pts/6 00:00:00 QThread
```
With this PR:
```bash
hebasto@redcat:~$ ps -T -p $(cat /home/hebasto/.bitcoin/testnet3/bitcoind.pid)
PID SPID TTY TIME CMD
25664 25664 pts/0 00:00:00 b-main
25664 25665 pts/0 00:00:00 QXcbEventReader
25664 25666 pts/0 00:00:00 bitcoin:disk$0
25664 25667 pts/0 00:00:00 QDBusConnection
25664 25668 pts/0 00:00:00 gmain
25664 25669 pts/0 00:00:00 gdbus
25664 25670 pts/0 00:00:07 b-qt-init
25664 25671 pts/0 00:00:00 b-scriptch.0
25664 25672 pts/0 00:00:00 b-scriptch.1
25664 25673 pts/0 00:00:00 b-scriptch.2
25664 25674 pts/0 00:00:00 b-scheduler
25664 25675 pts/0 00:00:00 b-http
25664 25676 pts/0 00:00:00 b-httpworker.0
25664 25677 pts/0 00:00:00 b-httpworker.1
25664 25678 pts/0 00:00:00 b-httpworker.2
25664 25679 pts/0 00:00:00 b-httpworker.3
25664 25680 pts/0 00:00:00 b-qt-init
25664 25682 pts/0 00:00:00 b-torcontrol
25664 25683 pts/0 00:00:00 b-upnp
25664 25684 pts/0 00:00:00 b-net
25664 25685 pts/0 00:00:00 b-dnsseed
25664 25686 pts/0 00:00:00 b-addcon
25664 25687 pts/0 00:00:00 b-opencon
25664 25688 pts/0 00:00:01 b-msghand
25664 25689 pts/0 00:00:00 QThread
25664 25690 pts/0 00:00:00 QThread
```
ACKs for top commit:
MarcoFalke:
ACK 386ae0f6916d11d72022cc6f6b62bb2b82594f24 (skimmed the diff on GitHub)
practicalswift:
ACK 386ae0f6916d11d72022cc6f6b62bb2b82594f24 -- diff looks correct
fanquake:
ACK 386ae0f6916d11d72022cc6f6b62bb2b82594f24 - quickly tested on a Debian system.
Tree-SHA512: 8f7f553213a5856943ddba50611814b771e7dc474101a3e1f98259091684c8d0358c541d32011bfe1da7f430225f01a2d6b514a3b7c5aaf671b2ca8fcf155c4a
|
|
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
|
|
57b0cd4db9ec26d5a00714016639e218cfa1ccda build: Installed Qt version only appears if being built (Jon Layton)
Pull request description:
Closes #16989.
Simplifies `./configure` output for `x$bitcoin_enable_qt`.
Now:
`checking whether to build Bitcoin Core GUI... no`
`checking whether to build Bitcoin Core GUI... yes (Qt5)`
ACKs for top commit:
laanwj:
ACK 57b0cd4db9ec26d5a00714016639e218cfa1ccda
fanquake:
ACK 57b0cd4db9ec26d5a00714016639e218cfa1ccda
Tree-SHA512: 3ca2082f251c206bb1661277dac1075acea046f0a1d2cd997550aa83dd886036b7282f0e30cdaf51ed636d8a40d9b7ffb8600743450f4f4c014c541cb7b2eb0d
|
|
string search
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
|
|
|
|
Thread names at the process level are limited by 15 characters. This
commit ensures that name 'b-httpworker.42' will not be cropped.
|
|
|
|
|
|
|
|
e28d8f893656a5b60dd9c0ec11a88611a56d2ab4 Correct docstring param name. (John Bampton)
Pull request description:
Small fix to correct the Python docstring.
ACKs for top commit:
laanwj:
ACK e28d8f893656a5b60dd9c0ec11a88611a56d2ab4
MarcoFalke:
ACK e28d8f8
Tree-SHA512: 7bec1c6b166c768dd69fc6b94eb80ceeaa0258985b9a11956e336940d403785e7d09d0084d9b870b637ec784db044cf4c0f8ac3f0fcdf431090f003016ef13a9
|
|
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
|
|
variables to release process
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
|
|
|
|
0cc7dd74e0af735dddf7e786f4ed136c382a4ad5 test: add unittests for CheckProofOfWork (soroosh-sdi)
Pull request description:
following situations are covered:
- negative target
- overflow target
- target easier then powLimit
- invalid hash (hash > target)
Signed-off-by: soroosh-sdi <soroosh.sardari@gmail.com>
ACKs for top commit:
promag:
ACK 0cc7dd74e0af735dddf7e786f4ed136c382a4ad5, just read the code.
laanwj:
ACK 0cc7dd74e0af735dddf7e786f4ed136c382a4ad5
Tree-SHA512: 9f9ee952ebb211202939450aa3d61b3c2fae992dcfcab085e877507d78e02ea39a51ccacfc4852a0555f3cba07504ee132abd5cbfed75489553bee45c760bc7e
|
|
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](https://github.com/openssl/openssl/commit/64e6bf64b36136d487e2fbf907f09612e69ae911).
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
![Before](https://i.imgur.com/ppZ3Gf9.png)
### After
![After](https://i.imgur.com/wcElqLA.png)
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
![out](https://user-images.githubusercontent.com/14386/65541887-69424e00-df0e-11e9-8644-b3a068ed8c3f.png)
ACKs for top commit:
jamesob:
Tested ACK https://github.com/bitcoin/bitcoin/pull/16957/commits/67d99900b0d770038c9c5708553143137b124a6c
Tree-SHA512: 9c44e3cca993b5a564dd61ebd2926b9c4a238609ea4d283514c018236f977d935e35a384dd4696486fd3d78781dd2ba190bb72596e20a5e931042fa465872a0b
|
|
Pull new translations from Transifex and run `make translate`.
|
|
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).
|
|
warning
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
|
|
fields
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
|
|
following situations are covered:
- negative target
- overflow target
- target easier then powLimit
- invalid hash (hash > target)
- zero target
Signed-off-by: soroosh-sdi <soroosh.sardari@gmail.com>
|
|
|
|
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
|
|
|
|
|
|
This part of the build system has been removed in #15529 and thus no
longer needs to be updated.
|
|
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
|
|
1b4030e26450b0f9320d64714d8e8dee37b050d2 depends: qt: Fix LDFLAGS pickup (Carl Dong)
6eb12ffcbdca53fac304d8fedcfc69effab63b0d depends: qt: Fix C{,XX}FLAGS pickup (Carl Dong)
Pull request description:
Note: ~~Will~~ [Did](https://github.com/bitcoin/bitcoin/issues/16838) open issue about robustness of `sed` calls in `depends`.
ACKs for top commit:
laanwj:
ACK 1b4030e26450b0f9320d64714d8e8dee37b050d2
Tree-SHA512: d0bfc8ea32118cd90bb323efab58661f2218a2cb0f150e716cfd5355c7e2a1eba70298a144b159941248170e2894659c376219edac2c79a9d777f6ada5fa6b2f
|
|
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 https://github.com/bitcoin/bitcoin/pull/16956/commits/fa607c229295e0f0b89d5177b94d3381ab5e37d8
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.
|
|
f4beb4996d27f2cdaf4f0a63e7dc044bf17decce test: Remove python dead code linter (Wladimir J. van der Laan)
Pull request description:
Primarily I'd like to remove this because it is very imprecise, due to Python's dynamic nature, giving it a large list of false positives that need to be listed as exceptions. See for example #16906.
It's also a frequent source of complaints. I'm doubtful of the usefulness of checking for dead code in a linter in the first place.
Having some dead code in the test framework for a while is not a
disaster.
ACKs for top commit:
sdaftuar:
utACK f4beb4996d27f2cdaf4f0a63e7dc044bf17decce
practicalswift:
ACK f4beb4996d27f2cdaf4f0a63e7dc044bf17decce -- diff looks correct
jamesob:
ACK https://github.com/bitcoin/bitcoin/pull/16961/commits/f4beb4996d27f2cdaf4f0a63e7dc044bf17decce
Tree-SHA512: 329b1555210311d5d15799fd2cb794b3208b0ac4d8a2ffaf4dece1bcc3e0e8b1fe952d5e7a394f94a98919cab579fb579eae7db2a796cc9a1a42ef495dd17507
|
|
fa7847d99b7084f1de5dc61506069e32de43a498 gui: Rename address checkbox back to bech32 (MarcoFalke)
Pull request description:
This is the wording that has been used in the previous release, so translations should still exist for it.
Fixes: #16924
ACKs for top commit:
promag:
ACK fa7847d99b7084f1de5dc61506069e32de43a498.
laanwj:
ACK fa7847d99b7084f1de5dc61506069e32de43a498
Tree-SHA512: 0ac6c47fe5eb2145b609a30fd3f56052d3e08abe6c67fc74b6d209a55a4df509c52f13eb1c759520a4fa43916ece0e6d4cefef87e061b51114a6582db911944a
|
|
This prevents the common annoyance of the text being included into PRs
accidentally.
|
|
fadd76acc25907bd94ea759f5f2b719ebc3fd2b4 ci: Remove TRAVIS env vars (MarcoFalke)
fa449b89b51df7670248cb3504d91f93fc68e0c2 ci: Set $HOST before setting fallback values (MarcoFalke)
Pull request description:
This shouldn't change anything, except that `$HOST` is now properly set in the `$BASE_OUTDIR`. (Previously it would always use `x86_64-unknown-linux-gnu` in the directory name and now it should use the correct host)
The second commit removes travis environment variables in the `ci` system. Also, shouldn't change any behavior.
ACKs for top commit:
fanquake:
ACK fadd76acc25907bd94ea759f5f2b719ebc3fd2b4 - assuming Travis etc is happy.
Tree-SHA512: aafd65bfc039523208b17d1ed886a3311995d984ec56c3de5f837b5a71d985061ee2da7af947f95a56ab101a0666fe7cd99434e196cd1b7ee9c460d156a185f6
|
|
6659810e2f38994813aa9d7644d570ae0152fa2c test: use named args for sendrawtransaction calls (Jon Atack)
5c1cd78b7e582660a78d9d9dec673967a6b78936 doc: improve rawtransaction code/test docs (Jon Atack)
acc14c50932c7353f94d3d4367d05021606e0ca9 test: fix incorrect value in rpc_rawtransaction.py (Jon Atack)
Pull request description:
Follow-up to PR #16521.
- Fix incorrect value in rpc_rawtransaction test as per https://github.com/bitcoin/bitcoin/pull/16521/files#r325842308
- Improve the code docs
- Use named arguments as per https://github.com/bitcoin/bitcoin/pull/16521/files#r310715127
Happy to squash or keep only the first commit if the others are too fixup-y.
ACKs for top commit:
laanwj:
ACK 6659810e2f38994813aa9d7644d570ae0152fa2c
Tree-SHA512: bf5258f23802ab3ba3defb8791097e08e63f3e2af21023f832cd270dc88d1fa04349e921d69f9f5fedac5dce5cd3c1cc46b48febbede4bc18dccb8be994565b2
|