💬 AdarshRawat1 commented on issue "When opening or autoloading wallets there should be clear messages about rescanning in progress and wallets' names.":
(https://github.com/bitcoin-core/gui/issues/259#issuecomment-1473479240)
can I work on this issue?
(https://github.com/bitcoin-core/gui/issues/259#issuecomment-1473479240)
can I work on this issue?
💬 willcl-ark commented on pull request "build: ignore common editor files":
(https://github.com/bitcoin/bitcoin/pull/27275#issuecomment-1473485508)
@Bushstar I add project specific things to `.git/info/exclude` inside the project. But perhaps more helpful for you if you are using an IDE like that is if you add a global gitignore and reference it in your git config:
```
[core]
excludesfile = /path/to/your/gitignore_global
```
Then you'll never be bothered by it again yourself.
(https://github.com/bitcoin/bitcoin/pull/27275#issuecomment-1473485508)
@Bushstar I add project specific things to `.git/info/exclude` inside the project. But perhaps more helpful for you if you are using an IDE like that is if you add a global gitignore and reference it in your git config:
```
[core]
excludesfile = /path/to/your/gitignore_global
```
Then you'll never be bothered by it again yourself.
💬 fanquake commented on pull request "build: ignore common editor files":
(https://github.com/bitcoin/bitcoin/pull/27275#issuecomment-1473507800)
> If so should the Qt Creator entry be removed or leave it as this project uses Qt?
Can probably just leave things as-is. At some point we had instructions for using Qt Creator.
(https://github.com/bitcoin/bitcoin/pull/27275#issuecomment-1473507800)
> If so should the Qt Creator entry be removed or leave it as this project uses Qt?
Can probably just leave things as-is. At some point we had instructions for using Qt Creator.
✅ fanquake closed a pull request: "build: ignore common editor files"
(https://github.com/bitcoin/bitcoin/pull/27275)
(https://github.com/bitcoin/bitcoin/pull/27275)
💬 Bushstar commented on pull request "build: ignore common editor files":
(https://github.com/bitcoin/bitcoin/pull/27275#issuecomment-1473512419)
@willcl-ark top tips. I now have a global .gitignore file.
(https://github.com/bitcoin/bitcoin/pull/27275#issuecomment-1473512419)
@willcl-ark top tips. I now have a global .gitignore file.
💬 fanquake commented on pull request "system: allow GUI to initialize default data dir":
(https://github.com/bitcoin/bitcoin/pull/27273#issuecomment-1473520297)
https://github.com/bitcoin/bitcoin/pull/27273/checks?check_run_id=12065322378
```bash
********* Start testing of OptionTests *********
Config: Using QtTest library 5.15.5, Qt 5.15.5 (x86_64-little_endian-lp64 static release build; by Clang 8.0.1 (tags/RELEASE_801/final)), debian 10
PASS : OptionTests::initTestCase()
PASS : OptionTests::migrateSettings()
PASS : OptionTests::integerGetArgBug()
PASS : OptionTests::parametersInteraction()
PASS : OptionTests::extractFilter()
QWARN
...
(https://github.com/bitcoin/bitcoin/pull/27273#issuecomment-1473520297)
https://github.com/bitcoin/bitcoin/pull/27273/checks?check_run_id=12065322378
```bash
********* Start testing of OptionTests *********
Config: Using QtTest library 5.15.5, Qt 5.15.5 (x86_64-little_endian-lp64 static release build; by Clang 8.0.1 (tags/RELEASE_801/final)), debian 10
PASS : OptionTests::initTestCase()
PASS : OptionTests::migrateSettings()
PASS : OptionTests::integerGetArgBug()
PASS : OptionTests::parametersInteraction()
PASS : OptionTests::extractFilter()
QWARN
...
💬 willcl-ark commented on pull request "test: Support decoding segwit address in address_to_scriptpubkey()":
(https://github.com/bitcoin/bitcoin/pull/27269#issuecomment-1473539401)
Concept ACK
Thanks for working on this. Increasing the address to scriptpubkey decoding tests to include bech32 seems desirable. I think this would mean that we could remove the `todo` on [L427](https://github.com/bitcoin/bitcoin/pull/27269/commits/4b95be0c496947606aaa661507ca06d53cc358c5#diff-7932a60a9127fd22d10d367526fd7b987f9647ce017595f8b1af5c32d5db0083R427) of wallet.py too?
The way the equivalent base58 function `test_base58encodedecode()` is implemented is slightly different from th
...
(https://github.com/bitcoin/bitcoin/pull/27269#issuecomment-1473539401)
Concept ACK
Thanks for working on this. Increasing the address to scriptpubkey decoding tests to include bech32 seems desirable. I think this would mean that we could remove the `todo` on [L427](https://github.com/bitcoin/bitcoin/pull/27269/commits/4b95be0c496947606aaa661507ca06d53cc358c5#diff-7932a60a9127fd22d10d367526fd7b987f9647ce017595f8b1af5c32d5db0083R427) of wallet.py too?
The way the equivalent base58 function `test_base58encodedecode()` is implemented is slightly different from th
...
💬 vasild commented on pull request "p2p: cleanup `LookupIntern`, `Lookup` and `LookupHost`":
(https://github.com/bitcoin/bitcoin/pull/26261#discussion_r1140013676)
Now it is:
```cpp
const std::optional<CNetAddr> addr{LookupHost(i->first, false)};
if (i->first.empty() && addr.has_value() && addr->IsBindAny()) {
LogPrintf("WARNING: the RPC server is not safe to expose to untrusted networks such as the public internet\n");
```
If the string (`i->first`) is empty then `addr.has_value()` will never be true. Thus this condition will never be true and the warning will never be printed. It should be:
```cpp
if (i->first.empty() || (add.has_value(
...
(https://github.com/bitcoin/bitcoin/pull/26261#discussion_r1140013676)
Now it is:
```cpp
const std::optional<CNetAddr> addr{LookupHost(i->first, false)};
if (i->first.empty() && addr.has_value() && addr->IsBindAny()) {
LogPrintf("WARNING: the RPC server is not safe to expose to untrusted networks such as the public internet\n");
```
If the string (`i->first`) is empty then `addr.has_value()` will never be true. Thus this condition will never be true and the warning will never be printed. It should be:
```cpp
if (i->first.empty() || (add.has_value(
...
💬 vasild commented on pull request "p2p: set `-dnsseed` and `-listen` false if `maxconnections=0`":
(https://github.com/bitcoin/bitcoin/pull/26899#issuecomment-1473586166)
ACK fabb95e7bf02f3d8e663a02dd845d42e09d330ec
(https://github.com/bitcoin/bitcoin/pull/26899#issuecomment-1473586166)
ACK fabb95e7bf02f3d8e663a02dd845d42e09d330ec
💬 glozow commented on pull request "Remove -mempoolfullrbf option":
(https://github.com/bitcoin/bitcoin/pull/26525#issuecomment-1473587233)
Philosophical arguments about full RBF aside, this PR proposes deleting an option that is (1) used by a significant amount of nodes and (2) does no harm to the node operator. It would be inappropriate to simply take such an option out from underneath users/downstream software without very strong rationale and a proper deprecation cycle.
(https://github.com/bitcoin/bitcoin/pull/26525#issuecomment-1473587233)
Philosophical arguments about full RBF aside, this PR proposes deleting an option that is (1) used by a significant amount of nodes and (2) does no harm to the node operator. It would be inappropriate to simply take such an option out from underneath users/downstream software without very strong rationale and a proper deprecation cycle.
💬 Daniel600 commented on pull request "Remove -mempoolfullrbf option":
(https://github.com/bitcoin/bitcoin/pull/26525#issuecomment-1473607546)
GAP600 stats update - GAP600 enables 0-conf acceptance
- circa 500k unique BTC Trx hashes in Jan 2023
- circa 460k unique BTC Trx hashes in Feb 2023
There is still a significant use case for 0-conf acceptance - primarily by payment processors (servicing industries like gaming) and non-custodial liquidity providers serving non-custodial wallets. GAP600 has not seen as of yet an impact of FULLRBF on activity and risk.
FullRBF approach is a significant change to the behavior of the protoc
...
(https://github.com/bitcoin/bitcoin/pull/26525#issuecomment-1473607546)
GAP600 stats update - GAP600 enables 0-conf acceptance
- circa 500k unique BTC Trx hashes in Jan 2023
- circa 460k unique BTC Trx hashes in Feb 2023
There is still a significant use case for 0-conf acceptance - primarily by payment processors (servicing industries like gaming) and non-custodial liquidity providers serving non-custodial wallets. GAP600 has not seen as of yet an impact of FULLRBF on activity and risk.
FullRBF approach is a significant change to the behavior of the protoc
...
💬 brunoerg commented on pull request "p2p: cleanup `LookupIntern`, `Lookup` and `LookupHost`":
(https://github.com/bitcoin/bitcoin/pull/26261#discussion_r1140043673)
Fixed it, thanks!
(https://github.com/bitcoin/bitcoin/pull/26261#discussion_r1140043673)
Fixed it, thanks!
💬 Daniel600 commented on pull request "Remove -mempoolfullrbf option":
(https://github.com/bitcoin/bitcoin/pull/26525#issuecomment-1473612148)
> Philosophical arguments about full RBF aside, this PR proposes deleting an option that is (1) used by a significant amount of nodes and (2) does no harm to the node operator. It would be inappropriate to simply take such an option out from underneath users/downstream software without very strong rationale and a proper deprecation cycle.
Isnt that an argument for not disabling 0-conf
(https://github.com/bitcoin/bitcoin/pull/26525#issuecomment-1473612148)
> Philosophical arguments about full RBF aside, this PR proposes deleting an option that is (1) used by a significant amount of nodes and (2) does no harm to the node operator. It would be inappropriate to simply take such an option out from underneath users/downstream software without very strong rationale and a proper deprecation cycle.
Isnt that an argument for not disabling 0-conf
💬 dergoegge commented on pull request "refactor, net: End friendship of CNode, CConnman and ConnmanTestMsg":
(https://github.com/bitcoin/bitcoin/pull/27257#issuecomment-1473619787)
@vasild Thanks for the review! Took all of your suggestions.
(https://github.com/bitcoin/bitcoin/pull/27257#issuecomment-1473619787)
@vasild Thanks for the review! Took all of your suggestions.
💬 vasild commented on pull request "p2p: cleanup `LookupIntern`, `Lookup` and `LookupHost`":
(https://github.com/bitcoin/bitcoin/pull/26261#issuecomment-1473619930)
ACK ec28c89ef69cd3fbb1a82785c841d303b2bf4826
(https://github.com/bitcoin/bitcoin/pull/26261#issuecomment-1473619930)
ACK ec28c89ef69cd3fbb1a82785c841d303b2bf4826
👍 vasild approved a pull request: "p2p: cleanup `LookupIntern`, `Lookup` and `LookupHost`"
(https://github.com/bitcoin/bitcoin/pull/26261)
(https://github.com/bitcoin/bitcoin/pull/26261)
💬 darosior commented on pull request "MiniTapscript: port Miniscript to Tapscript":
(https://github.com/bitcoin/bitcoin/pull/27255#issuecomment-1473626748)
Pushed to try to wake the CI up. Looks like it worked!
(https://github.com/bitcoin/bitcoin/pull/27255#issuecomment-1473626748)
Pushed to try to wake the CI up. Looks like it worked!
👍 vasild approved a pull request: "refactor, net: End friendship of CNode, CConnman and ConnmanTestMsg"
(https://github.com/bitcoin/bitcoin/pull/27257)
ACK d02b99b10ab6a748fd3c8b0d0faca013ada2d6e3
(https://github.com/bitcoin/bitcoin/pull/27257)
ACK d02b99b10ab6a748fd3c8b0d0faca013ada2d6e3
👍 vasild approved a pull request: "refactor, net: End friendship of CNode, CConnman and ConnmanTestMsg"
(https://github.com/bitcoin/bitcoin/pull/27257)
ACK e29ecece9fed29663cc21caff4b00ceb29ecb959
(https://github.com/bitcoin/bitcoin/pull/27257)
ACK e29ecece9fed29663cc21caff4b00ceb29ecb959
👍 vasild approved a pull request: "rpc, test: remove newline escape sequence from wallet warning fields"
(https://github.com/bitcoin/bitcoin/pull/27138)
ACK f0391cd3ea562d526db63996e7694ff4cd44b8b2
The newline is presentation-specific and does not belong to the server output. For example it will not render as expected if displayed in HTML (where newlines are displayed as a single space). Then it should be `<br/>` instead of `\n`, or maybe `\r\n` for MacOS?
I am happy to review more of the same in another PR or a change that returns an array of warnings so that display engines can chose the best separator for them.
(https://github.com/bitcoin/bitcoin/pull/27138)
ACK f0391cd3ea562d526db63996e7694ff4cd44b8b2
The newline is presentation-specific and does not belong to the server output. For example it will not render as expected if displayed in HTML (where newlines are displayed as a single space). Then it should be `<br/>` instead of `\n`, or maybe `\r\n` for MacOS?
I am happy to review more of the same in another PR or a change that returns an array of warnings so that display engines can chose the best separator for them.