💬 vasild commented on pull request "Avoid returning references to mutex guarded members":
(https://github.com/bitcoin/bitcoin/pull/28774#issuecomment-1898900596)
`7a8e5310e7...32a9f13cb8`: rebase due to conflicts and change the approach to use a callback, see https://github.com/bitcoin/bitcoin/pull/28774#discussion_r1456489383.

💬 pablomartin4btc commented on pull request "assumeutxo, rpc: Improve EOF error when reading snapshot metadata in loadtxoutset":
(https://github.com/bitcoin/bitcoin/pull/28670#discussion_r1457765120)
done, thanks!

💬 vasild commented on pull request "Avoid returning references to mutex guarded members":
(https://github.com/bitcoin/bitcoin/pull/28774#discussion_r1457766636)
Right, those methods were not very natural for the `WalletStorage` interface which contained a method to retrieve the encryption key. Adding a new interface seems a bit too complicated for this. Instead, I changed it to give the key to a callback function instead of returning the key to the caller.

💬 pablomartin4btc commented on pull request "assumeutxo, rpc: Improve EOF error when reading snapshot metadata in loadtxoutset":
(https://github.com/bitcoin/bitcoin/pull/28670#issuecomment-1898907043)
Addressed @hebasto's [suggestion](https://github.com/bitcoin/bitcoin/pull/28670#discussion_r1457712659) in order to fix the MSVC CI job.

💬 Sjors commented on pull request "Support self-hosted Cirrus workers on forks (and multi-user)":
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1457771731)
@hebasto @ryanofsky any thoughts on how to make the `test-each-commit` task work for a scenario like in https://github.com/Sjors/bitcoin/pull/30 where the base branch is `fork/some-branch` and the PR branch is `fork/some-pr`?

💬 Sjors commented on pull request "Support self-hosted Cirrus workers on forks (and multi-user)":
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1457773126)
Example of earlier failure: https://github.com/Sjors/bitcoin/actions/runs/7556559882/job/20573828392?pr=28#step:4:42

```
shell: /usr/bin/bash -e {0}
env:
DANGER_RUN_CI_ON_HOST: 1
CI_FAILFAST_TEST_LEAVE_DANGLING: 1
MAKEJOBS: -j10
MAX_COUNT: 6
FETCH_DEPTH: 8
Warning: you are leaving 1 commit behind, not connected to
any of your branches:

3b5c5d4 ci: add self-hosted runners for GitHub

If you want to keep it by creating a new branch, this may be a good time
...

💬 achow101 commented on pull request "test: Remove all-lint.py script":
(https://github.com/bitcoin/bitcoin/pull/29228#issuecomment-1898958526)
ACK fa2b95cf3f5148d27a8fd4fb3763ca1fc139bdd9

💬 ariard commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#issuecomment-1898959237)
> It's easy for Alice to make the revoked commitment confirm, she can use any output of that transaction to CPFP and she doesn't need to add any of her wallet inputs, she can just use the channel funds for that.

No you can’t spend in-mempool htlc outputs 1 CSV post-anchor. Assuming no 1 CSV, yes in my scenario. However attacker can construct revoked or valid pinning state to have the 483 outputs as inbound HTLC to inflate. Alice no preimage to spend them. Only anchor output available and liqu
...

💬 maflcko commented on pull request "Support self-hosted Cirrus workers on forks (and multi-user)":
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1457806226)
why is USER needed?

💬 maflcko commented on pull request "Support self-hosted Cirrus workers on forks (and multi-user)":
(https://github.com/bitcoin/bitcoin/pull/29274#issuecomment-1898964160)
lgtm ACK c65fde483133a04964cc8757c96005b78d9e8ca8

Didn't review the other commits.

🚀 achow101 merged a pull request: "test: Remove all-lint.py script"
(https://github.com/bitcoin/bitcoin/pull/29228)

💬 achow101 commented on pull request "rpc: Fix race in loadtxoutset":
(https://github.com/bitcoin/bitcoin/pull/29262#issuecomment-1898966431)
Should this be backported?

💬 achow101 commented on pull request "rpc: Fix race in loadtxoutset":
(https://github.com/bitcoin/bitcoin/pull/29262#issuecomment-1898969740)
ACK 5555d8db3313f893609eb0cf549bb597361d4466

💬 maflcko commented on pull request "rpc: Fix race in loadtxoutset":
(https://github.com/bitcoin/bitcoin/pull/29262#issuecomment-1898971746)
I don't think assumeutxo params were specified for the main chain, and a wrong RPC result output in some test cases seems harmless.

🚀 achow101 merged a pull request: "rpc: Fix race in loadtxoutset"
(https://github.com/bitcoin/bitcoin/pull/29262)

💬 DocBrown101 commented on issue "Unable to sync blockchain on laptop: ERROR: ReadBlockFromDisk: Deserialize or I/O error":
(https://github.com/bitcoin/bitcoin/issues/29255#issuecomment-1898994959)
I have the same problem on a raspberry pi 64 ...

💬 wizkid057 commented on issue "Witness scripts being abused to bypass datacarriersize limit (CVE-2023-50428)":
(https://github.com/bitcoin/bitcoin/issues/29187#issuecomment-1898995069)
> The previous thread was closed by Bitcoin Core, due to obvious lack of consensus, yet for some reason this new thread that duplicates the agenda of the previous thread was created. I am confused? People who voiced objection in the previous thread appear largely to be ignoring this thread, since their points were already made.

Every objection made both here and in the original PR was quite thoroughly and clearly debunked as not having any bearing whatsoever on the issue at hand. The post ab
...

💬 jamesob commented on pull request "Update libsecp256k1 subtree to current master":
(https://github.com/bitcoin/bitcoin/pull/29169#issuecomment-1898996304)
Quick update:

I did confirm (on another machine) that more contemporary versions of gcc (`12.2.0` in this case) do show a speed _up_ of about 4%, with slightly more time spent in kernel:

### #29169 vs. f8ca1357 (relative)
| bench name | x | #29169 | f8ca1357 |
| ---------------------------------------------------------- | --: | -----------------: | -----------------: |
| ibd.local.range.dbcache=4000.667200.697200.to
...

💬 maflcko commented on issue "test: Write assumeutxo tests":
(https://github.com/bitcoin/bitcoin/issues/28648#issuecomment-1899010205)
There'd be https://github.com/bitcoin/bitcoin/pull/27596#discussion_r1344713537 if someone wants to tackle it.

👍 ryanofsky approved a pull request: "kernel: Remove dependency on CScheduler"
(https://github.com/bitcoin/bitcoin/pull/28960#pullrequestreview-1830011358)
Code review ACK e3592ee55756c08bda3075b54cbff719d3fb964f. Nice improvement eliminating g_signals and letting libbitcoinkernel application code control where signals are sent and how they are processed. (I think this could be mentioned in the commit description. I think it is a bigger accomplishment than removing a small dependency on CScheduler code).

This PR is definitely an improvement over status the quo, but I did make a number of suggestions below to improve internal consistency and nami
...