Commit graph

23986 commits

Author SHA1 Message Date
Wladimir J. van der Laan dd3310bbb8
Merge #18854: doc: Fix typo in Coin doxygen comment
fa09110ebb doc: Fix typo in Coin doxygen comment (MarcoFalke)

Pull request description:

  `CTxOutCompressor` has been renamed in commit 4de934b9b5, so rename it in the docs as well.

ACKs for top commit:
  laanwj:
    ACK fa09110ebb
  hebasto:
    ACK fa09110ebb

Tree-SHA512: e16a21ac3112a67ee7d5ffabb3f47103aed8f91fdebf1bf96311cd0b7bdb9b7323ed826bfa95517386d4128ff0ae2c7c13bad047a7c5a0cc2458be7a43119157
2020-05-06 13:15:28 +02:00
fanquake 551dc7f664
Merge #18806: net: remove is{Empty,Full} flags from CBloomFilter, clarify CVE fix
1ad8ea2b73 net: remove is{Empty,Full} flags from CBloomFilter, clarify CVE fix (Sebastian Falbesoner)

Pull request description:

  The BIP37 bloom filter class `CBloomFilter` contains two flags `isEmpty`/`isFull` together with an update method with the purpose to, according to the comments, "avoid wasting cpu", i.e. the mechanism should serve as an optimization for the trivial cases of empty (all bits zero) or full (all bits one) filters.
  However, the real reason of adding those flags (introduced with commit 37c6389c5a by gmaxwell) was a _covert fix_ of [CVE-2013-5700](https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2013-5700), a vulnerability that allowed a divide-by-zero remote node crash.
  According to gmaxwell himself (https://github.com/bitcoin/bitcoin/pull/9060#issuecomment-257749165):
  > the IsEmpty/IsFull optimizations were largely a pretextual optimization intended to make unexploitable a remote crash vulnerability (integer division by zero) that existed in the original bloom filtering code without disclosing it. I'm doubtful that they are all that useful. :)

  For more information on how to trigger this crash, see PR https://github.com/bitcoin/bitcoin/pull/18515 which contains a detailled description and a regression test. It has also been discussed on a [recent PR club meeting on fuzzing](https://bitcoincore.reviews/18521.html).

  The covert fix code already led to issues and PR based on the wrong assumption that the flags are there for optimization reasons (see #16886 and #16922). This PR gets rid of the flags and the update method and just focuses on the CVE fix itself, i.e. it can be seen as a revert of the covert fix commit modulo the actual fix.

ACKs for top commit:
  meshcollider:
    utACK 1ad8ea2b73
  laanwj:
    Concept and code review ACK 1ad8ea2b73
  jkczyz:
    ACK 1ad8ea2b73
  MarcoFalke:
    ACK 1ad8ea2b73
  fjahr:
    Code review ACK 1ad8ea2b73

Tree-SHA512: 29f7ff9faece0285e11e16c024851f5bcb772dec64118ccc3f9067ec256267ec8e1b1e3105c7de2a72fd122c3b085e8fc840ab8f4e49813f1cc7a444df1867f7
2020-05-06 15:40:06 +08:00
fanquake d96fdc2a39
Merge #18741: guix: Make source tarball using git-archive
bfe1ba2f5b rel-builds: Specify core.abbrev for git-rev-parse (Carl Dong)
27e63e01cc build: Accomodate makensis v2.x (Carl Dong)
1f2c39a30e guix: Remove logical cores requirement (Carl Dong)
a4f6ffa71e lint: Also enable source statements for non-gitian (Carl Dong)
d256f91cb1 rel-builds: Directly deploy win installer to OUTDIR (Carl Dong)
fa791da02f nsis: Specify OutFile path only once (Carl Dong)
14701604d0 guix: Expose GIT_COMMON_DIR in container as readonly (Carl Dong)
f5a6ac4f48 guix: Make source tarball using git-archive (Carl Dong)
395c1137f6 gitian: Limit sourced script to just assignments (Carl Dong)

Pull request description:

  Based on: #18556
  Related: https://github.com/bitcoin/bitcoin/pull/17595#discussion_r399728721

ACKs for top commit:
  fanquake:
    ACK bfe1ba2f5b - I agree with Carl, and am going to merge this. I'd like for Linux Guix builds to be working again, and we can rebase #18818.

Tree-SHA512: c87ada7e3de17ca0b692a91029b86573442ded5780fc081c214773f6b374a0cdbeaf6f6898c36669c2e247ee32aa7f82defb1180f8decac52c65f0c140f18674
2020-05-06 13:13:36 +08:00
Samuel Dobson 60091d20f9
Merge #9381: Remove CWalletTx merging logic from AddToWallet
28b112e9bd Get rid of BindWallet (Russell Yanofsky)
d002f9d15d Disable CWalletTx copy constructor (Russell Yanofsky)
65b9d8f8dd Avoid copying CWalletTx in LoadToWallet (Russell Yanofsky)
bd2fbc7cdb Get rid of unneeded CWalletTx::Init parameter (Russell Yanofsky)
2b9cba2065 Remove CWalletTx merging logic from AddToWallet (Russell Yanofsky)

Pull request description:

  This is a pure refactoring, no behavior is changing.

  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.

  Motivation for this change came from the bumpfee PR #8456 where we wanted to be able to call AddToWallet to make a simple update to an existing transaction, but were reluctant to, because the existing CWalletTx merging logic did not apply and seemed dangerous try to update as part of that PR. After this refactoring, the bumpfee PR could call AddToWallet safely instead of implementing a duplicate AddToWallet function.

  This also allows getting rid of the CWalletTx copy constructor to prevent unintentional copying.

ACKs for top commit:
  MarcoFalke:
    Anyway, re-ACK 28b112e9bd

Tree-SHA512: 528dd088714472a237500b200f4433db850bdb7fc29c5e5d81cae48072061dfb967f7c37edd90b33f24901239f9be982988547c1f8c80abc25fb243fbf7330ef
2020-05-06 11:36:32 +12:00
MarcoFalke fbd522721c
Merge #18885: contrib: Move optimize-pngs.py script to the maintainer repo
fa13090d20 contrib: Remove optimize-pngs.py script, which lives in the maintainer repo (MarcoFalke)

Pull request description:

  Moved to https://github.com/bitcoin-core/bitcoin-maintainer-tools/blob/master/optimize-pngs.py

  Bitcoin Core should focus on the full node implementation, not on scripts to compress png images.

  This script is only used when new PNG files are added to the repo. This happens about once every two years. So fetching the script from the other repo should not be a burden, but removing it from this repo is going to cut down on the meta files we need to maintain in the main repo.

ACKs for top commit:
  practicalswift:
    ACK fa13090d20 -- `+0 lines, -82 lines` :)
  promag:
    ACK fa13090d20.
  hebasto:
    ACK fa13090d20, verified that script is already [moved](https://github.com/bitcoin-core/bitcoin-maintainer-tools/pull/56).

Tree-SHA512: 37d111adae769bcddc6ae88041032d5a2b8b228fec67f555c8333c38de3992f5138b30bea868d7d6d6b7f966a47133e5853134373b149ab23cba3b8b560ecb31
2020-05-05 10:40:39 -04:00
MarcoFalke 52ce396b2a
Merge #18879: valgrind: remove outdated suppressions
d7120f7f78 valgrind : remove duplicate BCLog::Logger suppression (fanquake)
708e3c7e85 valgrind: remove rest_blockhash_by_height suppression (fanquake)

Pull request description:

  708e3c7e85: `Suppress rest_blockhash_by_height` should no longer be needed after #18785.

  d7120f7f78 : Removes a duplicate `Suppress BCLog::Logger::StartLogging()` suppression that was added in #17770.

ACKs for top commit:
  MarcoFalke:
    ACK d7120f7f78
  practicalswift:
    ACK d7120f7f78 -- patch looks correct and valgrind Travis job is happy

Tree-SHA512: 45f5b9fa64bf83cada3cd9ad33c245f660376d5b29f51a2531d83133940090df945f5ef26c5847d6ec024ffab9528d55573c5cf9ca5e73795f9abfc971b3d29b
2020-05-05 10:06:35 -04:00
MarcoFalke fa13090d20
contrib: Remove optimize-pngs.py script, which lives in the maintainer repo
https://github.com/bitcoin-core/bitcoin-maintainer-tools/blob/master/optimize-pngs.py
2020-05-05 07:25:58 -04:00
Samuel Dobson ec79b5f86b
Merge #18782: wallet: Make sure no DescriptorScriptPubKeyMan or WalletDescriptor members are left uninitialized after construction
2a78098098 wallet: Make sure no WalletDescriptor members are uninitialized after construction (practicalswift)
ff046aeeba wallet: Make sure no DescriptorScriptPubKeyMan members are uninitialized after construction (practicalswift)

Pull request description:

  This is a small folllow-up to #16528 ("Native Descriptor Wallets using DescriptorScriptPubKeyMan") which was merged in to `master` a couple of hours ago.

  Make sure no `DescriptorScriptPubKeyMan` or `WalletDescriptor` members are left uninitialized after construction.

  Before this change `bool m_internal` was left uninitialized when using the `DescriptorScriptPubKeyMan(WalletStorage&, WalletDescriptor&)` ctor.

  The same goes for the now initialized integers which were left uninitialized when using the `WalletDescriptor()` ctor.

ACKs for top commit:
  instagibbs:
    utACK  2a78098098
  fjahr:
    Code review ACK 2a78098098
  Sjors:
    utACK 2a78098
  achow101:
    ACK 2a78098098
  brakmic:
    Code review ACK 2a78098098
  meshcollider:
    utACK 2a78098098

Tree-SHA512: c98e035268fdc7f65a423b73ac0cf010b0ef7c5e679b3cf170c1813efac8ab5c657dcbaf43c746770bea59e4772bfefe4caa834f1175260c39c7f35d92946ba5
2020-05-05 15:56:04 +12:00
fanquake d7120f7f78
valgrind : remove duplicate BCLog::Logger suppression
This was added in #17770, but is identical to the supression above.
2020-05-05 11:10:36 +08:00
fanquake 708e3c7e85
valgrind: remove rest_blockhash_by_height suppression
This should no-longer be necessary after #18785.
2020-05-05 11:10:35 +08:00
fanquake e727c2bdca
Merge #18088: build: ensure we aren't using GNU extensions
0ae8f18dfe build: add -Wgnu to compile flags (fanquake)
3a0fd7726b Remove use of non-standard zero variadic macros (Ben Woosley)
49f6178c3e Drop unused LOG_TIME_MICROS helper (Ben Woosley)
5d4999951e prevector: Avoid unnamed struct, which is a GNU extension (DesWurstes)

Pull request description:

  Since we [started using](https://github.com/bitcoin/bitcoin/pull/7165) the `ax_cxx_compile_stdcxx.m4` macro we've been passing `[noext]` to indicate that we don't want to use an extended mode, i.e GNU extensions. Speaking to Cory he clarified that the intention was to "require only vanilla c++11 and turn _off_ extension support so they would fail to compile".

  However in the codebase we are currently making use of some GNU extensions. We should either remove there usage, or at least amend our CXX compiler checks. I'd prefer the former.

  #### anonymous structs
  ```bash
  ./prevector.h:153:9: warning: anonymous structs are a GNU extension [-Wgnu-anonymous-struct]
          struct {
  ```

  This is fixed in b849212c1e.

  #### variadic macros

  ```bash
  ./undo.h:57:50: warning: must specify at least one argument for '...' parameter of variadic macro [-Wgnu-zero-variadic-macro-arguments]
              ::Unserialize(s, VARINT(nVersionDummy));
  ```

  This is taken care of in #18087.

  The `LOG_TIME_*` macros introduced in #16805 make use of a [GNU extension](https://gcc.gnu.org/onlinedocs/cpp/Variadic-Macros.html).

  ```bash
  In file included from validation.cpp:22:
  ./logging/timer.h:99:99: warning: token pasting of ',' and __VA_ARGS__ is a GNU extension [-Wgnu-zero-variadic-macro-arguments]
      BCLog::Timer<std::chrono::milliseconds> PASTE2(logging_timer, __COUNTER__)(__func__, end_msg, ## __VA_ARGS__)
                                                                                                    ^
  ./logging/timer.h:99:99: warning: token pasting of ',' and __VA_ARGS__ is a GNU extension [-Wgnu-zero-variadic-macro-arguments]
  ./logging/timer.h:99:99: warning: token pasting of ',' and __VA_ARGS__ is a GNU extension [-Wgnu-zero-variadic-macro-arguments]
  ./logging/timer.h:99:99: warning: token pasting of ',' and __VA_ARGS__ is a GNU extension [-Wgnu-zero-variadic-macro-arguments]
  ./logging/timer.h:99:99: warning: token pasting of ',' and __VA_ARGS__ is a GNU extension [-Wgnu-zero-variadic-macro-arguments]
  ./logging/timer.h:101:92: warning: token pasting of ',' and __VA_ARGS__ is a GNU extension [-Wgnu-zero-variadic-macro-arguments]
      BCLog::Timer<std::chrono::seconds> PASTE2(logging_timer, __COUNTER__)(__func__, end_msg, ## __VA_ARGS__)
                                                                                             ^
  6 warnings generated.
  ```

  This is fixed in 081a0ab64eb442bc85c4d4a4d3bc2c8e97ac2a6d and 612e8e138b97fc5ad2f38847300132a8fc423c3f.

  #### prevention
  To ensure that usage doesn't creep back in we can add [`-Wgnu`](https://clang.llvm.org/docs/DiagnosticsReference.html#wgnu) to our compile time flags, which will make Clang warn whenever it encounters GNU extensions.

  This would close #14130.
  Also related to #17230, where it's suggested we use a GNU extension, the `gnu::pure` attribute.

ACKs for top commit:
  practicalswift:
    ACK 0ae8f18dfe -- diff looks correct
  MarcoFalke:
    ACK 0ae8f18dfe
  vasild:
    utACK 0ae8f18df
  dongcarl:
    ACK 0ae8f18dfe

Tree-SHA512: c517404681ef8edf04c785731d26105bac9f3c9c958605aa24cbe399c649e7c5ee0c4aa8e714fd2b2d335e2fbea4d571e09b0dec36678ef871f0a6683ba6bb7f
2020-05-05 07:44:23 +08:00
Carl Dong bfe1ba2f5b
rel-builds: Specify core.abbrev for git-rev-parse
Chose 12 because the kernel uses it:

  https://public-inbox.org/git/CA+55aFy0_pwtFOYS1Tmnxipw9ZkRNCQHmoYyegO00pjMiZQfbg@mail.gmail.com/raw

And also because it's a nice number.
2020-05-04 13:00:08 -04:00
Wladimir J. van der Laan b549cb1bd2
Merge #18443: lockedpool: avoid sensitive data in core files (FreeBSD)
f85203097f lockedpool: avoid sensitive data in core files (FreeBSD) (Vasil Dimov)

Pull request description:

  This is a followup to
  23991ee53 / https://github.com/bitcoin/bitcoin/pull/15600
  to also use madvise(2) on FreeBSD to avoid sensitive data allocated
  with secure_allocator ending up in core files in addition to preventing
  it from going to the swap.

ACKs for top commit:
  sipa:
    ACK f85203097f if someone verifies this works as intended on *BSD.
  laanwj:
    ACK f85203097f
  practicalswift:
    Code-review ACK f85203097f assuming a reviewer with FreeBSD access verifies that the PR goal is achieved :)

Tree-SHA512: 2e6d4ab6a9fbe18732c8ba530eacc17f58128c97140758b80c905b5b838922a2bcaa5f9abc45ab69d5a1a2baa0cba322f006048b60a877228e089c7e64dadd2a
2020-05-04 16:31:07 +02:00
Wladimir J. van der Laan 23c926d859
Merge #18699: wallet: Avoid translating RPC errors
fa2cce4391 wallet: Remove trailing whitespace from potential translation strings (MarcoFalke)
fa59cc1c97 wallet: Report full error message in wallettool (MarcoFalke)
fae7776690 wallet: Avoid translating RPC errors when creating txs (MarcoFalke)
fae51a5c6f wallet: Avoid translating RPC errors when loading wallets (MarcoFalke)

Pull request description:

  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.

  Fixes #17072

ACKs for top commit:
  laanwj:
    ACK fa2cce4391, checked that no new translation messages are added compared to master.
  hebasto:
    ACK fa2cce4391

Tree-SHA512: c6a943ae9c3689ea3c48c20d26de6e4970de0257a1f1eec57a2bded67a4af9dcc5c45b2d64659d6fb4c4bc4d8103e28483ea3d14bb850df8db0ff9e8e5c77ee2
2020-05-04 16:29:22 +02:00
Wladimir J. van der Laan 42fd503819
Merge #18786: init: Remove boost from ThreadImport
faec3dc2ad init: Remove boost from ThreadImport (MarcoFalke)

Pull request description:

  Can be tested by calling `-reindex` or `-loadblock` and then pressing `CTRL`+`C`.

  Should print something like:

  ```
  ...
  2020-04-27T19:34:31Z [loadblk] Reindexing block file blk00005.dat...
  ^C2020-04-27T19:34:32Z [loadblk] Shutdown requested. Exit ThreadImport
  2020-04-27T19:34:32Z [qt-init] Interrupting HTTP server
  ...
  ```

ACKs for top commit:
  laanwj:
    Code review ACK faec3dc2ad
  hebasto:
    ACK faec3dc2ad, tested on Linux Mint 19.3 (x86_64) both `bitcoind` and `bitcoin-qt` binaries.

Tree-SHA512: e105af18d98296d82ec99f48e478cf44577e3c32f7e4b47617a7bc7cbf71d6becb92722f229a1be38d58ad29712704509ad9740d8ab8cd3104cf90057664b437
2020-05-04 16:06:42 +02:00
MarcoFalke 0a729b0e42
Merge #18783: tests: Add fuzzing harness for MessageSign, MessageVerify and other functions in util/message.h
38e49ded8b tests: Add fuzzing harness for MessageSign, MessageVerify and other functions in util/message.h (practicalswift)

Pull request description:

  Add fuzzing harness for `MessageSign`, `MessageVerify` and other functions in `util/message.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:
  vasild:
    utACK 38e49ded8b

Tree-SHA512: 4f83718365d9c7e772a4ccecb31817bf17117efae2bfaf6e9618ff17908def0c8b97b5fa2504d51ab38b2e6f82c046178dd751495cc37ab4779c0b1ac1a4d211
2020-05-04 09:02:21 -04:00
MarcoFalke 74a1152f25
Merge #18859: Remove CCoinsViewCache::GetValueIn(...)
b56607a89b Remove CCoinsViewCache::GetValueIn(...) (practicalswift)

Pull request description:

  Remove `CCoinsViewCache::GetValueIn(...)`.

  Fixes #18858.

  It seems like `GetValueIn` was added in #748 ("Pay-to-script-hash (OP_EVAL replacement)", merged in 2012) and the last use in validation code was removed in #8498 ("Near-Bugfix: Optimization: Minimize the number of times it is checked that no money...", merged in 2017).

  `CCoinsViewCache::GetValueIn(…)` performs money summation like this:

  ```c++
  CAmount CCoinsViewCache::GetValueIn(const CTransaction& tx) const
  {
      if (tx.IsCoinBase())
          return 0;

      CAmount nResult = 0;
      for (unsigned int i = 0; i < tx.vin.size(); i++)
          nResult += AccessCoin(tx.vin[i].prevout).out.nValue;

      return nResult;
  }
  ```

  Note that no check is done to make sure that the resulting `nResult` is such that it stays within the money bounds (`MoneyRange(nResult)`), or that the summation does not trigger a signed integer overflow.

  Proof of concept output:

  ```
  coins.cpp:243:17: runtime error: signed integer overflow: 9223200000000000000 + 2100000000000000 cannot be represented in type 'long'
  GetValueIn = -9221444073709551616
  ```

  Proof of concept code:

  ```c++
  CMutableTransaction mutable_transaction;
  mutable_transaction.vin.resize(4393);

  Coin coin;
  coin.out.nValue = MAX_MONEY;
  assert(MoneyRange(coin.out.nValue));

  CCoinsCacheEntry coins_cache_entry;
  coins_cache_entry.coin = coin;
  coins_cache_entry.flags = CCoinsCacheEntry::DIRTY;

  CCoinsView backend_coins_view;
  CCoinsViewCache coins_view_cache{&backend_coins_view};
  CCoinsMap coins_map;
  coins_map.emplace(COutPoint{}, std::move(coins_cache_entry));
  coins_view_cache.BatchWrite(coins_map, {});

  const CAmount total_value_in = coins_view_cache.GetValueIn(CTransaction{mutable_transaction});
  std::cout << "GetValueIn = " << total_value_in << std::endl;
  ```

ACKs for top commit:
  MarcoFalke:
    ACK b56607a89b
  promag:
    Code review ACK b56607a89b.
  jb55:
    ACK b56607a89b
  hebasto:
    ACK b56607a89b, I have not tested the code, but I have reviewed it and it looks OK, I agree it can be merged.

Tree-SHA512: 2c8402b5753ec96703d12c57c3eda8eccf999ed3519134a87faaf0838cfe44b94ef384296af2a524c06c8756c0245418d181af9083548e360905fac9d79215e6
2020-05-04 07:48:23 -04: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
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
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 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 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 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 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