Get rid of shutdown.cpp/shutdown.h, use SignalInterrupt directly #28051

ryanofsky · 2023-07-07T22:26:18Z

This change drops shutdown.h and shutdown.cpp files, replacing them with a NodeContext::shutdown member which is used to trigger shutdowns directly. This gets rid of an unnecessary layer of indirection, and allows getting rid of the kernel::g_context global.

Additionally, this PR tries to improve error handling of SignalInterrupt code by marking relevant methods [[nodiscard]] to avoid the possibility of uncaught exceptions mentioned #27861 (comment).

Behavior is changing In a few cases which are noted in individual commit messages. Particularly: GUI code more consistently interrupts RPCs when it is shutting down, shutdown state no longer persists between unit tests, the stop RPC now returns an RPC error if requesting shutdown fails instead of aborting, and other failed shutdown calls now log errors instead of aborting.

This PR is a net reduction in lines of code, but in some cases the explicit error handling and lack of global shutdown functions do make it more verbose. The verbosity can be seen as good thing if it discourages more code from directly triggering shutdowns, and instead encourages code to return errors or send notifications that could be translated into shutdowns. Probably a number of existing shutdown calls could just be replaced by better error handling.

DrahtBot · 2023-07-07T22:26:21Z

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage

For detailed information about the code coverage, see the test coverage report.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	TheCharlatan, maflcko, stickies-v, achow101
Concept ACK	fanquake

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#28960 (kernel: Remove dependency on CScheduler by TheCharlatan)
#28956 (Nuke adjusted time (attempt 2) by dergoegge)
#28863 (wallet, mempool: propagete checkChainLimits error message to wallet by ismaelsadeeq)
#28843 ([refactor] Remove BlockAssembler m_mempool member by TheCharlatan)
#28830 ([refactor] Check CTxMemPool options in ctor by TheCharlatan)
#28792 (Embedding ASMap files as binary dump header file by fjahr)
#28228 (kernel: Run sanity checks on context construction by TheCharlatan)
#28052 (blockstorage: XOR blocksdir *.dat files by maflcko)
#27253 (httpserver, rest: improving URI validation by pablomartin4btc)
#26966 (index: blockfilter initial sync speedup, parallelize process by furszy)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

maflcko · 2023-07-08T10:44:08Z

I have the same question here: #27861 (comment)

TheCharlatan

Concept ACK

src/rpc/server.cpp

src/qt/winshutdownmonitor.cpp

src/init.cpp

ryanofsky

Making this into a draft because I want to do more work to avoid the uncaught exceptions that Marco mentioned #27861 (comment), and address initialization order issues TheCharlatan mentioned #28051 (comment) and #28051 (comment)

Updated d9453bd -> 24169b1 (pr/noshut.2 -> pr/noshut.3, compare) with EnsureAnyNodeContext suggestion

src/rpc/server.cpp

fanquake · 2023-08-07T09:41:40Z

Concept ACK

ryanofsky · 2023-08-21T20:29:08Z

Making this into a draft because I want to do more work to avoid the uncaught exceptions that Marco mentioned #27861 (comment), and address initialization order issues TheCharlatan mentioned #28051 (comment) and #28051 (comment)

Removed draft state and rebased 0dce004 -> eccea15 (pr/noshut.4 -> pr/noshut.5, compare) removing all the SignalInterrupt exceptions, fixing initialization order issues that were mentioned, getting rid of the kernel::g_context variable, and rebasing to fix conflicts with #28048 and #28244

fanquake · 2023-08-22T09:17:08Z

Windows builds are currently unhappy:

  CXX      kernel/libbitcoin_node_a-mempool_persist.o
init.cpp: In function ‘BOOL consoleCtrlHandler(DWORD)’:
init.cpp:386:15: error: ignoring return value of ‘bool util::SignalInterrupt::operator()()’, declared with attribute ‘nodiscard’ [-Werror=unused-result]
  386 |     g_shutdown();
      |     ~~~~~~~~~~^~
In file included from ./kernel/context.h:8,
                 from ./node/context.h:8,
                 from init.cpp:48:
./util/signalinterrupt.h:32:24: note: declared here
   32 |     [[nodiscard]] bool operator()();
      |                        ^~~~~~~~
cc1plus: all warnings being treated as errors
make[2]: *** [Makefile:10167: libbitcoin_node_a-init.o] Error 1

…bal pointers Having InitContext() avoids the need to add duplicate code to src/init/*.cpp files in the next commit. It also lets these files avoid referencing global variables like gArgs. There is no change in behavior in this commit.

This change is mostly a refectoring that removes some code and gets rid of an unnecessary layer of indirection after bitcoin#27861 But it is not a pure refactoring since StartShutdown, AbortShutdown, and WaitForShutdown functions used to abort on failure, and the replacement code logs or returns errors instead.

ryanofsky

Rebased 437a0c2 -> 8a02ce5 (pr/noshut.18 -> pr/noshut.19, compare) due to conflict with #28946

stickies-v

ACK 8a02ce5

sorry my answers to your questions are so long

I really appreciate the time you've taken to thoroughly address my comments. Thank you, it's extremely helpful.

stickies-v · 2023-12-05T08:46:17Z

src/test/util/index.cpp

 #include <util/time.h>

-void IndexWaitSynced(const BaseIndex& index)
+void IndexWaitSynced(const BaseIndex& index, const util::SignalInterrupt& interrupt)


That makes sense, and I agree with your rationale, so I think my suggestion here in test/util/index.cpp indeed should be disregarded. In node::AbortNode() though, we are sending an interrupt signal to shutdown the node, so I still think it would make sense there? (non-blocking nit, ofc)

src/node/chainstate.cpp

src/init.cpp

stickies-v · 2023-12-05T09:09:19Z

src/init.cpp

+            if (!(*Assert(node.shutdown))()) {
+                LogPrintf("Error: failed to send shutdown signal after disk space check\n");
+            }


Lower level code should just be returning errors and notifications, and high level code should be using that information to decide whether to shut down

That does seem like a better design, indeed. I agree then that having the bad approach be more verbose is not something to worry about, and my suggestion should be discarded. Thank you for providing the rationale, I think this might be useful to add to the PR description?

TheCharlatan · 2023-12-06T11:49:30Z

src/qt/bitcoin.cpp

@@ -661,7 +661,7 @@ int GuiMain(int argc, char* argv[])
    app.installEventFilter(new GUIUtil::LabelOutOfFocusEventFilter(&app));
 #if defined(Q_OS_WIN)
    // Install global event filter for processing Windows session related Windows messages (WM_QUERYENDSESSION and WM_ENDSESSION)
-    qApp->installNativeEventFilter(new WinShutdownMonitor());
+    qApp->installNativeEventFilter(new WinShutdownMonitor(app.node()));


Calling app.node() here before app.createNode results in a crash on windows when running bitcoin-qt.exe. I think the most straight forward way to resolve this would be to move this block of code to after createNode is called.

Alternatively, to keep the current control flow, the way I read it, make_node does not actually manipulate the node context, besides eventually setting it in the NodeImpl further down the call stack. So I think the call to it could be moved further up:

diff --git a/src/qt/bitcoin.cpp b/src/qt/bitcoin.cpp index 8d4ca4ea92..a195db550f 100644 --- a/src/qt/bitcoin.cpp +++ b/src/qt/bitcoin.cpp @@ -301,0 +302,2 @@ void BitcoinApplication::createSplashScreen(const NetworkStyle *networkStyle) + assert(m_node); + m_splash->setNode(*m_node); @@ -658,0 +661 @@ int GuiMain(int argc, char* argv[]) + app.createNode(*init); @@ -675,2 +677,0 @@ int GuiMain(int argc, char* argv[]) - app.createNode(*init);

Would be nice if we had tests that would exercise this code. Maybe a quick start and kill routine can be added to the windows native CI job?

re: #28051 (comment)

Calling app.node() here before app.createNode results in a crash on windows when running bitcoin-qt.exe.

Great catch! This should be fixed now by passing a shutdown callback and delaying the call to app.node(). I didn't want to move the createNode call, because the interfaces::Node object is intentionally created after the splash screen, so the splash screen can show up without a delay. The delay wouldn't be significant now, but with #10102 creating the node requires an IPC round trip, so might be noticeable.

Also agree with you it's a shame this part of the Qt codebase doesn't have CI test coverage. Your suggestion to add a CI step starting and stopping bitcoin-qt on windows (and probably all platforms bitcoin-qt is built for) would be a nice thing to implement. I think it would be most easily implemented as a python test.

Other ways to get more test coverage for this code might be to change one of the existing CI jobs to run existing python tests with bitcoin-qt instead of bitcoind. Or to refactor the GuiMain function and write unit tests for it. But unlike your suggestion, these approaches would be unlikely to trigger this particular bug since it is windows-specific.

Thanks for the rationale, I indeed did not think of the future IPC delay. If we only start processing events after app.exec(), why not move this to just before registerShutdownBlockReason? Looking at the history it looks to me like this was put where it is to be in the general vicinity of the installEvent* calls.

Thanks for the rationale, I indeed did not think of the future IPC delay. If we only start processing events after app.exec(), why not move this to just before registerShutdownBlockReason? Looking at the history it looks to me like this was put where it is to be in the general vicinity of the installEvent* calls.

I can make this change if you still think it's a good idea after this, but I think it's reasonable to want to keep the installEvent calls together. I also didn't want to just move the install event call when it's not necessary.

I think the current approach of passing a lambda argument to shutdownmonitor instead of passing a interfaces::Node pointer like the previous approach is better in any case, because it makes it obvious that the only thing shutdownmonitor can do is call the startShutdown function, and that it is not going to interact with the node in some other way. It also makes the shutdown monitor class more future-proof so it the WM_QUERYENDSESSION / WM_ENDSESSION code shouldn't need to change when there are changes to the node interface.

src/qt/winshutdownmonitor.h

ryanofsky

Updated 8a02ce5 -> 4a3a265 (pr/noshut.19 -> pr/noshut.20, compare) with Qt windows bugfix

ryanofsky · 2023-12-06T16:00:48Z

src/qt/bitcoin.cpp

@@ -661,7 +661,7 @@ int GuiMain(int argc, char* argv[])
    app.installEventFilter(new GUIUtil::LabelOutOfFocusEventFilter(&app));
 #if defined(Q_OS_WIN)
    // Install global event filter for processing Windows session related Windows messages (WM_QUERYENDSESSION and WM_ENDSESSION)
-    qApp->installNativeEventFilter(new WinShutdownMonitor());
+    qApp->installNativeEventFilter(new WinShutdownMonitor(app.node()));


re: #28051 (comment)

Calling app.node() here before app.createNode results in a crash on windows when running bitcoin-qt.exe.

Great catch! This should be fixed now by passing a shutdown callback and delaying the call to app.node(). I didn't want to move the createNode call, because the interfaces::Node object is intentionally created after the splash screen, so the splash screen can show up without a delay. The delay wouldn't be significant now, but with #10102 creating the node requires an IPC round trip, so might be noticeable.

Also agree with you it's a shame this part of the Qt codebase doesn't have CI test coverage. Your suggestion to add a CI step starting and stopping bitcoin-qt on windows (and probably all platforms bitcoin-qt is built for) would be a nice thing to implement. I think it would be most easily implemented as a python test.

Other ways to get more test coverage for this code might be to change one of the existing CI jobs to run existing python tests with bitcoin-qt instead of bitcoind. Or to refactor the GuiMain function and write unit tests for it. But unlike your suggestion, these approaches would be unlikely to trigger this particular bug since it is windows-specific.

src/qt/winshutdownmonitor.h

TheCharlatan

Re-ACK 4a3a265

ryanofsky

Updated 4a3a265 -> eaf915d (pr/noshut.20 -> pr/noshut.22, compare) just implementing a suggested change to rename

ryanofsky · 2023-12-11T20:17:31Z

src/init.cpp

+            if (!(*Assert(node.shutdown))()) {
+                LogPrintf("Error: failed to send shutdown signal after disk space check\n");
+            }


re: #28051 (comment)

Thank you for providing the rationale, I think this might be useful to add to the PR description?

Definitely, and the PR description was pretty bad so I rewrote it. This is mentioned now at the end.

ryanofsky · 2023-12-11T20:18:09Z

src/test/util/index.cpp

 #include <util/time.h>

-void IndexWaitSynced(const BaseIndex& index)
+void IndexWaitSynced(const BaseIndex& index, const util::SignalInterrupt& interrupt)


re: #28051 (comment)

In node::AbortNode() though, we are sending an interrupt signal to shutdown the node, so I still think it would make sense there? (non-blocking nit, ofc)

Sorry I missed this comment before, and you are right about AbortNode. I renamed interrupt to shutdown there now and in the KernelNotifications class as well.

TheCharlatan

Re-ACK eaf915d

maflcko

It would be good to explain why the behavior changes are needed. Otherwise,

concept ACK eaf915d 👇

Show signature

Signature:

untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: concept ACK eaf915d61d470372e63f41f11d6a873c1936f73f 👇
Zj5llDu5414hpNoftK5OmsCqlMoeZXi2akcICzuiYesSYUrLhMj2dGrGP1Ibp7xziOqBbwqKkTbUNzoKgL5eBQ==

maflcko · 2023-12-12T09:48:41Z

src/node/abort.cpp

 {
    SetMiscWarning(Untranslated(debug_message));
    LogPrintf("*** %s\n", debug_message);
    InitError(user_message.empty() ? _("A fatal internal error occurred, see debug.log for details") : user_message);
    exit_status.store(EXIT_FAILURE);
-    if (shutdown) StartShutdown();
+    if (shutdown && !(*shutdown)()) {
+        LogPrintf("Error: failed to send shutdown signal\n");


ae66429: Is it required to change the behavior here? Why not keep the abort?

re: #28051 (comment)

ae66429: Is it required to change the behavior here? Why not keep the abort?

In general, I don't think dumping core is a good way to handle errors. I think it can be a reasonable thing to do if inconsistent internal state is detected or if a precondition is violated, and halting right away will provide better debug information. Or it could make sense if it's likely that data corruption or other bad outcomes will happen if execution doesn't stop as quickly as possible.

But in this case when an ordinary system call that is not saving data returns an error code, I think it it is better to log the failure and move on than to crash the entire process abruptly when we don't know what other threads are running, what wallets might be loaded, what data might be lost, etc, as a result of an abort().

I guess this code will never be hit, so it doesn't matter much. If it was hit, I don't see how the user would shutdown the process without an unclean termination, so might as well do an abort. Generally, it should be fine to abort Bitcoin Core at any time, because any critical data should always be flushed atomically to storage.

But as this code should never be hit, it also seems fine to give the user the option to call RPCs to unload wallets, flush the chainstate, and disconnect peers, etc, before doing the unclean termination.

maflcko · 2023-12-12T09:49:17Z

src/node/kernel_notifications.cpp

@@ -62,7 +61,9 @@ kernel::InterruptResult KernelNotifications::blockTip(SynchronizationState state
 {
    uiInterface.NotifyBlockTip(state, &index);
    if (m_stop_at_height && index.nHeight >= m_stop_at_height) {
-        StartShutdown();
+        if (!m_shutdown()) {
+            LogPrintf("Error: failed to send shutdown signal after reaching stop height\n");


re: #28051 (comment)

same

I'd give the same answer (#28051 (comment)) here too

maflcko · 2023-12-12T09:59:25Z

src/init.cpp

-// And then, WaitForShutdown() makes all other on-going threads
-// in the thread group join the main thread.
+// A clean exit happens when the SignalInterrupt object is triggered, which
+// makes main thread's SignalInterrupt::wait() call return, and join all other


nit in eaf915d: "the main thread"

re: #28051 (comment)

nit in eaf915d: "the main thread"

Fixed, thank you!

maflcko · 2023-12-12T10:00:15Z

src/init.cpp

@@ -222,6 +225,11 @@ void InitContext(NodeContext& node)
 // shutdown thing.
 //

+bool ShutdownRequested(node::NodeContext& node)
+{
+    return bool{*(Assert(node.shutdown))};


nit in eaf915d: Can drop the () around Assert()?

re: #28051 (comment)

nit in eaf915d: Can drop the () around Assert()?

Good catch, fixed

ryanofsky

Updated eaf915d -> 6db04be (pr/noshut.22 -> pr/noshut.23, compare) just making suggested code tweaks and updating all commit messages to say what behavior is changing.

re: #28051 (review)

It would be good to explain why the behavior changes are needed.

Thank you, good catch. Some of the commits were explaining behavior changes, but not not all, and the PR description did not mention them. This should all be fixed now.

concept ACK eaf915d 👇

FYI, I noticed DrahtBot is interpreting this concept ack as a normal ACK in #28051 (comment), probably because "concept" is not capitalized

ryanofsky · 2023-12-12T13:39:42Z

src/init.cpp

-// And then, WaitForShutdown() makes all other on-going threads
-// in the thread group join the main thread.
+// A clean exit happens when the SignalInterrupt object is triggered, which
+// makes main thread's SignalInterrupt::wait() call return, and join all other


re: #28051 (comment)

nit in eaf915d: "the main thread"

Fixed, thank you!

ryanofsky · 2023-12-12T13:40:01Z

src/init.cpp

@@ -222,6 +225,11 @@ void InitContext(NodeContext& node)
 // shutdown thing.
 //

+bool ShutdownRequested(node::NodeContext& node)
+{
+    return bool{*(Assert(node.shutdown))};


re: #28051 (comment)

nit in eaf915d: Can drop the () around Assert()?

Good catch, fixed

ryanofsky · 2023-12-12T13:40:11Z

src/node/abort.cpp

 {
    SetMiscWarning(Untranslated(debug_message));
    LogPrintf("*** %s\n", debug_message);
    InitError(user_message.empty() ? _("A fatal internal error occurred, see debug.log for details") : user_message);
    exit_status.store(EXIT_FAILURE);
-    if (shutdown) StartShutdown();
+    if (shutdown && !(*shutdown)()) {
+        LogPrintf("Error: failed to send shutdown signal\n");


re: #28051 (comment)

ae66429: Is it required to change the behavior here? Why not keep the abort?

In general, I don't think dumping core is a good way to handle errors. I think it can be a reasonable thing to do if inconsistent internal state is detected or if a precondition is violated, and halting right away will provide better debug information. Or it could make sense if it's likely that data corruption or other bad outcomes will happen if execution doesn't stop as quickly as possible.

But in this case when an ordinary system call that is not saving data returns an error code, I think it it is better to log the failure and move on than to crash the entire process abruptly when we don't know what other threads are running, what wallets might be loaded, what data might be lost, etc, as a result of an abort().

ryanofsky · 2023-12-12T13:40:19Z

src/node/kernel_notifications.cpp

@@ -62,7 +61,9 @@ kernel::InterruptResult KernelNotifications::blockTip(SynchronizationState state
 {
    uiInterface.NotifyBlockTip(state, &index);
    if (m_stop_at_height && index.nHeight >= m_stop_at_height) {
-        StartShutdown();
+        if (!m_shutdown()) {
+            LogPrintf("Error: failed to send shutdown signal after reaching stop height\n");


re: #28051 (comment)

same

I'd give the same answer (#28051 (comment)) here too

TheCharlatan

Re-ACK 6db04be

maflcko

ACK 6db04be 👗

Show signature

Signature:

untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: ACK 6db04be102807ee0120981a9b8de62a55439dabb 👗
svhRtNkGG1s2bMv7s6ElM2CG7nlbJ6ersNj58rdTM6YsNu2LTGwUMhVYMVduUxOzUnE++tiWYBOB91ptoUk5Ag==

maflcko · 2023-12-13T13:15:53Z

src/init.h

@@ -26,6 +26,11 @@ namespace node {
 struct NodeContext;
 } // namespace node

+/** Initialize node context shutdown and args variables. */
+void InitContext(node::NodeContext& node);


Nit in the last commit: Shouldn't new node code be in the node namespace? Otherwise it will have to be touched again when it is moved into the node namespace later?

re: #28051 (comment)

Nit in the last commit: Shouldn't new node code be in the node namespace? Otherwise it will have to be touched again when it is moved into the node namespace later?

This is true, but it is not different from other init.h methods. Logically init.h and init.cpp belong in node subdirectory and should use the node namespace, so I didn't want to make this one function stand out. The function also does need to be located in this file because it is accesses a static variable. So just keeping the Init functions in the same namespace seemed like the most straightforward approach for now.

stickies-v

re-ACK 6db04be

achow101 · 2023-12-14T20:03:51Z

ACK 6db04be

Code seems fine, but I'm not familiar enough with this area to say whether this is the right approach or if there are any side effects that haven't been thought of.

theuni · 2024-05-06T21:25:06Z

src/kernel/context.h

@@ -18,24 +18,9 @@ namespace kernel {
 //! State stored directly in this struct should be simple. More complex state
 //! should be stored to std::unique_ptr members pointing to opaque types.
 struct Context {
-    //! Interrupt object that can be used to stop long-running kernel operations.
-    util::SignalInterrupt interrupt;


Just noticed while reviewing #29252...

This was removed, but the #include <util/signalinterrupt.h> include wasn't. I can open a quick cleanup PR, or would you like to @ryanofsky ?

(Looks like <memory> can go too.)

(Looks like <memory> can go too.)

The iwyu output from https://cirrus-ci.com/task/5795750523699200 / https://api.cirrus-ci.com/v1/task/5795750523699200/logs/ci.log :

kernel/context.h should add these lines: kernel/context.h should remove these lines: - #include <util/signalinterrupt.h> // lines 8-8 - #include <memory> // lines 10-10 The full include-list for kernel/context.h: --- kernel/context.cpp should add these lines: kernel/context.cpp should remove these lines: - #include <pubkey.h> // lines 10-10 The full include-list for kernel/context.cpp: #include <kernel/context.h> #include <crypto/sha256.h> // for SHA256AutoDetect #include <key.h> // for ECC_Start, ECC_Stop #include <logging.h> // for LogPrintf_, LogPrintf #include <random.h> // for RandomInit #include <string> // for string ---

Good catch. I don't think I would open a new PR that just removing an unnecessary #include without having a larger goal, but I wouldn't object to one and would happily review. Maybe the include fixes could be part of #29252, too.

ryanofsky mentioned this pull request Jul 7, 2023

kernel: Remove StartShutdown calls from validation code #28048

Merged

TheCharlatan mentioned this pull request Jul 8, 2023

kernel: Remove shutdown globals from kernel library #27711

Closed

This was referenced Jul 8, 2023

index: make startup more efficient #27607

Merged

httpserver, rest: improving URI validation #27253

Closed

refactor: Move stopafterblockimport option out of blockstorage #28053

Merged

TheCharlatan reviewed Jul 8, 2023

View reviewed changes

src/rpc/server.cpp Outdated Show resolved Hide resolved

src/qt/winshutdownmonitor.cpp Outdated Show resolved Hide resolved

src/init.cpp Outdated Show resolved Hide resolved

DrahtBot added the Needs rebase label Jul 10, 2023

ryanofsky mentioned this pull request Jul 10, 2023

kernel: Rm ShutdownRequested and AbortNode from validation code. #27861

Merged

ryanofsky marked this pull request as draft July 10, 2023 23:43

ryanofsky force-pushed the pr/noshut branch from d9453bd to 24169b1 Compare July 10, 2023 23:56

ryanofsky commented Jul 10, 2023

View reviewed changes

src/rpc/server.cpp Outdated Show resolved Hide resolved

ryanofsky force-pushed the pr/noshut branch from 24169b1 to 0dce004 Compare July 13, 2023 03:49

DrahtBot removed the Needs rebase label Jul 13, 2023

DrahtBot mentioned this pull request Jul 14, 2023

indexes: Stop using node internal types and locking cs_main, improve sync logic #24230

Draft

DrahtBot added the Needs rebase label Jul 14, 2023

ryanofsky force-pushed the pr/noshut branch from 0dce004 to eccea15 Compare August 21, 2023 20:23

ryanofsky marked this pull request as ready for review August 21, 2023 20:27

DrahtBot added CI failed and removed Needs rebase labels Aug 21, 2023

DrahtBot mentioned this pull request Aug 22, 2023

[WIP] BIP300 (Drivechains) consensus-level logic #28311

Closed

This was referenced Aug 22, 2023

kernel: Run sanity checks on context construction #28228

Closed

blockstorage: Drop legacy -txindex check #28195

Merged

ryanofsky force-pushed the pr/noshut branch from eccea15 to c1ff72a Compare August 22, 2023 12:07

This was referenced Aug 22, 2023

blockstorage: XOR blocksdir *.dat files #28052

Merged

index: initial sync speedup, parallelize process #26966

Open

ryanofsky added 2 commits December 4, 2023 15:39

DrahtBot added the Needs rebase label Dec 4, 2023

ryanofsky force-pushed the pr/noshut branch from 437a0c2 to 8a02ce5 Compare December 4, 2023 20:32

ryanofsky commented Dec 4, 2023

View reviewed changes

DrahtBot removed the Needs rebase label Dec 4, 2023

stickies-v approved these changes Dec 5, 2023

View reviewed changes

DrahtBot requested a review from TheCharlatan December 5, 2023 09:13

TheCharlatan reviewed Dec 6, 2023

View reviewed changes

DrahtBot requested a review from TheCharlatan December 6, 2023 11:52

ryanofsky force-pushed the pr/noshut branch from 8a02ce5 to 4a3a265 Compare December 6, 2023 18:44

ryanofsky commented Dec 6, 2023

View reviewed changes

TheCharlatan approved these changes Dec 7, 2023

View reviewed changes

DrahtBot requested a review from stickies-v December 7, 2023 10:36

ryanofsky commented Dec 11, 2023

View reviewed changes

ryanofsky force-pushed the pr/noshut branch from 4a3a265 to eaf915d Compare December 11, 2023 21:39

TheCharlatan approved these changes Dec 11, 2023

View reviewed changes

maflcko approved these changes Dec 12, 2023

View reviewed changes

ryanofsky force-pushed the pr/noshut branch from eaf915d to 6db04be Compare December 12, 2023 19:40

ryanofsky commented Dec 12, 2023

View reviewed changes

TheCharlatan approved these changes Dec 12, 2023

View reviewed changes

DrahtBot requested a review from maflcko December 12, 2023 20:16

maflcko reviewed Dec 13, 2023

View reviewed changes

stickies-v approved these changes Dec 13, 2023

View reviewed changes

achow101 merged commit 4ad5c71 into bitcoin:master Dec 14, 2023

hebasto mentioned this pull request Jan 9, 2024

cmake: Regular rebasing of the cmake-staging branch hebasto/bitcoin#75

Closed

theuni reviewed May 6, 2024

View reviewed changes

theuni mentioned this pull request May 8, 2024

kernel: Remove key module from kernel library #29252

Merged

bitcoin locked and limited conversation to collaborators May 7, 2025

Get rid of shutdown.cpp/shutdown.h, use SignalInterrupt directly #28051

Get rid of shutdown.cpp/shutdown.h, use SignalInterrupt directly #28051

Uh oh!

Conversation

ryanofsky commented Jul 7, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DrahtBot commented Jul 7, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Coverage

Reviews

Conflicts

Uh oh!

maflcko commented Jul 8, 2023

Uh oh!

TheCharlatan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ryanofsky left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

fanquake commented Aug 7, 2023

Uh oh!

ryanofsky commented Aug 21, 2023

Uh oh!

fanquake commented Aug 22, 2023

Uh oh!

ryanofsky left a comment

Choose a reason for hiding this comment

Uh oh!

stickies-v left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ryanofsky left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

TheCharlatan left a comment

Choose a reason for hiding this comment

Uh oh!

ryanofsky left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ryanofsky Dec 11, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TheCharlatan left a comment

Choose a reason for hiding this comment

Uh oh!

maflcko left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ryanofsky commented Jul 7, 2023 •

edited

Loading

DrahtBot commented Jul 7, 2023 •

edited

Loading

stickies-v left a comment •

edited

Loading

ryanofsky left a comment •

edited

Loading

ryanofsky Dec 11, 2023 •

edited

Loading

ryanofsky Dec 13, 2023 •

edited

Loading