Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Block import notification for literally every imported block even when the chain is not synced to the tip #13315

Closed
2 tasks done
liuchengxu opened this issue Feb 6, 2023 · 2 comments · Fixed by #13372
Closed
2 tasks done

Comments

@liuchengxu
Copy link
Contributor

Is there an existing issue?

  • I have searched the existing issues

Experiencing problems? Have you tried our Stack Exchange first?

  • This is not a support question.

Description of bug

I'm looking for a block import notification that is fired on every imported block, the existing client.import_notification_stream() is fired only when on the tip or there is a re-org.

// We only notify when we are already synced to the tip of the chain
// or if this import triggers a re-org

In subspace, we have the consensus chain and execution chain, the execution chain is entirely driven by the consensus chain. Each domain block is derived from a consensus block in a deterministic way, we need a block notification of every single consensus block to build the execution chain accordingly even if the consensus chain is not fully synced.

Currently, we're using a custom block notification sent during the stage of consensus-level block import, but it has an issue (autonomys/subspace#1143) that some states might be unusable when the notification is received by the handler because the block import pipeline is not yet completed.

Steps to reproduce

No response

@bkchr
Copy link
Member

bkchr commented Feb 9, 2023

I would accept a pr adding a second method import_notification_stream_good_name that gives you the opportunity to get such a stream. Internally we should then have two vecs of notification receivers based on what kind of condition they are receiving block notifications.

@liuchengxu
Copy link
Contributor Author

Any chance we can change the semantic of import_notification_stream, concretely speaking,

  • partial_import_notification_stream: Fired when the chain is synced to the tip or there is a re-org.
  • import_notification_stream: Fired on every single block no matter what the sync state is.

This clearly requires larger changes, but I do think partial_import_notification_stream (or recent_import_notification_stream?) describes better the current behavior, the bigger reason is that I honestly don't a good concise name for import_notification_stream_good_name, naming is hard :P

liuchengxu added a commit to liuchengxu/substrate that referenced this issue Feb 13, 2023
paritytech-processbot bot pushed a commit that referenced this issue Feb 27, 2023
* Support the subscription of every import block

Close #13315

* Clean up any closed block import notification sinks

* Apply review suggestions

* Nit

* `every_block_import_notification_sinks` -> `every_import_notification_sinks`

* Apply review suggestions
ukint-vs pushed a commit to gear-tech/substrate that referenced this issue Apr 10, 2023
* Support the subscription of every import block

Close paritytech#13315

* Clean up any closed block import notification sinks

* Apply review suggestions

* Nit

* `every_block_import_notification_sinks` -> `every_import_notification_sinks`

* Apply review suggestions
nathanwhit pushed a commit to nathanwhit/substrate that referenced this issue Jul 19, 2023
* Support the subscription of every import block

Close paritytech#13315

* Clean up any closed block import notification sinks

* Apply review suggestions

* Nit

* `every_block_import_notification_sinks` -> `every_import_notification_sinks`

* Apply review suggestions
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants