💬 hebasto commented on issue "test: SegFault in `ismine_tests` on SunOS / illumos":
(https://github.com/bitcoin/bitcoin/issues/29908#issuecomment-2141785791)
> Are you planning on providing the requested information here, so this can be investigated further?
Sure. Thanks for reminding :)
(https://github.com/bitcoin/bitcoin/issues/29908#issuecomment-2141785791)
> Are you planning on providing the requested information here, so this can be investigated further?
Sure. Thanks for reminding :)
💬 fanquake commented on issue "build: Unaligned libsecp256k1 flags in debug builds":
(https://github.com/bitcoin/bitcoin/issues/30055#issuecomment-2141805492)
What's the status of this? Can you explain the use-case where this is an issue for you, or the other motivation?
(https://github.com/bitcoin/bitcoin/issues/30055#issuecomment-2141805492)
What's the status of this? Can you explain the use-case where this is an issue for you, or the other motivation?
💬 hebasto commented on pull request "contrib: Renew Windows code signing certificate":
(https://github.com/bitcoin/bitcoin/pull/30149#issuecomment-2141814209)
> Backported to 27.x in #30092.
Windows 11 shows the correct data in the "Digital Signatures Details" for the [`bitcoin-27.1rc1-win64-setup.exe`](https://bitcoincore.org/bin/bitcoin-core-27.1/test.rc1/bitcoin-27.1rc1-win64-setup.exe):

(https://github.com/bitcoin/bitcoin/pull/30149#issuecomment-2141814209)
> Backported to 27.x in #30092.
Windows 11 shows the correct data in the "Digital Signatures Details" for the [`bitcoin-27.1rc1-win64-setup.exe`](https://bitcoincore.org/bin/bitcoin-core-27.1/test.rc1/bitcoin-27.1rc1-win64-setup.exe):

💬 maflcko commented on pull request "fuzz: bound some miniscript operations to avoid fuzz timeouts":
(https://github.com/bitcoin/bitcoin/pull/30197#discussion_r1622309134)
This isn't available yet. You'll have to use a reverse iterator manually for now.
(https://github.com/bitcoin/bitcoin/pull/30197#discussion_r1622309134)
This isn't available yet. You'll have to use a reverse iterator manually for now.
💬 willcl-ark commented on issue "noban permission probably shouldn't give additional permissions implcitly":
(https://github.com/bitcoin/bitcoin/issues/19886#issuecomment-2141885344)
After re-thinking on this issue, in my opinion it's **correct** to send the requested block (if it exists) to a `NoBan` peer here.
The `NoBan` permission is designed to augment the Peer with:
```
// Can't be banned/disconnected/discouraged for misbehavior
```
... which I would expect to include the "misbehaviour" of requesting blocks past the `NODE_NETWORK_LIMITED` threshold.
- Changing this to the `Download` permission would have minimal practical impact, as `NoBan` [implies `Down
...
(https://github.com/bitcoin/bitcoin/issues/19886#issuecomment-2141885344)
After re-thinking on this issue, in my opinion it's **correct** to send the requested block (if it exists) to a `NoBan` peer here.
The `NoBan` permission is designed to augment the Peer with:
```
// Can't be banned/disconnected/discouraged for misbehavior
```
... which I would expect to include the "misbehaviour" of requesting blocks past the `NODE_NETWORK_LIMITED` threshold.
- Changing this to the `Download` permission would have minimal practical impact, as `NoBan` [implies `Down
...
✅ hebasto closed an issue: "build: Unaligned libsecp256k1 flags in debug builds"
(https://github.com/bitcoin/bitcoin/issues/30055)
(https://github.com/bitcoin/bitcoin/issues/30055)
💬 willcl-ark commented on issue "contrib: makeseeds.py improvements":
(https://github.com/bitcoin/bitcoin/issues/17020#issuecomment-2141896732)
> [ ] Bring back onion functionality past TorV3 switch
> [ ] We need a source of non-hardcoded V3 peers, currently the only ones are hardcoded and the seeder hasn't been updated to crawl v3 nodes yet (see crawler: Collect Tor v3 and I2P addresses? sipa/bitcoin-seeder#92)
I think these two can be checked off post-https://github.com/bitcoin/bitcoin/pull/30008 ?
(https://github.com/bitcoin/bitcoin/issues/17020#issuecomment-2141896732)
> [ ] Bring back onion functionality past TorV3 switch
> [ ] We need a source of non-hardcoded V3 peers, currently the only ones are hardcoded and the seeder hasn't been updated to crawl v3 nodes yet (see crawler: Collect Tor v3 and I2P addresses? sipa/bitcoin-seeder#92)
I think these two can be checked off post-https://github.com/bitcoin/bitcoin/pull/30008 ?
👍 hebasto approved a pull request: "depends: qt 5.15.14 and fix macOS build with Clang 18"
(https://github.com/bitcoin/bitcoin/pull/30198#pullrequestreview-2090665641)
ACK 0a3631fc352eda849290db940844e5ef723436df, a new patch indeed fixes cross-compiling on Ubuntu 24.04 with `FORCE_USE_SYSTEM_CLANG=1`.
(https://github.com/bitcoin/bitcoin/pull/30198#pullrequestreview-2090665641)
ACK 0a3631fc352eda849290db940844e5ef723436df, a new patch indeed fixes cross-compiling on Ubuntu 24.04 with `FORCE_USE_SYSTEM_CLANG=1`.
💬 maflcko commented on pull request "Several randomness improvements":
(https://github.com/bitcoin/bitcoin/pull/29625#discussion_r1622387886)
Clearing the cache helped, so likely it was a corrupt compilation unit, cached by ccache, which also explains why a re-run didn't help.
(https://github.com/bitcoin/bitcoin/pull/29625#discussion_r1622387886)
Clearing the cache helped, so likely it was a corrupt compilation unit, cached by ccache, which also explains why a re-run didn't help.
💬 maflcko commented on pull request "depends: consolidate dependency docs":
(https://github.com/bitcoin/bitcoin/pull/30204#issuecomment-2142122297)
ACK a27e1ceb9f9c9239af9b0a151c84a57573ea646a
(https://github.com/bitcoin/bitcoin/pull/30204#issuecomment-2142122297)
ACK a27e1ceb9f9c9239af9b0a151c84a57573ea646a
👍1
💬 sr-gi commented on pull request "init: fixes file descriptor accounting":
(https://github.com/bitcoin/bitcoin/pull/30065#discussion_r1622428800)
I agree with the "making it more confusing". Given this is an edge case, I'll remove the outbound count and we can account for it if someone has a better way of doing so, or if someone opens an issue about it.
(https://github.com/bitcoin/bitcoin/pull/30065#discussion_r1622428800)
I agree with the "making it more confusing". Given this is an edge case, I'll remove the outbound count and we can account for it if someone has a better way of doing so, or if someone opens an issue about it.
📝 dergoegge opened a pull request: "fuzz: Make FuzzedSock fuzz friendlier"
(https://github.com/bitcoin/bitcoin/pull/30211)
`FuzzedSock` has a few issues that block a fuzzer from making progress. See commit messages for details.
(https://github.com/bitcoin/bitcoin/pull/30211)
`FuzzedSock` has a few issues that block a fuzzer from making progress. See commit messages for details.
💬 Sjors commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#discussion_r1622453453)
Matches!
(https://github.com/bitcoin/bitcoin/pull/28241#discussion_r1622453453)
Matches!
💬 maflcko commented on pull request "lint/contrib/doc updates":
(https://github.com/bitcoin/bitcoin/pull/30084#discussion_r1622454655)
When fixing typos, it could make sense to cherry-pick the real ones (non-subtree, non-release notes) from the closed https://github.com/bitcoin/bitcoin/pull/30188/files. Otherwise, people will continue to open pull requests to fix them.
(https://github.com/bitcoin/bitcoin/pull/30084#discussion_r1622454655)
When fixing typos, it could make sense to cherry-pick the real ones (non-subtree, non-release notes) from the closed https://github.com/bitcoin/bitcoin/pull/30188/files. Otherwise, people will continue to open pull requests to fix them.
💬 dergoegge commented on pull request "fuzz: Make FuzzedSock fuzz friendlier":
(https://github.com/bitcoin/bitcoin/pull/30211#issuecomment-2142220760)
These issues were discovered by Marco (@marcofleon) and me while he was working on #28803. He will have a PR for that up soon and this PR is a prerequisite (although it also helps with other harnesses that use `FuzzedSock`).
cc @vasild perhaps you are interested in reviewing this.
(https://github.com/bitcoin/bitcoin/pull/30211#issuecomment-2142220760)
These issues were discovered by Marco (@marcofleon) and me while he was working on #28803. He will have a PR for that up soon and this PR is a prerequisite (although it also helps with other harnesses that use `FuzzedSock`).
cc @vasild perhaps you are interested in reviewing this.
💬 sr-gi commented on pull request "init: fixes file descriptor accounting":
(https://github.com/bitcoin/bitcoin/pull/30065#issuecomment-2142248272)
> ACK [fa42bf7](https://github.com/bitcoin/bitcoin/commit/fa42bf7ee89999b33d4af0600b0d8da1275e5d5e)
>
> This is already an improvement over `master`. Would be nice to resolve [#30065 (comment)](https://github.com/bitcoin/bitcoin/pull/30065#discussion_r1613422626) as well.
Addressed all comments now
(https://github.com/bitcoin/bitcoin/pull/30065#issuecomment-2142248272)
> ACK [fa42bf7](https://github.com/bitcoin/bitcoin/commit/fa42bf7ee89999b33d4af0600b0d8da1275e5d5e)
>
> This is already an improvement over `master`. Would be nice to resolve [#30065 (comment)](https://github.com/bitcoin/bitcoin/pull/30065#discussion_r1613422626) as well.
Addressed all comments now
💬 sr-gi commented on pull request "init: fixes file descriptor accounting":
(https://github.com/bitcoin/bitcoin/pull/30065#discussion_r1622471290)
I've rolled back this. After some of the refactoring, this looked like:
```
std::clamp(available_fds - min_required_fds, 0, nUserMaxConnections);
```
However, `available_fds` cannot be smaller than `min_required_fds`, and `nUserMaxConnections` cannot be negative. So calling `std::min` should be enough.
(https://github.com/bitcoin/bitcoin/pull/30065#discussion_r1622471290)
I've rolled back this. After some of the refactoring, this looked like:
```
std::clamp(available_fds - min_required_fds, 0, nUserMaxConnections);
```
However, `available_fds` cannot be smaller than `min_required_fds`, and `nUserMaxConnections` cannot be negative. So calling `std::min` should be enough.
📝 willcl-ark opened a pull request: "rename TransactionErrors: MISSING_INPUTS and ALREADY_IN_CHAIN"
(https://github.com/bitcoin/bitcoin/pull/30212)
Closes: #19363
Renaming these two errors improves clarity around the returned error when:
1. a transactions' outputs are already found in the utxo set (`ALREADY_IN_CHAIN -> ALREADY_IN_UTXO_SET`)
2. a transactions' inputs are missing from the utxo set, which could also be due to all inputs having already been spent (`MISSING_INPUTS -> INPUTS_MISSING_OR_SPENT`)
(https://github.com/bitcoin/bitcoin/pull/30212)
Closes: #19363
Renaming these two errors improves clarity around the returned error when:
1. a transactions' outputs are already found in the utxo set (`ALREADY_IN_CHAIN -> ALREADY_IN_UTXO_SET`)
2. a transactions' inputs are missing from the utxo set, which could also be due to all inputs having already been spent (`MISSING_INPUTS -> INPUTS_MISSING_OR_SPENT`)
💬 willcl-ark commented on pull request "rename TransactionErrors: MISSING_INPUTS and ALREADY_IN_CHAIN":
(https://github.com/bitcoin/bitcoin/pull/30212#issuecomment-2142284206)
This addresses #19363 's concern:
> If a transaction is already in the block chain, and all of its outputs are spent, the BroadcastTransaction() fails to detect the correct transaction status. It returns TransactionError::MISSING_INPUTS instead of TransactionError::ALREADY_IN_CHAIN.
...by now returning `TransactionError::INPUTS_MISSING_OR_SPENT`. The [translation](https://github.com/bitcoin/bitcoin/blob/62f7f59ff495fbcbfc10c25e97bb0dc032647abf/src/util/error.cpp#L19) for this error message
...
(https://github.com/bitcoin/bitcoin/pull/30212#issuecomment-2142284206)
This addresses #19363 's concern:
> If a transaction is already in the block chain, and all of its outputs are spent, the BroadcastTransaction() fails to detect the correct transaction status. It returns TransactionError::MISSING_INPUTS instead of TransactionError::ALREADY_IN_CHAIN.
...by now returning `TransactionError::INPUTS_MISSING_OR_SPENT`. The [translation](https://github.com/bitcoin/bitcoin/blob/62f7f59ff495fbcbfc10c25e97bb0dc032647abf/src/util/error.cpp#L19) for this error message
...
🚀 fanquake merged a pull request: "depends: qt 5.15.14 and fix macOS build with Clang 18"
(https://github.com/bitcoin/bitcoin/pull/30198)
(https://github.com/bitcoin/bitcoin/pull/30198)