💬 jonatack commented on pull request "p2p: Allow whitelisting outgoing connections":
(https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1186352750)
https://github.com/bitcoin/bitcoin/commit/f52fc86d0840221a5d106eec201a4dcffd7d5774 missing header, sort

```diff
#include <net_permissions.h>
+#include <netbase.h>
#include <test/fuzz/FuzzedDataProvider.h>
@@ -31,8 +32,8 @@ FUZZ_TARGET(net_permissions)

NetWhitelistPermissions net_whitelist_permissions;
- bilingual_str error_net_whitelist_permissions;
ConnectionDirection connection_direction;
+ bilingual_str error_net_whitelist_permissions;
if (NetWhitelistPe
...

💬 jonatack commented on pull request "p2p: Allow whitelisting outgoing connections":
(https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1186367052)
https://github.com/bitcoin/bitcoin/commit/f52fc86d0840221a5d106eec201a4dcffd7d5774 Can also test the error case.

```diff
@@ -446,6 +446,9 @@ BOOST_AUTO_TEST_CASE(netpermissions_test)
BOOST_CHECK(NetWhitebindPermissions::TryParse(",,@1.2.3.4:32", whitebindPermissions, error));
BOOST_CHECK_EQUAL(whitebindPermissions.m_flags, NetPermissionFlags::None);

+ BOOST_CHECK(!NetWhitebindPermissions::TryParse("out,forcerelay@1.2.3.4:32", whitebindPermissions, error));
+ BOOST_CHEC
...

💬 jonatack commented on pull request "p2p: Allow whitelisting outgoing connections":
(https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1186336634)
https://github.com/bitcoin/bitcoin/commit/f52fc86d0840221a5d106eec201a4dcffd7d5774

- The following error message would be clearer, I think

```suggestion
error = _("whitebind may only be used for incoming connections (\"out\" was passed)");
```

- My first thought was if it would make sense to pass `output_connection_direction` as a `std::optional<ConnectionDirection>`.

- But also, relying on `nullptr` and overloading this param for the callee to know who the ca
...

💬 jonatack commented on pull request "p2p: Allow whitelisting outgoing connections":
(https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1186324954)
f52fc86 named arg, if you retouch and stay with the nullptr approach
```suggestion
if (!TryParsePermissionFlags(str, flags, /*output_connection_direction=*/nullptr, offset, error)) return false;
```

💬 jonatack commented on pull request "p2p: Allow whitelisting outgoing connections":
(https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1186377787)
19f3f3a A comment here for future readers documenting when/why to use this would be good.

💬 jonatack commented on pull request "p2p: Allow whitelisting outgoing connections":
(https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1186373633)
https://github.com/bitcoin/bitcoin/commit/f52fc86d0840221a5d106eec201a4dcffd7d5774 without quotes here it doesn't seem clear that these are values to be passed

```suggestion
"Additional flags \"in\" and \"out\" control whether permissions apply to incoming connections and/or outgoing (default: %s). "
```

```
$ ./src/bitcoind -h | grep -A7 "\-whitelist="
-whitelist=<[permissions@]IP address or network>
Add permission flags to the peers using the given IP address (e.g.
...

💬 jonatack commented on pull request "p2p: Allow whitelisting outgoing connections":
(https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1186378760)
https://github.com/bitcoin/bitcoin/commit/19f3f3a316660890f03d6ddcb4a4230eb2ff5e91 This doesn't seem to be a pure refactoring here and in `mempool_packages.py` and `p2p_segwit.py`

💬 martinus commented on pull request "util: Use steady clock instead of system clock to measure durations":
(https://github.com/bitcoin/bitcoin/pull/27405#issuecomment-1536625417)
Code review ACK fa83fb3, also ran all tests. All usages of the steady_clock are just for duration measurements, so the change to a different epoch is ok.

👍 martinus approved a pull request: "util: Use steady clock instead of system clock to measure durations"
(https://github.com/bitcoin/bitcoin/pull/27405#pullrequestreview-1415266523)
Code review ACK https://github.com/bitcoin/bitcoin/commit/fa83fb31619c19a1a30b4181486601a944941b16, also ran all tests. All usages of the steady_clock are just for duration measurements, so the change to a different epoch is ok.

💬 pinheadmz commented on pull request "net: call getaddrinfo() in detachable thread to prevent stalling":
(https://github.com/bitcoin/bitcoin/pull/27557#issuecomment-1536631559)
...got thread sanitizer errors running locally on main when my local DNS is black-holed. Fixed that with a shared pointer.

💬 hebasto commented on pull request "build: Add CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/25797#issuecomment-1536643935)
Rebased on top of the bitcoin/bitcoin#27554.

The CI build is :green_circle:

🤔 glozow reviewed a pull request: "mempool / rpc: add getprioritisationmap, delete a mapDeltas entry when delta==0"
(https://github.com/bitcoin/bitcoin/pull/27501#pullrequestreview-1415387665)
Addressed review comments and moved it from rpc/mempool.cpp to rpc/mining.cpp.

> Probably miners would be thankful to not need multiply the result by 100000000 first in order to delete mapDelta entries (and it would also make the tests simpler 😅), but happy to ACK either variant.

I agree! I've changed it to satoshis to be the same as prioritisetransaction.

💬 glozow commented on pull request "mempool / rpc: add getprioritisationmap, delete a mapDeltas entry when delta==0":
(https://github.com/bitcoin/bitcoin/pull/27501#discussion_r1186458650)
Good point, changed to `getprioritisedtransactions`

💬 glozow commented on pull request "mempool / rpc: add getprioritisationmap, delete a mapDeltas entry when delta==0":
(https://github.com/bitcoin/bitcoin/pull/27501#discussion_r1186459877)
Done

💬 glozow commented on pull request "mempool / rpc: add getprioritisationmap, delete a mapDeltas entry when delta==0":
(https://github.com/bitcoin/bitcoin/pull/27501#discussion_r1186461359)
Fixed

💬 glozow commented on pull request "mempool / rpc: add getprioritisationmap, delete a mapDeltas entry when delta==0":
(https://github.com/bitcoin/bitcoin/pull/27501#discussion_r1186460872)
Would it be API-breaky to change the result of prioritisetransaction?

I've edited the logs though, to show whether the tx is in mempool and what the new delta is. Btw I don't think `nTransactionsUpdated` starts out at 0 when this is called.

💬 glozow commented on pull request "mempool / rpc: add getprioritisationmap, delete a mapDeltas entry when delta==0":
(https://github.com/bitcoin/bitcoin/pull/27501#discussion_r1186461032)
Done

💬 glozow commented on pull request "mempool / rpc: add getprioritisationmap, delete a mapDeltas entry when delta==0":
(https://github.com/bitcoin/bitcoin/pull/27501#discussion_r1186459024)
Removed

💬 glozow commented on pull request "mempool / rpc: add getprioritisationmap, delete a mapDeltas entry when delta==0":
(https://github.com/bitcoin/bitcoin/pull/27501#discussion_r1186459071)
Fixed

💬 glozow commented on pull request "mempool / rpc: add getprioritisationmap, delete a mapDeltas entry when delta==0":
(https://github.com/bitcoin/bitcoin/pull/27501#discussion_r1186459143)
Taken