Skip to content

Conversation

jurvis
Copy link
Collaborator

@jurvis jurvis commented Apr 19, 2022

Enable using Combine to subscribe to chain updates on an interval instead of Task.sleep

@@ -478,6 +478,18 @@ class BlockchainObserver {
return result
}

public func isMonitoring() async -> Bool {
return await self.monitoringTracker.startTracking()
Copy link
Contributor

Choose a reason for hiding this comment

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

that seems like a very dangerous method if it's called before the monitor is running, seeing how it will prevent it from ever being started.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

will this be better?

    public func isMonitoring() async throws -> Bool {
        try await preloadMonitor()
        return await self.monitoringTracker.startTracking()
    }

Copy link
Contributor

Choose a reason for hiding this comment

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

nah, just add a method to the monitoring Tracker to indicate whether or not it's tracking

Copy link
Contributor

Choose a reason for hiding this comment

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

it will be reliable at the moment simply because it's an actor

@@ -478,7 +478,8 @@ class BlockchainObserver {
return result
}

public func isMonitoring() async -> Bool {
public func isMonitoring() async throws -> Bool {
try await preloadMonitor()
Copy link
Contributor

Choose a reason for hiding this comment

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

sorry, that makes it worse. You don't want a check (basically a getter) to change state

@jurvis jurvis force-pushed the regtest_monitor_tooling branch from 484fbe8 to e02f22f Compare April 20, 2022 04:03
@jurvis jurvis force-pushed the regtest_monitor_tooling branch from e02f22f to ff6533a Compare April 20, 2022 04:04
Copy link
Contributor

@arik-so arik-so left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@arik-so arik-so merged commit f579b1d into lightningdevkit:main Apr 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants