💬 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.

✅ maflcko closed a pull request: "Fixed grammar in the README"
(https://github.com/bitcoin/bitcoin/pull/30351)

💬 maflcko commented on pull request "Fixed grammar in the README":
(https://github.com/bitcoin/bitcoin/pull/30351#issuecomment-2195220001)
If you are looking for other stuff to contribute to, see:

## Getting started to contribute to Bitcoin Core

### Setting up your development environment

New developers are very welcome and needed. There are a lot of open issues of any difficulty waiting to be fixed. However, before you start contributing, familiarize yourself with the Bitcoin Core build system and tests. Refer to the documentation in the repository on how to build Bitcoin Core and how to run the unit and functional tests.
...

💬 ternera commented on pull request "Fixed grammar in the README":
(https://github.com/bitcoin/bitcoin/pull/30351#discussion_r1657481230)
Definitely a typo. Shall I fix it and open a new PR?

💬 murchandamus commented on pull request "Refactor BnB tests":
(https://github.com/bitcoin/bitcoin/pull/29532#discussion_r1657406844)
Should be snake_case.

💬 murchandamus commented on pull request "Refactor BnB tests":
(https://github.com/bitcoin/bitcoin/pull/29532#discussion_r1657411032)
Remove spaces after comments describing params.

💬 murchandamus commented on pull request "Refactor BnB tests":
(https://github.com/bitcoin/bitcoin/pull/29532#discussion_r1657408296)
`nInput` is unused, let’s remove it.

💬 murchandamus commented on pull request "Refactor BnB tests":
(https://github.com/bitcoin/bitcoin/pull/29532#discussion_r1657411595)
Outdated comment, it’s an OutputGroup not coin now.

💬 murchandamus commented on pull request "Refactor BnB tests":
(https://github.com/bitcoin/bitcoin/pull/29532#discussion_r1657415795)
```suggestion
const auto result = SelectCoinsBnB(utxo_pool, selection_target, /*cost_of_change=*/default_cs_params.m_cost_of_change, /*max_weight=*/MAX_STANDARD_TX_WEIGHT);
```

💬 murchandamus commented on pull request "Refactor BnB tests":
(https://github.com/bitcoin/bitcoin/pull/29532#discussion_r1657413647)
Should be a reference.

```suggestion
static void TestBnBSuccess(std::string test_title, std::vector<OutputGroup>& utxo_pool, const CAmount& selection_target, const std::vector<CAmount>& expected_input_amounts, const CoinSelectionParams& cs_params = default_cs_params)
```

💬 murchandamus commented on pull request "Refactor BnB tests":
(https://github.com/bitcoin/bitcoin/pull/29532#discussion_r1657425209)
remove spaces

💬 murchandamus commented on pull request "Refactor BnB tests":
(https://github.com/bitcoin/bitcoin/pull/29532#discussion_r1657416339)
Remove spaces after param comments

💬 murchandamus commented on pull request "Refactor BnB tests":
(https://github.com/bitcoin/bitcoin/pull/29532#discussion_r1657423140)
```suggestion
CoinSelectionParams high_feerate_params = init_default_params();
high_feerate_params.m_effective_feerate = CFeeRate{25'000};
std::vector<OutputGroup> high_feerate_pool; // 25 sat/vB (greater than long_term_feerate of 10 sat/vB)
```