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

Supports accounts lt hash in snapshots #3096

Merged
merged 1 commit into from
Oct 29, 2024
Merged

Supports accounts lt hash in snapshots #3096

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 27 additions & 9 deletions runtime/src/bank.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -461,6 +461,8 @@ pub struct BankFieldsToDeserialize {
pub(crate) accounts_data_len: u64,
pub(crate) incremental_snapshot_persistence: Option<BankIncrementalSnapshotPersistence>,
pub(crate) epoch_accounts_hash: Option<Hash>,
// When removing the accounts lt hash featurization code, also remove this Option wrapper
pub(crate) accounts_lt_hash: Option<AccountsLtHash>,
}

/// Bank's common fields shared by all supported snapshot versions for serialization.
Expand Down Expand Up @@ -504,6 +506,8 @@ pub struct BankFieldsToSerialize {
pub is_delta: bool,
pub accounts_data_len: u64,
pub versioned_epoch_stakes: HashMap<u64, VersionedEpochStakes>,
// When removing the accounts lt hash featurization code, also remove this Option wrapper
pub accounts_lt_hash: Option<AccountsLtHash>,
}

// Can't derive PartialEq because RwLock doesn't implement PartialEq
Expand Down Expand Up @@ -662,6 +666,7 @@ impl BankFieldsToSerialize {
is_delta: bool::default(),
accounts_data_len: u64::default(),
versioned_epoch_stakes: HashMap::default(),
accounts_lt_hash: Some(AccountsLtHash(LtHash([0x7E57; LtHash::NUM_ELEMENTS]))),
Copy link

Choose a reason for hiding this comment

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

does it have any special meaning for 7e57?

Copy link
Author

Choose a reason for hiding this comment

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

This is my approximation for "TEST" 😸

}
}
}
Expand Down Expand Up @@ -1750,16 +1755,26 @@ impl Bank {
.fill_missing_sysvar_cache_entries(&bank);
bank.rebuild_skipped_rewrites();

let calculate_accounts_lt_hash_duration = bank.is_accounts_lt_hash_enabled().then(|| {
let (_, duration) = meas_dur!({
*bank.accounts_lt_hash.get_mut().unwrap() = bank
.rc
.accounts
.accounts_db
.calculate_accounts_lt_hash_at_startup_from_index(&bank.ancestors, bank.slot());
let mut calculate_accounts_lt_hash_duration = None;
if bank.is_accounts_lt_hash_enabled() {
// Use the accounts lt hash from the snapshot, if present, otherwise calculate it.
// When there is a feature gate for the accounts lt hash, if the feature is enabled
// then it will be *required* that the snapshot contains an accounts lt hash.
let accounts_lt_hash = fields.accounts_lt_hash.unwrap_or_else(|| {
let (accounts_lt_hash, duration) = meas_dur!({
bank.rc
.accounts
.accounts_db
.calculate_accounts_lt_hash_at_startup_from_index(
&bank.ancestors,
bank.slot(),
)
});
calculate_accounts_lt_hash_duration = Some(duration);
accounts_lt_hash
});
duration
});
*bank.accounts_lt_hash.get_mut().unwrap() = accounts_lt_hash;
}

// Sanity assertions between bank snapshot and genesis config
// Consider removing from serializable bank state
Expand Down Expand Up @@ -1849,6 +1864,9 @@ impl Bank {
is_delta: self.is_delta.load(Relaxed),
accounts_data_len: self.load_accounts_data_size(),
versioned_epoch_stakes,
accounts_lt_hash: self
.is_accounts_lt_hash_enabled()
.then(|| self.accounts_lt_hash.lock().unwrap().clone()),
}
}

Expand Down
45 changes: 39 additions & 6 deletions runtime/src/bank/serde_snapshot.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ mod tests {
solana_accounts_db::{
account_storage::{AccountStorageMap, AccountStorageReference},
accounts_db::{
get_temp_accounts_paths, AccountStorageEntry, AccountsDb, AtomicAccountsFileId,
ACCOUNTS_DB_CONFIG_FOR_TESTING,
get_temp_accounts_paths, AccountStorageEntry, AccountsDb, AccountsDbConfig,
AtomicAccountsFileId, ACCOUNTS_DB_CONFIG_FOR_TESTING,
},
accounts_file::{AccountsFile, AccountsFileError, StorageAccess},
accounts_hash::{AccountsDeltaHash, AccountsHash},
Expand Down Expand Up @@ -96,27 +96,41 @@ mod tests {
let storage_access_iter = [StorageAccess::Mmap, StorageAccess::File].into_iter();
let has_incremental_snapshot_persistence_iter = [false, true].into_iter();
let has_epoch_accounts_hash_iter = [false, true].into_iter();
let has_accounts_lt_hash_iter = [false, true].into_iter();

for (storage_access, has_incremental_snapshot_persistence, has_epoch_accounts_hash) in itertools::iproduct!(
for (
storage_access,
has_incremental_snapshot_persistence,
has_epoch_accounts_hash,
has_accounts_lt_hash,
) in itertools::iproduct!(
storage_access_iter,
has_incremental_snapshot_persistence_iter,
has_epoch_accounts_hash_iter
has_epoch_accounts_hash_iter,
has_accounts_lt_hash_iter
) {
do_serialize_bank_snapshot(
storage_access,
has_incremental_snapshot_persistence,
has_epoch_accounts_hash,
has_accounts_lt_hash,
);
}

fn do_serialize_bank_snapshot(
storage_access: StorageAccess,
has_incremental_snapshot_persistence: bool,
has_epoch_accounts_hash: bool,
has_accounts_lt_hash: bool,
) {
let (mut genesis_config, _) = create_genesis_config(500);
genesis_config.epoch_schedule = EpochSchedule::custom(400, 400, false);
let bank0 = Arc::new(Bank::new_for_tests(&genesis_config));
bank0
.rc
.accounts
.accounts_db
.set_is_experimental_accumulator_hash_enabled(has_accounts_lt_hash);
let deposit_amount = bank0.get_minimum_balance_for_rent_exemption(0);
let eah_start_slot = epoch_accounts_hash_utils::calculation_start(&bank0);
let bank1 = Bank::new_from_parent(bank0.clone(), &Pubkey::default(), 1);
Expand Down Expand Up @@ -166,6 +180,8 @@ mod tests {
.set_valid(epoch_accounts_hash, eah_start_slot);
epoch_accounts_hash
});
let expected_accounts_lt_hash =
has_accounts_lt_hash.then(|| bank2.accounts_lt_hash.lock().unwrap().clone());

// Only if a bank was recently recreated from a snapshot will it have an epoch stakes entry
// of type "delegations" which cannot be serialized into the versioned epoch stakes map. Simulate
Expand Down Expand Up @@ -202,6 +218,7 @@ mod tests {
assert!(!bank_fields.versioned_epoch_stakes.is_empty());

let versioned_epoch_stakes = mem::take(&mut bank_fields.versioned_epoch_stakes);
let accounts_lt_hash = bank_fields.accounts_lt_hash.clone().map(Into::into);
serde_snapshot::serialize_bank_snapshot_into(
&mut writer,
bank_fields,
Expand All @@ -215,6 +232,7 @@ mod tests {
.as_ref(),
epoch_accounts_hash: expected_epoch_accounts_hash,
versioned_epoch_stakes,
accounts_lt_hash,
},
accounts_db.write_version.load(Ordering::Acquire),
)
Expand All @@ -237,6 +255,10 @@ mod tests {
full_snapshot_stream: &mut reader,
incremental_snapshot_stream: None,
};
let accounts_db_config = AccountsDbConfig {
enable_experimental_accumulator_hash: has_accounts_lt_hash,
..ACCOUNTS_DB_CONFIG_FOR_TESTING
};
let (dbank, _) = serde_snapshot::bank_from_streams(
&mut snapshot_streams,
&dbank_paths,
Expand All @@ -247,7 +269,7 @@ mod tests {
None,
None,
false,
Some(ACCOUNTS_DB_CONFIG_FOR_TESTING),
Some(accounts_db_config),
None,
Arc::default(),
)
Expand Down Expand Up @@ -276,6 +298,14 @@ mod tests {
dbank.get_epoch_accounts_hash_to_serialize(),
expected_epoch_accounts_hash,
);
assert_eq!(
dbank.is_accounts_lt_hash_enabled().then(|| dbank
.accounts_lt_hash
.lock()
.unwrap()
.clone()),
expected_accounts_lt_hash,
);

assert_eq!(dbank, bank2);
}
Expand Down Expand Up @@ -510,8 +540,10 @@ mod tests {
super::*,
solana_accounts_db::{
account_storage::meta::StoredMetaWriteVersion, accounts_db::stats::BankHashStats,
accounts_hash::AccountsLtHash,
},
solana_frozen_abi::abi_example::AbiExample,
solana_lattice_hash::lt_hash::LtHash,
solana_sdk::clock::Slot,
std::marker::PhantomData,
};
Expand All @@ -537,7 +569,7 @@ mod tests {
#[cfg_attr(
feature = "frozen-abi",
derive(AbiExample),
frozen_abi(digest = "WZPdQsksD18CRLSPKbinaMU8uZ5zov3iHJMMNvcamMY")
frozen_abi(digest = "EEpMTdTGYGixHBVVtQM2yUJirUiMFMWxNdhABfdscpYW")
)]
#[derive(Serialize)]
pub struct BankAbiTestWrapper {
Expand Down Expand Up @@ -576,6 +608,7 @@ mod tests {
incremental_snapshot_persistence: Some(&incremental_snapshot_persistence),
epoch_accounts_hash: Some(EpochAccountsHash::new(Hash::new_unique())),
versioned_epoch_stakes,
accounts_lt_hash: Some(AccountsLtHash(LtHash::identity()).into()),
},
StoredMetaWriteVersion::default(),
)
Expand Down
9 changes: 7 additions & 2 deletions runtime/src/serde_snapshot.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,7 @@ impl From<DeserializableVersionedBank> for BankFieldsToDeserialize {
is_delta: dvb.is_delta,
incremental_snapshot_persistence: None,
epoch_accounts_hash: None,
accounts_lt_hash: None, // populated from ExtraFieldsToDeserialize
}
}
}
Expand Down Expand Up @@ -402,7 +403,6 @@ struct ExtraFieldsToDeserialize {
#[serde(deserialize_with = "default_on_eof")]
versioned_epoch_stakes: HashMap<u64, VersionedEpochStakes>,
#[serde(deserialize_with = "default_on_eof")]
#[allow(dead_code)]
accounts_lt_hash: Option<SerdeAccountsLtHash>,
}

Expand All @@ -420,6 +420,7 @@ pub struct ExtraFieldsToSerialize<'a> {
pub incremental_snapshot_persistence: Option<&'a BankIncrementalSnapshotPersistence>,
pub epoch_accounts_hash: Option<EpochAccountsHash>,
pub versioned_epoch_stakes: HashMap<u64, VersionedEpochStakes>,
pub accounts_lt_hash: Option<SerdeAccountsLtHash>,
brooksprumo marked this conversation as resolved.
Show resolved Hide resolved
}

fn deserialize_bank_fields<R>(
Expand All @@ -445,7 +446,7 @@ where
incremental_snapshot_persistence,
epoch_accounts_hash,
versioned_epoch_stakes,
accounts_lt_hash: _,
accounts_lt_hash,
} = extra_fields;

bank_fields.fee_rate_governor = bank_fields
Expand All @@ -462,6 +463,8 @@ where
.map(|(epoch, versioned_epoch_stakes)| (epoch, versioned_epoch_stakes.into())),
);

bank_fields.accounts_lt_hash = accounts_lt_hash.map(Into::into);

Ok((bank_fields, accounts_db_fields))
}

Expand Down Expand Up @@ -706,6 +709,7 @@ impl<'a> Serialize for SerializableBankAndStorage<'a> {
let write_version = accounts_db.write_version.load(Ordering::Acquire);
let lamports_per_signature = bank_fields.fee_rate_governor.lamports_per_signature;
let versioned_epoch_stakes = std::mem::take(&mut bank_fields.versioned_epoch_stakes);
let accounts_lt_hash = bank_fields.accounts_lt_hash.clone().map(Into::into);
let bank_fields_to_serialize = (
SerializableVersionedBank::from(bank_fields),
SerializableAccountsDb::<'_> {
Expand All @@ -721,6 +725,7 @@ impl<'a> Serialize for SerializableBankAndStorage<'a> {
incremental_snapshot_persistence: None,
epoch_accounts_hash: self.bank.get_epoch_accounts_hash_to_serialize(),
versioned_epoch_stakes,
accounts_lt_hash,
},
);
bank_fields_to_serialize.serialize(serializer)
Expand Down
1 change: 1 addition & 0 deletions runtime/src/snapshot_utils.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -884,6 +884,7 @@ fn serialize_snapshot(
incremental_snapshot_persistence: bank_incremental_snapshot_persistence,
epoch_accounts_hash,
versioned_epoch_stakes,
accounts_lt_hash: bank_fields.accounts_lt_hash.clone().map(Into::into),
};
serde_snapshot::serialize_bank_snapshot_into(
stream,
Expand Down
Loading