✅ fanquake closed an issue: "policy: allow RBF descendant carveout whenever conflicts exist, not just when number of conflicts == 1"
(https://github.com/bitcoin/bitcoin/issues/16819)
(https://github.com/bitcoin/bitcoin/issues/16819)
📝 fanquake opened a pull request: "doc: fix `LIBRARY_PATH` comment"
(https://github.com/bitcoin/bitcoin/pull/33308)
Now that we build capnp, qt isn't the only native package.
(https://github.com/bitcoin/bitcoin/pull/33308)
Now that we build capnp, qt isn't the only native package.
💬 katesalazar commented on pull request "gui: Avoid pathological QT text/markdown behavior...":
(https://github.com/bitcoin-core/gui/pull/886#issuecomment-3254546250)
> [...] I am not 100% sure what is causing this issue in QT's conversion of our HTML to markdown [...]
You implying that an upstream bug must be searched in Qt?
(https://github.com/bitcoin-core/gui/pull/886#issuecomment-3254546250)
> [...] I am not 100% sure what is causing this issue in QT's conversion of our HTML to markdown [...]
You implying that an upstream bug must be searched in Qt?
💬 katesalazar commented on pull request "gui: Avoid pathological QT text/markdown behavior...":
(https://github.com/bitcoin-core/gui/pull/886#issuecomment-3254557610)
Thank you
Approach ACK
possible nit:
```
PlainCopyQTextEdit
^
```
(https://github.com/bitcoin-core/gui/pull/886#issuecomment-3254557610)
Thank you
Approach ACK
possible nit:
```
PlainCopyQTextEdit
^
```
💬 fjahr commented on pull request "index: Fix coinstats overflow":
(https://github.com/bitcoin/bitcoin/pull/30469#issuecomment-3254558440)
Hm, not clear to me why clang-tidy CI complains here about lines I haven't touched in this PR. But that's what the last fixup commit I just pushed is for.
(https://github.com/bitcoin/bitcoin/pull/30469#issuecomment-3254558440)
Hm, not clear to me why clang-tidy CI complains here about lines I haven't touched in this PR. But that's what the last fixup commit I just pushed is for.
💬 katesalazar commented on pull request "gui: Avoid pathological QT text/markdown behavior...":
(https://github.com/bitcoin-core/gui/pull/886#issuecomment-3254577747)
nit: splash post in two paragraphs (none tiny) then the commit message is a one liner
(https://github.com/bitcoin-core/gui/pull/886#issuecomment-3254577747)
nit: splash post in two paragraphs (none tiny) then the commit message is a one liner
💬 glozow commented on issue "policy: allow RBF descendant carveout whenever conflicts exist, not just when number of conflicts == 1":
(https://github.com/bitcoin/bitcoin/issues/16819#issuecomment-3254599880)
Yeah the new design stages the changes and then checks policy limits. Currently in #28676: https://github.com/bitcoin/bitcoin/blob/61f51546af33923c9e5dc9632ba5544aa853ae71/src/validation.cpp#L1359-L1363
So the carveout becomes unnecessary since we aren't estimating cluster size, but calculating it
(https://github.com/bitcoin/bitcoin/issues/16819#issuecomment-3254599880)
Yeah the new design stages the changes and then checks policy limits. Currently in #28676: https://github.com/bitcoin/bitcoin/blob/61f51546af33923c9e5dc9632ba5544aa853ae71/src/validation.cpp#L1359-L1363
So the carveout becomes unnecessary since we aren't estimating cluster size, but calculating it
💬 fjahr commented on pull request "index: Force database compaction in coinstatsindex":
(https://github.com/bitcoin/bitcoin/pull/33306#issuecomment-3254606614)
Good question and I am definitely not a LevelDB expert either. I think that at the size and usage pattern of the coinstatsindex we are not really facing any significant problems on a modern computer and system since the size of the data we are storing is very small relatively speaking for what LevelDB is capable of handling. But I think the impact is probably also non-zero and might increase over time as the blockchain grows. Very high-level rationale: there must be a reason LevelDB does these c
...
(https://github.com/bitcoin/bitcoin/pull/33306#issuecomment-3254606614)
Good question and I am definitely not a LevelDB expert either. I think that at the size and usage pattern of the coinstatsindex we are not really facing any significant problems on a modern computer and system since the size of the data we are storing is very small relatively speaking for what LevelDB is capable of handling. But I think the impact is probably also non-zero and might increase over time as the blockchain grows. Very high-level rationale: there must be a reason LevelDB does these c
...
💬 fjahr commented on pull request "index: Force database compaction in coinstatsindex":
(https://github.com/bitcoin/bitcoin/pull/33306#issuecomment-3254623438)
FWIW, from the patterns I have observed so far it seems likely to me that the problem is only occurring during sync and afterwards the data of new blocks are actually compacted as it should, just those 55kb files are left over from the sync phase. So I am thinking about changing the approach to only compact once when the index is being set to `m_synced = true`, though this would make it a more complex change and I need to do some more testing if my anecdotal observation is correct.
(https://github.com/bitcoin/bitcoin/pull/33306#issuecomment-3254623438)
FWIW, from the patterns I have observed so far it seems likely to me that the problem is only occurring during sync and afterwards the data of new blocks are actually compacted as it should, just those 55kb files are left over from the sync phase. So I am thinking about changing the approach to only compact once when the index is being set to `m_synced = true`, though this would make it a more complex change and I need to do some more testing if my anecdotal observation is correct.
💬 maflcko commented on pull request "ci: Checkout latest merged pulls":
(https://github.com/bitcoin/bitcoin/pull/33303#discussion_r2322828365)
thx, all done
(https://github.com/bitcoin/bitcoin/pull/33303#discussion_r2322828365)
thx, all done
🤔 janb84 reviewed a pull request: "ci: Checkout latest merged pulls"
(https://github.com/bitcoin/bitcoin/pull/33303#pullrequestreview-3186383766)
ACK fab00ebbc74fe55c41b2d531a964cdb083b02ab0
changes sinds last ACK:
- changes related to use YAML KEYS / templating.
Kinda in doubt about the use of YAML_KEY_01, in code it makes it clear that it is a YAML key but in the action execution it is visible as a `env` variable :
<details>
``` YAML
• Run actions/checkout@v5 with:
ref: refs/pull/33303/merge repository: bitcoin/bitcoin token: ***
ssh-strict: true ssh-user: git
persist-credentials: true clean: true
sparse-checkout-cone
...
(https://github.com/bitcoin/bitcoin/pull/33303#pullrequestreview-3186383766)
ACK fab00ebbc74fe55c41b2d531a964cdb083b02ab0
changes sinds last ACK:
- changes related to use YAML KEYS / templating.
Kinda in doubt about the use of YAML_KEY_01, in code it makes it clear that it is a YAML key but in the action execution it is visible as a `env` variable :
<details>
``` YAML
• Run actions/checkout@v5 with:
ref: refs/pull/33303/merge repository: bitcoin/bitcoin token: ***
ssh-strict: true ssh-user: git
persist-credentials: true clean: true
sparse-checkout-cone
...
💬 sipa commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#issuecomment-3254821483)
I had missed a few sources of memory usage in the numbers posted in the PR description before. I've redone the derivation, and explained it in more detail. Thanks @glozow for trying to make sense of my incorrect numbers.
(https://github.com/bitcoin/bitcoin/pull/33157#issuecomment-3254821483)
I had missed a few sources of memory usage in the numbers posted in the PR description before. I've redone the derivation, and explained it in more detail. Thanks @glozow for trying to make sense of my incorrect numbers.
👍 hebasto approved a pull request: "ci: Checkout latest merged pulls"
(https://github.com/bitcoin/bitcoin/pull/33303#pullrequestreview-3186457692)
ACK fab00ebbc74fe55c41b2d531a964cdb083b02ab0.
This time, it took GitHub ~20min to update the reference.
(https://github.com/bitcoin/bitcoin/pull/33303#pullrequestreview-3186457692)
ACK fab00ebbc74fe55c41b2d531a964cdb083b02ab0.
This time, it took GitHub ~20min to update the reference.
💬 hebasto commented on pull request "ci, iwyu: Treat warnings as errors for specific directories":
(https://github.com/bitcoin/bitcoin/pull/31308#issuecomment-3254910524)
@ryanofsky
You may be interested in these changes.
(https://github.com/bitcoin/bitcoin/pull/31308#issuecomment-3254910524)
@ryanofsky
You may be interested in these changes.
💬 mzumsande commented on issue "natpmp: quiet down unconditional logging":
(https://github.com/bitcoin/bitcoin/issues/33301#issuecomment-3254930519)
> How about a exponential backoff, similar to how it's done in torcontrol.cpp?
That was just a random association, I don't know enough about PCP specifics to judge if it makes much sense.
Other questions would be:
- Does this situation deserve an unconditional warning at all instead of just logging to `net` - how actionable is the warning? (it seems to occur up for multiple people)
- Is it worth trying every 5 minutes forever instead of giving up after a number of tries? Maybe just log uncondi
...
(https://github.com/bitcoin/bitcoin/issues/33301#issuecomment-3254930519)
> How about a exponential backoff, similar to how it's done in torcontrol.cpp?
That was just a random association, I don't know enough about PCP specifics to judge if it makes much sense.
Other questions would be:
- Does this situation deserve an unconditional warning at all instead of just logging to `net` - how actionable is the warning? (it seems to occur up for multiple people)
- Is it worth trying every 5 minutes forever instead of giving up after a number of tries? Maybe just log uncondi
...
💬 fanquake commented on issue "natpmp: quiet down unconditional logging":
(https://github.com/bitcoin/bitcoin/issues/33301#issuecomment-3254938999)
cc @willcl-ark
(https://github.com/bitcoin/bitcoin/issues/33301#issuecomment-3254938999)
cc @willcl-ark
🤔 maflcko reviewed a pull request: "index: Fix coinstats overflow"
(https://github.com/bitcoin/bitcoin/pull/30469#pullrequestreview-3186533494)
> Hm, not clear to me why clang-tidy CI complains here about lines (or even files) I haven't touched in this PR. But that's what the last fixup commit I just pushed is for.
I haven't checked this, but the new struct is no longer trivially copyable, so it may have effects outside of the lines you have touched.
<details><summary>a diff</summary>
we should probably make it trivially copyable, but this seems unrelated.
```diff
diff --git a/src/arith_uint256.h b/src/arith_uint256.h
inde
...
(https://github.com/bitcoin/bitcoin/pull/30469#pullrequestreview-3186533494)
> Hm, not clear to me why clang-tidy CI complains here about lines (or even files) I haven't touched in this PR. But that's what the last fixup commit I just pushed is for.
I haven't checked this, but the new struct is no longer trivially copyable, so it may have effects outside of the lines you have touched.
<details><summary>a diff</summary>
we should probably make it trivially copyable, but this seems unrelated.
```diff
diff --git a/src/arith_uint256.h b/src/arith_uint256.h
inde
...
💬 maflcko commented on pull request "index: Fix coinstats overflow":
(https://github.com/bitcoin/bitcoin/pull/30469#discussion_r2323036958)
this looks like a false positive in clang-tidy and this looks like the wrong fix. `utxo_stats` is mutable, and taking a const reference does not make it immutable.
(In languages with a borrow checker, this wouldn't even compile, see https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=b8c78ddbe26b30473498b64a47ba2a6b)
The correct fix is to disable the clang-tidy rule in this line, or globally.
(https://github.com/bitcoin/bitcoin/pull/30469#discussion_r2323036958)
this looks like a false positive in clang-tidy and this looks like the wrong fix. `utxo_stats` is mutable, and taking a const reference does not make it immutable.
(In languages with a borrow checker, this wouldn't even compile, see https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=b8c78ddbe26b30473498b64a47ba2a6b)
The correct fix is to disable the clang-tidy rule in this line, or globally.
📝 glozow opened a pull request: "doc: archive v29.1 release notes"
(https://github.com/bitcoin/bitcoin/pull/33309)
(https://github.com/bitcoin/bitcoin/pull/33309)
💬 maflcko commented on pull request "ci: Checkout latest merged pulls":
(https://github.com/bitcoin/bitcoin/pull/33303#issuecomment-3254968070)
> Kinda in doubt about the use of YAML_KEY_01, in code it makes it clear that it is a YAML key but in the action execution it is visible as a `env` variable :
I know, but I don't know any alternative. See also the GitHub schema validator on a previous run: https://github.com/bitcoin/bitcoin/actions/runs/17471191651/workflow:
```
.github/workflows/ci.yml
Invalid workflow file
(Line: 21, Col: 1): Unexpected value 'checkout_ref_tmpl'
(https://github.com/bitcoin/bitcoin/pull/33303#issuecomment-3254968070)
> Kinda in doubt about the use of YAML_KEY_01, in code it makes it clear that it is a YAML key but in the action execution it is visible as a `env` variable :
I know, but I don't know any alternative. See also the GitHub schema validator on a previous run: https://github.com/bitcoin/bitcoin/actions/runs/17471191651/workflow:
```
.github/workflows/ci.yml
Invalid workflow file
(Line: 21, Col: 1): Unexpected value 'checkout_ref_tmpl'