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

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

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

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

💬 hebasto commented on issue "Call interfaces::Wallet::getWalletTxs asynchronous":
(https://github.com/bitcoin/bitcoin/issues/20241#issuecomment-1460255944)
> @hebasto is it worth moving this issue to the GUI repo?

I'd keep it here as this issue's solution potentially can touch code out of the `qt` subdirectory.

💬 Sjors commented on pull request "test: Default timeout factor to 4 under --valgrind":
(https://github.com/bitcoin/bitcoin/pull/27221#issuecomment-1460267118)
Concept ACK. On my test machine `--valgrind --timeout-factor=4` works, whereas without `--timeout-factor` it doesn't. Haven't tried a lower value.

I also have to set `--jobs` very low (e.g. 5 on a 32 thread CPU, well below the point where it uses ~100% CPU), or I still get timeouts and other errors (e.g. `no RPC connection` in `p2p_tx_download.py`). So perhaps 4 is not enough if you want to max-out a machine, but fine for CI. (Also, I compiled without BDB)

Even with that some tests fail, b
...

💬 petertodd commented on issue "Allow several OP_RETURN in one tx and no limited size":
(https://github.com/bitcoin/bitcoin/issues/27043#issuecomment-1460280446)
Alright, I'm going to take a crack at this.

I'll ask that anyone else who hasn't already implemented this patch wait until next Monday before starting work on it. :)

📝 Sjors opened a pull request: "test: skip backward compatibility tests under valgrind"
(https://github.com/bitcoin/bitcoin/pull/27228)
They fail for me and it seems useless to old release binaries under valgrind anyway.

💬 Sjors commented on pull request "test: skip backward compatibility tests under valgrind":
(https://github.com/bitcoin/bitcoin/pull/27228#issuecomment-1460284735)
Example failure:

```
```
$ test/functional/feature_txindex_compatibility.py --valgrind --timeout-factor=4
2023-03-08T14:43:48.054000Z TestFramework (INFO): Initializing test directory /tmp/bitcoin_func_test_c2c_8427
2023-03-08T14:43:49.852000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
File "/home/sjors/dev/bitcoin/test/functional/test_framework/test_framework.py", line 559, in start_nodes
node.wait_for_rpc_connection()
File "/home/sjors/dev/bit
...

💬 achow101 commented on issue "wallet: balance gone when tx broadcast failed":
(https://github.com/bitcoin/bitcoin/issues/20943#issuecomment-1460286500)
> should it be the case that the wallet can create a transaction, which would fail `ATMP(test_accept=true)`

No.

> but still result in a balance reduction for the user?

Yes, ish. It is not a balance reduction per se, we don't track the balance as a number that gets incremented and decremented as transactions are made. Rather the balance is (re)computed by examining all txs and adding up the outputs that belong to the wallet. One of the criteria is that unconfirmed transactions need to be
...

💬 MarcoFalke commented on pull request "test: skip backward compatibility tests under valgrind":
(https://github.com/bitcoin/bitcoin/pull/27228#issuecomment-1460293794)
What about `test/functional/mempool_compatibility.py` etc?

💬 Sjors commented on pull request "test: skip backward compatibility tests under valgrind":
(https://github.com/bitcoin/bitcoin/pull/27228#issuecomment-1460295325)
For some reason that didn't fail. So an alternative could be to fix the ones that break for me, but not sure if that's worth the effort.

💬 LarryRuane commented on pull request "Add pool based memory resource":
(https://github.com/bitcoin/bitcoin/pull/25325#discussion_r1128596813)
Possibly related to https://github.com/bitcoin/bitcoin/pull/25325#discussion_r1084564591, the first time this runs, both `m_available_memory_it` and `m_available_memory_end` are `nullptr`; is it UB to pass these to `std::distance()`? I think it's okay, just wanted to raise as a possible concern. You could do something like
```suggestion
size_t remaining_available_bytes = m_available_memory_it ? std::distance(m_available_memory_it, m_available_memory_end) : 0;
```

💬 LarryRuane commented on pull request "Add pool based memory resource":
(https://github.com/bitcoin/bitcoin/pull/25325#discussion_r1129602006)
This benchmark doesn't re-use memory (add to freelist then remove from freelist); maybe it would be better if it did because that's what happens with the real coins cache, maybe something like this:

<details>
<summary>patch</summary>

```diff
- size_t batch_size = 5000;
+ // The steady-state size of the map will be half of this value;
+ // power-of-2 to avoid expensive mod operation during benchmark.
+ constexpr size_t batch_size = 1 << 13;

// make sure each iterat
...

💬 LarryRuane commented on pull request "Add pool based memory resource":
(https://github.com/bitcoin/bitcoin/pull/25325#discussion_r1128581100)
Why is `std::size_t` preferred over `size_t` within this file?

👍 1440000bytes approved a pull request: "doc: docment json rpc endpoints"
(https://github.com/bitcoin/bitcoin/pull/27225)
ACK https://github.com/bitcoin/bitcoin/pull/27225/commits/86cff0ee0cb78f5eb2568e7e863eb72baa571a53

[Error](https://github.com/bitcoin/bitcoin/blob/8d12127a9c19cb218d661a88ab9b6871c9d853b9/src/wallet/rpc/util.cpp#L93) is also helpful if someone is unaware of this and tries to use `/` endpoint with 2 or more wallets for wallet based RPC commands.

```
{
"result": null,
"error": {
"code": -19,
"message": "Wallet file not specified (must request wallet RPC through /
...