💬 ryanofsky commented on pull request "Set notifications m_tip_block in LoadChainTip()":
(https://github.com/bitcoin/bitcoin/pull/31346#discussion_r1853901541)
Just ignoring the return value is probably the best thing for now since this function and its callers return bool. After #29700 it could more easily bubble up the interrupted status. The kernel API is designed for a better world where operations can be more easily interrupted but not everything is interruptible not now.
(https://github.com/bitcoin/bitcoin/pull/31346#discussion_r1853901541)
Just ignoring the return value is probably the best thing for now since this function and its callers return bool. After #29700 it could more easily bubble up the interrupted status. The kernel API is designed for a better world where operations can be more easily interrupted but not everything is interruptible not now.
👍 hebasto approved a pull request: "guix: swap `moreutils` for just `sponge`"
(https://github.com/bitcoin/bitcoin/pull/31323#pullrequestreview-2454434748)
ACK e8f50c5debe2b866e59872dbbf22db0fd592ec04.
My Guix build:
```
aarch64
c3d79b98874ba44dbe6550fd18cdd78443ee1b4d0594dc1cc2e6b423dcded117 guix-build-e8f50c5debe2/output/aarch64-linux-gnu/SHA256SUMS.part
159a09c6ed25d498a48f3e3d53dd4c77b61c2ecbd824661593316030678ee5b0 guix-build-e8f50c5debe2/output/aarch64-linux-gnu/bitcoin-e8f50c5debe2-aarch64-linux-gnu-debug.tar.gz
34cd42b0a3574085c173073bbb12cbb19db2169efadab752335071eb897c242e guix-build-e8f50c5debe2/output/aarch64-linux-gnu/bitcoi
...
(https://github.com/bitcoin/bitcoin/pull/31323#pullrequestreview-2454434748)
ACK e8f50c5debe2b866e59872dbbf22db0fd592ec04.
My Guix build:
```
aarch64
c3d79b98874ba44dbe6550fd18cdd78443ee1b4d0594dc1cc2e6b423dcded117 guix-build-e8f50c5debe2/output/aarch64-linux-gnu/SHA256SUMS.part
159a09c6ed25d498a48f3e3d53dd4c77b61c2ecbd824661593316030678ee5b0 guix-build-e8f50c5debe2/output/aarch64-linux-gnu/bitcoin-e8f50c5debe2-aarch64-linux-gnu-debug.tar.gz
34cd42b0a3574085c173073bbb12cbb19db2169efadab752335071eb897c242e guix-build-e8f50c5debe2/output/aarch64-linux-gnu/bitcoi
...
👍 TheCharlatan approved a pull request: "build: increase minimum supported Windows to 10.0"
(https://github.com/bitcoin/bitcoin/pull/31172#pullrequestreview-2454442606)
ACK ee1128ead846698db5e5633f193883837f2fbc64
(https://github.com/bitcoin/bitcoin/pull/31172#pullrequestreview-2454442606)
ACK ee1128ead846698db5e5633f193883837f2fbc64
👍 TheCharlatan approved a pull request: "guix: swap `moreutils` for just `sponge`"
(https://github.com/bitcoin/bitcoin/pull/31323#pullrequestreview-2454451753)
Re-ACK e8f50c5debe2
Guix build (aarch64):
```
c3d79b98874ba44dbe6550fd18cdd78443ee1b4d0594dc1cc2e6b423dcded117 guix-build-e8f50c5debe2/output/aarch64-linux-gnu/SHA256SUMS.part
159a09c6ed25d498a48f3e3d53dd4c77b61c2ecbd824661593316030678ee5b0 guix-build-e8f50c5debe2/output/aarch64-linux-gnu/bitcoin-e8f50c5debe2-aarch64-linux-gnu-debug.tar.gz
34cd42b0a3574085c173073bbb12cbb19db2169efadab752335071eb897c242e guix-build-e8f50c5debe2/output/aarch64-linux-gnu/bitcoin-e8f50c5debe2-aarch64-linux
...
(https://github.com/bitcoin/bitcoin/pull/31323#pullrequestreview-2454451753)
Re-ACK e8f50c5debe2
Guix build (aarch64):
```
c3d79b98874ba44dbe6550fd18cdd78443ee1b4d0594dc1cc2e6b423dcded117 guix-build-e8f50c5debe2/output/aarch64-linux-gnu/SHA256SUMS.part
159a09c6ed25d498a48f3e3d53dd4c77b61c2ecbd824661593316030678ee5b0 guix-build-e8f50c5debe2/output/aarch64-linux-gnu/bitcoin-e8f50c5debe2-aarch64-linux-gnu-debug.tar.gz
34cd42b0a3574085c173073bbb12cbb19db2169efadab752335071eb897c242e guix-build-e8f50c5debe2/output/aarch64-linux-gnu/bitcoin-e8f50c5debe2-aarch64-linux
...
💬 l0rinc commented on pull request "util: Improve documentation and negation of args":
(https://github.com/bitcoin/bitcoin/pull/31212#discussion_r1853917762)
That's one way to do it, but in that case it's possible that the test passes before the fix as well (had many such reviews), which would indicate that we haven't actually reproduced the problem, therefore the fix might not do what it claims.
Only by having the exact same test fail before the fix and pass after (without any change to the test) can we be sure the fix had an effect on the test.
(https://github.com/bitcoin/bitcoin/pull/31212#discussion_r1853917762)
That's one way to do it, but in that case it's possible that the test passes before the fix as well (had many such reviews), which would indicate that we haven't actually reproduced the problem, therefore the fix might not do what it claims.
Only by having the exact same test fail before the fix and pass after (without any change to the test) can we be sure the fix had an effect on the test.
💬 maflcko commented on pull request "doc: Fix dead links to mailing list archives":
(https://github.com/bitcoin/bitcoin/pull/31240#issuecomment-2493785136)
Looks like the links aren't dead, but redirects
(https://github.com/bitcoin/bitcoin/pull/31240#issuecomment-2493785136)
Looks like the links aren't dead, but redirects
💬 vasild commented on pull request "fuzz: set the output argument of FuzzedSock::Accept()":
(https://github.com/bitcoin/bitcoin/pull/31316#issuecomment-2493789741)
Such a test is in https://github.com/bitcoin/bitcoin/pull/28584 which adds a call to `CConnman::SocketHandler()` and `CConnman::InitBinds()` during fuzzing. `CConnman::InitBinds()` will add some listening sockets and from there:
`CConnman::SocketHandler()` calls
`CConnman::SocketHandlerListening()` calls
`CConnman::AcceptConnection()` calls
`FuzzedSock::Accept()`.
(https://github.com/bitcoin/bitcoin/pull/31316#issuecomment-2493789741)
Such a test is in https://github.com/bitcoin/bitcoin/pull/28584 which adds a call to `CConnman::SocketHandler()` and `CConnman::InitBinds()` during fuzzing. `CConnman::InitBinds()` will add some listening sockets and from there:
`CConnman::SocketHandler()` calls
`CConnman::SocketHandlerListening()` calls
`CConnman::AcceptConnection()` calls
`FuzzedSock::Accept()`.
💬 TheCharlatan commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-2493809430)
Updated fc67047b7e1fb7031285f790ea3a7ea349474f31 -> 34a8429ff3a870c0caaf4c4790becd86c5acde38 ([kernelApi_4](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_4) -> [kernelApi_5](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_5), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelApi_4..kernelApi_5))
* More consistent `const` usage
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-2493809430)
Updated fc67047b7e1fb7031285f790ea3a7ea349474f31 -> 34a8429ff3a870c0caaf4c4790becd86c5acde38 ([kernelApi_4](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_4) -> [kernelApi_5](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_5), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelApi_4..kernelApi_5))
* More consistent `const` usage
💬 Sjors commented on pull request "Add waitNext() to BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/31283#issuecomment-2493816544)
Fixed a bug found while testing https://github.com/Sjors/bitcoin/pull/49, where it was spuriously making new block templates every tick even if fees didn't rise. [diff](
https://github.com/bitcoin/bitcoin/compare/245660284de8b4ac29428737f088c22cb2812037..5d294d86f43c83603b5d965eff36c90159c6bffe)
(https://github.com/bitcoin/bitcoin/pull/31283#issuecomment-2493816544)
Fixed a bug found while testing https://github.com/Sjors/bitcoin/pull/49, where it was spuriously making new block templates every tick even if fees didn't rise. [diff](
https://github.com/bitcoin/bitcoin/compare/245660284de8b4ac29428737f088c22cb2812037..5d294d86f43c83603b5d965eff36c90159c6bffe)
💬 jonatack commented on pull request "doc: Fix dead links to mailing list archives":
(https://github.com/bitcoin/bitcoin/pull/31240#issuecomment-2493822803)
For more context: https://github.com/bitcoin/bips/pull/1698#issuecomment-2486422338
(https://github.com/bitcoin/bitcoin/pull/31240#issuecomment-2493822803)
For more context: https://github.com/bitcoin/bips/pull/1698#issuecomment-2486422338
📝 vasild opened a pull request: "ci: detect outbound internet traffic generated while running tests"
(https://github.com/bitcoin/bitcoin/pull/31349)
Make sure that CI fails if some of the tests generate an outbound traffic on the non-loopback interface.
Resolves https://github.com/bitcoin/bitcoin/issues/31339
(https://github.com/bitcoin/bitcoin/pull/31349)
Make sure that CI fails if some of the tests generate an outbound traffic on the non-loopback interface.
Resolves https://github.com/bitcoin/bitcoin/issues/31339
💬 Sjors commented on issue "ci: how to run native arm job on Apple silicon?":
(https://github.com/bitcoin/bitcoin/issues/31344#issuecomment-2493842596)
> It was also intentionally selected to support 32-bit mode.
That's useful to know.
(https://github.com/bitcoin/bitcoin/issues/31344#issuecomment-2493842596)
> It was also intentionally selected to support 32-bit mode.
That's useful to know.
💬 jamesob commented on pull request "rpc: add getdescriptoractivity":
(https://github.com/bitcoin/bitcoin/pull/30708#discussion_r1853928443)
Done.
(https://github.com/bitcoin/bitcoin/pull/30708#discussion_r1853928443)
Done.
💬 jamesob commented on pull request "rpc: add getdescriptoractivity":
(https://github.com/bitcoin/bitcoin/pull/30708#discussion_r1853938164)
Added.
(https://github.com/bitcoin/bitcoin/pull/30708#discussion_r1853938164)
Added.
🤔 jamesob reviewed a pull request: "rpc: add getdescriptoractivity"
(https://github.com/bitcoin/bitcoin/pull/30708#pullrequestreview-2454451702)
Feedback addressed. Thanks for all review.
(https://github.com/bitcoin/bitcoin/pull/30708#pullrequestreview-2454451702)
Feedback addressed. Thanks for all review.
💬 jamesob commented on pull request "rpc: add getdescriptoractivity":
(https://github.com/bitcoin/bitcoin/pull/30708#discussion_r1853917742)
Done.
(https://github.com/bitcoin/bitcoin/pull/30708#discussion_r1853917742)
Done.
💬 jamesob commented on pull request "rpc: add getdescriptoractivity":
(https://github.com/bitcoin/bitcoin/pull/30708#discussion_r1853918927)
Good point, removed. I forgot that I had a main-chain check in there. I debated about whether this call would be useful for checking balances on orphaned tips, but probably better to just restrict usage to the main chain to avoid confusion.
(https://github.com/bitcoin/bitcoin/pull/30708#discussion_r1853918927)
Good point, removed. I forgot that I had a main-chain check in there. I debated about whether this call would be useful for checking balances on orphaned tips, but probably better to just restrict usage to the main chain to avoid confusion.
💬 jamesob commented on pull request "rpc: add getdescriptoractivity":
(https://github.com/bitcoin/bitcoin/pull/30708#discussion_r1853914205)
Fixed.
(https://github.com/bitcoin/bitcoin/pull/30708#discussion_r1853914205)
Fixed.
💬 jamesob commented on pull request "rpc: add getdescriptoractivity":
(https://github.com/bitcoin/bitcoin/pull/30708#discussion_r1853916616)
I'm going to leave this as-is.
I think partial results would be weird. I like the current behavior that it will succeed if possible but fail if results would be missing; I think any kind of requirement about running pruned or not would impair cases for which the descriptors the user cares about are on the "right side" of the prune cliff.
(https://github.com/bitcoin/bitcoin/pull/30708#discussion_r1853916616)
I'm going to leave this as-is.
I think partial results would be weird. I like the current behavior that it will succeed if possible but fail if results would be missing; I think any kind of requirement about running pruned or not would impair cases for which the descriptors the user cares about are on the "right side" of the prune cliff.
💬 jamesob commented on pull request "rpc: add getdescriptoractivity":
(https://github.com/bitcoin/bitcoin/pull/30708#discussion_r1853919432)
Done.
(https://github.com/bitcoin/bitcoin/pull/30708#discussion_r1853919432)
Done.