Commit Graph

22764 Commits

Author SHA1 Message Date
MarcoFalke
fa52d3bc12 Merge #13712: wallet: Fix non-determinism in ParseHDKeypath(...). Avoid using an uninitialized variable in path calculation.
27ee53c1ae wallet: Add error handling. Check return value of ParseUInt32(...) in ParseHDKeypath(...). (practicalswift)
7223263899 wallet: Add tests for ParseHDKeypath(...) (practicalswift)

Pull request description:

  Add error handling. Check return value of `ParseUInt32(...)` in `ParseHDKeypath(...)`.

  `ParseUInt32(...)` returns `false` if the entire string could not be parsed or when an overflow or underflow occurred. In such case the uninitialized variable `number` would be used in the calculation of `path` (prior to this commit).

  An example key path triggering this is `m/0/4294967296`:

  ```
    ParseHDKeypath("m/0/4294967296", keypath);
  ```

  `4294967296` is `1` + `0xFFFFFFFF` (`uint32_t` max: `4294967295`).

  Introduced in a4b06fb42eb0ad94e562ca839391b57e69285136 which was merged into `master` 14 hours ago as part of #13557 ("BIP 174 PSBT Serializations and RPCs").

Tree-SHA512: e5ff423f67c18d82c1231bde6343587a453e793c32004d93dc9b61be6d9372b57a6b2c9978d9eb1000d6cc82fd180f2486013f928dca737fb92daad22c16e467
2023-02-04 10:02:37 -06:00
PastaPastaPasta
6757616559
Merge pull request #5168 from kittywhiskers/p2pbp
backport: merge bitcoin#19053, #19174, #19293, #19704, #19472, #19583 (net_processing.{cpp|h} refactoring)
2023-02-03 16:24:40 -06:00
Kittywhiskers Van Gogh
383f902abf merge bitcoin#19583: clean up Misbehaving() 2023-02-03 15:25:38 -06:00
Kittywhiskers Van Gogh
24546f93e5 merge bitcoin#19472: Reduce cs_main scope in MaybeDiscourageAndDisconnect() 2023-02-03 15:25:38 -06:00
Kittywhiskers Van Gogh
4dd3ec2cb9 refactor: pass CNode reference as const when possible 2023-02-03 15:25:38 -06:00
Kittywhiskers Van Gogh
99f6c314c0 refactor: rearrange argument order to be more consistent 2023-02-03 15:25:38 -06:00
Kittywhiskers Van Gogh
a4e0327c29 refactor: pass CNode by reference for ProcessMessage functions 2023-02-03 15:25:38 -06:00
Kittywhiskers Van Gogh
97d485159b refactor: rename ProcessSporkMessages to match other functions 2023-02-03 15:25:38 -06:00
Kittywhiskers Van Gogh
260ef0a811 merge bitcoin#19704: move ProcessMessage() to PeerLogicValidation 2023-02-03 15:25:38 -06:00
Kittywhiskers Van Gogh
af0dd17ca6 merge bitcoin#19293: Avoid redundant and confusing FAILED log 2023-02-03 15:25:38 -06:00
Kittywhiskers Van Gogh
0b1a30afc1 merge bitcoin#19174: replace CConnman pointers by references in net_processing.cpp 2023-02-03 15:25:38 -06:00
Kittywhiskers Van Gogh
0385df1bc7 merge bitcoin#19053: replace CNode pointers by references within net_processing.{h,cpp} 2023-02-03 15:25:38 -06:00
Konstantin Akimov
90d888dc31
refactor: tidy up implementation of special tx manager (#5172)
## Issue being fixed or feature implemented
It is preparation work for #5026 (moved out a refactoring as a new pull
request)

## What was done?
 - tidy up header dependencies
 - move general code to proper headers


## How Has This Been Tested?
Run unit/functional tests

## Breaking Changes
No breaking changes

## Checklist:
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas

**For repository code-owners and collaborators only**
- [x] I have assigned this pull request to a milestone
2023-02-02 23:22:51 -06:00
Konstantin Akimov
9a03334893
fix: keeping reference to local variable in Coin Join unit tests (#5174)
It become a visible UB (crash) after backport bitcoin#19848

## What was done?
Increased life-term for object under reference

## How Has This Been Tested?
Run unit/functional tests with and without bitcoin#19848

## Breaking Changes
No breaking changes

## Checklist:
- [x] I have performed a self-review of my own code
- [x] I have assigned this pull request to a milestone
2023-01-31 11:27:01 -06:00
Konstantin Akimov
967d413db3
fix: governance should not lock mempool mutex even call GetTransaction (#5175)
## Issue being fixed or feature implemented
Previously noticed, that GetTransaction is called deep inside governance
objects and it is true indeed.
But so far it is used `nullptr` as a mempol object instead any real
mempool in GetTransaction and no usage of a global mempool or any other
mempool, this locks are useless.

This changes are important for mempool globalization.

## What was done?
Removed LOCK for ::mempool.cs in governance's code

## How Has This Been Tested?
Run unit/functional tests. Also done deglobalization of mempool to
validate that governance indeed doesn't use global mempool implicit in
#5169

## Breaking Changes
No breaking changes

## Checklist:
- [x] I have performed a self-review of my own code
- [x] I have assigned this pull request to a milestone
2023-01-31 11:08:25 -06:00
PastaPastaPasta
aa3ecbf6af
Merge pull request #5171 from PastaPastaPasta/develop-trivial-2023-01-23
backport: trivial backports 2023 01 23
2023-01-31 11:07:55 -06:00
MarcoFalke
046eb910a1
Merge #20759: doc: [test] Remove outdated comment in fuzz runner
fa511042b0bbec02016761bcd0d30f57e0386550 doc: [test] Remove outdated comment in fuzz runner (MarcoFalke)

Pull request description:

  All folders are soft-created with `os.makedirs`

ACKs for top commit:
  RiccardoMasutti:
    ACK fa51104

Tree-SHA512: 4051688946a205a981bbb005300fe3263495ead26591042b38ae44f4297c7689a613b560052fb5405a62054734d2599cfb0554a37c7b7369fb3a3636743d04a8
2023-01-23 12:22:32 -06:00
MarcoFalke
83a47b79ff
Merge #20748: test: Add race:SendZmqMessage tsan suppression
fa957f8dc9990e4479e4d2af46a63ceae89cd39b test: Add race:SendZmqMessage tsan suppression (MarcoFalke)

Pull request description:

  Add suppression for `race:SendZmqMessage`, which isn't covered by the existing `zmq::*` suppression

  Fixes #20618

ACKs for top commit:
  hebasto:
    re-ACK fa957f8dc9990e4479e4d2af46a63ceae89cd39b, as my previous comment is not directly related to this pull changes.

Tree-SHA512: 8642a8b79bbfa4bee89042b66e528f27fd78c5e84a33023df440662e9114e31445fd7b04940f44b11fa4ab7438d346385a21816289c818cce9958a9b16730452
2023-01-23 12:22:32 -06:00
MarcoFalke
c6826bd02e
Merge #20199: wallet: ignore (but warn) on duplicate -wallet parameters
58cfbc38e040925b51cb8d35d23b50e9cf06fb2a Ignoring (but warn) on duplicate -wallet parameters (Jonas Schnelli)

Pull request description:

  I expect that there are many users with load on startup wallet definitions in `bitcoin.conf` or via startup CLI argument.
  With the new `settings.json` r/w configuration file, users unloading and loading a wallet through the GUI or via the RPC calls might end up with a duplicate `-wallet` entry (one that still remains in bitcoin.conf or CLI) plus the new duplication in `settings.json` due to the unload/load.

  Steps to reproduce
  * create wallet (if via RPC set `load_on_startup` or unloadwallet/loadwallet then set `load_on_startup`).
  * stop bitcoin
  * start bitcoind again with same `--wallet=mywallet`

  I guess it is acceptable to skip duplicates.

ACKs for top commit:
  achow101:
    Tested ACK 58cfbc38e040925b51cb8d35d23b50e9cf06fb2a
  meshcollider:
    Code review ACK 58cfbc38e040925b51cb8d35d23b50e9cf06fb2a
  ryanofsky:
    Code review ACK 58cfbc38e040925b51cb8d35d23b50e9cf06fb2a. Changes since previous review: rebased, tweaked warning message, squashed/fixed test

Tree-SHA512: f94e5a999bdd7dc291f0bc142911b0a8033929350d6f6a35b58c4a06a3c8f83147be0f0c402d4e946dedbbcc85b7e023b672c731b6d7a8984d4780017c961cfb
2023-01-23 12:22:32 -06:00
fanquake
8c5fc766e3
Merge #20214: test: Fix rpc_net intermittent issue
fa5f46600fb98f1b35346bedc1a66c9019d01114 test: Fix rpc_net intermittent issue (MarcoFalke)

Pull request description:

  Without the sync, the nodes might generate blocks at the same height and thus never be able to sync

ACKs for top commit:
  practicalswift:
    ACK fa5f46600fb98f1b35346bedc1a66c9019d01114: patch looks correct

Tree-SHA512: 21255795c2121c71fc620beb766855e57c7af94a668331d1b625665e22eb4b485a2b5c3ad2bb9a7042744f3c3e49c71251bcec41ba25bca03fd54aae32968a3a
2023-01-23 12:22:32 -06:00
fanquake
2434142e15
Merge #20055: rpc: Set HTTP Content-Type in bitcoin-cli
7eab781a145a35d0373c4ab4d237a82b4919e88d rpc: Set HTTP Content-Type in bitcoin-cli (Wladimir J. van der Laan)

Pull request description:

  We don't set any `Content-Type` in the client. It is more consistent with our other JSON-RPC use to set it to `application/json`.

  Note that our server doesn't enforce content types, so it doesn't make a difference in practice. But it is fairly strange HTTP behavior to not set it at all for a POST request.

  This came up in #18950.

ACKs for top commit:
  promag:
    ACK 7eab781a145a35d0373c4ab4d237a82b4919e88d.
  jonatack:
    Tested ACK 7eab781a145a35d0373c4ab4d237a82b4919e88d
  practicalswift:
    ACK 7eab781a145a35d0373c4ab4d237a82b4919e88d: patch looks correct
  fanquake:
    ACK 7eab781a145a35d0373c4ab4d237a82b4919e88d - Looks fine to me.

Tree-SHA512: a9fa155324d0f7bff955585a336ead6bb60b721039f424521a435e4bb0fad3f4532e5cc7b7a9acc4e93585e8d3db3082c010138810f22c0e92b8f749b86ef653
2023-01-23 12:22:31 -06:00
Samuel Dobson
d90567b719
Merge #19919: bugfix: make LoadWallet assigns status always
8b39a875581bed1c2f40a7d9616bdb7cc642bf59 bugfix: make LoadWallet assigns status always (Akio Nakamura)

Pull request description:

  In my enviroment, ```test/functional/wallet_multiwallet.py``` failed in line 237 for master( 147d50d63 ).
  It got an expected rpc-error-message, but error code was not (-4) but (-18).

  This is because that although loadwallet() in rpcwallet.cpp assumes LoadWallet() always assign some value to the 'status', but LoadWallet() does not do so in some situation.

  This PR intends to fix above and prevends loadwallet() returns ambiguous error code.

ACKs for top commit:
  hebasto:
    re-ACK 8b39a875581bed1c2f40a7d9616bdb7cc642bf59, that is the same as 1728059730abef04f3fa84de0b6e20044be7a9d6.
  ryanofsky:
    Code review ACK 8b39a875581bed1c2f40a7d9616bdb7cc642bf59 (same as previous)
  meshcollider:
    utACK 8b39a875581bed1c2f40a7d9616bdb7cc642bf59

Tree-SHA512: a75d8240f60325bfdb69a07d392269fec97de743f38fe108371eb63a0aba5d8ce3cc484ecc69e81febf8040f5ab64f3a9450b98f8e07a0c17803784bb6f342bf
2023-01-23 12:22:31 -06:00
MarcoFalke
9ada13f974
Merge #19508: Work around memory-aliasing in descriptor ParsePubkey
fa2ae0ac8d43086430a29c73940ad6b1cd129e96 span: Add Span::empty() and use it in script/descriptor (MarcoFalke)
fa8a99258947a9ee3749fa472180542920cd471c Work around memory-aliasing in descriptor ParsePubkey (MarcoFalke)

Pull request description:

  While this is not undefined behaviour, the memory aliasing trick is confusing when reading the code. Having `a.size()==0` and then access `a[0]` works in this particular case, but should probably be avoided to harden the code for the future.

ACKs for top commit:
  theStack:
    re-ACK fa2ae0ac8d
  elichai:
    ACK fa2ae0ac8d43086430a29c73940ad6b1cd129e96
  jonatack:
    ACK fa2ae0ac8d43086430a29c73940ad6b1cd129e96

Tree-SHA512: 0ec7b09eef45504973a195923cdf1aa8522117c8e2f69b453e5ce9aa8a7e327c71138518022c32d05133dc99cb861101ed0f60fa891814ee3e9dab3a6fa61a84
2023-01-23 12:22:31 -06:00
fanquake
931e2ad0cd
Merge #19555: rpc: deduplicate WriteHDKeypath() used in decodepsbt
55057ffc51697daafac1224664d5e3258ae0b116 rpc: deduplicate WriteHDKeypath() used in decodepsbt (Sebastian Falbesoner)

Pull request description:

  The functionality is already provided in the BIP32 utility library `util/bip32.h` with the exact same name and function signature.

ACKs for top commit:
  achow101:
    ACK 55057ffc51697daafac1224664d5e3258ae0b116
  instagibbs:
    utACK 55057ffc51
  jonatack:
    ACK 55057ffc51697daafac1224664d5e3258ae0b116

Tree-SHA512: 074c1a71ffb32908926bf07f0c5428a46309f6e0d21e7c20b1008197c820b97776a441736d0b6fd8ab0c0852522a0b5a5ddb26a1e4a1100ca02aabc65a07a018
2023-01-23 12:22:31 -06:00
Wladimir J. van der Laan
ae38bd6a52
Merge #19023: test: Fix intermittent ETIMEDOUT on FreeBSD
fab908f18a080912a0e833f12cccc27f27662e3b test: Fix intermittent ETIMEDOUT on FreeBSD (MarcoFalke)

Pull request description:

  Example backtrace: https://cirrus-ci.com/task/5307019047469056?command=functional_test#L1059

ACKs for top commit:
  laanwj:
    ACK fab908f18a080912a0e833f12cccc27f27662e3b

Tree-SHA512: 06e383e9993e083d6da4c546dde8811ace02559ddbb1c24e307938307763b3abe630699054e7067cf4aea993c82865e259b77b4bee518666a03d26e0f81c0656
2023-01-23 12:22:30 -06:00
MarcoFalke
405ab79e29
Merge #18879: valgrind: remove outdated suppressions
d7120f7f78cda5ed1ab91f83e9b546de68dbee47 valgrind : remove duplicate BCLog::Logger suppression (fanquake)
708e3c7e85a666d5b8da8638a819c0f3973fcca4 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 d7120f7f78cda5ed1ab91f83e9b546de68dbee47
  practicalswift:
    ACK d7120f7f78cda5ed1ab91f83e9b546de68dbee47 -- patch looks correct and valgrind Travis job is happy

Tree-SHA512: 45f5b9fa64bf83cada3cd9ad33c245f660376d5b29f51a2531d83133940090df945f5ef26c5847d6ec024ffab9528d55573c5cf9ca5e73795f9abfc971b3d29b
2023-01-23 12:22:30 -06:00
MarcoFalke
fa37e16dab
Merge #18510: fuzz: Add CScriptNum::getint coverage
faa64af960b64b522bb088e836c9d8cd6254c6c8 fuzz: Add CScriptNum::getint coverage (MarcoFalke)

Pull request description:

  Add coverage for

  * https://marcofalke.github.io/btc_cov/fuzz.coverage/src/script/script.h.gcov.html#311
  * https://marcofalke.github.io/btc_cov/fuzz.coverage/src/script/script.h.gcov.html#511

ACKs for top commit:
  practicalswift:
    ACK faa64af960b64b522bb088e836c9d8cd6254c6c8 -- more fuzzing coverage is better than less fuzzing coverage :)

Tree-SHA512: 1a66a2edc3740e8c286049f6c27458c59c45b01052e51684eec0e1be63ffcee94b4ba3d41d88ad715ceb3e4754fd997cf03899085982454905e86d0553d58199
2023-01-23 12:22:30 -06:00
MarcoFalke
35c023759b
Merge #18459: rpc: remove unused getbalances() code
6e0d82c55bf4a9aae98c47b7cd00b2828b5dd0ee rpc: remove unused getbalances() code (Jon Atack)

Pull request description:

  This line from 999931cf8f1 appears to be extraneous and replaced 2 lines after by `UniValue balances{UniValue::VOBJ};`.

ACKs for top commit:
  Empact:
    ACK 6e0d82c55b
  hebasto:
    ACK 6e0d82c55bf4a9aae98c47b7cd00b2828b5dd0ee, the `obj` local variable is not used until the end of the scope.

Tree-SHA512: a220ca9cda091e78144d9b7fbe4bf90e8338d6e8c8dc7bea27a8e62f3a8ac1d983ad12a48a0a3366b2d8b9586878dfc69c1ec34bf846b34c91e42cda48a59850
2023-01-23 12:22:30 -06:00
MarcoFalke
460ed06315
Merge #18219: doc: Add warning against wallet.dat re-use
c1e07423083cd2a7e3f2b28f69a573ea1837af4d doc: Warn about wallet.dat re-use and backups (Albert)

Pull request description:

  Following discussion in #18205, this PR adds a warning against re-use of the same wallet file on two different nodes, as that can cause problems due to race conditions between nodes (eg: both nodes using the same addresses at the same time for different things because they are not aware of the other node).

  I've also included the rationale behind the warning but I've kept it short to make it clearer to users, not sure if I should have written a longer explanation instead.

  Also, while this PR may help some users avoid problems, the changes are largely inconsequential, so feel free to close it if it's not worth the effort.

  On an unrelated note, I've also set up [this site](https://corollari.github.io/bitcoin-core-docs/), which periodically pulls bitcoin core and turns its docs into a webpage. Browsing the docs can also be done locally or on github, so this doesn't add much value, but I personally find that more comfortable and it makes them more searchable.

Top commit has no ACKs.

Tree-SHA512: 5ce06026176917304932714470be8c3410d35698f925875b0955ecd3b1756ef52793feb469dd4bdac4921f1a24daf59001e9911f1f096f559fb28c250baae378
2023-01-23 12:22:29 -06:00
MarcoFalke
f7dbd8c62e
Merge #18018: tests: reset fIsBareMultisigStd after bare-multisig tests
1b96a3cd1ebe725896f59614903184289fe62cf8 tests: reset fIsBareMultisigStd after bare-multisig tests (fanquake)

Pull request description:

  Fixes: #18015

  The bug this fixes is two-part.

  1. The `fIsBareMultisigStd` global is being reused by other tests,
  such as [script_p2sh_tests(set)](https://github.com/bitcoin/bitcoin/blob/master/src/test/script_p2sh_tests.cpp#L150), after being set to false.

  2. The order our tests run in doesn't always? seem to be random,
  which meant that the `script_p2sh` tests would only fail if they
  were run in an order where the `transaction_tests` ran first,
  mutating the `fIsBareMultisigStd` global.

  This doesn't seem to happen when running make check, but if you
  run `src/test/test_bitcoin and pass --random=99999`, the failure
  in `script_p2sh` will occur (on most, but maybe not all systems):

  ```bash
  src/test/test_bitcoin --random=99999
  Running 389 test cases...
  test/script_p2sh_tests.cpp:200: error: in "script_p2sh_tests/set": txTo[1].IsStandard
  test/script_p2sh_tests.cpp:200: error: in "script_p2sh_tests/set": txTo[2].IsStandard
  test/script_p2sh_tests.cpp:200: error: in "script_p2sh_tests/set": txTo[3].IsStandard

  *** 3 failures are detected in the test module "Bitcoin Core Test Suite"
  ```

  The new test for bare multisig was introduced in #17502.

ACKs for top commit:
  Empact:
    Code Review ACK 1b96a3cd1e
  theStack:
    ACK https://github.com/bitcoin/bitcoin/pull/18018/commits/1b96a3cd1ebe725896f59614903184289fe62c

Tree-SHA512: fd7578f9f3faa44d236cd007fc25e31f061acabdb8458559fde0e67d11ab5cafed15305993270c9943a50326574bc5f5301b09494a5b0d2de69e64978093ed45
2023-01-23 12:22:29 -06:00
MarcoFalke
031bea7214
Merge #17378: TestShell: Fix typos & implement cleanups
2493770e365a7e30117dc1b8d228f9cbed97f7e1 TestShell: Return self from setup() (James Chiang)
a8dea4552412668c2914b6395ef543341a9898cd TestShell: Simplify default setting of num_nodes (James Chiang)
9c7806e4bf113bee6c32cff7b46493fd1a5aa0ba Doc: Remove backticks in test-shell.md code block (James Chiang)
d3ed06e2cdb31dcecf5e647f7e1e52185cc76733 TestShell: Fix typo in TestShell warning printout (James Chiang)

Pull request description:

  This PR follows up on #17288 and fixes typos and implements code clean-ups suggested by reviewers of 19139ee.

  - Typo in `test_shell.py` warning
  - Typo in `test-shell.md` code block
  - Simplified default setting of `num_nodes` in `TestShell.setup()`
  - Enable initializer chaining: `TestShell().setup()`

ACKs for top commit:
  MarcoFalke:
    ACK 2493770e365a7e30117dc1b8d228f9cbed97f7e1
  instagibbs:
    tACK 2493770e36
  jnewbery:
    utACK 2493770e365a7e30117dc1b8d228f9cbed97f7e1

Tree-SHA512: 8fa7c2c550dbc3ec899de9dc328cd55cfa6daafe3b888aa5427e72fea69f064d938ec68e15bfa57109c0f6c3583e627ac4bd69303a11575d056941bd253adee0
2023-01-23 12:22:29 -06:00
MarcoFalke
65e72bf91f
Merge #16742: test: add executable flag for wallet_watchonly.py
71e08ab22d5b795de86782ca2a94db1a4e61a70f test: add executable flag for wallet_watchonly.py (Sebastian Falbesoner)

Pull request description:

ACKs for top commit:
  promag:
    ACK 71e08ab22d5b795de86782ca2a94db1a4e61a70f.

Tree-SHA512: a187d2f7590ad2450b8e8fa3d038c80a04fc3d903618c24222d7e3172250ce51badea35860c86101f2ba266eb4354e6efb8d7d508b353f29276e4665a1efdf74
2023-01-23 12:22:29 -06:00
PastaPastaPasta
02afdfa444
Merge pull request #5144 from vijaydasmp/bp21_11
backport: Merge bitcoin#18664,18917,18901,18939,18875,19452,19548,19595,20300,20375
2023-01-23 11:23:08 -06:00
MarcoFalke
c545e11426 Merge #20375: fuzz: Improve coverage for CPartialMerkleTree fuzzing harness
3c77b8009de9457c356c0bf4362d11bb99a17bb7 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 3c77b8009de9457c356c0bf4362d11bb99a17bb7

Tree-SHA512: a1fa0f7650a5ee5ff83f35e41b9faf6c34671fc304b9af00e5b83073f21d50bcbe91c2428fa64d05dc42a7c521bfd24031e307c7f4abf9ded469d69a55c5d64a
2023-01-23 11:21:05 -06:00
MarcoFalke
25ae9444cd Merge #20300: fuzz: Add missing ECC_Start to descriptor_parse test
5cafe2b25c1d2f6825b3c8103c280020929dd645 fuzz: Add missing ECC_Start to descriptor_parse test (Ivan Metlushko)

Pull request description:

  Fixes fuzzing harness.

  I also observed that the corpus for this test consists only of `xprv...` keys while we are using regtest parameters. So for proper fuzzing we need either A) to update the corpus and replace `xprv...` with `tprv...` B) switch to main net in the test

ACKs for top commit:
  MarcoFalke:
    review ACK 5cafe2b25c1d2f6825b3c8103c280020929dd645
  practicalswift:
    Tested ACK 5cafe2b25c1d2f6825b3c8103c280020929dd645

Tree-SHA512: 7415a98a445ce0f96219637d2362fecfc1191ad104f55d79ca92b0c92cde165e00646be5bf3fda956385e3cb22540eca457e575048493367cdf0e00a27d7cdb8
2023-01-23 11:21:05 -06:00
fanquake
3ada56d500 Merge #19595: Fix .gitignore for src/test/fuzz directory
623f66554d44611485fa14937b6fd4f1dc63b818 Fix .gitignore for src/test/fuzz directory (Hennadii Stepanov)

Pull request description:

  On master (31d2b4098a9e4ee9a694ba1ad42829637cbcf3c6):
  ```
  $ git ls-files --ignored --exclude-standard
  contrib/init/org.bitcoin.bitcoind.plist
  contrib/macdeploy/fancy.plist
  src/qt/Makefile
  src/qt/test/Makefile
  src/test/Makefile
  src/test/fuzz/FuzzedDataProvider.h
  src/test/fuzz/addition_overflow.cpp
  src/test/fuzz/addrdb.cpp
  src/test/fuzz/asmap.cpp
  src/test/fuzz/asmap_direct.cpp
  src/test/fuzz/autofile.cpp
  src/test/fuzz/banman.cpp
  src/test/fuzz/base_encode_decode.cpp
  src/test/fuzz/bech32.cpp
  src/test/fuzz/block.cpp
  src/test/fuzz/block_header.cpp
  src/test/fuzz/blockfilter.cpp
  src/test/fuzz/bloom_filter.cpp
  src/test/fuzz/buffered_file.cpp
  src/test/fuzz/chain.cpp
  src/test/fuzz/checkqueue.cpp
  src/test/fuzz/coins_view.cpp
  src/test/fuzz/crypto.cpp
  src/test/fuzz/crypto_aes256.cpp
  src/test/fuzz/crypto_aes256cbc.cpp
  src/test/fuzz/crypto_chacha20.cpp
  src/test/fuzz/crypto_chacha20_poly1305_aead.cpp
  src/test/fuzz/crypto_common.cpp
  src/test/fuzz/crypto_hkdf_hmac_sha256_l32.cpp
  src/test/fuzz/crypto_poly1305.cpp
  src/test/fuzz/cuckoocache.cpp
  src/test/fuzz/decode_tx.cpp
  src/test/fuzz/descriptor_parse.cpp
  src/test/fuzz/deserialize.cpp
  src/test/fuzz/eval_script.cpp
  src/test/fuzz/fee_rate.cpp
  src/test/fuzz/fees.cpp
  src/test/fuzz/flatfile.cpp
  src/test/fuzz/float.cpp
  src/test/fuzz/fuzz.cpp
  src/test/fuzz/fuzz.h
  src/test/fuzz/golomb_rice.cpp
  src/test/fuzz/hex.cpp
  src/test/fuzz/http_request.cpp
  src/test/fuzz/integer.cpp
  src/test/fuzz/key.cpp
  src/test/fuzz/key_io.cpp
  src/test/fuzz/kitchen_sink.cpp
  src/test/fuzz/load_external_block_file.cpp
  src/test/fuzz/locale.cpp
  src/test/fuzz/merkleblock.cpp
  src/test/fuzz/message.cpp
  src/test/fuzz/multiplication_overflow.cpp
  src/test/fuzz/net_permissions.cpp
  src/test/fuzz/netaddress.cpp
  src/test/fuzz/p2p_transport_deserializer.cpp
  src/test/fuzz/parse_hd_keypath.cpp
  src/test/fuzz/parse_iso8601.cpp
  src/test/fuzz/parse_numbers.cpp
  src/test/fuzz/parse_script.cpp
  src/test/fuzz/parse_univalue.cpp
  src/test/fuzz/policy_estimator.cpp
  src/test/fuzz/policy_estimator_io.cpp
  src/test/fuzz/pow.cpp
  src/test/fuzz/prevector.cpp
  src/test/fuzz/primitives_transaction.cpp
  src/test/fuzz/process_message.cpp
  src/test/fuzz/process_messages.cpp
  src/test/fuzz/protocol.cpp
  src/test/fuzz/psbt.cpp
  src/test/fuzz/random.cpp
  src/test/fuzz/rbf.cpp
  src/test/fuzz/rolling_bloom_filter.cpp
  src/test/fuzz/script.cpp
  src/test/fuzz/script_bitcoin_consensus.cpp
  src/test/fuzz/script_descriptor_cache.cpp
  src/test/fuzz/script_flags.cpp
  src/test/fuzz/script_interpreter.cpp
  src/test/fuzz/script_ops.cpp
  src/test/fuzz/script_sigcache.cpp
  src/test/fuzz/script_sign.cpp
  src/test/fuzz/scriptnum_ops.cpp
  src/test/fuzz/signature_checker.cpp
  src/test/fuzz/span.cpp
  src/test/fuzz/spanparsing.cpp
  src/test/fuzz/string.cpp
  src/test/fuzz/strprintf.cpp
  src/test/fuzz/system.cpp
  src/test/fuzz/timedata.cpp
  src/test/fuzz/transaction.cpp
  src/test/fuzz/tx_in.cpp
  src/test/fuzz/tx_out.cpp
  src/test/fuzz/util.h
  src/univalue/gen/gen.cpp
  test/functional/data/wallets/high_minversion/db.log
  test/functional/data/wallets/high_minversion/wallet.dat
  ```

  With this PR:
  ```
  $ git ls-files --ignored --exclude-standard
  contrib/init/org.bitcoin.bitcoind.plist
  contrib/macdeploy/fancy.plist
  src/qt/Makefile
  src/qt/test/Makefile
  src/test/Makefile
  src/univalue/gen/gen.cpp
  test/functional/data/wallets/high_minversion/db.log
  test/functional/data/wallets/high_minversion/wallet.dat
  ```

ACKs for top commit:
  MarcoFalke:
    review ACK 623f66554d44611485fa14937b6fd4f1dc63b818 seems like an improvement when writing new fuzz tests
  practicalswift:
    ACK 623f66554d44611485fa14937b6fd4f1dc63b818 -- thanks for fixing! ❤️
  theStack:
    tested ACK 623f66554d44611485fa14937b6fd4f1dc63b818

Tree-SHA512: 16b3854bf4fd8c3096d915a4efc5cbc63d28b18854b051bafee374508dfbb5871ae7dc6f303dbf57469473082d2c3a7df0a8170da22d60d13878544679363b5c
2023-01-23 11:21:05 -06:00
fanquake
b08295d793 Merge #19548: fuzz: add missing overrides to signature_checker
c0f09c2c9deaec4cfb35ea587363e6301dd17b88 fuzz: add missing overrides to signature_checker (Jon Atack)

Pull request description:

  These functions in `fuzz/signature_checker.cpp` override virtual member functions and should be marked `override` instead of `virtual`, which is for introducing a new virtual function. The overridden virtual functions are in `script/interpreter.h:151/156/161`.

  Also, per MarcoFalke suggestion, add missing parentheses in `fuzz/scriptnum_ops.cpp` and remove useless `unsigned int >= 0` conditional in `fuzz/script.cpp`.

  These changes fix 5 compile warnings in gcc 10 and 3 in clang 11/12.

ACKs for top commit:
  vasild:
    ACK c0f09c2
  MarcoFalke:
    review ACK c0f09c2c9deaec4cfb35ea587363e6301dd17b88

Tree-SHA512: 76ce73ec577c1f23cf8646c31d44dcd6c6303732c47187d041a8921d0d24a50163989a375352ebc221abf2ac337bc0902149be46b6f9eebc071d2f364c407f71
2023-01-23 11:21:05 -06:00
fanquake
ffd9d3de6f Merge #19452: doc: afl fuzzing comment about afl-gcc and afl-g++
2b78a11b48bad1fa30120ce851269ca9ce8833a5 doc: afl fuzzing comment about afl-gcc and afl-g++ (nsa)

Pull request description:

  When trying to build the fuzz tests with `--enable-lcov` on a Ubuntu machine, noticed that the documentation was lacking with regards to the afl-gcc and afl-g++ options. `afl-clang-fast` and `afl-clang-fast++` in the examples just need to be replaced with `afl-gcc` and `afl-g++`. I also had to set the `-m` flag as well to get the fuzzers to run.

ACKs for top commit:
  practicalswift:
    ACK 2b78a11b48bad1fa30120ce851269ca9ce8833a5
  MarcoFalke:
    Concept ACK 2b78a11b48bad1fa30120ce851269ca9ce8833a5, haven't tested

Tree-SHA512: d8151afd79de949e8c6da49b69bbbf1470eb478c8ddcbc69b30e86bf9396c0f13835a655d4ae658f7dc4f36c35b02cd23b08358fb73a71e15bf14e76c1f365a4
2023-01-23 11:21:05 -06:00
MarcoFalke
c44ef8e127 Merge #18939: doc: add c++17-enable flag to fuzzing instructions
872aa25fa1d71aa022cdfa02e5927d851d73b3a8 doc: add c++17-enable to fuzzing instructions (Martin Zumsande)

Pull request description:

  Update the fuzzing doc because after the merge of #18901, C++17 is required for compilation.

ACKs for top commit:
  practicalswift:
    ACK 872aa25fa1d71aa022cdfa02e5927d851d73b3a8
  MarcoFalke:
    ACK 872aa25fa1d71aa022cdfa02e5927d851d73b3a8

Tree-SHA512: 47e37c033690de1d1fa644bf0cebb256036b32a5784021cc0d3b32e6188822d7f517d4342990dc7ec98de6d650794aeb85483157e69e141d6bd011993e124575
2023-01-23 11:21:05 -06:00
MarcoFalke
b87625d738 Merge #18901: fuzz: use std::optional for sep_pos_opt variable
420fa0770f37619bfa29898d59dac45b6a477abb fuzz: use std::optional for sep_pos variable (Harris)

Pull request description:

  This PR changes the original `size_t sep_pos` to `std::optional<size_t> sep_post_opt` to remove the warning when compiling fuzz tests.

  ```shell
  warning: variable 'sep_pos' may be uninitialized when used here [-Wconditional-uninitialized]
  ```

  Also, it adds `--enable-c++17` flag to CI fuzz scripts.

ACKs for top commit:
  practicalswift:
    ACK 420fa0770f37619bfa29898d59dac45b6a477abb
  MarcoFalke:
    ACK 420fa07

Tree-SHA512: e967d5d8ab8ee7394b243ff5b28bac72d30bd14774e4a206f8c87474fad22769da76e4ba4e03cbef83b8f60e5293e9d9293b613e2e2e59e187d4e59ae6b874ca
2023-01-23 11:21:05 -06:00
MarcoFalke
1f49424788 Merge #18917: fuzz: fix vector size problem in system fuzzer
095bc9a10691505c3d0fdacb6caeb62bfdcf1732 fuzz: fix vector size problem in system fuzzer (Harris)

Pull request description:

  This PR fixes a problem with vector resizing in system fuzzer (*case 7* there). Originally, this problem was discussed in PR https://github.com/bitcoin/bitcoin/pull/18908

ACKs for top commit:
  MarcoFalke:
    ACK 095bc9a10691505c3d0fdacb6caeb62bfdcf1732
  practicalswift:
    ACK 095bc9a10691505c3d0fdacb6caeb62bfdcf1732
  brakmic:
    > ACK [095bc9a](095bc9a106)

Tree-SHA512: 73e6004ee51d68a34b49c79d1329a8c4865c21da888801c0fcc7f1bcacb510bf371bb61675eda83e53d08e0f24712e671369719523b0ced0eb2a22607bfa1d3d
2023-01-23 11:21:05 -06:00
MarcoFalke
fe57f23011 Merge #18664: fuzz: fix unused variable compiler warning
eab7367e25e35688a4d4a6c96701dd7149134df5 fuzz: fix unused variable compiler warning (Jon Atack)

Pull request description:

  Fixes the compiler warning while hopefully not invalidating the existing seeds. Added an explanatory comment.
  ```
  test/fuzz/locale.cpp:59:19: warning: unused variable 'random_int32' [-Wunused-variable]
      const int32_t random_int32 = fuzzed_data_provider.ConsumeIntegral<int32_t>();
  ```

ACKs for top commit:
  practicalswift:
    ACK eab7367e25e35688a4d4a6c96701dd7149134df5

Tree-SHA512: 4c90784518027cd3f85acd18030201efe4018f9da46365fef934e9a53a0b923031fec4c884a2da2f14232b6060aeb9016ac09950a18e31395de048548ecbc836
2023-01-23 11:21:05 -06:00
UdjinM6
74b273f308
Merge pull request #5162 from PastaPastaPasta/fix-crash-shutdown
fix: resolve crash on close bug in macos
2023-01-23 00:00:26 +03:00
Hennadii Stepanov
512503ca13 Merge bitcoin-core/gui#680: Fixes MacOS 13 segfault by preventing certain notifications after main window is destroyed
8a5014cd8a05b3ab86ae34a47653a82ce11bdf17 Fixes bitcoin#26490 by preventing notifications (John Moffett)

Pull request description:

  This is a PR to address https://github.com/bitcoin/bitcoin/issues/26490

  The menu bar currently subscribes to window focus change notifications to enable or disable certain menu options in response to the window status.

  Notifications are automatically unsubscribed (disconnected in Qt parlance) if the sender is deleted -- in this case, the sender is the QTApplication object (`qApp`). However, MacOS 13 sends a window focus change notification *after* the main window has been destroyed but *before* `qApp` has been fully destroyed.

  Since the menu bar is deleted in the main window's destructor, it no longer exists when it receives these notifications (in two different places via lambda expressions). The solution is to pass the main window (`this`) as context when subscribing to the notifications. In this [overloaded version](https://doc.qt.io/qt-5/qobject.html#connect-1) of `connect`, Qt automatically unsubscribes to notifications if the sender OR context (here the main window object) is destroyed. Since the spurious notifications are sent after the main window object is destroyed, this change prevents them from being sent.

  Tested on Mac OS 13 and 12 only.

ACKs for top commit:
  hebasto:
    ACK 8a5014cd8a05b3ab86ae34a47653a82ce11bdf17

Tree-SHA512: 3dff0a252fe0e93dd68cf5503135ecf6a72bcf385ba38407d6021ab77cca323f8bbe58aeca90ec124aa2a22ab9d35b706946179ac3b5d171c96a7010de51a090
2023-01-22 23:57:54 +03:00
PastaPastaPasta
259d22f22c
feat: reduce autoresending of wallet transactions to an average of 2 hours (#5166)
## Issue being fixed or feature implemented
autoresending was really slow

## What was done?
reduced the time range to from 1-3 hours from now

## How Has This Been Tested?
hasn't

## Breaking Changes
Shouldn't be

## Checklist:
<!--- Go over all the following points, and put an `x` in all the boxes
that apply. -->
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation

**For repository code-owners and collaborators only**
- [x] I have assigned this pull request to a milestone

Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
2023-01-22 23:57:36 +03:00
PastaPastaPasta
a2a1064a30
Merge pull request #5118 from vijaydasmp/bp21_6
backport: Merge #18670,18806,19060,19299,9828,20064,20105,18808
2023-01-22 00:29:39 -06:00
Wladimir J. van der Laan
08a7d1118f Merge #19060: test: Remove global wait_until from p2p_getdata
fa80b4788bbe3ef00c5d767c0d89ba9809d8707c test: Remove global wait_until from p2p_getdata (MarcoFalke)
999922baed3a80b581ce46daa01c4cbca4fcbfd8 test: Default mininode.wait_until timeout to 60s (MarcoFalke)
fab47375fe0bdec1e557e087fdb0707c4dfa7cc2 test: pep-8 p2p_getdata.py (MarcoFalke)

Pull request description:

  Using the global wait_until makes it impossible to adjust the timeout based on the hardware the test is running on.

  Fix that by using the mininode member function.

  So for example, `./test/functional/p2p_getdata.py  --timeout-factor=0.04` gives a timeout of 2.4 seconds.

ACKs for top commit:
  laanwj:
    ACK fa80b4788bbe3ef00c5d767c0d89ba9809d8707c

Tree-SHA512: ebb1b7860a64451de2b8ee9a0966faddb13b84af711f6744e8260d7c9bc0b382e8fb259897df5212190821e850ed30d4d5c2d7af45a97f207fd4511b06b6674a
2023-01-22 00:27:52 -06:00
fanquake
de50c799d4 Merge #18808: [net processing] Drop unknown types in getdata
9847e205bf7edcac4c30ce4b6d62f482aa7bc1b7 [docs] Improve commenting in ProcessGetData() (John Newbery)
2f032556e08a04807c71eb02104ca9589eaadf1b [test] test that an invalid GETDATA doesn't prevent processing of future messages (Amiti Uttarwar)
e257cf71c851e25e1a533bf1d4296f6b55c81332 [net processing] ignore unknown INV types in GETDATA messages (Amiti Uttarwar)
047ceac142246b5d51056a51dbf4645b31802be4 [net processing] ignore tx GETDATA from blocks-only peers (Amiti Uttarwar)

Pull request description:

  Currently we'll stall peers that send us an unknown INV type in a GETDATA message. Be a bit more friendly and just drop the invalid request.

  Ditto for blocks-relay-only peers that send us a GETDATA for a transaction.

  There's a test for the first part. The second is difficult to test in the functional test framework since we aren't able to make blocks-relay-only connections.

ACKs for top commit:
  sipa:
    utACK 9847e205bf7edcac4c30ce4b6d62f482aa7bc1b7
  brakmic:
    ACK 9847e205bf
  luke-jr:
    utACK 9847e205bf7edcac4c30ce4b6d62f482aa7bc1b7
  naumenkogs:
    utACK 9847e20
  ajtowns:
    utACK 9847e205bf7edcac4c30ce4b6d62f482aa7bc1b7

Tree-SHA512: 6007f2fd839ffe737727f6fb8e8f083b2d9e05a510748f1d40b8f9be8fdf7b5419a36d8f1039923eec1ba2983e8f6f0436ec5fc196d9f6dcb0657f2ff8ff8e4c
2023-01-22 00:27:52 -06:00
fanquake
72c08e4a98 Merge #20064: RPC: remove duplicate line in getblock help
1885ad35467f201f2a210057797aae8a450e7cdf RPC: remove duplicate line in getblock help (Fabian Jahr)

Pull request description:

  Line simply seems duplicated in error.

  Testing instructions:
  Run `src/bitcoin-cli help getblock` on master branch to reproduce. Then build this PR and compare its results.

ACKs for top commit:
  dhruv:
    tACK `1885ad3`
  kristapsk:
    ACK 1885ad35467f201f2a210057797aae8a450e7cdf
  Emzy:
    tACK 1885ad35467f201f2a210057797aae8a450e7cdf

Tree-SHA512: 870c035cb553b0e1d5ef72e64231ef277e0392efe94bc6ecf47129023bd94a6d5a276f46529807f68a1db55c7baa94d9119c7264d9947bc4e5dd9dcefd1b13e7
2023-01-22 00:27:52 -06:00
fanquake
12cad69c17 Merge #19828: wallet, refactor: Remove duplicate map lookups in GetAddressBalances
b35e74ba379bdc12ea6d49a45469f0d6aa74cc27 wallet, refactor: Remove duplicate map lookups in GetAddressBalances (João Barbosa)

Pull request description:

  Now just one lookup in `balances` instead of three.

ACKs for top commit:
  achow101:
    ACK b35e74ba379bdc12ea6d49a45469f0d6aa74cc27
  theStack:
    ACK b35e74ba379bdc12ea6d49a45469f0d6aa74cc27
  practicalswift:
    ACK b35e74ba379bdc12ea6d49a45469f0d6aa74cc27

Tree-SHA512: a73c1b336406a569e3bb10290618c5950b944db58ed0b05ff202d097684bb3ba3a5942c8d30443960052aa16438c054e2d02977b67aa901cce665c4df0ee5602
2023-01-22 00:27:52 -06:00