Bitcoin Core Github
44 subscribers
119K links
Download Telegram
💬 fanquake commented on pull request "cmake: scope Boost Test check to `vcpkg`":
(https://github.com/bitcoin/bitcoin/pull/30822#discussion_r1745742588)
Thanks, fixed the comments.
💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1745745593)
never actually reachable as this is already caught in `IsStandardTx`, removed this error condition
💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1745745724)
done
💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1745745966)
done
💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1745746064)
done
💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1745746181)
done
💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1745746321)
added an optional sync argument, only unused during reorg test
💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1745746418)
done
💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1745746514)
done
💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1745746680)
great idea, added
👍 Sjors approved a pull request: "test: add mocked Sock that can read/write custom data and/or CNetMessages"
(https://github.com/bitcoin/bitcoin/pull/30205#pullrequestreview-2282985518)
ACK e59097a0a56ecaffdb38aaa94e232f5b45881897

You can see it in action in `sv2_connman_tests` in https://github.com/Sjors/bitcoin/pull/50.

I didn't test the `CNetMessage` functionality and `Eof()` method.

Maybe split 187ba684a9d5e63d09b43511b7128cce9edbedf3 into one commit that moves the implementation and another that splits the class.
💬 Sjors commented on pull request "test: add mocked Sock that can read/write custom data and/or CNetMessages":
(https://github.com/bitcoin/bitcoin/pull/30205#discussion_r1745516413)
187ba684a9d5e63d09b43511b7128cce9edbedf3: shouldn't `Sock{INVALID_SOCKET}` be preserved in the new `ZeroSock` constructor?
💬 Sjors commented on pull request "test: add mocked Sock that can read/write custom data and/or CNetMessages":
(https://github.com/bitcoin/bitcoin/pull/30205#discussion_r1745496410)
187ba684a9d5e63d09b43511b7128cce9edbedf3 any particular reason for moving this?
💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1745746765)
done
💬 instagibbs commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1745746912)
elaborated a bit, let me know if it helps
💬 hebasto commented on pull request "cmake: scope Boost Test check to `vcpkg`":
(https://github.com/bitcoin/bitcoin/pull/30822#issuecomment-2331991030)
> The alternative [#30787 (comment)](https://github.com/bitcoin/bitcoin/issues/30787#issuecomment-2324671298) seems fine as well.

This PR change is minimal.
👍 hebasto approved a pull request: "cmake: scope Boost Test check to `vcpkg`"
(https://github.com/bitcoin/bitcoin/pull/30822#pullrequestreview-2283396791)
re-ACK a7a4e11db8722c85175bcc4d9f73a713e9e61513.
💬 kevkevinpal commented on pull request "cmake: scope Boost Test check to `vcpkg`":
(https://github.com/bitcoin/bitcoin/pull/30822#issuecomment-2331995116)
lgtm ACK [a7a4e11](https://github.com/bitcoin/bitcoin/pull/30822/commits/a7a4e11db8722c85175bcc4d9f73a713e9e61513)

did `time cmake -B build` and got the following results

PR30822 ([a7a4e11](https://github.com/bitcoin/bitcoin/pull/30822/commits/a7a4e11db8722c85175bcc4d9f73a713e9e61513))
```
real 0m13.740s
user 0m6.798s
sys 0m4.696s
```

master (d661e2b1b771abafb0b152842d775d3150032230)
```
real 0m25.583s
user 0m17.078s
sys 0m5.127s
```
💬 maflcko commented on pull request "cmake: scope Boost Test check to `vcpkg`":
(https://github.com/bitcoin/bitcoin/pull/30822#issuecomment-2331996771)
review ACK a7a4e11db8722c85175bcc4d9f73a713e9e61513
💬 brunoerg commented on pull request "net: call `Select` with reachable networks in `ThreadOpenConnections`":
(https://github.com/bitcoin/bitcoin/pull/29436#issuecomment-2332011275)
Rebased