Commit Graph

23728 Commits

Author SHA1 Message Date
Konstantin Akimov
359de5a696
feat: improve unit test "IsTriviallyValid" (#5516)
## Issue being fixed or feature implemented
It partially resolves issue https://github.com/dashpay/dash/issues/5471

Better unit tests are needed to validate changes in ProTx implementation
such as this PR: https://github.com/dashpay/dash/pull/5463

## What was done?
- Invalid ProTx transactions are checked more strictly. The flag "tx is
failed" is not enough now for test to succeed, but error code should
matched with expected error.
- Duplicated implementations of tests for "valid" and "invalid
transaction" are changed to more general code.
- Added extra log output with tx ID for easier debug - to see which
exactly tx is failed in test
 - Supported more by 256 txes in one json file

## How Has This Been Tested?
Run unit 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
- [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-31 12:15:03 -05:00
UdjinM6
e8fe452837
Merge branch 'master' into merge_master_19.3.0 2023-07-31 19:58:19 +03:00
Konstantin Akimov
9bb1b10871
refactor: improved initialization of members of LLMQContext and related changes (#5150)
LLMQContext uses RAII to initialize all members. Ensured that all
members always initialized correctly in proper order if LLMQContext
exists.

BlockAssembler, CChainState use too many agruments and they are making
wrong assumption that members of LLMQContext can be constructed and used
independently, but that's not true. Instead, let's pass LLMQContext
whenever possible.

## Issue being fixed or feature implemented
https://github.com/dashpay/dash-issues/issues/52

## How Has This Been Tested?
Run unit/functional test and introduce no breaking changes.


## Checklist:
- [x] I have performed a self-review of my own code
- [x] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone
2023-07-29 20:23:02 -05:00
strophy
5c9896e32e
ci: multi-runner guix builds (#5515)
## Issue being fixed or feature implemented
- We want to enable use of the AWS-hosted GitHub Actions runners, now
that [corresponding
infra](https://github.com/dcginfra/tf-aws-gh-runner/pull/8/files#diff-ad98d33884a302f6c747dc6b326c6b3af3887f2ec25e0bd7a0395f10444818f3)
exists to deploy these runners


## What was done?
Add new labels and workflow dispatch button to allow runner testing


## How Has This Been Tested?
Pending testing in CI


## Breaking Changes
None


## 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
- [ ] I have assigned this pull request to a milestone _(for repository
code-owners and collaborators only)_
2023-07-28 11:43:31 -05:00
PastaPastaPasta
d9a0b32cbf
Merge pull request #5492 from kittywhiskers/assumeutxo4
backport: merge bitcoin#21391, #19550, #15946, #21230, #21252, #21297, #20605, #21575, #22309, #19762 (assumeutxo backports: part 4)
2023-07-28 00:19:28 -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
4e82a960b6 merge bitcoin#22309: Add missing atomic include 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
94b706e402 merge bitcoin#20605: Signal-safe instant shutdown 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
b8fc743f1f trivial: consistently use context as the argument name for CoreContext 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
Kittywhiskers Van Gogh
77963ba29d rpc: Prune g_chainman usage within Dash-specific RPC modules 2023-07-28 00:18:27 -05:00
Kittywhiskers Van Gogh
60e93cabeb merge bitcoin#21391: Prune g_chainman usage in RPC modules 2023-07-28 00:18:27 -05:00
Kittywhiskers Van Gogh
bbb6815982 fix: resolve cppcheck shadow variable warning
```
src/governance/governance.cpp:558:18: note: Shadow variable
src/governance/governance.cpp:558:18: warning: Local variable '_' shadows outer function [shadowFunction]
```
2023-07-28 00:18:27 -05:00
UdjinM6
b710fe0836
feat: Enable fallbackfee by default on all networks (#5507)
## Issue being fixed or feature implemented
We have plenty of block space. Having `fallbackfee` disabled by default
is needlessly annoying.

## What was done?
Bump `DEFAULT_FALLBACK_FEE` to `1000`, same as it is on `master`
https://github.com/dashpay/dash/blob/master/src/wallet/wallet.h#L68

## How Has This Been Tested?
run tests, send txes on testnet

## Breaking Changes
should be none

## 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-27 18:47:30 +03:00
thephez
54a8c393cc
chore: correct rpc typo in mempoolentry help (#5512)
## Issue being fixed or feature implemented
RPC help for mempoolentry incorrectly called the "instantsend" field
"time". The "instantsend" and "unbroadcast" fields were also in a
different order than the actual response.

## What was done?
Changed "time" -> "instantsend" and flipped order of
"instantsend"/"unbroadcast"

## How Has This Been Tested?
Built and checked locally

## Breaking Changes
N/A


## 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-07-27 09:46:19 -05:00
PastaPastaPasta
a884f0c536
Merge pull request #5506 from UdjinM6/backport_19.3_candidates
[v19.x] Backport 19.3 candidates, v19.3.0 release notes and version bump
2023-07-27 09:23:38 -05:00
UdjinM6
1c81bddf1c
chore: Bump version to 19.3 2023-07-26 13:15:14 +03:00
UdjinM6
9158a16098
doc: Add 19.3 release notes 2023-07-26 13:15:14 +03:00
UdjinM6
91d0941217
doc: Archive 19.2 release notes 2023-07-25 23:37:26 +03:00
UdjinM6
580f8db73e
partial: feat: bury v19 activation (#5496)
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.

Bury v19, bump `MinBIP9WarningHeight`

Run tests, reindex on mainnet/testnet.

n/a

- [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-25 22:52:52 +03:00
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
Odysseas Gabrielides
c96edec318
chore: Added missing sources files in CMake (#5503)
## Issue being fixed or feature implemented
Added missing sources files (index, interfaces, node, logging, util) in
CMake so they can be indexed by IDE.

## What was done?

## 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-07-25 12:23:56 -05:00
Odysseas Gabrielides
9d89de2898
feat: Check if settings file is empty (#5504)
## Issue being fixed or feature implemented
Fixes issue #5497.

## What was done?
Checks if settings file is empty, and deletes it if that's the case.

It will will be generated with default value `{}` afterwards.

## How Has This Been Tested?
Running Dash Qt on regtest masternode with `--nocleanup` and
`./src/qt/dash-qt --regtest --datadir=`

## 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
- [ ] 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-25 19:14:49 +03:00
UdjinM6
66ef7943f5
fix: Process dsq messages even when CJ is disabled locally or disk space is low (#5498)
Refusing to process `dsq` will result in node not being able to process
`dstx`es later.

n/a

- [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-25 19:14:46 +03:00
UdjinM6
b7ef43f9dd
fix: start client-side CoinJoin scheduler even when CoinJoin is disabled (#5476)
`IsEnabled()` is checked inside anyway. Not starting the scheduler on
init results in no mixing on nodes with dynamically loaded wallets.

- [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-25 19:07:53 +03:00
UdjinM6
5b2541b1ff
Merge pull request #5466 from UdjinM6/dashbls_1.3.0
chore: bump `dashbls` subtree to 1.3.0
2023-07-25 19:06:55 +03:00
UdjinM6
804030c56f
fix: Fix trivial validation typo and test (#5465)
`GetVersion` expects `is_basic_scheme_active`, not
`is_bls_legacy_scheme`

see commits

`make check`

luckily only tests are affected

- [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-25 19:06:42 +03:00
UdjinM6
2c2f81675e
feat(wallet): TopUpKeyPool improvements (#5456)
- make progress calculations sane
- show progress in GUI but only when you need 100+ new keys
- make it stop on shutdown request
- spam less in debug.log

run tests, run `keypoolrefill` with `1100` (add 100 keys, no gui popup)
and `10000` (100+ keys, progress bar) on testnet wallet, check logs,
verify it can be interrupted on shutdown

n/a

- [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-25 19:02:47 +03:00
UdjinM6
8ef7002679
fix(rpc): Improve upgradetohd (#5455)
Allow `upgradetohd` in IBD, better errors, no GUI lock-up

Pls see individual commits. Most of it is changes in whitespaces, might
want to use ?w=1 to review i.e.
https://github.com/dashpay/dash/pull/5455/files?w=1

run tests, try `upgradetohd` on testnet

n/a

- [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-25 19:01:36 +03:00
UdjinM6
27e68397d0
fix: Allow tx index to catch up with the block index in TestChainSetup dtor (#5454)
## Issue being fixed or feature implemented
TL;DR: Should hopefully fix crashes like
https://gitlab.com/dashpay/dash/-/jobs/4522256293

In dashd we flush all callbacks first and then destroy `g_txindex`. In
tests we had to move `g_txindex` to `TestChainSetup` and its dtor is
executed first, so the order is broken. It also explains why this crash
happens so rare. In most cases tx index is up to date and you need some
kind of a hiccup for scheduler to lag behind a bit. Basically, between
`g_txindex.reset()` and `FlushBackgroundCallbacks`
`BaseIndex::BlockConnected` finally arrives. But it’s processed on a
(now) null instance hence a crash. If it’s earlier - it’s processed
normally, if it’s later - it’s flushed without execution, so there is a
tiny window to catch this crash.

## What was done?
Give tx index a bit of time to process everything

## How Has This Been Tested?
run tests (but this crash is rare 🤷‍♂️ )

## 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-07-25 18:54:49 +03:00
UdjinM6
f3dc889e93
feat(qt): refresh the whole wallet instead of processing individual updates for huge notification queues (#5453)
## Issue being fixed or feature implemented
It's super slow for wallets with 100.000s of txes to process lots of
notifications produced by rescan. Skip them all and simply refresh the
whole wallet instead. In my case (500k+ txes testnet wallet) gui update
after `rescanblockchain` time is down from _forever_ to ~30 seconds.
Same for `wipewallettxes true` (#5451 ). Gui update after
`wipewallettxes`/`wipewallettxes false` is instant (cause there are no
txes anymore) vs _forever_ before the patch.


## What was done?
refresh the whole wallet when notification queue is above 10K operations

actual changes (ignoring whitespaces):
d013cb4f5c

## How Has This Been Tested?
running on top of #5451 and #5452 , wiping and rescanning w/ and w/out
this patch.

## Breaking Changes
should be none


## 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-25 18:54:39 +03:00
UdjinM6
ff60d10934
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-07-25 18:54:26 +03:00
Odysseas Gabrielides
49e024338a
feat(rpc): Ability to filter HPMNs in masternodelist and protx list rpcs (#5447)
## Issue being fixed or feature implemented
Added the filter `hpmn` for both `masternodelist` and `protx list` rpcs.

## What was done?

## How Has This Been Tested?

Calling this RPC on Testnet.

## 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
- [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-07-25 18:54:16 +03:00
UdjinM6
950fdde4e8
docs: add 2 more contributors to 19.2.0 release notes (#5446)
## Issue being fixed or feature implemented
@ogabrielides @kittywhiskers I somehow failed to add you guys to the
list of v19.2 contributors 🙈 sorry!

## What was done?


## 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-07-25 18:54:06 +03:00
PastaPastaPasta
382400f0b5
perf: avoid rehashing block; use stored hash (#5435)
before 12%
<img width="1538" alt="image"
src="https://github.com/dashpay/dash/assets/6443210/fa5043fb-4e48-4728-bfaf-8636d5c20a8c">
after 10%
<img width="1544" alt="image"
src="https://github.com/dashpay/dash/assets/6443210/1df6aff4-2901-4af1-b421-3604f54df157">

Redundant rehash

Avoid redundant rehash

Reindexed 0-500000 on testnet

None

_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-07-25 18:53:36 +03:00
UdjinM6
1baf2d5933
feat: Allow mining blocks of a specific version on non-mainnet networks (#5433)
## Issue being fixed or feature implemented
Mining blocks with a specific version can be useful on testnet and
devnets too

## What was done?
lift restrictions for `-blockversion`

## How Has This Been Tested?
it should just work :)

## 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-25 18:51:41 +03:00
PastaPastaPasta
00cb31cc8c
Merge pull request #5488 from vijaydasmp/bp22_1
backport: Merge bitcoin#20408,19836,19851,(partial)21424, 21948, 20056,20432,19337,20448, 21010
2023-07-25 10:46:03 -05:00
MarcoFalke
5d91734065 Merge #21010: refactor: remove straggling boost::mutex usage
f827e151a2ce96e14aadb9e7d25045fe0a8afbd2 refactor: remove straggling boost::mutex usage (fanquake)

Pull request description:

  After the merge of #18710, the linter is warning:
  ```bash
  A new Boost dependency in the form of "boost/thread/mutex.hpp" appears to have been introduced:
  src/sync.cpp:#include <boost/thread/mutex.hpp>
  src/test/sync_tests.cpp:#include <boost/thread/mutex.hpp>

  ^---- failure generated from test/lint/lint-includes.sh
  ```

  #18710 removed `boost/thread/mutex.hpp` from lint-includes, however in the interim #19337 was merged, which introduced more `boost::mutex` usage.

  Given we no longer use `boost::mutex`, just remove the double lock test and remaining includes.

ACKs for top commit:
  laanwj:
    Code review ACK f827e151a2ce96e14aadb9e7d25045fe0a8afbd2
  hebasto:
    ACK f827e151a2ce96e14aadb9e7d25045fe0a8afbd2

Tree-SHA512: f738b12189fe5b39db3e8f8231e9002714413a962eaf98adc84a6614fa474df5616358cfb1c89b92a2b0564efa9b704a774c49d4a25dca18a0ccc3cd9eabfc0a
2023-07-25 10:45:09 -05: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
8fee13ef70 Merge #19337: sync: detect double lock from the same thread
95975dd08d8fdaaeaf28e0d06b861ce2748c17b6 sync: detect double lock from the same thread (Vasil Dimov)
4df6567e4cbb4677e8048de2f8008612e1b860b9 sync: make EnterCritical() & push_lock() type safe (Vasil Dimov)

Pull request description:

  Double lock of the same (non-recursive) mutex from the same thread would produce an undefined behavior. Detect this from `DEBUG_LOCKORDER` and react similarly to the deadlock detection.

  This came up during discussion in another, related PR: https://github.com/bitcoin/bitcoin/pull/19238#discussion_r442394521.

ACKs for top commit:
  laanwj:
    code review ACK 95975dd08d8fdaaeaf28e0d06b861ce2748c17b6
  hebasto:
    re-ACK 95975dd08d8fdaaeaf28e0d06b861ce2748c17b6

Tree-SHA512: 375c62db7819e348bfaecc3bd82a7907fcd8f5af24f7d637ac82f3f16789da9fc127dbd0e37158a08e0dcbba01a55c6635caf1d8e9e827cf5a3747f7690a498e
2023-07-25 10:45:09 -05:00
Wladimir J. van der Laan
c1b5ec6360 Merge #20432: net: Treat raw message bytes as uint8_t
fabecce71909c984504c21fa05f91d5f1b471e8c net: Treat raw message bytes as uint8_t (MarcoFalke)

Pull request description:

  Using `uint8_t` from the beginning when messages are `recv`ed has two style benefits:
  * The signedness is clear from reading the code, as it does not depend on the architecture
  * When passing the bytes on, the need for static signedness casts is dropped, making the code a bit less verbose and more coherent

ACKs for top commit:
  laanwj:
    Code review ACK fabecce71909c984504c21fa05f91d5f1b471e8c
  theStack:
    Code Review ACK fabecce71909c984504c21fa05f91d5f1b471e8c
  jonatack:
    Tested ACK fabecce71909c984504c21fa05f91d5f1b471e8c

Tree-SHA512: e6d9803c78633fde3304faf592afa961ff9462a7912d1da97a24720265274aa10ab4168d71b6ec2756b7448dd42585321afee0e5c889e705be778ce9a330d145
2023-07-25 10:45:09 -05:00
Wladimir J. van der Laan
21a6b171dc Merge #20056: net: Use Span in ReceiveMsgBytes
fa5ed3b4ca609426b2622cad235e107d33db7b30 net: Use Span in ReceiveMsgBytes (MarcoFalke)

Pull request description:

  Pass a data pointer and a size as span in `ReceiveMsgBytes` to get the benefits of a span

ACKs for top commit:
  jonatack:
    ACK fa5ed3b4ca609426b2622cad235e107d33db7b30 code review, rebased to current master 12a1c3ad1a43634, debug build, unit tests, ran bitcoind/-netinfo/getpeerinfo
  theStack:
    ACK fa5ed3b4ca609426b2622cad235e107d33db7b30

Tree-SHA512: 89bf111323148d6e6e50185ad20ab39f73ab3a58a27e46319e3a08bcf5dcf9d6aa84faff0fd6afb90cb892ac2f557a237c144560986063bc736a69ace353ab9d
2023-07-25 10:45:09 -05:00
MarcoFalke
994ce01023 Merge bitcoin/bitcoin#21948: test: Fix off-by-one in mockscheduler test RPC
fa2e614d16af84327adf1c02746d0f73e0f48111 test: Fix off-by-one in mockscheduler test RPC (MarcoFalke)

Pull request description:

  Fixes:

  ```
  fuzz: scheduler.cpp:83: void CScheduler::MockForward(std::chrono::seconds): Assertion `delta_seconds.count() > 0 && delta_seconds < std::chrono::hours{1}' failed.
  ==1059066== ERROR: libFuzzer: deadly signal
      #0 0x558f75449c10 in __sanitizer_print_stack_trace (/root/fuzz_dir/scratch/fuzz_gen/code/src/test/fuzz/fuzz+0x5fec10)
      #1 0x558f753f32b8 in fuzzer::PrintStackTrace() fuzzer.o
      #2 0x558f753d68d3 in fuzzer::Fuzzer::CrashCallback() fuzzer.o
      #3 0x7f4a3cbbb3bf  (/lib/x86_64-linux-gnu/libpthread.so.0+0x153bf)
      #4 0x7f4a3c7ff18a in raise (/lib/x86_64-linux-gnu/libc.so.6+0x4618a)
      #5 0x7f4a3c7de858 in abort (/lib/x86_64-linux-gnu/libc.so.6+0x25858)
      #6 0x7f4a3c7de728  (/lib/x86_64-linux-gnu/libc.so.6+0x25728)
      #7 0x7f4a3c7eff35 in __assert_fail (/lib/x86_64-linux-gnu/libc.so.6+0x36f35)
      #8 0x558f7588a913 in CScheduler::MockForward(std::chrono::duration<long, std::ratio<1l, 1l> >) scheduler.cpp:83:5
      #9 0x558f75b0e5b1 in mockscheduler()::$_7::operator()(RPCHelpMan const&, JSONRPCRequest const&) const rpc/misc.cpp:435:30
      #10 0x558f75b0e5b1 in std::_Function_handler<UniValue (RPCHelpMan const&, JSONRPCRequest const&), mockscheduler()::$_7>::_M_invoke(std::_Any_data const&, RPCHelpMan const&, JSONRPCRequest const&) /usr/bin/../lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/std_function.h:285:9
      #11 0x558f7587a141 in std::function<UniValue (RPCHelpMan const&, JSONRPCRequest const&)>::operator()(RPCHelpMan const&, JSONRPCRequest const&) const /usr/bin/../lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/std_function.h:688:14
      #12 0x558f7587a141 in RPCHelpMan::HandleRequest(JSONRPCRequest const&) const rpc/util.cpp:565:26
      #13 0x558f756c0086 in CRPCCommand::CRPCCommand(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, RPCHelpMan (*)())::'lambda'(JSONRPCRequest const&, UniValue&, bool)::operator()(JSONRPCRequest const&, UniValue&, bool) const ./rpc/server.h:110:91
      #14 0x558f756c0086 in std::_Function_handler<bool (JSONRPCRequest const&, UniValue&, bool), CRPCCommand::CRPCCommand(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, RPCHelpMan (*)())::'lambda'(JSONRPCRequest const&, UniValue&, bool)>::_M_invoke(std::_Any_data const&, JSONRPCRequest const&, UniValue&, bool&&) /usr/bin/../lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/std_function.h:285:9
      #15 0x558f756b8592 in std::function<bool (JSONRPCRequest const&, UniValue&, bool)>::operator()(JSONRPCRequest const&, UniValue&, bool) const /usr/bin/../lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/std_function.h:688:14
      #16 0x558f756b8592 in ExecuteCommand(CRPCCommand const&, JSONRPCRequest const&, UniValue&, bool) rpc/server.cpp:480:20
      #17 0x558f756b8592 in ExecuteCommands(std::vector<CRPCCommand const*, std::allocator<CRPCCommand const*> > const&, JSONRPCRequest const&, UniValue&) rpc/server.cpp:444:13
      #18 0x558f756b8017 in CRPCTable::execute(JSONRPCRequest const&) const rpc/server.cpp:464:13
      #19 0x558f7552457a in (anonymous namespace)::RPCFuzzTestingSetup::CallRPC(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > const&) test/fuzz/rpc.cpp:50:25
      #20 0x558f7552457a in rpc_fuzz_target(Span<unsigned char const>) test/fuzz/rpc.cpp:354:28
      #21 0x558f7544cf0f in std::_Function_handler<void (Span<unsigned char const>), void (*)(Span<unsigned char const>)>::_M_invoke(std::_Any_data const&, Span<unsigned char const>&&) /usr/bin/../lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/std_function.h:300:2
      #22 0x558f75c05197 in std::function<void (Span<unsigned char const>)>::operator()(Span<unsigned char const>) const /usr/bin/../lib/gcc/x86_64-linux-gnu/9/../../../../include/c++/9/bits/std_function.h:688:14
      #23 0x558f75c05197 in LLVMFuzzerTestOneInput test/fuzz/fuzz.cpp:74:5
      #24 0x558f753d8073 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) fuzzer.o
      #25 0x558f753c1f72 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) fuzzer.o
      #26 0x558f753c7d6a in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) fuzzer.o
      #27 0x558f753f3a92 in main (/root/fuzz_dir/scratch/fuzz_gen/code/src/test/fuzz/fuzz+0x5a8a92)
      #28 0x7f4a3c7e00b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x270b2)
      #29 0x558f7539cc9d in _start (/root/fuzz_dir/scratch/fuzz_gen/code/src/test/fuzz/fuzz+0x551c9d)

ACKs for top commit:
  practicalswift:
    cr ACK fa2e614d16af84327adf1c02746d0f73e0f48111

Tree-SHA512: cfa120265261f0ad019b46c426b915c1c007806b37aecb27016ce780a0ddea5e6fc9b09065fd40684b11183dcd3bf543558d7a655e604695021653540266baf7
2023-07-25 10:45:09 -05:00
MarcoFalke
aec81cde46 (partial) Merge #21424: Net processing: Tidy up CNodeState ctor
6927933782acb9b158787e6f35debb916793f6b1 [net processing] Add ChainSyncTimeoutState default initializers (John Newbery)
55966e0cc03f0e380d21a9434b048d4d515b6729 [net processing] Remove CNodeState ctor body (John Newbery)

Pull request description:

  This addresses the two outstanding review comments from #21370.

ACKs for top commit:
  practicalswift:
    cr ACK 6927933782acb9b158787e6f35debb916793f6b1: patch looks correct
  hebasto:
    ACK 6927933782acb9b158787e6f35debb916793f6b1, I have reviewed the code and it looks OK, I agree it can be merged.

Tree-SHA512: b3ef5c8a096e447887df255406b3a760f01c73e2b942374595416b4b4031fc69b89cd93168c45040489d581f340b2a62d3fbabd207d4307f587c00a7a7daacd1
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