💬 brunoerg commented on pull request "fuzz: a target for the block index database":
(https://github.com/bitcoin/bitcoin/pull/28209#discussion_r1441762014)
nit (feel free to ignore):

```diff
diff --git a/src/test/fuzz/block_index.cpp b/src/test/fuzz/block_index.cpp
index 7885eda747..bc073e4930 100644
--- a/src/test/fuzz/block_index.cpp
+++ b/src/test/fuzz/block_index.cpp
@@ -93,8 +93,8 @@ FUZZ_TARGET(block_index, .init = init_block_index)

// We should be able to read every block file info we stored. Its value should correspond to
// what we stored above.
- CBlockFileInfo info;
for (const auto& [n, file_info]: files_
...

👍 brunoerg approved a pull request: "fuzz: a target for the block index database"
(https://github.com/bitcoin/bitcoin/pull/28209#pullrequestreview-1804183397)
crACK 8083aa21a699cb240a11741be8ba5cfeeb58ee8b

left some nits

🤔 furszy reviewed a pull request: "sqlite: Disallow writing from multiple `SQLiteBatch`s"
(https://github.com/bitcoin/bitcoin/pull/29112#pullrequestreview-1804185742)
> I don't think this particular issue is a problem for us however. The fact that it deadlocks in this particular test case is probably a good thing since it means that BDB is handling locking internally. If there were two threads trying to read/write at the same time, BDB would force one thread to wait for the other, as this PR is doing for SQLite. That's the behavior that we want. It's only a deadlock (and test failure) since the test is trying to do the behavior of two threads within one threa
...

💬 josibake commented on pull request "wallet, rpc: `FundTransaction` refactor":
(https://github.com/bitcoin/bitcoin/pull/28560#discussion_r1441785861)
noted! will update

💬 josibake commented on pull request "wallet, rpc: `FundTransaction` refactor":
(https://github.com/bitcoin/bitcoin/pull/28560#discussion_r1441789986)
I initially started putting these tests in `wallet_basic.py`, but I'd prefer if we have RPC-specific test files and move all of the `sendmany` specific tests into the new `wallet_sendmany.py` file in a follow-up PR.

💬 darosior commented on pull request "fuzz: a target for the block index database":
(https://github.com/bitcoin/bitcoin/pull/28209#discussion_r1441791286)
Why?

💬 real-or-random commented on pull request "Update libsecp256k1 subtree for 0.4.1 release":
(https://github.com/bitcoin/bitcoin/pull/29169#issuecomment-1877165550)

> What do you mean by/where are we reporting the version number?

Plus, `./configure --help` in the subtree mentions `libsecp256k1 0.4.2-dev [...]`, but that's really a minor thing.

💬 maflcko commented on issue "ubsan: misaligned-pointer-use in crc32c/src/crc32c_arm64.cc":
(https://github.com/bitcoin/bitcoin/issues/29178#issuecomment-1877178197)
Same with `./ci/test/00_setup_env_native_asan.sh`.

I somehow assumed that this was fixed by:

* https://github.com/bitcoin/bitcoin/pull/27360
* https://github.com/bitcoin/bitcoin/pull/27298
* https://github.com/bitcoin/bitcoin/pull/27363

💬 crediblebytes commented on pull request "doc: revert clarify -datacarriersize":
(https://github.com/bitcoin/bitcoin/pull/29173#issuecomment-1877187993)
> The help text and documentation is not generally a statement of requirements

That is a shame. What the software does and more importantly what the software doesn't do is the very role of software requirements. So first of all get the act together with best software practices. You have a descriptive argument called datacarriersize that doesn't really leave room for debate on what it is supposed to do. It isn't called scriptPubKeySize. However the behavior of this variable was changed withou
...

💬 darosior commented on pull request "fuzz: a target for the block index database":
(https://github.com/bitcoin/bitcoin/pull/28209#discussion_r1441828968)
Heh, no that's a good point. There is a bunch happening in CDBWrapper's constructor so i'm sure this would reduce the time we spend on uninteresting inputs.

💬 brunoerg commented on pull request "fuzz: a target for the block index database":
(https://github.com/bitcoin/bitcoin/pull/28209#discussion_r1441845813)
Nevermind, my bad. It's better to reuse the same variable.

💬 achow101 commented on pull request "sqlite: Disallow writing from multiple `SQLiteBatch`s":
(https://github.com/bitcoin/bitcoin/pull/29112#discussion_r1441855248)
Done

💬 fanquake commented on pull request "Update libsecp256k1 subtree to current master":
(https://github.com/bitcoin/bitcoin/pull/29169#issuecomment-1877229355)
I've updated to commit message to make it clear that this isn't exactly the 0.4.1 tag.

> Plus, ./configure --help in the subtree mentions

I was thinking more like output from bitcoind or similar.

💬 fanquake commented on issue "new crash in v26.0":
(https://github.com/bitcoin/bitcoin/issues/29153#issuecomment-1877232062)
cc @hebasto

💬 kevkevinpal commented on pull request "test: Use test framework utils in functional tests":
(https://github.com/bitcoin/bitcoin/pull/28528#discussion_r1441859824)
```suggestion
assert_equal(txid, utxo["txid"])
```

💬 maflcko commented on issue "ubsan: misaligned-pointer-use in crc32c/src/crc32c_arm64.cc":
(https://github.com/bitcoin/bitcoin/issues/29178#issuecomment-1877240437)
I re-tried commit 7b45d171f549595a831489827c28e8493f36c00c and at least Asan is passing. Fuzz and Tsan are failing for different reasons.

💬 darosior commented on pull request "fuzz: a target for the block index database":
(https://github.com/bitcoin/bitcoin/pull/28209#discussion_r1441867954)
Ok, looks like i'm wrong. It doesn't make any difference in runtime when running my target over my local corpus. I'll hold off making this change then.

👍 hebasto approved a pull request: "Update libsecp256k1 subtree to current master"
(https://github.com/bitcoin/bitcoin/pull/29169#pullrequestreview-1804351648)
re-ACK e2cdeb592596432039d21f4c819d45f1e46d65ef

💬 murchandamus commented on pull request "wallet: Add CoinGrinder coin selection algorithm":
(https://github.com/bitcoin/bitcoin/pull/27877#issuecomment-1877253041)
After discussing Pieter’s review comments further with him, I decided to restructure the algorithm proposed in this PR. I’m returning the PR to draft for a few days while I rewrite the commits.

📝 murchandamus converted_to_draft a pull request: "wallet: Add CoinGrinder coin selection algorithm"
(https://github.com/bitcoin/bitcoin/pull/27877)
***Please refer to the [topic on Delving Bitcoin](https://delvingbitcoin.org/t/gutterguard-and-coingrinder-simulation-results/279) discussing Gutter Guard/Coingrinder simulation results.***

Adds a coin selection algorithm that minimizes the weight of the input set while creating change.

Motivations
---

- At high feerates, using unnecessary inputs can significantly increase the fees
- Users are upset when fees are relatively large compared to the amount sent
- Some users struggle to m
...