š¬ john-moffett commented on pull request "Switch RPCConsole wallet selection to the one most recently opened/restored/created":
(https://github.com/bitcoin-core/gui/pull/696#issuecomment-1618803949)
Thanks for pings, @hebasto and @jarolrod. Apologies for the delay. I've renamed the method and moved the declaration to the `Q_SLOTS` section.
(https://github.com/bitcoin-core/gui/pull/696#issuecomment-1618803949)
Thanks for pings, @hebasto and @jarolrod. Apologies for the delay. I've renamed the method and moved the declaration to the `Q_SLOTS` section.
š¬ theStack commented on pull request "Remove confusing "Dust" label from coincontrol / sendcoins dialog":
(https://github.com/bitcoin-core/gui/pull/719#issuecomment-1618813452)
> The commit message and the PR description should mention changes in the `SendCoinsDialog` as well.
Sorry for the reply there, missed that. Force-pushed with an adapted commit message (and changed PR title / description as well):
```
$ git range-diff 394300c18...ef4185d2b
1: 944263a93 ! 1: 210ef1e98 qt: remove confusing "Dust" label from coincontrol dialog
@@ Metadata
Author: Sebastian Falbesoner <sebastian.falbesoner@gmail.com>
## Commit message ##
- qt: rem
...
(https://github.com/bitcoin-core/gui/pull/719#issuecomment-1618813452)
> The commit message and the PR description should mention changes in the `SendCoinsDialog` as well.
Sorry for the reply there, missed that. Force-pushed with an adapted commit message (and changed PR title / description as well):
```
$ git range-diff 394300c18...ef4185d2b
1: 944263a93 ! 1: 210ef1e98 qt: remove confusing "Dust" label from coincontrol dialog
@@ Metadata
Author: Sebastian Falbesoner <sebastian.falbesoner@gmail.com>
## Commit message ##
- qt: rem
...
š¬ theStack commented on pull request "Remove confusing "Dust" label from coincontrol / sendcoins dialog":
(https://github.com/bitcoin-core/gui/pull/719#discussion_r1251087089)
I'm not too experienced with Qt, can anyone elaborate on that question (ping @furszy as author of the second commit)? It seems at least that we already have a couple of instances in master where both Qt::AlignLeading and Qt::AlignLeft are used together:
```
$ git grep AlignLeading
src/qt/forms/debugwindow.ui: <set>Qt::AlignBottom|Qt::AlignLeading|Qt::AlignLeft</set>
src/qt/forms/helpmessagedialog.ui: <set>Qt::AlignLeading|Qt::AlignLeft|Qt::AlignTop</set>
src/qt/forms/hel
...
(https://github.com/bitcoin-core/gui/pull/719#discussion_r1251087089)
I'm not too experienced with Qt, can anyone elaborate on that question (ping @furszy as author of the second commit)? It seems at least that we already have a couple of instances in master where both Qt::AlignLeading and Qt::AlignLeft are used together:
```
$ git grep AlignLeading
src/qt/forms/debugwindow.ui: <set>Qt::AlignBottom|Qt::AlignLeading|Qt::AlignLeft</set>
src/qt/forms/helpmessagedialog.ui: <set>Qt::AlignLeading|Qt::AlignLeft|Qt::AlignTop</set>
src/qt/forms/hel
...
š¬ pablomartin4btc commented on pull request "httpserver, rest: improving URI validation":
(https://github.com/bitcoin/bitcoin/pull/27253#issuecomment-1618849607)
Updates:
- Rebased
- Implemented refactoring [suggestion](https://github.com/bitcoin/bitcoin/pull/27253#discussion_r1245038873) from @stickies-v, moving other http reject validations into the constructor and regrouping them under a `Validate()` function to make the entire validation more consistent.
(https://github.com/bitcoin/bitcoin/pull/27253#issuecomment-1618849607)
Updates:
- Rebased
- Implemented refactoring [suggestion](https://github.com/bitcoin/bitcoin/pull/27253#discussion_r1245038873) from @stickies-v, moving other http reject validations into the constructor and regrouping them under a `Validate()` function to make the entire validation more consistent.
š¬ furszy commented on pull request "Remove confusing "Dust" label from coincontrol / sendcoins dialog":
(https://github.com/bitcoin-core/gui/pull/719#discussion_r1251098387)
> It seems at least that we already have a couple of instances in master where both Qt::AlignLeading and Qt::AlignLeft are used together.
Quite sure that we have them because of QT creator setting the value automatically. Probably because there is some ancient qt version that doesn't have them as synonyms.
But for us, it should be fine to drop them, even qt 4.8 has them as synonyms ([link](https://het.as.utexas.edu/HET/Software/html/qt.html#AlignmentFlag-enum)).
(https://github.com/bitcoin-core/gui/pull/719#discussion_r1251098387)
> It seems at least that we already have a couple of instances in master where both Qt::AlignLeading and Qt::AlignLeft are used together.
Quite sure that we have them because of QT creator setting the value automatically. Probably because there is some ancient qt version that doesn't have them as synonyms.
But for us, it should be fine to drop them, even qt 4.8 has them as synonyms ([link](https://het.as.utexas.edu/HET/Software/html/qt.html#AlignmentFlag-enum)).
š¬ theStack commented on pull request "Remove confusing "Dust" label from coincontrol / sendcoins dialog":
(https://github.com/bitcoin-core/gui/pull/719#discussion_r1251103633)
@furszy: Thanks a lot for checking. I removed the redundant `Qt::AlignLeading` properties from the second commit, i.e. it's only `Qt::AlignLeft` remaining.
(https://github.com/bitcoin-core/gui/pull/719#discussion_r1251103633)
@furszy: Thanks a lot for checking. I removed the redundant `Qt::AlignLeading` properties from the second commit, i.e. it's only `Qt::AlignLeft` remaining.
š¬ instagibbs commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#issuecomment-1618871642)
Without going full cluster mempool, I think whatever linearization is given out, we should probably ensure that each remaining prefix of the ancestor package is an ancestor package itself, before attempting submission to the mempool. Skip the entry if it's not. Maybe with ancestor set scoring linearizer this is redundant? I can't tell. It's probably going to get a lot closer to the ordering you want regardless, as it catches things like https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1
...
(https://github.com/bitcoin/bitcoin/pull/26711#issuecomment-1618871642)
Without going full cluster mempool, I think whatever linearization is given out, we should probably ensure that each remaining prefix of the ancestor package is an ancestor package itself, before attempting submission to the mempool. Skip the entry if it's not. Maybe with ancestor set scoring linearizer this is redundant? I can't tell. It's probably going to get a lot closer to the ordering you want regardless, as it catches things like https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1
...
š¬ 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-1618901090)
Modified to follow @hebasto 's idea of only allowing `bitcoin:` URIs if no other options follow.
(https://github.com/bitcoin-core/gui/pull/742#issuecomment-1618901090)
Modified to follow @hebasto 's idea of only allowing `bitcoin:` URIs if no other options follow.
š¬ pinheadmz commented on pull request "test: Add unit & functional test coverage for blockstore":
(https://github.com/bitcoin/bitcoin/pull/27850#issuecomment-1618905687)
@MarcoFalke @ryanofsky this is ready for review if you have time šŗ
(https://github.com/bitcoin/bitcoin/pull/27850#issuecomment-1618905687)
@MarcoFalke @ryanofsky this is ready for review if you have time šŗ
š¬ pinheadmz commented on pull request "Support JSON-RPC 2.0 when requested by client":
(https://github.com/bitcoin/bitcoin/pull/27101#issuecomment-1618910236)
> š This pull request conflicts with the target branch and [needs rebase](https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#rebasing-changes).
š
(https://github.com/bitcoin/bitcoin/pull/27101#issuecomment-1618910236)
> š This pull request conflicts with the target branch and [needs rebase](https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#rebasing-changes).
š
š¬ instagibbs commented on pull request "[25.x] Parallel compact block downloads":
(https://github.com/bitcoin/bitcoin/pull/27752#issuecomment-1618917886)
utACK https://github.com/bitcoin/bitcoin/pull/27752/commits/b8ad3220a9068f10c2b3b14b40f211372aeece31
(https://github.com/bitcoin/bitcoin/pull/27752#issuecomment-1618917886)
utACK https://github.com/bitcoin/bitcoin/pull/27752/commits/b8ad3220a9068f10c2b3b14b40f211372aeece31
š¬ instagibbs commented on pull request "Update MANDATORY_SCRIPT_VERIFY_FLAGS":
(https://github.com/bitcoin/bitcoin/pull/26291#issuecomment-1618920692)
I've been running this for the last week, no transaction-based misbehavior reports in my logs. still unsure the juice is worth the squeeze, but appears to not be obviously harmful.
(https://github.com/bitcoin/bitcoin/pull/26291#issuecomment-1618920692)
I've been running this for the last week, no transaction-based misbehavior reports in my logs. still unsure the juice is worth the squeeze, but appears to not be obviously harmful.
š¬ pablomartin4btc commented on pull request "httpserver, rest: improving URI validation":
(https://github.com/bitcoin/bitcoin/pull/27253#issuecomment-1618940865)
Updates:
- Checking fuzz test failure
(https://github.com/bitcoin/bitcoin/pull/27253#issuecomment-1618940865)
Updates:
- Checking fuzz test failure
š¬ Xekyo commented on pull request "Bump unconfirmed ancestor transactions to target feerate":
(https://github.com/bitcoin/bitcoin/pull/26152#discussion_r1251179626)
Ah drats, I did not realize that `CoinsResult.All()` produces a copy. Reverting this.
(https://github.com/bitcoin/bitcoin/pull/26152#discussion_r1251179626)
Ah drats, I did not realize that `CoinsResult.All()` produces a copy. Reverting this.
š¬ Xekyo commented on pull request "Bump unconfirmed ancestor transactions to target feerate":
(https://github.com/bitcoin/bitcoin/pull/26152#discussion_r1251181072)
I would prefer that the method `GetTotalBumpFee()` does not either return the sum of individual bump fees or the combined bump fee for the input set depending on when it is called. Iād be worried that it would be hard to understand later and might be a source of future bugs.
(https://github.com/bitcoin/bitcoin/pull/26152#discussion_r1251181072)
I would prefer that the method `GetTotalBumpFee()` does not either return the sum of individual bump fees or the combined bump fee for the input set depending on when it is called. Iād be worried that it would be hard to understand later and might be a source of future bugs.
š¬ Xekyo commented on pull request "Bump unconfirmed ancestor transactions to target feerate":
(https://github.com/bitcoin/bitcoin/pull/26152#discussion_r1251181233)
Done
(https://github.com/bitcoin/bitcoin/pull/26152#discussion_r1251181233)
Done
š¬ pinheadmz commented on pull request "net: support unix domain sockets for -proxy and -onion":
(https://github.com/bitcoin/bitcoin/pull/27375#discussion_r1251211129)
ah great tip thanks ;-)
(https://github.com/bitcoin/bitcoin/pull/27375#discussion_r1251211129)
ah great tip thanks ;-)
š¬ pinheadmz commented on pull request "net: support unix domain sockets for -proxy and -onion":
(https://github.com/bitcoin/bitcoin/pull/27375#issuecomment-1619034666)
> [64d0f23](https://github.com/bitcoin/bitcoin/commit/64d0f234e9aac490b03a5c019e86f1da56e62e01) looks good, except there is some messup in the contents of these two commits:
Thanks @vasild sorry about the messy rebase there. I cleaned up the commits and fixed the `const` nit. Rebased on master for merge conflicts.
(https://github.com/bitcoin/bitcoin/pull/27375#issuecomment-1619034666)
> [64d0f23](https://github.com/bitcoin/bitcoin/commit/64d0f234e9aac490b03a5c019e86f1da56e62e01) looks good, except there is some messup in the contents of these two commits:
Thanks @vasild sorry about the messy rebase there. I cleaned up the commits and fixed the `const` nit. Rebased on master for merge conflicts.
š¬ Sjors commented on pull request "wallet: bugfix, always use apostrophe for spkm descriptor ID":
(https://github.com/bitcoin/bitcoin/pull/27920#issuecomment-1619047639)
tACK 5df988b534df842ddb658ad2a7ff0f12996c8478
Also tested backporting the tests. Trick is to disable the norm_pub check (or manually replacing `'` with `h`, changing the checksum where needed).
(https://github.com/bitcoin/bitcoin/pull/27920#issuecomment-1619047639)
tACK 5df988b534df842ddb658ad2a7ff0f12996c8478
Also tested backporting the tests. Trick is to disable the norm_pub check (or manually replacing `'` with `h`, changing the checksum where needed).
š¬ ajtowns commented on pull request "Allow configuring target block time for a signet":
(https://github.com/bitcoin/bitcoin/pull/27446#issuecomment-1619112716)
> > I haven't come up with anything that seems like a good reason to vary consensus params though: if you want to test non-bitcoin behaviours, why modify bitcoin to do it?
>
> I just want to open a lightning channel and not need to wait an hour to use it...
With core lightning:
* on the "faucet" side, you need to create a zeroconf channel and fund it. to fund a 0.1 BTC channel with 0.05 BTC sent to the receiver, do: `lightning-cli -k --signet fundchannel id=$RECEIVER amount=1000000 fee
...
(https://github.com/bitcoin/bitcoin/pull/27446#issuecomment-1619112716)
> > I haven't come up with anything that seems like a good reason to vary consensus params though: if you want to test non-bitcoin behaviours, why modify bitcoin to do it?
>
> I just want to open a lightning channel and not need to wait an hour to use it...
With core lightning:
* on the "faucet" side, you need to create a zeroconf channel and fund it. to fund a 0.1 BTC channel with 0.05 BTC sent to the receiver, do: `lightning-cli -k --signet fundchannel id=$RECEIVER amount=1000000 fee
...