💬 Sjors commented on pull request "validation: log which peer sent us a header":
(https://github.com/bitcoin/bitcoin/pull/27826#issuecomment-1576703050)
Added a check to ensure the header was valid and newly seen before we log it. That matches the behaviour from before this PR.

💬 vasild commented on pull request "Relay own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/27509#discussion_r1218012577)
For `GETDATA`, this only applies to transactions which we have not `INV`ed to the peer. Otherwise `INV`ed transactions will be sent via:

https://github.com/bitcoin/bitcoin/blob/f4a8269dfc144cc918570bdb870aa5143a11c1fe/src/net_processing.cpp#L2307

💬 MatthewLM commented on issue "Remove Ambiguity of Script ASM Hex and Decimal Integer Representations":
(https://github.com/bitcoin/bitcoin/issues/27795#issuecomment-1576709225)
> Edit: Curious if anyone knows the original rationale for displaying small values as decimal, it was carved out [by satoshi](https://github.com/bitcoin/bitcoin/blob/4405b78d6059e536c36974088a8ed4d9f0f29898/script.h#L298-L305).

It is because any numerical/arithmetic operations only allow 32-bit values, so the ASM displays 32-bit values as decimal and longer values as hex. This makes sense as humans generally better comprehend decimal values when thinking arithmetically, but the values need to
...

💬 vasild commented on pull request "Relay own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/27509#discussion_r1218020772)
TLDR: @instagibbs, right! :)

First, the problem with the user-agent string is that it can be customized which will totally reveal the identity of the sender. Next idea is to use the non-customized user-agent even if `-uacomment=` is configured. But then, why even reveal the version - whether it is 26.0 or 27.0? Right after a new release there will be few nodes that run the just-released version and thus the anonymity set will be small. So this string better be constant - either empty string o
...

💬 hebasto commented on pull request "kernel: Remove args, settings, chainparams, chainparamsbase from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27576#discussion_r1218041412)
Side note: Removing of `common/config.cpp` is correct but not directly related to this PR changes as it can be done even on the current master branch.

👍 hebasto approved a pull request: "kernel: Remove args, settings, chainparams, chainparamsbase from kernel library"
(https://github.com/bitcoin/bitcoin/pull/27576#pullrequestreview-1462486263)
ACK db77f87c6365cb5f414036d6bfb1a12705772028, I have reviewed the code and it looks OK.

💬 furszy commented on pull request "init: return error when block index is non-contiguous, fix feature_init.py file perturbation":
(https://github.com/bitcoin/bitcoin/pull/27823#discussion_r1218054540)
Couldn't `vSortedByHeight` contain blocks from different chains at the same height?

💬 josibake commented on pull request "[Draft / POC] Silent Payments":
(https://github.com/bitcoin/bitcoin/pull/24897#issuecomment-1576783417)
I've rebased and created a slimmed-down version of this PR in https://github.com/bitcoin/bitcoin/pull/27827. Most notably, I took out labels and removed some of the RPC support, and made several updates as this silent payments spec has changed a bit since this PR was last updated. For anyone reviewing this PR, I'd appreciate your feedback on #27827.

Big thanks to @w0xlt for moving this draft along as far as they did! My plan is to keep cherry-picking commits from this draft for follow-up PRs
...

💬 instagibbs commented on pull request "test: avoid sporadic MINIMALDATA failure in feature_taproot.py (fixes #27595)":
(https://github.com/bitcoin/bitcoin/pull/27631#issuecomment-1576787374)
ACK https://github.com/bitcoin/bitcoin/pull/27631/commits/54877253c807dac7a3720b2c3d1d989c410259a7

💬 fanquake commented on pull request "[Draft / POC] Silent Payments":
(https://github.com/bitcoin/bitcoin/pull/24897#issuecomment-1576788976)
Closing this, in favour of #27827.

✅ fanquake closed a pull request: "[Draft / POC] Silent Payments"
(https://github.com/bitcoin/bitcoin/pull/24897)

👋 Sjors's pull request is ready for review: "validation: log which peer sent us a header"
(https://github.com/bitcoin/bitcoin/pull/27826)

💬 vasild commented on pull request "Relay own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/27509#discussion_r1218073603)
Just one ping will be send. It is needed because without it the receiving bitcoind may ignore the TX message since the sender disconnects immediately after that, see https://github.com/bitcoin/bitcoin/issues/4432. Sending a ping and waiting for pong ensures that the preceding TX message has been processed (unless somebody implements out-of-order messages processing).

👋 josibake's pull request is ready for review: "[DRAFT] Silent Payments"
(https://github.com/bitcoin/bitcoin/pull/27827)

💬 vasild commented on pull request "Relay own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/27509#discussion_r1218082238)
> Why disconnect the sensitive connection on PONG?

Because then we assume the TX message that preceded the PING message has been processed.

> Could that ever happen before we send our tx messages out?

Yes, if the peer sends us unsolicited PONG without us sending PING before that.

> But still, don't we also send automatic PINGs?

Yes, and for the sensitive relay connections this should be the first PING. The point is to send TX before any PING.

💬 vasild commented on pull request "Relay own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/27509#discussion_r1218088888)
Yes, exactly. Before this change the test sent the transaction to the node using RPC and so it becomes locally-submitted, sensitive, unbroadcast, our own, etc transaction. With this change to the test it sends the transaction to the node via the P2P interface, so it is treated as normal, foreign, non-sensitive transaction and thus included in the MEMPOOL reply.

Btw, if changes from c70da509e4224f738fa0229e1f434a854629acf2 `net_processing: omit unbroadcast txs from replies to GETDATA and MEMPO
...

💬 instagibbs commented on pull request "Relay own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/27509#issuecomment-1576826080)
would like to see an updated OP that details what changes have been made, including precisely how often these connections are being made

💬 hebasto commented on pull request "kernel: Remove shutdown from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27711#issuecomment-1576836334)
Concept ACK.

💬 petertodd commented on pull request "Remove -mempoolfullrbf option":
(https://github.com/bitcoin/bitcoin/pull/26525#issuecomment-1576840323)
On Sun, Jun 04, 2023 at 06:47:08PM -0700, Antoine Riard wrote:
> > To be clear, 0conf users have to "upgrade" every single time mempool policies/conditions change for any non-trivial amount of hashing power. Pretty much any mempool policy change can be exploited to double-spend unconfirmed transactions. That's why the only entities with any hope of relying on them are large, centralized, transaction processors with significant engineering resources. The nVersion=3 proposal is not special in this
...

💬 Sjors commented on pull request "validation: log which peer sent us a header":
(https://github.com/bitcoin/bitcoin/pull/27826#discussion_r1218113430)
Since it's possible for `ProcessNewBlockHeaders` to return false without us returning, I wonder if we should only log if it returned `true` (as we do above).