Bitcoin Core Github
43 subscribers
122K links
Download Telegram
💬 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.
maflcko closed a pull request: "doc: uppercase title for "Assumeutxo“"
(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
...
💬 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
```
💬 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.
💬 theStack commented on pull request "wallet: optimize migration process, batch db transactions":
(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.
💬 maflcko commented on pull request "ci: Switch IWYU to `clang_17` branch":
(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.
🚀 fanquake merged a pull request: "ci: Switch IWYU to `clang_17` branch"
(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 🥘
📝 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 .
🤔 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
💬 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
...
💬 ryanofsky commented on pull request "Get rid of shutdown.cpp/shutdown.h, use SignalInterrupt directly":
(https://github.com/bitcoin/bitcoin/pull/28051#discussion_r1387983151)
re: https://github.com/bitcoin/bitcoin/pull/28051#discussion_r1364373784

> NIt: s/namespaces/namespace/

Thanks, fixed
💬 ryanofsky commented on pull request "Get rid of shutdown.cpp/shutdown.h, use SignalInterrupt directly":
(https://github.com/bitcoin/bitcoin/pull/28051#discussion_r1387983370)
re: https://github.com/bitcoin/bitcoin/pull/28051#discussion_r1379378726

> I don't think we need to expose those internal error details. Also, I know we exclude `server.cpp` from the CHECK_NONFATAL linter, but given that this is an RPC method I think it would be preferable here?

Thanks, replaced JSONRPCError exception with nonfatal exception
💬 ryanofsky commented on pull request "Get rid of shutdown.cpp/shutdown.h, use SignalInterrupt directly":
(https://github.com/bitcoin/bitcoin/pull/28051#discussion_r1387983586)
re: https://github.com/bitcoin/bitcoin/pull/28051#discussion_r1377746899

> nit: missing `#include <util/check.h>`

Thanks, fixed
💬 ryanofsky commented on pull request "Get rid of shutdown.cpp/shutdown.h, use SignalInterrupt directly":
(https://github.com/bitcoin/bitcoin/pull/28051#discussion_r1387982522)
re: https://github.com/bitcoin/bitcoin/pull/28051#discussion_r1377777492

> nit: long line

Thanks, added line break
fanquake closed an issue: "TSAN: lock-order-inversion (potential deadlock) in ZapSelectTx test"
(https://github.com/bitcoin/bitcoin/issues/27582)
💬 fanquake commented on issue "TSAN: lock-order-inversion (potential deadlock) in ZapSelectTx test":
(https://github.com/bitcoin/bitcoin/issues/27582#issuecomment-1803871925)
Closing for now. Don't seem to be able to reproduce.