f36d1d5b8934aac60d3097047ecedeb58bae2185 Use void* throughout support/lockedpool.h (Jeffrey Czyz)
Pull request description:
Replace uses of char* with void* in Arena's member variables. Instead,
cast to char* where needed in the implementation.
Certain compiler environments disallow std::hash<char*> specializations
to prevent hashing the pointer's value instead of the string contents.
Thus, compilation fails when std::unordered_map is keyed by char*.
Explicitly using void* is a workaround in such environments. For
consistency, void* is used throughout all member variables similarly to
the public interface.
Changes to this code are covered by src/test/allocator_tests.cpp.
ACKs for top commit:
achow101:
ACK f36d1d5b8934aac60d3097047ecedeb58bae2185
theStack:
Code-review ACK f36d1d5b8934aac60d3097047ecedeb58bae2185
jonatack:
ACK f36d1d5b8934aac60d3097047ecedeb58bae2185 review, debug build, unit tests, checked clang 15 raises "error: arithmetic on a pointer to void" without the conversions here from the generic void* pointer back to char*
Tree-SHA512: f9074e6d29ef78c795a512a6e00e9b591e2ff34165d09b73eae9eef25098c59e543c194346fcd4e83185a39c430d43744b6f7f9d1728a132843c67bd27ea5189
4d84eaec82e7b5a450d47cd30e5936a717035f77 Raise PRNG seed log to INFO. (roconnor-blockstream)
Pull request description:
Some build infrastructure, such as Nix, will delete failed builds by default, keeping only the log (stdout/stderr) of the failed build.
For flaky tests, it would be very helpful to have the PRNG seed in the default log in order to redo the failed test.
By simply raising the PRNG seed logging to INFO, we can, by default, record the seed in the log of every build.
ACKs for top commit:
MarcoFalke:
lgtm ACK 4d84eaec82e7b5a450d47cd30e5936a717035f77
theStack:
ACK 4d84eaec82e7b5a450d47cd30e5936a717035f77
Tree-SHA512: 3ccb4a4e7639a3babc3b2a6456a6d0bffc090da34e4545b317f7bfbed4e9950d1b38ea5b2a90c37ccb49b3454bdeff03a6aaf86770b9c4dd14b26320aba50b94
14302a4802e2dbb41f5189de88f99ddd5dda7736 test: fix test abort for high timeout values (and `--timeout-factor 0`) (Sebastian Falbesoner)
Pull request description:
On master, the functional tests's option `--timeout-factor 0` (which according to the test docs and parameter description should disable the RPC timeouts) currently fails, same as high values like `--timeout-factor 999999`:
```
$ ./test/functional/wallet_basic.py --timeout-factor 0
2022-08-29T01:26:39.561000Z TestFramework (INFO): Initializing test directory /tmp/bitcoin_func_test_f24yxzp5
2022-08-29T01:26:40.262000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
File "/home/honey/bitcoin/test/functional/test_framework/test_framework.py", line 549, in start_nodes
node.wait_for_rpc_connection()
File "/home/honey/bitcoin/test/functional/test_framework/test_node.py", line 234, in wait_for_rpc_connection
rpc.getblockcount()
File "/home/honey/bitcoin/test/functional/test_framework/coverage.py", line 49, in __call__
return_val = self.auth_service_proxy_instance.__call__(*args, **kwargs)
File "/home/honey/bitcoin/test/functional/test_framework/authproxy.py", line 142, in __call__
response, status = self._request('POST', self.__url.path, postdata.encode('utf-8'))
File "/home/honey/bitcoin/test/functional/test_framework/authproxy.py", line 107, in _request
self.__conn.request(method, path, postdata, headers)
File "/usr/local/lib/python3.9/http/client.py", line 1285, in request
self._send_request(method, url, body, headers, encode_chunked)
File "/usr/local/lib/python3.9/http/client.py", line 1331, in _send_request
self.endheaders(body, encode_chunked=encode_chunked)
File "/usr/local/lib/python3.9/http/client.py", line 1280, in endheaders
self._send_output(message_body, encode_chunked=encode_chunked)
File "/usr/local/lib/python3.9/http/client.py", line 1040, in _send_output
self.send(msg)
File "/usr/local/lib/python3.9/http/client.py", line 980, in send
self.connect()
File "/usr/local/lib/python3.9/http/client.py", line 946, in connect
self.sock = self._create_connection(
File "/usr/local/lib/python3.9/socket.py", line 844, in create_connection
raise err
File "/usr/local/lib/python3.9/socket.py", line 832, in create_connection
sock.connect(sa)
OSError: [Errno 22] Invalid argument
```
This is caused by a high timeout value that Python's HTTP(S) client library can't cope with. Fix this by clamping down the connection's set timeout value in AuthProxy. The change can easily be tested by running an arbitrary test with `--timeout-factor 0` on master (should fail), on this PR (should pass) and on this PR with the clamping value increased by 1 (should fail).
// EDIT: The behaviour was observed on OpenBSD 7.1 and Python 3.9.12.
ACKs for top commit:
MarcoFalke:
lgtm ACK 14302a4802e2dbb41f5189de88f99ddd5dda7736
Tree-SHA512: 6469e8ac699f1bb7dea11d5fb8b3ae54d895bb908570587c5631144cd41fe980ca0b1e6d0b7bfa07983307cba15fb26ae92e6766375672bf5be838d8e5422dbc
304ae6dc8e2cd63142d34d190f93e95cbe83247a doc: remove mention of "proper signing key" (fanquake)
Pull request description:
This key is no-longer in use: https://lists.linuxfoundation.org/pipermail/bitcoin-core-dev/2023-February/000115.html
> Please remove it from verification pipelines.
ACKs for top commit:
hebasto:
ACK 304ae6dc8e2cd63142d34d190f93e95cbe83247a
Tree-SHA512: 3dfd221a48f69ac56b4568db06b5d5b5d6a60b7d027a26157912219a2073589a0a3934cb30e11a161d48db55d3a637338f96617e3f3b92cb9e60e0d1d1dd372a
906631450df9927e690658184aa8a6b1b3a29ee9 s/transcation/transaction/ (Greg Sanders)
Pull request description:
ACKs for top commit:
fanquake:
ACK 906631450df9927e690658184aa8a6b1b3a29ee9 - looks like other comments are being addressed elsewhere.
Tree-SHA512: c835a14db2e0cf5e0317c95c8c7441df1f7c6cb14be7809fd947e07ea9d23f1f171f111429aabd0509b7f17601bc742041316b18e1135e547a966961f2c65038
588fad868dd49b5baca26170c2adca8544fed04b descriptors: fix docstring (param [in] vs [out]) (SomberNight)
Pull request description:
As in title, these docstrings look incorrect.
ACKs for top commit:
john-moffett:
ACK 588fad868dd49b5baca26170c2adca8544fed04b
Tree-SHA512: 1ab343a1b1fc57a7d6bd8363b84db9d96e8ea11a4cec85bcf79885c9df53da889fe2fb10b1fa92d824ddf0dee800c07353f46f1fea9887d2ad518bed0afebe3d
3a11adc7004d21b3dfe028b190d83add31691c55 Zero out wallet master key upon lock (John Moffett)
Pull request description:
When an encrypted wallet is locked (for instance via the RPC `walletlock`), the documentation indicates that the key is removed from memory:
b92d609fb2/src/wallet/rpc/encrypt.cpp (L157-L158)
However, the vector (a `std::vector<unsigned char, secure_allocator<unsigned char>>`) is merely _cleared_. As it is a member variable, it also stays in scope as long as the wallet is loaded, preventing the secure allocator from deallocating. This allows the key to persist indefinitely in memory. I confirmed this behavior on my macOS machine by using an open-source third party memory inspector ("Bit Slicer"). I was able to find my wallet's master key in Bit Slicer after unlocking and re-locking my encrypted wallet. I then confirmed the key data was at the address in LLDB.
This PR manually fills the bytes with zeroes before calling `clear()` by using our `memory_cleanse` function, which is designed to prevent the compiler from optimizing it away. I confirmed that it does remove the data from memory on my machine upon locking.
Note: An alternative approach could be to call `vMasterKey.shrink_to_fit()` after the `clear()`, which would trigger the secure allocator's deallocation. However, `shrink_to_fit()` is not _guaranteed_ to actually change the vector's capacity, so I think it's unwise to rely on it.
## Edit: A little more clarity on why this is an improvement.
Since `mlock`ed memory is guaranteed not to be swapped to disk and our threat model doesn't consider a super-user monitoring the memory in realtime, why is this an improvement? Most importantly, consider hibernation. Even `mlock`ed memory may get written to disk. From the `mlock` [manpage](https://man7.org/linux/man-pages/man2/mlock.2.html):
> (But be aware that the suspend mode on laptops and some desktop computers will save a copy of the system's RAM to disk, regardless of memory locks.)
As far as I can tell, this is true of [Windows](https://web.archive.org/web/20190127110059/https://blogs.msdn.microsoft.com/oldnewthing/20140207-00/?p=1833#:~:text=%5BThere%20does%20not%20appear%20to%20be%20any%20guarantee%20that%20the%20memory%20won%27t%20be%20written%20to%20disk%20while%20locked.%20As%20you%20noted%2C%20the%20machine%20may%20be%20hibernated%2C%20or%20it%20may%20be%20running%20in%20a%20VM%20that%20gets%20snapshotted.%20%2DRaymond%5D) and macOS as well.
Therefore, a user with a strong OS password and a strong wallet passphrase could still have their keys stolen if a thief takes their (hibernated) machine and reads the permanent storage.
ACKs for top commit:
S3RK:
Code review ACK 3a11adc7004d21b3dfe028b190d83add31691c55
achow101:
ACK 3a11adc7004d21b3dfe028b190d83add31691c55
Tree-SHA512: c4e3dab452ad051da74855a13aa711892c9b34c43cc43a45a3b1688ab044e75d715b42843c229219761913b4861abccbcc8d5cb6ac54957d74f6e357f04e8730
d627a6ee52 chore: bump version to 21.1.1 (pasta)
5f9700c69a docs: release notes for v21.1.1 (pasta)
1c00726aca Merge #6277: chore: add builder key for kittywhiskers (pasta)
a2bc0f1b1b Merge #6290: chore: update pasta gpg key to reflect new subkeys (pasta)
167608c7c7 Merge #6338: ci: attest results of guix builds (pasta)
6fb4e49ae5 Merge #6197: ci: always build guix, save artifacts (pasta)
c0ca93cf7a Merge #6340: fix: make 6336 compile in v21.1.x branch, using older CHECK_NONFATAL functionality (pasta)
bb96df428f Merge #6336: fix: rpc getblock and getblockstats for blocks with withdrawal transactions (asset unlock) (pasta)
8e70262db4 Merge #6131: feat: make a support of Qt app to show Platform transfer Tx (pasta)
80ed27914e Merge #6328: backport: bitcoin/bitcoin#30131, #23258, #30504 - fix bild for Ubuntu 24.10 + clang (pasta)
bd772fbe8f Merge #6229: fix: `creditOutputs` in AssetLock tx json output should be an array of objects, not debug strings (pasta)
9bf39a93d3 Merge #6222: fix: adjust payee predictions after mn_rr activation, add tests (pasta)
87bebfc246 Merge #6219: fix: correct is_snapshot_cs in VerifyDB (pasta)
a4e6b8a993 Merge #6208: fix: persist coinjoin denoms options from gui over restarts (pasta)
Pull request description:
## Issue being fixed or feature implemented
See commits, backports, release notes, version bump
## What was done?
## How Has This Been Tested?
## Breaking Changes
## 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
- [ ] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_
ACKs for top commit:
knst:
utACK d627a6ee52
kwvg:
ACK d627a6ee52
UdjinM6:
utACK d627a6ee52
ogabrielides:
utACK d627a6e
Tree-SHA512: cde7e40760e16e9f48da8149c3742d18a34029b057405e4d55b87110da96acbcd19b47280451dd7b5ad1ccfc91fde655452cf5f0f0d1e01a41b4c685337c64b8
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
c3f2474898 chore: update pasta gpg key to reflect new subkeys (pasta)
Pull request description:
## Issue being fixed or feature implemented
I've added 2 subkeys to my GPG key `29590362EC878A81FD3C202B52527BEDABE87984` to better follow best practices, which avoids using your primary key whenever possible. All future git commit signing, and potentially releases will be signed by a subkey instead of the primary key.
These updated subkeys keys are now included on all the major keyservers
hkps://keyserver.ubuntu.com
hkps://pgp.mit.edu
hkps://keyserver.ubuntu.com
keybase has 1 of the 2 subkeys already, will add the other soon.
## What was done?
## How Has This Been Tested?
## Breaking Changes
Users who validate my signatures may have to refresh the key from keyservers via `gpg --refresh-keys` or pull down from keybase via `curl https://keybase.io/pasta/pgp_keys.asc | gpg --import`
## Checklist:
- [ ] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_
ACKs for top commit:
UdjinM6:
utACK c3f2474898
Tree-SHA512: 87d33caceb1973a54877c5d5a8b85a48e1373b7709698efc793598bf7453608979bfe1c75e4ea0c9ec852c0343b43b06357c58f6c4fbf72915eb910788cc705f
cd712e86b7 ci: attest results of guix builds (pasta)
Pull request description:
## Issue being fixed or feature implemented
This simply adds attestations to guix results by GitHub. This way, not only can someone verify that all us developers agree, but also that GitHub hosted runners agree :)
## What was done?
Add actions/attest-build-provenance to guix-build CI
## How Has This Been Tested?
see: https://github.com/PastaPastaPasta/dash/actions/runs/11239755631
## 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)_
ACKs for top commit:
UdjinM6:
utACK cd712e86b7
Tree-SHA512: b590ee2cf29aa57f78cb68c22d5327e8c9272d63d523c3b64fbbdffabb90981a6b6505c5f511bde19310ea1d8c96fc6d181359a7d7a0672612473110cbe079ef
770651aa15 set hosts in guix-check (pasta)
580bbe6d1c feat: improve guix building; run always, save artifacts (pasta)
101a31555f refactor: simplify caching setup, add a restore key to actually cache besides 1 run (pasta)
1b139e4837 feat: automatically run guix-build on all tags pushed (pasta)
Pull request description:
## Issue being fixed or feature implemented
Previously, we only ran guix on 1 machine for all hosts; this slowed it down a lot. Let's move to GitHub action runners, but run them all separately. Then upload the artifacts.
In the future there is significant caching I can add that should help a lot more. But currently, takes about 1 hour
## What was done?
## How Has This Been Tested?
see: https://github.com/PastaPastaPasta/dash/actions/runs/10345024600
## 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)_
ACKs for top commit:
UdjinM6:
utACK 770651aa15
Tree-SHA512: 639b95c3b6a26f205ed00c138a9189f915cfc36a815516035e59ceda82675414b1bd31a361b33449b5e4c58a7655f3a7d616b362c23f7fa75e72b1284be06b9e
a7bbcc823d fix: make 6336 compile in v21.1.x branch, using older CHECK_NONFATAL functionality (pasta)
Pull request description:
## Issue being fixed or feature implemented
Resolve build failures when 6336 is back ported
## What was done?
Use older functionality of CHECK_NONFATAL
## How Has This Been Tested?
built on both branches
## Breaking Changes
## 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)_
ACKs for top commit:
knst:
utACK a7bbcc823d
UdjinM6:
utACK a7bbcc823d (#6339 compiles with this one applied on top of it)
kwvg:
utACK a7bbcc823d
Tree-SHA512: 17b6d8223f653eaafa6ef9d1a4e8fc14ca1fe1623fbb13d23a9429e87a64c8fae3ddaf6d2d8d5a52138ab712a36949662b38a8a9cbbc5db3618ce24f565f6f2a
a7bbcc823d fix: make 6336 compile in v21.1.x branch, using older CHECK_NONFATAL functionality (pasta)
Pull request description:
## Issue being fixed or feature implemented
Resolve build failures when 6336 is back ported
## What was done?
Use older functionality of CHECK_NONFATAL
## How Has This Been Tested?
built on both branches
## Breaking Changes
## 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)_
ACKs for top commit:
knst:
utACK a7bbcc823d
UdjinM6:
utACK a7bbcc823d (#6339 compiles with this one applied on top of it)
kwvg:
utACK a7bbcc823d
Tree-SHA512: 17b6d8223f653eaafa6ef9d1a4e8fc14ca1fe1623fbb13d23a9429e87a64c8fae3ddaf6d2d8d5a52138ab712a36949662b38a8a9cbbc5db3618ce24f565f6f2a
b9a46f6d2c refactor: use IsPlatformTransfer in core_write and rpc/blockchain (Konstantin Akimov)
f6169fade4 fix: make composite rpc 'masternode payments' to work with withdrawals (Konstantin Akimov)
e498378eb7 feat: add test for fee in getmempoolentry (Konstantin Akimov)
b0d06f0b5f feat: add regression test for `getblock` and `getblockstats` for withdrawal fee calculation failure (Konstantin Akimov)
ab7172bc8f fix: getblockstats rpc to work with withdrawal transactions (Konstantin Akimov)
96c9b469ca fix: getblock for withdrawal transaction if verbosity level is 2 (Konstantin Akimov)
Pull request description:
## Issue being fixed or feature implemented
https://github.com/dashpay/dash/issues/6335
## What was done?
Applied fixes for `getblock` rpc and `getblockstats` rpc to make them work with withdrawal transactions (asset unlock).
## How Has This Been Tested?
Run updated functional test `feature_asset_locks.py` without the fix causes a failure:
```
2024-10-22T12:01:35.902000Z TestFramework (ERROR): JSONRPC error
Traceback (most recent call last):
File "/home/knst/projects/dash-reviews/test/functional/test_framework/test_framework.py", line 160, in main
self.run_test()
File "/home/knst/projects/dash-reviews/test/functional/feature_asset_locks.py", line 273, in run_test
self.test_asset_unlocks(node_wallet, node, pubkey)
File "/home/knst/projects/dash-reviews/test/functional/feature_asset_locks.py", line 410, in test_asset_unlocks
self.log.info(f"block info: {node.getblock(block_asset_unlock, 2)}")
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/knst/projects/dash-reviews/test/functional/test_framework/coverage.py", line 49, in __call__
return_val = self.auth_service_proxy_instance.__call__(*args, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/knst/projects/dash-reviews/test/functional/test_framework/authproxy.py", line 148, in __call__
raise JSONRPCException(response['error'], status)
test_framework.authproxy.JSONRPCException: Internal bug detected: "MoneyRange(fee)"
core_write.cpp:338 (TxToUniv)
Please report this issue here: https://github.com/dashpay/dash/issues
(-1)
```
With patch functional test `feature_asset_locks.py` succeed.
## 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:
PastaPastaPasta:
utACK b9a46f6d2c
kwvg:
utACK b9a46f6d2c
UdjinM6:
utACK b9a46f6d2c
ogabrielides:
utACK b9a46f6d2c
Tree-SHA512: e49cf73bff5fabc9463ae538c6c556d02b3f9e396e0353f5ea0661afa015259409cdada406d05b77bf0414761c76a013cd428ffc283cbdefbefe3384c9d6ccc5
21f174aff1 feat: improve query categorisation in Qt App (Konstantin Akimov)
c863473286 test: add spending asset unlock tx in functional tests (Konstantin Akimov)
1fb67ece0e feat: make a support of Qt app to show Platform Transfer transaction as a new type of transaction (Konstantin Akimov)
Pull request description:
## Issue being fixed or feature implemented
Transfers from platform have incorrectly shown amount in Dash Core wallet app.
They also shown in Qt app as self-send that is not completely true.
## What was done?
Added new type of transaction to Qt App, added a filter for its type, fixed calculation of output for tx records.
As well added a new type of transaction `platform-transfer` in rpc output of `gettransaction` RPC
## How Has This Been Tested?
Make a Platform Transfer transaction on RegTest and check it in Dash Core
Helper to see it: export dpath=/tmp/dash_func_test_PATHPATH/ ; src/qt/dash-qt -regtest -conf=$dpath/node0/dash.conf -datadir=$dpath/node0/ -debug=0 -debuglogfile=/dev/stdout
## Breaking Changes
There's new type of transaction "platform-transfer" in rpc output of `gettransaction`.
**This PR DOES NOT change any consensus rules.**
Breaking changes that makes withdrawal transaction immature is moved to https://github.com/dashpay/dash/pull/6128
## 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: ec2a54a910f121ad30ff8e94cf17080b5b3c651872e9bc3de9ec0924ca7f7a0e526b74b05cde26aaf860e3809e67f66142112319a69c216527e5bcb1b8a2b8f6
cd712e86b7 ci: attest results of guix builds (pasta)
Pull request description:
## Issue being fixed or feature implemented
This simply adds attestations to guix results by GitHub. This way, not only can someone verify that all us developers agree, but also that GitHub hosted runners agree :)
## What was done?
Add actions/attest-build-provenance to guix-build CI
## How Has This Been Tested?
see: https://github.com/PastaPastaPasta/dash/actions/runs/11239755631
## 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)_
ACKs for top commit:
UdjinM6:
utACK cd712e86b7
Tree-SHA512: b590ee2cf29aa57f78cb68c22d5327e8c9272d63d523c3b64fbbdffabb90981a6b6505c5f511bde19310ea1d8c96fc6d181359a7d7a0672612473110cbe079ef
b9a46f6d2c refactor: use IsPlatformTransfer in core_write and rpc/blockchain (Konstantin Akimov)
f6169fade4 fix: make composite rpc 'masternode payments' to work with withdrawals (Konstantin Akimov)
e498378eb7 feat: add test for fee in getmempoolentry (Konstantin Akimov)
b0d06f0b5f feat: add regression test for `getblock` and `getblockstats` for withdrawal fee calculation failure (Konstantin Akimov)
ab7172bc8f fix: getblockstats rpc to work with withdrawal transactions (Konstantin Akimov)
96c9b469ca fix: getblock for withdrawal transaction if verbosity level is 2 (Konstantin Akimov)
Pull request description:
## Issue being fixed or feature implemented
https://github.com/dashpay/dash/issues/6335
## What was done?
Applied fixes for `getblock` rpc and `getblockstats` rpc to make them work with withdrawal transactions (asset unlock).
## How Has This Been Tested?
Run updated functional test `feature_asset_locks.py` without the fix causes a failure:
```
2024-10-22T12:01:35.902000Z TestFramework (ERROR): JSONRPC error
Traceback (most recent call last):
File "/home/knst/projects/dash-reviews/test/functional/test_framework/test_framework.py", line 160, in main
self.run_test()
File "/home/knst/projects/dash-reviews/test/functional/feature_asset_locks.py", line 273, in run_test
self.test_asset_unlocks(node_wallet, node, pubkey)
File "/home/knst/projects/dash-reviews/test/functional/feature_asset_locks.py", line 410, in test_asset_unlocks
self.log.info(f"block info: {node.getblock(block_asset_unlock, 2)}")
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/knst/projects/dash-reviews/test/functional/test_framework/coverage.py", line 49, in __call__
return_val = self.auth_service_proxy_instance.__call__(*args, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/knst/projects/dash-reviews/test/functional/test_framework/authproxy.py", line 148, in __call__
raise JSONRPCException(response['error'], status)
test_framework.authproxy.JSONRPCException: Internal bug detected: "MoneyRange(fee)"
core_write.cpp:338 (TxToUniv)
Please report this issue here: https://github.com/dashpay/dash/issues
(-1)
```
With patch functional test `feature_asset_locks.py` succeed.
## 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:
PastaPastaPasta:
utACK b9a46f6d2c
kwvg:
utACK b9a46f6d2c
UdjinM6:
utACK b9a46f6d2c
ogabrielides:
utACK b9a46f6d2c
Tree-SHA512: e49cf73bff5fabc9463ae538c6c556d02b3f9e396e0353f5ea0661afa015259409cdada406d05b77bf0414761c76a013cd428ffc283cbdefbefe3384c9d6ccc5
7d933d876d test: should have no spork with an empty name (UdjinM6)
d3345c9ee4 fix: adjust the number of spork defaults (UdjinM6)
Pull request description:
## Issue being fixed or feature implemented
```
{
"SPORK_2_INSTANTSEND_ENABLED": 4070908800,
"SPORK_3_INSTANTSEND_BLOCK_FILTERING": 4070908800,
"SPORK_9_SUPERBLOCKS_ENABLED": 4070908800,
"SPORK_17_QUORUM_DKG_ENABLED": 4070908800,
"SPORK_19_CHAINLOCKS_ENABLED": 4070908800,
"SPORK_21_QUORUM_ALL_CONNECTED": 4070908800,
"SPORK_23_QUORUM_POSE": 4070908800,
"": 0 <----- this line shouldn't exist
}
```
6275 follow-up
## What was done?
## How Has This Been Tested?
## Breaking Changes
## Checklist:
- [ ] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e tests
- [ ] I have made corresponding changes to the documentation
- [ ] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_
ACKs for top commit:
knst:
utACK 7d933d876d
PastaPastaPasta:
utACK 7d933d876d
Tree-SHA512: bacdbf95bff2c3aec6b5767caa06d2850a7ed554231160a914e12b9c4aa8d14237b98bef22739440feec616bd525fa4fb7dcfc2185e206d73f1ac3c6f2988b88
aa5311d0fc merge bitcoin#29212: Fix -netinfo backward compat with getpeerinfo pre-v26 (Kittywhiskers Van Gogh)
1a293c7cc5 merge bitcoin#29006: fix v2 transport intermittent test failure (Kittywhiskers Van Gogh)
d0804d4bf0 merge bitcoin#28822: Add missing wait for version to be sent in add_outbound_p2p_connection (Kittywhiskers Van Gogh)
c0b3062215 merge bitcoin#28782: Add missing sync on send_version in peer_connect (Kittywhiskers Van Gogh)
35253cdd15 merge bitcoin#28632: make python p2p not send getaddr on incoming connections (Kittywhiskers Van Gogh)
6a4ca62fd1 merge bitcoin#28645: fix `assert_debug_log` call-site bugs, add type checks (Kittywhiskers Van Gogh)
deaee147b7 merge bitcoin-core/gui#754: Add BIP324-specific labels to peer details (Kittywhiskers Van Gogh)
fffe6e716b merge bitcoin#27986: remove race in the user-agent reception check (Kittywhiskers Van Gogh)
1bf135bbc9 merge bitcoin#26553: Fix intermittent failure in rpc_net.py (Kittywhiskers Van Gogh)
5bf245b4a0 partial bitcoin#26448: fix intermittent failure in p2p_sendtxrcncl.py (Kittywhiskers Van Gogh)
b7c0030d3d partial bitcoin#23443: Erlay support signaling (Kittywhiskers Van Gogh)
c709df74cc merge bitcoin#20524: Move MIN_VERSION_SUPPORTED to p2p.py (Kittywhiskers Van Gogh)
Pull request description:
## Additional Information
* Dependent on https://github.com/dashpay/dash/pull/6324
* Dependency for https://github.com/dashpay/dash/pull/6330
## How Has This Been Tested?
* Changes to Qt client were validated by running the client
<details>
<summary>Screenshot</summary>
</details>
* Changes to `dash-cli` were validated by running it with different node versions
**Against a node built on this PR**
<details>
<summary>Screenshot</summary>
</details>
**Against a node built running the last release**
<details>
<summary>Screenshot</summary>
</details>
## Breaking Changes
None expected.
## 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:
ACK aa5311d0fc
PastaPastaPasta:
utACK aa5311d0fc
Tree-SHA512: 8cca324ac988a73c0590a4e9b318e81ce951ac55fb173cf507fa647cab01ab4981e6a06d4792376b4bfb44ff09d4811de05fadb9ba793dd00b4c7965b4b22654
e994691e2d Merge bitcoin/bitcoin#30504: doc: use proper doxygen formatting for CTxMemPool::cs (merge-script)
a3e6378108 Merge bitcoin/bitcoin#23258: doc: Fix outdated comments referring to ::ChainActive() (fanquake)
dcbf671551 Merge bitcoin/bitcoin#30131: wallet, tests: Avoid stringop-overflow warning in PollutePubKey (merge-script)
Pull request description:
## Issue being fixed or feature implemented
It fixes build with clang 19.1.1 (default clang version on Ubuntu 24.10)
## What was done?
Backport 30131, 30504 to fix compilation error and warning; 23258 to reduce conflicts.
See commit messages for details
## How Has This Been Tested?
Build and succeed
```
CC=clang CXX=clang++ ./configure --prefix=$(pwd)/depends/x86_64-pc-linux-gnu --enable-suppress-external-warnings --enable-debug --enable-stacktraces --enable-werror --enable-crash-hooks --enable-maintainer-mode --enable-multiprocess
```
## 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:
PastaPastaPasta:
utACK e994691e2d
UdjinM6:
utACK e994691e2d
Tree-SHA512: 82623030c164c0852d87e8497a59157630f87a385050b6c58d79bf5a8f32462fb26cb02e61b1062afdf9f835e10b9baf4590c326c0fb41bbdd79c82e9d105513
9876c2d78b docs: add partial release notes (UdjinM6)
b330318db7 refactor: drop circular dependency (UdjinM6)
e54fe42ce8 refactor: use `key_to_p2pkh_script` in more places (UdjinM6)
3ed6246889 test: check `creditOutputs` format (UdjinM6)
ba0e64505b fix: `creditOutputs` in AssetLock tx json output should be an array of objects, not debug strings (UdjinM6)
Pull request description:
## Issue being fixed or feature implemented
Txout-s in `creditOutputs` for AssetLock txes should be shown the way txout-s are shown in other places. We should not be using debug strings there.
Example: `getrawtransaction 50757f651f335e22c5a810bd05c1e5aac0d95b132f6454e2a72683f88e3983f3 1`
develop:
```
"assetLockTx": {
"version": 1,
"creditOutputs": [
"CTxOut(nValue=0.01000000, scriptPubKey=76a914cdfca4ae1cf2333056659a2c)"
]
},
```
This PR:
```
"assetLockTx": {
"version": 1,
"creditOutputs": [
{
"value": 0.01000000,
"valueSat": 1000000,
"scriptPubKey": {
"asm": "OP_DUP OP_HASH160 cdfca4ae1cf2333056659a2c8dc656f36d228402 OP_EQUALVERIFY OP_CHECKSIG",
"hex": "76a914cdfca4ae1cf2333056659a2c8dc656f36d22840288ac",
"address": "yf6c2VSpWGXUgmjQSHRpfEcTPsbqN4oL4c",
"type": "pubkeyhash"
}
}
]
},
```
kudos to @coolaj86 for finding the issue
## What was done?
Change `CAssetLockPayload::ToJson()` output to be closer to [`TxToUniv()`](https://github.com/dashpay/dash/blob/develop/src/core_write.cpp#L262-L272)
NOTE: `refactor: use key_to_p2pkh_script in more places` commit is a bit unrelated but I decided to add it anyway to make it easier to follow assetlock creation vs getrawtransaction rpc check.
## How Has This Been Tested?
Try example above, run tests
## Breaking Changes
RPC output is different for AssetLock txes
## 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
ACKs for top commit:
PastaPastaPasta:
utACK 9876c2d78b
Tree-SHA512: 158c98ac9e4979bb29c4f54cb1b71806f22aaec92218d92cd2b2e9b9f74df721563e7a6c5f517ea358ac74659fa79f51d1b683002a1cdceb1b8ee80f8fd79375
715bc1af66 test: check `masternode winners` before and after mn_rr (UdjinM6)
9d47cd2226 fix: adjust payee predictions after mn_rr activation (UdjinM6)
Pull request description:
## Issue being fixed or feature implemented
Payment predictions in GUI are wrong when mn_rr is active, `masternode winners` RPC is affected by the same issue too. Actual payments aren't affected.
## What was done?
Adjust calculations, add tests for `masternode winners`.
## How Has This Been Tested?
Run dash-qt on testnet, check "Next Payment" on "Masternode" tab. 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
ACKs for top commit:
PastaPastaPasta:
utACK 715bc1a
Tree-SHA512: 293c77974bcb50c6f9c51449d7bb12f89ad8db5871cad3a6083fe1951fe77e0deba8de7688b2f600fabe977bdc7390a66a984a6a076be19183c23742e00e27bf
bf377d47e5 fix: correct is_snapshot_cs in VerifyDB (James O'Beirne)
Pull request description:
## Issue being fixed or feature implemented
Flag `is_snapshot_cs` has been inverted in bitcoin#21584
Discovered during investigation of issue:
```
Verifying last 6 blocks at level 3
2024-08-14T14:51:55Z [0%]...*** Found EvoDB inconsistency, you must reindex to continue
```
So far as code below does:
```
if ((fPruneMode || is_snapshot_cs) && !(pindex->nStatus & BLOCK_HAVE_DATA)) {
// If pruning or running under an assumeutxo snapshot, only go
// back as far as we have data.
LogPrintf("VerifyDB(): block verification stopping at height %d (pruning, no data)\n", pindex->nHeight);
break;
}
```
In case of missing data in evo db we will get instead of "block verification stopping at height" we may get data inconsistency issue.
## What was done?
Inverted condition back (same fix in bitcoin/bitcoin#27596)
## How Has This Been Tested?
Unit/functional tests doesn't cover it, but they do no fail after fix.
## 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 bf377d47e5
PastaPastaPasta:
utACK bf377d47e5
Tree-SHA512: ac21e6db6e23c4c7dc150fb16171aef47c9f42c29466b403bca7d56ed6faa2fccc41df92e1fabec4d6e9fd56991e152dea168593a4550fc3583631a63009c27f
3ec0c8ca0a fix: persist coinjoin denoms and sessions options from gui over restarts (pasta)
Pull request description:
## Issue being fixed or feature implemented
Persist coinjoin denoms over restarts, fixes#5975
## What was done?
Soft set the argument into the daemon from GUI settings
## How Has This Been Tested?
follow procedure in 5975
## Breaking Changes
## 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)_
ACKs for top commit:
knst:
utACK 3ec0c8ca0a
UdjinM6:
utACK 3ec0c8ca0a
Tree-SHA512: b7378460b3990713b755f36de506b94e7d0005f19cf1155f2fc12191ba03f2e16c35049ddbd89f578acd89bc8eae5e432913114e1ff5ef7ab2cc30628aeff3f2
90744d0d65 Merge bitcoin/bitcoin#25115: scripted-diff: replace non-standard fixed width integer types (`u_int`... -> `uint`...) (fanquake)
e4f8b7097d Merge bitcoin/bitcoin#24852: util: optimize HexStr (laanwj)
1288494d4a Merge bitcoin/bitcoin#24976: netgroup: Follow-up for #22910 (fanquake)
656f525855 Merge bitcoin-core/gui#543: peers-tab: add connection duration column to tableview (Hennadii Stepanov)
33b9771ebc Merge bitcoin/bitcoin#24749: test: use MiniWallet for mempool_unbroadcast.py (MarcoFalke)
36e9b5fead Merge bitcoin/bitcoin#24381: test: Run symlink regression tests on Windows (laanwj)
a1691c7c2a Merge bitcoin/bitcoin#24102: mempool: Run coin.IsSpent only once in a row (MarcoFalke)
acbf718b57 Merge bitcoin/bitcoin#23976: document and clean up MaybeUpdateMempoolForReorg (MarcoFalke)
73e1861576 Merge bitcoin/bitcoin#23750: rpcwallet: mention labels are disabled for ranged descriptors (MarcoFalke)
c2fd4fe379 Merge bitcoin/bitcoin#23515: test: Return the largest utxo in MiniWallet.get_utxo (MarcoFalke)
7455b5557a Merge bitcoin-core/gui#454: Use only Qt translation primitives in GUI code (Hennadii Stepanov)
95aeb6a08d Merge bitcoin-core/gui#436: Include vout when copying transaction ID from coin selection (Hennadii Stepanov)
02b5fce942 Merge bitcoin-core/gui#318: Add `Copy address` Peers Tab Context Menu Action (Hennadii Stepanov)
e4774b9dad Merge bitcoin-core/gui#384: Add copy IP/Netmask action for banned peer (Hennadii Stepanov)
Pull request description:
## Issue being fixed or feature implemented
batch of trivial backports
## What was done?
## How Has This Been Tested?
## Breaking Changes
## 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)_
ACKs for top commit:
UdjinM6:
utACK 90744d0d65
kwvg:
utACK 90744d0d65
Tree-SHA512: 64b562f559f0be9f04b033a642aea3f9a9b49c69a957fa2fd4a1dbc263c465ca26ef2db987b7200cf861ff3989a54376273eeb224f60a54308dfa19897b67724
7d9ff96091 merge bitcoin#16981: Improve runtime performance of --reindex (Kittywhiskers Van Gogh)
e531dff5f7 merge bitcoin#26417: fix intermittent failure in feature_index_prune.py (Kittywhiskers Van Gogh)
b04b71a957 merge bitcoin#24858: incorrect blk file size calculation during reindex results in recoverable blk file corruption (Kittywhiskers Van Gogh)
9e75b99c53 merge bitcoin#26215: Improve BaseIndex::BlockUntilSyncedToCurrentChain reliability (Kittywhiskers Van Gogh)
3bd584c845 merge bitcoin#24832: Verify the block filter hash when reading the filter from disk (Kittywhiskers Van Gogh)
e507a51323 fix: avoid `mandatory-script-verify-flag-failed` crash in bench test (Kittywhiskers Van Gogh)
a86109a017 merge bitcoin#25074: During sync, commit best block after indexing (Kittywhiskers Van Gogh)
e6867a35ce merge bitcoin#25123: Fix race condition in index prune test (Kittywhiskers Van Gogh)
baf6e26eed merge bitcoin#21726: Improve Indices on pruned nodes via prune blockers (Kittywhiskers Van Gogh)
c65ec190c5 merge bitcoin#24626: disallow reindex-chainstate when pruning (Kittywhiskers Van Gogh)
bcd24a25e3 fix: push activation height for forks ahead, fix `feature_pruning.py` (Kittywhiskers Van Gogh)
10203560f5 merge bitcoin#24812: Add CHECK_NONFATAL identity function and NONFATAL_UNREACHABLE macro (Kittywhiskers Van Gogh)
1caaa85716 merge bitcoin#24138: Commit MuHash and best block together for coinstatsindex (Kittywhiskers Van Gogh)
b218f123b7 merge bitcoin#23046: Add txindex migration test (Kittywhiskers Van Gogh)
ebae59eedf fix: make sure we flush our committed best block in no-upgrade cases (Kittywhiskers Van Gogh)
Pull request description:
## Additional Information
* When backporting [bitcoin#23046](https://github.com/bitcoin/bitcoin/pull/23046), it was discovered that there has been a longstanding bug in `CDeterministicMNManager::MigrateDBIfNeeded`(`2`)`()` that flagged a database taken from an older version for failing its "previous migration attempt", requiring the database to be fully rebuilt through a reindex.
This occurred because the older database would be read pre-DIP3 in `MigrateDBIfNeeded()`, which then caused the migration logic to write the new best block ([source](3f0c2ff324/src/evo/deterministicmns.cpp (L1236-L1241))) (the legacy best block is erased before the DIP3 condition is checked, [source](3f0c2ff324/src/evo/deterministicmns.cpp (L1233))) but while it completed the transaction ([source](3f0c2ff324/src/evo/deterministicmns.cpp (L1240))), critically, it didn't write it to disk (example of writing to disk, [here](3f0c2ff324/src/evo/deterministicmns.cpp (L1288-L1292))).
This meant that when it was read again by `MigrateDBIfNeeded2()`, it saw three things a) there is no new best block (because it didn't get written), b) there is no legacy best block (because it gets erased before the new best block is written) and c) that the chain height is greater than 1 (since this isn't a new datadir and the chain has already advanced), it concludes that it was a botched migration attempt and fails ([source](3f0c2ff324/src/evo/deterministicmns.cpp (L1337-L1343))).
This bug affects v19 to `develop` (`3f0c2ff3` as of this writing) and prevents `feature_txindex_compatibility.py` from working as expected as it would migrate legacy databases to newer versions to test txindex migration code and get stuck at unhappy EvoDB migration logic, to allow the test to function properly when testing against the latest version of the client, this bug has been fixed as part of this PR.
* In [bitcoin#23046](https://github.com/bitcoin/bitcoin/pull/23046), version v0.17 was used as the last version to support legacy txindex as the updated txindex format was introduced in [dash#4178](https://github.com/dashpay/dash/pull/4178) (i.e. after v0.17) and the version selected for having migration code in it (note, migration code was removed in [dash#6296](https://github.com/dashpay/dash/pull/6296), so far not included as part of any release) was v18.2.2 despite the range being v18.x to v21.x was a) due to the bug mentioned above affecting v19.x onwards and b) v18.2.2 being the latest release in the v18.x lifecycle.
* The specific version number used for v0.17 is `170003` as the binaries corresponding to `170000` are not populated in `releases/`, which causes a CI failure ([source](https://gitlab.com/dashpay/dash/-/jobs/8073041955#L380))
* As of `develop` (`3f0c2ff3` as of this writing), `feature_pruning.py` was broken due to changes in Core that were not adjusted for, namely:
* The enforcement of `MAX_STANDARD_TX_SIZE` ([source](3f0c2ff324/src/policy/policy.h (L23-L24))) from DIP1 onwards ([source](3f0c2ff324/src/validation.cpp (L299-L301))) resulting in `bad-txns-oversize` errors in blocks generated for the test as the transactions generated are ~9.5x larger than the now-enforced limit ([source](3f0c2ff324/test/functional/feature_pruning.py (L48C51-L48C57))), this is resolved by pushing the DIP1 activation height upwards to `2000` (the same activation height used for DIP3 and DIP8).
* Change in subsidy logic in v20 ([source](3f0c2ff324/src/validation.cpp (L1073-L1082))) that results in `bad-cb-amount` errors, this is resolved by pushing the v20 activation height upwards.
Additionally, an inopportune implicit post-`generate` sync ([source](3f0c2ff324/test/functional/feature_pruning.py (L215))) also causes the test to fail. Alongside the above, they have been resolved in this PR.
* As of `develop` (`3f0c2ff3` as of this writing), `bench_dash` crashes when running the `AssembleBlock` benchmark. The regression was traced back to [bitcoin#21840](https://github.com/bitcoin/bitcoin/pull/21840) (5d10b41) in [dash#6152](https://github.com/dashpay/dash/pull/6152) due to the switch to `P2SH_OP_TRUE`.
This has been resolved by reverting this particular change.
<details>
<summary>Pre-fix test failure:</summary>
```
$ ./src/bench/bench_dash
Warning, results might be unstable:
* CPU governor is '' but should be 'performance'
* Turbo is enabled, CPU frequency will fluctuate
Recommendations
* Use 'pyperf system tune' before benchmarking. See https://github.com/psf/pyperf
| ns/op | op/s | err% | ins/op | bra/op | miss% | total | benchmark
|--------------------:|--------------------:|--------:|----------------:|---------------:|--------:|----------:|:----------
| 17,647,657.00 | 56.66 | 0.1% | 231,718,349.00 | 42,246,265.00 | 0.1% | 0.20 | `AddrManAdd`
| 42,201,861.00 | 23.70 | 0.1% | 544,366,811.00 | 102,569,244.00 | 0.0% | 0.46 | `AddrManAddThenGood`
| 189,697.83 | 5,271.54 | 0.1% | 1,763,991.40 | 356,189.40 | 0.3% | 0.01 | `AddrManGetAddr`
| 454.38 | 2,200,808.04 | 0.6% | 6,229.11 | 1,343.92 | 0.1% | 0.01 | `AddrManSelect`
| 1,066,471.00 | 937.67 | 67.6% | 13,350,463.00 | 3,150,465.00 | 0.4% | 0.01 | 〰️ `AddrManSelectByNetwork` (Unstable with ~1.0 iters. Increase `minEpochIterations` to e.g. 10)
| 1,181,774.50 | 846.19 | 49.0% | 18,358,489.50 | 4,224,642.50 | 0.0% | 0.02 | 〰️ `AddrManSelectFromAlmostEmpty` (Unstable with ~1.1 iters. Increase `minEpochIterations` to e.g. 11)
bench_dash: bench/block_assemble.cpp:46: void AssembleBlock(benchmark::Bench &): Assertion `res.m_result_type == MempoolAcceptResult::ResultType::VALID' failed.
[1] 2343746 IOT instruction (core dumped) ./src/bench/bench_dash
```
</details>
## Breaking changes
None expected
## 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)_
ACKs for top commit:
UdjinM6:
utACK 7d9ff96091
PastaPastaPasta:
utACK 7d9ff96091
Tree-SHA512: e2f1e58abb0a0368c4f1d5e488520957e042e6207b7d2d68a15eb18662405a3cdac91c5ff8e93c8a94c0fdab9b1412bd608d055f196230506c1640439939c25d
d690309513 fix: limit amount of attempts for test `test_withdrawal_fork` (Konstantin Akimov)
ecd0a96f63 test: fix test_withdrawal_fork (UdjinM6)
6fbd128947 chore: clang format/reword/typos (UdjinM6)
5fd7b07ddc chore: test new validation of asset unlocks tests after fork 'withdrawals' (Konstantin Akimov)
27040030e9 fix: using proper quorums for asset unlock validation (Konstantin Akimov)
0253438503 feat: new fork WITHDRAWALS introduced (Konstantin Akimov)
Pull request description:
## Issue being fixed or feature implemented
https://github.com/dashpay/dash-issues/issues/83
## What was done?
Introduces new fork "withdrawals" which let Asset Unlock be valid for any active quorum + 1 extra inactive (in opposite of hard-coded 2 of them).
## How Has This Been Tested?
See new test section `test_withdrawals_fork` in functional test feature_asset_locks.py
## Breaking Changes
- new fork "withdrawals"
- new logic of validation of Asset Unlock transactions's signature. Even no asset-unlock is mined yet, previous version of clients will refuse blocks which is fine for current system
## 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 d690309513
PastaPastaPasta:
utACK d690309513
Tree-SHA512: cac28cf974e97d4a4100c22d5aa07b398b15413235383c68a0f4ff005005b28892c7ac8e6b703582c126cb6e0ff80a950f190dd32268655fac534ed6f2a90d03
1db9e6ac76 Merge #19979: Replace LockAssertion with AssertLockHeld, remove LockAssertion (MarcoFalke)
d0a4198166 Merge #21598: refactor: Remove negative lock annotations from globals (MarcoFalke)
55114a682e Merge #19668: Do not hide compile-time thread safety warnings (MarcoFalke)
898282d620 Merge bitcoin/bitcoin#28774: wallet: avoid returning a reference to vMasterKey after releasing the mutex that guards it (Ava Chow)
6d452845dc fix: add missing lock annotation for ContextualCheckBlock and related TODO (Konstantin Akimov)
846ebab6e0 fix: fixes for orphange's locks and cs_main (Konstantin Akimov)
002580c42d fix: add ignoring safety-annotation for llmq/signing_shares due to template lambdas (Konstantin Akimov)
502d6ae8ef fix: add multiple missing annotation about locks for dash specific code (Konstantin Akimov)
a219a8be15 partial Merge #29040: refactor: Remove pre-C++20 code, fs::path cleanup (Konstantin Akimov)
042e8a4101 fix: drop unneded lock assert in net.cpp for m_nodes_mutex (Konstantin Akimov)
Pull request description:
## What was done?
These 4 backports improve noticeable implementation of thread-safety analysis by moving many possible warnings to compilation level.
There's a lot of fixes for annotations after that!
## How Has This Been Tested?
Build with clang:
```
CC=clang CXX=clang++ ./configure --prefix=$(pwd)/depends/x86_64-pc-linux-gnu --enable-suppress-external-warnings --enable-debug --enable-stacktraces --enable-werror --enable-crash-hooks --enable-maintainer-mode
```
## 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 1db9e6ac76
PastaPastaPasta:
utACK 1db9e6ac76
Tree-SHA512: 42a73e423f311689babc366b58a92f024556cf63af93357cfa0715c471efc1491d02ed9e7ba19b23efcdf423ab5bd051a24da2ad64c32e4b6db132a05f6669c1
fa1571b156dfa757d0ce0fddd53c25256196522b doc: Add missing optional to MempoolEntryDescription (MarcoFalke)
Pull request description:
Needed for https://github.com/bitcoin/bitcoin/pull/23083.
Can be reviewed with `--word-diff-regex=.`.
ACKs for top commit:
josibake:
ACK fa1571b156
shaavan:
ACK fa1571b156dfa757d0ce0fddd53c25256196522b
Tree-SHA512: b4370003d2aeadce438778e15bd9a0d6a7fef4711acbe8471a50a9d72bbf74e1705fecbaae6f7eb367ece7c795a816c4b8b6583ed6c8f91b35621ca30fd95c18
faa9c19a4b27e7fabf7c5deae1b4c4ca612ed01a doc: Add 23061 release notes (MarcoFalke)
faff17bbde6dcb1482a6210bc48b3192603a446f Fix (inverse) meaning of -persistmempool (MarcoFalke)
Pull request description:
Passing `-persistmempool` is currently treated as `-nopersistmempool`
ACKs for top commit:
jnewbery:
reACK faa9c19a4b27e7fabf7c5deae1b4c4ca612ed01a
hebasto:
ACK faa9c19a4b27e7fabf7c5deae1b4c4ca612ed01a, I have reviewed the code and it looks OK, I agree it can be merged.
Tree-SHA512: f34a89a07745dabe340eb845b2a348b79c093e9056f7a21c17e1ba2e278177c9b4cf30e8095791fd645a7f90eb34850b2eee0c869b4f6ec02bf749c73b0e52ee
faad05c6d2ad1840c26f6fdea7d358bf9ba6efea Crash debug builds when mempool ConsensusScriptChecks fails (MarcoFalke)
Pull request description:
Currently a bug in the function might sneak around our testing infrastructure.
Fix that by turning bugs into crashes during tests.
ACKs for top commit:
glozow:
utACK faad05c6d2ad1840c26f6fdea7d358bf9ba6efea, there's something seriously wrong with the code if this returns false, good to throw in debug mode
Tree-SHA512: dfea1cd9ce3f1c303f49cca1417cd5c77c6ed12849aaff7b6ab1b6060f2f0c9cf5d4689017355d11f66639bab35823f65f848e6979042fa875181509dfd5d3d7
