π¬ 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`
(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.
(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.
(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?
(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.
(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.
(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.
(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`
(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?
(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?
(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). */
```
(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.
(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.
(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?
(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
(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.
(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
...
(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. π€
(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
...
(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.
(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.