Skip to content

Conversation

ryanofsky
Copy link
Contributor

Delay wallet client construction until after logging, thread and other init for two reasons:

  • More responsive multiprocess GUI startup. When bitcoin-gui is started this moves the call from bitcoin-gui to bitcoin-node that spawns bitcoin-wallet off of the GUI event thread and onto the background GUI init executor thread.

  • Avoids feature_logging.py test failures with bitcoin-node by making bitcoin-wallet logging start after bitcoin-node logging starts,
    because the tests are not written to handle the bitcoin-wallet logging init code running first.

This partially reverts commit b266b3e, moving wallet client creation back to the place it was located before.


This PR is part of the process separation project.

Delay wallet client construction until after logging, thread and other
init for two reasons:

- More responsive multiprocess GUI startup. When bitcoin-gui is started
  this moves the call from bitcoin-gui to bitcoin-node that spawns
  bitcoin-wallet off of the GUI event thread and onto the background GUI
  init executor thread.

- Avoids feature_logging.py test failures with bitcoin-node by making
  bitcoin-wallet logging start after bitcoin-node logging starts,
  because the tests are not written to handle the bitcoin-wallet logging
  init code running first.

This partially reverts commit b266b3e,
moving wallet client creation back to the place it was located before.
@ryanofsky
Copy link
Contributor Author

Updated ca0704c -> ad085f9 (pr/ipc-client.1 -> pr/ipc-client.2, compare) to fix clang preprocessor warning https://cirrus-ci.com/task/6303923532201984

@DrahtBot
Copy link
Contributor

DrahtBot commented Sep 18, 2021

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

Conflicts

No conflicts as of last run.

Copy link
Member

@hebasto hebasto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK ad085f9, I have reviewed the code and it looks OK.
Also slightly tested, including the GUI: can observe wallet init messages on the splash screen, as expected.

@laanwj
Copy link
Member

laanwj commented Nov 15, 2021

code review ACK ad085f9

@laanwj laanwj merged commit 7f0f853 into bitcoin:master Nov 15, 2021
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Nov 16, 2021
@@ -82,6 +82,9 @@ class CClientUIInterface
/** Progress message during initialization. */
ADD_SIGNALS_DECL_WRAPPER(InitMessage, void, const std::string& message);

/** Wallet client created. */
ADD_SIGNALS_DECL_WRAPPER(InitWallet, void, );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

any reason for the trailing comma?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

any reason for the trailing comma?

This is to fix the clang warning ./node/ui_interface.h:86:46: error: must specify at least one argument for '...' parameter of variadic macro [-Werror,-Wgnu-zero-variadic-macro-arguments] ADD_SIGNALS_DECL_WRAPPER(InitWallet, void); https://cirrus-ci.com/task/6303923532201984

@bitcoin bitcoin locked and limited conversation to collaborators Nov 17, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
Development

Successfully merging this pull request may close these issues.

6 participants