This marks the completion of our transition from code based on
`talebook/statsd-client-cpp` to code based on `vthiery/cpp-statsd-client`.
Also, we long stopped using `snprintf`, remove it from exclusions list.
`statsns`, aside from being an unclear name, in its default behaviour,
doesn't use the namespace delimiter contaminates the root namespace
of the key.
Let's take care of that by deprecating `statsns` with its replacement,
`statsprefix`, that behaves properly.
`statshostname` implies that alongside specifying the host, the user
needs to specify a separate "hostname" field in order to connect when
in fact, it is an entirely optional suffix field applied to stats keys.
Rename it to `statssuffix` and deprecate `statshostname`, the latter
will be removed in a subsequent major release.
This is to avoid the odd circumstance where stats don't work because you
specified everything but didn't explicitly enable it. Using
`-statsenabled` has been deprecated and will be removed in the next
major release.
- `timing` cannot contain a negative value, change to `uint64_t`
- remove `sendDouble`, use template specialization of `Send` instead,
that can be reused as `send`
- rename `value` in `count()` to `delta`
- add more comments
`gaugeDouble` has been kept around because utilizing templates for it
would require us to either write template specializations for every kind
of `gauge` value (implicit conversions will not save us from this) or
move all logic to the header, neither option seems desirable.
`RawSender` is inspired by `UDPSender` from `vthiery/cpp-statsd-client`
and separating it out of `StatsdClient` is needed to implement queueing
and batching support in upcoming commits.
This is the start of migrating our Statsd codebase to `cpp-statsd-client`.
In upcoming commits, message sending will be split off into a separate
file and stats capabilities will be fleshed out. Prepare for that by
giving it its own directory.
Also get rid of `statsd` namespace, it is entirely unnecessary.
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
e544d3c741 fmt: apply `clang-format-diff.py` suggestions, satisfy linter (Kittywhiskers Van Gogh)
7da74ffcd5 merge bitcoin#28341: Use HashWriter over legacy CHashWriter (Kittywhiskers Van Gogh)
c798b496cd merge bitcoin#27529: fix `feature_addrman.py` on big-endian systems (Kittywhiskers Van Gogh)
7d149c97be merge bitcoin#27745: select addresses by network follow-up (Kittywhiskers Van Gogh)
1d82994383 merge bitcoin#26261: cleanup `LookupIntern`, `Lookup` and `LookupHost` (Kittywhiskers Van Gogh)
231ff82c2e merge bitcoin#27214: Enable selecting addresses by network (Kittywhiskers Van Gogh)
e82559516c merge bitcoin#25619: avoid overriding non-virtual ToString() in CService and use better naming (Kittywhiskers Van Gogh)
2e9b48a910 merge bitcoin#26847: track AddrMan totals by network and table, improve precision of adding fixed seeds (Kittywhiskers Van Gogh)
79a550ec15 merge bitcoin#26040: comment "add only reachable addresses to addrman" (Kittywhiskers Van Gogh)
1adb635ec6 merge bitcoin#25678: skip querying dns seeds if -onlynet disables IPv4 and IPv6 (Kittywhiskers Van Gogh)
2d99be0aea partial bitcoin#25331: Add HashWriter without ser-type and ser-version (Kittywhiskers Van Gogh)
Pull request description:
## Additional Information
* Dependent on https://github.com/dashpay/dash/pull/6243
* Dependent on https://github.com/dashpay/dash/pull/5167
## 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:
UdjinM6:
utACK e544d3c741
Tree-SHA512: 63f014142c39c47bda3ac85dc6afeee8f2bfec71f033631bca16d41bb0785f4b090b3c860ddc3b3cf6c4a23558d3d102144fc83b065130c3f9ab91d0de8e4457
cc998abec1 fmt: apply `clang-format-diff.py` suggestions (Kittywhiskers Van Gogh)
0401c581eb stats: `const`-ify variables and arguments (Kittywhiskers Van Gogh)
9f96723774 stats: stop using error codes, switch over to `bool` (Kittywhiskers Van Gogh)
1a81979c1e stats: initialize socket after we have a valid socket address (Kittywhiskers Van Gogh)
dbbfc8d766 stats: use `Socks` wrapper, use `CService` to generate our `sockaddr` (Kittywhiskers Van Gogh)
2def905044 stats: move init logic into constructor (Kittywhiskers Van Gogh)
4bc727cd6c stats: clean up randomization code, move `FastRandomContext` inward (Kittywhiskers Van Gogh)
840241eefd stats: cleanup error logging, improve code sanity (Kittywhiskers Van Gogh)
85890ddb13 docs: add copyright notice to source file, update notice in header (Kittywhiskers Van Gogh)
a9d1b1494d stats: move `_StatsdClientData` variables into `StatsdClient` (Kittywhiskers Van Gogh)
30c30c1397 stats: fetch all arguments needed when constructing `g_stats_client` (Kittywhiskers Van Gogh)
5133d88415 stats: s/statsClient/g_stats_client/g (Kittywhiskers Van Gogh)
f81951dd00 stats: make `statsClient` a `std::unique_ptr`, denote as global variable (Kittywhiskers Van Gogh)
Pull request description:
## Additional Information
Support for transmitting stats to a Statsd server has been courtesy of Statoshi ([repo](https://github.com/jlopp/statoshi)), implemented Dec, 2020 by [dash#2515](https://github.com/dashpay/dash/pull/2515) but since then, it hasn't gotten much attention aside from benefiting from codebase-wide changes and the occasional compiler appeasement. This pull request aims to give our statistics code some TLC.
Changes include:
* Limiting initialization to solely during construction and moving the responsibility of fetching arguments outside of `statsd::StatsdClient`.
* Using the RAII `Socks` wrapper as early as possible (we still need to construct a raw socket ourselves but this is done in the initializer and control is moved to the wrapper and everywhere else, the wrapper is used)
* Utilizing existing networking code to generate the socket address
* This lets us trivially allow IPv6 connections as the responsibility to construct it safely is moved to `CService`.
* Using `std::string` and our string manipulation capabilities (replacing `snprintf` with `strprintf`), replacing platform-specific types (replacing `short` with `uint16_t`).
## 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 **(note: N/A)**
- [x] I have made corresponding changes to the documentation **(note: N/A)**
- [x] I have assigned this pull request to a milestone
ACKs for top commit:
PastaPastaPasta:
utACK [cc998ab](cc998abec1)
UdjinM6:
utACK cc998abec1
Tree-SHA512: 433c92160d6ac7ebb8582ada3cbb65ead7913618266b773619a528c90dfe0e286aafa46dc3b0bca62f246938e5948a732080e2cddba942d3627f007ca6efcc1f
4faf35f749 chore: add `stats` as a pull request header scope (Kittywhiskers Van Gogh)
Pull request description:
## Additional Information
Would allow tagging https://github.com/dashpay/dash/pull/5167, https://github.com/dashpay/dash/pull/6267 and https://github.com/dashpay/dash/pull/6237 as `feat(stats)`.
## Breaking Changes
None.
## Checklist:
- [x] I have performed a self-review of my own code **(note: N/A)**
- [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 **(note: N/A)**
- [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)_ **(note: N/A)**
ACKs for top commit:
PastaPastaPasta:
utACK 4faf35f749
UdjinM6:
utACK 4faf35f749
thephez:
utACK 4faf35f749
Tree-SHA512: 25cc28792351852b9e920d980b8814d93274bc53d22ce63fb1a5bf32821b10902d22b384a408e1c4a7b97239e53e235c45ac008d0f82e1afe5d6071b392acb47
6c7335e002 merge bitcoin#24331: Revert back `MoveFileExW` call for MinGW-w64 (Kittywhiskers Van Gogh)
15e794bdd8 merge bitcoin#24238: use arc4random on OpenBSD (Kittywhiskers Van Gogh)
e039aecbdc merge bitcoin#23936: Add and use EnsureArgsman helper (Kittywhiskers Van Gogh)
b4bfacfd24 merge bitcoin#23769: Disallow copies of CChain (Kittywhiskers Van Gogh)
5b66688160 merge bitcoin#22362: Drop only invalid entries when reading banlist.json (Kittywhiskers Van Gogh)
109c963f6a merge bitcoin#23175: Add CJDNS network to -addrinfo and -netinfo (Kittywhiskers Van Gogh)
d57c96ea37 merge bitcoin#23398: add return message to savemempool RPC (Kittywhiskers Van Gogh)
22e59fb464 merge bitcoin#23054: Use C++11 member initializer in CTxMemPoolEntry (Kittywhiskers Van Gogh)
d158063b6d merge bitcoin#22653: Rename JoinErrors and re-use it (Kittywhiskers Van Gogh)
e24324d266 merge bitcoin#22221: Pass block reference instead of pointer to PeerManagerImpl::BlockRequested (Kittywhiskers Van Gogh)
68657efc03 merge bitcoin#22141: net processing: Remove hash and fValidatedHeaders from QueuedBlock (Kittywhiskers Van Gogh)
c0e6792e27 merge bitcoin#20018: ProcessAddrFetch(-seednode) is unnecessary if -connect is specified (Kittywhiskers Van Gogh)
Pull request description:
## Additional Information
* When backporting [bitcoin#23054](https://github.com/bitcoin/bitcoin/pull/23054), `sigOpCount` and `nSigOpCountWithAncestors` were switched from `unsigned int` to `int64_t`. This change was done to prevent integer narrowing when accepting the `int64_t` taken from the constructor.
This isn't a problem upstream as the same changes were as part of [bitcoin#8149](https://github.com/bitcoin/bitcoin/pull/8149/files#diff-8a2230436880b65a205db9299ab2e4e4adb1d4069146791b5db566f3fb752adaL90-L107), which was omitted but the type changes remain valid as sigop count cannot be a negative number.
## 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 6c7335e002
UdjinM6:
utACK 6c7335e002
Tree-SHA512: 29cae42dd82235305d3558562bae346170e742ce0b65897e396b331294b39cad0dd831fa9a09b34780a67844e55292e5b4e784cf544a894cc3f8897afe617ca1
As creating the socket is now the last step, we don't need
`m_init` anymore. We can just see if a socket is successfully
constructed and take that as our validity indicator.
We'll also move it out of the inner `send` function so we can bail out
before we bother with all the string processing and manipulation.
We can skip the computationally expensive dice-roll if our sample rate
is zero as that means we should never send it. Also get rid of the
`FastRandomContext::operator()` that isn't used anywhere else in the
codebase.
- Use `uint16_t` instead of `short`, `int64_t` instead of `size_t`
- Get rid of the `errmsg` buffer and use `LogPrintf` to report errors
- Use `strprintf` instead of `snprintf`
- Rephrase networking error logs to allow inclusion of error strings
0c5179462e test: rework feature_governance_cl.py (UdjinM6)
11ac0819da feat: bump_mocktime also bumps schedulers now (UdjinM6)
1937f503fe feat: regtest-only: do not auto-reset mnsync after 1h (UdjinM6)
Pull request description:
## Issue being fixed or feature implemented
1. The original idea behind forced reset was to help desktop clients which go into sleep/hibernation mode from time to time to sync with no issues once they are online again. For regtest however it doesn't do anything good and only causes issues.
2. We rely on schedulers a lot, bumping them should let nodes behave more like on a real network.
3. Forcing mnsync to skip governance votes doesn't always work as we'd expect cause we don't control connection creation. To make it more deterministic create a proposal that should get into one superblock only. This way both the proposal and the trigger will expire/be deleted after another superblock meaning we can be 100% sure the isolated node never gets any of them and can only sync thanks to chainlock.
## 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
- [ ] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_
ACKs for top commit:
knst:
utACK 0c5179462e
Tree-SHA512: 9a40eeaba880f3f27f86736e92afa19a4ecb2a2d157bc42b65dd9da0d4109c9cd1d83a5abdf1ec16be2f8a8b31821fb700a7f0d2265c094fd4fdff7f18bc6ec7
57fb0874ef feat: broadcast dsq messages using the inventory system (pasta)
Pull request description:
## Issue being fixed or feature implemented
DSQ messages are 142 bytes.
Previously, assuming a relatively highly connected masternode hosting 100 connection, each round of coinjoin will result in 14.2KB (100*142) of inbound and outbound traffic each.
## What was done?
Now, using the inventory system, a message will first use 36 bytes per peer (sending and receiving), plus the size of a `getdata` message and the actual message itself. As a result, bandwidth usage for 1 round of mixing would be closer to 36 * 100 + 142 (dsq) + 36 (getdata) = ~3.8KB, a reduction of around ~73%
## How Has This Been Tested?
Has not been; @UdjinM6 especially please review well :)
## Breaking Changes
Does introduce a new protocol version, but in a backwards compatible way. I don't think this would need to be delayed to v22 for any reason.
## 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)_
Top commit has no ACKs.
Tree-SHA512: 3dc39a339cba29d8cf207cec76ecace5ad0e11d1892ca0f65f9253a2b1d90313da21c6c178c2476756c5566ece0fab777006cd609b7984df906a9206c25d921d
DSQ messages are 142 bytes.
Previously, assuming a relatively highly connected masternode hosting 100 connection, each round of coinjoin will result in 14.2KB (100*142) of inbound and outbound traffic each.
Now, using the inventory system, a message will first use 36 bytes per peer (sending and receiving), plus the size of a `getdata` message and the actual message itself. As a result, bandwidth usage for 1 round of mixing would be closer to 36 * 100 + 142 (dsq) + 36 (getdata) = ~3.8KB, a reduction of around ~73%
23812555b1 fix: possible deadlock during calculation of signals for historical blocks during re-index (Konstantin Akimov)
1087489fd4 feat: bury v20 deployment (Konstantin Akimov)
64cedb30bd feat: actually test something EHF unit tests (Konstantin Akimov)
762a808b8c chore: drop irrelevant bip9 code from feature_llmq_rotation.py (Konstantin Akimov)
7735631aad fix: remove v20 from test feature_llmq_evo as far as mn_rr used (Konstantin Akimov)
ca83b26815 fix: crash in CreditPool: it meant to check that DIP0003 is activated (Konstantin Akimov)
Pull request description:
## Issue being fixed or feature implemented
V20 is activated on mainnet: time to bury it!
https://github.com/dashpay/dash/issues/6186
## What was done?
Hard-fork v20 is buried and it requires to implement multiple fixes, simplifications, refactoring:
- some tests for EHF moved from functional tests to unit tests
- fixed crash in Credit Pool if DIP3 is not activated yet
- added a requirement for v20 activation for `CMNHFManager::GetSignalsStage`
- removed useless code from functional test feature_llmq_rotation
- renamed variables "v20" to "mn_rr" in feature_llmq_evo.py so far as actually used fork is mn_rr
## How Has This Been Tested?
Some unit and functional tests to succeed.
Done reindex (just in case):
src/qt/dash-qt -reindex -assumevalid=0
src/qt/dash-qt -reindex -assumevalid=0 -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
- [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
ACKs for top commit:
UdjinM6:
ACK 23812555b1
PastaPastaPasta:
utACK 23812555b1
Tree-SHA512: eec35745baa695f3f286d39b6a61fa0a9f34820b13d1dd4cfbd1efe707850283892c39bf7fe49c49c812e0c02465d64df11480b3f12aa7f21b59a71eeae7260e
