💬 pinheadmz commented on pull request "test: fix intermittent issue in `feature_bip68_sequence`":
(https://github.com/bitcoin/bitcoin/pull/27177#discussion_r1154561718)
Two little questions about this:
- Why does this solve the immature coinbase spending?
- Does it make sense to just squash into the previous line? i.e. `COINBASE_MATURITY + 35` if those 35 blocks even needed anymore
(https://github.com/bitcoin/bitcoin/pull/27177#discussion_r1154561718)
Two little questions about this:
- Why does this solve the immature coinbase spending?
- Does it make sense to just squash into the previous line? i.e. `COINBASE_MATURITY + 35` if those 35 blocks even needed anymore
💬 pinheadmz commented on pull request "test: fix intermittent issue in `feature_bip68_sequence`":
(https://github.com/bitcoin/bitcoin/pull/27177#discussion_r1154562830)
It's always nice to have options - but since none of the tests actually need to set this to `False` anymore, does it make sense to remove the argument and just filter out immature coins all the time no matter what anyway?
(https://github.com/bitcoin/bitcoin/pull/27177#discussion_r1154562830)
It's always nice to have options - but since none of the tests actually need to set this to `False` anymore, does it make sense to remove the argument and just filter out immature coins all the time no matter what anyway?
💬 theStack commented on pull request "test: fix intermittent issue in `feature_bip68_sequence`":
(https://github.com/bitcoin/bitcoin/pull/27177#discussion_r1154547880)
fell-free-to-ignore tiny-nit: could just do it in one call
```suggestion
self.generate(self.wallet, 35 + COINBASE_MATURITY)
```
(https://github.com/bitcoin/bitcoin/pull/27177#discussion_r1154547880)
fell-free-to-ignore tiny-nit: could just do it in one call
```suggestion
self.generate(self.wallet, 35 + COINBASE_MATURITY)
```
👍 theStack approved a pull request: "test: fix intermittent issue in `feature_bip68_sequence`"
(https://github.com/bitcoin/bitcoin/pull/27177)
ACK f8ebcb823fc5f39adc87ea6cf079f3e72ee44f30
(Didn't run interface_usdt_mempool.py locally, as I'm currently not having a build with tracepoints available, but the changes look reasonable.)
(https://github.com/bitcoin/bitcoin/pull/27177)
ACK f8ebcb823fc5f39adc87ea6cf079f3e72ee44f30
(Didn't run interface_usdt_mempool.py locally, as I'm currently not having a build with tracepoints available, but the changes look reasonable.)
💬 hebasto commented on pull request "refactor (tidy): Fixes after enable-debug configure":
(https://github.com/bitcoin/bitcoin/pull/27353#issuecomment-1492035512)
> I'll undraft this then, even if I still don't understand why `clang-tidy` seems to catch more with debug enabled.
I suspect the `-DBOOST_MULTI_INDEX_ENABLE_SAFE_MODE` definition.
(https://github.com/bitcoin/bitcoin/pull/27353#issuecomment-1492035512)
> I'll undraft this then, even if I still don't understand why `clang-tidy` seems to catch more with debug enabled.
I suspect the `-DBOOST_MULTI_INDEX_ENABLE_SAFE_MODE` definition.
💬 hebasto commented on pull request "refactor (tidy): Fixes after enable-debug configure":
(https://github.com/bitcoin/bitcoin/pull/27353#issuecomment-1492042782)
> > > Besides, I'm also not sold that this lint is an actual improvement.
> >
> >
> > I think this PR diff _is_ an improvement.
>
> Could you explain why it's an improvement? It's unclear to me why const references to iterators would be an improvement, I was under the impression that iters by value were fine.
Well, my main point was enforcing const-corectness, but I _missed_ the fact that `CTxMemPool::txiter` _is_ a `const_iterator`.
Making a constness at the call site obvious for
...
(https://github.com/bitcoin/bitcoin/pull/27353#issuecomment-1492042782)
> > > Besides, I'm also not sold that this lint is an actual improvement.
> >
> >
> > I think this PR diff _is_ an improvement.
>
> Could you explain why it's an improvement? It's unclear to me why const references to iterators would be an improvement, I was under the impression that iters by value were fine.
Well, my main point was enforcing const-corectness, but I _missed_ the fact that `CTxMemPool::txiter` _is_ a `const_iterator`.
Making a constness at the call site obvious for
...
💬 stickies-v commented on pull request "miniscript: explicit cast instead of comparing integers of different signs":
(https://github.com/bitcoin/bitcoin/pull/27382#discussion_r1154600109)
Maybe best to avoid C-style casts?
```suggestion
return static_cast<uint32_t>(std::count(subs.begin(), subs.end(), true)) >= node.k;
```
(https://github.com/bitcoin/bitcoin/pull/27382#discussion_r1154600109)
Maybe best to avoid C-style casts?
```suggestion
return static_cast<uint32_t>(std::count(subs.begin(), subs.end(), true)) >= node.k;
```
✅ Doodoobrown23 closed an issue: "Hey"
(https://github.com/bitcoin/bitcoin/issues/27384)
(https://github.com/bitcoin/bitcoin/issues/27384)
💬 stickies-v commented on pull request "test: Remove python3.5 workaround":
(https://github.com/bitcoin/bitcoin/pull/27378#issuecomment-1492152185)
This PR seems to remove 2 workarounds: https://bugs.python.org/issue3566 and https://bugs.python.org/issue33450. 3566 is marked as resolved (and indeed for Python 3.5) so I think that can be safely removed. 33450 however is still open and also affects Python 3.6. In the description, I can't find in which macOS version this stopped being an issue. Do you have any sources that confirm we can indeed remove this safely?
(https://github.com/bitcoin/bitcoin/pull/27378#issuecomment-1492152185)
This PR seems to remove 2 workarounds: https://bugs.python.org/issue3566 and https://bugs.python.org/issue33450. 3566 is marked as resolved (and indeed for Python 3.5) so I think that can be safely removed. 33450 however is still open and also affects Python 3.6. In the description, I can't find in which macOS version this stopped being an issue. Do you have any sources that confirm we can indeed remove this safely?
💬 darosior commented on pull request "miniscript: explicit cast instead of comparing integers of different signs":
(https://github.com/bitcoin/bitcoin/pull/27382#discussion_r1154626692)
Sure, done. (I don't think it matters in this precise case though.)
(https://github.com/bitcoin/bitcoin/pull/27382#discussion_r1154626692)
Sure, done. (I don't think it matters in this precise case though.)
:lock: fanquake locked an issue: "Hey"
(https://github.com/bitcoin/bitcoin/issues/27384)
(https://github.com/bitcoin/bitcoin/issues/27384)
💬 fanquake commented on issue "Feature request: alert PR author in case of CI failure":
(https://github.com/bitcoin/bitcoin/issues/27178#issuecomment-1492182408)
> Can I also get check runs?
Should be available now.
(https://github.com/bitcoin/bitcoin/issues/27178#issuecomment-1492182408)
> Can I also get check runs?
Should be available now.
💬 fanquake commented on pull request "test: Remove python3.5 workaround":
(https://github.com/bitcoin/bitcoin/pull/27378#issuecomment-1492183698)
> however is still open and also affects Python 3.6.
Our minimum required Python is 3.7.
(https://github.com/bitcoin/bitcoin/pull/27378#issuecomment-1492183698)
> however is still open and also affects Python 3.6.
Our minimum required Python is 3.7.
🚀 fanquake merged a pull request: "refactor: remove unused param from legacy pubkey interface"
(https://github.com/bitcoin/bitcoin/pull/27274)
(https://github.com/bitcoin/bitcoin/pull/27274)
💬 jonatack commented on pull request "Fix logging RPC and -debugexclude with 0/none values, add test coverage, improve docs":
(https://github.com/bitcoin/bitcoin/pull/27231#issuecomment-1492195680)
Rebased.
> could take advantage of that, to remove some of the complexity here; do we definitely need multiple different ways of achieving the same logging toggling?
Exploring this, it doesn't look like there would be much code simplification gained by dropping `none` for `0` only, which would no longer be in symmetry with `all/1` that have been operational for a long time. A couple lines could be saved by removing the `-debugexclude` config option, but that option is practical to have, a
...
(https://github.com/bitcoin/bitcoin/pull/27231#issuecomment-1492195680)
Rebased.
> could take advantage of that, to remove some of the complexity here; do we definitely need multiple different ways of achieving the same logging toggling?
Exploring this, it doesn't look like there would be much code simplification gained by dropping `none` for `0` only, which would no longer be in symmetry with `all/1` that have been operational for a long time. A couple lines could be saved by removing the `-debugexclude` config option, but that option is practical to have, a
...
👋 jonatack's pull request is ready for review: "Fix logging RPC and -debugexclude with 0/none values, add test coverage, improve docs"
(https://github.com/bitcoin/bitcoin/pull/27231)
(https://github.com/bitcoin/bitcoin/pull/27231)
💬 jnewbery commented on pull request "net processing: #26140 follow-ups":
(https://github.com/bitcoin/bitcoin/pull/27379#issuecomment-1492199212)
utACK 3fa4c54ac54b2d738e0c43b57b5c232ee02fe3b3
(https://github.com/bitcoin/bitcoin/pull/27379#issuecomment-1492199212)
utACK 3fa4c54ac54b2d738e0c43b57b5c232ee02fe3b3
💬 Sjors commented on issue "Selecting two coins will abort with "The amount exceeds your balance."":
(https://github.com/bitcoin-core/gui/issues/688#issuecomment-1492207012)
Just to clarify: IIUC this was "caused" by https://github.com/bitcoin/bitcoin/pull/25685 which is only in master. We assuming we fix this before v25.0 there's nothing to backport.
(https://github.com/bitcoin-core/gui/issues/688#issuecomment-1492207012)
Just to clarify: IIUC this was "caused" by https://github.com/bitcoin/bitcoin/pull/25685 which is only in master. We assuming we fix this before v25.0 there's nothing to backport.
👍 stickies-v approved a pull request: "miniscript: explicit cast instead of comparing integers of different signs"
(https://github.com/bitcoin/bitcoin/pull/27382)
ACK 9a54d88c8cb0c5d529f388c2ce53008e1ff126dd
(https://github.com/bitcoin/bitcoin/pull/27382)
ACK 9a54d88c8cb0c5d529f388c2ce53008e1ff126dd