315fcea834 chore: add builder key for kittywhiskers (Kittywhiskers Van Gogh)
Pull request description:
## Additional Information
Key ID `30CD 0C06 5E5C 4AAD`. Same key registered with Keybase ([source](https://keybase.io/kittywhiskers/pgp_keys.asc?fingerprint=969187a8e74fe40a8a48067430cd0c065e5c4aad)) and used to sign commits on GitHub. PGP key named after Keybase username.
ACKs for top commit:
UdjinM6:
utACK 315fcea834
knst:
ACK 315fcea834
PastaPastaPasta:
utACK 315fcea834
Tree-SHA512: f566c514831cfaf0a8bf95ebfb8aa5629474bdf0b88fd8948d4c1d3f1340ccdd3a9c67c817bd08d2f4d2e477b1599bf4fd148ad50fe68357d24feba651d832f7
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
fa3bdbd37be81b613e48a10aa77dfd3bcede61e1 test: remove unused sanitizer suppressions (MarcoFalke)
Pull request description:
Looks like those are not needed (anymore)
ACKs for top commit:
fanquake:
ACK fa3bdbd37be81b613e48a10aa77dfd3bcede61e1
Tree-SHA512: 4bedb6363aba8ea7763291ee0cd074e6bfd77e691bb32999c3959393864dc396bacba1eced2b10d9d600b66e8b83b91f7bc6692331dbd113bbaa87e72d11e2e8
fa2c991ec93bc72d276f0dcd747b3e57c246139b refactor: Use underlying type of isminetype for isminefilter (MarcoFalke)
Pull request description:
This does not change behavior, but it would be good for code clarity and to avoid `-Wimplicit-int-conversion` compiler warnings to use the an int of the same width for both `isminetype` and `isminefilter`.
ACKs for top commit:
laanwj:
Code review ACK fa2c991ec93bc72d276f0dcd747b3e57c246139b
shaavan:
crACK fa2c991ec93bc72d276f0dcd747b3e57c246139b
promag:
Code review ACK fa2c991ec93bc72d276f0dcd747b3e57c246139b.
Tree-SHA512: b3e255de7c9b1dea272bc8cb9386b339fe701f18580e03e997c270cac6453088ca2032e26e39f536d66cd1b6fda3e96bdbdc6e960879030e635338d0916277e6
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>
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>
c821ab8be8dffb749853c05e05cb515c11e6328a Use `GetAllOutputTypes` in `getblock` RPC function (Kiminuo)
d970a85d335202cc85f6604f794c43af6645673f Move `GetAllOutputTypes` function from `rpc/rawtransaction.cpp` to `rpc/util.{h|cpp}` (Kiminuo)
Pull request description:
This PR attempts to replicate 0ccf9b2e55/src/rpc/rawtransaction.cpp (L547) to one other place (at the moment) so that users have better idea what RPC methods can actually return.
I created this PR as a follow-up to the idea mentioned here https://github.com/bitcoin/bitcoin/pull/23320#discussion_r732458112 (resolved).
ACKs for top commit:
kristapsk:
re-ACK c821ab8be8dffb749853c05e05cb515c11e6328a
Tree-SHA512: 5ff66a41ad7c43ec769f4a99933d2d070feea7c617286d94b6f9bfa1a2547a42211915778210a89074ad4b14d99f34852cc6871efed5e6f1e2ffedd40d669386
9bdda50151dd808cbad094d457bf0ed7939a7c87 Enable TLS in links in documentation (Jeremy Rand)
Pull request description:
This PR enables TLS in several documentation links, which improves security.
ACKs for top commit:
fanquake:
ACK 9bdda50151dd808cbad094d457bf0ed7939a7c87
Tree-SHA512: 9d04d8771a9daf3c3b9914ff324e2eabfdf3ff5ae7f7dc92b84a1f3527010ceb860e73873a8f24d6051763eb472d9ea324ccbd6129a40318a520ca88c05f0586
faa75fa19335e3e826efa4f2280609a2db34425d Avoid unsigned integer overflow in bitcoin-tx (MarcoFalke)
Pull request description:
While `npos` means "largest unsigned value" and adding `1` to it yields `0`, it may be clearer to just assign `0` to it and only increment otherwise.
This also allows to remove a file-wide suppression for `unsigned-integer-overflow`.
ACKs for top commit:
hebasto:
ACK faa75fa19335e3e826efa4f2280609a2db34425d, I have reviewed the code and it looks OK, I agree it can be merged.
theStack:
Code-review ACK faa75fa19335e3e826efa4f2280609a2db34425d
Tree-SHA512: c24436641e5d801341c948b812c7f711d5dff70efdf04af00fd3221f4b81d93f25608dddaa36230ba81ca7ab0d18bdd957095d4561e22621e4d69017934f0a16
fa694f61ab21cb973843e5234a6aeacd4a957e05 doc: Explain that feedback needs to be addressed (MarcoFalke)
fa0819eea380047c9d7404895d3e4e8ea37930bb doc: Move peer-review paragraph to right section (MarcoFalke)
fa2b65b3583e7e7ffa892bf2453720b0d2bd7904 doc: Add link to release-process.md in CONTRIBUTING.md (MarcoFalke)
Pull request description:
Generally, the pull request author is expected to reply to all comments or iterate the code before merge. Of course, it is allowed to reject feedback, but it should not be done by silently ignoring it.
Clarify this in the docs.
Also, some minor copy edits.
ACKs for top commit:
michaelfolkson:
ACK fa694f61ab21cb973843e5234a6aeacd4a957e05
Sjors:
ACK fa694f61ab21cb973843e5234a6aeacd4a957e05
jamesob:
ACK fa694f61ab
prayank23:
ACK fa694f61ab
brunoerg:
ACK fa694f61ab21cb973843e5234a6aeacd4a957e05
w0xlt:
ACK fa694f6
Tree-SHA512: 339d6f252395664442f4bfb73d839314de8c9b0fdc8900a1c4a67b1cef9c73ecb98c7587a842dd5a36a4a3efbd8270d2162962013893706313f7ef34491db18c
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
fac49470ca36ff944a613f4358386bf8e0967427 doc: Fix incorrect C++ named args (MarcoFalke)
Pull request description:
Incorrect named args are source of bugs, like #22979.
Fix that by correcting them and adjust the format, so that clang-tidy can check it.
ACKs for top commit:
fanquake:
ACK fac49470ca36ff944a613f4358386bf8e0967427 - `run-clang-tidy` works for me now.
Tree-SHA512: 2694e17a1586394baf30bbc479a913e4bad361221e8470b8739caf30aacea736befc73820f3fe56f6207d9f5d969323278d43a647f58c3497e8e44cad79f8934
fa24a3df8796cbf4eeb35d950a4c848d605e5b22 rpc: Quote user supplied strings in error messages (MarcoFalke)
Pull request description:
I can't see a downside doing this and this fixes a fuzzing crash
Background:
This is a follow-up to commit 926fc2a0d4ff64cf2ff8e1dfa64eca2ebd24e090, which introduced the "starts_with-hack". Maybe an alternative to the hack would be to assign a unique error code to internal bugs? However, I think this can be done in an separate pull request and the changes here make sense even on their own.
ACKs for top commit:
fanquake:
ACK fa24a3df8796cbf4eeb35d950a4c848d605e5b22 - to fix the fuzzers.
Tree-SHA512: d998626406a64396a037a6d1fce22fce3dadb7567c2f9638e450ebe8fb8ae77d134e15dd02555326732208f698d77b0028bc62be9ceee9c43282b61fe95fccbd
01bff8f0494ba1c02c087b7186c892785f326fd9 qt: Fix WalletControllerActivity progress dialog title (Shashwat)
Pull request description:
Throughout the GUI, the title of the window, tells about the purpose of the window. This was not true for the title of wallet loading wallet.
This PR fixes this issue by renaming the wallet loading window title to 'Open Wallet'
Changes introduced in this PR (Runned Bitcoin-GUI on signet network)
|Master|PR|
|---|---|
|||
ACKs for top commit:
jarolrod:
ACK 01bff8f
hebasto:
ACK 01bff8f0494ba1c02c087b7186c892785f326fd9, tested on Linux Mint 20.2 (Qt 5.12.8).
Tree-SHA512: cd21c40752eb1c0afb5ec61b8a40e900bc3aa05749963f7957ece6024e4957f5bb37e0eb4f95aac488f5e08aea51fe13b023b05d8302a08c88dcc6790410ba64
0ab4c3b27265401c59e40adc494041927dc9dbe3 Return false on corrupt tx rather than asserting (Samuel Dobson)
Pull request description:
Takes up #19793
Rather than asserting, we log an error and return CORRUPT so that the user is informed. This type of error isn't critical so it isn't worth `assert`ing.
ACKs for top commit:
achow101:
ACK 0ab4c3b27265401c59e40adc494041927dc9dbe3
laanwj:
Code review ACK 0ab4c3b27265401c59e40adc494041927dc9dbe3
ryanofsky:
Code review ACK 0ab4c3b27265401c59e40adc494041927dc9dbe3. There may be room for more improvements later like better error messages or easier recovery options, but changing from an assert to an error seems like a clear improvement, and this seems to avoid all the pitfalls of the last PR that tried this.
Tree-SHA512: 4a1a412e7c473d176c4e09123b85f390a6b0ea195e78d28ebd50b13814b7852f8225a172511a2efb6affb555b11bd4e667c19eb8c78b060c5444b62f0fae5f7a
