💬 glozow commented on pull request "TxDownloadManager followups":
(https://github.com/bitcoin/bitcoin/pull/31190#issuecomment-2459565875)
@dergoegge is that on master, or with the changes in this PR? Would like to see if it could be a negative time thing.

💬 rkrux commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1830902499)
Aah nvm, `IsDust` works on TxOuts.

💬 dergoegge commented on pull request "TxDownloadManager followups":
(https://github.com/bitcoin/bitcoin/pull/31190#issuecomment-2459568882)
> is that on master

Sorry I should have clarified, it is on master.

💬 fanquake commented on pull request "cmake: Add `FindQRencode` module and enable `libqrencode` package for MSVC":
(https://github.com/bitcoin/bitcoin/pull/31173#issuecomment-2459572965)
Guix Build:
```bash
ea6e6faf76005701a3251872e64ee1dd5c7e1222d28b38a273d9c1851470e927 guix-build-9e5089dbb02e/output/aarch64-linux-gnu/SHA256SUMS.part
b71cc1acf87a5394334149a2cd724e6acd97b2660d7f74d32b97e62f83a539d6 guix-build-9e5089dbb02e/output/aarch64-linux-gnu/bitcoin-9e5089dbb02e-aarch64-linux-gnu-debug.tar.gz
d4879760d76da7aa4a41fe56981a0c2992032ea41e9460ef60036dc57599e9e7 guix-build-9e5089dbb02e/output/aarch64-linux-gnu/bitcoin-9e5089dbb02e-aarch64-linux-gnu.tar.gz
2cbd20b0b10f97fb
...

💬 glozow commented on pull request "TxDownloadManager followups":
(https://github.com/bitcoin/bitcoin/pull/31190#issuecomment-2459581052)
Thanks @dergoegge, I probably also could have inferred from it being oss-fuzz.

Even though I wrote it, the assertion looks wrong 😅 overloaded inflight is handled with just a delay

https://github.com/bitcoin/bitcoin/blob/80cb630bd945e7f1777c7bab041c411b6916e651/src/node/txdownloadman_impl.cpp#L199

I think what I was trying to do was ensure `MAX_PEER_TX_ANNOUNCEMENTS` isn't busted. Adding a fix here.

👍 fanquake approved a pull request: "cmake: Add `FindQRencode` module and enable `libqrencode` package for MSVC"
(https://github.com/bitcoin/bitcoin/pull/31173#pullrequestreview-2418105101)
ACK 9e5089dbb02ef8a32f484aab682ad06748e1a532

💬 hebasto commented on pull request "cmake: Revamp `FindLibevent` module":
(https://github.com/bitcoin/bitcoin/pull/31181#issuecomment-2459671135)
> Can you rebase this.

Sure. Rebased.

👍 hebasto approved a pull request: "ci: `add second_deadlock_stack=1` to TSAN options"
(https://github.com/bitcoin/bitcoin/pull/31232#pullrequestreview-2418199389)
ACK 5161c2618cd36cd2d64c64d42d6f6d3b1929cb28.

💬 theStack commented on pull request "tests: Handle BDB dynamic pagesize":
(https://github.com/bitcoin/bitcoin/pull/31222#issuecomment-2459691229)
Concept ACK

Note that a similar change is also included in #30125, but happy to rebase if this one goes in first.

💬 vasild commented on pull request "Fuzz: extend CConnman tests":
(https://github.com/bitcoin/bitcoin/pull/28584#discussion_r1830980191)
I like simulating some time passage here.

Changing `static std::atomic<std::chrono::seconds> g_mock_time{};` to a finer precision is indeed out of the scope here.

Following a discussion on IRC I checked that indeed the time is frozen already for fuzz tests so calling `SetMockTime()` here is not going to freeze it (since it is already frozen). I added:

```cpp
SetMockTime(ConsumeTime(m_fuzzed_data_provider)); // Time could go backwards.
```

It could end up increasing the time more th
...

💬 vasild commented on pull request "Fuzz: extend CConnman tests":
(https://github.com/bitcoin/bitcoin/pull/28584#issuecomment-2459694396)
`cf83f0c...c97d496`: simulate time passage and (hopefully) fix CI

🤔 glozow reviewed a pull request: "Ephemeral Dust"
(https://github.com/bitcoin/bitcoin/pull/30239#pullrequestreview-2409302554)
code review ack. I still need to review tests more thoroughly. None of these comments are blocking.

Trying to summarize why these would propagate via 1p1c package relay, please correct me if I'm wrong:

- Essentially there is no longer anything "wrong" with a transaction that has 1 dust output in it since `IsStandardTx` has been changed. It's not `TX_NOT_STANDARD`. It needs to be 0 fee to pass `CheckValidEphemeralTx`, and then it should fail on feerate for `TX_RECONSIDERABLE`. That's why th
...

💬 glozow commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1830004418)
nit: const reference would be better

💬 glozow commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1825312146)
191ca0506ca69caa4d24c1f38b6f68742abb84dc nit: maybe `test_dustrelayfee_zero` or something

💬 glozow commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1830045893)
was this supposed to spend the non-dust from dust_txid 1?

💬 glozow commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1830977092)
Noting multiple ephemeral txns spent by 1 child is allowed in this logic, but not won't propagate via opportunistic package relay. This would still be the case if we relax the child-with-unconfirmed-parents requirement because none of the parents could be submitted alone.

💬 glozow commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1830012890)
Could speed up the common case a bit by checking `unspent_parent_dust.empty()` here, but perhaps not significantly

💬 glozow commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1830037288)
Question, why minrelay instead of `DUST_RELAY_TX_FEE`?

💬 glozow commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1830051037)
It also doesn't know about ancestors, given that the parent isn't given, right?

💬 glozow commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1830997947)
nit: `test_no_minrelay_fee` could be better