💬 pinheadmz commented on issue "Bitcoin ignores datadir and blocksdir parameter in .conf":
(https://github.com/bitcoin/bitcoin/issues/27246#issuecomment-1470506278)
👍 PR in progress on this branch: https://github.com/pinheadmz/bitcoin/commits/conf-file

💬 achow101 commented on pull request "wallet: Implement independent BDB parser":
(https://github.com/bitcoin/bitcoin/pull/26606#discussion_r1137556050)
Done

💬 achow101 commented on pull request "wallet: Implement independent BDB parser":
(https://github.com/bitcoin/bitcoin/pull/26606#discussion_r1137556512)
It's kind of annoying to do that, so I'll leave it as is. The other functions do the same.

💬 achow101 commented on pull request "wallet: Implement independent BDB parser":
(https://github.com/bitcoin/bitcoin/pull/26606#discussion_r1137556948)
The intent is to have all of the migration stuff in these files, so I will leave it as is.

💬 achow101 commented on pull request "wallet: Implement independent BDB parser":
(https://github.com/bitcoin/bitcoin/pull/26606#discussion_r1137557089)
Updated the message.

💬 LarryRuane commented on pull request "reduce cs_main scope, guard block index 'nFile' under a local mutex":
(https://github.com/bitcoin/bitcoin/pull/27006#discussion_r1137557817)
Shouldn't these be fetched atomically? Maybe there should be a getter that returns all three values, similar to how there's a setter (`CBlockIndex::SetFileData`) than sets all three atomically.

💬 achow101 commented on pull request "Introduce `MockableDatabase` for wallet unit tests":
(https://github.com/bitcoin/bitcoin/pull/26715#discussion_r1137567867)
Done

💬 achow101 commented on pull request "Introduce `MockableDatabase` for wallet unit tests":
(https://github.com/bitcoin/bitcoin/pull/26715#discussion_r1137567968)
Done

💬 stratospher commented on pull request "rpc: Add test-only RPC getaddrmaninfo for new/tried table address count":
(https://github.com/bitcoin/bitcoin/pull/26988#discussion_r1137596851)
discussed this more with @amitiuttarwar and understood that `nKey` would remain the [same](https://github.com/bitcoin/bitcoin/blob/8c4958bd4c06026dc108bc7f5f063d1f389d279b/src/addrman.cpp#L109) for a node's addrman and hence same bucket position.

i tried generating random ipv6 addresses and inserting into addrman:
```
while len(node.getnodeaddresses(count=0)) < 3:
ipv6_address = ":".join(format(randint(0, 0xffff), '04x') for _ in range(8))
node.addpeeraddress(addre
...

📝 theStack opened a pull request: "test: check that sigop limit also affects ancestor/descendant size (27171 follow-up)"
(https://github.com/bitcoin/bitcoin/pull/27265)
This is a follow-up to #27171, adding a check that the sigop-limit vsize logic is also respected for {ancestor,descendant}size calculation (as suggested in https://github.com/bitcoin/bitcoin/pull/27171#pullrequestreview-1331143909). For simplicity, we use a one-parent-one-child cluster here and only check for the case that the sigop-limit equivalent size is larger than the serialized vsize.

💬 stratospher commented on pull request "rpc: Add test-only RPC getaddrmaninfo for new/tried table address count":
(https://github.com/bitcoin/bitcoin/pull/26988#discussion_r1137601762)
> hm, seems a little low-level for release notes. maybe something like

sounds much better! thank you!

> the target group for release notes are regular users, while test-only options are meant for devs and may not be stable.

interesting, thanks! i've updated it.

💬 TheCharlatan commented on pull request "Introduce `MockableDatabase` for wallet unit tests":
(https://github.com/bitcoin/bitcoin/pull/26715#discussion_r1137603241)
ACK, this is resolved.

💬 TheCharlatan commented on pull request "Introduce `MockableDatabase` for wallet unit tests":
(https://github.com/bitcoin/bitcoin/pull/26715#discussion_r1137603410)
ACK, this is resolved.

💬 stratospher commented on pull request "rpc: Add test-only RPC getaddrmaninfo for new/tried table address count":
(https://github.com/bitcoin/bitcoin/pull/26988#discussion_r1137605901)
makes sense, sporadic failures do happen in that commit (removed it now).
continued this discussion here - https://github.com/bitcoin/bitcoin/pull/26988#discussion_r1137596851

💬 brunoerg commented on pull request "rpc: Add test-only RPC getaddrmaninfo for new/tried table address count":
(https://github.com/bitcoin/bitcoin/pull/26988#discussion_r1137608018)
What's the reason for `-cjdnsreachable` here?

💬 stratospher commented on pull request "p2p: Improve diversification of new connections":
(https://github.com/bitcoin/bitcoin/pull/27264#issuecomment-1470614184)
I wasn't able to answer this https://github.com/bitcoin/bitcoin/pull/19860#discussion_r829878150. (so didn't include it)
Do you(/anyone) know why that commit is needed?

💬 stratospher commented on pull request "rpc: Add test-only RPC getaddrmaninfo for new/tried table address count":
(https://github.com/bitcoin/bitcoin/pull/26988#discussion_r1137617169)
oops, forgot to revert that. thanks!

💬 brunoerg commented on pull request "rpc: Add test-only RPC getaddrmaninfo for new/tried table address count":
(https://github.com/bitcoin/bitcoin/pull/26988#discussion_r1137621405)
ohh, got it! thank you!

💬 ryanofsky commented on issue "Bitcoin ignores datadir and blocksdir parameter in .conf":
(https://github.com/bitcoin/bitcoin/issues/27246#issuecomment-1470663620)
> +1 PR in progress on this branch: https://github.com/pinheadmz/bitcoin/commits/conf-file

Thanks, just some quick feedback on 2a2488fab691a69b66395929a2c645b4a2649ace: I don't think it would be good to add a new level of settings precedence just to deal with this one GUI setting. We migrated all settings out of QSettings which are not GUI specific in https://github.com/bitcoin-core/gui/pull/602 and we can migrate `strDataDir` as well to avoid unnecessary differences between CLI and GUI behav
...

💬 ryanofsky commented on pull request "refactor: Remove CAddressBookData::destdata":
(https://github.com/bitcoin/bitcoin/pull/27224#discussion_r1137647274)
> I strongly dislike using the empty string to indicate deletion. I think it would be better to have a separate `RemoveReceiveRequest`.

You are right. This is shitty and I will add a separate method to avoid continuing this pattern.