Commit graph

26421 commits

Author SHA1 Message Date
Anthony Towns
0c471a5f30
tests: check never active versionbits 2021-03-03 14:14:40 +01:00
Anthony Towns
3ba9283a47
tests: more helpful errors for failing versionbits tests
Co-authored-by: Sjors Provoost <sjors@sprovoost.nl>
2021-03-03 14:12:16 +01:00
fanquake
831675c8dc
Merge #20401: qt: Pre-splitoff translations update
bb6441b7a4 qt: Pre-splitoff translations update (Wladimir J. van der Laan)

Pull request description:

  0.21 split-off should be near now. Let's do one final translations update just before the split-off.
  (Hopefully it won't take too long, but might want to keep this open to be the last thing merged)

ACKs for top commit:
  hebasto:
    ACK bb6441b7a4
  MarcoFalke:
    ACK bb6441b7a4 (checked that only changes are translation changes in `src/qt`)

Tree-SHA512: 3273246923d3020e1f7ae46cbb59f1ed45a35acb5e1582b55486c5723f5aa1e5809fe2fd87b1ac34d308eef2902e621d0ace97181a044262b2c8f002bf50daac
2020-11-17 20:57:36 +08:00
MarcoFalke
80e32e120e
Merge #20305: wallet: introduce fee_rate sat/vB param/option
05e82d86b0 wallet: override minfee checks (fOverrideFeeRate) for fee_rate (Jon Atack)
9a670b4f07 wallet: update sendtoaddress, send RPC examples with fee_rate (Jon Atack)
be481b72e2 wallet: use MIN_RELAY_TX_FEE in bumpfee help (Jon Atack)
449b730579 wallet: provide valid values if invalid estimate mode passed (Jon Atack)
6da3afbaee wallet: update remaining rpcwallet fee rate units to BTC/kvB (Jon Atack)
173b5b5fe0 wallet: update fee rate units, use sat/vB for fee_rate error messages (Jon Atack)
7f9835a05a wallet: remove fee rates from conf_target helps (Jon Atack)
b7994c01e9 wallet: add fee_rate unit warnings to bumpfee (Jon Atack)
410e471fa4 wallet: remove redundant bumpfee fee_rate checks (Jon Atack)
a0d4957473 wallet: introduce fee_rate (sat/vB) param/option (Jon Atack)
e21212f01b wallet: remove unneeded WALLET_BTC_KB_TO_SAT_B constant (Jon Atack)
6112cf20d4 wallet: add CFeeRate ctor doxygen documentation (Jon Atack)
3f72791613 wallet: fix bug in RPC send options (Jon Atack)

Pull request description:

  This PR builds on #11413 and #20220 to address #19543.

  - replace overloading the conf_target and estimate_mode params with `fee_rate` in sat/vB in the sendtoaddress, sendmany, send, fundrawtransaction, walletcreatefundedpsbt, and bumpfee RPCs

  - allow non-actionable conf_target value of `0` and estimate_mode value of `""` to be passed to use `fee_rate` as a positional argument, in addition to as a named argument

  - fix a bug in the experimental send RPC described in https://github.com/bitcoin/bitcoin/pull/20220#discussion_r513789526 where args were not being passed correctly into the options values

  - update the feerate error message units for these RPCs from BTC/kB to sat/vB

  - update the test coverage, help docs, doxygen docs, and some of the RPC examples

  - other changes to address the excellent review feedback

  See this wallet meeting log for more context: http://www.erisian.com.au/bitcoin-core-dev/log-2020-11-06.html#l-309

ACKs for top commit:
  achow101:
    re-ACK 05e82d8
  MarcoFalke:
    review ACK 05e82d86b0 did not test and found a few style nits, which can be fixed later 🍯
  Xekyo:
    tACK 05e82d86b0
  Sjors:
    utACK 05e82d86b0

Tree-SHA512: a4ee5f184ada53f1840b2923d25873bda88c5a2ae48e67eeea2417a0b35154798cfdb3c147b05dd56bd6608a784e1b91623bb985ee2ab9ef2baaec22206d0a9c
2020-11-17 13:49:12 +01:00
MarcoFalke
e7986c51bc
Merge bitcoin-core/gui#96: Slight improve create wallet dialog
ac64cec4ce gui: create wallet: add advanced section (Sjors Provoost)
c99d6f644a gui: create wallet: name placeholder (Sjors Provoost)
5bff82540b [gui] create wallet: smarter checkbox toggling (Sjors Provoost)

Pull request description:

  Previously only users who needed a second wallet had to use to the create wallet dialog. With the merge of https://github.com/bitcoin/bitcoin/pull/15454 now all new users have to. I don't think it was user-friendly enough for that.

  <img width="403" alt="Schermafbeelding 2020-09-18 om 09 41 44" src="https://user-images.githubusercontent.com/10217/93574129-52ef9680-f998-11ea-9a6f-31144f66d3bf.png">

  This PR makes a few simple improvements so that new users don't have to think too much:

  <img width="369" alt="Schermafbeelding 2020-10-15 om 16 45 22" src="https://user-images.githubusercontent.com/10217/96145959-0c914700-0f06-11eb-9526-cf447d841d7a.png">

  It's lightly inspired by #77. It would be better if those changes made it into the upcoming release, but this PR is a good start imo.

  * wallet encryption is no longer checked by default, because such a change in the default needs a separate discussion (fwiw, I suspect it increases the number of users losing access to coins)
  * watch-only and descriptor wallet stuff is moved to advanced, so new users know they can safely ignore these check boxes
  * bonus: when you click on "disable private keys" it disables encrypt wallet and checks blank wallet
  * label changes: see screenshot
  * tooltip changes: see code diff

  Note that a blank wallet name isn't allowed in the dialog; I haven't addressed that.

  _Update 2020-10-30_, dropped the new strings for now:
  <img width="450" alt="Schermafbeelding 2020-10-30 om 11 26 55" src="https://user-images.githubusercontent.com/10217/97694591-1b99fc80-1aa3-11eb-8b85-e19f1ad5add4.png">

ACKs for top commit:
  fjahr:
    Tested ACK ac64cec4ce
  jonatack:
    re-ACK ac64cec4ce, per `git diff d393708 ac64cec` only change since my last review is improving the placeholder from "MyWallet" to "Wallet" and dropping the last commit. Tested creating a dozen wallets in signet with different combinations of options and then verifying/comparing their characteristics in the console with getwalletinfo. My remaining caveats are (1) the need for less user surprise by either (a) improving the user info or (b) with less auto-(un)selecting as mentioned in https://github.com/bitcoin-core/gui/pull/96#issuecomment-727017409 and (2) I prefer the "Encrypt private keys" and "Watch-only" wording and descriptions below over the current ones; hopefully these can be addressed in a follow-up.
  hebasto:
    re-ACK ac64cec4ce
  ryanofsky:
    Code review ACK ac64cec4ce. Only changes since last review are tweaking placeholder text and dropping "allow nameless" commit

Tree-SHA512: a25f84eb66ee4f99af441d73e33928df9d9cf592177398ef48f0037f5913699e47a162cf1301c83b34501546d43ff4ae12607fd078c5c03b92f573bf7604a9f2
2020-11-17 13:03:59 +01:00
MarcoFalke
c463f70fb0
Merge #20139: Wallet: do not return warnings from UpgradeWallet()
9636962889 [upgradewallet] removed unused warning param (Sishir Giri)

Pull request description:

  The `warning` variable was unused in `upgradewallet` so I removed it

ACKs for top commit:
  practicalswift:
    ACK 9636962889: diff looks correct
  MarcoFalke:
    review ACK 9636962889
  jonatack:
    ACK 9636962889

Tree-SHA512: 1d63186ce1e05e86a778340f2d7986c2cee1523de0a11cea39e8d148ac7ee26c49741dfa302b5c1cd1c8d74e67c1f9baee2763720c2d850b57da9a3fdce24565
2020-11-17 12:43:43 +01:00
fanquake
3457054c61
Merge #20346: script: modify security-check.py to use "==" instead of "is" for literal comparison
b6121edf70 swapped "is" for "==" in literal comparison (Tyler Chambers)

Pull request description:

  In Python 3.8+ literal comparisons using "is" instead of "==" produce a SyntaxWarning [source](https://docs.python.org/3.8/whatsnew/3.8.html#changes-in-python-behavior).

  I checked the entire devtools directory, this seems to be the only occurrence.

  This is a small fix, but removes the SyntaxWarning.
  Fixes: #20338

ACKs for top commit:
  hebasto:
    re-ACK b6121edf70, only squashed since my [previous](https://github.com/bitcoin/bitcoin/pull/20346#pullrequestreview-525934568) review.
  practicalswift:
    re-ACK b6121edf70: patch still looks correct
  theStack:
    utACK b6121edf70

Tree-SHA512: 82a43495d6552fbaa3b02b58f0930b049d27aa937fe44b47714e3c059f844cc494de20674557371cbccf24fb8873ecb7376fb965ae326847eed2b855ed2d59c6
2020-11-17 13:57:40 +08:00
fanquake
7c0d412a74
Merge #20405: p2p: avoid calculating onion address checksum when version is not 3
d355a302d9 Break circuit earlier (lontivero)

Pull request description:

  Currently when parsing an onion v3 address the pubic key checksum is calculated in order to compare it with the received address checksum. However this step is not necessary if the address version byte is not 3, in which case the method can return with false immediately.

ACKs for top commit:
  jonatack:
    ACK d355a302d9
  practicalswift:
    ACK d355a302d9 -- patch looks correct
  hebasto:
    ACK d355a302d9, I have reviewed the code and it looks OK, I agree it can be merged.
  sipa:
    utACK d355a302d9

Tree-SHA512: 9e4506793b7f4a62ce8edc41a260a8c125ae81ed2f90cd850eb2a9214d323c446edc7586c7b0590dcbf3aed5be534718b77bb19c45b48f8f52553d32a3663a65
2020-11-17 13:56:22 +08:00
Sishir Giri
9636962889 [upgradewallet] removed unused warning param 2020-11-16 13:22:42 -08:00
lontivero
d355a302d9 Break circuit earlier
There is no need to calculate the full checksum for an Tor v3 onion
address if the version byte is not the expected one.
2020-11-16 15:54:24 -03:00
Wladimir J. van der Laan
bb6441b7a4 qt: Pre-splitoff translations update 2020-11-16 11:15:11 +01:00
Wladimir J. van der Laan
c48e788246
Merge #18836: wallet: upgradewallet fixes and additional tests
5f9c0b6360 wallet: Remove -upgradewallet from dummywallet (MarcoFalke)
a314271f08 test: Remove unused wallet.dat (MarcoFalke)
bf7635963c tests: Test specific upgradewallet scenarios and that upgrades work (Andrew Chow)
4b418a9dec test: Add test_framework/bdb.py module for inspecting bdb files (Andrew Chow)
092fc43485 tests: Add a sha256sum_file function to util (Andrew Chow)
0bd995aa19 wallet: upgrade the CHDChain version number when upgrading to split hd (Andrew Chow)
8e32e1c41c wallet: remove nWalletMaxVersion (Andrew Chow)
bd7398cc62 wallet: have ScriptPubKeyMan::Upgrade check against the new version (Andrew Chow)
5f720544f3 wallet: Add GetClosestWalletFeature function (Andrew Chow)
842ae3842d wallet: Add utility method for CanSupportFeature (Andrew Chow)

Pull request description:

  This PR cleans up the wallet upgrade mechanism a bit, fixes some probably bugs, and adds more test cases.

  The `nWalletMaxVersion` member variable has been removed as it made `CanSupportFeature` unintuitive and was causing a couple of bugs. The reason this was introduced originally was to allow a wallet upgrade to only occur when the new feature is first used. While this makes sense for the old `-upgradewallet` option, for an RPC, this does not quite make sense. It's more intuitive for an upgrade to occur if possible if the `upgradewallet` RPC is used as that's an explicit request to upgrade a particular wallet to a newer version. `nWalletMaxVersion` was only relevant for upgrades to `FEATURE_WALLETCRYPT` and `FEATURE_COMPRPUBKEY` both of which are incredibly old features. So for such wallets, the behavior of `upgradewallet` will be that the feature is enabled immediately without the wallet needing to be encrypted at that time (note that `FEATURE_WALLETCRYPT` indicates support for encryption, not that the wallet is encrypted) or for a new key to be generated.

  `CanSupportFeature` would previously indicate whether we could upgrade to `nWalletMaxVersion` not just whether the current wallet version supported a feature. While this property was being used to determine whether we should upgrade to HD and HD chain split, it was also causing a few bugs. Determining whether we should upgrade to HD or HD chain split is resolved by passing into `ScriptPubKeyMan::Upgrade` the version we are upgrading to and checking against that. By removing `nWalletMaxVersion` we also fix a bug where you could upgrade to HD chain split without the pre-split keypool.

  `nWalletMaxVersion` was also the version that was being reported by `getwalletinfo` which meant that the version reported was not always consistent across restarts as it depended on whether `upgradewallet` was used. Additionally to make the wallet versions consistent with actually supported versions, instead of just setting the wallet version to whatever is given to `upgradewallet`, we normalize the version number to the closest supported version number. For example, if given 150000, we would store and report 139900.

  Another bug where CHDChain was not being upgraded to the version supporting HD chain split is also fixed by this PR.

  Lastly several more tests have been added. Some refactoring to the test was made to make these tests easier. These tests check specific upgrading scenarios, such as from non-HD (version 60000) to HD to pre-split keypool. Although not specifically related to `upgradewallet`, `UpgradeKeyMetadata` is now being tested too.

  Part of the new tests is checking that the wallet files are identical before and after failed upgrades. To facilitate this, a utility function `sha256sum_file` has been added. Another part of the tests is to examine the wallet file itself to ensure that the records in the wallet.dat file have been correctly modified. So a new `bdb.py` module has been added to deserialize the BDB db of the wallet.dat file. This format isn't explicitly documented anywhere, but the code and comments in BDB's source code in file `dbinc/db_page.h` describe it. This module just dumps all of the fields into a dict.

ACKs for top commit:
  MarcoFalke:
    approach ACK 5f9c0b6360
  laanwj:
    Code review ACK 5f9c0b6360
  jonatack:
    ACK 5f9c0b6360, approach seems fine, code review, only skimmed the test changes but they look well done, rebased on current master, debug built and verified the `wallet_upgradewallet.py` test runs green both before and after running `test/get_previous_releases.py -b v0.19.1 v0.18.1 v0.17.2 v0.16.3 v0.15.2`

Tree-SHA512: 7c4ebf420850d596a586cb6dd7f2ef39c6477847d12d105fcd362abb07f2a8aa4f7afc5bfd36cbc8b8c72fcdd1de8d2d3f16ad8e8ba736b6f4f31f133fe5feba
2020-11-16 11:03:25 +01:00
MarcoFalke
1e17114917
Merge #20238: doc: Missing comments for signet parameters
9c08f3332c doc: Missing comments for signet parameters (kanon)

Pull request description:

  We have such comment in chainparams.cpp. However in Signet the comments are missing.

  In example...

  - Mainnet
  d67883d01e/src/chainparams.cpp (L83-L84)

  - Testnet
  d67883d01e/src/chainparams.cpp (L196-L197)

  - Regtest
  d67883d01e/src/chainparams.cpp (L392-L393)

ACKs for top commit:
  theStack:
    ACK 9c08f3332c

Tree-SHA512: d4e488cf01e50d6320282b29d776c11e6b3d423f9268226749f738a57a51f456b6bd48334d2d5a43afa782df65ea15525a0af1688003c1be6ef915c05650e147
2020-11-16 10:40:56 +01:00
MarcoFalke
37a4634811
Merge #20390: CI/Cirrus: Skip merge_base step for non-PRs
20e491ddcb CI/Cirrus: Skip merge_base step for non-PRs (Luke Dashjr)

Pull request description:

  CIRRUS_BASE_BRANCH is a PR-specific variable and undocumented on non-PR builds.
  In practice (at the moment), it seems to be HEAD, which in private repositories can be pretty much anything, causing CI to fail if it can't be cleanly merged.

  By checking CIRRUS_PR first, we can reliably do CI builds of branches outside PRs.

ACKs for top commit:
  MarcoFalke:
    review ACK 20e491ddcb

Tree-SHA512: 9fd8db2e19a3145f7dccfca107631b20df8c94d385f624e2bcef2fa18e38bf3e23c6c68fc8241decedbf1413bf69ca572cff75e1ccf82c09ac50443001ec5ae5
2020-11-16 07:59:43 +01:00
MarcoFalke
2fa085a5d7
Merge #20033: refactor: minor whitespace fixups, s/const/constexpr/ and remove template (followup to #19845)
89836a82ee style: minor improvements as a followup to #19845 (Vasil Dimov)

Pull request description:

  Address suggestions:
  https://github.com/bitcoin/bitcoin/pull/19845#discussion_r495486760
  https://github.com/bitcoin/bitcoin/pull/19845#discussion_r495488051
  https://github.com/bitcoin/bitcoin/pull/19845#discussion_r495730125

ACKs for top commit:
  jonatack:
    re-ACK 89836a8 change since previous review is replacing std::runtime_error with std::exception, built/ran unit tests with gcc debian 10.2.0-15, then broke a few v3 net_tests involving `BOOST_CHECK_EXCEPTION`, rebuilt, ran `src/test/test_bitcoin -t net_tests -l all` and checked the error reporting.
  hebasto:
    re-ACK 89836a82ee
  theStack:
    ACK 89836a82ee

Tree-SHA512: 36477fdccabe5a8ad91fbabb4655cc363a3a7ca237a98ae6dd4a9fae4a4113762040f864d4ca13a47d081f7d16e5bd487edbfb61ab50a37e4a0424e9bec30b24
2020-11-16 07:57:34 +01:00
MarcoFalke
0ede354367
Merge #20386: Silence false positive GCC warning in wallet/rpcwallet.cpp
049feabf28 Add missing optional.h include (Kristaps Kaupe)
29c66ace5c Silence false positive GCC warning (Kristaps Kaupe)

Pull request description:

  Resolves #20381.

ACKs for top commit:
  jnewbery:
    utACK 049feabf28
  practicalswift:
    ACK 049feabf28: diagnostics signal to noise is increased by getting rid of false positives
  hebasto:
    ACK 049feabf28, I have reviewed the code and it looks OK, I agree it can be merged.

Tree-SHA512: 05d84f51521c3b843ed6bf284a83a91db015ad0cd4fcf8b602275812575c1f6b4899286a89d360fbd3caef184abdfb9d834e119842d8740919892f05a0f9e1f8
2020-11-15 18:33:59 +01:00
MarcoFalke
fb7726e56d
Merge #20395: ci: Use the previous build worker image in AppVeyor
406097c810 ci: Use the previous build worker image in AppVeyor (Hennadii Stepanov)

Pull request description:

  This is a workaround as the [recent](https://www.appveyor.com/updates/2020/11/14/) Visual Studio 2019 image update breaks our builds.

  This PR is alternative to #20392 due to its build [failure](https://ci.appveyor.com/project/DrahtBot/bitcoin/builds/36314660).

ACKs for top commit:
  MarcoFalke:
    review ACK 406097c810 also seems to pass

Tree-SHA512: f9b722d8e67dd7f0745de6da385064630adb27ecbc0a919be47f62217a2bb7a27a6fa00a7536a24bf17500a77160ca3b92b3c8619047171a6f5198b434015221
2020-11-15 09:44:05 +01:00
Hennadii Stepanov
406097c810
ci: Use the previous build worker image in AppVeyor
This is a workaround as the recent Visual Studio 2019 image update
breaks our builds.
2020-11-15 09:12:06 +02:00
Luke Dashjr
20e491ddcb CI/Cirrus: Skip merge_base step for non-PRs
CIRRUS_BASE_BRANCH is a PR-specific variable and undocumented on non-PR builds.
In practice (at the moment), it seems to be HEAD, which in private repositories can be pretty much anything, causing CI to fail if it can't be cleanly merged.

By checking CIRRUS_PR first, we can reliably do CI builds of branches outside PRs.
2020-11-14 17:51:59 +00:00
Sjors Provoost
ac64cec4ce
gui: create wallet: add advanced section 2020-11-14 13:07:49 +01:00
Sjors Provoost
c99d6f644a
gui: create wallet: name placeholder 2020-11-14 13:07:49 +01:00
Kristaps Kaupe
049feabf28
Add missing optional.h include 2020-11-14 10:56:38 +02:00
Kristaps Kaupe
29c66ace5c
Silence false positive GCC warning 2020-11-14 03:20:07 +02:00
Jonas Schnelli
543693b92b
Merge #20378: wallet: fix potential division by 0 in WalletLogPrintf
440f8d3abe fix potential devision by 0 (Jonas Schnelli)

Pull request description:

  #20344 removed the divide-by-zero sanitizer suppression in `wallet/wallet.cpp` but kept a potential devision by zero in `wallet.cpp`'s fee logging.

  Detected here https://bitcoinbuilds.org/index.php?job=ffb7d59f-379f-4f27-a273-a5595b8c5f07

ACKs for top commit:
  practicalswift:
    ACK 440f8d3abe
  laanwj:
    Code review ACK 440f8d3abe
  hebasto:
    re-ACK 440f8d3abe

Tree-SHA512: 9f7903d1e567497c5f972d39e9629c059151e705dbed0a6b88f7c6650c50ecf820f78e3e0f3e629c661d45a938c5d7659faae7c61e47ca8b3bdb029661bca55a
2020-11-13 08:26:57 +01:00
MarcoFalke
28cb61646b
Merge #19065: tests: Add fuzzing harness for CAddrMan
d04a17a790 fuzz: Use ConsumeRandomLengthBitVector(...) in src/test/fuzz/connman and src/test/fuzz/net (practicalswift)
e6bb9fde85 tests: Add fuzzing harness for CAddrMan (practicalswift)

Pull request description:

  Add fuzzing harness for `CAddrMan`.

  ~~Fill some fuzzing coverage gaps for functions in `addrdb.h`, `merkleblock.h` and `outputtype.h`.~~

  See [`doc/fuzzing.md`](https://github.com/bitcoin/bitcoin/blob/master/doc/fuzzing.md) for information on how to fuzz Bitcoin Core. Don't forget to contribute any coverage increasing inputs you find to the [Bitcoin Core fuzzing corpus repo](https://github.com/bitcoin-core/qa-assets).

  Happy fuzzing :)

ACKs for top commit:
  MarcoFalke:
    review ACK d04a17a790

Tree-SHA512: a6b627e3a0cb51e3a8cb02ad0f19088fc0e965ca34ab110b68d5822d0ea7f473207ae312b49fb217cb6cf2f9f211d00bb69c83bac9f50d79c9ed1e157e85775d
2020-11-13 07:40:35 +01:00
MarcoFalke
99fcc2b477
Merge #20379: tests: Remove no longer needed UBSan suppression (float divide-by-zero in validation.cpp)
0ccb3addf6 tests: Remove no longer needed UBSan suppression (float-divide-by-zero in validation.cpp) (practicalswift)

Pull request description:

  Remove no longer needed UBSan suppression.

  The float divide-by-zero in `validation.cpp` was fixed by instagibbs in ec30a79f1c (#15283).

ACKs for top commit:
  MarcoFalke:
    ACK 0ccb3addf6

Tree-SHA512: 89a4f4b7371fa5725d9f801cee7ebbd17523f66017c9acfa813657dcb8d837f42209eff44ce9e5d48296a630bab9599d75f10024a0c7da7defb228f4eae3392a
2020-11-13 07:19:23 +01:00
practicalswift
0ccb3addf6 tests: Remove no longer needed UBSan suppression (float-divide-by-zero in validation.cpp) 2020-11-12 18:58:59 +00:00
Jonas Schnelli
440f8d3abe fix potential devision by 0 2020-11-12 17:11:18 +01:00
Wladimir J. van der Laan
0bd4929cd0
Merge #20284: addrman: ensure old versions don't parse peers.dat
38ada892ed addrman: ensure old versions don't parse peers.dat (Vasil Dimov)

Pull request description:

  Even though the format of `peers.dat` was changed in a backwards
  incompatible way, it is not guaranteed that old versions will fail to
  parse it. There is a chance that old versions parse its contents as
  garbage and use it.

  Old versions expect the "key size" field to be 32 and fail the parsing
  if it is not. Thus, we put something other than 32 in it. This will make
  versions between 0.11.0 and 0.20.1 deterministically fail on the new
  format. Versions prior to https://github.com/bitcoin/bitcoin/pull/5941
  will still parse it as garbage.

  Also, introduce a way to increment the `peers.dat` format in a way that
  does not necessary make older versions refuse to read it.

ACKs for top commit:
  jnewbery:
    ACK 38ada892ed
  laanwj:
    Code review ACK 38ada892ed
  MarcoFalke:
    re-ACK 38ada892ed 🥐

Tree-SHA512: 550bd660c5019dba0f9c334aca8a11c4a0463cfddf11efe7a4a5585ffb05549c82b95066fba5d073ae37893e0eccc158a7ffea9b33ea031d9be4a39e44f6face
2020-11-12 17:05:51 +01:00
practicalswift
d04a17a790 fuzz: Use ConsumeRandomLengthBitVector(...) in src/test/fuzz/connman and src/test/fuzz/net 2020-11-12 15:33:43 +00:00
practicalswift
e6bb9fde85 tests: Add fuzzing harness for CAddrMan 2020-11-12 14:23:17 +00:00
Jonas Schnelli
9bd1316697
Merge bitcoin-core/gui#120: Fix multiwallet transaction notifications
241434200e refactor: qt: Use vQueueNotifications.clear() (João Barbosa)
989e579d07 qt: Make transaction notification queue wallet specific (João Barbosa)
7b3b2303f4 move-only: Define TransactionNotification before  TransactionTablePriv (João Barbosa)

Pull request description:

  Currently `vQueueNotifications` holds transactions of any wallet, but the queue is dispatched on a given wallet and it assumes notifications are of that wallet.

  This means that some transactions can be missed if multiple wallets are loaded.

  Fix this by having a queue for each wallet.

ACKs for top commit:
  jonasschnelli:
    utACK 241434200e
  hebasto:
    ACK 241434200e, I have reviewed the code and it looks OK, I agree it can be merged.
  ryanofsky:
    Code review ACK 241434200e. Only change is dropping one commit

Tree-SHA512: 61beac5a16ed659e3a25ad145dbceafcef963aaf8f9838355298949ec2324e2bd760f59353cd251d30cf0334d8dc1642a1f3821d8a9eec092533b581f6ce86db
2020-11-12 12:23:55 +01:00
Jon Atack
05e82d86b0
wallet: override minfee checks (fOverrideFeeRate) for fee_rate
in RPCs fundrawtransaction and walletcreatefundedpsbt only.

This provides the existing feeRate (BTC/kvB) behavior in these two RPCs to the
new fee_rate (sat/vB) param also.

See these two GitHub review discussions for more info:
https://github.com/bitcoin/bitcoin/pull/10706/#discussion_r126560525
https://github.com/bitcoin/bitcoin/pull/20305#discussion_r520032533
2020-11-12 11:44:15 +01:00
Jon Atack
9a670b4f07
wallet: update sendtoaddress, send RPC examples with fee_rate 2020-11-12 11:43:22 +01:00
Jon Atack
be481b72e2
wallet: use MIN_RELAY_TX_FEE in bumpfee help
Co-authored-by: Murch <murch@murch.one>
2020-11-12 11:43:19 +01:00
Jon Atack
449b730579
wallet: provide valid values if invalid estimate mode passed
Co-authored-by: Murch <murch@murch.one>
2020-11-12 11:43:17 +01:00
Jon Atack
6da3afbaee
wallet: update remaining rpcwallet fee rate units to BTC/kvB 2020-11-12 11:43:14 +01:00
Jon Atack
173b5b5fe0
wallet: update fee rate units, use sat/vB for fee_rate error messages
and BTC/kvB for feeRate error messages.
2020-11-12 11:43:03 +01:00
MarcoFalke
8a486158cb
Merge #20188: tests: Add fuzzing harness for CConnman
79ef8324d4 tests: Add fuzzing harness for CConnman (practicalswift)

Pull request description:

  Add fuzzing harness for `CConnman`.

  See [`doc/fuzzing.md`](https://github.com/bitcoin/bitcoin/blob/master/doc/fuzzing.md) for information on how to fuzz Bitcoin Core. Don't forget to contribute any coverage increasing inputs you find to the [Bitcoin Core fuzzing corpus repo](https://github.com/bitcoin-core/qa-assets).

  Happy fuzzing :)

ACKs for top commit:
  MarcoFalke:
    review ACK 79ef8324d4

Tree-SHA512: eb9ffae20e939b818f8b9def064544b9a8fcd127ca22d1a54af1afedf1d24143be42419f3a03d684be59a5ff07b29d8bfa34ef2aaf1d9f9f75c4c1aaa90a29a8
2020-11-12 10:06:42 +01:00
MarcoFalke
af8ec1d3e5
Merge #20375: fuzz: Improve coverage for CPartialMerkleTree fuzzing harness
3c77b8009d fuzz: Improve coverage for CPartialMerkleTree fuzzing harness (practicalswift)

Pull request description:

  Improve coverage for `CPartialMerkleTree` fuzzing harness.

  See [`doc/fuzzing.md`](https://github.com/bitcoin/bitcoin/blob/master/doc/fuzzing.md) for information on how to fuzz Bitcoin Core. Don't forget to contribute any coverage increasing inputs you find to the [Bitcoin Core fuzzing corpus repo](https://github.com/bitcoin-core/qa-assets).

  Happy fuzzing :)

ACKs for top commit:
  MarcoFalke:
    ACK 3c77b8009d

Tree-SHA512: a1fa0f7650a5ee5ff83f35e41b9faf6c34671fc304b9af00e5b83073f21d50bcbe91c2428fa64d05dc42a7c521bfd24031e307c7f4abf9ded469d69a55c5d64a
2020-11-12 09:57:35 +01:00
MarcoFalke
027e51f715
Merge #20372: Avoid signed integer overflow when loading a mempool.dat file with a malformed time field
ee11a412a5 Avoid signed integer overflow when loading a mempool.dat file with a malformed time field (practicalswift)

Pull request description:

  Avoid signed integer overflow when loading a `mempool.dat` file with a malformed time field.

  Avoid the following signed integer overflow:

  ```
  $ xxd -p -r > mempool.dat-crash-1 <<EOF
  0100000000000000000000000004000000000000000000000000ffffffff
  ffffff7f00000000000000000000000000
  EOF
  $ cp mempool.dat-crash-1 ~/.bitcoin/regtest/mempool.dat
  $ UBSAN_OPTIONS="print_stacktrace=1:halt_on_error=1:report_error_type=1" src/bitcoind -regtest
  validation.cpp:5079:23: runtime error: signed integer overflow: 9223372036854775807 + 1209600 cannot be represented in type 'long'
      #0 0x5618d335197f in LoadMempool(CTxMemPool&) src/validation.cpp:5079:23
      #1 0x5618d3350df3 in CChainState::LoadMempool(ArgsManager const&) src/validation.cpp:4217:9
      #2 0x5618d2b9345f in ThreadImport(ChainstateManager&, std::vector<boost::filesystem::path, std::allocator<boost::filesystem::path> >, ArgsManager const&) src/init.cpp:762:33
      #3 0x5618d2b92162 in AppInitMain(util::Ref const&, NodeContext&, interfaces::BlockAndHeaderTipInfo*)::$_14::operator()() const src/init.cpp:1881:9
  ```

  This PR was broken out from PR #20089. Hopefully this PR is trivial to review.

  Fixes a subset of #19278.

ACKs for top commit:
  MarcoFalke:
    review ACK ee11a412a5
  Crypt-iQ:
    crACK ee11a412a5

Tree-SHA512: 227ab95cd7d22f62f3191693b455eacfa8e36534961bee12c622fc9090957cfb29992eabafa74d806a336e03385aa8f98b7ce734f04b0b400e33aa187d353337
2020-11-12 09:34:48 +01:00
MarcoFalke
bcd142e479
Merge #20285: Remove references to CreateWalletFromFile
c82336c493 Remove references to CreateWalletFromFile (fanquake)

Pull request description:

  `CWallet::CreateWalletFromFile()` was removed in 8b5e7297c0 but these references remain.

ACKs for top commit:
  hebasto:
    ACK c82336c493

Tree-SHA512: 3dd50fe0cd5a60bbc96d265107d4739f3e08f943435f3772038963ac4be9e4a87a863412ac0d571226ea66d71550b17b52f01b9d46a6282d49feae1508fd682e
2020-11-12 09:32:56 +01:00
fanquake
c82336c493
Remove references to CreateWalletFromFile
CWallet::CreateWalletFromFile() was removed in
8b5e7297c0 but these references remain.
2020-11-12 13:12:29 +08:00
Samuel Dobson
c2d8ba6265
Merge #19502: Bugfix: Wallet: Soft-fail exceptions within ListWalletDir file checks
24d2d3341d QA: wallet_multiwallet: Check that recursive symlink directory and wallet.dat loops are ignored (Luke Dashjr)
69f59af54d Bugfix: Wallet: Soft-fail exceptions within ListWalletDir file checks (Luke Dashjr)

Pull request description:

  Previously, an exception would be thrown, which could kill the node in some circumstances.

  Includes test changes to cause failure.

  Review with `?w=1`

ACKs for top commit:
  hebasto:
    re-ACK 24d2d3341d, rebased only since my [previous](https://github.com/bitcoin/bitcoin/pull/19502#pullrequestreview-520552944) review.
  promag:
    Tested ACK 24d2d3341d, test change fails on master.
  meshcollider:
    utACK 24d2d3341d

Tree-SHA512: f701f81b3aa3d3e15cee52ac9e7c31a73c0d8166e56bf077235294507cbcee099829fedc432a1c4b6d8780885f4e37897b44b980b08125771de3c849c000499e
2020-11-12 13:26:38 +13:00
practicalswift
3c77b8009d fuzz: Improve coverage for CPartialMerkleTree fuzzing harness 2020-11-11 22:31:40 +00:00
MarcoFalke
d9f5132736
Merge #20344: wallet: fix scanning progress calculation for single block range
5e146022da wallet: fix scanning progress calculation for single block range (Sebastian Falbesoner)

Pull request description:

  If the blockchain is rescanned for a single block (i.e. start and stop hashes are equal, and with that also the estimated start/stop verification progress values) the progress calculation could lead to a NaN value caused by a division by zero (0.0/0.0), resulting in an invalid JSON result for the `getwalletinfo` RPC.  This PR fixes this behaviour by setting the progress to zero in that special case. Fixes #20297.

  The behaviour can easily be reproduced by continuously running single block rescans in an endless loop, e.g. via
  ```bash
  #!/bin/bash
  while true
  do
      bitcoin-cli rescanblockchain $(bitcoin-cli getblockcount)
  done
  ```

  and at the same time perform some `getwalletinfo` RPCs.

  On the master branch, this leads to frequent invalid responses (tested on mainchain):
  ```
  $ bitcoin-cli getwalletinfo
  error: couldn't parse reply from server
  $ curl --user `cat ~/.bitcoin/.cookie` --data-binary '{"jsonrpc": "1.0", "id": "curltest", "method": "getwalletinfo", "params": []}' -H 'content-type: text/plain;' http://127.0.0.1:8332/
  {"result":{"walletname":"","walletversion":169900,"format":"bdb","balance":0.00000000,"unconfirmed_balance":0.00000000,"immature_balance":0.00000000,"txcount":0,"keypoololdest":1603677276,"keypoolsize":1000,"hdseedid":"3196e33ecb47c7130e6ca60f2f895f9259860dca","keypoolsize_hd_internal":1000,"paytxfee":0.00000000,"private_keys_enabled":true,"avoid_reuse":false,"scanning":{"duration":0,"progress":},"descriptors":false},"error":null,"id":"curltest"}
  ```
  (note that missing value for "progress" in the JSON result).

  On the PR branch, the behaviour doesn't occur anymore.

ACKs for top commit:
  MarcoFalke:
    review ACK 5e146022da
  promag:
    Core review ACK 5e146022da.

Tree-SHA512: f0e6aad5a6cd08b36c5fe820fff0ef26663229b39169a4dbe757f3c795a41cf5c69c9dc90efe7515675ae1059307f8971123781a0514d10704123a6f28b125ab
2020-11-11 16:11:01 +01:00
Vasil Dimov
38ada892ed
addrman: ensure old versions don't parse peers.dat
Even though the format of `peers.dat` was changed in an incompatible
way (old software versions <0.21 cannot understand the new file format),
it is not guaranteed that old versions will fail to parse it. There is a
chance that old versions parse its contents as garbage and use it.

Old versions expect the "key size" field to be 32 and fail the parsing
if it is not. Thus, we put something other than 32 in it. This will make
versions between 0.11.0 and 0.20.1 deterministically fail on the new
format. Versions prior to https://github.com/bitcoin/bitcoin/pull/5941
(<0.11.0) will still parse it as garbage.

Also, introduce a way to increment the `peers.dat` format in a way that
does not necessary make older versions refuse to read it.
2020-11-11 16:05:15 +01:00
Jon Atack
7f9835a05a
wallet: remove fee rates from conf_target helps 2020-11-11 15:56:05 +01:00
Jon Atack
b7994c01e9
wallet: add fee_rate unit warnings to bumpfee 2020-11-11 15:56:03 +01:00
Jon Atack
410e471fa4
wallet: remove redundant bumpfee fee_rate checks
SetFeeEstimateMode() handles these checks now and provides a more actionable
error message.
2020-11-11 15:56:01 +01:00