💬 TheCharlatan commented on pull request "build: always set `-g -O2` in `CORE_CXXFLAGS`":
(https://github.com/bitcoin/bitcoin/pull/29205#discussion_r1446088741)
Might have been a typo?

💬 fanquake commented on issue "new crash in v26.0":
(https://github.com/bitcoin/bitcoin/issues/29153#issuecomment-1883055919)
I assume this be moved to the GUI repo? @hebasto

💬 fanquake commented on pull request "build: always set `-g -O2` in `CORE_CXXFLAGS`":
(https://github.com/bitcoin/bitcoin/pull/29205#discussion_r1446092071)
@theuni If you want I can also just include a fix for this in here, while we are sorting out flags.

💬 furszy commented on pull request "init: handle empty settings file gracefully":
(https://github.com/bitcoin/bitcoin/pull/29144#issuecomment-1883057125)
> I think the error message "Unable to parse settings file %s" could definitely be improved though. Would suggest maybe "Settings file %s does not contain valid JSON. This is probably caused by disk corruption or a crash, and can be fixed by removing the file, which will reset settings to default values." This would probably work OK for both command line and GUI (since the GUI already offers the option to reset the settings see [bitcoin-core/gui#379](https://github.com/bitcoin-core/gui/pull/379)
...

⚠️ hebasto opened an issue: "New crash in v26.0"
(https://github.com/bitcoin-core/gui/issues/785)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

bitcoin-qt crashed while loading wallets at startup.

I used to see occasional crashes on startup a few years ago, but it hasn't been happening at all in the last couple of major releases.

I've been running v26.0 for a week or two and haven't had any problem with it crashing until today.

Here's a backtrace. I run it in gdb habitually because I used to see a lot of crashes and
...

💬 hebasto commented on issue "new crash in v26.0":
(https://github.com/bitcoin/bitcoin/issues/29153#issuecomment-1883062230)
Moved to https://github.com/bitcoin-core/gui/issues/785.

✅ hebasto closed an issue: "new crash in v26.0"
(https://github.com/bitcoin/bitcoin/issues/29153)

💬 dergoegge commented on pull request "fuzz: a target for the block index database":
(https://github.com/bitcoin/bitcoin/pull/28209#discussion_r1446096447)
```suggestion
const std::string flag_name = fuzzed_data_provider.ConsumeRandomLengthString(100);
```

`ConsumeRandomLengthString`: "Designed to be more stable with respect to a fuzzer inserting characters than just picking a random length and then consuming that many bytes with |ConsumeBytes|."

💬 dergoegge commented on pull request "fuzz: a target for the block index database":
(https://github.com/bitcoin/bitcoin/pull/28209#discussion_r1446090797)
nit: I think constants are not supposed to be prefixed with `g_`

```suggestion
const uint256 g_block_hash{uint256S("000000002c05cc2e78923c34df87fd108b22221ac6076c18f3ade378a4d915e9")};
const uint32_t g_bits{0x1d00ffff};
```

I think you could also just use the genesis block (`Params().GenesisBlock()`) for these two values.

💬 dergoegge commented on pull request "fuzz: a target for the block index database":
(https://github.com/bitcoin/bitcoin/pull/28209#discussion_r1446101167)
Can we get rid of `g_setup` if we initialize a new `SignalInterrupt` here each interation?

🤔 furszy reviewed a pull request: "init: handle empty settings file gracefully"
(https://github.com/bitcoin/bitcoin/pull/29144#pullrequestreview-1811179954)
Cool notification. The CI should be happy now.

🤔 glozow reviewed a pull request: "[26.x] Backports"
(https://github.com/bitcoin/bitcoin/pull/29011#pullrequestreview-1811251821)
ACK 7b79e54474b86864c81148c74824bfe4b732412d, matches mine

💬 glozow commented on pull request "test: wallet rescan with reorged parent + IsFromMe child in mempool":
(https://github.com/bitcoin/bitcoin/pull/29179#discussion_r1446161874)
We are testing that they are marked as in-mempool wallet transactions.

💬 glozow commented on pull request "test: wallet rescan with reorged parent + IsFromMe child in mempool":
(https://github.com/bitcoin/bitcoin/pull/29179#issuecomment-1883154232)
> Is there a reason both legacy and descriptor wallet tests aren't in the new wallet_rescan_unconfirmed test, though? That seems like a more logical grouping with less duplication?

They use different RPCs and different codepaths, so I don't really see why they need to be in the same file or tied together. I think it's more readable for them to be separate (having most the test as `if is_sqlite_compiled() elif is_bdb_compiled()` is pretty annoying). I think it's much easier to delete legacy wi
...

✅ reardencode closed a pull request: "OP_CHECKTEMPLATEVERIFY, OP_CHECKSIGFROMSTACK(VERIFY), OP_INTERNALKEY validation (LNHANCE)"
(https://github.com/bitcoin/bitcoin/pull/29198)

💬 reardencode commented on pull request "OP_CHECKTEMPLATEVERIFY, OP_CHECKSIGFROMSTACK(VERIFY), OP_INTERNALKEY validation (LNHANCE)":
(https://github.com/bitcoin/bitcoin/pull/29198#issuecomment-1883158798)
Hi Sjors, thanks for your thoughts!

> It seems too early in the process to open a pull request like this. It's also possible to open a pull request against your own repo-fork of Bitcoin Core, or Inquisition as suggested above.

> For comparison, the first Taproot pull request to this repo #17977 was made in early 2020, which was _after_ about year of in person workshops studying the proposal. The BIP's themselves were first proposed in 2018. The first work-in-progress taproot branch, not a
...

📝 reardencode reopened a pull request: "OP_CHECKTEMPLATEVERIFY, OP_CHECKSIGFROMSTACK(VERIFY), OP_INTERNALKEY validation (LNHANCE)"
(https://github.com/bitcoin/bitcoin/pull/29198)
This pull request contains a the same implementation of OP_CHECKTEMPLATEVERIFY (BIP119) as @jamesob's [Covenant Tools](https://github.com/bitcoin/bitcoin/pull/28550), an implementation of [OP_CHECKSIGFROMSTACK(VERIFY)](https://github.com/bitcoin/bips/pull/1535) and of [OP_INTERNALKEY](https://github.com/bitcoin/bips/pull/1534).

There are no testnet or mainnet activation parameters proposed in this pull request. I am deeply uninterested in the details of activation semantics.

This combinati
...

🤔 furszy reviewed a pull request: "assumeutxo, rpc: Improve EOF error when reading snapshot metadata in loadtxoutset"
(https://github.com/bitcoin/bitcoin/pull/28670#pullrequestreview-1811280860)
Would be good to add test coverage for this error.

Also, the "file size being too small" error message isn't really descriptive and it does not provide further guidance to solve the problem. The file could be corrupt or could had been crafted erroneously.
I would go simpler for now and change "file size being too small" for "Unable to load UTXO snapshot, the file %s cannot be parsed."

I see your alternative solution better. More comprehensive.

💬 reardencode commented on pull request "OP_CHECKTEMPLATEVERIFY, OP_CHECKSIGFROMSTACK(VERIFY), OP_INTERNALKEY validation (LNHANCE)":
(https://github.com/bitcoin/bitcoin/pull/29198#issuecomment-1883175973)
Hi Sjors, thanks for your thoughts!

> It seems too early in the process to open a pull request like this. It's also possible to open a pull request against your own repo-fork of Bitcoin Core, or Inquisition as suggested above.

I'm not aware of a well defined process for making changes to bitcoin consensus (nor do I think there should be one), so I'm not sure how we can be too early in the process.

CTV had (a year of?) workshops and review with Jeremy Rubin about 2 years ago. OP_INTERNA
...

💬 Ayush170-Future commented on pull request "fuzz: wallet, add target for Spend":
(https://github.com/bitcoin/bitcoin/pull/28236#discussion_r1446205449)
I've thought about it, and I think a global Singleton might not be the smartest way to conserve resources. The `wallet` object definitely mutates during execution. I think I've to remove the Singleton pattern, but doing so will make the file very slow, as creating the `Descriptor Wallet` is really time-consuming.