Commit graph

24369 commits

Author SHA1 Message Date
Sjors Provoost
89a28e02fa
[test] add v0.16.3 backwards compatibility test 2020-05-04 13:36:40 +02:00
João Barbosa
e8123eae40 gui: Fix itemWalletAddress leak when not tree mode 2020-05-04 12:05:42 +01:00
Jonas Schnelli
afa577c323
Merge #15768: gui: Add close window shortcut
f5a3a5b9ab gui: Add close window shortcut (Miguel Herranz)

Pull request description:

  CMD+W is the standard shortcut in macOS to close a window without
  exiting the program.

  This adds support to use the shortcut in both main and debug windows.

ACKs for top commit:
  jonasschnelli:
    Tested ACK f5a3a5b9ab
  hebasto:
    ACK f5a3a5b9ab, tested on Linux Mint 19.3 by manually opening available dialogs and sub-windows, and applying the `Ctrl+W` shortcut. Also tested with "Minimize on close" option enabled / disabled.

Tree-SHA512: 39851f6680cf97c334d5759c6f8597cb45685359417493ff8b0566672edbd32303fa15ac4260ec8ab5ea1458a600a329153014f25609e1db9cf399aa851ae2f9
2020-05-04 11:53:34 +02:00
practicalswift
b56607a89b Remove CCoinsViewCache::GetValueIn(...) 2020-05-03 18:42:14 +00:00
Vasil Dimov
71f183a49b
build: warn on potentially uninitialized reads
Enable -Wconditional-uninitialized to warn on potentially uninitialized
reads.

Fix the sole such warning in Bitcoin Core in GetRdRand(): r1 would be
set to 0 on rdrand failure, so initializing it to 0 is a non-functional
change.

From "Intel 64 and IA-32 ArchitecturesSoftware Developer's Manual" [1],
page 1711: "CF=1 indicates that the data in the destination is valid.
Otherwise CF=0 and the data in the destination operand will be returned
as zeros for the specified width."

[1] https://software.intel.com/sites/default/files/managed/39/c5/325462-sdm-vol-1-2abcd-3abcd.pdf
2020-05-03 17:21:45 +02:00
MarcoFalke
2e6a16b968
Merge #18855: tests: feature_backwards_compatibility.py test downgrade after upgrade
489ebfd7a1 tests: feature_backwards_compatibility.py test downgrade after upgrade (Andrew Chow)

Pull request description:

  After upgrading the node, try to go back to the original version to make sure that using a newer node version does not prevent the wallet file from being downgraded again.

ACKs for top commit:
  MarcoFalke:
    ACK 489ebfd7a1

Tree-SHA512: 86231de6514b3657912fd9d6621212166fd2b29b591fc97120092c548babcf1d6f50b5bd103b28cecde395a26809134f01c1a198725596c3626420de3fd1f017
2020-05-03 10:35:21 -04:00
MarcoFalke
cce034b028
Merge #18839: test: Fix intermittent issues
fab7ee3990 test: Fix p2p_leak intermittent issue (MarcoFalke)
fa8614aea9 test: Fix intermittent p2p_segwit issue (MarcoFalke)

Pull request description:

  Fixes #18801
  Fixes #18802

ACKs for top commit:
  practicalswift:
    ACK fab7ee3990 -- diff looks correct

Tree-SHA512: b5d0473ec1133aded127bef4c0c90f6b55906327cb49cbbd0776c4971cd9d5c9375fb70cc3dc8dd1f4bd8b1245c4414e803dc0d29ed4553b00eb131d27c9c128
2020-05-03 10:30:47 -04:00
MarcoFalke
ddc0a600b3
Merge #18617: test: add factor option to adjust test timeouts
2742c34286 test: add factor option to adjust test timeouts (Harris)

Pull request description:

  This PR adds a new option **factor** that can be used to adjust timeouts in various functional tests.
  Several timeouts and functions from `authproxy`, `mininode`, `test_node` and `util` have been adapted to use this option. The factor-option definition is located in `test_framework.py`.

  Fixes https://github.com/bitcoin/bitcoin/issues/18266
  Also Fixes https://github.com/bitcoin/bitcoin/issues/18834

ACKs for top commit:
  MarcoFalke:
    Thanks! ACK 2742c34286

Tree-SHA512: 6d8421933ba2ac1b7db00b70cf2bc242d9842c48121c11aadc30b0985c4a174c86a127d6402d0cd73b993559d60d4f747872d21f9510cf4806e008349780d3ef
2020-05-03 08:58:56 -04:00
Andrew Chow
489ebfd7a1 tests: feature_backwards_compatibility.py test downgrade after upgrade
After upgrading the node, try to go back to the original version to make
sure that using a newver node version does not prevent the wallet file
from being downgraded again.
2020-05-02 23:45:17 -04:00
Harris
2742c34286
test: add factor option to adjust test timeouts
Co-authored-by: MarcoFalke <falke.marco@gmail.com>
2020-05-03 01:42:40 +02:00
MarcoFalke
fa09110ebb
doc: Fix typo in Coin doxygen comment 2020-05-02 19:30:58 -04:00
fanquake
68ef9523d1
Merge #18413: script: prevent UB when computing abs value for num opcode serialize
2748e87932 script: prevent UB when computing abs value for num opcode serialize (pierrenn)

Pull request description:

  This was reported by practicalswift here #18046

  It seems that the original author of the line used a reference to glibc `abs`: https://github.com/lattera/glibc/blob/master/stdlib/abs.c

  However depending on some implementation details this can be undefined behavior for unusual values.

  A detailed explanation of the UB is provided here : https://stackoverflow.com/questions/17313579/is-there-a-safe-way-to-get-the-unsigned-absolute-value-of-a-signed-integer-with (by [Billy O'Neal](https://twitter.com/malwareminigun))

  Simple relevant godbolt example :  https://godbolt.org/z/yRwtCG

  Thanks!

ACKs for top commit:
  sipa:
    ACK 2748e87932
  MarcoFalke:
    ACK 2748e87932, only checked that the bitcoind binary does not change with clang -O2 🎓
  practicalswift:
    ACK 2748e87932

Tree-SHA512: 539a34c636c2674c66cb6e707d9d0dfdce63f59b5525610ed88da10c9a8d59d81466b111ad63b850660cef3750d732fc7755530c81a2d61f396be0707cd86dec
2020-05-02 21:24:05 +08:00
Andrew Chow
b59b4504ab have GenerateNewKey and DeriveNewChildKey take a CHDChain as an argument 2020-05-01 18:46:00 -04:00
Hennadii Stepanov
35f1189ea7
build: Rename BUILD_* macros and the code self-descriptive 2020-05-02 01:00:07 +03:00
Hennadii Stepanov
dc1fba9389
scripted-diff: Rename share/genbuild.sh macros to more meaningful ones
-BEGIN VERIFY SCRIPT-
sed -i 's/\bDESC\b/GIT_TAG/' share/genbuild.sh
sed -i 's/\bSUFFIX\b/GIT_COMMIT/g' share/genbuild.sh
-END VERIFY SCRIPT-
2020-05-02 00:59:20 +03:00
MarcoFalke
844d2070a2
Merge #18828: test: Strip down previous releases boilerplate
fa359d14c0 test: Strip down previous releases boilerplate (MarcoFalke)

Pull request description:

  Reduces code bloat and mental load to write compatibility tests

ACKs for top commit:
  Sjors:
    tACK fa359d14c0 on macOS

Tree-SHA512: dc66286b24b2f137e5bca99412850ec7eee8cc61cf9cdc7ab532d529220808189baea8d1b077f8b7f40d3e8881d981e1ffc5a877adb394816f1225b1186253e4
2020-05-01 14:57:59 -04:00
Carl Dong
27e63e01cc
build: Accomodate makensis v2.x
Apparently the -X flag doesn't work as expected in makensis v2.x

For example:

  makensis -V2 share/setup.nsi -X'OutFile "test.exe"'

Will output:

  OutFile expects 1 parameters, got 0.
  Usage: OutFile install_output.exe

So let's instead construct the file using POSIX-compliant commands and
shell constructs
2020-05-01 14:27:57 -04:00
Carl Dong
1f2c39a30e
guix: Remove logical cores requirement
Thanks MarcoFalke for pushing this to its limits and testing :-)
2020-05-01 12:33:47 -04:00
Carl Dong
a4f6ffa71e
lint: Also enable source statements for non-gitian 2020-05-01 12:31:07 -04:00
Carl Dong
d256f91cb1
rel-builds: Directly deploy win installer to OUTDIR 2020-05-01 12:30:20 -04:00
MarcoFalke
fab7ee3990
test: Fix p2p_leak intermittent issue 2020-05-01 08:45:26 -04:00
MarcoFalke
fa8614aea9
test: Fix intermittent p2p_segwit issue 2020-05-01 08:36:35 -04:00
MarcoFalke
fa2cce4391
wallet: Remove trailing whitespace from potential translation strings
If the potential translation strings are translated in the future,
trailing whitespace is going to make translation effort harder.
2020-05-01 07:41:32 -04:00
MarcoFalke
fa59cc1c97
wallet: Report full error message in wallettool 2020-05-01 07:39:35 -04:00
MarcoFalke
fae7776690
wallet: Avoid translating RPC errors when creating txs
Also, mark feebumper bilingual_str as Untranslated

They are technical and have previously not been translated either.
It is questionable whether they can even appear in the GUI.
2020-05-01 07:39:06 -04:00
MarcoFalke
fae51a5c6f
wallet: Avoid translating RPC errors when loading wallets
Common errors and warnings should be translated when displayed in the
GUI, but not translated when displayed elsewhere. The wallet method
CreateWalletFromFile does not know its caller, so this commit changes it
to return a bilingual_str to the caller.
2020-05-01 07:39:00 -04:00
Russell Yanofsky
d3a56be77a Revert "gui: Avoid Wallet::GetBalance in WalletModel::pollBalanceChanged"
This reverts commit 0933a37078 from
https://github.com/bitcoin/bitcoin/pull/18160 which no longer an optimization
since commit "gui: Avoid wallet tryGetBalances calls before TransactionChanged
or BlockTip notifications".
2020-05-01 06:59:09 -04:00
Russell Yanofsky
28b112e9bd Get rid of BindWallet
CWalletTx initialization has been fixed so it's no longer necessary to change
which wallet a transaction is bound to.
2020-05-01 05:59:09 -05:00
MarcoFalke
608359b071
Merge #16426: Reverse cs_main, cs_wallet lock order and reduce cs_main locking
6a72f26968 [wallet] Remove locked_chain from CWallet, its RPCs and tests (Antoine Riard)
841178820d [wallet] Move methods from Chain::Lock interface to simple Chain (Antoine Riard)
0a76287387 [wallet] Move getBlockHash from Chain::Lock interface to simple Chain (Antoine Riard)
de13363a47 [wallet] Move getBlockHeight from Chain::Lock interface to simple Chain (Antoine Riard)
b855592d83 [wallet] Move getHeight from Chain::Lock interface to simple Chain (Antoine Riard)

Pull request description:

  This change is intended to make the bitcoin node and its rpc, network and gui interfaces more responsive while the wallet is in use. Currently, because the node's `cs_main` mutex is always locked before the wallet's `cs_wallet` mutex (to prevent deadlocks), `cs_main` currently stays locked while the wallet does relatively slow things like creating and listing transactions.

  Switching the lock order so `cs_main` is acquired after `cs_wallet` allows `cs_main` to be only locked intermittently while the wallet is doing slow operations, so the node is not blocked waiting for the wallet.

  To review the present PR, most of getting right the move is ensuring any `LockAssertion` in `Chain::Lock` method is amended as a `LOCK(cs_main)`. And in final commit, check that any wallet code which was previously locking the chain is now calling a  method, enforcing the lock taking job. So far the only exception I found is `handleNotifications`, which should be corrected.

ACKs for top commit:
  MarcoFalke:
    re-ACK 6a72f26968 🔏
  fjahr:
    re-ACK 6a72f26968
  ryanofsky:
    Code review ACK 6a72f26968. Only difference compared to the rebase I posted is reverting unneeded SetLastBlockProcessed change in wallet_disableprivkeys test

Tree-SHA512: 9168b3bf3432d4f8bc4d9fa9246ac057050848e673efc264c8f44345f243ba9697b05c22c809a79d1b51bf0de1c4ed317960e496480f8d71e584468d4dd1b0ad
2020-05-01 06:59:09 -04:00
Russell Yanofsky
bf0a510981 gui: Avoid wallet tryGetBalances calls before TransactionChanged or BlockTip notifications
interfaces::Wallet::tryGetBalances was recently updated in
https://github.com/bitcoin/bitcoin/pull/18160 to avoid computing balances
internally, but this not efficient as it could be with #10102 because
tryGetBalances is an interprocess call.

Implementing the TransactionChanged / BlockTip check outside of tryGetBalances
also allows tryGetBalances to be simplified in next commit 'Revert "gui: Avoid
Wallet::GetBalance in WalletModel::pollBalanceChanged"'.
2020-05-01 06:59:09 -04:00
Russell Yanofsky
d002f9d15d Disable CWalletTx copy constructor
Disable copying of CWalletTx objects to prevent bugs where instances get copied
in and out of the mapWallet map and fields are updated in the wrong copy.
2020-05-01 05:59:09 -05:00
Russell Yanofsky
2bc9b92ed8 Cancel wallet balance timer when shutdown requested
This doesn't fix any current problem, but it makes balance checking code less
fragile, and prevents use-after free travis error in next commit:
https://travis-ci.org/github/bitcoin/bitcoin/jobs/675367629#L4240
2020-05-01 06:59:09 -04:00
Russell Yanofsky
65b9d8f8dd Avoid copying CWalletTx in LoadToWallet
The change in walletdb.cpp is easier to review ignoring whitespace.

This change is need to get rid of CWalletTx copy constructor.
2020-05-01 05:59:09 -05:00
Russell Yanofsky
83f69fab3a Switch transaction table to use wallet height not node height
Tweak of #17905 to make gui display of transactions and balances more
consistent. This change shouldn't cause visible effects in normal cases, just
make GUI wallet code more internally correct and consistent.
2020-05-01 06:59:09 -04:00
Russell Yanofsky
bd2fbc7cdb Get rid of unneeded CWalletTx::Init parameter 2020-05-01 05:59:09 -05:00
Russell Yanofsky
2b9cba2065 Remove CWalletTx merging logic from AddToWallet
Instead of AddToWallet taking a temporary CWalletTx object and then potentially
merging it with a pre-existing CWalletTx, have it take a callback so callers
can update the pre-existing CWalletTx directly.

This makes AddToWallet simpler because now it is only has to be concerned with
saving CWalletTx objects and not merging them.

This makes AddToWallet calls clearer because they can now make direct updates to
CWalletTx entries without having to make temporary objects and then worry about
how they will be merged.

This is a pure refactoring, no behavior is changing.
2020-05-01 05:59:09 -05:00
Antoine Riard
6a72f26968 [wallet] Remove locked_chain from CWallet, its RPCs and tests
This change is intended to make the bitcoin node and its rpc, network
and gui interfaces more responsive while the wallet is in use. Currently
because the node's cs_main mutex is always locked before the wallet's
cs_wallet mutex (to prevent deadlocks), cs_main currently stays locked
while the wallet does relatively slow things like creating and listing
transactions.

This commit only remmove chain lock tacking in wallet code, and invert
lock order from cs_main, cs_wallet to cs_wallet, cs_main.
must happen at once to avoid any deadlock. Previous commit were only
removing Chain::Lock methods to Chain interface and enforcing they
take cs_main.

Remove LockChain method from CWallet and Chain::Lock interface.
2020-04-30 14:41:24 -04:00
Antoine Riard
841178820d [wallet] Move methods from Chain::Lock interface to simple Chain
Remove findPruned and findFork, no more used after 17954.
2020-04-30 14:37:21 -04:00
Antoine Riard
0a76287387 [wallet] Move getBlockHash from Chain::Lock interface to simple Chain 2020-04-30 14:37:21 -04:00
Antoine Riard
de13363a47 [wallet] Move getBlockHeight from Chain::Lock interface to simple Chain
Add HaveChain to assert chain access for wallet-tool in LoadToWallet.
2020-04-30 14:37:21 -04:00
Antoine Riard
b855592d83 [wallet] Move getHeight from Chain::Lock interface to simple Chain
Instead of calling getHeight, we rely on CWallet::m_last_block
processed_height where it's possible.
2020-04-30 14:31:19 -04:00
MarcoFalke
e5b9308920
Merge #18829: ci: Merge C++17 build with one of the existing ones
aaaacff107 ci: Merge C++17 build with one of the existing ones (MarcoFalke)

Pull request description:

  No need to spin up an extra vm for each pull request for a simple sanity check that any of the other already running machines can test.

Top commit has no ACKs.

Tree-SHA512: ce1609aa4cbc057fc9e85e61f300eac2317cc206647a20f6b001180ffed6623e2243a408476546e16baacb91cb2dd9a2e7b6e2402c8fd829a72860b2c3eb7be6
2020-04-30 14:04:24 -04:00
MarcoFalke
ef8ef31fd2
Merge #18775: tests: Add fuzzing harnesses for various classes/functions in policy/ (CBlockPolicyEstimator, IsRBFOptIn(…), etc.)
2bcc2bd742 tests: Clarify how we avoid hitting the signed integer overflow in CFeeRate::GetFeePerK() when fuzzing (practicalswift)
13c1f6b24f tests: Add fuzzing harness for IsRBFOptIn(...) (practicalswift)
3439c88a5d tests: Add fuzzing harness for CBlockPolicyEstimator (practicalswift)

Pull request description:

  Add fuzzing harnesses for various classes/functions in `policy/` (`CBlockPolicyEstimator`, `IsRBFOptIn(…)`, etc.).

  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 :)

Top commit has no ACKs.

Tree-SHA512: a756687216802a3b260def5706798a1eb673b3447408561728af77a1d61c8bfb3a7d9b874e16bf619565e9d093f9b595e07d6e6fa430ac08dfeed4f45b01fbc3
2020-04-30 12:53:42 -04:00
MarcoFalke
aaaacff107
ci: Merge C++17 build with one of the existing ones 2020-04-30 12:50:36 -04:00
MarcoFalke
fa359d14c0
test: Strip down previous releases boilerplate 2020-04-30 11:45:16 -04:00
practicalswift
2bcc2bd742 tests: Clarify how we avoid hitting the signed integer overflow in CFeeRate::GetFeePerK() when fuzzing 2020-04-30 14:19:49 +00:00
MarcoFalke
a66ba6d029
Merge #18576: test: use unittest for test_framework unit testing
de8905adf2 test: use unittest and test_runner for test framework unit testing (Gloria Zhao)

Pull request description:

  Proposal for unit testing on test_framework functions:
  1. Use the python `unittest` library. Don't use test_framework to test itself.
  2. Put the tests inside the same file as the functions they are testing.
  3. Call the tests from `test_runner.py`. To include more Test Framework tests, add the filename to the list `TEST_FRAMEWORK_MODULES`. Don't add new files or change the list of accepted script prefixes.

  Makes these changes for `bn2vch` (followup to [this comment](https://github.com/bitcoin/bitcoin/pull/18378#pullrequestreview-377271264)).

ACKs for top commit:
  jnewbery:
    Tested ACK de8905adf2. Great stuff gzhao408 . Thanks for this!

Tree-SHA512: 91572d43e203a1864765b93a9472667994115ec38b271f2b2f9fcd0f0112b393fc24ba7d2371d5a34b0a6a4522f6b934fc5164363819aa7ed8d6c6c9a60cc101
2020-04-30 09:30:13 -04:00
practicalswift
13c1f6b24f tests: Add fuzzing harness for IsRBFOptIn(...) 2020-04-30 13:19:24 +00:00
practicalswift
3439c88a5d tests: Add fuzzing harness for CBlockPolicyEstimator 2020-04-30 13:19:24 +00:00
MarcoFalke
0000ea3265
test: Add test for GetRandMillis and GetRandMicros 2020-04-30 09:19:16 -04:00