0
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-02-05 10:17:30 -05:00
Commit graph

4196 commits

Author SHA1 Message Date
MarcoFalke
16d698cdcf
Merge bitcoin/bitcoin#23517: scripted-diff: Move miner to src/node
fa4e09924b refactor: Replace validation.h include with forward-decl in miner.h (MarcoFalke)
fa0739a7d3 style: Sort file list after rename (MarcoFalke)
fa53e3a58c scripted-diff: Move miner to src/node (MarcoFalke)

Pull request description:

  It is impossible to run the miner without a node (validation, chainstate, mempool, rpc, ...). Also, the module is in the node library. Thus, it should be moved to `src/node`.

  Also, replace the `validation.h` include in the header with a forward-declaration.

ACKs for top commit:
  theStack:
    Code-review ACK fa4e09924b

Tree-SHA512: 791e6caa5839d8dc83b0f58f3f49bc0a7e3c1710822e8a44dede254c87b6f7531a0586fb95e8a067c181457a3895ad6041718aa2a2fac64cfc136bf04bb851d5
2021-11-26 09:03:39 +01:00
MarcoFalke
76392b042e
Merge bitcoin/bitcoin#22829: refactor: various RecursiveMutex replacements in CConnman
3726a45958 refactor: replace RecursiveMutex m_added_nodes_mutex with Mutex (Sebastian Falbesoner)
7d52ff5c38 refactor: replace RecursiveMutex m_addr_fetches_mutex with Mutex (Sebastian Falbesoner)
d51d2a3bb5 scripted-diff: rename node vector/mutex members in CConnman (Sebastian Falbesoner)
574cc4271a refactor: remove RecursiveMutex cs_totalBytesRecv, use std::atomic instead (Sebastian Falbesoner)

Pull request description:

  This PR is related to #19303 and gets rid of the following RecursiveMutex members in class `CConnman`:
  * for `cs_totalBytesRecv`, protecting `nTotalBytesRecv`, `std::atomic` is used instead (the member is only increment at one and read at another place, so this is sufficient)
  * for `m_addr_fetches_mutex`, protecting `m_addr_fetches`, a regular `Mutex` is used instead (there is no chance that within one critical section, another one is called)
  * for `cs_vAddedNodes`, protecting `vAddedNodes`, a regular `Mutex` is used instead (there is no chance that within one critical section, another one is called)

  Additionally, the PR takes the chance to rename all node vector members (vNodes, vAddedNodes) and its corresponding mutexes (cs_vNodes, cs_vAddedNodes) to match the coding guidelines via a scripted-diff.

ACKs for top commit:
  vasild:
    ACK 3726a45958
  promag:
    Code review ACK 3726a45958.
  hebasto:
    re-ACK 3726a45958

Tree-SHA512: 4f5ad41ba2eca397795080988c1739c6abb44c1204dddaa75cc38a396fa821fbe1010694ba7bead1b606beaa677661e66da2a5dca233b2937214f63a54848348
2021-11-25 11:55:37 +01:00
MarcoFalke
064c729a96
Merge bitcoin/bitcoin#23512: policy: Treat taproot as always active
fa3e0da06b policy: Treat taproot as always active (MarcoFalke)

Pull request description:

  Now that taproot is active, it can be treated as if it was always active for policy for the next major release. This simplifies the code and changes two things:

  * Importing `tr` descriptors can be done before the chain is fully synced. This is fine, because the wallet will already generate `tr` descriptors by default (regardless of the taproot status) after commit 47fe7445e7.
  * Valid taproot spends won't be rejected from the mempool before taproot is active. This is strictly speaking a bugfix after commit 47fe7445e7, since the wallet may generate taproot spends before the chain is fully synced. For example, a slow node or a purposefully offline node. Currently, the wallet needs the mempool to account for change. See https://github.com/bitcoin/bitcoin/issues/11887.

  A similar change was done for segwit v0 in https://github.com/bitcoin/bitcoin/pull/13120 .

  This effectively reverts commit c5ec0367d7.

ACKs for top commit:
  mjdietzx:
    Code Review ACK fa3e0da06b
  achow101:
    ACK fa3e0da06b
  sipa:
    utACK fa3e0da06b
  gruve-p:
    ACK fa3e0da06b
  gunar:
    Code Review + tACK fa3e0da06
  rajarshimaitra:
    code review + tACK fa3e0da06b

Tree-SHA512: c6dc7a4e6c345bdec33f256847dc63906ab1696aa683ab9b32a79e715613950884ac3a1a7a44e95f31bb28e58dd64679a616175f7e152b21f5550f3337c8e622
2021-11-25 08:16:19 +01:00
Sebastian Falbesoner
d51d2a3bb5 scripted-diff: rename node vector/mutex members in CConnman
-BEGIN VERIFY SCRIPT-

ren() { sed -i "s/$1/$2/g" $3 $4 $5; }

ren cs_vAddedNodes         m_added_nodes_mutex     src/net.h src/net.cpp
ren vAddedNodes            m_added_nodes           src/net.h src/net.cpp
ren cs_vNodes              m_nodes_mutex           src/net.h src/net.cpp src/test/util/net.h
ren vNodesDisconnectedCopy nodes_disconnected_copy src/net.cpp
ren vNodesDisconnected     m_nodes_disconnected    src/net.h src/net.cpp
ren vNodesCopy             nodes_copy              src/net.cpp
ren vNodesSize             nodes_size              src/net.cpp
ren vNodes                 m_nodes                 src/net.h src/net.cpp src/test/util/net.h

-END VERIFY SCRIPT-
2021-11-24 19:34:21 +01:00
MarcoFalke
9394964f6b
Merge bitcoin/bitcoin#23451: span: Add std::byte helpers
faa3ec2304 span: Add std::byte helpers (MarcoFalke)
fa18038f51 refactor: Use ignore helper when unserializing an invalid pubkey (MarcoFalke)
fabe18d0b3 Use value_type in CDataStream where possible (MarcoFalke)

Pull request description:

  This adds (currently unused) span std::byte helpers, so that they can be used in new code.

  The refactors are also required for https://github.com/bitcoin/bitcoin/pull/23438, but they are split up because the other pull doesn't compile with msvc right now.

  The third commit is not needed for the other pull, but still nice.

ACKs for top commit:
  klementtan:
    reACK  faa3ec2. Verified that all the new `std::byte` helper functions are tested.
  laanwj:
    Code review ACK faa3ec2304

Tree-SHA512: b1f6af39f03ea4dfebf20d4a8538fa993a6104e7fc92ddf0c4606a7efc3ca9a8c1a4741d98a1418569c11bb9ce9258bf0c0c06d93d85ed7e208902a2db04e407
2021-11-24 11:04:37 +01:00
MarcoFalke
73ac195e29
Merge bitcoin/bitcoin#23249: util: ParseByteUnits - Parse a string with suffix unit
21b58f430f util: ParseByteUnits - Parse a string with suffix unit [k|K|m|M|g|G|t|T] (Douglas Chimento)

Pull request description:

  A convenience utility for parsing human readable strings sizes e.g. `500G` is `500 * 1 << 30`

  The argument/setting `maxuploadtarget`  now accept human readable byte units `[k|K|m|M|g|G||t|T]`
  This change  backward compatible, defaults to `M` if no unit specified.

ACKs for top commit:
  vasild:
    ACK 21b58f430f
  ryanofsky:
    Code review ACK 21b58f430f. Only changes since last review are dropping optional has_value call, fixing comment punctuation, squashing commits.

Tree-SHA512: c9b85acc0f77c847a0290b27ac5dc586ecc078110cf133063140576a04c11aa9c553159b9b4993488edcf6e60db6837de7c83b2964639bc21e8ffa4d455a5eb7
2021-11-24 10:49:13 +01:00
Samuel Dobson
c8b9a224e7 Report encoding type in bech32 error message 2021-11-23 15:48:59 +13:00
Samuel Dobson
92f0cafdca Improve Bech32 boost tests 2021-11-23 15:48:59 +13:00
W. J. van der Laan
95d19f8c1a
Merge bitcoin/bitcoin#16807: Let validateaddress locate error in Bech32 address
88cc481092 Modify copyright header on Bech32 code (Samuel Dobson)
5599813b80 Add lots of comments to Bech32 (Samuel Dobson)
2eb5792ec7 Add release notes for validateaddress Bech32 error detection (MeshCollider)
42d6a029e5 Refactor and add more tests for validateaddress (Samuel Dobson)
c4979f77c1 Add boost tests for bech32 error detection (MeshCollider)
02a7bdee42 Add error_locations to validateaddress RPC (Samuel Dobson)
b62b67e06c Add Bech32 error location function (Samuel Dobson)
0b06e720c0 More detailed error checking for base58 addresses (Samuel Dobson)

Pull request description:

  Addresses (partially) #16779 - no GUI change in this PR

  Adds a LocateError function the bech32 library, which is then called by `validateaddress` RPC, (and then eventually from a GUI tool too, future work). I think modifying validateaddress is nicer than adding a separate RPC for this.
  Includes tests.

  Based on https://github.com/sipa/bech32/blob/master/ecc/javascript/bech32_ecc.js
  Credit to sipa for that code

ACKs for top commit:
  laanwj:
    Code review and manually tested ACK 88cc481092
  ryanofsky:
    Code review ACK 88cc481092 with caveat that I only checked the new `LocateErrors` code to try to verify it didn't have unsafe or unexpected operations or loop forever or crash. Did not try to verify behavior corresponds to the spec. In the worst case bugs here should just affect error messages not actual decoding of addresses so this seemed ok.
  w0xlt:
    tACK 88cc481

Tree-SHA512: 9c7fe9745bc7527f80a30bd4c1e3034e16b96a02cc7f6c268f91bfad08a6965a8064fe44230aa3f87e4fa3c938f662ff4446bc682c83cb48c1a3f95cf4186688
2021-11-22 13:26:01 +01:00
MarcoFalke
fa00447442
scripted-diff: Use clang-tidy syntax for C++ named arguments
-BEGIN VERIFY SCRIPT-
 perl -0777 -pi -e 's:((\(|\{|,)(\n| )*)\/\* ?([^=* ]+) ?\*\/ ?:\1/*\4=*/:g' $( git ls-files ./src/test ./src/wallet/test )
-END VERIFY SCRIPT-
2021-11-19 12:41:47 +01:00
MarcoFalke
fae13c3989
doc: Use clang-tidy comments in crypto_tests
Also, fix argument name for FastRandomContext.
2021-11-19 12:40:13 +01:00
Douglas Chimento
21b58f430f
util: ParseByteUnits - Parse a string with suffix unit [k|K|m|M|g|G|t|T]
A convenience utility for human readable arguments/config e.g. -maxuploadtarget=500g
2021-11-17 12:47:30 +02:00
MarcoFalke
fac49470ca
doc: Fix incorrect C++ named args 2021-11-17 09:25:14 +01:00
MarcoFalke
cb2392c5fb
Merge bitcoin/bitcoin#23496: fuzz: Add minisketch fuzz test
fa74d45306 fuzz: Add minisketch fuzz test (MarcoFalke)

Pull request description:

ACKs for top commit:
  mjdietzx:
    re-ACK fa74d45
  sipa:
    utACK fa74d45306

Tree-SHA512: 3d30095c85032139c37c7a2811dd417441a5105cb70af8250000d7b56aeda1e8fab5e65e683fb49d513ef40a81da3967a8a9a70caf40f56cef1dd96c6d4a05f6
2021-11-17 07:28:23 +01:00
MarcoFalke
fa74d45306
fuzz: Add minisketch fuzz test 2021-11-16 19:18:05 +01:00
MarcoFalke
fa0739a7d3
style: Sort file list after rename 2021-11-16 10:05:21 +01:00
MarcoFalke
fa53e3a58c
scripted-diff: Move miner to src/node
-BEGIN VERIFY SCRIPT-
 # Move module
 git mv src/miner.cpp src/node/
 git mv src/miner.h   src/node/
 # Replacements
 sed -i 's:miner\.h:node/miner.h:g'     $(git grep -l miner)
 sed -i 's:miner\.cpp:node/miner.cpp:g' $(git grep -l miner)
 sed -i 's:MINER_H:NODE_MINER_H:g'      $(git grep -l MINER_H)
-END VERIFY SCRIPT-
2021-11-16 10:04:55 +01:00
MarcoFalke
fa44237d76
doc: Fix typos in endif header comments 2021-11-16 09:56:45 +01:00
fanquake
d0923098c6
Merge bitcoin/bitcoin#23491: scripted-diff: Move minisketchwrapper to src/node
faba1abe46 Sort file list after rename (MarcoFalke)
fa8f60e311 scripted-diff: Move minisketchwrapper to src/node (MarcoFalke)

Pull request description:

  The newly added wrapper is currently in the node library, but not placed in the node directory. While it is possible to use the wrapper outside of a node context (for example in a utility), it seems unlikely. Either way, I think the wrapper should either be moved to the util lib+dir or the node lib+dir, not something in-between.

  Also, fix incorrect comment `BITCOIN_DBWRAPPER_H`.

ACKs for top commit:
  fanquake:
    ACK faba1abe46. I saw the comment in #21515, however given there hasn't been any new activity there, I'm going to merge this now.

Tree-SHA512: fccc0cfd1fee661152a1378587b96795ffb7a7eceb6d2c27ea5401993fd8b9c0a92579fdba61203917ae6565269cb28d0973464fb6201dabf72a5143495d3e77
2021-11-16 16:09:25 +08:00
MarcoFalke
fa3e0da06b
policy: Treat taproot as always active 2021-11-16 08:20:33 +01:00
W. J. van der Laan
5ccab7187b
Merge bitcoin/bitcoin#23394: Taproot wallet test vectors (generation+tests)
f1c33ee4ac tests: implement BIP341 test vectors (Pieter Wuille)
ac3037df11 tests: BIP341 test vector generation (Pieter Wuille)
ca83ffc2ea tests: add deterministic signing mode to ECDSA (Pieter Wuille)
c98c53f20c tests: abstract out precomputed BIP341 signature hash elements (Pieter Wuille)
a5bde018b4 tests: give feature_taproot access to sighash preimages (Pieter Wuille)
5140825096 tests: add more fields to TaprootInfo (Pieter Wuille)
2478c6730a Make signing follow BIP340 exactly w.r.t. aux randomness (Pieter Wuille)

Pull request description:

  This PR adds code to `test/functional/feature_taproot.py` which runs through a (deterministic) scenario covering several aspects of the wallet side of BIP341 (scriptPubKey computation from keys/scripts, control block computation, key path spending), with the ability to output test vectors in mediawiki format based on this scenario. The generated tests are then also included directly in `src/test/script_tests.cpp` and `src/test/script_standard_tests.cpp`.

  I intend to add these test vectors to BIP341 itself: https://github.com/bitcoin/bips/pull/1225

ACKs for top commit:
  laanwj:
    Code review ACK f1c33ee4ac

Tree-SHA512: fcf7109539cb214d3190516b205cd32d2b1b452f14aa66f4107acfaa8bfc7d368f626857f1935665a4342eabc0b9ee8aba608a7c0a2494bec0b498e723439c9d
2021-11-15 20:32:42 +01:00
MarcoFalke
024e4debc5
Merge bitcoin/bitcoin#23408: fuzz: Rework ConsumeScript
fa4baf0756 fuzz: Rework ConsumeScript (MarcoFalke)

Pull request description:

  This should make it easier for the fuzz engine to explore multisig code
  paths. See discussion in https://github.com/bitcoin/bitcoin/issues/23105

  The downside is that all fuzz inputs that use ConsumeScript are now
  invalidated and need to be re-generated.

  Another downside may be that most multisig scripts from ConsumeScript are
  using likely not fully valid pubkeys.

ACKs for top commit:
  jamesob:
    ACK fa4baf0756

Tree-SHA512: 15814afdee76b05ff7a71c0f07bbd1b3cff30d709d5c1e68fd230c5f5d16e673e42709a4fab84d4a896bc27f972f917fe7c1d1b32c2bf4209658b18da97e478b
2021-11-15 17:17:14 +01:00
MarcoFalke
36d184d0c8
Merge bitcoin/bitcoin#22508: fuzz: replace every fuzzer-controlled while loop with a macro
214d9055ac fuzz: replace every fuzzer-controlled loop with a LIMITED_WHILE loop (Andrew Poelstra)

Pull request description:

  Limits the number of iterations to 1000 rather than letting the fuzzer do millions or billions of iterations on a single core.

ACKs for top commit:
  MarcoFalke:
    cr ACK 214d9055ac

Tree-SHA512: 9741c32ccd126ea656e5c93371b7136eaa2f92dc9a490dd4d39642503b1a41174f3368245153e508c3b608fe37ab89800b67ada97b740e3b5a3728bb506429d3
2021-11-15 16:52:00 +01:00
Andrew Poelstra
214d9055ac fuzz: replace every fuzzer-controlled loop with a LIMITED_WHILE loop
Blindly chose a cap of 10000 iterations for every loop, except for
the two in script_ops.cpp and scriptnum_ops.cpp which appeared to
(sometimes) be deserializing individual bytes; capped those to one
million to ensure that sometimes we try working with massive scripts.

There was also one fuzzer-controlled loop in timedata.cpp which was
already capped, so I left that alone.

git grep 'while (fuzz' should now run clean except for timedata.cpp
2021-11-12 19:51:55 +00:00
Pieter Wuille
f1c33ee4ac tests: implement BIP341 test vectors 2021-11-12 12:05:00 -05:00
Pieter Wuille
2478c6730a Make signing follow BIP340 exactly w.r.t. aux randomness
libsecp256k1's secp256k1_schnorrsig_sign only follows BIP340 exactly
if an aux_rand32 argument is passed. When no randomness is used
(as is the case in the current codebase here), there is no impact
on security between not providing aux_rand32 at all, or providing
an empty one. Yet, for repeatability/testability it is simpler
to always use an all-zero one.
2021-11-12 12:04:20 -05:00
MarcoFalke
0000edaba3
style: Use 4 spaces for indentation, not 5
The wrong indentation breaks editor workflows.

Can be reviewed with --ignore-all-space
2021-11-12 11:41:55 +01:00
MarcoFalke
fab9264be5
test: Remove unused CDataStream copy 2021-11-12 11:40:39 +01:00
MarcoFalke
fa8f60e311
scripted-diff: Move minisketchwrapper to src/node
-BEGIN VERIFY SCRIPT-
 # Move module
 git mv src/minisketchwrapper.cpp src/node/
 git mv src/minisketchwrapper.h   src/node/
 # Replacements
 sed -i 's:minisketchwrapper:node/minisketchwrapper:g'     $(git grep -l minisketchwrapper)
 sed -i 's:MINISKETCHWRAPPER_H:NODE_MINISKETCHWRAPPER_H:g' $(git grep -l MINISKETCHWRAPPER_H)
 sed -i 's:DBWRAPPER_H:NODE_MINISKETCHWRAPPER_H:g'         ./src/node/minisketchwrapper.h
-END VERIFY SCRIPT-
2021-11-12 10:56:08 +01:00
MarcoFalke
1ff265a20c
Merge bitcoin/bitcoin#23477: addrman: tidy up unit tests
36d3510303 [addrman] [tests] Remove AddrManUncorrupted subclass (John Newbery)
dfbd3a6d71 [addrman] [tests] Remove AddrManCorrupted subclass (John Newbery)
d02098d1f0 [addrman] [tests] Tidy up unused arguments in addrman test functions (John Newbery)
7784a9a374 [addrman] [tests] Remove deterministic argument and member from AddrManTest (John Newbery)
a749fa539a [addrman] Remove AddrMan friends (John Newbery)

Pull request description:

  Various tidy-ups to the addrman tests.

ACKs for top commit:
  shaavan:
    crACK 36d3510303
  promag:
    Code review ACK 36d3510303.
  theStack:
    Code-review ACK 36d3510303

Tree-SHA512: bbdb9d70863c15b023714ba3c73e816c635204f949c39678dd932a6e9a2e57b51b5d50332ec6843cf1b98a2fcbbdd5e6779f2e9c7e9cf90f4a6b3b4a7a1abe2f
2021-11-12 09:47:14 +01:00
fanquake
c1fb30633b
Merge bitcoin/bitcoin#23114: Add minisketch subtree and integrate into build/test
29173d6c6c ubsan: add minisketch exceptions (Cory Fields)
54b5e1aeab Add thin Minisketch wrapper to pick best implementation (Pieter Wuille)
ee9dc71c1b Add basic minisketch tests (Pieter Wuille)
0659f12b13 Add minisketch dependency (Gleb Naumenko)
0eb7928ab8 Add MSVC build configuration for libminisketch (Pieter Wuille)
8bc166d5b1 build: add minisketch build file and include it (Cory Fields)
b2904ceb85 build: add configure checks for minisketch (Cory Fields)
b6487dc4ef Squashed 'src/minisketch/' content from commit 89629eb2c7 (fanquake)

Pull request description:

  This takes over #21859, which has [recently switched](https://github.com/bitcoin/bitcoin/pull/21859#issuecomment-921899200) to my integration branch. A few more build issues came up (and have been fixed) since, and after discussing with sipa it was decided I would open a PR to shepherd any final changes through.

  > This adds a `src/minisketch` subtree, taken from the master branch of https://github.com/sipa/minisketch, to prepare for Erlay implementation (see #21515). It gets configured for just supporting 32-bit fields (the only ones we're interested in in the context of Erlay), and some code on top is added:
  > * A very basic unit test (just to make sure compilation & running works; actual correctness checking is done through minisketch's own tests).
  > * A wrapper in `minisketchwrapper.{cpp,h}` that runs a benchmark to determine which field implementation to use.

  Only changes since my last update to the branch in the previous PR have been rebasing on master and fixing an issue with a header in an introduced file.

ACKs for top commit:
  naumenkogs:
    ACK 29173d6c6c

Tree-SHA512: 1217d3228db1dd0de12c2919314e1c3626c18a416cf6291fec99d37e34fb6eec8e28d9e9fb935f8590273b8836cbadac313a15f05b4fd9f9d3024c8ce2c80d02
2021-11-12 10:00:49 +08:00
MarcoFalke
38b2a0a3f9
Merge bitcoin/bitcoin#23173: Add ChainstateManager::ProcessTransaction
0fdb619aaf [validation] Always call mempool.check() after processing a new transaction (John Newbery)
2c64270bbe [refactor] Don't call AcceptToMemoryPool() from outside validation.cpp (John Newbery)
92a3aeecf6 [validation] Add CChainState::ProcessTransaction() (John Newbery)
36167faea9 [logging/documentation] Remove reference to AcceptToMemoryPool from error string (John Newbery)
4c24142b1e [validation] Remove comment about AcceptToMemoryPool() (John Newbery)
5759fd12b8 [test] Don't set bypass_limits to true in txvalidation_tests.cpp (John Newbery)
497c9e2964 [test] Don't set bypass_limits to true in txvalidationcache_tests.cpp (John Newbery)

Pull request description:

  Similarly to how #18698 added `ProcessNewBlock()` and `ProcessNewBlockHeaders()` methods to the `ChainstateManager` class, this PR adds a new `ProcessTransaction()` method. Code outside validation no longer calls `AcceptToMemoryPool()` directly, but calls through the higher-level `ProcessTransaction()` method. Advantages:

  - The interface is simplified. Calling code no longer needs to know about the active chainstate or mempool object, since `AcceptToMemoryPool()` can only ever be called for the active chainstate, and that chainstate knows which mempool it's using. We can also remove the `bypass_limits` argument, since that can only be used internally in validation.
  - responsibility for calling `CTxMemPool::check()` is removed from the callers, and run automatically by `ChainstateManager` every time `ProcessTransaction()` is called.

ACKs for top commit:
  lsilva01:
    tACK 0fdb619 on Ubuntu 20.04
  theStack:
    Code-review ACK 0fdb619aaf
  ryanofsky:
    Code review ACK 0fdb619aaf. Only changes since last review: splitting & joining commits, adding more explanations to commit messages, tweaking MEMPOOL_ERROR string, fixing up argument name comments.

Tree-SHA512: 0b395c2e3ef242f0d41d47174b1646b0a73aeece38f1fe29349837e6fb832f4bf8d57e1a1eaed82a97c635cfd59015a7e07f824e0d7c00b2bee4144e80608172
2021-11-10 14:35:22 +01:00
John Newbery
36d3510303 [addrman] [tests] Remove AddrManUncorrupted subclass
It doesn't do anything different from the base AddrMan class.
2021-11-09 17:09:50 +00:00
John Newbery
dfbd3a6d71 [addrman] [tests] Remove AddrManCorrupted subclass
It's only used to create a corrupted peers.dat file. We can do that directly
in a pure function.
2021-11-09 17:09:50 +00:00
John Newbery
d02098d1f0 [addrman] [tests] Tidy up unused arguments in addrman test functions 2021-11-09 17:09:50 +00:00
John Newbery
7784a9a374 [addrman] [tests] Remove deterministic argument and member from AddrManTest
It's always set to true.
2021-11-09 17:09:50 +00:00
MarcoFalke
faa3ec2304
span: Add std::byte helpers
Also, add Span<std::byte> interface to strencondings.
2021-11-09 17:42:13 +01:00
MarcoFalke
fabe18d0b3
Use value_type in CDataStream where possible
Also, simplify unit tests with the CDataStream::str method.
2021-11-09 17:41:21 +01:00
W. J. van der Laan
e70fb87a4f
Merge bitcoin/bitcoin#23381: validation/refactor: refactoring for package submission
14cd7bf793 [test] call CheckPackage for package sanitization checks (glozow)
6876378365 MOVEONLY: move package unit tests to their own file (glozow)
c9b1439ca9 MOVEONLY: mempool checks to their own functions (glozow)
9e910d8152 scripted-diff: clean up MemPoolAccept aliases (glozow)
fd92b0c398 document workspace members (glozow)
3d3e4598b6 [validation] cache iterators to mempool conflicts (glozow)
36a8441912 [validation/rpc] cache + use vsize calculated in PreChecks (glozow)
8fa2936b34 [validation] re-introduce bool for whether a transaction is RBF (glozow)
cbb3598b5c [validation/refactor] store precomputed txdata in workspace (glozow)
0a79eaba72 [validation] case-based constructors for ATMPArgs (glozow)

Pull request description:

  This contains the refactors and moves within #22674. There are no behavior changes, so it should be simpler to review.

ACKs for top commit:
  ariard:
    Code Review ACK 14cd7bf
  jnewbery:
    Code review ACK 14cd7bf793
  laanwj:
    Code review ACK 14cd7bf793, thanks for adding documentation and clarifying the code
  t-bast:
    Code Review ACK 14cd7bf793

Tree-SHA512: 580ed48b43713a3f9d81cd9b573ef6ac44efe5df2fc7b7b7036c232b52952b04bf5ea92940cf73739f4fbd54ecf980cef58032e8a2efe05229ad0b3c639de8a0
2021-11-09 16:46:23 +01:00
W. J. van der Laan
8346004ac8
Merge bitcoin/bitcoin#23077: Full CJDNS support
420695c193 contrib: recognize CJDNS seeds as such (Vasil Dimov)
f9c28330a0 net: take the first 4 random bits from CJDNS addresses in GetGroup() (Vasil Dimov)
29ff79c0a2 net: relay CJDNS addresses even if we are not connected to CJDNS (Vasil Dimov)
d96f8d304c net: don't skip CJDNS from GetNetworkNames() (Vasil Dimov)
c2d751abba net: take CJDNS into account in CNetAddr::GetReachabilityFrom() (Vasil Dimov)
9b43b3b257 test: extend feature_proxy.py to test CJDNS (Vasil Dimov)
508eb258fd test: remove default argument of feature_proxy.py:node_test() (Vasil Dimov)
6387f397b3 net: recognize CJDNS addresses as such (Vasil Dimov)
e6890fcb44 net: don't skip CJDNS from GetNetworksInfo() (Vasil Dimov)
e9d90d3c11 net: introduce a new config option to enable CJDNS (Vasil Dimov)
78f456c576 net: recognize CJDNS from ParseNetwork() (Vasil Dimov)
de01e312b3 net: use -proxy for connecting to the CJDNS network (Vasil Dimov)
aedd02ef27 net: make it possible to connect to CJDNS addresses (Vasil Dimov)

Pull request description:

  CJDNS overview
  =====

  CJDNS is like a distributed, shared VPN with multiple entry points where every participant can reach any other participant. All participants use addresses from the `fc00::/8` network (reserved IPv6 range). Installation and configuration is done outside of applications, similarly to VPN (either in the host/OS or on the network router).

  Motivation
  =====

  Even without this PR it is possible to connect two Bitcoin Core nodes through CJDNS manually by using e.g. `-addnode` in environments where CJDNS is set up. However, this PR is necessary for address relay to work properly and automatic connections to be made to CJDNS peers. I.e. to make CJDNS a first class citizen network like IPv4, IPv6, Tor and I2P.

  Considerations
  =====

  An address from the `fc00::/8` network, could mean two things:
  1. Part of a local network, as defined in RFC 4193. Like `10.0.0.0/8`. Bitcoin Core could be running on a machine with such address and have peers with those (e.g. in a local network), but those addresses are not relayed to other peers because they are not globally routable on the internet.
  2. Part of the CJDNS network. This is like Tor or I2P - if we have connectivity to that network then we could reach such peers and we do relay them to other peers.

  So, Bitcoin Core needs to be able to tell which one is it when it encounters a bare `fc00::/8` address, e.g. from `-externalip=` or by looking up the machine's own addresses. Thus a new config option is introduced `-cjdnsreacable`:
  * `-cjdnsreacable=0`: it is assumed a `fc00::/8` address is a private IPv6 (1.)
  * `-cjdnsreacable=1`: it is assumed a `fc00::/8` address is a CJDNS one (2.)

  After setting up CJDNS outside of Bitcoin Core, a node operator only needs to enable this option.
  Addresses from P2P relay/gossip don't need that because they are properly tagged as IPv6 or as CJDNS.

  For testing
  =====
  ```
  [fc32:17ea:e415:c3bf:9808:149d:b5a2:c9aa]:8333
  [fc68:7026:cb27:b014:5910:e609:dcdb:22a2]:8333
  [fcb3:dc50:e1ae:7998:7dc0:7fa6:4582:8e46]:8333
  [fcc7:be49:ccd1:dc91:3125:f0da:457d:8ce]:8333
  [fcf2:d9e:3a25:4eef:8f84:251b:1b4d:c596]:8333
  ```

ACKs for top commit:
  dunxen:
    ACK 420695c
  jonatack:
    re-ACK 420695c193 per `git range-diff 23ae793 4fbff39 420695c`
  laanwj:
    Code review ACK 420695c193

Tree-SHA512: 21559886271aa84671d52b120fa3fa5a50fdcf0fcb26e5b32049c56fab0d606438d19dd366a9c8ce612d3894237ae6d552ead3338b326487e3534399b88a317a
2021-11-08 14:44:37 +01:00
W. J. van der Laan
aecc08f62e
Merge bitcoin/bitcoin#23409: refactor: Take Span in SetSeed
fa93ef5a8a refactor: Take Span in SetSeed (MarcoFalke)

Pull request description:

  This makes calling code less verbose and less fragile. Also, by adding
  the CKey::data() member function, it is now possible to call HexStr()
  with a CKey object.

ACKs for top commit:
  sipa:
    utACK fa93ef5a8a
  laanwj:
    Code review ACK fa93ef5a8a
  theStack:
    Code-review ACK fa93ef5a8a

Tree-SHA512: 73fb999320719ad4b9ab5544018a7a083d140545c2807ee3582ecf7f441040a30b5157e85790b6b840af82f002a7faf30bd8162ebba5caaf2067391c43dc7e25
2021-11-08 12:48:25 +01:00
glozow
14cd7bf793 [test] call CheckPackage for package sanitization checks
Makes the test more minimal. We're just trying to test that our package
sanitization logic is correct. Now that this code lives in its own
function (rather than inside of AcceptMultipleTransactions), there's no
need to call ProcessNewPackage to test this.
2021-11-04 14:55:12 -04:00
glozow
6876378365 MOVEONLY: move package unit tests to their own file 2021-11-04 14:55:12 -04:00
Martin Leitner-Ankerl
a04350b86c
Open streams_test_tmp file in temporary folder
The tests `streams_tests/streams_buffered_file` and `streams_tests/streams_buffered_file_rand`
did not use a the temporary directory provided by  `BasicTestingSetup`, so it was not possible
to execute multiple of them in parallel. This fixes that.

To reproduce, run

```sh
parallel --halt now,fail=1 './src/test/test_bitcoin --run_test=streams_tests/streams_buffered_file_rand' -- ::: {1..1000}
```

This executes the test 1000 times, one job per CPU. It works on that commit but mergebase fails quickly.
2021-11-04 18:47:14 +01:00
Samuel Dobson
24abd8312e
Merge bitcoin/bitcoin#22949: fee: Round up fee calculation to avoid a lower than expected feerate
80dc829be7 tests: Calculate fees more similarly to CFeeRate::GetFee (Andrew Chow)
ce2cc44afd tests: Test for assertion when feerate is rounded down (Andrew Chow)
0fbaef9676 fees: Always round up fee calculated from a feerate (Andrew Chow)

Pull request description:

  When calculating the fee for a feerate, it is possible that the final calculation will have fractional satoshis. Currently those are ignored via truncation which results in the absolute fee being rounded down. Rounding down is problematic because it results in a feerate that is slightly lower than the feerate represented by the `CFeeRate` object. A slightly lower feerate particularly causes issues for coin selection as it can trigger an assertion error. To avoid potentially underpaying the feerate (and the assertion), always round up the calculated fee.

  A test is added for the assertion, along with a comment explaining what happens.

  It is unlikely that a user can trigger this as it requires a very specific set of rounding errors to occur as well as the transaction not needing any change and being right on the lower bound of the exact match window. However I was able to trigger the assertion while running coin selection simulations, albeit after thousands of transactions and with some weird feerates.

ACKs for top commit:
  ryanofsky:
    Code review ACK 80dc829be7
  promag:
    Tested ACK 80dc829be7.
  lsilva01:
    tACK 80dc829
  meshcollider:
    utACK 80dc829be7

Tree-SHA512: fe26684c60f236cab48ea6a4600c141ce766dbe59504ec77595dcbd7fd0b34559acc617007f4f499c9155d8fda0a336954413410ba862b19c765c0cfac79d642
2021-11-05 00:08:00 +13:00
John Newbery
2c64270bbe [refactor] Don't call AcceptToMemoryPool() from outside validation.cpp 2021-11-03 14:34:41 +00:00
Vasil Dimov
78f456c576
net: recognize CJDNS from ParseNetwork()
This allows to use "cjdns" as an argument to the `getnodeaddresses` RPC
and to the `-onlynet=` parameter.
2021-11-03 14:41:14 +01:00
John Newbery
5759fd12b8 [test] Don't set bypass_limits to true in txvalidation_tests.cpp
AcceptToMemoryPool() is called for an invalid coinbase transaction, so
setting bypass_limits to true or false has no impact on the test.

The only way that changing bypass_limits from true to false could change
the result would be to change the outcome to INVALID(TX_MEMPOOL_POLICY).
Since the ATMP call in this test results in INVALID(TX_CONSENSUS) both
before and after this change, there is no change in behavior.
2021-11-03 12:04:49 +00:00
John Newbery
497c9e2964 [test] Don't set bypass_limits to true in txvalidationcache_tests.cpp
AcceptToMemoryPool() is called for transactions with fees above
minRelayTxFee and with the mempool not full, so setting bypass_limits to
true or false has no impact on the test.

The only way that changing bypass_limits from true to false could change
the result would be to change the outcome to INVALID(TX_MEMPOOL_POLICY).
Since all the ATMP calls in this test result in VALID both before and
after this change, there is no change in behavior.
2021-11-03 12:04:46 +00:00
MarcoFalke
3c4729a515
Merge bitcoin/bitcoin#23223: Disable lock contention logging in checkqueue_tests
6ae9f1cf96 Disable lock contention logging in checkqueue_tests (Jon Atack)

Pull request description:

  This patch disables lock contention logging in the checkqueue_tests as some of these tests are designed to be heavily contested to trigger race conditions or other issues. This created very large log files when run with DEBUG_LOCKCONTENTION defined (up to v22) or with lock logging enabled by default in current master.

  Examples running the following command:

  ```
  $ ./src/test/test_bitcoin -t checkqueue_tests/test_CheckQueue_Correct_Random -- DEBUG_LOG_OUT > testlog.txt

  -rw-r--r--   87042178 Oct  8 12:41 testlog-with-DEBUG_LOCKCONTENTION-at-v22-run1.txt
  -rw-r--r--   73879896 Oct  8 12:42 testlog-with-DEBUG_LOCKCONTENTION-at-v22-run2.txt
  -rw-r--r--   65150518 Oct  8 12:51 testlog-with-DEBUG_LOCKCONTENTION-at-bb9f76a-run1.txt
  -rw-r--r--   65774554 Oct  8 12:52 testlog-with-DEBUG_LOCKCONTENTION-at-bb9f76a-run2.txt
  -rw-r--r--   73493309 Oct  8 13:00 testlog-current-master-at-991753e-run1.txt
  -rw-r--r--   65616977 Oct  8 13:01 testlog-current-master-at-991753e-run2.txt
  -rw-r--r--       5093 Oct  8 13:04 testlog-with-this-commit-run1.txt
  -rw-r--r--       5093 Oct  8 13:05 testlog-with-this-commit-run2.txt
  ```

  Resolves #23167.

ACKs for top commit:
  vasild:
    ACK 6ae9f1cf96

Tree-SHA512: b16812ed60c58a1cf40c04ebeca9197ac076b2415f71673ac7bb5b7960a1ff80ba2c909345ad221c7689b0562d17f63a32a629f5d6dbcf0e57130bf5760388c1
2021-11-02 20:54:19 +01:00