💬 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
...
💬 MarcoFalke commented on pull request "test: Add unit & functional test coverage for blockstore":
(https://github.com/bitcoin/bitcoin/pull/27850#issuecomment-1640702774)
Are you still working on this?
(https://github.com/bitcoin/bitcoin/pull/27850#issuecomment-1640702774)
Are you still working on this?
💬 pinheadmz commented on pull request "test: Add unit & functional test coverage for blockstore":
(https://github.com/bitcoin/bitcoin/pull/27850#issuecomment-1640704021)
> Are you still working on this?
Absolutely, thanks. Had a few other things to catch up on. This will be updated before the end of the week
(https://github.com/bitcoin/bitcoin/pull/27850#issuecomment-1640704021)
> Are you still working on this?
Absolutely, thanks. Had a few other things to catch up on. This will be updated before the end of the week
💬 MarcoFalke commented on issue "ci: Future of macOS and Windows MSVC CI tasks":
(https://github.com/bitcoin/bitcoin/issues/28098#issuecomment-1640710663)
We already use credits for some Linux tasks, so nothing will change for them. The others (or all) can be moved to persistent workers easily, I suspect.
Edited OP to mention GitHub Actions CI as alternative. Last time we looked it required write access, but GitHub already has full write access, so I am not sure if this is still a concern or if it even applies?
(https://github.com/bitcoin/bitcoin/issues/28098#issuecomment-1640710663)
We already use credits for some Linux tasks, so nothing will change for them. The others (or all) can be moved to persistent workers easily, I suspect.
Edited OP to mention GitHub Actions CI as alternative. Last time we looked it required write access, but GitHub already has full write access, so I am not sure if this is still a concern or if it even applies?
💬 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-1640745025)
Rebased.
(https://github.com/bitcoin/bitcoin/pull/25380#issuecomment-1640745025)
Rebased.
📝 sipa opened a pull request: "crypto: ChaCha20 `Span<std::byte>` modernization & follow-ups"
(https://github.com/bitcoin/bitcoin/pull/28100)
This modernizes the ChaCha20 and ChaCha20Aligned interfaces to be `Span<std::byte>` based, and other improvements.
* Removes our OpenSSH-inspired variant of the ChaCha20Poly1305 AEAD (which was added in anticipation of BIP324 using it, but the spec was later changed to using the RFC8439 one unmodified). #28008 replaces the code anyway, so it is pointless to apply the refectoring done in this PR to that old code.
* Modifies all functions and constructors of `ChaCha20` and `ChaCha20Aligned` to
...
(https://github.com/bitcoin/bitcoin/pull/28100)
This modernizes the ChaCha20 and ChaCha20Aligned interfaces to be `Span<std::byte>` based, and other improvements.
* Removes our OpenSSH-inspired variant of the ChaCha20Poly1305 AEAD (which was added in anticipation of BIP324 using it, but the spec was later changed to using the RFC8439 one unmodified). #28008 replaces the code anyway, so it is pointless to apply the refectoring done in this PR to that old code.
* Modifies all functions and constructors of `ChaCha20` and `ChaCha20Aligned` to
...
💬 sipa commented on pull request "Reduce wasted pseudorandom bytes in ChaCha20 + various improvements":
(https://github.com/bitcoin/bitcoin/pull/26153#discussion_r1267164225)
Done in #28100.
(https://github.com/bitcoin/bitcoin/pull/26153#discussion_r1267164225)
Done in #28100.
💬 sipa commented on pull request "Reduce wasted pseudorandom bytes in ChaCha20 + various improvements":
(https://github.com/bitcoin/bitcoin/pull/26153#discussion_r1267164605)
Done in #28100.
(https://github.com/bitcoin/bitcoin/pull/26153#discussion_r1267164605)
Done in #28100.
💬 sipa commented on pull request "Reduce wasted pseudorandom bytes in ChaCha20 + various improvements":
(https://github.com/bitcoin/bitcoin/pull/26153#discussion_r1267164850)
Done in #28100.
(https://github.com/bitcoin/bitcoin/pull/26153#discussion_r1267164850)
Done in #28100.
💬 sipa commented on pull request "Reduce wasted pseudorandom bytes in ChaCha20 + various improvements":
(https://github.com/bitcoin/bitcoin/pull/26153#discussion_r1267165531)
I had forgotten about this PR comment, but this was effectively done in #27985.
(https://github.com/bitcoin/bitcoin/pull/26153#discussion_r1267165531)
I had forgotten about this PR comment, but this was effectively done in #27985.