💬 theStack commented on pull request "Test: followups to #27823":
(https://github.com/bitcoin/bitcoin/pull/28612#issuecomment-1795046156)
Light ACK 5ab6419f380cc0a8cde78b125f3eeee5fcba43ae

The refactoring changes in the first commit look correct and randomizing offset/size of the perturbation data seems to be an improvement for discovering more issues. Can't say much about the concrete random ranges chosen though, it might make sense to have someone review this that has more insight into the file format or specific cases that were in mind to be triggered (on the other hand, perfect is the enemy of good...).

💬 maflcko commented on pull request "suppressions: note that `type:ClassName::MethodName` should be used":
(https://github.com/bitcoin/bitcoin/pull/28147#issuecomment-1795056202)
Same on OpenSuse Tumbleweed:

`zypper in -y libevent-devel boost-devel awk find bison gcc-c++ libtool make autoconf automake python3 clang llvm lbzip2 patch xz curl wget htop git vim ccache && git clone https://github.com/bitcoin/bitcoin.git --depth=1 ./bitcoin-core && cd bitcoin-core && ./autogen.sh && ./configure CC=clang CXX=clang++ --with-sanitizers=fuzzer,integer,undefined --enable-fuzz && make -j $(nproc) && ./test/fuzz/test_runner.py -j 1 -l DEBUG /tmp/empty_folder/ addi
...

📝 maflcko opened a pull request: "Revert "suppressions: note that 'type:ClassName::MethodName' should be used""
(https://github.com/bitcoin/bitcoin/pull/28804)

💬 maflcko commented on pull request "Test: followups to #27823":
(https://github.com/bitcoin/bitcoin/pull/28612#issuecomment-1795075577)
lgtm ACK 5ab6419f380cc0a8cde78b125f3eeee5fcba43ae

💬 theStack commented on pull request "Improve peformance of CTransaction::HasWitness (28107 follow-up)":
(https://github.com/bitcoin/bitcoin/pull/28766#issuecomment-1795136718)
Concept ACK

💬 achow101 commented on pull request "policy: Enable full-rbf by default":
(https://github.com/bitcoin/bitcoin/pull/28132#issuecomment-1795191581)
> has this PR been decided to be merged already?

No. No decision has been made as to whether this PR will or will not be merged.

✅ achow101 closed an issue: "Write instructions on offline signing."
(https://github.com/bitcoin/bitcoin/issues/9492)

🚀 achow101 merged a pull request: "doc: Add offline signing tutorial"
(https://github.com/bitcoin/bitcoin/pull/28363)

💬 TheBlueMatt commented on issue "Wallet Missing Balances/Unspent":
(https://github.com/bitcoin/bitcoin/issues/28797#issuecomment-1795347642)
```
getwalletinfo
{
"walletname": "",
"walletversion": 169900,
"format": "bdb",
...
"keypoolsize": 1000,
"keypoolsize_hd_internal": 1000,
"paytxfee": 0.00000000,
"private_keys_enabled": true,
"avoid_reuse": false,
"scanning": false,
"descriptors": false,
"external_signer": false
}
```

💬 1440000bytes commented on issue "Wallet Missing Balances/Unspent":
(https://github.com/bitcoin/bitcoin/issues/28797#issuecomment-1795362833)
> "walletconflicts": [
> "1bd8d162be5a4b91c083866386a9e6ca0201f2c837241350855a61a7d4a9f78c"

Maybe this is the reason for incorrect balance?

💬 andrewtoth commented on pull request "cli: improve error message on multiwallet and add validation to cli-side commands":
(https://github.com/bitcoin/bitcoin/pull/26990#discussion_r1383787269)
Here we are using `std::set::find` and below on line 1141 we are using `std::set::count` to determine if an element already exists. We should use a consistent method for each. I'm not sure if C++20 is supported in this codebase yet, but we should use `std::set::contains` if it is.

🤔 andrewtoth reviewed a pull request: "cli: improve error message on multiwallet and add validation to cli-side commands"
(https://github.com/bitcoin/bitcoin/pull/26990#pullrequestreview-1715911067)
style-nit: all variable and function names should be snake_case https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#coding-style-c.

💬 TheBlueMatt commented on issue "Wallet Missing Balances/Unspent":
(https://github.com/bitcoin/bitcoin/issues/28797#issuecomment-1795856728)
Probably, but just because a transaction was RBF-bumped there shouldn't be an incorrect balance/missing unspent entry :)

🤔 katesalazar reviewed a pull request: "gui: add used balance to overview page"
(https://github.com/bitcoin-core/gui/pull/775#pullrequestreview-1715933567)
Concept ACK

💬 katesalazar commented on pull request "gui: add used balance to overview page":
(https://github.com/bitcoin-core/gui/pull/775#discussion_r1383801194)
https://bitcoin.stackexchange.com/a/38998/126793

💬 pablomartin4btc commented on pull request "cli: improve error message on multiwallet and add validation to cli-side commands":
(https://github.com/bitcoin/bitcoin/pull/26990#issuecomment-1795922777)
> style-nit: all variable and function names should be snake_case https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#coding-style-c.

Thanks for checking that, you are right, I'll fix them all ASAP.

💬 katesalazar commented on pull request "Wallet: Functions to enable adding used balance to GUI overview page":
(https://github.com/bitcoin/bitcoin/pull/28776#issuecomment-1795925965)
Concept ACK

💬 katesalazar commented on pull request "gui: add used balance to overview page":
(https://github.com/bitcoin-core/gui/pull/775#discussion_r1383815524)
> > Because the space is tiny, and the impact should be big, every word in an effective tooltip needs to pack a punch.
>
> Source: Internet

as the tooltip for the "used" line of the "balances" block,
"your current used balance" doesnt feel like a great tooltip

💬 pablomartin4btc commented on pull request "cli: improve error message on multiwallet and add validation to cli-side commands":
(https://github.com/bitcoin/bitcoin/pull/26990#discussion_r1383818109)
It makes sense, I'll use `std::set::find` instead of `std::set::count` (L#1141).

Regarding C++20. it's [not enabled](https://github.com/bitcoin/bitcoin/blob/master/configure.ac#L99) yet but perhaps will be soon (#28349). If this PR doesn't get merged first I'll update the code as you suggested so I'm leaving this conversation as unresolved, otherwise I'll do a follow-up.

💬 fanquake commented on issue "Wallet Missing Balances/Unspent":
(https://github.com/bitcoin/bitcoin/issues/28797#issuecomment-1795955354)
cc @achow101