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

💬 pinheadmz commented on pull request "doc: docment json rpc endpoints":
(https://github.com/bitcoin/bitcoin/pull/27225#discussion_r1129612291)
I think these can be folded in above if you like my suggestions.

I think a few `curl` examples might be appropriate here as well since this document is covering raw endpoint stuff

💬 pinheadmz commented on pull request "doc: docment json rpc endpoints":
(https://github.com/bitcoin/bitcoin/pull/27225#discussion_r1129610797)
actually seems like its the right endpoint to use for all requests all the time! For non wallet requests you can still use `/wallet` with no wallet name

```
$ curl http://u:p@localhost:18443/wallet/ --data '{"method":"getblockcount"}'
{"result":206,"error":null,"id":null}
```

💬 pinheadmz commented on pull request "doc: docment json rpc endpoints":
(https://github.com/bitcoin/bitcoin/pull/27225#discussion_r1129607633)
It is also always active for non-wallet requests before any wallets are loaded.

💬 pinheadmz commented on pull request "doc: docment json rpc endpoints":
(https://github.com/bitcoin/bitcoin/pull/27225#discussion_r1129599313)
A few ideas about these:
- could include full path `http://localhost:{port}/`
- could list relevant ports per network (i.e. main=8332, regtest=18443 etc)
- not sure if this applies here but I'm used to seeing API endpoints with variables expressed like `/wallet/:walletname`, at least for REST APIs. I dunno if there is otherwise a convention for that

💬 pinheadmz commented on pull request "doc: docment json rpc endpoints":
(https://github.com/bitcoin/bitcoin/pull/27225#discussion_r1129604712)
```suggestion
This endpoint is always active for non-wallet requests, and can service wallet requests when exactly one wallet is loaded.
```