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

rpc: Fix block connected handler. #412

Merged
merged 1 commit into from
Aug 24, 2023
Merged

Conversation

jholdstock
Copy link
Member

Fix a bug (introduced in #407) where it appeared as though a notification handler was being attached to the dcrd RPC client, but actually no notifications were received until the client was disconnected a new one was created.

Now, rather than delaying attaching the notification handler, it is attached immediately upon client creation and vspd does not start handling the received notifications until it is ready.

Fix a bug where it appeared as though a notification handler was being
attached to the dcrd RPC client, but actually no notifications were
received until the client was disconnected a new one was created.

Now, rather than delaying attaching the notification handler, it is
attached immediately upon client creation and vspd does not start
handling the received notifications until it is ready.
Copy link
Member

@davecgh davecgh left a comment

Choose a reason for hiding this comment

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

Nice catch. I missed this on my review of 407 as well. I wouldn't have expected the RPC client to only register for notifyblocks on connection while simultaneously offering the ability to dynamically attach a handler.

Given that client behavior, it makes much more sense to require the creation of a channel during setup and nuke the ability to asynchronously attach a handler as this does.

@jholdstock jholdstock merged commit 49b9db1 into decred:master Aug 24, 2023
2 checks passed
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.

None yet

2 participants