From 800b8f456282685e81d3423ba3e27d017db2f183 Mon Sep 17 00:00:00 2001 From: perekopskiy <53865202+perekopskiy@users.noreply.github.com> Date: Wed, 5 Jun 2024 15:28:57 +0300 Subject: [PATCH] feat(api): Rework zks_getProtocolVersion (#2146) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 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 - [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`. --- ...6d2fe2908a22e933b2f25ce6b4357e51ed9b.json} | 18 +-- ...3c2a3d0a09c5ee88bdd671462904d4d27a355.json | 46 ++++++++ .../src/models/storage_protocol_version.rs | 45 +++----- core/lib/dal/src/protocol_versions_dal.rs | 1 - .../lib/dal/src/protocol_versions_web3_dal.rs | 24 ++-- core/lib/types/src/api/mod.rs | 106 +++++++++++++++++- core/node/node_sync/src/external_io.rs | 30 +++-- core/node/node_sync/src/tests.rs | 27 +++-- .../ts-integration/tests/api/web3.test.ts | 5 +- 9 files changed, 219 insertions(+), 83 deletions(-) rename core/lib/dal/.sqlx/{query-67852a656494ec8381b253b71e1b3572757aba0580637c0ef0e7cc5cdd7396f3.json => query-33b1fbb1e80c3815d30da5854c866d2fe2908a22e933b2f25ce6b4357e51ed9b.json} (64%) create mode 100644 core/lib/dal/.sqlx/query-5556ebdb040428b42c04ea9121b3c2a3d0a09c5ee88bdd671462904d4d27a355.json diff --git a/core/lib/dal/.sqlx/query-67852a656494ec8381b253b71e1b3572757aba0580637c0ef0e7cc5cdd7396f3.json b/core/lib/dal/.sqlx/query-33b1fbb1e80c3815d30da5854c866d2fe2908a22e933b2f25ce6b4357e51ed9b.json similarity index 64% rename from core/lib/dal/.sqlx/query-67852a656494ec8381b253b71e1b3572757aba0580637c0ef0e7cc5cdd7396f3.json rename to core/lib/dal/.sqlx/query-33b1fbb1e80c3815d30da5854c866d2fe2908a22e933b2f25ce6b4357e51ed9b.json index 6defdf7afeb..bb38503cc35 100644 --- a/core/lib/dal/.sqlx/query-67852a656494ec8381b253b71e1b3572757aba0580637c0ef0e7cc5cdd7396f3.json +++ b/core/lib/dal/.sqlx/query-33b1fbb1e80c3815d30da5854c866d2fe2908a22e933b2f25ce6b4357e51ed9b.json @@ -1,6 +1,6 @@ { "db_name": "PostgreSQL", - "query": "\n SELECT\n protocol_versions.id AS \"minor!\",\n protocol_versions.timestamp,\n protocol_versions.bootloader_code_hash,\n protocol_versions.default_account_code_hash,\n protocol_versions.upgrade_tx_hash,\n protocol_patches.patch,\n protocol_patches.recursion_scheduler_level_vk_hash,\n protocol_patches.recursion_node_level_vk_hash,\n protocol_patches.recursion_leaf_level_vk_hash,\n protocol_patches.recursion_circuits_set_vks_hash\n FROM\n protocol_versions\n JOIN protocol_patches ON protocol_patches.minor = protocol_versions.id\n WHERE\n id = $1\n ORDER BY\n protocol_patches.patch DESC\n LIMIT\n 1\n ", + "query": "\n SELECT\n protocol_versions.id AS \"minor!\",\n protocol_versions.timestamp,\n protocol_versions.bootloader_code_hash,\n protocol_versions.default_account_code_hash,\n protocol_patches.patch,\n protocol_patches.recursion_scheduler_level_vk_hash,\n protocol_patches.recursion_node_level_vk_hash,\n protocol_patches.recursion_leaf_level_vk_hash,\n protocol_patches.recursion_circuits_set_vks_hash\n FROM\n protocol_versions\n JOIN protocol_patches ON protocol_patches.minor = protocol_versions.id\n WHERE\n id = $1\n ORDER BY\n protocol_patches.patch DESC\n LIMIT\n 1\n ", "describe": { "columns": [ { @@ -25,31 +25,26 @@ }, { "ordinal": 4, - "name": "upgrade_tx_hash", - "type_info": "Bytea" - }, - { - "ordinal": 5, "name": "patch", "type_info": "Int4" }, { - "ordinal": 6, + "ordinal": 5, "name": "recursion_scheduler_level_vk_hash", "type_info": "Bytea" }, { - "ordinal": 7, + "ordinal": 6, "name": "recursion_node_level_vk_hash", "type_info": "Bytea" }, { - "ordinal": 8, + "ordinal": 7, "name": "recursion_leaf_level_vk_hash", "type_info": "Bytea" }, { - "ordinal": 9, + "ordinal": 8, "name": "recursion_circuits_set_vks_hash", "type_info": "Bytea" } @@ -64,7 +59,6 @@ false, false, false, - true, false, false, false, @@ -72,5 +66,5 @@ false ] }, - "hash": "67852a656494ec8381b253b71e1b3572757aba0580637c0ef0e7cc5cdd7396f3" + "hash": "33b1fbb1e80c3815d30da5854c866d2fe2908a22e933b2f25ce6b4357e51ed9b" } diff --git a/core/lib/dal/.sqlx/query-5556ebdb040428b42c04ea9121b3c2a3d0a09c5ee88bdd671462904d4d27a355.json b/core/lib/dal/.sqlx/query-5556ebdb040428b42c04ea9121b3c2a3d0a09c5ee88bdd671462904d4d27a355.json new file mode 100644 index 00000000000..5e9051587bb --- /dev/null +++ b/core/lib/dal/.sqlx/query-5556ebdb040428b42c04ea9121b3c2a3d0a09c5ee88bdd671462904d4d27a355.json @@ -0,0 +1,46 @@ +{ + "db_name": "PostgreSQL", + "query": "\n SELECT\n id AS \"minor!\",\n timestamp,\n bootloader_code_hash,\n default_account_code_hash,\n upgrade_tx_hash\n FROM\n protocol_versions\n WHERE\n id = $1\n ", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "minor!", + "type_info": "Int4" + }, + { + "ordinal": 1, + "name": "timestamp", + "type_info": "Int8" + }, + { + "ordinal": 2, + "name": "bootloader_code_hash", + "type_info": "Bytea" + }, + { + "ordinal": 3, + "name": "default_account_code_hash", + "type_info": "Bytea" + }, + { + "ordinal": 4, + "name": "upgrade_tx_hash", + "type_info": "Bytea" + } + ], + "parameters": { + "Left": [ + "Int4" + ] + }, + "nullable": [ + false, + false, + false, + false, + true + ] + }, + "hash": "5556ebdb040428b42c04ea9121b3c2a3d0a09c5ee88bdd671462904d4d27a355" +} diff --git a/core/lib/dal/src/models/storage_protocol_version.rs b/core/lib/dal/src/models/storage_protocol_version.rs index f21fa594f66..7ac6d70f38c 100644 --- a/core/lib/dal/src/models/storage_protocol_version.rs +++ b/core/lib/dal/src/models/storage_protocol_version.rs @@ -19,7 +19,6 @@ pub struct StorageProtocolVersion { pub recursion_circuits_set_vks_hash: Vec, pub bootloader_code_hash: Vec, pub default_account_code_hash: Vec, - pub upgrade_tx_hash: Option>, } pub(crate) fn protocol_version_from_storage( @@ -56,36 +55,28 @@ pub(crate) fn protocol_version_from_storage( } } -impl From 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, + pub default_account_code_hash: Vec, + pub upgrade_tx_hash: Option>, +} + +impl From 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, - } + ) } } diff --git a/core/lib/dal/src/protocol_versions_dal.rs b/core/lib/dal/src/protocol_versions_dal.rs index c395d8cba4c..212be734f0b 100644 --- a/core/lib/dal/src/protocol_versions_dal.rs +++ b/core/lib/dal/src/protocol_versions_dal.rs @@ -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, diff --git a/core/lib/dal/src/protocol_versions_web3_dal.rs b/core/lib/dal/src/protocol_versions_web3_dal.rs index 5b5e1e21dca..a3a7a162c3d 100644 --- a/core/lib/dal/src/protocol_versions_web3_dal.rs +++ b/core/lib/dal/src/protocol_versions_web3_dal.rs @@ -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> { @@ -14,28 +14,18 @@ impl ProtocolVersionsWeb3Dal<'_, '_> { version_id: u16, ) -> DalResult> { 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) ) diff --git a/core/lib/types/src/api/mod.rs b/core/lib/types/src/api/mod.rs index 6bebb238880..5c0bfe2d848 100644 --- a/core/lib/types/src/api/mod.rs +++ b/core/lib/types/src/api/mod.rs @@ -640,18 +640,81 @@ impl From 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, + /// Minor version of the protocol + #[serde(rename = "minorVersion")] + pub minor_version: Option, /// Timestamp at which upgrade should be performed pub timestamp: u64, /// Verifier configuration - pub verification_keys_hashes: L1VerifierConfig, + #[deprecated] + pub verification_keys_hashes: Option, /// Hashes of base system contracts (bootloader and default account) - pub base_system_contracts: BaseSystemContractsHashes, + #[deprecated] + pub base_system_contracts: Option, + /// Bootloader code hash + #[serde(rename = "bootloaderCodeHash")] + pub bootloader_code_hash: Option, + /// Default account code hash + #[serde(rename = "defaultAccountCodeHash")] + pub default_account_code_hash: Option, /// L2 Upgrade transaction hash + #[deprecated] pub l2_system_upgrade_tx_hash: Option, + /// L2 Upgrade transaction hash + #[serde(rename = "l2SystemUpgradeTxHash")] + pub l2_system_upgrade_tx_hash_new: Option, +} + +#[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, + ) -> 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 { + self.bootloader_code_hash + .or_else(|| self.base_system_contracts.map(|hashes| hashes.bootloader)) + } + + pub fn default_account_code_hash(&self) -> Option { + self.default_account_code_hash + .or_else(|| self.base_system_contracts.map(|hashes| hashes.default_aa)) + } + + pub fn minor_version(&self) -> Option { + self.minor_version.or(self.version_id) + } + + pub fn l2_system_upgrade_tx_hash(&self) -> Option { + self.l2_system_upgrade_tx_hash_new + .or(self.l2_system_upgrade_tx_hash) + } } #[derive(Debug, Serialize, Deserialize, Clone)] @@ -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, + } + + serde_json::from_str::(&serde_json::to_string(&new_version).unwrap()) + .unwrap(); + } +} diff --git a/core/node/node_sync/src/external_io.rs b/core/node/node_sync/src/external_io.rs index 630ecc36c41..559c377e453 100644 --- a/core/node/node_sync/src/external_io.rs +++ b/core/node/node_sync/src/external_io.rs @@ -337,35 +337,43 @@ impl StateKeeperIO for ExternalIO { .await .context("failed to fetch protocol version from the main node")? .context("protocol version is missing on the main node")?; + let minor = protocol_version + .minor_version() + .context("Missing minor protocol version")?; + let bootloader_code_hash = protocol_version + .bootloader_code_hash() + .context("Missing bootloader code hash")?; + let default_account_code_hash = protocol_version + .default_account_code_hash() + .context("Missing default account code hash")?; + let l2_system_upgrade_tx_hash = protocol_version.l2_system_upgrade_tx_hash(); self.pool .connection_tagged("sync_layer") .await? .protocol_versions_dal() .save_protocol_version( ProtocolSemanticVersion { - minor: protocol_version - .version_id + minor: minor .try_into() .context("cannot convert protocol version")?, patch: VersionPatch(0), }, protocol_version.timestamp, - protocol_version.verification_keys_hashes, - protocol_version.base_system_contracts, - protocol_version.l2_system_upgrade_tx_hash, + Default::default(), // verification keys are unused for EN + BaseSystemContractsHashes { + bootloader: bootloader_code_hash, + default_aa: default_account_code_hash, + }, + l2_system_upgrade_tx_hash, ) .await?; - let BaseSystemContractsHashes { - bootloader, - default_aa, - } = protocol_version.base_system_contracts; let bootloader = self - .get_base_system_contract(bootloader, cursor.next_l2_block) + .get_base_system_contract(bootloader_code_hash, cursor.next_l2_block) .await .with_context(|| format!("cannot fetch bootloader code for {protocol_version:?}"))?; let default_aa = self - .get_base_system_contract(default_aa, cursor.next_l2_block) + .get_base_system_contract(default_account_code_hash, cursor.next_l2_block) .await .with_context(|| format!("cannot fetch default AA code for {protocol_version:?}"))?; Ok(BaseSystemContracts { diff --git a/core/node/node_sync/src/tests.rs b/core/node/node_sync/src/tests.rs index 1d278d1af38..9830641a9fa 100644 --- a/core/node/node_sync/src/tests.rs +++ b/core/node/node_sync/src/tests.rs @@ -77,10 +77,11 @@ impl MockMainNodeClient { pub fn insert_protocol_version(&mut self, version: api::ProtocolVersion) { self.system_contracts - .insert(version.base_system_contracts.bootloader, vec![]); + .insert(version.bootloader_code_hash.unwrap(), vec![]); self.system_contracts - .insert(version.base_system_contracts.default_aa, vec![]); - self.protocol_versions.insert(version.version_id, version); + .insert(version.default_account_code_hash.unwrap(), vec![]); + self.protocol_versions + .insert(version.minor_version.unwrap(), version); } } @@ -300,12 +301,10 @@ async fn external_io_works_without_local_protocol_version(snapshot_recovery: boo let (actions_sender, action_queue) = ActionQueue::new(); let mut client = MockMainNodeClient::default(); let next_protocol_version = api::ProtocolVersion { - version_id: ProtocolVersionId::next() as u16, + minor_version: Some(ProtocolVersionId::next() as u16), timestamp: snapshot.l2_block_timestamp + 1, - base_system_contracts: BaseSystemContractsHashes { - bootloader: H256::repeat_byte(1), - default_aa: H256::repeat_byte(2), - }, + bootloader_code_hash: Some(H256::repeat_byte(1)), + default_account_code_hash: Some(H256::repeat_byte(1)), ..api::ProtocolVersion::default() }; client.insert_protocol_version(next_protocol_version.clone()); @@ -335,8 +334,16 @@ async fn external_io_works_without_local_protocol_version(snapshot_recovery: boo next_protocol_version.timestamp ); assert_eq!( - persisted_protocol_version.base_system_contracts_hashes, - next_protocol_version.base_system_contracts + persisted_protocol_version + .base_system_contracts_hashes + .bootloader, + next_protocol_version.bootloader_code_hash.unwrap() + ); + assert_eq!( + persisted_protocol_version + .base_system_contracts_hashes + .default_aa, + next_protocol_version.default_account_code_hash.unwrap() ); let l2_block = storage diff --git a/core/tests/ts-integration/tests/api/web3.test.ts b/core/tests/ts-integration/tests/api/web3.test.ts index ff590a24cf5..39f9eb610d3 100644 --- a/core/tests/ts-integration/tests/api/web3.test.ts +++ b/core/tests/ts-integration/tests/api/web3.test.ts @@ -833,7 +833,10 @@ describe('web3 API compatibility tests', () => { }; let expectedProtocolVersion = { version_id: expect.any(Number), + minorVersion: expect.any(Number), base_system_contracts: expectedSysContractsHashes, + bootloaderCodeHash: expect.stringMatching(HEX_VALUE_REGEX), + defaultAccountCodeHash: expect.stringMatching(HEX_VALUE_REGEX), verification_keys_hashes: { params: { recursion_circuits_set_vks_hash: expect.stringMatching(HEX_VALUE_REGEX), @@ -847,7 +850,7 @@ describe('web3 API compatibility tests', () => { expect(latestProtocolVersion).toMatchObject(expectedProtocolVersion); const exactProtocolVersion = await alice.provider.send('zks_getProtocolVersion', [ - latestProtocolVersion.version_id + latestProtocolVersion.minorVersion ]); expect(exactProtocolVersion).toMatchObject(expectedProtocolVersion); });