💬 TheCharlatan commented on pull request "test: Add bitcoin-chainstate test for assumeutxo functionality":
(https://github.com/bitcoin/bitcoin/pull/33728#discussion_r2472836534)
It is weaker, yes, but we similarly guard in other situations. We also still have the assertion in ActivateSnapshot to guard this condition at a more critical point in time.
(https://github.com/bitcoin/bitcoin/pull/33728#discussion_r2472836534)
It is weaker, yes, but we similarly guard in other situations. We also still have the assertion in ActivateSnapshot to guard this condition at a more critical point in time.
💬 TheCharlatan commented on pull request "test: Add bitcoin-chainstate test for assumeutxo functionality":
(https://github.com/bitcoin/bitcoin/pull/33728#discussion_r2472614198)
Since this is an optional positional argument, I think I would drop the `-` prefix.
(https://github.com/bitcoin/bitcoin/pull/33728#discussion_r2472614198)
Since this is an optional positional argument, I think I would drop the `-` prefix.
✅ maflcko closed a pull request: "wallet: Improve wallet creation error message with usage hint"
(https://github.com/bitcoin/bitcoin/pull/33124)
(https://github.com/bitcoin/bitcoin/pull/33124)
💬 maflcko commented on pull request "wallet: Improve wallet creation error message with usage hint":
(https://github.com/bitcoin/bitcoin/pull/33124#issuecomment-3461323619)
Yeah, tend to agree. One will have to read the full help anyway to understand it.
(https://github.com/bitcoin/bitcoin/pull/33124#issuecomment-3461323619)
Yeah, tend to agree. One will have to read the full help anyway to understand it.
✅ maflcko closed a pull request: "[wip] [nomerge] [draft] 2510 msan zero"
(https://github.com/bitcoin/bitcoin/pull/33686)
(https://github.com/bitcoin/bitcoin/pull/33686)
💬 maflcko commented on pull request "[wip] [nomerge] [draft] 2510 msan zero":
(https://github.com/bitcoin/bitcoin/pull/33686#issuecomment-3461341894)
yeah, closing for now. It is a bit faster with the fast machines from the CI here, but it isn't going to pass in 6 hours, even with them, so moving to dedicated hardware without timeouts seems the only option.
(https://github.com/bitcoin/bitcoin/pull/33686#issuecomment-3461341894)
yeah, closing for now. It is a bit faster with the fast machines from the CI here, but it isn't going to pass in 6 hours, even with them, so moving to dedicated hardware without timeouts seems the only option.
💬 glozow commented on pull request "chainparams: remove dnsseed.bitcoin.dashjr-list-of-p2p-nodes.us":
(https://github.com/bitcoin/bitcoin/pull/33723#issuecomment-3461383230)
> I'd highly suggest waiting until @luke-jr can comment and weigh in on this directly before even considering hastily merging this.
We have previously given operators time to respond when we noticed problems with their DNS seed. I'm not sure what is giving people the impression that we're not doing the same here; Luke has been pinged both on this PR and directly through other platforms.
@luke-jr please update us here instead of on twitter.
(https://github.com/bitcoin/bitcoin/pull/33723#issuecomment-3461383230)
> I'd highly suggest waiting until @luke-jr can comment and weigh in on this directly before even considering hastily merging this.
We have previously given operators time to respond when we noticed problems with their DNS seed. I'm not sure what is giving people the impression that we're not doing the same here; Luke has been pinged both on this PR and directly through other platforms.
@luke-jr please update us here instead of on twitter.
💬 maflcko commented on pull request "ci: run native fuzz with MSAN job":
(https://github.com/bitcoin/bitcoin/pull/33626#discussion_r2432198820)
```suggestion
cirrus-runner: 'ghcr.io/cirruslabs/ubuntu-runner-amd64:24.04-md'
```
I wonder if md gives a speedup, similar to the valgrind task? https://github.com/bitcoin/bitcoin/pull/33461#discussion_r2451180232
Also on GHA runners, the timeout is short a few minutes of being hit?
(https://github.com/bitcoin/bitcoin/pull/33626#discussion_r2432198820)
```suggestion
cirrus-runner: 'ghcr.io/cirruslabs/ubuntu-runner-amd64:24.04-md'
```
I wonder if md gives a speedup, similar to the valgrind task? https://github.com/bitcoin/bitcoin/pull/33461#discussion_r2451180232
Also on GHA runners, the timeout is short a few minutes of being hit?
💬 maflcko commented on pull request "net: Fix Discover() not running when using -bind=0.0.0.0:port":
(https://github.com/bitcoin/bitcoin/pull/32757#issuecomment-3461409851)
Are you still working on this?
(https://github.com/bitcoin/bitcoin/pull/32757#issuecomment-3461409851)
Are you still working on this?
💬 brunoerg commented on issue "RFC: Do we want to support fuzzing on MacOS?":
(https://github.com/bitcoin/bitcoin/issues/33731#issuecomment-3461417256)
Concept ACK on option 2. I use macOS and I agree this is a pain -- The effort of updating the documentation is not worth it. The only downside of it would be not attracting macOS people to fuzzing development (?). But anyway, if someone really wants to get in fuzzing dev would have to have a linux at some point.
(https://github.com/bitcoin/bitcoin/issues/33731#issuecomment-3461417256)
Concept ACK on option 2. I use macOS and I agree this is a pain -- The effort of updating the documentation is not worth it. The only downside of it would be not attracting macOS people to fuzzing development (?). But anyway, if someone really wants to get in fuzzing dev would have to have a linux at some point.
💬 fanquake commented on pull request "ci: run native fuzz with MSAN job":
(https://github.com/bitcoin/bitcoin/pull/33626#discussion_r2472963951)
Pushed up `-md` for a look.
(https://github.com/bitcoin/bitcoin/pull/33626#discussion_r2472963951)
Pushed up `-md` for a look.
💬 hebasto commented on pull request "guix: Use UCRT runtime for Windows release binaries":
(https://github.com/bitcoin/bitcoin/pull/33593#discussion_r2473009165)
We cannot phase out the `with-winpthreads?` argument, as during the recursive invocation it is unset:https://github.com/bitcoin/bitcoin/blob/f380e6251021cc7aa0c47f564147abea922dc050/contrib/guix/manifest.scm#L154-L157
(https://github.com/bitcoin/bitcoin/pull/33593#discussion_r2473009165)
We cannot phase out the `with-winpthreads?` argument, as during the recursive invocation it is unset:https://github.com/bitcoin/bitcoin/blob/f380e6251021cc7aa0c47f564147abea922dc050/contrib/guix/manifest.scm#L154-L157
💬 trevarj commented on pull request "guix: Use UCRT runtime for Windows release binaries":
(https://github.com/bitcoin/bitcoin/pull/33593#discussion_r2473010211)
It should be possible to `inherit` the Guix package and then modify `arguments` to replace `"--with-default-msvcrt=msvcrt" with ""--with-default-msvcrt=ucrt"
```scheme
(use-modules
(guix packages)
(guix gexp)
(guix utils)
(gnu packages mingw)
(srfi srfi-1))
(define (override-mingw base-pkg)
(package
(inherit base-pkg)
(name "mingw-ucrt")
(arguments
(substitute-keyword-arguments (package-arguments base-pkg)
((#:configure-flags flags)
;; Force evaluation of
...
(https://github.com/bitcoin/bitcoin/pull/33593#discussion_r2473010211)
It should be possible to `inherit` the Guix package and then modify `arguments` to replace `"--with-default-msvcrt=msvcrt" with ""--with-default-msvcrt=ucrt"
```scheme
(use-modules
(guix packages)
(guix gexp)
(guix utils)
(gnu packages mingw)
(srfi srfi-1))
(define (override-mingw base-pkg)
(package
(inherit base-pkg)
(name "mingw-ucrt")
(arguments
(substitute-keyword-arguments (package-arguments base-pkg)
((#:configure-flags flags)
;; Force evaluation of
...
📝 maflcko converted_to_draft a pull request: "ci: Call docker exec from Python script to fix word splitting"
(https://github.com/bitcoin/bitcoin/pull/33732)
The remaining `ci/test/02_run_container.sh` is fine, but has a bunch of shellcheck SC2086 word splitting violations.
This is fine currently, because the only place that needed them had additional escaping, and all other commands happened to split fine on spaces.
However, this may change in the future. So fix it now, by rewriting it in Python, which is recommended in the dev notes.
(https://github.com/bitcoin/bitcoin/pull/33732)
The remaining `ci/test/02_run_container.sh` is fine, but has a bunch of shellcheck SC2086 word splitting violations.
This is fine currently, because the only place that needed them had additional escaping, and all other commands happened to split fine on spaces.
However, this may change in the future. So fix it now, by rewriting it in Python, which is recommended in the dev notes.
💬 maflcko commented on pull request "ci: run native fuzz with MSAN job":
(https://github.com/bitcoin/bitcoin/pull/33626#discussion_r2473028992)
on the slow GHA runners, the timeout is short a few minutes of being hit?
(https://github.com/bitcoin/bitcoin/pull/33626#discussion_r2473028992)
on the slow GHA runners, the timeout is short a few minutes of being hit?
💬 fanquake commented on pull request "ci: run native fuzz with MSAN job":
(https://github.com/bitcoin/bitcoin/pull/33626#discussion_r2473053980)
Ah, will bump this to ~150.
(https://github.com/bitcoin/bitcoin/pull/33626#discussion_r2473053980)
Ah, will bump this to ~150.
💬 glozow commented on issue "Batch tx broadcast RPC":
(https://github.com/bitcoin/bitcoin/issues/33700#issuecomment-3461570152)
This definitely seems doable. A couple more API questions since you're here:
What would you prefer to happen if a transaction fails? Is it fine if we quit altogether or do you want the node to keep trying with everything else?
We currently just have an error message that's just passed/failed. Would this change work: all passed / some passed / you submitted invalid stuff / maxfeerate or maxburnamount exceeded ?
Are you ok with _some_ requirements, e.g. "your batch contains transactions that confl
...
(https://github.com/bitcoin/bitcoin/issues/33700#issuecomment-3461570152)
This definitely seems doable. A couple more API questions since you're here:
What would you prefer to happen if a transaction fails? Is it fine if we quit altogether or do you want the node to keep trying with everything else?
We currently just have an error message that's just passed/failed. Would this change work: all passed / some passed / you submitted invalid stuff / maxfeerate or maxburnamount exceeded ?
Are you ok with _some_ requirements, e.g. "your batch contains transactions that confl
...
👍 hebasto approved a pull request: "random: scope environ extern to macOS, BSDs and Illumos"
(https://github.com/bitcoin/bitcoin/pull/33714#pullrequestreview-3393497979)
ACK fcdbce4a040e4a40aac9ed5ef8634b09d092063e.
(https://github.com/bitcoin/bitcoin/pull/33714#pullrequestreview-3393497979)
ACK fcdbce4a040e4a40aac9ed5ef8634b09d092063e.
🤔 jaonoctus reviewed a pull request: "chainparams: remove *petertodd.net"
(https://github.com/bitcoin/bitcoin/pull/33730#pullrequestreview-3393535153)
nACK
(https://github.com/bitcoin/bitcoin/pull/33730#pullrequestreview-3393535153)
nACK
🤔 jaonoctus reviewed a pull request: "chainparams: remove dnsseed.bitcoin.dashjr-list-of-p2p-nodes.us"
(https://github.com/bitcoin/bitcoin/pull/33723#pullrequestreview-3393546490)
ACK, I agree with @achow101 and @niteshbalusu11
(https://github.com/bitcoin/bitcoin/pull/33723#pullrequestreview-3393546490)
ACK, I agree with @achow101 and @niteshbalusu11