💬 sr-gi commented on pull request "net processing: avoid serving non-announced txs as a result of a MEMPOOL message":
(https://github.com/bitcoin/bitcoin/pull/27602#issuecomment-1682070095)
> Are you still working on this?
I wonder if this is still meaningful now that #27675 has landed
(https://github.com/bitcoin/bitcoin/pull/27602#issuecomment-1682070095)
> Are you still working on this?
I wonder if this is still meaningful now that #27675 has landed
🤔 MarcoFalke reviewed a pull request: "p2p: Drop m_recently_announced_invs bloom filter"
(https://github.com/bitcoin/bitcoin/pull/27675#pullrequestreview-1582115745)
lgtm, one doc nit.
Approach ACK fb02ba3c5f5bcd96b5e3622ef001b8e57ce63fc0, though I have not closely reviewed e4ffabbffacc4b890d393aafcc8286916ef887d8 🏁
<details><summary>Show signature</summary>
Signature:
```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/Kl
...
(https://github.com/bitcoin/bitcoin/pull/27675#pullrequestreview-1582115745)
lgtm, one doc nit.
Approach ACK fb02ba3c5f5bcd96b5e3622ef001b8e57ce63fc0, though I have not closely reviewed e4ffabbffacc4b890d393aafcc8286916ef887d8 🏁
<details><summary>Show signature</summary>
Signature:
```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/Kl
...
💬 MarcoFalke commented on pull request "p2p: Drop m_recently_announced_invs bloom filter":
(https://github.com/bitcoin/bitcoin/pull/27675#discussion_r1297021567)
a70beafdb22564043dc24fc98133fdadbaf77d8a: This seems fragile, because someone may assume the sequence number is both unique for any tx, and strictly monotonically increasing for any child tx. Both assumptions are violated here, and it could make sense to update the doxygen of `CTxMemPoolEntry.m_sequence` for that?
Also, may be better to drop the `entry_` prefix from the `CTxMemPoolEntry::entry_sequence` member, since the scope is already clear from the surrounding class. Also, could drop th
...
(https://github.com/bitcoin/bitcoin/pull/27675#discussion_r1297021567)
a70beafdb22564043dc24fc98133fdadbaf77d8a: This seems fragile, because someone may assume the sequence number is both unique for any tx, and strictly monotonically increasing for any child tx. Both assumptions are violated here, and it could make sense to update the doxygen of `CTxMemPoolEntry.m_sequence` for that?
Also, may be better to drop the `entry_` prefix from the `CTxMemPoolEntry::entry_sequence` member, since the scope is already clear from the surrounding class. Also, could drop th
...
💬 mehran636311 commented on issue "Package Relay Project Tracking":
(https://github.com/bitcoin/bitcoin/issues/27463#issuecomment-1682073596)
Pellse full item my account open and stat
(https://github.com/bitcoin/bitcoin/issues/27463#issuecomment-1682073596)
Pellse full item my account open and stat
💬 MarcoFalke commented on pull request "net processing: avoid serving non-announced txs as a result of a MEMPOOL message":
(https://github.com/bitcoin/bitcoin/pull/27602#issuecomment-1682074827)
Maybe the test? Though, I haven't looked at it recently.
(https://github.com/bitcoin/bitcoin/pull/27602#issuecomment-1682074827)
Maybe the test? Though, I haven't looked at it recently.
👍 theStack approved a pull request: "test: Fix intermittent issue in mining_getblocktemplate_longpoll.py"
(https://github.com/bitcoin/bitcoin/pull/27941#pullrequestreview-1582383927)
ACK fa748c6f2ac2f9cac7ce42fd745ed3c3eae093b7
Seems reasonable to me to ensure that the RPC call has indeed reached the node before continuing, and should prevent races as described in #26962.
(https://github.com/bitcoin/bitcoin/pull/27941#pullrequestreview-1582383927)
ACK fa748c6f2ac2f9cac7ce42fd745ed3c3eae093b7
Seems reasonable to me to ensure that the RPC call has indeed reached the node before continuing, and should prevent races as described in #26962.
💬 MarcoFalke commented on pull request "rpc: Add Arg() default helper":
(https://github.com/bitcoin/bitcoin/pull/28230#discussion_r1297075358)
I don't like macros, so I'll leave as-is for now, because my version has less lines of code inside a macro
(https://github.com/bitcoin/bitcoin/pull/28230#discussion_r1297075358)
I don't like macros, so I'll leave as-is for now, because my version has less lines of code inside a macro
💬 MarcoFalke commented on pull request "rpc: Add Arg() default helper":
(https://github.com/bitcoin/bitcoin/pull/28230#discussion_r1297087713)
thx, fixed
(https://github.com/bitcoin/bitcoin/pull/28230#discussion_r1297087713)
thx, fixed
💬 MarcoFalke commented on pull request "rpc: Add Arg() default helper":
(https://github.com/bitcoin/bitcoin/pull/28230#discussion_r1297088104)
thx, I changed the doc a bit.
(https://github.com/bitcoin/bitcoin/pull/28230#discussion_r1297088104)
thx, I changed the doc a bit.
💬 MarcoFalke commented on pull request "ci: Ensure that only a single workflow processes `github.ref` at a time":
(https://github.com/bitcoin/bitcoin/pull/28282#issuecomment-1682133874)
Could do the same in the secp repo?
(https://github.com/bitcoin/bitcoin/pull/28282#issuecomment-1682133874)
Could do the same in the secp repo?
💬 naumenkogs commented on pull request "Fix potential network stalling bug":
(https://github.com/bitcoin/bitcoin/pull/27981#issuecomment-1682148113)
ACK 3388e523a129ad9c7aef418c9f57491f8c2d9df8
(https://github.com/bitcoin/bitcoin/pull/27981#issuecomment-1682148113)
ACK 3388e523a129ad9c7aef418c9f57491f8c2d9df8
🚀 fanquake merged a pull request: "Break up script/standard.{h/cpp}"
(https://github.com/bitcoin/bitcoin/pull/28244)
(https://github.com/bitcoin/bitcoin/pull/28244)
💬 MarcoFalke commented on pull request "ci: Run "macOS native x86_64" job on GitHub Actions":
(https://github.com/bitcoin/bitcoin/pull/28187#discussion_r1297114440)
Done in https://github.com/bitcoin/bitcoin/pull/28278
(https://github.com/bitcoin/bitcoin/pull/28187#discussion_r1297114440)
Done in https://github.com/bitcoin/bitcoin/pull/28278
💬 naumenkogs commented on pull request "p2p: adaptive connections services flags":
(https://github.com/bitcoin/bitcoin/pull/28170#discussion_r1297115636)
Was it just outdated already? I'm confused by what NODE_NONE meant here before this change.
(https://github.com/bitcoin/bitcoin/pull/28170#discussion_r1297115636)
Was it just outdated already? I'm confused by what NODE_NONE meant here before this change.
💬 naumenkogs commented on pull request "p2p: adaptive connections services flags":
(https://github.com/bitcoin/bitcoin/pull/28170#discussion_r1297117898)
does `peerServices` refer to the outdated code? Pehraps we should change it here?
(https://github.com/bitcoin/bitcoin/pull/28170#discussion_r1297117898)
does `peerServices` refer to the outdated code? Pehraps we should change it here?
💬 dergoegge commented on pull request "ci: Run "macOS 11.0 [gui, no tests] [jammy]" job on GitHub Actions":
(https://github.com/bitcoin/bitcoin/pull/28265#issuecomment-1682170413)
Any reason to not run this on a self-hosted worker through cirrus? I thought we only wanted to convert to GHA where nobody is willing to self host e.g. the native macOS jobs.
(https://github.com/bitcoin/bitcoin/pull/28265#issuecomment-1682170413)
Any reason to not run this on a self-hosted worker through cirrus? I thought we only wanted to convert to GHA where nobody is willing to self host e.g. the native macOS jobs.
👍 dergoegge approved a pull request: "ci: Ensure that only a single workflow processes `github.ref` at a time"
(https://github.com/bitcoin/bitcoin/pull/28282#pullrequestreview-1582471767)
utACK 0080b5650e0bf125612e62b591f348076e9ad5d7
(https://github.com/bitcoin/bitcoin/pull/28282#pullrequestreview-1582471767)
utACK 0080b5650e0bf125612e62b591f348076e9ad5d7
💬 dergoegge commented on pull request "ci: Add test-each-commit task":
(https://github.com/bitcoin/bitcoin/pull/28279#issuecomment-1682173028)
This would be a cirrus self-hosted worker though, right?
(https://github.com/bitcoin/bitcoin/pull/28279#issuecomment-1682173028)
This would be a cirrus self-hosted worker though, right?
💬 naumenkogs commented on pull request "p2p: adaptive connections services flags":
(https://github.com/bitcoin/bitcoin/pull/28170#discussion_r1297132683)
What do you think they would help with?
(https://github.com/bitcoin/bitcoin/pull/28170#discussion_r1297132683)
What do you think they would help with?
💬 fanquake commented on pull request "Fix potential network stalling bug":
(https://github.com/bitcoin/bitcoin/pull/27981#issuecomment-1682180623)
We can follow up with the suggestions / changes:
> [If you want, feel free to include the commits from 202208_test_sendmsg - alternatively, if you'd prefer not to deal with test / rpc feedback here, I'd also be happy to open a separate PR that builds on this branch.](https://github.com/bitcoin/bitcoin/pull/27981#pullrequestreview-1579515115)
> [For your consideration: https://github.com/ajtowns/bitcoin/commit/87c509c6d6ee0d355d08e0d4bc60bc01d4a0ad60 . I expanded the WITH_LOCK out](https://
...
(https://github.com/bitcoin/bitcoin/pull/27981#issuecomment-1682180623)
We can follow up with the suggestions / changes:
> [If you want, feel free to include the commits from 202208_test_sendmsg - alternatively, if you'd prefer not to deal with test / rpc feedback here, I'd also be happy to open a separate PR that builds on this branch.](https://github.com/bitcoin/bitcoin/pull/27981#pullrequestreview-1579515115)
> [For your consideration: https://github.com/ajtowns/bitcoin/commit/87c509c6d6ee0d355d08e0d4bc60bc01d4a0ad60 . I expanded the WITH_LOCK out](https://
...