💬 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)
💬 sipa commented on pull request "miniscript: make operator""_mst consteval":
(https://github.com/bitcoin/bitcoin/pull/28657#issuecomment-1874247244)
@fanquake See #29167
(https://github.com/bitcoin/bitcoin/pull/28657#issuecomment-1874247244)
@fanquake See #29167
💬 achow101 commented on pull request "fuzz: set `m_fallback_fee` and `m_fee_mode` in `wallet_fees` target":
(https://github.com/bitcoin/bitcoin/pull/29076#issuecomment-1874248910)
ACK e03d6f7ed534f423f58236866f8e83beee1871e1
(https://github.com/bitcoin/bitcoin/pull/29076#issuecomment-1874248910)
ACK e03d6f7ed534f423f58236866f8e83beee1871e1
💬 dergoegge commented on pull request "Nuke adjusted time (attempt 2)":
(https://github.com/bitcoin/bitcoin/pull/28956#issuecomment-1874258028)
Only rebased for now
(https://github.com/bitcoin/bitcoin/pull/28956#issuecomment-1874258028)
Only rebased for now
🚀 achow101 merged a pull request: "fuzz: set `m_fallback_fee` and `m_fee_mode` in `wallet_fees` target"
(https://github.com/bitcoin/bitcoin/pull/29076)
(https://github.com/bitcoin/bitcoin/pull/29076)
✅ sipa closed 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)
💬 LarryRuane commented on pull request "bitcoin-cli help detail to show full help for all RPCs":
(https://github.com/bitcoin/bitcoin/pull/29163#issuecomment-1874278059)
Force pushed to add release note (not sure if this is important enough to have a release note for, but just in case), and also updated the `help help` text to document the `detail` argument:
```
$ bitcoin-cli help help
help ( "command" )
List all commands, or get help for a specified command.
Arguments:
1. command (string, optional, default=all commands) The command to get help on, or "detail" for full help on all commands
Result:
"str" (string) The help text
```
(https://github.com/bitcoin/bitcoin/pull/29163#issuecomment-1874278059)
Force pushed to add release note (not sure if this is important enough to have a release note for, but just in case), and also updated the `help help` text to document the `detail` argument:
```
$ bitcoin-cli help help
help ( "command" )
List all commands, or get help for a specified command.
Arguments:
1. command (string, optional, default=all commands) The command to get help on, or "detail" for full help on all commands
Result:
"str" (string) The help text
```
💬 glozow commented on pull request "Change Luke Dashjr seed to dashjr-list-of-p2p-nodes-maybe-malware.us":
(https://github.com/bitcoin/bitcoin/pull/29145#issuecomment-1874280212)
> I've chosen a domain name that is explicitly verbose about its purpose
Er, how is "maybe malware" the purpose of the seeder? It seems like this would just confuse/alarm users, maybe choose something else instead. I don't think adding a log filter makes sense either.
(https://github.com/bitcoin/bitcoin/pull/29145#issuecomment-1874280212)
> I've chosen a domain name that is explicitly verbose about its purpose
Er, how is "maybe malware" the purpose of the seeder? It seems like this would just confuse/alarm users, maybe choose something else instead. I don't think adding a log filter makes sense either.
💬 stratospher commented on pull request "test/BIP324: functional tests for v2 P2P encryption":
(https://github.com/bitcoin/bitcoin/pull/24748#discussion_r1439634994)
nice! done.
(https://github.com/bitcoin/bitcoin/pull/24748#discussion_r1439634994)
nice! done.
💬 stratospher commented on pull request "test/BIP324: functional tests for v2 P2P encryption":
(https://github.com/bitcoin/bitcoin/pull/24748#discussion_r1439635162)
added a [description](https://github.com/bitcoin/bitcoin/blob/42203f84bba7e4ba206c9f951c3f4b6922a44f81/test/functional/test_framework/v2_p2p.py#L260-L265) and also an [extra assert](https://github.com/bitcoin/bitcoin/blob/42203f84bba7e4ba206c9f951c3f4b6922a44f81/test/functional/test_framework/p2p.py#L326) to make sure we don't receive b"" as an application layer message.
(https://github.com/bitcoin/bitcoin/pull/24748#discussion_r1439635162)
added a [description](https://github.com/bitcoin/bitcoin/blob/42203f84bba7e4ba206c9f951c3f4b6922a44f81/test/functional/test_framework/v2_p2p.py#L260-L265) and also an [extra assert](https://github.com/bitcoin/bitcoin/blob/42203f84bba7e4ba206c9f951c3f4b6922a44f81/test/functional/test_framework/p2p.py#L326) to make sure we don't receive b"" as an application layer message.
💬 dergoegge commented on pull request "PoC: fuzz chainstate and block managers":
(https://github.com/bitcoin/bitcoin/pull/29158#issuecomment-1874318288)
Thanks for working on this!
One alternative that I have considered before (for chainstate fuzzing) is to abstract and further modularize `BlockManager`, which would allow us to have an `InMemoryBlockManager` for tests (especially useful for fuzzing but also nice in unit tests).
This would require a bunch of work:
* Breaking up the friendship between `BlockManager`, `Chainstate` & `ChainstateManager`
* Abstracting `BlockManager`'s interface away from being file based
* Hiding access to
...
(https://github.com/bitcoin/bitcoin/pull/29158#issuecomment-1874318288)
Thanks for working on this!
One alternative that I have considered before (for chainstate fuzzing) is to abstract and further modularize `BlockManager`, which would allow us to have an `InMemoryBlockManager` for tests (especially useful for fuzzing but also nice in unit tests).
This would require a bunch of work:
* Breaking up the friendship between `BlockManager`, `Chainstate` & `ChainstateManager`
* Abstracting `BlockManager`'s interface away from being file based
* Hiding access to
...