💬 pattiscott973 commented on pull request "[IBD] prevector: store `P2WSH`/`P2TR`/`P2PK` scripts inline":
(https://github.com/bitcoin/bitcoin/pull/32279#discussion_r2053150648)
Send

💬 achow101 commented on pull request "wallet: Fix relative path backup during migration.":
(https://github.com/bitcoin/bitcoin/pull/32273#issuecomment-2819784017)
> So it would not be possible to load a bare db outside -walletdir.

Good point

> And if the migration code was backing up a bare db file inside -walletdir like `arbitrarywalletname.db` it would just be backed up as `arbitrarywalletname.db_1223.bak`

This PR currently breaks that. Although `test_direct_file` doesn't currently fail, adding a check for the wallet's name in the backup path does make it fail, e.g.

```diff
diff --git a/test/functional/wallet_migration.py b/test/functional/
...

🤔 shahsb reviewed a pull request: "Fix failing util_time_GetTime test on Windows"
(https://github.com/bitcoin/bitcoin/pull/32318#pullrequestreview-2782621768)
Concept ACK

🤔 shahsb reviewed a pull request: "Fix failing util_time_GetTime test on Windows"
(https://github.com/bitcoin/bitcoin/pull/32318#pullrequestreview-2782622019)
Approach ACK

💬 shahsb commented on pull request "Fix failing util_time_GetTime test on Windows":
(https://github.com/bitcoin/bitcoin/pull/32318#issuecomment-2819918130)
ACK https://github.com/bitcoin/bitcoin/pull/32318/commits/51e4c5b42260855e5b5306d677ad34560976b9fd

📝 leonarddt05 opened a pull request: "Update multiprocess.md"
(https://github.com/bitcoin/bitcoin/pull/32321)
Hi,

Two suggestions for this doc:

1. "the getBlockHash method of the generated Chain server subclass in bitcoin-wallet" Correction: This should be "in bitcoin-node", not "bitcoin-wallet", since the server class resides in the node process.

2. "capnp files in src/ipc/capnp/ was mostly done" Correction: "were mostly done" (subject-verb agreement: plural noun "files")

Thanks.

🤔 ajtowns reviewed a pull request: "miner: timelock the coinbase to the mined block's height"
(https://github.com/bitcoin/bitcoin/pull/32155#pullrequestreview-2782594487)
Approach ACK 895beb30b375c04804e9596ed798ab56730dcae1

💬 ajtowns commented on pull request "miner: timelock the coinbase to the mined block's height":
(https://github.com/bitcoin/bitcoin/pull/32155#discussion_r2053224512)
Should these be `(height - 1) % LOCKTIME_THRESHOLD` ? Or are we just assuming a hard fork to a new transaction format in the next 9500 years?

💬 ajtowns commented on pull request "miner: timelock the coinbase to the mined block's height":
(https://github.com/bitcoin/bitcoin/pull/32155#discussion_r2053298127)
Maybe add something like

```python
genblock = create_block(hashprev=int(block["hash"], 16), tmpl=dict(height=block["height"]+1), ntime=block["time"]+1)
genblock.vtx[0].vin[0].nSequence = SEQUENCE_FINAL
genblock.vtx[0].rehash()
genblock.hashMerkleRoot = genblock.calc_merkle_root()
genblock.solve()
self.nodes[0].submitblock(genblock.serialize().hex())
assert_equal(self.nodes[0].getbestblockhash(), genblock.hash)
```

to ensure th
...

💬 ajtowns commented on pull request "miner: timelock the coinbase to the mined block's height":
(https://github.com/bitcoin/bitcoin/pull/32155#discussion_r2053231954)
Would it be more elegant to leave the genesis block's nSequence as `MAX_SEQUENCE` to avoid the implication that the genesis block might be timelocked? ie,

```c++
coinbase_tx.vin.resize(1);
coinbase_tx.vin[0].prevout.SetNull();
if (height > 0) {
coinbase_tx.nLockTime = static_cast<uint32_t>(height - 1);
coinbase_tx.vin[0].nSeqeunece = CTxIn::MAX_SEQUENCE_NONFINAL;
}
```

💬 ajtowns commented on pull request "miner: timelock the coinbase to the mined block's height":
(https://github.com/bitcoin/bitcoin/pull/32155#discussion_r2053297415)
Should be sorted

⚠️ whitslack opened an issue: "PIE+LTO causes Bitcoin-Qt to segfault at startup"
(https://github.com/bitcoin-core/gui/issues/867)
### Issues, reports or feature requests related to the GUI should be opened directly on the GUI repo

- [x] I still think this issue should be opened here

### Report

When PIE and LTO are combined, Qt applications crash at startup. See [Gentoo bug 954171](https://bugs.gentoo.org/954171). You can read all the gorey details about the underlying issue (that also affected Wireshark) [here](https://bugs.gentoo.org/754021). Basically, the issue is that Qt's `qcompilerdetection.h` prohibits building Q
...

✅ maflcko closed a pull request: "Update multiprocess.md"
(https://github.com/bitcoin/bitcoin/pull/32321)

💬 eli-schwartz commented on issue "PIE+LTO causes Bitcoin-Qt to segfault at startup":
(https://github.com/bitcoin-core/gui/issues/867#issuecomment-2820150044)
To be clear, the actual issue is that cmake has POSITION_INDEPENDENT_CODE, which claims to implement PIC, but it does not. It enables PIE instead, which is not what you actually asked for, and they are unfortunately different enough to matter.

It's quite fine to enforce -fPIC (and also to link with -pie), but cmake simply does not provide the necessary builtin functionality to do it correctly.

💬 maflcko commented on pull request "test: Run all benchmarks in the sanity check":
(https://github.com/bitcoin/bitcoin/pull/32310#issuecomment-2820281253)
> Why is the CI failing?

Thx, excluded msvc for now, with the same approach as the temporary win-cross exclusion.

💬 TheCharlatan commented on pull request "kernel: Separate UTXO set access from validation functions":
(https://github.com/bitcoin/bitcoin/pull/32317#discussion_r2053511978)
Added a commit.

💬 ajtowns commented on pull request "fuzz: enable running fuzz test cases in Debug mode":
(https://github.com/bitcoin/bitcoin/pull/32113#issuecomment-2820499887)
Renamed to `G_FUZZING_BUILD`. Added ryanofsky's docs. Left the constexpr optimisations in.

💬 Sjors commented on pull request "miner: timelock the coinbase to the mined block's height":
(https://github.com/bitcoin/bitcoin/pull/32155#discussion_r2053579424)
It affects any miner that uses the Mining interface, because `createNewBlock` returns a `BlockTemplate` which includes a `getBlock()` method which returns a `CBlock` that contains the coinbase transaction.

💬 Sjors commented on pull request "doc: warn that CheckBlock() underestimates sigops":
(https://github.com/bitcoin/bitcoin/pull/31624#discussion_r2053597806)
The original context was that a miner created an invalid block with too many sigops. I wanted to make sure that it was because they made an invalid patch, and not because our code is broken. I wrote this comment while trying to understand the behaviour better.

It may be a good idea to actually improve the check here, since it's relied on by `getblocktemplate` in proposal mode and by #31981. But that would be another PR imo.

💬 Sjors commented on pull request "Add checkBlock() to Mining interface":
(https://github.com/bitcoin/bitcoin/pull/31981#issuecomment-2820550469)
Note to self, figure out if the sigops check needs to be fixed here or in another PR: https://github.com/bitcoin/bitcoin/pull/31624/files#r2053597806