Skip to content

Commit

Permalink
feat(api): Rework zks_getProtocolVersion (#2146)
Browse files Browse the repository at this point in the history
## What ❔

- renames fields in method's response
- removes `verification_keys_hashes` completely

## Why ❔

- more meaningfully field names and camelCase for consistency
- `verification_keys_hashes` doesn't make sense in the context of minor
protocol version

## Checklist

<!-- Check your PR fulfills the following items. -->
<!-- For draft PRs check the boxes as you complete them. -->

- [x] PR title corresponds to the body of PR (we generate changelog
entries from PRs).
- [x] Tests for the changes have been added / updated.
- [x] Documentation comments have been added / updated.
- [x] Code has been formatted via `zk fmt` and `zk lint`.
- [x] Spellcheck has been run via `zk spellcheck`.
  • Loading branch information
perekopskiy committed Jun 5, 2024
1 parent 5c03964 commit 800b8f4
Show file tree
Hide file tree
Showing 9 changed files with 219 additions and 83 deletions.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

45 changes: 18 additions & 27 deletions core/lib/dal/src/models/storage_protocol_version.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ pub struct StorageProtocolVersion {
pub recursion_circuits_set_vks_hash: Vec<u8>,
pub bootloader_code_hash: Vec<u8>,
pub default_account_code_hash: Vec<u8>,
pub upgrade_tx_hash: Option<Vec<u8>>,
}

pub(crate) fn protocol_version_from_storage(
Expand Down Expand Up @@ -56,36 +55,28 @@ pub(crate) fn protocol_version_from_storage(
}
}

impl From<StorageProtocolVersion> for api::ProtocolVersion {
fn from(storage_protocol_version: StorageProtocolVersion) -> Self {
#[derive(sqlx::FromRow)]
pub struct StorageApiProtocolVersion {
pub minor: i32,
pub timestamp: i64,
pub bootloader_code_hash: Vec<u8>,
pub default_account_code_hash: Vec<u8>,
pub upgrade_tx_hash: Option<Vec<u8>>,
}

impl From<StorageApiProtocolVersion> for api::ProtocolVersion {
#[allow(deprecated)]
fn from(storage_protocol_version: StorageApiProtocolVersion) -> Self {
let l2_system_upgrade_tx_hash = storage_protocol_version
.upgrade_tx_hash
.as_ref()
.map(|hash| H256::from_slice(hash));
api::ProtocolVersion {
version_id: storage_protocol_version.minor as u16,
timestamp: storage_protocol_version.timestamp as u64,
verification_keys_hashes: L1VerifierConfig {
params: VerifierParams {
recursion_node_level_vk_hash: H256::from_slice(
&storage_protocol_version.recursion_node_level_vk_hash,
),
recursion_leaf_level_vk_hash: H256::from_slice(
&storage_protocol_version.recursion_leaf_level_vk_hash,
),
recursion_circuits_set_vks_hash: H256::from_slice(
&storage_protocol_version.recursion_circuits_set_vks_hash,
),
},
recursion_scheduler_level_vk_hash: H256::from_slice(
&storage_protocol_version.recursion_scheduler_level_vk_hash,
),
},
base_system_contracts: BaseSystemContractsHashes {
bootloader: H256::from_slice(&storage_protocol_version.bootloader_code_hash),
default_aa: H256::from_slice(&storage_protocol_version.default_account_code_hash),
},
api::ProtocolVersion::new(
storage_protocol_version.minor as u16,
storage_protocol_version.timestamp as u64,
H256::from_slice(&storage_protocol_version.bootloader_code_hash),
H256::from_slice(&storage_protocol_version.default_account_code_hash),
l2_system_upgrade_tx_hash,
}
)
}
}
1 change: 0 additions & 1 deletion core/lib/dal/src/protocol_versions_dal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,6 @@ impl ProtocolVersionsDal<'_, '_> {
protocol_versions.timestamp,
protocol_versions.bootloader_code_hash,
protocol_versions.default_account_code_hash,
protocol_versions.upgrade_tx_hash,
protocol_patches.patch,
protocol_patches.recursion_scheduler_level_vk_hash,
protocol_patches.recursion_node_level_vk_hash,
Expand Down
24 changes: 7 additions & 17 deletions core/lib/dal/src/protocol_versions_web3_dal.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use zksync_db_connection::{connection::Connection, error::DalResult, instrument::InstrumentExt};
use zksync_types::api::ProtocolVersion;

use crate::{models::storage_protocol_version::StorageProtocolVersion, Core, CoreDal};
use crate::{models::storage_protocol_version::StorageApiProtocolVersion, Core, CoreDal};

#[derive(Debug)]
pub struct ProtocolVersionsWeb3Dal<'a, 'c> {
Expand All @@ -14,28 +14,18 @@ impl ProtocolVersionsWeb3Dal<'_, '_> {
version_id: u16,
) -> DalResult<Option<ProtocolVersion>> {
let storage_protocol_version = sqlx::query_as!(
StorageProtocolVersion,
StorageApiProtocolVersion,
r#"
SELECT
protocol_versions.id AS "minor!",
protocol_versions.timestamp,
protocol_versions.bootloader_code_hash,
protocol_versions.default_account_code_hash,
protocol_versions.upgrade_tx_hash,
protocol_patches.patch,
protocol_patches.recursion_scheduler_level_vk_hash,
protocol_patches.recursion_node_level_vk_hash,
protocol_patches.recursion_leaf_level_vk_hash,
protocol_patches.recursion_circuits_set_vks_hash
id AS "minor!",
timestamp,
bootloader_code_hash,
default_account_code_hash,
upgrade_tx_hash
FROM
protocol_versions
JOIN protocol_patches ON protocol_patches.minor = protocol_versions.id
WHERE
id = $1
ORDER BY
protocol_patches.patch DESC
LIMIT
1
"#,
i32::from(version_id)
)
Expand Down
106 changes: 102 additions & 4 deletions core/lib/types/src/api/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -640,18 +640,81 @@ impl From<Call> for DebugCall {
}
}

// TODO (PLA-965): remove deprecated fields from the struct. It is currently in a "migration" phase
// to keep compatibility between old and new versions.
#[derive(Default, Serialize, Deserialize, Clone, Debug)]
pub struct ProtocolVersion {
/// Protocol version ID
pub version_id: u16,
/// Minor version of the protocol
#[deprecated]
pub version_id: Option<u16>,
/// Minor version of the protocol
#[serde(rename = "minorVersion")]
pub minor_version: Option<u16>,
/// Timestamp at which upgrade should be performed
pub timestamp: u64,
/// Verifier configuration
pub verification_keys_hashes: L1VerifierConfig,
#[deprecated]
pub verification_keys_hashes: Option<L1VerifierConfig>,
/// Hashes of base system contracts (bootloader and default account)
pub base_system_contracts: BaseSystemContractsHashes,
#[deprecated]
pub base_system_contracts: Option<BaseSystemContractsHashes>,
/// Bootloader code hash
#[serde(rename = "bootloaderCodeHash")]
pub bootloader_code_hash: Option<H256>,
/// Default account code hash
#[serde(rename = "defaultAccountCodeHash")]
pub default_account_code_hash: Option<H256>,
/// L2 Upgrade transaction hash
#[deprecated]
pub l2_system_upgrade_tx_hash: Option<H256>,
/// L2 Upgrade transaction hash
#[serde(rename = "l2SystemUpgradeTxHash")]
pub l2_system_upgrade_tx_hash_new: Option<H256>,
}

#[allow(deprecated)]
impl ProtocolVersion {
pub fn new(
minor_version: u16,
timestamp: u64,
bootloader_code_hash: H256,
default_account_code_hash: H256,
l2_system_upgrade_tx_hash: Option<H256>,
) -> Self {
Self {
version_id: Some(minor_version),
minor_version: Some(minor_version),
timestamp,
verification_keys_hashes: Some(Default::default()),
base_system_contracts: Some(BaseSystemContractsHashes {
bootloader: bootloader_code_hash,
default_aa: default_account_code_hash,
}),
bootloader_code_hash: Some(bootloader_code_hash),
default_account_code_hash: Some(default_account_code_hash),
l2_system_upgrade_tx_hash,
l2_system_upgrade_tx_hash_new: l2_system_upgrade_tx_hash,
}
}

pub fn bootloader_code_hash(&self) -> Option<H256> {
self.bootloader_code_hash
.or_else(|| self.base_system_contracts.map(|hashes| hashes.bootloader))
}

pub fn default_account_code_hash(&self) -> Option<H256> {
self.default_account_code_hash
.or_else(|| self.base_system_contracts.map(|hashes| hashes.default_aa))
}

pub fn minor_version(&self) -> Option<u16> {
self.minor_version.or(self.version_id)
}

pub fn l2_system_upgrade_tx_hash(&self) -> Option<H256> {
self.l2_system_upgrade_tx_hash_new
.or(self.l2_system_upgrade_tx_hash)
}
}

#[derive(Debug, Serialize, Deserialize, Clone)]
Expand Down Expand Up @@ -751,3 +814,38 @@ pub struct ApiStorageLog {
pub key: U256,
pub written_value: U256,
}

#[cfg(test)]
mod tests {
use super::*;

// TODO (PLA-965): remove test after removing deprecating fields.
#[allow(deprecated)]
#[test]
fn check_protocol_version_type_compatibility() {
let new_version = ProtocolVersion {
version_id: Some(24),
minor_version: Some(24),
timestamp: 0,
verification_keys_hashes: Some(Default::default()),
base_system_contracts: Some(Default::default()),
bootloader_code_hash: Some(Default::default()),
default_account_code_hash: Some(Default::default()),
l2_system_upgrade_tx_hash: Default::default(),
l2_system_upgrade_tx_hash_new: Default::default(),
};

#[derive(Deserialize)]
#[allow(dead_code)]
struct OldProtocolVersion {
pub version_id: u16,
pub timestamp: u64,
pub verification_keys_hashes: L1VerifierConfig,
pub base_system_contracts: BaseSystemContractsHashes,
pub l2_system_upgrade_tx_hash: Option<H256>,
}

serde_json::from_str::<OldProtocolVersion>(&serde_json::to_string(&new_version).unwrap())
.unwrap();
}
}
Loading

0 comments on commit 800b8f4

Please sign in to comment.