💬 maflcko commented on pull request "build: Require C++20 compiler":
(https://github.com/bitcoin/bitcoin/pull/28349#issuecomment-1794726325)
Rebased. This should be possible to review, and should work on all platforms, I guess, except for macOS?

Once and if this is merged, I'll follow-up with a `fs.h` cleanup.

💬 TheCharlatan commented on pull request "refactor: Simplify CTxMempool/BlockAssembler fields, remove some external mapTx access":
(https://github.com/bitcoin/bitcoin/pull/28391#discussion_r1383268650)
Yeah, I thought this would better convey its semantics, but with all the force pushes over the past two hours, it seems like it is not intuitive to work with, so I'll revert to using a pointer.

💬 fanquake commented on pull request "guix: switch to 6.1 kernel headers over 5.15":
(https://github.com/bitcoin/bitcoin/pull/28786#issuecomment-1794834244)
> I can't see anything obvious that would do that.

Sent a patch upstream, https://lists.gnu.org/archive/html/guix-patches/2023-11/msg00362.html, to see if we can get some unversioned pointers to stable/longterm added.

👍 BrandonOdiwuor approved a pull request: "wallet: cache descriptor ID to avoid repeated descriptor string creation"
(https://github.com/bitcoin/bitcoin/pull/28799#pullrequestreview-1715206734)
ACK 5e6bc6d830664a5afeb5d5bd7e7b3818a01376b7

looks good to me

🤔 glozow reviewed a pull request: "test: bugfix CheckPackageMempoolAcceptResult return all error strings"
(https://github.com/bitcoin/bitcoin/pull/28788#pullrequestreview-1715221980)
utACK 5380f055136ea99f76cd3df2c2add081852d35d0

💬 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_r1383369213)
Added comment

💬 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_r1383369301)
Taken

💬 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_r1383369984)
Changed :+1:

💬 TheCharlatan commented on pull request "refactor: Simplify CTxMempool/BlockAssembler fields, remove some external mapTx access":
(https://github.com/bitcoin/bitcoin/pull/28391#issuecomment-1794886627)
Updated 66a4c881e3387b55b26cd2a890bd0d5d522c6727 -> 995aa6b9cb5d1ea685a5d2ac6767d21b373e4c84 ([simplifyMemPoolInteractions_9](https://github.com/TheCharlatan/bitcoin/tree/simplifyMemPoolInteractions_9) -> [simplifyMemPoolInteractions_10](https://github.com/TheCharlatan/bitcoin/tree/simplifyMemPoolInteractions_10), [compare](https://github.com/TheCharlatan/bitcoin/compare/simplifyMemPoolInteractions_9..simplifyMemPoolInteractions_10))

* Addressed @maflcko's [comment](https://github.com/bitcoi
...

💬 maflcko commented on pull request "ArgsManager: support subcommand-specific options":
(https://github.com/bitcoin/bitcoin/pull/28802#issuecomment-1794891484)
tool_wallet.py fails CI

👍 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...).