💬 ryanofsky commented on issue "Memory leak loading legacy wallet (BDB 4.8.30)":
(https://github.com/bitcoin/bitcoin/issues/27283#issuecomment-1477004863)
This is puzzling because https://github.com/bitcoin/bitcoin/issues/27283#issuecomment-1476822455 seems to suggest that memory is leaked while creating the BDB environment, but the commit which triggers the leak 8b5e7297c02f3100a9cb27bfe206e3fc617ec173 does not seem to be changing the way the environment is created or how it is used. The commit does switch from using a function called `CreateWalletDatabase` to another one called `MakeWalletDatabase`, but in both cases the functions are calling th
...

💬 furszy commented on pull request "RPC: Fix fund transaction crash when at 0-value, 0-fee":
(https://github.com/bitcoin/bitcoin/pull/27271#issuecomment-1477006955)
> What scenario are you envisioning where we have a selection_target of 0, a fee rate of 0, and no pre-selected inputs that we would then want to call SelectCoins? With how it currently works, this would lead to SelectCoins assuming we DID have pre-selected and trying to continue, which is definitely wrong: https://github.com/bitcoin/bitcoin/blob/master/src/wallet/spend.cpp#L604-L609

That is the "little change" that needs to be done that I mentioned above.

The simplest scenario is what is
...

💬 achow101 commented on issue "Memory leak loading legacy wallet (BDB 4.8.30)":
(https://github.com/bitcoin/bitcoin/issues/27283#issuecomment-1477072935)
I've been able to reproduce the leak that valgrind finds on master. It appears to be related to the `-privdb` option; looking at the BDB source, the problem occurs inside a block of code guarded by a check for the `DB_PRIVATE` flag. Disabling that with `-privdb=0` "resolves" the leak.

The bisect appears to be incorrect as the parent commit shows the same issue for me. I don't think there's really a solution for us other than upgrading BDB or ditching it entirely.

👍 theStack approved a pull request: "p2p: cleanup `LookupIntern`, `Lookup` and `LookupHost`"
(https://github.com/bitcoin/bitcoin/pull/26261)
re-ACK ec28c89ef69cd3fbb1a82785c841d303b2bf4826

💬 jamesob commented on pull request "Add pool based memory resource":
(https://github.com/bitcoin/bitcoin/pull/25325#issuecomment-1477135406)
Kicked off a bitcoinperf run; will have some results tomorrow.

👍 theStack approved a pull request: "refactor, net: End friendship of CNode, CConnman and ConnmanTestMsg"
(https://github.com/bitcoin/bitcoin/pull/27257)
Code-review ACK 6514352829364ab1ccff3a58faf6ca6ef4aafbbb

💬 theStack commented on pull request "refactor, net: End friendship of CNode, CConnman and ConnmanTestMsg":
(https://github.com/bitcoin/bitcoin/pull/27257#discussion_r1142807687)
in commit d816bcced0c1afb22f97df4650a35301a25cd9de: nit: since `fMoreWork` is unused above, could reduce it's scope and declare/init it right here:
```suggestion
bool fMoreWork = poll_result->second;
```

💬 instagibbs commented on pull request "Replace MIN_STANDARD_TX_NONWITNESS_SIZE to preclude 64 non-witness bytes only":
(https://github.com/bitcoin/bitcoin/pull/26398#issuecomment-1477159508)
Closing for now. We can revisit this at some later date.

✅ instagibbs closed a pull request: "Replace MIN_STANDARD_TX_NONWITNESS_SIZE to preclude 64 non-witness bytes only"
(https://github.com/bitcoin/bitcoin/pull/26398)

💬 crystalshay2es commented on pull request "system: allow GUI to initialize default data dir":
(https://github.com/bitcoin/bitcoin/pull/27273#issuecomment-1477197720)
> Default

💬 dhruv commented on pull request "BIP324: Cipher suite":
(https://github.com/bitcoin/bitcoin/pull/25361#discussion_r1142867521)
I think I forgot to remove this previously used code. Thanks for pointing it out.

💬 dhruv commented on pull request "BIP324: Cipher suite":
(https://github.com/bitcoin/bitcoin/pull/25361#discussion_r1142867917)
Done. Although I did leave it as-is in the second commit and addressed it in the fourth commit because I was getting a linker error for multiple definitions trying it on the second commit. The net change isn't different however.

💬 dhruv commented on pull request "BIP324: Cipher suite":
(https://github.com/bitcoin/bitcoin/pull/25361#issuecomment-1477247368)
Addressed review by @theStack.

💬 dhruv commented on pull request "BIP324: Add encrypted p2p transport {de}serializer":
(https://github.com/bitcoin/bitcoin/pull/23233#issuecomment-1477250064)
Latest push:
- Upstream changes from #25361
- Rebased

💬 dhruv commented on pull request "BIP324: CKey encode/decode to elligator-swift":
(https://github.com/bitcoin/bitcoin/pull/23432#issuecomment-1477250673)
Rebased.

💬 dhruv commented on pull request "BIP324: Handshake prerequisites":
(https://github.com/bitcoin/bitcoin/pull/23561#issuecomment-1477251465)
Rebased.

💬 dhruv commented on pull request "BIP324: Enable v2 P2P encrypted transport":
(https://github.com/bitcoin/bitcoin/pull/24545#issuecomment-1477252274)
Rebased.

💬 MarcoFalke commented on issue "test: `wallet_importdescriptors.py --descriptors` failure under `--valgrind` ":
(https://github.com/bitcoin/bitcoin/issues/27282#issuecomment-1477374232)
Can remove `--valgrind` from title? See also https://github.com/bitcoin/bitcoin/pull/27199#pullrequestreview-1349425797

💬 MarcoFalke commented on pull request "Add feerate histogram to getmempoolinfo":
(https://github.com/bitcoin/bitcoin/pull/21422#issuecomment-1477392747)
> How can we expect the Bitcoin Core wallet to behave reasonably if it can't predict the fees well?

The Bitcoin Core wallet does not use this feature?

💬 jonasschnelli commented on pull request "Add feerate histogram to getmempoolinfo":
(https://github.com/bitcoin/bitcoin/pull/21422#issuecomment-1477425501)
The original idea of this patch was to add a small and easy to maintain **efficient** way to generate a fee histogram to bitcoin core (see original PR description https://github.com/bitcoin/bitcoin/pull/15836#issue-434109585).

As described there, it was always possible to use `getrawmempool` (@jhoenicke script) to achieve this goal.

However, it is **not efficient** to dumb out all transactions of the complete mempool in a json format just to calculate the fee histogram with a python script
...

💬 S3RK commented on pull request "wallet, tests: Expand and test when the blank wallet flag should be un/set":
(https://github.com/bitcoin/bitcoin/pull/25634#issuecomment-1477439839)
As far as I understand after this PR is merged we will start unsetting the blank flag and loosing the users' intent.
Are we okay with loosing the intent for the wallets that used the code between this and the follow up PR?