💬 LarryRuane commented on pull request "util: improve FindByte() performance":
(https://github.com/bitcoin/bitcoin/pull/19690#issuecomment-1476862325)
Force-pushed again to fix CI failures.

💬 S3RK commented on pull request "wallet: return error msg for "too-long-mempool-chain"":
(https://github.com/bitcoin/bitcoin/pull/24845#discussion_r1142628251)
Yes you're right, I somehow managed to confuse myself about the filters. Thanks for patient explanation.

Still something doesn't sit right. So if a coin is discarded because of `max_ancestors` we give this error message, but if it's discarded by the filter `max_ancestors/2` we just say insufficient funds when the coins are there they just don't meet the filters. There are also other reasons when the coins are discarded, for example unsafe utxos. Even you enable unsafe utxos, we don't allow an
...

👍 TheCharlatan approved a pull request: "Introduce `MockableDatabase` for wallet unit tests"
(https://github.com/bitcoin/bitcoin/pull/26715)
Code review ACK [a9c58a9](https://github.com/bitcoin/bitcoin/pull/26715/commits/a9c58a93bc4929724ff539bb5be457e8b2eb7dc0)

💬 TheCharlatan commented on pull request "Introduce `MockableDatabase` for wallet unit tests":
(https://github.com/bitcoin/bitcoin/pull/26715#discussion_r1142666804)
nit: I don't think this include is required.

📝 achow101 opened a pull request: "wallet: Keep track of the wallet's own transaction outputs in memory"
(https://github.com/bitcoin/bitcoin/pull/27286)
Currently, the wallet is not actually aware about its own transaction outputs. Instead, it will iterate all of the transactions stored in `mapWallet`, and then all of the outputs of those transactions, in order to figure out what belongs to it for the purposes of coin selection and balance calculation. For balance calculation, there is caching that results in it only iterating all of the transactions, but not all of the outputs. However when the cache is dirty, everything is iterated. This is es
...

💬 furszy commented on pull request "wallet: return error msg for "too-long-mempool-chain"":
(https://github.com/bitcoin/bitcoin/pull/24845#discussion_r1142716211)
> Still something doesn't sit right. So if a coin is discarded because of max_ancestors we give this error message, but if it's discarded by the filter max_ancestors/2 we just say insufficient funds when the coins are there they just don't meet the filters

no, those coins will be added to the last non-customizable filter which has `max_ancestors-1` and `max_descendants-1`.

> There are also other reasons when the coins are discarded, for example unsafe utxos. Even you enable unsafe utxos, w
...

💬 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