💬 glozow commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#issuecomment-2288577940)
> > to you, but there is a small benefit of going with the fuzz tests first (if possible)
>
> It looks like the tests here mostly operate on the split out modules after the refactoring, as opposed to a implementation agnostic test through the p2p interface (which would be a giant pita to write).
Indeed, all of these tests require the refactors (on master we'd have to create a chain to fire validation events + construct actual transactions with each type of invalidity).
> So splitting th
...
(https://github.com/bitcoin/bitcoin/pull/30110#issuecomment-2288577940)
> > to you, but there is a small benefit of going with the fuzz tests first (if possible)
>
> It looks like the tests here mostly operate on the split out modules after the refactoring, as opposed to a implementation agnostic test through the p2p interface (which would be a giant pita to write).
Indeed, all of these tests require the refactors (on master we'd have to create a chain to fire validation events + construct actual transactions with each type of invalidity).
> So splitting th
...
💬 maflcko commented on pull request "refactor: Replace ParseHex with consteval ArrayFromHex":
(https://github.com/bitcoin/bitcoin/pull/30377#discussion_r1716833566)
What is your macOS and XCode version? IIRC Xcode 14 is no longer supported, see also https://github.com/bitcoin/bitcoin/pull/29934, but I think this isn't documented, so probably up for debate.
The CI passes, because it is using Xcode 15, see d742be3d3f5d5063d7160f72422bce2fec953f38
(https://github.com/bitcoin/bitcoin/pull/30377#discussion_r1716833566)
What is your macOS and XCode version? IIRC Xcode 14 is no longer supported, see also https://github.com/bitcoin/bitcoin/pull/29934, but I think this isn't documented, so probably up for debate.
The CI passes, because it is using Xcode 15, see d742be3d3f5d5063d7160f72422bce2fec953f38
💬 paplorinc commented on pull request "refactor: Replace ParseHex with consteval ArrayFromHex":
(https://github.com/bitcoin/bitcoin/pull/30377#discussion_r1716839513)
It was working for me with:
```
% clang --version
Homebrew clang version 18.1.8
Target: arm64-apple-darwin23.5.0
Thread model: posix
InstalledDir: /opt/homebrew/opt/llvm/bin
% xcodebuild -version
Xcode 15.4
Build version 15F31d
```
(https://github.com/bitcoin/bitcoin/pull/30377#discussion_r1716839513)
It was working for me with:
```
% clang --version
Homebrew clang version 18.1.8
Target: arm64-apple-darwin23.5.0
Thread model: posix
InstalledDir: /opt/homebrew/opt/llvm/bin
% xcodebuild -version
Xcode 15.4
Build version 15F31d
```
👍 vasild approved a pull request: "log: expand BCLog::LogFlags (categories) to 64 bits"
(https://github.com/bitcoin/bitcoin/pull/26619#pullrequestreview-2238035999)
ACK b31a0cd0378184b2b9eb8f4bd3120cbd32c62005
(https://github.com/bitcoin/bitcoin/pull/26619#pullrequestreview-2238035999)
ACK b31a0cd0378184b2b9eb8f4bd3120cbd32c62005
💬 maflcko commented on pull request "fuzz: Faster utxo_snapshot fuzz target":
(https://github.com/bitcoin/bitcoin/pull/30644#issuecomment-2288632385)
Thanks for the review! Obviously this is fine to merge, but I am still unhappy about the average performance increase, which isn't 100x, but closer to the worst case performance increase (1.35x). I'll push once more, sorry.
(https://github.com/bitcoin/bitcoin/pull/30644#issuecomment-2288632385)
Thanks for the review! Obviously this is fine to merge, but I am still unhappy about the average performance increase, which isn't 100x, but closer to the worst case performance increase (1.35x). I'll push once more, sorry.
🤔 BrandonOdiwuor reviewed a pull request: "test: assumeutxo: check that UTXO-querying RPCs operate on snapshot chainstate"
(https://github.com/bitcoin/bitcoin/pull/30636#pullrequestreview-2238068310)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/30636#pullrequestreview-2238068310)
Concept ACK
💬 marcofleon commented on pull request "fuzz: Faster utxo_snapshot fuzz target":
(https://github.com/bitcoin/bitcoin/pull/30644#issuecomment-2288658870)
Sounds good, I'll review again when it's improved then. Looked to be about 2x when I just tested it.
(https://github.com/bitcoin/bitcoin/pull/30644#issuecomment-2288658870)
Sounds good, I'll review again when it's improved then. Looked to be about 2x when I just tested it.
🤔 furszy reviewed a pull request: "Migrate legacy wallets that are not loaded"
(https://github.com/bitcoin-core/gui/pull/824#pullrequestreview-2238083184)
ACK 8f2522d
(https://github.com/bitcoin-core/gui/pull/824#pullrequestreview-2238083184)
ACK 8f2522d
💬 epiccurious commented on pull request "feat(build): improve dependency error reporting in autogen.sh":
(https://github.com/bitcoin/bitcoin/pull/30646#issuecomment-2288687022)
Thanks for reviewing. I'll close the PR as won't do.
(https://github.com/bitcoin/bitcoin/pull/30646#issuecomment-2288687022)
Thanks for reviewing. I'll close the PR as won't do.
✅ epiccurious closed a pull request: "feat(build): improve dependency error reporting in autogen.sh"
(https://github.com/bitcoin/bitcoin/pull/30646)
(https://github.com/bitcoin/bitcoin/pull/30646)
✅ maflcko closed an issue: "contrib: Automation for Bitcoin Full Node Deployment"
(https://github.com/bitcoin/bitcoin/issues/30638)
(https://github.com/bitcoin/bitcoin/issues/30638)
💬 maflcko commented on issue "contrib: Automation for Bitcoin Full Node Deployment":
(https://github.com/bitcoin/bitcoin/issues/30638#issuecomment-2288699195)
@willcl-ark Maybe leave a new comment in the packaging issue 55 about your dockerfile?
I agree closing this for now. It may be best to continue discussion in https://github.com/bitcoin-core/packaging/issues/55 (yes it is closed, but discussion can continue).
And given that the discussion here seems about a *user* dockerfile, not a *dev* dockerfile, the packaging repo seems more appropriate.
(https://github.com/bitcoin/bitcoin/issues/30638#issuecomment-2288699195)
@willcl-ark Maybe leave a new comment in the packaging issue 55 about your dockerfile?
I agree closing this for now. It may be best to continue discussion in https://github.com/bitcoin-core/packaging/issues/55 (yes it is closed, but discussion can continue).
And given that the discussion here seems about a *user* dockerfile, not a *dev* dockerfile, the packaging repo seems more appropriate.
🚀 hebasto merged a pull request: "Migrate legacy wallets that are not loaded"
(https://github.com/bitcoin-core/gui/pull/824)
(https://github.com/bitcoin-core/gui/pull/824)
💬 stickies-v commented on pull request "refactor: Replace ParseHex with consteval ArrayFromHex":
(https://github.com/bitcoin/bitcoin/pull/30377#discussion_r1716909297)
> What is your macOS and XCode version? IIRC Xcode 14 is no longer supported, see also https://github.com/bitcoin/bitcoin/pull/29934, but I think this isn't documented, nor tested, so probably up for debate.
Thanks, I'm on macOS 14.3.1 and just bumped XCode from 14.3.1.0.1.1683849156 to 15.3.0.0.1.1708646388 which resolves the issue 👍
(https://github.com/bitcoin/bitcoin/pull/30377#discussion_r1716909297)
> What is your macOS and XCode version? IIRC Xcode 14 is no longer supported, see also https://github.com/bitcoin/bitcoin/pull/29934, but I think this isn't documented, nor tested, so probably up for debate.
Thanks, I'm on macOS 14.3.1 and just bumped XCode from 14.3.1.0.1.1683849156 to 15.3.0.0.1.1708646388 which resolves the issue 👍
💬 vasild commented on pull request "Halt processing of unrequested transactions v2":
(https://github.com/bitcoin/bitcoin/pull/30572#issuecomment-2288719391)
> @vasild you appear to have disregarded my above message about this change is desirable. Was it unclear?
You mean this one: "At times there have been parties that mass connect..."? If that would be the driving force behind this, then maybe it would be better to put it in the PR description.
So this is not about CPU DoS attack by slow-invalid transactions, but about somebody doing something stupid and using the Bitcoin network in an ineffective way, sending 500 bytes instead of 30? Requiri
...
(https://github.com/bitcoin/bitcoin/pull/30572#issuecomment-2288719391)
> @vasild you appear to have disregarded my above message about this change is desirable. Was it unclear?
You mean this one: "At times there have been parties that mass connect..."? If that would be the driving force behind this, then maybe it would be better to put it in the PR description.
So this is not about CPU DoS attack by slow-invalid transactions, but about somebody doing something stupid and using the Bitcoin network in an ineffective way, sending 500 bytes instead of 30? Requiri
...
💬 hebasto commented on pull request "Update translation source file for v28.0 string freeze":
(https://github.com/bitcoin-core/gui/pull/833#issuecomment-2288738565)
@pablomartin4btc @stickies-v
I had to update this PR due to the recently merged bitcoin-core/gui#824.
Sorry for invalidating your ACKs. Mind re-reviewing please?
(https://github.com/bitcoin-core/gui/pull/833#issuecomment-2288738565)
@pablomartin4btc @stickies-v
I had to update this PR due to the recently merged bitcoin-core/gui#824.
Sorry for invalidating your ACKs. Mind re-reviewing please?
💬 stickies-v commented on pull request "Update translation source file for v28.0 string freeze":
(https://github.com/bitcoin-core/gui/pull/833#issuecomment-2288796044)
re-ACK 770b0348c0abe2f63b56cc78c7b7a0e6b4057ce1
(https://github.com/bitcoin-core/gui/pull/833#issuecomment-2288796044)
re-ACK 770b0348c0abe2f63b56cc78c7b7a0e6b4057ce1
💬 maflcko commented on pull request "refactor: Replace ParseHex with consteval ArrayFromHex":
(https://github.com/bitcoin/bitcoin/pull/30377#discussion_r1716956530)
So I guess this commit drops support for XCode on macOS Ventura 13. Not sure if `doc/build-osx.md` needs to be adjusted, similar to https://github.com/bitcoin/bitcoin/pull/29934/files
(https://github.com/bitcoin/bitcoin/pull/30377#discussion_r1716956530)
So I guess this commit drops support for XCode on macOS Ventura 13. Not sure if `doc/build-osx.md` needs to be adjusted, similar to https://github.com/bitcoin/bitcoin/pull/29934/files
👍 pablomartin4btc approved a pull request: "Update translation source file for v28.0 string freeze"
(https://github.com/bitcoin-core/gui/pull/833#pullrequestreview-2238247544)
re-ACK 770b0348c0abe2f63b56cc78c7b7a0e6b4057ce1
no diffs.
(https://github.com/bitcoin-core/gui/pull/833#pullrequestreview-2238247544)
re-ACK 770b0348c0abe2f63b56cc78c7b7a0e6b4057ce1
no diffs.
💬 instagibbs commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#issuecomment-2288892100)
personally speaking I'm unsure what is gained by moving the fuzz tests to their own PR since they depend on the refactors. I don't think we want to merge the other commits unless the fuzz harnesses are attached to it. I'm continuing to review either way.
(https://github.com/bitcoin/bitcoin/pull/30110#issuecomment-2288892100)
personally speaking I'm unsure what is gained by moving the fuzz tests to their own PR since they depend on the refactors. I don't think we want to merge the other commits unless the fuzz harnesses are attached to it. I'm continuing to review either way.