Age | Commit message (Collapse) | Author |
|
31a15f0aff79d2b34a9640909b9e6fb39a647b60 bench: Disable WalletCreate* benchmarks when building with MSVC (Hennadii Stepanov)
23dc0c19acd54cad1bed2f14df024b6b533f2330 msvc, bench: Add missing source files to bench_bitcoin project (Hennadii Stepanov)
Pull request description:
On the master branch, the `bench_bitcoin.vcxproj` MSVC project misses wallet-specific source files.
This PR fixes this issue.
Benchmark run on Windows:
```
> src\bench_bitcoin.exe -filter="CoinSelection|BnBExhaustion|Wallet.*"
| ns/op | op/s | err% | total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
| 398,800.00 | 2,507.52 | 1.5% | 0.01 | `BnBExhaustion`
| 584,450.00 | 1,711.01 | 1.5% | 0.01 | `CoinSelection`
| 86,603,650.00 | 11.55 | 0.4% | 1.91 | `WalletAvailableCoins`
| 7,604.00 | 131,509.73 | 0.9% | 0.01 | `WalletBalanceClean`
| 124,028.57 | 8,062.66 | 2.6% | 0.01 | `WalletBalanceDirty`
| 7,587.12 | 131,802.30 | 1.9% | 0.01 | `WalletBalanceMine`
| 48.58 | 20,583,872.99 | 0.9% | 0.01 | `WalletBalanceWatch`
| 2,371,060.00 | 421.75 | 1.3% | 0.13 | `WalletCreateTxUseOnlyPresetInputs`
| 96,861,760.00 | 10.32 | 0.9% | 5.31 | `WalletCreateTxUsePresetInputsAndCoinSelection`
| 280.71 | 3,562,424.13 | 1.5% | 0.01 | `WalletIsMineDescriptors`
| 1,033.47 | 967,618.32 | 0.3% | 0.01 | `WalletIsMineLegacy`
| 282.36 | 3,541,599.91 | 0.5% | 0.01 | `WalletIsMineMigratedDescriptors`
| 484,547,300.00 | 2.06 | 1.0% | 2.43 | `WalletLoadingDescriptors`
| 29,924,300.00 | 33.42 | 0.4% | 0.15 | `WalletLoadingLegacy`
```
ACKs for top commit:
maflcko:
lgtm ACK 31a15f0aff79d2b34a9640909b9e6fb39a647b60
Tree-SHA512: 0241af06126edf612489322cdce66ba43792066b5400b1719a8b9d1ec62030e8a9d497e2f01e38290e94c387db59ccf2a458f4b35d3dc8030a1a1413d89eb792
|
|
This also changes behavior if ReadBlockFromDisk or
ReadRawBlockFromDisk fails. Previously, the node would crash
due to an assert. This has been replaced with logging the error,
disconnecting the peer, and returning early.
|
|
Also adds a static assertion that MAX_BLOCKTXN_DEPTH <= MIN_BLOCKS_TO_KEEP
|
|
63317103c9f2b0635558da814567bb79c17ae851 miniscript: make operator_mst consteval (Pieter Wuille)
Pull request description:
It seems modern compilers don't realize that all invocations of operator""_mst can be evaluated at compile time, despite the `constexpr` keyword.
Since C++20, we can force them to evaluate at compile time using `consteval`, turning all the miniscript type constants into actual compile-time constants.
This should give a nice but not very important speedup for miniscript logic, but it's also a way to start testing C++20 features.
ACKs for top commit:
hebasto:
re-ACK 63317103c9f2b0635558da814567bb79c17ae851.
theuni:
utACK 63317103c9f2b0635558da814567bb79c17ae851
Tree-SHA512: bdc9f1a6499b8bb3ca04f1a158c31e6876ba97206f95ee5718f50efd58b5b4e6b8867c07f791848430bfaa130b9676d8a68320b763cda9a340c75527acbfcc9e
|
|
on SunOS / illumos
976e5d8f7b2bc77cb1443b8bf0f38cb07db70e9b test: Fix `test/streams_tests.cpp` compilation on SunOS / illumos (Hennadii Stepanov)
Pull request description:
On systems where `int8_t` is defined as `char`, the `{S,Uns}erialize(Stream&, signed char)` functions become undefined.
This PR resolves the issue by testing `{S,Uns}erialize(Stream&, int8_t)` instead.
No behavior change on systems where `int8_t` is defined as `signed char`, which is the case for most other systems.
Fixes https://github.com/bitcoin/bitcoin/issues/29884.
An alternative approach is mentioned in https://github.com/bitcoin/bitcoin/issues/29884#issuecomment-2058434577 as well.
ACKs for top commit:
maflcko:
lgtm ACK 976e5d8f7b2bc77cb1443b8bf0f38cb07db70e9b
theuni:
ACK 976e5d8f7b2bc77cb1443b8bf0f38cb07db70e9b. Nice to have the serialization concept actually tested :)
Tree-SHA512: 1033863e584fa8e99a281b236fa01fc919f610a024bcec792116762e28c1c16ee481bd01325c3a0ca9dd9d753176aa63bd9ac7e08a9bbce772db2949d06f6e61
|
|
MAX_SCRIPT_ELEMENT_SIZE
ffc674595cb19b2fdc5705b355bdd3e7f724b860 Replace remaining "520" magic numbers with MAX_SCRIPT_ELEMENT_SIZE (Jon Atack)
Pull request description:
Noticed these while reviewing BIPs yesterday.
It would be clearer and more future-proof to refer to their constant name.
ACKs for top commit:
instagibbs:
ACK ffc674595cb19b2fdc5705b355bdd3e7f724b860
sipa:
ACK ffc674595cb19b2fdc5705b355bdd3e7f724b860
achow101:
ACK ffc674595cb19b2fdc5705b355bdd3e7f724b860
glozow:
ACK ffc674595cb19b2fdc5705b355bdd3e7f724b860, agree it's clearer for these comments to refer to the greppable name of the limit rather than the number
Tree-SHA512: 462afc1c64543877ac58cb3acdb01d42c6d08abfb362802f29f3482d75401a2a8adadbc2facd222a9a9fefcaab6854865ea400f50ad60bec17831d29f7798afe
|
|
It seems modern compilers don't realize that all invocations of operator""_mst
can be evaluated at compile time, despite the constexpr keyword.
Since C++20, we can force them to evaluate at compile time, turning all the
miniscript type constants into actual compile-time constants.
It appears that MSVC does not support consteval operator"" when used inside
certain expressions. For the few places where this happens, define a
constant outside the operator call.
Co-Authored-By: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
|
|
bd2de7ac591d7704b79304089ad1fb57e085da8b refactor, test: Always initialize pointer (Hennadii Stepanov)
Pull request description:
This change fixes MSVC warning [C4703](https://learn.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-4-c4703).
All `DisableSpecificWarnings` dropped from `test_bitcoin.vcxproj` as all remained are inherited from `common.init.vcxproj`.
Required to simplify warning suppression porting to the CMake-based build system.
ACKs for top commit:
maflcko:
utACK bd2de7ac591d7704b79304089ad1fb57e085da8b
sipsorcery:
utACK bd2de7ac591d7704b79304089ad1fb57e085da8b.
ryanofsky:
Code review ACK bd2de7ac591d7704b79304089ad1fb57e085da8b
Tree-SHA512: 006db041d3c3697a77d9df14de86cf7c8a10804b45789df01268b2236cf6452e77dc57e89f5d5a6bc26d4b5cd483f0722d6035649c8a523b57954bb1fc810d0c
|
|
As described in #10542 (and numerous other places), message signing in
Bitcoin Core only supports message signing using P2PKH addresses, at
least until a new message-signing standard is agreed upon.
Therefore update the possibly-misleading error message presented to the
user in the GUI to detail more specifically the reason their message
cannot be signed, in the case that a non P2PKH address is entered.
|
|
7f6fb73c82fdff2afe5edefcc335ba6707d5627d [refactor] use reference in for loop through iters (glozow)
6119f76ef72c1adc55c1a384c20f8ba9e1a01206 Process every MempoolAcceptResult regardless of PackageValidationResult (glozow)
2b482dc1f3fb248ccbe7eeb8c9c07d4bf1295a70 [refactor] have ProcessPackageResult take a PackageToValidate (glozow)
c2ada0530719e044bb498e0d78907a208214a71e [doc] remove redundant PackageToValidate comment (glozow)
9a762efc7a118c44556fa9ad404f6b54103bad41 [txpackages] use std::lexicographical_compare instead of sorting hex strings (glozow)
8496f69e1c2d1961db2604829cb6a289eb8dd3d6 [refactor] make MempoolAcceptResult::m_replaced_transactions non-optional (glozow)
Pull request description:
Followups from #28970:
- https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1568781077
- https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1585554972
- https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1581019326
- https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1581036209
- https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1586258730
ACKs for top commit:
instagibbs:
reACK https://github.com/bitcoin/bitcoin/pull/30012/commits/7f6fb73c82fdff2afe5edefcc335ba6707d5627d
Tree-SHA512: 9d8393d5f2fedbc6ebce582ff2a8ed074a02dd8e7dbf562c14d48b439fdc1ee6c3203b3609366d3c883d44655cc1a5c83a75ca56e490d25c1a34d95a0622d458
|
|
b50d127a7710d790c2ba4a08f01b832c2a0b1203 refactor: Make 64-bit shift explicit (Hennadii Stepanov)
Pull request description:
This PR fixes MSVC warning [C4334](https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-3-c4334) in the fuzzing code. Similar to https://github.com/bitcoin/bitcoin/pull/26252.
All `DisableSpecificWarnings` dropped from `fuzz.vcxproj` as all remained are inherited from `common.init.vcxproj`.
Required to simplify warning suppression porting to the CMake-based build system.
ACKs for top commit:
maflcko:
utACK b50d127a7710d790c2ba4a08f01b832c2a0b1203
sipsorcery:
utACK b50d127a7710d790c2ba4a08f01b832c2a0b1203
Tree-SHA512: 18f6082b4234506ad2f9df54e577031b97cdf9f7ef64cad4162f275660716ab73587a97d3af0f778dfd48d2751d8676b5d3381d0aa837fcc60a09704473a9209
|
|
This change fixes MSVC warning C4703.
See: https://learn.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-4-c4703
All `DisableSpecificWarnings` dropped from `test_bitcoin.vcxproj` as all
remained are inherited from `common.init.vcxproj`.
|
|
cpp-subprocess
8b52e7f628304e83b0e36fd97e617de0f71c5a62 update comments in cpp-subprocess (check_output references) (Sebastian Falbesoner)
97f159776ec06f767df1d4990aa7d0859140f52f remove unused method `Popen::kill` from cpp-subprocess (Sebastian Falbesoner)
908c51fe4afeba0af500c6275027b1afa1b3bd19 remove commented out code in cpp-subprocess (Sebastian Falbesoner)
ff79adbe056220202f7a56d67f788c38fc49ef9f remove unused templates from cpp-subprocess (Sebastian Falbesoner)
Pull request description:
This PR removes remaining code that is unused within the cpp-subprocess module (templates and commented out code). Happy to add more removals if anyone finds more unused parts. Note that there are some API functions of the `Popen` class that we don't use, e.g. `wait()`, `pid()`, `poll()`, `kill()`, but they sound IMHO common enough to be useful in the future, so not sure how deep we should go there.
ACKs for top commit:
fjahr:
Code review ACK 8b52e7f628304e83b0e36fd97e617de0f71c5a62
achow101:
ACK 8b52e7f628304e83b0e36fd97e617de0f71c5a62
hebasto:
ACK 8b52e7f628304e83b0e36fd97e617de0f71c5a62.
Tree-SHA512: 14c1cd2216185d941923f06fdc7acbeed66cd87e2691d9a352f7309b3e07fe4877b580f598a2e4106f9c48395ed6de00a0bfb5d3c3af9c4624d1956a0f543e99
|
|
|
|
|
|
|
|
This change fixes MSVC level-3 warning C4334.
See: https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-3-c4334
All `DisableSpecificWarnings` dropped from `fuzz.vcxproj` as all
remained are inherited from `common.init.vcxproj`.
|
|
e504b1fa1fa4d014b329dea81bfdf1aa55db238f test: Add test case for spending bare multisig (Brandon Odiwuor)
Pull request description:
Fixes https://github.com/bitcoin/bitcoin/issues/29113
ACKs for top commit:
ajtowns:
ACK e504b1fa1fa4d014b329dea81bfdf1aa55db238f ; LGTM and just checking the 1-of-3 case seems fine
maflcko:
utACK e504b1fa1fa4d014b329dea81bfdf1aa55db238f
achow101:
ACK e504b1fa1fa4d014b329dea81bfdf1aa55db238f
willcl-ark:
reACK e504b1fa1fa4d014b329dea81bfdf1aa55db238f
Tree-SHA512: 641a12599efa34e1a3eb65b125318df326628fef3e6886410ea9e63a044664fad7bcad46d1d6f41ddc59630746b9963cedb569c2682b5940b32b9225883da8f2
|
|
Adds the following fixups in txorphan fuzz tests:
- Don't bond the output count of the created orphans based on the number of available coins
- Allow duplicate inputs, when applicable, but don't store duplicate outpoints
Rationale
---------
The way the test is currently written, duplicate inputs are allowed based on a random flag (`duplicate_input`).
If the flag is unset, upon selecting an outpoint as input for a new transaction, the input is popped to prevent re-selection,
and later re-added to the collection (once all inputs have been picked). However, the re-addition to the collection is performed independently of whether the flag was set or not.
This means that, if the flag is set, the selected inputs are duplicated which in turn makes these inputs more likely to be re-picked in the following iteration of the loop.
Additionally, both the input and output count of the transaction and bonded to the number of available outpoints. This makes sense for the former, but the latter shouldn't be.
|
|
The RPC documentation for `getblockchaininfo`, `getmininginfo` and
`getnetworkinfo` states that "warnings" returns "any network and
blockchain warnings". In practice, only a single warning is returned.
Fix that by returning all warnings as an array.
As a side benefit, cleans up the GetWarnings() logic.
|
|
|
|
|
|
|
|
No behavior change, but getting the hex string is more expensive than
necessary.
|
|
|
|
-BEGIN VERIFY SCRIPT-
perl -0777 -pi -e 's/#if defined\(HAVE_CONFIG_H\)\n#include <config\/bitcoin-config.h>.*\n#endif.*\n/#include <config\/bitcoin-config.h> \/\/ IWYU pragma: keep\n/g' $( git grep -l '#include <config/bitcoin-config.h>' )
-END VERIFY SCRIPT-
|
|
774359b4a96d2724dc70f900cb71e084a77164da build, msvc: Compile `test\fuzz\bitdeque.cpp` (Hennadii Stepanov)
85f50a46c50e7e56b5ee2d7258021939cd80c550 refactor: Fix "error C2248: cannot access private member" on MSVC (Hennadii Stepanov)
Pull request description:
This PR resolves one point from the https://github.com/bitcoin/bitcoin/pull/29774#issuecomment-2028808614:
> What is the issue with the bitdeque... ?
ACKs for top commit:
maflcko:
lgtm ACK 774359b4a96d2724dc70f900cb71e084a77164da
sipa:
utACK 774359b4a96d2724dc70f900cb71e084a77164da
achow101:
ACK 774359b4a96d2724dc70f900cb71e084a77164da
dergoegge:
utACK 774359b4a96d2724dc70f900cb71e084a77164da
Tree-SHA512: dba5c0217b915468af08475795437a10d8e8dedfadeb319f36d9b1bf54a91a8b2c61470a6047565855276c2bc8589c7776dc19237610b65b57cc841a303de8b3
|
|
both are provided
82f41d76f1c6ad38290917dad5499ffbe6b3974d Added seednode prioritization message to help output (tdb3)
3120a4678ab2a71a381e847688f44068749cfa97 Gives seednode priority over dnsseed if both are provided (Sergi Delgado Segura)
Pull request description:
This is a follow-up of #27577
If both `seednode` and `dnsseed` are provided, the node will start a race between them in order to fetch data to feed the `addrman`.
This PR gives priority to `seednode` over `dnsseed` so if some nodes are provided as seeds, they can be tried before defaulting to the `dnsseeds`
ACKs for top commit:
davidgumberg:
untested reACK https://github.com/bitcoin/bitcoin/commit/82f41d76f1c6ad38290917dad5499ffbe6b3974d
itornaza:
tested re-ACK 82f41d76f1c6ad38290917dad5499ffbe6b3974d
achow101:
ACK 82f41d76f1c6ad38290917dad5499ffbe6b3974d
cbergqvist:
ACK 82f41d76f1c6ad38290917dad5499ffbe6b3974d
Tree-SHA512: 4e39e10a7449af6cd9b8f9f6878f846b94bca11baf89ff2d4fbcd4f28293978a6ed71a3a86cea36d49eca891314c834e32af93f37a09c2cc698a878f84d31c62
|
|
c6be144c4b774a03a8bcab5a165768cf81e9b06b Remove timedata (stickies-v)
92e72b5d0d49aa395e626c238bc28aba8e4c3d44 [net processing] Move IgnoresIncomingTxs to PeerManagerInfo (dergoegge)
7d9c3ec622d73a98d07ab3cee78751718982a5bc [net processing] Introduce PeerManagerInfo (dergoegge)
ee178dfcc1175e0af8163216c9c024f4bfc97965 Add TimeOffsets helper class (stickies-v)
55361a15d1aa6984051441bce88112000688fb43 [net processing] Use std::chrono for type-safe time offsets (stickies-v)
038fd979effb54ee76ce1b7cf078e920c652326a [net processing] Move nTimeOffset to net_processing (dergoegge)
Pull request description:
[An earlier approach](https://github.com/bitcoin/bitcoin/commits/1d226ae1f984c5c808f5c24c431b959cdefa692e/) in #28956 involved simplifying and refactoring the network-adjusted time calculation logic, but this was eventually [left out](https://github.com/bitcoin/bitcoin/pull/28956#issuecomment-1904214370) of the PR to make it easier for reviewers to focus on consensus logic changes.
Since network-adjusted time is now only used for warning/informational purposes, cleaning up the logic (building on @dergoegge's approach in #28956) should be quite straightforward and uncontroversial. The main changes are:
- Previously, we would only calculate the time offset from the first 199 outbound peers that we connected to. This limitation is now removed, and we have a proper rolling calculation. I've reduced the set to 50 outbound peers, which seems plenty.
- Previously, we would automatically use the network-adjusted time if the difference was < 70 mins, and warn the user if the difference was larger than that. Since there is no longer any automated time adjustment, I've changed the warning threshold to ~~20~~ 10 minutes (which is an arbitrary number).
- Previously, a warning would only be raised once, and then never again until node restart. This behaviour is now updated to 1) warn to log for every new outbound peer for as long as we appear out of sync, 2) have the RPC warning toggled on/off whenever we go in/out of sync, and 3) have the GUI warn whenever we are out of sync (again), but limited to 1 messagebox per 60 minutes
- no more globals
- remove the `-maxtimeadjustment` startup arg
Closes #4521
ACKs for top commit:
sr-gi:
Re-ACK [c6be144](https://github.com/bitcoin/bitcoin/pull/29623/commits/c6be144c4b774a03a8bcab5a165768cf81e9b06b)
achow101:
reACK c6be144c4b774a03a8bcab5a165768cf81e9b06b
dergoegge:
utACK c6be144c4b774a03a8bcab5a165768cf81e9b06b
Tree-SHA512: 1063d639542e882186cdcea67d225ad1f97847f44253621a8c4b36c4d777e8f5cb0efe86bc279f01e819d33056ae4364c3300cc7400c087fb16c3f39b3e16b96
|
|
packages
e518a8bf8abf3d7b83c9013f56d0dca18ae04d6f [functional test] opportunistic 1p1c package submission (glozow)
87c5c524d63c833cf490c7f2f73d72695ad480df [p2p] opportunistically accept 1-parent-1-child packages (glozow)
6c51e1d7d021ed6523107a6db87a865aaa8fc4c9 [p2p] add separate rejections cache for reconsiderable txns (glozow)
410ebd6efaf20fe4715c9b825103b74db69f35ac [fuzz] break out parent functions and add GetChildrenFrom* coverage (glozow)
d095316c1c23e9460dfbd9fdbaf292063adcd080 [unit test] TxOrphanage::GetChildrenFrom* (glozow)
2f51cd680fb4323f1c792dae37d4c4e0e0e35804 [txorphanage] add method to get all orphans spending a tx (glozow)
092c978a42e8f4a02291b994713505ba8aac8b28 [txpackages] add canonical way to get hash of package (glozow)
c3c1e15831c463df7968b028a77e787da7e6256d [doc] restore comment about why we check if ptx HasWitness before caching rejected txid (glozow)
6f4da19cc3b1b7cd23cb4be95a6bb9acb79eb3bf guard against MempoolAcceptResult::m_replaced_transactions (glozow)
Pull request description:
This enables 1p1c packages to propagate in the "happy case" (i.e. not reliable if there are adversaries) and contains a lot of package relay-related code. See https://github.com/bitcoin/bitcoin/issues/27463 for overall package relay tracking.
Rationale: This is "non-robust 1-parent-1-child package relay" which is immediately useful.
- Relaying 1-parent-1-child CPFP when mempool min feerate is high would be a subset of all package relay use cases, but a pretty significant improvement over what we have today, where such transactions don't propagate at all. [1]
- Today, a miner can run this with a normal/small maxmempool to get revenue from 1p1c CPFP'd transactions without losing out on the ones with parents below mempool minimum feerate.
- The majority of this code is useful for building more featureful/robust package relay e.g. see the code in #27742.
The first 2 commits are followups from #29619:
- https://github.com/bitcoin/bitcoin/pull/29619#discussion_r1523094034
- https://github.com/bitcoin/bitcoin/pull/29619#discussion_r1519819257
Q: What makes this short of a more full package relay feature?
(1) it only supports packages in which 1 of the parents needs to be CPFP'd by the child. That includes 1-parent-1-child packages and situations in which the other parents already pay for themselves (and are thus in mempool already when the package is submitted). More general package relay is a future improvement that requires more engineering in mempool and validation - see #27463.
(2) We rely on having kept the child in orphanage, and don't make any attempt to protect it while we wait to receive the parent. If we are experiencing a lot of orphanage churn (e.g. an adversary is purposefully sending us a lot of transactions with missing inputs), we will fail to submit packages. This limitation has been around for 12+ years, see #27742 which adds a token bucket scheme for protecting package-related orphans at a limited rate per peer.
(3) Our orphan-handling logic is somewhat opportunistic; we don't make much effort to resolve an orphan beyond asking the child's sender for the parents. This means we may miss packages if the first sender fails to give us the parent (intentionally or unintentionally). To make this more robust, we need receiver-side logic to retry orphan resolution with multiple peers. This is also an existing problem which has a proposed solution in #28031.
[1]: see this writeup and its links https://github.com/bitcoin/bips/blob/02ec218c7857ef60914e9a3d383b68caf987f70b/bip-0331.mediawiki#propagate-high-feerate-transactions
ACKs for top commit:
sr-gi:
tACK e518a8bf8abf3d7b83c9013f56d0dca18ae04d6f
instagibbs:
reACK e518a8bf8abf3d7b83c9013f56d0dca18ae04d6f
theStack:
Code-review ACK e518a8bf8abf3d7b83c9013f56d0dca18ae04d6f :package:
dergoegge:
light Code review ACK e518a8bf8abf3d7b83c9013f56d0dca18ae04d6f
achow101:
ACK e518a8bf8abf3d7b83c9013f56d0dca18ae04d6f
Tree-SHA512: 632579fbe7160cb763bbec6d82ca0dab484d5dbbc7aea90c187c0b9833b8d7c1e5d13b8587379edd3a3b4a02a5a1809020369e9cd09a4ebaf729921f65c15943
|
|
6a8b2befeab25e4e92d8e947a23e78014695e06c refactor: Avoid copying util::Result values (Ryan Ofsky)
834f65e82405bbed336f98996bc8cef366bbed0f refactor: Drop util::Result operator= (Ryan Ofsky)
Pull request description:
This PR just contains the first two commits of #25665.
It disables copying of `util::Result` objects because unnecessary copies are inefficient and not possible after #25665, which makes `util::Result` object move-only.
It disables the assignment operator and replaces it with an `Update()` method, because #25665 adds more information to `util::Result` objects (warning and error messages and failure values) and having an assignment operator that overwrites data instead of merging it would make it easy to accidentally erase existing information while trying to assign new information.
ACKs for top commit:
stickies-v:
re-ACK 6a8b2befeab25e4e92d8e947a23e78014695e06c
achow101:
ACK 6a8b2befeab25e4e92d8e947a23e78014695e06c
furszy:
re-ACK https://github.com/bitcoin/bitcoin/commit/6a8b2befeab25e4e92d8e947a23e78014695e06c
Tree-SHA512: 3f21af9031d50d6c68cca69133de03080f69b1ddcf8b140bdeb762069f14645209b2586037236d15b6ebd8973af0fbefd7e83144aeb7b84078a4cb4df812f984
|
|
to the mempool
cc15c5bfd1eb3903de246c124702a7c66c687008 fuzz: don't allow adding duplicate transactions to the mempool (Suhas Daftuar)
Pull request description:
Filter duplicate transaction ids from being added to the mempool in the `partially_downloaded_block` fuzz target.
I think a prerequisite for calling `CTxMemPool::addUnchecked` should be that the underlying txid doesn't already exist in the mempool (otherwise `addUnchecked` would need a way to return failure, which we don't currently have).
ACKs for top commit:
glozow:
utACK cc15c5bfd1eb3903de246c124702a7c66c687008 makes sense to me
maflcko:
lgtm ACK cc15c5bfd1eb3903de246c124702a7c66c687008
brunoerg:
ACK cc15c5bfd1eb3903de246c124702a7c66c687008
dergoegge:
utACK cc15c5bfd1eb3903de246c124702a7c66c687008
Tree-SHA512: 85f84ce405aba584e6d00391515f0a86c5648ce8b2da69036e50a6c1f6833d050d09b1972cc5ffbe7c4edb3e5f7f965ef34bd839deeddac27a889cc8d2e53b8f
|
|
This help text was introduced about two years ago and now a significant
portion of users are using version 23.0 and higher
|
|
30a6c999351041d6a1e8712a9659be1296a1b46a rpc: access some args by name (stickies-v)
bbb31269bfa449e82d3b6a20c2c3481fb3dcc316 rpc: add named arg helper (stickies-v)
13525e0c248eab9b199583cde76430c6da2426e2 rpc: add arg helper unit test (stickies-v)
Pull request description:
Adds string overloads for the `RPCHelpMan::Arg` and `RPCHelpMan::MaybeArg` helpers to be able to access RPC arguments by name instead of index number. Especially in RPCs with a large number of parameters, this can be quite helpful.
Example usage:
```cpp
const auto action{self.Arg<std::string>("action")};
```
Most of the LoC is adding test coverage and documentation updates. No behaviour change.
An alternative approach to #27788 with significantly less overhaul.
ACKs for top commit:
fjahr:
Code review ACK 30a6c999351041d6a1e8712a9659be1296a1b46a
maflcko:
ACK 30a6c999351041d6a1e8712a9659be1296a1b46a 🥑
ryanofsky:
Code review ACK 30a6c999351041d6a1e8712a9659be1296a1b46a. Nice change! Implementation is surprisingly simple and additional unit test coverage is welcome, too.
Tree-SHA512: 4904f5f914fe1d421d32f60edb7c5a028c8ea0f140a2f207a106b4752d441164e073066a6bf2e17693f859fe847815a96609d3cf521e0ac4178d8cd09362ea3d
|
|
SetMockTime
fae0db555c12dca75fb09e5fa7bbabdf39b8c1df refactor: Use chrono type for g_mock_time (MarcoFalke)
fa382d3dd0592f3cbd6e1de791449f49e06dae86 test: Add missing Assert(mock_time_in >= 0s) to SetMockTime (MarcoFalke)
Pull request description:
Seems odd to have the assert in the *deprecated* function, but not in the other.
Fix this by adding it to the other, and by inlining the deprecated one.
Also, use chrono type for the global mocktime variable.
ACKs for top commit:
davidgumberg:
crACK https://github.com/bitcoin/bitcoin/pull/29872/commits/fae0db555c12dca75fb09e5fa7bbabdf39b8c1df
stickies-v:
ACK fae0db555c12dca75fb09e5fa7bbabdf39b8c1df
Tree-SHA512: 630c2917422ff2a7fa307114f95f22ad3c205429ffe36e67f0b2650733e40c876289c1aecebe882a9123d3106db7606bd6eff067ed6e2ecb95765984d3fe8612
|
|
|
|
Remove obsolete `check_output` references in the comments and remove
the numbering of the Popen API methods, as they don't seem to provide a
value and just make diffs larger for future changes.
|
|
|
|
All network addresses are being iterated over and added, not just the first one per interface.
|
|
Checking the interface name is kind of brittle. In the age of network
namespaces and containers, there is no reason a loopback interface can't
be called differently.
Check for the `IFF_LOOPBACK` flag to detect loopback interface instead.
|
|
|
|
18fd522ca9a74cf8690a6c9b9b729e78c6ed41fd ci, msvc: Add "Run fuzz binaries" step (Hennadii Stepanov)
52933d7283736fe3ae15e7ac44c02ca3bd95fe6d fuzz: Pass `SystemRoot` environment variable to subprocess (Hennadii Stepanov)
23cb8207cdd6c674480840b76626039cdfe7cb13 ci, msvc: Add "Clone fuzz corpus" step (Hennadii Stepanov)
19dceddf4bcdb74e84cf27229039a239b873d41b build, msvc: Build `fuzz.exe` binary (Hennadii Stepanov)
4c078d7bd278fa8b4db6e1da7b9b747f49a8ac4c build, msvc: Enable preprocessor conformance mode (Hennadii Stepanov)
09f5a74198c328c80539c17d951a70558e6b361e fuzz: Re-implement `read_stdin` in portable way (Hennadii Stepanov)
Pull request description:
Closes https://github.com/bitcoin/bitcoin/issues/29760.
Suggested in https://github.com/bitcoin/bitcoin/pull/29758#issuecomment-2025593572.
ACKs for top commit:
maflcko:
lgtm ACK 18fd522ca9a74cf8690a6c9b9b729e78c6ed41fd 🔍
sipsorcery:
tACK 18fd522ca9a74cf8690a6c9b9b729e78c6ed41fd
sipa:
utACK 18fd522ca9a74cf8690a6c9b9b729e78c6ed41fd
Tree-SHA512: 672ed6926ee9091f68f13780e77b60fc1d48731f16e847d849374f8426ffe1dafd9bcab06a27af62e8052ba345bb57f20f40579d6be8540c12ef85c23a6eec8b
|
|
ConsiderEviction
|
|
|
|
|
|
It's very hard to randomly construct a transaction that would be the
parent of an existing orphanage tx. For functions like
AddChildrenToWorkSet and GetChildren that take orphan parents, use a tx
that was previously constructed.
|
|
|
|
|
|
|
|
|