👍 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 /
...
(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
(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}
```
(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.
(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
(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.
```
(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.
```
💬 pinheadmz commented on pull request "doc: docment json rpc endpoints":
(https://github.com/bitcoin/bitcoin/pull/27225#discussion_r1129608697)
```suggestion
This is the endpoint used by bitcoin-cli when a `-rpcwallet=` parameter is passed in.
```
(https://github.com/bitcoin/bitcoin/pull/27225#discussion_r1129608697)
```suggestion
This is the endpoint used by bitcoin-cli when a `-rpcwallet=` parameter is passed in.
```
💬 pinheadmz commented on issue "Blockchain sync - highly variable time to connect trasactions":
(https://github.com/bitcoin/bitcoin/issues/21722#issuecomment-1460330476)
> Ah that makes sense, i suppose checking the balance is part of the validation...
Not just balance but the UTXOs store the output script needed for any spending validation. Have you been able to reproduce this issue with the latest release of Bitcoin Core @benjamin-wilson ?
(https://github.com/bitcoin/bitcoin/issues/21722#issuecomment-1460330476)
> Ah that makes sense, i suppose checking the balance is part of the validation...
Not just balance but the UTXOs store the output script needed for any spending validation. Have you been able to reproduce this issue with the latest release of Bitcoin Core @benjamin-wilson ?
💬 dhruv commented on pull request "BIP324: Add encrypted p2p transport {de}serializer":
(https://github.com/bitcoin/bitcoin/pull/23233#issuecomment-1460331730)
Latest push:
- Implemented changes from https://github.com/bitcoin/bips/pull/1428 involving fixed length message type ids (there's also a relevant discussion on the ML)
- While implementing it, I found a bug in the arm of code that serialized/deserialized message types without assigned short ids. That branch of code was previously unexercised by any tests. This is no longer the case.
(https://github.com/bitcoin/bitcoin/pull/23233#issuecomment-1460331730)
Latest push:
- Implemented changes from https://github.com/bitcoin/bips/pull/1428 involving fixed length message type ids (there's also a relevant discussion on the ML)
- While implementing it, I found a bug in the arm of code that serialized/deserialized message types without assigned short ids. That branch of code was previously unexercised by any tests. This is no longer the case.
💬 dhruv commented on pull request "BIP324: Handshake prerequisites":
(https://github.com/bitcoin/bitcoin/pull/23561#issuecomment-1460334287)
Rebased. Brought in changes from https://github.com/bitcoin-core/secp256k1/pull/1129.
(https://github.com/bitcoin/bitcoin/pull/23561#issuecomment-1460334287)
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_r1129623418)
(but only when the wallet is compiled in).
Also I think i've covered that it handles both types of request when no wallet is loaded in the Quirks section?
(https://github.com/bitcoin/bitcoin/pull/27225#discussion_r1129623418)
(but only when the wallet is compiled in).
Also I think i've covered that it handles both types of request when no wallet is loaded in the Quirks section?
💬 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.