💬 ryanofsky commented on issue "Bitcoin ignores datadir and blocksdir parameter in .conf":
(https://github.com/bitcoin/bitcoin/issues/27246#issuecomment-1470430978)
@desirepl thanks for clarifying your use case, that makes a lot of sense. I see the appeal of placing a stub bitcoin.conf in the default location that points to other blocks, index, and log directories, as well as to another config file. This lets you specify disk layout separately from network configuration, and makes applications and command line utilities work "magically" as you say without requiring manual arguments.

So I can see how behavior (3) from https://github.com/bitcoin/bitcoin/is
...

💬 pinheadmz commented on issue "Bitcoin ignores datadir and blocksdir parameter in .conf":
(https://github.com/bitcoin/bitcoin/issues/27246#issuecomment-1470439854)
@ryanofsky there is a discrepancy between bitcoind and the GUI though. I think it comes down to this line:

https://github.com/bitcoin/bitcoin/blob/8c4958bd4c06026dc108bc7f5f063d1f389d279b/src/qt/intro.cpp#L259-L265

The comment is well-meaning but I think what ends up happening is that `SoftSetArg()` ends up calling `ForceSetArg()` and that setting takes precedence over the `datadir` setting in the actual conf file.

with bitcoind you CAN:
- put conf file in default location
- that conf
...

💬 achow101 commented on pull request "bumpfee: Allow the user to choose which output is change":
(https://github.com/bitcoin/bitcoin/pull/26467#discussion_r1137498953)
They will be re-randomized.

💬 achow101 commented on pull request "bumpfee: Allow the user to choose which output is change":
(https://github.com/bitcoin/bitcoin/pull/26467#discussion_r1137504055)
Done

💬 achow101 commented on pull request "bumpfee: Allow the user to choose which output is change":
(https://github.com/bitcoin/bitcoin/pull/26467#discussion_r1137504281)
Done

💬 vasild commented on pull request "Handle CJDNS from LookupSubNet()":
(https://github.com/bitcoin/bitcoin/pull/27071#issuecomment-1470477392)
`c2a86ba667...fcd4d34b96`: rebase even though it is not strictly necessary. Now (after https://github.com/bitcoin-core/gui/pull/717 is merged) the GUI does not call `LookupSubNet()`.

Invalidates ACK from @jonatack

💬 mzumsande commented on pull request "addrman: Enable selecting addresses by network":
(https://github.com/bitcoin/bitcoin/pull/27214#discussion_r1137526957)
> Hmm? Now on master on mainnet it takes 0.18 microseconds and with this PR on mainnet it will take 6000 microseconds. That is the average case (averaged by the benchmark executing Select about 3500 times). The difference in the worst case is even more pronounced.

We might be using slightly different terminology here: With "worst case", I mean the edge case where we have just one entry in AddrMan of the kind of address that we want. This is the benchmark `AddrManSelectFromAlmostEmpty` for uns
...

💬 achow101 commented on pull request "bumpfee: Allow the user to choose which output is change":
(https://github.com/bitcoin/bitcoin/pull/26467#discussion_r1137532345)
I've added a fix, and adopted your test. The fix is simple enough to do here that I think a separate PR is unnecessary.

💬 ryanofsky commented on issue "Bitcoin ignores datadir and blocksdir parameter in .conf":
(https://github.com/bitcoin/bitcoin/issues/27246#issuecomment-1470504330)
> @ryanofsky there is a discrepancy between bitcoind and the GUI though. I think it comes down to this line:

Yes, that's a bug. That's the same line I pointed to above in "Unfortunately the behavior for bitcoin-qt is stupider than bitcoind" https://github.com/bitcoin/bitcoin/issues/27246#issuecomment-1470284856. It's a simple precedence bug and fix should be straightforward.

> the GUI also has this `QSettings` object which is stored in the system and is parsed before anything else. so if a
...

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