💬 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
...
(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.
(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.
(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.
(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.
(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
...
(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
(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.
(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).
(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.
(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.
(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.
(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
(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.
(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.
(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.
(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)
(https://github.com/bitcoin/bitcoin/issues/27182)
💬 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-1460211133)
@stickies-v Thanks for making this issue clear!
(https://github.com/bitcoin/bitcoin/issues/27182#issuecomment-1460211133)
@stickies-v Thanks for making this issue clear!
💬 stickies-v commented on issue "libevent: event_enable_debug_logging() not to be used after creating event base":
(https://github.com/bitcoin/bitcoin/issues/27182#issuecomment-1460242132)
Agreed, looks like we don't need to take any further action. I'll follow up upstream next week. Closed.
(https://github.com/bitcoin/bitcoin/issues/27182#issuecomment-1460242132)
Agreed, looks like we don't need to take any further action. I'll follow up upstream next week. Closed.
💬 willcl-ark commented on issue "wallet: balance gone when tx broadcast failed":
(https://github.com/bitcoin/bitcoin/issues/20943#issuecomment-1460255465)
```
2021-01-15T09:26:28Z [default wallet] CommitTransaction(): Transaction cannot be broadcast immediately, mempool min fee not met, 142 < 152
```
This error is surfacing from `Prechecks()` checking the fee rate:
https://github.com/bitcoin/bitcoin/blob/8d12127a9c19cb218d661a88ab9b6871c9d853b9/src/validation.cpp#L847-L850
@achow101 should it be the case that the wallet can create a transaction, which would fail `ATMP(test_accept=true)` but still result in a balance reduction for the us
...
(https://github.com/bitcoin/bitcoin/issues/20943#issuecomment-1460255465)
```
2021-01-15T09:26:28Z [default wallet] CommitTransaction(): Transaction cannot be broadcast immediately, mempool min fee not met, 142 < 152
```
This error is surfacing from `Prechecks()` checking the fee rate:
https://github.com/bitcoin/bitcoin/blob/8d12127a9c19cb218d661a88ab9b6871c9d853b9/src/validation.cpp#L847-L850
@achow101 should it be the case that the wallet can create a transaction, which would fail `ATMP(test_accept=true)` but still result in a balance reduction for the us
...