💬 maflcko commented on pull request "Set notifications m_tip_block in LoadChainTip()":
(https://github.com/bitcoin/bitcoin/pull/31346#issuecomment-2493675258)
This is incomplete. You'll have to update the docs as well: "If the tip was not connected on
* startup, this will wait.", and "which may
//! be true even long after startup, until shutdown.". Also, in init.cpp you can remove the if-guard?
(https://github.com/bitcoin/bitcoin/pull/31346#issuecomment-2493675258)
This is incomplete. You'll have to update the docs as well: "If the tip was not connected on
* startup, this will wait.", and "which may
//! be true even long after startup, until shutdown.". Also, in init.cpp you can remove the if-guard?
💬 orangesurf commented on pull request "miner: Reorg Testnet4 minimum difficulty blocks":
(https://github.com/bitcoin/bitcoin/pull/31117#issuecomment-2493677462)
A miner (earning coinbase rewards to [tb1q2dsc...](https://mempool.space/testnet4/address/tb1q2dsc94zq40nwnz27w5rxljwllutnwjtlxk44fz) is currently overwhelmingly dominant on testnet4.
Since block [000000000000000167286a4bea3bca60d6c3ab2cbbef79537bdf07b6420a2d2d / 54780](https://mempool.space/testnet4/block/000000000000000167286a4bea3bca60d6c3ab2cbbef79537bdf07b6420a2d2d) this miner has mined every bl
...
(https://github.com/bitcoin/bitcoin/pull/31117#issuecomment-2493677462)
A miner (earning coinbase rewards to [tb1q2dsc...](https://mempool.space/testnet4/address/tb1q2dsc94zq40nwnz27w5rxljwllutnwjtlxk44fz) is currently overwhelmingly dominant on testnet4.
Since block [000000000000000167286a4bea3bca60d6c3ab2cbbef79537bdf07b6420a2d2d / 54780](https://mempool.space/testnet4/block/000000000000000167286a4bea3bca60d6c3ab2cbbef79537bdf07b6420a2d2d) this miner has mined every bl
...
💬 maflcko commented on issue "ci: how to run native arm job on Apple silicon?":
(https://github.com/bitcoin/bitcoin/issues/31344#issuecomment-2493687553)
Yes, the cirrus machine is intentionally an aarch64 (also referred to as arm64) machine, because arm64 is generally available as performant hardware, compared to 32-bit arm. It was also intentionally selected to support 32-bit mode. (If you are looking for a cloud solution, IIRC only Hetzner cloud ships compatible VMs). Otherwise, you'll have to shop for compatible bare-metal yourself, or use qemu (or similar).
(https://github.com/bitcoin/bitcoin/issues/31344#issuecomment-2493687553)
Yes, the cirrus machine is intentionally an aarch64 (also referred to as arm64) machine, because arm64 is generally available as performant hardware, compared to 32-bit arm. It was also intentionally selected to support 32-bit mode. (If you are looking for a cloud solution, IIRC only Hetzner cloud ships compatible VMs). Otherwise, you'll have to shop for compatible bare-metal yourself, or use qemu (or similar).
⚠️ rkrux opened an issue: "Add support for creating v3 raw transactions in `createrawtransaction` RPC"
(https://github.com/bitcoin/bitcoin/issues/31348)
### Please describe the feature you'd like to see added.
Currently, `createrawtransaction` RPC creates only v2 raw transaction, i.e. the first byte of the serialised transaction hex is `02`. It'd be helpful for the RPC to conditionally create V3 raw transactions if such intent is passed in the arguments of the RPC call.
### Is your feature related to a problem, if so please describe it.
_No response_
### Describe the solution you'd like
- A new argument to the RPC call can be passed that al
...
(https://github.com/bitcoin/bitcoin/issues/31348)
### Please describe the feature you'd like to see added.
Currently, `createrawtransaction` RPC creates only v2 raw transaction, i.e. the first byte of the serialised transaction hex is `02`. It'd be helpful for the RPC to conditionally create V3 raw transactions if such intent is passed in the arguments of the RPC call.
### Is your feature related to a problem, if so please describe it.
_No response_
### Describe the solution you'd like
- A new argument to the RPC call can be passed that al
...
💬 ryanofsky commented on pull request "util: Improve documentation and negation of args":
(https://github.com/bitcoin/bitcoin/pull/31212#discussion_r1853870363)
> I'm not against that, my concern is just that reproducing old behavior before the fix isn't very useful activity
Strongly disagree :). In 2ca1460ae3a7217eaa8c5972515bf622bedadfce if tests had not been added *before* that commit checking for incorrect HTTP error codes, you would not know from looking at the change what the behavior was before the bugfix, or how new tests were connected to it. It is very useful to have test coverage for bugs before they are fixed.
> it's a lot more useful
...
(https://github.com/bitcoin/bitcoin/pull/31212#discussion_r1853870363)
> I'm not against that, my concern is just that reproducing old behavior before the fix isn't very useful activity
Strongly disagree :). In 2ca1460ae3a7217eaa8c5972515bf622bedadfce if tests had not been added *before* that commit checking for incorrect HTTP error codes, you would not know from looking at the change what the behavior was before the bugfix, or how new tests were connected to it. It is very useful to have test coverage for bugs before they are fixed.
> it's a lot more useful
...
✅ maflcko closed a pull request: "ci: Place datadirs for tests under tmpfs"
(https://github.com/bitcoin/bitcoin/pull/31182)
(https://github.com/bitcoin/bitcoin/pull/31182)
💬 maflcko commented on pull request "ci: Place datadirs for tests under tmpfs":
(https://github.com/bitcoin/bitcoin/pull/31182#issuecomment-2493700147)
I tested it on a Pi 4B, with 4GB of memory, Ubuntu 24.04, using 1 CPU thread: `CCACHE_MAXSIZE=1500M MAKEJOBS="-j1" FILE_ENV="./ci/test/00_setup_env_native_nowallet_libbitcoinkernel.sh" /usr/bin/time --format='%U+%S %e (%E)' ./ci/test_run_all.sh`. However, I couldn't find any difference either. Moreover, with `MAKEJOBS="-j4"`, this pull performed worse than the commit prior to it (55min vs 1h5min).
Not sure why, but I guess modern Linux will cache the file IO sufficiently to not make a differe
...
(https://github.com/bitcoin/bitcoin/pull/31182#issuecomment-2493700147)
I tested it on a Pi 4B, with 4GB of memory, Ubuntu 24.04, using 1 CPU thread: `CCACHE_MAXSIZE=1500M MAKEJOBS="-j1" FILE_ENV="./ci/test/00_setup_env_native_nowallet_libbitcoinkernel.sh" /usr/bin/time --format='%U+%S %e (%E)' ./ci/test_run_all.sh`. However, I couldn't find any difference either. Moreover, with `MAKEJOBS="-j4"`, this pull performed worse than the commit prior to it (55min vs 1h5min).
Not sure why, but I guess modern Linux will cache the file IO sufficiently to not make a differe
...
🤔 glozow reviewed a pull request: "Package validation: accept packages of size 1"
(https://github.com/bitcoin/bitcoin/pull/31096#pullrequestreview-2454400142)
I still don't really like that the `IsChildWithParents()` function returns true for a tx that isn't a child / doesn't have any parents. Also dislike that we run the whole input-loading thing for the child-with-unconfirmed-parents check even for a singleton tx. I guess one way around this is to rename the 2 `IsChildWithParents` functions to something along the lines of "is topology we can handle" and to add a `package.size() > 1` gate to the child-with-unconfirmed-parents check.
I spent some t
...
(https://github.com/bitcoin/bitcoin/pull/31096#pullrequestreview-2454400142)
I still don't really like that the `IsChildWithParents()` function returns true for a tx that isn't a child / doesn't have any parents. Also dislike that we run the whole input-loading thing for the child-with-unconfirmed-parents check even for a singleton tx. I guess one way around this is to rename the 2 `IsChildWithParents` functions to something along the lines of "is topology we can handle" and to add a `package.size() > 1` gate to the child-with-unconfirmed-parents check.
I spent some t
...
💬 l0rinc commented on pull request "util: Improve documentation and negation of args":
(https://github.com/bitcoin/bitcoin/pull/31212#discussion_r1853891120)
> if tests had not been added before that commit checking for incorrect HTTP error codes, you would not know from looking at the change what the behavior was before the bugfix
The test would fail before the fix with the exact reason, not sure we're in a disagreement here.
> If the bugfix commit show a line of test code changing from [...]
> if the bugfix commit just included an entirely new [...]
But my point was exactly that if there's a failing test in the commit before the fix, whic
...
(https://github.com/bitcoin/bitcoin/pull/31212#discussion_r1853891120)
> if tests had not been added before that commit checking for incorrect HTTP error codes, you would not know from looking at the change what the behavior was before the bugfix
The test would fail before the fix with the exact reason, not sure we're in a disagreement here.
> If the bugfix commit show a line of test code changing from [...]
> if the bugfix commit just included an entirely new [...]
But my point was exactly that if there's a failing test in the commit before the fix, whic
...
💬 fanquake commented on pull request "build: increase minimum supported Windows to 10.0":
(https://github.com/bitcoin/bitcoin/pull/31172#issuecomment-2493742473)
Guix Build:
```bash
606d0909c4591fc7dac3759e230e7bd3de00555c1c535d437ca8bc19df85fc70 guix-build-ee1128ead846/output/aarch64-linux-gnu/SHA256SUMS.part
4aca1c476b6824d485044c6636ce2ecf45f542a89bc501493f4868cf16c13d1f guix-build-ee1128ead846/output/aarch64-linux-gnu/bitcoin-ee1128ead846-aarch64-linux-gnu-debug.tar.gz
46d17a50226b60af12124b9c2b70cdab1c257a2034463999c0340d8a8b573cdd guix-build-ee1128ead846/output/aarch64-linux-gnu/bitcoin-ee1128ead846-aarch64-linux-gnu.tar.gz
d82c381c8d4e888e
...
(https://github.com/bitcoin/bitcoin/pull/31172#issuecomment-2493742473)
Guix Build:
```bash
606d0909c4591fc7dac3759e230e7bd3de00555c1c535d437ca8bc19df85fc70 guix-build-ee1128ead846/output/aarch64-linux-gnu/SHA256SUMS.part
4aca1c476b6824d485044c6636ce2ecf45f542a89bc501493f4868cf16c13d1f guix-build-ee1128ead846/output/aarch64-linux-gnu/bitcoin-ee1128ead846-aarch64-linux-gnu-debug.tar.gz
46d17a50226b60af12124b9c2b70cdab1c257a2034463999c0340d8a8b573cdd guix-build-ee1128ead846/output/aarch64-linux-gnu/bitcoin-ee1128ead846-aarch64-linux-gnu.tar.gz
d82c381c8d4e888e
...
💬 maflcko commented on pull request "util: Improve documentation and negation of args":
(https://github.com/bitcoin/bitcoin/pull/31212#discussion_r1853899185)
> > if tests had not been added before that commit checking for incorrect HTTP error codes, you would not know from looking at the change what the behavior was before the bugfix
>
> The test would fail before the fix with the exact reason, not sure we're in a disagreement here.
The test would be reproducing the bug, thus pass. The next commit would fix the pre-existing bug in the code and the test, and would thus pass as well.
(https://github.com/bitcoin/bitcoin/pull/31212#discussion_r1853899185)
> > if tests had not been added before that commit checking for incorrect HTTP error codes, you would not know from looking at the change what the behavior was before the bugfix
>
> The test would fail before the fix with the exact reason, not sure we're in a disagreement here.
The test would be reproducing the bug, thus pass. The next commit would fix the pre-existing bug in the code and the test, and would thus pass as well.
💬 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)