Bitcoin Core Github
43 subscribers
123K links
Download Telegram
💬 l0rinc commented on pull request "clang-tidy: Apply modernize-deprecated-headers":
(https://github.com/bitcoin/bitcoin/pull/32673#discussion_r2124193602)
looks to me like we might as well remove this
💬 l0rinc commented on pull request "clang-tidy: Apply modernize-deprecated-headers":
(https://github.com/bitcoin/bitcoin/pull/32673#discussion_r2124231739)
Q: is there any reason the header include guard is after the includes here?
💬 Muniru0 commented on issue "doc: references to unshipped documentation":
(https://github.com/bitcoin/bitcoin/issues/32565#issuecomment-2936112247)
@fanquake why not just updating the the docs to point to the online versions. e.g:

**Replace:**
`see doc/external-signer.md`

With:
`see https://github.com/bitcoin/bitcoin/blob/master/doc/cjdns.md`

This make's sure that:
1. **Accessibility**: Users can easily access the documentation regardless of how they obtained the binaries.

2. **Up-to-Date Information**: Linking to the online repository ensures that users are directed to the most current version of the documentation.

3. **Reduced Confu
...
💬 hebasto commented on pull request "ci, iwyu: Treat warnings as errors for specific directories":
(https://github.com/bitcoin/bitcoin/pull/31308#discussion_r2124341111)
I've tested the pre-CMake master branch @ 80f00cafdeef0600fa1a5e906686720786c2336c, and this line wasn't necessary there.
💬 m3dwards commented on pull request "test: enabling wallet migration functional test on windows":
(https://github.com/bitcoin/bitcoin/pull/32219#discussion_r2124354681)
@achow101 @maflcko do you or do you know of any use of using the `get_previous_releases.py` script to build from source? Am I being a bit eager in dropping this feature?
💬 ryanofsky commented on pull request "thread-safety: fix annotations with REVERSE_LOCK":
(https://github.com/bitcoin/bitcoin/pull/32465#issuecomment-2936288218)
To be clear, I all think the changes in this PR are improvements, even the last commit annotating REVERSE_LOCK, so I'd be happy to merge see this PR merged.

I'm just saying the last commit changing and annotating REVERSE_LOCK could be unnecessary if we decide to remove it later, and think we should remove it for reasons discussed above.
💬 hebasto commented on pull request "ci, iwyu: Treat warnings as errors for specific directories":
(https://github.com/bitcoin/bitcoin/pull/31308#issuecomment-2936303614)
Rebased.

All feedback from @maflcko and @fanquake has been addressed.

The IWYU-related workarounds are now documented with references to the relevant upstream issues.
💬 hebasto commented on pull request "ci, iwyu: Treat warnings as errors for specific directories":
(https://github.com/bitcoin/bitcoin/pull/31308#discussion_r2124428421)
> Doesn't matter much, but the comment could also say "... so skip the patch because it is not needed and to avoid issues ...".

Thanks! [Adjusted](https://github.com/bitcoin/bitcoin/pull/31308#issuecomment-2936303614).
💬 hebasto commented on pull request "ci, iwyu: Treat warnings as errors for specific directories":
(https://github.com/bitcoin/bitcoin/pull/31308#discussion_r2124430164)
[Reworked](https://github.com/bitcoin/bitcoin/pull/31308#issuecomment-2936303614) into a local pragma.
💬 glozow commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2124443576)
Sure, yes. I'll add a commit to the beginning to refactor to this structure + change all clients to do `MakeTxOrphanage`, and then swap out the `Impl`s basically.
💬 l3x3l commented on issue "Unusual "Wallet requires newer version" Error with wallet.dat on macOS, Even with Older Client":
(https://github.com/bitcoin/bitcoin/issues/32548#issuecomment-2936382014)
@maflcko Remove the data corruption label.
📝 brunoerg opened a pull request: "test: wallet: cover wallet passphrase with a null char"
(https://github.com/bitcoin/bitcoin/pull/32675)
This PR adds test coverage for the `walletpassphrase` RPC when the passphrase is incorrect due to a null character.

For reference: https://github.com/bitcoin/bitcoin/pull/27068 introduced the usage of `SecureString` to allow null characters.
💬 marcofleon commented on pull request "refactor: Convert GenTxid to `std::variant`":
(https://github.com/bitcoin/bitcoin/pull/32631#discussion_r2124510957)
done
💬 marcofleon commented on pull request "refactor: Convert GenTxid to `std::variant`":
(https://github.com/bitcoin/bitcoin/pull/32631#discussion_r2124511972)
completed
📝 mzumsande opened a pull request: "test: apply microsecond precision to test framework logging"
(https://github.com/bitcoin/bitcoin/pull/32676)
When analyzing functional test logs (produced with `combine_logs.py`), entries sometimes sort slightly out of order because even though python prints 6 digits for microsecond precision, it fills up the last 3 digits up with zeroes. For example, it may look like a message was received by the test framework before it was sent by the node.

Change this to actually use microsecond precision - this should make combined logs a little bit easier to analyze.
🤔 maflcko reviewed a pull request: "clang-tidy: Apply modernize-deprecated-headers"
(https://github.com/bitcoin/bitcoin/pull/32673#pullrequestreview-2893488292)
pushed a commit to sort
💬 maflcko commented on pull request "clang-tidy: Apply modernize-deprecated-headers":
(https://github.com/bitcoin/bitcoin/pull/32673#discussion_r2124554243)
thx, pushed a commit
💬 maflcko commented on pull request "clang-tidy: Apply modernize-deprecated-headers":
(https://github.com/bitcoin/bitcoin/pull/32673#discussion_r2124518776)
It is needed, see:

[09:40:31.692] The full include-list for /ci_container_base/src/bech32.h:
[09:40:31.692] #include <cstdint> // for uint8_t
💬 maflcko commented on pull request "clang-tidy: Apply modernize-deprecated-headers":
(https://github.com/bitcoin/bitcoin/pull/32673#discussion_r2124554639)
thx, sorted
💬 maflcko commented on pull request "clang-tidy: Apply modernize-deprecated-headers":
(https://github.com/bitcoin/bitcoin/pull/32673#discussion_r2124553705)
there are several, so maybe a separate pull request or linter could be added for this? Although, I don't see a risk here.

```
src/base58.h:14:#ifndef BITCOIN_BASE58_H
src/bech32.h:14:#ifndef BITCOIN_BECH32_H
src/common/messages.h:11:#ifndef BITCOIN_COMMON_MESSAGES_H
src/common/types.h:13:#ifndef BITCOIN_COMMON_TYPES_H
src/net_permissions.h:12:#ifndef BITCOIN_NET_PERMISSIONS_H
src/node/types.h:13:#ifndef BITCOIN_NODE_TYPES_H
src/txgraph.h:14:#ifndef BITCOIN_TXGRAPH_H
src/wallet/types.h
...