0b8fe48fbe refactor: add cs_mapSocketToNode and cs_sendable_receivable_nodes to minimize cs_vNode contention (and document an undocumented lock requirement) (pasta)
Pull request description:
## Issue being fixed or feature implemented
Reduce locking contention on cs_vNode
## What was done?
added new mutex
## How Has This Been Tested?
Building running locally on testnet
## Breaking Changes
None
## Checklist:
- [ ] 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
Top commit has no ACKs.
Tree-SHA512: b7a3aa8078cf7f2b5f9a833a345e405351a927fa517993d77cefa77c9d109c5d92e06b6d3f91093b0fa4d91c4a464f19ce4eb457e486f9532ce890f02b9b99e1
e89c555b0f merge bitcoin#22107: rename GetSystemTimeInSeconds to GetTimeSeconds (Kittywhiskers Van Gogh)
2e55327f55 net: introduce CanRelayAddrs as RelayAddrsWithConn substitute (Kittywhiskers Van Gogh)
5478001a81 partial bitcoin#21186: Move addr data into net_processing (Kittywhiskers Van Gogh)
26c39f5b92 net: replace RelayAddrsWithConn check with !IsBlockOnlyConn (Kittywhiskers Van Gogh)
4844e729e2 merge bitcoin#21506: make NetPermissionFlags an enum class (Kittywhiskers Van Gogh)
03ab144b8f merge bitcoin#21785: Fix intermittent issue in p2p_addr_relay.py (Kittywhiskers Van Gogh)
6d27db58d1 merge bitcoin#21707: Extend functional tests for addr relay (Kittywhiskers Van Gogh)
39384ba461 merge bitcoin#21198: Address outstanding review comments from PR20721 (Kittywhiskers Van Gogh)
d34d2c4efb merge bitcoin#21236: Extract addr send functionality into MaybeSendAddr() (Kittywhiskers Van Gogh)
ba1df91d8d merge bitcoin#21425: Pass PeerManagerImpl members only once (Kittywhiskers Van Gogh)
5c4c7c55f8 merge bitcoin#19771: Replace enum CConnMan::NumConnections with enum class ConnectionDirection (Kittywhiskers Van Gogh)
62a7311fe4 merge bitcoin#21015: Make all of net_processing (and some of net) use std::chrono types (Kittywhiskers Van Gogh)
8b204c4c82 merge bitcoin-core/gui#226: Add "Last Block" and "Last Tx" rows to peer details area (Kittywhiskers Van Gogh)
3e8ba24c87 partial bitcoin-core/gui#206: Display fRelayTxes and bip152_highbandwidth_{to, from} in peer details (Kittywhiskers Van Gogh)
e109c0042a merge bitcoin#20646: refer to BIPs 339/155 in feature negotiation (Kittywhiskers Van Gogh)
1d4f10a378 merge bitcoin#19315: Allow outbound & block-relay-only connections in functional tests (Kittywhiskers Van Gogh)
b76e029e44 merge bitcoin#20756: Add missing field (permissions) to the getpeerinfo help (Kittywhiskers Van Gogh)
d0c596e91d merge bitcoin#20653: Move addr relay comment in net to correct place (Kittywhiskers Van Gogh)
017d1b40e3 merge bitcoin#19763: don't relay to the address' originator (Kittywhiskers Van Gogh)
Pull request description:
## Additional Information
* [bitcoin#19763](https://github.com/bitcoin/bitcoin/pull/19763) doesn't play nice on its own, `p2p_addr_relay.py` fails because of `assert_equal(total_ipv4_received, num_ipv4_addrs * ipv4_branching_factor)` where `not(0 == 20)`. It only works with [bitcoin#21707](https://github.com/bitcoin/bitcoin/pull/21707).
* In the GUI/Qt wallet, the peer information tab in the debug window has three fields added to them, "Wants Tx Relay", "Last Block" and "Last Tx".
* The RPC help text for `getpeerinfo` now includes help text for the `permissions` return value and a debug RPC called `addconnection` has been introduced.
## Breaking Changes
None observed.
## Checklist:
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas **(note: N/A)**
- [x] I have added or updated relevant unit/integration/functional/e2e tests
- [x] I have made corresponding changes to the documentation **(note: N/A)**
- [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_
ACKs for top commit:
PastaPastaPasta:
utACK e89c555
Tree-SHA512: 2be14351c22852b1318ded496fea60536db40659ab14e71bde02082935120bfb9df275790a4cb3170f54bcb4b2fb4f44096dd0e7cc703a705127ee294be39894
afbae06520 fix: check if message can be handled before attempting to deserialize (thephez)
Pull request description:
## Issue being fixed or feature implemented
Currently `message-capture-parser.py` crashes when encountering certain messages (e.g. mnauth). This at least makes it possible to run the script without crashing. There may be better options for solving this.
## What was done?
Check if the dictionary is going to return `None` before we attempt to do something further with it. Hide whitespace changes to see the few lines that were added: https://github.com/dashpay/dash/pull/5927/files?diff=unified&w=1
## How Has This Been Tested?
Running script 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
- [ ] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_
Top commit has no ACKs.
Tree-SHA512: 041af57afcfd1d93487fd41d34a50e3a99f7fa129563dfe1e1cf2498974c8e658bd6acb9c810887c841160074056ba999e9b6607ac9336b98b9d42806682c607
7f57516d04 docs: add v20.0.4 release notes (pasta)
Pull request description:
## Issue being fixed or feature implemented
We didn't archive v20.0.4 release notes somehow in #5910
## What was done?
Archive 20.0.4 release notes
## 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)_
Top commit has no ACKs.
Tree-SHA512: d703dedf1aad2afb5eb8af291f89e38fa2dfb4185ef9e25194c273bb9f04ae8075ea87318552b468b1a487b39b95cf1dee794d3d3c5b8a863dbfd6d458ee8e72
7f57516d04 docs: add v20.0.4 release notes (pasta)
Pull request description:
## Issue being fixed or feature implemented
We didn't archive v20.0.4 release notes somehow in #5910
## What was done?
Archive 20.0.4 release notes
## 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)_
Top commit has no ACKs.
Tree-SHA512: d703dedf1aad2afb5eb8af291f89e38fa2dfb4185ef9e25194c273bb9f04ae8075ea87318552b468b1a487b39b95cf1dee794d3d3c5b8a863dbfd6d458ee8e72
Since bitcoin#21186, mutual exclusivity is not a given (i.e.
RelayAddrsWithConn != !IsBlockOnlyConn), we should use RelayPeersWithConn
for a definitive answer and since relying on a no-longer-true property
breaks InstantSend, let's fetch the right answer instead.
Dash uses a lot more CNode::RelayAddrsWithConn checks than Bitcoin (esp.
since a483122f (#4888)), so bitcoin#21186 will not adequately cover the
removal of RelayAddrsWithConn usages.
When possible to query with RelayAddrsWithPeer, that should be used, as
that value is the most reliable, else we rely on the former mutual
exclusivity of IsBlockOnlyConn and RelayAddrsWithConn to fill in the
blanks where a more reliable query isn't available.
Note: To prevent builds from breaking, a change has been made in
InstantSend code despite it breaking functionality. A commit later will
repair it by creating a way to access RelayAddrsWithPeer.
comment was moved to net.h in 678df631 (#4888) and removed entirely in
796353ad (#5771). the comment is being restored back to where it is
upstream, in CNode::RelayAddrsWithConn.
ded1b5a3df fix: deadlock over cs_main and contributionsCacheCs in dkssessionmgr (Konstantin Akimov)
Pull request description:
## Issue being fixed or feature implemented
**It fixes rpc failure: "Work queue depth exceeded"**
As I checked on running `dashd` in deadlock condition:
Thread 78 is a thread that keep `cs_main`:
```
#14 0x0000aaaad1f8d604 in BuildSimplifiedMNListDiff () at evo/simplifiedmns.cpp:364
```
but it is locked by `contributionsCacheCs`
```
#8 llmq::CDKGSessionManager::GetVerifiedContributions () at llmq/dkgsessionmgr.cpp:392
```
On other hand, `contributionsCacheCs` is blocked by Thread 59
```
#17 0x0000aaaad1ba1940 in llmq::CDKGSessionManager::GetVerifiedContributions () at llmq/dkgsessionmgr.cpp:393
```
and it makes circuit lock by waiting `cs_main` in
```
#9 ReadBlockFromDisk () at node/blockstorage.cpp:75
```
See https://github.com/dashpay/dash-issues/issues/69 for more details
Seems introduced there: dashpay/dash#3911
## What was done?
Deadlock is removed by reducing scope of mutex
## How Has This Been Tested?
I reviewed 2 different servers which have status `work queue exceeded`, both have same deadlock, so, this patch should fix this issue. Once this fix is merged, we can test it on testnet.
## 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
Top commit has no ACKs.
Tree-SHA512: 4fe5c03c464ee6934fb927b897f007b65a8995723196edaffdae067edee7067da151130d4c4bac47d3418fdad5c8e130682f42d7ef9c044380a8c8fff78ee008
069282611c refactor: make CActiveMasternodeManager::cs SharedMutex and private (pasta)
663774c544 feat: implement Read Write Locks in threading (pasta)
Pull request description:
## Issue being fixed or feature implemented
We have some caches or other information in codebase which are read from a lot; but rarely written to. We can use a RW lock here instead of a normal Mutex
## What was done?
Implement a RW lock and use them
## How Has This Been Tested?
Hasn't been much; looking for review atm. Maybe should deploy this on testnet for a bit and make sure it doesn't break.
## Breaking Changes
## 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
- [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)_
ACKs for top commit:
knst:
utACK 069282611c
Tree-SHA512: a9759d4904580eebb5ddf9e05d3d54cf4b0b0db971f09d2f4cb093fddc0a13094998ef2af301de581fd64dc1235df80bace7f701ab437c2ecfa663b4fc6e25ed
a47635baad feat: drop symbol check from CI to unify with bitcoin, keep it for qt5 only (Konstantin Akimov)
0c38cc325e fix: drop -static-libstc++ from depends/hosts/{linux,mingw32}.mk which is clang-only (Konstantin Akimov)
14a67ee85e Merge #20182: ci: Build with --enable-werror by default, and document exceptions (MarcoFalke)
Pull request description:
## Issue being fixed or feature implemented
As discovered in #5957:
Jobs tsan/ubsan fails with backport bitcoin#20182
it fails, because both tsan and ubsan jobs use clang but all other jobs use gcc. Somehow, after configure there are set incorrect options:
```
clang++-16 -std=c++17 -c -pipe -static-libstdc++ -O1 -Werror -D_GLIBCXX_DEBUG -D_GLIBCXX_DEBUG_PEDANTIC -D_LIBCPP_DEBUG=1 -I/builds/dashpay/dash/depends/x86_64-pc-linux-gnu/include/ conftest.cpp
```
`clang` doesn't support `-static-llibstdc++` which is supposed to be gcc-only, it cause this failure:
```
clang: error: argument unused during compilation: '-static-libstdc++' [-Werror,-Wunused-command-line-argument]
```
This failure make `autoconf` to think that clang doesn't support `-Werror` and fails with this error. So, you can't activate this flag.
## What was done?
Backport bitcoin#20182 and related fixes to make it works
## How Has This Been Tested?
CI now succeed with bitcoin#20182. It means, that -Werror activated for clang; also there are not warnings such as:
```
clang: error: argument unused during compilation: '-static-libstdc++' [-Werror,-Wunused-command-line-argument]
```
https://gitlab.com/dashpay/dash/-/jobs/6494328698
## 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
Top commit has no ACKs.
Tree-SHA512: f912824eaa1ec7513cda7278a3df9e067b0ab48a2d174b18654c8070aa6544bac33a52f494c1e35b4eab10392c1f26df4663e21d12a4dfff7c0a4a6a01ff9551
c8653387b1 Merge #21112: ci: use Focal for macOS cross builds (MarcoFalke)
1995d2e7ca Merge #20451: lint: run mypy over contrib/devtools (Wladimir J. van der Laan)
eba325d7a2 Merge #16551: test: Test that low difficulty chain fork is rejected (MarcoFalke)
b193c63fed Merge #20611: Move TX_MAX_STANDARD_VERSION to policy (Wladimir J. van der Laan)
41505e64aa Merge #20588: Remove unused and confusing CTransaction constructor (fanquake)
d186b3714a Merge #20587: [doc] Tidy up Tor doc (more stringent) (Wladimir J. van der Laan)
def356e2cb Merge #20512: doc: Add bash as an OpenBSD dependency (Jonas Schnelli)
d01973cc08 Merge #20491: refactor: Drop noop gcc version checks (fanquake)
455bb2e117 Merge #20329: docs/descriptors.md: Remove hardened marker in the path after xpub (MarcoFalke)
9eec4cc2e1 Merge #20288: script, doc: contrib/seeds updates (Wladimir J. van der Laan)
Pull request description:
## Issue being fixed or feature implemented
Just regular backports from v19 and v22
## What was done?
- bitcoin/bitcoin#20288
- bitcoin/bitcoin#20329
- bitcoin/bitcoin#20491
- bitcoin/bitcoin#20512
- bitcoin/bitcoin#20587
- bitcoin/bitcoin#20588
- bitcoin/bitcoin#20611
- bitcoin/bitcoin#16551
- bitcoin/bitcoin#20451
- bitcoin/bitcoin#21112
## 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
ACKs for top commit:
PastaPastaPasta:
utACK c8653387b1
Tree-SHA512: 38e747a82091874f8cb87f35dc99aa8498e798f3818525c3fe99b2ed454fd8490d710c6eb7c3a0840105bb126503d301262d2f473a2502c66de8c32c8de94922
2f6fe4e4e9e9e35e713c0a20cf891b023592110a ci: Build with --enable-werror by default, and document exceptions (Hennadii Stepanov)
Pull request description:
This PR prevents introducing of new compiler warnings in the master branch, e.g., #19986, #20162.
ACKs for top commit:
practicalswift:
cr ACK 2f6fe4e4e9e9e35e713c0a20cf891b023592110a: patch looks correct
MarcoFalke:
re-ACK 2f6fe4e4e9 🏏
vasild:
ACK 2f6fe4e
Tree-SHA512: 23b5feb5bc472658c992d882ef61af23496f25adaa19f9c79bfaef5d2db273d44981aa93b1631a7d37cb58755283c1dacf3f2d68e501522d3fa8c965ab646d19
ac24af453d16afd1993fa1f292aa41ae6b16f138 ci: use Ubuntu Focal for macOS cross build (fanquake)
Pull request description:
I had assumed Cirrus was spinning up Docker containers to run the CI,
however we are actaully running on the Cirrus machines themselves. See
`DANGER_RUN_CI_ON_HOST` and in the logs:
```bash
Running on host system without docker wrapper
```
So with this change we will actually be using Focal for the macOS cross build.
Follow up to #21036.
This originally contained Windows changes, and an attempt to get Cirrus running without `DANGER_RUN_CI_ON_HOST`, however that seems non-trival, so Windows changes have been dropped from here for now.
ACKs for top commit:
MarcoFalke:
cr ACK ac24af453d16afd1993fa1f292aa41ae6b16f138
Tree-SHA512: 587ba5acf741bcefecf1bc262fa1177f565ebfa9de56125eca19ed3c7db7b9aabfb96866e9c140681b88cb7015a3ded2bc6b4b1b235543d6f6e9dfc6984d569f
1ef2138c0db3bd4f9332c777fa3fb2770dc1b08c lint: run mypy over contrib/devtools (fanquake)
Pull request description:
wumpus mentioned on IRC that we don't currently run `mypy` over the `contrib/devtools` directory, and that it would likely be worthwhile given #20434. This just adds that dir to the linter, as well as some missing annotations to fix existing errors. Note that now we require Python 3.6 we can make use of variable annotations.
master (patched to check contrib devtools):
```bash
test/lint/lint-python.sh
contrib/devtools/symbol-check.py:154: error: Incompatible types in assignment (expression has type "List[str]", variable has type "str")
contrib/devtools/circular-dependencies.py:35: error: Need type annotation for 'deps' (hint: "deps: Dict[<type>, <type>] = ...")
contrib/devtools/circular-dependencies.py:67: error: Need type annotation for 'closure' (hint: "closure: Dict[<type>, <type>] = ...")
Found 4 errors in 3 files (checked 187 source files)
```
I haven't quite gone as far as to add annotations like
```python
CHECKS: Dict[str, List[Tuple[str, Callable[[Any], bool]]]] = {...
```
to `symbol-check.py`.
ACKs for top commit:
laanwj:
ACK 1ef2138c0db3bd4f9332c777fa3fb2770dc1b08c
Tree-SHA512: a58c2ece588c640289dc1d35dad5b1b8732788272daa0965d6bf44ee8a7f7c8e8585f94d233ac41c84b9ffcfc97841a00fe2c9acba41f58fd164f01de4b6512b
Backport notice:
- data have real blocks from testnet
- due to big gap in blocks before checkpoints (half-year) for sack of this test is added one more checkpoint, otherwise blocks are not accepted due to non-mockable time for other chains except regtest
- data for 2 blocks in split chain are generated locally for testnet
--------------
333317ce6b67aa92f7363d48cd750712190b4b6b test: Test that low difficulty chain fork is rejected (MarcoFalke)
fa31dc1bf4ee471c4641eef8de02702ba0619ae7 test: Pass down correct chain name in tests (MarcoFalke)
Pull request description:
To prevent OOM, Bitcoin Core will reject chain forks at low difficulty by default. This is the only use-case of checkpoints, so add a test for it to make sure the feature works as expected. If it didn't work, checkpoints would have no use-case and we might as well remove them
ACKs for top commit:
Sjors:
Thanks for adding the node 1 example. Code review ACK 333317c
Tree-SHA512: 90dffa540d0904f3cffb61d2382b1a26f84fe9560b7013e4461546383add31a8757b350616a6d43217c59ef7b8b2a1b62bb3bab582c679cbb2c660a782ce7be1
fade6195b1c230edd561443637a7bde81c2594a4 Move TX_MAX_STANDARD_VERSION to policy (MarcoFalke)
Pull request description:
`primitives` should only be used for the raw datastructures (parsing and format). It is not the right place to document relay policy.
ACKs for top commit:
laanwj:
Code review ACK fade6195b1c230edd561443637a7bde81c2594a4
lontivero:
Concept ACK fade6195b1
Tree-SHA512: f809c4aecd14d7e9feaa7b50b9c0697232991eef36190cd960bcfb0ad6e20c71a4f6aab48c7747cf8a681eb14feda60c55b09a37f128673d519567224f29cd97
fac39c198324715565897f4240709340477af0bf wallet: document that tx in CreateTransaction is purely an out-param (MarcoFalke)
faac31521bb7ecbf999541cf918d3750ff589de4 Remove unused and confusing CTransaction constructor (MarcoFalke)
Pull request description:
The constructor is confusing and dangerous (as explained in the TODO), fix that by removing it.
ACKs for top commit:
laanwj:
Code review ACK fac39c198324715565897f4240709340477af0bf
promag:
Code review ACK fac39c198324715565897f4240709340477af0bf.
theStack:
Code review ACK fac39c198324715565897f4240709340477af0bf
Tree-SHA512: e0c8cffce8d8ee0166b8e1cbfe85ed0657611e26e2af0d69fde70eceaa5d75cbde3eb489af0428fe4fc431360b4c791fb1cc21b8dee7d4c7a4f17df00836229d
32045bbfd5d77513efc162be8d4e24ea67539e27 [doc] Tidy up Tor doc (more stringent) (wodry)
Pull request description:
This is a follow up to https://github.com/bitcoin/bitcoin/pull/19638 that left two deprecated "hidden service/server" naming occurences.
It also shall make the chapter titles regarding creation of onion services stringent and easy to read and distinguish.
It removes the one and only reference to the testnet (here the testnet onion service port), as it is not explained that it references to the testnet and I do not know why it is mentioned there. It is only confusing. Also, as said, the testnet is not referenced at any other place in this document.
ACKs for top commit:
practicalswift:
ACK 32045bbfd5d77513efc162be8d4e24ea67539e27
laanwj:
Review ACK 32045bbfd5d77513efc162be8d4e24ea67539e27
RiccardoMasutti:
ACK 32045bb
Tree-SHA512: c623387b76d68845c0fa47f67a6f8ef70c9c560e3f8f8754e45a4f51e43198c2092be789588acd4ada607f42fbe62d51a4b1888d81b225df19b6557a081835c0
1d578c078f0ce00cb032d3c6c689fd199b8d2f35 doc: Add bash as an OpenBSD dependency (Emil Engler)
Pull request description:
If we require Python for the test framework, we should also require
bash. It is required for the linters and other scripts and does not
comes in a default OpenBSD installation.
ACKs for top commit:
practicalswift:
ACK 1d578c078f0ce00cb032d3c6c689fd199b8d2f35
RiccardoMasutti:
ACK 1d578c0
theStack:
ACK 1d578c078f0ce00cb032d3c6c689fd199b8d2f35
Tree-SHA512: ef14e801983093a99d4c28d134adbc589ca06e5437bf1ff34f8e593968c59793e9d99e8a48f577f847acdfc5185e193276526894b4c632c59d66d87939977910
830ddf413934226d0b6ca99165916790cc52ca18 Drop noop gcc version checks (Hennadii Stepanov)
Pull request description:
Since #20413 the minimum required GCC version is 7.
ACKs for top commit:
fanquake:
ACK 830ddf413934226d0b6ca99165916790cc52ca18
Tree-SHA512: 36264661d6ced1683a0c907efba7c700502acaf8e9fd50d9066bc9c7b877b25165b0684c2d7fe74bd58e500a77d7702bdbdd53691c274f29e4abccd241c10964
dc80a7d0b0817b43d41af3a08b5800c425ebd5d8 docs/descriptors.md: Remove hardened marker in the path after xpub (Dmitry Petukhov)
Pull request description:
As described in "Key origin identification" section, a descriptor
that has hardened derivation after xpub does not let you compute scripts
without access to the corresponding private keys. Such a descriptor is
practically useless.
The text after the descriptor said "with child key *1'/2* of the
specified xpub", and clearly an xpub cannot have "child key" with
hardened derivation. Therefore it makes sense to fix this inconsistency
to not confuse the reader of the doc
Top commit has no ACKs.
Tree-SHA512: 9f35951d90868585303681c27ea2ef9d36d4036181e337cfbd48730de0c346320b08dd089350b116d4c8542f3b506868cf318796bb713e5f80600ccbd96f9970
961f148cb1b4caab86b4354357999e03433b04b1 doc: update contrib/seeds/README dnspython installation info (Jon Atack)
dd7b5f46d85401254630abf6976f59b5b8eed181 script: fix deprecation warning in makeseeds.py (Jon Atack)
Pull request description:
Seen while reviewing #20237.
1. Fix a deprecation warning in `contrib/seeds/makeseeds.py`
```
makeseeds.py:139: DeprecationWarning: please use dns.resolver.resolve() instead
asn = int([x.to_text() for x in dns.resolver.query('.'.join(
```
- Per https://dnspython.readthedocs.io/en/latest/whatsnew.html, `dns.resolver.query()` was deprecated in `dnspython` version 2.0.0.
- See https://dnspython.readthedocs.io/en/latest/resolver-class.html for more info on the resolver class.
2. Update the `dnspython` dependency installation instructions in `contrib/seeds/README`
- The markdown rendering can be seen here: https://github.com/jonatack/bitcoin/tree/contrib-seeds-fixups/contrib/seeds
ACKs for top commit:
laanwj:
code review ACK 961f148cb1b4caab86b4354357999e03433b04b1
Tree-SHA512: f9c4f318a1a0d35b8de147d24b72c534a1f58eece31e7cfa00b4149a63b6a618d8ca0312f52fd8056f3c645cf2ee68574ca02319fddffdad919a70cd33395d33
436a5783c7 Merge bitcoin/bitcoin#22736: log, sync: change lock contention from preprocessor directive to log category (MarcoFalke)
Pull request description:
7e698732836121912f179b7c743a72dd6fdffa72 sync: remove DEBUG_LOCKCONTENTION preprocessor directives (Jon Atack)
9b08006bc502e67956d6ab518388fad6397cac8d log, sync: improve lock contention logging and add time duration (Jon Atack)
3f4c6b87f1098436693c4990f2082515ec0ece26 log, timer: add timing macro in usec LOG_TIME_MICROS_WITH_CATEGORY (Jon Atack)
b7a17444e0746c562ae97b26eba431577947b06a log, sync: add LOCK logging category, apply it to lock contention (Jon Atack)
Pull request description:
To enable lock contention logging, `DEBUG_LOCKCONTENTION` has to be defined at compilation. Once built, the logging is not limited to a category and is high frequency, verbose and in all-caps. With these factors combined, it seems likely to be rarely used.
This patch:
- adds a `lock` logging category
- adds a timing macro in microseconds, `LOG_TIME_MICROS_WITH_CATEGORY`
- updates `BCLog::LogMsg()` to omit irrelevant decimals for microseconds and skip unneeded code and math
- improves the lock contention logging, drops the all-caps, and displays the duration in microseconds
- removes the conditional compilation directives
- allows lock contentions to be logged on startup with `-debug=lock` or at run time with `bitcoin-cli logging '["lock"]'`
```
$ bitcoind -signet -debug=lock
2021-09-01T12:40:01Z LockContention: cs_vNodes, net.cpp:1920 started
2021-09-01T12:40:01Z LockContention: cs_vNodes, net.cpp:1920 completed (4μs)
2021-09-01T12:40:01Z LockContention: cs_vNodes, net.cpp:1302 started
2021-09-01T12:40:01Z LockContention: cs_vNodes, net.cpp:1302 completed (4μs)
2021-09-01T12:40:02Z LockContention: cs_vNodes, net.cpp:2242 started
2021-09-01T12:40:02Z LockContention: cs_vNodes, net.cpp:2242 completed (20μs)
2021-09-01T12:43:04Z LockContention: ::cs_main, validation.cpp:4980 started
2021-09-01T12:43:04Z LockContention: ::cs_main, validation.cpp:4980 completed (3μs)
$ bitcoin-cli -signet logging
"lock": true,
$ bitcoin-cli -signet logging [] '["lock"]'
"lock": false,
$ bitcoin-cli -signet logging '["lock"]'
"lock": true, ```
I've tested this with Clang 13 and GCC 10.2.1, on Debian, with and without `--enable-debug`.
## 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)_
Top commit has no ACKs.
Tree-SHA512: ecd15833b9e5752067c92a074f61d88fb8528157b1c0446f9f74d4898acb9c09ebb5f652c75152785a41126fdf777032a6f85d472cd1d9a0b6013519329dc2e7
7e698732836121912f179b7c743a72dd6fdffa72 sync: remove DEBUG_LOCKCONTENTION preprocessor directives (Jon Atack)
9b08006bc502e67956d6ab518388fad6397cac8d log, sync: improve lock contention logging and add time duration (Jon Atack)
3f4c6b87f1098436693c4990f2082515ec0ece26 log, timer: add timing macro in usec LOG_TIME_MICROS_WITH_CATEGORY (Jon Atack)
b7a17444e0746c562ae97b26eba431577947b06a log, sync: add LOCK logging category, apply it to lock contention (Jon Atack)
Pull request description:
To enable lock contention logging, `DEBUG_LOCKCONTENTION` has to be defined at compilation. Once built, the logging is not limited to a category and is high frequency, verbose and in all-caps. With these factors combined, it seems likely to be rarely used.
This patch:
- adds a `lock` logging category
- adds a timing macro in microseconds, `LOG_TIME_MICROS_WITH_CATEGORY`
- updates `BCLog::LogMsg()` to omit irrelevant decimals for microseconds and skip unneeded code and math
- improves the lock contention logging, drops the all-caps, and displays the duration in microseconds
- removes the conditional compilation directives
- allows lock contentions to be logged on startup with `-debug=lock` or at run time with `bitcoin-cli logging '["lock"]'`
```
$ bitcoind -signet -debug=lock
2021-09-01T12:40:01Z LockContention: cs_vNodes, net.cpp:1920 started
2021-09-01T12:40:01Z LockContention: cs_vNodes, net.cpp:1920 completed (4μs)
2021-09-01T12:40:01Z LockContention: cs_vNodes, net.cpp:1302 started
2021-09-01T12:40:01Z LockContention: cs_vNodes, net.cpp:1302 completed (4μs)
2021-09-01T12:40:02Z LockContention: cs_vNodes, net.cpp:2242 started
2021-09-01T12:40:02Z LockContention: cs_vNodes, net.cpp:2242 completed (20μs)
2021-09-01T12:43:04Z LockContention: ::cs_main, validation.cpp:4980 started
2021-09-01T12:43:04Z LockContention: ::cs_main, validation.cpp:4980 completed (3μs)
$ bitcoin-cli -signet logging
"lock": true,
$ bitcoin-cli -signet logging [] '["lock"]'
"lock": false,
$ bitcoin-cli -signet logging '["lock"]'
"lock": true,
```
I've tested this with Clang 13 and GCC 10.2.1, on Debian, with and without `--enable-debug`.
ACKs for top commit:
hebasto:
re-ACK 7e698732836121912f179b7c743a72dd6fdffa72, added a contention duration to the log message since my [previous](https://github.com/bitcoin/bitcoin/pull/22736#pullrequestreview-743764606) review.
theStack:
re-ACK 7e698732836121912f179b7c743a72dd6fdffa72 🔏⏲️
Tree-SHA512: c4b5eb88d3a2c051acaa842b3055ce30efde1f114f61da6e55fcaa27476c1c33a60bc419f7f5ccda532e1bdbe70815222ec2b2b6d9226f29c8e94e598aacfee7
Moving out of a variable technically leaves it in an unspecified and potentially invalid state; yet I don't see anywhere we are reseting or otherwise clearing pendingContributionVerifications to ensure it is valid. Instead simply take a ref to it; and then clear it at the end.
c79f8b5ea2 fix: keep ADDRS outside (UdjinM6)
a39065be88 test: fix incorrect nServices assertion, use NODE_NETWORK value (Kittywhiskers Van Gogh)
0e78555a5b fix: add missing check for sending addrs (UdjinM6)
3cd69377b6 fix: test nodes should use mocktime (UdjinM6)
acbbe8c9a2 fix: relayed addresses should use mocktime (UdjinM6)
Pull request description:
## Issue being fixed or feature implemented
`p2p_addr_relay.py` is broken but we don't see it in CI because it doesn't test everything it should atm. This weird behaviour was discovered by @kwvg while preparing #5964.
## What was done?
Bring back the missing check and fix the test. Borrowed one fix from #5964 to make this PR complete.
## How Has This Been Tested?
Run `p2p_addr_relay.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)_
ACKs for top commit:
kwvg:
ACK c79f8b5ea2
Tree-SHA512: a88f9c3563205b092ebd9ab7e8f0c034b6ef5bef2adbf57c269c444ef334e519e30d028325e73e99de025654a0dbd94baad033fb1538896e85572d4db2a00b23
