Bitcoin Core Github
44 subscribers
119K links
Download Telegram
πŸ’¬ BrandonOdiwuor commented on pull request "ci: use Mold linker for asan-lsan-ubsan-integer-no-depends-usdt workflow":
(https://github.com/bitcoin/bitcoin/pull/33370#issuecomment-3288151583)
@fanquake I updated the PR and moved the changes to `00_setup_env_native_asan.sh`
πŸ’¬ sdaftuar commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2346506939)
I think this type of p2p change is something we should discuss separately, outside of this PR, because it's not clear to me that there's a simple way to make a strict improvement in relay behavior. And the interpretation of what the `feefilter` message means/should mean is something we should document and coordinate across implementations as well.
πŸ’¬ sstone commented on pull request "Add a "tx output spender" index":
(https://github.com/bitcoin/bitcoin/pull/24539#issuecomment-3288247205)
> Approach ACK on using siphash and `FlatFilePos`.
>
> Do you want to squash the commits? This just feels like an improvement in every regard.

Rebased and squashed in https://github.com/bitcoin/bitcoin/commit/845b54defebc1987b9d654ea7b611a4ddebe345a. I also updated the PR description.
πŸ’¬ sdaftuar commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2346522367)
@instagibbs @sipa I think switching from `TxGraph::Ref()` to something like `/* ref = */ {}` is not worth the change, as the existing code seems just as good from a self-documentation standpoint. Will plan to leave as-is unless there's some benefit from invoking `{}` that I'm missing?
πŸ’¬ sipa commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2346524631)
It was just a nit, in case you weren't aware that it was possible to do this more succinctly. Agreed that it's more self-documenting as is.
πŸ’¬ sipa commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2346526733)
Sorry, ignore this. I misread `ConsumeTxMemPoolEntry` as being the constructor, rather than a separate function.
πŸ’¬ sipa commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2346533797)
I don't think it is; TxGraph should be pretty efficient at gathering all ancestors.
πŸ“ pythcoiner opened a pull request: "descriptor: fix comments in descriptor.cpp::DescriptorImpl"
(https://github.com/bitcoin/bitcoin/pull/33384)
`P2TR` was missing in `m_pubkey_args` description and there was a typo in `doc/descriptors.md`
πŸ’¬ pythcoiner commented on pull request "wallet: warn against accidental unsafe older() import":
(https://github.com/bitcoin/bitcoin/pull/33135#discussion_r2346604199)
nit: should we keep this debug log?
πŸ’¬ pythcoiner commented on pull request "wallet: warn against accidental unsafe older() import":
(https://github.com/bitcoin/bitcoin/pull/33135#discussion_r2346586549)
nit: not sure if those includes are stricly required?
πŸ’¬ pythcoiner commented on pull request "wallet: warn against accidental unsafe older() import":
(https://github.com/bitcoin/bitcoin/pull/33135#discussion_r2346503278)
nit (for consistency)
```suggestion
/** Semantic/safety warnings (includes subdescriptors). */
```
πŸ’¬ HowHsu commented on pull request "help: enrich help text for `-loadblock`":
(https://github.com/bitcoin/bitcoin/pull/33343#discussion_r2346751467)
Hi l0rinc,
I’ve updated the code. Since I’m not very familiar with the Bitcoin Core workflow, I wasn’t sure if it’s appropriate for me to mark this conversation as resolved, but I went ahead and did so. Please feel free to reopen it if that’s not the right process.
πŸ’¬ Raimo33 commented on issue "bench: unrealistic ConnectBlock benchmarks":
(https://github.com/bitcoin/bitcoin/issues/33375#issuecomment-3288478820)
it's not "a little bit of faithfulness" that you sacrifice. you're ignoring the cache completely.
πŸ’¬ Raimo33 commented on pull request "log: show reindex progress in `ImportBlocks`":
(https://github.com/bitcoin/bitcoin/pull/33353#discussion_r2346773084)
precompute the division?
πŸ’¬ Raimo33 commented on pull request "log: show reindex progress in `ImportBlocks`":
(https://github.com/bitcoin/bitcoin/pull/33353#issuecomment-3288490603)
Concept ACK
πŸ’¬ sdaftuar commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2346795265)
I didn't want to impose an ordering requirement on transactions being added to staging, which means that every time we add a new transaction, we'd have to loop over all the other transactions to see if any of them depend on the new one, so we get a complexity improvement (quadratic vs linear) by doing it in one pass at the end.
πŸ“ caesrcd opened a pull request: "contrib: add bash completion for new bitcoin command"
(https://github.com/bitcoin/bitcoin/pull/33385)
Adds a bash completion script for the new bitcoin command-line tool (introduced in #31375), which unifies the main Bitcoin Core executables under a single interface. This feature improves usability, reduces errors, and makes the command-line tools more easily discoverable for users working in a Linux bash environment.

The completion script dynamically lists available commands and options by parsing `bitcoin --help` and `bitcoin help`. It also incorporates the existing bash completions for `bi
...
πŸ’¬ stringintech commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#discussion_r2346797667)
Ah, I see. Thanks!
Just noticed another potential race condition between user checking for index out of bounds and the actual `btck_chain_get_by_height()` call. Like [here](https://github.com/TheCharlatan/rust-bitcoinkernel/blob/92f9e51555dcafb00702e665c865e5d9715a2f4d/src/lib.rs#L1774:L1786) in the rust wrapper; which could trigger the assert condition. πŸ€”
πŸ“ fanquake reopened a pull request: "contrib: add bash completion for new bitcoin command"
(https://github.com/bitcoin/bitcoin/pull/33385)
Adds a bash completion script for the new bitcoin command-line tool (introduced in #31375), which unifies the main Bitcoin Core executables under a single interface. This feature improves usability, reduces errors, and makes the command-line tools more easily discoverable for users working in a Linux bash environment.

The completion script dynamically lists available commands and options by parsing `bitcoin --help` and `bitcoin help`. It also incorporates the existing bash completions for `bi
...
πŸ’¬ caesrcd commented on pull request "Bash completion for the new bitcoin wrapper command":
(https://github.com/bitcoin/bitcoin/pull/33383#issuecomment-3288553563)
@fanquake I opened a new (third) PR: #33385
Unfortunately, the bot closed it again, likely due to the same heuristic applied to the previously closed PRs (first-time contributor + new file).

However, as requested, the third PR has been created from a new branch. If possible, I kindly ask that this new PR be reopened.