👍 dergoegge approved a pull request: "test: bugfix CheckPackageMempoolAcceptResult return all error strings"
(https://github.com/bitcoin/bitcoin/pull/28788#pullrequestreview-1715285547)
utACK 5380f055136ea99f76cd3df2c2add081852d35d0

🚀 fanquake merged a pull request: "test: bugfix CheckPackageMempoolAcceptResult return all error strings"
(https://github.com/bitcoin/bitcoin/pull/28788)

💬 murchandamus commented on pull request "validation: return more helpful results for reconsiderable fee failures and skipped transactions":
(https://github.com/bitcoin/bitcoin/pull/28785#discussion_r1383433511)
It was not obvious to me what the motivation for `SINGLE` in the constant name here is. After discussing with @glozow, maybe:

```suggestion
TX_RECONSIDERABLE, //!< mining score was insufficient to meet some policy, but transaction might be acceptable if submitted in a (different) package
```

💬 Zero-1729 commented on pull request "doc: Add offline signing tutorial":
(https://github.com/bitcoin/bitcoin/pull/28363#discussion_r1383445833)
Minor capitalization nit.

```suggestion
1. Unlock the `offline_wallet` with the passphrase:
```

💬 Zero-1729 commented on pull request "doc: Add offline signing tutorial":
(https://github.com/bitcoin/bitcoin/pull/28363#discussion_r1383446166)
Minor capitalization nit.

```suggestion
* `offline` host which is disconnected from all networks (Internet, Tor, wifi, Bluetooth, etc.) and does not have, or need, a copy of the blockchain.
```

💬 Zero-1729 commented on pull request "doc: Add offline signing tutorial":
(https://github.com/bitcoin/bitcoin/pull/28363#discussion_r1383447044)
Minor hyphenation nit.

```suggestion
Maintaining an air gap between private keys and any network connections drastically reduces the opportunity for those keys to be exfiltrated from the user.
```

🤔 Zero-1729 reviewed a pull request: "doc: Add offline signing tutorial"
(https://github.com/bitcoin/bitcoin/pull/28363#pullrequestreview-1715377723)
ACK 3c208cc05ea9efb145c956e70f80efd8b027ff33

Great work!

💬 glozow commented on pull request "validation: return more helpful results for reconsiderable fee failures and skipped transactions":
(https://github.com/bitcoin/bitcoin/pull/28785#discussion_r1383454127)
Yeah renamed, I think it'll help make the connection with `m_reconsiderable_rejects` filter later as well.

💬 instagibbs commented on pull request "validation: return more helpful results for reconsiderable fee failures and skipped transactions":
(https://github.com/bitcoin/bitcoin/pull/28785#issuecomment-1795002089)
ACK https://github.com/bitcoin/bitcoin/pull/28785/commits/0150e860863f95b18448e7b67f5db27017660670

suggested changes plus `TX_RECONSIDERABLE` renaming which makes sense

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