💬 MarcoFalke commented on pull request "util: Allow std::byte and char Span serialization":
(https://github.com/bitcoin/bitcoin/pull/27927#issuecomment-1609799341)
> I do wonder: is there any reason why we wouldn't want to support serializing Spans in general (where its serialization is defined as the concatenation of the serialization of its elements)?

I think that should be fine to add, once there is a use case. Or is there already one?

💬 kristapsk commented on issue "RFE: Add "Edit Label" for addresses in Receive tab ("Requested payments history")":
(https://github.com/bitcoin-core/gui/issues/415#issuecomment-1609799711)
Agree, this would be very useful feature. Currently there is no way to edit label for receiving addresses neither from "Receive" tab nor coin selection dialog. User could make a typo when initially creating receive address or he would like to add comma separated list of associated inputs with change when doing manual control and end up using different set of inputs as initially thought.

💬 ryanofsky commented on pull request "util: Safer MakeByteSpan with ByteSpanCast":
(https://github.com/bitcoin/bitcoin/pull/27973#issuecomment-1609823534)
> FWIW, I don't believe `AsBytes` (or `std::as_bytes`) are ever undefined behavior

Yes that should be true as long as span is pointing to live memory. I should replace "platform specific or undefined" with "implementation specific" in the #27978 change description

💬 jonatack commented on issue "Enable consistency checks by default with `--enable-debug`":
(https://github.com/bitcoin/bitcoin/issues/24709#issuecomment-1609831542)
> The drawback of setting these options on by default is that they might make a node unusable under load ([#27300 (comment)](https://github.com/bitcoin/bitcoin/pull/27300#issuecomment-1479890611) for addrman checks or #27586 for others). Due to `cs_main` and the mempool, a lot of what we do is effectively single-threaded, so using 100% of a single core can easily be a pretty severe bottleneck.

I can confirm that my node sees block download timeouts and has trouble keeping up with the mainnet
...

💬 sipa commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#issuecomment-1609832143)
I've discussed some of the concerns here with @instagibbs, though haven't reviewed the code or read all the discussion.

As I understand it, there are (at least) these two issues to be addressed:
* If a package is received, but its overall feerate is too low, it may still be the case that there are acceptable subpackages (as the top of the graph) we would have liked to accept (and this could happen when e.g. we missed the initial announcement of those subpackages). This is a regression compar
...

💬 sdaftuar commented on pull request "Rework validation logic for assumeutxo":
(https://github.com/bitcoin/bitcoin/pull/27746#discussion_r1244021660)
Agreed, done.

💬 sdaftuar commented on pull request "Rework validation logic for assumeutxo":
(https://github.com/bitcoin/bitcoin/pull/27746#discussion_r1244022042)
Done.

💬 sdaftuar commented on pull request "Rework validation logic for assumeutxo":
(https://github.com/bitcoin/bitcoin/pull/27746#discussion_r1244023980)
Done (combined with taking @ryanofsky's approach for caching this value).

💬 sdaftuar commented on pull request "Rework validation logic for assumeutxo":
(https://github.com/bitcoin/bitcoin/pull/27746#discussion_r1244024243)
Agreed, gone now.

💬 sdaftuar commented on pull request "Rework validation logic for assumeutxo":
(https://github.com/bitcoin/bitcoin/pull/27746#discussion_r1244025126)
Can you explain a bit more what you have in mind for rewriting this function? Not sure I follow but happy to try to make this more readable/less error-prone.

💬 sdaftuar commented on pull request "Rework validation logic for assumeutxo":
(https://github.com/bitcoin/bitcoin/pull/27746#discussion_r1244025319)
Fixed, thanks.

💬 sdaftuar commented on pull request "Rework validation logic for assumeutxo":
(https://github.com/bitcoin/bitcoin/pull/27746#discussion_r1244026109)
Fixed.

💬 sdaftuar commented on pull request "Rework validation logic for assumeutxo":
(https://github.com/bitcoin/bitcoin/pull/27746#issuecomment-1609852620)
Thanks all for the additional review. I've updated based on feedback and cleaned up the commit history.

> would make sense but if we are gonna pass ChainstateManager to static functions, then I think we should avoid reaching into its internals and use the available interfaces or define new ones. I left a couple comments about this inline.

> It might also be better to do this in a separate PR and stick to the assumeutxo related improvements in this one. There are likely a lot more interfac
...

💬 sdaftuar commented on pull request "Rework validation logic for assumeutxo":
(https://github.com/bitcoin/bitcoin/pull/27746#discussion_r1244027946)
No longer relevant now that AcceptBlock is staying in CSM.

💬 sdaftuar commented on pull request "Rework validation logic for assumeutxo":
(https://github.com/bitcoin/bitcoin/pull/27746#discussion_r1244028312)
No longer relevant now that AcceptBlock/AcceptBlockHeader are staying in CSM.

💬 jonatack commented on pull request "doc: i2p documentation updates":
(https://github.com/bitcoin/bitcoin/pull/27937#discussion_r1244030626)
Done.

💬 jonatack commented on issue "I2P: Creating SAM session with 127.0.0.1:7656":
(https://github.com/bitcoin/bitcoin/issues/22759#issuecomment-1609907115)
> > The "cannot decode" message above repeats every 0.002 seconds, so ends up flooding the log file.
>
> > "Cannot decode Base64: "STREAM STATUS RESULT=I2P_ERROR MESSAGE="Session was closed""
> > and repeat same message hundreds time per second!
>
> Indeed, I have a few instances in my mainnet debug log when this occurred, and like these reports, in high-frequency bursts (unsure, but they seem to have only happened when using the Java I2P router, not i2pd). Will look more into it.

I've
...

💬 carnhofdaki commented on issue "Add maxrelaytxfee":
(https://github.com/bitcoin/bitcoin/issues/27983#issuecomment-1609907306)
![Uploading mutinynet.com_tx_68be05aec97b7d114e978185f0df76e494196e2b160330c97870b284f444e1c4.png…]()

Adding the screenshot in case that explorer will need to switch to pruned mode.

This is the raw hex transaction:

```
0200000000010238662df9105edbfc3bc1afb921e3bd382098624f9b2f5b3efa3144fb6c0c95340000000000fdffffffbb5667fde0eb1f0e169b85377ec7b79c07adfd680f84b0bc5e36180912abdf000000000000fdffffff0100000000000000000f6a0d746561636820426974636f696e0247304402202a9db30eae0197c835093b2837aec99
...

💬 ryanofsky commented on pull request "refactor: Drop unsafe AsBytePtr function":
(https://github.com/bitcoin/bitcoin/pull/27978#issuecomment-1609910681)
Rebased 2109a147406247bb654ee5fb8421a8594919fbd5 -> 650ca0d937c2c90adeeac60adae090902618d771 ([`pr/noptr.2`](https://github.com/ryanofsky/bitcoin/commits/pr/noptr.2) -> [`pr/noptr.3`](https://github.com/ryanofsky/bitcoin/commits/pr/noptr.3), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/noptr.2-rebase..pr/noptr.3)) due to conflict with #27479

💬 soheilwikey12 commented on pull request "bumpfee: ignore WALLET_INCREMENTAL_RELAY_FEE when user specifies fee_rate":
(https://github.com/bitcoin/bitcoin/pull/27969#issuecomment-1609942086)
14tDQpPnHqoOFvjBQzKoNM6bgwHL1oklVNLPRQr4Kvauq8oHICNDFvHv