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

feat: account serialization overhaul #12794

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

pugachAG
Copy link
Contributor

@pugachAG pugachAG commented Jan 24, 2025

Change Account to be enum and use different representations for serde and borsh ser/deser:

  • SerdeAccount is introduced for serde to maintain backward and forward compatibility. Previously Account struct was directly annotated to support serde, we are opting out of this.
  • BorshVersionedAccount is introduced for borsh serialization. Accounts in old format are serialized directly as AccountV1. Note that we continue serializing in old format when possible to avoid 16 bytes overhead of sentinel padding.

Account::SERIALIZATION_SENTINEL hack is preserved for borsh serialization as I could not find a better way to handle backward compatibility.

This is part of #12716, in a separate PR global contracts related fields will be added as part of Account.

@pugachAG pugachAG requested a review from staffik January 24, 2025 13:03
@pugachAG pugachAG force-pushed the rework-account-ser branch 5 times, most recently from ee8d576 to 2fb8991 Compare January 25, 2025 18:24
Copy link

codecov bot commented Jan 25, 2025

Codecov Report

Attention: Patch coverage is 90.44586% with 15 lines in your changes missing coverage. Please review.

Project coverage is 70.46%. Comparing base (5b32984) to head (2fb8991).
Report is 9 commits behind head on master.

Files with missing lines Patch % Lines
core/primitives-core/src/account.rs 90.44% 11 Missing and 4 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #12794      +/-   ##
==========================================
- Coverage   70.53%   70.46%   -0.08%     
==========================================
  Files         846      847       +1     
  Lines      174904   175051     +147     
  Branches   174904   175051     +147     
==========================================
- Hits       123372   123341      -31     
- Misses      46282    46453     +171     
- Partials     5250     5257       +7     
Flag Coverage Δ
backward-compatibility 0.16% <0.00%> (-0.01%) ⬇️
db-migration 0.16% <0.00%> (-0.01%) ⬇️
genesis-check 1.41% <26.36%> (+0.06%) ⬆️
linux 70.06% <76.43%> (+0.89%) ⬆️
linux-nightly 70.10% <90.44%> (-0.05%) ⬇️
pytests 1.70% <26.36%> (+0.05%) ⬆️
sanity-checks 1.52% <26.36%> (+0.06%) ⬆️
unittests 70.29% <90.44%> (-0.08%) ⬇️
upgradability 0.20% <0.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@pugachAG pugachAG requested a review from stedfn January 25, 2025 18:42
@pugachAG pugachAG marked this pull request as ready for review January 25, 2025 18:43
@pugachAG pugachAG requested a review from a team as a code owner January 25, 2025 18:43
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.

1 participant