Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix(kura): correctly handle replace_top_block #4870

Merged
merged 4 commits into from
Aug 26, 2024

Conversation

Erigara
Copy link
Contributor

@Erigara Erigara commented Jul 22, 2024

Description

I've noticed multiple issues with kura which this PR is aimed to fix.

  1. Quick sequential call of replace_top_block and store_block cause kura to omit overwriting soft-fork block
    • this happened because kura checked latest_written_block hash against latest_block_hash which is not correct because between kura loop iteration more than one block could be added
  2. Call store_block before or just after start cause kura to incorrectly assume amount of already written blocks

Benefits

Fewer bugs.

How to test

To check this issues create_blocks test was added.

core/src/kura.rs Outdated Show resolved Hide resolved
@nxsaken nxsaken force-pushed the kura_miss_replace_top branch from 6f7d968 to 38d7eed Compare July 26, 2024 10:12
core/Cargo.toml Outdated Show resolved Hide resolved
core/src/kura.rs Outdated Show resolved Hide resolved
DCNick3
DCNick3 previously approved these changes Aug 7, 2024
core/src/kura.rs Outdated Show resolved Hide resolved
core/src/kura.rs Show resolved Hide resolved
@DCNick3 DCNick3 self-requested a review August 7, 2024 07:50
@Erigara Erigara force-pushed the kura_miss_replace_top branch from 38d7eed to b71ec1a Compare August 7, 2024 08:25
@Erigara Erigara requested a review from mversic August 7, 2024 09:22
core/src/kura.rs Outdated Show resolved Hide resolved
@Erigara Erigara force-pushed the kura_miss_replace_top branch from b71ec1a to 351d603 Compare August 8, 2024 09:00
@Erigara Erigara requested a review from DCNick3 August 8, 2024 09:01
DCNick3
DCNick3 previously approved these changes Aug 8, 2024
@Erigara
Copy link
Contributor Author

Erigara commented Aug 26, 2024

@mversic @DCNick3 updated, reapprove pls

Signed-off-by: Shanin Roman <shanin1000@yandex.ru>
Signed-off-by: Shanin Roman <shanin1000@yandex.ru>
Signed-off-by: Shanin Roman <shanin1000@yandex.ru>
Signed-off-by: Shanin Roman <shanin1000@yandex.ru>
@mversic mversic force-pushed the kura_miss_replace_top branch from e603436 to 72eae0c Compare August 26, 2024 13:17
@nxsaken nxsaken merged commit 168b0b3 into hyperledger-iroha:main Aug 26, 2024
12 of 14 checks passed
mversic pushed a commit that referenced this pull request Aug 30, 2024
* test(kura): add test kura not miss replace top block call
* fix(kura): properly initialize kura
* fix(kura): correctly handle replace_top_block
* fix(kura): fix warnings inside kura
---------
Signed-off-by: Shanin Roman <shanin1000@yandex.ru>
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants