Skip to content
This repository was archived by the owner on Jan 22, 2025. It is now read-only.

More serde snapshot cleanup#22449

Merged
brooksprumo merged 1 commit intosolana-labs:masterfrom
brooksprumo:serde-snapshot-2
Jan 13, 2022
Merged

More serde snapshot cleanup#22449
brooksprumo merged 1 commit intosolana-labs:masterfrom
brooksprumo:serde-snapshot-2

Conversation

@brooksprumo
Copy link
Copy Markdown
Contributor

@brooksprumo brooksprumo commented Jan 12, 2022

This PR is part two of cleanup for serde snapshot, following PR #22431.

@codecov
Copy link
Copy Markdown

codecov Bot commented Jan 12, 2022

Codecov Report

Merging #22449 (ae92a36) into master (9c3144e) will decrease coverage by 0.0%.
The diff coverage is 30.0%.

@@            Coverage Diff            @@
##           master   #22449     +/-   ##
=========================================
- Coverage    81.1%    81.1%   -0.1%     
=========================================
  Files         555      556      +1     
  Lines      150664   150665      +1     
=========================================
- Hits       122301   122292      -9     
- Misses      28363    28373     +10     

Comment on lines -10 to -44
/// the serialized type is fixed as usize
pub type AppendVecIdSerialized = usize;

// Serializable version of AccountStorageEntry for snapshot format
#[derive(Clone, Copy, Debug, Default, Eq, PartialEq, Serialize, Deserialize)]
pub(super) struct SerializableAccountStorageEntry {
id: AppendVecIdSerialized,
accounts_current_len: usize,
}

pub trait SerializableStorage {
fn id(&self) -> AppendVecIdSerialized;
fn current_len(&self) -> usize;
}

impl SerializableStorage for SerializableAccountStorageEntry {
fn id(&self) -> AppendVecIdSerialized {
self.id
}
fn current_len(&self) -> usize {
self.accounts_current_len
}
}

#[cfg(RUSTC_WITH_SPECIALIZATION)]
impl solana_frozen_abi::abi_example::IgnoreAsHelper for SerializableAccountStorageEntry {}

impl From<&AccountStorageEntry> for SerializableAccountStorageEntry {
fn from(rhs: &AccountStorageEntry) -> Self {
Self {
id: rhs.append_vec_id() as AppendVecIdSerialized,
accounts_current_len: rhs.accounts.len(),
}
}
}
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I moved all this over to a new file, storage.rs, because I'm going to add a new serde style, which would result in these types/traits being duplicated and causing build errors.

// This some what long test harness is required to freeze the ABI of
// Bank's serialization due to versioned nature
#[frozen_abi(digest = "4xi75P1M48JwDjxf5k8y43r2w57AjYmgjMB1BmX6hXKK")]
#[frozen_abi(digest = "7PcarCw6gpw9Yw8xypdxQP24TFjLiaHyuDkq95cgwtte")]
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The diff is due to moving the storage fields into a new module.

$ diff
1629c1629
<                                     element solana_runtime::serde_snapshot::newer::SerializableAccountStorageEntry
---
>                                     element solana_runtime::serde_snapshot::storage::SerializableAccountStorageEntry

// because it's handled by SerializableVersionedBank.
// So, sync fields with it!
#[derive(Clone, Deserialize)]
pub(crate) struct DeserializableVersionedBank {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Nothing in here is used outside of this module, so removing the pub. Same with SerializableVersionedBank below.

@brooksprumo brooksprumo marked this pull request as ready for review January 13, 2022 14:27
@brooksprumo brooksprumo requested a review from mvines January 13, 2022 14:27
@brooksprumo brooksprumo merged commit 2756abc into solana-labs:master Jan 13, 2022
@brooksprumo brooksprumo deleted the serde-snapshot-2 branch January 13, 2022 15:20
mergify Bot pushed a commit that referenced this pull request Feb 1, 2022
(cherry picked from commit 2756abc)

# Conflicts:
#	runtime/src/serde_snapshot.rs
#	runtime/src/serde_snapshot/newer.rs
mergify Bot added a commit that referenced this pull request Feb 1, 2022
* More serde snapshot cleanup (#22449)

(cherry picked from commit 2756abc)

# Conflicts:
#	runtime/src/serde_snapshot.rs
#	runtime/src/serde_snapshot/newer.rs

* fixup

Co-authored-by: Brooks Prumo <brooks@solana.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants