b8f17fbcb4 [tests] Move TxOrphange tests to orphange_tests.cpp (John Newbery)
Pull request description:
PR #21148 moved the orphan transaction handling functionality from net_processing into its own translation unit txorphanage.cpp. The unit tests for that code should be in its own file rather than mixed with the net_processing unit tests in denialofservive_tests.cpp.
ACKs for top commit:
vincenzopalazzo:
ACK b8f17fbcb4
Tree-SHA512: 32da89b3792abcbdcf897d66276225731c8976e1e0cd902c4b5ad8aff02104719c3aee2990cc2fcbe3eddede8a59472266e0ad1ce2ac11d66fe52c8cbe705161
This change also introduces an aditional buffer of 10 blocks (PRUNE_LOCK_BUFFER) that will not be pruned before the best block.
Co-authored-by: Luke Dashjr <luke-jr+git@utopios.org>
172c2333f0 Porting lint-assertions.sh to lint-assertions.py (hiago)
Pull request description:
This PR is converting `test/lint/lint-assertions.sh` to `test/lint/lint-assertions.py`. It's an item of #24783.
ACKs for top commit:
laanwj:
Tested ACK 172c2333f0
Tree-SHA512: 94d5b03acfeaf2303fad95d489d6c3aa7bd655889ddaa807cc97e0613b8eb8f5ef094feee2a98d974606890deb554e76490a5c523d64eb5bc55afa6a43221aae
79635c79e0 lint: Convert lint-circular-dependencies.sh to Python (Smlep)
Pull request description:
Here is a port of `/test/lint/lint-circular-dependencies.sh` to a Python-script as part of the request of https://github.com/bitcoin/bitcoin/issues/24783.
It aims to provide the same output as the bash version.
ACKs for top commit:
laanwj:
Tested ACK 79635c79e0
Tree-SHA512: f18077018f1229dd933cfe2bf0cfe7dc7d6538961c96a83c7a1f05e0cec4b068ca05502d68410d2aa4b6864523424db386e38233735190525904c2a8e9d2ba13
267684ee34 lint: convert format strings linter test to python (Eunoia)
Pull request description:
Refs #24783
Attempted to keep the style and flow of implementation as it is.
### Additional Notes(Optional):
1. There is scope of improvement on how the related files are fetched. In this `git grep` with `subprocess` is still used as I found it to be the simplest. Any pointers on this are appreciated.
2. Removed sort operation on the matching files as I couldn't think of any strong arguments to have it. Any pointers on this are appreciated.
3. Not important, but one small detail is that the previous implementation was storing matched files for all the `function_names` iterated so far. Fixed that in this PR.
ACKs for top commit:
laanwj:
Code review ACK 267684ee34
Tree-SHA512: 54ceae0c3501e561fdd9c5167b2dd8dd06da1b3697a077a042210970ce7004bda8c4e19abb1905ee64cbdce635f0a078508da645846ae7e81c016091f3f02458
035eef4be6 lint: Convert lint-python-utf8-encoding.sh to Python (Dimitri)
Pull request description:
A port of `test/lint/lint-python-utf8-encoding.sh` to a Python-script as part of the request of #24783. Checked for output-consistency.
ACKs for top commit:
laanwj:
Code review ACK 035eef4be6
Tree-SHA512: a8a2f505bf7953d318837182101346c44e73cfd1bf3b5342ff1400fb1c67c5292519fa99db1035da87cf27fb5f5ac5d28871bf55a1c085b5f8a3bb33ff0fa3fb
3043a1bc9d lint: Make known violations more specific in lint-locale-dependence (Dimitri)
229917d3d4 lint: Convert lint-locale-dependence.sh to Python (Dimitri)
Pull request description:
A port of `test/lint/lint-locale-dependence.sh` to a Python-script as part of the request of #24783. Checked for output-consistency.
ACKs for top commit:
laanwj:
Tested and code review ACK 3043a1bc9d
Tree-SHA512: 80555cf7aac156bab5488f85098731d1c12a42667fe7d0df0c35487ab8fc951654a70a15351a759282eabab8319f5aabd8bdb153412b9edc3a9033bef64fd609
2abfb6cb09 doc: Add 23.0 release notes (laanwj)
Pull request description:
Add 23.0 release notes from the wiki. We forgot to move these back to the branch, so they go direct-to-master.
Only change from the wiki is to add the credits.
ACKs for top commit:
Kvaciral:
re-ACK 2abfb6cb09
dunxen:
Re-ACK 2abfb6c
Tree-SHA512: 8541cb1d258ff9a3c6a42875434ae30638343c3192b268b53b92c8abc7a09685d778c3e44d08aa10da71f87a24933c0485dbc5d32c509c699d30886d57a45aff
The current help text for active and internal in listdescriptors is not
particularly helpful. They require the reader to already know what those
terms mean. This help text is updated to actually explain the
definitions of those words in context of a descriptor wallet.
Should we ever introduce a new address type that makes use of
`nStartByte` and adds fractional bytes to the group, then nStartByte
should be used as the offset for the last byte.
914076ed5f guix: Improve error message about missed macOS SDK (Hennadii Stepanov)
Pull request description:
The error message now mentions another option for users to specify the path to the macOS SDK.
ACKs for top commit:
fanquake:
ACK 914076ed5f
Tree-SHA512: e8b9a6c8aa984f02cdb4ad1eeb6c22510c0c74c47c831104ac410ef68b3bc40024a52400b5065a3e25603f8a040e72c8c464436cb9a0a018ca73b0d28e0d609d
9f5ab670e7 tests: Use descriptor that requires both legacy and segwit (Andrew Chow)
8a04a386f7 tests: Calculate input weight more accurately (Andrew Chow)
Pull request description:
The external input tests with specifying input weight would sometimes result in a test failure because it would add 2 to the calculated byte size in order to account for some of the variation in signature and script sizes. However 1 in 128 signatures are actually 1 byte smaller than we expect, so the difference between the actual signature size and our calculated size becomes 3 bytes which is outside of the tolerance of `assert_fee_amount` and would thus cause the test failure.
To resolve this, the 2 byte buffer is reduced to 1 byte, so in the above scenario, the difference is 2 bytes which is within the tolerance of `assert_fee_amount`. Additionally, instead of putting a fixed size that we assume is the correct size for the length of the compact size length prefix of data, we actually get the length of the compact size uint.
Lastly, the size calculation for a scriptWitness was simply incorrect and used fields that did not exist. This is fixed, and the test slightly modified so that it also produces a scriptWitness.
Fixes#24151
ACKs for top commit:
jonatack:
re-ACK 9f5ab670e7
glozow:
code review ACK 9f5ab670e7
Tree-SHA512: b7c7ffe8fb0c07bc9e72fbff1f9ef57ee01a57c56bf54b8873345c8b9572c3ce9402b24dc211910b478114a9e6420faef5a4bf8866f38c299971354e54ec4745
The prevector implementation currently can't be used with types that are
not trivially copyable, due to the use of memmove. Trivially copyable
implies that it is trivially destructible, see
https://eel.is/c++draft/class.prop#1.3
That means that the checks for std::is_trivially_destructible are not
necessary, and in fact where used it wouldn't be enough. E.g. in
`erase(iterator, iterator)` the elements in range first-last are destructed,
but it does not destruct elements left after `memmove`.
This commit removes the checks for `std::is_trivially_destructible`
and instead adds a `static_assert(std::is_trivially_copyable_v<T>);` to
make sure `prevector` is only used with supported types.
ee02c8bd9a util/check: Add CHECK_NONFATAL identity function, NONFATAL_UNREACHABLE AND UNREACHABLE macros (Aurèle Oulès)
Pull request description:
This PR replaces the macro `CHECK_NONFATAL` with an identity function.
I simplified the usage of `CHECK_NONFATAL` where applicable in `src/rpc`.
This function is useful in sanity checks for RPC and command-line interfaces.
Context: https://github.com/bitcoin/bitcoin/pull/24804#discussion_r846182474.
Also adds `UNREACHABLE_NONFATAL` macro.
ACKs for top commit:
jonatack:
ACK ee02c8bd9a
MarcoFalke:
ACK ee02c8bd9a🍨
Tree-SHA512: 3cba09223cd7b22e62fe5d0b46c4a024c1d9957d4268ba6d3fb07fcc0a5854fc0886bb3266184e6a7df5df91373b3e84edd6adf6999c4e934aeef8c043b01aa2
6958a26aa1 Revert "qt: Add ObjectInvoke template function" (Hennadii Stepanov)
249984f4f9 qt: Replace `GUIUtil::ObjectInvoke()` with `QMetaObject::invokeMethod()` (Hennadii Stepanov)
Pull request description:
A comment in 5659e73493 states that `GUIUtil::ObjectInvoke`
> can be replaced by a call to the QMetaObject::invokeMethod functor overload after Qt 5.10
ACKs for top commit:
w0xlt:
tACK 6958a26aa1 on Ubuntu 21.10, Qt 5.15.2.
promag:
Code review ACK 6958a26aa1.
Tree-SHA512: 6a840289568113cf38df6c1092821d626c2d206768a21d4dc6846b9dcccb4130477adb45ba718bb6bc15a3041871a7df3238983ac03db80406732be597693266
36f814c0e8 [netgroupman] Remove NetGroupManager::GetAsmap() (John Newbery)
4709fc2019 [netgroupman] Move asmap checksum calculation to NetGroupManager (John Newbery)
1b978a7e8c [netgroupman] Move GetMappedAS() and GetGroup() logic to NetGroupManager (John Newbery)
ddb4101e63 [net] Only use public CNetAddr functions and data in GetMappedAS() and GetGroup() (John Newbery)
6b2268162e [netgroupman] Add GetMappedAS() and GetGroup() (John Newbery)
19431560e3 [net] Move asmap into NetGroupManager (John Newbery)
17c24d4580 [init] Add netgroupman to node.context (John Newbery)
9b3836710b [build] Add netgroup.cpp|h (John Newbery)
Pull request description:
The asmap data is currently owned by addrman, but is used by both addrman and connman. #22791 made the data const and private (so that it can't be updated by other components), but it is still passed out of addrman as a reference to const, and used by `CNetAddress` to calculate the group and AS of the net address.
This RFC PR proposes to move all asmap data and logic into a new `NetGroupManager` component. This is initialized at startup, and the client components addrman and connman simply call `NetGroupManager::GetGroup(const CAddress&)` and `NetGroupManager::GetMappedAS(const CAddress&)` to get the net group and AS of an address.
ACKs for top commit:
mzumsande:
Code Review ACK 36f814c0e8
jnewbery:
CI failure seems spurious. I rebased onto latest master to trigger a new CI run, but whilst I was doing that, mzumsande ACKed 36f814c0e8, so I've reverted to that.
dergoegge:
Code review ACK 36f814c0e8
Tree-SHA512: 244a89cdfd720d8cce679eae5b7951e1b46b37835fccb6bdfa362856761bb110e79e263a6eeee8246140890f3bee2850e9baa7bc14a388a588e0e29b9d275175
MiniWallet's core method for creating txs (`create_self_transfer`)
right now always executes the `testmempoolaccept` RPC to check for
mempool validity or invalidity. In some test cases where we use
MiniWallet to create a huge number of transactions this can lead
to performance issues (e.g. feature_fee_estimation.py where the
execution time after MiniWallet usage almost doubled). Providing
the possibility to skip the mempool checks is a mitigation for
this.
master branch:
$ time ./test/functional/feature_fee_estimation.py
real 3m20.771s
user 2m52.360s
sys 0m39.340s
PR branch:
$ time ./test/functional/feature_fee_estimation.py
real 2m1.386s
user 1m42.510s
sys 0m22.980s
Also explicitly rehash in the cases where we modify a tx after signing
in feature_csv_activation.py. Parts of this test relied on the fact that
rehashing of transactions is done in the course of calculating a block's
merkle root (`calc_merkle_root`), which only works if no hash was
calculated before due to a caching mechanism.
In the following commit the txid in MiniWallet is calculated via
`rehash()`, i.e. this doesn't work anymore and we always have to
explicitely have the right hash before we calculate the merkle root.
Add more fs::path operator/ and operator+ overloads to prevent unsafe
string->path conversions on Windows that would cause strings to be
decoded according to the current Windows locale & code page instead of
the correct string encoding.
Update application code to deal with loss of implicit string->path
conversions by calling fs::u8path or fs::PathFromString explicitly, or
by just changing variable types from std::string to fs::path to avoid
conversions altoghther, or make them happen earlier.
In all cases, there's no change in behavior either (1) because strings
only contained ASCII characters and would be decoded the same regardless
of what encoding was used, or (2) because of the 1:1 mapping between
paths and strings using the PathToString and PathFromString functions.
Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
