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

[1.0] Update FSI serialization to support dynamically sized data and verify a checksum #624

Merged
merged 4 commits into from
Aug 27, 2024
Merged

[1.0] Update FSI serialization to support dynamically sized data and verify a checksum #624

merged 4 commits into from
Aug 27, 2024

Conversation

heifner
Copy link
Member

@heifner heifner commented Aug 23, 2024

  • Add version 1 of FSI safety file.

    • Supports variable size FSI with inactive FSIs written at the top of the file
    • Adds checksum verification of FSI data

  • Added a new fc::datastream_crc which uses boost::crc for checksum. Defaults to boost::crc_32_type, but any boost::crc type will work.

Resolves #617

@heifner heifner requested a review from greg7mdp August 23, 2024 17:37
@heifner heifner added the OCI Work exclusive to OCI team label Aug 23, 2024
@arhag arhag linked an issue Aug 23, 2024 that may be closed by this pull request
Update fsi serialization implementation to support dynamically size serialized blobs #617
Closed
@heifner heifner requested a review from arhag August 23, 2024 20:23
greg7mdp
greg7mdp reviewed Aug 23, 2024
View reviewed changes
libraries/chain/include/eosio/chain/finality/finalizer.hpp
Comment on lines +80 to +81
static constexpr uint64_t safety_file_version_0 = 0;
static constexpr uint64_t safety_file_version_1 = 1;
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think these are very useful, but OK.

Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe they represent all valid options? 🤷 Not sure, but seems better than just 0 and 1 hard coded.

Copy link
Contributor

Choose a reason for hiding this comment

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

I feel since version_0 will always be 0 and version_1 will always be 1, hardcoded is just as good and one less indirection, but personal preference so no need to change.

libraries/chain/finality/finalizer.cpp Show resolved Hide resolved
libraries/libfc/include/fc/io/datastream_crc.hpp Show resolved Hide resolved
@ericpassmore
Copy link
Contributor

Note:start
group: STABILITY
category: INTERNALS
summary: New version of Finalizer Safety Information file to support dynamically sized data, with checksum.
Note:end

@heifner heifner mentioned this pull request Aug 27, 2024
Closed
@heifner heifner merged commit 5fc37f8 into release/1.0 Aug 27, 2024
36 checks passed
@heifner heifner deleted the GH-617-fsi-serialization branch August 27, 2024 03:16
This was referenced Aug 27, 2024
Merged
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@linh2931 linh2931 linh2931 approved these changes

@arhag arhag arhag approved these changes

@greg7mdp greg7mdp greg7mdp approved these changes

Assignees
No one assigned
Labels
OCI Work exclusive to OCI team
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Update fsi serialization implementation to support dynamically size serialized blobs
5 participants
@heifner @ericpassmore @greg7mdp @arhag @linh2931