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

Store a block's canonicality directly in the blocks table #1449

Merged
merged 1 commit into from
Sep 26, 2024

Conversation

JamesHinshelwood
Copy link
Contributor

One of the causes of #1056 was that we could add a block to blocks but not add the number -> hash mapping in main_chain_canonical_blocks. This meant the block existed if you looked it up via the hash or view, but didn't if looked up via its number.

We could fix this by wrapping both mutations in a database operation, but I think its cleaner to just add an is_canonical column to blocks. We maintain the invariant that, for all canonical blocks, the block number is unique. This is easy to maintain because blocks always start as canonical and only ever become non-canonical once (i.e. a block can't become canonical again later).

One of the causes of #1056 was that we could add a block to
`blocks` but not add the `number` -> `hash` mapping in
`main_chain_canonical_blocks`. This meant the block existed if you
looked it up via the hash or view, but didn't if looked up via its
number.

We could fix this by wrapping both mutations in a database
operation, but I think its cleaner to just add an `is_canonical`
column to `blocks`. We maintain the invariant that, for all
canonical blocks, the block `number` is unique. This is easy to
maintain because blocks always start as canonical and only ever
become non-canonical once (i.e. a block can't become canonical
again later).
Copy link
Contributor

@rrw-zilliqa rrw-zilliqa left a comment

Choose a reason for hiding this comment

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

Looks good to me :-)

@JamesHinshelwood JamesHinshelwood added this pull request to the merge queue Sep 26, 2024
Merged via the queue into main with commit b4d73f4 Sep 26, 2024
5 checks passed
@JamesHinshelwood JamesHinshelwood deleted the move-canonical branch September 26, 2024 14:55
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