💬 ismaelsadeeq commented on pull request "Fee Estimator updates from Validation Interface/CScheduler thread":
(https://github.com/bitcoin/bitcoin/pull/28368#discussion_r1386422215)
Done

💬 fanquake commented on pull request "build: remove `-bind_at_load` usage":
(https://github.com/bitcoin/bitcoin/pull/28783#issuecomment-1801614766)
Guix Build (arm64):
```bash
cf3250e32cf25ee1a75372bed2fb105fc893abd40662ed60b6376cd1f77586a1 guix-build-144deffe35b7/output/arm64-apple-darwin/SHA256SUMS.part
11a505365063b6234a8e205e3b30a5a7a9bb1b1cc6bf266b5038b18474edbd1f guix-build-144deffe35b7/output/arm64-apple-darwin/bitcoin-144deffe35b7-arm64-apple-darwin-unsigned.tar.gz
674f71f6c59e064e8f42a2f11c2406fea08873b6369b472f9662e8ee0c5567e4 guix-build-144deffe35b7/output/arm64-apple-darwin/bitcoin-144deffe35b7-arm64-apple-darwin-unsigned
...

💬 ismaelsadeeq commented on pull request "mempool: Persist with XOR":
(https://github.com/bitcoin/bitcoin/pull/28207#discussion_r1386428470)
Yes resolve, will review soon ✅

💬 dergoegge commented on issue "fuzz: Fix timeouts":
(https://github.com/bitcoin/bitcoin/issues/28812#issuecomment-1801622789)
Are the graphs for the entire corpus or just the slow inputs?

💬 maflcko commented on issue "fuzz: Fix timeouts":
(https://github.com/bitcoin/bitcoin/issues/28812#issuecomment-1801626636)
Just the slow inputs. I can add steps to reproduce, if you want.

💬 kristapsk commented on pull request "rpc: Add script verification flags to getdeploymentinfo":
(https://github.com/bitcoin/bitcoin/pull/28806#issuecomment-1801643452)
Concept ACK

💬 dergoegge commented on pull request "fuzz: Avoid timeout and bloat in fuzz targets":
(https://github.com/bitcoin/bitcoin/pull/28815#issuecomment-1801699456)
There seems to be a pattern here, maybe it makes sense to have something like a `RepeatedCallOneOf` that does the exit on bad data internally?

💬 real-or-random commented on pull request "Add ASM optimizations for MuHash3072":
(https://github.com/bitcoin/bitcoin/pull/19181#issuecomment-1801703220)
There's a lot of activity recently in GCC towards generating adc instructions on x86(_64): https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79173 This thread also has some hints at possible other implementations such as [GCC builtins](https://gcc.gnu.org/onlinedocs/gcc/Integer-Overflow-Builtins.html), which may be simpler to review.

I don't know. Checking the asm annotations is not trivial and not many people are familiar with inline asm. But on the other hand, I'm not saying that this PR is a h
...

🚀 glozow merged a pull request: "net: improves addnode / m_added_nodes logic"
(https://github.com/bitcoin/bitcoin/pull/28155)

💬 maflcko commented on pull request "fuzz: Avoid timeout and bloat in fuzz targets":
(https://github.com/bitcoin/bitcoin/pull/28815#issuecomment-1801829102)
Not sure, there are also some loops that do not pick out of several callables. Happy to push any refactoring, if someone uploads a diff, patch, or commit.

👍 dergoegge approved a pull request: "fuzz: Avoid timeout and bloat in fuzz targets"
(https://github.com/bitcoin/bitcoin/pull/28815#pullrequestreview-1720320820)
utACK fabb5046a7365af3079e6e45606d63576bc6ad12

Didn't test but the rational makes a lot of sense to me. This should be better even if it doesn't solve the timeouts.

👍 ismaelsadeeq approved a pull request: "refactor: Simplify CTxMempool/BlockAssembler fields, remove some external mapTx access"
(https://github.com/bitcoin/bitcoin/pull/28391#pullrequestreview-1720237245)
re ACK 995aa6b9cb5d1ea685a5d2ac6767d21b373e4c84

💬 ismaelsadeeq commented on pull request "refactor: Simplify CTxMempool/BlockAssembler fields, remove some external mapTx access":
(https://github.com/bitcoin/bitcoin/pull/28391#discussion_r1386552287)

Assert?
```suggestion
AssertLockHeld(cs);
indexed_transaction_set::const_iterator i = mapTx.find(hash);
```

💬 ismaelsadeeq commented on pull request "refactor: Simplify CTxMempool/BlockAssembler fields, remove some external mapTx access":
(https://github.com/bitcoin/bitcoin/pull/28391#discussion_r1386609897)
nit
https://github.com/bitcoin/bitcoin/blob/6903d8aa37e01d16aea0b53c3729f8f92c7d2bf0/src/test/miniminer_tests.cpp#L97
Can also be
```cpp
BOOST_CHECK(negative_modified_fees < feerate_zero.GetFee(pool.GetEntry(tx_mod_negative->GetHash())->GetTxSize()));
```

👍 brunoerg approved a pull request: "fuzz: Avoid timeout and bloat in fuzz targets"
(https://github.com/bitcoin/bitcoin/pull/28815#pullrequestreview-1720390505)
crACK fabb5046a7365af3079e6e45606d63576bc6ad12

Logic makes sense for me. I didn't check all the targets, but it seems we could apply it in `coincontrol` as well.

💬 maflcko commented on pull request "fuzz: Avoid timeout and bloat in fuzz targets":
(https://github.com/bitcoin/bitcoin/pull/28815#issuecomment-1801931728)
> Logic makes sense for me. I didn't check all the targets, but it seems we could apply it in `coincontrol` as well.

Yes, I was thinking about this, too. In practise it doesn't matter because `COutPoint` happens to be (de)serializes as a fixed-size byte blob, which can only fail if the fuzz provider is out of data. In that case the loop will already abort. However, it could make sense to change this for consistency as well, or to protect against the case where `COutPoint` is "dynamically" des
...

💬 brunoerg commented on pull request "fuzz: add target for `DescriptorScriptPubKeyMan`":
(https://github.com/bitcoin/bitcoin/pull/28578#issuecomment-1801958804)
Force-pushed covering more functions. Left it running for a long time and didn't see any downside.

💬 maflcko commented on pull request "Show transactions as not fully confirmed during background validation":
(https://github.com/bitcoin/bitcoin/pull/28616#discussion_r1386694325)
Did that for fun, and it prints the error message:

```
test_framework.authproxy.JSONRPCException: Wallet loading failed. Error loading wallet. Wallet requires blocks to be downloaded, and software does not currently support loading wallets while blocks are being downloaded out of order when using assumeutxo snapshots. Wallet should be able to load successfully after node sync reaches height 299 (-4)
```

Hacky diff:

```diff
diff --git a/test/functional/feature_assumeutxo.py b/test/fun
...

🚀 fanquake merged a pull request: "fuzz: Avoid timeout and bloat in fuzz targets"
(https://github.com/bitcoin/bitcoin/pull/28815)

💬 maflcko commented on issue "test: Write assumeutxo tests":
(https://github.com/bitcoin/bitcoin/issues/28648#issuecomment-1801990207)
Another idea would be to start a new wallet test. For now it could only check that it throws an error. See the example hacky diff I wrote, which will need to be moved to a new test file: https://github.com/bitcoin/bitcoin/pull/28616#discussion_r1386694325