💬 dergoegge commented on pull request "net processing, refactor: Decouple PeerManager from gArgs":
(https://github.com/bitcoin/bitcoin/pull/27499#discussion_r1266953778)
We don't do this to any of the other Options we have (afaict). There is also little benefit since this only happens once at startup.
(https://github.com/bitcoin/bitcoin/pull/27499#discussion_r1266953778)
We don't do this to any of the other Options we have (afaict). There is also little benefit since this only happens once at startup.
💬 dergoegge commented on pull request "net processing, refactor: Decouple PeerManager from gArgs":
(https://github.com/bitcoin/bitcoin/pull/27499#discussion_r1266954026)
Done
(https://github.com/bitcoin/bitcoin/pull/27499#discussion_r1266954026)
Done
💬 dergoegge commented on pull request "net processing, refactor: Decouple PeerManager from gArgs":
(https://github.com/bitcoin/bitcoin/pull/27499#discussion_r1266957788)
Done
(https://github.com/bitcoin/bitcoin/pull/27499#discussion_r1266957788)
Done
💬 dergoegge commented on pull request "net processing, refactor: Decouple PeerManager from gArgs":
(https://github.com/bitcoin/bitcoin/pull/27499#discussion_r1266954608)
Will leave as is, can be done in a follow up
(https://github.com/bitcoin/bitcoin/pull/27499#discussion_r1266954608)
Will leave as is, can be done in a follow up
💬 dergoegge commented on pull request "net processing, refactor: Decouple PeerManager from gArgs":
(https://github.com/bitcoin/bitcoin/pull/27499#discussion_r1266953921)
Done
(https://github.com/bitcoin/bitcoin/pull/27499#discussion_r1266953921)
Done
💬 dergoegge commented on pull request "net processing, refactor: Decouple PeerManager from gArgs":
(https://github.com/bitcoin/bitcoin/pull/27499#discussion_r1266941065)
From the developer notes (see discussion in #23810):
```
When casting between integer types, use functional casts such as int(x) or int{x} instead of (int) x.
```
Changed it to a functional cast.
(https://github.com/bitcoin/bitcoin/pull/27499#discussion_r1266941065)
From the developer notes (see discussion in #23810):
```
When casting between integer types, use functional casts such as int(x) or int{x} instead of (int) x.
```
Changed it to a functional cast.
📝 fanquake opened a pull request: "contrib: move user32.dll from bitcoind.exe libs"
(https://github.com/bitcoin/bitcoin/pull/28099)
The user interface library is no-longer needed by `bitcoind.exe`, or utils, only `bitcoin-qt.exe`.
Add missing doc.
(https://github.com/bitcoin/bitcoin/pull/28099)
The user interface library is no-longer needed by `bitcoind.exe`, or utils, only `bitcoin-qt.exe`.
Add missing doc.
🚀 fanquake merged a pull request: "ci: Use DOCKER_BUILDKIT for lint image"
(https://github.com/bitcoin/bitcoin/pull/28083)
(https://github.com/bitcoin/bitcoin/pull/28083)
💬 furszy commented on pull request "init: return error when block index is non-contiguous, fix feature_init.py file perturbation":
(https://github.com/bitcoin/bitcoin/pull/27823#discussion_r1266974334)
In ad66ca1e:
What if the genesis block index is missing?
The first round of the loop would set `previous_index` to block_index_1. Second one to block_index_2, etc. Making `LoadBlockIndex()` pass.
(https://github.com/bitcoin/bitcoin/pull/27823#discussion_r1266974334)
In ad66ca1e:
What if the genesis block index is missing?
The first round of the loop would set `previous_index` to block_index_1. Second one to block_index_2, etc. Making `LoadBlockIndex()` pass.
💬 darosior commented on pull request "Detect and ignore transactions that were CPFP'd in the fee estimator":
(https://github.com/bitcoin/bitcoin/pull/25380#issuecomment-1640493242)
Good point. I would have bet it's a non-trivial percentage (and an increasing one), so i wrote a quick Python script to measure this. It's [there](https://github.com/darosior/count_cpfps/blob/master/count_cpfps.py).
Here is the results for a few (small) block ranges i picked at random. The CPFP usage seems non trivial to me (between 10% to 15% of all transactions in a block).
<details>
<summary>Between block heights 799241 and 799251</summary>
```
Between block heights 799241 and 79
...
(https://github.com/bitcoin/bitcoin/pull/25380#issuecomment-1640493242)
Good point. I would have bet it's a non-trivial percentage (and an increasing one), so i wrote a quick Python script to measure this. It's [there](https://github.com/darosior/count_cpfps/blob/master/count_cpfps.py).
Here is the results for a few (small) block ranges i picked at random. The CPFP usage seems non trivial to me (between 10% to 15% of all transactions in a block).
<details>
<summary>Between block heights 799241 and 799251</summary>
```
Between block heights 799241 and 79
...
💬 TheCharlatan commented on pull request "util: Replace std::filesystem with util/fs.h":
(https://github.com/bitcoin/bitcoin/pull/28076#issuecomment-1640501925)
I'd love for Rust to be "backdoored" into the project, but I am not sure if adding yet another linting infrastructure is the right way to do this.
What does this check do that a check with a clang-tidy plugin could not? I have pushed a dirty prototype (likely incomplete) for such a plugin check here: https://github.com/TheCharlatan/bitcoin-tidy-experiments/commit/5c1f1e1289758df4a0407cddcfd1f8b043c53be9. It checks for declarations of std::filesystem::path, and calls to its string method. It
...
(https://github.com/bitcoin/bitcoin/pull/28076#issuecomment-1640501925)
I'd love for Rust to be "backdoored" into the project, but I am not sure if adding yet another linting infrastructure is the right way to do this.
What does this check do that a check with a clang-tidy plugin could not? I have pushed a dirty prototype (likely incomplete) for such a plugin check here: https://github.com/TheCharlatan/bitcoin-tidy-experiments/commit/5c1f1e1289758df4a0407cddcfd1f8b043c53be9. It checks for declarations of std::filesystem::path, and calls to its string method. It
...
💬 hebasto commented on pull request "contrib: move user32.dll from bitcoind.exe libs":
(https://github.com/bitcoin/bitcoin/pull/28099#issuecomment-1640539928)
Concept ACK.
(https://github.com/bitcoin/bitcoin/pull/28099#issuecomment-1640539928)
Concept ACK.
💬 darosior commented on pull request "Detect and ignore transactions that were CPFP'd in the fee estimator":
(https://github.com/bitcoin/bitcoin/pull/25380#issuecomment-1640560703)
Here are the results for a few block ranges with the percentage of candidates for fee estimation among all transactions, and the percentage of CPFP among these candidates. It's around 10%.
<details>
<summary>Between block heights 798241 and 798251</summary>
```
Between block heights 798241 and 798251:
- The average percentage of transactions with a descendant in the same block is 8.198356016709338%
- The average percentage of transactions with an ancestor in the same block is
...
(https://github.com/bitcoin/bitcoin/pull/25380#issuecomment-1640560703)
Here are the results for a few block ranges with the percentage of candidates for fee estimation among all transactions, and the percentage of CPFP among these candidates. It's around 10%.
<details>
<summary>Between block heights 798241 and 798251</summary>
```
Between block heights 798241 and 798251:
- The average percentage of transactions with a descendant in the same block is 8.198356016709338%
- The average percentage of transactions with an ancestor in the same block is
...
💬 darosior commented on pull request "Detect and ignore transactions that were CPFP'd in the fee estimator":
(https://github.com/bitcoin/bitcoin/pull/25380#discussion_r1267035663)
> Also why is /* inBlock = */true ?
That's so it's not counted as a failure.
(https://github.com/bitcoin/bitcoin/pull/25380#discussion_r1267035663)
> Also why is /* inBlock = */true ?
That's so it's not counted as a failure.
💬 pinheadmz commented on pull request "Support JSON-RPC 2.0 when requested by client":
(https://github.com/bitcoin/bitcoin/pull/27101#discussion_r1267073906)
Added in push to 26dcb359acf421dc3f448b49e0b564e714c54792. Also had to tweak `authproxy.py` to accommodate code 204
(https://github.com/bitcoin/bitcoin/pull/27101#discussion_r1267073906)
Added in push to 26dcb359acf421dc3f448b49e0b564e714c54792. Also had to tweak `authproxy.py` to accommodate code 204
💬 jonatack commented on pull request "net, refactor: remove unneeded exports, use helpers over low-level code, use optional":
(https://github.com/bitcoin/bitcoin/pull/28078#discussion_r1267078962)
Done
(https://github.com/bitcoin/bitcoin/pull/28078#discussion_r1267078962)
Done
💬 jonatack commented on pull request "net, refactor: remove unneeded exports, use helpers over low-level code, use optional":
(https://github.com/bitcoin/bitcoin/pull/28078#discussion_r1267079059)
Done
(https://github.com/bitcoin/bitcoin/pull/28078#discussion_r1267079059)
Done
💬 jonatack commented on pull request "net, refactor: remove unneeded exports, use helpers over low-level code, use optional":
(https://github.com/bitcoin/bitcoin/pull/28078#discussion_r1267080536)
Nice! Took the liberty of adding a commit credited to you. LMK if that's ok and if you'd like any changes.
(https://github.com/bitcoin/bitcoin/pull/28078#discussion_r1267080536)
Nice! Took the liberty of adding a commit credited to you. LMK if that's ok and if you'd like any changes.
💬 jonatack commented on pull request "net, refactor: remove unneeded exports, use helpers over low-level code, use optional":
(https://github.com/bitcoin/bitcoin/pull/28078#discussion_r1267080665)
Done
(https://github.com/bitcoin/bitcoin/pull/28078#discussion_r1267080665)
Done
💬 MarcoFalke commented on pull request "util: Replace std::filesystem with util/fs.h":
(https://github.com/bitcoin/bitcoin/pull/28076#issuecomment-1640695209)
> What do you think?
I'd say we should pick what reviewers like the best. However, as you say the clang-tidy plugin has many downsides:
* Likely incomplete (May be missing a Decl matcher, ignores comments, ...)
* Harder to review, and easier to break
* More code
* Requires a full build system
* Slower
I think it should be decided on a case-by-case basis whether a simple `git grep`-based linter is preferred, or a fully engineered clang-tidy plugin. Here, I think a `git grep` seems a
...
(https://github.com/bitcoin/bitcoin/pull/28076#issuecomment-1640695209)
> What do you think?
I'd say we should pick what reviewers like the best. However, as you say the clang-tidy plugin has many downsides:
* Likely incomplete (May be missing a Decl matcher, ignores comments, ...)
* Harder to review, and easier to break
* More code
* Requires a full build system
* Slower
I think it should be decided on a case-by-case basis whether a simple `git grep`-based linter is preferred, or a fully engineered clang-tidy plugin. Here, I think a `git grep` seems a
...