💬 mzumsande commented on pull request "p2p: Diversify automatic outbound connections with respect to networks":
(https://github.com/bitcoin/bitcoin/pull/27213#discussion_r1241361274)
nit: unrelated whitespace change

💬 mzumsande commented on pull request "p2p: Diversify automatic outbound connections with respect to networks":
(https://github.com/bitcoin/bitcoin/pull/27213#discussion_r1241362625)
I think `addrConnect` can't be used here, because it can be empty if `pszDest` is set (that's why the CI fails too). You could use `pnode->addr` instead, which is set from either `addrConnect` or `pszDest` in `ConnectNode()`!

💬 mzumsande commented on pull request "p2p: Diversify automatic outbound connections with respect to networks":
(https://github.com/bitcoin/bitcoin/pull/27213#discussion_r1241361033)
here and in `test/util/net.h` - this is still using `ConnectedThroughNetwork` (see previous comment).

📝 Jezebel5134567 opened a pull request: "Create bitcoinhh"
(https://github.com/bitcoin/bitcoin/pull/27970)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...

💬 Jezebel5134567 commented on pull request "Create bitcoinhh":
(https://github.com/bitcoin/bitcoin/pull/27970#issuecomment-1606398607)
btc

💬 ariard commented on pull request "p2p: Advertise `NODE_FULL_RBF` and connect to 4 outbound full-rbf peers if `-mempoolfullrbf` is set":
(https://github.com/bitcoin/bitcoin/pull/25600#issuecomment-1606414648)
In the context of #27926, I think we might have to reconsider automatic transaction-relay preferential peering on the table, therefore allowing full-node operators to opt-in with `annexrelay=true`.

Beyond, such use-case, I think the preferential peering could be abstracted like we have for consensus-deployment code in `/consensus/params.h` and `DeploymentHeight()` in src/validation.cpp`. This could be re-used not only for `mempoolfullrbf` and future replacemnt logic upgrade, but also for `nVe
...

⚠️ tansanDOTeth opened an issue: "Keep getting errors after a while of syncing"
(https://github.com/bitcoin/bitcoin/issues/27972)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

Spent the last few weeks trying to sync a full node and keep getting a wild error after making lots of progress. I had to restart at least 3 times, but decided to finally report it here.

> 2023-06-26T04:05:04Z LevelDB read failure: Corruption: block checksum mismatch: /Volumes/Crucial X8/BTC Blockchain Data/chainstate/031566.ldb
2023-06-26T04:05:04Z Fatal LevelDB error: Corruption: blo
...

✅ achow101 closed a pull request: "Create bitcoinhh"
(https://github.com/bitcoin/bitcoin/pull/27970)

📝 achow101 locked a pull request: "."
(https://github.com/bitcoin/bitcoin/pull/27970)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...

💬 tansanDOTeth commented on issue "Fatal LevelDB error: Corruption: block checksum mismatch":
(https://github.com/bitcoin/bitcoin/issues/21724#issuecomment-1606570109)
> I had this checksum LevelDB problem frequently in the last weeks, following this answer [#10897 (comment)](https://github.com/bitcoin/bitcoin/issues/10897#issuecomment-317355977) I managed to solve it, the HD I have been using to keep a copy of bitcoins blockchain is not working properly anymore, copying everything to a new HD and using it completely solve the errors.

I read that comment, but not sure what I have to do run tests against my external. Do you have more instructions written som
...

💬 MarcoFalke commented on pull request "bumpfee: ignore WALLET_INCREMENTAL_RELAY_FEE when user specifies fee_rate":
(https://github.com/bitcoin/bitcoin/pull/27969#discussion_r1241602260)
missing test for larger than original fee rate, but not enough to be larger by incrementalrelayfee?

💬 S3RK commented on pull request "Disable and uncheck blank when private keys are disabled":
(https://github.com/bitcoin-core/gui/pull/739#issuecomment-1606849299)
I'm nor a GUI code expert neither a UX (user experience) expert, so take all of this with a healthy skepticism.
The code changes look good to me, but I'm unsure about whether this is a better UX.
Code review ACK 9ea31eba04ff8dcb9d7d993ce28bb10731a35177

I'm unsure if it's a clear win for UX because there's no hint to the user why the checkboxes change their state. I also like the idea with radio buttons and I'd be happy to review a new PR or re-review this one though.

Maybe change the de
...

📝 MarcoFalke opened a pull request: "util: Safer MakeByteSpan with ByteSpanCast "
(https://github.com/bitcoin/bitcoin/pull/27973)
The `AsBytes` helpers (or `std::as_bytes` helpers) are architecture
dependent. For example, the below test case diff [0], applied before
this commit will pass on x86_64, but fail on s390x [1].

Fix this by replacing `AsBytes` with a new `ByteSpanCast` in the `MakeByteSpan` helper.
This will turn the test case diff into a compile failure instead of a runtime error.

[0] test case diff:

```diff
diff --git a/src/test/serialize_tests.cpp b/src/test/serialize_tests.cpp
index 09f77d2b61..4
...

🤔 MarcoFalke reviewed a pull request: "BIP324: ElligatorSwift integrations"
(https://github.com/bitcoin/bitcoin/pull/27479#pullrequestreview-1492430513)
left some style nits, feel free to ignore, unless you retouch for other reasons.

💬 MarcoFalke commented on pull request "BIP324: ElligatorSwift integrations":
(https://github.com/bitcoin/bitcoin/pull/27479#discussion_r1238543437)
```suggestion
auto ret = key.EllSwiftCreate(MakeByteSpan(entropy));
```

style nit: Can use the alias which does the same, with the added bonus that the viewed data is const and future improvements to `MakeByteSpan` will benefit this call site as well.

💬 MarcoFalke commented on pull request "BIP324: ElligatorSwift integrations":
(https://github.com/bitcoin/bitcoin/pull/27479#discussion_r1238043422)
style-nit: Obviously doesn't matter here, but could use `reinterpret_cast` to guard against accidentally removing constness. (And for symmetry with `UCharCast(const std::byte*)`)

```suggestion
inline unsigned char* UCharCast(std::byte* c) { return reinterpret_cast<unsigned char*>(c); }
```

💬 MarcoFalke commented on pull request "BIP324: ElligatorSwift integrations":
(https://github.com/bitcoin/bitcoin/pull/27479#discussion_r1238546031)
```suggestion
auto ellswift = key.EllSwiftCreate(MakeByteSpan(ent32));
```

👍 dergoegge approved a pull request: "Remove the syscall sandbox"
(https://github.com/bitcoin/bitcoin/pull/27896#pullrequestreview-1497976492)
ACK 32e2ffc39374f61bb2435da507f285459985df9e

✅ fanquake closed an issue: "rpc: signed-integer-overflow in analyzepsbt["estimated_feerate"]"
(https://github.com/bitcoin/bitcoin/issues/27913)

🚀 fanquake merged a pull request: "feerate: For GetFeePerK() return nSatoshisPerK instead of round trip through GetFee"
(https://github.com/bitcoin/bitcoin/pull/27914)