💬 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
✅ maflcko closed an issue: "wallet: CPU use proportional to wallet transaction count when idle"
(https://github.com/bitcoin/bitcoin/issues/16815)
(https://github.com/bitcoin/bitcoin/issues/16815)
💬 maflcko commented on issue "wallet: CPU use proportional to wallet transaction count when idle":
(https://github.com/bitcoin/bitcoin/issues/16815#issuecomment-3430799802)
Closing for now, but a comment can be left below to have it reopened. Also, a new issue can be filed.
(https://github.com/bitcoin/bitcoin/issues/16815#issuecomment-3430799802)
Closing for now, but a comment can be left below to have it reopened. Also, a new issue can be filed.
✅ maflcko closed an issue: "Node stuck with repeated "Cache size exceeds total space" log message"
(https://github.com/bitcoin/bitcoin/issues/27599)
(https://github.com/bitcoin/bitcoin/issues/27599)
💬 maflcko commented on issue "Node stuck with repeated "Cache size exceeds total space" log message":
(https://github.com/bitcoin/bitcoin/issues/27599#issuecomment-3430802736)
Yeah, closing for now, but this can be re-opened when it happens again, or a new issue can be filed.
(https://github.com/bitcoin/bitcoin/issues/27599#issuecomment-3430802736)
Yeah, closing for now, but this can be re-opened when it happens again, or a new issue can be filed.
💬 rkrux commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#issuecomment-3430815886)
Opened a PR for removal of redundant sighash calculation here: #33665.
(https://github.com/bitcoin/bitcoin/pull/29675#issuecomment-3430815886)
Opened a PR for removal of redundant sighash calculation here: #33665.
💬 Raimo33 commented on pull request "refactor: optimize: reuse containers in transaction policy verification":
(https://github.com/bitcoin/bitcoin/pull/33645#issuecomment-3430818448)
> Not so sure about the `Assemble Block` bench though. Doesn't it measure block assembly **_after_** admission, therefore not timing the policy checks you changed?
Thanks for the review & benchmarks. I'm positive about the fact that the `AssembleBlock` bench calls some of the impacted functions. I've not delved into details, and I think it's irrelevant considering performance is the same.
(https://github.com/bitcoin/bitcoin/pull/33645#issuecomment-3430818448)
> Not so sure about the `Assemble Block` bench though. Doesn't it measure block assembly **_after_** admission, therefore not timing the policy checks you changed?
Thanks for the review & benchmarks. I'm positive about the fact that the `AssembleBlock` bench calls some of the impacted functions. I've not delved into details, and I think it's irrelevant considering performance is the same.
💬 Raimo33 commented on pull request "refactor: optimize: reuse containers in transaction policy verification":
(https://github.com/bitcoin/bitcoin/pull/33645#discussion_r2450692465)
yes, it is deliberate to prevent future programming errors.
(https://github.com/bitcoin/bitcoin/pull/33645#discussion_r2450692465)
yes, it is deliberate to prevent future programming errors.
🤔 mzumsande reviewed a pull request: "test: Use unassigned p2p_port instead of hardcoded 60000 in p2p_i2p_ports.py"
(https://github.com/bitcoin/bitcoin/pull/33670#pullrequestreview-3364100215)
ACK fa20275db32c5b9b0fe35effe2d1cf3d958e7310
LGTM, I also took a look recently trying to find the root cause but didn't come up with a better idea than just changing the port and hoping for the best.
(https://github.com/bitcoin/bitcoin/pull/33670#pullrequestreview-3364100215)
ACK fa20275db32c5b9b0fe35effe2d1cf3d958e7310
LGTM, I also took a look recently trying to find the root cause but didn't come up with a better idea than just changing the port and hoping for the best.