💬 new-light-106 commented on issue "build: Windows debug cross-build fails with `-O0`":
(https://github.com/bitcoin/bitcoin/issues/28109#issuecomment-1646834821)
jahad57ghasem@gmail.com

💬 ishaanam commented on pull request "wallet: track mempool conflicts with wallet transactions":
(https://github.com/bitcoin/bitcoin/pull/27307#discussion_r1271448113)
Fixed

💬 hebasto commented on issue "libsecp256k1 not instrumented when building with sanitizers":
(https://github.com/bitcoin/bitcoin/issues/27990#issuecomment-1646846003)
FWIW, the libsecp256k1 is instrumented properly when using the CMake-based build system (see the full prototype [branch](https://github.com/hebasto/bitcoin/tree/pr25797.69) from https://github.com/bitcoin/bitcoin/pull/25797):
```
$ objdump --disassemble=secp256k1_xonly_pubkey_serialize build/src/test/fuzz/fuzz | grep __sanitizer_cov
d42e90: e8 8b 23 c0 ff call 945220 <__sanitizer_cov_trace_const_cmp8>
d42ff2: e8 39 21 c0 ff call 945130 <__sanitizer_cov_trace_pc_indi
...

💬 hebasto commented on pull request "build: pass sanitize flags to instrument libsecp256k1 code":
(https://github.com/bitcoin/bitcoin/pull/27991#issuecomment-1646846285)
FWIW, CMake [works](https://github.com/bitcoin/bitcoin/issues/27990#issuecomment-1646846003) out of the box :)

📝 hebasto opened a pull request: "Add UBSan `-fsanitize=integer` suppressions for `src/secp256k1` subtree"
(https://github.com/bitcoin/bitcoin/pull/28131)
Required for https://github.com/bitcoin/bitcoin/pull/27991 (see the [comment](https://github.com/bitcoin/bitcoin/pull/27991#issuecomment-1611472816)) and for the upcoming CMake-based build system.

💬 manfreddd commented on issue "Bitcoin Core v25.0 Crashes":
(https://github.com/bitcoin/bitcoin/issues/28119#issuecomment-1646873055)
@russeree thank you for joining the support effort. I ran Bitcoin Core with the option that you and @MarcoFalke recommended. Here is the command and command line error message produced.

C:\Users\xxxx>bitcoind -reindex-chainstate -debug
Error: A fatal internal error occurred, see debug.log for details

This system won't let me load the entire 365 MB debug.log file. So, I am displaying below the tail end of the file including the fatal error message.

2023-07-23T14:28:27Z [validation] Bloc
...

📝 petertodd opened a pull request: "Enable full-rbf by default"
(https://github.com/bitcoin/bitcoin/pull/28132)
In addition to Luxor (3.3% hash power), the following pools are have enabled full-rbf:

- [Antpool, 21%](https://mempool.space/tx/53cec64b52989c531550ac4606bedf1ff83d5bfd90efdc4006f122ac6b1b7643)
- [Binance Pool, 8.2%](https://mempool.space/tx/64a26f750c7c58812bd475d054a9aa05248b6a8a2d53c0db38c0624197a4c68a)
- [KuCoinPool, 1.4%](https://mempool.space/tx/8df183b087d433eac4d8e44eca5c7162a2540ee078a28ca04316fd82e12ec435)
- [Poolin, 1.4%](https://mempool.space/tx/d14956420077974d8d28c2fcd005cd9
...

💬 petertodd commented on pull request "Enable full-rbf by default":
(https://github.com/bitcoin/bitcoin/pull/28132#issuecomment-1646874124)
FYI [mempool.space has enabled full-rbf by default](https://github.com/mempool/mempool/pull/3867), among many others.

💬 luke-jr commented on pull request "Remove arbitrary restrictions on OP_RETURN by default":
(https://github.com/bitcoin/bitcoin/pull/28130#issuecomment-1646878130)
>except storage

Blockchains don't do storage. There's a tool for that called ext4, and BitTorrent for distribution.

💬 jonatack commented on pull request "Silent Payments: Implement BIP352":
(https://github.com/bitcoin/bitcoin/pull/28122#discussion_r1271484187)
c75d9de0683a91151eb4a508cb64a8937ca92 `s/PAYMENT_H/PAYMENTS_H/` in `ifndef/define/endif` or rename the files to `silentpayment.{h,cpp}`

```bash
$ test/lint/lint-include-guards.py
src/wallet/silentpayments.h seems to be missing the expected include guard:
#ifndef BITCOIN_WALLET_SILENTPAYMENTS_H
#define BITCOIN_WALLET_SILENTPAYMENTS_H
...
#endif // BITCOIN_WALLET_SILENTPAYMENTS_H
```

💬 ChristopherA commented on pull request "Remove arbitrary restrictions on OP_RETURN by default":
(https://github.com/bitcoin/bitcoin/pull/28130#issuecomment-1646904832)
Concept ACK. Current limits on op_return force people to use hacks to store data hiding in op_codes. Using op_return is more `honest’. In my own case, current op_return limits keep me from posting a signed hash plus some metadata (<256 bytes) and thus I must use Taproot tricks instead.

🤔 jonatack reviewed a pull request: "kernel: Remove UniValue from kernel library"
(https://github.com/bitcoin/bitcoin/pull/28113#pullrequestreview-1542308295)
Approach ACK b89567f51ade926af8c918e4787046b7ccec8eb0

💬 jonatack commented on pull request "kernel: Remove UniValue from kernel library":
(https://github.com/bitcoin/bitcoin/pull/28113#discussion_r1271496876)
29e31d8

<details><summary><code>ParseSighash()</code> suggestions</summary><p>

```diff
--- a/src/core_io.h
+++ b/src/core_io.h
@@ -47,7 +47,7 @@ bool DecodeHexBlockHeader(CBlockHeader&, const std::string&
-util::Result<int> ParseSighash(const std::optional<std::string>& sighash);
+[[nodiscard]] util::Result<int> ParseSighash(const std::optional<std::string>& sighash);

--- a/src/core_read.cpp
+++ b/src/core_read.cpp
@@ -245,7 +245,7 @@ bool ParseHashStr(const std::string& strHex,
...

💬 jonatack commented on pull request "kernel: Remove UniValue from kernel library":
(https://github.com/bitcoin/bitcoin/pull/28113#discussion_r1271533741)
https://github.com/bitcoin/bitcoin/commit/29e31d894bd8bedb55c3a5a2d4d2681ba10a9b88 this ought to be added to `core_read.cpp` too; see full iwyu suggestions at https://cirrus-ci.com/task/4627440203464704?logs=ci#L1786

💬 jonatack commented on pull request "kernel: Remove UniValue from kernel library":
(https://github.com/bitcoin/bitcoin/pull/28113#discussion_r1271543977)
https://github.com/bitcoin/bitcoin/commit/29e31d894bd8bedb55c3a5a2d4d2681ba10a9b88 When storing a `Result` type, I think it's clearer to name it `result` (seems to be becoming a convention). In particular, `util::ErrorString(parsed_sighash)` is a little confusing.

```cpp
const auto result{ParseSighash(sighash.isNull() ? std::nullopt : std::make_optional(sighash.get_str()))};
if (!result) {
throw JSONRPCError(RPC_INVALID_PARAMETER, util::ErrorString(result).original);
}
...

💬 jonatack commented on pull request "kernel: Remove UniValue from kernel library":
(https://github.com/bitcoin/bitcoin/pull/28113#discussion_r1271540053)
https://github.com/bitcoin/bitcoin/commit/29e31d894bd8bedb55c3a5a2d4d2681ba10a9b88 (feel free to ignore) I wouldn't find review harder if the missing brackets were added

```cpp
if (v.isStr()) {
strHex = v.getValStr();
}
if (!IsHex(strHex)) {
throw std::runtime_error(strName + " must be hexadecimal string (not '" + strHex + "')");
}
```

💬 jonatack commented on pull request "refactor: Add util::Result failure values, multiple error and warning messages":
(https://github.com/bitcoin/bitcoin/pull/25665#discussion_r1271547869)
> I should probably add a tutorial-style document that describes how to use the Result class with standalone functions that return util::Result, chained functions that return util::Result values of the same type, and chained functions that return util::Result values of different types.

I think this would be valuable, either in `util/result.h` directly or in `doc/developer-notes.md`.

⚠️ jonatack opened an issue: "p2p_getaddr_caching.py failure in TSan CI"
(https://github.com/bitcoin/bitcoin/issues/28133)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

https://cirrus-ci.com/task/5471865133596672?logs=ci#L3981


### Expected behaviour

https://cirrus-ci.com/task/5471865133596672?logs=ci#L3981


### Steps to reproduce

https://cirrus-ci.com/task/5471865133596672?logs=ci#L3981


### Relevant log output

https://cirrus-ci.com/task/5471865133596672?logs=ci#L3981

```
test 2023-07-22T19:16:17.696000Z TestFramework (ERROR): Assertion fa
...

💬 emc99 commented on pull request "Enable full-rbf by default":
(https://github.com/bitcoin/bitcoin/pull/28132#issuecomment-1646954964)
When you say "by default", do you mean that full-rbf would come by default as part of IBD or when you update Bitcoin Core? When would full-rbf be "by default"?

💬 hebasto commented on issue "ci: Future of macOS and Windows MSVC CI tasks":
(https://github.com/bitcoin/bitcoin/issues/28098#issuecomment-1646955146)
Among free options, CircleCI looks better than others, IMO:
- https://circleci.com/docs/plan-free/
- https://circleci.com/pricing/

> Personally, I don't use macOS nor Windows, and I can't recommend it to anyone, so I don't mind if the tasks are removed.

We are responsible for quality of every release we are [shipping](https://bitcoincore.org/en/download/). And the user is free to make their own choice.