💬 MarcoFalke commented on issue "Error: no RPC connection" when trying to run Bitcoin Core functional tests":
(https://github.com/bitcoin/bitcoin/issues/28000#issuecomment-1613727692)
Let us know if this is still an issue

✅ MarcoFalke closed an issue: "Error: no RPC connection" when trying to run Bitcoin Core functional tests"
(https://github.com/bitcoin/bitcoin/issues/28000)

💬 MarcoFalke commented on pull request "util: Allow std::byte and char Span serialization":
(https://github.com/bitcoin/bitcoin/pull/27927#issuecomment-1613732499)
Yeah, I am planning to do a more thorough cleanup with C++20, see my comment directly above and the other one earlier in the thread:

> Also, `Span<B>` could be used internally to serialize byte-holding (pre)vectors. In fact this is likely a performance bug-fix, because currently, serialization accepts `std::vector<B>` for `B=signed char`, or `B=int8_t`. (`B=char` is deleted). However, serialization seems to only apply the optimization to only call `write` once for `B=unsigned char`. See
>

...

📝 techy2 opened a pull request: "fix: delay in TimeOffset applied to AdjustedTime caused by send/recei…"
(https://github.com/bitcoin/bitcoin/pull/28007)
On busy VPS and shared host with limited resources, the time between when a messages is sent to the tcpip send or receive
queue and when it is sent in the case of send queue, or when it is processed (ProcessMessage) can be in excess of 30 seconds.
This delay introduces a skew in AdjustedTime.

For the receive queue, the post processing uses the receive time prior to entering the queue to calculate TimeOffset rather than Now() which currently includes the delay in the queue.

For the sen
...

💬 MarcoFalke commented on pull request "fix: delay in TimeOffset applied to AdjustedTime caused by send/recei…":
(https://github.com/bitcoin/bitcoin/pull/28007#issuecomment-1613756862)
Duplicate of https://github.com/bitcoin/bitcoin/pull/6642?

💬 MarcoFalke commented on pull request "fix: delay in TimeOffset applied to AdjustedTime caused by send/recei…":
(https://github.com/bitcoin/bitcoin/pull/28007#issuecomment-1613758123)
See also #25908

💬 MarcoFalke commented on pull request "wallet: Implement independent BDB parser":
(https://github.com/bitcoin/bitcoin/pull/26606#discussion_r1247132355)
```suggestion
throw std::ios_base::failure("AutoFile::seek: file handle is nullptr");
```

?

💬 john-moffett commented on pull request "Exit and show error if unrecognized command line args are present":
(https://github.com/bitcoin-core/gui/pull/742#issuecomment-1613768548)
I'm not entirely sure why they weren't the same. In the beginning, `bitcoind` would handle RPC commands. Then QT got special-cased so that you [couldn't do that](https://github.com/theuni/bitcoin/commit/b2d1129f27a499fa00ab8f5b8f0e9f926ae66362) for `bitcoin-qt` -- though it wouldn't give any errors. It would just silently ignore the subsequent options.

Then the argument-handling code split entirely [here](https://github.com/bitcoin/bitcoin/pull/690). Now `bitcoind` and `bitcoin-qt` mostly did
...

💬 achow101 commented on pull request "Exit and show error if unrecognized command line args are present":
(https://github.com/bitcoin-core/gui/pull/742#issuecomment-1613788093)
I think this is needed to handle BIP 21 URIs. The "loose" argument is interpreted as the URI, although ignored if they could not be recognized as one. Anything that follows it is explicitly ignored to avoid argument injection, see https://achow101.com/2021/02/0.18-uri-vuln.

💬 achow101 commented on pull request "wallet: Implement independent BDB parser":
(https://github.com/bitcoin/bitcoin/pull/26606#discussion_r1247172893)
Done

💬 achow101 commented on pull request "refactor: Drop unsafe AsBytePtr function":
(https://github.com/bitcoin/bitcoin/pull/27978#issuecomment-1613822240)
ACK 7c853619ee9ea17a79f1234b6c7871a45ceadcb9

🚀 achow101 merged a pull request: "refactor: Drop unsafe AsBytePtr function"
(https://github.com/bitcoin/bitcoin/pull/27978)

💬 john-moffett commented on pull request "Exit and show error if unrecognized command line args are present":
(https://github.com/bitcoin-core/gui/pull/742#issuecomment-1613840855)
Ah, thanks @achow101. I think this may still be salvageable by making an exception for just those arguments.

https://github.com/bitcoin-core/gui/blob/561915f35f4f75365c78df939b68c9062d3d3581/src/qt/paymentserver.cpp#L83

Will revisit tomorrow.

👍 theStack approved a pull request: "test: add python implementation of Elligator swift"
(https://github.com/bitcoin/bitcoin/pull/24005#pullrequestreview-1506182973)
ACK 4f4d039a98370a91e3cd5977352a9a4b260aa06b :crocodile:

Checked that since my previous ACK, review suggestions were tackled and the ellswift encoding/decoding test vectors were added (nice, wasn't aware of [csv.DictReader](https://docs.python.org/3/library/csv.html#csv.DictReader))! Also verified that the added .csv files are identical to the ones from the BIP repository.

💬 furszy commented on pull request "wallet: don't duplicate change output if already exist":
(https://github.com/bitcoin/bitcoin/pull/27601#discussion_r1247203254)
It depends on how the output was crafted. If it was created manually, it will not account for the fee. And also, it will not account for it if it was created in a previous `createTransaction` call with SFFO enabled.

It is fine to "overpay" the fee at this line because later in the process (50 lines below) we check if the final fee is above the needed one, automatically increasing the change by the surplus if needed.

📝 sipa opened a pull request: "BIP324 ciphers: FSChaCha20 and FSChaCha20Poly1305"
(https://github.com/bitcoin/bitcoin/pull/28008)
Depends on #27985 and #27993, based on and partially replaces #25361, part of #27634. Draft while dependencies are not merged.

This adds implementations of:
* The ChaCha20Poly1305 AEAD from RFC8434, including test vectors.
* The FSChaCha20 stream cipher as specified in BIP324, a rekeying wrapper around ChaCha20.
* The FSChaCha20Poly1305 AEAD as specified in BIP324, a rekeying wrapper around ChaCha20Poly1305.

The ChaCha20Poly1305 and FSChaCha20Poly1305 implementations are new, taking adv
...

💬 luke-jr commented on pull request "Translate unit names & fix external signer error":
(https://github.com/bitcoin-core/gui/pull/599#issuecomment-1613860180)
Rebased. Only units and the external signer bugfix remain. I didn't regenerate translations this time.

🤔 stickies-v reviewed a pull request: "blockstorage: Return on fatal flush errors"
(https://github.com/bitcoin/bitcoin/pull/27866#pullrequestreview-1506167577)
Concept ACK

The blockstorage changes seem pretty straightforward, the `validation.cpp` one I'll need to dive deeper into since it's got quite a few callsites.

💬 stickies-v commented on pull request "blockstorage: Return on fatal flush errors":
(https://github.com/bitcoin/bitcoin/pull/27866#discussion_r1247198730)
Any reason why this isn't `[[nodiscard]]` too? Given that it returns the `AbortNode` result, and is used in blockstorage?

🤔 Xekyo reviewed a pull request: "Bump unconfirmed ancestor transactions to target feerate"
(https://github.com/bitcoin/bitcoin/pull/26152#pullrequestreview-1506175113)
Took one suggestion, will revisit the other tomorrow.