[Wallet] Start API on rewinding

[dev0tion]

Currently when the node starts rewinding on startup, the API is not started. Core UI can't detect the API so it shows the unable to connect message, advising to restart Core and try again. Doing this will also stop the rewind process.

Is there anyway to start the API and give me a rewinding status? Preferably with an "x blocks left" tied to it so I can display it in the UI?

We can generalise this issue to start the API before anything really, this way you can include the given exception in the status call if anything goes wrong, allowing me to display it in the UI.

[fassadlr]

@dev0tion I will take this one :)

[MithrilMan]

While this may be a workaround for this issue, I didn't consider this a proper way to handle the node initialization problem. As it is it doesn't fully solve the

We can generalise this issue to start the API before anything really

because just moving .UseApi() after .UseNodeSettings will initialize anyway API after BaseFeature, and so I think node will still be "unresponsive" at API level until API are initialized

I remember that feature Initialization is a synchronous process where we initialize features in the order they are defined, one by one.

To proper handle the node initialization process I think we should have a component that's used to communicate with other components about the status of the node initialization, or we can rely on a couple of property on the FullNode itself, anyway I don't think ConsensusManager is the right place to place a Status message.

Probably the better way is to have a kind of FullNodeLifeCycle component (suggest a better name) that has a Status enum that track the status of the fullnode like "building, initializing, starting, started, stopping, stopped" and a bunch of method to set status messages.

We could add this component as an argument to feature InitializeAsync method so every feature has already a way to update the status without having to add another component in their constructor.

Who want to know the status of the node has then just to query that component (we could signal whenever the status/message change).

There is then the language problem to take care of, but this is another subject.

[MithrilMan]

I'll add another consideration: probably while the FullNode is not started, every API call should return a status like 503 where we can return the reason of the unavailability of the service (like the Status enum and Status Message)

[fassadlr]

@MithrilMan you are right, we still have an issue here as Consensus gets initialized before the API as it is part of the base feature.

We want a mechanism whereby we can query each component for its state, not just an enum but also some log message should we wish to use it.

The API should look something like this:

api/[component]/status

i.e.

api/consensus/status

We can implement an interface on each component that requires it, something like IReturnStatus:

public interface IReturnStatus
{
    FullNodeComponentStatus Status {get;set;}
    string MessageStatus {get;set;}
}

The enum:

public enum FullNodeComponentStatus
{
    Initializing,
    Started,
    Stopped
}

[MithrilMan]

We can just have the FullNodeLifeCycle component to cache last status message for each component that's updating the status, so we don't have to implement that interface on components, no need to implement any interface, and the API can query like FullNodeLifeCycle.GetStatus<ComponentToQuery>()

Btw I'm not sure we need to query status by component, what's the reason?

[dangershony]

This might prove to be difficult but I suggest a global states (initializing, running, shutting-down) But still it does not solve the problem of rewind as that happens in the base I beleive.

[MithrilMan]

We can create a new Issue for the component and i could work on that if you want.

About the order of execution, I think we'd need to refactor the initialization process (we could even find the need of split BaseFeature is sub-features) and to enforce DependsOn usage, with an argument (bool ?) that specify if we need the dependant feature be available during our feature initialization or not. Then we can use these informations to build a proper dependency tree and Initialize feature in the proper order. This is something that must be well thought.

Meanwhile I think that we have to remove .UseBaseFeature from .UseNodeSettings as a starting point, so that we can start Api before base features (but IMO any API call done before the node is started, should return 503 as I suggested above, except if we want some special API calls to be available to query components (but I don't see why, having a current status should already be enough)

[fassadlr]

@dangershony I think that the first task here should be to get the API to start before all the other components are initialized.

@MithrilMan I like the idea of a FullNodeLifeCycle component. We can make this a static object that lives throughout the life time of the node and can then easily be logged to and/or queried by whichever consumer.

The component can then store say the last 100 messages or so, which the API can then query.

This way we don't have to inject a component everywhere and it is available anywhere in the Node.

[bokobza]

Granular states will be difficult for such a big code base with so many things happening. It's very easy to get lost and for flag/states to get overwritten by different components. I don't want our node to turn into this:

Having said that, a bunch of state for initialization/disposal could be cool.

Also, @fassadlr are you sure that when you put the APi at the beginning, all the controllers get discovered?

[fassadlr]

@bokobza If we have a static log container that any component can log to and component can query from, wont it solve this issue? The API can then call it very easily and see what the node of the node is in a more verbose manner (instead of just a enum).

[bokobza]

I don't think it would. A static container with unstructured inputs (logs vs predefined-values) is going to be a nightmare to maintain (goodbye integration tests).

[fassadlr]

I don't understand the issue with integration tests?

I was basically suggesting this:

    public static class NodeStatusLog
    {
        private static List<string> messages;

        static NodeStatusLog()
        {
            messages = new List<string>();
        }

        public static void LogMessage(string message)
        {
            if (message.Count() > 100)
            {
                var last = message.Last();
                message.Remove(last);
            }

            messages.Add(message);
        }

        public static IEnumerable<string> Get()
        {
            return messages;
        }
    }

[bokobza]

The issue with integration tests is that we'll be in the same position as when we had static flags, i.e if you run 2 nodes during a test and you query the static log container, you won't be able to rely on the data as it'll be shared between the nodes.

[fassadlr]

@bokobza I completely forgot about that issue we had 👎 LOL.

OK so 2 other options is to:

1> Extend the current NodeStats component to have a list of messages that can be added to or queried. 2> Implement the aforementioned NodeStatusLog component but register it in the DI as a singleton. Then we can inject it to any component wishing to log more verbose messages about its state.

[MithrilMan]

I'm against static instance, but I'd go for a singleton and I'd populate their status messages only during Initialization, so we can pass that component in InitializeAsync method. If we feel the need to update an internal status even when the node is running, then we may just inject the "statusLogger" component into the constructor

[bokobza]

I'm quite against logs and messages in this case. I think we should have data/flags/states and the consuming service (for example StratisCore) can create their own messages.

[fassadlr]

OK but then we have to create states for each component?

The issue here is that we need to let the wallet know that we are rewinding (which is specific to consensus) as well as some message stating the progress of it ("rewinding from block 400 to 234").

I think that the best to do this is to extend NodeStats to have a status message that each component can push to it if need be.

[MithrilMan]

Ok let me recap: If the goal is to have a responsive UI that know that a Node is doing something, just having a status (enum) and message is enough and we can get rid of other more "complicated" solution.

If I remember correctly all originatead by the problem that users of Core were closing the App because it seemed to be unresponsive, while the node itself were just doing its startup stuffs that took a lot, but the user didn't had evidence of this.

I already faced kind of this problem when I had to interact with BTC-like RPC and it ended up in a change that got even merged in bitcoincore, so I can understand well the need of such way to know node is warming up.

Now, giving the fact that each feature InitializeAsync method is called syncronously, the problem would be solved by having a kind of frendly message returned by the API, that state current warmup node status.

The easiest way I can think about is to pass an argument to InitializeAsync that rapresents a class that has just to hold the Initialization status message

Internally in FullNode (if we don't have an ad hoc component) we should hold its current state (initializing, running,etc...) and of course that state has to be updated to Initializing before calling the initialization status, and setting it to Starting after that

At this point, we just have to start API before everything, and whenever an API method is called, we have to check the node Status. If the status is Initializing we have just to query fullnode.statusMessage or how we want to name that property and return a 503 error with the status as message

The Core wallet than has just to wrap the calls and handle the 503 error to display properly the message to user

Then it's a matter of having a proper Initialization method in the feature we want to generate a precise status message (e.g. if we want Blockstore to report what the hell he's doing, then we need to change that behavior, otherwise if it's enough to say "Loading Transactions", "Loading Blocks", etc... then it's easier

[fassadlr]

This will be implemented in #3210

[fassadlr]

@dev0tion this has now been implemented. Fixed with #3227 .