🤔 stratospher reviewed a pull request: "test: p2p: check disconnect due to lack of desirable service flags"
(https://github.com/bitcoin/bitcoin/pull/29279#pullrequestreview-1848022516)
tested ACK ff5f994.

💬 stratospher commented on pull request "test: p2p: check disconnect due to lack of desirable service flags":
(https://github.com/bitcoin/bitcoin/pull/29279#discussion_r1469196012)
ff5f994: nit: `NODE_NONE` here too?

💬 theDavidCoen commented on pull request "set `DEFAULT_PERMIT_BAREMULTISIG` to false":
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1914214959)
I keep reading ideological points and no one insists on a very basic thing:
consistency.
This PR should be merged because:
1. The code has been tested and it works.
2. Flags that activate a feature should be always set to false. The Bitcoin users (node operators) should be in charge and establish their node's policies actively.

An example:
with version 26.0 we now have the ability to activate v2 transport Protocol, which is a great improvement for Bitcoin.
In order to do that, the users
...

💬 dexX7 commented on pull request "Make provably unsignable standard P2PK and P2MS outpoints unspendable.":
(https://github.com/bitcoin/bitcoin/pull/28400#issuecomment-1914218855)
Concept ACK.

Note, however, that pubkeys can be modified to be valid and still be used to store data, for example by using a byte that is shuffled until the pubkey becomes valid.

💬 naumenkogs commented on pull request "p2p: adaptive connections services flags":
(https://github.com/bitcoin/bitcoin/pull/28170#discussion_r1469260564)
9f36e591c551ec2e58a6496334541bfdae8fdfe5

i see that this is a copy-paste from another place, but since it's technically green (and i want to sure it still makes sense after your change), can you explain what is this about? especially `the returned desirable service flags are guaranteed to not change dependent on state`

👍 naumenkogs approved a pull request: "p2p: adaptive connections services flags"
(https://github.com/bitcoin/bitcoin/pull/28170#pullrequestreview-1848123617)
ACK 27f260aa6e04f82dad78e9a06d58927546143a27

💬 naumenkogs commented on pull request "p2p: adaptive connections services flags":
(https://github.com/bitcoin/bitcoin/pull/28170#discussion_r1469267141)
6ed53602ac7c565273b5722de167cb2569a0e381

if you touch it again, i suggest removing `GetServicesFlagsIBDCache` definition here rather than in the last commit. At this point it's already unused. Saves some review time.

💬 naumenkogs commented on pull request "p2p: adaptive connections services flags":
(https://github.com/bitcoin/bitcoin/pull/28170#discussion_r1469274352)
aff7d92b1500e2478ce36a7e86ae47df47dda178

would be nice to test reacting to reorgs.

💬 glozow commented on pull request "test: ensure output is large enough to pay for its fees":
(https://github.com/bitcoin/bitcoin/pull/29283#discussion_r1469286220)
Does it need to be cast to an `int` maybe?

💬 Sjors commented on pull request "Stratum v2 Template Provider (take 2)":
(https://github.com/bitcoin/bitcoin/pull/28983#issuecomment-1914277528)
(I plan to rebase this after some other stuff is merged)

💬 vostrnad commented on pull request "set `DEFAULT_PERMIT_BAREMULTISIG` to false":
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1914287655)
@theDavidCoen "Default off everything" is not at all how Bitcoin Core development works. If it did, the node would by default not communicate with the network (`-networkactive`), broadcast wallet transactions (`-walletbroadcast`), persist the mempool between restarts (`-persistmempool`) etc. It would also mean you can just rename an option (e.g. `-permitbaremultisig` to `-disablebaremultisig`) to force the opposite default.

How it actually (usually) works is the default parameters are changed
...

💬 glozow commented on pull request "Wallet: Add `maxfeerate` wallet startup option":
(https://github.com/bitcoin/bitcoin/pull/29278#discussion_r1469291356)
Maybe add tests where you (successfully) configure the node's wallet using `-maxfeerate`?

💬 glozow commented on pull request "Wallet: Add `maxfeerate` wallet startup option":
(https://github.com/bitcoin/bitcoin/pull/29278#discussion_r1469296961)
1000 is redundant

💬 glozow commented on pull request "Wallet: Add `maxfeerate` wallet startup option":
(https://github.com/bitcoin/bitcoin/pull/29278#discussion_r1469293744)
Similarly, it would be good to test that both maxfee and maxfeerate requirements must be met.

💬 Sjors commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#issuecomment-1914312450)
Will do soon(tm)

✅ willcl-ark closed an issue: "Unable to open wallet UI with ubuntu 23.10"
(https://github.com/bitcoin/bitcoin/issues/29311)

💬 willcl-ark commented on issue "Unable to open wallet UI with ubuntu 23.10":
(https://github.com/bitcoin/bitcoin/issues/29311#issuecomment-1914316466)
I think we can close this out for now as it appears to have been a `fontconfig` issue which is now resolved.

💬 naumenkogs commented on pull request "Nuke adjusted time from validation (attempt 2)":
(https://github.com/bitcoin/bitcoin/pull/28956#issuecomment-1914324428)
ACK ff9039f6ea876bab2c40a06a93e0dd087f445fa2

After a few attempts, I haven't found a way to trigger a consensus fork through adjusted time. It's not like this could "infect" half of the network (legacy) through block propagation or something. Because of that, if there was/is a legacy way to exploit it (say an overflow in `nTimeOffset` calculation), this new code won't make it worse.

💬 naumenkogs commented on pull request "test/BIP324: functional tests for v2 P2P encryption":
(https://github.com/bitcoin/bitcoin/pull/24748#issuecomment-1914329134)
ACK bc9283c4415a932ec1eeb70ca2aa4399c80437b3

💬 naumenkogs commented on pull request "test/BIP324: functional tests for v2 P2P encryption":
(https://github.com/bitcoin/bitcoin/pull/24748#discussion_r1464523738)
This comes unexpectedly. This is merely an unintentional side-effect of saving LOC in this test, right?