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

💬 Xekyo commented on pull request "Bump unconfirmed ancestor transactions to target feerate":
(https://github.com/bitcoin/bitcoin/pull/26152#discussion_r1247203795)
Thanks, went with your suggestion

💬 Xekyo commented on pull request "Bump unconfirmed ancestor transactions to target feerate":
(https://github.com/bitcoin/bitcoin/pull/26152#discussion_r1247221538)
I tried this change, and remembered why I put in the "discount" in the first place: I think I have incorporated the bump fees in the effective values before, so I need to know whether there is a difference between the sum of the bump fees and the combined inputs’ bump fee. I have an idea how to incorporate your suggestion, but I gotta try tomorrow.

📝 jonatack opened a pull request: "script, test: python typing and linter updates"
(https://github.com/bitcoin/bitcoin/pull/28009)
With these updates, `./test/lint/lint-python.py` and `./test/lint/lint-spelling.py` should be green again for developers using relatively recent Python dependencies, in particular mypy 0.991 (released 11/2022) and later. Please see the commit messages for details.

💬 achow101 commented on pull request "Fix transaction view/table":
(https://github.com/bitcoin-core/gui/pull/662#issuecomment-1613900091)
Reopening was requested.

📝 achow101 reopened a pull request: "Fix transaction view/table"
(https://github.com/bitcoin-core/gui/pull/662)
#204 reverted a necessary bugfix, and #205 introduced regressions since `setModel` resets column widths. Note that you need to delete your saved GUI config to see the fix, otherwise the prior widths are restored.

Before regressions:


After regressions / current master:

![Screenshot_20220905_233054 branch-22](https://user-images.githubusercontent
...

💬 luke-jr commented on pull request "Fix transaction view/table":
(https://github.com/bitcoin-core/gui/pull/662#issuecomment-1613900449)
Re-confirmed bug still exists, and rebased this fix.

💬 luke-jr commented on pull request "Bugfix: Address broken things around Peers detail view":
(https://github.com/bitcoin-core/gui/pull/677#discussion_r1247249570)
Doesn't seem worth splitting up such a minor change IMO

💬 luke-jr commented on pull request "Improve 'Requested Payments History' Multiselect":
(https://github.com/bitcoin-core/gui/pull/684#issuecomment-1613909123)
Would prefer the refactoring split into a different commit