Bitcoin Core Github
44 subscribers
120K links
Download Telegram
📝 MarcoFalke opened a pull request: "ci: Use credits for ARM task"
(https://github.com/bitcoin/bitcoin/pull/27690)
After https://github.com/bitcoin/bitcoin/pull/27562 the task should finish in less than 10 minutes, so also using credits for it will be cheap and improve dev experience.
👋 hebasto's pull request is ready for review: "msvc: Provide `ObjectFileName` explicitly"
(https://github.com/bitcoin/bitcoin/pull/27687)
💬 hebasto commented on pull request "msvc: Provide `ObjectFileName` explicitly":
(https://github.com/bitcoin/bitcoin/pull/27687#issuecomment-1551496570)
MSVC build is [free](https://api.cirrus-ci.com/v1/task/5943704519704576/logs/build.log) from `MSB8027` now.
👍 fanquake approved a pull request: "ci: Use credits for ARM task"
(https://github.com/bitcoin/bitcoin/pull/27690#pullrequestreview-1430835654)
ACK aaaa07bc845f0b8741270552e77b9694802fb0e3 - if green
👍 fanquake approved a pull request: "doc: Rework build-unix.md"
(https://github.com/bitcoin/bitcoin/pull/27685#pullrequestreview-1430856715)
ACK fa29651c3ff3e13a42d6505b14971265562f088a
💬 hebasto commented on pull request "ci: A few fixes of `ccache` issues":
(https://github.com/bitcoin/bitcoin/pull/27084#issuecomment-1551531499)
> What is the status of this

I've analyzed the [recent](https://cirrus-ci.com/build/5086582458286080) `ccache` summaries at https://github.com/bitcoin/bitcoin/commit/a75c77ea903c100531e0fc5fde94bb9b52642145.

I don't think this PR can improve them.
hebasto closed a pull request: "ci: A few fixes of `ccache` issues"
(https://github.com/bitcoin/bitcoin/pull/27084)
💬 fjahr commented on pull request "assumeutxo: net_processing changes":
(https://github.com/bitcoin/bitcoin/pull/24008#issuecomment-1551540184)
> I guess a simpler way to phrase it is: what can go wrong if [c14ae13](https://github.com/bitcoin/bitcoin/commit/c14ae132c5a5204a9a755c84c6de05fb30459221) picks the incorrect chainstate for a block? What are all the different ways in which we can receive a block? What do we expect to happen in each of these cases? Tests would be a really nice way to illustrate that, but are apparently a pain to write for net_processing.

@Sjors I have tried to create an overview of the different cases and add
...
👍 ryanofsky approved a pull request: "kernel: Remove interface_ui, util/system from kernel library"
(https://github.com/bitcoin/bitcoin/pull/27636#pullrequestreview-1430775316)
Code review ACK 0b88c307a8ed81705cf8e6fb6332fdf969eb0e2e. These changes all seem very clean and straightforward now. I left some more suggestions, but you can feel free to ignore them them
💬 ryanofsky commented on pull request "kernel: Remove interface_ui, util/system from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27636#discussion_r1196631303)
In commit "kernel: Add fatalError method to notifications" (0b88c307a8ed81705cf8e6fb6332fdf969eb0e2e)

Would maybe make `notifications` the first parameter. It seems awkward for it to placed between the debug message and the user message.
💬 ryanofsky commented on pull request "kernel: Remove interface_ui, util/system from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27636#discussion_r1196623448)
In commit "refactor: Move system from util to common library" (0a7b3e5058b58c448bbc473ffb347910d781c3bd)

I think it makes sense to keep `util::insert` and `util::AnyPtr` functions in `util/`, not `common/` as these are header-only template functions with no dependencies, and there's no particular reason they shouldn't be used by kernel code even if they aren't used right now. In terms of functionality they are also more related to other helper functions like `util/overloaded.h` `util/strings.
...
💬 ryanofsky commented on pull request "kernel: Remove interface_ui, util/system from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27636#discussion_r1196568945)
In commit "kernel: Add notification interface" (c6f2fbef2c8ecc0ded3abb9ab2fdd5369e954d52)

It would be good to write this accessor as an inline method `kernel::Notifications& GetNotifications() const { return m_options.notifications; }` for discoverability and consistency with other accessors above. Could also drop "Get" prefix since the other accessors don't seem to have it, but whatever you prefer is fine.
💬 ryanofsky commented on pull request "kernel: Remove interface_ui, util/system from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27636#discussion_r1196627541)
In commit "kernel: Add fatalError method to notifications" (0b88c307a8ed81705cf8e6fb6332fdf969eb0e2e)

Would be good to change `bilingual_str` to `const bilingual_str&`. That should also avoid the need to remove the forward declaration and include `#include <util/translation.h>` above.
💬 MarcoFalke commented on pull request "ci: Use credits for ARM task":
(https://github.com/bitcoin/bitcoin/pull/27690#issuecomment-1551551092)
added a commit to improve tsan scheduling times as well
💬 fanquake commented on pull request "depends: Boost 1.82.0":
(https://github.com/bitcoin/bitcoin/pull/27671#issuecomment-1551562113)
I'll leave this as part of #24742 for now.
fanquake closed a pull request: "depends: Boost 1.82.0"
(https://github.com/bitcoin/bitcoin/pull/27671)
fanquake closed a pull request: "p2p: remove adjusted time"
(https://github.com/bitcoin/bitcoin/pull/25908)
💬 willcl-ark commented on pull request "Improve performance of p2p inv to send queues":
(https://github.com/bitcoin/bitcoin/pull/27610#issuecomment-1551582691)
@pinheadmz I am also still running this patch, but I still see pretty stable utilisation in the range of ~2-12%, currently with 88 inbound peers.

![image](https://github.com/bitcoin/bitcoin/assets/6606587/a7e61cef-4a31-4404-b9e2-80348d59bc0c)
💬 willcl-ark commented on issue "Mac osx 12.6.5 ":
(https://github.com/bitcoin/bitcoin/issues/27681#issuecomment-1551598180)

> Please check your debug.log for possible causes; Alternatively you can upload it here.
>
> You can find the debug.log in your [data dir](https://github.com/bitcoin/bitcoin/blob/master/doc/files.md?rgh-link-date=2023-05-17T06%3A16%3A25Z#data-directory-location).
>
> Please be aware that the debug log might contain personally identifying information.

Did you check for the log file in the datadir, as requested by this comment? There should be a file `debug.log` in directory `$HOME/Libr
...
💬 TheCharlatan commented on pull request "kernel: Remove interface_ui, util/system from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27636#discussion_r1196711775)
Mmh, I was originally thinking of moving `util::insert` to `wallet/walletutil.h` and `util::AnyPtr` to `rpc/util.h` in a follow up after this pull request. I think your suggestion is better though. Moving around these kind of headers depending on which part of the code needs them seems silly. I'll patch this then and will apply your other suggestions.