💬 dhruv commented on pull request "BIP324: CKey encode/decode to elligator-swift":
(https://github.com/bitcoin/bitcoin/pull/23432#issuecomment-1460333061)
Rebased. Brought in changes from https://github.com/bitcoin-core/secp256k1/pull/1129.
(https://github.com/bitcoin/bitcoin/pull/23432#issuecomment-1460333061)
Rebased. Brought in changes from https://github.com/bitcoin-core/secp256k1/pull/1129.
💬 willcl-ark commented on pull request "doc: docment json rpc endpoints":
(https://github.com/bitcoin/bitcoin/pull/27225#discussion_r1129620778)
I see your point here, but I was also trying to distinguish: active=enabled, services=wallet/non-wallet.
How about:
> This endpoint is always active. It can always service non-wallet requests and can service wallet requests when exactly one wallet is loaded.
(https://github.com/bitcoin/bitcoin/pull/27225#discussion_r1129620778)
I see your point here, but I was also trying to distinguish: active=enabled, services=wallet/non-wallet.
How about:
> This endpoint is always active. It can always service non-wallet requests and can service wallet requests when exactly one wallet is loaded.
💬 dhruv commented on pull request "BIP324: Enable v2 P2P encrypted transport":
(https://github.com/bitcoin/bitcoin/pull/24545#issuecomment-1460336635)
Rebased with master. Brought in changes to #23233, #23561 and #23432. **The changes to the BIP [here](https://github.com/bitcoin/bips/pull/1428) are implemented and result in a breaking change for everyone running v2 nodes on mainnet.**
(https://github.com/bitcoin/bitcoin/pull/24545#issuecomment-1460336635)
Rebased with master. Brought in changes to #23233, #23561 and #23432. **The changes to the BIP [here](https://github.com/bitcoin/bips/pull/1428) are implemented and result in a breaking change for everyone running v2 nodes on mainnet.**
💬 willcl-ark commented on pull request "doc: docment json rpc endpoints":
(https://github.com/bitcoin/bitcoin/pull/27225#discussion_r1129626897)
Oh, I didn't know you could use the endpoint without a wallet name at all!
IMO it makes sense to specify that it should be used with the `wallet/<walletname>/` syntax for multiwallet (as designed?)
(https://github.com/bitcoin/bitcoin/pull/27225#discussion_r1129626897)
Oh, I didn't know you could use the endpoint without a wallet name at all!
IMO it makes sense to specify that it should be used with the `wallet/<walletname>/` syntax for multiwallet (as designed?)
💬 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!