💬 monlovesmango commented on pull request "wallet: don't consider unconfirmed TRUC coins with ancestors":
(https://github.com/bitcoin/bitcoin/pull/33528#issuecomment-3573168495)
> I think it would be reasonable to stop this transaction from being created and throw a "invalid parameters" error, telling the user they selected inputs that can't be spent due to policy.

@glozow I created 2 commits that build off of this pr's branch in an attempt to address this here:
https://github.com/monlovesmango/bitcoin/tree/pr33528-add-invalid-parameters-error

Any feedback is greatly appreciated. Feel free to take anything you think is useful. If you don't think its appropriate f
...

💬 fjahr commented on pull request "refactor, docs: Embedded ASMap [2/3]: Refactor asmap internals and add documentation":
(https://github.com/bitcoin/bitcoin/pull/33878#discussion_r2558058740)
Removed the change from `constexpr`.

💬 fjahr commented on pull request "refactor, docs: Embedded ASMap [2/3]: Refactor asmap internals and add documentation":
(https://github.com/bitcoin/bitcoin/pull/33878#discussion_r2558059839)
I actually slightly prefer to keep it as is, because if I do this earlier I also have to change the tests because the hash result changes too with the span usage. I think it makes more sense to have this in the later commit where there is a much bigger overall benefit of introducing span evident that is actually what justifies the hash result being changed.

💬 fjahr commented on pull request "refactor, docs: Embedded ASMap [2/3]: Refactor asmap internals and add documentation":
(https://github.com/bitcoin/bitcoin/pull/33878#discussion_r2558060398)
Renamed to `CheckStandardAsmap` and using it in a two more places, but keeping it out of asmap_direct fuzzer because we are testing the bits there explicitly. Moved it below below the sanity check in .h which is the same location as .cpp

💬 fjahr commented on pull request "refactor, docs: Embedded ASMap [2/3]: Refactor asmap internals and add documentation":
(https://github.com/bitcoin/bitcoin/pull/33878#discussion_r2558060593)
Done

💬 fjahr commented on pull request "refactor, docs: Embedded ASMap [2/3]: Refactor asmap internals and add documentation":
(https://github.com/bitcoin/bitcoin/pull/33878#discussion_r2558060750)
Done

💬 fjahr commented on pull request "refactor, docs: Embedded ASMap [2/3]: Refactor asmap internals and add documentation":
(https://github.com/bitcoin/bitcoin/pull/33878#discussion_r2558061851)
Thanks, I have applied this latest patch, I think that also resolves your earlier comment that was in a different comment thread.

💬 fjahr commented on pull request "refactor, docs: Embedded ASMap [2/3]: Refactor asmap internals and add documentation":
(https://github.com/bitcoin/bitcoin/pull/33878#issuecomment-3573190858)
Addressed the latest review comments from @hodlinator , thanks for the detailed patch suggestions and rationale, co-authorship added!

💬 plebhash commented on pull request "[30.x] Backports":
(https://github.com/bitcoin/bitcoin/pull/33609#issuecomment-3573215231)
please add https://github.com/bitcoin/bitcoin/pull/33676, it would be highly appreciated as a way to expedite https://github.com/stratum-mining/sv2-apps/issues/81

💬 fjahr commented on pull request "build: Embedded ASMap [3/3]: Build binary dump header file":
(https://github.com/bitcoin/bitcoin/pull/28792#issuecomment-3573340203)
Addressed comments and rebased after #33770 including the latest changes in #33878. This PR now necessarily re-adds the possibility to set a bool option for `-asmap` to make it possible to use asmap without an external file. I have done this a bit differently now and I think it's better.

Re @hodlinator https://github.com/bitcoin/bitcoin/pull/28792#discussion_r2549632572: I don't think I understand your comment. I am pretty sure I have addressed your previous comment to use `OFF` but the file
...

💬 fjahr commented on pull request "build: Embedded ASMap [3/3]: Build binary dump header file":
(https://github.com/bitcoin/bitcoin/pull/28792#discussion_r2558166994)
Done

💬 frankomosh commented on pull request "doc: clarify and cleanup macOS fuzzing notes":
(https://github.com/bitcoin/bitcoin/pull/33921#issuecomment-3573874026)
ACK c34bc01

💬 diegoviola commented on pull request "Bugfix - don't allow multiple dialogs for same tx in TransactionView":
(https://github.com/bitcoin-core/gui/pull/817#issuecomment-3574043761)
@pablomartin4btc I've just tried your code from this branch and it works fine on top of the current master, take a look:

https://github.com/user-attachments/assets/a6dd0b43-15c0-42de-b379-598a581ab44a

💬 diegoviola commented on pull request "Bugfix - don't allow multiple dialogs for same tx in TransactionView":
(https://github.com/bitcoin-core/gui/pull/817#issuecomment-3574067005)
@pablomartin4btc like I said, this variation when applied to your branch also makes it work:

```diff
diff --git a/src/qt/transactionview.cpp b/src/qt/transactionview.cpp
index 48e41bc352..72c9a65cd3 100644
--- a/src/qt/transactionview.cpp
+++ b/src/qt/transactionview.cpp
@@ -36,6 +36,7 @@
#include <QScrollBar>
#include <QSettings>
#include <QString>
+#include <QWindow>
#include <QTableView>
#include <QTimer>
#include <QUrl>
@@ -671,7 +672,9 @@ bool TransactionView::detailsA
...

💬 diegoviola commented on pull request "Bugfix - don't allow multiple dialogs for same tx in TransactionView":
(https://github.com/bitcoin-core/gui/pull/817#issuecomment-3574153166)
@pablomartin4btc this also works:

```diff
diff --git a/src/qt/transactionview.cpp b/src/qt/transactionview.cpp
index 48e41bc352..e8b42d4983 100644
--- a/src/qt/transactionview.cpp
+++ b/src/qt/transactionview.cpp
@@ -671,7 +671,7 @@ bool TransactionView::detailsAlreadyShown(const QModelIndex &idx)
{
for (TransactionDescDialog* dlg : m_opened_dialogs) {
if (dlg->getTransactionId() == idx.data(TransactionTableModel::TxHashRole).toString()) {
- GUIUtil::bringTo
...

💬 diegoviola commented on pull request "Bugfix - don't allow multiple dialogs for same tx in TransactionView":
(https://github.com/bitcoin-core/gui/pull/817#issuecomment-3574162074)
`dlg->activateWindow()` is essentially the same as calling `GUIUtil::bringToFront(dlg)`.

💬 hodlinator commented on pull request "build: Embedded ASMap [3/3]: Build binary dump header file":
(https://github.com/bitcoin/bitcoin/pull/28792#discussion_r2558941954)
I was pointing to this sentence in the PR description at the top, where it's still using "NO" rather than "OFF", as was changed in the code:
> By default the data at that location is embedded into the binary but there is also a build option to prevent this (`-DWITH_EMBEDDED_ASMAP=NO`).

(You can respond by jumping here: https://github.com/bitcoin/bitcoin/pull/28792#discussion_r2514815293).

✅ laanwj closed an issue: "guix build fails on RISC-V due to python-py-cpuinfo test failure"
(https://github.com/bitcoin/bitcoin/issues/33873)

💬 laanwj commented on issue "guix build fails on RISC-V due to python-py-cpuinfo test failure":
(https://github.com/bitcoin/bitcoin/issues/33873#issuecomment-3574292725)
Clearly a hardware issue, closing.

🤔 rkrux requested changes to a pull request: "wallet: Expand MuSig test coverage and follow-ups"
(https://github.com/bitcoin/bitcoin/pull/33636#pullrequestreview-3503877587)
Thanks for accepting the refactoring suggestions!