💬 pinheadmz commented on pull request "doc: docment json rpc endpoints":
(https://github.com/bitcoin/bitcoin/pull/27225#discussion_r1129651141)
👍

💬 pinheadmz commented on pull request "doc: docment json rpc endpoints":
(https://github.com/bitcoin/bitcoin/pull/27225#discussion_r1129653576)
yeah i dunno whats best to reccomend. I think personally Id use `/` for node requests and `/wallet/walletname` for any wallet request no matter how many wallets are loaded.

💬 LarryRuane commented on pull request "Add pool based memory resource":
(https://github.com/bitcoin/bitcoin/pull/25325#discussion_r1129703688)
That's a good point, I hadn't noticed that. But perhaps it's good to make the load more realistic to get better benchmarking results. I'm assuming that when UTXOs become spent TXOs during block validation, they're removed from this unordered_map, but maybe that's not true? Just seems like doing many interleaving allocations and deallocations would be more realistic, but maybe not. But this suggestion is non-blocking.

In case anyone is wondering, I observed that the benchmark does cause alloca
...

💬 MarcoFalke commented on pull request "test: skip some backward compatibility tests under valgrind":
(https://github.com/bitcoin/bitcoin/pull/27228#issuecomment-1460499784)
It might be more useful to disable them all here for the compat test only and instead run the full test suite on the guix binaries on release tags.

💬 willcl-ark commented on pull request "doc: docment json rpc endpoints":
(https://github.com/bitcoin/bitcoin/pull/27225#discussion_r1129764736)
Done

💬 willcl-ark commented on pull request "doc: docment json rpc endpoints":
(https://github.com/bitcoin/bitcoin/pull/27225#discussion_r1129766192)
Leaving this for now, as using the bare `/wallet/` endpoint could be considered unsupported behaviour.

💬 willcl-ark commented on pull request "doc: docment json rpc endpoints":
(https://github.com/bitcoin/bitcoin/pull/27225#discussion_r1129768646)
Done

💬 willcl-ark commented on pull request "doc: docment json rpc endpoints":
(https://github.com/bitcoin/bitcoin/pull/27225#discussion_r1129768997)
Removed quirks

💬 fanquake commented on pull request "test: skip some backward compatibility tests under valgrind":
(https://github.com/bitcoin/bitcoin/pull/27228#issuecomment-1460507219)
This seems arbitrary, and the `can't run this test under valgrind` message is not useful. I assume the first questions someone will have are going to be "Why can't I run _this_ test under Valgrind?", "Does it need fixing then?" etc.

💬 willcl-ark commented on pull request "doc: docment json rpc endpoints":
(https://github.com/bitcoin/bitcoin/pull/27225#discussion_r1129764196)
Personally I feel it's clearer just to use the endpoints themselves, rather than cluttering with hosts and ports.

Also, we have examples now!

💬 achow101 commented on pull request "rpc, wallet: use the same `next_index` key in `listdescriptors` and `importdescriptors`":
(https://github.com/bitcoin/bitcoin/pull/26194#issuecomment-1460516035)
ACK b082f28101773e0ef0281d97025e65d0364d6f29

💬 MarcoFalke commented on pull request "test: Make the unlikely race in p2p_invalid_messages impossible?":
(https://github.com/bitcoin/bitcoin/pull/27212#issuecomment-1460519235)
> Would it make any more sense to instead add wait_for_verack=True to the initial add_p2p_connection()?

Yes, this is the default value already.

💬 MarcoFalke commented on pull request "test: Make the unlikely race in p2p_invalid_messages impossible?":
(https://github.com/bitcoin/bitcoin/pull/27212#discussion_r1129786075)
> Is the flush on the node side? Like the pong has been sent but is still sitting in nodes buffer?

Correct, it is in the node's receive buffer (from `conn`).

💬 pinheadmz commented on pull request "test: Make the unlikely race in p2p_invalid_messages impossible?":
(https://github.com/bitcoin/bitcoin/pull/27212#discussion_r1129773661)
Is the flush on the node side? Like the pong has been sent but is still sitting in nodes buffer? I'm just trying to reliably break the test on master to examine the patch

🚀 achow101 merged a pull request: "rpc, wallet: use the same `next_index` key in `listdescriptors` and `importdescriptors`"
(https://github.com/bitcoin/bitcoin/pull/26194)

👍 theStack approved a pull request: "test: Flatten miniwallet array and remove random fee in longpoll"
(https://github.com/bitcoin/bitcoin/pull/26996)
re-ACK fa0abcdafeb74aa7a312095becd55b1cee48cd99

💬 glozow commented on pull request "test: add coverage for sigop limit policy (`-bytespersigop` setting)":
(https://github.com/bitcoin/bitcoin/pull/27171#discussion_r1129803993)
Ah I see where that comes from, thanks for clarifying! Comment does look better to me now.

💬 MarcoFalke commented on pull request "test: skip some backward compatibility tests under valgrind":
(https://github.com/bitcoin/bitcoin/pull/27228#issuecomment-1460524502)
I presume the reason is that the valgrind supp file is not valid for previous releases. Actually, it might not even be valid for guix-compiled binaries on master?

💬 Sjors commented on pull request "test: skip some backward compatibility tests under valgrind":
(https://github.com/bitcoin/bitcoin/pull/27228#issuecomment-1460547152)
I disabled all tests that use previous releases under valgrind. Also tried a different error message.

> "Does it need fixing then?"

I added a code comment and some more context to the commit to help answer that, if anyone _does_ want to run these under valgrind.

💬 MarcoFalke commented on pull request "test: skip some backward compatibility tests under valgrind":
(https://github.com/bitcoin/bitcoin/pull/27228#discussion_r1129815496)
nit: Might be a smaller diff, and more clear, if this was put into the `skip_if_no_previous_releases` function?