💬 stickies-v commented on pull request "doc: Add multiprocess design doc":
(https://github.com/bitcoin/bitcoin/pull/28978#discussion_r1439470004)
```suggestion
- When the call returns, it encapsulates the return value in a Cap’n Proto response, which it sends back to the `bitcoin-wallet` process.
```
(https://github.com/bitcoin/bitcoin/pull/28978#discussion_r1439470004)
```suggestion
- When the call returns, it encapsulates the return value in a Cap’n Proto response, which it sends back to the `bitcoin-wallet` process.
```
👍 stickies-v approved a pull request: "doc: Add multiprocess design doc"
(https://github.com/bitcoin/bitcoin/pull/28978#pullrequestreview-1800375604)
ACK 91dc48c14825a9075a57c1eefda202b83b6346ba - left a couple of improvements but agreed that iterating in future PRs is better.
(https://github.com/bitcoin/bitcoin/pull/28978#pullrequestreview-1800375604)
ACK 91dc48c14825a9075a57c1eefda202b83b6346ba - left a couple of improvements but agreed that iterating in future PRs is better.
💬 Randy808 commented on pull request "Add OP_TXHASH and OP_CHECKTXHASHVERIFY opcodes":
(https://github.com/bitcoin/bitcoin/pull/29050#discussion_r1439490798)
nit: Consider adding curly braces for here for clarity (since it's so close to the break)
(https://github.com/bitcoin/bitcoin/pull/29050#discussion_r1439490798)
nit: Consider adding curly braces for here for clarity (since it's so close to the break)
💬 Randy808 commented on pull request "Add OP_TXHASH and OP_CHECKTXHASHVERIFY opcodes":
(https://github.com/bitcoin/bitcoin/pull/29050#discussion_r1439490848)
Leaving it on the stack seems inconsistent with the semantics of the other 'VERIFY' opcodes
(https://github.com/bitcoin/bitcoin/pull/29050#discussion_r1439490848)
Leaving it on the stack seems inconsistent with the semantics of the other 'VERIFY' opcodes
💬 glozow commented on issue "is a getBlockTemplate transactions's bug?":
(https://github.com/bitcoin/bitcoin/issues/29166#issuecomment-1874086428)
If you have a general question, consider asking on [stack exchange](https://bitcoin.stackexchange.com/).
If this is a bug report, what was the JSON result returned from `getblocktemplate` when this happened?
(https://github.com/bitcoin/bitcoin/issues/29166#issuecomment-1874086428)
If you have a general question, consider asking on [stack exchange](https://bitcoin.stackexchange.com/).
If this is a bug report, what was the JSON result returned from `getblocktemplate` when this happened?
💬 Randy808 commented on pull request "Add OP_TXHASH and OP_CHECKTXHASHVERIFY opcodes":
(https://github.com/bitcoin/bitcoin/pull/29050#discussion_r1439497216)
I guess there's nothing wrong with concatenating the arguments and pushing it on the stack as one element to save bytes, but this part also seems inconsistent with the other opcodes. Not the most important thing in the world though, I'm curious what others think
(https://github.com/bitcoin/bitcoin/pull/29050#discussion_r1439497216)
I guess there's nothing wrong with concatenating the arguments and pushing it on the stack as one element to save bytes, but this part also seems inconsistent with the other opcodes. Not the most important thing in the world though, I'm curious what others think
💬 Randy808 commented on pull request "Add OP_TXHASH and OP_CHECKTXHASHVERIFY opcodes":
(https://github.com/bitcoin/bitcoin/pull/29050#discussion_r1439504948)
Good list of options 👍
(https://github.com/bitcoin/bitcoin/pull/29050#discussion_r1439504948)
Good list of options 👍
💬 dergoegge commented on pull request "PoC: fuzz chainstate and block managers":
(https://github.com/bitcoin/bitcoin/pull/29158#discussion_r1439533284)
Have you checked if using `fmemopen` is better/faster than using a ram disk?
(https://github.com/bitcoin/bitcoin/pull/29158#discussion_r1439533284)
Have you checked if using `fmemopen` is better/faster than using a ram disk?
💬 hebasto commented on pull request "build: use `-no_exported_symbols` on macOS":
(https://github.com/bitcoin/bitcoin/pull/29072#discussion_r1439553990)
This makes `libbitcoinconsensus.0.dylib` export no symbols, which is not what we want, right?
```
% dyld_info -exports src/.libs/libbitcoinconsensus.0.dylib
src/.libs/libbitcoinconsensus.0.dylib [arm64]:
-exports:
offset symbol
```
(https://github.com/bitcoin/bitcoin/pull/29072#discussion_r1439553990)
This makes `libbitcoinconsensus.0.dylib` export no symbols, which is not what we want, right?
```
% dyld_info -exports src/.libs/libbitcoinconsensus.0.dylib
src/.libs/libbitcoinconsensus.0.dylib [arm64]:
-exports:
offset symbol
```
💬 achow101 commented on pull request "doc: Add multiprocess design doc":
(https://github.com/bitcoin/bitcoin/pull/28978#issuecomment-1874192332)
ACK 91dc48c14825a9075a57c1eefda202b83b6346ba
(https://github.com/bitcoin/bitcoin/pull/28978#issuecomment-1874192332)
ACK 91dc48c14825a9075a57c1eefda202b83b6346ba
💬 martinus commented on pull request "test: doc: follow-up #28368":
(https://github.com/bitcoin/bitcoin/pull/29013#issuecomment-1874194468)
re-ACK b1318dcc56a0181783ee7ddbd388ae878a0efc52
(https://github.com/bitcoin/bitcoin/pull/29013#issuecomment-1874194468)
re-ACK b1318dcc56a0181783ee7ddbd388ae878a0efc52
🚀 achow101 merged a pull request: "doc: Add multiprocess design doc"
(https://github.com/bitcoin/bitcoin/pull/28978)
(https://github.com/bitcoin/bitcoin/pull/28978)
💬 jamesob commented on pull request "PoC: fuzz chainstate and block managers":
(https://github.com/bitcoin/bitcoin/pull/29158#issuecomment-1874196101)
Cool, this is a great thing to investigate. I'll be giving the approach a look this week.
(https://github.com/bitcoin/bitcoin/pull/29158#issuecomment-1874196101)
Cool, this is a great thing to investigate. I'll be giving the approach a look this week.
💬 achow101 commented on pull request "refactor: share and use `GenerateRandomKey` helper":
(https://github.com/bitcoin/bitcoin/pull/28455#issuecomment-1874196735)
ACK fa1d49542e4b69a5d8b1177ffe4207f051a468bb
(https://github.com/bitcoin/bitcoin/pull/28455#issuecomment-1874196735)
ACK fa1d49542e4b69a5d8b1177ffe4207f051a468bb
🚀 achow101 merged a pull request: "refactor: share and use `GenerateRandomKey` helper"
(https://github.com/bitcoin/bitcoin/pull/28455)
(https://github.com/bitcoin/bitcoin/pull/28455)
💬 achow101 commented on pull request "bench: add readblock benchmark":
(https://github.com/bitcoin/bitcoin/pull/26684#issuecomment-1874221401)
ACK 1c4b9cbe906507295d8b7d52855de1441ad411dd
(https://github.com/bitcoin/bitcoin/pull/26684#issuecomment-1874221401)
ACK 1c4b9cbe906507295d8b7d52855de1441ad411dd
🚀 achow101 merged a pull request: "bench: add readblock benchmark"
(https://github.com/bitcoin/bitcoin/pull/26684)
(https://github.com/bitcoin/bitcoin/pull/26684)
💬 pinheadmz commented on pull request "Support JSON-RPC 2.0 when requested by client":
(https://github.com/bitcoin/bitcoin/pull/27101#issuecomment-1874230644)
Thanks @ryanofsky for the thorough review. I was misunderstanding the different roles of `httprpc` and `server` and I think tiptoeing around minimal commits when more changes were needed to really clean everything up.
`JSONRPCExec`: the only function now that executes calls, batches are just handled in `HTTPReq_JSONRPC` as you suggested.
Tests: additional coverage is now the first commit, and new tests are in the last commits. I replaced the helper functions in interface_rpc.py to provide
...
(https://github.com/bitcoin/bitcoin/pull/27101#issuecomment-1874230644)
Thanks @ryanofsky for the thorough review. I was misunderstanding the different roles of `httprpc` and `server` and I think tiptoeing around minimal commits when more changes were needed to really clean everything up.
`JSONRPCExec`: the only function now that executes calls, batches are just handled in `HTTPReq_JSONRPC` as you suggested.
Tests: additional coverage is now the first commit, and new tests are in the last commits. I replaced the helper functions in interface_rpc.py to provide
...
💬 hebasto commented on issue "./bitcoin.conf file should not cause confusion with ./datadir/bitcoin.conf":
(https://github.com/bitcoin/bitcoin/issues/29139#issuecomment-1874244991)
> Ok I don't agree that that would be an improvement, because:
>
> 2. Changing that file's name would also require an additional comment in the header instructing the user to rename the file as well as copy it to their data directory
I agree with this point. No need to force the average-skilled user to perform an extra action.
(https://github.com/bitcoin/bitcoin/issues/29139#issuecomment-1874244991)
> Ok I don't agree that that would be an improvement, because:
>
> 2. Changing that file's name would also require an additional comment in the header instructing the user to rename the file as well as copy it to their data directory
I agree with this point. No need to force the average-skilled user to perform an extra action.
📝 sipa opened a pull request: "[DONTMERGE] See if just constexpr->consteval for _mst works"
(https://github.com/bitcoin/bitcoin/pull/29167)
(https://github.com/bitcoin/bitcoin/pull/29167)