💬 polespinasa commented on pull request "RPC: add sendrawtransactiontopeer":
(https://github.com/bitcoin/bitcoin/pull/33507#discussion_r2395336697)
how so? Care to explain? :)
(https://github.com/bitcoin/bitcoin/pull/33507#discussion_r2395336697)
how so? Care to explain? :)
💬 ryanofsky commented on pull request "init: Signal m_tip_block_cv on Ctrl-C":
(https://github.com/bitcoin/bitcoin/pull/33511#issuecomment-3357409495)
<!-- begin push-2 -->
Updated 4ee8e2248e54a33f0a3a2e808d46a7b0b8ea7bc3 -> 68cad90dace40f7a015ca4ff81b878fc8fdc1dd5 ([`pr/sigwait.1`](https://github.com/ryanofsky/bitcoin/commits/pr/sigwait.1) -> [`pr/sigwait.2`](https://github.com/ryanofsky/bitcoin/commits/pr/sigwait.2), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/sigwait.1..pr/sigwait.2))<!-- end --> to fix CI failures, also added more documentation and explanation in commit description
(https://github.com/bitcoin/bitcoin/pull/33511#issuecomment-3357409495)
<!-- begin push-2 -->
Updated 4ee8e2248e54a33f0a3a2e808d46a7b0b8ea7bc3 -> 68cad90dace40f7a015ca4ff81b878fc8fdc1dd5 ([`pr/sigwait.1`](https://github.com/ryanofsky/bitcoin/commits/pr/sigwait.1) -> [`pr/sigwait.2`](https://github.com/ryanofsky/bitcoin/commits/pr/sigwait.2), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/sigwait.1..pr/sigwait.2))<!-- end --> to fix CI failures, also added more documentation and explanation in commit description
🚀 achow101 merged a pull request: "[28.x] backports + 28.3rc1"
(https://github.com/bitcoin/bitcoin/pull/33476)
(https://github.com/bitcoin/bitcoin/pull/33476)
👍 hodlinator approved a pull request: "net: improve the interface around FindNode() and avoid a recursive mutex lock"
(https://github.com/bitcoin/bitcoin/pull/32326#pullrequestreview-3290340107)
re-ACK 87e7f37918d42c28033e9f684db52f94eeed617b
Changes since previous ACK:
* Avoided function overloading and documented that hostnames can be used in more places, not just raw IP addresses and the like.
* `--auto`
(https://github.com/bitcoin/bitcoin/pull/32326#pullrequestreview-3290340107)
re-ACK 87e7f37918d42c28033e9f684db52f94eeed617b
Changes since previous ACK:
* Avoided function overloading and documented that hostnames can be used in more places, not just raw IP addresses and the like.
* `--auto`
👍 theStack approved a pull request: "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys"
(https://github.com/bitcoin/bitcoin/pull/29675#pullrequestreview-3290195368)
Code-review ACK ac599c4a9cb3b2d424932d3fd91f9eed17426827 :old_key:
Happy to review potential follow-ups.
(https://github.com/bitcoin/bitcoin/pull/29675#pullrequestreview-3290195368)
Code-review ACK ac599c4a9cb3b2d424932d3fd91f9eed17426827 :old_key:
Happy to review potential follow-ups.
💬 theStack commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2395372134)
nit: seems that this shouldn't be in upper-case, if it's not really a constant
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2395372134)
nit: seems that this shouldn't be in upper-case, if it's not really a constant
💬 theStack commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2395283294)
nit: could move the MUSIG_CHAINCODE constant from the header to musig.cpp, as its only used there now
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2395283294)
nit: could move the MUSIG_CHAINCODE constant from the header to musig.cpp, as its only used there now
📝 sipa opened a pull request: "Improve LastCommonAncestor performance + add tests"
(https://github.com/bitcoin/bitcoin/pull/33515)
In theory, the `LastCommonAncestor` function in chain.cpp can take $\mathcal{O}(n)$ time, walking over the entire chain, if the forking point is very early, which could take ~milliseconds. I expect this to be very rare in normal occurrences, but it seems nontrivial to reason about worst cases as it's accessible from several places in net_processing.
This PR modifies the algorithm to make use of the `CBlockIndex::pskip` skip pointers to find the forking point in sublinear time (a simulation sh
...
(https://github.com/bitcoin/bitcoin/pull/33515)
In theory, the `LastCommonAncestor` function in chain.cpp can take $\mathcal{O}(n)$ time, walking over the entire chain, if the forking point is very early, which could take ~milliseconds. I expect this to be very rare in normal occurrences, but it seems nontrivial to reason about worst cases as it's accessible from several places in net_processing.
This PR modifies the algorithm to make use of the `CBlockIndex::pskip` skip pointers to find the forking point in sublinear time (a simulation sh
...
💬 ryanofsky commented on issue "How to backport libmultiprocess changes?":
(https://github.com/bitcoin/bitcoin/issues/33439#issuecomment-3357606911)
> when the subtree bump is created on master, it could make sense to base it before the branch-off point, so that the exact same commit ID can be merged into both branches
This is a nice idea. Right now it looks like https://github.com/bitcoin/bitcoin/pull/33322 was the last update merged before the [30.x](https://github.com/bitcoin/bitcoin/commits/30.x) branch point and https://github.com/bitcoin/bitcoin/pull/33412 was more recently merged after, so it may be too late to use that strategy in t
...
(https://github.com/bitcoin/bitcoin/issues/33439#issuecomment-3357606911)
> when the subtree bump is created on master, it could make sense to base it before the branch-off point, so that the exact same commit ID can be merged into both branches
This is a nice idea. Right now it looks like https://github.com/bitcoin/bitcoin/pull/33322 was the last update merged before the [30.x](https://github.com/bitcoin/bitcoin/commits/30.x) branch point and https://github.com/bitcoin/bitcoin/pull/33412 was more recently merged after, so it may be too late to use that strategy in t
...
💬 davidgumberg commented on pull request "depends: static libxcb-cursor":
(https://github.com/bitcoin/bitcoin/pull/33434#issuecomment-3357685190)
Addendum ACK https://github.com/bitcoin/bitcoin/commit/eca50854e1cb04e20478bd3df4762e18520a3611
After uninstalling and reinstalling guix on the machine that encountered the build issue earlier, this branch built fine without any errors.
```console
$ ./contrib/guix/guix-build
$ uname -m && find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
```
```
x86_64
cf7f86d91288477c11afdc767969d6b19b0ba1e62c73924e72b22a252737465
...
(https://github.com/bitcoin/bitcoin/pull/33434#issuecomment-3357685190)
Addendum ACK https://github.com/bitcoin/bitcoin/commit/eca50854e1cb04e20478bd3df4762e18520a3611
After uninstalling and reinstalling guix on the machine that encountered the build issue earlier, this branch built fine without any errors.
```console
$ ./contrib/guix/guix-build
$ uname -m && find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
```
```
x86_64
cf7f86d91288477c11afdc767969d6b19b0ba1e62c73924e72b22a252737465
...
💬 davidgumberg commented on pull request "Split `CWallet::Create()` into `CreateNew` and `LoadExisting`":
(https://github.com/bitcoin/bitcoin/pull/32636#discussion_r2395570236)
This is out of scope, but I can address this in a follow-up.
(https://github.com/bitcoin/bitcoin/pull/32636#discussion_r2395570236)
This is out of scope, but I can address this in a follow-up.
💬 davidgumberg commented on pull request "Split `CWallet::Create()` into `CreateNew` and `LoadExisting`":
(https://github.com/bitcoin/bitcoin/pull/32636#discussion_r2395573945)
This PR does not change the error handling here, but I can address this in a follow-up.
(https://github.com/bitcoin/bitcoin/pull/32636#discussion_r2395573945)
This PR does not change the error handling here, but I can address this in a follow-up.
💬 davidgumberg commented on pull request "Split `CWallet::Create()` into `CreateNew` and `LoadExisting`":
(https://github.com/bitcoin/bitcoin/pull/32636#issuecomment-3357753607)
Rebased to address a small merge conflict, since some instances of `LoadWallet()` (renamed PopulateWalletFromDB in this branch) were removed in #33082.
(https://github.com/bitcoin/bitcoin/pull/32636#issuecomment-3357753607)
Rebased to address a small merge conflict, since some instances of `LoadWallet()` (renamed PopulateWalletFromDB in this branch) were removed in #33082.
🤔 furszy reviewed a pull request: "Improve LastCommonAncestor performance + add tests"
(https://github.com/bitcoin/bitcoin/pull/33515#pullrequestreview-3290718016)
Code reviewed ACK [4fe3de2](https://github.com/bitcoin/bitcoin/pull/33515/commits/4fe3de2aa7fda58d5c56987aee13ae87191da1c6)
(https://github.com/bitcoin/bitcoin/pull/33515#pullrequestreview-3290718016)
Code reviewed ACK [4fe3de2](https://github.com/bitcoin/bitcoin/pull/33515/commits/4fe3de2aa7fda58d5c56987aee13ae87191da1c6)
💬 l0rinc commented on pull request "Use number of dirty cache entries in flush warnings/logs":
(https://github.com/bitcoin/bitcoin/pull/31703#discussion_r2395761545)
It seems https://github.com/bitcoin/bitcoin/pull/32602/files#diff-1ef3b6a1936b50f3d5ec4a1786d9e2d63d1a3e1815b103e67f20601995f355b4R312-R321 revealed that there's a silent conflict here, we need the [first commit](https://github.com/bitcoin/bitcoin/pull/32313/commits/85979e2015344f05aa60db52b4a76eeadef216ad) from https://github.com/bitcoin/bitcoin/pull/32313 for this to pass after rebase
(https://github.com/bitcoin/bitcoin/pull/31703#discussion_r2395761545)
It seems https://github.com/bitcoin/bitcoin/pull/32602/files#diff-1ef3b6a1936b50f3d5ec4a1786d9e2d63d1a3e1815b103e67f20601995f355b4R312-R321 revealed that there's a silent conflict here, we need the [first commit](https://github.com/bitcoin/bitcoin/pull/32313/commits/85979e2015344f05aa60db52b4a76eeadef216ad) from https://github.com/bitcoin/bitcoin/pull/32313 for this to pass after rebase
💬 l0rinc commented on pull request "coins: fix `cachedCoinsUsage` accounting in `CCoinsViewCache`":
(https://github.com/bitcoin/bitcoin/pull/32313#issuecomment-3357991920)
Rebased and adjusted the commits to minimize conflict with https://github.com/bitcoin/bitcoin/pull/33512
(https://github.com/bitcoin/bitcoin/pull/32313#issuecomment-3357991920)
Rebased and adjusted the commits to minimize conflict with https://github.com/bitcoin/bitcoin/pull/33512
👍 TheCharlatan approved a pull request: "init: Fix Ctrl-C shutdown hangs during wait calls"
(https://github.com/bitcoin/bitcoin/pull/33511#pullrequestreview-3291091712)
ACK ef7be125ea92e182f087d2c8e3d5c605b8df31d9
This should be backported in my opinion.
(https://github.com/bitcoin/bitcoin/pull/33511#pullrequestreview-3291091712)
ACK ef7be125ea92e182f087d2c8e3d5c605b8df31d9
This should be backported in my opinion.
💬 stringintech commented on pull request "p2p: Advance pindexLastCommonBlock early after connecting blocks":
(https://github.com/bitcoin/bitcoin/pull/32180#discussion_r2395906680)
This makes sense and if we end up doing it, it seems it also allows removing some logic from `FindNextBlocksToDownload()` and have this:
```diff
diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index 9ab6fb53ed..423834118c 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -1397,24 +1397,15 @@ void PeerManagerImpl::FindNextBlocksToDownload(const Peer& peer, unsigned int co
return;
}
- // Bootstrap quickly by guessing a parent of our bes
...
(https://github.com/bitcoin/bitcoin/pull/32180#discussion_r2395906680)
This makes sense and if we end up doing it, it seems it also allows removing some logic from `FindNextBlocksToDownload()` and have this:
```diff
diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index 9ab6fb53ed..423834118c 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -1397,24 +1397,15 @@ void PeerManagerImpl::FindNextBlocksToDownload(const Peer& peer, unsigned int co
return;
}
- // Bootstrap quickly by guessing a parent of our bes
...
💬 achow101 commented on pull request "net: improve the interface around FindNode() and avoid a recursive mutex lock":
(https://github.com/bitcoin/bitcoin/pull/32326#issuecomment-3358144836)
ACK 87e7f37918d42c28033e9f684db52f94eeed617b
(https://github.com/bitcoin/bitcoin/pull/32326#issuecomment-3358144836)
ACK 87e7f37918d42c28033e9f684db52f94eeed617b
🚀 achow101 merged a pull request: "net: improve the interface around FindNode() and avoid a recursive mutex lock"
(https://github.com/bitcoin/bitcoin/pull/32326)
(https://github.com/bitcoin/bitcoin/pull/32326)