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

A0-3032: Handle own blocks in sync #1426

Merged
merged 3 commits into from
Oct 9, 2023

Conversation

timorleph
Copy link
Contributor

Description

Blocks created by us should go through sync and get broadcast as soon as we create them.

Type of change

  • New feature (non-breaking change which adds functionality)

Checklist:

(I might need to add tests, but I wanted to have this mostly done before my days off.)

  • I have made corresponding changes to the existing documentation
  • I have created new documentation

Copy link
Contributor

@woocash2 woocash2 left a comment

Choose a reason for hiding this comment

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

I approve not to block, but I would need more context to understand better how Aura interacts with this code. I'd be grateful for a short explanation or some reference.

I understand that we have a wrapper for block_import which is adjusted for Aura. I get that check_block is aligned with inner.check_block, but import_block doesn't check_block which I perceive as some simplification which Aura won't break.

Tho I don't get why it doesn't break with Aura, and why we need this wrapper in the first place. Was there an issue that Aura sometimes didn't work with the original block_import?

@timorleph
Copy link
Contributor Author

The issue this is dealing with is passing the blocks Aura produces through sync, so that they can be broadcast. The wrapper intercepts created blocks and first passes them through our sync, which only afterwards forwards them to the proper import queue.

Aura only makes very limited use of the results returned by the import queue we give it (as I outlined in one of the responses above), so intercepting the newly created blocks this way cannot impact its logic. I haven't checked whether check_block is used anywhere, but it does not modify the wrapped object and the current implementation returns the exact same thing as it did previously, so it should still work. The implementations of import_block do not usually call check_block in the first place, so that shouldn't be too weird – anyway, we let Aura perform all the necessary checks later, when it passes through the other import queue after going through our sync.

@timorleph timorleph added this pull request to the merge queue Oct 9, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 9, 2023
@timorleph timorleph added this pull request to the merge queue Oct 9, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 9, 2023
@timorleph timorleph added this pull request to the merge queue Oct 9, 2023
Merged via the queue into Cardinal-Cryptography:main with commit f7c7a90 Oct 9, 2023
45 checks passed
@timorleph timorleph deleted the A0-3032-obmna branch October 9, 2023 15:40
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.

3 participants