Commit graph

2867 commits

Author SHA1 Message Date
MarcoFalke
a22653a636
Merge #21371: fuzz: fix gcc Woverloaded-virtual build warnings
36aa2955b8 fuzz: fix gcc Woverloaded-virtual build warnings (Jon Atack)

Pull request description:

  Possible fixup to gcc build warnings since merge of b22d4c1607. Closes #21369.

ACKs for top commit:
  practicalswift:
    cr ACK 36aa2955b8: patch looks correct
  achow101:
    ACK 36aa2955b8
  kristapsk:
    ACK 36aa2955b8, this fixes compiler warnings for me with GCC 9.3.0.

Tree-SHA512: b6c99690ff72b809ce8105696744546252691b618f54311a9d930d9975fc692071ef408450f618fbb4aa99ee5390028a6eabbc968e22b2e8d2bd56bbafef49f8
2021-03-08 08:23:10 +01:00
MarcoFalke
8c049fe9af
Merge #19771: net: Replace enum CConnMan::NumConnections with enum class ConnectionDirection
c77de622dd net: Replace enum CConnMan::NumConnections with enum class ConnectionDirection (Luke Dashjr)

Pull request description:

  Refactor split out of #17167

ACKs for top commit:
  practicalswift:
    cr ACK c77de622dd: patch looks correct & `enum class` is strictly better

Tree-SHA512: 40a1bf69d8ab2651b04ba6adbab789369a5a1a29a64ba764c3e6aab575b7943ea8dfd6e35b0abf5bcffa10e7265f4b523a93aa899c0fd581a84fc51ae5377b90
2021-03-07 14:21:02 +01:00
MarcoFalke
1020b04c39
Merge #21334: test: Additional (refactored) BIP9 tests
0c471a5f30 tests: check never active versionbits (Anthony Towns)
3ba9283a47 tests: more helpful errors for failing versionbits tests (Anthony Towns)

Pull request description:

  Extracted from #19573 to make review easier. I also reviewed it myself.

  I added some comments to the test: bae9a45219 (r585486781)

  I also moved some `TestState` changes from the second to the first commit, to reduce the latter diff.

ACKs for top commit:
  ajtowns:
    ACK 0c471a5f30
  MarcoFalke:
    review ACK 0c471a5f30 🔓

Tree-SHA512: 61f8d1ecaf38a6cd13db1cf71c89b8c4d2f5852ef77c5e7ecb9bd78eb216545037411641bb101cf0740c5c47845ac327954ee25b676d63779c5f148719ac5caf
2021-03-07 12:47:50 +01:00
Jon Atack
36aa2955b8
fuzz: fix gcc Woverloaded-virtual build warnings 2021-03-05 22:30:00 +01:00
practicalswift
3f3646855c fuzz: Avoid -Wreturn-type warnings 2021-03-04 20:29:28 +00:00
Luke Dashjr
c77de622dd net: Replace enum CConnMan::NumConnections with enum class ConnectionDirection 2021-03-04 19:54:17 +00:00
Wladimir J. van der Laan
702cfc8c53
Merge #21055: [Bundle 3/n] Prune remaining g_chainman usage in validation functions
e11b649650 validation: CVerifyDB::VerifyDB: Use locking annotation (Carl Dong)
03f75c42e1 validation: Use existing chain member in CChainState::LoadGenesisBlock (Carl Dong)
5e4af77380 validation: Use existing chain member in CChainState::AcceptBlock (Carl Dong)
fee73347c0 validation: Pass in chain to FindBlockPos+SaveBlockToDisk (Carl Dong)
a9d28bcd8d validation: Use *this in CChainState::ActivateBestChainStep (Carl Dong)
4744efc9ba validation: Pass in chainstate to CTxMemPool::check (Carl Dong)
1fb7b2c595 validation: Use *this in CChainState::InvalidateBlock (Carl Dong)
8cdb2f7e58 validation: Move LoadBlockIndexDB to CChainState (Carl Dong)
8b99efbcc0 validation: Move invalid block handling to CChainState (Carl Dong)
2bdf37fe18 validation: Pass in chainstate to CVerifyDB::VerifyDB (Carl Dong)
31eac50c72 validation: Remove global ::VersionBitsTip{State,SinceHeight,Statistics} (Carl Dong)
63e4c7316a validation: Pass in chainstate to ::PruneBlockFilesManual (Carl Dong)
4bada76237 validation: Pass in chainstate to UpdateTip (Carl Dong)
a3ba08ba7d validation: Remove global ::{{Precious,Invalidate}Block,ResetBlockFailureFlags} (Carl Dong)
4927c9e699 validation: Remove global ::LoadGenesisBlock (Carl Dong)
9da106be4d validation: Check chain tip is non-null in CheckFinalTx (Carl Dong)

Pull request description:

  Overall PR: #20158 (tree-wide: De-globalize ChainstateManager)

  Based on:
  - [x] #20750 | [Bundle 2/n] Prune g_chainman usage in mempool-related validation functions

  Note to reviewers:
  1. This bundle may _apparently_ introduce usage of `g_chainman` or `::Chain(state|)Active()` globals, but these are resolved later on in the overall PR. [Commits of overall PR](https://github.com/bitcoin/bitcoin/pull/20158/commits)
  2. There may be seemingly obvious local references to `ChainstateManager` or other validation objects which are not being used in callers of the current function in question, this is done intentionally to **_keep each commit centered around one function/method_** to ease review and to make the overall change systematic. We don't assume anything about our callers. Rest assured that once we are considering that particular caller in later commits, we will use the obvious local references. [Commits of overall PR](https://github.com/bitcoin/bitcoin/pull/20158/commits)
  3. When changing a function/method that has many callers (e.g. `LookupBlockIndex` with 55 callers), it is sometimes easier (and less error-prone) to use a scripted-diff. When doing so, there will be 3 commits in sequence so that every commit compiles like so:
  1. Add `new_function`, make `old_function` a wrapper of `new_function`, divert all calls to `old_function` to `new_function` **in the local module only**
  2. Scripted-diff to divert all calls to `old_function` to `new_function` **in the rest of the codebase**
  3. Remove `old_function`

  Note to self:
  - [x] Address: https://github.com/bitcoin/bitcoin/pull/20750#discussion_r579400663

ACKs for top commit:
  laanwj:
    Code review ACK e11b649650

Tree-SHA512: 205a451a741e32f17d5966de289f2f5a3f0817738c0087b70ff4755ddd217b53d01050ed396669bda2b1d216a88d927b9778777f9ff95ab1fe20e59c5f341776
2021-03-04 14:55:47 +01:00
fanquake
83bdbbd300
Merge #21003: test: Move MakeNoLogFileContext to libtest_util, and use it in bench
fa576b4532 Move MakeNoLogFileContext to common libtest_util, and use it in bench (MarcoFalke)

Pull request description:

  To avoid verbose code duplication, which may lead to accidental mishaps https://github.com/bitcoin/bitcoin/pull/20998/files#r563624041.

  Also fix a nit I found in https://github.com/bitcoin/bitcoin/pull/20946#discussion_r561949731.

ACKs for top commit:
  dongcarl:
    Light Code-Review ACK fa576b4532
  fanquake:
    ACK fa576b4532

Tree-SHA512: d39ac9c0957813ebb20ed13bd25a8ef8469377ce2651245638bd761c796feac2a17e30dd16f1e5c8db57737fb918c81d56a3d784c33258275e426a1b11e69eb2
2021-03-04 20:29:09 +08:00
fanquake
7450a01691
Merge #21358: fuzz: Add missing include (test/util/setup_common.h)
fa59ad5130 fuzz: Add missing include (test/util/setup_common.h) (MarcoFalke)

Pull request description:

  `src/test/fuzz/socks5.cpp` is using the symbol `BasicTestingSetup`, which is defined in `src/test/util/setup_common.h`.

  Currently compilation happens to succeed because the needed dependency is indirectly included. Compilation will break as soon as the indirect dependency is broken. According to the dev notes, everything that is used must be included.

  Fix the issue by including the missing include.

ACKs for top commit:
  fanquake:
    ACK fa59ad5130

Tree-SHA512: 9359d5d288ebc5a53d753ebed1ee8d49ddcfe12aeb56054ea43654c0d915337bb0dce7c8a7178e94711ff8dacd1b3ea0a2871b21b1709cd9786efc0c1ef532b3
2021-03-04 20:20:43 +08:00
fanquake
33921379b6
Merge #21015: Make all of net_processing (and some of net) use std::chrono types
0eaea66e8b Make tx relay data structure use std::chrono types (Pieter Wuille)
55e82881a1 Make all Poisson delays use std::chrono types (Pieter Wuille)
c733ac4d8a Convert block/header sync timeouts to std::chrono types (Pieter Wuille)
4d98b401fb Change all ping times to std::chrono types (Pieter Wuille)

Pull request description:

  (Picking up #20044. Rebased against master.)

  This changes various uses of integers to represent timestamps and durations to `std::chrono` duration types with type-safe conversions, getting rid of various `.count()`, constructors, and conversion factors.

ACKs for top commit:
  jnewbery:
    utACK 0eaea66e8b
  vasild:
    ACK 0eaea66e8b
  MarcoFalke:
    re-ACK 0eaea66e8b, only changes: minor rename, using C++11 member initializer, using 2min chrono literal, rebase 🤚
  ajtowns:
    utACK 0eaea66e8b

Tree-SHA512: 2dbd8d53bf82e98f9b4611e61dc14c448e8957d1a02575b837fadfd59f80e98614d0ccf890fc351f960ade76a6fb8051b282e252e81675a8ee753dba8b1d7f57
2021-03-04 20:13:43 +08:00
Wladimir J. van der Laan
92b7efcf54
Merge #21148: Split orphan handling from net_processing into txorphanage
5e50e2d1b9 txorphanage: comment improvements (Anthony Towns)
eeeafb324e net_processing: move AddToCompactExtraTransactions into PeerManagerImpl (Anthony Towns)
f8c0688b94 scripted-diff: Update txorphanage naming convention (Anthony Towns)
6bd4963c06 txorphanage: Move functions and data into class (Anthony Towns)
03257b832d txorphanage: Extract EraseOrphansForBlock (Anthony Towns)
3c4c3c2fdd net_processing: drop AddOrphanTx (Anthony Towns)
26d1a6ccd5 denialofservices_tests: check txorphanage's AddTx (Anthony Towns)
1041616d7e txorphanage: Extract OrphanageAddTx (Anthony Towns)
f294da7274 txorphanage: Extract GetOrphanTx (Anthony Towns)
83679ffc60 txorphanage: Extract HaveOrphanTx (Anthony Towns)
ee135c8d5b txorphanage: Extract AddChildrenToWorkSet (Anthony Towns)
38a11c355a txorphanage: Add lock annotations (Anthony Towns)
81dd57e5b1 txorphanage: Pass uint256 by reference instead of value (Anthony Towns)
9d5313df7e move-only: Add txorphanage module (Anthony Towns)

Pull request description:

  Splits orphan handling into its own module and reduces global usage.

ACKs for top commit:
  jnewbery:
    utACK 5e50e2d1b9
  amitiuttarwar:
    utACK 5e50e2d1b9
  glozow:
    re ACK 5e50e2d1b9, comment updates
  laanwj:
    Code review ACK 5e50e2d1b9

Tree-SHA512: 92a959bb5dd414c96f78cb8dcaa68adb85faf16b8b843a2cbe0bb2aa08df13ad6bd9424d29b98f57a82ec29c942fbdbea3011883d00bf0b0feb643e295174e46
2021-03-04 10:16:38 +01:00
MarcoFalke
fa59ad5130
fuzz: Add missing include (test/util/setup_common.h) 2021-03-04 08:39:27 +01:00
Wladimir J. van der Laan
47b99ab1a9
Merge #20406: util: Avoid invalid integer negation in FormatMoney and ValueFromAmount
1f05dbd06d util: Avoid invalid integer negation in ValueFromAmount: make ValueFromAmount(const CAmount& n) well-defined also when n is std::numeric_limits<CAmount>::min() (practicalswift)
7cc75c9ba3 util: Avoid invalid integer negation in FormatMoney: make FormatMoney(const CAmount& n) well-defined also when n is std::numeric_limits<CAmount>::min() (practicalswift)

Pull request description:

  Avoid invalid integer negation in `FormatMoney` and `ValueFromAmount`.

  Fixes #20402.

  Before this patch:

  ```
  $ CC=clang CXX=clang++ ./configure --with-sanitizers=undefined
  $ make -C src/ test/test_bitcoin
  $ src/test/test_bitcoin -t rpc_tests/rpc_format_monetary_values -t util_tests/util_FormatMoney
  core_write.cpp:21:29: runtime error: negation of -9223372036854775808 cannot be represented in type 'CAmount'
    (aka 'long'); cast to an unsigned type to negate this value to itself
  SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior core_write.cpp:21:29 in
  test/rpc_tests.cpp(186): error: in "rpc_tests/rpc_format_monetary_values":
    check ValueFromAmount(std::numeric_limits<CAmount>::min()).write() == "-92233720368.54775808" has failed
    [--92233720368.-54775808 != -92233720368.54775808]
  util/moneystr.cpp:16:34: runtime error: negation of -9223372036854775808 cannot be represented in type 'CAmount'
    (aka 'long'); cast to an unsigned type to negate this value to itself
  SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior util/moneystr.cpp:16:34 in
  test/util_tests.cpp(1188): error: in "util_tests/util_FormatMoney":
    check FormatMoney(std::numeric_limits<CAmount>::min()) == "-92233720368.54775808" has failed
    [--92233720368.-54775808 != -92233720368.54775808]
  ```

  After this patch:

  ```
  $ CC=clang CXX=clang++ ./configure --with-sanitizers=undefined
  $ make -C src/ test/test_bitcoin
  $ src/test/test_bitcoin -t rpc_tests/rpc_format_monetary_values -t util_tests/util_FormatMoney
  ```

ACKs for top commit:
  laanwj:
    re-ACK 1f05dbd06d

Tree-SHA512: 5aaeb8e2178f1597921f53c12bdfc2f3d5993d10c41658dcd25943e54e8cc2116a411bc71d928f890b33bc0b3761a8ee4449b0532bce41125b6c60692808c8c3
2021-03-03 19:04:36 +01:00
Pieter Wuille
55e82881a1 Make all Poisson delays use std::chrono types 2021-03-03 09:48:07 -08:00
Pieter Wuille
4d98b401fb Change all ping times to std::chrono types 2021-03-03 09:48:07 -08:00
MarcoFalke
ebd8d66454
Merge #19203: net: Add regression fuzz harness for CVE-2017-18350. Add FuzzedSocket.
366e3e1f89 fuzz: Add FUZZED_SOCKET_FAKE_LATENCY mode to FuzzedSock to allow for fuzzing timeout logic (practicalswift)
b22d4c1607 fuzz: Add fuzzing harness for Socks5(...) (practicalswift)

Pull request description:

  Add [regression fuzz harness](https://twitter.com/kayseesee/status/1205287895923212289) for CVE-2017-18350. This fuzzing harness would have found CVE-2017-18350 within a minute of fuzzing :)

  See [`doc/fuzzing.md`](https://github.com/bitcoin/bitcoin/blob/master/doc/fuzzing.md) for information on how to fuzz Bitcoin Core. Don't forget to contribute any coverage increasing inputs you find to the [Bitcoin Core fuzzing corpus repo](https://github.com/bitcoin-core/qa-assets).

  Happy fuzzing :)

ACKs for top commit:
  vasild:
    ACK 366e3e1f89

Tree-SHA512: 5d8e1863b635efd10ccb11678b71472ba1523c3ef16affa7f9cd638635c1a9c307e28f432d5b87eb0c9cd1c3c1aeafbb24fa7ae86fe4e5090fda2e20d542b6ca
2021-03-03 14:41:05 +01:00
Anthony Towns
0c471a5f30
tests: check never active versionbits 2021-03-03 14:14:40 +01:00
Anthony Towns
3ba9283a47
tests: more helpful errors for failing versionbits tests
Co-authored-by: Sjors Provoost <sjors@sprovoost.nl>
2021-03-03 14:12:16 +01:00
MarcoFalke
fa576b4532
Move MakeNoLogFileContext to common libtest_util, and use it in bench
Can be reviewed with --color-moved=dimmed-zebra
2021-03-03 09:17:37 +01:00
practicalswift
366e3e1f89 fuzz: Add FUZZED_SOCKET_FAKE_LATENCY mode to FuzzedSock to allow for fuzzing timeout logic 2021-03-02 21:44:51 +00:00
practicalswift
b22d4c1607 fuzz: Add fuzzing harness for Socks5(...) 2021-03-02 21:43:42 +00:00
practicalswift
1f05dbd06d util: Avoid invalid integer negation in ValueFromAmount: make ValueFromAmount(const CAmount& n) well-defined also when n is std::numeric_limits<CAmount>::min() 2021-03-02 16:05:28 +00:00
practicalswift
7cc75c9ba3 util: Avoid invalid integer negation in FormatMoney: make FormatMoney(const CAmount& n) well-defined also when n is std::numeric_limits<CAmount>::min() 2021-03-02 16:05:28 +00:00
practicalswift
10d4477dae tests: Add fuzzing harness for TorController 2021-03-02 12:21:32 +00:00
Vasil Dimov
cff65c4a27
net: extend CNetAddr::SetSpecial() to support I2P
Recognize also I2P addresses in the form `base32hashofpublickey.b32.i2p`
from `CNetAddr::SetSpecial()`.

This makes `Lookup()` support them, which in turn makes it possible to
manually connect to an I2P node by using
`-proxy=i2p_socks5_proxy:port -addnode=i2p_address.b32.i2p:port`

Co-authored-by: Lucas Ontivero <lucasontivero@gmail.com>
2021-03-01 13:22:11 +01:00
Anthony Towns
f8c0688b94 scripted-diff: Update txorphanage naming convention
-BEGIN VERIFY SCRIPT-
sed -i 's/mapOrphanTransactionsByPrev/m_outpoint_to_orphan_it/g' src/txorphanage.h src/txorphanage.cpp
sed -i 's/mapOrphanTransactions/m_orphans/g' src/txorphanage.h src/txorphanage.cpp src/net_processing.cpp src/test/denialofservice_tests.cpp
sed -i 's/g_orphan_list/m_orphan_list/g' src/txorphanage.h src/txorphanage.cpp
sed -i 's/g_orphans_by_wtxid/m_wtxid_to_orphan_it/g' src/txorphanage.h src/txorphanage.cpp
sed -i 's/nMaxOrphans/max_orphans/g' src/txorphanage.h src/txorphanage.cpp
sed -i 's/COrphanTx/OrphanTx/g' src/txorphanage.h src/txorphanage.cpp src/test/denialofservice_tests.cpp
-END VERIFY SCRIPT-
2021-02-27 01:08:09 +10:00
Anthony Towns
6bd4963c06 txorphanage: Move functions and data into class
Collects all the orphan handling globals into a single member var in
net_processing, and ensures access is encapuslated into the interface
functions. Also adds doxygen comments for methods.
2021-02-27 01:07:55 +10:00
Anthony Towns
26d1a6ccd5 denialofservices_tests: check txorphanage's AddTx
Rather than checking net_processing's internal implementation of
AddOrphanTx, test txorphanage's exported AddTx interface. Note that
this means AddToCompactExtraTransactions is no longer tested here.
2021-02-26 23:55:10 +10:00
Anthony Towns
9d5313df7e move-only: Add txorphanage module
This module captures orphan tracking code for tx relay.

Can be reviewed with --color-moved=dimmed-zebra
2021-02-26 23:55:03 +10:00
MarcoFalke
cac10e66d2
Merge #21264: fuzz: Two scripted diff renames
fae216a73d scripted-diff: Rename MakeFuzzingContext to MakeNoLogFileContext (MarcoFalke)
fa4fbec03e scripted-diff: Rename PROVIDE_MAIN_FUNCTION -> PROVIDE_FUZZ_MAIN_FUNCTION (MarcoFalke)

Pull request description:

  Split out two renames from #21003:

  * `PROVIDE_FUZZ_MAIN_FUNCTION`. *Reason*: This in only used by fuzzing, so the name should indicate that.
  * `MakeNoLogFileContext`. *Reason*: Better reflects what the helper does. Also, prepares it to be used in non-fuzz tests in the future.

ACKs for top commit:
  practicalswift:
    cr ACK fae216a73d: scripted-diff looks correct

Tree-SHA512: e5d347746f5da72b0c86fd4f07ac2e4b3016e88e8c97a830c73bd79d0af6d0245fe7712487fc20344d6cc25958941716c1678124a123930407e3a437265b71df
2021-02-25 14:42:48 +01:00
Hennadii Stepanov
df8f2a11dc
test: Replace accidentally placed bit-OR with logical-OR 2021-02-24 12:57:03 +02:00
Wladimir J. van der Laan
a9335e4f12
Merge #16546: External signer support - Wallet Box edition
f75e0c1edd doc: add external-signer.md (Sjors Provoost)
d4b0107d68 rpc: send: support external signer (Sjors Provoost)
245b4457cf rpc: signerdisplayaddress (Sjors Provoost)
7ebc7c0215 wallet: ExternalSigner: add GetDescriptors method (Sjors Provoost)
fc5da520f5 wallet: add GetExternalSigner() (Sjors Provoost)
259f52cc33 test: external_signer wallet flag is immutable (Sjors Provoost)
2655197e1c rpc: add external_signer option to createwallet (Sjors Provoost)
2700f09c41 rpc: signer: add enumeratesigners to list external signers (Sjors Provoost)
07b7c940a7 rpc: add external signer RPC files (Sjors Provoost)
8ce7767071 wallet: add ExternalSignerScriptPubKeyMan (Sjors Provoost)
157ea7c614 wallet: add external_signer flag (Sjors Provoost)
f3e6ce78fb test: add external signer test (Sjors Provoost)
8cf543f96d wallet: add -signer argument for external signer command (Sjors Provoost)
f7eb7ecc67 test: framework: add skip_if_no_external_signer (Sjors Provoost)
87a97941f6 configure: add --enable-external-signer (Sjors Provoost)

Pull request description:

  Big picture overview in [this gist](https://gist.github.com/Sjors/29d06728c685e6182828c1ce9b74483d).

  This PR lets `bitcoind` call an arbitrary command `-signer=<cmd>`, e.g. a hardware wallet driver,  where it can fetch public keys, ask to display an address, and sign a transaction (using PSBT under the hood).

  It's design to work with https://github.com/bitcoin-core/HWI, which supports multiple hardware wallets. Any command with the same arguments and return values will work. It simplifies the manual procedure described [here](https://github.com/bitcoin-core/HWI/blob/master/docs/bitcoin-core-usage.md).

  Usage is documented in [doc/external-signer.md](
  https://github.com/Sjors/bitcoin/blob/2019/08/hww-box2/doc/external-signer.md), which also describes what protocol a different signer binary should conform to.

  Use `--enable-external-signer` to opt in, requires Boost::Process:

  ```
  Options used to compile and link:
    with wallet     = yes
    with gui / qt   = no
    external signer = yes
  ```

  It adds the following RPC methods:
  * `enumeratesigners`: asks <cmd> for a list of signers (e.g. devices) and their master key fingerprint
  * `signerdisplayaddress <address>`:  asks <cmd> to display an address

  It enhances the following RPC methods:
  * `createwallet`: takes an additional `external_signer` argument and fetches keys from device
  * `send`: automatically sends transaction to device and waits

  Usage TL&DR:
  * clone HWI repo somewhere and launch `bitcoind -signer=../HWI/hwi.py`
  * check if you can see your hardware device: `bitcoin-cli enumeratesigners`
  * create wallet and auto import keys `bitcoin-cli createwallet "hww" true true "" true true true`
  * display address on device: `bitcoin-cli signerdisplayaddress ...`
  * to spend, use `send` RPC and approve transaction on device

  Prerequisites:
  - [x] #21127 load wallet flags before everything else
  - [x] #21182 remove mostly pointless BOOST_PROCESS macro

  Potentially useful followups:
  - GUI support: bitcoin-core/gui#4
  - bumpfee support
  - (automatically) verify (a subset of) keys on the device after import, through message signing

ACKs for top commit:
  laanwj:
    re-ACK f75e0c1edd

Tree-SHA512: 7db8afd54762295c1424c3f01d8c587ec256a72f34bd5256e04b21832dabd5dc212be8ab975ae3b67de75259fd569a561491945750492f417111dc7b6641e77f
2021-02-23 17:56:43 +01:00
Wladimir J. van der Laan
c263c3d7d2
Merge #19698: test: apply strict verification flags for transaction tests and assert backwards compatibility
5786a818e1 Verify that all validation flags are backward compatible (gzhao408)
b10ce9aa48 [test] check verification flags are minimal/maximal (gzhao408)
a260c22cad [test] Check for invalid flag combinations (gzhao408)
a7098a2a8d [refactor] use CheckTxScripts, TrimFlags, FillFlags (gzhao408)
7a77727b2f Apply minimal validation flags to tx_invalid tests (gzhao408)
9532591bed [test] add BADTX setting for invalid txns that fail CheckTransaction (gzhao408)
4c06ebf128 [test] fix two witness tests in invalid tests with empty vout (gzhao408)
158a0b268c Apply maximal validation flags to tx_valid tests (gzhao408)
0a76a39b63 [test] fix CSV test missing OP_ADD (gzhao408)
19db590d04 [test] remove unnecessary OP_1s from CSV and CLTV tests (gzhao408)

Pull request description:

  This uses the first 4 commits of #15045, rebased and added some comments. The diff is quite large already and I want to make it easy to review, so I'm splitting it into 2 PRs (transaction and script). Script one is WIP, I'll link it when I open it.

  Interpretation of scripts is dependent on the script verification flags passed in.
  In tests, we should always apply **maximal** verification flags when checking that a transaction is **valid**; any additional flags should invalidate the transaction. A transaction should not be valid because we forgot to include a flag, and we should apply all flags by default.
  We should apply **minimal** verification flags when asserting that a transaction is **invalid**; if verification flags are applied, removing any one of them should mean the transaction is valid.
  New verify flags must be backwards compatible; tests should check backwards compatibility and apply the new flags by default. All `tx_invalid` tests should continue to be invalid with the exact same verify flags. All `tx_valid` tests that don't pass with new flags should _explicitly_ indicate that the flags need to be excluded, and fail otherwise.

  1. Flip the meaning of `verifyFlags` in tx_valid.json to mean _excluded_ verification flags instead of included flags. Edit the test data accordingly.
  2. Trim unneeded flags from tx_invalid.json.
  3. Add check to verify that tx_valid tests have maximal flags and tx_invalid tests have minimal flags.
  4. Add checks to verify that flags are soft forks (#10699) i.e. adding any flag should only decrease the number of acceptable scripts. Test by adding/removing random flags.

ACKs for top commit:
  achow101:
    ACK 5786a818e1
  laanwj:
    ACK 5786a818e1

Tree-SHA512: 19195d8cf3299e62f47dd3443ae4a95430c5c9d497993a18ab80de9e24b1869787af972774993bf05717784879bc4592fdabaae0fddebd437963d8f3c96d9a73
2021-02-23 11:15:24 +01:00
Carl Dong
4927c9e699 validation: Remove global ::LoadGenesisBlock 2021-02-22 11:48:39 -05:00
MarcoFalke
fae216a73d
scripted-diff: Rename MakeFuzzingContext to MakeNoLogFileContext
-BEGIN VERIFY SCRIPT-
 # Rename
 sed -i -e 's/MakeFuzzingContext/MakeNoLogFileContext/g' $(git grep -l MakeFuzzingContext)
 # Bump the copyright of touched files in this scripted diff to avoid touching them again later
 ./contrib/devtools/copyright_header.py update ./src/test/fuzz/
-END VERIFY SCRIPT-
2021-02-22 10:27:22 +01:00
MarcoFalke
fa4fbec03e
scripted-diff: Rename PROVIDE_MAIN_FUNCTION -> PROVIDE_FUZZ_MAIN_FUNCTION
-BEGIN VERIFY SCRIPT-
sed -i -e 's/PROVIDE_MAIN_FUNCTION/PROVIDE_FUZZ_MAIN_FUNCTION/g' $(git grep -l PROVIDE_MAIN_FUNCTION)
-END VERIFY SCRIPT-
2021-02-22 10:27:17 +01:00
MarcoFalke
34d7030063
Merge #21202: [validation] Two small clang lock annotation improvements
25c57d6409 [doc] Add a note about where lock annotations should go. (Amiti Uttarwar)
ad5f01b960 [validation] Move the lock annotation from function definition to declaration (Amiti Uttarwar)

Pull request description:

  Based on reviewing #21188

  the first commit switches the lock annotations on `CheckInputScripts` to be on the function declaration instead of on the function definition. this ensures that all call sites are checked, not just ones that come after the definition.

  the second commit adds a note to the developer-notes section to clarify where the annotations should be applied.

ACKs for top commit:
  MarcoFalke:
    ACK 25c57d6409 🥘
  promag:
    Code review ACK 25c57d6409.

Tree-SHA512: 61b6ef856bf6c6016d535fbdd19daf57b9e59fe54a1f30d47282a071b9b9d60b2466b044ee57929e0320cb1bdef52e7a1687cacaa27031bbc43d058ffffe22ba
2021-02-22 09:47:15 +01:00
Sjors Provoost
87a97941f6
configure: add --enable-external-signer
This option replaces --with-boost-process

This prepares external signer support to be disabled by default.
It adds a configure option to enable this feature and to check
if Boost::Process is present.

This also exposes ENABLE_EXTERNAL_SIGNER to the test suite via test/config.ini
2021-02-21 16:27:10 +01:00
MarcoFalke
828bb776d2
Merge #20750: [Bundle 2/n] Prune g_chainman usage in mempool-related validation functions
e8ae1db864 style-only: Make AcceptToMemoryPool signature readable (Carl Dong)
8f5c100064 style-only: Make CheckSequenceLock signature readable (Carl Dong)
8c824819c8 validation: Use *this in CChainState::LoadMempool (Carl Dong)
0a9a24d8c7 validation: Pass in chainstate to UpdateMempoolForReorg (Carl Dong)
7142018812 validation: Pass in chainstate to CTxMemPool::removeForReorg (Carl Dong)
71734c65dc validation: Pass in chain to ::TestLockPointValidity (Carl Dong)
120aaba9ac tree-wide: Fix erroneous AcceptToMemoryPool replacements (Carl Dong)
417dafc1ee validation: Remove old AcceptToMemoryPool w/o chainstate param (Carl Dong)
3704433c4f scripted-diff: Invoke ::AcceptToMemoryPool with chainstate (Carl Dong)
229bc37b5f validation: Pass in chainstate to ::AcceptToMemoryPool (Carl Dong)
d0da7ea57a validation: Pass in chainstate to ::LoadMempool (Carl Dong)
3a205c43dc validation: Pass in chainstate to AcceptToMemoryPoolWithTime (Carl Dong)
d8a816329c validation: Add chainstate member to MemPoolAccept (Carl Dong)
4c15942b79 validation: Pass in chainstate to ::CheckSequenceLocks (Carl Dong)
577b774d0c validation: Remove old CheckFinalTx w/o chain tip param (Carl Dong)
7031cf89db scripted-diff: Invoke ::CheckFinalTx with chain tip (Carl Dong)
d015eaa550 validation: Pass in chain tip to ::CheckFinalTx (Carl Dong)
252b489c9f validation: Pass in coins tip to CheckInputsFromMempoolAndCache (Carl Dong)
73a6d2b7be validation: Pass in chainstate to IsCurrentForFeeEstimation (Carl Dong)
d1f932b0b0 validation: Pass in coins cache to ::LimitMempoolSize (Carl Dong)

Pull request description:

  Overall PR: #20158 (tree-wide: De-globalize ChainstateManager)

  Note to reviewers:
  1. This bundle may _apparently_ introduce usage of `g_chainman` or `::Chain(state|)Active()` globals, but these are resolved later on in the overall PR. [Commits of overall PR](https://github.com/bitcoin/bitcoin/pull/20158/commits)
  2. There may be seemingly obvious local references to `ChainstateManager` or other validation objects which are not being used in callers of the current function in question, this is done intentionally to **_keep each commit centered around one function/method_** to ease review and to make the overall change systematic. We don't assume anything about our callers. Rest assured that once we are considering that particular caller in later commits, we will use the obvious local references. [Commits of overall PR](https://github.com/bitcoin/bitcoin/pull/20158/commits)
  3. When changing a function/method that has many callers (e.g. `LookupBlockIndex` with 55 callers), it is sometimes easier (and less error-prone) to use a scripted-diff. When doing so, there will be 3 commits in sequence so that every commit compiles like so:
  	1. Add `new_function`, make `old_function` a wrapper of `new_function`, divert all calls to `old_function` to `new_function` **in the local module only**
  	2. Scripted-diff to divert all calls to `old_function` to `new_function` **in the rest of the codebase**
  	3. Remove `old_function`

ACKs for top commit:
  glozow:
    reACK e8ae1db864 via `git range-diff 15f0042...e8ae1db`, only change is fixing ATMP call from conflict
  MarcoFalke:
    ACK e8ae1db864 📣

Tree-SHA512: 6af50f04940a69c5c3d3796a24f32f963fa02503cdc1155cc11fff832a99172b407cd163a19793080a5af98580f051b48195b62ec4a797ba2763b4883174153d
2021-02-20 09:21:45 +01:00
MarcoFalke
f1c339d452
Merge #21211: test: Move P2WSH_OP_TRUE to shared test library
22220ef6d5 test: Move P2WSH_OP_TRUE to shared test library (MarcoFalke)

Pull request description:

  Otherwise it can't be used in other tests (unit, fuzz, bench, ...)

ACKs for top commit:
  darosior:
    ACK 22220ef6d5

Tree-SHA512: 1b636e751281291f7c21ac51c3d014f6a565144c9482974391c516228e756442b077655eda970eb8bdb12974b97855a909b2b60d518026a8d5f41aa15ec7cbc8
2021-02-19 13:14:25 +01:00
MarcoFalke
09eb46c943
Merge #21187: Net processing: Only call PushAddress() from net_processing
3e68efa615 [net] Move checks from GetLocalAddrForPeer to caller (John Newbery)
d21d2b264c [net] Change AdvertiseLocal to GetLocalAddrForPeer (John Newbery)

Pull request description:

  This is the first part of #21186. It slightly disentangles addr handling in net/net_processing by making it explicit that net_processing is responsible for pushing addr records into `vAddrToSend`.

ACKs for top commit:
  MarcoFalke:
    re-ACK 3e68efa615 🍅

Tree-SHA512: 9af50c41f5a977e2e277f24a589db38e2980b353401def5e74b108ac5f493d9b5d6b1b8bf15323a4d66321495f04bc271450fcef7aa7d1c095f051a4f8e9b15f
2021-02-19 12:58:41 +01:00
MarcoFalke
6a680a6236
Merge #21226: build: Fix fuzz binary compilation under windows
56ace907b9 Fix fuzz binary compilation under windows (Dan Benjamin)

Pull request description:

  Small change to allow the fuzz binary to compile under windows. Also removed --disable-fuzz-binary from the windows CI test. This fixes #21212.

ACKs for top commit:
  MarcoFalke:
    review ACK 56ace907b9 the best bugfixes are the ones removing code

Tree-SHA512: 6088fd955a5e511b5ca1b3eaa8469a889eb6d994c2827acac7695dac6e4e320a344b45f4015a2f279b16df0d4b23ec4df13304ae6315395ad2fe8c5b526cada4
2021-02-19 11:30:41 +01:00
Dan Benjamin
56ace907b9 Fix fuzz binary compilation under windows 2021-02-18 17:35:24 -05:00
Carl Dong
3704433c4f scripted-diff: Invoke ::AcceptToMemoryPool with chainstate
-BEGIN VERIFY SCRIPT-
find_regex='\bAcceptToMemoryPool\(' \
    && git grep -l -E "$find_regex" -- src \
        | grep -v '^src/validation\.\(cpp\|h\)$' \
        | xargs sed -i -E 's@'"$find_regex"'@\0::ChainstateActive(), @g'
-END VERIFY SCRIPT-
2021-02-18 14:49:06 -05:00
Carl Dong
4c15942b79 validation: Pass in chainstate to ::CheckSequenceLocks 2021-02-18 14:43:28 -05:00
Carl Dong
7031cf89db scripted-diff: Invoke ::CheckFinalTx with chain tip
-BEGIN VERIFY SCRIPT-
find_regex='\bCheckFinalTx\(' \
    && git grep -l -E "$find_regex" -- src \
        | grep -v '^src/validation\.\(cpp\|h\)$' \
        | xargs sed -i -E 's@'"$find_regex"'@\0::ChainActive().Tip(), @g'
-END VERIFY SCRIPT-
2021-02-18 14:43:28 -05:00
Jonas Schnelli
bedb8d88bc Avoid comparision of integers with different signs 2021-02-18 20:34:10 +01:00
Wladimir J. van der Laan
b805dbb0b9
Merge #19809: log: Prefix log messages with function name and source code location if -logsourcelocations is set
b4511e2e2e log: Prefix log messages with function name if -logsourcelocations is set (practicalswift)

Pull request description:

  Prefix log messages with function name if `-logfunctionnames` is set.

  Yes, exactly like `-logthreadnames` but for function names instead of thread names :)

  This is a small developer ergonomics improvement: I've found this to be a cheap/simple way to correlate log output and originating function.

  For me it beats the ordinary cycle of 1.) try to figure out a regexp matching the static part of the dynamic log message, 2.) `git grep -E 'Using .* MiB out of .* requested for signature cache'`, 3.) `mcedit filename.cpp` (`openemacs filename.cpp` works too!) and 4.) search for log message and scroll up to find the function name :)

  Without any logging parameters:

  ```
  $ src/bitcoind -regtest
  2020-08-25T03:29:04Z Using RdRand as an additional entropy source
  2020-08-25T03:29:04Z Using 16 MiB out of 32/2 requested for signature cache, able to store 524288 elements
  2020-08-25T03:29:04Z Using 16 MiB out of 32/2 requested for script execution cache, able to store 524288 elements
  2020-08-25T03:29:04Z Loaded best chain: hashBestChain=0fff88f13cb7b2c71f2a335e3a4fc328bf5beb436012afca590b1a11466e22ff height=0 date=2011-02-02T23:16:42Z progress=1.000000
  2020-08-25T03:29:04Z block tree size = 1
  2020-08-25T03:29:04Z nBestHeight = 0
  2020-08-25T03:29:04Z Imported mempool transactions from disk: 0 succeeded, 0 failed, 0 expired, 0 already there, 0 waiting for initial broadcast
  2020-08-25T03:29:04Z 0 addresses found from DNS seeds
  ```

  With `-logthreadnames` and `-logfunctionnames`:

  ```
  $ src/bitcoind -regtest -logthreadnames -logfunctionnames
  2020-08-25T03:29:04Z [init] [ReportHardwareRand] Using RdRand as an additional entropy source
  2020-08-25T03:29:04Z [init] [InitSignatureCache] Using 16 MiB out of 32/2 requested for signature cache, able to store 524288 elements
  2020-08-25T03:29:04Z [init] [InitScriptExecutionCache] Using 16 MiB out of 32/2 requested for script execution cache, able to store 524288 elements
  2020-08-25T03:29:04Z [init] [LoadChainTip] Loaded best chain: hashBestChain=0fff88f13cb7b2c71f2a335e3a4fc328bf5beb436012afca590b1a11466e22ff height=0 date=2011-02-02T23:16:42Z progress=1.000000
  2020-08-25T03:29:04Z [init] [AppInitMain] block tree size = 1
  2020-08-25T03:29:04Z [init] [AppInitMain] nBestHeight = 0
  2020-08-25T03:29:04Z [loadblk] [LoadMempool] Imported mempool transactions from disk: 0 succeeded, 0 failed, 0 expired, 0 already there, 0 waiting for initial broadcast
  2020-08-25T03:29:04Z [dnsseed] [ThreadDNSAddressSeed] 0 addresses found from DNS seeds
  ```

ACKs for top commit:
  laanwj:
    Code review ACK b4511e2e2e
  MarcoFalke:
    review ACK b4511e2e2e 🌃

Tree-SHA512: d100f5364630c323f31d275259864c597f7725e462d5f4bdedcc7033ea616d7fc0d16ef1b2af557e692f4deea73c6773ccfc681589e7bf6ba970b9ec169040c7
2021-02-18 14:37:51 +01:00
John Newbery
d21d2b264c [net] Change AdvertiseLocal to GetLocalAddrForPeer
Gossiping addresses to peers is the responsibility of net processing.
Change AdvertiseLocal() in net to just return an (optional) address
for net processing to advertise. Update function name to reflect
new responsibility.
2021-02-18 09:28:06 +00:00
Samuel Dobson
db656db2ed
Merge #19136: wallet: add parent_desc to getaddressinfo
de6b389d5d tests: Test getaddressinfo parent_desc (Andrew Chow)
e4ac869a0a rpc: Add parent descriptor to getaddressinfo output (Andrew Chow)
bbe4a36152 wallet: Add GetDescriptorString to DescriptorScriptPubKeyMan (Andrew Chow)
9be1437c49 descriptors: Add ToNormalizedString and tests (Andrew Chow)

Pull request description:

  Adds `parent_desc` field to the `getaddressinfo` RPC to export a public descriptor. Using the given address, `getaddressinfo` will look up which `DescriptorScriptPubKeyMan` can be used to produce that address. It will then return the descriptor for that `DescriptorScriptPubKeyMan` in the `parent_desc` field. The descriptor will be in a normalized form where the xpub at the last hardened step is derived so that the descriptor can be imported to other wallets. Tests are added to check that the correct descriptor is being returned for the wallet's addresses and that these descriptors can be imported and used in other wallets.

  As part of this PR, a `ToNormalizedString` function is added to the descriptor classes. This really only has an effect on `BIP32PubkeyProvider`s that have hardened derivation steps. Tests are added to check that normalized descriptors are returned.

ACKs for top commit:
  Sjors:
    utACK de6b389d5d
  S3RK:
    Tested ACK de6b389
  jonatack:
    Tested ACK de6b389d5d modulo a few minor comments
  fjahr:
    Code review ACK de6b389d5d
  meshcollider:
    Tested ACK de6b389d5d

Tree-SHA512: a633e4a39f2abbd95afd7488484cfa66fdd2651dac59fe59f2b80a0940a2a4a13acf889c534a6948903d701484a2ba1218e3081feafe0b9a720dccfa9e43ca2b
2021-02-18 21:51:16 +13:00