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

Add plugin notification topic "block_added" #5581

Merged
merged 2 commits into from
Sep 14, 2022

Conversation

fiatjaf
Copy link
Contributor

@fiatjaf fiatjaf commented Sep 9, 2022

This would be useful for two completely unrelated plugins I'm working on recently, and it's a simple change that prevents the plugins from having to reimplement the block polling logic all by themselves every time.

@fiatjaf fiatjaf force-pushed the notify_block_processed branch 2 times, most recently from 582d310 to 29f035d Compare September 9, 2022 19:20
Changelog-Added: Plugins: Added notification topic "block_processed".
@fiatjaf
Copy link
Contributor Author

fiatjaf commented Sep 10, 2022

I think the failing tests are unrelated to this PR.

Copy link
Collaborator

@vincenzopalazzo vincenzopalazzo left a comment

Choose a reason for hiding this comment

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

ACK c8819a0

Yes the test are failing regarding some timeout

@fiatjaf
Copy link
Contributor Author

fiatjaf commented Sep 12, 2022

I don't like this name, though, maybe block_added is better?

@vincenzopalazzo
Copy link
Collaborator

I don't like this name, though, maybe block_added is better?

As a notification, also new_block sounds good, but I do not have any preference here :) up to you

@fiatjaf
Copy link
Contributor Author

fiatjaf commented Sep 12, 2022

new_block was my first pick, but the function name notify_new_block was already taken and was causing conflicts.

@vincenzopalazzo
Copy link
Collaborator

+1 for block_added

@niftynei
Copy link
Collaborator

niftynei commented Sep 14, 2022

what about new_block_found :trollface:

@niftynei
Copy link
Collaborator

ACK 357404b

@niftynei niftynei merged commit 1ef8fb7 into ElementsProject:master Sep 14, 2022
@fiatjaf fiatjaf changed the title Add plugin notification topic "block_processed" Add plugin notification topic "block_added" Sep 14, 2022
@fiatjaf fiatjaf deleted the notify_block_processed branch September 14, 2022 23:23
Comment on lines +8 to +19
blocks_catched = []


@plugin.subscribe("block_added")
def notify_block_added(plugin, block, **kwargs):
global blocks_catched
blocks_catched.append(block["height"])


@plugin.method("blockscatched")
def return_moves(plugin):
return blocks_catched

Choose a reason for hiding this comment

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

I first read that as “cached”, but should that perhaps be “caught”?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I don't speak English. I will submit a PR renaming that.

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.

4 participants