378aedc45248cea82d9a3e6dc1038d6828008a76 [net] Add cs_vSend lock annotations (John Newbery)
673254515a2f97e53dd8c7335c836b083ba7e31a [net] Move RecordBytesSent() call out of cs_vSend lock (John Newbery)
Pull request description:
RecordBytesSent() does not require cs_vSend to be locked, so reduce the scope of cs_vSend.
Also correctly annotate the CNode data members that are guarded by cs_vSend.
This is a simpler alternative to #19673.
ACKs for top commit:
jnewbery:
ok, reverting to commit 378aedc which has two ACKs already. Any style issues can be fixed up in future PRs.
troygiorshev:
ACK 378aedc45248cea82d9a3e6dc1038d6828008a76
theStack:
re-ACK 378aedc45248cea82d9a3e6dc1038d6828008a76
MarcoFalke:
review ACK 378aedc45248cea82d9a3e6dc1038d6828008a76 🔌
Tree-SHA512: e9cd6c472b7e1479120c1bf2d1c640cf6d18c7d589a5f9b7dfc4875e5790adaab403a7a1b945a47e79e7249a614b8583270e4549f89b22e8a9edb2e4818b0d07
## Issue being fixed or feature implemented
When building with `-Wreorder-ctor` capture, the build fails with
`error: field 'lastMNListForVotingKeys' will be initialized after field
'votedFundingYesTriggerHash'`
## What was done?
Moved down `votedFundingYesTriggerHash` to last position in
`CGovernanceManager`.
## How Has This Been Tested?
## 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
- [ ] 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)_
## 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>
0000a0c7e9e4e7c1afafe6ef75b7624f4c573190 Remove confusing and almost useless "unexpected version" warning (MarcoFalke)
Pull request description:
It is useless because it isn't displayed for most users:
* It isn't displayed in normal operation (because the validation debug category is disabled by default)
* It isn't displayed for users that sync up their nodes intermittently, e.g. once a day or once a week (because it is disabled for IBD)
* It is only displayed in the debug log (as opposed to the versionbits warning, which is displayed more prominently)
It is confusing because it doesn't have a use case:
Despite the above, if a user *did* see the warning, it would most likely be a false positive (like it has been in the past). Even if it wasn't, there is nothing they can do about it. The only thing they could do is to check for updates and hope that a fixed version is available. But why would the user be so scrupulously precise in enabling the warning and reading the log, but then fail to regularly check update channels for updated software?
ACKs for top commit:
practicalswift:
ACK 0000a0c7e9e4e7c1afafe6ef75b7624f4c573190
decryp2kanon:
ACK 0000a0c
LarryRuane:
ACK 0000a0c7e9e4e7c1afafe6ef75b7624f4c573190
Tree-SHA512: 16e069c84be6ab6034baeefdc515d0e5cdf560b2005d2faec5f989d45494bd16cfcb4ffca6a17211d9556ae44f9737a60a476c08b5c2bb5e1bd29724ecd6d5c1
e5faec65bd06a3b14175aca3040290f343bd6e9c doc: Fix doxygen comment silent merge conflict in descriptor.cpp (W. J. van der Laan)
Pull request description:
It looks like #21238 introduced a silent merge conflict in the documentation, which fails with `-Wdocumentation` in the CI.
(please merge only if CI passes)
ACKs for top commit:
ajtowns:
ACK e5faec65bd06a3b14175aca3040290f343bd6e9c -- fixed it for me
meshcollider:
ACK e5faec65bd06a3b14175aca3040290f343bd6e9c modulo CI
Tree-SHA512: b07d50fd12aa7c239a92aad8ef29f4e88583c3ce701ebedba7c426aac4981c79113adc4670b7d055ab9535a28bdc3f9a30e6ca1b1ed0d7b9a333a3d9c4b40d8a
4eca20d6f7d850492d331d89d1cdd77abb3c70c1 [doc] correct comment about ATMPW (glozow)
8fa74aeb5b96419c7d40b40f8e1e1269509278e2 [doc] correct comment in chainparams (glozow)
2f8272c2a4b6fa84c04dfeb4d751bb218f2d4c78 [doc] GetBestBlock() doesn't do nothing (gzhao408)
Pull request description:
Came across a few misleading comments, wanted to fix them
ACKs for top commit:
jnewbery:
ACK 4eca20d6f7
MarcoFalke:
ACK 4eca20d6f7d850492d331d89d1cdd77abb3c70c1
laanwj:
Code review ACK 4eca20d6f7d850492d331d89d1cdd77abb3c70c1
Tree-SHA512: 5bef1f1e7703f304128cf0eb8945e139e031580c99062bbbe15bf4db8443c2ba5a8c65844833132e6646c8980c678fc1d2ab0c63e17105585d583570ee350fd0
fab633d2dbfed1efcc3a02061685d56327ae51fd doc: Update fuzzing docs for afl-clang-lto (MarcoFalke)
Pull request description:
Update the docs to default to `afl-clang-lto`. The afl-gcc (and other afl legacy fuzz engines) are still supported, though discouraged.
ACKs for top commit:
fanquake:
ACK fab633d2dbfed1efcc3a02061685d56327ae51fd - seems to work for me. Compiled and ran some fuzzers using Clang 11 on Bionic. Set `llvm-config` so that `clang-11` would be used over `clang` (10).
jarolrod:
ACK fab633d2dbfed1efcc3a02061685d56327ae51fd, tested on Ubuntu Focal
Tree-SHA512: 3d1969c167bea45a9d691f3b757f51213d550c9c1b895bed1fcf3c2f7345791787cfb13c376291b94eb3181caf4ae3126f4d01c7cebda7b2bb1c40a1294e9a68
ebde946a527e50630df180c6565ea5bf8d2ab5aa [doc] Improve comment about protected peers (Amiti Uttarwar)
Pull request description:
The comment currently suggests a long-standing node would infrequently protect peers under normal circumstances. Clarify that we also protect peers that are synced to the same work as our chain tip. [Relevant check here](ee0dc02c6f/src/net_processing.cpp (L1997)).
ACKs for top commit:
Empact:
ACK ebde946a52
jnewbery:
ACK ebde946a527e50630df180c6565ea5bf8d2ab5aa
Tree-SHA512: 3692f4098e95f935d801e0ee6bbd3a7c9480e66ca070a7c68ba79c4fc2e62377f5d37080c7b6a7d15ab617aaf4d3df9b26abc4f1b090d572ba46fdd092a6a64a
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
6f2c4fd0775a9c45eacc4bab8f138528852fdf44 netinfo: add user help documentation (Jon Atack)
Pull request description:
This is the help doc commit of #20764 without the rest of the PR or anything new since the 0.21.0 branch-off in order to target giving users a -netinfo help doc for 0.21.
- to test the new help
```
$ ./src/bitcoin-cli -netinfo help
```
- to see the updated short help
```
$ ./src/bitcoin-cli -help | grep -A4 netinfo
```
<details><summary><code>-netinfo</code> help doc</summary><p>
```
$ ./src/bitcoin-cli -netinfo help
-netinfo level "help"
Returns a network peer connections dashboard with information from the remote server.
Under the hood, -netinfo fetches the data by calling getpeerinfo and getnetworkinfo.
An optional integer argument from 0 to 4 can be passed for different peers listings.
Pass "help" to see this detailed help documentation.
If more than one argument is passed, only the first one is read and parsed.
Suggestion: use with the Linux watch(1) command for a live dashboard; see example below.
Arguments:
1. level (integer 0-4, optional) Specify the info level of the peers dashboard (default 0):
0 - Connection counts and local addresses
1 - Like 0 but with a peers listing (without address or version columns)
2 - Like 1 but with an address column
3 - Like 1 but with a version column
4 - Like 1 but with both address and version columns
2. help (string "help", optional) Print this help documentation instead of the dashboard.
Result:
* The peers listing in levels 1-4 displays all of the peers sorted by direction and minimum ping time:
Column Description
------ -----------
<-> Direction
"in" - inbound connections are those initiated by the peer
"out" - outbound connections are those initiated by us
type Type of peer connection
"full" - full relay, the default
"block" - block relay; like full relay but does not relay transactions or addresses
net Network the peer connected through ("ipv4", "ipv6", "onion", "i2p", or "cjdns")
mping Minimum observed ping time, in milliseconds (ms)
ping Last observed ping time, in milliseconds (ms)
send Time since last message sent to the peer, in seconds
recv Time since last message received from the peer, in seconds
txn Time since last novel transaction received from the peer and accepted into our mempool, in minutes
blk Time since last novel block passing initial validity checks received from the peer, in minutes
age Duration of connection to the peer, in minutes
asmap Mapped AS (Autonomous System) number in the BGP route to the peer, used for diversifying
peer selection (only displayed if the -asmap config option is set)
id Peer index, in increasing order of peer connections since node startup
address IP address and port of the peer
version Peer version and subversion concatenated, e.g. "70016/Satoshi:21.0.0/"
* The connection counts table displays the number of peers by direction, network, and the totals
for each, as well as a column for block relay peers.
* The local addresses table lists each local address broadcast by the node, the port, and the score.
Examples:
Connection counts and local addresses only
> bitcoin-cli -netinfo
Compact peers listing
> bitcoin-cli -netinfo 1
Full dashboard
> bitcoin-cli -netinfo 4
Full live dashboard, adjust --interval or --no-title as needed (Linux)
> watch --interval 1 --no-title ./src/bitcoin-cli -netinfo 4
See this help
> bitcoin-cli -netinfo help
```
</p></details>
ACKs for top commit:
laanwj:
ACK 6f2c4fd0775a9c45eacc4bab8f138528852fdf44
Tree-SHA512: dd49b1ce65546dacfb8ba9f9d57de0eae55560fd05533cf26c0b5d6ec65bf1de789c3287e90a0e2f47707532fab2fe62919a4192a7ffd58ac8eec18293e9aaeb
7b6887e75aec7e0d5c25682c26943073b7a728e5 doc: Convert depends options list from html to markdown (Wladimir J. van der Laan)
Pull request description:
This makes it easier to read in `less`, which is important for install instructions.
Rendered: [before](7ef6b1c51d/depends (dependency-options)) - [after](d97042406f/depends/README.md (dependency-options))
ACKs for top commit:
jonatack:
Code review re-ACK 7b6887e75aec7e0d5c25682c26943073b7a728e5 per `git diff d970424 7b6887e`
hebasto:
re-ACK 7b6887e75aec7e0d5c25682c26943073b7a728e5
Tree-SHA512: 02970b2bb97d2e8fb2d66470f6d70662653fda176bf6f4861742823b361fdc7ab6a2b44143480ac1a525b8d7808b6a068e8b3677dbba16cd783b4cab90470af5
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
3d552b0d788a7d3102396b32d0de08e57cbfd297 [doc] explain why CheckBlock() is called before AcceptBlock() (Sjors Provoost)
Pull request description:
Based on https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2019-February/016697.html and its PDF attachment.
ACKs for top commit:
MarcoFalke:
cr ACK 3d552b0d788a7d3102396b32d0de08e57cbfd297
Tree-SHA512: d1ef39855317853e0e7e051ec6015054d0d227fcdf20281c2c1921056537f1f79044aa1bdd35f46475edd17596fbcae79aeb338c4865b1269a01b158f6cb2ac4
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
8c09c0c1d18885ef94f79b3f2d073f43269bc95d fuzz: Avoid time-based "non-determinism" in fuzzing harnesses by using mocked GetTime() (practicalswift)
Pull request description:
Avoid time-based "non-determinism" in fuzzing harnesses by using mocked `GetTime()`.
Prior to this commit the fuzzing harnesses `banman`, `connman`, `net` and `rbf` had time-based "non-determinism". `addrman` is fixed in #20425. `process_message` and `process_messages` are left to fix: simply using mock time is not enough for them due to interaction with `IsInitialBlockDownload()`.
See [`doc/fuzzing.md`](https://github.com/bitcoin/bitcoin/blob/master/doc/fuzzing.md) for information on how to fuzz Bitcoin Core. Don't forget to contribute any coverage increasing inputs you find to the [Bitcoin Core fuzzing corpus repo](https://github.com/bitcoin-core/qa-assets).
Happy fuzzing :)
ACKs for top commit:
MarcoFalke:
review ACK 8c09c0c1d18885ef94f79b3f2d073f43269bc95d
practicalswift:
> review ACK [8c09c0c](8c09c0c1d1)
Tree-SHA512: 32dfbead3dfd18cf4ff56dc2ea341aa977441b4e19a54879cf54fa5820c7e2b14b92c7e238d32fd785654f3b28cc82826ae66c03e94c292633c63c41196ba9a8
fabce459bb44e90dc7ae9c44eeedab707435af5b fuzz: version handshake (MarcoFalke)
Pull request description:
Not fuzzing the version handshake will limit fuzz coverage
ACKs for top commit:
practicalswift:
cr ACK fabce459bb44e90dc7ae9c44eeedab707435af5b: patch looks very much correct
Tree-SHA512: 4091d27d39edee781d033e471b352084bb54df250d0890e4821a325926a44dff9b26a2614d67dd0529f73bd366b075d7a0a1a570c2837de286a1b93a59a8fb91
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
5e531e6beb5381c0be5efaa24b7e423e593568e4 assumptions: check C++17 assumption with MSVC (fanquake)
c7b46489f8c4d880382248fb47266d81948bbce0 assumptions: assume a C++17 compiler (fanquake)
Pull request description:
This has been the case since #20413.
This should also enable the check for MSVC. From my reading of https://docs.microsoft.com/en-us/cpp/build/reference/zc-cplusplus?view=msvc-160 and https://devblogs.microsoft.com/cppblog/msvc-now-correctly-reports-__cplusplus/ if we set the `/Zc:__cplusplus` switch in additional options, MSVC will report the correct value for `__cplusplus`. However I have not tested this.
ACKs for top commit:
laanwj:
Code review ACK 5e531e6beb5381c0be5efaa24b7e423e593568e4
hebasto:
ACK 5e531e6beb5381c0be5efaa24b7e423e593568e4, checked the MS docs, and AppVeyor build is green.
practicalswift:
ACK 5e531e6beb5381c0be5efaa24b7e423e593568e4
Tree-SHA512: a4fb525cf5c33abc944c614edb0313a39c8a39a1637a03c09342c15ba0925f4eb037062e65e51b42ade667506b7e554c7159acf86e6b8c35d0a87dd79a6f239b
77772a1b809e443a6861ee49009ff8bc55cff9c3 doc: Rework internal and external links (MarcoFalke)
Pull request description:
Some minor changes:
* Move Bitcoin Core download link to the very top. *Reason:* The download link has nothing to do with the section that explains Bitcoin. Also, anyone quickly looking for the download link will find it faster.
* Add a new link to the doc folder. *Reason*: Apart from the documentation that is shipped with the binary software, the doc folder is the primary location for Bitcoin Core related documentation.
* Remove dead link to pdf. *Reason*: The pdf can be trivially found by asking a search engine.
* Remove reference to "build server". *Reason*: There is no "build server". The CI system is explained in the next sentence in detail.
* Remove dead? link to translation mailing list. *Reason*: The translation process is explained in `doc/translation_process.md`, no need to explain it in detail in the main readme.
ACKs for top commit:
laanwj:
ACK 77772a1b809e443a6861ee49009ff8bc55cff9c3
RiccardoMasutti:
ACK 77772a1
Tree-SHA512: 365824c6da519892ff239842b0c9525f559513eb800afd77b14febdc9d1a79294d9448df6cd25b623f1734ff859d9e20951e78fb8091d19fdadde0dd24eeddd4
ae98aec9c0521cdcec76459c8200bd45ff6a1485 refactor: Make CAddrMan::cs non-recursive (Hennadii Stepanov)
f5d1c7fac70f424114dae3be270fdc31589a8c34 Add AssertLockHeld to CAddrMan private functions (Hennadii Stepanov)
5ef1d0b6982f05f70ff2164ab9af1ac1d2f97f5d Add thread safety annotations to CAddrMan public functions (Hennadii Stepanov)
b138973a8b4bbe061ad97011f278a21e08ea79e6 refactor: Avoid recursive locking in CAddrMan::Clear (Hennadii Stepanov)
f79a664314b88941c1a2796623e846d0a5916c06 refactor: Apply consistent pattern for CAddrMan::Check usage (Hennadii Stepanov)
187b7d2bb36e6de9cd960378021ebe690619a2ef refactor: Avoid recursive locking in CAddrMan::Check (Hennadii Stepanov)
f77d9c79aa41dab4285e95c9432cc6d853be67a3 refactor: Fix CAddrMan::Check style (Hennadii Stepanov)
06703973c758c2c5d0ff916993aa7055f609d2d7 Make CAddrMan::Check private (Hennadii Stepanov)
efc6fac951e75ba913350bb470c3d4e6a4e284b9 refactor: Avoid recursive locking in CAddrMan::size (Hennadii Stepanov)
2da95545ea42f925dbc7703e42e9356908a8c83e test: Drop excessive locking in CAddrManTest::SimConnFail (Hennadii Stepanov)
Pull request description:
This PR replaces `RecursiveMutex CAddrMan::cs` with `Mutex CAddrMan::cs`.
All of the related code branches are covered by appropriate lock assertions to insure that the mutex locking policy has not been changed by accident.
Related to #19303.
Based on #22025, and first three commits belong to it.
ACKs for top commit:
vasild:
ACK ae98aec9c0521cdcec76459c8200bd45ff6a1485
Tree-SHA512: c3a2d3d955a75befd7e497a802b8c10730e393be9111ca263ad0464d32fae6c7edf9bd173ffb6bc9bb61c4b39073a74eba12979d47f26b0b7b4a861d100942df
3e68efa615968e0c9d68a7f197c7852478f6be78 [net] Move checks from GetLocalAddrForPeer to caller (John Newbery)
d21d2b264cd77c027a06f68289cf4c3f177d1ed0 [net] Change AdvertiseLocal to GetLocalAddrForPeer (John Newbery)
Pull request description:
This is the first part of #21186. It slightly disentangles addr handling in net/net_processing by making it explicit that net_processing is responsible for pushing addr records into `vAddrToSend`.
ACKs for top commit:
MarcoFalke:
re-ACK 3e68efa615968e0c9d68a7f197c7852478f6be78 🍅
Tree-SHA512: 9af50c41f5a977e2e277f24a589db38e2980b353401def5e74b108ac5f493d9b5d6b1b8bf15323a4d66321495f04bc271450fcef7aa7d1c095f051a4f8e9b15f
## Issue being fixed or feature implemented
Unlike bitcoin we are using PREVIOUS block in `GetBlockSubsidy()`.
That creates special case for genesis block, because it doesn't have
previous block. In this special case instead of calling
`GetBlockSubsidy` should be used pre-calculated value. To avoid
confusion for new code and simplify implementation, there's introduced a
new method `GetBlockSubsidyPrev` that has other interface: it takes
pointer `CBlockIndex* prev` in agruments instead pair of height + nbits.
These changes are follow-up for #5501
## What was done?
Implemented new method `GetBlockSubsidyPrev()` and used instead of
`GetBlockSubsidy` when it is more convenient.
## 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
- [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>
## Issue being fixed or feature implemented
Make CI builds faster.
We have this
https://github.com/dashpay/dash/blob/develop/ci/dash/build_src.sh#L18
but by that time `CCACHE_SIZE` is set to 100M via
https://github.com/dashpay/dash/blob/develop/ci/test/00_setup_env.sh#L51
already.
## What was done?
set `CCACHE_SIZE` variable in `.gitlab-ci.yml`, drop confusing and
useless lines from `ci/dash/build_src.sh` (`CCACHE_SIZE` and
`CCACHE_COMPRESS` defaults are handled by `ci/test/00_setup_env.sh`)
## How Has This Been Tested?
I set `CCACHE_SIZE` to 400M yesterday via Gitlab UI to test it.
results:
before, trivial doc change, 100M:
https://gitlab.com/dashpay/dash/-/jobs/4931233566#L59 (~17 minutes)
after, trivial code change, 400M:
https://gitlab.com/dashpay/dash/-/jobs/4935764148#L80 (~12 minutes)
Removed it from Gitlab UI now but it should still say `Set cache size
limit to 400.0 MB` in logs for this PR.
## 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)_
## Issue being fixed or feature implemented
fix buid errors like https://gitlab.com/dashpay/dash/-/jobs/4933232262
## What was done?
reorder initializations
## How Has This Been Tested?
local build with `-werror`
## 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)_
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
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
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
## Issue being fixed or feature implemented
The order of members in a class/struct definition and the order of their
initialization should match. This ensures that the code is more
error-proof in cases where the order of member initializations is
important, as they may depend on each other.
Instead manual checking of member initialization better let CI handle
it.
Last PR where it's noticed:
https://github.com/dashpay/dash/pull/5531#discussion_r1299404387
## What was done?
New flag "-Werror=reorder" for `configure.ac` and fixes existing code.
## How Has This Been Tested?
Build code with `--enable-werror`
## 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
## Issue being fixed or feature implemented
Should make debugging CoinJoin in multi-wallet use cases a bit easier
## What was done?
Borrowed the idea from `WalletLogPrintf`
## How Has This Been Tested?
Run local node, looks like this (for a wallet named `mixing`)
```
2023-08-08T15:11:06Z [mixing] CCoinJoinClientManager::CheckAutomaticBackup -- Keys left since latest backup: 882
```
etc.
## 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
## Issue being fixed or feature implemented
We only lock them on node load atm.
Fixes#5535
## What was done?
## How Has This Been Tested?
close and open a wallet with a mn collateral
## 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)_
## Issue being fixed or feature implemented
`-1` will only mean `not initialized` from now
Should fix crashes like
https://gitlab.com/dashpay/dash/-/jobs/4893359925
This fix applied on top of #5525https://gitlab.com/UdjinM6/dash/-/pipelines/972154075
## What was done?
Introduce and use `m_initial_snapshot_index` instead of re-using
`nHeight`. Added a couple of asserts to make sure:
1. we never create mn lists with `nHeight` set to `-1` _explicitly_ (but
it's ok for ctor with no params to do so)
2. we never set `nHeight` to `-1` for an existing mn list
3. we never try to get a height for a non-initialized list
4. `GetListForBlockInternal` never returns non-initialized mn lists
## How Has This Been Tested?
Run tests, run regtest/testnet wallets.
## Breaking Changes
We never stored snapshots with `nHeight == -1`, should be no breaking
changes I think.
## 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)_
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
This data structure provide efficient storage for numbers if amount of gaps between them is not too big
It works similarly to CSkipSet but amount of gaps now in unlimited
## 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>
## Issue being fixed or feature implemented
`JoinExistingQueue` was tweaked for regtest/devnets in #4394 but we have
"skipping winners" logic in `StartNewQueue` too. We should also use
weighted count when checking "skip winners" conditions.
## What was done?
Add a helper to calculate the number and use it in both methods. Adjust
logic.
## How Has This Been Tested?
Running a local mixing node on devnet ~- no "skipping winners" in logs
anymore.~ and 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
## 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
fa2630328687645fbc7dd1ea46aac32514025715 fuzz: Check that NULL_DATA is unspendable (MarcoFalke)
Pull request description:
* Every script of type NULL_DATA must be unspendable
* The only know types of unspendable scripts are NULL_DATA and certain NONSTANDARD scripts
ACKs for top commit:
sipa:
utACK fa2630328687645fbc7dd1ea46aac32514025715
Tree-SHA512: 8297fbacf32b4868b12accc1c052d352d02d96540a1fc883de9d04a3df8734116deecc33046495c9a3af6d79fec7f8d63afbfa5e401a2ca8d7c70f0f13735c0d
