Bitcoin Core Github
42 subscribers
126K links
Download Telegram
👋 paplorinc's pull request is ready for review: "optimization: Speed up TryParseHex by 300%"
(https://github.com/bitcoin/bitcoin/pull/29458)
💬 theuni commented on pull request "serialization: c++20 endian/byteswap/clz modernization":
(https://github.com/bitcoin/bitcoin/pull/29263#issuecomment-1954651604)
Rebased. We'll see what c-i thinks now. Benchmarks should be all good.

I think it makes sense for #29408 to go first though.
💬 instagibbs commented on pull request "Mempool util: Add RBF diagram checks for single chunks against clusters of size 2":
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1496191920)
took in a modified form, left the direct exposure of `tail_feerate` as a YNGNI extension
💬 instagibbs commented on pull request "Mempool util: Add RBF diagram checks for single chunks against clusters of size 2":
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1496192018)
updated to be more explicit on what those values represent
💬 instagibbs commented on pull request "Mempool util: Add RBF diagram checks for single chunks against clusters of size 2":
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1496192101)
changed to standard lingo
💬 instagibbs commented on pull request "Mempool util: Add RBF diagram checks for single chunks against clusters of size 2":
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1496192220)
taken with some liberties
💬 instagibbs commented on pull request "Mempool util: Add RBF diagram checks for single chunks against clusters of size 2":
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1496192331)
replaced, even though I thought it sounded pretty smart
💬 instagibbs commented on pull request "Mempool util: Add RBF diagram checks for single chunks against clusters of size 2":
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1496192417)
@sdaftuar going to hold off on this for now if that's ok
💬 instagibbs commented on pull request "Mempool util: Add RBF diagram checks for single chunks against clusters of size 2":
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1496193191)
taken with only light modifications, thanks!
💬 instagibbs commented on pull request "Mempool util: Add RBF diagram checks for single chunks against clusters of size 2":
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1496193271)
fixed
💬 instagibbs commented on pull request "Mempool util: Add RBF diagram checks for single chunks against clusters of size 2":
(https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1496193353)
removed
💬 instagibbs commented on pull request "Mempool util: Add RBF diagram checks for single chunks against clusters of size 2":
(https://github.com/bitcoin/bitcoin/pull/29242#issuecomment-1954669587)
> I commented https://github.com/bitcoin/bitcoin/pull/29242#discussion_r1493358857 that I think we should drop the Assume(size != 0 || fee == 0) calls in feefrac.h, so that we don't have to police all the places we invoke operator- to make sure we can never accidentally violate that.
Another nit: in the commit message for commit https://github.com/bitcoin/bitcoin/commit/860823fb93212fa78ab928589bd403da462be222, it should say "FeeFrac" and not "FeeFrace"

Addressed these by removing Assumes an
...
🤔 instagibbs reviewed a pull request: "policy: enable sibling eviction for v3 transactions"
(https://github.com/bitcoin/bitcoin/pull/29306#pullrequestreview-1891040322)
looking good, will do some testing
💬 instagibbs commented on pull request "policy: enable sibling eviction for v3 transactions":
(https://github.com/bitcoin/bitcoin/pull/29306#discussion_r1496216091)
was this always erroneous?
💬 instagibbs commented on pull request "policy: enable sibling eviction for v3 transactions":
(https://github.com/bitcoin/bitcoin/pull/29306#discussion_r1496208017)
let's get a bit of test coverage of `->second`
💬 instagibbs commented on pull request "policy: enable sibling eviction for v3 transactions":
(https://github.com/bitcoin/bitcoin/pull/29306#discussion_r1496221417)
log/description of this test should get updated or maybe merged into test_v3_sibling_eviction
💬 instagibbs commented on pull request "policy: enable sibling eviction for v3 transactions":
(https://github.com/bitcoin/bitcoin/pull/29306#discussion_r1496235168)
```Suggestion
# However, an RBF (with conflicting inputs) is possible even if the resulting cluster exceeds size 2
```
🤔 mzumsande reviewed a pull request: "net: call `Select` with reachable networks in `ThreadOpenConnections`"
(https://github.com/bitcoin/bitcoin/pull/29436#pullrequestreview-1891096814)
I tried to test the performance impact on the "normal" case of an addrman that only has addrs for reachable nets:
If I change the `AddrManSelect` benchmark to use `addrman.Select(true, g_reachable_nets.All()); ` the performance goes down by a factor of ~5 for me (from `~160ns/op` to `~770 ns/op`). Do you see a similar effect?
💬 paplorinc commented on pull request "optimization: Speed up TryParseHex by 300%":
(https://github.com/bitcoin/bitcoin/pull/29458#discussion_r1496260138)
we don't expect most hex strings to contain spaces so let's fall back to this when we can't parse the string instead
💬 paplorinc commented on pull request "optimization: Speed up TryParseHex by 300%":
(https://github.com/bitcoin/bitcoin/pull/29458#discussion_r1496260453)
c1 validation can be done sooner