Commit Graph

23407 Commits

Author SHA1 Message Date
UdjinM6
87863a68a3
fix: reset chainman to allow reindex on failure (#5405)
## Issue being fixed or feature implemented
current develop fails to reindex whenever there is an issue at node
start (prints `should not be overwriting a chainstate` in `debug.log`)

## What was done?
reset chainman to allow it re-initialize chainstate


## How Has This Been Tested?
simulated an issue with
```
 if (!fReset) {
     strLoadError = _("DEBUG");
     break;
 }
```


## Breaking Changes
should not be any but pls test


## Checklist:
- [x] I have performed a self-review of my own code
- [ ] 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
- [x] I have assigned this pull request to a milestone _(for repository
code-owners and collaborators only)_
2023-06-06 19:31:11 +03:00
PastaPastaPasta
d49239cd4d
Merge pull request #5415 from kittywhiskers/bp_26633
backport: bitcoin#26633 (update qt 5.12 url to archive location)
2023-06-06 10:56:53 -05:00
Kittywhiskers Van Gogh
313326e846 merge bitcoin#26633: update qt 5.12 url to archive location 2023-06-06 16:19:57 +05:30
UdjinM6
0cbf85a6d2
fix(ci): only run jobs when their ancestors on prevoius stages succeeded (#5412)
## Issue being fixed or feature implemented
We should not be running jobs if their ancestors failed. This was broken
with the introduction of `FAST_MODE`/`.skip-in-fast-mode-template` I
think (via #3635).

## What was done?
Adjust ci rules, make them explicit.

## How Has This Been Tested?
Compare `test` stages:
https://gitlab.com/dashpay/dash/-/pipelines/888510672 (note: some of
`test` stage jobs were started, even though all related jobs from
`build` stage have failed)
vs
https://gitlab.com/UdjinM6/dash/-/pipelines/888923382 (note: none of
`test` stage jobs were started)


## Breaking Changes
n/a

## Checklist:
- [x] I have performed a self-review of my own code
- [ ] 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
- [x] I have assigned this pull request to a milestone _(for repository
code-owners and collaborators only)_
2023-06-06 11:54:29 +03:00
Konstantin Akimov
e23449aa81
fix: follow-up #5403: remove double serialization in CDeterministicMNState (#5413)
## Issue being fixed or feature implemented
Member obj.keyIDOwner is read & write twice



## What was done?
Fixed: it is serialized once

## How Has This Been Tested?
Unit/functional tests in CI


## Breaking Changes
Data format in database changed in incompatible way

## Checklist:
- [x] I have performed a self-review of my own code
- [ ] 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
- [x] I have assigned this pull request to a milestone
2023-06-05 23:39:16 -05:00
PastaPastaPasta
696ffebc73
Merge pull request #5306 from kittywhiskers/auxports10
backport: merge bitcoin#19192, #18968, #19191, #18991, #19658, #19697, #20000, #19638, #20120, #20757, #20965, #20284, #22025, #22179 (auxiliary backports: part 10)
2023-06-05 10:13:34 -05:00
Kittywhiskers Van Gogh
99b336deba merge bitcoin#22179: Torv2 removal followups 2023-06-05 10:11:03 -05:00
Kittywhiskers Van Gogh
31acbf1e2d merge bitcoin#22025: Group and re-order CAddrMan members by access type 2023-06-05 10:11:03 -05:00
Kittywhiskers Van Gogh
1ef6322fc2 merge bitcoin#20284: ensure old versions don't parse peers.dat 2023-06-05 10:11:03 -05:00
Kittywhiskers Van Gogh
3bfcf7a16b merge bitcoin#20965: return NET_UNROUTABLE as not_publicly_routable, automate helps 2023-06-05 10:11:03 -05:00
Kittywhiskers Van Gogh
052b2d0dc1 merge bitcoin#20757: tor.md and -onlynet help updates 2023-06-05 10:11:03 -05:00
Kittywhiskers Van Gogh
f003f00e9a merge bitcoin#19638: Replace hidden service with onion service 2023-06-05 10:11:03 -05:00
Kittywhiskers Van Gogh
81c686e7db merge bitcoin#20000: fix creation of "std::string"s with \0s 2023-06-05 10:11:03 -05:00
UdjinM6
d0d598dbb0 fix: resolve persistent p2p_getaddr_caching.py failures 2023-06-05 10:11:03 -05:00
Kittywhiskers Van Gogh
01130228f0 merge bitcoin#19697: Improvements on ADDR caching 2023-06-05 10:11:03 -05:00
Kittywhiskers Van Gogh
56cd0dd80a merge bitcoin#19658: Allow RPC to fetch all addrman records and add records to addrman 2023-06-05 10:11:03 -05:00
Kittywhiskers Van Gogh
1244d59522 merge bitcoin#18991: Cache responses to GETADDR to prevent topology leaks 2023-06-05 10:11:03 -05:00
Kittywhiskers Van Gogh
a13b72397b merge bitcoin#19191: Extract download permission from noban 2023-06-05 10:11:03 -05:00
Kittywhiskers Van Gogh
91d800dce3 merge bitcoin#18968: noban precludes maxuploadtarget disconnects 2023-06-05 10:11:03 -05:00
Kittywhiskers Van Gogh
8806c7baeb merge bitcoin#19192: Extract net permissions doc 2023-06-05 10:11:03 -05:00
UdjinM6
54fb76f2f1
fix: Resolve mainnet v19 fork issues (#5403)
## Issue being fixed or feature implemented
same as  #5392, alternative solution

~based on #5402 atm, will rebase later~

## What was done?
pls see individual commits

## How Has This Been Tested?
reorg mainnet around forkpoint with a patched client (to allow low
difficulty), run tests

## Breaking Changes
Another evodb migration is required. Going back to an older version or
migrating after the fork requires reindexing.

## Checklist:
- [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
- [x] I have assigned this pull request to a milestone _(for repository
code-owners and collaborators only)_
2023-06-04 23:45:56 +03:00
Konstantin Akimov
86dc99f10d
refactor: using reference instead reference to unique_ptr with object (#5381)
## Issue being fixed or feature implemented
Many objects created and functions called by passing `const
std::unique_ptr<Obj>& obj` instead directly passing `Obj& obj`

In some cases it is indeed needed, but in most cases it is just extra
complexity that is better to avoid.

Motivation:
- providing reference to object instead `unique_ptr` is giving warranty
that there's no `nullptr` and no need to keep it in mind
- value inside unique_ptr by reference can be changed externally and
instead `nullptr` it can turn to real object later (or in opposite)
 - code is shorter but cleaner

Based on that this refactoring is useful as it reduces mental load when
reading or writing code.
`std::unique` should be used ONLY for owning object, but not for passing
it everywhere.

## What was done?
Replaced most of usages `std::unique_ptr<Obj>& obj` to `Obj& obj`.
Btw, in several cases implementation assumes that object can be nullptr
and replacement to reference is not possible.
Even using raw pointer is not possible, because the empty
std::unique_ptr can be initialized later somewhere in code.
For example, in `src/init.cpp` there's called `PeerManager::make` and
pass unique_ptr to the `node.llmq_ctx` that would be initialized way
later.
That is out of scope this PR.
List of cases, where reference to `std::unique_ptr` stayed as they are:
- `std::unique_ptr<LLMQContext>& llmq_ctx` in `PeerManagerImpl`,
`PeerManager` and `CDSNotificationInterface`
- `std::unique_ptr<CDeterministicMNManager>& dmnman` in
`CDSNotificationInterface`

Also `CChainState` have 3 references to `unique_ptr` that can't be
replaced too:
 - `std::unique_ptr<llmq::CChainLocksHandler>& m_clhandler;`
 - `std::unique_ptr<llmq::CInstantSendManager>& m_isman;`
- `std::unique_ptr<llmq::CQuorumBlockProcessor>&
m_quorum_block_processor;`


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

## Breaking Changes
No breaking changes, all of these changes - are internal APIs for Dash
Core developers only.

## Checklist:
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [x] I have added or updated relevant unit/integration/functional/e2e
tests
- [x] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone

---------

Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
2023-06-04 15:26:23 -05:00
UdjinM6
89fcecbe4b
Merge pull request #5373 from knst/bc-bp-v20-missing-6
backport: bitcoin#17585, #17804, #17819, #18122, #18192, #18208, #18234, #18292, #18305, #18314, partial #18224
2023-06-01 16:53:48 +03:00
Samuel Dobson
68dfc06916 partial Merge #18224: Make AnalyzePSBT next role calculation simple, correct
1ef28b4f7cfba410fef524def1dac24bbc4086ca Make AnalyzePSBT next role calculation simple, correct (Gregory Sanders)

Pull request description:

  Sniped test and alternative to https://github.com/bitcoin/bitcoin/pull/18220

  Sjors documenting the issue:
  ```
  A PSBT signed by ColdCard was analyzed as follows (see #17509 (comment))

  {
    "inputs": [
      {
        "has_utxo": true,
        "is_final": false,
        "next": "finalizer"
      }
    ],
    "estimated_vsize": 141,
    "estimated_feerate": 1e-05,
    "fee": 1.41e-06,
    "next": "signer"
  }
  I changed AnalyzePSBT so that it returns "next": "finalizer" instead.
  ```

  It makes it much clearer that the role has been decided before hitting the `calc_fee` block, and groups all state-deciding in one spot instead of 2.

  Note that this assumes that PSBT roles are a complete ordering, which for now and in the future seems to be a correct assumption.

ACKs for top commit:
  Sjors:
    ACK 1ef28b4f7cfba410fef524def1dac24bbc4086ca, much nicer. Don't forget to document the bug fix.
  achow101:
    ACK 1ef28b4f7cfba410fef524def1dac24bbc4086ca
  Empact:
    ACK 1ef28b4f7c

Tree-SHA512: 22ba4234985c6f9c1445b14565c71268cfaa121c4ef000ee3d5117212b09442dee8d46d9701bceddaf355263fe25dfe40def2ef614d4f2fe66c9ce876cb49934
2023-05-31 18:14:23 -05:00
Konstantin Akimov
a5778e1db9 Merge #18314: tests: Add deserialization fuzzing of SnapshotMetadata (utxo_snapshot). Increase fuzzing coverage. 2023-05-31 18:14:23 -05:00
MarcoFalke
87de1bc7c7 Merge #18305: test: Explain why test logging should be used
ffff9dcdc3cbe427739cc19cc7a53f032474fa2a test: Explain why test logging should be used (MarcoFalke)

Pull request description:

  Background is that some tests don't have any `self.log` call at all. Thus there are no "anchor points" and those tests are hard to debug because the logs can't easily be parsed by a human.

ACKs for top commit:
  jonatack:
    ACK ffff9dcdc3cbe427739cc19cc7a53f032474fa2a
  instagibbs:
    ACK ffff9dcdc3
  fanquake:
    re-ACK ffff9dcdc3cbe427739cc19cc7a53f032474fa2a

Tree-SHA512: 08d962e85c4892c2a0c58feb5dc697c680a9d68e41a79417da6fcd415e0c5c735c4533a985cf225bb89deb5ca717d9bedf990657958079185804caa512b10f5a
2023-05-31 18:14:23 -05:00
MarcoFalke
c2b4dd698c Merge #18292: fuzz: Add assert(script == decompressed_script)
fab0e5ba7f2777b86f1487234ed7cc4be9f72ea8 fuzz: Add assert(script == decompressed_script) (MarcoFalke)

Pull request description:

  Presumably an oversight in https://github.com/bitcoin/bitcoin/pull/17926#discussion_r389262151

ACKs for top commit:
  practicalswift:
    Tested ACK fab0e5ba7f2777b86f1487234ed7cc4be9f72ea8

Tree-SHA512: 6dcec06169df497a540fd6ebbcd89f5db22257241b2bbe756de868742f9bc324b80d38dbababfa07e5f3a830aaae9fc6d168dcc2ca5d75da437bdf4dc4e0f370
2023-05-31 18:14:23 -05:00
Konstantin Akimov
c0ba1e3269 Merge #18234: refactor: Replace boost::mutex,condition_var,chrono with std equivalents in scheduler 2023-05-31 18:14:23 -05:00
MarcoFalke
cedb78e157 Merge #18192: Bugfix: Wallet: Safely deal with change in the address book
b5795a788639305bab86a8b3f6b75d6ce81be083 Wallet: Add warning comments and assert to CWallet::DelAddressBook (Luke Dashjr)
6d2905f57aaeb3ec3b63d31043f7673ca10003f2 Wallet: Avoid unnecessary/redundant m_address_book lookups (Luke Dashjr)
c751d886f499257627b308b11ffaa51c22db6cc0 Wallet: Avoid treating change-in-the-addressbook as non-change everywhere (Luke Dashjr)
8e64b8c84bcbd63caea06f3af087af1f0609eaf5 Wallet: New FindAddressBookEntry method to filter out change entries (and skip ->second everywhere) (Luke Dashjr)
65b6bdc2b164343ec3cc3d32a0297daff9e24fec Wallet: Add CAddressBookData::IsChange which returns true iff label has never been set (Luke Dashjr)
144b2f85da4d51bf7d72b987888ddcaf5b429eed Wallet: Require usage of new CAddressBookData::setLabel to change label (Luke Dashjr)
b86cd155f6f661052042048aa7cfc2a397afe4f7 scripted-diff: Wallet: Rename mapAddressBook to m_address_book (Luke Dashjr)

Pull request description:

  In many places, our code assumes that presence in the address book indicates a non-change key, and absence of an entry in mapAddressBook indicates change.

  This no longer holds true after #13756 (first released in 0.19) since it added a "used" DestData populated even for change addresses. Only avoid-reuse wallets should be affected by this issue.

  Thankfully, populating DestData does not write a label to the database, so we can retroactively fix this (so long as the user didn't see the change address and manually assign it a real label).

  Fixing it is accomplished by:

  * Adding a new bool to CAddressBookData to track if the label has ever been assigned, either by loading one from the database, or by assigning one at runtime.
  * `CAddressBookData::IsChange` and `CWallet::FindAddressBookEntry` are new methods to assist in excluding change from code that doesn't expect to see them.
  * For safety in merging, `CAddressBookData::name` has been made read-only (the actual data is stored in `m_label`, a new private member, and can be changed only with `setLabel` which updates the `m_change` flag), and `mapAddressBook` has been renamed to `m_address_book` (to force old code to be rebased to compile).

  A final commit also does some minor optimisation, avoiding redundant lookups in `m_address_book` when we already have a pointer to the `CAddressBookData`.

ACKs for top commit:
  ryanofsky:
    Code review ACK b5795a788639305bab86a8b3f6b75d6ce81be083. Pretty clever and nicely implemented fix!
  jonatack:
    ACK b5795a788639305bab86a8b3f6b75d6ce81be083 nice improvements -- code review, built/ran tests rebased on current master ff53433fe4ed06893d7c4 and tested manually with rpc/cli
  jnewbery:
    Good fix. utACK b5795a788.

Tree-SHA512: 40525185a0bcc1723f602243c269499ec86ecb298fecb5ef24d626bbdd5e3efece86cdb1084ad7eebf7eeaf251db4a6e056bcd25bc8457b417fcbb53d032ebf0
2023-05-31 18:14:23 -05:00
Samuel Dobson
984aae497d Merge #17585: rpc: deprecate getaddressinfo label
d3bc18408146e91b3836f72360ff6fa2420b6887 doc: update release notes with getaddressinfo label deprecation (Jon Atack)
72af93f36479dc12d795f1d05fa3d8fbd9b293bd test: getaddressinfo label deprecation test (Jon Atack)
d48875fa20d0b71b978cb3d1f85dd9ec14e664cc rpc: deprecate getaddressinfo label field (Jon Atack)
dc0cabeda49a7edbfa71df22846721b6f6224aea test: remove getaddressinfo label tests (Jon Atack)
c7654af6f830577a54df12b5d65df93532db0dc2 doc: address pr17578 review feedback (Jon Atack)

Pull request description:

  This PR builds on #17578 (now merged) and deprecates the rpc getaddressinfo `label` field. The deprecated behavior can be re-enabled by starting bitcoind with `-deprecatedrpc=label`.

  See http://www.erisian.com.au/bitcoin-core-dev/log-2019-11-22.html#l-622 and https://github.com/bitcoin/bitcoin/pull/17283#issuecomment-554458001 for more context.

  Reviewers: This PR may be tested manually by building, then running bitcoind with and without the `-deprecatedrpc=label` flag while verifying the rpc getaddressinfo output and help text.

  Next step: add support for multiple labels.

ACKs for top commit:
  jnewbery:
    ACK d3bc18408146e91b3836f72360ff6fa2420b6887
  laanwj:
    ACK d3bc18408146e91b3836f72360ff6fa2420b6887
  meshcollider:
    utACK d3bc18408146e91b3836f72360ff6fa2420b6887

Tree-SHA512: f954402884ec54977def332c8160fd892f289b0d2aee1e91fed9ac3220f7e5b1f7fc6421b84cc7a5c824a0582eca4e6fc194e4e33ddd378c733c8941ac45f56d
2023-05-31 18:14:23 -05:00
Wladimir J. van der Laan
4e62a8b8e4 Merge #17804: doc: Misc RPC help fixes
fa5c6622c8ecf1954e7177888ad8c97a77b16fb7 doc: Use proper RPC help syntax in importmulti (MarcoFalke)
fab63111bec73859597e6ce0986f76e5e9959091 doc: Remove duplicate "comment" from listsinceblock RPC help (MarcoFalke)
fa04cd6cfc0330b62058ed169d621e08108dc87e doc: Properly document proxy_randomize_credentials as bool in getnetworkinfo (MarcoFalke)
fa9dec7c395897e8dbbb6de7a16ec5185a609d41 doc: Fix syntax error (trailing square bracket) in finalizepsbt (MarcoFalke)
faff5a60ed328d4c5fdef253e8935a351cb57bd0 doc: Fix syntax error (trailing square bracket) in walletprocesspsbt (MarcoFalke)
fa0545901daad32b09511cc61c4af1400c48088d doc: Add missing "optional" to "long" estimaterawfee RPC help (MarcoFalke)

Pull request description:

  This fixes documentation of the following RPCs:

  * estimaterawfee (hidden)
  * https://bitcoincore.org/en/doc/0.19.0/rpc/wallet/walletprocesspsbt/
  * https://bitcoincore.org/en/doc/0.19.0/rpc/rawtransactions/finalizepsbt/
  * https://bitcoincore.org/en/doc/0.19.0/rpc/network/getnetworkinfo/
  * https://bitcoincore.org/en/doc/0.19.0/rpc/wallet/listsinceblock/
  * https://bitcoincore.org/en/doc/0.19.0/rpc/wallet/importmulti/

  <!-- Also, it comes with a scripted diff to normalize whitespace and type names. (Previous attempts: #14601 and #14459)

ACKs for top commit:
  laanwj:
    ACK fa5c6622c8ecf1954e7177888ad8c97a77b16fb7

Tree-SHA512: 5a10956e12f8ce23e93a2ce8bafd6cae759d8a21658f79397e3bfce3e4aabd9658bdbd40acde49323dca958a9befee7166654994208c182dd60f483109621e17
2023-05-31 18:14:23 -05:00
MarcoFalke
52581d3f2b Merge #18208: rpc: Change RPCExamples to bech32
3e32499909ca8127baaa9b40ad113b25ee151bbd Change example addresses to bech32 (Yusuf Sahin HAMZA)

Pull request description:

  This is a follow-up PR to #18197 that fixes RPCExamples.

  Fixes #18185.

ACKs for top commit:
  MarcoFalke:
    ACK 3e32499909ca8127baaa9b40ad113b25ee151bbd
  jonatack:
    ACK 3e32499

Tree-SHA512: c7a6410ef8b6e169016c2c5eac3e6b9501caabd0e8a0871ec31e56bfc44589f056d3f5cb55b5a13bba36f6c15136c2352f883e30e4dcc0997ffd36b27f9173b9
2023-05-31 18:14:23 -05:00
Sebastian Falbesoner
c06e887385 Merge #18122: rpc: update validateaddress RPCExamples to bech32
rpc: update validateaddress RPCExamples to bech32

also contains the following changes:
- rpc: factor out example bech32 address for RPCExamples
- doc: update developer notes wrt RPCExamples addresses
 (mention the EXAMPLE_ADDRESS constant as an example for an invalid bech32
  address suitable for RPCExamples help documentation)
2023-05-31 18:14:23 -05:00
MarcoFalke
df51c199b4 Merge #17819: doc: developer notes guideline on RPCExamples addresses
42ec4994892e67e3430f867af069aafcc2e08593 doc: developer notes guideline on RPCExamples addresses (Jon Atack)

Pull request description:

  to make explicit the use of invalid addresses for user safety and to encourage
  the use of bech32 addresses by default. See https://github.com/bitcoin/bitcoin/pull/17578#discussion_r361752570 and https://github.com/bitcoin/bitcoin/pull/17578#discussion_r362564492.

  Fix a typo to appease the linter.

ACKs for top commit:
  promag:
    ACK 42ec4994892e67e3430f867af069aafcc2e08593, no strong opinion as whether this belongs to developer notes or not but why not.
  fjahr:
    ACK 42ec499
  michaelfolkson:
    ACK 42ec4994892e67e3430f867af069aafcc2e08593

Tree-SHA512: 64f90e227d256aa194c4fd48435440bdc233a51213dd4a6ac5b05d04263f729c6b4bb5f3afd3b87719b20cb1b159d5a9673d58a11b72823a4a6a16e8a26ae10e
2023-05-31 18:14:23 -05:00
UdjinM6
b741f3a4b0
test: Fix dynamically_smth_masternode helpers, extend feature_dip3_v19.py (#5402)
## Issue being fixed or feature implemented
fix a couple of issues in helpers, extend feature_dip3_v19.py to check
more after v19 fork

## What was done?
pls see individual PRs


## How Has This Been Tested?
run tests

## Breaking Changes
n/a

## Checklist:
- [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
- [x] I have assigned this pull request to a milestone _(for repository
code-owners and collaborators only)_
2023-05-31 23:34:30 +03:00
PastaPastaPasta
3bf7d2a38c
feat: ability to disable clsig creation while retaining clsig enforcement (#5398)
## Issue being fixed or feature implemented
Currently, Chainlocks are either enabled or disabled. This PR adds a
third state: enabled but we will not sign new ones.

Should probably backport this to v19.x

## What was done?
Spork state != 0 but active will now result in chain locks being
enforced but not created.

## How Has This Been Tested?

## Breaking Changes
None

## Checklist:
_Go over all the following points, and put an `x` in all the boxes that
apply._
- [ ] I have performed a self-review of my own code
- [ ] 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
- [x] I have assigned this pull request to a milestone _(for repository
code-owners and collaborators only)_

---------

Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
2023-05-31 23:34:14 +03:00
PastaPastaPasta
2c0cb13f09
Merge pull request #5393 from knst/bc-bp-v20-missing-7
backport: bitcoin#16902, #17720, #18107, #18278, #18360, #18397, #18402, #18420, #18447, #18481, #18494, #18496, #18503, #18515
2023-05-31 12:19:27 -05:00
MarcoFalke
11eefa21d4 Merge #18515: test: add BIP37 remote crash bug [CVE-2013-5700] test to p2p_filter.py
0ed2d8e07d3806d78d03a77d2153f22f9d733a07 test: add BIP37 remote crash bug [CVE-2013-5700] test to p2p_filter.py (Sebastian Falbesoner)

Pull request description:

  Integrates the missing message type `filteradd` to the test framework and checks that the BIP37 implementation is not vulnerable to the "remote crash bug" [CVE-2013-5700](https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2013-5700) anymore. Prior to v.0.8.4, it was possible to trigger a division-by-zero error on the following line in the function `CBloomFilter::Hash()`:
  f0d6487e29/src/bloom.cpp (L45)
  By setting a zero-length filter via `filterload`, `vData.size()` is 0, so the modulo operation above, called on any .insert() or .contains() operation then crashed the node. The test uses the approach of just sending an arbitrary `filteradd` message after, which calls `CBloomFilter::insert()` (and in turn `CBloomFilter::Hash()`) on the node. The vulnerability was fixed by commit 37c6389c5a (an intentional covert fix, [according to gmaxwell](https://github.com/bitcoin/bitcoin/issues/18483#issuecomment-608224095)), which introduced flags `isEmpty`/`isFull` that wouldn't call the `Hash()` member function if `isFull` is true (set to true by default constructor).

  To validate that the test fails if the implementation is vulnerable, one can simply set the flags to false in the member function `UpdateEmptyFull()` (that is called after a filter received via `filterload` is constructed), which activates the vulnerable code path calling `Hash` in any case on adding or testing for data in the filter:
  ```diff
  diff --git a/src/bloom.cpp b/src/bloom.cpp
  index bd6069b..ef294a3 100644
  --- a/src/bloom.cpp
  +++ b/src/bloom.cpp
  @@ -199,8 +199,8 @@ void CBloomFilter::UpdateEmptyFull()
           full &= vData[i] == 0xff;
           empty &= vData[i] == 0;
       }
  -    isFull = full;
  -    isEmpty = empty;
  +    isFull = false;
  +    isEmpty = false;
   }
  ```
  Resulting in:
  ```
  $ ./p2p_filter.py
  [...]
  2020-04-03T14:38:59.593000Z TestFramework (INFO): Check that division-by-zero remote crash bug [CVE-2013-5700] is fixed
  2020-04-03T14:38:59.695000Z TestFramework (ERROR): Assertion failed
  [...]
  [... some exceptions following ...]
  ```

ACKs for top commit:
  naumenkogs:
    utACK 0ed2d8e07d3806d78d03a77d2153f22f9d733a07

Tree-SHA512: 02d0253d13eab70c4bd007b0750c56a5a92d05d419d53033523eeb3ed80318bc95196ab90f7745ea3ac9ebae7caee3adbf2a055a40a4124e0915226e49018fe8
2023-05-31 12:01:04 -05:00
MarcoFalke
ff89f705c0 Merge #18496: test: remove redundant sync_with_ping after add_p2p_connection
4670006762ffce654bb12edb5a7e64ad004122a7 test: remove redundant sync_with_ping after add_p2p_connection (Jon Atack)

Pull request description:

  Now that #18247 is merged, these calls are redundant.

ACKs for top commit:
  vasild:
    utACK 4670006

Tree-SHA512: bdbfe8bcf9dbdde0a8115e3a62bfe359910798d7a3010d920ffca07049cb5f97bf8fb9b6f70079b0607105192b61a6d665774e59a2b678597b47ad6237595ad5
2023-05-31 12:01:04 -05:00
MarcoFalke
0784b6a148 Merge #18494: test: replace (send_message + sync_with_ping) with send_and_ping
6112a209828c43930f677c45461339cdf68a56e9 test: replace (send_message + sync_with_ping) with send_and_ping (Jon Atack)

Pull request description:

  This is a follow-up to faf1d047313e71658fb31f6b94fdd5d37705ab85 yesterday.

ACKs for top commit:
  vasild:
    utACK 6112a20
  MarcoFalke:
    ACK 6112a209828c43930f677c45461339cdf68a56e9 🎞

Tree-SHA512: 749644ac9a1ef0e1aa6c3ac5e899eb3fa7fb9c0909352f922a80412df2bc0e539692a7757af550eff4d4914cbe57b0c75ce3948f569acc7a52852e91a55ad457
2023-05-31 12:01:04 -05:00
MarcoFalke
fa6aafa19b Merge #18481: test: add BIP37 'filterclear' test to p2p_filter.py
00559229588feb19de2a0cb7506f70c483a1f433 test: add BIP37 'filterclear' test to p2p_filter.py (Sebastian Falbesoner)

Pull request description:

  Integrates the message type `filterclear` to the test framework and adds a simple test to `p2p_filter.py`, checking that arbitrary txs get relayed again after deleting the filter.

ACKs for top commit:
  naumenkogs:
    utACK 00559229588feb19de2a0cb7506f70c483a1f433

Tree-SHA512: fe64e99a526865770707d8077b9968d3923f248045ec7fa56cd380dba85ac77a71a473d244ef3aede2fc0d287b8d7c6bc0156b6033b0c949c2058cc08e255697
2023-05-31 12:01:04 -05:00
MarcoFalke
dabaaf59e4 Merge #18420: test: listsinceblock block height checks
83e1d92413e262e6a876336ec433a6fbc335223a test: listsinceblock block height checks (Jon Atack)

Pull request description:

  This is the second commit of #17535.

  This PR extends a listsinceblock test to check the new transaction 'blockheight' field recently added in #17437. It also cleans up code in the test function without changing or removing existing checks.

ACKs for top commit:
  fjahr:
    tested ACK 83e1d92413e262e6a876336ec433a6fbc335223a
  ryanofsky:
    Code review ACK 83e1d92413e262e6a876336ec433a6fbc335223a. Nice test improvements!

Tree-SHA512: 92874b49a3bc0236500495f32dfcf683e1971ca3d4c51702c69ed4ce7dfce21273754f02f93d1243d73793701d9fdf49e14b149477cd249cbbd9e4e8d5bd49f8
2023-05-31 12:01:04 -05:00
fanquake
67ac335e75 Merge #18503: init: Replace URL_WEBSITE with PACKAGE_URL
fad2f68353944c83cad7b77415645215ae3f1093 init: Replace URL_WEBSITE with PACKAGE_URL (MarcoFalke)

Pull request description:

  This is needed for rebranding efforts such as #18489

ACKs for top commit:
  hebasto:
    ACK fad2f68353944c83cad7b77415645215ae3f1093, tested on Linux Mint 19.3:
  fanquake:
    ACK fad2f68353944c83cad7b77415645215ae3f1093 - clicked a link.

Tree-SHA512: c26e18cd328d3dd3fd7e25413e1bab780026687a148f126b8673e5f6cc13249f6c16689e45eba9da1545915c6001f96cd33f4e656c08cda3eae1c3fd88da23ea
2023-05-31 12:01:04 -05:00
MarcoFalke
1fd6393d32 Merge #18447: test: Add coverage for script parse error in ParseScript
dcda81c47101196e53e379d965a2692515ef8363 test: add coverage for script parse error in ParseScript (pierrenn)

Pull request description:

  Follow up on this suggestion :  https://github.com/bitcoin/bitcoin/pull/18416#issuecomment-603966799

  This adds a test case to raise the `script parse error` in `ParseScript`.

ACKs for top commit:
  instagibbs:
    utACK dcda81c471

Tree-SHA512: ae0ef2c00f34cee818c83582f190d5f4043159e922862f2b442b7b895b8ff3ca421533699247c12c367be77813b5205830a771cd47a18e8932807ccace2d6a1c
2023-05-31 12:01:04 -05:00
Wladimir J. van der Laan
e479892050 Merge #18107: build: Add cov_fuzz target
faf7d4fa86b700ec272806cd2bd8666a92405619 build: Add cov_fuzz target (MarcoFalke)
fac71e364e4bbaeffc35e45aff8c8c2c6f2b5c67 build: link fuzz/test_runner.py for out-of-tree builds (MarcoFalke)
faf2c5aca01643eb560287e08f9c0a7ca0ac9c88 build: Remove unused USE_COVERAGE (MarcoFalke)

Pull request description:

  Only libFuzzer is supported right now, so clang is required. Thus, this needs a workaround such as https://github.com/bitcoin/bitcoin/issues/12602#issuecomment-562788247

  Can be tested with:

  ```
  mkdir build && cd build
  ../configure --enable-fuzz --with-sanitizers=fuzzer --enable-lcov --enable-lcov-branch-coverage CC=clang CXX=clang++
  make $MAKEJOBS
  make cov_fuzz

ACKs for top commit:
  practicalswift:
    ACK faf7d4fa86b700ec272806cd2bd8666a92405619

Tree-SHA512: 6828f8f81d95f6781713d0b09d7eba2ffdb50217e09ca839db61791a4ed70024859c7a0cb01d9eede79166d574dd57ece01f9d9fe2610d4a72a4ca4a4ce0b838
2023-05-31 12:01:04 -05:00
fanquake
8ae94ada7c Merge #18397: build: Fix libevent linking for bench_bitcoin binary
cd04286825c6512b46bf59ab7b3dfffb0e36d65b build: Fix typo in EVENT_CFLAGS variable (Hennadii Stepanov)
f709ad0c907d87d03002455967cc30ae7d704d80 build: Fix libevent linking for bench_bitcoin binary (Hennadii Stepanov)

Pull request description:

  This change fixes `libevent` linking error for the `bench_bitcoin` binary.

  This PR is an alternative to #18377.
  Fix #18373.

  Also fixed a typo in `EVENT_CFLAGS` variable noted by **brakmic**.

ACKs for top commit:
  fanquake:
    ACK cd04286825c6512b46bf59ab7b3dfffb0e36d65b

Tree-SHA512: a62f7457e86b11d3a55d603ea5d83f3a413792e2f28a0c72300e54d12591bd6f0acc1d76a4bd4b591e0223bc6d530e7a4b9a8b939fe2fdbf2dddfda5b1b537be
2023-05-31 12:01:04 -05:00
Wladimir J. van der Laan
aebde26ca0 Merge #18402: gui: display mapped AS in peers info window
76db4b260e4826a1e59a5e44c92e4c10ec986527 gui: avoid QT Designer/Form Editor re-formatting (Jon Atack)
aae26053f958ae9a96a25d32c6341b14daaa4f26 gui: display Mapped AS in peers info window (Jon Atack)

Pull request description:

  Continuing the asmap integration of #16702 which added `mapped_as` to the rpc getpeerinfo output, this adds the mapped AS to the Peers detail window in the GUI wallet.

  `$ src/qt/bitcoin-qt -asmap=<path-to-asmap-file>` (asmap on)

  ![Screenshot from 2020-03-22 12-29-56](https://user-images.githubusercontent.com/2415484/77248754-c0ae4600-6c33-11ea-9d27-a06560c180c0.jpg)

  `$ src/qt/bitcoin-qt` (asmap off)

  ![Screenshot from 2020-03-22 12-32-46](https://user-images.githubusercontent.com/2415484/77248749-bdb35580-6c33-11ea-925c-6e19ecc083ab.jpg)

  Added a tooltip and a couple of minor fixups.

ACKs for top commit:
  laanwj:
    ACK 76db4b260e4826a1e59a5e44c92e4c10ec986527

Tree-SHA512: 5f44c05c247bfabc9c161884d3af47c50a571cd02777b320ce389e61efa47706adbf0ea5e6644ae40423cb579d8bd0bb3c84fc6b618293a7add8e4327f07f63f
2023-05-31 12:01:04 -05:00
MarcoFalke
66d5d5a798 Merge #17720: test: add unit test for non-standard "scriptsig-not-pushonly" txs
5aab011805ceb12801644170700b1a62e0bf4a5d test: add unit test for non-standard "scriptsig-not-pushonly" txs (Sebastian Falbesoner)

Pull request description:

  Approaches another missing unit test of issue #17394: Checks that the function `IsStandardTx()` returns rejection reason "scriptsig-not-pushonly" if any one of the input's scriptSig consists of any other ops than just PUSHs.

ACKs for top commit:
  MarcoFalke:
    ACK 5aab011805ceb12801644170700b1a62e0bf4a5d 🍟
  practicalswift:
    ACK 5aab011805ceb12801644170700b1a62e0bf4a5d -- patch looks correct

Tree-SHA512: fbe25bcf57e5f0c8d2397eb67e61fe8d9145ba83032789adb2b67d6fcbcd87e6427e9d965e8cd7bbaaea482e39ec2f110f71ef2de079c7d1fba2712848caa9ba
2023-05-31 12:01:04 -05:00
MarcoFalke
fc930dc0f4 Merge #18278: interfaces: Describe and follow some code conventions
3dc27a15242a22b5301904375e5880372e9b7f4d doc: Add internal interface conventions to developer notes (Russell Yanofsky)
1dca9dc4c772fa0a4ec52c4d88b7cd3d243aea7b refactor: Change createWallet, fillPSBT argument order (Russell Yanofsky)
96dfe5ced64979e51649d20555aa182defc80119 refactor: Change Chain::broadcastTransaction param order (Russell Yanofsky)
6ceb21909ce66b7b4762a855889acd46bb6b77f3 refactor: Rename Chain::Notifications methods to be consistent with other interfaces methods (Russell Yanofsky)
1c2ab1a6d29f2c6c065dae4f4a4e2ad1286311b3 refactor: Rename Node::disconnect methods (Russell Yanofsky)
77e4b0657298c715c835d8d2eb11e173852e6815 refactor: Get rid of Wallet::IsWalletFlagSet method (Russell Yanofsky)

Pull request description:

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

  This PR doesn't change behavior at all, it just cleans up code in [`src/interfaces`](https://github.com/bitcoin/bitcoin/tree/master/src/interfaces) to simplify #10102, and [documents](https://github.com/ryanofsky/bitcoin/blob/pr/ipc-conv/doc/developer-notes.md#internal-interface-guidelines) coding conventions there better

ACKs for top commit:
  hebasto:
    re-ACK 3dc27a15242a22b5301904375e5880372e9b7f4d, the only change since the [previous](https://github.com/bitcoin/bitcoin/pull/18278#pullrequestreview-372582146) review is rebasing.
  MarcoFalke:
    ACK 3dc27a15242a22b5301904375e5880372e9b7f4d 🕍

Tree-SHA512: 62e6a0f2488e3924e559d2074ed460b92e7a0a5d98eab492221cb20d59d04bbe32aef2a8aeba5e4ea9168cfa91acd5bc973dce6677be0180bd7a919354df53ed
2023-05-31 12:01:04 -05:00
Wladimir J. van der Laan
0a1803a13a Merge #18360: Bump transifex slug and update English translations for 0.20
2a1b85f3c54fdaa8d13aeaf7e1acab1afb98fd97 tx: Bump transifex slug to 020x (Wladimir J. van der Laan)
82dd8860bbe81567e4afb033d9026dc0e69b1574 qt: Periodical translations update (Wladimir J. van der Laan)

Pull request description:

  Need to merge this so that translations for 0.20 can start.

Top commit has no ACKs.

Tree-SHA512: c78307eea3130b9bbc301f23790340e4c86a3f3877ab308bf67d9e1c9e977f7dda4b2cd83b96d9398b01dce616a6c962b98115123628bba35170eb0f301c4dd8
2023-05-31 12:01:04 -05:00