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.

Sign up for GitHub

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

Jump to bottom

fix(snapshot): allow last block mismatch; relative store path #4868

Merged
merged 3 commits into from
Jul 24, 2024
Merged

fix(snapshot): allow last block mismatch; relative store path #4868

merged 3 commits into from
Jul 24, 2024

Conversation

Erigara
Copy link
Contributor

@Erigara Erigara commented Jul 19, 2024 •
edited
Loading

Description

Changes:

  • relative path to snapshot folder is resolved in the same way as for kura
  • last block hash check of snapshot was missed before:
    • not it is checked
    • if check fail last changes to the state are discarded without losing the whole snapshot

Benefits

  • Behavior of configuration is consistent
  • All blocks are checked

How to test

  1. Run iroha
  2. Wait until iroha create snapshot
  3. Stop iroha
  4. Modify last hash in block_hashes.blocks (any character except for last one)
  5. Load iroha
  6. Verify that iroha is loaded from snapshot
  7. Check that last block was discarded (in logs)
  8. Check that iroha can continue to commit new blocks

@Erigara Erigara added the Bug Something isn't working label Jul 19, 2024
@Erigara Erigara self-assigned this Jul 19, 2024
mversic
mversic previously approved these changes Jul 23, 2024
View reviewed changes
0x009922
0x009922 reviewed Jul 23, 2024
View reviewed changes
Copy link
Contributor

@0x009922 0x009922 left a comment

Choose a reason for hiding this comment

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

LGTM in general.

I think PR's title is not accurate; maybe, "fix(snapshot): allow last block mismatch; relative store path"?

core/src/snapshot.rs Outdated Show resolved Hide resolved
@Erigara Erigara changed the title fix(snapshot): verify all blocks while reading snapshot fix(snapshot): allow last block mismatch; relative store path Jul 24, 2024
@Erigara
fix(snapshot): snapshot dir_path is resolved
b43e965 
Signed-off-by: Shanin Roman <shanin1000@yandex.ru>
@Erigara
fix(snapshot): verify all blocks while loading snapshot
63ad43a 
Signed-off-by: Shanin Roman <shanin1000@yandex.ru>
@Erigara
test(snapshot): add test to check if incorrect latest block is discarded
bf974a3 
Signed-off-by: Shanin Roman <shanin1000@yandex.ru>
@Erigara Erigara merged commit 2f5cb5f into hyperledger:main Jul 24, 2024
11 of 12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SamHSmith SamHSmith SamHSmith approved these changes

@0x009922 0x009922 0x009922 approved these changes

@mversic mversic mversic left review comments

@s8sato s8sato Awaiting requested review from s8sato s8sato is a code owner

@outoftardis outoftardis Awaiting requested review from outoftardis outoftardis is a code owner

@DCNick3 DCNick3 Awaiting requested review from DCNick3 DCNick3 is a code owner

@dima74 dima74 Awaiting requested review from dima74 dima74 is a code owner

Assignees

@DCNick3 DCNick3

@Erigara Erigara

@0x009922 0x009922

Labels
Bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@Erigara @SamHSmith @0x009922 @mversic @DCNick3