sikkienl/dash
1
0
Fork 0
mirror of https://github.com/dashpay/dash.git synced 2024-12-26 04:22:55 +01:00
Code Issues Actions Packages Projects Releases Wiki Activity
24,835 Commits 24 Branches 554 Tags 573 MiB
5abe35afa7
Commit Graph

2028 Commits

Author SHA1 Message Date
fanquake
6e7b402fe9 partial Merge bitcoin/bitcoin#27483: Bump python minimum version to 3.8 
fac395e5eb2cd3210ba6345f777a586a9bec84e3 ci: Bump ci/lint/Dockerfile (MarcoFalke)
fa6eb6516727a8675dc6e46634d8343e282528ab test: Use python3.8 pow() (MarcoFalke)
88881cf7ac029aea660c2413ca8e2a5136fcd41b Bump python minimum version to 3.8 (MarcoFalke)

Pull request description:

  There is no pressing reason to drop support for 3.7, however there are several maintenance issues:

  * There is no supported operating system that ships 3.7 by default. (debian:buster is EOL and unmaintained to the extent that it doesn't run in the CI environment. See https://github.com/bitcoin/bitcoin/pull/27340#issuecomment-1484988445)
  * Compiling python 3.7 from source is also unsupported on at least macos, according to https://github.com/bitcoin/bitcoin/pull/24017#issuecomment-1107820790
  * Recent versions of lief require 3.8, see https://github.com/bitcoin/bitcoin/pull/27507#issuecomment-1517561645

  Fix all maintenance issues by bumping the minimum.

ACKs for top commit:
  RandyMcMillan:
    ACK fac395e
  fjahr:
    ACK fac395e5eb2cd3210ba6345f777a586a9bec84e3
  fanquake:
    ACK fac395e5eb2cd3210ba6345f777a586a9bec84e3

Tree-SHA512: c198decdbbe29d186d73ea3f6549d8a38479383495d14a965a2f9211ce39637b43f13a4c2a5d3bf56e2d468be4bbe49b4ee8e8e19ec69936ff43ddf2b714c712
 2023-10-23 10:48:39 -05:00
Konstantin Akimov
6b5b8973cc fix: missing changes from Merge #15404: [test] Remove -txindex to start nodes 2023-10-23 10:46:52 -05:00
fanquake
4cd51487c4 fix: missing changes from Merge #16917: tests: Move common function assert_approx() into util.py 
96299a9d6c0a6b9125a58a63ee3147e55d1b086b Test: Move common function assert_approx() into util.py (fridokus)

Pull request description:

  To reduce code duplication, move `assert_approx` into common framework `util.py`.

  `assert_approx()` is used in two functional tests.

ACKs for top commit:
  theStack:
    ACK 96299a9
  practicalswift:
    ACK 96299a9d6c0a6b9125a58a63ee3147e55d1b086b -- DRY is good and diff looks correct
  fanquake:
    ACK 96299a9d6c0a6b9125a58a63ee3147e55d1b086b - thanks for contributing 🍻

Tree-SHA512: 8e9d397222c49536c7b3d6d0756cc5af17113e5af8707ac48a500fff1811167fb2e03f3c0445b0b9e80f34935f4d57cfb935c4790f6f5463a32a67df5f736939
 2023-10-23 10:46:52 -05:00
UdjinM6
faba796c73
fix: actually show json for assetlock/unlock txes (#5633) 
## Issue being fixed or feature implemented
The bug was introduced in the original PR #5026 and refactored later
(which is good actually cause we shouldn't mix refactoring and
bug-fixing :) )

## What was done?
fix conditions, add tests

## How Has This Been Tested?
`feature_asset_locks.py`

## 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-10-23 10:36:50 -05:00
PastaPastaPasta
c51cec606d
refactor: add gsl::not_null to get compile time / run time pointer guarantees (#5595) 
## Issue being fixed or feature implemented
Current implementation relies either on asserts or sometimes checks then
returning a special value; In the case of asserts (or no assert where we
use the value without checks) it'd be better to make it explicit to
function caller that the ptr must be not_null; otherwise gsl::not_null
will call terminate.

See
https://github.com/microsoft/GSL/blob/main/docs/headers.md#user-content-H-pointers-not_null
and
https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rf-nullptr

I'm interested in a conceptual review; specifically on if this is
beneficial over just converting these ptrs to be a reference?

## What was done?
 *Partial* implementation on using gsl::not_null in dash code


## How Has This Been Tested?
Building

## Breaking Changes
None

## Checklist:
_Go over all the following points, and put an `x` in all the boxes that
apply._
- [x] I have performed a self-review of my own code
- [ ] 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)_

---------

Signed-off-by: pasta <pasta@dashboost.org>
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
 2023-10-22 09:14:30 -05:00
Konstantin Akimov
63ed462c54
feat: auto generation EHF and spork+EHF activation for MN_RR (#5597) 
Implementation EHF mechanism, part 4. Previous changes are: 
 - https://github.com/dashpay/dash/pull/4577
 - https://github.com/dashpay/dash/pull/5505
 - https://github.com/dashpay/dash/pull/5469

## Issue being fixed or feature implemented
Currently MN_RR is activated automatically by soft-fork activation after
v20 is activated.
It is not flexible enough, because platform may not be released by that
time yet or in opposite it can be too long to wait.
Also, any signal of EHF requires manual actions from MN owners to sign
EHF signal - it is automated here.

## What was done?
New spork `SPORK_24_MN_RR_READY`; new EHF manager that sign EHF signals
semi-automatically without manual actions; and send transaction with EHF
signal when signal is signed to network.
Updated rpc `getblockchaininfo` to return information about of EHF
activated forks.
Fixed function `IsTxSafeForMining` in chainlock's handler to skip
transactions without inputs (empty `vin`).

## How Has This Been Tested?
Run unit/functional tests. Some tests have been updated due to new way
of MN_RR activation: `feature_asset_locks.py`, `feature_mnehf.py`,
`feature_llmq_evo.py` and unit test `block_reward_reallocation_tests`.


## Breaking Changes
New way of MN_RR activation.

## 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
- [ ] 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>
Co-authored-by: PastaPastaPasta <6443210+PastaPastaPasta@users.noreply.github.com>
 2023-10-17 22:31:40 -05:00
Odysseas Gabrielides
cecf63e0b7
feat!: exclude fees when calculating platformReward (#5612) 
## Issue being fixed or feature implemented
Calculation of `platformReward` should ignore fees and rely only on
Block subsidy.

cc @QuantumExplorer 

## What was done?
From now on, the following formula is applied:
```
blockReward = blockSubsidy + feeReward
masternodeReward = masternodeShare(blockSubsidy)
platformReward = platformShare(masternodeReward)
masternodeReward += masternodeShare(feeReward)
```


## How Has This Been Tested?


## Breaking Changes
`plaftormReward` differs in networks where `mn_rr` is already active

## Checklist:
_Go over all the following points, and put an `x` in all the boxes that
apply._
- [x] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [x] 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-10-17 22:07:37 -05:00
Odysseas Gabrielides
848ed765e0
feat!: constant subsidy base for blocks in v20 (#5611) 
## Issue being fixed or feature implemented
Currently, the `nSubsidyBase` calculation relies on difficulty. This
leads to variable Block Subsidity.
When Platform will be live, it would constantly require blocks
difficulty in order to calculate the `platformReward` (which relies on
Block Subsidy)

cc @QuantumExplorer 

## What was done?
Starting from v20 activation, `nSubsidyBase` will no longer rely on
difficulty and will be constant to 5.

## How Has This Been Tested?


## Breaking Changes
Block rewards will differ.

## 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
- [ ] 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-10-17 15:50:23 -05:00
Odysseas Gabrielides
5ca6382bfa
test: correct calculation of coinbasevalue in feature_asset_locks.py (#5603) 
## Issue being fixed or feature implemented
Fixed a problem forgotten in #5588 in feature_asset_locks.py.

## What was done?
Avoid floating operations when calculating `coinbasevalue`

## How Has This Been Tested?

## Breaking Changes

## 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)_

---------

Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
 2023-10-11 08:13:29 -05:00
UdjinM6
30f3f50928
fix: Let CDeterministicMN::ToJson() return correct collateralAddress for spent collaterals (#5607) 
## Issue being fixed or feature implemented
Historical masternode data returned via rpcs like `protx listdiff` can
be broken because some collaterals might be spent already and
`GetUTXOCoin` wasn't able to get any info.

## What was done?
Use `GetTransaction` as a fallback.

## How Has This Been Tested?
run tests

## 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-10-09 11:14:51 -05:00
UdjinM6
2004a855d9
fix!: avoid float calculations in PlatformShare (#5604) 
## Issue being fixed or feature implemented
avoid potential discrepancies in block reward calculations

## What was done?
use integers (int64_t) only when dealing with block rewards, no
float/double

## How Has This Been Tested?
run tests

## Breaking Changes
might fork off on devnets that use previous version

## 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-10-09 09:15:23 -05:00
UdjinM6
4b046bb608 use deployment nStartTime as a signal expiration mark, adjust tests 
if a signal is mined prior to nStartTime then it means it was mined for one of the previous deployments with the same bit and we can ignore it
 2023-10-06 11:02:15 -05:00
Konstantin Akimov
d83dbd287a fix: fix previous commit with fixes 2023-10-06 11:02:15 -05:00
Konstantin Akimov
5e31bd5545 refactor: multiple fixes, cleanups, improvements and refactorings 2023-10-06 11:02:15 -05:00
Konstantin Akimov
92be5e0be7 fix: now EHF transactions expires after nExpiryEHF blocks 2023-10-06 11:02:15 -05:00
Konstantin Akimov
3973f2b925 feat: update functional tests for Mn EHF - to use same bit more than once 2023-10-06 11:02:15 -05:00
Konstantin Akimov
ef14b53b3d feat: add functional test for unknown and invalid version bits of EHF release 2023-10-06 11:02:15 -05:00
Konstantin Akimov
2c4597db9f feat: improve functional tests for MnEHF to check block reconsideration 2023-10-06 11:02:15 -05:00
Konstantin Akimov
b85a497cca feat: new functional test for feature MnEHF 2023-10-06 11:02:15 -05:00
Konstantin Akimov
33ab3187b2 feat: add CMNHFManager and logic to make hard-forks accordingly received signals 2023-10-06 11:02:15 -05:00
Konstantin Akimov
612faa8868 feat: imlemented new hard-fork mechanism that uses MN Activation Height 
Altough, it's still disabled because no calls of related methods after processing MnEHF tx
 2023-10-06 11:02:15 -05:00
UdjinM6
c814dcaaea
fix: Move CreditPoolDiff checks out of ProcessSpecialTxsInBlock, use correct block reward (#5594) 
## Issue being fixed or feature implemented
The block reward calculation logic in `SetTarget` doesn't work on
superblocks.

## What was done?
Move `CreditPoolDiff` checks out of `ProcessSpecialTxsInBlock` to use
correct block reward.

## How Has This Been Tested?
run tests

## Breaking Changes
n/a, sb blocks should now be processed correctly, non-sb blocks
shouldn't be affected

## 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-10-04 12:47:21 -05:00
Odysseas Gabrielides
e72eb40024
feat!: Block Reward Reallocation (Doubling Treasury) (#5588) 
## Issue being fixed or feature implemented
Implementation of accepted proposal:
https://www.dashcentral.org/p/TREASURY-REALLOCATION-60-20-20

## What was done?
Once Masternode Reward Location Reallocation activates:
- Treasury is bumped to 20% of block subsidy.
- Block reward shares are immediately set to 75% for MN and 25% miners.
(Previous reallocation periods are dropped)
MN reward share should be 75% of block reward in order to represent 60%
of the block subsidy. (according to the proposal)
- `governancebudget` is returned from `getgovernanceinfo` RPC.

## How Has This Been Tested?
`block_reward_reallocation_tests`

## Breaking Changes


## Checklist:
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [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 _(for repository
code-owners and collaborators only)_

---------

Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
 2023-10-03 09:32:53 -05:00
PastaPastaPasta
7ca61abed0
refactor: remove some now unneeded cppcheck suppressions (#5589) 
## Issue being fixed or feature implemented
Unneeded suppressions were present

## What was done?
Removed them

## How Has This Been Tested?
Running linter

## Breaking Changes
None

## Checklist:
_Go over all the following points, and put an `x` in all the boxes that
apply._
- [x] I have performed a self-review of my own code
- [ ] 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-09-29 07:53:22 -05:00
Kittywhiskers Van Gogh
c3b4b6746a merge bitcoin#20966: save the banlist in a JSON format on disk 2023-09-24 09:50:50 -05:00
Kittywhiskers Van Gogh
5d7367e366 merge bitcoin#22050: remove tor v2 support 2023-09-24 09:50:50 -05:00
Kittywhiskers Van Gogh
4b29a74904 merge bitcoin#20852: allow CSubNet of non-IP networks 2023-09-24 09:50:50 -05:00
Konstantin Akimov
633cc3260f
feat: new rpc `gettxchainlocks' to get transaction statuses by batch (#5578) 
## Issue being fixed or feature implemented
Requested by @QuantumExplorer for platform needs

## What was done?
New rpc `gettransactionsarelocked` that returns list of txes.
it does less heavy calculations and transfer less data by gRPC.


## How Has This Been Tested?
```
$ src/dash-cli gettransactionsarelocked  '["e469de7994b9c1da8efd262fee8843efd7bdcab80c700dc1059c98b28f7c5c1b", "0d9fdf00c9568ff9103742b64e6b8287794633072f8824fa2c475f59e71dbace","0d3f48eebead54d640a7fc5692ddfcba619d8b49347d9a7c04586057c02dec9f"]'

[
  {
    "height": 907801,
    "chainlock": true
  },
  {
    "height": 101,
    "chainlock": true
  },
  {
    "height": -1,
    "chainlock": false
  }
]
```
Limiter tested by this call:
```
src/dash-cli gettransactionsarelocked  '["", "","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","","",""]'  | wc
```

## 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

---------

Co-authored-by: pasta <pasta@dashboost.org>
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
 2023-09-20 09:07:24 -05:00
merge-script
90285f5327 Merge bitcoin/bitcoin#22790: test: add aarch64-apple-darwin platform entry to get_previous_releases 
f6e4db27ceb67157dc13d13f34351cb87fec2be5 test: add aarch64-apple-darwin platform entry to get_previous_releases (Zero-1729)

Pull request description:

  Over the course of reviewing a PR, I had to edit `test/get_previous_releases.py` (after I ran `git clean -xdff`) to run the backwards compatibility tests (e.g. `wallet_upgradewallet`, `feature_backwards_compatibility`, etc.), as currently on master, running the script as indicated in [`test/README.md`](https://github.com/bitcoin/bitcoin/blob/master/test/README.md), for example, on an M1 machine results in the following error, as the `aarch64-apple-darwin*` platform entry is presently not recognised:

  > Output from an M1 machine running macOS v11.5.2

  ```sh
  $ test/get_previous_releases.py -b v0.20.1 v0.19.1 v0.18.1 v0.17.2 v0.16.3 v0.15.2
  Releases directory: releases
  Not sure which binary to download for aarch64-apple-darwin20.6.0
  ```

  As a quick fix, this PR adds the missing `aarch64-apple-darwin*` platform entry. Running the script now results in fetching the old binaries, as expected:

  ```sh
  $ test/get_previous_releases.py -b v0.20.1 v0.19.1 v0.18.1 v0.17.2 v0.16.3 v0.15.2

  Releases directory: releases
  Fetching: https://bitcoincore.org/bin/bitcoin-core-0.20.1/bitcoin-0.20.1-osx64.tar.gz
    % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                   Dload  Upload   Total   Spent    Left  Speed
    0 20.9M    0     0    0     0      0      0 --:--:--  0:00:02 --:--:--     0
    % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                   Dload  Upload   Total   Spent    Left  Speed
  100 20.9M  100 20.9M    0     0   136k      0  0:02:37  0:02:37 --:--:-- 95607
  Checksum matched

  …

  Checksum matched
  ```

  After this patch, the backwards compatibility tests also run successfully, as expected.

  **Note**: I am open to other possible solutions.

  ---

  Steps to reproduce:

  > Ensure you take out the binaries in `releases` if they already exist.

  Try running `test/get_previous_releases.py -b v0.15.2` or similar to fetch the old release binaries.

Top commit has no ACKs.

Tree-SHA512: a238d909b70a61be622234bc49b05d2e91a8acfc5ea348d29f2c8a927fb793cb97365e558571e3f46d6a5650c4f3c6e28fa126c6e56b38e1eb98f7c3e3594d0f
 2023-09-19 08:54:12 -05:00
Konstantin Akimov
f878b281ba fix: fillow-up Merge #18774: test: added test for upgradewallet RPC 
- partial dashification
 - disabling this test so far as it does not work anyway
 2023-09-19 08:54:12 -05:00
Konstantin Akimov
fab1031a70 fix: missing changes from Merge #18873: Fix intermittent sync_blocks failures 2023-09-19 08:54:12 -05:00
Sjors Provoost
fe1e8c2d13 fix: missing changes from Merge #18067: wallet: Improve LegacyScriptPubKeyMan::CanProvide script recognition 
[test] check for addmultisigaddress regression
 2023-09-19 08:54:12 -05:00
Konstantin Akimov
08a37a3c8c fix: missing changes from bitcoin#20034 test: Get rid of default wallet hacks 2023-09-19 08:54:12 -05:00
Konstantin Akimov
f093c9478c fix: dashification of of hashsums in get_previous_releases.py 2023-09-19 08:54:12 -05:00
Konstantin Akimov
8a9712e75b fix: dashification of test/get_previous_releases.py 2023-09-19 08:54:12 -05:00
MarcoFalke
a3684885e9 partial Merge bitcoin/bitcoin#20354: test: Add feature_taproot.py --previous_release 
fa80e10d94dbf86da84fc761b09fb631155a5b25 test: Add feature_taproot.py --previous_release (MarcoFalke)
85ccffa26686c6c9adbd18bdde37fc1747281bab test: move releases download incantation to README (Sjors Provoost)
29d6b1da2a862bfbb14e7821979c97416c5400e8 test: previous releases: add v0.20.1 (Sjors Provoost)

Pull request description:

  Disabling the new consensus code at runtime is fine, but potentially fragile and incomplete. Fix that by giving the option to run with a version that has been compiled without any taproot code.

ACKs for top commit:
  Sjors:
    tACK fa80e10
  NelsonGaldeman:
    tACK fa80e10d94dbf86da84fc761b09fb631155a5b25

Tree-SHA512: 1a1feef823f08c05268759645a8974e1b2d39a024258f5e6acecbe25097aae3fa9302c27262978b40f1aa8e7b525b60c0047199010f2a5d6017dd6434b4066f0
 2023-09-19 08:54:12 -05:00
MarcoFalke
67481eeb69 Merge bitcoin/bitcoin#22442: util: improves error messages on get_previous_releases script 
179a051704321ba40277a5855d6ac0dbb45689dd util: improves error messages on get_previous_releases script (Nelson Galdeman)

Pull request description:

  When previous releases are fetched and the specified version wasn't added to the checksum list we used to get a "Checksum did not match" which isn't true (https://github.com/bitcoin-core/bitcoincore.org/issues/753#issuecomment-879546719).

  If the specified version number is not on the list, it now logs cannot do the comparison instead.

ACKs for top commit:
  practicalswift:
    cr ACK 179a051704321ba40277a5855d6ac0dbb45689dd
  theStack:
    tACK 179a051704321ba40277a5855d6ac0dbb45689dd, tested on Debian bullseye/sid

Tree-SHA512: 2a07ce75232f853fd311c43581f8faf12d423668946ae6ad784feece5b4d0edd57fc018ba1f0c5a73bfaccb326e0df9a643580d16bf427c1ec3ff34a9cdbc80c
 2023-09-19 08:54:12 -05:00
Wladimir J. van der Laan
9cbe0de929 Merge #20955: test: Fix get_previous_releases.py for aarch64 
fa1d5e51374bce9607b429836bbd3fe059e3c78c test: Fix get_previous_releases.py for aarch64 (MarcoFalke)

Pull request description:

  Otherwise it will fail with "Not sure which binary to download..."

ACKs for top commit:
  laanwj:
    Code review ACK fa1d5e51374bce9607b429836bbd3fe059e3c78c

Tree-SHA512: 0db71e898a431665757ce835016a4e05c629a95abc4a2951eac9bd9b5876ec3dc3d6f156d58565e2bcdf918cde4f2649183d4a58038ac13c705a7e914c0094d1
 2023-09-19 08:54:12 -05:00
Wladimir J. van der Laan
b68936f8a8 Merge #19859: qa: Fixes failing functional test by changing version 
6de942908726480fb2919ed1f1b7906a63ec576d qa: Changes v0.17.1 to v0.17.2 (nthumann)

Pull request description:

  As of 0374e821bd v0.17.2 is downloaded instead of v0.17.1 for functional testing. This causes `test/functional/feature_backwards_compatibility.py` to fail, because it [requires](https://github.com/bitcoin/bitcoin/blob/master/test/functional/feature_backwards_compatibility.py#L57) v0.17.1.

  Steps to reproduce:
  Run `test/get_previous_releases.py -b v0.19.1 v0.18.1 v0.17.1 v0.16.3 v0.15.2`. It cannot be downloaded at all because the sha256sum is missing [here](c1e0c2ad3b/test/get_previous_releases.py (L23)).
  Or adjust the command and run `test/get_previous_releases.py -b v0.19.1 v0.18.1 v0.17.2 v0.16.3 v0.15.2`, then run `test/functional/test_runner.py feature_backwards_compatibility`. It´ll fail because the test is missing v0.17.1.

  This PR changes v0.17.1 to v0.17.2 in this test and in a few comments.

ACKs for top commit:
  laanwj:
    ACK 6de942908726480fb2919ed1f1b7906a63ec576d
  fanquake:
    ACK 6de942908726480fb2919ed1f1b7906a63ec576d - looks correct. Surprised this wasn't caught/part of #19813. In future you could add any explanations & extra info as part of your commit message as well (even though PR descriptions are included as part of the merge).

Tree-SHA512: bbe50c4fd5c1aedd6dc1cdc3d93ef9005db1c67adca3f263b6b0d869c40b495a3221e706c9389fedea4748e31911dbd591062f60ce9836e58099fbdd9515b4d9
 2023-09-19 08:54:12 -05:00
MarcoFalke
da35562fa1 Merge #19813: util, ci: Hard code previous release tarball checksums 
0374e821bd9e9498ce9c03aa8e5435870019978b util: Hard code previous release tarball checksums (Hennadii Stepanov)
bd897ce79f72a44a2e609f95433e251a3fd9eb9c scripted-diff: Move previous_release.py to test/get_previous_releases.py (Hennadii Stepanov)

Pull request description:

  #19205 introduced signature verifying for the downloaded `SHA256SUMS.asc`.
  This approach is brittle and does not work in CI environment for many reasons:
  - https://github.com/bitcoin/bitcoin/issues/19812#issuecomment-680760663
  - https://github.com/bitcoin/bitcoin/pull/19013#discussion_r459590779

  This PR:
  - implements **Sjors**' [idea](https://github.com/bitcoin/bitcoin/pull/19205#pullrequestreview-426080048):
  > Alternatively we might as well hard code the checksum for each `tar.gz` release in the source code, here.

  - is an alternative to 5a2c31e528e6bd60635096f233252f3c717f366d (#19013)

  - fixes #19812

  - updates v0.17.1 to v0.17.2

ACKs for top commit:
  MarcoFalke:
    cr ACK 0374e821bd9e9498ce9c03aa8e5435870019978b
  Sjors:
    tACK 0374e821bd9e9498ce9c03aa8e5435870019978b

Tree-SHA512: cacdcf9f5209eae7da357abb3445585ad2f980920fd5bf75527ce89974d3f531a4cf8b5b35edfc116b23bfdfb45c0437cb14cbc416d76ed2dc5b9e6d33cdad71
 2023-09-19 08:54:12 -05:00
MarcoFalke
7c43fa5426 Merge #19205: script: previous_release.sh rewritten in python 
9c34aff39309b8adc99d347e07b6ddb5366498e9 Remove previous_release.sh (Brian Liotti)
e1e5960e10a9329d9f55a3967d546ffbdd896030 script: Add previous_release.py (Brian Liotti)

Pull request description:

  Closes #18132

  Added functionality:
  1) checks file hash before untarring when using the binary download option

ACKs for top commit:
  fjahr:
    re-ACK 9c34aff39309b8adc99d347e07b6ddb5366498e9
  Sjors:
    tACK 9c34aff39309b8adc99d347e07b6ddb5366498e9

Tree-SHA512: 323f11828736a372a47f048592de8b027ddcd75b38f312dfc73f7b495d1e078bfeb384d9cdf434b3e70f2c6c0ce2da2df48e9a6460ac0e1967c6829a411c52d5
 2023-09-19 08:54:12 -05:00
Konstantin Akimov
7ddcf4a282 Merge #18864: Add v0.16.3 backwards compatibility test, bump v0.19.0.1 to v0.19.1 
d135c294764add81683ba47575f9a5dde7d7c07f [ci] make list of previous releases to download a setting (Sjors Provoost)
9c246b873c74834a121edba00fcaecf0cba6f9b4 [test] backwards compatibility: bump v0.19.0.1 to v0.19.1 (Sjors Provoost)
89a28e02fa46f3d5eb07ab02aa34aa95c6fcee11 [test] add v0.16.3 backwards compatibility test (Sjors Provoost)

Pull request description:

  Thanks to #18774's `adjust_bitcoin_conf_for_pre_17` we can now test backwards compatibility for v0.16.3, both for sync and loading a recent wallet.

  This PR bumps v0.19.0.1 to v0.19.1.

  I also made the version list consistent for the `contrib/devtools/previous_release.sh` instruction, between both tests.

ACKs for top commit:
  MarcoFalke:
    ACK d135c294764add81683ba47575f9a5dde7d7c07f

Tree-SHA512: 5ff137a7a934237fa220f1c2807ce9abeeb75929266558bf3e4045bec7dfcd0a8747fa74d700065c568330b18badf58c60c308eb13d1eed444d4bbfe6decc48b
 2023-09-19 08:54:12 -05:00
MarcoFalke
f68d12703d Merge #19153: test: mempool compatibility test 
16d4b3fd6d5aad18ebb731a5006a15180d3661ef test: mempool.dat compatibility between versions (Ivan Metlushko)

Pull request description:

  Rationale: Verify mempool.dat compatibility between versions

  The format of mempool.dat has been changed in #18038
  The tests verifies the fix made in #18807 and ensures that the file format is compatible between current version and v0.19.1
  The test verifies both backward and forward compatibility.

  This PR also adds a log when we fail to add a tx loaded from mempool.dat.
  It was useful when debugging this test and could be potentially useful to debug other scenarios as well.

  Closes #19037

ACKs for top commit:
  Sjors:
    tACK 16d4b3fd6d5aad18ebb731a5006a15180d3661ef

Tree-SHA512: 00a38bf528c6478cb0da467af216488f83c1e3ca4d9166c109202ea8284023e99d87a3d6e252c4d88d08d9b5ed1a730b3e1970d6e5c0aef526fa7ced40de7490
 2023-09-19 08:54:12 -05:00
MarcoFalke
a5cb668b83 Merge #18828: test: Strip down previous releases boilerplate 
fa359d14c09c6b139dead5da17c5a1c02f68393c test: Strip down previous releases boilerplate (MarcoFalke)

Pull request description:

  Reduces code bloat and mental load to write compatibility tests

ACKs for top commit:
  Sjors:
    tACK fa359d14c09c6b139dead5da17c5a1c02f68393c on macOS

Tree-SHA512: dc66286b24b2f137e5bca99412850ec7eee8cc61cf9cdc7ab532d529220808189baea8d1b077f8b7f40d3e8881d981e1ffc5a877adb394816f1225b1186253e4
 2023-09-19 08:54:12 -05:00
MarcoFalke
04cc0ac58f Merge #18534: test: skip backwards compat tests if not compiled with wallet 
c0c43ae1471347ea93614e9a25989f13b021f8a8 test: skip backwards compat tests if not compiled with wallet (fanquake)

Pull request description:

Top commit has no ACKs.

Tree-SHA512: d9975a1490e69134408b6b724cea26a6c1397d43f59850283b9e338ae38e00fefbcd868fb141e0a4bb55f02076690a99331f29cfa2d0fa66c165032b24a94081
 2023-09-19 08:54:12 -05:00
MarcoFalke
207b1c5877 Merge #12134: Build previous releases and run functional tests 
c456145b2c65f580683df03bf10cd39000cf24d5 [test] add 0.19 backwards compatibility tests (Sjors Provoost)
b769cd142deda74fe46e231cc7b687a86514f2f1 [test] add v0.17.1 wallet upgrade test (Sjors Provoost)
9d9390dab716f07057c94e8e21f3c7dd06192f35 [tests] add wallet backwards compatility tests (Sjors Provoost)
c7ca6308968b29a0e0edc485cd06e68e5edb7c7d [scripts] support release candidates of earlier releases (Sjors Provoost)
8b1460dbd1b732f06d4cebe1fa6844286c7a0056 [tests] check v0.17.1 and v0.18.1 backwards compatibility (Sjors Provoost)
ae379cf7d12943fc192d58176673bcfe7d53da53 [scripts] build earlier releases (Sjors Provoost)

Pull request description:

  This PR adds binaries for 0.17, 0.18 and 0.19 to Travis and runs a basic block propagation test.

  Includes test for upgrading v0.17.1 wallets and opening master wallets with older versions.

  Usage:

  ```sh
  contrib/devtools/previous_release.sh -f -b v0.19.0.1 v0.18.1 v0.17.1
  test/functional/backwards_compatibility.py
  ```

  Travis caches these earlier releases, so it should be able to run these tests with little performance impact.

  Additional scenarios where it might be useful to run tests against earlier releases:

  * creating a wallet with #11403's segwit implementation, copying it to an older node and making sure the user didn't lose any funds (although this PR doesn't support `v0.15.1`)
  * future consensus changes
  * P2P changes (e.g. to make sure we don't accidentally ban old nodes)

ACKs for top commit:
  MarcoFalke:
    ACK c456145b2c65f580683df03bf10cd39000cf24d5 🔨

Tree-SHA512: 360bd870603f95b14dc0cd629532cc147344f632b808617c18e1b585dfb1f082b401e5d493a48196b719e0aeaee533ae0a773dfc9f217f704aae898576c19232
 2023-09-19 08:54:12 -05:00
Kittywhiskers Van Gogh
41a6613fba
refactor: subsume CoinJoin objects under CJContext, deglobalize coinJoin{ClientQueueManager,Server} (#5337) 
## Motivation

CoinJoin's subsystems are initialized by variables and managers that
occupy the global context. The _extent_ to which these subsystems
entrench themselves into the codebase is difficult to assess and moving
them out of the global context forces us to enumerate the subsystems in
the codebase that rely on CoinJoin logic and enumerate the order in
which components are initialized and destroyed.

Keeping this in mind, the scope of this pull request aims to:

* Reduce the amount of CoinJoin-specific entities present in the global
scope
* Make the remaining usage of these entities in the global scope
explicit and easily searchable

## Additional Information

* The initialization of `CCoinJoinClientQueueManager` is dependent on
blocks-only mode being disabled (which can be alternatively interpreted
as enabling the relay of transactions). The same applies to
`CBlockPolicyEstimator`, which `CCoinJoinClientQueueManager` depends.

Therefore, `CCoinJoinClientQueueManager` is only initialized if
transaction relaying is enabled and so is its scheduled maintenance
task. This can be found by looking at `init.cpp`
[here](93f8df1c31/src/init.cpp (L1681-L1683)),
[here](93f8df1c31/src/init.cpp (L2253-L2255))
and
[here](93f8df1c31/src/init.cpp (L2326-L2327)).
  
For this reason, `CBlockPolicyEstimator` is not a member of `CJContext`
and its usage is fulfilled by passing it as a reference when
initializing the scheduling task.

* `CJClientManager` has not used `CConnman` or `CTxMemPool` as `const`
as existing code that is outside the scope of this PR would cast away
constness, which would be unacceptable. Furthermore, some logical paths
are taken that will grind to a halt if they are stored as `const`.

  Examples of such a call chains would be:

* `CJClientManager::DoMaintenance >
CCoinJoinClientManager::DoMaintenance > DoAutomaticDenominating >
CCoinJoinClientSession::DoAutomaticDenominating >
CCoinJoinClientSession::StartNewQueue > CConnman::AddPendingMasternode`
which modifies `CConnman::vPendingMasternodes`, which is non-const
behaviour

* `CJClientManager::DoMaintenance >
CCoinJoinClientManager::DoMaintenance > DoAutomaticDenominating >
CCoinJoin::IsCollateralValid > AcceptToMemoryPool` which adds a
transaction to the memory pool, which is non-const behaviour

* There were cppcheck [linter
failures](https://github.com/dashpay/dash/pull/5337#issuecomment-1685084688)
that seemed to be caused by the usage of `Assert` in
`coinjoin/client.h`. This seems to be resolved by backporting
[bitcoin#24714](https://github.com/bitcoin/bitcoin/pull/24714). (Thanks
@knst!)
    * Depends on #5546

---------

Co-authored-by: Kittywhiskers Van Gogh <63189531+kittywhiskers@users.noreply.github.com>
Co-authored-by: PastaPastaPasta <6443210+PastaPastaPasta@users.noreply.github.com>
 2023-09-13 12:52:38 -05:00
UdjinM6
be7dcd4281 get rid of magic 2299859813 in feature_asset_locks.py 2023-09-05 11:25:28 -05:00
Konstantin Akimov
c5d11a241b feat: use getblockcount() instead of over-complex node.getblock(node.getbestblockhash())["height"] 2023-09-05 11:25:28 -05:00
Konstantin Akimov
bdd38adc3f fix: change reward: 37.5% reward of masternodes are on platform now 2023-09-05 11:25:28 -05:00
Konstantin Akimov
5583890689 feat: masternode payment reallocation from coin base to platform 
Move funds from the coinbase, into the Asset Lock Pool. This is to incentivize MNs to upgrade to platform, because only MNs running platform will get these migrated rewards
 2023-09-05 11:25:28 -05:00
UdjinM6
baa28b9854
fix: Only approve triggers that match our expectations (#5565) 
## Issue being fixed or feature implemented
#5564 is a bit too optimistic about incoming triggers

## What was done?
Rework governance logic to only approve triggers that match our
expectations i.e. have the same data hash as our own trigger would have
if we would have to submit it.

## How Has This Been Tested?
Run tests

## Breaking Changes
Voting is done in `CreateGovernanceTrigger` only now meaning that it
only happens on next block for incoming triggers. Tweaked tests
accordingly.

## 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-09-05 10:04:21 -05:00
Kittywhiskers Van Gogh
8c742e9d15 merge bitcoin#23756: Fix implicit integer sign changes in strencodings 2023-09-04 20:50:27 -05:00
Kittywhiskers Van Gogh
1cf996ac8c merge bitcoin#21059: Drop boost/preprocessor dependencies 2023-09-04 20:50:27 -05:00
Odysseas Gabrielides
d080b4cc38
fix: vote funding yes when receiving triggers if hasn't created own trigger (#5564) 
## Issue being fixed or feature implemented
In case MNs didn't submit their own trigger, should vote for funding yes
when receiving triggers from other nodes.

## What was done?
Check if already submitted theirs and vote accordingly. 

## How Has This Been Tested?


## Breaking Changes


## 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-09-04 13:29:44 -05:00
Konstantin Akimov
ba68ea50f9
feat: various Asset Locks improvement (#5527) 
## What was done?
 - remove dependency of Asset Lock txes on CCreditPool
- new case for functional tests of Asset Locks - more than one output
for Asset Lock tx.


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

## Breaking Changes
Slightly changes behaviour of TxMempool. Tx can be accepted in mempool
even if Asset Unlock transaction with same index is already mined. But
final consensus rules are same.


## 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
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone
 2023-08-31 12:28:17 -05:00
MarcoFalke
c4234a5e78 Merge #18451: test: shift coverage from getunconfirmedbalance to getbalances 
0306d78cb49d1684cc96ba3512b582a1fdaf78cc Use getbalances in wallet_address_types tests (Jon Atack)
7eacdc5167c8db94df84e206db85817bc64e4921 Shift coverage from getunconfirmedbalance to getbalances in wallet_abandonconflict tests (Jon Atack)
3e6f7377f600e47e5e3d439fc5d6ccf3db210038 Improve getbalances coverage in wallet_balance tests (Jon Atack)

Pull request description:

  <strike>This PR updates several tests and then removes the `getunconfirmedbalance` RPC which was deprecated in facfb4111d14a3b06c46690a2cca7ca91cea8a96 a year ago.

  Next steps: remove the deprecated `getwalletinfo` fields and the `getbalance` RPC in follow-ups, if there seems to be consensus on those removals.</strike>

  Update:

  `getunconfirmedbalance` RPC was deprecated in facfb4111d14a3b06c46690a2cca7ca91cea8a96 a year ago, but following the review comments below, this PR now only updates the test coverage to use `getbalances` while still leaving basic coverage for `getunconfirmedbalance` in wallet_balance.py.

  That said, I've seen 3 regular contributors confused in the past 10 days by "DEPRECATED" warnings in the code that are not following the deprecation policy in [JSON-RPC-interface.md#versioning](https://github.com/bitcoin/bitcoin/blob/master/doc/JSON-RPC-interface.md#versioning).

  ISTM these warnings should either be removed, or the calls deprecated (`-deprecatedrpc`), or the policy updated to describe these warnings as a pre-deprecation practice.

ACKs for top commit:
  jnewbery:
    utACK 0306d78cb

Tree-SHA512: 692e43e9bed5afa97d905740666e365f0b64e559e1c75a6a398236d9e943894e3477947fc11324f420a6feaffa0c0c1532aa983c50090ca39d06551399e6ddd1
 2023-08-29 22:00:59 -05:00
Konstantin Akimov
4aa197dbdb Merge #18673: scripted-diff: Sort test includes 
fa4632c41714dfaa699bacc6a947d72668a4deef test: Move boost/stdlib includes last (MarcoFalke)
fa488f131fd4f5bab0d01376c5a5013306f1abcd scripted-diff: Bump copyright headers (MarcoFalke)
fac5c373006a9e4bcbb56843bb85f1aca4d87599 scripted-diff: Sort test includes (MarcoFalke)

Pull request description:

  When writing tests, often includes need to be added or removed. Currently the list of includes is not sorted, so developers that write tests and have `clang-format` installed will either have an unrelated change (sorting) included in their commit or they will have to manually undo the sort.

  This pull preempts both issues by just sorting all includes in one commit.

  Please be aware that this is **NOT** a change to policy to enforce clang-format or any other developer guideline or process. Developers are free to use whatever tool they want, see also #18651.

  Edit: Also includes a commit to bump the copyright headers, so that the touched files don't need to be touched again for that.

ACKs for top commit:
  practicalswift:
    ACK fa4632c41714dfaa699bacc6a947d72668a4deef
  jonatack:
    ACK fa4632c41714dfaa, light review and sanity checks with gcc build and clang fuzz build

Tree-SHA512: 130a8d073a379ba556b1e64104d37c46b671425c0aef0ed725fd60156a95e8dc83fb6f0b5330b2f8152cf5daaf3983b4aca5e75812598f2626c39fd12b88b180
 2023-08-29 22:00:59 -05:00
Kittywhiskers Van Gogh
e1d3be4adc partial bitcoin#11389: Support having SegWit always active in regtest 
excludes:
- d618458184
 2023-08-29 21:55:45 -05:00
Wladimir J. van der Laan
069624dd62 Merge #20844: test: Add sanitizer suppressions for AMD EPYC CPUs 
fa6c114ae604571435e8c4d25906a8b6d5b9984c test: Add sanitizer suppressions for AMD EPYC CPUs (MarcoFalke)

Pull request description:

  Currently the ci system only runs on intel cpus (and some arm devices), but it won't run on CPUs `Using the 'shani(1way,2way)' SHA256 implementation` (excerpt from debug log).

  For reference, google cloud CPUs (which is what Cirrus CI uses) print `Using the 'sse4(1way),sse41(4way),avx2(8way)' SHA256 implementation`

  The traceback I got:

  ```
  crypto/sha256_shani.cpp:87:18: runtime error: unsigned integer overflow: 0 - 1 cannot be represented in type 'size_t' (aka 'unsigned long')
      #0 0x55c0000e95ec in sha256_shani::Transform(unsigned int*, unsigned char const*, unsigned long) /root/bitcoin/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/crypto/sha256_shani.cpp:87:18
      #1 0x55bfffb926f8 in (anonymous namespace)::SelfTest() /root/bitcoin/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/crypto/sha256.cpp:517:9
      #2 0x55bfffb906ed in SHA256AutoDetect[abi:cxx11]() /root/bitcoin/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/crypto/sha256.cpp:626:5
      #3 0x55bfff87ab97 in BasicTestingSetup::BasicTestingSetup(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::vector<char const*, std::allocator<char const*> > const&) /root/bitcoin/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/test/util/setup_common.cpp:104:5
      #4 0x55bffe885877 in main /root/bitcoin/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/qt/test/test_main.cpp:52:27
      #5 0x7f20c3bf60b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x270b2)
      #6 0x55bffe7a5f6d in _start (/root/bitcoin/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src/qt/test/test_bitcoin-qt+0x1d00f6d)

  SUMMARY: UndefinedBehaviorSanitizer: unsigned-integer-overflow crypto/sha256_shani.cpp:87:18 in

ACKs for top commit:
  laanwj:
    Anyhow ACK fa6c114ae604571435e8c4d25906a8b6d5b9984c

Tree-SHA512: 968a1d28eedec58c337b1323862f583cb1bcd78c5f03396940b9ab53ded12f8c6652877909aba05ee5586532137418fd817ff979bd7bef6e07856094f9d7f9b1
 2023-08-29 21:40:46 -05:00
Odysseas Gabrielides
ceb84d5b51
feat: Superblock creation (Sentinel elimination) (#5525) 
## Issue being fixed or feature implemented

Implementation of issue https://github.com/dashpay/dash-issues/issues/43

## What was done?

Masternode will try to create, sign and submit a Superblock (GovTrigger)
during the `nSuperblockMaturityWindow`.

## How Has This Been Tested?

## Breaking Changes


## Checklist:
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [ ] 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>
Co-authored-by: PastaPastaPasta <6443210+PastaPastaPasta@users.noreply.github.com>
 2023-08-29 10:31:59 -05:00
MarcoFalke
00d2d7fac3 Merge #21042: doc, test: Improve setup_clean_chain documentation 
590bda79e876d9b959083105b8c7c41dd87706eb scripted-diff: Remove setup_clean_chain if default is not changed (Fabian Jahr)
98892f39e3d079c73bff7f2a5d5420fa95270497 doc: Improve setup_clean_chain documentation (Fabian Jahr)

Pull request description:

  The first commit improves documentation on setup_clean_chain which is misunderstood quite frequently. Most importantly it fixes the TestShell docs which are simply incorrect.

  The second commit removes the instances of `setup_clean_clain` in functional tests where it is not changing the default.

  This used to be part of #19168 which also sought to rename`setup_clean_chain`.

ACKs for top commit:
  jonatack:
    ACK 590bda79e876d9b959083105b8c7c41dd87706eb

Tree-SHA512: a7881186e65d31160b8f84107fb185973b37c6e50f190a85c6e2906a13a7472bb4efa9440bd37fe0a9ac5cd2d1e8559870a7e4380632d9a249eca8980b945f3e
 2023-08-28 11:31:55 -05:00
fanquake
283c5592c8 Merge bitcoin/bitcoin#18418: wallet: Increase OUTPUT_GROUP_MAX_ENTRIES to 100 
e6fe1c37d0a2f8037996dd80619d6c23ec028729 rpc: Improve avoidpartialspends and avoid_reuse documentation (Fabian Jahr)
8f073076b102b77897e5a025ae555baae3d1f671 wallet: Increase OUTPUT_GROUP_MAX_ENTRIES to 100 (Fabian Jahr)

Pull request description:

  Follow-up to #17824.

  This increases OUTPUT_GROUP_MAX_ENTRIES to 100 which means that OutputGroups will now be up to 100 outputs large, up from previously 10. The main motivation for this change is that during the PR review club on #17824 [several participants signaled](https://bitcoincore.reviews/17824.html#l-339) that 100 might be a better value here.

  I think fees should be manageable for users but more importantly, users should know what they can expect when using the wallet with this configuration, so I also tried to clarify the documentation on `-avoidpartialspends` and `avoid_reuse` a bit. If there are other additional ways how or docs where users can be made aware of the potential consequences of using these parameters, please let me know. Another small upside is that [there seem to be a high number of batching transactions with 100 and 200 inputs](https://miro.medium.com/max/3628/1*sZ5eaBSbsJsHx-J9iztq2g.png)([source](https://medium.com/@hasufly/an-analysis-of-batching-in-bitcoin-9bdf81a394e0)) giving these transactions a bit of a larger anonymity set, although that is probably a very weak argument.

ACKs for top commit:
  jnewbery:
    ACK e6fe1c37d0
  Xekyo:
    retACK e6fe1c37d0a2f8037996dd80619d6c23ec028729
  rajarshimaitra:
    tACK `e6fe1c3`
  achow101:
    ACK e6fe1c37d0a2f8037996dd80619d6c23ec028729
  glozow:
    code review ACK e6fe1c37d0

Tree-SHA512: 79685c58bafa64ed8303b0ecd616fce50fc9a2b758aa79833e4ad9f15760e09ab60c007bc16ab4cbc4222e644cfd154f1fa494b0f3a5d86faede7af33a6f2826
 2023-08-28 11:31:55 -05:00
MarcoFalke
7bd149f034 Merge #21388: doc: Rename fuzz seed_dir to corpus_dir 
fad0ae6bb8e10b5cb82a5ec014e59b5aafc85b5e doc: Rename fuzz seed_dir to corpus_dir (MarcoFalke)

Pull request description:

  The fuzz corpus directory might contain hand-crafted seeds, but generally it is a set of test inputs. See also https://github.com/google/fuzzing/blob/master/docs/glossary.md#corpus

ACKs for top commit:
  practicalswift:
    cr ACK fad0ae6bb8e10b5cb82a5ec014e59b5aafc85b5e: patch looks correct and "why not?" :)
  fanquake:
    ACK fad0ae6bb8e10b5cb82a5ec014e59b5aafc85b5e - did not test

Tree-SHA512: 38c952feb07aeeeb038b3261a12c824fab9ce5153d75f0ecf6d3f43db4f50998eeb2b14b11b7155f529189c93783fa2c11c81059021a04398c43f3505b31a2d4
 2023-08-28 11:24:41 -05:00
MarcoFalke
9daa8a2fd0 (Partial) Merge #21053: rpc, test: document {previous,next}blockhash as optional 
ba7e17e073f833eccd4c7c111ae9058c3f123371 rpc, test: document {previous,next}blockhash as optional (Sebastian Falbesoner)

Pull request description:

  This PR updates the result help of the following RPCs w.r.t. the `previousblockhash` and `nextblockhash` fields:
  - getblockheader
  - getblock

  Also adds trivial tests on genesis block (should not contain "previousblockhash") and best block (should not contain "nextblockhash").

Top commit has no ACKs.

Tree-SHA512: ef42c5c773fc436e1b4a67be14e2532e800e1e30e45e54a57431c6abb714d2c069c70d40ea4012d549293b823a1973b3f569484b3273679683b28ed40abf46bb
 2023-08-28 11:24:41 -05:00
Vijay Das Manikpuri
cc885e0b33 (partial) Merge #18764: refactor: test: replace inv type magic numbers by constants 2023-08-23 12:36:35 -05:00
MarcoFalke
d5fbd4a92a Merge #18672: test: add further BIP37 size limit checks to p2p_filter.py 
c7437185589926ec8def2af6bede6a407b3d2e4a test: add further BIP37 size limit checks to p2p_filter.py (Sebastian Falbesoner)

Pull request description:

  This is a follow-up PR to #18628. In addition to the hash-functions limit test introduced with commit fa4c29bc1d, it adds checks for the following size limits as defined in [BIP37](https://github.com/bitcoin/bips/blob/master/bip-0037.mediawiki):

  ad message type `filterload`:
  > The filter itself is simply a bit field of arbitrary byte-aligned size. The maximum size is **36,000 bytes**.

  ad message type `filteradd`:
  > The data field must be smaller than or equal to **520 bytes** in size (the maximum size of any potentially matched object).

  Also introduces new constants for the limits (or reuses the max script size constant in case for the `filteradd` limit).

  Also fixes #18711 by changing the misbehaviour check on "filteradd without filterset" (introduced with #18544) below to also use the more commonly used `assert_debug_log` method.

ACKs for top commit:
  MarcoFalke:
    ACK c7437185589926ec8def2af6bede6a407b3d2e4a
  robot-visions:
    ACK c7437185589926ec8def2af6bede6a407b3d2e4a
  jonasschnelli:
    utACK c7437185589926ec8def2af6bede6a407b3d2e4a. Seems to fix it: https://bitcoinbuilds.org/index.php?build=2524

Tree-SHA512: a03e7639263eb36a381922afb4e1d0ed2ae286f2ad2e7bbd922509a043ddf6cfd08747e01d54d29bfb8f54b66908f653974b9c347e4ca4f43332b586778893be
 2023-08-23 12:36:35 -05:00
MarcoFalke
a0b608d5a5 Merge #18544: net: limit BIP37 filter lifespan (active between 'filterload'..'filterclear') 
a9ecbdfcaa15499644d16e9c8ad2c63dfc45b37b test: add more inactive filter tests to p2p_filter.py (Sebastian Falbesoner)
5eae034996b340c19cebab9efb6c89d20fe051ef net: limit BIP37 filter lifespan (active between 'filterload' and 'filterclear') (Sebastian Falbesoner)

Pull request description:

  This PR fixes https://github.com/bitcoin/bitcoin/issues/18483. On the master branch, there is currently _always_ a BIP37 filter set for every peer: if not a specific filter is set through a `filterload` message, a default match-everything filter is instanciated and pointed to via the `CBloomFilter` default constructor; that happens both initially, when the containing structure `TxRelay` is constructed:

  c0b389b335/src/net.h (L812)

  and after a loaded filter is removed again through a `filterclear` message:

  c0b389b335/src/net_processing.cpp (L3201)

  The behaviour was introduced by commit 37c6389c5a (an intentional covert fix for [CVE-2013-5700](https://github.com/bitcoin/bitcoin/pull/18515), according to gmaxwell).

  This default match-everything filter leads to some unintended side-effects:
  1. `getdata` request for filtered blocks (i.e. type `MSG_FILTERED_BLOCK`) are always responded to with `merkleblock`s, even if no filter was set by the peer, see issue #18483 (strictly speaking, this is a violation of BIP37) c0b389b335/src/net_processing.cpp (L1504-L1507)
  2. if a peer sends a `filteradd` message without having loaded a filter via `filterload` before, the intended increasing of the banscore never happens (triggered if `bad` is set to true, a few lines below) c0b389b335/src/net_processing.cpp (L3182-L3186)

  This PR basically activates the `else`-branch code paths for all checks of `pfilter` again (on the master branch, they are dead code) by limiting the pointer's lifespan: instead of always having a filter set, the `pfilter` is only pointing to a `CBloomFilter`-instance after receiving a `filterload` message and the instance is destroyed again (and the pointer nullified) after receiving a `filterclear` message.

  Here is a before/after comparison in behaviour:
  | code part / scenario                          |    master branch                   |   PR branch                                          |
  | --------------------------------------------- | ---------------------------------- | ---------------------------------------------------- |
  | `getdata` processing for `MSG_FILTERED_BLOCK` | always responds with `merkleblock` | only responds if filter was set via `filterload`     |
  | `filteradd` processing, no filter was loaded  | nothing                            | peer's banscore increases by 100 (i.e. disconnect)   |

  On the other code parts where `pfilter` is checked there is no change in the logic behaviour (except that `CBloomFilter::IsRelevantAndUpdate()` is unnecessarily called and immediately returned in the master branch).
  Note that the default constructor of `CBloomFilter` is only used for deserializing the received `filterload` message and nowhere else. The PR also contains a functional test checking that sending `getdata` for filtered blocks is ignored by the node if no bloom filter is set.

ACKs for top commit:
  MarcoFalke:
    re-ACK a9ecbdfcaa, only change is in test code 🕙

Tree-SHA512: 1a656a6d74ccaf628e7fdca063ba63fbab2089e0b6d0a11be9bbd387c2ee6d3230706ff8ffc1a55711481df3d4547137dd7c9d9184d89eaa43ade4927792d0b6
 2023-08-23 12:36:35 -05:00
MarcoFalke
55424ea3f3 (partial) Merge #18628: test: Add various low-level p2p tests 
fa4c29bc1d2425f861845bae4f3816d9817e622a test: Add various low-level p2p tests (MarcoFalke)

Pull request description:

ACKs for top commit:
  jonatack:
    ACK fa4c29bc1d242

Tree-SHA512: 842821b97359d4747c763398f7013415858c18a300cd882887bc812d039b5cbb67b9aa6f68434575dbc3c52f7eb8c43d1b293a59555a7242c0ca615cf44dc0aa
 2023-08-23 12:36:35 -05:00
MarcoFalke
f362cf5a55 Merge #18726: test: check misbehavior more independently in p2p_filter.py 
cd543d9193ac1882c1b4a8a84e3ac7356a8b7ce9 test: check misbehavior more independently in p2p_filter.py (Danny Lee)

Pull request description:

  This expands on #18672 in two ways:

  - Check positive cases (`filterload` accepted, `filteradd` accepted) in addition to the negative cases added in #18672
  - Address MarcoFalke 's [suggestion](https://github.com/bitcoin/bitcoin/pull/18672#discussion_r412101752) to successfully load a filter before testing `filteradd`

ACKs for top commit:
  theStack:
    re-ACK cd543d9193

Tree-SHA512: f82402f6287ccddf08b38b6432d5e2b2b2ef528802a981d04c24bac459022f732d9090d4849d72d3d1eb2c757161dcb18c4c036b6e11dc80114e9cd49f21c3bd
 2023-08-23 12:36:35 -05:00
Konstantin Akimov
15bca8493b feat!: replaced CSkipList to CRangesSet in credit pool 
By design we can have more and more and more gaps in indexes list so far as
we can not re-sign expired transaction of asset-unlock. CRangesList is protected from this situation
 2023-08-21 10:19:29 -05:00
PastaPastaPasta
690f47c493
Merge pull request #5490 from vijaydasmp/bp22_2 
backport: Merge bitcoin#20023, 21713, 20575, 21989, 20971, 20964, 20497, 20425, 19980, (partial) 20125
 2023-08-20 23:39:50 -05:00
Odysseas Gabrielides
93f8df1c31
refactor: Global renaming from hpmn to evo (#5508) 
## Issue being fixed or feature implemented

## What was done?
Renaming of all classes/variables/functions/rpcs from `hpmn` to `evo`.

## How Has This Been Tested?
All unit and func tests are passing.
Sync of Testnet.

## Breaking Changes
All protx RPCs ending with `_hpmn` were converted to `_evo`.
`_hpmn` RPCs are now deprecated.
Although, they can still be enabled by adding `-deprecatedrpc=hpmn`.


## Checklist:
- [x] I have performed a self-review of my own code
- [ ] 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 _(for repository
code-owners and collaborators only)_

---------

Co-authored-by: thephez <thephez@users.noreply.github.com>
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
 2023-08-17 14:01:12 -05:00
UdjinM6
9f7322b34a
feat: Add -chainlocknotify cmd-line option, update -instantsendnotify (#5522) 
## Issue being fixed or feature implemented
Execute command when the best chainlock changes (`%s` in cmd is replaced
by chainlocked block hash). Same as `-blocknotify` but for chainlocks.
Let `-instantsendnotify` replace `%w` with wallet name like
`-walletnotify` does.

## What was done?

## How Has This Been Tested?
run tests

## 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
 2023-08-15 11:10:21 -05:00
Wladimir J. van der Laan
a365ed03c0 Merge #20683: test: Fix restart node race 
fab46b34f4b13abbb0af276c3fb548f25ccc28bd test: Fix restart node race (MarcoFalke)

Pull request description:

  It is not allowed to start a node before it has been fully stopped. Otherwise it could lead to intermittent issues due to access issues (e.g. cookie file https://cirrus-ci.com/task/6409665024098304?command=ci#L4793)

  Fix that by waiting for the node to fully stop.

ACKs for top commit:
  laanwj:
    code review ACK fab46b34f4b13abbb0af276c3fb548f25ccc28bd

Tree-SHA512: 7605cac0573a7b04f05ff110d0131e8940d87f7baf6d698505ed16b363d4d15b1e552c5ffd1a187c8fe5639f7e265c3122734c85283275746e46bd789614fd21
 2023-08-08 06:33:29 -05:00
MarcoFalke
45399b96a7 Merge #20569: test: Fix intermittent wallet_multiwallet issue with got_loading_error 
fab48da908f3f81135b9163edf5011d1e5f6ef6e test: Fix intermittent wallet_multiwallet issue with got_loading_error (MarcoFalke)
fa8e15f7b75e35846b86e8627a3612e31eb22dcb test: pep8 wallet_multiwallet.py (MarcoFalke)

Pull request description:

  Failing the test after 10 iterations without a loading error is problematic because it may take 11 iterations to get a loading error.

  Fix that by running until a loading error occurs, which should happen in almost all runs within the first 10 iterations.

ACKs for top commit:
  ryanofsky:
    Code review ACK fab48da908f3f81135b9163edf5011d1e5f6ef6e. This seems like a good workaround. I think more ideally think load and unload RPCs would not have racy status reporting (suggested previously https://github.com/bitcoin/bitcoin/pull/19300#pullrequestreview-435362710 and

Tree-SHA512: 6b80b26d916276efe2a01af93bca7dbf71a3e67db9d3deb15175070719bf7d1325a1410d93e74c0316942e388faa2ba185dc9d3759c82d1c73c3c509b9997f05
 2023-08-08 06:26:09 -05:00
MarcoFalke
9a3abd973c Merge #20613: test: Use Popen.wait instead of RPC in assert_start_raises_init_error 
fa918dd537fea775c19a590e5f9161bf51a5839b test: Use Popen.wait instead of RPC in assert_start_raises_init_error (MarcoFalke)

Pull request description:

  Using RPC (`wait_for_rpc_connection`) has several issue:

  * It polls in a loop, which might be slow
  * It tries to read the RPC cookie file, which might not be present, thus leading to intermittent issues

  Fix both by using `Popen.wait`

ACKs for top commit:
  laanwj:
    Code review ACK ~~faf7b05be9c86ee61c39e5314511fe2410128a6b~~ fa918dd537fea775c19a590e5f9161bf51a5839b
  darosior:
    ACK fa918dd537fea775c19a590e5f9161bf51a5839b

Tree-SHA512: 5368ad0d0ea2deb0af9582a42667c9290efe8f2705f37a236afc2c7908b04265ab342e2dd356a57156e99389f4a27ab6da9fa7bf9161fb7568240aa005e693b9
 2023-08-08 06:26:09 -05:00
Konstantin Akimov
93bd0c70a2
refactor: rename assetLockedAmount in CbTx to creditPoolBalance (#5526) 
## Issue being fixed or feature implemented
Bad naming is noticed in https://github.com/dashpay/dash/pull/5026 by
thephez

## What was done?
Renamed `assetLockedAmount` in CbTx to `creditPoolBalance`
Renamed also some local variables and functions to make it matched also.

## How Has This Been Tested?
Run functional/unit tests - succeed
Called python's rpc binding `node.getblock(block_hash)['cbTx']`:
Got this result:
```
{'version': 3, 'height': 1556, 'merkleRootMNList': '978b2b4d1b884de62799b9eaee75c7812fea59f98f80d5ff9c963b0f0f195e14', 'merkleRootQuorums': 'bc7a34eb114f4e4bf38a11080b5d8ac41bdb36dd41e17467bae23c94ba06b013', 'bestCLHeightDiff': 0, 'bestCLSignature': '000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000', 'creditPoolBalance': Decimal('7.00141421')}
```

## Breaking Changes
Renamed `assetLockedAmount` in CbTx to `creditPoolBalance`. @shumkov be
informed


## Checklist:
- [x] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [x] 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-08-08 05:49:31 -05:00
MarcoFalke
2c9d41d073 Merge #18454: net: Make addr relay mockable, add test 
NOTE: There is slight difference with original backport due to future changes
in bitcoin#19272, bitcoin#19763 - otherwise functional test p2p_addr_relay.py fails

fa1da3d4bfc0511a89f5b19d5a4d89e55ff7ccde test: Add basic addr relay test (MarcoFalke)
fa1793c1c44a3f75a09f9c636467b8274c541bdd net: Pass connman const when relaying address (MarcoFalke)
fa47a0b003f53708b6d5df1ed4e7f8a7c68aa3ac net: Make addr relay mockable (MarcoFalke)

Pull request description:

  As usual:

  * Switch to std::chrono time to be type-safe and mockable
  * Add basic test that relies on mocktime to add code coverage

ACKs for top commit:
  naumenkogs:
    utACK  fa1da3d
  promag:
    ACK fa1da3d4bfc0511a89f5b19d5a4d89e55ff7ccde (fabe56e44b6f683e24e37246a7a8851190947cb3 before https://github.com/bitcoin/bitcoin/pull/18454#issuecomment-607866453), fa5bf23d527a450e72c2bf13d013e5393b664ca3 was dropped since last review.

Tree-SHA512: 0552bf8fcbe375baa3cab62acd8c23b2994efa47daff818ad1116d0ffaa0b9e520dc1bca2bbc68369b25584e85e54861fe6fd0968de4f503b95439c099df9bd7

fixup - see #19272, #19763
 2023-08-03 11:16:41 -05:00
MarcoFalke
5460866184 Merge #18561: test: Properly raise FailedToStartError when rpc shutdown before warmup finished 
faede1b293354560317b67f0b4e6874dcac6ef41 test: Properly raise FailedToStartError when rpc shutdown before warmup finished (MarcoFalke)

Pull request description:

  Should fix issues such as https://travis-ci.org/github/bitcoin/bitcoin/jobs/671910152#L7034

Top commit has no ACKs.

Tree-SHA512: ac659f29c5ec91985c916b734e24911cbf4e2c5c4b1f1891a7e6c2d2511ec285167550fb03848eee4a7a3cbc9f8cdb0c766f4e881d9e44368c7415d007006368
 2023-08-03 11:16:41 -05:00
MarcoFalke
2046ae1bc0 Merge #18546: Bugfix: Wallet: Safely deal with change in the address book [part 2] 
7a2ecf16df938dd95d3130a46082def7a02338eb Wallet: Change IsMine check in CWallet::DelAddressBook from assert to failure (Luke Dashjr)
2952c46b923042f2de801f319e03ed5c4c4eb735 Wallet: Replace CAddressBookData.name with GetLabel() method (Luke Dashjr)
d7092c392e10889cd7a080b3d22ed6446a59b87a QA: Test that change doesn't turn into non-change when spent in an avoid-reuse wallet (Luke Dashjr)

Pull request description:

  Follow-up to #18192, not strictly necessary for 0.20

ACKs for top commit:
  MarcoFalke:
    re-ACK 7a2ecf16df, only change is adding an assert_equal in the test 🔰
  jnewbery:
    utACK 7a2ecf16df938dd95d3130a46082def7a02338eb

Tree-SHA512: e0933ee40f705b751697dc27249e1868ed4874254b174ebdd0a7150125d8c818402e66df2371718c7eeb90e67ee2317215fb260aa9b9d7b9b45ee436de2988ff
 2023-08-03 11:16:41 -05:00
Kittywhiskers Van Gogh
d577d8bc57 merge bitcoin#21796: Avoid async shutdown on init error 2023-08-02 10:19:02 -05:00
Kittywhiskers Van Gogh
d97dcb22e1 merge bitcoin#22047: Coinstatsindex follow-ups 2023-08-02 10:19:02 -05:00
Kittywhiskers Van Gogh
28dcd381cd fix: adjust values of gettxoutsetinfo to match Dash's mint rate 2023-08-02 10:19:02 -05:00
Kittywhiskers Van Gogh
31bbd52784 fix: implement subset of GetBlockSubsidy for feature_coinstatsindex 
It's highly unlikely the test will ever deal with chains with >4500
blocks, so only the subset of the subsidy logic that is needed to
validate `gettxoutsetinfo` output has been included
 2023-08-02 10:19:02 -05:00
Kittywhiskers Van Gogh
040cd922f6 merge bitcoin#19521: Coinstats Index 2023-08-02 10:19:02 -05:00
Konstantin Akimov
d914bf2b6e
feat: add bloom filter for Asset Lock transactions (#5521) 
## Issue being fixed or feature implemented
There's one type of output that potentially can be useful for bloom
filter.
It's follow-up for TODO for dashpay/dash#4857.

Asset  Lock transactions have:
 - standard inputs (covered by regular bloom filter implementation)
 - standard outputs (covered by regular bloom filter implementation)
- special outputs that have public key to proof owing this credits on
platform and claiming it.

Asset Unlock transactions have:
 - no inputs (no need bloom)
 - standard outputs (covered by regular bloom filter implementation)

So far as there's only one special case, let's have this data in the
bloom filter because it can potentially help to show information such as
"Deposit to platform" on mobile clients.

## What was done?
 - added special case for Asset Lock transactions for bloom filter

## How Has This Been Tested?
Run unit/functional tests. Doesn't actually tested how bloom filter
works.


## 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

---------

Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
 2023-08-02 10:08:39 -05:00
fanquake
de28a0e10c Merge #20530: lint, refactor: Update cppcheck linter to c++17 and improve explicit usage 
1e62350ca20898189904a88dfef9ea11ddcd8626 refactor: Improve use of explicit keyword (Fabian Jahr)
c502a6dbfb854ca827a5a3925394f9e09d29b898 lint: Use c++17 std in cppcheck linter (Fabian Jahr)

Pull request description:

  I found the `extended-lint-cppcheck` linter still uses `std=c++11` when reviewing #20471. The only difference in the output after this change is one line is missing:

  ```
  src/script/descriptor.cpp:159:5: warning: Struct 'PubkeyProvider' has a constructor with 1 argument that is not explicit. [noExplicitConstructor]
  ```

  After some digging, I am still not sure why this one is ignored with c++17 when 40 other`noExplicitConstructor` warnings were still appearing.

  In the second commit, I fix these warnings, adding `explicit` where appropriate and adding fixes to ignore otherwise.

ACKs for top commit:
  practicalswift:
    cr ACK 1e62350ca20898189904a88dfef9ea11ddcd8626: patch looks correct!
  MarcoFalke:
    review ACK 1e62350ca20898189904a88dfef9ea11ddcd8626

Tree-SHA512: dff7b324429a57160e217cf38d9ddbb6e70c6cb3d3e3e0bd4013d88e07afc2292c3df94d0acf7122e9d486322821682ecf15c8f2724a78667764c05d47f89a12
 2023-08-01 12:24:36 -05:00
Wladimir J. van der Laan
361d1e18d1 Merge #20606: Remove unused bits from service flags enum 
fa40168ab3102b9ad850f967a0e7fa22dbfbd0c6 Remove unused bits from service flags enum (MarcoFalke)

Pull request description:

  Remove service bits that haven't been observed on the active network for years and won't ever be observed on the network with this meaning. Keeping this dead assignment in our source code forever doesn't add any value.

  I somehow forgot to do this in commit fa0d0ff6e1bee60fde63724ae28a51aac5a94d4a.

ACKs for top commit:
  laanwj:
    Code review ACK fa40168ab3102b9ad850f967a0e7fa22dbfbd0c6
  practicalswift:
    cr ACK fa40168ab3102b9ad850f967a0e7fa22dbfbd0c6
  fanquake:
    ACK fa40168ab3102b9ad850f967a0e7fa22dbfbd0c6

Tree-SHA512: 376e5ac05940493cf2209fea60515c843e978c4b476f2524f6bf7a37a646d237c3ddcf6c0fa23641f9ba550f625609703d9b51b4be631a7f2a90e1092b557232
 2023-08-01 12:21:16 -05:00
Wladimir J. van der Laan
b25fa7dcb9 Merge #20568: doc: Use FeeModes doc helper in estimatesmartfee 
fa8abdc9953e381715493b259908e246914793b0 rpc: Use FeeModes doc helper in estimatesmartfee (MarcoFalke)

Pull request description:

  Not sure why this doesn't use the doc helper, probably an oversight?

ACKs for top commit:
  laanwj:
    Code review ACK fa8abdc9953e381715493b259908e246914793b0

Tree-SHA512: 1f2dc8356e3476ddcf9cafafa7f9865ad95bed1e3067c0edab8e3c483e374bdbdbecc066167554b4a1b479e28f6a52c4ae6a75a70c67ee4e1ff4f3ba36b04001
 2023-08-01 12:21:16 -05:00
Odysseas Gabrielides
6bacf5423b
feat: v20 evonodes payment adjustment (#5493) 
## Issue being fixed or feature implemented
Since v19, Evo nodes are paid 4x blocks in a row.
This needs to be reverted when MN Reward Reallocation activates.

## What was done?
Starting from MN Reward Reallocation activation, Evo nodes are paid one
block in a row (like regular masternodes).
In addition, `nConsecutivePayments` isn't incremented anymore for Evo
nodes.

## How Has This Been Tested?
`feature_llmq_hpmn.py` with MN Reward Reallocation activation.

## Breaking Changes
no

## Checklist:
- [x] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [x] 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: Konstantin Akimov <knstqq@gmail.com>
 2023-07-31 23:52:48 -05:00
Kittywhiskers Van Gogh
d40f28edb4 merge bitcoin#19762: Allow named and positional arguments to be used together 2023-07-28 00:18:27 -05:00
Kittywhiskers Van Gogh
b1643e7c86 merge bitcoin#21575: Create blockstorage module 2023-07-28 00:18:27 -05:00
Kittywhiskers Van Gogh
3450cc755c merge bitcoin#21297: feature_blockfilterindex_prune.py improvements 2023-07-28 00:18:27 -05:00
Kittywhiskers Van Gogh
8cf0748bba merge bitcoin#21252: Intermittent issue in feature_blockfilterindex_prune 2023-07-28 00:18:27 -05:00
Kittywhiskers Van Gogh
e30626dca5 merge bitcoin#21230: Fix NODE_NETWORK_LIMITED_MIN_BLOCKS disconnection 2023-07-28 00:18:27 -05:00
Kittywhiskers Van Gogh
6c09b33479 merge bitcoin#15946: Allow maintaining the blockfilterindex when using prune 2023-07-28 00:18:27 -05:00
Kittywhiskers Van Gogh
9307a22117 merge bitcoin#19550: Add getindexinfo RPC 2023-07-28 00:18:27 -05:00
Wladimir J. van der Laan
331991b0d0
Merge #20964: rpc: Add specific error code for "wallet already loaded" 
a6739cc86827759c543bf81f5532ec46e40549c3 rpc: Add specific error code for "wallet already loaded" (Wladimir J. van der Laan)

Pull request description:

  Add a separate RPC error code for "wallet already loaded" to avoid having to match on message to detect this.
  Requested by shesek for rust-bitcoinrpc.

  If concept ACKed needs:
  - [ ]  Release note
  - [x]  A functional test (updated the existing test to make it pass, I think this is enough)

ACKs for top commit:
  jonasschnelli:
    Code Review ACK a6739cc86827759c543bf81f5532ec46e40549c3
  promag:
    Code review ACK a6739cc86827759c543bf81f5532ec46e40549c3.

Tree-SHA512: 9091872e6ea148aec733705d6af330f72a02f23b936b892ac28f9023da7430af6332418048adbee6014305b812316391812039e9180f7f3362d11f206c13b7d0
 2023-07-26 09:37:52 +05:30
MarcoFalke
720eb7e352
Merge #20971: test: Work around libFuzzer deadlock 
fa39c8a3e8f1090103468780d122a4cf4191bc13 test: Work around libFuzzer deadlock (MarcoFalke)

Pull request description:

  Only required part is `symbolize=0`, but the other changes shouldn't hurt

ACKs for top commit:
  practicalswift:
    cr ACK fa39c8a3e8f1090103468780d122a4cf4191bc13: patch looks correct

Tree-SHA512: 9cddf1de46ad12aea9b8be2c1acb86ba0e07ffdb52f8155d943edf970955551c7cb049a3a6c027846b45dab0dc0966dec42999476ebde50aa761a08dbb751eae
 2023-07-26 09:37:52 +05:30
MarcoFalke
8db2aabee0
Merge bitcoin/bitcoin#21989: test: Use COINBASE_MATURITY in functional tests 
bfa9309ad606102f24c9bd3c33dfe78949f09418 Use COINBASE_MATURITY constant in functional tests. (Kiminuo)
525448df9dc2ab6b7e960ff138956ae3e2efdf60 Move COINBASE_MATURITY from `feature_nulldummy` test to `blocktools`. (Kiminuo)

Pull request description:

  `COINBASE_MATURITY` constant was added to `feature_nulldummy` test in #21373. This PR moves the constant to `blocktools.py` file and uses the constant in more tests as suggested [here](https://github.com/bitcoin/bitcoin/pull/21373#discussion_r605418462).

  Edit: Goal of this PR is to replace integer constants with `COINBASE_MATURITY` but not necessarily in *all* cases because that would mean to read and fully understand all tests. That's out of my time constraints. Any reports where `COINBASE_MATURITY` should be used are welcome though!

ACKs for top commit:
  theStack:
    ACK bfa9309ad606102f24c9bd3c33dfe78949f09418 🌇

Tree-SHA512: 01f04645f05a39028681f355cf3d42dd63ea3303f76d93c430e0fdce441934358a2d847a54e6068d61932f1b75e1d406f51859b057b3e4b569f7083915cb317f
 2023-07-26 09:37:52 +05:30
MarcoFalke
e649229046
Merge #20023: test: remove unused constants in functional tests 
92e28fa8b2590cce0e8f0adadae80e46cb63a9ef test: remove unused constants in functional tests (Sebastian Falbesoner)

Pull request description:

  This mini-PR gets rid of constants in functional tests that are not used anymore. Found by [vulture ](https://pypi.org/project/vulture/)via the following script that has been lying around here locally for quite some time (I think it was once proposed by practicalswift, but I don't remember the concrete topic/PR):
  ```
  #!/bin/sh
  for F in $(git ls-files -- "*.py"); do vulture "$F" | grep "unused variable"; done
  ```

ACKs for top commit:
  practicalswift:
    ACK 92e28fa8b2590cce0e8f0adadae80e46cb63a9ef: patch looks correct.

Tree-SHA512: 16516abc8014207bcefdf0545dffaecff1fbba66f45b54c02371dcfd1f18194855c6b72598c11b5407009561eafe8048d47af3471f0efb1795d52477d5a0232e
 2023-07-26 09:37:51 +05:30
Konstantin Akimov
42dcb3ddca
fix!: making MnEhfTx to comply DIP-0023 (#5505) 
## Issue being fixed or feature implemented
Current implementation of MnEhfTx is not matched with DIP-0023, this PR
fixes it. It is a prior work for
https://github.com/dashpay/dash/pull/5469

## What was done?
- requestID is fixed from `clsig{quorumHeight}` to `mnhf{versionBit}` +
fixes for signature validation properly
 - v20 is minimal height to accept MnEHF special transactions
- versionBit is not BLS version - removed unrelated wrong code and
validations
- TxMempool will accept MnEHF transaction even if inputs/outputs are
zeroes and no fee
- implemented python's serialization/deserialization of MnEHF
transactions for future using in functional tests
 

## How Has This Been Tested?
Run functional/unit tests. Beside that there's new functional test in
https://github.com/dashpay/dash/pull/5469 that actually test format of
transaction and signature validation - to be merged later.

## Breaking Changes
Payload of MnEhf tx is changed, related consensus rules are changed.


## 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
 2023-07-25 21:46:55 +03:00
MarcoFalke
8b4982bc0b Merge #20448: RPC/Wallet: unloadwallet: Allow specifying wallet_name param matching RPC endpoint wallet 
89bdad5b25ae4ac03a486f729a5b58ae6f21946d RPC/Wallet: unloadwallet: Allow specifying wallet_name param matching RPC endpoint (Luke Dashjr)

Pull request description:

  Allow specifying the `wallet_name` param to `unloadwallet` on RPC wallet endpoints, so long as it matches the endpoint wallet.

ACKs for top commit:
  jonatack:
    ACK 89bdad5b25ae4ac03a486f729a5b58ae6f21946d
  MarcoFalke:
    review ACK 89bdad5b25ae4ac03a486f729a5b58ae6f21946d

Tree-SHA512: efb399c33f7b5596870a26a8680f453ca47aa7a6db4e550f9435d13044f1c4bad0ae11e8f0205213409d08b75c4188c3be782e54aafab1f65b97eb8cf5c252a9
 2023-07-25 10:45:09 -05:00
Wladimir J. van der Laan
5e20e69415 Merge #19851: refactor: Extract ParseOpCode from ParseScript 
c92387232f750397da7d131f262c150a608408c2 refactor: Extract ParseOpCode from ParseScript (João Barbosa)

Pull request description:

  Seems more natural to have `mapOpNames` "hidden" in `ParseOpCode` than in `ParseScript`.

  A second lookup in `mapOpNames` is also removed.

ACKs for top commit:
  laanwj:
    ACK c92387232f750397da7d131f262c150a608408c2
  theStack:
    re-ACK c92387232f750397da7d131f262c150a608408c2

Tree-SHA512: d59d1964760622cf365479d44e3e676aa0bf46b60e77160140d967e012042df92121d3224c7551dc96eff5ff3294598cc6bade82adb3f60d28810e18e60e1257
 2023-07-25 10:45:09 -05:00
UdjinM6
48a1632688
fix: implement missing logic for additional indexes, fix bugs and logging (#5477) 
## Issue being fixed or feature implemented
The logic for additional indexes is incomplete, handling of P2PK on
block disconnect is broken (luckily no one is using P2PK and reorgs are
rare) and there are a few other small issues that would be nice to have
fixed.

## What was done?
Pls see individual commits

## How Has This Been Tested?
Run `feature_dbcrash.py`, it should succeed (NOTE: it takes ~30 minutes
to complete, that's normal).
Run `feature_addressindex.py`, `feature_timestampindex.py` and
`feature_spentindex.py` (and other tests) should still succeed too.

## 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
- [x] 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-07-24 20:54:24 +03:00
Kittywhiskers Van Gogh
5088da93db merge bitcoin#22112: Force port 0 in I2P 2023-07-24 20:45:49 +03:00
Kittywhiskers Van Gogh
bf31070808 merge bitcoin#20685: Add I2P support using I2P SAM 2023-07-24 20:45:49 +03:00
MarcoFalke
e9311d8098 Merge #18712: test: display command line options passed to send_cli() in debug log 
8f5dc8800aeb524eee2fa2451cd22883b7b2bfec test: display command line options passed to send_cli() in debug log (Jon Atack)

Pull request description:

  as per https://github.com/bitcoin/bitcoin/pull/18691#discussion_r411382589, and revert two cli calls changed in #18691 from rpc commands back to command line options (these were the only occurrences).

ACKs for top commit:
  MarcoFalke:
    ACK 8f5dc8800aeb524eee2fa2451cd22883b7b2bfec

Tree-SHA512: fcb3eca00aa4099066028c90d5e50a02e074366e09a17f5f5b937d9f7562dd054ff65681aa0ad4c94f6de1e98b1e2b9ac4cd084ddc297010253989a80483b1b9
 2023-07-24 11:42:34 -05:00
Konstantin Akimov
8a0e681cea
feat!: add an implementation of DIP 0027 Credit Asset Locks (#5026) 
## Issue being fixed or feature implemented
This is an implementation of DIP0027 "Credit Asset Locks".
It's a mechanism to fluidly exchange between Dash and credits.

## What was done?
This pull request includes:
      - Asset Lock transaction
      - Asset Unlock transaction (withdrawal)
      - Credit Pool in coinbase
      - Unit tests for Asset Lock/Unlock tx
      - New functional test `feature_asset_locks.py`

RPC: currently locked amount (credit pool) is available through rpc call
`getblock`.

## How Has This Been Tested?
There added new unit tests for basic checks of transaction validity
(asset lock/unlock).
Also added new functional test "feature_asset_locks.py" that cover
typical cases, but not all corner cases yet.

## Breaking Changes
This feature should be activated as hard-fork because:
- It adds 2 new special transaction and one of them [asset unlock tx]
requires update consensus rulels
 - It adds new data in coinbase tx (credit pool)

## 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
- [ ] I have made corresponding changes to the documentation
**To release DIP 0027**
- [x] I have assigned this pull request to a milestone

---------

Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
 2023-07-24 11:39:38 -05:00
UdjinM6
5382d05b7e
feat: bury v19 activation (#5496) 
## Issue being fixed or feature implemented
V19 is active on mainnet/testnet now, no need to check activation bits
anymore. This PR also bumps `MinBIP9WarningHeight` to
post-v19-activation height which should stop `unknown new rules
activated (versionbit 8)` warning from appearing.

## What was done?
Bury v19, bump `MinBIP9WarningHeight`

## How Has This Been Tested?
Run tests, reindex on mainnet/testnet.

## 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-07-23 15:19:38 -05:00
MarcoFalke
945aca8b01 Merge #20039: test: Convert amounts from float to decimal 
5aadd4be1883386a04bef6a04e9a1142601ef7a7 Convert amounts from float to decimal (Prayank)

Pull request description:

  > decimal is preferred in accounting applications

  https://docs.python.org/3.8/library/decimal.html

  Decimal type saves an exact value so better than using float.

  ~~3 variables declared with type as 'Decimal' in [test/functional/mempool_accept.py](https://github.com/bitcoin/bitcoin/blob/master/test/functional/mempool_accept.py): fee, fee_expected, output_amount~~

  ~~Not required to convert to string anymore for using the above variables as decimal~~

  + fee, fee_expected, output_amount
  ~~+ 8 decimal places~~
  + Using value of coin['amount'] as decimal and removed 'int'
  + Removed unnecessary parentheses
  + Remove str() and use quotes

  Fixes https://github.com/bitcoin/bitcoin/issues/20011

ACKs for top commit:
  guggero:
    ACK 5aadd4be1883386a04bef6a04e9a1142601ef7a7

Tree-SHA512: 5877cf3837e5b65bec0fc8909de141a720bfa02a747513e21d20f3c41ec0cfecc524d2c347a96596b0a1a97900da2acf08b799f26b11d537e4dcddc6ce45f38e
 2023-07-21 16:03:00 -05:00
MarcoFalke
4e7cb26160 Merge #18585: test: use zero-argument super() shortcut (Python 3.0+) 
0956e46bff7f0b6da65a4de6d4f8261fe9d7055c test: use zero-argument super() shortcut (Python 3.0+) (Sebastian Falbesoner)

Pull request description:

  This mini-PR replaces all calls to `super(...)` with arguments with the zero-argument shortcut `super()` where applicable. See [PEP 3135](https://www.python.org/dev/peps/pep-3135/#specification):

  > The new syntax:
  >
  >     super()
  >
  > is equivalent to:
  >
  >     super(__class__, <firstarg>)
  >
  > where __class__ is the class that the method was defined in, and <firstarg> is
  > the first parameter of the method (normally self for instance methods, and cls
  > for class methods).

ACKs for top commit:
  fanquake:
    ACK 0956e46bff7f0b6da65a4de6d4f8261fe9d7055c

Tree-SHA512: 4ac66fe7ab2be2e8a514e5fcfc41dbb298f21b23ebb7b7b0310d704b0b3cef8adf287a8d80346d1ea9418998c597b4f0ff1f66148d0d806bb43db6607e0fe1cf
 2023-07-21 16:03:00 -05:00
Samuel Dobson
f83b4bfdb3 Merge #17824: wallet: Prefer full destination groups in coin selection 
a2324e4d3f47f084b07a364c9a360a0bf31e86a0 test: Improve naming and logging of avoid_reuse tests (Fabian Jahr)
1abbdac6777bc5396d17a6772c8176a354730997 wallet: Prefer full destination groups in coin selection (Fabian Jahr)

Pull request description:

  Fixes #17603 (together with #17843)

  In the case of destination groups of >10 outputs existing in a wallet with `avoid_reuse` enabled, the grouping algorithm is adding left-over outputs as an "incomplete" group to the list of groups even when a full group has already been added. This leads to the strange behavior that if there are >10 outputs for a destination the transaction spending from that will effectively use `len(outputs) % 10` as inputs for that transaction.

  From the original PR and the code comment I understand the correct behavior should be the usage of 10 outputs. I opted for minimal changes in the current code although there maybe optimizations possible for cases with >20 outputs on a destination this sounds like too much of an edge case right now.

ACKs for top commit:
  jonatack:
    Re-ACK a2324e4
  achow101:
    ACK a2324e4d3f47f084b07a364c9a360a0bf31e86a0
  kallewoof:
    ACK a2324e4d3f47f084b07a364c9a360a0bf31e86a0
  meshcollider:
    Tested ACK a2324e4d3f47f084b07a364c9a360a0bf31e86a0 (verified the new test fails on master without this change)

Tree-SHA512: 4743779c5d469fcd16df5baf166024b1d3c8eaca151df1e8281b71df62b29541cf7bfee3f8ab48d83e3b34c9256e53fd38a7b146a54c79f9caa44cce3636971a
 2023-07-21 16:03:00 -05:00
Wladimir J. van der Laan
2c5cb249be Merge #11413: [wallet] [rpc] sendtoaddress/sendmany: Add explicit feerate option 
25dac9fa65243ca8db02df22f484039c08114401 doc: add release notes for explicit fee estimators and bumpfee change (Karl-Johan Alm)
05227a35545d7656450874b3668bf418c73813fb tests for bumpfee / estimate_modes (Karl-Johan Alm)
3404c1b753432c4859a4ca245f01c240610a00cb policy: optional FeeEstimateMode param to CFeeRate::ToString (Karl-Johan Alm)
6fcf4484302d13bd7739b617470d8c8e31974908 rpc/wallet: add two explicit modes to estimate_mode (Karl-Johan Alm)
b188d80c2de9ebb114da5ceea78baa46bde7dff6 MOVEONLY: Make FeeEstimateMode available to CFeeRate (Karl-Johan Alm)
5d1a411eb12fc700804ffe5d6e205234d30edd5f fees: add FeeModes doc helper function (Karl-Johan Alm)
91f6d2bc8ff4d4cd1b86daa370ec9d2d9662394d rpc/wallet: add conf_target as alias to confTarget in bumpfee (Karl-Johan Alm)
69158b41fc488e4f220559da17a475eff5923a95 added CURRENCY_ATOM to express minimum indivisible unit (Karl-Johan Alm)

Pull request description:

  This lets users pick their own fees when using `sendtoaddress`/`sendmany` if they prefer this over the estimators.

ACKs for top commit:
  Sjors:
    re-utACK 25dac9fa65: rebased, more fancy C++,
  jonatack:
    ACK 25dac9fa65243ca8db02df2 I think this should be merged after all this time, even though it looks to me like there are needed follow-ups, fixes and test coverage to be added (see further down), which I don't mind helping out with, if wanted.
  fjahr:
    Code review ACK 25dac9fa65243ca8db02df22f484039c08114401

Tree-SHA512: f31177e6cabf3187a43cdfe93477144f8e8385c7344613743cbbd16e8490d53ff5144aec7b9de6c9a65eb855b55e0f99d7f164dee4b6bf3cfea4dce51cf11d33
 2023-07-21 16:03:00 -05:00
UdjinM6
639d7d254a refactor: Replace encode(X, 'hex_codec').decode('ascii') with X.hex() 2023-07-17 01:00:48 +03:00
UdjinM6
5a48c8b322 refactor: "regtest" -> self.chain 2023-07-17 01:00:48 +03:00
UdjinM6
4fcc1ab4ce chore: post-merge-conflict cleanup in test/functional/rpc_blockchain.py 2023-07-17 01:00:48 +03:00
Odysseas Gabrielides
494b5c744c
feat: mnlistdiff v20 CL sig quorums (#5377) 
## Issue being fixed or feature implemented
Implementation of Randomness Beacon Part 3.

Starting from v20 activation fork, members for quorums are sorted using
(if available) the best CL signature found in Coinbase.
If no CL signature is present yet, then the usual way is used (By using
Blockhash instead)

The actual new way to shuffle is already implemented in
https://github.com/dashpay/dash/pull/5366.

SPV clients also need to calculate members, but they only know block
headers.
Since Coinbase is in the actual block, then they lack the required
information to correctly calculate quorum members.

## What was done?
- Message `MNLISTIDFF` is enriched with a new field `quorumsCLSigs`.
This field holds the Chainlock Signature required for each set of
indexes corresponding to quorums in field `newQuorums`.
-  Protocol version has been bumped to `70230`.
- Clients with protocol version greater or equal to `70230` will receive
the new field `quorumsCLSigs`.
- The same field is returned in `protx diff` RPC.

Note:
- Field `quorumsCLSigs` will populated only after v20 activation
- If for one or more quorums, no non-null CL sig was found in CbTx then
a null signature is returned in `quorumsCLSigs`.

## How Has This Been Tested?
- Functional test mininode's protocol version was bumped to `70230`.
- `feature_llmq_rotation.py` checks that `quorumsCLSigs` match in both
P2P and RPC messages.

## Breaking Changes
No

## 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 _(for repository
code-owners and collaborators only)_

---------

Co-authored-by: thephez <thephez@users.noreply.github.com>
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
Co-authored-by: pasta <pasta@dashboost.org>
 2023-07-10 11:23:09 -05:00
Konstantin Akimov
32a2543faf
refactor: trivial refactorings of llmq/ (#5486) 
## Issue being fixed or feature implemented
It splits from https://github.com/dashpay/dash/pull/5150/ by
@PastaPastaPasta request.


## What was done?
See commits

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


## 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
 2023-07-10 10:13:42 -05:00
MarcoFalke
91c6a8ed42
Merge #20462: RPC/Wallet: unloadwallet: Clarify docs/error when both the RPC request and wallet_name parameter specify a wallet 
b1f59d55d920d2b35269b474762f94fec87bfb16 RPC/Wallet: unloadwallet: Clarify docs/error when both the RPC endpoint and wallet_name parameter specify a wallet (Luke Dashjr)

Pull request description:

  Just documentation clarifications from #20448

ACKs for top commit:
  MarcoFalke:
    review ACK b1f59d55d920d2b35269b474762f94fec87bfb16
  jonatack:
    re-ACK b1f59d55d920d2b35269b474762f94fec87bfb16  per `git diff e8303a0 b1f59d5`

Tree-SHA512: ac068b0aa7ceed49496367fdd9425b59dbba18b56e89b26afc22a6c8ece51f0b92a169cacd55740b1cadab2b32f4f8e8700e609066ab7e59d3b53c7891da585e
 2023-07-09 17:52:51 +05:30
UdjinM6
d97ec350f0
feat(wallet): make mnemonic bits tweakable, default to 128 bit / 12 words (#5457) 
## Issue being fixed or feature implemented
Allow generating 12, 18, 24 words mnemonics. Default to 12 words as it's
the most popular option/de-facto a standard now imo.

## What was done?
Add `-mnemonicbits` option, add tests

## How Has This Been Tested?
run tests, play with wallets on regtest

## Breaking Changes
n/a, old wallets should not be affected

## 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-28 19:01:24 +03:00
UdjinM6
78fa019952
feat: introduce wipewallettxes RPC and wipetxes command for dash-wallet tool (#5451) 
## Issue being fixed or feature implemented
Given the hard fork that happened on testnet, there is now lots of the
transactions that were made on the fork that is no longer valid. Some
transactions could be relayed and mined again but some like coinjoin
mixing won't be relayed because of 0 fee and transactions spending
coinbases from the forked branch are no longer valid at all.

## What was done?
Introduce `wipewallettxes` RPC and `wipetxes` command for `dash-wallet`
tool to be able to get rid of some/all txes in the wallet.

## How Has This Been Tested?
run tests, use rpc/command on testnet wallet

## 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-27 21:51:40 +03:00
Odysseas Gabrielides
0e53540f64
feat: mnlistdiff move nversion to first position (#5450) 
## Issue being fixed or feature implemented

Version field should always be the first field of a message for better
readibility.

## What was done?

- Introduced new protocol version `MNLISTDIFF_VERSION_ORDER` (`70229`).
- `nVersion` serialisation order is changed for clients with protocol
version greater than or equal to `70229`.
- For clients with protocol version >= `70225` and < `70229` the old
order is used: can be deprecated in the future.
- Increased functional test P2P mininode's protocol version to `70229`.

## How Has This Been Tested?
`feature_llmq_rotation.py` with new protocol version.

## Breaking Changes

## Checklist:
- [x] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [x] 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-26 00:01:17 -05:00
UdjinM6
55008b0b01
fix: do not check chainlock state in IsTxSafeForMining (#5444) 
## Issue being fixed or feature implemented
Disabled or non-enforced Chainlocks does not mean you can safely mine
non-locked txes, you could end up mining a block that is going to be
rejected by everyone else if a conflicting tx (missing on your node)
would be IS-locked. I can't find any reason why we have this besides "if
Chainlocks are disabled then smth is wrong so let them all be mined" but
we have spork_2 and spork_3 to control IS behaviour and we check them in
`IsTxSafeForMining` already, that would be a much more straightforward
way to deal with a potential issue.

Noticed this while reviewing #5150 and also while testing v19.2 during
recent testnet v19 re-fork.

## What was done?
Drop this check, adjust tests

## How Has This Been Tested?
Run tests locally

## Breaking Changes
Not quote breaking changes but a change in behaviour: with CLs disabled
it will now take 10 minutes for non-locked txes to be mined, same as
when CLs are enabled.

## 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-20 22:49:41 -05:00
UdjinM6
a760e33236
feat: store protx version in CSimplifiedMNListEntry and use it to ser/deser pubKeyOperator (#5397) 
## Issue being fixed or feature implemented
Mobile wallets would have to convert 4k+ pubkeys at the V19 fork point
and it's a pretty hard job for them that can easily take 10-15 seconds
if not more. Also after the HF, if a masternode list is requested from
before the HF, the operator keys come in basic scheme, but the
merkelroot was calculated with legacy. From mobile team work it wasn't
possible to convert all operator keys to legacy and then calculate the
correct merkleroot.

~This PR builds on top of ~#5392~ #5403 (changes that belong to this PR:
26f7e966500bdea4c604f1d16716b40b366fc707 and
4b42dc8fcee3354afd82ce7e3a72ebe1659f5f22) and aims to solve both of
these issues.~

cc @hashengineering @QuantumExplorer 

## What was done?
Introduce `nVersion` on p2p level for every CSimplifiedMNListEntry. Set
`nVersion` to the same value we have it in CDeterministicMNState i.e.
pubkey serialization would not be via basic scheme only after the V19
fork, it would match the way it’s serialized on-chain/in
CDeterministicMNState for that specific MN.

## How Has This Been Tested?
run tests

## Breaking Changes
NOTE: `testnet` is going to re-fork at v19 forkpoint because
`merkleRootMNList` is not going to match

## 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
- [ ] I have assigned this pull request to a milestone _(for repository
code-owners and collaborators only)_
 2023-06-11 12:29:00 -05:00
MarcoFalke
ddcaadd7a8 Merge bitcoin/bitcoin#22249: test: kill process group to avoid dangling processes when using --failfast 
451b96f7d2796d00eabaec56d831f9e9b1a569cc test: kill process group to avoid dangling processes (S3RK)

Pull request description:

  This is an alternative to #19281

  This PR fixes a problem when after test failure with `--failfast` option there could be dangling nodes. The nodes will continue to occupy rpc/p2p ports on the machine and will cause further test failures.

  If there are any dangling nodes left at the end of the test run we kill the whole process group.
  Pros: the operations is immediate and won't lead to CI timeout
  Cons: the test_runner process is also killed and exit code is 137

  Example output:
  ```
  ...
  Early exiting after test failure

  TEST                           | STATUS    | DURATION

  rpc_decodescript.py            | ✓ Passed  | 2 s
  rpc_deprecated.py              | ✓ Passed  | 2 s
  rpc_deriveaddresses.py         | ✓ Passed  | 2 s
  rpc_dumptxoutset.py            | ✖ Failed  | 2 s

  ALL                            | ✖ Failed  | 8 s (accumulated)
  Runtime: 4 s

  Killed: 9
  > echo $?
  137
  ```

ACKs for top commit:
  MarcoFalke:
    review ACK 451b96f7d2796d00eabaec56d831f9e9b1a569cc
  aitorjs:
    ACK 451b96f7d2796d00eabaec56d831f9e9b1a569cc. Manual testing with and without **--failfast**.

Tree-SHA512: 87e510a1411b9e7571e63cf7ffc8b9a8935daf9112ffc0f069d6c406ba87743ec439808181f7e13cb97bb200fad528589786c47f0b43cf3a2ef0d06a23cb86dd
 2023-06-10 17:40:23 -05:00
Konstantin Akimov
028281ae15 fix: add supression for integer conversion in crc32 library 2023-06-07 01:50:18 -05:00
MarcoFalke
32b4f8dd65 Merge #18392: ci: Run feature_block in valgrind 
fa92af5af39a08982f785542df5419d6d5a4706d ci: Run feature_block and feature_abortnode in valgrind (MarcoFalke)
fa01febeaf801bade77a613e64f18b556ae16d86 test: Remove ci timeout restriction in test_runner (MarcoFalke)

Pull request description:

  Also revert commit 0a4912e46a, because some tests take too long for this to be useful anymore.

Top commit has no ACKs.

Tree-SHA512: 363f14766e1f4a5860ab668a516b41acebc6fbdf11d8defb3a95a772dbf82304ca1f5f14b1dbad97f2029503e03d92e8c69df0466a8872409c20665838f617ed
 2023-06-07 01:50:18 -05:00
MarcoFalke
c2554ab891 Merge #18304: ci: Enable all functional tests in valgrind 
4444edc2e6671d3f73de3725447130f73ecf0375 ci: Enable all functional tests in valgrind (MarcoFalke)

Pull request description:

  The travis timeout for our repo has been bumped to 2h, so we can run all tests in valgrind now

ACKs for top commit:
  practicalswift:
    ACK 4444edc2e6671d3f73de3725447130f73ecf0375 -- regarding the three disabled cases (`feature_abortnode`, `feature_block` and `rpc_bind`): not a big deal since MSan will take care of those once #18288 is merged. More is more :)

Tree-SHA512: ea2f798112911b6d1f3d88cfcdf0a7cdb698687248343703d6fe55da144542c961c15d888bffb41672c10aa76765615cb7c7ff93d468bfad3c51f962f24e7abb
 2023-06-07 01:50:18 -05:00
Kittywhiskers Van Gogh
6bf39d7632 merge bitcoin#19806: UTXO snapshot activation 2023-06-06 22:38:56 +05:30
Kittywhiskers Van Gogh
3bfcf7a16b merge bitcoin#20965: return NET_UNROUTABLE as not_publicly_routable, automate helps 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
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
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
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
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
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
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
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
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
fanquake
807720666c Merge bitcoin/bitcoin#22002: Fix crash when parsing command line with -noincludeconf=0 
fad0867d6ab9430070aa7d60bf7617a6508e0586 Cleanup -includeconf error message (MarcoFalke)
fa9f711c3746ca3962f15224285a453744cd45b3 Fix crash when parsing command line with -noincludeconf=0 (MarcoFalke)

Pull request description:

  The error message has several issues:

  * It may crash instead of cleanly shutting down, when `-noincludeconf=0` is passed
  * It doesn't quote the value
  * It includes an erroneous trailing `\n`
  * It is redundantly mentioning `"-includeconf cannot be used from commandline;"` several times, when once should be more than sufficient

  Fix all issues by:
  * Replacing `get_str()` with `write()` to fix the crash and quoting issue
  * Remove the `\n` and only print the first value to fix the other issues

  Before:

  ```
  $ ./src/bitcoind -noincludeconf=0
  terminate called after throwing an instance of 'std::runtime_error'
    what():  JSON value is not a string as expected
  Aborted (core dumped)

  $ ./src/bitcoind -includeconf='a b' -includeconf=c
  Error: Error parsing command line arguments: -includeconf cannot be used from commandline; -includeconf=a b
  -includeconf cannot be used from commandline; -includeconf=c
  ```

  After:

  ```
  $ ./src/bitcoind -noincludeconf=0
  Error: Error parsing command line arguments: -includeconf cannot be used from commandline; -includeconf=true

  $ ./src/bitcoind -includeconf='a b' -includeconf=c
  Error: Error parsing command line arguments: -includeconf cannot be used from commandline; -includeconf="a b"
  ```

  Hopefully fixes https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=34493

  Testcase: https://github.com/bitcoin/bitcoin/files/6515429/clusterfuzz-testcase-minimized-system-6328535926046720.log

  ```
  FUZZ=system ./src/test/fuzz/fuzz ./clusterfuzz-testcase-minimized-system-6328535926046720.log
  ```

  See https://github.com/bitcoin/bitcoin/blob/master/doc/fuzzing.md

ACKs for top commit:
  sipa:
    utACK fad0867d6ab9430070aa7d60bf7617a6508e0586

Tree-SHA512: b44af93be6bf71b43669058c1449c4c6999f03b5b01b429851b149b12d77733408cb207e9a3edc6f0bffd6030c4c52165e8e23a1c2718ff5082a6ba254cc94a4
 2023-05-31 10:52:02 -05:00
MarcoFalke
0903c3bb5b Merge #20857: test: update documentation in feature_csv_activation.py 
9053b88b1c15f57cdcff2fc1c761efebb2ebfefe update docstring in feature_csv_activation.py (Pierre K)

Pull request description:

  These changes in the test documentation reflect the changes introduced in #17921.

ACKs for top commit:
  MarcoFalke:
    review ACK  9053b88

Tree-SHA512: 17fb954baded8dab1c869dd48b76b516150bae616c792c573e4114d4adfdd40195745c56570aa3050cc0015ee496acd7ec178df8ba14831dd22f9722fda84da2
 2023-05-31 10:52:02 -05:00
Wladimir J. van der Laan
6b0d660e74 Merge #20120: net, rpc, test, bugfix: update GetNetworkName, GetNetworksInfo, regression tests 
7b5bd3102e06f7ff34b5d0f1d45a005560f265a5 test: add getnetworkinfo network name regression tests (Jon Atack)
9a75e1e5697476058b56cd8014a36de31bfecd4c rpc: update GetNetworksInfo() to not return unsupported networks (Jon Atack)
ba8997fb2eda73603ce457bfec668cb7e0acbc89 net: update GetNetworkName() with all enum Network cases (Jon Atack)

Pull request description:

  Following up on the BIP155 addrv2 changes, and starting with 7be6ff6 in #19845, RPC getnetworkinfo began returning networks with empty names.

  <details><summary><code>getnetworkinfo</code> on current master</summary><p>

  ```
    "networks": [
      {
        "name": "ipv4",
        "limited": false,
        "reachable": true,
        "proxy": "",
        "proxy_randomize_credentials": false
      },
      {
        "name": "ipv6",
        "limited": false,
        "reachable": true,
        "proxy": "",
        "proxy_randomize_credentials": false
      },
      {
        "name": "onion",
        "limited": false,
        "reachable": true,
        "proxy": "127.0.0.1:9050",
        "proxy_randomize_credentials": true
      },
      {
        "name": "",
        "limited": false,
        "reachable": true,
        "proxy": "",
        "proxy_randomize_credentials": false
      },
      {
        "name": "",
        "limited": false,
        "reachable": true,
        "proxy": "",
        "proxy_randomize_credentials": false
      }
    ],
  ```
  </p></details>

  <details><summary><code>getnetworkinfo</code> on this branch</summary><p>

  ```
    "networks": [
      {
        "name": "ipv4",
        "limited": false,
        "reachable": true,
        "proxy": "",
        "proxy_randomize_credentials": false
      },
      {
        "name": "ipv6",
        "limited": false,
        "reachable": true,
        "proxy": "",
        "proxy_randomize_credentials": false
      },
      {
        "name": "onion",
        "limited": false,
        "reachable": true,
        "proxy": "127.0.0.1:9050",
        "proxy_randomize_credentials": true
      }
    ],
  ```
  </p></details>

  This patch:
  - updates `GetNetworkName()` to the current Network enum
  - updates `getNetworksInfo()` to ignore as-yet unsupported networks
  - adds regression tests

ACKs for top commit:
  hebasto:
    re-ACK 7b5bd3102e06f7ff34b5d0f1d45a005560f265a5
  vasild:
    ACK 7b5bd3102

Tree-SHA512: 8f12363eb430e6f45e59e3b1d69c2f2eb5ead254ce7a67547d116c3b70138d763157335a3c85b51f684a3b3b502c6aace4f6fa60ac3b988cf7a9475a7423c4d7
 2023-05-31 10:52:02 -05:00
Wladimir J. van der Laan
c1a42cfdb9 Merge #16688: log: Add validation interface logging 
f9abf4ab6d3d3e4d4b7e90723020b5422a141a6f Add logging for CValidationInterface events (Jeffrey Czyz)
6edebacb2191373e76d79a4972d6192300976096 Refactor FormatStateMessage for clarity (Jeffrey Czyz)
72f3227c83810936e7a334304e5fd7c6dab8e91b Format CValidationState properly in all cases (Jeffrey Czyz)
428ac70095253225f64462ee15c595644747f376 Add VALIDATION to BCLog::LogFlags (Jeffrey Czyz)

Pull request description:

  Add logging of `CValidationInterface` callbacks using a new `VALIDATIONINTERFACE` log flag (see #12994). A separate flag is desirable as the logging can be noisy and thus may need to be disabled without affecting other logging.

  This could help debug issues where there may be race conditions at play, such as #12978.

ACKs for top commit:
  jnewbery:
    ACK f9abf4ab6d3d3e4d4b7e90723020b5422a141a6f
  hebasto:
    ACK f9abf4ab6d3d3e4d4b7e90723020b5422a141a6f
  ariard:
    ACK f9abf4a, only changes since 0cadb12 are replacing log indication `VALIDATIONINTERFACE` by `VALIDATION` and avoiding a forward declaration with a new include
  ryanofsky:
    Code review ACK f9abf4ab6d3d3e4d4b7e90723020b5422a141a6f. Just suggested changes since last review (thanks!)

Tree-SHA512: 3e0f6e2c8951cf46fbad3ff440971d95d526df2a52a2e4d6452a82785c63d53accfdabae66b0b30e2fe0b00737f8d5cb717edbad1460b63acb11a72c8f5d4236
 2023-05-24 12:43:57 -05:00
Konstantin Akimov
b8b37f314b Merge #17891: scripted-diff: Replace CCriticalSection with RecursiveMutex 
e09c701e0110350f78366fb837308c086b6503c0 scripted-diff: Bump copyright of files changed in 2020 (MarcoFalke)
6cbe6209646db8914b87bf6edbc18c6031a16f1e scripted-diff: Replace CCriticalSection with RecursiveMutex (MarcoFalke)

Pull request description:

  `RecursiveMutex` better clarifies that the mutex is recursive, see also the standard library naming: https://en.cppreference.com/w/cpp/thread/recursive_mutex

  For that reason, and to avoid different people asking me the same question repeatedly (e.g. https://github.com/bitcoin/bitcoin/pull/15932#pullrequestreview-339175124 ), remove the outdated alias `CCriticalSection` with a scripted-diff
 2023-05-24 12:43:57 -05:00
UdjinM6
3d77c539d2
test: Various test improvements (#5382) 
## Issue being fixed or feature implemented
Speed thing up: 8075fc0c61
Unify things: ff1a390224 (and _probably_
fix issues like https://gitlab.com/dashpay/dash/-/jobs/4304343867),
876f5c3a9f,
ed58cdda13
Let tsan tests finish on smaller/slower machines:
ba1e3360f9

## What was done?
pls see individual commits


## How Has This Been Tested?
run tests locally and my in gitlab ci
https://gitlab.com/UdjinM6/dash/-/jobs/4319419014

## 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
- [x] 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-24 12:38:33 -05:00
Konstantin Akimov
e3a97b0156
feat: bury dash deployments: dip0003, dip0020, dip0024, brr, bip147 (#5356) 
## Issue being fixed or feature implemented
This changes are follow up for backport bitcoin/bitcoin#16060 

## What was done?
Buried all hardened dash deployments

## How Has This Been Tested?
Run unit/functional tests.
Run dash with option `-reindex` for both mainnet/testnet - both succeed.

## Breaking Changes
No breaking changes, it should be fully compatible.

## 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-05-18 12:15:08 -05:00
UdjinM6
3edc876f99
refactor: move BuildSimplifiedDiff to src/evo/simplifiedmns.cpp to resolve 3 circular dependencies (#5380) 
## Issue being fixed or feature implemented
Move `BuildSimplifiedDiff` to the place it's actually used. This also
resolves 3 circular dependencies we have atm.

## What was done?
mostly trivial move-only changes

## How Has This Been Tested?
it compiles and linter is happy locally

## 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-05-18 12:03:19 -05:00
Odysseas Gabrielides
9eee9ee680
feat!: calculate quorum members using v20 cbtx clsig (#5366) 
## Issue being fixed or feature implemented

Implementation of Randomness Beacon Part 2.
This PR is the next step of #5262.

Starting from v20 activation fork, members for quorums are sorted using
(if available) the best CL signature found in Coinbase.
If no CL signature is present yet, then the usual way is used (By using
Blockhash instead)

## What was done?

## How Has This Been Tested?
Test `feature_llmq_rotation.py` was updated to cover both rotated and
non-rotated quorums.
2 quorums are mined first to ensure Chainlock are working earlier.
Then dip_24 activation is replaced by v20 activation.

The only direct way to test this change is to make sure that all
expected quorums after v20 activation are properly formed.

Note: A `wait_for_chainlocked_block_all_nodes` is called between every
rotation cycle to ensure that Coinbase will use a different Chainlock
signature.

## Breaking Changes
Yes, quorum members will be calculated differently.

## 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
- [ ] 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-17 20:27:15 +03:00
PastaPastaPasta
04a31c76e0
chore: harden dip 20 and 24 activation (#5344) 
## Issue being fixed or feature implemented
We had forgotten to harden dip20 and dip24 activation

## What was done?
Hardened dip20 and dip24 activation

## How Has This Been Tested?
Hasn't yet; should do an assumevalid=0 reindex

## Breaking Changes
Hopefully 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
- [ ] I have assigned this pull request to a milestone _(for repository
code-owners and collaborators only)_

---------

Co-authored-by: Konstantin Akimov <knstqq@gmail.com>
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
 2023-05-17 14:11:33 +03:00
thephez
490f32c8d1
chore: update dash_hash install and instructions (#5291) 
## Issue being fixed or feature implemented
Install of dash_hash will change once
https://github.com/dashpay/dash_hash/pull/17/ is merged

## What was done?
- Changed install instructions to match new install in dash_hash README
- Updated Dockerfile to install correctly

## How Has This Been Tested?
N/A

## 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
- [x] I have made corresponding changes to the documentation
 2023-05-11 20:19:45 -05:00
Kittywhiskers Van Gogh
1bfb3328e6 merge bitcoin#19504: Bump minimum python version to 3.6 2023-05-11 09:18:48 -05:00
UdjinM6
e78ce77ee6
test: Fix activate_by_name (#5367) 
## Issue being fixed or feature implemented
I think the logic in activate_by_name is broken 

## What was done?
fix it

## 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-11 12:25:41 +03:00
UdjinM6
72899530bf
test: Fix feature_pruning.py (#5353) 
## Issue being fixed or feature implemented
`feature_pruning.py` is failing atm

## What was done?
pls see individual commits

## How Has This Been Tested?
run `feature_pruning.py` locally

gitian for this branch with `INTEGRATION_TESTS_ARGS` set to `--extended
--exclude feature_dbcrash --timeoutscale=4 --jobs=4`:
https://gitlab.com/dashpay/dash/-/pipelines/852044104
NOTE:
[`linux64_tsan-test`](https://gitlab.com/dashpay/dash/-/jobs/4197383660)
failed because tsan build binaries are super slow and we hit 30 minutes
timeout for 1 single test because of that. This is not an actual test
failure.

## 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-09 12:13:14 -05:00
Odysseas Gabrielides
b1626f9af0
feat!: Insertion of best CL signature in CbTx (#5262) 
## Issue being fixed or feature implemented


## What was done?
- Bumped version of `CbTx`. Added fields `bestCLHeightDiff`,
`bestCLSignature`
- Miner starting from v20 fork, includes best CL signature in `CbTx` (if
available) or null signature.
- All nodes should verify included CL signature before accepting the
block.

## How Has This Been Tested?
Basically, activated v20 on in the beginning of
`feature_llmq_chainlocks.py`

## Breaking Changes
Yes, new version of CbTx

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

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

---------

Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
 2023-05-08 22:34:26 -05:00
Konstantin Akimov
750f627a90
fix: improved reliability of functional tests that activates dip24 (#5313) 
## Issue being fixed or feature implemented
Functional tests in CI and locally often fails without a good reason
(pretty randomly)

## What was done?
It was re-implemented `get_recovered_sig` and updated `create_raw_tx`
for better selection/change output.

## How Has This Been Tested?
Run functional times with bug amount of parrallel jobs:
```
test/functional/test_runner.py -j 20 feature_llmq_is_cl_conflicts.py feature_llmq_is_cl_conflicts.py feature_llmq_is_cl_conflicts.py feature_llmq_is_cl_conflicts.py feature_llmq_is_cl_conflicts.py feature_llmq_is_cl_conflicts.py feature_llmq_is_cl_conflicts.py feature_llmq_is_cl_conflicts.py feature_llmq_is_cl_conflicts.py feature_llmq_is_cl_conflicts.py  feature_llmq_is_cl_conflicts.py feature_llmq_is_cl_conflicts.py feature_llmq_is_cl_conflicts.py feature_llmq_is_cl_conflicts.py feature_llmq_is_cl_conflicts.py feature_llmq_is_cl_conflicts.py feature_llmq_is_cl_conflicts.py feature_llmq_is_cl_conflicts.py feature_llmq_is_cl_conflicts.py feature_llmq_is_cl_conflicts.py feature_llmq_is_cl_conflicts.py
```
Without these changes usually 2-3 instance fails.
With these changes all failures happened only for `p2p_addrv2_relay.py`
and `mempool_unbroadcast.py`. Beside feature_llmq_is_conflicts.py
improved stability of `interface_zmq_dash.py` also.

## Breaking Changes
No breaking changes

## 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

---------

Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
 2023-05-06 21:36:45 -05:00
Konstantin Akimov
84573364f5 feat: burry DIP0008 deployment to follow-up bitcoin#16060 2023-04-25 23:41:20 -05:00
Konstantin Akimov
d26b07a8f7 feat: burry DIP0001 deployment to follow-up bitcoin#16060 2023-04-25 23:41:20 -05:00
MarcoFalke
6ad9bdf722 Merge #16060: Bury bip9 deployments 
e78aaf41f43d0e2ad78fa6d8dad61032c8ef73d0 [docs] Add release notes for burying bip 9 soft fork deployments (John Newbery)
8319e738f9f118025b332e4fa804d4c31e4113f4 [tests] Add coverage for the content of getblockchaininfo.softforks (James O'Beirne)
0328dcdcfcb56dc8918697716d7686be048ad0b3 [Consensus] Bury segwit deployment (John Newbery)
1c93b9b31c2ab7358f9d55f52dd46340397c906d [Consensus] Bury CSV deployment height (John Newbery)
3862e473f0cb71a762c0306b171b591341d58142 [rpc] Tidy up reporting of buried and ongoing softforks (John Newbery)

Pull request description:

  This hardcodes CSV and segwit activation heights, similar to the BIP 90 buried deployments for BIPs 34, 65 and 66.

  CSV and segwit have been active for over 18 months. Hardcoding the activation height is a code simplification, makes it easier to understand segwit activation status, and reduces technical debt.

  This was originally attempted by jl2012 in #11398 and again by me in #12360.

ACKs for top commit:
  ajtowns:
    ACK e78aaf41f43d0e2ad78fa6d8dad61032c8ef73d0 ; checked diff to previous acked commit, checked tests still work
  ariard:
    ACK e78aaf4, check diff, run the tests again and successfully activated csv/segwit heights on mainnet as expected.
  MarcoFalke:
    ACK e78aaf41f43d0e2ad78fa6d8dad61032c8ef73d0 (still didn't check if the mainnet block heights are correct, but the code looks good now)

Tree-SHA512: 7e951829106e21a81725f7d3e236eddbb59349189740907bb47e33f5dbf95c43753ac1231f47ae7bee85c8c81b2146afcdfdc11deb1503947f23093a9c399912
 2023-04-25 23:41:20 -05:00
MarcoFalke
3e1c8a6a58 Merge #16465: test: Test p2sh-witness and bech32 in wallet_import_rescan 
fa3c6575cac5e3841797980fe60b8368ae579dba lint: Add false positive to python dead code linter (MarcoFalke)
fa25668e1c8982548f1c6f94780709c625811469 test: Test p2sh-witness and bech32 in wallet_import_rescan (MarcoFalke)
fa79af298917d501cee26370fdf9d44d05133d15 test: Replace fragile "rng" with call to random() (MarcoFalke)
fac3dcf7d052586548f2100a0d576618a85741f9 test: Generate one block for each send in wallet_import_rescan (MarcoFalke)

Pull request description:

  This adds test coverage for segwit in the `wallet_import_rescan` test, among other cleanups.

ACKs for top commit:
  jnewbery:
    ACK fa3c6575cac5e3841797980fe60b8368ae579dba

Tree-SHA512: 877741763c62c1bf9d868864a1e3f0699857e8c028e9fcd65c7eeb73600c22cbe97b7b51093737743d9e87bcb991c1fe1086f673e18765aef0fcfe27951402f0
 2023-04-25 23:41:20 -05:00
Wladimir J. van der Laan
2307f97784 Merge #16991: qa: Fix service flag comparison check in rpc_net test (luke-jr) 
9c23ebd6b18fb1058a8d3e8aae9e0595d3a57ad5 qa: Fix service flag comparison check in rpc_net test (Luke Dashjr)

Pull request description:

  Rebase of #16936

ACKs for top commit:
  darosior:
    ACK 9c23ebd6b18fb1058a8d3e8aae9e0595d3a57ad5

Tree-SHA512: 74f287740403da1040ab1e235ef6eba4e304f3ee5d57a3b25d1e2e1f2f982d256528d398a4d6cb24ba393798e680a8f46cd7dae54ed84ab2c747e96288f1f884
 2023-04-25 23:41:20 -05:00
Samuel Dobson
4fcd7850b0 Merge #17578: rpc: simplify getaddressinfo labels, deprecate previous behavior 
8925df86c4df16b1070343fef8e4d238f3cc3bd1 doc: update release notes (Jon Atack)
8bb405bbadf11391ccba7b334b4cfe66dc85b390 test: getaddressinfo labels purpose deprecation test (Jon Atack)
60aba1f2f11529add115d963d05599130288ae28 rpc: simplify getaddressinfo labels, deprecate previous behavior (Jon Atack)
7851f14ccf2bcd1e9b2ad48e5e08881be06d9d21 rpc: incorporate review feedback from PR 17283 (Jon Atack)

Pull request description:

  This PR builds on #17283 (now merged) and is followed by #17585.

  It modifies the value returned by rpc getaddressinfo `labels` to an array of label name strings and deprecates the previous behavior of returning an array of JSON hash structures containing label `name` and address `purpose` key/value pairs.

  before
  ```
    "labels": [
      {
        "name": "DOUBLE SPEND",
        "purpose": "receive"
      }
  ```
  after
  ```
    "labels": [
      "DOUBLE SPEND"
    ]
  ```

  The deprecated behavior can be re-enabled by starting bitcoind with `-deprecatedrpc=labelspurpose`.

  For context, see:
  - https://github.com/bitcoin/bitcoin/pull/17283#issuecomment-554458001
  - http://www.erisian.com.au/bitcoin-core-dev/log-2019-12-13.html#l-425 (lines 425-427)
  - http://www.erisian.com.au/bitcoin-core-dev/log-2019-11-22.html#l-622

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

  Next steps: deprecate the rpc getaddressinfo `label` field (EDIT: done in #17585) and add support for multiple labels per address. This PR will unblock those.

ACKs for top commit:
  jnewbery:
    reACK 8925df8
  promag:
    Code review ACK 8925df86c4df16b1070343fef8e4d238f3cc3bd1.
  meshcollider:
    Code review ACK 8925df86c4df16b1070343fef8e4d238f3cc3bd1

Tree-SHA512: c2b717209996da32b6484de7bb8800e7048410f9ce6afdb3e02a6866bd4a8f2c730f905fca27b10b877b91cf407f546e69e8c4feb9cd934325a6c71c166bd438
 2023-04-25 23:14:25 +03:00
MarcoFalke
ca7ee7c278 Merge #16658: validation: Rename CheckInputs to CheckInputScripts 
3bd8db80d8d335ab63ece4f110b0fadd562e80b7 [validation] fix comments in CheckInputScripts() (John Newbery)
6f6465cefcd599c89c00f7b51f42a4b87a5ffb0b scripted-diff: [validation] Rename CheckInputs to CheckInputScripts (John Newbery)

Pull request description:

  CheckInputs() used to check no double spends, scripts & sigs and amounts. Since
  832e074, the double spend and amount checks
  have been moved to CheckTxInputs(), and CheckInputs() now just validates
  input scripts. Rename the function to CheckInputScripts().

  Also fix incorrect comments.

ACKs for top commit:
  MarcoFalke:
    re-ACK 3bd8db80d8d335ab63ece4f110b0fadd562e80b7, did the rebase myself, checked the scripted diff 👡
  promag:
    ACK 3bd8db80d8d335ab63ece4f110b0fadd562e80b7 :trollface:

Tree-SHA512: 7b3f8597d210492798fb784ee8ea47ea6377519111190161c7cc34a967509013f4337304f52e9bedc97b7710de7b0ff8880e08cd7f867754567f82e7b02c794c
 2023-04-25 23:14:25 +03:00
fanquake
fdbf27b775 Merge bitcoin/bitcoin#24426: test: Fix intermittent Tsan issue 
fa7e1471c0dcd6770a724da4a63d433fc9b4cbc1 test: Fix intermittent Tsan issue (MarcoFalke)

Pull request description:

  Fix https://cirrus-ci.com/task/5176769937408000?logs=ci#L5161

  ```
  WARNING: ThreadSanitizer: data race (pid=22965)
    Write of size 8 at 0x7f74d5e21f50 by main thread:
      #0 std::__1::ios_base::precision(long) /usr/lib/llvm-13/bin/../include/c++/v1/ios:513:18 (test_bitcoin+0x1a8366)
      #1 boost::io::ios_base_all_saver::restore() /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/io/ios_state.hpp:341:17 (test_bitcoin+0x1a8366)
      #2 boost::unit_test::unit_test_log_t::operator<<(boost::unit_test::log::begin const&) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/unit_test_log.ipp:336:55 (test_bitcoin+0x1a8366)
      #3 boost::test_tools::tt_detail::report_assertion(boost::test_tools::assertion_result const&, boost::unit_test::lazy_ostream const&, boost::unit_test::basic_cstring<char const>, unsigned long, boost::test_tools::tt_detail::tool_level, boost::test_tools::tt_detail::check_type, unsigned long, ...) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/test_tools.ipp:359:19 (test_bitcoin+0x1b3b9b)
      #4 txindex_tests::txindex_initial_sync::test_method() src/test/txindex_tests.cpp:31:5 (test_bitcoin+0x78aebc)
      #5 txindex_tests::txindex_initial_sync_invoker() src/test/txindex_tests.cpp:16:1 (test_bitcoin+0x78a384)
      #6 boost::detail::function::void_function_invoker0<void (*)(), void>::invoke(boost::detail::function::function_buffer&) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/function/function_template.hpp:117:11 (test_bitcoin+0x2bf30d)
      #7 boost::function0<void>::operator()() const /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/function/function_template.hpp:763:14 (test_bitcoin+0x224027)
      #8 boost::detail::forward::operator()() /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/execution_monitor.ipp:1368:32 (test_bitcoin+0x224027)
      #9 boost::detail::function::function_obj_invoker0<boost::detail::forward, int>::invoke(boost::detail::function::function_buffer&) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/function/function_template.hpp:137:18 (test_bitcoin+0x224027)
      #10 boost::function0<int>::operator()() const /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/function/function_template.hpp:763:14 (test_bitcoin+0x1ac66c)
      #11 int boost::detail::do_invoke<boost::shared_ptr<boost::detail::translator_holder_base>, boost::function<int ()> >(boost::shared_ptr<boost::detail::translator_holder_base> const&, boost::function<int ()> const&) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/execution_monitor.ipp:290:30 (test_bitcoin+0x1ac66c)
      #12 boost::execution_monitor::catch_signals(boost::function<int ()> const&) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/execution_monitor.ipp:879:16 (test_bitcoin+0x1ac66c)
      #13 boost::execution_monitor::execute(boost::function<int ()> const&) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/execution_monitor.ipp:1277:16 (test_bitcoin+0x1ac980)
      #14 boost::execution_monitor::vexecute(boost::function<void ()> const&) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/execution_monitor.ipp:1377:5 (test_bitcoin+0x1a7f9b)
      #15 boost::unit_test::unit_test_monitor_t::execute_and_translate(boost::function<void ()> const&, unsigned long) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/unit_test_monitor.ipp:49:9 (test_bitcoin+0x1a7f9b)
      #16 boost::unit_test::framework::state::execute_test_tree(unsigned long, unsigned long, boost::unit_test::framework::state::random_generator_helper const*) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/framework.ipp:823:44 (test_bitcoin+0x1e0d5c)
      #17 boost::unit_test::framework::state::execute_test_tree(unsigned long, unsigned long, boost::unit_test::framework::state::random_generator_helper const*) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/framework.ipp:792:58 (test_bitcoin+0x1e14a6)
      #18 boost::unit_test::framework::state::execute_test_tree(unsigned long, unsigned long, boost::unit_test::framework::state::random_generator_helper const*) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/framework.ipp:792:58 (test_bitcoin+0x1e14a6)
      #19 boost::unit_test::framework::run(unsigned long, bool) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/framework.ipp:1696:29 (test_bitcoin+0x1a6bfb)
      #20 boost::unit_test::unit_test_main(boost::unit_test::test_suite* (*)(int, char**), int, char**) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/unit_test_main.ipp:248:9 (test_bitcoin+0x1c4ed6)
      #21 main /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/unit_test_main.ipp:304:12 (test_bitcoin+0x1c5506)
    Previous write of size 8 at 0x7f74d5e21f50 by thread T4:
      [failed to restore the stack]
    Location is global 'std::__1::cout' of size 160 at 0x7f74d5e21f30 (libc++.so.1+0x0000000cdf50)
    Thread T4 'b-txindex' (tid=22989, running) created by main thread at:
      #0 pthread_create <null> (test_bitcoin+0x1184cd)
      #1 std::__1::__libcpp_thread_create(unsigned long*, void* (*)(void*), void*) /usr/lib/llvm-13/bin/../include/c++/v1/__threading_support:514:10 (test_bitcoin+0xa23f1b)
      #2 std::__1:🧵:thread<void (*)(char const*, std::__1::function<void ()>), char const*, BaseIndex::Start(CChainState&)::$_0, void>(void (*&&)(char const*, std::__1::function<void ()>), char const*&&, BaseIndex::Start(CChainState&)::$_0&&) /usr/lib/llvm-13/bin/../include/c++/v1/thread:307:16 (test_bitcoin+0xa23f1b)
      #3 BaseIndex::Start(CChainState&) src/index/base.cpp:363:21 (test_bitcoin+0xa23f1b)
      #4 txindex_tests::txindex_initial_sync::test_method() src/test/txindex_tests.cpp:31:5 (test_bitcoin+0x78adfa)
      #5 txindex_tests::txindex_initial_sync_invoker() src/test/txindex_tests.cpp:16:1 (test_bitcoin+0x78a384)
      #6 boost::detail::function::void_function_invoker0<void (*)(), void>::invoke(boost::detail::function::function_buffer&) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/function/function_template.hpp:117:11 (test_bitcoin+0x2bf30d)
      #7 boost::function0<void>::operator()() const /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/function/function_template.hpp:763:14 (test_bitcoin+0x224027)
      #8 boost::detail::forward::operator()() /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/execution_monitor.ipp:1368:32 (test_bitcoin+0x224027)
      #9 boost::detail::function::function_obj_invoker0<boost::detail::forward, int>::invoke(boost::detail::function::function_buffer&) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/function/function_template.hpp:137:18 (test_bitcoin+0x224027)
      #10 boost::function0<int>::operator()() const /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/function/function_template.hpp:763:14 (test_bitcoin+0x1ac66c)
      #11 int boost::detail::do_invoke<boost::shared_ptr<boost::detail::translator_holder_base>, boost::function<int ()> >(boost::shared_ptr<boost::detail::translator_holder_base> const&, boost::function<int ()> const&) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/execution_monitor.ipp:290:30 (test_bitcoin+0x1ac66c)
      #12 boost::execution_monitor::catch_signals(boost::function<int ()> const&) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/execution_monitor.ipp:879:16 (test_bitcoin+0x1ac66c)
      #13 boost::execution_monitor::execute(boost::function<int ()> const&) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/execution_monitor.ipp:1277:16 (test_bitcoin+0x1ac980)
      #14 boost::execution_monitor::vexecute(boost::function<void ()> const&) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/execution_monitor.ipp:1377:5 (test_bitcoin+0x1a7f9b)
      #15 boost::unit_test::unit_test_monitor_t::execute_and_translate(boost::function<void ()> const&, unsigned long) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/unit_test_monitor.ipp:49:9 (test_bitcoin+0x1a7f9b)
      #16 boost::unit_test::framework::state::execute_test_tree(unsigned long, unsigned long, boost::unit_test::framework::state::random_generator_helper const*) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/framework.ipp:823:44 (test_bitcoin+0x1e0d5c)
      #17 boost::unit_test::framework::state::execute_test_tree(unsigned long, unsigned long, boost::unit_test::framework::state::random_generator_helper const*) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/framework.ipp:792:58 (test_bitcoin+0x1e14a6)
      #18 boost::unit_test::framework::state::execute_test_tree(unsigned long, unsigned long, boost::unit_test::framework::state::random_generator_helper const*) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/framework.ipp:792:58 (test_bitcoin+0x1e14a6)
      #19 boost::unit_test::framework::run(unsigned long, bool) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/framework.ipp:1696:29 (test_bitcoin+0x1a6bfb)
      #20 boost::unit_test::unit_test_main(boost::unit_test::test_suite* (*)(int, char**), int, char**) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/unit_test_main.ipp:248:9 (test_bitcoin+0x1c4ed6)
      #21 main /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/unit_test_main.ipp:304:12 (test_bitcoin+0x1c5506)
  SUMMARY: ThreadSanitizer: data race /usr/lib/llvm-13/bin/../include/c++/v1/ios:513:18 in std::__1::ios_base::precision(long)
  ==================
  Exit status: 2

ACKs for top commit:
  fanquake:
    CI ignored ACK fa7e1471c0dcd6770a724da4a63d433fc9b4cbc1

Tree-SHA512: 5194e026410b96ad3c8addeecce0a55ee0271c3cfac9fa0715345b1a50d59925549cee0a3e415e5837ae6d2f214a7b622c73cfc7fdf41d5e55c24fb87fddb9d1
 2023-04-18 23:24:06 +03:00
MarcoFalke
58df43c05e Merge bitcoin/bitcoin#22684: test: check for invalid -prune parameters 
e2ff385e138562fb3e1cc63bdd58715a2d8bad98 test: check for invalid `-prune` parameters (Sebastian Falbesoner)

Pull request description:

  This small PR adds missing test coverage for invalid `-prune` parameter values / combinations:

  77e23ca945/src/init.cpp (L926-L928)

  77e23ca945/src/init.cpp (L935-L937)

  77e23ca945/src/init.cpp (L844-L849)

  Not sure if the tests fit into `feature_config_args.py` or should rather be moved into `feature_pruning.py`; the latter though seems to be run less often due to being very memory-hungry.

ACKs for top commit:
  laanwj:
    Code review ACK e2ff385e138562fb3e1cc63bdd58715a2d8bad98

Tree-SHA512: bb0db98090058ecac9f8a01301634e9dba9a65fd56b6a0b770f88da28c4f01e240e22b1225f0d231e28bdd4b5b51bff0e6853cccc46ed0190e91b84f7954a9db
 2023-04-18 23:24:06 +03:00
MarcoFalke
9bfb90d6df Merge bitcoin/bitcoin#21822: test: resolve bug in interface_bitcoin_cli.py 
c5bb142817c53c6a217163958b5d511f12171004 test: resolve bug in test/functional/interface_bitcoin_cli.py - Test -getinfo with -rpcwallet=unloaded wallet returns no balances (klementtan)

Pull request description:

  I think there is a bug in this test case where the new value of `cli_get_info` is not asserted.

ACKs for top commit:
  jonatack:
    ACK c5bb142817c53c6a217163958b5d511f12171004

Tree-SHA512: 50c0c2c8fe63c95f951dee892fbacedf92208f47efe5ed481fbb255f15137c799d9200fa3ff31a442df0691248d7ff04d899842722c3032cd7f35553622ba38c
 2023-04-18 23:24:06 +03:00
fanquake
6146d2991d Merge #17568: wallet: fix when sufficient preset inputs and subtractFeeFromOutputs 
eadd1304c81e0b89178e4cc7630bd31650850c85 tests: Add a test for funding with sufficient preset inputs and subtractFeeFromOutputs (Andrew Chow)
ff330badd45067cb520b1cfa1844f60a4c9f2031 Default to bnb_used = false as there are many cases where BnB is not used (Andrew Chow)

Pull request description:

  #17290 introduced a bug where, when we had preset inputs that covered the amount being sent and subtractFeeFrromOutputs was being used, transaction funding would result in a `Fee exceeds maximum configured by -maxtxfee` error. This was happening because we weren't setting `bnb_used = false` when the preset inputs were used as it should have been. This resulted in a too high fee because the change would go to fees accidentally.

  Apparently this particular case doesn't have a test, so I've added one as well.

ACKs for top commit:
  Sjors:
    ACK eadd130. I can't get this new test to fail on macOS (without this PR). It passes whether or not I compile with `--enable-debug`. It does fail on Ubuntu. Yay undefined behavior... Anyway, it's a useful test.
  fanquake:
    ACK eadd1304c81e0b89178e4cc7630bd31650850c85
  instagibbs:
    utACK eadd1304c8

Tree-SHA512: 7286c321f78666eea558cc591174630d210263594df41cab1065417510591ee514ade0e1d0cec8af09a785757da68de82592b013e8fe8d4966cec3254368706e
 2023-04-17 19:34:02 +03:00
Wladimir J. van der Laan
00b3dbd877 Merge #17283: rpc: improve getaddressinfo test coverage, help, code docs 
33f5fc32e5bfbe1e89c4d20ce455bcc6dc194151 test: add rpc getaddressinfo labels test coverage (Jon Atack)
0f3539ac6d772fc646b5f184fa1efe77bf632f6a test: add listlabels test in wallet_labels.py (Jon Atack)
1388de83900eaced906d369fe9e8887ae74b2dcf rpc: add getaddressinfo code documentation (Jon Atack)
2ee0cb3330ccf70f0540cb42370796e32eff1569 rpc: update getaddressinfo RPCExamples to bech32 (Jon Atack)
8d1ed0c263f8cdff7189f02040b5d02238d93da0 rpc: clarify label vs labels in getaddressinfo RPCHelpman (Jon Atack)
5a0ed850700dfb19167d40b38f80313bd5e427ca rpc: improve getaddressinfo RPCHelpman content (Jon Atack)
70cda342cd20d0e0cd9f28405457544036968f2d rpc: improve getaddressinfo RPCHelpman formatting (Jon Atack)

Pull request description:

  This PR is a continuation of the work in https://github.com/bitcoin/bitcoin/pull/12892.

  Main motivations:
  - There is currently no test coverage for the getaddressinfo `labels` response. Coverage here is a prerequisite before deprecating the `label` response or adding multiple labels per address.
  - `bitcoin-cli help getaddressinfo` returns a few content errors, difficult-to-read formatting, and no explanation why it returns both `label` and `labels` and how they relate, which can be confusing for application developers.

  Changes by order of commits:
  - [x] improve/fix getaddressinfo RPCHelpman layout formatting
  - [x] improve/fix getaddressinfo RPCHelpman content
  - [x] clarify the `label` and `labels` fields in getaddressinfo RPCHelpman
  - [x] update getaddressinfo RPCExamples addresses to bech32
  - [x] add getaddressinfo code docs
  - [x] add a `listlabels` test assertion in wallet_labels.py
  - [x] add missing getaddressinfo `labels` test coverage and improve the existing `label` tests

  Here are gists of the CLI help output:
  [`bitcoin-cli help getaddressinfo` before this PR](https://gist.github.com/jonatack/022af5221a85c069780359a22643c810)
  [`bitcoin-cli help getaddressinfo` after this PR](https://gist.github.com/jonatack/4ee5f6abc62a3d99269570206a5f90ba)

  It seems we ought to begin a deprecation process for the getaddressinfo `label` field? If yes, I have a follow-up ready. _--> EDIT: Deprecation follow-ups #17578 and #17585 now build on this PR._

ACKs for top commit:
  fjahr:
    Re-ACK 33f5fc32e5bfbe1e89c4d20ce455bcc6dc194151
  jnewbery:
    ACK 33f5fc32e5bfbe1e89c4d20ce455bcc6dc194151.

Tree-SHA512: a001aa863090ec2566a31059477945b1c303ebeb430b33472f8b150e420fa5742fc33bca9d95571746395b607f43f6078dd5b53e238ac1f3fc648b51c8f79a07
 2023-04-17 19:34:02 +03:00
MarcoFalke
41b4f003da Merge #17599: ci: Run functional tests on s390x 
fabd71076cd9493bd2d30a198467f5ea621b27aa ci: Print free disk space (MarcoFalke)
fad9fdbea5dfb19328282afda9588edc6f1d0ddf test: Properly deserialize integers in little-endian (MarcoFalke)
fa94fc10c881e502e6c9a71f3b7719aa955900f9 ci: Run functional tests on s390x (MarcoFalke)

Pull request description:

Top commit has no ACKs.

Tree-SHA512: 98ba77eb56f283131fdaeb393fda86cc308f1bf9781e1e0e5736b8d616528dc8ff2e494d55ba107c138083025c66a59e382fcfa9962d4349a5fd6cbbc52484c3
 2023-04-17 19:34:02 +03:00
MarcoFalke
bdf36f952b Merge #17437: rpc: Expose block height of wallet transactions 
a5e77959c8ff6a8bffa1621d7ea29ee8603c5a14 rpc: Expose block height of wallet transactions (João Barbosa)

Pull request description:

  Closes #17296.

ACKs for top commit:
  practicalswift:
    ACK a5e77959c8ff6a8bffa1621d7ea29ee8603c5a14 -- diff looks correct now (good catch @theStack!)
  theStack:
    ACK a5e77959c8ff6a8bffa1621d7ea29ee8603c5a14
  ryanofsky:
    Code review ACK a5e77959c8ff6a8bffa1621d7ea29ee8603c5a14. Changes since last review getblockhash python test fixes, and removing the last hardcoded height

Tree-SHA512: 57dcd0e4e7083f34016bf9cf8ef578fbfde49e882b6cd8623dd1c64716e096e62f6177a4c2ed94f5de304e751fe23fb9d11cf107a86fbf0a3c5f539cd2844916
 2023-04-17 19:34:02 +03:00
Samuel Dobson
2ea5283cf8 Merge #16766: wallet: Make IsTrusted scan parents recursively 
4671fc3d9e669da8b8781f0cbefee43cb9acd527 Expand on wallet_balance.py comment from https://github.com/bitcoin/bitcoin/pull/16766\#issuecomment-527563982 (Jeremy Rubin)
91f3073f08aff395dd813296bf99fd8ccc81bb27 Update release notes to mention changes to IsTrusted and impact on wallet (Jeremy Rubin)
8f174ef112199aa4e98d756039855cc561687c2e Systematize style of IsTrusted single line if (Jeremy Rubin)
b49dcbedf79613f0e0f61bfd742ed265213ed280 update variable naming conventions for IsTrusted (Jeremy Rubin)
5ffe0d144923f365cb1c2fad181eca15d1668692 Update comment in test/functional/wallet_balance.py (Jeremy Rubin)
a550c58267f50c59c2eea1d46edaa5019a8ad5d8 Update wallet_balance.py test to reflect new behavior (Jeremy Rubin)
5dd7da4ccd1354f09e2d00bab29288db0d5665d0 Reuse trustedParents in looped calls to IsTrusted (Jeremy Rubin)
595f09d6de7f1b94428cdd1310777aa6a4c584e5 Cache tx Trust per-call to avoid DoS (Jeremy Rubin)
dce032ce294fe0d531770f540b1de00dc1d13f4b Make IsTrusted scan parents recursively (Jeremy Rubin)

Pull request description:

  This slightly modifies the behavior of IsTrusted to recursively check the parents of a transaction. Otherwise, it's possible that a parent is not IsTrusted but a child is. If a parent is not trusted, then a child should not be either.

  This recursive scan can be a little expensive, so ~it might be beneficial to have a way of caching IsTrusted state, but this is a little complex because various conditions can change between calls to IsTrusted (e.g., re-org).~ I added a cache which works per call/across calls, but does not store the results semi-permanently. Which reduces DoS risk of this change. There is no risk of untrusted parents causing a resource exploitation, as we immediately return once that is detected.

  This is a change that came up as a bug-fix esque change while working on OP_SECURETHEBAG. You can see the branch where this change is important here: https://github.com/bitcoin/bitcoin/compare/master...JeremyRubin:stb-with-rpc?expand=1. Essentially, without this change, we can be tricked into accepting an OP_SECURETHEBAG output because we don't properly check the parents. As this was a change which, on its own, was not dependent on OP_SECURETHEBAG, I broke it out as I felt the change stands on its own by fixing a long standing wallet bug.

  The test wallet_balance.py has been corrected to meet the new behavior. The below comment, reproduced, explains what the issue is and the edge cases that can arise before this change.

          # Before `test_balance()`, we have had two nodes with a balance of 50
          # each and then we:
          #
          # 1) Sent 40 from node A to node B with fee 0.01
          # 2) Sent 60 from node B to node A with fee 0.01
          #
          # Then we check the balances:
          #
          # 1) As is
          # 2) With transaction 2 from above with 2x the fee
          #
          # Prior to #16766, in this situation, the node would immediately report
          # a balance of 30 on node B as unconfirmed and trusted.
          #
          # After #16766, we show that balance as unconfirmed.
          #
          # The balance is indeed "trusted" and "confirmed" insofar as removing
          # the mempool transactions would return at least that much money. But
          # the algorithm after #16766 marks it as unconfirmed because the 'taint'
          # tracking of transaction trust for summing balances doesn't consider
          # which inputs belong to a user. In this case, the change output in
          # question could be "destroyed" by replace the 1st transaction above.
          #
          # The post #16766 behavior is correct; we shouldn't be treating those
          # funds as confirmed. If you want to rely on that specific UTXO existing
          # which has given you that balance, you cannot, as a third party
          # spending the other input would destroy that unconfirmed.
          #
          # For example, if the test transactions were:
          #
          # 1) Sent 40 from node A to node B with fee 0.01
          # 2) Sent 10 from node B to node A with fee 0.01
          #
          # Then our node would report a confirmed balance of 40 + 50 - 10 = 80
          # BTC, which is more than would be available if transaction 1 were
          # replaced.

  The release notes have been updated to note the new behavior.

ACKs for top commit:
  ariard:
    Code Review ACK 4671fc3, maybe extend DoS protection in a follow-up PR.
  fjahr:
    Code review ACK 4671fc3d9e669da8b8781f0cbefee43cb9acd527
  ryanofsky:
    Code review ACK 4671fc3d9e669da8b8781f0cbefee43cb9acd527. Changes since last review: 2 new commits adding suggested release note and python test comment, also a clean rebase with no changes to the earlier commits. The PR description is more comprehensive now, too. Looks good!
  promag:
    Code review ACK 4671fc3d9e669da8b8781f0cbefee43cb9acd527.

Tree-SHA512: 6b183ff425304fef49724290053514cb2770f4a2350dcb83660ef24af5c54f7c4c2c345b0f62bba60eb2d2f70625ee61a7fab76a7f491bb5a84be5c4cc86b92f
 2023-04-17 19:34:02 +03:00
MarcoFalke
cd201d2d6c Merge bitcoin/bitcoin#24054: test: rest /tx with an invalid/unknown txid 
bd52684508ca2964e6a3af503d21ff99675380c7 test: rest /tx with an invalid/unknown txid (brunoerg)

Pull request description:

  This PR adds test coverage to the endpoint `/tx` (rest) passing an invalid and an unknown txid to test its return.
  Invalid -> should return status code 400 (bad request)
  Unknown -> should return status code 404 (not found)

ACKs for top commit:
  kallewoof:
    ACK bd52684508ca2964e6a3af503d21ff99675380c7

Tree-SHA512: a7fbb63f30d06fc0855133a36e8317c7930ba13aa2b4a2dd1fc35079d59eacace72e1ffe7ae1b3e067066fe51792415940d72d923e83a659a0d5965e4110b32a
 2023-04-17 11:17:34 -05:00
MarcoFalke
32aed5dfa0 Merge bitcoin/bitcoin#23963: test: run feature_pruning.py without wallet compiled 
0754e9c01bd3d57aa241e313ba34c18c4897ba98 test: run feature_pruning.py without wallet compiled (Sebastian Falbesoner)

Pull request description:

  Only one small part of the pruning test (sub-test `wallet_test`) is wallet-related, hence we can run all other parts without wallet compiled.

ACKs for top commit:
  MarcoFalke:
    cr ACK 0754e9c01bd3d57aa241e313ba34c18c4897ba98

Tree-SHA512: 856856903d21d64953ed0102cc2a96f55975c4b7d8e93e57b82c266024967160df64df2b6068be089efc05e883e8d6d12e7327053420d4c640b9d8cc5bcb1c58
 2023-04-17 11:17:34 -05:00
W. J. van der Laan
076e0528ef Merge bitcoin/bitcoin#23370: test: Add ios_base::width tsan suppression 
96c7db9373014ce232ab01d11333650c9ddf9ee5 test: Add ios_base::width tsan suppression (Hennadii Stepanov)

Pull request description:

  This PR:
  - adds tsan suppression for intermittent failures in CI
  ```
  SUMMARY: ThreadSanitizer: data race /usr/lib/llvm-12/bin/../include/c++/v1/ios:523:12 in std::__1::ios_base::width() const
  ```

  - fixes #23366

ACKs for top commit:
  laanwj:
    Concept and code review ACK 96c7db9373014ce232ab01d11333650c9ddf9ee5

Tree-SHA512: fcad296e8da4a6d94dcbb011c3d9b3d07f6983818be16cfff8341a035fa6abe2777ae72409c9bc83083097660408a850c1e9cd6f0ad3ea7976e4a4768f1e1858
 2023-04-17 11:17:34 -05:00
fanquake
0c52db6174 Merge bitcoin/bitcoin#23214: Replace stoul with ToIntegral in dbwrapper 
fa165e954579436fe4b636e4222d8ce0c1269786 Replace stoul with ToIntegral in dbwrapper (MarcoFalke)

Pull request description:

  The string is created with `%llu`. See: 7fcf53f7b4/src/leveldb/db/db_impl.cc (L1436-L1437)

  So it seems odd to silently accept when parsing: whitespace, a sign character, trailing chars, overflow, ....

  Fix that by using the stricter ToIntegral.

ACKs for top commit:
  laanwj:
    Code review ACK fa165e954579436fe4b636e4222d8ce0c1269786
  practicalswift:
    cr ACK fa165e954579436fe4b636e4222d8ce0c1269786
  theStack:
    Code-review ACK fa165e954579436fe4b636e4222d8ce0c1269786

Tree-SHA512: b87f01431ca0b971ff84610022da8679d3c33470b88cfc3f4a337e6e176a0455715588aefd40e8e2bbe7459d902dc89d7bfe34e7fd66755f631cc18dc039fa2f
 2023-04-17 11:17:34 -05:00
Wladimir J. van der Laan
eec81f7b33 Merge #15921: validation: Tidy up ValidationState interface 
3004d5a12d09d94bfc4dee2a8e8f2291996a4aaf [validation] Remove fMissingInputs from AcceptToMemoryPool() (John Newbery)
c428622a5bb1e37b2e6ab2c52791ac05d9271238 [validation] Remove unused first_invalid parameter from ProcessNewBlockHeaders() (John Newbery)
7204c6434b944f6ad51b3c895837729d3aa56eea [validation] Remove useless ret parameter from Invalid() (John Newbery)
1a37de4b3174d19a6d8691ae07e92b32fdfaef11 [validation] Remove error() calls from Invalid() calls (John Newbery)
067981e49246822421a7bcc720491427e1dba8a3 [validation] Tidy Up ValidationResult class (John Newbery)
a27a2957ed9afbe5a96caa5f0f4cbec730d27460 [validation] Add CValidationState subclasses (John Newbery)

Pull request description:

  Carries out some remaining tidy-ups remaining after PR 15141:

  - split ValidationState into TxValidationState and BlockValidationState (commit from ajtowns)
  - various minor code style tidy-ups to the ValidationState class
  - remove the useless `ret` parameter from `ValidationState::Invalid()`
  - remove the now unused `first_invalid` parameter from `ProcessNewBlockHeaders()`
  - remove the `fMissingInputs` parameter from `AcceptToMemoryPool()`, and deal with missing inputs the same way as other errors by using the `TxValidationState` object.

  Tip for reviewers (thanks ryanofsky!): The first commit ("[validation] Add CValidationState subclasses" ) is huge and can be easier to start reviewing if you revert the rote, mechanical changes:

  Substitute the commit hash of commit "[validation] Add CValidationState subclasses" for <CommitHash> in the commands below.

  ```sh
  git checkout <CommitHash>
  git grep -l ValidationState | xargs sed -i 's/BlockValidationState\|TxValidationState/CValidationState/g'
  git grep -l ValidationResult | xargs sed -i 's/BlockValidationResult\|TxValidationResult/ValidationInvalidReason/g'
  git grep -l MaybePunish | xargs sed -i 's/MaybePunishNode\(ForBlock\|ForTx\)/MaybePunishNode/g'
  git diff HEAD^
  ```

  After that it's possible to easily see the mechanical changes with:

  ```sh
  git log -p -n1 -U0 --word-diff-regex=. <CommitHash>
  ```

ACKs for top commit:
  laanwj:
    ACK 3004d5a12d09d94bfc4dee2a8e8f2291996a4aaf
  amitiuttarwar:
    code review ACK 3004d5a12d09d94bfc4dee2a8e8f2291996a4aaf. Also built & ran tests locally.
  fjahr:
    Code review ACK 3004d5a12d09d94bfc4dee2a8e8f2291996a4aaf . Only nit style change and pure virtual destructor added since my last review.
  ryanofsky:
    Code review ACK 3004d5a12d09d94bfc4dee2a8e8f2291996a4aaf. Just whitespace change and pure virtual destructor added since last review.

Tree-SHA512: 511de1fb380a18bec1944ea82b513b6192df632ee08bb16344a2df3c40811a88f3872f04df24bc93a41643c96c48f376a04551840fd804a961490d6c702c3d36
 2023-04-17 10:42:25 -05:00
Wladimir J. van der Laan
091d813e00 Merge #17004: validation: Remove REJECT code from CValidationState 
9075d13153ce06cd59a45644831ecc43126e1e82 [docs] Add release notes for removal of REJECT reasons (John Newbery)
04a2f326ec0f06fb4fce1c4f93500752f05dede8 [validation] Fix REJECT message comments (John Newbery)
e9d5a59e34ff2d538d8f5315efd9908bf24d0fdc [validation] Remove REJECT code from CValidationState (John Newbery)
0053e16714323c1694c834fdca74f064a1a33529 [logging] Don't log REJECT code when transaction is rejected (John Newbery)
a1a07cfe99fc8cee30ba5976dc36b47b1f6532ab [validation] Fix peer punishment for bad blocks (John Newbery)

Pull request description:

  We no longer send BIP 61 REJECT messages, so there's no need to set
  a REJECT code in the CValidationState object.

  Note that there is a minor bug fix in p2p behaviour here. Because the
  call to `MaybePunishNode()` in `PeerLogicValidation::BlockChecked()` only
  previously happened if the REJECT code was > 0 and < `REJECT_INTERNAL`,
  then there are cases were `MaybePunishNode()` can get called where it
  wasn't previously:

  - when `AcceptBlockHeader()` fails with `CACHED_INVALID`.
  - when `AcceptBlockHeader()` fails with `BLOCK_MISSING_PREV`.

  Note that `BlockChecked()` cannot fail with an 'internal' reject code. The
  only internal reject code was `REJECT_HIGHFEE`, which was only set in
  ATMP.

  This reverts a minor bug introduced in 5d08c9c579.

ACKs for top commit:
  ariard:
    ACK 9075d13, changes since last reviewed are splitting them in separate commits to ease understanding and fix nits
  fjahr:
    ACK 9075d13153ce06cd59a45644831ecc43126e1e82, confirmed diff to last review was fixing nits in docs/comments.
  ryanofsky:
    Code review ACK 9075d13153ce06cd59a45644831ecc43126e1e82. Only changes since last review are splitting the main commit and updating comments

Tree-SHA512: 58e8a1a4d4e6f156da5d29fb6ad6a62fc9c594bbfc6432b3252e962d0e9e10149bf3035185dc5320c46c09f3e49662bc2973ec759679c0f3412232087cb8a3a7
 2023-04-17 10:42:25 -05:00
MarcoFalke
5d8f250270 Merge #16524: Wallet: Disable -fallbackfee by default 
ea4cc3a7b36a9c77dbf0aff439da3ef0ea58e6e4 Truly decouple wallet from chainparams for -fallbackfee (Jorge Timón)

Pull request description:

  Before it was 0 by default for main and 20000 for test and regtest.
  Now it is 0 by default for all chains, thus there's no need to call Params().

  Also now the default for main is properly documented.

  Suggestion for release notes:

  -fallbackfee was 0 (disabled) by default for the main chain, but 20000 by default for the test chains. Now it is 0 by default for all chains. Testnet and regtest users will have to add fallbackfee=20000 to their configuration if they weren't setting it and they want it to keep working like before.

  Should I propose them to the wiki for the release notes or only after merge?

  For more context, see https://github.com/bitcoin/bitcoin/pull/16402#issuecomment-515701042

ACKs for top commit:
  MarcoFalke:
    ACK ea4cc3a7b36a9c77dbf0aff439da3ef0ea58e6e4

Tree-SHA512: fdfaba5d813da4221e405e0988bef44f3856d10f897a94f9614386d14b7716f4326ab8a6646e26d41ef3f4fa61b936191e216b1b605e9ab0520b0657fc162e6c

----

Co-Authored-By: UdjinM6 <UdjinM6@users.noreply.github.com>
 2023-04-17 10:42:25 -05:00
Kittywhiskers Van Gogh
f4eaad0bb9 merge bitcoin#20002: expose peer network in getpeerinfo; simplify/improve -netinfo 2023-04-17 08:36:33 +00:00
Kittywhiskers Van Gogh
1fab09d8fa merge bitcoin#19133: add bitcoin-cli -generate command 2023-04-17 08:30:49 +00:00
Kittywhiskers Van Gogh
95d462d01d merge bitcoin#18594: display multiwallet balances in -getinfo 2023-04-17 08:30:49 +00:00
Kittywhiskers Van Gogh
50b4901bce merge bitcoin#18724: add coverage for -rpcwallet cli option 2023-04-17 08:30:49 +00:00
Kittywhiskers Van Gogh
0d8932c979 merge bitcoin#18691: add wait_for_cookie_credentials() to framework for rpcwait tests 2023-04-17 08:30:49 +00:00
Kittywhiskers Van Gogh
6c5eda9ad8 merge bitcoin#18653: add coverage for bitcoin-cli -rpcwait 2023-04-17 08:30:49 +00:00
Kittywhiskers Van Gogh
668baf262f merge bitcoin#18574: call getbalances.ismine.trusted instead of getwalletinfo.balance 2023-04-17 08:30:49 +00:00
MarcoFalke
e38953f0cb Merge #20379: tests: Remove no longer needed UBSan suppression (float divide-by-zero in validation.cpp) 
0ccb3addf68067200892963521a92713c4667a63 tests: Remove no longer needed UBSan suppression (float-divide-by-zero in validation.cpp) (practicalswift)

Pull request description:

  Remove no longer needed UBSan suppression.

  The float divide-by-zero in `validation.cpp` was fixed by instagibbs in ec30a79f1c430cc7fbda37e5d747b0b31b262fa5 (#15283).

ACKs for top commit:
  MarcoFalke:
    ACK 0ccb3addf68067200892963521a92713c4667a63

Tree-SHA512: 89a4f4b7371fa5725d9f801cee7ebbd17523f66017c9acfa813657dcb8d837f42209eff44ce9e5d48296a630bab9599d75f10024a0c7da7defb228f4eae3392a
 2023-04-16 23:40:59 +03:00
MarcoFalke
f8d32e7e1b Merge #20344: wallet: fix scanning progress calculation for single block range 
5e146022daa4336de94447e5b8e5418296286927 wallet: fix scanning progress calculation for single block range (Sebastian Falbesoner)

Pull request description:

  If the blockchain is rescanned for a single block (i.e. start and stop hashes are equal, and with that also the estimated start/stop verification progress values) the progress calculation could lead to a NaN value caused by a division by zero (0.0/0.0), resulting in an invalid JSON result for the `getwalletinfo` RPC.  This PR fixes this behaviour by setting the progress to zero in that special case. Fixes #20297.

  The behaviour can easily be reproduced by continuously running single block rescans in an endless loop, e.g. via
  ```bash
  #!/bin/bash
  while true
  do
      bitcoin-cli rescanblockchain $(bitcoin-cli getblockcount)
  done
  ```

  and at the same time perform some `getwalletinfo` RPCs.

  On the master branch, this leads to frequent invalid responses (tested on mainchain):
  ```
  $ bitcoin-cli getwalletinfo
  error: couldn't parse reply from server
  $ curl --user `cat ~/.bitcoin/.cookie` --data-binary '{"jsonrpc": "1.0", "id": "curltest", "method": "getwalletinfo", "params": []}' -H 'content-type: text/plain;' http://127.0.0.1:8332/
  {"result":{"walletname":"","walletversion":169900,"format":"bdb","balance":0.00000000,"unconfirmed_balance":0.00000000,"immature_balance":0.00000000,"txcount":0,"keypoololdest":1603677276,"keypoolsize":1000,"hdseedid":"3196e33ecb47c7130e6ca60f2f895f9259860dca","keypoolsize_hd_internal":1000,"paytxfee":0.00000000,"private_keys_enabled":true,"avoid_reuse":false,"scanning":{"duration":0,"progress":},"descriptors":false},"error":null,"id":"curltest"}
  ```
  (note that missing value for "progress" in the JSON result).

  On the PR branch, the behaviour doesn't occur anymore.

ACKs for top commit:
  MarcoFalke:
    review ACK 5e146022daa4336de94447e5b8e5418296286927
  promag:
    Core review ACK 5e146022daa4336de94447e5b8e5418296286927.

Tree-SHA512: f0e6aad5a6cd08b36c5fe820fff0ef26663229b39169a4dbe757f3c795a41cf5c69c9dc90efe7515675ae1059307f8971123781a0514d10704123a6f28b125ab
 2023-04-16 23:40:59 +03:00
PastaPastaPasta
770eefcd08
ci: upgrade cppcheck to 2.10; fix / suppress reported issues (#5328) 
## Issue being fixed or feature implemented
Upgraded version of cppcheck

## What was done?

## How Has This Been Tested?
Ran cppcheck

## 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)_
 2023-04-16 12:08:33 -05:00
Kittywhiskers Van Gogh
dab1fbc010
fix: specify expected_stderr within restart_node in feature_pruning.py (#5324) 
## Checklist:
- [x] I have performed a self-review of my own code
- [ ] ~~I have commented my code, particularly in hard-to-understand
areas~~ N/A
- [ ] ~~I have added or updated relevant unit/integration/functional/e2e
tests~~ N/A
- [ ] ~~I have made corresponding changes to the documentation~~ N/A
- [ ] 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-04-16 11:34:03 +03:00