💬 maflcko commented on pull request "Remove the legacy wallet and BDB dependency":
(https://github.com/bitcoin/bitcoin/pull/28710#discussion_r2063255483)
dc7bf5fd6a320c4528a28cef2a565366bbab3877: Needs to update the RPC help, to remove the field?
(https://github.com/bitcoin/bitcoin/pull/28710#discussion_r2063255483)
dc7bf5fd6a320c4528a28cef2a565366bbab3877: Needs to update the RPC help, to remove the field?
💬 maflcko commented on pull request "Remove the legacy wallet and BDB dependency":
(https://github.com/bitcoin/bitcoin/pull/28710#discussion_r2063310245)
(same commit): I wonder if this is fine to remove, given that someone could have a non-hd backup where some keys in the keypool have been used. For migration, it could make sense to take those as well, or document that they will not be taken?
(https://github.com/bitcoin/bitcoin/pull/28710#discussion_r2063310245)
(same commit): I wonder if this is fine to remove, given that someone could have a non-hd backup where some keys in the keypool have been used. For migration, it could make sense to take those as well, or document that they will not be taken?
💬 maflcko commented on pull request "Remove the legacy wallet and BDB dependency":
(https://github.com/bitcoin/bitcoin/pull/28710#discussion_r2063257060)
Same?
(https://github.com/bitcoin/bitcoin/pull/28710#discussion_r2063257060)
Same?
💬 maflcko commented on pull request "Remove the legacy wallet and BDB dependency":
(https://github.com/bitcoin/bitcoin/pull/28710#discussion_r2063301086)
(same commit): Should remove both comments, or none?
(https://github.com/bitcoin/bitcoin/pull/28710#discussion_r2063301086)
(same commit): Should remove both comments, or none?
💬 maflcko commented on pull request "Remove the legacy wallet and BDB dependency":
(https://github.com/bitcoin/bitcoin/pull/28710#discussion_r2063317183)
cc3c3b9958b31ddfc03905ed969c4da0a3bf4409: could probably fully remove this, given that it would be trivial to re-add, if ever needed.
(https://github.com/bitcoin/bitcoin/pull/28710#discussion_r2063317183)
cc3c3b9958b31ddfc03905ed969c4da0a3bf4409: could probably fully remove this, given that it would be trivial to re-add, if ever needed.
💬 maflcko commented on pull request "Remove the legacy wallet and BDB dependency":
(https://github.com/bitcoin/bitcoin/pull/28710#discussion_r2063275827)
Also, some more stuff:
```
$ git grep -i 'legacy wallets' src/script/ src/wallet doc/*.md
doc/build-netbsd.md:`db4` is required to enable support for legacy wallets.
doc/managing-wallets.md:## Migrating Legacy Wallets to Descriptor Wallets
doc/managing-wallets.md:Legacy wallets (traditional non-descriptor wallets) can be migrated to become Descriptor wallets
doc/managing-wallets.md:Legacy wallets can know about, be watching for, and be able to sign for, `migratewallet` only
doc/psbt.md:
...
(https://github.com/bitcoin/bitcoin/pull/28710#discussion_r2063275827)
Also, some more stuff:
```
$ git grep -i 'legacy wallets' src/script/ src/wallet doc/*.md
doc/build-netbsd.md:`db4` is required to enable support for legacy wallets.
doc/managing-wallets.md:## Migrating Legacy Wallets to Descriptor Wallets
doc/managing-wallets.md:Legacy wallets (traditional non-descriptor wallets) can be migrated to become Descriptor wallets
doc/managing-wallets.md:Legacy wallets can know about, be watching for, and be able to sign for, `migratewallet` only
doc/psbt.md:
...
💬 maflcko commented on pull request "Remove the legacy wallet and BDB dependency":
(https://github.com/bitcoin/bitcoin/pull/28710#issuecomment-2834673183)
> 2\. In that same commit, the churn in `wallet/scriptpubkeyman.cpp` is hard to follow, both here on Github as well as a `git show --color-moved=dimmed-zebra`. I'm guessing you're moving stuff around, deleting and modifying?
You can use `--patience`. Maybe add this to the commit message?
(https://github.com/bitcoin/bitcoin/pull/28710#issuecomment-2834673183)
> 2\. In that same commit, the churn in `wallet/scriptpubkeyman.cpp` is hard to follow, both here on Github as well as a `git show --color-moved=dimmed-zebra`. I'm guessing you're moving stuff around, deleting and modifying?
You can use `--patience`. Maybe add this to the commit message?
💬 Sjors commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2834678435)
Concept ACK
Moderation suggestion: close to non-contributors.
It's a bit draconian, but it's hard to review code in a pull request that's being brigaded, which unfortunately often happens when `OP_RETURN` or full RBF is involved. At the same time this isn't a technically difficult PR that needs thousands of eyes on it. Even if a non-contributor finds a bug and can't report it here, there's plenty of time to fix it in a followup.
The conceptual discusion should be continued on the mailin
...
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2834678435)
Concept ACK
Moderation suggestion: close to non-contributors.
It's a bit draconian, but it's hard to review code in a pull request that's being brigaded, which unfortunately often happens when `OP_RETURN` or full RBF is involved. At the same time this isn't a technically difficult PR that needs thousands of eyes on it. Even if a non-contributor finds a bug and can't report it here, there's plenty of time to fix it in a followup.
The conceptual discusion should be continued on the mailin
...
💬 michael1011 commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2834687180)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2834687180)
Concept ACK
💬 maflcko commented on issue "[rfc] build: Reject unclean configure?":
(https://github.com/bitcoin/bitcoin/issues/31942#issuecomment-2834719208)
I guess an alternative could be to take the message from https://github.com/bitcoin/bitcoin/issues/31942#issuecomment-2737914378 and adjust it to also include linker or compile errors?
(https://github.com/bitcoin/bitcoin/issues/31942#issuecomment-2834719208)
I guess an alternative could be to take the message from https://github.com/bitcoin/bitcoin/issues/31942#issuecomment-2737914378 and adjust it to also include linker or compile errors?
💬 hebasto commented on pull request "[29.x] Backports":
(https://github.com/bitcoin/bitcoin/pull/32292#issuecomment-2834719701)
> * [Replace stray tfm::format to cerr with qWarning bitcoin-core/gui#868](https://github.com/bitcoin-core/gui/pull/868)
It seems `#include <QDebug>` is missed.
(https://github.com/bitcoin/bitcoin/pull/32292#issuecomment-2834719701)
> * [Replace stray tfm::format to cerr with qWarning bitcoin-core/gui#868](https://github.com/bitcoin-core/gui/pull/868)
It seems `#include <QDebug>` is missed.
💬 fanquake commented on issue "[rfc] build: Reject unclean configure?":
(https://github.com/bitcoin/bitcoin/issues/31942#issuecomment-2834730805)
> and adjust it to also include linker or compile errors?
I guess we should also be adjusting it to recommend deleting the whole build dir? Otherwise I'd assume it's just going to lead to even more subtle CMake issues later on.
(https://github.com/bitcoin/bitcoin/issues/31942#issuecomment-2834730805)
> and adjust it to also include linker or compile errors?
I guess we should also be adjusting it to recommend deleting the whole build dir? Otherwise I'd assume it's just going to lead to even more subtle CMake issues later on.
💬 maflcko commented on pull request "doc: Improve `dependencies.md`":
(https://github.com/bitcoin/bitcoin/pull/31895#discussion_r2063356972)
Is there any value in documenting the version used twice with the additional maintenance overhead and risk of it being stale? The depends package definition is already linked, so anyone can get the real version with just one more click/open. If they want to find out the pull request/commit it will be just another step (git blame).
Could consider removing it?
(https://github.com/bitcoin/bitcoin/pull/31895#discussion_r2063356972)
Is there any value in documenting the version used twice with the additional maintenance overhead and risk of it being stale? The depends package definition is already linked, so anyone can get the real version with just one more click/open. If they want to find out the pull request/commit it will be just another step (git blame).
Could consider removing it?
⚠️ maflcko reopened an issue: "[rfc] build: Reject unclean configure?"
(https://github.com/bitcoin/bitcoin/issues/31942)
When re-configuring a build, it is possible to do so in an unclean way on a stale build folder.
The average person initiating such a build does not know when a stale build is safe and when not. Time is wasted for the builder when a broken build silently succeeds and then leads to a nonsensical error later on. Just to give the last two examples of the last few days: (https://github.com/bitcoin/bitcoin/issues/31922#issuecomment-2673888781, https://github.com/bitcoin/bitcoin/pull/31933#discussion_
...
(https://github.com/bitcoin/bitcoin/issues/31942)
When re-configuring a build, it is possible to do so in an unclean way on a stale build folder.
The average person initiating such a build does not know when a stale build is safe and when not. Time is wasted for the builder when a broken build silently succeeds and then leads to a nonsensical error later on. Just to give the last two examples of the last few days: (https://github.com/bitcoin/bitcoin/issues/31922#issuecomment-2673888781, https://github.com/bitcoin/bitcoin/pull/31933#discussion_
...
💬 Sjors commented on pull request "Remove the legacy wallet and BDB dependency":
(https://github.com/bitcoin/bitcoin/pull/28710#discussion_r2063363548)
It causes a bug in the GUI, see (1) here: https://github.com/bitcoin/bitcoin/pull/28710#pullrequestreview-2798630677
(https://github.com/bitcoin/bitcoin/pull/28710#discussion_r2063363548)
It causes a bug in the GUI, see (1) here: https://github.com/bitcoin/bitcoin/pull/28710#pullrequestreview-2798630677
💬 Sjors commented on pull request "Remove the legacy wallet and BDB dependency":
(https://github.com/bitcoin/bitcoin/pull/28710#issuecomment-2834750612)
@maflcko `--patience` did the trick. It can be made default too if your computer is fast enough: `git config --global diff.algorithm patience`
(https://github.com/bitcoin/bitcoin/pull/28710#issuecomment-2834750612)
@maflcko `--patience` did the trick. It can be made default too if your computer is fast enough: `git config --global diff.algorithm patience`
💬 1ma commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2834776509)
@Sjors are you not interested in the feedback of the end users of the software?
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2834776509)
@Sjors are you not interested in the feedback of the end users of the software?
💬 hebasto commented on issue "ctest -C Debug fails on vs2022 (miniscript_tests (SEGFAULT))":
(https://github.com/bitcoin/bitcoin/issues/32341#issuecomment-2834824481)
> > I'd say for the CI we want to reset `/Ob0 /Od` back to `/O2 /Ob1` to avoid slowness.
>
> [@hebasto](https://github.com/hebasto) If you don't mind, could you update your CI branch with this. If not, that's fine also and I'll try it myself.
https://github.com/hebasto/bitcoin-core-nightly/actions/runs/14705754388/job/41265724818
(https://github.com/bitcoin/bitcoin/issues/32341#issuecomment-2834824481)
> > I'd say for the CI we want to reset `/Ob0 /Od` back to `/O2 /Ob1` to avoid slowness.
>
> [@hebasto](https://github.com/hebasto) If you don't mind, could you update your CI branch with this. If not, that's fine also and I'll try it myself.
https://github.com/hebasto/bitcoin-core-nightly/actions/runs/14705754388/job/41265724818
💬 Sjors commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2834825682)
> @Sjors are you not interested in the feedback of the end users of the software?
Yes, but brigading is generally done by a very small group of users that don't represent the whole ecosystem. And we already know the reasons some people insist on keeping limits on `OP_RETURN`. That's why I suggested releasing a forked version of the software for those people (and I would recommend against running it, but it's your decision).
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2834825682)
> @Sjors are you not interested in the feedback of the end users of the software?
Yes, but brigading is generally done by a very small group of users that don't represent the whole ecosystem. And we already know the reasons some people insist on keeping limits on `OP_RETURN`. That's why I suggested releasing a forked version of the software for those people (and I would recommend against running it, but it's your decision).
🤔 shahsb reviewed a pull request: "depends: Fix cross-compiling `qt` package from macOS to Windows"
(https://github.com/bitcoin/bitcoin/pull/32357#pullrequestreview-2798879688)
ACK https://github.com/bitcoin/bitcoin/pull/32357/commits/35e57fbe336cdcb348ff088fc04216f1f5cf2742
(https://github.com/bitcoin/bitcoin/pull/32357#pullrequestreview-2798879688)
ACK https://github.com/bitcoin/bitcoin/pull/32357/commits/35e57fbe336cdcb348ff088fc04216f1f5cf2742