💬 sipa commented on pull request "Miniscript: always treat unsatisfiable scripts as insane":
(https://github.com/bitcoin/bitcoin/pull/27997#issuecomment-1613689759)
So I think the definition for "sane" that we want is "does the apparent policy of this miniscript match its actual semantics" - if you drop all wrappers and naively replace all fragments with their corresponding policy, is that guaranteed to match the policy of the actual script.

Arguably, this is the case for "0", despite not having any keys, because its apparent policy is that it's never spendable, and that is also what it actually implements. Of course, we could expand the definition and e
...

✅ fanquake closed an issue: "Stole wallet and paperwork fraude and robbery this ea stolen from my assets with the help of others dates change false tells created I have the original with dates and time you guys did get all of them showing the truth also the Ethereum accounts stole change dates dates I have yoke pocket all the way to now and father dates times contracts you guys are in deeper you gotta keep lieng stilling my assets with the help of Debbie she told you I knew she would tell you about my Bitcoin and you would come I needemore evidence to prove ,y cases thanks. "
(https://github.com/bitcoin-core/gui/issues/743)

:lock: fanquake locked an issue: "Stole wallet and paperwork fraude and robbery this ea stolen from my assets with the help of others dates change false tells created I have the original with dates and time you guys did get all of them showing the truth also the Ethereum accounts stole change dates dates I have yoke pocket all the way to now and father dates times contracts you guys are in deeper you gotta keep lieng stilling my assets with the help of Debbie she told you I knew she would tell you about my Bitcoin and you would come I needemore evidence to prove ,y cases thanks. "
(https://github.com/bitcoin-core/gui/issues/743)

💬 sipa commented on pull request "util: Allow std::byte and char Span serialization":
(https://github.com/bitcoin/bitcoin/pull/27927#issuecomment-1613702864)
> I think that should be fine to add, once there is a use case. Or is there already one?

Not that I know, no. I was more wondering why we'd only add a specialized one for byte-like types when a generic one for all spans would work equally well. And I think the answer is that even with a generic one, we'd probably want a more efficient specialization for byte-like types anyway, to avoid processing one byte at a time.

💬 MarcoFalke commented on issue "Intermittent failures in interface_usdt_mempool.py":
(https://github.com/bitcoin/bitcoin/issues/27380#issuecomment-1613723053)
https://cirrus-ci.com/task/4626519738286080?logs=ci#L3262

```
node0 2023-06-29T19:45:37.525879Z (mocktime: 2023-07-13T19:45:38Z) [httpworker.2] [rpc/request.cpp:181] [parse] [rpc] ThreadRPCServer method=getrawmempool user=__cookie__
test 2023-06-29T19:45:37.526000Z TestFramework (INFO): Polling buffer...
test 2023-06-29T19:45:37.527000Z TestFramework (INFO): Ensuring mempool:rejected event was handled successfully...
test 2023-06-29T19:45:37.527000Z TestFramework (ERROR): Assert
...

💬 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.