💬 dougEfresh commented on pull request "doc: Fixup remove 'omitted...' doc for rpc getrawtransaction when verbose is 2":
(https://github.com/bitcoin/bitcoin/pull/26968#issuecomment-1460338540)
@stickies-v I felt the same as you. The time spent on such a trivial change was too much. My apologies for not being more thorough with communication when I had questions. I assumed too much without 1st getting clarification. There too many threads and thus communication was lost. Again, my apologies for not being more communicative.
(https://github.com/bitcoin/bitcoin/pull/26968#issuecomment-1460338540)
@stickies-v I felt the same as you. The time spent on such a trivial change was too much. My apologies for not being more thorough with communication when I had questions. I assumed too much without 1st getting clarification. There too many threads and thus communication was lost. Again, my apologies for not being more communicative.
💬 willcl-ark commented on pull request "doc: docment json rpc endpoints":
(https://github.com/bitcoin/bitcoin/pull/27225#discussion_r1129624326)
Agree with this.
(https://github.com/bitcoin/bitcoin/pull/27225#discussion_r1129624326)
Agree with this.
💬 willcl-ark commented on pull request "doc: docment json rpc endpoints":
(https://github.com/bitcoin/bitcoin/pull/27225#discussion_r1129628727)
Again I think I agree here, especially seeing as there are no /wallet/ examples given in RPCHelp
(https://github.com/bitcoin/bitcoin/pull/27225#discussion_r1129628727)
Again I think I agree here, especially seeing as there are no /wallet/ examples given in RPCHelp
💬 martinus commented on pull request "Add pool based memory resource":
(https://github.com/bitcoin/bitcoin/pull/25325#discussion_r1129641656)
This is explicitly defined in the standard here, in 5.1: http://eel.is/c++draft/expr.add#5
(https://github.com/bitcoin/bitcoin/pull/25325#discussion_r1129641656)
This is explicitly defined in the standard here, in 5.1: http://eel.is/c++draft/expr.add#5
💬 martinus commented on pull request "Add pool based memory resource":
(https://github.com/bitcoin/bitcoin/pull/25325#discussion_r1129645169)
No real reason. Technically in C++ I believe `std::size_t` should be preferred, especially when using includes like `<cstddef>` instead of `<stddef.h>`. I personally usually use just `size_t` everywhere and never had a compile problem.
(https://github.com/bitcoin/bitcoin/pull/25325#discussion_r1129645169)
No real reason. Technically in C++ I believe `std::size_t` should be preferred, especially when using includes like `<cstddef>` instead of `<stddef.h>`. I personally usually use just `size_t` everywhere and never had a compile problem.
💬 pinheadmz commented on issue "Cannot do HTTP JSON RPC request on wallet":
(https://github.com/bitcoin/bitcoin/issues/25635#issuecomment-1460369990)
Your issue is that since your wallet name contains slashes, the http server is mis-reading the endpoint you are sending the request to. Or rather, it is reading it correctly but to sneak in those slashes you need to URI-encode you full wallet path.
For example I created a wallet at the path `/tmp/a/b/c/bitcoin/d/e/f`.
`bitcoin-cli` can handle the slashy wallet name:
```
$ src/bitcoin-cli -regtest -datadir=/tmp/a/b/c/bitcoin -rpcuser=u -rpcpassword=p -rpcwallet=/tmp/a/b/c/bitcoin/d/e/f
...
(https://github.com/bitcoin/bitcoin/issues/25635#issuecomment-1460369990)
Your issue is that since your wallet name contains slashes, the http server is mis-reading the endpoint you are sending the request to. Or rather, it is reading it correctly but to sneak in those slashes you need to URI-encode you full wallet path.
For example I created a wallet at the path `/tmp/a/b/c/bitcoin/d/e/f`.
`bitcoin-cli` can handle the slashy wallet name:
```
$ src/bitcoin-cli -regtest -datadir=/tmp/a/b/c/bitcoin -rpcuser=u -rpcpassword=p -rpcwallet=/tmp/a/b/c/bitcoin/d/e/f
...
💬 martinus commented on pull request "Add pool based memory resource":
(https://github.com/bitcoin/bitcoin/pull/25325#discussion_r1129650014)
Actually it does reuse memory, when calling `map.clear()` the nodes are put back into the freelist. All elements are deallocated, so he `pool_resource` gets them and puts them into their freelist. The memory is only ever released in the destructor of `pool_resource`
(https://github.com/bitcoin/bitcoin/pull/25325#discussion_r1129650014)
Actually it does reuse memory, when calling `map.clear()` the nodes are put back into the freelist. All elements are deallocated, so he `pool_resource` gets them and puts them into their freelist. The memory is only ever released in the destructor of `pool_resource`
💬 pinheadmz commented on pull request "doc: docment json rpc endpoints":
(https://github.com/bitcoin/bitcoin/pull/27225#discussion_r1129651141)
👍
(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.
(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
...
(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.
(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
(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.
(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
(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
(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.
(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!
(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
(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.
(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`).
(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`).