Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

App is crashing at startup since upgrade to 1.6.0 #1661

Closed
holzeis opened this issue Nov 30, 2023 · 2 comments · Fixed by #1665
Closed

App is crashing at startup since upgrade to 1.6.0 #1661

holzeis opened this issue Nov 30, 2023 · 2 comments · Fixed by #1665
Assignees
Labels
bug Something isn't working critical prod-environment

Comments

@holzeis
Copy link
Contributor

holzeis commented Nov 30, 2023

The app is crashing for some of our users after upgrading to 1.6.0.

[E] TIME: 2023-11-24T18:42:12.320227Z Launching the app failed FfiException(RESULT_ERROR, Failed to start the backend

Caused by:
    0: Failed to initialise DlcManager
    1: Storage error Unknown required feature preventing decode, null)
@holzeis holzeis added bug Something isn't working prod-environment critical labels Nov 30, 2023
@holzeis holzeis assigned holzeis and unassigned holzeis Nov 30, 2023
@luckysori luckysori self-assigned this Dec 1, 2023
@luckysori
Copy link
Contributor

It might be helpful to see more logs from the user.

The user reported that they were able to open up the app on version 1.5.0. Unclear whether they were able to do it on 1.6.0 or not, their messages were ambiguous.

The error above is associated with being unable to load the ChainMonitor when constructing the dlc_manager::Manager (aka the DlcManager).

We introduced breaking changes to the ChainMonitor with version 1.5.0, but we also added some code before building the DlcManager to ensure backwards-compatibility:

// FIXME: We need to do this to ensure that we can upgrade `Node`s from LDK 0.0.114 to 0.0.116.
// We should remove this workaround as soon as possible.
if let Err(e) = dlc_storage.get_chain_monitor() {
    tracing::error!("Failed to load DLC ChainMonitor from storage: {e:#}");

    tracing::info!("Overwriting old DLC ChainMonitor with empty one to be able to proceed");
    dlc_storage.persist_chain_monitor(&dlc_manager::chain_monitor::ChainMonitor::new(0))?;
}

In theory, any time we fail to load the ChainMonitor because of a breaking change, we should print these logs and overwrite the ChainMonitor with an empty one. This is not a long term fix, but we thought it was acceptable for now, since we weren't even loading the persisted ChainMonitor before these changes due to a bug in rust-dlc.

I don't really see how we could be failing on startup given all this, since the code pasted above is still present in >=1.6.0 and our rust-dlc and rust-lightning dependencies haven't been updated since then.

A theory could be that our relatively new implementation of DlcStorageProvider is not working correctly because:

  1. we are not loading the ChainMonitor from the same spot where we persist it; or
  2. when we call persist_chain_monitor we are not actually overwriting the existing ChainMonitor.

Full logs would be helpful here.

@luckysori
Copy link
Contributor

luckysori commented Dec 1, 2023

1. we are not loading the `ChainMonitor` from the same spot where we persist it; or

We have a test for this, so this is covered.

2. when we call `persist_chain_monitor` we are not actually overwriting the existing `ChainMonitor`.

Just tested it and it is being overwritten correctly 🤷

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working critical prod-environment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants