💬 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.

💬 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
...

💬 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
...

💬 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(
...

💬 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

💬 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.

💬 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
...

💬 brunoerg commented on pull request "p2p: cleanup `LookupIntern`, `Lookup` and `LookupHost`":
(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

💬 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.

💬 vasild commented on pull request "p2p: cleanup `LookupIntern`, `Lookup` and `LookupHost`":
(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)

💬 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!

👍 vasild approved a pull request: "refactor, net: End friendship of CNode, CConnman and ConnmanTestMsg"
(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

👍 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.

💬 hebasto 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-1473654097)
@AdarshRawat1
> can I work on this issue?

This project is permissionless.

Feel free to open a PR :)

💬 BitcoinErrorLog commented on pull request "Remove -mempoolfullrbf option":
(https://github.com/bitcoin/bitcoin/pull/26525#issuecomment-1473707197)
> 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.

Where were you when this was happening to first-seen 0conf users? Where was the very strong rationale and proper deprecation cycle then?

Separately, wha
...

💬 Sjors commented on pull request "refactor: remove unused param from legacy pubkey interface":
(https://github.com/bitcoin/bitcoin/pull/27274#discussion_r1140141439)
The following style is more consistent with what we do in other places:
'''cpp
if (!CanGetAddresses(/*internal*/=false)) {
``

💬 Sjors commented on pull request "refactor: remove unused param from legacy pubkey interface":
(https://github.com/bitcoin/bitcoin/pull/27274#issuecomment-1473738260)
Concept ACK