Age | Commit message (Collapse) | Author |
|
|
|
|
|
|
|
returning `complete=true`
7e36dca657c66bc70b04d5b850e5a335aecfb902 test: add test for modififed walletprocesspsbt calls (willcl-ark)
39cea21ec51b9838669c38fefa14f25c36ae7096 wallet: fix FillPSBT errantly showing as complete (willcl-ark)
Pull request description:
Fixes: #30077
Fix cases of calls to `FillPSBT` returning `complete=true` when it's not
the case.
This can happen when some inputs have been signed but the transaction is
subsequently modified, e.g. in the context of PayJoins.
Also fixes a related bug where a finalized hex string is attempted to be
added during `walletprocesspsbt` but a CHECK_NONFATAL causes an abort.
ACKs for top commit:
achow101:
ACK 7e36dca657c66bc70b04d5b850e5a335aecfb902
ismaelsadeeq:
Tested ACK 7e36dca657c66bc70b04d5b850e5a335aecfb902
pinheadmz:
re-ACK 7e36dca657c66bc70b04d5b850e5a335aecfb902
Tree-SHA512: e35d19789899c543866d86d513506494d672e4bed9aa36a995dbec4e72f0a8ec5536b57c4a940a18002ae4a8efd0b007c77ba64e57cd52af98e4ac0e7bf650d6
|
|
startup on bind failure
bca346a97056748f1e3fb899f336d56d9fd45a64 net: require P2P binds to succeed (Vasil Dimov)
af552534ab83c572d3bc3f93ccaee5c1961ccab5 net: report an error if unable to bind on the Tor listening addr:port (Vasil Dimov)
9a7e5f4d68dadc64a675f32d1e91199d6b1aa095 net: don't extra bind for Tor if binds are restricted (Vasil Dimov)
Pull request description:
Make it possible to disable the Tor binding on `127.0.0.1:8334` and stop startup if any P2P bind fails instead of "if all P2P binds fail".
Fixes https://github.com/bitcoin/bitcoin/issues/22726
Fixes https://github.com/bitcoin/bitcoin/issues/22727
ACKs for top commit:
achow101:
ACK bca346a97056748f1e3fb899f336d56d9fd45a64
cbergqvist:
ACK bca346a97056748f1e3fb899f336d56d9fd45a64
pinheadmz:
ACK bca346a97056748f1e3fb899f336d56d9fd45a64
Tree-SHA512: fabef89a957191eea4f3e3b6109d2b8389f27ecc74440a920b0c10f31fff00a85bcfd1eb3c91826c7169c618f4de8a8d0a260e2caf40fd854f07ea9a980d8603
|
|
verification after signing (+introduce signing benchmarks)
fe92c15f0c44d1405b9048306736bd0eae868506 script/sign: avoid duplicated signature verification after signing (Sebastian Falbesoner)
080089567ca766d4c1cde8ec0e5c7df48f566e07 bench: add benchmark for `SignTransaction` (Sebastian Falbesoner)
Pull request description:
This PR is a small performance improvement on the `SignTransaction` function, which is used mostly by the wallet (obviously) and the `signrawtransactionwithkey` RPC. The lower-level function `ProduceSignature` already calls `VerifyScript` internally as last step in order to check whether the signature data is complete:
https://github.com/bitcoin/bitcoin/blob/daa56f7f665183bcce3df146f143be37f33c123e/src/script/sign.cpp#L568-L570
If and only if that is the case, the `complete` field of the `SignatureData` is set to `true` accordingly and there is no need then to verify the script after again, as we already know that it would succeed.
This leads to a rough ~20% speed-up for `SignTransaction` for single-input ECDSA or Taproot transactions, according to the newly introduced `SignTransaction{ECDSA,Taproot}` benchmarks:
```
$ ./src/bench/bench_bitcoin --filter=SignTransaction.*
```
without commit 18185f4f578b8795fdaa75926630a691e9c8d0d4:
| ns/op | op/s | err% | total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
| 185,597.79 | 5,388.00 | 1.6% | 0.22 | `SignTransactionECDSA`
| 141,323.95 | 7,075.94 | 2.1% | 0.17 | `SignTransactionSchnorr`
with commit 18185f4f578b8795fdaa75926630a691e9c8d0d4:
| ns/op | op/s | err% | total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
| 149,757.86 | 6,677.45 | 1.4% | 0.18 | `SignTransactionECDSA`
| 108,284.40 | 9,234.94 | 2.0% | 0.13 | `SignTransactionSchnorr`
Note that there are already signing benchmarks in the secp256k1 library, but `SignTransaction` does much more than just the cryptographical parts, i.e.:
* calculate the unsigned tx's `PrecomputedTransactionData` if necessary
* apply Solver on the prevout scriptPubKey, fetch the relevant keys from the signing provider
* perform the actual signing operation (for ECDSA signatures, that could be more than once due to low-R grinding)
* verify if the signatures are correct by calling `VerifyScript` (more than once currently, which is fixed by this PR)
so it probably makes sense to also have benchmarks from that higher-level application perspective.
ACKs for top commit:
achow101:
ACK fe92c15f0c44d1405b9048306736bd0eae868506
furszy:
utACK fe92c15f0c44
glozow:
light review ACK fe92c15f0c44d1405b9048306736bd0eae868506
Tree-SHA512: b7225ff9e8a640ca5222dea5b2a463a0f9b9de704e4330b5b9a7bce2d63a1f4620575c474a8186f4708d7d9534eab55d000393d99db79c0cfc046b35f0a4a778
|
|
fa6270737eb9655bfb4e29b7070ecb6cd2087b7f rpc: Use CHECK_NONFATAL over Assert (MarcoFalke)
Pull request description:
Any RPC method should not abort the whole node when an internal logic error happens.
Fix it by just aborting this single RPC method call when an error happens.
Also, fix the linter to find the fixed cases.
ACKs for top commit:
achow101:
ACK fa6270737eb9655bfb4e29b7070ecb6cd2087b7f
stickies-v:
ACK fa6270737eb9655bfb4e29b7070ecb6cd2087b7f
tdb3:
ACK fa6270737eb9655bfb4e29b7070ecb6cd2087b7f
hodlinator:
ACK fa6270737eb9655bfb4e29b7070ecb6cd2087b7f
Tree-SHA512: dad2f31b01a66578949009499e4385fb4d72f0f897419f2a6e0ea02e799b9a31e6ecb5a67fa5d27fcbc7939fe8acd62dc04e877b35831493b7f2c604dec7dc64
|
|
thread and scheduler
5fd48360198d2ac49e43b24cc1469557b03567b8 init: change shutdown order of load block thread and scheduler (Martin Zumsande)
Pull request description:
This avoids situations during a reindex, in which the shutdown doesn't finish since `LimitValidationInterfaceQueue()` is called by the load block thread when the scheduler is already stopped, in which case it would block indefinitely. This can lead to intermittent failures in `feature_reindex.py` (#30424), which I could locally reproduce with
```diff
diff --git a/src/validation.cpp b/src/validation.cpp
index 74f0e4975c..be1706fdaf 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -3446,6 +3446,7 @@ static void LimitValidationInterfaceQueue(ValidationSignals& signals) LOCKS_EXCL
AssertLockNotHeld(cs_main);
if (signals.CallbacksPending() > 10) {
+ std::this_thread::sleep_for(std::chrono::milliseconds(50));
signals.SyncWithValidationInterfaceQueue();
}
}
```
It has also been reported by users running `reindex-chainstate` (#23234).
I thought for a bit about potential downsides of changing this order, but couldn't find any.
Fixes #30424
Fixes #23234
ACKs for top commit:
maflcko:
review ACK 5fd48360198d2ac49e43b24cc1469557b03567b8
hebasto:
re-ACK 5fd48360198d2ac49e43b24cc1469557b03567b8.
tdb3:
ACK 5fd48360198d2ac49e43b24cc1469557b03567b8
BrandonOdiwuor:
Code Review ACK 5fd48360198d2ac49e43b24cc1469557b03567b8
Tree-SHA512: 3b8894e99551c5d4392b55eaa718eee05841a7287aeef2978699e1d633d5234399fa2f5a3e71eac1508d97845906bd33e0e63e5351855139e7be04c421359b36
|
|
51fa26239af9bbfd44029aaf595cb4c6a8d4a75d refactor: Mark some static global vars as const (TheCharlatan)
39f9b80fba85d9818222c4d76e99ea1a804f5dda refactor: De-globalize last notified header index (TheCharlatan)
3443943f86678eb27d9895f3871fcf3945eb1c4f refactor: De-globalize validation benchmark timekeeping (TheCharlatan)
Pull request description:
In future, users of the kernel library might run multiple chainstates in parallel, or create and destroy multiple chainstates over the lifetime of a process. Having static, mutable variables could lead to state inconsistencies in these scenarios.
---
This pull request is part of the [libbitcoinkernel project](https://github.com/bitcoin/bitcoin/issues/27587).
ACKs for top commit:
dergoegge:
Code review ACK 51fa26239af9bbfd44029aaf595cb4c6a8d4a75d
maflcko:
ACK 51fa26239af9bbfd44029aaf595cb4c6a8d4a75d 🍚
tdb3:
code review ACK 51fa26239af9bbfd44029aaf595cb4c6a8d4a75d
Tree-SHA512: da91aa7ffa343325cabb8764ef03c8358845662cf0ba8a6cc1dd38e40e5462d88734f2b459c2de8e7a041551eda9143d92487842609f7f30636f61a0cd3c57ee
|
|
AEADChacha20Poly1305
8607773750e60931e51a33e48cd077a1dedf9db3 Add fuzz test for FSChaCha20Poly1305 (stratospher)
c807f3322897ca8c0da114556e5936e389da5059 Add fuzz test for AEADChacha20Poly1305 (stratospher)
Pull request description:
This PR adds fuzz tests for `AEADChaCha20Poly1305` and `FSChaCha20Poly1305` introduced in #28008.
Run using:
```
$ FUZZ=crypto_aeadchacha20poly1305 src/test/fuzz/fuzz
$ FUZZ=crypto_fschacha20poly1305 src/test/fuzz/fuzz
```
ACKs for top commit:
dergoegge:
tACK 8607773750e60931e51a33e48cd077a1dedf9db3
marcofleon:
Tested ACK 8607773750e60931e51a33e48cd077a1dedf9db3. Ran both targets for ~200 CPU hours. Coverage of intended targets looks good to me. The simulation of damaged keys and checks that follow seem useful as well.
Tree-SHA512: b6b85661d896e653caeed330f941fde665fc2bbd97ecd340808a3f365c469fe9134aa77316569a771dc36d1158cac1a5f76700bcfc45fff12aef07562e48feb9
|
|
|
|
The TxOrphanage is now guarded externally by m_tx_download_mutex.
|
|
|
|
This also means AlreadyHaveTx no longer needs cs_main held.
|
|
Resetting m_recent_rejects once per block is more efficient than
comparing hashRecentRejectsChainTip with the chain tip every time we
call AlreadyHaveTx. We keep hashRecentRejectsChainTip for now to assert
that updates happen correctly; it is removed in the next commit.
|
|
This is a synchronous callback notifying clients of all tip changes.
It allows clients to respond to a new block immediately after it is
connected. The synchronicity is important for things like
m_recent_rejects, in which a transaction's validity can change (rejected
vs accepted) when this event is processed. For example, the transaction
might have a timelock condition that has just been met. This is distinct
from something like m_recent_confirmed_transactions, in which the
validation outcome is the same (valid vs already-have), so it does not
need to be reset immediately.
|
|
We need to synchronize between various tx download structures.
TxRequest does not inherently need cs_main for synchronization, and it's
not appropriate to lock all of the tx download logic under cs_main.
|
|
16bd283b3ad05daa41259a062aee0fc05b463fa6 Reapply "test: p2p: check that connecting to ourself leads to disconnect" (Sebastian Falbesoner)
0dbcd4c14855fe2cba15a32245572b693dc18c4e net: prevent sending messages in `NetEventsInterface::InitializeNode` (Sebastian Falbesoner)
66673f1c1302c986e344c7f44bb0b352213d5dc8 net: fix race condition in self-connect detection (Sebastian Falbesoner)
Pull request description:
This PR fixes a recently discovered race condition in the self-connect detection (see #30362 and #30368).
Initiating an outbound network connection currently involves the following steps after the socket connection is established (see [`CConnman::OpenNetworkConnection`](https://github.com/bitcoin/bitcoin/blob/bd5d1688b4311e21c0e0ff89a3ae02ef7d0543b8/src/net.cpp#L2923-L2930) method):
1. set up node state
2. queue VERSION message (both steps 1 and 2 happen in [`InitializeNode`](https://github.com/bitcoin/bitcoin/blob/bd5d1688b4311e21c0e0ff89a3ae02ef7d0543b8/src/net_processing.cpp#L1662-L1683))
3. add new node to vector `m_nodes`
If we connect to ourself, it can happen that the sent VERSION message (step 2) is received and processed locally *before* the node object is added to the connection manager's `m_nodes` vector (step 3). In this case, the self-connect remains undiscovered, as the detection doesn't find the outbound peer in `m_nodes` yet (see `CConnman::CheckIncomingNonce`).
Fix this by swapping the order of 2. and 3., by taking the `PushNodeVersion` call out of `InitializeNode` and doing that in the `SendMessages` method instead, which is only called for `CNode` instances in `m_nodes`.
The temporarily reverted test introduced in #30362 is readded. Fixes #30368.
Thanks go to vasild, mzumsande and dergoegge for suggestions on how to fix this (see https://github.com/bitcoin/bitcoin/issues/30368#issuecomment-2200625017 ff. and https://github.com/bitcoin/bitcoin/pull/30394#discussion_r1668290789).
ACKs for top commit:
naiyoma:
tested ACK [https://github.com/bitcoin/bitcoin/pull/30394/commits/16bd283b3ad05daa41259a062aee0fc05b463fa6](https://github.com/bitcoin/bitcoin/pull/30394/commits/16bd283b3ad05daa41259a062aee0fc05b463fa6), built and tested locally, test passes successfully.
mzumsande:
ACK 16bd283b3ad05daa41259a062aee0fc05b463fa6
tdb3:
ACK 16bd283b3ad05daa41259a062aee0fc05b463fa6
glozow:
ACK 16bd283b3ad05daa41259a062aee0fc05b463fa6
dergoegge:
ACK 16bd283b3ad05daa41259a062aee0fc05b463fa6
Tree-SHA512: 5b8aced6cda8deb38d4cd3fe4980b8af505d37ffa0925afaa734c5d81efe9d490dc48a42e1d0d45dd2961c0e1172a3d5b6582ae9a2d642f2592a17fbdc184445
|
|
Rather than pass options individually to createNewBlock and then
combining them into BlockAssembler::Options, this commit introduces
BlockCreateOptions and passes that instead.
Currently there's only one option (use_mempool) but the next
commit adds more.
Co-authored-by: Ryan Ofsky <ryan@ofsky.org>
|
|
|
|
fa14e1d9d5c5dc44396a01583ae94480b7bc29ee log: Fix __func__ in LogError in blockstorage module (MarcoFalke)
fad59a2f0f37f5b7f6076fd91be43448e35f4b7e log: LogError with FlatFilePos in UndoReadFromDisk (MarcoFalke)
aaaa3323f37526862ebf2a2a4bf522c661e6976e refactor: Mark IsBlockPruned const (MarcoFalke)
Pull request description:
These errors should never happen in normal operation. If they do,
knowing the `FlatFilePos` may be useful to determine if data corruption
happened. Also, handle the error `pos.IsNull()` as part of `OpenUndoFile`,
because it may as well have happened due to data corruption.
This mirrors the `LogError` behavior from `ReadBlockFromDisk`.
Also, two other fixup commits in this module.
ACKs for top commit:
kevkevinpal:
ACK [fa14e1d](https://github.com/bitcoin/bitcoin/pull/30428/commits/fa14e1d9d5c5dc44396a01583ae94480b7bc29ee)
tdb3:
cr and light test ACK fa14e1d9d5c5dc44396a01583ae94480b7bc29ee
ryanofsky:
Code review ACK fa14e1d9d5c5dc44396a01583ae94480b7bc29ee. This should make logging clearer and more consistent
Tree-SHA512: abb492a919b4796698d1de0a7874c8eae355422b992aa80dcd6b59c2de1ee0d2949f62b3cf649cd62892976fee640358f7522867ed9d48a595d6f8f4e619df50
|
|
fa690c8e532672f7ab53be6f7a0bb3070858745e test: [refactor] Pass TestOpts (MarcoFalke)
Pull request description:
Currently optional test context setup settings are passed by adding a new optional argument to the constructors. For example `extra_args`. This is problematic, because:
* Adding more optional settings in the future requires touching all affected constructors, increasing their verbosity.
* Setting only a later option requires setting the earlier ones.
* Clang-tidy named args passed to `std::make_unique` are not checked.
Fix all issues by adding a new struct `TestOpts`, which holds all options. Notes:
* The chain type is not an option in the struct for now, because the default values vary.
* The struct holds all possible test options globally. Not all fields may be used by all constructors. Albeit harmless, it is up to the test author to not set a field that is unused.
ACKs for top commit:
kevkevinpal:
utACK [fa690c8](https://github.com/bitcoin/bitcoin/pull/30407/commits/fa690c8e532672f7ab53be6f7a0bb3070858745e)
dergoegge:
utACK fa690c8e532672f7ab53be6f7a0bb3070858745e
TheCharlatan:
Nice, ACK fa690c8e532672f7ab53be6f7a0bb3070858745e
Tree-SHA512: 8db8efa5dff854a73757d3f454f8f902e41bb4358f5f9bae29dbb3e251e20ee93489605de51d0822ba31d97835cd15526a29c075278dd6a8bbde26134feb4f49
|
|
The comment in the code regarding the use of an "&"
on a menu item is misleading. If a wallet name has an "&" in it,
it is not supposed to be interpreted as a hot-key, but it should be
shown as it is without replacing it to an underscore.
|
|
fuzz timeouts
bc34bc288824978ef4b98e8802b47cb863c8a8c2 fuzz: limit the number of nested wrappers in descriptors (Antoine Poinsot)
8d7340105f5299e9a45e84f1704b8b4545cb85f0 fuzz: limit the number of sub-fragments per fragment for descriptors (Antoine Poinsot)
Pull request description:
Some of the logic in the miniscript module is quadratic. It only becomes an issue for very large uninteresting descriptors (like a `thresh` with 130k sub-fragments or a fragment with more than 60k nested `j:` wrappers).
This PR fixes the two types of fuzz timeouts reported by Marco in https://github.com/bitcoin/bitcoin/issues/28812 by trying to pinpoint the problematic descriptors through a simple analysis of the string, without limiting the size of the string itself. This is the same approach as was adopted for limiting the depth of derivation paths.
ACKs for top commit:
dergoegge:
utACK bc34bc288824978ef4b98e8802b47cb863c8a8c2
stickies-v:
Light ACK bc34bc288824978ef4b98e8802b47cb863c8a8c2
marcofleon:
Code review ACK bc34bc288824978ef4b98e8802b47cb863c8a8c2. The added comments are useful, thanks for those. Tested on the three inputs in https://github.com/bitcoin/bitcoin/issues/28812 that caused the timeouts.
Tree-SHA512: 8811c7b225684c5ecc1eb1256cf39dfa60d4518161e70210086c8a01b38927481ebe747af86aa5f4803187672d43fadabcfdfbf4e3b049738d629a25143f0e77
|
|
|
|
|
|
area rather than waste space
b71bfd9eef8b0017cef3c05361c02ddbd837e6ac GUI/OptionsDialog: Prefer to stretch actual options area rather than waste space (Luke Dashjr)
Pull request description:
ACKs for top commit:
hebasto:
ACK b71bfd9eef8b0017cef3c05361c02ddbd837e6ac
Tree-SHA512: b706a07292fe81379e303f9069fca6efd5ceb15ee5bb77c6aeddbf63f736494ce877b76767ff17d7becf98d07209e51c74bdb99365596b7b9f4904a30438d72d
|
|
4383dc90bac1b5def73352fe222f99807d8ca4dd fuzz: fix key size in crypter target (brunoerg)
Pull request description:
Fixes #30251
This PR:
1. Limits `cipher_text_ed` and `random_string` (`SecureString`) size.
2. Replace `ConsumeRandomLengthByteVector` for keys to `ConsumeFixedLengthByteVector` with `WALLET_CRYPTO_KEY_SIZE`.
3. Replace `ConsumeRandomLengthByteVector` for `chSalt` to `ConsumeFixedLengthByteVector` with `WALLET_CRYPTO_SALT_SIZE`.
ACKs for top commit:
marcofleon:
Tested ACK 4383dc90bac1b5def73352fe222f99807d8ca4dd. I ran this:
dergoegge:
utACK 4383dc90bac1b5def73352fe222f99807d8ca4dd
Tree-SHA512: 6f09cca0b4627f49152b685ac03659c01004f2131c6aada7654606ea01f6619b1611b1d17624d2cddce277c1afdddda5f656d99f6ca8f72a22f5c0541762c964
|
|
992b1bbd5da95ee782515fb0f5674bb7a02684b2 qt: keep focus on "Hide" while ModalOverlay is visible (Jadi)
Pull request description:
During the initial sync, the Tab moves the focus to the widgets of the main window, even when the ModalOverlay is visible. This creates some weird rectangular *selections on the screen*.
This PR fixes this by keeping the focus on the "Hide" button while the ModalOverlay is visible.
Fixes #783
ACKs for top commit:
pablomartin4btc:
Concept & approach ACK 992b1bbd5da95ee782515fb0f5674bb7a02684b2
hebasto:
re-ACK 992b1bbd5da95ee782515fb0f5674bb7a02684b2
Tree-SHA512: f702a3fd51db4bc10780bccf76394e35a6b5fb45db72c9c23cd10d777106b08c61077d2d989003838921e76d2cb44f809399f31df76448e4305a6c2a71b5c6a3
|
|
09370529fb9f6d06f6d16bdb1fb336f7a265d8ba fuzz: mini_miner_selection fixups. (glozow)
de273d53004f48e4c8c965f7ce0bd375fd8d0d69 MiniMiner: use FeeFrac in AncestorFeerateComparator (glozow)
Pull request description:
Closes #30284. Closes #30367, see https://github.com/bitcoin/bitcoin/issues/30367#issuecomment-2217459257
Previously, we were only comparing feerates up to 1/1000 precision, since CFeeRate comparison just looks at their respective nSatoshisPerK. This could lead to MiniMiner selecting packages in the wrong order (i.e. by txid) if their feerates were less than 0.001sat/vB different. Fix this by creating + comparing `FeeFrac`s instead.
Also, `FeeFrac::Mul` doesn't have the overflow problem.
Also added a few minor fuzzer fixups that caught my eye while I was debugging this.
ACKs for top commit:
ismaelsadeeq:
Tested ACK 09370529fb9f6d06f6d16bdb1fb336f7a265d8ba
murchandamus:
ACK 09370529fb9f6d06f6d16bdb1fb336f7a265d8ba with nits
dergoegge:
tACK 09370529fb9f6d06f6d16bdb1fb336f7a265d8ba
Tree-SHA512: e5b6d6c3f7289f30cd8280d0a47cd852d0180b83d1b27ff9514f50c97103b0f069484e48cba2ca3a57419beadc1996c1b9dd8d0a0f34bc4f4223d8adaf414ce5
|
|
The script building logic performs a quadratic number of copies in the
number of nested wrappers in the miniscript. Limit the number of nested
wrappers to avoid fuzz timeouts.
Thanks to Marco Falke for reporting the fuzz timeouts and providing a
minimal input to reproduce.
|
|
This target may call into logic quadratic over the number of
sub-fragments. Limit the number of sub-fragments to keep the runtime
reasonable.
Thanks to Marco Falke for reporting the fuzz timeouts with a minimized
input.
|
|
4a028cf54c0502bc9ba0804bf1ae413b20a007cb gui: show maximum mempool size in information window (Sebastian Falbesoner)
bbde6ffefea9587b07a9f8d4043b2dd23ef8c3c5 add node interface method for getting maximum mempool size (Sebastian Falbesoner)
Pull request description:
This PR adds the maximum mempool size to the information window (Menu "Window" -> "Information" -> section "Memory Pool" -> line "Memory usage").
master:
![image](https://github.com/bitcoin-core/gui/assets/91535/157e92f5-7d06-4303-b4ef-bcdfac5527e3)
PR:
![image](https://github.com/bitcoin-core/gui/assets/91535/796322aa-9f16-4b09-9893-bf52a3898a5c)
ACKs for top commit:
MarnixCroes:
tested ACK 4a028cf54c0502bc9ba0804bf1ae413b20a007cb
pablomartin4btc:
tACK 4a028cf54c0502bc9ba0804bf1ae413b20a007cb
luke-jr:
tACK 4a028cf54c0502bc9ba0804bf1ae413b20a007cb & in Knots
hebasto:
ACK 4a028cf54c0502bc9ba0804bf1ae413b20a007cb, tested on Ubuntu 24.04.
Tree-SHA512: c10fb23605d060cea19a86d11822fc4d12496b19547870052aace503670e62e4c4e19ae4c2c4fbf7420a472adb071c9ddebe82447e0cfbce5a6fb9fcd7b9eda3
|
|
AI_ADDRCONFIG prevents ::1 from being considered a valid address on hosts that have a IPV6 loopback IP address but no other IPV6 interfaces.
|
|
3f00aae14092ca076cff7f9ddf6155c601979fcd package rbf: cpfp structure requires package > parent feerate (Greg Sanders)
ad7f1f697f01845470f8df0944a94012fabcba09 test package rbf boundary conditions more closely (Greg Sanders)
ff4558d441f377a372647972d35b5476b94579c9 doc: reword package RBF documentation (Greg Sanders)
de669a883bf65c576cc596b20a576d7bb1618182 doc: replace mention of V3 with TRUC (Greg Sanders)
Pull request description:
Some suggested nits/changes from #28984
ACKs for top commit:
glozow:
ACK 3f00aae1409
murchandamus:
ACK 3f00aae14092ca076cff7f9ddf6155c601979fcd
Tree-SHA512: 79434cc8aba25a43e99793298cdc99cad807db2c3a2e780a31953f244b95eecd97b90559abd67fbf30996c00966675fa257253a7812ec4727420226162c629ae
|
|
|
|
This avoids situations during a reindex in which shutdown
doesn't finish since SyncWithValidationInterfaceQueue is
called by the load block thread when the scheduler is already stopped.
|
|
fa360b047fc578fd855b8f7420d84dc967884f4a util: Use SteadyClock in RandAddSeedPerfmon (MarcoFalke)
Pull request description:
`GetTime` is mockable in tests and system-changeable in production. This should be fine and not lead to issues, but using `SteadyClock` is more correct in this context to do an expensive task only so often.
ACKs for top commit:
sipa:
utACK fa360b047fc578fd855b8f7420d84dc967884f4a
TheCharlatan:
ACK fa360b047fc578fd855b8f7420d84dc967884f4a
Tree-SHA512: 1958b9e9e356c9801ac981014b4b528cfc8ce6612853d8b45f6519b16f0b1839ff765abb8b3368b86f00958ddc6a686f6b90278c57a7ad4858bdf3ea33775cca
|
|
|
|
|
|
26a7f70b5d2b1cbfbf03e0b57b9b5ea92dafbb19 ci: enable self-assignment clang-tidy check (Cory Fields)
32b1d1379258aa57c2a7e278f60d1117fcf17953 refactor: add self-assign checks to classes which violate the clang-tidy check (Cory Fields)
Pull request description:
See comment here: https://github.com/bitcoin/bitcoin/pull/30161#issuecomment-2148229582
Our code failed these checks in three places, which have been fixed up here. Though these appear to have been harmless, adding the check avoids the copy in the self-assignment case so there should be no downside.
~Additionally, minisketch failed the check as well. See https://github.com/sipa/minisketch/pull/87~
Edit: Done
After fixing up the violations, turn on the aggressive clang-tidy check.
Note for reviewers: `git diff -w` makes this trivial to review.
ACKs for top commit:
hebasto:
ACK 26a7f70b5d2b1cbfbf03e0b57b9b5ea92dafbb19, I have reviewed the code and it looks OK.
TheCharlatan:
ACK 26a7f70b5d2b1cbfbf03e0b57b9b5ea92dafbb19
Tree-SHA512: 74d8236a1b5a698f2f61c4740c4fc77788b7f882c4b395acc4e6bfef1ec8a4554ea8821a26b14d70cfa6c8e2e9ea305deeea3fbf323967fa19343c007a53c5ba
|
|
3333bae9b2a6c1ee2314d33361c93944c12001f9 tidy: modernize-use-equals-default (MarcoFalke)
Pull request description:
Prior to C++20, `modernize-use-equals-default` could have been problematic because it could turn a non-aggregate into an aggregate. The risk would be that aggregate initialization would be enabled where the author did not intend to enable it.
With C++20, aggregate for those is forbidden either way. (https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2018/p1008r1.pdf)
So enabled it for code clarity, consistency, and possibly unlocking compiler optimizations. See https://clang.llvm.org/extra/clang-tidy/checks/modernize/use-equals-default.html
ACKs for top commit:
stickies-v:
ACK 3333bae9b2a6c1ee2314d33361c93944c12001f9
Tree-SHA512: ab42ff01be7ca7e7d8b4c6a485e68426f59627d83dd827cf292304829562348dc17a52ee009f5f6f3c1c2081d7166ffac4baef23197ebeba8de7767c6ddfe255
|
|
compile time
fa601ab9f7142cdb18c18c1128fc893cdffb3463 util: Catch translation string errors at compile time (MarcoFalke)
Pull request description:
The translation helper function `_()` has many problems. For example, the following compiles:
```cpp
auto ptr{"wrong"};
_(ptr);
_(nullptr);
_(0);
_(NULL);
```
However, it is wrong, because none of the arguments passed to the function can be picked up by the translation tooling for transifex.
Fix all issues by enforcing only real string literals can be passed to the function.
ACKs for top commit:
ryanofsky:
Code review ACK fa601ab9f7142cdb18c18c1128fc893cdffb3463
hebasto:
ACK fa601ab9f7142cdb18c18c1128fc893cdffb3463.
Tree-SHA512: 33aed02d7e8fc9bfb8f90746f5c8072a8c0910fa900ec3516af2e732780b0fee8b07b6596c0fc210b018c0869111d6c34bf8d083de0e88ecdb4dee88e809186d
|
|
test/util/net.cpp
e233ec036dc972a5847ab769ad22d418fd9404d1 refactor: Use designated initializer (Hodlinator)
Pull request description:
Block was recently touched (e2d1f84858485650ff743753ffa5c679f210a992) and the codebase recently switched to C++20 which allows this to improve robustness.
Follow-up suggested in https://github.com/bitcoin/bitcoin/pull/29625#discussion_r1664818014
ACKs for top commit:
maflcko:
ACK e233ec036dc972a5847ab769ad22d418fd9404d1
Tree-SHA512: ce3a18f513421e923710a43c8f97db1badb7ff5c6bdbfd62d9543312d2225731db5c14bef16feb47c43b84fad4dc24485086634b680feba422d2b7b363e13fa6
|
|
If the snapshot is not an ancestor of the most-work header (m_best_header),
syncing from that alternative chain should be prioritised.
Therefore don't accept loading a snapshot in this situation.
If that other chain turns out to be invalid, m_best_header
would be reset and loading the snapshot should be possible again.
Because of the work required to generate a conflicting headers chain,
this should only be possible under extreme circumstances, such as major forks.
|
|
e8c3b7172c33929e4e5bf6059da2d25a4ea8779c remove truc_policy.cpp from libbitcoin_common_a_SOURCES (glozow)
Pull request description:
Hebasto pointed out that it doesn't need to be there since it's in `libbitcoin_node_a_SOURCES`
ACKs for top commit:
maflcko:
ACK e8c3b7172c33929e4e5bf6059da2d25a4ea8779c
hebasto:
ACK e8c3b7172c33929e4e5bf6059da2d25a4ea8779c, this change follows the design [docs](https://github.com/bitcoin/bitcoin/blob/master/doc/design/libraries.md).
ismaelsadeeq:
ACK e8c3b7172c33929e4e5bf6059da2d25a4ea8779c
Tree-SHA512: ebe6b0dda2d097d88c37d2b071ac99da3e9c519ec473d4b8f870a50f1b24d00e2e5deef317fb0f6a91c96103e7f37468cb8f13395818eab55a42af48df4e0fc6
|
|
wallets without requiring BDB
8ce3739edbcf6437bf2695087e0ebe8c633df19b test: verify wallet is still active post-migration failure (furszy)
771bc60f134c002a027e799639f0fed59ae8123d wallet: Use LegacyDataSPKM when loading (Ava Chow)
61d872f1b3d0dbfd0de4ff9b7deff40eeffa6a14 wallet: Move MigrateToDescriptor and DeleteRecords to LegacyDataSPKM (Ava Chow)
b231f4d556876ae70305e8710e31d53525ded8ae wallet: Move LegacyScriptPubKeyMan::IsMine to LegacyDataSPKM (Ava Chow)
7461d0c006c92ede2f2595b79a5509eaf3509fb7 wallet: Move LegacySPKM data storage and handling to LegacyDataSPKM (Ava Chow)
517e204bacd9dcea6612aae57d5a2813b89cd01d Change MigrateLegacyToDescriptor to reopen wallet as BERKELEY_RO (Ava Chow)
Pull request description:
#26606 introduced `BerkeleyRODatabase` which is an independent parser for BDB files. This PR uses this in legacy wallet migration so that migration will continue to work once the legacy wallet and BDB are removed. `LegacyDataSPKM` is introduced to have the minimum data and functions necessary for a legacy wallet to be loaded for migration.
ACKs for top commit:
cbergqvist:
ACK 8ce3739edbcf6437bf2695087e0ebe8c633df19b
theStack:
Code-review ACK 8ce3739edbcf6437bf2695087e0ebe8c633df19b
furszy:
Code review ACK 8ce3739edbcf6437bf2695087e0ebe8c633df19b
Tree-SHA512: dccea12d6c597de15e3e42f97ab483cfd069e103611200279a177e021e8e9c4e74387c4f45d2e58b3a1e7e2bdb32a1d2d2060b1f8086c03eeaa0c68579d9d54e
|
|
These errors should never happen. However, when they do happen, it is
useful to log the correct error location (function name).
For example, this fixes an incorrect "ConnectBlock()" in
"WriteUndoDataForBlock".
|
|
These errors should never happen in normal operation. If they do,
knowing the FlatFilePos may be useful to determine if data corruption
happened. Also, handle the error pos.IsNull() as part of OpenUndoFile,
because it may as well have happened due to data corruption.
This mirrors the LogError behavior from ReadBlockFromDisk.
|
|
Member fields are used read-only in this method.
|