💬 instagibbs commented on pull request "AcceptMultipleTransactions: Fix workspace not being set as client_maxfeerate failure":
(https://github.com/bitcoin/bitcoin/pull/29735#discussion_r1555830543)
Made it shorter :)
(https://github.com/bitcoin/bitcoin/pull/29735#discussion_r1555830543)
Made it shorter :)
💬 instagibbs commented on pull request "AcceptMultipleTransactions: Fix workspace not being set as client_maxfeerate failure":
(https://github.com/bitcoin/bitcoin/pull/29735#discussion_r1555831622)
bit of an annoying circular reference issue(blocktools requiring utils) so for now at least make it grep-able what the magic value should be(and more greppable)
(https://github.com/bitcoin/bitcoin/pull/29735#discussion_r1555831622)
bit of an annoying circular reference issue(blocktools requiring utils) so for now at least make it grep-able what the magic value should be(and more greppable)
💬 instagibbs commented on pull request "AcceptMultipleTransactions: Fix workspace not being set as client_maxfeerate failure":
(https://github.com/bitcoin/bitcoin/pull/29735#discussion_r1555832375)
done
(https://github.com/bitcoin/bitcoin/pull/29735#discussion_r1555832375)
done
💬 instagibbs commented on pull request "AcceptMultipleTransactions: Fix workspace not being set as client_maxfeerate failure":
(https://github.com/bitcoin/bitcoin/pull/29735#discussion_r1555832564)
done, I think
(https://github.com/bitcoin/bitcoin/pull/29735#discussion_r1555832564)
done, I think
💬 instagibbs commented on pull request "AcceptMultipleTransactions: Fix workspace not being set as client_maxfeerate failure":
(https://github.com/bitcoin/bitcoin/pull/29735#discussion_r1555832735)
done
(https://github.com/bitcoin/bitcoin/pull/29735#discussion_r1555832735)
done
💬 laanwj commented on pull request "tracing: Only prepare tracepoint arguments when actually tracing":
(https://github.com/bitcoin/bitcoin/pull/26593#issuecomment-2042741859)
For some reason, after building with this, I dont get any output from:
```
$ gdb src/bitcoind
...
(gdb) info tracepoints
No tracepoints.
```
Tested with gdb `15.0.50.20240219-git` and `13.1`.
I checked that the tracepoints are included:
```
$ readelf -n ./src/bitcoind | grep NT_STAPSDT -A 4 -B 2
stapsdt 0x00000062 NT_STAPSDT (SystemTap probe descriptors)
Provider: net
Name: outbound_message
Location: 0x00000000001809bc, Base: 0x0000000000a2cb60, S
...
(https://github.com/bitcoin/bitcoin/pull/26593#issuecomment-2042741859)
For some reason, after building with this, I dont get any output from:
```
$ gdb src/bitcoind
...
(gdb) info tracepoints
No tracepoints.
```
Tested with gdb `15.0.50.20240219-git` and `13.1`.
I checked that the tracepoints are included:
```
$ readelf -n ./src/bitcoind | grep NT_STAPSDT -A 4 -B 2
stapsdt 0x00000062 NT_STAPSDT (SystemTap probe descriptors)
Provider: net
Name: outbound_message
Location: 0x00000000001809bc, Base: 0x0000000000a2cb60, S
...
💬 stickies-v commented on pull request "clang-tidy: Enable misc-no-recursion":
(https://github.com/bitcoin/bitcoin/pull/29690#issuecomment-2042758379)
ACK 78407b99ed6dd17f687fcbfb0486ecc433302287
(https://github.com/bitcoin/bitcoin/pull/29690#issuecomment-2042758379)
ACK 78407b99ed6dd17f687fcbfb0486ecc433302287
👍 ismaelsadeeq approved a pull request: "doc: 25.2 historical release notes"
(https://github.com/bitcoin/bitcoin/pull/29830#pullrequestreview-1986526306)
ACK 93bd2e2f6c9672fbf1120d1e25349f6edd29cfef
(https://github.com/bitcoin/bitcoin/pull/29830#pullrequestreview-1986526306)
ACK 93bd2e2f6c9672fbf1120d1e25349f6edd29cfef
💬 laanwj commented on pull request "tracing: Only prepare tracepoint arguments when actually tracing":
(https://github.com/bitcoin/bitcoin/pull/26593#issuecomment-2042827943)
Oh, it's `info probes` not `info tracepoints`. Whoops. It checks out now.
Code review and lightly tested ACK 3933bdd6cfc9accab9e0ed47e83a5cc27ada6b68
(https://github.com/bitcoin/bitcoin/pull/26593#issuecomment-2042827943)
Oh, it's `info probes` not `info tracepoints`. Whoops. It checks out now.
Code review and lightly tested ACK 3933bdd6cfc9accab9e0ed47e83a5cc27ada6b68
💬 laanwj commented on pull request "tracing: network connection tracepoints":
(https://github.com/bitcoin/bitcoin/pull/25832#discussion_r1555936795)
It's slightly inconsistent to pass this parameter as int, while the other enum (Connection Type) gets passed as string. Might similarly pass `GetNetworkName(...).c_str()`. Especially after #26593 which would remove the call overhead in the happy path.
No strong opinion though, I think the integer value is easier to use if you're going to make a histogram or such.
(https://github.com/bitcoin/bitcoin/pull/25832#discussion_r1555936795)
It's slightly inconsistent to pass this parameter as int, while the other enum (Connection Type) gets passed as string. Might similarly pass `GetNetworkName(...).c_str()`. Especially after #26593 which would remove the call overhead in the happy path.
No strong opinion though, I think the integer value is easier to use if you're going to make a histogram or such.
💬 sr-gi commented on pull request "test: Makes `wait_for_getdata` delete data on checks, plus allows to check the getdata message type":
(https://github.com/bitcoin/bitcoin/pull/29748#issuecomment-2042913328)
> Concept ACK, nit: Could you also take a look at `announce_tx_and_wait_for_getdata` in `p2p_segwit.py` for a similar cleanup?
Covered that in [067b009](https://github.com/bitcoin/bitcoin/pull/29748/commits/067b009bbf47f7bc5adeb6b500042f7c44bfb03f) by deleting the data on checks (pop instead of get) plus allowing to check the inv type.
For the latter, I've done it in a way that all items need to be of the same type. We could address this so every single inv has its own type, but for that w
...
(https://github.com/bitcoin/bitcoin/pull/29748#issuecomment-2042913328)
> Concept ACK, nit: Could you also take a look at `announce_tx_and_wait_for_getdata` in `p2p_segwit.py` for a similar cleanup?
Covered that in [067b009](https://github.com/bitcoin/bitcoin/pull/29748/commits/067b009bbf47f7bc5adeb6b500042f7c44bfb03f) by deleting the data on checks (pop instead of get) plus allowing to check the inv type.
For the latter, I've done it in a way that all items need to be of the same type. We could address this so every single inv has its own type, but for that w
...
💬 laanwj commented on pull request "Drop Windows Socket dependency for `randomenv.cpp`":
(https://github.com/bitcoin/bitcoin/pull/29786#issuecomment-2042945377)
Looks correct to me "If the function succeeds, the return value is a nonzero value.".
I'm not sure the netbios name is always the name as the hostname, but it is a good enough equivalent for randomness purposes, and I agree it is preferable not to depend on winsock for the kernel. The API, introduced in Windows 2000 is also old enough.
Code review ACK 03b87a3e64305ba651e22a730e35271dea8fea64.
(https://github.com/bitcoin/bitcoin/pull/29786#issuecomment-2042945377)
Looks correct to me "If the function succeeds, the return value is a nonzero value.".
I'm not sure the netbios name is always the name as the hostname, but it is a good enough equivalent for randomness purposes, and I agree it is preferable not to depend on winsock for the kernel. The API, introduced in Windows 2000 is also old enough.
Code review ACK 03b87a3e64305ba651e22a730e35271dea8fea64.
💬 epompeii commented on issue "Continuous benchmark tracking":
(https://github.com/bitcoin/bitcoin/issues/27284#issuecomment-2042950381)
> For Bitcoin Core, it would be useful to have an adapter for the nanobench JSON output. To track this, I've opened https://github.com/bencherdev/bencher/issues/361.
@0xB10C I would be more than happy to implement a nanobench JSON output adapter. It is going to take me a couple of weeks or so to get to it though. So you could either:
1. Use the nanobench output format template to [Bencher Metric Format](https://bencher.dev/docs/explanation/adapters/#-json)
2. Implement the adapter in Benc
...
(https://github.com/bitcoin/bitcoin/issues/27284#issuecomment-2042950381)
> For Bitcoin Core, it would be useful to have an adapter for the nanobench JSON output. To track this, I've opened https://github.com/bencherdev/bencher/issues/361.
@0xB10C I would be more than happy to implement a nanobench JSON output adapter. It is going to take me a couple of weeks or so to get to it though. So you could either:
1. Use the nanobench output format template to [Bencher Metric Format](https://bencher.dev/docs/explanation/adapters/#-json)
2. Implement the adapter in Benc
...
🚀 fanquake merged a pull request: "doc: 25.2 historical release notes"
(https://github.com/bitcoin/bitcoin/pull/29830)
(https://github.com/bitcoin/bitcoin/pull/29830)
💬 laanwj commented on pull request "net: update comment for service bit support info for seed.bitcoin.sipa.be":
(https://github.com/bitcoin/bitcoin/pull/29809#issuecomment-2042981215)
> Perhaps this tool can be modified to scan which filters are in use: https://github.com/bitcoin-core/bitcoin-maintainer-tools/blob/main/check-dnsseeds.py
This would be nice, and fairly easy if we can assume disallowed filters return nothing/an error, but seems like there's a exponential number of potential combinations to test? Or is something smarter than brute force possible?
(https://github.com/bitcoin/bitcoin/pull/29809#issuecomment-2042981215)
> Perhaps this tool can be modified to scan which filters are in use: https://github.com/bitcoin-core/bitcoin-maintainer-tools/blob/main/check-dnsseeds.py
This would be nice, and fairly easy if we can assume disallowed filters return nothing/an error, but seems like there's a exponential number of potential combinations to test? Or is something smarter than brute force possible?
👍 ismaelsadeeq approved a pull request: "AcceptMultipleTransactions: Fix workspace not being set as client_maxfeerate failure"
(https://github.com/bitcoin/bitcoin/pull/29735#pullrequestreview-1986776879)
ACK https://github.com/bitcoin/bitcoin/pull/29735/commits/4fe7d150eb3c85a6597d8fc910fe1490358197ad
I've reviewed [73b68....4fe7d15](https://github.com/bitcoin/bitcoin/compare/73b68bd8b4f9447e30091c7f8c3dc91a086bd93b..4fe7d150eb3c85a6597d8fc910f)
---
I've Ran the fuzz test without f28338b82be9820380ef217905ae9c167164f181
And verified the crash
```terminal
Test unit written to ./crash-7fd49c80d3088f672b4bf03ab8a1d7f1aedff0aa
Base64: q6sAIqsA/6sACQCrAAAAANr/+zUBAQENASUABAB5aKqgnQAA
...
(https://github.com/bitcoin/bitcoin/pull/29735#pullrequestreview-1986776879)
ACK https://github.com/bitcoin/bitcoin/pull/29735/commits/4fe7d150eb3c85a6597d8fc910fe1490358197ad
I've reviewed [73b68....4fe7d15](https://github.com/bitcoin/bitcoin/compare/73b68bd8b4f9447e30091c7f8c3dc91a086bd93b..4fe7d150eb3c85a6597d8fc910f)
---
I've Ran the fuzz test without f28338b82be9820380ef217905ae9c167164f181
And verified the crash
```terminal
Test unit written to ./crash-7fd49c80d3088f672b4bf03ab8a1d7f1aedff0aa
Base64: q6sAIqsA/6sACQCrAAAAANr/+zUBAQENASUABAB5aKqgnQAA
...
💬 ismaelsadeeq commented on pull request "AcceptMultipleTransactions: Fix workspace not being set as client_maxfeerate failure":
(https://github.com/bitcoin/bitcoin/pull/29735#discussion_r1556048973)
looking at this again, what do you think about just restarting the node with the `-datacarriersize=100000`, ` -maxmempool=5`, `-minrelaytxfee=0.00001000` after filling the mempool we then restart the node with default node settings?
Are there any advantage of delegating setting this to the caller?
If not then this will reduce duplication of all callers of `fill_mempool` settings this values.
(https://github.com/bitcoin/bitcoin/pull/29735#discussion_r1556048973)
looking at this again, what do you think about just restarting the node with the `-datacarriersize=100000`, ` -maxmempool=5`, `-minrelaytxfee=0.00001000` after filling the mempool we then restart the node with default node settings?
Are there any advantage of delegating setting this to the caller?
If not then this will reduce duplication of all callers of `fill_mempool` settings this values.
🚀 fanquake merged a pull request: "clang-tidy: Enable misc-no-recursion"
(https://github.com/bitcoin/bitcoin/pull/29690)
(https://github.com/bitcoin/bitcoin/pull/29690)
🚀 fanquake merged a pull request: "Change Luke Dashjr seed to dashjr-list-of-p2p-nodes.us"
(https://github.com/bitcoin/bitcoin/pull/29691)
(https://github.com/bitcoin/bitcoin/pull/29691)
⚠️ fanquake opened an issue: "ci: failure in `rpc_scanblocks.py`"
(https://github.com/bitcoin/bitcoin/issues/29831)
https://github.com/bitcoin/bitcoin/actions/runs/8600455763/job/23565458580?pr=29707#step:7:3826:
```bash
node0 2024-04-08T13:15:12.710688Z [httpworker.3] [rpc/request.cpp:187] [parse] [rpc] ThreadRPCServer method=getindexinfo user=__cookie__
test 2024-04-08T13:15:12.711000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
File "/Users/runner/work/bitcoin/bitcoin/ci/scratch/build/bitcoin-x
...
(https://github.com/bitcoin/bitcoin/issues/29831)
https://github.com/bitcoin/bitcoin/actions/runs/8600455763/job/23565458580?pr=29707#step:7:3826:
```bash
node0 2024-04-08T13:15:12.710688Z [httpworker.3] [rpc/request.cpp:187] [parse] [rpc] ThreadRPCServer method=getindexinfo user=__cookie__
test 2024-04-08T13:15:12.711000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
File "/Users/runner/work/bitcoin/bitcoin/ci/scratch/build/bitcoin-x
...