💬 sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1842810444)
Not sure I follow how the first comment is an issue, but I do agree with the second. This needs to be conditional to `TryRemovingFromSet` succeeding.
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1842810444)
Not sure I follow how the first comment is an issue, but I do agree with the second. This needs to be conditional to `TryRemovingFromSet` succeeding.
💬 0xB10C commented on issue "Tracepoint Interface Tracking Issue":
(https://github.com/bitcoin/bitcoin/issues/31274#issuecomment-2477300406)
> > We could internalize the relevant macro parts of systemtap's sys/sdt.h for the Linux tracepoints. This would allow us to drop the external dependency on systemtap
>
> This still makes sense to me, it's just a few macros which haven't changed in years and are super unlikely to change significantly, because they insert ELF annotations (according to a well-known spec) and NOP instructions.
>
> Dropping a guix dependency would be good!
I just found out about https://github.com/libbpf/us
...
(https://github.com/bitcoin/bitcoin/issues/31274#issuecomment-2477300406)
> > We could internalize the relevant macro parts of systemtap's sys/sdt.h for the Linux tracepoints. This would allow us to drop the external dependency on systemtap
>
> This still makes sense to me, it's just a few macros which haven't changed in years and are super unlikely to change significantly, because they insert ELF annotations (according to a well-known spec) and NOP instructions.
>
> Dropping a guix dependency would be good!
I just found out about https://github.com/libbpf/us
...
💬 sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1842841578)
That is fair, I'll change it to bool to be explicit
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1842841578)
That is fair, I'll change it to bool to be explicit
💬 sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1842867828)
Done
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1842867828)
Done
💬 sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1842870647)
Mave the move dependant on `TryRemovingFromSet`
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1842870647)
Mave the move dependant on `TryRemovingFromSet`
💬 sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1842873615)
The whole thing from L151-L160?
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1842873615)
The whole thing from L151-L160?
💬 sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1842875471)
I'm guessing this is related to https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1842873615, in which case I'll drop that
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1842875471)
I'm guessing this is related to https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1842873615, in which case I'll drop that
📝 furszy opened a pull request: "test: add global time to place exec tests within the same dir"
(https://github.com/bitcoin/bitcoin/pull/31291)
Solving https://github.com/bitcoin/bitcoin/pull/31000#discussion_r1836348991.
Modifies the unit/bench/fuzz tests default datadir path.
Groups all tests executed within each binary call under
a single directory prefixed by the current time.
Replicating the function test framework behavior.
Switching from:
```tmp/test_common bitcoin/test_name/current_time```
To:
```tmp/test_common bitcoin/current_time/test_name```
(https://github.com/bitcoin/bitcoin/pull/31291)
Solving https://github.com/bitcoin/bitcoin/pull/31000#discussion_r1836348991.
Modifies the unit/bench/fuzz tests default datadir path.
Groups all tests executed within each binary call under
a single directory prefixed by the current time.
Replicating the function test framework behavior.
Switching from:
```tmp/test_common bitcoin/test_name/current_time```
To:
```tmp/test_common bitcoin/current_time/test_name```
👍 TheCharlatan approved a pull request: "guix: scope pkg-config to Linux only"
(https://github.com/bitcoin/bitcoin/pull/31276#pullrequestreview-2437211248)
ACK bcd82b13f4649e57d7d106856aab7b2a6296d728
(https://github.com/bitcoin/bitcoin/pull/31276#pullrequestreview-2437211248)
ACK bcd82b13f4649e57d7d106856aab7b2a6296d728
💬 achow101 commented on pull request "psbt: MuSig2 Fields":
(https://github.com/bitcoin/bitcoin/pull/31247#discussion_r1842895148)
If i need to retouch.
(https://github.com/bitcoin/bitcoin/pull/31247#discussion_r1842895148)
If i need to retouch.
💬 sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1842897353)
I changed `ReconSetSize` to return a tuple, and updated this to read:
```
"Now the set contains %i reconcilable transactions (plus %i delayed transactions).\n",
```
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1842897353)
I changed `ReconSetSize` to return a tuple, and updated this to read:
```
"Now the set contains %i reconcilable transactions (plus %i delayed transactions).\n",
```
💬 achow101 commented on pull request "wallet: Remove IsMine from migration code":
(https://github.com/bitcoin/bitcoin/pull/30328#issuecomment-2477425377)
> Commit message in [d5d994c](https://github.com/bitcoin/bitcoin/commit/d5d994c02bb54db395da457724ec45539f1c10a8) incorrectly states: "This reverts commit [bbb1d51](https://github.com/bitcoin/bitcoin/commit/bbb1d51e1240da61db2ca1036f9ec91fd2f36f2d)."
>
> I believe that commit ended up being merged as [b231f4d](https://github.com/bitcoin/bitcoin/commit/b231f4d556876ae70305e8710e31d53525ded8ae).
Indeed, fixed.
(https://github.com/bitcoin/bitcoin/pull/30328#issuecomment-2477425377)
> Commit message in [d5d994c](https://github.com/bitcoin/bitcoin/commit/d5d994c02bb54db395da457724ec45539f1c10a8) incorrectly states: "This reverts commit [bbb1d51](https://github.com/bitcoin/bitcoin/commit/bbb1d51e1240da61db2ca1036f9ec91fd2f36f2d)."
>
> I believe that commit ended up being merged as [b231f4d](https://github.com/bitcoin/bitcoin/commit/b231f4d556876ae70305e8710e31d53525ded8ae).
Indeed, fixed.
💬 furszy commented on pull request "bench: add support for custom data directory":
(https://github.com/bitcoin/bitcoin/pull/31000#discussion_r1842899557)
Done in #31291
(https://github.com/bitcoin/bitcoin/pull/31000#discussion_r1842899557)
Done in #31291
💬 achow101 commented on pull request "[refactor] Cleanup BlockAssembler mempool usage":
(https://github.com/bitcoin/bitcoin/pull/28843#issuecomment-2477429522)
ACK 192dac1d3370edd579db235d69c034726f37c8da
(https://github.com/bitcoin/bitcoin/pull/28843#issuecomment-2477429522)
ACK 192dac1d3370edd579db235d69c034726f37c8da
📝 casey opened a pull request: "Add `contrib/justfile` containing useful development workflow commands."
(https://github.com/bitcoin/bitcoin/pull/31292)
Add `contrib/justfile` containing useful development workflow commands.
Just recipes can be run by symlinking `contrib/justfile` into the repository root:
ln -s contrib/justfile justfile
And running:
just RECIPE
From any subdirectory.
Also add `/justfile` to `.gitignore`, to ignore the symlink into the repository root.
`just` is command runner with make-like syntax. It is not a build system, and only serves as convenient way of saving and running commands. It is avai
...
(https://github.com/bitcoin/bitcoin/pull/31292)
Add `contrib/justfile` containing useful development workflow commands.
Just recipes can be run by symlinking `contrib/justfile` into the repository root:
ln -s contrib/justfile justfile
And running:
just RECIPE
From any subdirectory.
Also add `/justfile` to `.gitignore`, to ignore the symlink into the repository root.
`just` is command runner with make-like syntax. It is not a build system, and only serves as convenient way of saving and running commands. It is avai
...
💬 sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1842903990)
I have also addressed this when removing
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1842903990)
I have also addressed this when removing
💬 achow101 commented on pull request "benchmark: Improve SipHash_32b accuracy to avoid potential optimization issues":
(https://github.com/bitcoin/bitcoin/pull/30349#issuecomment-2477439889)
ACK 42066f45ff5d48e78a317eda63c035809bd657c6
(https://github.com/bitcoin/bitcoin/pull/30349#issuecomment-2477439889)
ACK 42066f45ff5d48e78a317eda63c035809bd657c6
💬 achow101 commented on pull request "ci: skip Github CI on branch pushes for forks":
(https://github.com/bitcoin/bitcoin/pull/30487#issuecomment-2477449008)
ACK 8610bcef9d030013f9e36cffe0c58dd2cfe85d66
(https://github.com/bitcoin/bitcoin/pull/30487#issuecomment-2477449008)
ACK 8610bcef9d030013f9e36cffe0c58dd2cfe85d66
🚀 achow101 merged a pull request: "[refactor] Cleanup BlockAssembler mempool usage"
(https://github.com/bitcoin/bitcoin/pull/28843)
(https://github.com/bitcoin/bitcoin/pull/28843)
💬 sr-gi commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1842913498)
Not sure if this comment only applied before https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1842870647. But I think this is not currently the case.
Here we are only adding the parents. Leaving `fanout=true` triggers adding the actual transaction `RelayTransaction` was called with in the following `if (fanout)` block.
We could also add it here, but it would involve adding another boolean to skip the `if (fanout)` block, since we need to populate `m_tx_inventory_to_send` with data
...
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1842913498)
Not sure if this comment only applied before https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1842870647. But I think this is not currently the case.
Here we are only adding the parents. Leaving `fanout=true` triggers adding the actual transaction `RelayTransaction` was called with in the following `if (fanout)` block.
We could also add it here, but it would involve adding another boolean to skip the `if (fanout)` block, since we need to populate `m_tx_inventory_to_send` with data
...