💬 aureleoules commented on pull request "ci: Add test coverage job":
(https://github.com/bitcoin/bitcoin/pull/27547#issuecomment-1568997159)
> I presume there is a setting to disable the comment and just have DrahtBot add a link in the form of https://app.codecov.io/gh/{owner}/{repo}/pull/{number} to the summary comment?

Yes there is. Done in 320eb03aeb097f6d14757c66a1b0bf40d0473b88.

👍 theStack approved a pull request: "test: fix intermittent error in getblockfrompeer.py"
(https://github.com/bitcoin/bitcoin/pull/27784#pullrequestreview-1451720045)
ACK 9fe9074266c6d7ddb40384d81741df1fc94980ff

Unfortunately there seems to be no reliable way to reproduce the issue on master (which could then be also ran on this PR to ensure that it is indeed fixed), but the explanation in https://github.com/bitcoin/bitcoin/issues/27749#issuecomment-1566554075 makes sense to me.

💬 pinheadmz commented on pull request "ZMQ: Support UNIX domain sockets":
(https://github.com/bitcoin/bitcoin/pull/27679#issuecomment-1569013910)
> 🐙 This pull request conflicts with the target branch and [needs rebase](https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#rebasing-changes).

Relax bot! sheesh. I'm gonna wait until #27375 gets approved

💬 Sjors commented on pull request "test: fix intermittent error in getblockfrompeer.py":
(https://github.com/bitcoin/bitcoin/pull/27784#issuecomment-1569023147)
9fe9074266c6d7ddb40384d81741df1fc94980ff looks harmless, but I haven't delved into the underlying issue. Can the original be reproduced by running the test in a loop??

CI failure seems spurious.

💬 theStack commented on pull request "Fix rpc_getblockfrompeer intermittency by introducing 'getblockfileinfo' RPC command":
(https://github.com/bitcoin/bitcoin/pull/27770#issuecomment-1569050370)
I agree with @furszy and @mzumsande that a `getblockfileinfo` RPC could be quite handy, both for making some of the pruning-related tests more robust and readable (potentially eliminating some magic numbers that can only be explained by the one who introduced them) and also for testing AssumeUTXO. As an illustration, the blocks directory of a mainnet test-run of AssumeUTXO ended up at some point like this on my disk:
```
$ ls -l ./datadir2/blocks/blk*.dat
-rw------- 1 thestack thestack 13
...

💬 pinheadmz commented on pull request "net: support unix domain sockets for -proxy and -onion":
(https://github.com/bitcoin/bitcoin/pull/27375#issuecomment-1569053476)
> 🐙 This pull request conflicts with the target branch and [needs rebase](https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#rebasing-changes).

🕺

💬 mzumsande commented on pull request "test: fix intermittent error in getblockfrompeer.py":
(https://github.com/bitcoin/bitcoin/pull/27784#issuecomment-1569054690)
> Can the original be reproduced by running the test in a loop??

I haven't been able to reproduce it, I think it requires a constellation with a very special timing which should make it very rare even in the CI. One thing I did is check in the log is that node2 doesn't use `HeadersDirectFetchBlocks` for any blocks > 205, and uses compact block reconstruction instead. This is different in the log of the failed run.

💬 theStack commented on pull request "test: fix intermittent error in getblockfrompeer.py":
(https://github.com/bitcoin/bitcoin/pull/27784#issuecomment-1569062154)
> [9fe9074](https://github.com/bitcoin/bitcoin/commit/9fe9074266c6d7ddb40384d81741df1fc94980ff) looks harmless, but I haven't delved into the underlying issue. Can the original be reproduced by running the test in a loop??

If you want to give that approach a try, the following shell script (which I have been running for days already, without any issue) should do exactly that:
```bash
#!/usr/bin/env bash
set -e
while true; do ./test/functional/rpc_getblockfrompeer.py; done
```

💬 erikarvstedt commented on issue "bitcoind hangs waiting for `g_requests.empty()`":
(https://github.com/bitcoin/bitcoin/issues/27722#issuecomment-1569074534)
@Crypt-iQ, the slow progress of `importmulti` is indeed a separate issue. I've updated my post.

💬 willcl-ark commented on pull request "test: cover addrv2 anchors by adding TorV3 to CAddress in messages.py":
(https://github.com/bitcoin/bitcoin/pull/27452#discussion_r1210781561)
Import helper from _test_framework.messages_?

```suggestion
new_data_hash = hash256(new_data)

```

💬 willcl-ark commented on pull request "test: cover addrv2 anchors by adding TorV3 to CAddress in messages.py":
(https://github.com/bitcoin/bitcoin/pull/27452#discussion_r1210795850)
Why do we need to keep the `Socks5Server` alive to ensure success when performing `"Check for addrv2 addresses in anchors.dat"`?

I changed this to `False` and saw intermittent failures on Line 123

```
assert_equal(data[services_index], 0x00) # services == NONE
```

💬 willcl-ark commented on pull request "test: cover addrv2 anchors by adding TorV3 to CAddress in messages.py":
(https://github.com/bitcoin/bitcoin/pull/27452#discussion_r1210781782)
```suggestion
from test_framework.messages import CAddress, hash256
```

📝 furszy opened a pull request: "fuzz: fix wallet notifications.cpp"
(https://github.com/bitcoin/bitcoin/pull/27786)
Fixing https://github.com/bitcoin/bitcoin/pull/27469#issuecomment-1568815816.

As the fuzzer test requires all blocks to be scanned by the wallet
(because it is asserting the wallet balance at the end), we need to
ensure that no blocks are skipped by the recently added wallet
birth time functionality.

This just means setting the chain accumulated time to the maximum
value, so the wallet birth time is always below it, and the block is
always processed by the wallet.

💬 furszy commented on pull request "wallet: improve IBD sync time by skipping block scanning prior birth time":
(https://github.com/bitcoin/bitcoin/pull/27469#issuecomment-1569086614)
Thanks Marco. Fixed in #27786.
Would be nice if the CI alert us about this kind of stuff prior merging the PR.
The base fuzzer corpus should have detected it right?.

⚠️ Macoophonic opened an issue: "Adchoose"
(https://github.com/bitcoin/bitcoin/issues/27787)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

thuannguyen19992000@gmail.com

### Expected behaviour

Eppo

### Steps to reproduce

It idon's data no way of the world

### Relevant log output

......

### How did you obtain Bitcoin Core

Package manager

### What version of Bitcoin Core are you using?

Đọc.join.29.v

### Operating system and version

13.0.

### Machine specifications

_No response_

✅ willcl-ark closed an issue: "Adchoose"
(https://github.com/bitcoin/bitcoin/issues/27787)

:lock: fanquake locked an issue: "Adchoose"
(https://github.com/bitcoin/bitcoin/issues/27787)

👍 ryanofsky approved a pull request: "wallet: Load database records in a particular order"
(https://github.com/bitcoin/bitcoin/pull/24914#pullrequestreview-1442091917)
Code review ACK 3c51e44380ba8041e5d6c4cb29b9b2c54fad0b4b

Overall changes look good, but I think two things could be done to make this easier to review:

1. The main thing would be making the "walletdb: refactor X loading" commits self contained and not leave behind dead code that needs to be cleaned up later. It's hard to identify what code is being replaced in current commits when not all the code being replaced is actually removed.
2. I think it might be good to move first 2 commits up t
...

💬 ryanofsky commented on pull request "wallet: Load database records in a particular order":
(https://github.com/bitcoin/bitcoin/pull/24914#discussion_r1204298801)
In commit "wallet: Add GetPrefixCursor to DatabaseBatch" (422a8436089c844934903a61fcec6b7b93995c07)

If prefix being non-empty is a requirement, it would be good to add this `Assume` to the `MockableBatch` cursor as well, so all of the implementations of this method consistently reject empty prefixes, and test cases don't appear to work with the mock database and then fail with real databases.

Alternately you could consider just moving the `Assume` checks out of the GetNewPrefixCursor metho
...

💬 ryanofsky commented on pull request "wallet: Load database records in a particular order":
(https://github.com/bitcoin/bitcoin/pull/24914#discussion_r1204326544)
In commit "walletdb: Refactor key reading and loading to its own function" (82e025297ba8939660b15fad738be723d783165e)

Note for other reviewers: There appears to be a slight change of behavior because now `wss.nKeys` will be incremented if the public key is not valid. This doesn't actually change behavior because counts are not used if there are any failures:

https://github.com/bitcoin/bitcoin/blob/214f8f18b310e3af88eba6a005439ae423ccd76a/src/wallet/walletdb.cpp#L932-L933

Also `wss` vari
...