💬 sipa commented on pull request "wallet: Add CoinGrinder coin selection algorithm":
(https://github.com/bitcoin/bitcoin/pull/27877#discussion_r1438429769)
I'd avoid the `std::vector::at` everywhere and use `std::vector::operator[]` instead, as `at` involves conditional branches to detect out-of-bounds access (well-predicted ones, but for a tight loop like this I do expect it to still be impactful). It's generally better to crash than to have UB due to out-of-bounds access, but inside performance-critical algorithms I think it's worth the extra review effort to make sure there are no out-of-bounds accesses.

💬 sipa commented on pull request "wallet: Add CoinGrinder coin selection algorithm":
(https://github.com/bitcoin/bitcoin/pull/27877#discussion_r1438411976)
It took me a while to figure out what the intent of each operation is. It may be worthwhile to document that here too. My belief is:
* ADD: make the next undecided transaction selected (ignoring equal-value transactions if the last one was unselected).
* SHIFT: the last selected transaction is bad, skip any configurations that select it, and ADD something after that.
* CUT: the last selected transaction is bad even when unselected, skip any configurations that select or unselect it, and ADD s
...

💬 sipa commented on pull request "wallet: Add CoinGrinder coin selection algorithm":
(https://github.com/bitcoin/bitcoin/pull/27877#discussion_r1438411775)
Perhaps this explanation of the algorithm belongs more inside the body of the function than at the top, as it's explaining the implementation and not the interface?

💬 sipa commented on pull request "wallet: Add CoinGrinder coin selection algorithm":
(https://github.com/bitcoin/bitcoin/pull/27877#discussion_r1438428556)
Given that any of the {add, shift, cut} states ends with executing an add, and the end of the add operation (the block of code on this line and below) determines what the next iteration will be, I think it would be more natural to move this block to the beginning of the while loop, before the switch case. So then it becomes a processing loop that in every iteration decides what it will be doing (cut, shift, or add), without state carried between iterations? This doesn't work for "done", but that
...

💬 sipa commented on pull request "wallet: Add CoinGrinder coin selection algorithm":
(https://github.com/bitcoin/bitcoin/pull/27877#discussion_r1438435912)
One complication with the "move decision for add/shift/cut to beginning of loop" approach is that this skipping doesn't work anymore as-is. I'd suggest turning it into a while loop that increments `utxo_pool_index` until the skip condition is no longer satisfied.

📝 SatoshiCopyright opened a pull request: "Licence MIT Original"
(https://github.com/bitcoin/bitcoin/pull/29157)
That must be the correct MIT license. Because it was taken from Satoshi Nakamoto (satoshinakam0t0@duck.com) original project Bitcoin-Blockchain.

<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain ho
...

💬 SatoshiCopyright commented on pull request "Licence MIT Original":
(https://github.com/bitcoin/bitcoin/pull/29157#issuecomment-1872446772)
That must be the correct MIT license. Because it was taken from Satoshi Nakamoto original project Bitcoin.

💬 SatoshiCopyright commented on pull request "Licence MIT Original":
(https://github.com/bitcoin/bitcoin/pull/29157#issuecomment-1872448116)
**@SatoshiNT0 _is owner of Bitcoin._**
**### **

✅ achow101 closed a pull request: "Licence MIT Original"
(https://github.com/bitcoin/bitcoin/pull/29157)

📝 achow101 locked a pull request: "Licence MIT Original"
(https://github.com/bitcoin/bitcoin/pull/29157)
That must be the correct MIT license. Because it was taken from Satoshi Nakamoto (satoshinakam0t0@duck.com) original project Bitcoin-Blockchain.

<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain ho
...

📝 darosior opened a pull request: "PoC: fuzz chainstate and block managers"
(https://github.com/bitcoin/bitcoin/pull/29158)
We don't have a fuzzing harness for our main consensus engine [0]. This PR introduces two new targets which respectively fuzz the `BlockManager` and `ChainstateManager` (process headers, blocks, reorgs and assert some invariants in doing so).

There is two main obstacles to achieving this: PoW and io. The blocks and chainstate databases can be stored in memory but blocks still need a valid proof of work and to be stored on disk. Niklas solved the first issue in #28043 by simply introducing a g
...

📝 bitnet-io opened a pull request: "Update net.h bigger TCP socket using larger buffer"
(https://github.com/bitcoin/bitcoin/pull/29159)
completely sync a 2gb prune in 8-10 hours instead of 3 days see src/net_processing.cpp also

prototype https://github.com/c4pt000/bitcoin 2 years ago testing

✅ maflcko closed a pull request: "Update net.h bigger TCP socket using larger buffer"
(https://github.com/bitcoin/bitcoin/pull/29159)

💬 furszy commented on pull request "wallet: move lock at the top of ReleaseWallet":
(https://github.com/bitcoin/bitcoin/pull/29155#discussion_r1438591791)
If this would be the issue presented in #29073, the test error logs would be showing a node crash. It wouldn't reach the `got_loading_error` check.

🤔 ismaelsadeeq reviewed a pull request: "Cluster size 2 package rbf"
(https://github.com/bitcoin/bitcoin/pull/28984#pullrequestreview-1779525637)
Concept ACK

💬 ismaelsadeeq commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1425282679)
In 671de055c8b3ff48b16b93d8389b80fead6342ac
Commit title indicate " [doc] cpfp carveout is excluded in packages "

But the diff is not a only a doc change.

💬 ismaelsadeeq commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1425551039)
In 228a5ed16586ef93e7ec5c5da3ef4df0d5fc322f
```cpp
#include <alogorithm>
```

💬 ismaelsadeeq commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1425583249)
Would be nice if this commit to be split into two, RBF utility functions and test in one commit and package rbf validation in another?

💬 andrewtoth commented on pull request "During IBD, prune as much as possible until we get close to where we will eventually keep blocks":
(https://github.com/bitcoin/bitcoin/pull/20827#issuecomment-1872545821)
ACK d298ff8b62b2624ed390c8a2f905c888ffc956ff

Ran benchmark with hyperfine:
```
hyperfine --show-output --parameter-list commit 96ec3b67a7a7f968d002e13d6fc227f69b7f07d7,d298ff8b62b2624ed390c8a2f905c888ffc956ff --setup 'git checkout {commit} && make -j$(nproc) src/bitcoind' --prepare 'sync; sudo /sbin/sysctl vm.drop_caches=3; rm -r /home/user/.bitcoin/blocks /home/user/.bitcoin/chainstate' -M 3 './src/bitcoind -dbcache=2000 -prune=2000 -printtoconsole=0 -stopatheight=800000'
```

Results w
...

💬 darosior commented on pull request "kernel: Remove dependency on CScheduler":
(https://github.com/bitcoin/bitcoin/pull/28960#discussion_r1438670874)
Technically i think this reference can be dangling at `Shutdown()` (if there is still some wallets loaded) since we don't `reset()` the `node.wallet_loader` before resetting `node.main_signals`.