💬 fanquake commented on pull request "rpc, test: remove newline escape sequence from wallet warning fields":
(https://github.com/bitcoin/bitcoin/pull/27138#issuecomment-1460011191)
> Saw those for (maybe, maybe not) another pull; prefer to keep this small and focused.

If they are the same issue, what's the reasoning for leaving them for another PR? Not sure how the PR becomes less focused by fixing more of the same problem.

💬 fanquake commented on pull request "refactor: replace all implicit C-style const/const+reinterpret with explicit casts":
(https://github.com/bitcoin/bitcoin/pull/27126#discussion_r1129284068)
> That might make sense. If I get more time sometime soon, I'll see if I can easily span-ify it.

There doesn't seem to be much buy-in to making this change, and there's no real rush in any case, so feel free to look at using the Spans. Could probably mark this as a draft for now, if that is what you're going to look at.

💬 real-or-random commented on pull request "Reduce wasted pseudorandom bytes in ChaCha20 + various improvements":
(https://github.com/bitcoin/bitcoin/pull/26153#discussion_r1129293131)
```suggestion
static_assert(Num3072::BYTE_SIZE % 64 == 0);
ChaCha20Aligned(hashed_in.data(), hashed_in.size()).Keystream64(tmp, Num3072::BYTE_SIZE / 64);

```

💬 real-or-random commented on pull request "Reduce wasted pseudorandom bytes in ChaCha20 + various improvements":
(https://github.com/bitcoin/bitcoin/pull/26153#discussion_r1129311426)
(not this PR)

Should `SetIV()` also set `m_bufleft = 0`? If the caller sets an IV without seeking, and we have bytes left in the buffer, then the caller will get those remaining bytes which belong to the wrong buffer.

I mean, setting an IV without seeking is probably not a good idea anyway, and none of the callers currently do this. But I feel that's something that then should be excluded by the interface of the class, i.e., `SetIV()` and `SetKey()` should ideally seek to 0 (and perhaps be
...

💬 real-or-random commented on pull request "Reduce wasted pseudorandom bytes in ChaCha20 + various improvements":
(https://github.com/bitcoin/bitcoin/pull/26153#discussion_r1129318294)
Since this is a rekeying operation, consider `memory_cleanse`ing the cache buffer. (It will probably be overwritten soon anyway. but `SetKey()` should not happen frequently, so we don't need to worry about performance.)

I guess there should also be destructor that does the same. And (not this PR), a destructor of ChaCha20Aligned should `memory_cleanse` the key.

💬 real-or-random commented on pull request "Reduce wasted pseudorandom bytes in ChaCha20 + various improvements":
(https://github.com/bitcoin/bitcoin/pull/26153#discussion_r1129322207)
(not this PR)

nit: I'd prefer an interface that distinguishes between encryption and decryption. That makes the code easier to change if it will be reused for other ciphers.

💬 real-or-random commented on pull request "Reduce wasted pseudorandom bytes in ChaCha20 + various improvements":
(https://github.com/bitcoin/bitcoin/pull/26153#discussion_r1129313162)
(not this PR)

But I think initializing with an effective zero key should not be allowed by the interface of the class.

💬 fanquake commented on pull request "Reduce wasted pseudorandom bytes in ChaCha20 + various improvements":
(https://github.com/bitcoin/bitcoin/pull/26153#issuecomment-1460055753)
@real-or-random thanks for circling around to this post-merge.

💬 fanquake commented on issue "libevent: event_enable_debug_logging() not to be used after creating event base":
(https://github.com/bitcoin/bitcoin/issues/27182#issuecomment-1460060866)
Looks like, [according to the upstream reply](https://github.com/libevent/libevent/issues/1426#issuecomment-1455189774), this is not actually a problem at all, and the documentation is incorrect/outdated:

> I don't see any possible issues with calling `event_enable_debug_logging` after creating of `event_base`, and basically anytime (except for maybe a TSan warning, since the code is not uses proper atomics, but this should not be a problem, but it can leads to loosing few messages before/aft
...

💬 Sjors commented on pull request "Switch hardened derivation marker to h (in normalized descriptors and new wallets)":
(https://github.com/bitcoin/bitcoin/pull/26076#discussion_r1129354074)
Done

💬 Crypto2 commented on issue "Option to ignore small inputs when internal wallet is building TXes?":
(https://github.com/bitcoin/bitcoin/issues/20870#issuecomment-1460166098)
We already sweep them daily with scripts so they don't build up, would just be nice for them to not be spent automatically in between sweeps so it wouldn't have to be done as often.

💬 sipa commented on issue "Option to ignore small inputs when internal wallet is building TXes?":
(https://github.com/bitcoin/bitcoin/issues/20870#issuecomment-1460168607)
The coin selection algorithm should automatically avoid small inputs if they're uneconomical to spend (depending on feerate).

💬 hebasto commented on issue "libevent: event_enable_debug_logging() not to be used after creating event base":
(https://github.com/bitcoin/bitcoin/issues/27182#issuecomment-1460174988)
> So I think we can close this, and potentially follow up with documentation updates upstream?

Agree.

💬 Crypto2 commented on issue "Option to ignore small inputs when internal wallet is building TXes?":
(https://github.com/bitcoin/bitcoin/issues/20870#issuecomment-1460175164)
It doesn't seem to a lot of the time, or it just has it's own idea of what's economical lol.

📝 furszy opened a pull request: "wallet: 25806 follow-up"
(https://github.com/bitcoin/bitcoin/pull/27227)
Few small findings post-#25806 and extra cleanups, nothing biggie.

💬 MarcoFalke commented on issue "wallet_backup.py fails with AssertionError: not(50 == 0) [assert_equal(self.nodes[2].getbalance(), 0)]":
(https://github.com/bitcoin/bitcoin/issues/25652#issuecomment-1460182659)
https://cirrus-ci.com/task/5620574878171136?logs=ci#L3154

💬 sipa commented on issue "Option to ignore small inputs when internal wallet is building TXes?":
(https://github.com/bitcoin/bitcoin/issues/20870#issuecomment-1460186201)
@Crypto2 Examples would be very helpful to see where the algorithm can be improved.

That's of course orthogonal to providing an option like the one you're asking for, but I did want to point out that the system is already designed so that this shouldn't be necessary in general. If that system isn't doing its job, it seems higher priority to address (or at least investigate) that, as it could affect a lot more people.

💬 Sjors commented on pull request "Switch hardened derivation marker to h (in normalized descriptors and new wallets)":
(https://github.com/bitcoin/bitcoin/pull/26076#discussion_r1129462144)
Missed a few spots for legacy, pushed again.

💬 furszy commented on issue "Option to ignore small inputs when internal wallet is building TXes?":
(https://github.com/bitcoin/bitcoin/issues/20870#issuecomment-1460190872)
If you have a high number of small inputs (to the point of exceeding the max weight), maybe you are experiencing what #26720 is fixing.

✅ stickies-v closed an issue: "libevent: event_enable_debug_logging() not to be used after creating event base"
(https://github.com/bitcoin/bitcoin/issues/27182)