💬 instagibbs commented on pull request "RPC: Add maxfeerate and maxburnamount args to submitpackage":
(https://github.com/bitcoin/bitcoin/pull/28950#discussion_r1495839770)
to make sure I'm not misreading, this is a bump of the default feerate value by 10x? Making sure since I don't see any tests that had to be changed.

💬 Sjors commented on pull request "Support self-hosted Cirrus workers on forks (and multi-user)":
(https://github.com/bitcoin/bitcoin/pull/29274#issuecomment-1954241752)
Rebased after #29441 landed which contains two commits from this PR.

💬 instagibbs commented on pull request "rpc: Fixed signed integer overflow for large feerates":
(https://github.com/bitcoin/bitcoin/pull/29434#discussion_r1495847533)
I'm likely misreading this, but `DEFAULT_MAX_RAW_TX_FEE_RATE` doesn't seem to actually be used anywhere except `PSBTOperationsDialog::broadcastTransaction()`?

👋 maflcko's pull request is ready for review: "lint: Check for missing bitcoin-config.h includes"
(https://github.com/bitcoin/bitcoin/pull/29408)

💬 instagibbs commented on pull request "RPC: Add maxfeerate and maxburnamount args to submitpackage":
(https://github.com/bitcoin/bitcoin/pull/28950#discussion_r1495861406)
taken, thanks

💬 instagibbs commented on pull request "RPC: Add maxfeerate and maxburnamount args to submitpackage":
(https://github.com/bitcoin/bitcoin/pull/28950#issuecomment-1954265365)
rebased

💬 fjahr commented on pull request "test: use v2 everywhere for P2PConnection if --v2transport is enabled":
(https://github.com/bitcoin/bitcoin/pull/29358#issuecomment-1954281008)
tACK bf5662c678455fb47c402f8520215726ddfe7a94

💬 maflcko commented on pull request "lint: Check for missing bitcoin-config.h includes":
(https://github.com/bitcoin/bitcoin/pull/29408#issuecomment-1954281330)
Added the check to error on redundant (harmless) includes as well, since it was trivial. Should be trivial to re-ACK.

💬 maflcko commented on pull request "lint: Check for missing bitcoin-config.h includes":
(https://github.com/bitcoin/bitcoin/pull/29408#issuecomment-1954286731)
Rebased, for silent merge conflict in `src/bench/wallet_ismine.cpp`. I guess this immediately shows that the check is useful :)

💬 fanquake commented on pull request "serialization: c++20 endian/byteswap/clz modernization":
(https://github.com/bitcoin/bitcoin/pull/29263#issuecomment-1954289542)
@theuni want to rebase this now that #29404 is in?

💬 sipa commented on pull request "p2p: Don't process mutated blocks":
(https://github.com/bitcoin/bitcoin/pull/29412#discussion_r1495884531)
Since this "in case of non-coinbase first transaction, treat existence of 64-byte transaction as malleation" is new behavior, would it make sense to move it to a separate commit at the end of the PR? (A clearer comment would also be helpful, because I briefly thought this was a consensus change before it was pointed out to be it only triggers in case the block would already be invalid).

💬 Sjors commented on pull request "Silent Payments: Implement BIP352":
(https://github.com/bitcoin/bitcoin/pull/28122#discussion_r1495894984)
Moving to `bip352.h` sounds good too.

✅ so7ow closed an issue: "When selecting a custom data directory on first launch of Bitcoin Core GUI on Mac, where is that setting stored?"
(https://github.com/bitcoin-core/gui/issues/798)

💬 so7ow commented on issue "When selecting a custom data directory on first launch of Bitcoin Core GUI on Mac, where is that setting stored?":
(https://github.com/bitcoin-core/gui/issues/798#issuecomment-1954310379)
Thank you! Confirmed this works:

```
rm ~/Library/Preferences/org.bitcoin.Bitcoin-Qt.plist
killall -u YourUserName cfprefsd
```

💬 instagibbs commented on pull request "p2p: Don't process mutated blocks":
(https://github.com/bitcoin/bitcoin/pull/29412#discussion_r1495954173)
I also had the same misconception the first time I reviewed.

💬 fjahr commented on pull request "assumeutxo: Get rid of faked nTx and nChainTx values":
(https://github.com/bitcoin/bitcoin/pull/29370#discussion_r1495983754)
CI failed on this line: https://cirrus-ci.com/task/5549867297144832?logs=ci#L2880

📝 paplorinc opened a pull request: "benchmark/speedup: optimize TryParseHex calls considerably"
(https://github.com/bitcoin/bitcoin/pull/29458)

💬 fjahr commented on pull request "assumeutxo: Get rid of faked nTx and nChainTx values":
(https://github.com/bitcoin/bitcoin/pull/29370#issuecomment-1954438224)
Concept ACK, looks good at first sight, will do a deeper review when it's clear if the CI failure is an issue. I couldn't reproduce it locally so far...

💬 maflcko commented on pull request "assumeutxo: Get rid of faked nTx and nChainTx values":
(https://github.com/bitcoin/bitcoin/pull/29370#issuecomment-1954448961)
To reproduce locally, you'll have to compile with the `integer` sanitizer (not to be confused by the `undefined` sanitizer)

💬 ryanofsky commented on pull request "assumeutxo: Get rid of faked nTx and nChainTx values":
(https://github.com/bitcoin/bitcoin/pull/29370#issuecomment-1954455499)
CI failure is just a preexisting bug in a line of code that doesn't affect anything:

https://github.com/bitcoin/bitcoin/blob/45b2a91897ca8f4a3e0c1adcfb30cf570971da4e/src/rpc/blockchain.cpp#L1658

Two `nChainTx` values in the `getchaintxstats` are subtracted and this triggers the undefined behavior sanitizer because they are unsigned and the result is negative. `getchaintxstats` has a number of edge case problems like this that should be cleaned up but don't really affect anything. I'll prob
...