847288df07b45ca535c849e518b22818ab492896 test: fee rate values that cannot be represented as sat/vB (Jon Atack)
06a90fa0381c790f7bde2ab9bf47d2b22acef4a5 rpc: for sat/vB fee rates, limit ParseFixedPoint decimals to 3 (Jon Atack)
0742c7840f03505597fd2de87db97f12597ef667 rpc: enable passing decimals to AmountFromValue, add doxygen (Jon Atack)
8ce3ef57a3e9ad13c0aaa4648e8584241d53592d test: ParseFixedPoint with 3 decimals for sat/vB fee rates (Jon Atack)
b5033275979a2a495b02b25f70cadbdcc8b6eb6a test: type error and out of range fee rates where missing (Jon Atack)
c5fd4344f7fcc257062a610c8ff26ffcc9b53953 test: explicit fee rates with invalid amounts (Jon Atack)
ea6f76b66ecc52360719053489e0ec9f9a673eab test: improve zero-value explicit fee rate coverage (Jon Atack)
Pull request description:
- Improve/close gaps in existing test coverage before making the change
- Enable passing `decimals` to `ParseFixedPoint()` when calling `AmountFromValue()`
- Limit explicit fee rates in sat/vB passed in by users to 3 decimals, and raise otherwise
- Add regression test coverage
Closes#20534.
ACKs for top commit:
MarcoFalke:
review ACK 847288df07b45ca535c849e518b22818ab492896 🔷
Tree-SHA512: c539d07ae9b21c0d6c8ea460beb9c8dad5559445518aace560abc3c05c588907bae189b6fd7602b3b397de4a42356136c3ec6f960d3dcf2d5d16377aef4ab5a2
f09e6b2585200040be2e8ee44fa79b86b1970d70 test: fix off-by-ones in rpc_fundrawtransaction (Jon Atack)
Pull request description:
The variables in these assertions should be the same within each line.
ACKs for top commit:
laanwj:
ACK f09e6b2585200040be2e8ee44fa79b86b1970d70
theStack:
ACK f09e6b2585200040be2e8ee44fa79b86b1970d70
Tree-SHA512: 7ac754eaadd8cb00a725afa55bccbb8de7547dedac9350d79a9a470918245617e075c56a91adc36fb653bbe8a0a325d59b00443155a7e1a81ebf22e4e4cf56d9
f9cd2bfbccb7a2b8ff07cec5f6d2adbeca5f07c3 Rename CoinSelectionParams::effective_fee to m_effective_feerate (Andrew Chow)
bdd0c2934b7f389ffcfae3b602ee3ecee8581acd wallet: Move discard feerate fetching to CreateTransaction (Andrew Chow)
448d04b931f86941903e855f831249ff5ec77485 wallet: Move long term feerate setting to CreateTransaction (Andrew Chow)
e2f429e6bbf7098f278c0247b954ecd3ba53cf37 wallet: Replace nFeeRateNeeded with effective_fee (Andrew Chow)
1a6a0b0dfb90f9ebd4b86d7934c6aa5594974f5f wallet: Use existing feerate instead of getting a new one (Andrew Chow)
Pull request description:
During coin selection, there are various places where we need to have a feerate. We need the feerate for the transaction itself, the discard fee rate, and long term feerate. Fetching these each time we need them can lead to a race condition where two feerates that should be the same are actually different. One particular instance where this can happen is during the loop in `CreateTransactionInternal`. After inputs are chosen, the expected transaction fee is calculated using a newly fetched feerate. If `pick_new_inputs == false`, the loop will go again with the assumption that the fee for the transaction remains the same. However because the feerate is fetched again, it is possible that it actually isn't and this causes coin selection to fail.
Instead of fetching the feerate each time it is needed, we fetch them all at once at the top of `CreateTransactionInternal`, store them in `CoinSelectionParams`, and use them where needed.
While some of these fee rates probably don't need this caching, I've done it for consistency and the guarantee that they remain the same.
Fixes#19229
ACKs for top commit:
glozow:
reACK f9cd2bfbcc
fjahr:
Code review re-ACK f9cd2bfbccb7a2b8ff07cec5f6d2adbeca5f07c3
Xekyo:
tACK f9cd2bfbcc
meshcollider:
Code review + test run ACK f9cd2bfbccb7a2b8ff07cec5f6d2adbeca5f07c3
Tree-SHA512: be83ff64ba473c3cdd3469c812e214659b6e2a9584c22ed2b1595618fce0d4b35d0901e61068cd1069fc1a8fb911db01dd7312d05c3b8cbafbe2504ab7a3e863
6bfbc97d716faad38c87603ac6049d222236d623 test: disallow sendtoaddress/sendmany when private keys disabled (Jon Atack)
0997019e7681efb00847a7246c15ac8f235128d8 Disallow sendtoaddress and sendmany when private keys disabled (Andrew Chow)
Pull request description:
Since `sendtoaddress` and `sendmany` (which use the `SendMoney` function) create and commit a transaction, they should not do anything when the wallet does not have private keys. Otherwise a valid transaction cannot be made.
Fixes#21104
ACKs for top commit:
jonatack:
ACK 6bfbc97d716faad38c87603ac6049d222236d623
meshcollider:
utACK 6bfbc97d716faad38c87603ac6049d222236d623
kristapsk:
ACK 6bfbc97d716faad38c87603ac6049d222236d623. "Error: Private keys are disabled for this wallet" is definitely a better error message than "Insufficient funds" here. Hopefully change of error code from -6 to -4 doesn't break any software using Bitcoin JSON-RPC API.
Tree-SHA512: f277d6b5252e43942d568614032596f2c0827f00cd0cb71e44ffcb9822bfb15a71730a3e3688f31e59ba4eb7d275250c4e65ad4b6b3e96be6314c56a672432fb
6fa72ceb8021c3b5aea62f6cfe92665c29212923 test: add coverage for passing fee rate as a string (Jon Atack)
ce207d6b93d35bc02fcd2dd28f1fd95869261d43 wallet, bugfix: allow send to take string fee rate values (Jon Atack)
Pull request description:
RPC send currently only accepts fee rates as numbers, which is a user-facing bug. It should accept fee rates as an amount, e.g. a string or a number, as documented in its help and like sendtoaddress, sendmany, fundrawtransaction, walletcreatefundedpsbt, and bumpfee. Provide a fix and regression test coverage.
ACKs for top commit:
MarcoFalke:
review ACK 6fa72ceb8021c3b5aea62f6cfe92665c29212923
achow101:
Code review ACK 6fa72ceb8021c3b5aea62f6cfe92665c29212923
promag:
Code review ACK 6fa72ceb8021c3b5aea62f6cfe92665c29212923.
Tree-SHA512: 735f9269cb1b81953764b5283449c0b154bd62de034225be5bcedc515c84faf767fe8fe0741008679fe412922c847b00d116cb11aab775236b779c847ba87167
fa69c2c78455fd0dc436018fece9ff7fc83a180d wallet: Do not treat default constructed types as None-type (MarcoFalke)
fac4e136fa3d0fab7fde900a6be921313e16e7a6 refactor: Change pointer to reference because it can not be null (MarcoFalke)
Pull request description:
Equating `0==None` and `""==None` is confusing, unneeded and undocumented
ACKs for top commit:
jonatack:
ACK fa69c2c78455fd0dc436018fece9ff7fc83a180d
achow101:
ACK fa69c2c78455fd0dc436018fece9ff7fc83a180d
Sjors:
tACK fa69c2c78455fd0dc436018fece9ff7fc83a180d modulo `unset`
Tree-SHA512: c4c8d0ad80c6697621d356a9545caf28ca2facc82bb2fa8e70eceb52372d25f0685237c73688c4b01da0e75d213c77c0d45011a8bdfe81ea783d85f045786dac
9f08780dd7946b63476e9736745131db8e7f4e93 Use the correct incremental fee constant in bumpfee help (Jon Atack)
3f1e10b2b1cd11f7112fbad6355464bd4adbbc5c Update feeRate (BTC/kvB) to fee_rate (sat/vB) in wallet_bumpfee (Jon Atack)
1b3d7009280595108eb22ac1188bc4367804fc5d Allow zero-fee fundrawtxn and walletcreatefundedpsbt calls (Jon Atack)
Pull request description:
- Fixes https://github.com/bitcoin/bitcoin/pull/20305/files#r525406176. A check to raise an error on zero-fee txns was mistakenly extended in a0d4957 from the bumpfee and send{toaddress, many} RPCs to also include fundrawtransaction and walletcreatefundedpsbt. This commit re-overrides zero fee rate checking for these two RPCs, not only for the feeRate (BTC/kvB) arg to return to previous behavior, but also for the new fee_rate (sat/vB) arg. Negative fee rates will still raise "amount out of range" by the MoneyRange check in src/bitcoin-tx.cpp::AmountFromValue.
- Updates a wallet bumpfee test from feeRate (BTC/kvB) to fee_rate (sat/vB)
- Fixes https://github.com/bitcoin/bitcoin/pull/20305/files#r525405363 to use the correct incremental fee rate constant in the bumpfee help (thanks Marco Falke for the catch) and rectifies "1.000 sat/vB sat/vB" in the help to "1.000 sat/vB"
ACKs for top commit:
MarcoFalke:
review ACK 9f08780dd7946b63476e9736745131db8e7f4e93 🐾
promag:
Code review ACK 9f08780dd7946b63476e9736745131db8e7f4e93.
Xekyo:
Code review reACK 9f08780dd7946b63476e9736745131db8e7f4e93.
Tree-SHA512: 413dfb4f23ebaf3d2ef210dd04610a843272e64eabba428699f5de4d646a86ac4911dab66b5e2f5ebea53b76e4be8347ef40824c1592c750d5eaa12579d3cdf6
05e82d86b09d914ebce05dbc92a7299cb026847b wallet: override minfee checks (fOverrideFeeRate) for fee_rate (Jon Atack)
9a670b4f07a6140de809d73cbd7f3e614eb6ea74 wallet: update sendtoaddress, send RPC examples with fee_rate (Jon Atack)
be481b72e24fb6834bd674cd8daee67c6938b42d wallet: use MIN_RELAY_TX_FEE in bumpfee help (Jon Atack)
449b730579566459e350703611629e63e54657ed wallet: provide valid values if invalid estimate mode passed (Jon Atack)
6da3afbaee5809ebf6d88efaa3958c505c2d71c7 wallet: update remaining rpcwallet fee rate units to BTC/kvB (Jon Atack)
173b5b5fe07d45be5a1e5bc7a5df996f20ab1e85 wallet: update fee rate units, use sat/vB for fee_rate error messages (Jon Atack)
7f9835a05abf3e168ad93e7195cbaa4bf61b9b07 wallet: remove fee rates from conf_target helps (Jon Atack)
b7994c01e9a3251536fe6538a22f614774eec82d wallet: add fee_rate unit warnings to bumpfee (Jon Atack)
410e471fa42d3db04e8879c71f8c824dcc151a83 wallet: remove redundant bumpfee fee_rate checks (Jon Atack)
a0d495747320c79b27a83c216dcc526ac8df8f24 wallet: introduce fee_rate (sat/vB) param/option (Jon Atack)
e21212f01b7c41eba13b0479b252053cf482bc1f wallet: remove unneeded WALLET_BTC_KB_TO_SAT_B constant (Jon Atack)
6112cf20d43b0be34fe0edce2ac3e6b27cae1bbe wallet: add CFeeRate ctor doxygen documentation (Jon Atack)
3f7279161347543ce4e997d78ea89a4043491145 wallet: fix bug in RPC send options (Jon Atack)
Pull request description:
This PR builds on #11413 and #20220 to address #19543.
- replace overloading the conf_target and estimate_mode params with `fee_rate` in sat/vB in the sendtoaddress, sendmany, send, fundrawtransaction, walletcreatefundedpsbt, and bumpfee RPCs
- allow non-actionable conf_target value of `0` and estimate_mode value of `""` to be passed to use `fee_rate` as a positional argument, in addition to as a named argument
- fix a bug in the experimental send RPC described in https://github.com/bitcoin/bitcoin/pull/20220#discussion_r513789526 where args were not being passed correctly into the options values
- update the feerate error message units for these RPCs from BTC/kB to sat/vB
- update the test coverage, help docs, doxygen docs, and some of the RPC examples
- other changes to address the excellent review feedback
See this wallet meeting log for more context: http://www.erisian.com.au/bitcoin-core-dev/log-2020-11-06.html#l-309
ACKs for top commit:
achow101:
re-ACK 05e82d8
MarcoFalke:
review ACK 05e82d86b0 did not test and found a few style nits, which can be fixed later 🍯
Xekyo:
tACK 05e82d86b09d914ebce05dbc92a7299cb026847b
Sjors:
utACK 05e82d86b09d914ebce05dbc92a7299cb026847b
Tree-SHA512: a4ee5f184ada53f1840b2923d25873bda88c5a2ae48e67eeea2417a0b35154798cfdb3c147b05dd56bd6608a784e1b91623bb985ee2ab9ef2baaec22206d0a9c
0be29000c011dec0722481dbebb159873da6fa54 rpc: update conf_target helps for correctness/consistency (Jon Atack)
778b9be40667876c705e586849ea9c9e44cf451c wallet, rpc: fix send subtract_fee_from_outputs help (Jon Atack)
603c0050837ec65765208dd54dde354145fbe098 wallet: add rpc send explicit fee rate coverage (Jon Atack)
dd341e602d5160fc621c0299179b91403756b61d wallet: add sendtoaddress/sendmany explicit fee rate coverage (Jon Atack)
44e7bfa60313e4ae67da49e5ba4535038b71b453 wallet: add walletcreatefundedpsbt explicit fee rate coverage (Jon Atack)
6e1ea4273e52fdcd86c87628aa595c03a071ca8c test: refactor for walletcreatefundedpsbt fee rate coverage (Jon Atack)
3ac7b0c6f1c68e74a84d868a454f508bada6b09d wallet: fundrawtx fee rate coverage, fixup ParseConfirmTarget() (Jon Atack)
2d8eba8f8425a2515022d51f1f5b4911329fbf55 wallet: combine redundant bumpfee invalid params and args tests (Jon Atack)
1697a40b6f841a54ee0d9744ed7fd09034b0ddad wallet: improve bumpfee error/help, add explicit fee rate coverage (Jon Atack)
fc5721723d34f76f9e1ffd2e31f274ea6b22f894 wallet: fix SetFeeEstimateMode() error message (Jon Atack)
052427eef1c9da84c474c5161b1910d3328ef0da wallet, bugfix: fix bumpfee with explicit fee rate modes (Jon Atack)
Pull request description:
Follow-up to #11413 providing a base to build on for #19543:
- bugfix for `bumpfee` raising a JSON error with explicit feerates, fixes issue #20219
- adds explicit feerate test coverage for `bumpfee`, `fundrawtransaction`, `walletcreatefundedpsbt`, `send`, `sendtoaddress`, and `sendmany`
- improves a few related RPC error messages and `ParseConfirmTarget()` / error message
- fixes/improves the explicit fee rate information in the 6 RPC helps, of which 2 were also missing `conf_target` sat/B units
This provides a spec and regression coverage for the potential next step of a universal `sat/vB` feerate argument (see #19543), as well as immediate coverage and minimum fixes for 0.21.
ACKs for top commit:
kallewoof:
Concept/Tested ACK 0be29000c011dec0722481dbebb159873da6fa54
meshcollider:
Code review + functional test run ACK 0be29000c011dec0722481dbebb159873da6fa54
Tree-SHA512: efd965003e991cba51d4504e2940f06ab3d742e34022e96a673606b44fad85596aa03a8c1809f06df7ebcf21a38e18a891e54392fe3d6fb4d120bbe4ea0cf5e0
44cc75f80ee7805a117e9298a182af1a44bcbff4 wallet: error if an explicit fee rate was given but the needed fee rate differed (Karl-Johan Alm)
Pull request description:
This ensures that the code doesn't silently ignore too low fee reates. It will now trigger an error in the QT client, if the user provides a fee rate below the minimum, and becomes a necessary check for #11413.
ACKs for top commit:
Sjors:
utACK 44cc75f80ee7805a117e9298a182af1a44bcbff4 (rebased)
fjahr:
re-ACK 44cc75f80ee7805a117e9298a182af1a44bcbff4
Tree-SHA512: cd5a60ee496e64f7ab37aaa53f7748a7393357b1629ccd9660839d366c6191b6413b871ce3aa7293fce1539336222c300ef6f86304f30a1ae8fe361b02310483
056d869571 refactor: use testdummy in feature_mnehf functional test, removed useless checks (Konstantin Akimov)
0351469bb5 refactor: removed duplicated meaningless condition from Check mnhftx (Konstantin Akimov)
Pull request description:
## Issue being fixed or feature implemented
Using mn_rr in feature_mnehf.py is a blocker for burying mn_rr fork.
## What was done?
Removed useless conditions, uses testdummy fork instead mn_rr in ehf functional test.
## How Has This Been Tested?
Run it `test/functional/feature_mnehf.py`
## 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:
utACK 056d869571
PastaPastaPasta:
utACK 056d869571
Tree-SHA512: aa016dc21fee3afdb5a4172e757cdd0b92867c23eb5241287f641d3c97d363ab1c46eba06423481ecc7ba05f6c0010b65eadfe77d4a1fd6074706cc6f3a71309
42dffe8541 chore: improve logging of functional tests feature_governance.py (Konstantin Akimov)
922b796800 feat: simplify and speedup feature_governance.py test by generating less blocks (Konstantin Akimov)
Pull request description:
## Issue being fixed or feature implemented
Too many blocks are generated in a functional tests `feature_governance.py`
## What was done?
1. Activate v20 faster for `feature_governance.py`, drop generation of useless blocks, simplify code.
2. Added extra logging for this test by converting comments to logs.
## How Has This Been Tested?
Run `test/functional/feature_governance.py`
Locally improvement of performance is less than 5 seconds.
`tsan` jobs on CI:
```
204/264 - feature_governance.py --legacy-wallet passed, Duration: 243 s
208/264 - feature_governance.py --descriptors passed, Duration: 270 s
```
↑ [old version](https://gitlab.com/dashpay/dash/-/jobs/7805625816) vs [new version](https://gitlab.com/dashpay/dash/-/jobs/7819897536) ↓:
```
193/264 - feature_governance.py --legacy-wallet passed, Duration: 220 s
199/264 - feature_governance.py --descriptors passed, Duration: 240 s
```
Most of the time is spent in `feature_governance.py` on stage "let all fulfilled requests expire for re-sync to work correctly" so that's a very minor optimization.
## 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:
UdjinM6:
utACK 42dffe8541
PastaPastaPasta:
utACK 42dffe8541
Tree-SHA512: 9257301043efca8b3775fff64e598b153e00bc48779212e6237ab63f06a043a66764d72e38d9e6071b82477e55884b9bce410f4d3fabeb13da631505113e86bc
8320e0ca8e merge bitcoin#27411: Restrict self-advertisements with privacy networks to avoid fingerprinting (Kittywhiskers Van Gogh)
1376289b11 merge bitcoin#27467: skip netgroup diversity follow-up (Kittywhiskers Van Gogh)
a52b3a3bf0 merge bitcoin#27374: skip netgroup diversity of new connections for tor/i2p/cjdns (Kittywhiskers Van Gogh)
ab11e0f998 merge bitcoin#27324: bitcoin#27257 follow-ups (Kittywhiskers Van Gogh)
9023dd25af merge bitcoin#27257: End friendship of CNode, CConnman and ConnmanTestMsg (Kittywhiskers Van Gogh)
3465df2689 merge bitcoin#27264: Improve diversification of new connections (Kittywhiskers Van Gogh)
d3f5b3881b merge bitcoin#26888: simplify the call to vProcessMsg.splice() (Kittywhiskers Van Gogh)
d9e56f3e78 merge bitcoin#25962: Add CNodeOptions and increase constness (Kittywhiskers Van Gogh)
79e67fd96a merge bitcoin#25814: simplify GetLocalAddress() (Kittywhiskers Van Gogh)
6d4945418a partial bitcoin#25472: Increase MS Visual Studio minimum version (Kittywhiskers Van Gogh)
54bb3a438f merge bitcoin#25500: Move inbound eviction logic to its own translation unit (Kittywhiskers Van Gogh)
b50febc0f0 merge bitcoin#24531: Use designated initializers (Kittywhiskers Van Gogh)
Pull request description:
## Additional Information
* Dependent on https://github.com/dashpay/dash/pull/6254
* When backporting [bitcoin#27411](https://github.com/bitcoin/bitcoin/pull/27411), the `CNetAddr*` variant of `GetLocal()` was not removed (upstream it was replaced by the `CNode&` variant with additional checks that rely on fields in `CNode`) as `CActiveMasternodeManager` relies on `GetLocal()` to detect a valid external address.
* While it can also rely on other nodes to determine that, removing code that tests against a well-known public address would increase the number of reported failures esp. if the checks are run _before_ the node has a chance to connect to any peers.
## 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 8320e0ca8e
PastaPastaPasta:
utACK 8320e0ca8e
Tree-SHA512: 1d02bc33c8d62c392960d4dd044edf3de08515a5e8c8794d95cd95e9654da91b20e7290436cf9c79b0ea8dbd42b27dcc61c8eb17e573902574d7b281b8874584
925870d7d0 merge bitcoin#27015: bitcoin#26847 fixups (AddrMan totals) (Kittywhiskers Van Gogh)
Pull request description:
## Additional Information
As part of [dash#6254](https://github.com/dashpay/dash/pull/6254), [bitcoin#26847](https://github.com/bitcoin/bitcoin/pull/26847) was backported and since then, it was observed that unit tests were flakier than expected ([build](https://gitlab.com/dashpay/dash/-/jobs/7811041841), [build](https://gitlab.com/dashpay/dash/-/jobs/7802460298)).
The flakiness was caused by behavior introduced by the aforementioned backport, this was resolved upstream with [bitcoin#27015](https://github.com/bitcoin/bitcoin/pull/27015), which this pull request contains.
## 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 925870d7d0
knst:
ACK 925870d7d0
Tree-SHA512: 20fc8fb1b162803a71ec4087685460f52ed56c3c86d46ecac4cc0ef59c95b4b6206f0c53bef256242a4a5babb76e3564cfba56a84cbe844e187035de2308b818
The old `GetLocal()` has been moved to `masternode/node.cpp` due to its
use in determining a node's external address. We don't want the old
variant to be used otherwise so we'll move it out of `net.{cpp,h}` for
good measure.
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