💬 stickies-v commented on pull request "MiniMiner changes for package linearization":
(https://github.com/bitcoin/bitcoin/pull/28762#discussion_r1387845814)
nit: I think this docstring should now be removed, I don't think the constructor requires a mempool.cs lock anymore?
(https://github.com/bitcoin/bitcoin/pull/28762#discussion_r1387845814)
nit: I think this docstring should now be removed, I don't think the constructor requires a mempool.cs lock anymore?
🚀 fanquake merged a pull request: "ci: win64 task does use boost:process"
(https://github.com/bitcoin/bitcoin/pull/28829)
(https://github.com/bitcoin/bitcoin/pull/28829)
📝 TheCharlatan opened a pull request: "[refactor] Check CTxMemPool options in ctor"
(https://github.com/bitcoin/bitcoin/pull/28830)
The tests should run the same checks on the mempool options that the init code also applies. The downside to this patch is that the log line may now be printed more than once in the for loop.
This was originally noticed here https://github.com/bitcoin/bitcoin/pull/25290#discussion_r900272797.
(https://github.com/bitcoin/bitcoin/pull/28830)
The tests should run the same checks on the mempool options that the init code also applies. The downside to this patch is that the log line may now be printed more than once in the for loop.
This was originally noticed here https://github.com/bitcoin/bitcoin/pull/25290#discussion_r900272797.
💬 TheCharlatan commented on pull request "build: Introduce internal kernel library":
(https://github.com/bitcoin/bitcoin/pull/28690#issuecomment-1803653460)
Rebased 1a589335617944b755235597aafc254e28e4acf9 -> 40e151697d3d1ed594364498d9e6220219d9b545 ([kernelInternalLib_4](https://github.com/TheCharlatan/bitcoin/tree/kernelInternalLib_4) -> [kernelInternalLib_5](https://github.com/TheCharlatan/bitcoin/tree/kernelInternalLib_5), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelInternalLib_4..kernelInternalLib_5))
* Fixed merge conflict.
(https://github.com/bitcoin/bitcoin/pull/28690#issuecomment-1803653460)
Rebased 1a589335617944b755235597aafc254e28e4acf9 -> 40e151697d3d1ed594364498d9e6220219d9b545 ([kernelInternalLib_4](https://github.com/TheCharlatan/bitcoin/tree/kernelInternalLib_4) -> [kernelInternalLib_5](https://github.com/TheCharlatan/bitcoin/tree/kernelInternalLib_5), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelInternalLib_4..kernelInternalLib_5))
* Fixed merge conflict.
👍 darosior approved a pull request: "mempool: Persist with XOR"
(https://github.com/bitcoin/bitcoin/pull/28207#pullrequestreview-1722332060)
utACK fa520848da6d718a07368b42b1a44bd2515e6e5a
(https://github.com/bitcoin/bitcoin/pull/28207#pullrequestreview-1722332060)
utACK fa520848da6d718a07368b42b1a44bd2515e6e5a
💬 0xB10C commented on pull request "doc: uppercase title for "Assumeutxo“":
(https://github.com/bitcoin/bitcoin/pull/28828#issuecomment-1803679113)
I don't think this adds any value here.
(https://github.com/bitcoin/bitcoin/pull/28828#issuecomment-1803679113)
I don't think this adds any value here.
💬 maflcko commented on pull request "doc: uppercase title for "Assumeutxo“":
(https://github.com/bitcoin/bitcoin/pull/28828#issuecomment-1803684814)
Closing for now due to controversy.
(https://github.com/bitcoin/bitcoin/pull/28828#issuecomment-1803684814)
Closing for now due to controversy.
✅ maflcko closed a pull request: "doc: uppercase title for "Assumeutxo“"
(https://github.com/bitcoin/bitcoin/pull/28828)
(https://github.com/bitcoin/bitcoin/pull/28828)
💬 maflcko commented on pull request "doc: uppercase title for "Assumeutxo“":
(https://github.com/bitcoin/bitcoin/pull/28828#issuecomment-1803686986)
If you are looking for other good first issues:
## Getting started to contribute to Bitcoin Core
### Setting up your development environment
New developers are very welcome and needed. There are a lot of open issues of any difficulty waiting to be fixed. However, before you start contributing, familiarize yourself with the Bitcoin Core build system and tests. Refer to the documentation in the repository on how to build Bitcoin Core and how to run the unit and functional tests. Once that
...
(https://github.com/bitcoin/bitcoin/pull/28828#issuecomment-1803686986)
If you are looking for other good first issues:
## Getting started to contribute to Bitcoin Core
### Setting up your development environment
New developers are very welcome and needed. There are a lot of open issues of any difficulty waiting to be fixed. However, before you start contributing, familiarize yourself with the Bitcoin Core build system and tests. Refer to the documentation in the repository on how to build Bitcoin Core and how to run the unit and functional tests. Once that
...
💬 maflcko commented on pull request "ci: Switch IWYU to `clang_17` branch":
(https://github.com/bitcoin/bitcoin/pull/28826#discussion_r1387897830)
```suggestion
${CI_RETRY_EXE} git clone --depth=1 https://github.com/include-what-you-use/include-what-you-use -b clang_TIDY_LLVM_V /include-what-you-use
```
(https://github.com/bitcoin/bitcoin/pull/28826#discussion_r1387897830)
```suggestion
${CI_RETRY_EXE} git clone --depth=1 https://github.com/include-what-you-use/include-what-you-use -b clang_TIDY_LLVM_V /include-what-you-use
```
💬 brunoerg commented on pull request "fuzz: call lookup functions before calling `Ban`":
(https://github.com/bitcoin/bitcoin/pull/27935#issuecomment-1803693573)
> Would be good to explain this a bit more, to make sure this is not a bug.
I think fuzz can generate subnets with a zone identifier, like: "676:c962:7962:b787:b392:fed8:7058:c500%2038004089/121", which is a valid one. However, the lookup call might change the zone identifier. In my machine (macOS), "676:c962:7962:b787:b392:fed8:7058:c500%2038004089/121" becomes "676:c962:7962:b787:b392:fed8:7058:c500%31097/121" after lookup and it makes the assertion fails.
(https://github.com/bitcoin/bitcoin/pull/27935#issuecomment-1803693573)
> Would be good to explain this a bit more, to make sure this is not a bug.
I think fuzz can generate subnets with a zone identifier, like: "676:c962:7962:b787:b392:fed8:7058:c500%2038004089/121", which is a valid one. However, the lookup call might change the zone identifier. In my machine (macOS), "676:c962:7962:b787:b392:fed8:7058:c500%2038004089/121" becomes "676:c962:7962:b787:b392:fed8:7058:c500%31097/121" after lookup and it makes the assertion fails.
💬 theStack commented on pull request "wallet: optimize migration process, batch db transactions":
(https://github.com/bitcoin/bitcoin/pull/28574#issuecomment-1803694990)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/28574#issuecomment-1803694990)
Concept ACK
💬 hebasto commented on pull request "ci: Switch IWYU to `clang_17` branch":
(https://github.com/bitcoin/bitcoin/pull/28826#discussion_r1387909280)
Thanks! Done.
(https://github.com/bitcoin/bitcoin/pull/28826#discussion_r1387909280)
Thanks! Done.
💬 maflcko commented on pull request "ci: Switch IWYU to `clang_17` branch":
(https://github.com/bitcoin/bitcoin/pull/28826#issuecomment-1803710156)
lgtm ACK 9f208c017174132dbefbc917aa9824c279382597
(https://github.com/bitcoin/bitcoin/pull/28826#issuecomment-1803710156)
lgtm ACK 9f208c017174132dbefbc917aa9824c279382597
💬 maflcko commented on pull request "fuzz: call lookup functions before calling `Ban`":
(https://github.com/bitcoin/bitcoin/pull/27935#issuecomment-1803717471)
Ah ok. And about the efficiency, is it possible to keep the fuzz input format unchanged instead of using a string representation (Maybe via a string-lookup roundtrip)? I wonder if that'd be more efficient.
(https://github.com/bitcoin/bitcoin/pull/27935#issuecomment-1803717471)
Ah ok. And about the efficiency, is it possible to keep the fuzz input format unchanged instead of using a string representation (Maybe via a string-lookup roundtrip)? I wonder if that'd be more efficient.
🚀 fanquake merged a pull request: "ci: Switch IWYU to `clang_17` branch"
(https://github.com/bitcoin/bitcoin/pull/28826)
(https://github.com/bitcoin/bitcoin/pull/28826)
👍 ismaelsadeeq approved a pull request: "refactor: Simplify CTxMempool/BlockAssembler fields, remove some external mapTx access"
(https://github.com/bitcoin/bitcoin/pull/28391#pullrequestreview-1722502299)
ACK 2be5e799ba623f969f5ffc59667a5bc6799df290 🥘
(https://github.com/bitcoin/bitcoin/pull/28391#pullrequestreview-1722502299)
ACK 2be5e799ba623f969f5ffc59667a5bc6799df290 🥘
📝 maflcko opened a pull request: "test: Avoid intermittent failures in feature_init"
(https://github.com/bitcoin/bitcoin/pull/28831)
The code not only modifies block dat files, but also leveldb files, which may be of smaller size. Such corruption may not force leveldb to abort, according to the intermittent test failures.
Fix the intermittent test failures by reverting https://github.com/bitcoin/bitcoin/commit/5ab6419f380cc0a8cde78b125f3eeee5fcba43ae .
(https://github.com/bitcoin/bitcoin/pull/28831)
The code not only modifies block dat files, but also leveldb files, which may be of smaller size. Such corruption may not force leveldb to abort, according to the intermittent test failures.
Fix the intermittent test failures by reverting https://github.com/bitcoin/bitcoin/commit/5ab6419f380cc0a8cde78b125f3eeee5fcba43ae .
🤔 ryanofsky reviewed a pull request: "Get rid of shutdown.cpp/shutdown.h, use SignalInterrupt directly"
(https://github.com/bitcoin/bitcoin/pull/28051#pullrequestreview-1722504219)
Thanks for the reviews.
Updated d7477b1c9cde1e795e006d1788c79b8985970eee -> 52ecb6547dd2b59643483e43531d1201941c0df0 ([`pr/noshut.16`](https://github.com/ryanofsky/bitcoin/commits/pr/noshut.16) -> [`pr/noshut.17`](https://github.com/ryanofsky/bitcoin/commits/pr/noshut.17), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/noshut.16..pr/noshut.17)) with suggested changes
(https://github.com/bitcoin/bitcoin/pull/28051#pullrequestreview-1722504219)
Thanks for the reviews.
Updated d7477b1c9cde1e795e006d1788c79b8985970eee -> 52ecb6547dd2b59643483e43531d1201941c0df0 ([`pr/noshut.16`](https://github.com/ryanofsky/bitcoin/commits/pr/noshut.16) -> [`pr/noshut.17`](https://github.com/ryanofsky/bitcoin/commits/pr/noshut.17), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/noshut.16..pr/noshut.17)) with suggested changes
💬 ryanofsky commented on pull request "Get rid of shutdown.cpp/shutdown.h, use SignalInterrupt directly":
(https://github.com/bitcoin/bitcoin/pull/28051#discussion_r1387982956)
re: https://github.com/bitcoin/bitcoin/pull/28051#discussion_r1377678893
> note for other reviewers (and future self): `NodeImpl::startShutDown()` does have [this additional code block](https://github.com/bitcoin/bitcoin/blob/d51fb9caa622add96c6b594e162da5584eb927fc/src/node/interfaces.cpp#L124-L128)
There is a change in behavior here but the new behavior should be more correct. RPC calls should be interrupted whenever the GUI is shut down, regardless of how it is shut down. Calls to `Star
...
(https://github.com/bitcoin/bitcoin/pull/28051#discussion_r1387982956)
re: https://github.com/bitcoin/bitcoin/pull/28051#discussion_r1377678893
> note for other reviewers (and future self): `NodeImpl::startShutDown()` does have [this additional code block](https://github.com/bitcoin/bitcoin/blob/d51fb9caa622add96c6b594e162da5584eb927fc/src/node/interfaces.cpp#L124-L128)
There is a change in behavior here but the new behavior should be more correct. RPC calls should be interrupted whenever the GUI is shut down, regardless of how it is shut down. Calls to `Star
...