Commit graph

30520 commits

Author SHA1 Message Date
MarcoFalke
5e3380b9f5
Merge bitcoin/bitcoin#22916: doc: add missing copyright header to getuniquepath.cpp
69a439b880 doc: add missing copyright header to getuniquepath.cpp (fanquake)

Pull request description:

  This was missed in #21052.

ACKs for top commit:
  hebasto:
    ACK 69a439b880, but a scripted-diff using `copyright_header.py insert` looks preferable.

Tree-SHA512: 1a75ffd93fa8e5e83821e1fe984353422740ebf9e203dc873debf4dffec0e23f55a1e31f806dd2d66087d3620f6dc447af69f9124f0bcc6676ef91ee35374832
2021-09-08 10:57:47 +02:00
fanquake
69a439b880
doc: add missing copyright header to getuniquepath.cpp
This was missed in #21052.
2021-09-08 16:28:21 +08:00
fanquake
7d7d5e8efd
Merge bitcoin/bitcoin#22879: addrman: Fix format string in deserialize error
fab0b55cf0 addrman: Fix format string in deserialize error (MarcoFalke)
facce4ca44 test: Remove useless overwrite (MarcoFalke)

Pull request description:

  The format string is evaluated differently on modern compilers (clang 10 and later, as well as gcc 10 and later).

  Work around the behaviour change in compilers by pinning the underlying type of the format arguments.

  Can be tested by observing a failing test when running against master compiled with clang 10 or gcc 10 (or later).

ACKs for top commit:
  jonatack:
    ACK fab0b55cf0 verified the test fails on master as expected only at line 61 (assertion fixed by the code change); the last two test additions pass as expected
  mzumsande:
    ACK fab0b55cf0

Tree-SHA512: 07462901435107f3bc79098fd7d06446bfe8fe065fffdd35adfcba8f1dd3c499575006557afe7bc74b79d690c5ef7b58e3e031e908161be5529cf237e3b30609
2021-09-08 15:17:42 +08:00
MarcoFalke
ecf580e40f
Merge bitcoin/bitcoin#22899: ci: Build and cache static Qt instead of downloading a pre-built one
3a68546fd0 ci: Build and cache static Qt instead of downloading a pre-built one (Hennadii Stepanov)

Pull request description:

  This PR makes the MSVC build CI task free of [pre-built static Qt binaries](https://github.com/sipsorcery/qt_win_binary/releases). It uses the approach which is documented in #22890.

  It takes about 13 minutes to build a static Qt dependency (for 8 CPUs):

  ![Screenshot from 2021-09-06 08-59-08](https://user-images.githubusercontent.com/32963518/132167857-bce49c74-f258-4468-b45b-75d0cf3c670c.png)

  with the maximum total time:

  ![Screenshot from 2021-09-06 08-59-26](https://user-images.githubusercontent.com/32963518/132167881-b84bd4de-38cc-4cb1-b9f7-35642cbea8cc.png)

  There is an additional benefit of this PR. It is no longer required to build a new static Qt package when a CI Windows image upgrades its building tools, and breaks the compatibility with the recent Qt package.

ACKs for top commit:
  sipsorcery:
    utACK 3a68546fd0.

Tree-SHA512: 2cf358ccecb26293b52c04158d6d3366ae6257cc3c04262e02234f7d7a03086885c67f0aad5702fcaa6f035fe4a09967a81245c561614875ecd2e90e2e00bbaa
2021-09-07 17:48:01 +02:00
Hennadii Stepanov
3a68546fd0
ci: Build and cache static Qt instead of downloading a pre-built one 2021-09-07 16:49:35 +03:00
MarcoFalke
e0ca5d0513
Merge bitcoin/bitcoin#22909: ci: Add missed comments and test_bitcoin.exe command line option
64015eb014 ci: Add missed comments and test_bitcoin.exe command line option (Hennadii Stepanov)

Pull request description:

  This PR is a #21551 follow up, and it:
  - adds missed comments, see https://github.com/bitcoin/bitcoin/pull/21551#discussion_r703342550
  - restores missed `-l test_suite` command line option for `test_bitcoin.exe`, see https://github.com/bitcoin/bitcoin/pull/21551#discussion_r703348955

ACKs for top commit:
  MarcoFalke:
    cr ACK 64015eb014

Tree-SHA512: ad1c91544da39a94f033bc55ae5fdaf5774475702edd026635389e68d20e2608cb599dd51f3c1412e0287beef073352e48d9ec005c94df38cfe4fe2d21a94fe3
2021-09-07 13:21:33 +02:00
Hennadii Stepanov
64015eb014
ci: Add missed comments and test_bitcoin.exe command line option 2021-09-07 13:31:28 +03:00
MarcoFalke
e7c6ed605c
Merge bitcoin/bitcoin#21551: ci: Move Windows MSVC build from AppVeyor to Cirrus
97292b1914 ci: Drop AppVeyor CI integration (Hennadii Stepanov)
1fb70793b2 ci: Add Windows task to Cirrus CI (Hennadii Stepanov)

Pull request description:

  This PR:
  - gets rid off unreliable AppVeyor CI
  - places all CI configs in one place
  - allows to enable functional tests in the future (using a persistent worker)

  | no populated `vcpkg` cache | populated `vcpkg` cache |
  |---|---|
  | ![Screenshot from 2021-09-02 15-47-04](https://user-images.githubusercontent.com/32963518/131846156-9367bffc-9093-40ca-98c3-15db74e24113.png) | ![Screenshot from 2021-09-02 14-06-26](https://user-images.githubusercontent.com/32963518/131833053-a501454d-eecf-463c-a3a4-b89d2a494058.png) |

  Currently, AppVeyor builds take about 44..48 minutes.

ACKs for top commit:
  sipsorcery:
    re-ACK 97292b1914.

Tree-SHA512: 3af50d9fd68eb12f39724810dacf948e4068573b5dfd0dbaeb05d19d4bd6f10bd9a432656dcc32b742684b438d31305eace85c602296d7a1bf84b2f1fcc06f02
2021-09-07 11:20:42 +02:00
MarcoFalke
9393666e78
Merge bitcoin/bitcoin#19833: wallet: Avoid locking cs_wallet recursively
5fabde6fad wallet: AddWalletDescriptor requires cs_wallet lock (João Barbosa)
32d036e8da wallet: GetLabelAddresses requires cs_wallet lock (João Barbosa)

Pull request description:

  This is another small change towards non recursive wallet lock.

ACKs for top commit:
  hebasto:
    ACK 5fabde6fad, I have reviewed the code and it looks OK, I agree it can be merged.

Tree-SHA512: 00506f0159c56854a171e58a451db8dd9b9f735039697b1cf2ca7f54de61fb51cc1e5eff42265233e041b4b1bfd29c2247496dc4456578e1a23c323bdec2901b
2021-09-07 10:34:44 +02:00
Hennadii Stepanov
503194d2ee
Merge bitcoin-core/gui#398: refactor: Pass WalletModel object to the WalletView constructor
d319c4dae9 qt, refactor: Replace WalletFrame::addWallet with WalletFrame::addView (Hennadii Stepanov)
92ddc02a16 qt, refactor: Declare getWalletModel with const and noexcept qualifiers (Hennadii Stepanov)
ca0e680bdc qt, refactor: Drop redundant checks of walletModel (Hennadii Stepanov)
404373bc6a qt, refactor: Pass WalletModel object to WalletView constructor (Hennadii Stepanov)

Pull request description:

  An instance of the `WalletView` class without the `walletModel` data member being set is invalid. So, it is better to set it in the constructor.

  Establishing one more `WalletView` class's invariant in constructor:
  - allows to drop all of checks of the`walletModel` in member functions
  - makes reasoning about the code that uses instances of the `WalletView` class  easier

  Possible follow ups could extend this approach to other classes, e.g., `OverviewPage`, `TransactionView`, `ReceiveCoinsDialog`, `SendCoinsDialog`, `AddressBookPage`.

ACKs for top commit:
  ShaMan239:
    Code review ACK d319c4dae9
  promag:
    Code review ACK d319c4dae9.
  jarolrod:
    ACK d319c4dae9

Tree-SHA512: b0c61f82811bb5aba2738067b53dc9ea4439230d547ce5c8fd85c480d8d70ea15f9942dbf13842383acbce467fba1ab4e132e37c56b654b46ba897301a41066e
2021-09-07 08:48:52 +03:00
Hennadii Stepanov
97292b1914
ci: Drop AppVeyor CI integration 2021-09-07 06:12:53 +03:00
Hennadii Stepanov
1fb70793b2
ci: Add Windows task to Cirrus CI 2021-09-07 06:12:51 +03:00
W. J. van der Laan
6718fbe90a
Merge bitcoin/bitcoin#22397: build: Fix macOS Apple Silicon build with miniupnpc and libnatpmp
2445df4eb3 build: Fix macOS Apple Silicon build with miniupnpc and libnatpmp (Hennadii Stepanov)

Pull request description:

  On master (7a49fdc581) the `configure` script does not pick up Homebrew's `miniupnpc` and `libnatpmp` packages on macOS Apple Silicon:

  ```
  % ./configure --with-miniupnpc
  ...
  checking for miniupnpc/miniupnpc.h... no
  checking for miniupnpc/upnpcommands.h... no
  checking for miniupnpc/upnperrors.h... no
  ...
  checking whether to build with support for UPnP... configure: error: "UPnP requested but cannot be built. Use --without-miniupnpc."
  ```

  ```
  % ./configure --with-natpmp
  ...
  checking for natpmp.h... no
  ...
  checking whether to build with support for NAT-PMP... configure: error: NAT-PMP requested but cannot be built. Use --without-natpmp
  ```

  The preferred Homebrew [prefix for Apple Silicon](https://docs.brew.sh/Installation) is `/opt/homebrew`. Therefore, if we do not use `pkg-config` to detect packages, we should set the `CPPFLAGS` and `LDFLAGS` variables for them explicitly.

ACKs for top commit:
  Zero-1729:
    re-tACK 2445df4eb3 (re-tested on an M1 Machine running macOS 11.4).
  jarolrod:
    re-ACK 2445df4eb3

Tree-SHA512: d623d26492f463812bf66ca519847ff4b23d517466b6c51c3caf3642a582d02e5f03ce57915742b29f01bf9bceb731a3978ef9a5fdc82e568bcb62548eda758a
2021-09-06 17:55:34 +02:00
MarcoFalke
92aad5303b
Merge bitcoin/bitcoin#21930: doc: update enumerator naming in developer notes
77f37f58ad doc: update enum naming in developer notes (Jon Atack)

Pull request description:

  Per our current doc, the general rule is we follow the C++ Core Guidelines, which for enumerator naming stipulate:

  https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Renum-caps
  ```
  Don’t use ALL_CAPS for enumerators
  Reason: Avoid clashes with macros.
  ```

  but our examples (and often, codebase) are contradictory to it (and perhaps to common usage), which can be confusing when a contributor needs to choose a style to use. This patch:

  - updates the enumerator examples to snake_case per CPP Core Guidelines
  - clarifies for contributors that in this project enumerators may be snake_case, PascalCase or ALL_CAPS, and to use what seems appropriate.

ACKs for top commit:
  practicalswift:
    cr ACK 77f37f58ad
  ryanofsky:
    ACK 77f37f58ad. I think this is good because it modernizes the example and clarifies current conventions.
  promag:
    ACK 77f37f58ad.

Tree-SHA512: 7facc607fe5e1abab0f635864340143f13c2e4bb074eb17eac7d829dcd0cf244c5c617fc49d35e8774e8af1fa1205eeebe0cca81f538a2a61f6a7ba200878bc6
2021-09-06 14:14:22 +02:00
MarcoFalke
a8fdfea77b
Merge bitcoin/bitcoin#22791: init: Fix asmap/addrman initialization order bug
724c497562 [fuzz] Add ConsumeAsmap() function (John Newbery)
5840476714 [addrman] Make m_asmap private (John Newbery)
f9002cb5db [net] Rename the copyStats arg from m_asmap to asmap (John Newbery)
f572f2b204 [addrman] Set m_asmap in CAddrMan initializer list (John Newbery)
593247872d [net] Remove CConnMan::SetAsmap() (John Newbery)
50fd77045e [init] Read/decode asmap before constructing addrman (John Newbery)

Pull request description:

  Commit 181a1207 introduced an initialization order bug: CAddrMan's m_asmap must be set before deserializing peers.dat.

  The first commit restores the correct initialization order. The remaining commits make `CAddrMan::m_asmap` usage safer:

  - don't reach into `CAddrMan`'s internal data from `CConnMan`
  - set `m_asmap` in the initializer list and make it const
  - make `m_asmap` private, and access it (as a reference to const) from a getter.

  This ensures that peers.dat deserialization must happen after setting m_asmap, since m_asmap is set during CAddrMan construction.

ACKs for top commit:
  mzumsande:
    Tested ACK 724c497562
  amitiuttarwar:
    code review but utACK 724c497562
  naumenkogs:
    utACK 724c497562
  vasild:
    ACK 724c497562
  MarcoFalke:
    review ACK 724c497562 👫

Tree-SHA512: 684a4cf9e3d4496c9997fb2bc4ec874809987055c157ec3fad1d2143b8223df52b5a0af787d028930b27388c8efeba0aeb2446cb35c337a5552ae76112ade726
2021-09-06 12:41:36 +02:00
MarcoFalke
2c6707be8b
Merge bitcoin/bitcoin#22848: MOVEONLY: Expose BanMapToJson / BanMapFromJson
6919c823cb MOVEONLY: Expose BanMapToJson / BanMapFromJson (Russell Yanofsky)

Pull request description:

  This PR is part of the [process separation project](https://github.com/bitcoin/bitcoin/projects/10).

  ---

  CSubNet serialization code that was removed in #22570 fa4e6afdae was needed by multiprocess code to share ban map between gui and node processes.

  Rather than adding it back, use suggestion from MarcoFalke https://github.com/bitcoin/bitcoin/pull/10102#discussion_r690922929 to use JSON serialization. This requires making BanMapToJson / BanMapFromJson functions public.

ACKs for top commit:
  promag:
    reACK 6919c823cb.

Tree-SHA512: ce909a61b7869d16cf2e9f91b643dd9d2604efc5777703d3b77a4c40cb0ccdd20396ba87b1ec85aade142e12ff9ea4c95c7155840354873579565471779f5a33
2021-09-06 10:35:30 +02:00
MarcoFalke
fa92777448
Merge bitcoin/bitcoin#22736: log, sync: change lock contention from preprocessor directive to log category
7e69873283 sync: remove DEBUG_LOCKCONTENTION preprocessor directives (Jon Atack)
9b08006bc5 log, sync: improve lock contention logging and add time duration (Jon Atack)
3f4c6b87f1 log, timer: add timing macro in usec LOG_TIME_MICROS_WITH_CATEGORY (Jon Atack)
b7a17444e0 log, sync: add LOCK logging category, apply it to lock contention (Jon Atack)

Pull request description:

  To enable lock contention logging, `DEBUG_LOCKCONTENTION` has to be defined at compilation. Once built, the logging is not limited to a category and is high frequency, verbose and in all-caps. With these factors combined, it seems likely to be rarely used.

  This patch:
  - adds a `lock` logging category
  - adds a timing macro in microseconds, `LOG_TIME_MICROS_WITH_CATEGORY`
  - updates `BCLog::LogMsg()` to omit irrelevant decimals for microseconds and skip unneeded code and math
  - improves the lock contention logging, drops the all-caps, and displays the duration in microseconds
  - removes the conditional compilation directives
  - allows lock contentions to be logged on startup with `-debug=lock` or at run time with `bitcoin-cli logging '["lock"]'`

  ```
  $ bitcoind -signet -debug=lock
  2021-09-01T12:40:01Z LockContention: cs_vNodes, net.cpp:1920 started
  2021-09-01T12:40:01Z LockContention: cs_vNodes, net.cpp:1920 completed (4μs)
  2021-09-01T12:40:01Z LockContention: cs_vNodes, net.cpp:1302 started
  2021-09-01T12:40:01Z LockContention: cs_vNodes, net.cpp:1302 completed (4μs)
  2021-09-01T12:40:02Z LockContention: cs_vNodes, net.cpp:2242 started
  2021-09-01T12:40:02Z LockContention: cs_vNodes, net.cpp:2242 completed (20μs)
  2021-09-01T12:43:04Z LockContention: ::cs_main, validation.cpp:4980 started
  2021-09-01T12:43:04Z LockContention: ::cs_main, validation.cpp:4980 completed (3μs)

  $ bitcoin-cli -signet logging
    "lock": true,

  $ bitcoin-cli -signet logging [] '["lock"]'
    "lock": false,

  $ bitcoin-cli -signet logging '["lock"]'
    "lock": true,
  ```

  I've tested this with Clang 13 and GCC 10.2.1, on Debian, with and without `--enable-debug`.

ACKs for top commit:
  hebasto:
    re-ACK 7e69873283, added a contention duration to the log message since my [previous](https://github.com/bitcoin/bitcoin/pull/22736#pullrequestreview-743764606) review.
  theStack:
    re-ACK 7e69873283 🔏 ⏲️

Tree-SHA512: c4b5eb88d3a2c051acaa842b3055ce30efde1f114f61da6e55fcaa27476c1c33a60bc419f7f5ccda532e1bdbe70815222ec2b2b6d9226f29c8e94e598aacfee7
2021-09-06 10:31:12 +02:00
MarcoFalke
e567dd53d1
Merge bitcoin/bitcoin#22841: ci: Fuzz with -ftrivial-auto-var-init=pattern
fa0a5fa744 ci: Fuzz with -ftrivial-auto-var-init=pattern (MarcoFalke)

Pull request description:

  This makes memory bugs deterministic. `-ftrivial-auto-var-init=pattern` is incompatible with other memory sanitizers (like valgrind and msan), but that is irrelevant here, because the address sanitizer in this fuzz CI config is already incompatible with them.

  `-ftrivial-auto-var-init=pattern` goes well with `-fsanitize=bool` and `-fsanitize=enum`, but those are already enabled via `-fsanitize=undefined`. See  https://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html#available-checks

ACKs for top commit:
  practicalswift:
    cr ACK fa0a5fa744

Tree-SHA512: ed6be953cd99eadb1ba245ba30170747eff66be54d2773c8d26a3a6aee0fdcd6967c596f4f4ab1d238de6a6526623dac5211f0ba77f1986639395d7921bdc19f
2021-09-06 09:59:50 +02:00
MarcoFalke
4bd56bc291
Merge bitcoin/bitcoin#22894: netinfo: clarify client and server versions in header
e952d7557e netinfo: clarify client and server versions in header (Jon Atack)

Pull request description:

  Clarify in -netinfo output that both the client and the server versions are provided.

  before
  ```
  Bitcoin Core v22.0.0rc3 - 70016/Satoshi:22.99.0/
  ```

  after
  ```
  Bitcoin Core client v22.0.0rc3 - server 70016/Satoshi:22.99.0/
  ```

  Closes #22873.

ACKs for top commit:
  benthecarman:
    utACK e952d7557e
  prayank23:
    ACK e952d7557e
  Zero-1729:
    tACK e952d7557e

Tree-SHA512: 3e817892d398aabacb1401fd5b1816c4d4f563b4f8cf1096bdb8b53f7c4ef82d4caee09f5c7724f1fe292f837434a332acefba735152ed24a238bb6f006df909
2021-09-06 09:16:32 +02:00
fanquake
78d39f8438
Merge bitcoin/bitcoin#22861: test: Update test README and lint script
fa0937de35 test: Rename bitcoin-util-test.py to util/test_runner.py (MarcoFalke)
fa050bbc0a test: Update test README and lint script (MarcoFalke)

Pull request description:

  * Remove unused `yq`
  * Update fuzzing docs

ACKs for top commit:
  Saviour1001:
    ACK <code>[fa0937d](fa0937de35)</code>
  practicalswift:
    cr ACK fa0937de35
  fanquake:
    ACK fa0937de35

Tree-SHA512: 6b148d838e1fcf219ab92e579948e34ea7ce8b4692a3d28bb2a51aaa34cbc7cdbd79e72ce787b485fdf524e5b3521b033692583602d4e379bd160e0e41d66e28
2021-09-06 09:53:41 +08:00
MarcoFalke
0ebd88fe0b
Merge bitcoin/bitcoin#22888: build: Update default platform toolset in msvc-autogen.py
d1267fdbb0 build: Update default platform toolset in msvc-autogen.py (Hennadii Stepanov)

Pull request description:

  The platform toolset was updated from v141 to v142 in #17364.

ACKs for top commit:
  sipsorcery:
    ACK d1267fdbb0.

Tree-SHA512: 390dc4876948af7f6b9f26eb1e64262f6386c2541a4647a6cb7abd8f1283c63ffbf5efba5e67a8075f0085a0557fc3f1b41e70f3ede0e8cef60d0fe2d6bf3be8
2021-09-05 18:05:02 +02:00
MarcoFalke
91e07cc50d
Merge bitcoin/bitcoin#22591: Util: error if settings json exists, but is unreadable
2b071265c3 error if settings.json exists, but is unreadable (Tyler Chambers)

Pull request description:

  If settings.json exists, but is unreadable, we should error instead of overwriting.

  Fixes #22571

ACKs for top commit:
  Zero-1729:
    tACK 2b071265c3
  ShaMan239:
    tACK 2b071265c3
  prayank23:
    tACK 2b071265c3
  ryanofsky:
    Code review ACK 2b071265c3. Thanks for the fix! Note that PR   https://github.com/bitcoin-core/gui/pull/379 will change the appearance of dialogs shown in screenshots above. So it could be interesting to test the two PRs together (but current testing seems more than sufficient)
  theStack:
    ACK 2b071265c3 📁

Tree-SHA512: 6f7f96ce8a13213d0335198a2245d127264495c877105058d1503252435915b332a6e55068ac21088f4c0c017d564689f4956213328d5bdee81d73711efc5511
2021-09-05 17:12:37 +02:00
Jon Atack
e952d7557e
netinfo: clarify client and server versions in header 2021-09-05 12:44:16 +02:00
MarcoFalke
fab0b55cf0
addrman: Fix format string in deserialize error
Also add a regression test.
2021-09-05 10:26:03 +02:00
fanquake
6490a3ef6c
Merge bitcoin/bitcoin#22859: Replace uses of boost::trim* with locale-independent alternatives (#18130 rebased)
696c76d660 tests: Add TrimString(...) tests (practicalswift)
4bf18b089e Replace use of boost::trim_right with locale-independent TrimString (Ben Woosley)
93551862a1 Replace use of boost::trim use with locale-independent TrimString (Ben Woosley)

Pull request description:

  This is [#18130 rebased](https://github.com/bitcoin/bitcoin/pull/18130#issuecomment-900158759).

  > `TrimString` is an existing alternative.

  > Note `TrimString` uses `" \f\n\r\t\v"` as the pattern, which is consistent with the default behavior of `std::isspace`. See: https://en.cppreference.com/w/cpp/string/byte/isspace

ACKs for top commit:
  jb55:
    utACK 696c76d660
  practicalswift:
    ACK 696c76d660
  jonatack:
    ACK 696c76d660
  theStack:
    Code-review ACK 696c76d660

Tree-SHA512: 6a70e3777602dfa65a60353e5c6874eb951e4a806844cd4bdaa4237cad980a4f61ec205defc05a29f9707776835975838f6cc635259c42adfe37ceb02ba9358d
2021-09-05 11:13:36 +08:00
Hennadii Stepanov
d1267fdbb0
build: Update default platform toolset in msvc-autogen.py
The platform toolset was updated from v141 to v142 in #17364.
2021-09-04 15:34:58 +03:00
fanquake
f4e12fd50c
Merge bitcoin/bitcoin#22278: Add LIFETIMEBOUND to CScript where needed
fa7e6c56f5 Add LIFETIMEBOUND to InitializeChainstate (MarcoFalke)
fa5c896724 Add LIFETIMEBOUND to CScript where needed (MarcoFalke)

Pull request description:

  Without this, stack-use-after-scope can only be detected at runtime with ASan or code review, both of which are expensive.

  Use `LIFETIMEBOUND` to turn this error into a compile warning.

  See https://releases.llvm.org/12.0.0/tools/clang/docs/AttributeReference.html#lifetimebound

  Example:

  ```cpp
  const CScript a{WITH_LOCK(::cs_main, return CScript{} << OP_0 << OP_1)};
  ```

  Before: (no warning)
  After:

  ```
  warning: returning reference to local temporary object [-Wreturn-stack-address]
      const CScript a{WITH_LOCK(::cs_main, return CScript{} << OP_0 << OP_1)};
                                                  ^~~~~~~~~
  ./sync.h:276:65: note: expanded from macro 'WITH_LOCK'
  #define WITH_LOCK(cs, code) [&]() -> decltype(auto) { LOCK(cs); code; }()
                                                                  ^~~~

ACKs for top commit:
  theuni:
    utACK fa7e6c56f5.
  jonatack:
    Light ACK fa7e6c56f5 debug build with clang 13, reproduced the example compiler warning in the pull description, and briefly looked at `clang::lifetimebound` support in earlier versions of clang; it is in clang 7 (https://releases.llvm.org/7.0.0/tools/clang/docs/AttributeReference.html#lifetimebound-clang-lifetimebound), did not see references to it in earlier docs

Tree-SHA512: e915acdc4532445205b7703fab61a5d682231ace78ecfb274cb8523ca2bddefd85828f50ac047cfb1afaff92a331f5f7b5a1472539f999e30f7cf8ac8c3222f3
2021-09-04 10:51:36 +08:00
João Barbosa
5fabde6fad wallet: AddWalletDescriptor requires cs_wallet lock
No change in behavior, the lock is already held at call sites.
2021-09-03 18:30:01 +01:00
João Barbosa
32d036e8da wallet: GetLabelAddresses requires cs_wallet lock
No change in behavior, the lock is already held at call sites.
2021-09-03 18:28:12 +01:00
MarcoFalke
5fb6701bdb
Merge bitcoin/bitcoin#22860: test: Always clear reject reason in IsStandard tx test
fa1b08eb14 test: Always clear reject reason in IsStandard tx test (MarcoFalke)

Pull request description:

  For some tests the reject reason wasn't cleared between runs and thus subsequent tests might (theoretically) fail to verify the correct reject reason.

ACKs for top commit:
  benthecarman:
    ACK fa1b08eb14
  theStack:
    Code-review ACK fa1b08eb14

Tree-SHA512: fcb727a690f92a4cf06127c302ba464f1e8cb997498e4f7fd9e210d193559b07e6efdb9d5c8a0bef3fe643bdfd5fedd431aaace20978dd49e56b8e770cb9f930
2021-09-03 17:49:09 +02:00
MarcoFalke
facce4ca44
test: Remove useless overwrite 2021-09-03 16:14:57 +02:00
MarcoFalke
fa7e6c56f5
Add LIFETIMEBOUND to InitializeChainstate 2021-09-03 13:42:59 +02:00
MarcoFalke
fa5c896724
Add LIFETIMEBOUND to CScript where needed 2021-09-03 13:42:03 +02:00
Russell Yanofsky
6919c823cb MOVEONLY: Expose BanMapToJson / BanMapFromJson
CSubNet serialization code that was removed in
fa4e6afdae was needed by multiprocess code
to share ban map between gui and node processes.

Rather than adding it back, use suggestion from MarcoFalke
<falke.marco@gmail.com>
https://github.com/bitcoin/bitcoin/pull/10102#discussion_r690922929 to
use JSON serialization. This requires making BanMapToJson /
BanMapFromJson functions public.
2021-09-03 05:21:58 -04:00
Samuel Dobson
629c4ab2e3
Merge bitcoin/bitcoin#22100: refactor: Clean up new wallet spend, receive files added #21207
b11a195ef4 refactor: Detach wallet transaction methods (followup for move-only) (Russell Yanofsky)

Pull request description:

  This makes `CWallet` and `CWalletTx` methods in `spend.cpp` and `receive.cpp` files into standalone functions.

  It's a followup to [#21207 MOVEONLY: CWallet transaction code out of wallet.cpp/.h](https://github.com/bitcoin/bitcoin/pull/21207), which moved code from `wallet.cpp` to new `spend.cpp` and `receive.cpp` files.

  There are no changes in behavior. This is just making methods into functions and removing circular dependencies created by #21207. There are no comment or documentation changes, either. Removed comments from `transaction.h` are just migrated to `spend.h`, `receive.h`, and `wallet.h`.

  ---

  This commit was split off from #21206 so there are a few earlier review comments there

ACKs for top commit:
  achow101:
    ACK b11a195ef4
  Sjors:
    utACK b11a195ef4
  meshcollider:
    light ACK b11a195ef4

Tree-SHA512: 75ce818d3f03b728b14b12e2d21bd20b7be73978601989cb37ff98254393300d1bb7823281449cd3d9e40756d67d42bd9a46bbdafd2e8baa95aaf2cb1c84549f
2021-09-03 21:21:58 +12:00
Hennadii Stepanov
d319c4dae9
qt, refactor: Replace WalletFrame::addWallet with WalletFrame::addView
No need to pass an instance of the WalletModel class to this method.

Co-authored-by: João Barbosa <joao.paulo.barbosa@gmail.com>
2021-09-03 00:27:14 +03:00
MarcoFalke
dd097c42df
Merge bitcoin/bitcoin#18448: rpc: fix/add missing RPCExamples for "Util" RPCs
ea98d9c2ef rpc: fix/add missing RPCExamples for "Util" RPCs (Sebastian Falbesoner)

Pull request description:

  Similar to https://github.com/bitcoin/bitcoin/pull/18398, this PR gives the RPCExamples in the RPC category "Util" (that currently contains `createmultisig`, `deriveaddresses`, `estimatesmartfee`, `getdescriptorinfo`, `signmessagewithprivkey`, `validateaddress`, `verifymessage`) some love by fixing one broken and adding three missing examples:
  - fixed `HelpExampleRpc` for `createmultisig` (disturbing escape characters and quotation marks)
  - added missing `HelpExampleRpc` for
      -  `deriveaddresses` (also put descriptor in a new string constant)
      - `estimatesmartfee`
      - `getdescriptorinfo` (also put descriptor in a new string constant)

  Output for `createmultisig` example on the master branch:
  ```
  $ curl --user __cookie__ --data-binary '{"jsonrpc": "1.0", "id": "curltest", "method": "createmultisig", "params": [2, "[\"03789ed0bb717d88f7d321a368d905e7430207ebbd82bd342cf11ae157a7ace5fd\",\"03dbc6764b8884a92e871274b87583e6d5c2a58819473e17e107ef3f6aa5a61626\"]"]}' -H 'content-type: text/plain;' http://127.0.0.1:8332/
  Enter host password for user '__cookie__':
  {"result":null,"error":{"code":-1,"message":"JSON value is not an array as expected"},"id":"curltest"}
  ```

  Output for `createmultisig` example on the PR branch:
  ```
  $ curl --user __cookie__ --data-binary '{"jsonrpc": "1.0", "id": "curltest", "method": "createmultisig", "params": [2, ["03789ed0bb717d88f7d321a368d905e7430207ebbd82bd342cf11ae157a7ace5fd","03dbc6764b8884a92e871274b87583e6d5c2a58819473e17e107ef3f6aa5a61626"]]}' -H 'content-type: text/plain;' http://127.0.0.1:8332/
  Enter host password for user '__cookie__':
  {"result":{"address":"3QsFXpFJf2ZY6GLWVoNFFd2xSDwdS713qX","redeemScript":"522103789ed0bb717d88f7d321a368d905e7430207ebbd82bd342cf11ae157a7ace5fd2103dbc6764b8884a92e871274b87583e6d5c2a58819473e17e107ef3f6aa5a6162652ae","descriptor":"sh(multi(2,03789ed0bb717d88f7d321a368d905e7430207ebbd82bd342cf11ae157a7ace5fd,03dbc6764b8884a92e871274b87583e6d5c2a58819473e17e107ef3f6aa5a61626))#4djp057k"},"error":null,"id":"curltest"}
  ```

ACKs for top commit:
  jonatack:
    ACK ea98d9c2ef looked at the code, rebased to master, ran the helps, did not try running the added json-rpc examples

Tree-SHA512: d6ecb6da66f19517065453357d210102e2cc9f1f8037aeb6a9177ff036d0c21773dddf5e0acdbc71edbbde3026e4d1e7ce7c0935cd3e023c60f34e1b173b3299
2021-09-02 18:04:54 +02:00
W. J. van der Laan
a70768d159
Merge bitcoin/bitcoin#22501: netinfo: display addr_{processed, rate_limited, relay_enabled} and relaytxes data
218862a018 Display peers in -netinfo that we don't relay addresses to (Jon Atack)
3834e23b25 Display peers in -netinfo that request we not relay transactions (Jon Atack)
0a9ee3a2c7 Simplify a few conditionals in -netinfo (Jon Atack)
5eeea8e257 Add addr_processed and addr_rate_limited stats to -netinfo (Jon Atack)

Pull request description:

  Update CLI -netinfo to display the getpeerinfo `addr_processed`, `addr_rate_limited`, `addr_relay_enabled` and `relaytxes` data with auto-adjusting column widths.

  ```
  $ ./src/bitcoin-cli -netinfo help

    txn      Time since last novel transaction received from the peer and accepted into our mempool, in minutes
             "*" - the peer requested we not relay transactions to it (relaytxes is false)

    addrp    Total number of addresses processed, excluding those dropped due to rate limiting
             "." - we do not relay addresses to this peer (addr_relay_enabled is false)

    addrl    Total number of addresses dropped due to rate limiting
  ```

  ![Screenshot from 2021-08-22 14-31-40](https://user-images.githubusercontent.com/2415484/130355514-f6fd4f21-79d6-463b-9791-de01ebef20b1.png)

ACKs for top commit:
  0xB10C:
    Code review and tested ACK 218862a018
  Zero-1729:
    re-tACK 218862a018
  vasild:
    ACK 218862a018
  jarolrod:
    tACK 218862a018

Tree-SHA512: bb9da4bdd71859b234f6e4c2c46257a57ef0d0e0b363d2b8fded128bcaa28132f64a0a4651c622e1de1e3b7c05c7587a4369e9e79799895884fda9745c63409d
2021-09-02 17:12:48 +02:00
fanquake
b997dd211e
Merge bitcoin/bitcoin#22809: test: Check that non-signaling BIP125 tx can be replaced via parent
fa2e9de59f test: Check that non-signaling BIP125 tx can be replaced via parent (MarcoFalke)

Pull request description:

  While `optout_child_tx` in the `test_no_inherited_signaling` test is reported as "bip125-replaceable", it is not *directly* replaceable. For example by bumping the fee of `optout_child_tx`. However, it is still replaceable *indirectly* via it's BIP-125 signalling parent.

  Clarify this by extending the test.

ACKs for top commit:
  mjdietzx:
    Tested ACK fa2e9de59f
  josibake:
    ACK fa2e9de59f

Tree-SHA512: b3608beae743dcb6152df4d2cfe1c0af6b4404ba3837f73e1d1431bd7c637f0c7fab0379aaab2218d5cd63e71070a079c0595ec031056058e8d3c933c2bae0a9
2021-09-02 17:26:53 +08:00
MarcoFalke
fa0937de35
test: Rename bitcoin-util-test.py to util/test_runner.py
To normalize the name of all three test runners (fuzz, functional, util).
2021-09-02 10:43:19 +02:00
MarcoFalke
fa050bbc0a
test: Update test README and lint script 2021-09-02 10:26:35 +02:00
MarcoFalke
245462b66c
Merge bitcoin/bitcoin#22418: release: Remove gitian
ab9c34237a release: remove gitian (fanquake)

Pull request description:

  Note that this doesn't yet touch any glibc back compat related code.

ACKs for top commit:
  laanwj:
    Code review ACK ab9c34237a

Tree-SHA512: 8e2fe3ec1097f54bb11ab9136b43818d90eab5dbb0a663ad6a552966ada4bdb49cc12ff4e66f0ec0ec5400bda5c81f3a3ce70a9ebb6fe1e0db612da9f00a51a7
2021-09-02 10:09:53 +02:00
fanquake
d6034e4122
Merge bitcoin/bitcoin#22843: msvc: update bitcoin_config.h defines
ae4ad1a125 msvc: update bitcoin_config.h defines (fanquake)

Pull request description:

  While not many of these are used, some, like `HAVE_DLLEXPORT_ATTRIBUTE`, are used when creating libbitcoinconsensus.

  Given this file is static, also just remove everything that is commented out.

ACKs for top commit:
  sipsorcery:
    tACK ae4ad1a125.

Tree-SHA512: fe9bb3381f9219617092fde57c7db25e8f9a5a2ecb9a77a8d61d9ae74ffecd93024aaa17fb6685cda74d20efc5975feb12f9817bceb895f91b79fc5cbee895ca
2021-09-02 15:44:01 +08:00
MarcoFalke
fa1b08eb14
test: Always clear reject reason in IsStandard tx test 2021-09-02 09:40:07 +02:00
MarcoFalke
6bf706a056
Merge bitcoin/bitcoin#22846: policy: unit test Segwit dust thresholds
97cea1a93a policy: unit test Segwit dust thresholds (Antoine Poinsot)

Pull request description:

  This is the unit testing part of #22779, hence without the threshold modification.

ACKs for top commit:
  MarcoFalke:
    cr ACK 97cea1a93a
  benthecarman:
    crACK 97cea1a93a

Tree-SHA512: 96fb194709ae44364455eb920ed3ecff2e11e5327e0a72b9eeec9f9445894302099a0c4ffb1e0c8d4d523c0bfe06c57f1ebb0c03cf3389a73f518e3b174c45aa
2021-09-02 09:03:11 +02:00
practicalswift
696c76d660
tests: Add TrimString(...) tests 2021-09-02 13:20:08 +08:00
Ben Woosley
4bf18b089e
Replace use of boost::trim_right with locale-independent TrimString
Note the only use of readStdin is fed to DecodeHexTx, which fails in
IsHex on non-hex characters as recorded in p_util_hexdigit.
2021-09-02 13:19:42 +08:00
Ben Woosley
93551862a1
Replace use of boost::trim use with locale-independent TrimString 2021-09-02 13:17:46 +08:00
fanquake
77e77e8544
Merge bitcoin/bitcoin#22631: doc: Add packages that provide Qt Wayland plugin for Linux
5559cf1460 doc: Add packages that provide Qt Wayland plugin for Linux (Hennadii Stepanov)

Pull request description:

  When building on Linux using system packages (without depends) the support of Wayland protocol for modern desktop environments (e.g., GNOME, KDE Plasma) depends on the presence of the installed Qt Wayland plugin which is loaded dynamically at the GUI startup.

  1. On Debian/Ubuntu, the [`qtwayland5`](https://packages.ubuntu.com/focal/qtwayland5) package is required (also see this [patch](https://codereview.qt-project.org/c/qt/qtbase/+/231227), and this [doc](https://wiki.debian.org/Wayland#Qt_.28supported_since_5.29)):
  - with `qtwayland5` installed:
  ```
  $ QT_QPA_PLATFORM="wayland;xcb" ./src/qt/bitcoin-qt -printtoconsole
  Warning: Ignoring XDG_SESSION_TYPE=wayland on Gnome. Use QT_QPA_PLATFORM=wayland to run on Wayland anyway.
  2021-08-05T09:51:31Z Bitcoin Core version v22.99.0-c4b42aa4ffa1 (release build)
  2021-08-05T09:51:31Z Qt 5.11.3 (dynamic), plugin=wayland (dynamic)
  2021-08-05T09:51:31Z No static plugins.
  2021-08-05T09:51:31Z Style: fusion / QFusionStyle
  2021-08-05T09:51:31Z System: Debian GNU/Linux 10 (buster), x86_64-little_endian-lp64
  ...
  ```

  - without `qtwayland5`:
  ```
  $ QT_QPA_PLATFORM="wayland;xcb" ./src/qt/bitcoin-qt -printtoconsole
  Warning: Ignoring XDG_SESSION_TYPE=wayland on Gnome. Use QT_QPA_PLATFORM=wayland to run on Wayland anyway.
  qt.qpa.plugin: Could not find the Qt platform plugin "wayland" in ""
  2021-08-05T09:48:55Z Bitcoin Core version v22.99.0-c4b42aa4ffa1 (release build)
  2021-08-05T09:48:55Z Qt 5.11.3 (dynamic), plugin=xcb (dynamic)
  2021-08-05T09:48:55Z No static plugins.
  2021-08-05T09:48:55Z Style: fusion / QFusionStyle
  2021-08-05T09:48:55Z System: Debian GNU/Linux 10 (buster), x86_64-little_endian-lp64
  2021-08-05T09:48:55Z Screen: XWAYLAND0 1920x1200, pixel ratio=1.0
  ...
  ```

  2. On Fedora, the [`qt5-qtwayland`](https://fedora.pkgs.org/34/fedora-x86_64/qt5-qtwayland-5.15.2-4.fc34.x86_64.rpm.html) package is required:
  - with `qt5-qtwayland` installed:
  ```
  $ ./src/qt/bitcoin-qt -printtoconsole
  QSocketNotifier: Can only be used with threads started with QThread
  2021-08-05T08:41:03Z Bitcoin Core version v22.99.0-c4b42aa4ffa1 (release build)
  2021-08-05T08:41:03Z Qt 5.15.2 (dynamic), plugin=wayland (dynamic)
  2021-08-05T08:41:03Z No static plugins.
  2021-08-05T08:41:03Z Style: fusion / QFusionStyle
  2021-08-05T08:41:03Z System: Fedora 34 (Workstation Edition), x86_64-little_endian-lp64
  ...
  ```
  - without `qt5-qtwayland`:
  ```
  $ ./src/qt/bitcoin-qt -printtoconsole
  qt.qpa.plugin: Could not find the Qt platform plugin "wayland" in ""
  2021-08-05T07:50:41Z Bitcoin Core version v22.99.0-c4b42aa4ffa1 (release build)
  2021-08-05T07:50:41Z Qt 5.15.2 (dynamic), plugin=xcb (dynamic)
  2021-08-05T07:50:41Z No static plugins.
  2021-08-05T07:50:41Z Style: fusion / QFusionStyle
  2021-08-05T07:50:41Z System: Fedora 34 (Workstation Edition), x86_64-little_endian-lp64
  2021-08-05T07:50:41Z Screen: XWAYLAND0 1920x1200, pixel ratio=1.0
  ...
  ```

ACKs for top commit:
  fanquake:
    ACK 5559cf1460 - I don't think there's any harm to point this out in our Linux build docs. It's not changing our binaries or dependencies in any way.

Tree-SHA512: e26856586b29540b55c12905a091408e95ce59ea2c952520086b41138c955fba1b78e95e868f75205af07c6eccae51644177f7165d837ae058aaf0c0abf3ccf5
2021-09-02 11:25:42 +08:00
fanquake
ae4ad1a125
msvc: update bitcoin_config.h defines
While not many of these are used, some, like HAVE_DLLEXPORT_ATTRIBUTE, are used
when creating libbitcoinconsensus.

Given this file is static, also just remove everything that is commented out.
2021-09-02 11:14:18 +08:00