💬 l0rinc commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1780084321)
Isn't that what we'd be calculating via `std::ranges::count_if(vInv, &CInv::IsGenTxMsg)`?
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1780084321)
Isn't that what we'd be calculating via `std::ranges::count_if(vInv, &CInv::IsGenTxMsg)`?
🤔 l0rinc reviewed a pull request: "refactor: TxDownloadManager + fuzzing"
(https://github.com/bitcoin/bitcoin/pull/30110#pullrequestreview-2335892884)
> It's less productive to request minor changes to something if you haven't decided whether it looks good on a macro level.
I had to reduce the scope of the reviews since I'm not yet familiar with this part of the code.
But I understand it's not what's needed here, so I will let others (having deeper, more holistic experience) do concept reviews.
(https://github.com/bitcoin/bitcoin/pull/30110#pullrequestreview-2335892884)
> It's less productive to request minor changes to something if you haven't decided whether it looks good on a macro level.
I had to reduce the scope of the reviews since I'm not yet familiar with this part of the code.
But I understand it's not what's needed here, so I will let others (having deeper, more holistic experience) do concept reviews.
💬 starius commented on pull request "Extend signetchallenge to set target block spacing":
(https://github.com/bitcoin/bitcoin/pull/29365#issuecomment-2381424630)
@Zk2u Thanks for the patch! I applied it.
(https://github.com/bitcoin/bitcoin/pull/29365#issuecomment-2381424630)
@Zk2u Thanks for the patch! I applied it.
💬 vasild commented on pull request "net: option to disallow v1 connection on ipv4 and ipv6 peers":
(https://github.com/bitcoin/bitcoin/pull/30951#issuecomment-2381424649)
> > what are the reasons to be concerned about somebody inspecting the traffic to my node?
>
> * You don't want others (e.g. network admins, internet providers, etc.) to know that you are running a bitcoin node at all. Maybe it's not allowed in your network / country etc., maybe for other reasons such as personal security.
This is what is worrying me - V2-only will not hide the fact that I am running a bitcoin node, but may give the false impression that it does. This is dangerous. My node
...
(https://github.com/bitcoin/bitcoin/pull/30951#issuecomment-2381424649)
> > what are the reasons to be concerned about somebody inspecting the traffic to my node?
>
> * You don't want others (e.g. network admins, internet providers, etc.) to know that you are running a bitcoin node at all. Maybe it's not allowed in your network / country etc., maybe for other reasons such as personal security.
This is what is worrying me - V2-only will not hide the fact that I am running a bitcoin node, but may give the false impression that it does. This is dangerous. My node
...
💬 l0rinc commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1780087050)
I was referring to getting rid of the cast, but not important, please resolve these.
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1780087050)
I was referring to getting rid of the cast, but not important, please resolve these.
✅ russeree closed an issue: "CI : Permission denied error - Multiple failing PRs"
(https://github.com/bitcoin/bitcoin/issues/30998)
(https://github.com/bitcoin/bitcoin/issues/30998)
💬 russeree commented on issue "CI : Permission denied error - Multiple failing PRs":
(https://github.com/bitcoin/bitcoin/issues/30998#issuecomment-2381443863)
Sipa has clarified that these are likely spurious error on IRC, failed tests are now passing.
(https://github.com/bitcoin/bitcoin/issues/30998#issuecomment-2381443863)
Sipa has clarified that these are likely spurious error on IRC, failed tests are now passing.
💬 stratospher commented on pull request "net: option to disallow v1 connection on ipv4 and ipv6 peers":
(https://github.com/bitcoin/bitcoin/pull/30951#issuecomment-2381451594)
@vasild V2 only doesn't protect against active attacks like the ones you've mentioned which require resources from the other party to run a node to spy the traffic.
V2 only protects against a cheaper attack vectors possible because of passive inspection of network traffic flowing through different medium. They don't need the other party to run a node to spy.
> > True. Lets dive a bit deeper - what are the reasons to be concerned about somebody inspecting the traffic to my node?
> Some t
...
(https://github.com/bitcoin/bitcoin/pull/30951#issuecomment-2381451594)
@vasild V2 only doesn't protect against active attacks like the ones you've mentioned which require resources from the other party to run a node to spy the traffic.
V2 only protects against a cheaper attack vectors possible because of passive inspection of network traffic flowing through different medium. They don't need the other party to run a node to spy.
> > True. Lets dive a bit deeper - what are the reasons to be concerned about somebody inspecting the traffic to my node?
> Some t
...
👍 itornaza approved a pull request: "rpc: add getorphantxs"
(https://github.com/bitcoin/bitcoin/pull/30793#pullrequestreview-2335910672)
Concept ACK.
The changes look good to me and the added functionality looks even greater! Built and run the extended test harness and everything checks out. Left more like a question rather than a nit.
(https://github.com/bitcoin/bitcoin/pull/30793#pullrequestreview-2335910672)
Concept ACK.
The changes look good to me and the added functionality looks even greater! Built and run the extended test harness and everything checks out. Left more like a question rather than a nit.
💬 itornaza commented on pull request "rpc: add getorphantxs":
(https://github.com/bitcoin/bitcoin/pull/30793#discussion_r1780105223)
My comment in the function declaration at the `txorphanage.h:84` applies here as well.
(https://github.com/bitcoin/bitcoin/pull/30793#discussion_r1780105223)
My comment in the function declaration at the `txorphanage.h:84` applies here as well.
💬 itornaza commented on pull request "rpc: add getorphantxs":
(https://github.com/bitcoin/bitcoin/pull/30793#discussion_r1780104846)
Do you think defining this as a const function `std::vector<OrphanTxBase> GetOrphanTransactions() const;` makes sense? I do not think it changes any members of the `TxOrphanage`.
(https://github.com/bitcoin/bitcoin/pull/30793#discussion_r1780104846)
Do you think defining this as a const function `std::vector<OrphanTxBase> GetOrphanTransactions() const;` makes sense? I do not think it changes any members of the `TxOrphanage`.
📝 l0rinc opened a pull request: "test: streamline CheckFormatSpecifiers testability"
(https://github.com/bitcoin/bitcoin/pull/30999)
Split out of https://github.com/bitcoin/bitcoin/pull/30928#discussion_r1779584565
The current string formatter couldn't validate every string format template that we were using.
Extended it with dynamic widths, fixed a number parsing bug that could go over the string's content and added a `%n` validation.
The tests are all runtime checked now - the rest of the production code is still compile-time checked.
This simplifies debugging (e.g. debugging why invalid values are parsed differentl
...
(https://github.com/bitcoin/bitcoin/pull/30999)
Split out of https://github.com/bitcoin/bitcoin/pull/30928#discussion_r1779584565
The current string formatter couldn't validate every string format template that we were using.
Extended it with dynamic widths, fixed a number parsing bug that could go over the string's content and added a `%n` validation.
The tests are all runtime checked now - the rest of the production code is still compile-time checked.
This simplifies debugging (e.g. debugging why invalid values are parsed differentl
...
💬 l0rinc commented on pull request "tinyformat: refactor: increase compile-time checks and don't throw for tfm::format_error":
(https://github.com/bitcoin/bitcoin/pull/30928#discussion_r1780117724)
Extracted it to https://github.com/bitcoin/bitcoin/pull/30999/files#diff-71badc1cc71ba46244f7841a088251bb294265f4fe9e662c0ad6b15be540eee4R45
(https://github.com/bitcoin/bitcoin/pull/30928#discussion_r1780117724)
Extracted it to https://github.com/bitcoin/bitcoin/pull/30999/files#diff-71badc1cc71ba46244f7841a088251bb294265f4fe9e662c0ad6b15be540eee4R45
💬 tdb3 commented on pull request "rpc: add getorphantxs":
(https://github.com/bitcoin/bitcoin/pull/30793#discussion_r1780149394)
Thanks! Will include this in next push.
(https://github.com/bitcoin/bitcoin/pull/30793#discussion_r1780149394)
Thanks! Will include this in next push.
👍 l0rinc approved a pull request: "test: Remove dead code from interface_zmq test"
(https://github.com/bitcoin/bitcoin/pull/30942#pullrequestreview-2335985809)
ACK eb20d8263b0da828624261eb9df0f9cc1f4f9f96
(https://github.com/bitcoin/bitcoin/pull/30942#pullrequestreview-2335985809)
ACK eb20d8263b0da828624261eb9df0f9cc1f4f9f96
💬 fjahr commented on pull request "test: Remove dead code from interface_zmq test":
(https://github.com/bitcoin/bitcoin/pull/30942#discussion_r1780157551)
`get_raw_seq` is always 6 and `num_txs` is always 5, so we can write it like this. Not sure if this is an improvement but it doesn't hurt so I have changed it.
(https://github.com/bitcoin/bitcoin/pull/30942#discussion_r1780157551)
`get_raw_seq` is always 6 and `num_txs` is always 5, so we can write it like this. Not sure if this is an improvement but it doesn't hurt so I have changed it.
💬 l0rinc commented on pull request "test: Remove dead code from interface_zmq test":
(https://github.com/bitcoin/bitcoin/pull/30942#discussion_r1780157721)
should be `num_txs + `
(https://github.com/bitcoin/bitcoin/pull/30942#discussion_r1780157721)
should be `num_txs + `
💬 fjahr commented on pull request "test: Remove dead code from interface_zmq test":
(https://github.com/bitcoin/bitcoin/pull/30942#discussion_r1780157728)
Added as belt and suspender
(https://github.com/bitcoin/bitcoin/pull/30942#discussion_r1780157728)
Added as belt and suspender
💬 fjahr commented on pull request "test: Remove dead code from interface_zmq test":
(https://github.com/bitcoin/bitcoin/pull/30942#discussion_r1780157944)
fixed
(https://github.com/bitcoin/bitcoin/pull/30942#discussion_r1780157944)
fixed
💬 l0rinc commented on pull request "test: Remove dead code from interface_zmq test":
(https://github.com/bitcoin/bitcoin/pull/30942#issuecomment-2381580319)
ACK c4dc81f9c6980964f63b9ad5166cd4cfaa86f3e6
(https://github.com/bitcoin/bitcoin/pull/30942#issuecomment-2381580319)
ACK c4dc81f9c6980964f63b9ad5166cd4cfaa86f3e6