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

💬 sipa commented on pull request "Several randomness improvements":
(https://github.com/bitcoin/bitcoin/pull/29625#discussion_r1657539130)
Moved `GetOSRand` to the anonymous namespace (meaning it can stay in its relative location in random.cpp, reducing the diff size), and dropped it from random.h.

💬 sipa commented on pull request "Several randomness improvements":
(https://github.com/bitcoin/bitcoin/pull/29625#discussion_r1657539323)
Done, in a new commit.

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