💬 laanwj commented on pull request "Crash fix, disconnect numBlocksChanged() signal during shutdown":
(https://github.com/bitcoin-core/gui/pull/864#issuecomment-2828299385)
That's really strange. Is there some thread race maybe?
(https://github.com/bitcoin-core/gui/pull/864#issuecomment-2828299385)
That's really strange. Is there some thread race maybe?
💬 maflcko commented on pull request "ci: Merge fuzz task for macOS and Windows":
(https://github.com/bitcoin/bitcoin/pull/32339#issuecomment-2828303457)
To give some numbers for the macOS task: It passed here in 29min (with an empty ccache), whereas on master, the times were 19min+14min=32min (https://github.com/bitcoin/bitcoin/actions/runs/14638899803/job/41076397151)
(https://github.com/bitcoin/bitcoin/pull/32339#issuecomment-2828303457)
To give some numbers for the macOS task: It passed here in 29min (with an empty ccache), whereas on master, the times were 19min+14min=32min (https://github.com/bitcoin/bitcoin/actions/runs/14638899803/job/41076397151)
⚠️ achow101 opened an issue: "Moving this repo to bitcoin-core"
(https://github.com/bitcoin/bitcoin/issues/32340)
Organizationally, it doesn't make sense for Bitcoin Core to be under the bitcoin GitHub organization. All of our other repos (gui, qa-assets, dev wiki, etc.) are under bitcoin-core, so having just this one repo in a different organization doesn't totally make sense. It's only in bitcoin/ for historical reasons. The bitcoin-core organization was created several years ago for the express purpose of moving this repo there, but we still haven't done it.
Conceptually, it makes sense to complete the
...
(https://github.com/bitcoin/bitcoin/issues/32340)
Organizationally, it doesn't make sense for Bitcoin Core to be under the bitcoin GitHub organization. All of our other repos (gui, qa-assets, dev wiki, etc.) are under bitcoin-core, so having just this one repo in a different organization doesn't totally make sense. It's only in bitcoin/ for historical reasons. The bitcoin-core organization was created several years ago for the express purpose of moving this repo there, but we still haven't done it.
Conceptually, it makes sense to complete the
...
💬 achow101 commented on issue "Moving this repo to bitcoin-core":
(https://github.com/bitcoin/bitcoin/issues/32340#issuecomment-2828326895)
One concern that was brought up during the IRC meeting is whether github might recycle the bitcoin org if it were to have no activity. This would be problematic as if it were recycled this opens up the possibility of scammers to create a new bitcoin/bitcoin that points to something malicious. However, several contributors have tried to get github to release inactive usernames and organizations with extreme difficulty, and generally unable to actually get them to do that, so I think this risk is
...
(https://github.com/bitcoin/bitcoin/issues/32340#issuecomment-2828326895)
One concern that was brought up during the IRC meeting is whether github might recycle the bitcoin org if it were to have no activity. This would be problematic as if it were recycled this opens up the possibility of scammers to create a new bitcoin/bitcoin that points to something malicious. However, several contributors have tried to get github to release inactive usernames and organizations with extreme difficulty, and generally unable to actually get them to do that, so I think this risk is
...
⚠️ maflcko opened an issue: "ctest -C Debug fails on vs2022 with test_bitcoin-qt (Exit code 0xc0000135)"
(https://github.com/bitcoin/bitcoin/issues/32341)
cmd: `ctest --output-on-failure --stop-on-failure -j $NUMBER_OF_PROCESSORS -C Debug`
output:
```
88% tests passed, 1 tests failed out of 8
Total Test time (real) = 98.97 sec
The following tests FAILED:
8 - test_bitcoin-qt (Exit code 0xc0000135
)
Errors while running CTest
```
log: https://github.com/bitcoin/bitcoin/actions/runs/14646596997/job/41102269274?pr=32339#step:11:78
ref: https://github.com/bitcoin/bitcoin/pull/32339
(https://github.com/bitcoin/bitcoin/issues/32341)
cmd: `ctest --output-on-failure --stop-on-failure -j $NUMBER_OF_PROCESSORS -C Debug`
output:
```
88% tests passed, 1 tests failed out of 8
Total Test time (real) = 98.97 sec
The following tests FAILED:
8 - test_bitcoin-qt (Exit code 0xc0000135
)
Errors while running CTest
```
log: https://github.com/bitcoin/bitcoin/actions/runs/14646596997/job/41102269274?pr=32339#step:11:78
ref: https://github.com/bitcoin/bitcoin/pull/32339
💬 kanzure commented on issue "Moving this repo to bitcoin-core":
(https://github.com/bitcoin/bitcoin/issues/32340#issuecomment-2828433172)
I would like to see a write-up of the different GitHub arrangements so that we can compare the plans by name.
(https://github.com/bitcoin/bitcoin/issues/32340#issuecomment-2828433172)
I would like to see a write-up of the different GitHub arrangements so that we can compare the plans by name.
📝 tomasandroil opened a pull request: "Fix missing error check in `set_clo_on_exec` for FD_CLOEXEC handling"
(https://github.com/bitcoin/bitcoin/pull/32342)
#### Changes:
- Replaces unchecked `fcntl(fd, F_SETFD, flags);` with a version that checks the return value.
- Throws `OSError` on failure with an appropriate error message.
#### Motivation:
Proper error handling is critical in system-level code to avoid silent failures, especially when setting file descriptor flags like `FD_CLOEXEC`. This fix ensures robustness and helps catch configuration issues early during process setup.
### Test coverage
No functional change beyond improved err
...
(https://github.com/bitcoin/bitcoin/pull/32342)
#### Changes:
- Replaces unchecked `fcntl(fd, F_SETFD, flags);` with a version that checks the return value.
- Throws `OSError` on failure with an appropriate error message.
#### Motivation:
Proper error handling is critical in system-level code to avoid silent failures, especially when setting file descriptor flags like `FD_CLOEXEC`. This fix ensures robustness and helps catch configuration issues early during process setup.
### Test coverage
No functional change beyond improved err
...
💬 laanwj commented on pull request "Fix missing error check in `set_clo_on_exec` for FD_CLOEXEC handling":
(https://github.com/bitcoin/bitcoin/pull/32342#discussion_r2058986286)
This `fcntl` can return `-1` on error too.
(https://github.com/bitcoin/bitcoin/pull/32342#discussion_r2058986286)
This `fcntl` can return `-1` on error too.
💬 sr-gi commented on issue "Moving this repo to bitcoin-core":
(https://github.com/bitcoin/bitcoin/issues/32340#issuecomment-2828495015)
I expressed my concerns about moving bitcoin out of the bitcoin org / leaving the bitcoin org empty during the meeting, but I think it is worth elaborating on them.
While I can see the logistical benefits of splitting the orgs and how it may make admin/moderator lives easier, I don't think that's enough motivation to overcome the potential risks of redirections being broken, github giving up the org due to inactivity, or a takeover of the bitcoin/bitcoin repo. I do agree that these threads are
...
(https://github.com/bitcoin/bitcoin/issues/32340#issuecomment-2828495015)
I expressed my concerns about moving bitcoin out of the bitcoin org / leaving the bitcoin org empty during the meeting, but I think it is worth elaborating on them.
While I can see the logistical benefits of splitting the orgs and how it may make admin/moderator lives easier, I don't think that's enough motivation to overcome the potential risks of redirections being broken, github giving up the org due to inactivity, or a takeover of the bitcoin/bitcoin repo. I do agree that these threads are
...
💬 furszy commented on pull request "Crash fix, disconnect numBlocksChanged() signal during shutdown":
(https://github.com/bitcoin-core/gui/pull/864#issuecomment-2828497800)
> That's really strange. Is there some thread race maybe?
> Does the old solution (to check clientModel in the callback) work better?
Hmm, I should check, but I have the hunch that events already queued are still delivered after disconnection. Which, if that is the case, the previous `clientModel` check would work better.
(https://github.com/bitcoin-core/gui/pull/864#issuecomment-2828497800)
> That's really strange. Is there some thread race maybe?
> Does the old solution (to check clientModel in the callback) work better?
Hmm, I should check, but I have the hunch that events already queued are still delivered after disconnection. Which, if that is the case, the previous `clientModel` check would work better.
💬 laanwj commented on pull request "Crash fix, disconnect numBlocksChanged() signal during shutdown":
(https://github.com/bitcoin-core/gui/pull/864#issuecomment-2828513340)
> Hmm, I should check, but I have the hunch that events already queued are still delivered after disconnection. Which, if that is the case, the previous clientModel check would work better.
The signals are queued, but if no handler is connected, they sould just be dropped.
(https://github.com/bitcoin-core/gui/pull/864#issuecomment-2828513340)
> Hmm, I should check, but I have the hunch that events already queued are still delivered after disconnection. Which, if that is the case, the previous clientModel check would work better.
The signals are queued, but if no handler is connected, they sould just be dropped.
✅ maflcko closed an issue: "ctest -C Debug fails on vs2022 (Exit code 0xc0000135)"
(https://github.com/bitcoin/bitcoin/issues/32341)
(https://github.com/bitcoin/bitcoin/issues/32341)
💬 maflcko commented on issue "ctest -C Debug fails on vs2022 (Exit code 0xc0000135)":
(https://github.com/bitcoin/bitcoin/issues/32341#issuecomment-2828524157)
I guess I caused the issue myself. False alarm
(https://github.com/bitcoin/bitcoin/issues/32341#issuecomment-2828524157)
I guess I caused the issue myself. False alarm
💬 achow101 commented on issue "Moving this repo to bitcoin-core":
(https://github.com/bitcoin/bitcoin/issues/32340#issuecomment-2828537251)
> I would like to see a write-up of the different GitHub arrangements so that we can compare the plans by name.
I've written it in this gist: https://gist.github.com/achow101/e20dcc0818e2b346e699438b70ee8b8c
I think discussion of bips is a bit off topic for this repo as the actions taken by either repo are largely unrelated.
> If the main motivation is making moderation easier, moving out the bips repository and leaving bitcoin under the bitcoin org seems a less risky option.
I don't think t
...
(https://github.com/bitcoin/bitcoin/issues/32340#issuecomment-2828537251)
> I would like to see a write-up of the different GitHub arrangements so that we can compare the plans by name.
I've written it in this gist: https://gist.github.com/achow101/e20dcc0818e2b346e699438b70ee8b8c
I think discussion of bips is a bit off topic for this repo as the actions taken by either repo are largely unrelated.
> If the main motivation is making moderation easier, moving out the bips repository and leaving bitcoin under the bitcoin org seems a less risky option.
I don't think t
...
💬 tomasandroil commented on pull request "Fix missing error check in `set_clo_on_exec` for FD_CLOEXEC handling":
(https://github.com/bitcoin/bitcoin/pull/32342#issuecomment-2828562780)
@laanwj Thanks for the review!
I've updated `set_clo_on_exec` to check the return value of `fcntl(fd, F_GETFD, 0)` and throw OSError on failure, similar to the existing check for `F_SETFD`
(https://github.com/bitcoin/bitcoin/pull/32342#issuecomment-2828562780)
@laanwj Thanks for the review!
I've updated `set_clo_on_exec` to check the return value of `fcntl(fd, F_GETFD, 0)` and throw OSError on failure, similar to the existing check for `F_SETFD`
💬 mzumsande commented on pull request "validation: stricter internal handling of invalid blocks":
(https://github.com/bitcoin/bitcoin/pull/31405#issuecomment-2828565105)
[96e6371](https://github.com/bitcoin/bitcoin/commit/96e6371328c7715ef814da5831d08b5d30285dd3) to [41860d9](https://github.com/bitcoin/bitcoin/commit/41860d98408f52c29e78f7708e8a0355ea5cc04c): rebased due to conflict with https://github.com/bitcoin/bitcoin/pull/31835
(https://github.com/bitcoin/bitcoin/pull/31405#issuecomment-2828565105)
[96e6371](https://github.com/bitcoin/bitcoin/commit/96e6371328c7715ef814da5831d08b5d30285dd3) to [41860d9](https://github.com/bitcoin/bitcoin/commit/41860d98408f52c29e78f7708e8a0355ea5cc04c): rebased due to conflict with https://github.com/bitcoin/bitcoin/pull/31835
💬 maflcko commented on pull request "ci: Merge fuzz task for macOS":
(https://github.com/bitcoin/bitcoin/pull/32339#issuecomment-2828589191)
(dropped the windows changes)
(https://github.com/bitcoin/bitcoin/pull/32339#issuecomment-2828589191)
(dropped the windows changes)
💬 furszy commented on pull request "Crash fix, disconnect numBlocksChanged() signal during shutdown":
(https://github.com/bitcoin-core/gui/pull/864#issuecomment-2828595545)
> > Hmm, I should check, but I have the hunch that events already queued are still delivered after disconnection. Which, if that is the case, the previous clientModel check would work better.
>
> It's worth checking but imo it's unlikely it works that way. The signals are queued, but if no handler is connected, they sould just be dropped (next time the event loop runs).
It seems to be the case: https://stackoverflow.com/questions/2532341/problem-with-qtqueuedconnection-signal-delivered-aft
...
(https://github.com/bitcoin-core/gui/pull/864#issuecomment-2828595545)
> > Hmm, I should check, but I have the hunch that events already queued are still delivered after disconnection. Which, if that is the case, the previous clientModel check would work better.
>
> It's worth checking but imo it's unlikely it works that way. The signals are queued, but if no handler is connected, they sould just be dropped (next time the event loop runs).
It seems to be the case: https://stackoverflow.com/questions/2532341/problem-with-qtqueuedconnection-signal-delivered-aft
...
📝 laanwj opened a pull request: "common: Close non-std fds before exec in RunCommandJSON"
(https://github.com/bitcoin/bitcoin/pull/32343)
Picks up stale #30756, while addressing my fallback comment (https://github.com/bitcoin/bitcoin/pull/30756#discussion_r2030844440).
> Currently, RunCommandParseJSON runs its target with whatever fds happen to be open inherited on POSIX platforms. I don't think there's any practical scenario where this is a problem right now, but there's a lot of potential for weird problems (eg, if a process manages to outlive bitcoind - perhaps it's hanging - the listening port(s) won't get released and star
...
(https://github.com/bitcoin/bitcoin/pull/32343)
Picks up stale #30756, while addressing my fallback comment (https://github.com/bitcoin/bitcoin/pull/30756#discussion_r2030844440).
> Currently, RunCommandParseJSON runs its target with whatever fds happen to be open inherited on POSIX platforms. I don't think there's any practical scenario where this is a problem right now, but there's a lot of potential for weird problems (eg, if a process manages to outlive bitcoind - perhaps it's hanging - the listening port(s) won't get released and star
...
💬 l0rinc commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2059063160)
Done (except for the edit part, it's better documentation this way I think)
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2059063160)
Done (except for the edit part, it's better documentation this way I think)