5a8c321444c10c7c89c4222c0b47c2d83a1a9ea4 test: check for `getblocktxn` request with out-of-bounds tx index (Sebastian Falbesoner)
Pull request description:
This PR adds missing test coverage for the `getblocktxn` message handler, in the case that any of the contained indices is out-of-bounds:
a05876619a/src/net_processing.cpp (L2180-L2183)
ACKs for top commit:
dunxen:
ACK 5a8c321
Tree-SHA512: 2743c2c6d8aed57b22f825aefd60ba3e670321b60625a42ea7248e7b0fc41c73e9a5945153567c02824ba3b5f0fce7f4125bffc974973fc608b6ffbe49e14b65
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
42bbbba7c83d1e2baad18b4c6f05bad1358eb117 message-capture-parser: fix out of bounds error for empty vectors (Sebastian Falbesoner)
Pull request description:
The script [message-capture-parser.py](https://github.com/bitcoin/bitcoin/blob/master/contrib/message-capture/message-capture-parser.py) currently throws an "out of bounds" error if a message containing an empty integer vector element is tried to converted to JSON (e.g. by the BIP157 message `cfcheckpt` with empty `FilterHeaders` vector):
```
Traceback (most recent call last):
File "/home/honey/bitcoin/./contrib/message-capture/message-capture-parser.py", line 217, in <module>
main()
File "/home/honey/bitcoin/./contrib/message-capture/message-capture-parser.py", line 202, in main
process_file(str(capture), messages, "recv" in capture.stem, progress_bar)
File "/home/honey/bitcoin/./contrib/message-capture/message-capture-parser.py", line 162, in process_file
msg_dict["body"] = to_jsonable(msg)
File "/home/honey/bitcoin/./contrib/message-capture/message-capture-parser.py", line 85, in to_jsonable
elif slot in HASH_INT_VECTORS and isinstance(val[0], int):
IndexError: list index out of range
```
Fix this by using the `all(...)` predicate rather to access the first element `val[0]` (which in the error case doesn't exist).
ACKs for top commit:
laanwj:
Code review ACK 42bbbba7c83d1e2baad18b4c6f05bad1358eb117
Tree-SHA512: 139ec6b90304a69f26ec731e6f12b216fa10e554f777505b61adfa1e569f6861a4a849159dd1eae7a1aa0427e8598af226b6f0c4015020dcac8ab109fbc35dba
fb272dd8ff refactor: use NodeContext members instead of globals in interface logic (Kittywhiskers Van Gogh)
aba57cecb4 qt: add interface for CGovernanceObject querying (Kittywhiskers Van Gogh)
8d73cec438 refactor: reduce globals use in RPC and bench, use LLMQContext members (Kittywhiskers Van Gogh)
e8270ca91b refactor: use NodeContext members instead of globals in RPC logic (Kittywhiskers Van Gogh)
2efebcac81 refactor: use aliases of globals in unit tests (Kittywhiskers Van Gogh)
89f41fa826 refactor: dereference CDeterministicMNManager instance before use (Kittywhiskers Van Gogh)
d3dfdf353c refactor: use aliases of globals in initialization logic (Kittywhiskers Van Gogh)
bb7fe582f8 refactor: rename creditPoolManager to cpoolman in NodeContext, fix order (Kittywhiskers Van Gogh)
6f08e10540 refactor: remove extraneous creditPoolManager initialization (Kittywhiskers Van Gogh)
d136c407d7 refactor: add aliases for Dash-specific global pointers in NodeContext (Kittywhiskers Van Gogh)
Pull request description:
## Additional Notes
* In some limited circumstances, we need to pass a reference to a smart pointer _before_ it is initialized. This is permissible if we know for sure that the pointer will be dereferenced _after_ it is initialized (usually, by `assert`'ing `!= nullptr`). This requires us to pass the const ref of the smart pointer object, to ensure that when dereferenced, we get access to the updated value that is set later on the initialization sequence.
This means we cannot use the bare pointer alias as an argument as it would remain `nullptr` and will not be updated.
Currently known examples are:
* `CChainState` through `ChainstateManager::InitializeChainstate`
* `llmq::CChainLocksHandler` (as `llmq::chainLocksHandler`)
* `llmq::CInstantSendManager` (as `llmq::quorumInstantSendManager`)
* `llmq::CQuorumBlockProcessor` (as `llmq::quorumBlockProcessor`)
* `CDSNotificationInterface`
* `CDeterministicMNManager` (as `deterministicMNManager`)
* `CJContext` (as `NodeContext::cj_ctx`)
* `LLMQContext` (as `NodeContext::llmq_ctx`)
* We can verify the absence of globals in GUI, interface, RPC and test logic with the commands below (the above caveat applies)
```bash
$ function make_filter() { echo "$1->|\*$1|::$1"; }
$ PROHIBITED_TERMS="$(make_filter creditPoolManager)|$(make_filter deterministicMNManager)|$(make_filter dstxManager)|$(make_filter governance)|$(make_filter mmetaman)|$(make_filter netfulfilledman)|$(make_filter sporkManager)";
$ PROHIBITED_LLMQ_TERMS="$(make_filter quorumBlockProcessor)|$(make_filter quorumManager)|$(make_filter chainLocksHandler)|$(make_filter quorumInstantSendManager)";
$ grep -E "$PROHIBITED_TERMS|$PROHIBITED_LLMQ_TERMS" src/bench/*.{cpp,h} src/node/interfaces.cpp src/qt/*.{cpp,h} src/qt/test/*.{cpp,h} src/rpc/*.{cpp,h} src/test/*.{cpp,h} src/wallet/rpc*.{cpp,h};
src/test/validation_chainstate_tests.cpp: CChainState& c1 = WITH_LOCK(cs_main, return manager.InitializeChainstate(&mempool, *m_node.mnhf_manager, *m_node.evodb, llmq::chainLocksHandler, llmq::quorumInstantSendManager, llmq::quorumBlockProcessor));
src/test/validation_chainstatemanager_tests.cpp: CChainState& c1 = WITH_LOCK(::cs_main, return manager.InitializeChainstate(&mempool, *m_node.mnhf_manager, evodb, llmq::chainLocksHandler, llmq::quorumInstantSendManager, llmq::quorumBlockProcessor));
src/test/validation_chainstatemanager_tests.cpp: &mempool, *m_node.mnhf_manager, evodb, llmq::chainLocksHandler, llmq::quorumInstantSendManager, llmq::quorumBlockProcessor,
src/test/validation_chainstatemanager_tests.cpp: CChainState& c1 = WITH_LOCK(cs_main, return manager.InitializeChainstate(&mempool, *m_node.mnhf_manager, evodb, llmq::chainLocksHandler, llmq::quorumInstantSendManager, llmq::quorumBlockProcessor));
src/test/validation_chainstatemanager_tests.cpp: CChainState& c2 = WITH_LOCK(cs_main, return manager.InitializeChainstate(&mempool, *m_node.mnhf_manager, evodb, llmq::chainLocksHandler, llmq::quorumInstantSendManager, llmq::quorumBlockProcessor, GetRandHash()));
src/test/validation_flush_tests.cpp: CChainState chainstate(&mempool, blockman, *m_node.mnhf_manager, *m_node.evodb, llmq::chainLocksHandler, llmq::quorumInstantSendManager, llmq::quorumBlockProcessor);
```
* We can also check the usage of globals in (test) initialization logic (it will also include all the actual (de)initialization logic and any usage of the bare pointer alias if the name of the global and the alias are the same, as is the case with `netfulfilledman`)
```bash
$ grep -E "$PROHIBITED_TERMS|$PROHIBITED_LLMQ_TERMS" src/init.cpp src/test/util/setup_common.cpp;
src/init.cpp: ::netfulfilledman.reset();
src/init.cpp: ::mmetaman.reset();
src/init.cpp: ::dstxManager.reset();
src/init.cpp: ::sporkManager.reset();
src/init.cpp: ::governance.reset();
src/init.cpp: assert(!::governance);
src/init.cpp: ::governance = std::make_unique<CGovernanceManager>();
src/init.cpp: node.govman = ::governance.get();
src/init.cpp: assert(!::sporkManager);
src/init.cpp: ::sporkManager = std::make_unique<CSporkManager>();
src/init.cpp: node.sporkman = ::sporkManager.get();
src/init.cpp: *node.connman, *node.mn_sync, ::deterministicMNManager, *node.govman, node.llmq_ctx, node.cj_ctx
src/init.cpp: chainman.InitializeChainstate(Assert(node.mempool.get()), *node.mnhf_manager, *node.evodb, llmq::chainLocksHandler, llmq::quorumInstantSendManager, llmq::quorumBlockProcessor);
src/init.cpp: assert(!::dstxManager);
src/init.cpp: ::dstxManager = std::make_unique<CDSTXManager>();
src/init.cpp: node.dstxman = ::dstxManager.get();
src/init.cpp: assert(!::mmetaman);
src/init.cpp: ::mmetaman = std::make_unique<CMasternodeMetaMan>(fLoadCacheFiles);
src/init.cpp: node.mn_metaman = ::mmetaman.get();
src/init.cpp: assert(!::netfulfilledman);
src/init.cpp: ::netfulfilledman = std::make_unique<CNetFulfilledRequestManager>(fLoadCacheFiles);
src/init.cpp: node.netfulfilledman = ::netfulfilledman.get();
src/init.cpp: if (!node.netfulfilledman->IsValid()) {
src/test/util/setup_common.cpp: ::deterministicMNManager = std::make_unique<CDeterministicMNManager>(chainstate, *node.connman, *node.evodb);
src/test/util/setup_common.cpp: node.dmnman = ::deterministicMNManager.get();
src/test/util/setup_common.cpp: ::deterministicMNManager.reset();
src/test/util/setup_common.cpp: ::sporkManager = std::make_unique<CSporkManager>();
src/test/util/setup_common.cpp: m_node.sporkman = ::sporkManager.get();
src/test/util/setup_common.cpp: ::governance = std::make_unique<CGovernanceManager>();
src/test/util/setup_common.cpp: m_node.govman = ::governance.get();
src/test/util/setup_common.cpp: ::dstxManager = std::make_unique<CDSTXManager>();
src/test/util/setup_common.cpp: m_node.dstxman = ::dstxManager.get();
src/test/util/setup_common.cpp: ::mmetaman = std::make_unique<CMasternodeMetaMan>(/* load_cache */ false);
src/test/util/setup_common.cpp: m_node.mn_metaman = ::mmetaman.get();
src/test/util/setup_common.cpp: ::netfulfilledman = std::make_unique<CNetFulfilledRequestManager>(/* load_cache */ false);
src/test/util/setup_common.cpp: m_node.netfulfilledman = ::netfulfilledman.get();
src/test/util/setup_common.cpp: ::netfulfilledman.reset();
src/test/util/setup_common.cpp: ::mmetaman.reset();
src/test/util/setup_common.cpp: ::dstxManager.reset();
src/test/util/setup_common.cpp: ::governance.reset();
src/test/util/setup_common.cpp: ::sporkManager.reset();
src/test/util/setup_common.cpp: m_node.chainman->InitializeChainstate(m_node.mempool.get(), *m_node.mnhf_manager, *m_node.evodb, llmq::chainLocksHandler, llmq::quorumInstantSendManager, llmq::quorumBlockProcessor);
```
## Breaking Changes
None, changes are limited to refactoring and do not logically change behaviour.
## 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 **(note: N/A)**
- [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_
Top commit has no ACKs.
Tree-SHA512: f6d1a668728525e7cab7af200858a6ab6e20a24786d691630530a9019e8d2b4f474ed4358f40b76266b938dfe96420e0c64ee66a1a0986a3d3f254670162bb1c
The GetAbsoluteYesCount -> Get{Yes,No}Count -> CountMatchingVotes call
chain eventually results in needing to call GetListAtChainTip from the
CDeterministicMNManager instance.
Currently it's being accessed as a global but future deglobalization
will move fetching the CDeterministicMNList outside CGovernanceObject,
requiring an instance of CDeterministicMNManager.
In preparation of that, these changes are being made in advance.
We must pass ::deterministicMNManager to CDSNotificationInterface as
we are passing a const ref of the smart pointer object, because it is
init'ed further down the line.
The alias in NodeContext is a raw pointer that is assigned after init
and is unsuitable for this purpose.
All members within NodeContext are lower-case and the bare pointer should
be null'ed before the smart pointer is, doing it in reverse creates a
small gap where the bare pointer is directing to freed space.
creditPoolManager is already initialized in BasicTestingSetup, parent
of ChainTestingSetup. This means that creditPoolManager is being init'ed
twice, once in the parent's constructor and again in the child's
constructor.
75d81fd2c0 refactor: use atomic to avoid blocking chainlocks cs on each call to cleanup (pasta)
Pull request description:
## Issue being fixed or feature implemented
Avoid needing to lock CS on each call to cleanup. Cleanup only gets called once every 5 seconds it seems; but maybe that'll change in the future.
## What was done?
Make `lastCleanupTime` atomic instead of CS guarded
## How Has This Been Tested?
Building
## Breaking Changes
None
## 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: 587dbfbecce430c25b4d572b2e158fcda86aced3db976c16166c0cabcf960f46d300739bbf54644def30769347cc88ac14c58d71dd78d848f7d8af562cd12bc6
a50f20d779 backport: partial Merge bitcoin-core/gui#96: Slight improve create wallet dialog (Konstantin Akimov)
Pull request description:
## Issue being fixed or feature implemented
It fixes strange behaviour of enable/disable checkboxes and checking/unchecking them
kudos to thephez to found an issue.
## What was done?
Partial backport bitcoin-core/gui#96
## How Has This Been Tested?
Build & run `dash-qt`
## Breaking Changes
No actual code changed for creating wallet, only UI for it
## 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
Top commit has no ACKs.
Tree-SHA512: 7b16d33af181e44ef5b77dbb0eabb3a435e5f0b3c0a5a5c4ce17c95bb5949a5a41d605f7e6100d0f994b50edf087b777cbed6fe6d29952c2446ac9f8229f1a3a
39db1fbabb fix: adjust verify-binaries script to properly handle RCs (pasta)
Pull request description:
## What was done?
Adjusted verify.py to properly handle RC
## How Has This Been Tested?
Ran verify on both 20.0.4 and 20.1.0-rc.1
## Breaking Changes
None
## 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
Top commit has no ACKs.
Tree-SHA512: 894261004fb89588c8c01c67a95dd999a03657dc68895af566c8dd98deb82fbc5b5847382b052eb5a28284aba810052a5ee8d21ad3efeb9af6557302759a6e35
097a8e7196 non-scripted-diff: bump copyright year to 2023 (Konstantin Akimov)
a8e0f54e27 fix: wrong copyright for dash files (Konstantin Akimov)
Pull request description:
## Issue being fixed or feature implemented
Bump copyright year to 2023-2024
## What was done?
run `copyright_header.py report` and `copyright_header.py update`
previous work: https://github.com/dashpay/dash/pull/5160/files
## How Has This Been Tested?
Please, notice, that copyrights should be updated at the end of year, not at the beginning of year for next years.
That prevent bumping year for files where is only changes in that year - bumping a copyright. This misbehavior can be validated by running `copyright_header.py update` twice. So, bump 2024 should be done in December 2024, not in 2025.
## 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
Top commit has no ACKs.
Tree-SHA512: 7cc2a37bd3852456fc686ed02b1cc137a9d736bbc0d3957208d9c52ae76b3233f18404f47ebf916d800fa24b7e56c32ff0324c7e0c282b0fb22c67e831ef2c9c
that's a result of:
contrib/devtools/copyright_header.py update ./
it is not scripted diff, because it works differentlly on my localhost and in CI:
CI doesn't want to use git commit date which is mocked to 30th Dec of 2023
37589507e5 feat: migrate to a CA issued certificate (pasta)
Pull request description:
## Issue being fixed or feature implemented
Uses a new CA issued certificate
## What was done?
Certificate changed; the issuer according to the certificate is `GoGetSSL G4 CS RSA4096 SHA256 2022 CA-11` In reality, the CA is digicert and the root CA is `DigiCert Trusted Root G4` and is issued to "Dash Core Group, Inc."
## How Has This Been Tested?
Signed a binary with it see: https://www.virustotal.com/gui/file/a6577f3b8b474cfb1def1ea339795cb229b26d248d71f0b6f0b65e9e9ba3411b/details. I can share the binary if you'd like.
## Breaking Changes
Not really any; unless someone is relying on a specific certificate.
## Signature
```
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256
8154f5a92ffe1124ad1573487f6e1842fe28eed7a455416c82fc6211253117ed contrib/windeploy/win-codesign.cert
-----BEGIN PGP SIGNATURE-----
iQIzBAEBCAAdFiEEKVkDYuyHioH9PCArUlJ77avoeYQFAmXWkZEACgkQUlJ77avo
eYSU2Q//fa6yzHrjW9cjsuxyRn/b+t7ry9lplzHtQDqxADxq5ANhddfurDWKOVNc
Fu3iiK1iCtdaBMDSvlAW9/nWXaJaMSqkCdExaCS63EDyItPRSwiEmCIMe1EzR1Ed
0AWrnWYIRk27huQ3RbCcnsQUEoMI3DbBMDN19ClMzyMtFiTjQfIgB0WO7OLsI785
ZXx2dqcrRYSkDd72YYfuoamJPxFXC9+kOa1Ss531w4F/86jIG+VnqVuH6L0iPkJ1
HM5jF2jLDCiU8eFN/ANsCvC65+a0nSq+xKJLeqSen0KwAPy/lBAcqY9VduvDpAyE
a6gWDhTEbYGzQicVTS3EDp0360Y0UOKixuLibzD/8Vlhk0stgEyOwoeKbNj6vRt4
MuKjAsmvh/WpuYXHmycJU1pJZ1V/udyjmpSRsr8+xB6Ww5T/epu2OJ3Yjsyny5P+
ncVtQfO18rAVIRKb6zWBygz1ITqdU23bVHApp+3pwbZTQP1ilIMRadTX7cMic6/a
eKc8jEW6BWYiXNjCOu3PPOE/P9ML537LqSg4XJXyxdemmJofzcPb1HMpngwmdQHO
jm4tOzAFwe/rdISpthED1zfgLAuWiIYNstCtzdTeDizLOfYICe8kPBjsc3zaZqCx
Cj8qlV/BqvXpua+KpuDLHa11n3amN+trNd5oOb6ewZmJqxLo4NM=
=d3Bu
-----END PGP SIGNATURE-----
-----BEGIN PGP SIGNATURE-----
iQEzBAEBCAAdFiEEYKz3C/cSZFBJ7m8V7+rxZoYiX2QFAmXWkZsACgkQ7+rxZoYi
X2QOOwf/YUm2TPXtrxOT7/CJVTITQa3u0XMwJ6JADuHtJaXlmA9b9ucz2cSKaFdC
noA1q8FFWbMUDM9+RkudAjhO5JCLMkJrfCK/mG2FlZrXV/61lounxyRdMerxk9qF
8/iJS1cf6t3tE4/SziMgo3uCZf5tsORIiNRxdEhBITVmDenPNTpKKGetijH7gr7o
lRHP5lNNPWH629eFqFtVORYiyvFOtSMV2PfG+lauBnm8DMoHIXC+Zs3lT6Soes+L
hxli9z+/ZGnQdHHIFIM+qcQNKIiEoWb4iUmsMK2couGk6beoWZNUmEpTjzubEfbN
QU252WZBsdGwQ/NEr5Gi4rNeUSfvig==
=hlUn
-----END PGP SIGNATURE-----
```
```
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256
8154f5a92ffe1124ad1573487f6e1842fe28eed7a455416c82fc6211253117ed win-codesign.cert
5dcc037bb56205a8744c99d1ba15e8fc7e64144ba3ce728ce57546d804469ea1 win-codesign.cert.1
-----BEGIN PGP SIGNATURE-----
iQIzBAEBCAAdFiEEKVkDYuyHioH9PCArUlJ77avoeYQFAmXXbrAACgkQUlJ77avo
eYSItRAAtVeQDgfl2Blxya6MuvnKJiyS9W2fSfjTchUo5zmhqcbhTLcCVh9JG25T
ebpNRmf5GFzCIVDc3SRj9EuZ4SjRyf4ZcRBPq25+Q6IJUeI5S+W+v09LQ3k7nE44
g9lgHVh1KDKzxmCiEoavQ5UhND0pi9A7HreDXS6Ntc84BDgHCOAF+7tGyq6APP8y
LU35zlv6iJJcy+fCm/UspHQVYIsyShrMYNlOarK/6ZuBoCEAjPWU4L3fsaYIwxfU
6JQyhKRe3XWMcOxbb3a0ZX1wTZNsFYl5o/1guTOwH6WUPIzOdYCQEkA3+z9fNs6n
IZOzB8UdTx4Uhig8zxIlVw1EgBzvIjxbIqfBiCzKLNeEK8YFmQrmwemKtdOzPU0O
dnKDxSvQirJ/YxESJF9zyzplBIGxHluT3xTAY3JdcEYs+Xc/RKmb6eGaP5MAO8z6
lOVLbDObBMOvoPWHZaCoykGJvFvF9YXR0MpbxPtCqnpK6kTXU9z+qPDGkMLJ7pUj
X/kZneW2Mril+t7g9CEqKKQcUkdf4MoonddDGdQ2fbfrYpEJ+b5jsUbI4XkbhCwl
4KmAaBx7y1Zmxt83o2aSm1mNu5B687f8bUNOd7twE42sbsJ6w/YyKB7/JRbhIPDg
vEHn9omSA2XHGxcteFA1LBbKjDvhKsCENIY64+lwOxv8fJ4XEiE=
=TGa5
-----END PGP SIGNATURE-----
```
## 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: 36af24773bb7897332fb7703e7509903e945e6033637ec07acb6e6985dc1f05e41d51776fa4db5a7d4be012b7faa3e89e8ed3c2e8079ebb173cecf372e972397
96ac317c27 Merge #19104: gui, refactor: Register Qt meta types in application constructor (Jonas Schnelli)
32f717c015 Merge #19111: Limit scope of all global std::once_flag (MarcoFalke)
7a6667f323 Merge #18452: qt: Fix shutdown when waitfor* cmds are called from RPC console (Jonas Schnelli)
Pull request description:
## Issue being fixed or feature implemented
## What was done?
## How Has This Been Tested?
CI passing
## 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: a1a9d18674b7c55549edd684211bb9de983a67876ed292d90ef3dccb126122af9b5d8c887608bbce74b7fa92c020a542d2ea1acdc693d85eba889aa8c56ac8df
4f49d5222eca11c149713ad34113d5a3d1c577b1 gui, refactor: Register Qt meta types in application constructor (João Barbosa)
Pull request description:
Removes a warning when running `QT_QPA_PLATFORM=cocoa src/qt/test/test_bitcoin-qt`.
ACKs for top commit:
jonasschnelli:
Re utACK 4f49d5222eca11c149713ad34113d5a3d1c577b1
hebasto:
ACK 4f49d5222eca11c149713ad34113d5a3d1c577b1, tested on macOS 10.15.5.
Tree-SHA512: e931a022ba83cb0ef04d82544ebd9b18242f8fc2b41443afce4d5c4222f222e8b3517bdb484a1a4f61377c5dceca067d8ccf250da3a727299448e54bec33ed6e
fa9c67559186f5416c1c0b26c0a1d5e72c234ccb Limit scope of all global std::once_flag (MarcoFalke)
Pull request description:
`once_flag` is a helper (as the name might suggest) to execute a callable only once. Thus, the scope of the flag does never need to extend beyond where the callable is called. Typically this is function scope.
Move all the flags to function scope to
* simplify code review
* avoid mistakes where similarly named flags are accidentally exchanged
* avoid polluting the global scope
ACKs for top commit:
hebasto:
ACK fa9c67559186f5416c1c0b26c0a1d5e72c234ccb, tested on Linux Mint 19.3 (x86_64).
promag:
Code review ACK fa9c67559186f5416c1c0b26c0a1d5e72c234ccb.
Tree-SHA512: 095a0c11d93d0ddcb82b3c71676090ecc7e3de3d5e7a2a63ab2583093be279242acac43523bbae2060b4dcfa8f92b54256a0e91fbbae78fa92d2d49e9db62e57
da73f1513a637a9f347b64de66564d6cdb2541f8 qt: Fix shutdown when waitfor* cmds are called from RPC console (Hennadii Stepanov)
Pull request description:
On master (7eed413e72a236b6f1475a198f7063fd24929e23), if the GUI has been started with`-server=1`, `bitcoin-qt` hangs on shutdown during calling any of the `waitfor*` commands in the GUI RPC console.
This PR suggests minimal changes to fix this bug.
Fix#17495
ACKs for top commit:
jonasschnelli:
utACK da73f1513a637a9f347b64de66564d6cdb2541f8
Tree-SHA512: 469f5332945a5f2c57d19336cda5df79b123ccc494aea6d58a85eb1293be52708b2b9c5bb6bc2c402a90b7b4e9e8d7ab8fe84cf201cf7ce612c9290c57e43681
99402584d7 Merge bitcoin/bitcoin#25332: build: test for timingsafe_bcmp (laanwj)
ca4b0fe4d6 Merge bitcoin/bitcoin#25320: util: modify Win32LockedPageAllocator to query windows for limit. (laanwj)
edaf9cc646 Merge bitcoin/bitcoin#25359: doc: add distcc to productivity notes (laanwj)
c1fa5a0f15 Merge bitcoin/bitcoin#25312: test: Fix port collisions caused by p2p_getaddr_caching.py (MacroFake)
aaa83ae043 Merge bitcoin/bitcoin#25165: doc: Explain squashing with merge commits (laanwj)
ef13101d2b Merge bitcoin/bitcoin#25210: doc: remove misleading AreInputsStandard() comment (laanwj)
0a847fd980 Merge bitcoin/bitcoin#25046: build: Fix `libmultiprocess` cross-compiling to Linux hosts (fanquake)
82eec21fee Merge bitcoin/bitcoin#24754: build: specify cmake build dir for multiprocess depends build (fanquake)
a91512e1d4 Merge bitcoin/bitcoin#24574: test: Actually print TSan tracebacks (fanquake)
2e01c11146 Merge bitcoin-core/gui#568: options: flip listenonion to false if not listening (Hennadii Stepanov)
200d2d58dc Merge bitcoin/bitcoin#24385: build: remove boost dep from libmultiprocess (MarcoFalke)
28ed1a23ac Merge bitcoin/bitcoin#24316: ci: Rename Cirrus CI osx_instance to macos_instance (MarcoFalke)
8d3eceacc0 Merge bitcoin/bitcoin#23130: doc: Revert "Remove outdated comments" and place comment correctly (W. J. van der Laan)
5fbdfa1163 Merge bitcoin/bitcoin#23094: doc: Remove outdated comments (W. J. van der Laan)
Pull request description:
## Issue being fixed or feature implemented
Small batch of trivial backports
## What was done?
_Describe your changes in detail_
## How Has This Been Tested?
Previous versions had been build and tested locally I believe. Please wait for CI
## 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: 0e3cfb7aab0a6373cf4167ae18849262fc4a5a62c0d301f558c7efd7c5db5a18e025aec0eba7b642829b2102133f92eda15aa5263319c65d158c2dd1506a13a2
1cb42aeda37f4979923cd7e1c85febe994480de6 util: modify Win32LockedPageAllocator to query windows for limit (Oskar Mendel)
Pull request description:
This PR resolves a todo within the Win32LockedPageAllocator: `// TODO is there a limit on Windows, how to get it?`.
The idea is to use the Windows API to get the limits like the posix based allocator does with `getrlimit`.
I use [GetProcessWorkingSetSize](https://docs.microsoft.com/en-us/windows/win32/api/memoryapi/nf-memoryapi-getprocessworkingsetsize) to perform this task and fallback to `return std::numeric_limits<size_t>::max();` just like the posix implementation does.
ACKs for top commit:
sipsorcery:
tACK 1cb42aeda37f4979923cd7e1c85febe994480de6.
Tree-SHA512: 7bdd8a57a4e64ee59d752417a519656e03526878462060753be4dce481eff4889fb5edc1bdbd575b707d9b2dfe255c87da9ef67baac97de9ac5e70a04c852081
14093d5d243f6eb9cfef721c80f92848d95032ee doc: add distcc to productivity notes (Sjors Provoost)
Pull request description:
If you have more than one computer at your disposal, you can use [distcc](https://www.distcc.org) to speed up compilation.
ACKs for top commit:
laanwj:
ACK 14093d5d243f6eb9cfef721c80f92848d95032ee
brunoerg:
ACK 14093d5d243f6eb9cfef721c80f92848d95032ee
w0xlt:
ACK 14093d5d24
Tree-SHA512: 2c436bdea5ab750330055778eb5817361d16b046f219d53692577439e2fd8403febf78ac8e8b20ed158c650c76252b50cfc91f4ec8375cdd522cc408068d547b
ea54ba2f42f6d0b23570c665c2369f977bf55cf6 [test] Fix port collisions caused by p2p_getaddr_caching.py (dergoegge)
f9682e75ac184a62c7e29287882df34c25303033 [test_framework] Set PortSeed.n directly after initialising params (dergoegge)
Pull request description:
This PR fixes the issue mentioned [here](https://github.com/bitcoin/bitcoin/pull/25096#discussion_r892558783), to avoid port collisions between nodes spun up by the test framework.
Top commit has no ACKs.
Tree-SHA512: ec9159f0af90db636f7889d664c24e1430cf2bcb3c02a9ab2dcfe531b2a4d18f6e3a0f8ba73071bdf2f7db518df9d5d86a9cd06695e67644d20fe4515fac32b7
fa2d226ac950d8b4f7e430732f13ad408c504745 doc: Explain squashing with merge commits (MacroFake)
Pull request description:
This avoids having to explain it in each thread
ACKs for top commit:
laanwj:
ACK fa2d226ac950d8b4f7e430732f13ad408c504745
Tree-SHA512: e1533ee7c0ab0101c78aaebed97dc889b5eb941cf4c2dfbabbb5f0ec1bb7b1313a1a2e2405235d68c761f039373cebac67ce691a72c820a9252429d50c1ac7d5
be6d4315c150646cf672778e9232f086403e95df doc: remove misleading AreInputsStandard() comment (James O'Beirne)
Pull request description:
This check isn't any longer just about bad pay-to-script-hash inputs; it
also excludes any kind of nonstandard input, unknown witness versions,
coinbases, etc.
ACKs for top commit:
laanwj:
ACK be6d4315c150646cf672778e9232f086403e95df
dunxen:
ACK be6d431
jonatack:
ACK be6d4315c150646cf672778e9232f086403e95df
Tree-SHA512: 1c4befadff6a7b5789901ca2a2cc39adc35c688f7e3c093ab5292123f9193ce078731016b773b3d05f7004ff01ee62f23f8362ae8d05134d41dc097ba094a42b
c0f5cc14ef9fae2b2de4222ee061729629ebb6b4 build: Fix `libmultiprocess` cross-compiling to Linux hosts (Hennadii Stepanov)
Pull request description:
To successfully call the [`capnp_generate_cpp()`](d576d975de/CMakeLists.txt (L45)) function, the `libmultiprocess` build system must be provided with paths to the native `capnp` and `capnpc-c++` tools.
This [comment](https://github.com/bitcoin/bitcoin/issues/24387#issuecomment-1054776195) points the same:
> I think `packages/libmultiprocess.mk` probably needs to be passing a `-DCAPNP_EXECUTABLE=.../depends/arm-linux-gnueabihf/native/bin/capnp` argument to cmake. Also the package should have dependencies on both `capnp` and `native_capnp`.
Fixesbitcoin/bitcoin#24387.
ACKs for top commit:
ryanofsky:
Code review ACK c0f5cc14ef9fae2b2de4222ee061729629ebb6b4
Tree-SHA512: 2986d8bf98d2761eceba21b1897145c5185a0922d4c2084e8812d4d07dc94237e5c2809036641c4f7c491a3414727fff328cba91ce138b89e37ec5cba61d8f61
7c218dacd0e9602b8f755be42e96c49706f96305 build: specify cmake build dir for multiprocess depends build (fanquake)
Pull request description:
When no build dir is specified, cmake will warn:
```bash
Preprocessing libmultiprocess...
Configuring libmultiprocess...
CMake Warning:
No source or binary directory provided. Both will be assumed to be the
same as the current working directory, but note that this warning will
become a fatal error in future CMake releases.
```
It's unclear if this will actually ever become an error, but it's also easy
enough to just supply the directory, and save this maybe breaking in
future.
ACKs for top commit:
ryanofsky:
Code review ACK 7c218dacd0e9602b8f755be42e96c49706f96305. I guess the purpose of the warning is to encourage people not to build in the source directory, but reasons for encouraging this don't really apply to the depends build system, so it is appropriate to disable the warning.
hebasto:
ACK 7c218dacd0e9602b8f755be42e96c49706f96305, tested on Ubuntu 22.04.
Tree-SHA512: 6904f2095fe62cead4abc644ec888c5d836e54a3c0b2a84c467029116e5d14eba35190570acaa23c6831aed9a4a65898134480cc46cdb141279ec0dc6f534d5f
fa76d8d4d71d844e217686881d4f630eac3a8e10 test: Actually print TSan tracebacks (MarcoFalke)
Pull request description:
Commit 5e5138a721738f47053d915e4c65f925838ad5b4 made the TSan logs to be printed before returning an error from the ci script.
However, it seems that on Cirrus CI, the `--failfast` option will kill not only all python process and bitcoind child process, but also the parent CI bash script, rendering the `trap` inefficient. I believe this bug was introduced in commit 451b96f7d2796d00eabaec56d831f9e9b1a569cc.
ACKs for top commit:
fanquake:
utACK fa76d8d4d71d844e217686881d4f630eac3a8e10
Tree-SHA512: 686f889d38a343882cb62ad6e0c2080196330e7cc7086891a7ff66d9443b455c82ba8d7e4a5cc42daa0513b0ad2743055bfe90e2f6ac88a910ee3b663fabddcd
7f90dc26c8938f348938929b6d8bf1ea6f149209 options: flip listenonion to false if not listening (Vasil Dimov)
Pull request description:
If the user has unchecked "Allow incoming connections" in
`Settings->Options...->Network` then `fListen=false` is saved in
`~/.config/Bitcoin/Bitcoin-Qt.conf`. This flips `-listen` to `false`
during startup, but leaves `-listenonion` to `true`.
This flipping of `-listen` is done in `OptionsModel::Init()` after
`InitParameterInteraction()` has been executed which would have flipped
`-listenonion`, should it have seen `-listen` being `false`
(this is a difference between `bitcoind` and `bitcoin-qt`).
Fixes: https://github.com/bitcoin-core/gui/issues/567
ACKs for top commit:
mzumsande:
Tested ACK 7f90dc26c8938f348938929b6d8bf1ea6f149209
hebasto:
ACK 7f90dc26c8938f348938929b6d8bf1ea6f149209
jonatack:
utACK 7f90dc26c8938f348938929b6d8bf1ea6f149209
ryanofsky:
Code review ACK 7f90dc26c8938f348938929b6d8bf1ea6f149209.
Tree-SHA512: ff5095096858eae696293dc58d1cd5bd1bb60ef7c5d07d87308a0cf71c67da88cc00b301b550704625f136c4ba3a29905a934a766535a6422fe85d9662299d32
07dcf1a76e34a6f7c919e7d5c57fa61caea6007b build: remove boost dep from libmultiprocess (fanquake)
Pull request description:
Looks like this hasn't been needed since https://github.com/chaincodelabs/libmultiprocess/pull/25 and was just missed in #19160.
ACKs for top commit:
ryanofsky:
Code review ACK 07dcf1a76e34a6f7c919e7d5c57fa61caea6007b. Should probably wait for GUIX build results, but I think this should be fine
hebasto:
ACK 07dcf1a76e34a6f7c919e7d5c57fa61caea6007b
Tree-SHA512: 7988efd4aaf6ad512d60cfd33f350df56090daf88aac3aed2a1d400e80bc723dc27d27f5fa5d75359f9fae60d04b87d4b120d4e79e3079df8631956ab6c3b83c
8ff3743f5e99c693710bc446bfd595687156ca6b Revert "doc: Remove outdated comments" (Hennadii Stepanov)
Pull request description:
Unfortunately, in #23094 the assumption that #14336 makes comments outdated is wrong. As pointed in https://github.com/bitcoin/bitcoin/pull/23094#discussion_r717226839, the #14336 just moved the relevant code a few lines down.
This PR reverts commit ee7891a0c412728cf8bec667f25263682a9baaaf, and moves the comments into the right place.
I apologize about that.
ACKs for top commit:
MarcoFalke:
cr ACK 8ff3743f5e99c693710bc446bfd595687156ca6b
laanwj:
ACK 8ff3743f5e99c693710bc446bfd595687156ca6b
Tree-SHA512: 84aca627bb5b49c06fc172778f9b9407482c5a873ccbc3dc40167e6a8ad0bc60475d6a469c843b7b42712e35cf3fc2d3518923e791d5e0c59628e042acc72747
ee7891a0c412728cf8bec667f25263682a9baaaf doc: Remove outdated comments (Hennadii Stepanov)
Pull request description:
The first removed comment was introduced in #5288, the second one in #13503.
Both are outdated since #14336.
ACKs for top commit:
duncandean:
crACK ee7891a0
Tree-SHA512: a2d6071919e81c916bfc2178109bbc464417321bcc567ed0644448c5faea8e58cb08a7657afa1b6ffe1fb63e114a2a47b31c893e471839ba9d49a3986e68b2a7
## What was done?
Happy new year and happy new lunar year!
2024 is here and 20.1 is coming.
## 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
6c07c2c80c Merge #19858: Periodically make block-relay connections and sync headers (MarcoFalke)
5e23f3506c Merge #20561: p2p: periodically clear m_addr_known (Wladimir J. van der Laan)
6abbbe12c7 Merge #20138: net: Assume that SetCommonVersion is called at most once per peer (MarcoFalke)
5154fa0ebf Merge #19847: rpc, refactor: Avoid duplicate set lookup in gettxoutproof (Jonas Schnelli)
ea828164e6 Merge #18948: qt: Call setParent() in the parent's context (Jonas Schnelli)
Pull request description:
Bitcoin Backports
Top commit has no ACKs.
Tree-SHA512: 369ce49a510b77d05aeeb50612ff827b0c994a1511ff72dadeb71f6a115a9624b0d08292119ba8387b71e4ba7b679c6ce23aba03ac8f849f68ed93f6d061f29e
b3a515c0bec97633a76bec101af47c3c90c0b749 Clarify comments around outbound peer eviction (Suhas Daftuar)
daffaf03fbede6c01287779e464379ee3acb005a Periodically make block-relay connections and sync headers (Suhas Daftuar)
3cc8a7a0f5fa183cd7f0cf5e56f16f9a9d1f2441 Use conn_type to identify block-relay peers, rather than m_tx_relay == nullptr (Suhas Daftuar)
91d61952a82af3e8887e8ae532ecc19d87fe9073 Simplify and clarify extra outbound peer counting (Suhas Daftuar)
Pull request description:
To make eclipse attacks more difficult, regularly initiate outbound connections
and stay connected long enough to sync headers and potentially learn of new
blocks. If we learn a new block, rotate out an existing block-relay peer in
favor of the new peer.
This augments the existing outbound peer rotation that exists -- currently we
make new full-relay connections when our tip is stale, which we disconnect
after waiting a small time to see if we learn a new block. As block-relay
connections use minimal bandwidth, we can make these connections regularly and
not just when our tip is stale.
Like feeler connections, these connections are not aggressive; whenever our
timer fires (once every 5 minutes on average), we'll try to initiate a new
block-relay connection as described, but if we fail to connect we just wait for
our timer to fire again before repeating with a new peer.
ACKs for top commit:
ariard:
Code Review ACK b3a515c, only change since last time is dropping a useless `cs_main` taking. I manually tested a previous version of the PR, and not substantial change has been introduced since then which would alter behavior IMO.
jonatack:
Tested ACK b3a515c0bec97633a76bec101af47c3c90c0b749 over several weeks, though this change and behavior could benefit from test coverage and other follow-ups (refactoring, etc.) described in the review feedback. I did not verify the behavior of `m_start_extra_block_relay_peers` only being enabled after initial chain sync. Since my last review, one unneeded `cs_main` lock was removed.
Tree-SHA512: 75fc6f8e8003e88e93f86b845caf2d30b8b9c0dbb0a6b8aabe4e24ea4f6327351f736a068a3b2720a8a581b789942a3a47f921e2afdb47e88bc50d078aa37b6f
65273fa0e74f0c11dfbf0645dd962bdc779ea558 Clear m_addr_known before our periodic self-advertisement (Suhas Daftuar)
Pull request description:
We use a rolling bloom filter to track which addresses we've previously sent a peer, but after #7125 we no longer clear it every day before our own announcement. This looks to me like an oversight which has the effect of reducing the frequency with which we actually self-announce our own address, so this reintroduces resetting that filter.
ACKs for top commit:
naumenkogs:
ACK 65273fa0e74f0c11dfbf0645dd962bdc779ea558
laanwj:
Code review ACK 65273fa0e74f0c11dfbf0645dd962bdc779ea558
sipa:
utACK 65273fa0e74f0c11dfbf0645dd962bdc779ea558
Tree-SHA512: 602c155fb6d2249b054fcb6f1c0dd17143605ceb87132286bbd90babf26d258ff6c41f9925482c17e2be41805d33f9b83926cb447f394969ffecd4bccfa0a64f
fa0f4157098ea68169ced44730986d0ed2c3a5aa net: Assume that SetCommonVersion is called at most once per peer (MarcoFalke)
Pull request description:
This restores the check removed in https://github.com/bitcoin/bitcoin/pull/17785#discussion_r503224381
Instead of using `error`, which was used previously, it uses a newly introduced `Assume()`. `error` had several issues:
* It logs unconditionally to the debug log
* It doesn't abort the program when the error is hit in tests
ACKs for top commit:
practicalswift:
cr ACK fa0f4157098ea68169ced44730986d0ed2c3a5aa: patch looks correct
jnewbery:
utACK fa0f4157098ea68169ced44730986d0ed2c3a5aa
Tree-SHA512: cd7424a9485775e8c7093b725f8f52a90d47485185e79bac80f7810e450d0b3fda608d8805e9239094929f7bad2dca3fe772fb78ae606c2399d15405521e136b
