💬 theuni commented on pull request "kernel: De-globalize validation caches":
(https://github.com/bitcoin/bitcoin/pull/30141#discussion_r1657394662)
Oh, I guess the mutex takes care of that for us. Still, wouldn't hurt to be explicit.

💬 murchandamus commented on pull request "Refactor BnB tests":
(https://github.com/bitcoin/bitcoin/pull/29532#discussion_r1622850893)
Thanks, fixed

💬 murchandamus commented on pull request "Refactor BnB tests":
(https://github.com/bitcoin/bitcoin/pull/29532#discussion_r1657289988)
Made all four tests output messages that indicate what went wrong in the success case.

💬 murchandamus commented on pull request "Refactor BnB tests":
(https://github.com/bitcoin/bitcoin/pull/29532#discussion_r1629892844)
Removed the static keyword and the clear as suggested.

💬 murchandamus commented on pull request "Refactor BnB tests":
(https://github.com/bitcoin/bitcoin/pull/29532#discussion_r1657397165)
Amended all the functions to directly create OutputGroups instead.

💬 murchandamus commented on pull request "Refactor BnB tests":
(https://github.com/bitcoin/bitcoin/pull/29532#discussion_r1622850804)
I have removed `nInput` from the function parameters of `MakeCoin(…)`.

💬 murchandamus commented on pull request "Refactor BnB tests":
(https://github.com/bitcoin/bitcoin/pull/29532#discussion_r1622850394)
I was able to remove all except for

```cpp
#include <consensus/amount.h>
#include <wallet/coinselection.h>
#include <wallet/test/wallet_test_fixture.h>

#include <boost/test/unit_test.hpp>
```

I’m not familiar with IOWY and could not turn up anything with a websearch, what is that?

💬 murchandamus commented on pull request "Refactor BnB tests":
(https://github.com/bitcoin/bitcoin/pull/29532#discussion_r1622920206)
Thanks, fixed

💬 murchandamus commented on pull request "Refactor BnB tests":
(https://github.com/bitcoin/bitcoin/pull/29532#discussion_r1622920679)
Thanks, fixed

👍 1alexbc1 approved a pull request: "test: Added coverage to Block not found error using gettxoutsetinfo"
(https://github.com/bitcoin/bitcoin/pull/30340#pullrequestreview-2145858736)
Amadeevantv yimelaja

💬 ariard commented on issue "Brainstorm: Transaction issuer-selected policy limits":
(https://github.com/bitcoin/bitcoin/issues/29454#issuecomment-2195109286)
> The problem with policy limits is they're all based on asking miners to ignore transactions that are potentially profitable. This is similar in spirit to opt-in RBF. And as we've seen, the supermajority of hash power eventually decided to ignore the opt-in flag and just do RBF all the time. If push came to shove, it wouldn't be surprising to run into the same class of problem with policy limits.

> Much better to focus on solutions like replace-by-fee-rate that rely on better aligning miner
...

💬 furszy commented on pull request "Refactor BnB tests":
(https://github.com/bitcoin/bitcoin/pull/29532#discussion_r1657420121)
> I’m not familiar with IOWY and could not turn up anything with a websearch, what is that?

Upps, the acronym is IWYU from include-what-you-use. I wrote it fast as IOWY "include only what you use".
Should stop using acronyms..

💬 Sjors commented on pull request "Remove the legacy wallet and BDB dependency":
(https://github.com/bitcoin/bitcoin/pull/28710#issuecomment-2195114617)
You can also drop the `BerkeleyDatabaseSanityCheck` related suppression in `contrib/devtools/check-devs.sh`

💬 darosior commented on pull request "fuzz: a new target for the coins database":
(https://github.com/bitcoin/bitcoin/pull/28216#issuecomment-2195118674)
Alright, i can't reproduce the non-determinism he observed.

🤔 furszy reviewed a pull request: "Wallet: Add `max_tx_weight` to transaction funding options (take 2)"
(https://github.com/bitcoin/bitcoin/pull/29523#pullrequestreview-2145904445)
utACK 734076c6de1781f957c8bc3bf7ed6951920cfcf6

💬 furszy commented on pull request "Wallet: Add `max_tx_weight` to transaction funding options (take 2)":
(https://github.com/bitcoin/bitcoin/pull/29523#discussion_r1657436891)
In 734076c6de178:

nano nit: shorter
```suggestion
if (*coin_selection_params.m_max_tx_weight < minimum_tx_weight || *coin_selection_params.m_max_tx_weight > MAX_STANDARD_TX_WEIGHT) {
```

💬 mjdietzx commented on pull request "descriptors: Be able to specify change and receiving in a single descriptor string":
(https://github.com/bitcoin/bitcoin/pull/22838#issuecomment-2195192807)
Tested ACK cb41e26eef3a92a7704a0e01eb9b272f7ed78eeb. Note: I did not do a thorough code review

What I've tested:
- `getdescriptorinfo`, `importdescriptors`, `listdescriptors` with a miniscript wallet similar to [#29156](https://github.com/bitcoin/bitcoin/pull/29156). All works as expected
- import this wallet from file to Coldcard Mk4 (Edge FW 6.2.2 w/ miniscript support)
- `<0;1>` notation works properly as Coldcard has already adopted this convention
- Coldcard also signs psbt prope
...

📝 ternera opened a pull request: "Fixed grammar in the README"
(https://github.com/bitcoin/bitcoin/pull/30351)
I made some minor changes to the project by cleaning up grammar and punctuation mistakes in the README.

This will make it easier for people to read it and increase the professionalism of repo since the README is the first thing people see.

🤔 maflcko reviewed a pull request: "Fixed grammar in the README"
(https://github.com/bitcoin/bitcoin/pull/30351#pullrequestreview-2145972278)
Closing for now. Not sure those changes are an improvement. Thus, it may be too controversial.

💬 maflcko commented on pull request "Fixed grammar in the README":
(https://github.com/bitcoin/bitcoin/pull/30351#discussion_r1657477704)
not sure this is an improvement.