💬 l0rinc commented on pull request "qa: Improvements to debug_assert_log + busy_wait_for_debug_log":
(https://github.com/bitcoin/bitcoin/pull/33423#issuecomment-3572692780)
rfm?
(https://github.com/bitcoin/bitcoin/pull/33423#issuecomment-3572692780)
rfm?
💬 romanz commented on pull request "rest: allow reading partial block data from storage":
(https://github.com/bitcoin/bitcoin/pull/33657#issuecomment-3572749774)
Thanks!
> Left minor comment/question in related presentation: https://docs.google.com/presentation/d/1Zez-6DApKRu59kke4i_g9jwxQlaFKKRpOPdYFYsFXfA/edit?disco=AAABwl3JY0U
Good point - I've changed a bit the slide, and added a few comments.
This should be similar to the [electrs schema](https://github.com/romanz/electrs/blob/master/doc/schema.md), but with `txnum` instead of block height.
(https://github.com/bitcoin/bitcoin/pull/33657#issuecomment-3572749774)
Thanks!
> Left minor comment/question in related presentation: https://docs.google.com/presentation/d/1Zez-6DApKRu59kke4i_g9jwxQlaFKKRpOPdYFYsFXfA/edit?disco=AAABwl3JY0U
Good point - I've changed a bit the slide, and added a few comments.
This should be similar to the [electrs schema](https://github.com/romanz/electrs/blob/master/doc/schema.md), but with `txnum` instead of block height.
💬 151henry151 commented on pull request "Align legacy script policy with P2SH policy in AreInputsStandard":
(https://github.com/bitcoin/bitcoin/pull/33926#issuecomment-3572809238)
The `process_messages` fuzz test fails with `Assertion failed: (mempoolDuplicate.HaveCoin(txin.prevout))` at `txmempool.cpp:727` after allowing NONSTANDARD scripts with ≤15 sigops to pass `AreInputsStandard`.
I added a coin validity check in `AreInputsStandard` (verifying `coin.IsSpent()` before `GetSigOpCount`), but the test still fails. Since this PR only aligns policy behavior and shouldn't require mempool changes, perhaps the issue is how the policy change interacts with the consistency c
...
(https://github.com/bitcoin/bitcoin/pull/33926#issuecomment-3572809238)
The `process_messages` fuzz test fails with `Assertion failed: (mempoolDuplicate.HaveCoin(txin.prevout))` at `txmempool.cpp:727` after allowing NONSTANDARD scripts with ≤15 sigops to pass `AreInputsStandard`.
I added a coin validity check in `AreInputsStandard` (verifying `coin.IsSpent()` before `GetSigOpCount`), but the test still fails. Since this PR only aligns policy behavior and shouldn't require mempool changes, perhaps the issue is how the policy change interacts with the consistency c
...
⚠️ jdavidthomson opened an issue: "Spherical chains of Bitcoin Core"
(https://github.com/bitcoin/bitcoin/issues/33941)
### Please describe the feature you'd like to see added.
Spherical chains
### Is your feature related to a problem, if so please describe it.
Spherical chains
### Describe the solution you'd like
Current btc genesis block is of a known pytz. There are 21,000,000 coins.
This cannot be a new idea:
Are we able to modify the ideas around btc to ensure that there is a progressive addition of chains that have an origin chain with a genesis block of:
0000000040e73bc82487a271757458f091ba9ecf
...
(https://github.com/bitcoin/bitcoin/issues/33941)
### Please describe the feature you'd like to see added.
Spherical chains
### Is your feature related to a problem, if so please describe it.
Spherical chains
### Describe the solution you'd like
Current btc genesis block is of a known pytz. There are 21,000,000 coins.
This cannot be a new idea:
Are we able to modify the ideas around btc to ensure that there is a progressive addition of chains that have an origin chain with a genesis block of:
0000000040e73bc82487a271757458f091ba9ecf
...
💬 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
...
(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`.
(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.
(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
(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
(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
(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.
(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!
(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
(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
...
(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
(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
(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
(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
...
(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
...
(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)`.
(https://github.com/bitcoin-core/gui/pull/817#issuecomment-3574162074)
`dlg->activateWindow()` is essentially the same as calling `GUIUtil::bringToFront(dlg)`.