💬 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)
```

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

💬 ismaelsadeeq commented on pull request "Fee Estimation: change `estimatesmartfee` default mode to `economical`":
(https://github.com/bitcoin/bitcoin/pull/30275#discussion_r1657504370)
I added a commit, that add more information about the modes.

On master b27afb7fb774809c9c075d56e44657e0b607a00c
This is the help output
```
2. estimate_mode (string, optional, default="conservative") The fee estimate mode.
Whether to return a more conservative estimate which also satisfies
a longer history. A conservative estimate potentially returns a
higher feerate and is more likely to be sufficient for the desired

...

💬 sipa commented on pull request "Several randomness improvements":
(https://github.com/bitcoin/bitcoin/pull/29625#issuecomment-2195282495)
I've addressed the outstanding comments, I believe, including:
* Rename `allow_deterministic` to `always_use_real_rng` (with inverted meaning).
* Dropped `GetRand()` (but not `GetRand(nRange)`), inlining it instead.
* Removed `GetOSRand` from the public random.h interface, and moved it into the anonymous namespace.
* Fixed/improved a few comments
* Dropped most of the specializations in `randbits<BITS>`.

💬 sipa commented on pull request "Several randomness improvements":
(https://github.com/bitcoin/bitcoin/pull/29625#discussion_r1657537328)
Done.