💬 murchandamus commented on pull request "wallet: Add CoinGrinder coin selection algorithm":
(https://github.com/bitcoin/bitcoin/pull/27877#discussion_r1446141597)
Good idea.

I’ve added ```auto curr_tail = curr_selection.back();``` to reference that instead throughout the evaluation block.

💬 murchandamus commented on pull request "wallet: Add CoinGrinder coin selection algorithm":
(https://github.com/bitcoin/bitcoin/pull/27877#discussion_r1446479218)
Thanks, I’ve adopted your proposed change.

For context on the `!best_selection.empty())`: we need to check whether we have found any solution so far, as we would otherwise no longer trigger the "max weight exceeded" failure message.

💬 murchandamus commented on pull request "wallet: Add CoinGrinder coin selection algorithm":
(https://github.com/bitcoin/bitcoin/pull/27877#discussion_r1446151655)
Ah, oops. Yes of course.

💬 murchandamus commented on pull request "wallet: Add CoinGrinder coin selection algorithm":
(https://github.com/bitcoin/bitcoin/pull/27877#discussion_r1446094975)
Thanks, I went with

```// EVALUATE current selection: check for solutions and determine whether we can CUT or SHIFT before EXPLORING further```

💬 murchandamus commented on pull request "wallet: Add CoinGrinder coin selection algorithm":
(https://github.com/bitcoin/bitcoin/pull/27877#discussion_r1446186725)
Thanks, I did some hard staring at this while-loop. I restructured it to use a `break` as you suggested, and was able to reduce the loop’s condition to:

```diff
- while (!should_shift && next_utxo > 0
- && (curr_selection.empty() || curr_selection.back() != next_utxo - 1)
- && utxo_pool[next_utxo - 1].GetSelectionAmount() == utxo_pool[next_utxo].GetSelectionAmount()
+ while (utxo_pool[next_utxo - 1].GetSelectionAmount() == utxo_p
...

💬 jamesob commented on pull request "fuzz: Test headers pre-sync through p2p interface":
(https://github.com/bitcoin/bitcoin/pull/28043#discussion_r1446512814)
Should probably have some belt-and-suspenders check that we're 100% not running on mainnet, maybe based on the contents of `params` and whatever applicable global.

🤔 sr-gi reviewed a pull request: "p2p: Fill reconciliation sets (Erlay)"
(https://github.com/bitcoin/bitcoin/pull/28765#pullrequestreview-1811832956)
Some additional comments after the recent optimizations + caching

💬 sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay)":
(https://github.com/bitcoin/bitcoin/pull/28765#discussion_r1446503850)
nit: `targes` -> `targets`

💬 sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay)":
(https://github.com/bitcoin/bitcoin/pull/28765#discussion_r1446517082)
nit: I'd be nice to define this as a constant

💬 sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay)":
(https://github.com/bitcoin/bitcoin/pull/28765#discussion_r1446518794)
I don't think this is right(?), this logic is never triggered.

You are checking whether the size of `tx_fanout_targes_cache_order` is over the limit, but you are only adding elements to the queue within that same `if` context, meaning that the queue is always empty.

👍 alfonsoromanz approved a pull request: "doc: Add example of mixing private and public keys in descriptors"
(https://github.com/bitcoin/bitcoin/pull/28373#pullrequestreview-1811870092)
ACK

💬 josibake commented on pull request "wallet, rpc: `FundTransaction` refactor":
(https://github.com/bitcoin/bitcoin/pull/28560#issuecomment-1883691930)
Updated https://github.com/bitcoin/bitcoin/commit/909c6bb1dbc6f60c5ad3200d83f3fce98f4c3706 -> https://github.com/bitcoin/bitcoin/commit/c856a1093009c6098c7c9fed5cc3f03fd0c0cf5b ([compare](https://github.com/josibake/bitcoin/compare/909c6bb1dbc6f60c5ad3200d83f3fce98f4c3706..c856a1093009c6098c7c9fed5cc3f03fd0c0cf5b))

* Moved `CTxDestination`, `Amount` into loop
* Refactored `InterpretSubtractFeeFromAmountInstructions` to infer parsing logic based on the instruction type (int, bool, str)

Tha
...

💬 josibake commented on pull request "wallet, rpc: `FundTransaction` refactor":
(https://github.com/bitcoin/bitcoin/pull/28560#discussion_r1446538974)
Fixed

💬 josibake commented on pull request "wallet, rpc: `FundTransaction` refactor":
(https://github.com/bitcoin/bitcoin/pull/28560#discussion_r1446539102)
Done

💬 josibake commented on pull request "wallet, rpc: `FundTransaction` refactor":
(https://github.com/bitcoin/bitcoin/pull/28560#discussion_r1446539347)
Done

💬 josibake commented on pull request "wallet, rpc: `FundTransaction` refactor":
(https://github.com/bitcoin/bitcoin/pull/28560#discussion_r1446539625)
Fixed

💬 instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1446545360)
done

💬 instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1446545395)
left a comment for future work

👍 alfonsoromanz approved a pull request: "getrawtransaction implementation"
(https://github.com/bitcoin-core/gui/pull/777#pullrequestreview-1811945070)
Tested ACK https://github.com/bitcoin-core/gui/pull/777/commits/9659890f7c9223d85dda3d066c2e3de42cabe7cf


💬 maflcko commented on pull request "rpc,rest,zmq: faster getblock, NotifyBlock and rest_block by reading raw block":
(https://github.com/bitcoin/bitcoin/pull/26415#discussion_r1446601858)
```
node0 stderr node/blockstorage.cpp:1070:15: runtime error: unsigned integer overflow: 0 - 8 cannot be represented in type 'unsigned int'
#0 0x561baaf1fb61 in node::BlockManager::ReadRawBlockFromDisk(std::vector<unsigned char, std::allocator<unsigned char>>&, FlatFilePos const&) const src/node/blockstorage.cpp:1070:15
#1 0x561bab1a45bf in GetRawBlockChecked(node::BlockManager&, CBlockIndex const&) src/rpc/blockchain.cpp:610:19
#2 0x561bab1a45bf in getblock()::$_0::operator()(
...