💬 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.
(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.
(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?
(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
...
(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
(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.
(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)
(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)
(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).
(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)
(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.
(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
...
(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
(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.
(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
...
(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).
(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).
💬 brunoerg commented on issue "ignored getblocktxn takes longer to detect than is possible":
(https://github.com/bitcoin/bitcoin/issues/22295#issuecomment-1576863888)
Perhaps #27626 fixed it?
(https://github.com/bitcoin/bitcoin/issues/22295#issuecomment-1576863888)
Perhaps #27626 fixed it?
💬 vasild commented on pull request "Relay own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/27509#issuecomment-1576875849)
> In case my node doesn't accept inbound connections and supports clearnet + tor, however, my addrman has majority ipv4/6 addresses, seems like there is a chance of not having any connection with a Tor peer, so it would make harder to know that my transaction has been propagated?
@brunoerg, Why? I couldn't follow. But anyway - as of now this PR does not wait for more than one peer to give back the transaction to us.
(https://github.com/bitcoin/bitcoin/pull/27509#issuecomment-1576875849)
> In case my node doesn't accept inbound connections and supports clearnet + tor, however, my addrman has majority ipv4/6 addresses, seems like there is a chance of not having any connection with a Tor peer, so it would make harder to know that my transaction has been propagated?
@brunoerg, Why? I couldn't follow. But anyway - as of now this PR does not wait for more than one peer to give back the transaction to us.
💬 willcl-ark commented on issue "Remove Ambiguity of Script ASM Hex and Decimal Integer Representations":
(https://github.com/bitcoin/bitcoin/issues/27795#issuecomment-1576885847)
Well, they would still be 32 bit values, just hex-encoded? But I take the point. I personally think prefixing hex with `0x` would be kind of horrible, as this would cascade into including pubkeys for example.
Is prefixing decimals with `0d` the best approach then?
I am still unclear on how much impact this would have downstream, and therefore whether any of these discussed changes would be accepted in any form. It seems like this could break much of the little scripting tooling we have av
...
(https://github.com/bitcoin/bitcoin/issues/27795#issuecomment-1576885847)
Well, they would still be 32 bit values, just hex-encoded? But I take the point. I personally think prefixing hex with `0x` would be kind of horrible, as this would cascade into including pubkeys for example.
Is prefixing decimals with `0d` the best approach then?
I am still unclear on how much impact this would have downstream, and therefore whether any of these discussed changes would be accepted in any form. It seems like this could break much of the little scripting tooling we have av
...
💬 instagibbs commented on pull request "[NO MERGE] BIP331 Ancestor Package Relay":
(https://github.com/bitcoin/bitcoin/pull/27742#discussion_r1218144248)
IIUC
total number of inflights doesn't appear to be changed here, it's still `MAX_PEER_TX_ANNOUNCEMENTS`
If the protected buckets are taken by attackers, an honest party can still insert an orphan into the unprotected bucket(under the MAX_PEER_TX_ANNOUNCEMENTS limit), and the whole package including the child should get fetched after a short delay.
So from a time-sensitive contract perspective, things should be strictly better than today?
(https://github.com/bitcoin/bitcoin/pull/27742#discussion_r1218144248)
IIUC
total number of inflights doesn't appear to be changed here, it's still `MAX_PEER_TX_ANNOUNCEMENTS`
If the protected buckets are taken by attackers, an honest party can still insert an orphan into the unprotected bucket(under the MAX_PEER_TX_ANNOUNCEMENTS limit), and the whole package including the child should get fetched after a short delay.
So from a time-sensitive contract perspective, things should be strictly better than today?
💬 willcl-ark commented on issue "Sync does not continue after reindex until a block is inved":
(https://github.com/bitcoin/bitcoin/issues/8239#issuecomment-1576902123)
I also seem unable to reproduce this behaviour on master at 7f2019755d, but I didn't check on mainnet with a long chain (and high chain work as suggested by Marco), instead I stopped a regtest node (a) in GDB while it was reindexing and generated new blocks on a second already-connected node (b), before continuing with the reindex.
The headers were received on node (a) before the reindex had completed, which then sent `getheaders` and synced to the new tip, without recieving any further block
...
(https://github.com/bitcoin/bitcoin/issues/8239#issuecomment-1576902123)
I also seem unable to reproduce this behaviour on master at 7f2019755d, but I didn't check on mainnet with a long chain (and high chain work as suggested by Marco), instead I stopped a regtest node (a) in GDB while it was reindexing and generated new blocks on a second already-connected node (b), before continuing with the reindex.
The headers were received on node (a) before the reindex had completed, which then sent `getheaders` and synced to the new tip, without recieving any further block
...