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

Save blocks to db #359

Merged
merged 15 commits into from
Aug 4, 2023
Merged

Save blocks to db #359

merged 15 commits into from
Aug 4, 2023

Conversation

qiweiii
Copy link
Member

@qiweiii qiweiii commented Aug 2, 2023

related to #347

image

@qiweiii qiweiii mentioned this pull request Aug 2, 2023
3 tasks
@qiweiii qiweiii marked this pull request as ready for review August 2, 2023 05:55
@qiweiii qiweiii requested a review from xlc August 2, 2023 06:31
@ermalkaleci
Copy link
Collaborator

@xlc @qiweiii this isn't making much sense to me. We need to save block on db for caching, not every block fetched/created. Basically when block is removed from memory it is saved into db for later use

@qiweiii
Copy link
Member Author

qiweiii commented Aug 2, 2023

@xlc @qiweiii this isn't making much sense to me. We need to save block on db for caching, not every block fetched/created. Basically when block is removed from memory it is saved into db for later use

right, maybe my understanding is wrong, if we want to resume from where we last stopped, is it necessary to save every block? If not, I guess we don't need to save them

@xlc
Copy link
Member

xlc commented Aug 2, 2023

We want to save all the new blocks. We don't need to save blocks fetched from remote API. Right now it is saving all the blocks including the one fetched from remote API.

@ermalkaleci
Copy link
Collaborator

ermalkaleci commented Aug 2, 2023

should work like this

chain: 1 -> 2 -> 3 -> 4 -> 5 -> head 6
fork at 3 |
chopsticks. 3(memory only) -> 4(memory+db) -> head 5(memory+db)

chopsticks start with --resume flag then load head from db
chopsticks. 2 (db ?? api) -> 3 (db ?? api) -> 4(db ?? api) -> head 5(memory+db)

@qiweiii
Copy link
Member Author

qiweiii commented Aug 2, 2023

Made some changes, now only save blocks created by us

@qiweiii qiweiii requested a review from ermalkaleci August 2, 2023 11:10
@qiweiii qiweiii requested a review from ermalkaleci August 3, 2023 00:31
packages/chopsticks/src/blockchain/index.ts Outdated Show resolved Hide resolved
packages/chopsticks/src/blockchain/index.ts Outdated Show resolved Hide resolved
packages/e2e/src/blocks-save.test.ts Outdated Show resolved Hide resolved
@xlc
Copy link
Member

xlc commented Aug 3, 2023

I think with this, we will be able to load chain from db by specifying right block hash. Need a manual test and e2e test to confirm that.

expect(blockData?.parentHash).toEqual((await block?.parentBlock)?.hash)
expect(JSON.stringify(blockData?.extrinsics)).toEqual(JSON.stringify(await block?.extrinsics))
expect(JSON.stringify(blockData?.storageDiff)).toEqual(JSON.stringify(await block?.storageDiff()))
const blockData = await chain.db!.getRepository('Block').findOne({ where: { number: chain.head.number } })
Copy link
Member

Choose a reason for hiding this comment

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

should check block and blockData is not undefined and no need all the !

@qiweiii
Copy link
Member Author

qiweiii commented Aug 4, 2023

I think with this, we will be able to load chain from db by specifying right block hash. Need a manual test and e2e test to confirm that.

Maybe do this in a separate PR coz may need to change setup for load to work properly?

@xlc xlc enabled auto-merge (squash) August 4, 2023 03:57
@xlc xlc merged commit 2dc60ba into AcalaNetwork:master Aug 4, 2023
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