fix windows test run
unit test round 2
attempting to fix ccache use on darwin
made ccache optional, no longer pulls clang on darwin build
fixing darwin build from Dockerfile
fixed missing nproc on OSX
updated readme to include regtest example, build examples
fix QT unit tests
made -j get passed down, added build.sh
This contains significant rebase / merge / testing work by Naut
<lbrynaut@protonmail.com>, Anthony Fieroni <bvbfan@abv.bg> and Brannon
King <countprimes@gmail.com>.
Instead of adding BerkeleyEnvironment objects permanently to the g_dbenvs map,
use reference counted shared pointers and remove map entries when the last
BerkeleyEnvironment reference goes out of scope.
This change was requested by Matt Corallo <git@bluematt.me> and makes code that
sets up mock databases cleaner. The mock database environment will now go out
of scope and be reset on destruction so there is no need to call
BerkeleyEnvironment::Reset() during wallet construction to clear out prior
state.
This change does affect bitcoin behavior slightly. On startup, instead of same
wallet environments staying open throughout VerifyWallets() and OpenWallets()
calls, VerifyWallets() will open and close an environment once for each wallet,
and OpenWallets() will create its own environment(s) later.
Github-Pull: #11911
Rebased-From: f1f4bb7
Adds a ReloadDbEnv function to BerkeleyEnvironment in order to close all Db
instances, closes the environment, resets it, and then reopens
the BerkeleyEnvironment.
Also adds a ReloadDbEnv function to BerkeleyDatabase that calls
BerkeleyEnvironment's ReloadDbEnv.
Github-Pull: #12493
Rebased-From: 5d296ac
This moves the Dock icon click reaction code to the common place and
allows some cleanup in obj_c code.
According to the Apple's docs `class_replaceMethod` behaves as
`class_addMethod`, if the method identified by name does not yet exist;
or as `method_setImplementation`, if it does exist.
Github-Pull: #14597
Rebased-From: 2464925e7b
Refactor the process of PSBTInput signing to enforce the invariant that
a PSBTInput always has _either_ a witness_utxo or a non_witness_utxo,
never both.
This simplifies the logic of SignPSBTInput slightly, since it no longer
has to deal with the "both" case. When calling it, we now give it, in
order of preference: (1) whichever of the utxo fields was already
present in the PSBT we received, or (2) if neither, the
non_witness_utxo field, which is just a copy of the input transaction,
which we get from the wallet.
SignPSBTInput no longer has to remove one of the two fields; instead, it
will check if we have a witness signature, and if so, it will replace
the non_witness_utxo with the witness_utxo (which is smaller, as it is
just a copy of the output being spent.)
Add PSBTInput::IsSane checks in two more places, which checks for
both utxo fields being present; we will now give an RPC error early on
if we are supplied such a malformed PSBT to fill in.
Also add a check to FillPSBT, to avoid touching any input that is
already signed. (This is now redundant, since we should no longer
potentially harm an already-signed input, but it's harmless.)
fixes#14473
Github-Pull: #14588
Remove redundant arguments to SignPSBTInput -- since it needs several
bits of the PartiallySignedTransaction, pass in a reference instead of
doing it piecemeal. This saves us having to pass in both a PSBTInput and
its index, as well as having to pass in the CTransaction. Also avoid
redundantly passing the sighash_type, which is contained in the
PSBTInput already.
Github-Pull: #14588
Rebased-From: 0f5bda2bd9
Refactor out a "PSBTInputSigned" function to check if a PSBT is signed,
for use in subsequent commits.
Also improve a related comment.
GitHub-Pull: #14588
Rebased-From: 53e6fffb8f
Poll function was wrongly setting cached height to the current chain height
instead of the chain height at the time of polling.
This bug could cause balances to appear out of date, and was first introduced
a0704a8996 (r378452145)
Before that commit, there wasn't a problem because cs_main was held during the
poll update.
Currently, the problem should be rare. But if
8937d99ce81a27ae5e1012a28323c0e26d89c50b from #17954 were merged, the problem
would get worse, because the wrong cachedNumBlocks value would be set if the
wallet was polled in the interval between a block being connected and it
processing the BlockConnected notification.
MarcoFalke <falke.marco@gmail.com> also points out that a0704a8996 could lead
to GUI hangs as well, because previously the pollBalanceChanged method, which
runs on the GUI thread, would only make a nonblocking TRY_LOCK(cs_main) call,
but after could make blocking LOCK(cs_main) calls, potentially locking up the
GUI.
Thanks to John Newbery <john@johnnewbery.com> for finding this bug this while
reviewing https://github.com/bitcoin/bitcoin/pull/17954.
Github-Pull: #18123
Rebased-From: bf36a3ccc212ad4d7c5cb8f26d7a22e279fe3cec
f5fb7fca96 psbt: check output index is within bounds before accessing (Andrew Chow)
1cf77a2dc3 Don't calculate tx fees for PSBTs with invalid money values (Andrew Chow)
Pull request description:
Backport of #17156, non-trivial due to crossing the refactor in #17371
ACKs for top commit:
laanwj:
ACK f5fb7fca96
Tree-SHA512: 7aabf9a6b8a8e287a26dfbf73a437a3bc55177bef8fc5149d822ef81b8ef2458e1d718c3a19c73532c5cef0f9bd8144574c7fad90ca89f13a08b44edf3a2656d
If a destination is reused we mark the cache of the other transactions going to that destination dirty so they are not accidentally reported as trusted when the cache is hit.
Github-Pull: #17843
Rebased-From: 6fc554f591d8ea1681b8bb25aa12da8d4f023f66
In decodepsbt if an invalid amount is seen, don't calculate the fee
but still show the invalid value in the decode.
In analyze psbt, if an invalid amount is seen, set the next step to
be the creator as the creator needs to remake the transaction so that
it is valid.
Github-Pull: #17156
Rebased-From: f1ef7f0aa46338f4cd8de79696027a1bf868f359
daf2fff236 test: add missing #include to fix compiler errors (Karl-Johan Alm)
c8ad23c529 bug-fix macos: give free bytes to F_PREALLOCATE (Karl-Johan Alm)
Pull request description:
We're about ready to do a [0.19.1 release](https://github.com/bitcoin/bitcoin/milestone/44); so I've opened this to collect the last remaining backports.
If there's something that's been missed / or isn't tagged ["Needs backport (0.19)"](https://github.com/bitcoin/bitcoin/issues?q=is%3Aopen+label%3A%22Needs+backport+%280.19%29%22) that you think should be, please comment.
Currently backports:
* #17887 - bug-fix macos: give free bytes to F_PREALLOCATE
* #17980 - test: add missing #include to fix compiler errors
ACKs for top commit:
laanwj:
ACK daf2fff236
Tree-SHA512: 8438f992d0c39315a4da4c3f8ab5c92acefada4b0ba5a5cec3775dea2541492d386bf4e7c9e76e1494a6d4cf16a9205287d27ffd23d9c3056f213d733605eeee
The macos manpage for fcntl (for F_PEOFPOSMODE) states:
> Allocate from the physical end of file. In this case, fst_length indicates the number of newly allocated bytes desired.
Github-Pull: #17887
Rebased-From: 75163f4729c10c40d2843da28a8c79ab89193f6a
listsinceblock now checks that returned transactions are not
conflicting with any transactions that are filtered out by
the given blockhash
Co-Authored-By: Michael Chrostowski <michael.chrostowski@gmail.com>
Github-Pull: #17258
Rebased-From: 436ad436434b94982bcb7dc1d13a21949263ef73
The slot BitcoinGUI::addWallet can be invoked twice for the same
WalletModel due to a concurrent wallet being loaded after the first `connect()`:
```cpp
connect(wallet_controller, &WalletController::walletAdded, this, &BitcoinGUI::addWallet);
connect(wallet_controller, &WalletController::walletRemoved, this, &BitcoinGUI::removeWallet);
for (WalletModel* wallet_model : m_wallet_controller->getOpenWallets()) {
addWallet(wallet_model);
```
Github-Pull: #16963
Rebased-From: 6d6a7a8403ae923f189812edebdd95761de0e7f2
99b54076ff scripts: fix check-symbols & check-security argument passing (fanquake)
4330a1ee7f Update msvc build for Visual Studio 2019 v16.4 (Aaron Clauson)
b0f9b8e648 Moves vcpkg list to a text file and updates the appveyor job and readme to use it. (Aaron Clauson)
cd7b3b254a Updated appveyor config: - Update build image from Visual Studio 2017 to Visual Studio 2019. - Updated Qt static library from Qt5.9.7 to Qt5.9.8. - Added commands to update vcpkg port files (this does not update already installed packages). - Updated vcpkg package list as per #17309. - Removed commands setting common project file options. Now done via common.init.vcxproj include. - Changed msbuild verbosity from normal to quiet. Normal rights a LOT of logs and impacts appveyor job duration. Updated msvc project configs: - Updated platform toolset from v141 to v142. - Updated Qt static library from Qt5.9.7 to Qt5.9.8. - Added ignore for linker warning building bitcoin-qt program. - Added missing util/str.cpp class file to test_bitcoin project file. (Aaron Clauson)
112144dc52 Add missing typeinfo includes (Wladimir J. van der Laan)
1a6a534665 net: Log to net category for exceptions in ProcessMessages (Wladimir J. van der Laan)
c0dc728206 test: fix bitcoind already running warnings on macOS (fanquake)
5276b0e5a2 util: Add missing headers to util/fees.cpp (Hennadii Stepanov)
4d7875c555 rpc: require second argument only for scantxoutset start action (Andrew Chow)
bda2f5b3c9 cli: fix Fatal LevelDB error when specifying -blockfilterindex=basic twice (Harris)
d14ab7c522 gui: disable File->CreateWallet during startup (fanquake)
b9f1bc0fc1 wallet: unbreak with boost 1.72 (Jan Beich)
Pull request description:
Backports the following PRs to the 0.19 branch:
* https://github.com/bitcoin/bitcoin/pull/17654 - Unbreak build with Boost 1.72.0
* https://github.com/bitcoin/bitcoin/pull/17695 - gui: disable File->CreateWallet during startup
* https://github.com/bitcoin/bitcoin/pull/17687 - cli: fix Fatal LevelDB error when specifying -blockfilterindex=basic twice
* https://github.com/bitcoin/bitcoin/pull/17728 - rpc: require second argument only for scantxoutset start action
* https://github.com/bitcoin/bitcoin/pull/17450 - util: Add missing headers to util/fees.cpp
* https://github.com/bitcoin/bitcoin/pull/17488 - test: fix "bitcoind already running" warnings on macOS
* https://github.com/bitcoin/bitcoin/pull/17762 - Log to net category for exceptions in ProcessMessages
* https://github.com/bitcoin/bitcoin/pull/17364 - Updates to appveyor config for VS2019 and Qt5.9.8 + msvc project fixes
* https://github.com/bitcoin/bitcoin/pull/17416 - Appveyor improvement - text file for vcpkg package list
* https://github.com/bitcoin/bitcoin/pull/17736 - Update msvc build for Visual Studio 2019 v16.4
* https://github.com/bitcoin/bitcoin/pull/17857 - scripts: fix symbol-check & security-check argument passing
Fixes#17856.
ACKs for top commit:
sipsorcery:
ACK (tested: Windows 10 & msvc build) 99b54076ff.
Tree-SHA512: 91313de56fb0825e70a4be30ba0bf561b8c26d7dcf60549185df4f5e3524099398c828bb46faae807b631634d1afd5a1d397fb41e61ecfa0d746e4bf10b923cb
Brings back `bitcoin_en` which was accidentally removed last time (see #17886).
Tree-SHA512: a4aaa6f22e9ad3e87952f2a87829d675158cb26ed1fe61ef8e62646c22e21ccc0dec0f890b726f8adb76d7609e0274af8574b67201d30eff941cb60b1b76ff8c
The first argument in bin_PROGRAMS (bitcoind) was being silently
dropped and never passed into the check-security.py or check-symbols.py scripts.
This has been the case since the scripts were added to the makefile in
f3d3eaf78e.
Example of the behavior:
```python
import sys
if __name__ == '__main__':
print(sys.argv)
# ['args.py', 'b', 'c']
# if you add some lines to "a",
# you'll see them here..
for line in sys.stdin:
print(line)
```
Github-Pull: #17857
Rebased-From: 71af793512100ee7d508c3fb815af47925fe80ba
Remove the forest of special exceptions, and simply log a short
message to the NET logging category when an exception happens during
packet processing. It is not good to panick end users with errors
that any peer can generate (let alone writing to stderr).
Github-Pull: #17762
Rebased-From: 4d88c3dcb61e7c075ed3dd442044e0eff4e3c8de
ca5f8deefd Mark PSBTs spending unspendable outputs as invalid in analysis (Andrew Chow)
551583398b Have a PSBTAnalysis state that indicates invalid PSBT (Andrew Chow)
Pull request description:
Backport of #17524
ACKs for top commit:
achow101:
ACK ca5f8deefd
Tree-SHA512: b5f2b951beb9477ac3176a0aade845654d2108ca3a9fbc72097ba4b4797df5419053d6b489bbaa03be08cb8cfdc37a83db8b7642ffa52d42b7aa8ea14aff39cc
The second argument of scanobjects is only required for the start action.
Stop and abort actions do not need this.
Github-Pull: #17728
Rebased-From: 7d263571bee8c36fbe3c854b69c6f31cf1ee3b9b
Invalid PSBTs need to be re-created, so the next role is the
Creator (new PSBTRole). Additionally, we need to know what went
wrong so an error field was added to PSBTAnalysis.
A PSBTAnalysis indicating invalid will have empty everything,
next will be set to PSBTRole::CREATOR, and an error message.
Github-Pull: #17524
Rebased-From: 638e40cb6080800c7b0a7f4028f63326acbe4700
It is required in order to use size_t in QueuedConnections.
Github-Pull: #17427
Rebased-From: 88a94f7bb8ba2b0257315d70717f9af928ca6561
Tree-SHA512: 55accd997209c559dfc8e88b0db189ba314ac31e265ba2f94fa2009f1aba6b96213e2aa8cbad492b1230078f2e6cf1cca7a233dc6f54e9bc449f4e5438330b4d
fix uninitialized variable hard code the MinBIP9WarningHeight
fix uninitialized var hard code the MinBIP9WarningHeight instead
Github-Pull: #17449
Rebased-From: edb6b768a4185a4aaa6281ee50a6538f7426cb1e
Tree-SHA512: 6192940e5e13ad1176aa380da9f3287ff1eb0c8c2a78571a6c45fe0e100417452c8503b9ffc5c8b2a89c4a5e8811b9d2bfec95366e1de00f3365ba06959e9a9a
More info available here: https://doc.qt.io/qt-5/qfont.html#StyleStrategy-enum
Github-Pull: #17257
Rebased-From: e156b9d8b974f57253306b693a03aa80322ebc6c
Tree-SHA512: 6e36d626215d5e66796bd3935037eef89ae810e4855e321f9f37b90c3be0b61de21cd655f23bc6a29f41038b5179d92578ab19ab89ccd525d9389ab868456827
With this change polling runs in a different thread to prevent
disturbing the event loop.
Github-Pull: #17135
Rebased-From: 6b6be41c36e4fe9a74bed50e7f0a06532ab1260b
Belts and suspenders: make sure outgoing log messages don't contain
potentially suspicious characters, such as terminal control codes.
This escapes control characters except newline ('\n') in C syntax.
It escapes instead of removes them to still allow for troubleshooting
issues where they accidentally end up in strings.
Github-Pull: #17095
Rebased-From: d7820a1250070f3640246ae497e049bee0b3516f
Much of our code assumes 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`.
Github-Pull: #17085
Rebased-From: facb9a1315f97489a20eb0e969fdb14b5128ed2f
Pull new translations from Transifex.
Fixes#17027.
Tree-SHA512: 0c1708e9cf8180fd98715a078d49f417c78418b7c42fa31ca5a9c0d5766a54c217cef1f76a9edee1780d65635194536f2e63109a8a8560589aa2f6bb98d6b0de
Set only the internal name.
Fixes#17036 for both `bitcoind` and `bitcoin-qt`.
Github-Pull: #17038
Rebased-From: 07e4bdba3bd46c3a15dedb0a2660453c300643dc
Tree-SHA512: ed6f1b95a23c4c7863982ee6972429be5af0702ea93f0f17d32d2ef4b01446b1c0528eeadc45289609eda5c02ea68b3d722b8ecdfdf4fff4b02592c2188cc0a0
c812aba394 test bumpfee fee_rate argument (ezegom)
9f25de3d9e rpc bumpfee check fee_rate argument (ezegom)
88e5f997df rpc bumpfee: add fee_rate argument (ezegom)
1a4c791cf4 rpc bumpfee: move feerate estimation logic into separate method (ezegom)
Pull request description:
Taking over for https://github.com/bitcoin/bitcoin/pull/16492 which seems to have gone inactive.
Only minor commit cleanups, rebase, and some help text fixes on top of previous PR. Renamed `feeRate` to `fee_rate` to reflect updated guidelines.
ACKs for top commit:
Sjors:
Code review ACK c812aba
laanwj:
ACK c812aba394
Tree-SHA512: 5f7f51bd780a573ccef1ccd72b0faf3e5d143f6551060a667560c5163f7d9480e17e73775d1d7bcac0463f3b6b4328f0cff7b27e39483bddc42a530f4583ce30
addaf8af82 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 addaf8af82 - 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 addaf8af82 (tested both with and without this change)
Tree-SHA512: 65e8c690847f7499e82c9fef67b60d9aaa63c853732fe7fa7281da33054fcdcd9d24f5b86de71b0827728c25bac8efb7db445863f990304ebfee6fc450620c47
2a4e60b482 Fix block index inconsistency in InvalidateBlock() (Suhas Daftuar)
Pull request description:
Previously, we could release `cs_main` while leaving the block index in a state
that would fail `CheckBlockIndex()`, because `setBlockIndexCandidates` was not being
fully populated before releasing `cs_main`.
ACKs for top commit:
TheBlueMatt:
utACK 2a4e60b482. I also discovered another issue in InvalidateBlock while reviewing, see #16856.
Sjors:
ACK 2a4e60b. Tested on top of #16899. Also tested `invalidateblock` with `-checkblockindex=1`.
fjahr:
ACK 2a4e60b. Ran tests, reviewed code, inspected behavior while manually testing `invalidateblock`.
Tree-SHA512: ced12f9dfff0d413258c709921543fb154789898165590b30d1ee0cdc72863382f189744f7669a7c924d3689a1cc623efdf4e5ae3efc60054572c1e6826de612
0218171a24 contrib: Remove invalid nodes from seeds list (Wladimir J. van der Laan)
3b09f2b9d9 net: 0.19 hardcoded seeds update (Wladimir J. van der Laan)
801d341f3a contrib: makeseeds: More fancy output (Wladimir J. van der Laan)
ed76299bea contrib: makeseeds: Limit per network, instead of total (Wladimir J. van der Laan)
c254a9ef69 contrib: makeseeds: dedup by ip,port (Wladimir J. van der Laan)
3314d87966 contrib: makeseeds: Factor out ASN lookup (Wladimir J. van der Laan)
301c2b1ab5 contrib: makeseeds: Improve logging and filtering (Wladimir J. van der Laan)
Pull request description:
- contrib: Improve makeseeds script
- net: 0.19 hardcoded seeds update
Sources:
- http://bitcoin.sipa.be/seeds.txt.gz (Sipa)
- https://github.com/bitcoin/bitcoin/files/3671913/dnsseed.dump.tar.gz (Sjors)
Output:
```
Initial: IPv4 418690, IPv6 55861, Onion 2747
Skip entries with invalid address: IPv4 418690, IPv6 55861, Onion 2747
After removing duplicates: IPv4 409220, IPv6 54028, Onion 2717
Skip entries from suspicious hosts: IPv4 409219, IPv6 54028, Onion 2717
Enforce minimal number of blocks: IPv4 106719, IPv6 46342, Onion 2621
Require service bit 1: IPv4 106384, IPv6 46241, Onion 2542
Require minimum uptime: IPv4 5300, IPv6 1153, Onion 201
Require a known and recent user agent: IPv4 4642, IPv6 1060, Onion 141
Filter out hosts with multiple bitcoin ports: IPv4 4642, IPv6 1060, Onion 141
Look up ASNs and limit results, both per ASN and globally: IPv4 464, IPv6 48, Onion 141
```
ACKs for top commit:
Sjors:
ACK 0218171. I also checked that `chainparamsseeds.h` is generated from `nodes_main.txt`. Sounds like we should look at this script a bit more outside release moments :-)
Tree-SHA512: c1f5795fe88d14800c4da918387368d51e85f4319f2ce3c0359851d041767e2883f32b1da371bba22bd5f0b442ac3e5ea7d685c233ad2cc4045c930f973b0aa2
73aaf4ecf8 Make SignatureExtractorChecker private to its own file (Ben Woosley)
Pull request description:
~If we add a CTxIn constructor to SignatureData, then constructing the
SignatureData directly is no more verbose than calling DataFromTransaction,
and grants the caller additional flexibiliy in how to provide the CTxIn.~
A simple change to enhance encapsulation.
ACKs for top commit:
MarcoFalke:
utACK 73aaf4ecf8
laanwj:
ACK 73aaf4ecf8
Tree-SHA512: f7eafbce22b0e9917a8487e88d1f5a1061f2a0959ae1a097cbd9c8ea0d774edfb807da56813cb5fb26f6ca98499a0604a8ff024c198a7c8dc755164de66d972a
fadd6e0d2a 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 fadd6e0d2a -- diff looks correct
promag:
ACK fadd6e0d2a.
laanwj:
ACK fadd6e0d2a (as argument help is not translated this doesn't have to wait for the split-off)
Tree-SHA512: 8ad64965ab5bbba4b92933a5adcb0c9eda5bdb0cc080840a4a97b12c67f41f9b789fd289df4932d748f5a7eebc7305a000f03ceb968a78c9b5d9f34af61f0b15
85973bcc44 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 85973bcc44
Tree-SHA512: 50fdb60d418e2f9eb65a4b52477be16189f00bfc30493adb27d9fb62100fd5bca33b98b8db6caa8485db424838d3b7a1da802c14ff4917943464401f47391616
3eea6a8f26 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 3eea6a8f26
promag:
ACK 3eea6a8f26.
fanquake:
ACK 3eea6a8f26
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.
e2ce392aec test: Avoid whitespace linting in qt translations (Wladimir J. van der Laan)
977dd23e40 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
8cf9898b53 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 8cf9898b53 - 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 8cf9898b53
Sjors:
Tested ACK 8cf9898 on macOS. English already fit, so to reproduce the issue, launch in German with `-resetguisettings -lang=de`.
laanwj:
ACK 8cf9898b53
Tree-SHA512: 568b0ae0d5feeda603c0ccf67b5bb3857becea8f22fb98695e1901e662cb1e76377589e39ec743258154d7f6c4a5e544bb003fcc73597400dd427db047392638
67d99900b0 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
4320bfc0c0 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 4320bfc0c0.
Tree-SHA512: 7133d0103bcf97672ae5aa40ba35d4b81331a8c179190031bbc887da6a5ccc929428e522938db43d87dbcbf9ad3b121dac1e6faf1daa5ae81d0b5fed7f053b5f
43e7d576f5 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 43e7d576f5
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).
fdb3e8f8b2 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 fdb3e8f8b2 for what it's worth
achow101:
ACK fdb3e8f8b2
Sjors:
ACK fdb3e8f8b2. 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 fdb3e8f8b2
Tree-SHA512: e6fd34e8902f8c7affb28e8951803e47d542710d5f1229000746656a37ee59d754439fc33e36b7eef87544262e5aac374645db91b74cb507e73514003ca7a67f
1a02edb3f2 [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 1a02edb3f2
Tree-SHA512: 6f0eaf2b4aaf73c9a9bf1fbd4af59af5f95fc012fa88f94e050e6ae273b3ad647f5729df53bfce91e1a925fe4fd7b14818908bb6131a81413a555137d1007d7c
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
efd2474d17 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.~ efd2474d17
mzumsande:
I had intended to follow up earlier on my last comment, ACK efd2474d17. I reviewed the code, ran tests and did a successful reindex on testnet with this branch.
Tree-SHA512: 695529e0af38bae2af4e0cc2895dda56a71b9059c3de04d32e09c0165a50f6aacee499f2042156ab5eaa6f0349bab6bcca4ef9f6f9ded4e60d4483beab7e4554
fa607c2292 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 fa607c2292
jamesob:
ACK fa607c2292
promag:
ACK fa607c2292.
fanquake:
ACK fa607c2292 - 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.
fa7847d99b 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 fa7847d99b.
laanwj:
ACK fa7847d99b
Tree-SHA512: 0ac6c47fe5eb2145b609a30fd3f56052d3e08abe6c67fc74b6d209a55a4df509c52f13eb1c759520a4fa43916ece0e6d4cefef87e061b51114a6582db911944a
6659810e2f test: use named args for sendrawtransaction calls (Jon Atack)
5c1cd78b7e doc: improve rawtransaction code/test docs (Jon Atack)
acc14c5093 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 6659810e2f
Tree-SHA512: bf5258f23802ab3ba3defb8791097e08e63f3e2af21023f832cd270dc88d1fa04349e921d69f9f5fedac5dce5cd3c1cc46b48febbede4bc18dccb8be994565b2
6170ec5d3a Do not query all DNS seed at once (Pieter Wuille)
Pull request description:
Before this PR, when we don't have enough connections after 11 seconds, we proceed to query all DNS seeds in a fixed order, loading responses from all of them.
Change this to to only query three randomly-selected DNS seed. If 11 seconds later we still don't have enough connections, try again with another one, and so on.
This reduces the amount of information DNS seeds can observe about the requesters by spreading the load over all of them.
ACKs for top commit:
Sjors:
ACK 6170ec5d3
sdaftuar:
ACK 6170ec5d3a
jonasschnelli:
utACK 6170ec5d3a - I think the risk of a single seeder codebase is orthogonal to this PR. Such risks could also be interpreted differently (diversity could also increase the risk based on the threat model).
fanquake:
ACK 6170ec5d3a - Agree with the reasoning behind the change. Did some testing with and without `-forcednsseed` and/or a `peers.dat` and monitored the DNS activity.
Tree-SHA512: 33f6be5f924a85d312303ce272aa8f8d5e04cb616b4b492be98832e3ff37558d13d2b16ede68644ad399aff2bf5ff0ad33844e55eb40b7f8e3fddf9ae43add57
fa8d65f071 doc: Fix doxygen comment for SignTransaction in rpc/rawtransaction_util (MarcoFalke)
Pull request description:
The param `coins` to `SignTransaction` is final and can thus not be extended (as suggested by the doc).
ACKs for top commit:
practicalswift:
ACK fa8d65f071 -- const correctness is good and diff looks correct
fanquake:
ACK fa8d65f071
Tree-SHA512: 041e159f2c3cf96e296173c31f3e5f35bbc7711cc888aa4bf08aaa8c65c95ee7f7672f65396690a9af45795a618eea0fadde7fb02d29ec85f1b4df5e6d9e0c7a
3cf36736e5 refactoring: move ReplayBlocks under CChainState (James O'Beirne)
bcf73d3b84 refactoring: move LoadChainTip to CChainState method (James O'Beirne)
f5809d5b13 doc: fix CChainState::ActivateBestChain doc (James O'Beirne)
Pull request description:
This is part of the [assumeutxo project](https://github.com/bitcoin/bitcoin/projects/11):
Parent PR: #15606
Issue: #15605
Specification: https://github.com/jamesob/assumeutxo-docs/tree/master/proposal
---
Move more chainstate-related functionality to methods on CChainState. Nothing too interesting here, but needed to work with multiple chainstates. And brief to review. :)
Also fixes doc on ActivateBestChain.
ACKs for top commit:
MarcoFalke:
ACK 3cf36736e5
ryanofsky:
Can confirm. utACK 3cf36736e5. Removes wrapper functions and removes more ::ChainActive() and ::ChainstateActive() calls than it adds, so seems good.
Tree-SHA512: 4bf8a1dd454ca9d61c85f6736910fa7354c57acc0002e3a8e5ce494035d8280e4c20e066f03478eeff7d44195e7912c282a486526da9be53854b478b961affaa
c4b0c08f7c Update tx-size-small comment with relevant CVE disclosure (Gregory Sanders)
Pull request description:
Code first introduced under https://github.com/bitcoin/bitcoin/pull/11423 with essentially no description and no discussion.
ACKs for top commit:
MarcoFalke:
ACK c4b0c08f7c
fanquake:
ACK c4b0c08f7c
Tree-SHA512: 95d5c92998b8b1e944c477dbaee265b62612b6e815099ab31d9ff580b4dff777abaf7f326a284644709f918aa1510412d62310689b1250ef6e64de7b19ca9f71
2dfd6834ef test: Add test for default maxfeerate in sendrawtransaction (Joonmo Yang)
261843e4be wallet/rpc: Use the default maxfeerate value as BTC/kB (Joonmo Yang)
Pull request description:
Fixes https://github.com/bitcoin/bitcoin/issues/16382
This patch tries to treat `maxfeerate` in sendrawtransaction/testmempoolaccept RPC as a rate(BTC/kB) instead of an absolute value(BTC).
The included test case checks if the new behavior works correctly, by using the transaction with an absolute fee of ~0.02BTC, where the fee rate is ~0.2BTC/kB.
This test should be failing if the default `maxfeerate` is 0.1BTC, but pass if the default value is 0.1BTC/kB
ACKs for top commit:
laanwj:
ACK 2dfd6834ef (ACKs by Sjors and MarcoFalke above for trivially different code)
Tree-SHA512: a1795bffe8a182acef8844797955db1f60bb0c0ded97148f3572dc265234d5219271a3a7aa0b6418a43f73b2b2720ef7412ba169c99bb1cdcac52051f537d6af
b4fd0ca9be Include cstring for sanity_test_fdelt if required (Ben Woosley)
7fb886b1b1 [moveonly] Split glibc sanity_test_fdelt out (Ben Woosley)
Pull request description:
SmartOS FD_ZERO is implemented in a way that requires
an external declaration of memcpy. We can not simply
include cstring in the existing file because
sanity_test_memcpy is attempting to replace memcpy.
Instead split glibc_sanity into fdelt and memcpy files,
and include <cstring> in glibc_sanity/fdelt.cpp.
Fixes#13581, see also #13619
ACKs for top commit:
laanwj:
Code review an lightly tested (but not on SmartOS) ACK b4fd0ca9be
Tree-SHA512: 231306da291ad9eca8ba91bea1e9c27b6c2e96e484d1602e1c2cf27761202f9287ce0bc19fefd000943d2b449d0e5929cd39e2f7e09cf930d89fa520228ccbec
c0b5d97103 Test that joinpsbts randomly shuffles the inputs (Andrew Chow)
6f405a1d3b Shuffle inputs and outputs after joining psbts (Andrew Chow)
Pull request description:
`joinpsbts` currently just adds the inputs and outputs in the order of that the PSBTs were provided. This makes it extremely easy to identify which outputs belong to which inputs. This PR changes that so that all of the inputs and outputs are shuffled in the joined transaction.
ACKs for top commit:
instagibbs:
utACK c0b5d97103
jonatack:
ACK c0b5d97103 modulo suggestions for later.
Tree-SHA512: 14a0b7aae07d92e6d2c76a3a3b228b481e1964cb7d34f97515bdda18e2ea05a9f97c5a22affc143b86ae8b95c3cb239849fb54219d65512bc2112264dca915c8
4a87c5cfdf [refactor] Rewrite AcceptToMemoryPoolWorker() using smaller parts (Suhas Daftuar)
Pull request description:
This is in preparation for re-using these validation components for a new version of AcceptToMemoryPool() that can operate on multiple transactions ("package relay").
ACKs for top commit:
MarcoFalke:
re-ACK 4a87c5cfdf (did the rebase myself and arrived at the same result, mod whitespace)
laanwj:
ACK 4a87c5cfdf
Tree-SHA512: b0495c026ffe06146258bace3d5e0c9aaf23fa65f89f258abc4af5980812e68e63a799f1d923e78ac1ee6bcafaf1222b2c2690a527df9b65dff7b48a013f154e
0c62e3aa73 New regression testing for CVE-2018-17144, CVE-2012-2459, and CVE-2010-5137. (lucash-dev)
38bfca6bb2 Added comments referencing multiple CVEs in tests and production code. (lucash-dev)
Pull request description:
This functional test includes two scenarios that test for regressions of vulnerabilities, but they are only briefly described. There are freely available documents explaining in detail the issues, but without explicit mentions, the developer trying to maintain the code needs an additional step of digging in commit history and PR conversations to figure it out.
Added comments to explicitly mention CVE-2018-17144 and CVE-2012-2459, for more complete documentation.
This improves developer experience by making understanding the tests easier.
ACKs for top commit:
laanwj:
ACK 0c62e3aa73, checked the CVE numbers, thanks for adding documentation
Tree-SHA512: 3ee05351745193b8b959e4a25d50f25a693b2d24b0732ed53cf7d5882df40b5dd0f1877bd5c69cffb921d4a7acf9deb3cc1160b96dc730d9b5984151ad06b7c9
8573429d46 test: add some unit tests for merkle.cpp (soroosh-sdi)
Pull request description:
following situations are covered:
- empty block
- one Tx
- Merkle root of a block with odd Txs should not change with repeating
last one
- Merkle root is computed with combining Merkle root of left subtree and right subtree
- block witness is Merkle root of a block when setting first Tx
to zero.
Signed-off-by: soroosh-sdi <soroosh.sardari@gmail.com>
ACKs for top commit:
laanwj:
ACK 8573429d46
Tree-SHA512: e12228171de8f8480f173c9d9d0359f00f46bf09075e0767f5f1a367478a1b7b6d177d230f7e930914915cd2c6b66b18d24b1682f1233c38e97954ba331e5773
