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

[WIP] Add block justification support #309

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jakehemmerle
Copy link
Contributor

@jakehemmerle jakehemmerle commented Jun 27, 2021

waiting on a blocking sqlx bug mentioned in #147, should close #147 when this is merged

In the meantime, can you let me know if the next steps sound right?:

  1. sqlx prepare to generate proper sqlx-data.json (this is blocking bug)
  2. is there a preferred way to test insert/query/delete other than configuring and spinning up substrate-archive? I did not find any reference to the 10K_BLOCKS.bin file so I'm assuming there aren't test cases to test insertion/query/deletion.

Copy link
Collaborator

@insipx insipx 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! Some small nits. Hopefully the SQLx bug gets fixed sooner rather than later :).

As for testing, there is a way. You can look here:

fn should_get_missing_storage() -> Result<(), Error> {
for how I tested blocks_paginated query, etc.

It's relatively new code with some of the recent PR's, so open to suggestions, but you can see/use the setup_data_scheme for an example of populating the SQL database with the 10,000 blocks. Also good to keep in mind that the 10,000 blocks start from block height 3,000,000. Those blocks are accessed from the test_common crate:

let blocks: Vec<BlockModel> = test_common::get_kusama_blocks()?.drain(0..1000).map(BlockModel::from).collect();

setup_data_scheme already inserts a bunch of blocks into SQL, so a good test might just be running that and then checking if the expected justifications exist 👍

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.

Store Justifications along with the rest of the block
2 participants