💬 ajtowns commented on pull request "[wip] wallet: Add separate balance info for non-mempool wallet txs":
(https://github.com/bitcoin/bitcoin/pull/33671#issuecomment-3427837874)
Relevant for #29415 -- private relay of our wallet transactions would create spends that weren't in the mempool and would thus disappear mysteriously from the wallet balance which seems unacceptable. I think I've seen similar behaviour when doing lots of consolidations of my signet wallet, which has also been disturbing.
Perhaps see #11020 for a similar previous attempt.
This PR currently doesn't include tests that this works sensibly for nonzero nonmempool balances, hence wip/draft.
(https://github.com/bitcoin/bitcoin/pull/33671#issuecomment-3427837874)
Relevant for #29415 -- private relay of our wallet transactions would create spends that weren't in the mempool and would thus disappear mysteriously from the wallet balance which seems unacceptable. I think I've seen similar behaviour when doing lots of consolidations of my signet wallet, which has also been disturbing.
Perhaps see #11020 for a similar previous attempt.
This PR currently doesn't include tests that this works sensibly for nonzero nonmempool balances, hence wip/draft.
💬 hebasto commented on pull request "CMake: Add dynamic test discovery":
(https://github.com/bitcoin/bitcoin/pull/33483#issuecomment-3428571662)
Concept ACK.
(https://github.com/bitcoin/bitcoin/pull/33483#issuecomment-3428571662)
Concept ACK.
💬 hebasto commented on pull request "CMake: Add dynamic test discovery":
(https://github.com/bitcoin/bitcoin/pull/33483#discussion_r2449351716)
Why is this needed?
(https://github.com/bitcoin/bitcoin/pull/33483#discussion_r2449351716)
Why is this needed?
📝 ilkinsufi opened a pull request: "qt: Increase tooltip wrap threshold from 80 to 100 characters"
(https://github.com/bitcoin/bitcoin/pull/33672)
## Summary
This PR increases the tooltip wrap threshold from 80 to 100 characters to improve user experience.
## Changes
- Modified `TOOLTIP_WRAP_THRESHOLD` constant in `src/qt/guiconstants.h`
- Changed value from 80 to 100 characters
## Rationale
The previous threshold of 80 characters was too restrictive for modern displays and longer descriptive tooltips. This change allows more content to be displayed before tooltips are converted to rich text format.
## Testing
- No linter err
...
(https://github.com/bitcoin/bitcoin/pull/33672)
## Summary
This PR increases the tooltip wrap threshold from 80 to 100 characters to improve user experience.
## Changes
- Modified `TOOLTIP_WRAP_THRESHOLD` constant in `src/qt/guiconstants.h`
- Changed value from 80 to 100 characters
## Rationale
The previous threshold of 80 characters was too restrictive for modern displays and longer descriptive tooltips. This change allows more content to be displayed before tooltips are converted to rich text format.
## Testing
- No linter err
...
💬 hebasto commented on pull request "qt: Increase tooltip wrap threshold from 80 to 100 characters":
(https://github.com/bitcoin/bitcoin/pull/33672#issuecomment-3428797618)
Please move this PR to the GUI repo: https://github.com/bitcoin-core/gui/pulls.
(https://github.com/bitcoin/bitcoin/pull/33672#issuecomment-3428797618)
Please move this PR to the GUI repo: https://github.com/bitcoin-core/gui/pulls.
✅ ilkinsufi closed a pull request: "qt: Increase tooltip wrap threshold from 80 to 100 characters"
(https://github.com/bitcoin/bitcoin/pull/33672)
(https://github.com/bitcoin/bitcoin/pull/33672)
📝 ilkinsufi opened a pull request: "qt: Increase tooltip wrap threshold from 80 to 100 characters"
(https://github.com/bitcoin-core/gui/pull/905)
## Summary
This PR increases the tooltip wrap threshold from 80 to 100 characters to improve user experience.
## Changes
- Modified `TOOLTIP_WRAP_THRESHOLD` constant in `src/qt/guiconstants.h`
- Changed value from 80 to 100 characters
## Rationale
The previous threshold of 80 characters was too restrictive for modern displays and longer descriptive tooltips. This change allows more content to be displayed before tooltips are converted to rich text format.
## Testing
- No linter err
...
(https://github.com/bitcoin-core/gui/pull/905)
## Summary
This PR increases the tooltip wrap threshold from 80 to 100 characters to improve user experience.
## Changes
- Modified `TOOLTIP_WRAP_THRESHOLD` constant in `src/qt/guiconstants.h`
- Changed value from 80 to 100 characters
## Rationale
The previous threshold of 80 characters was too restrictive for modern displays and longer descriptive tooltips. This change allows more content to be displayed before tooltips are converted to rich text format.
## Testing
- No linter err
...
🤔 hebasto reviewed a pull request: "CMake: Add dynamic test discovery"
(https://github.com/bitcoin/bitcoin/pull/33483#pullrequestreview-3362343633)
The following comments no longer seem needed:https://github.com/bitcoin/bitcoin/blob/2bedad455934c9b407329d0bf58521cf24510465/src/test/CMakeLists.txt#L7-L8 https://github.com/bitcoin/bitcoin/blob/2bedad455934c9b407329d0bf58521cf24510465/src/wallet/test/CMakeLists.txt#L5-L6
(https://github.com/bitcoin/bitcoin/pull/33483#pullrequestreview-3362343633)
The following comments no longer seem needed:https://github.com/bitcoin/bitcoin/blob/2bedad455934c9b407329d0bf58521cf24510465/src/test/CMakeLists.txt#L7-L8 https://github.com/bitcoin/bitcoin/blob/2bedad455934c9b407329d0bf58521cf24510465/src/wallet/test/CMakeLists.txt#L5-L6
💬 trevarj commented on pull request "guix: Use UCRT runtime for Windows release binaries":
(https://github.com/bitcoin/bitcoin/pull/33593#issuecomment-3429226876)
I could submit an upstream Guix patch for this. It will help with build times since the package with ucrt will be in the main substitute servers.
(https://github.com/bitcoin/bitcoin/pull/33593#issuecomment-3429226876)
I could submit an upstream Guix patch for this. It will help with build times since the package with ucrt will be in the main substitute servers.
💬 romanz commented on pull request "rest: allow reading partial block data from storage":
(https://github.com/bitcoin/bitcoin/pull/33657#discussion_r2449615816)
Added in [c9bfd298be..301116e855](https://github.com/bitcoin/bitcoin/compare/c9bfd298be422de7e989fe244fb4281c507068a3..301116e85540e49c27473f450fc210b702db4cf5).
(https://github.com/bitcoin/bitcoin/pull/33657#discussion_r2449615816)
Added in [c9bfd298be..301116e855](https://github.com/bitcoin/bitcoin/compare/c9bfd298be422de7e989fe244fb4281c507068a3..301116e85540e49c27473f450fc210b702db4cf5).
💬 trevarj commented on pull request "guix: Use UCRT runtime for Windows release binaries":
(https://github.com/bitcoin/bitcoin/pull/33593#issuecomment-3429494296)
https://codeberg.org/guix/guix/pulls/3727
(https://github.com/bitcoin/bitcoin/pull/33593#issuecomment-3429494296)
https://codeberg.org/guix/guix/pulls/3727
🤔 cedwies reviewed a pull request: "refactor: optimize: reuse containers in transaction policy verification"
(https://github.com/bitcoin/bitcoin/pull/33645#pullrequestreview-3362906716)
Concept ACK
- reusing vector capacity across loop iterations is makes sense
- no functional or policy logic changes introduced
- Code refactoring (range-based loops, variable renames) improves readability
Here is my bench on MacOS M2 MAX:
**Before:**
| ns/op | op/s | err% | total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
| 134,805.18 | 7,418.11 | 0.7% | 3.30 | `AssembleBlock`
|
...
(https://github.com/bitcoin/bitcoin/pull/33645#pullrequestreview-3362906716)
Concept ACK
- reusing vector capacity across loop iterations is makes sense
- no functional or policy logic changes introduced
- Code refactoring (range-based loops, variable renames) improves readability
Here is my bench on MacOS M2 MAX:
**Before:**
| ns/op | op/s | err% | total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
| 134,805.18 | 7,418.11 | 0.7% | 3.30 | `AssembleBlock`
|
...
💬 cedwies commented on pull request "refactor: optimize: reuse containers in transaction policy verification":
(https://github.com/bitcoin/bitcoin/pull/33645#discussion_r2449841418)
Just double checking: `vSolutions.clear();` is intentional despite `Solver(...)` already clearing `vSolutions`? For me, I think it's good to keep.
(https://github.com/bitcoin/bitcoin/pull/33645#discussion_r2449841418)
Just double checking: `vSolutions.clear();` is intentional despite `Solver(...)` already clearing `vSolutions`? For me, I think it's good to keep.
⚠️ carlyluminattinetprise-code opened an issue: "Bitcoin"
(https://github.com/bitcoin/bitcoin/issues/33673)
### Please describe the feature you'd like to see added.
Bitcoin Core
### Is your feature related to a problem, if so please describe it.
_No response_
### Describe the solution you'd like
_No response_
### Describe any alternatives you've considered
_No response_
### Please leave any additional context
_No response_
(https://github.com/bitcoin/bitcoin/issues/33673)
### Please describe the feature you'd like to see added.
Bitcoin Core
### Is your feature related to a problem, if so please describe it.
_No response_
### Describe the solution you'd like
_No response_
### Describe any alternatives you've considered
_No response_
### Please leave any additional context
_No response_
✅ pinheadmz closed an issue: "Bitcoin"
(https://github.com/bitcoin/bitcoin/issues/33673)
(https://github.com/bitcoin/bitcoin/issues/33673)
💬 instagibbs commented on pull request "p2p: Drop unsolicited CMPCTBLOCK from non-HB peer":
(https://github.com/bitcoin/bitcoin/pull/32606#issuecomment-3430679733)
concept ACK
(https://github.com/bitcoin/bitcoin/pull/32606#issuecomment-3430679733)
concept ACK
🤔 maflcko reviewed a pull request: "ci: add Valgrind fuzz"
(https://github.com/bitcoin/bitcoin/pull/33461#pullrequestreview-3363885023)
lgtm
Conceptually, the same feedback on valgrind applies here (https://github.com/bitcoin/bitcoin/pull/33411#issuecomment-3410475429), but if an issue arises, it should be fine to quickly drop this task again to unblock the CI caused by a false-positive.
(https://github.com/bitcoin/bitcoin/pull/33461#pullrequestreview-3363885023)
lgtm
Conceptually, the same feedback on valgrind applies here (https://github.com/bitcoin/bitcoin/pull/33411#issuecomment-3410475429), but if an issue arises, it should be fine to quickly drop this task again to unblock the CI caused by a false-positive.
💬 maflcko commented on pull request "ci: add Valgrind fuzz":
(https://github.com/bitcoin/bitcoin/pull/33461#discussion_r2450576229)
```suggestion
cirrus-runner: 'ghcr.io/cirruslabs/ubuntu-runner-amd64:24.04-md'
```
Looks like the runtime with lg was 50 minutes. I wonder if it will be the same speed, using md, as there are only three trailing, slow fuzz tests.
(https://github.com/bitcoin/bitcoin/pull/33461#discussion_r2450576229)
```suggestion
cirrus-runner: 'ghcr.io/cirruslabs/ubuntu-runner-amd64:24.04-md'
```
Looks like the runtime with lg was 50 minutes. I wonder if it will be the same speed, using md, as there are only three trailing, slow fuzz tests.
💬 laanwj commented on pull request "test: Use unassigned p2p_port instead of hardcoded 60000 in p2p_i2p_ports.py":
(https://github.com/bitcoin/bitcoin/pull/33670#issuecomment-3430717688)
Code review ACK fa20275db32c5b9b0fe35effe2d1cf3d958e7310
(https://github.com/bitcoin/bitcoin/pull/33670#issuecomment-3430717688)
Code review ACK fa20275db32c5b9b0fe35effe2d1cf3d958e7310
📝 maflcko opened a pull request: "ci: Doc ASLR workaround for sanitizer tasks"
(https://github.com/bitcoin/bitcoin/pull/33674)
Fixes https://github.com/bitcoin/bitcoin/issues/30674
(https://github.com/bitcoin/bitcoin/pull/33674)
Fixes https://github.com/bitcoin/bitcoin/issues/30674