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: (DB migration) Rename recursion_scheduler_level_vk_hash to snark_wrapper_vk_hash #2809

Merged
merged 3 commits into from
Sep 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions Cargo.lock

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

25 changes: 24 additions & 1 deletion core/lib/basic_types/src/protocol_version.rs
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,12 @@ impl Detokenize for VerifierParams {

#[derive(Debug, Clone, Copy, Default, Eq, PartialEq, Serialize, Deserialize)]
pub struct L1VerifierConfig {
pub recursion_scheduler_level_vk_hash: H256,
// Rename is required to not introduce breaking changes in the API for existing clients.
#[serde(
alias = "recursion_scheduler_level_vk_hash",
rename(serialize = "recursion_scheduler_level_vk_hash")
)]
pub snark_wrapper_vk_hash: H256,
}

impl From<ProtocolVersionId> for VmVersion {
Expand Down Expand Up @@ -394,4 +399,22 @@ mod tests {

assert_eq!(version, unpacked);
}

#[test]
fn test_verifier_config_serde() {
let de = [
r#"{"recursion_scheduler_level_vk_hash": "0x1111111111111111111111111111111111111111111111111111111111111111"}"#,
r#"{"snark_wrapper_vk_hash": "0x1111111111111111111111111111111111111111111111111111111111111111"}"#,
];
for de in de.iter() {
let _: L1VerifierConfig = serde_json::from_str(de)
.unwrap_or_else(|err| panic!("Failed deserialization. String: {de}, error {err}"));
}
let ser = L1VerifierConfig {
snark_wrapper_vk_hash: H256::repeat_byte(0x11),
};
let ser_str = serde_json::to_string(&ser).unwrap();
let expected_str = r#"{"recursion_scheduler_level_vk_hash":"0x1111111111111111111111111111111111111111111111111111111111111111"}"#;
assert_eq!(ser_str, expected_str);
}
}
3 changes: 3 additions & 0 deletions core/lib/config/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ rand.workspace = true
secrecy.workspace = true
serde = { workspace = true, features = ["derive"] }

[dev-dependencies]
serde_json.workspace = true

[features]
default = []
observability_ext = ["zksync_vlog", "tracing"]
45 changes: 43 additions & 2 deletions core/lib/config/src/configs/genesis.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,14 @@ pub struct GenesisConfig {
pub l1_chain_id: L1ChainId,
pub sl_chain_id: Option<SLChainId>,
pub l2_chain_id: L2ChainId,
pub recursion_scheduler_level_vk_hash: H256,
// Note: `serde` isn't used with protobuf config. The same alias is implemented in
// `zksync_protobuf_config` manually.
// Rename is required to not introduce breaking changes in the API for existing clients.
#[serde(
alias = "recursion_scheduler_level_vk_hash",
rename(serialize = "recursion_scheduler_level_vk_hash")
)]
pub snark_wrapper_vk_hash: H256,
pub fee_account: Address,
pub dummy_verifier: bool,
pub l1_batch_commit_data_generator_mode: L1BatchCommitmentMode,
Expand All @@ -37,7 +44,7 @@ impl GenesisConfig {
GenesisConfig {
genesis_root_hash: Some(H256::repeat_byte(0x01)),
rollup_last_leaf_index: Some(26),
recursion_scheduler_level_vk_hash: H256::repeat_byte(0x02),
snark_wrapper_vk_hash: H256::repeat_byte(0x02),
fee_account: Default::default(),
genesis_commitment: Some(H256::repeat_byte(0x17)),
bootloader_hash: Default::default(),
Expand All @@ -54,3 +61,37 @@ impl GenesisConfig {
}
}
}

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

// This test checks that serde overrides (`rename`, `alias`) work for `snark_wrapper_vk_hash` field.
#[test]
fn genesis_serde_snark_wrapper_vk_hash() {
let genesis = GenesisConfig::for_tests();
let genesis_str = serde_json::to_string(&genesis).unwrap();

// Check that we use backward-compatible name in serialization.
// If you want to remove this check, make sure that all the potential clients are updated.
assert!(
genesis_str.contains("recursion_scheduler_level_vk_hash"),
"Serialization should use backward-compatible name"
);

let genesis2: GenesisConfig = serde_json::from_str(&genesis_str).unwrap();
assert_eq!(genesis, genesis2);

let genesis_json = r#"{
"snark_wrapper_vk_hash": "0x1111111111111111111111111111111111111111111111111111111111111111",
"l1_chain_id": 1,
"l2_chain_id": 1,
"fee_account": "0x1111111111111111111111111111111111111111",
"dummy_verifier": false,
"l1_batch_commit_data_generator_mode": "Rollup"
}"#;
serde_json::from_str::<GenesisConfig>(genesis_json).unwrap_or_else(|err| {
panic!("Failed to parse genesis config with a new name: {}", err)
});
}
}
2 changes: 1 addition & 1 deletion core/lib/config/src/testonly.rs
Original file line number Diff line number Diff line change
Expand Up @@ -728,7 +728,7 @@ impl Distribution<configs::GenesisConfig> for EncodeDist {
l1_chain_id: L1ChainId(self.sample(rng)),
sl_chain_id: None,
l2_chain_id: L2ChainId::default(),
recursion_scheduler_level_vk_hash: rng.gen(),
snark_wrapper_vk_hash: rng.gen(),
dummy_verifier: rng.gen(),
l1_batch_commit_data_generator_mode: match rng.gen_range(0..2) {
0 => L1BatchCommitmentMode::Rollup,
Expand Down

This file was deleted.

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.

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

This file was deleted.

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

Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
UPDATE protocol_patches SET recursion_scheduler_level_vk_hash = snark_wrapper_vk_hash WHERE recursion_scheduler_level_vk_hash = ''::bytea;
ALTER TABLE protocol_patches DROP COLUMN snark_wrapper_vk_hash;
ALTER TABLE protocol_patches ALTER COLUMN recursion_scheduler_level_vk_hash DROP DEFAULT;
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
ALTER TABLE protocol_patches ADD COLUMN snark_wrapper_vk_hash BYTEA NOT NULL DEFAULT ''::bytea;
ALTER TABLE protocol_patches ALTER COLUMN recursion_scheduler_level_vk_hash SET DEFAULT ''::bytea;
UPDATE protocol_patches SET snark_wrapper_vk_hash = recursion_scheduler_level_vk_hash;
-- Default was only needed to migrate old rows, we don't want this field to be forgotten by accident after migration.
ALTER TABLE protocol_patches ALTER COLUMN snark_wrapper_vk_hash DROP DEFAULT;

-- Old column should be removed once the migration is on the mainnet.
COMMENT ON COLUMN protocol_patches.recursion_scheduler_level_vk_hash IS 'This column is deprecated and will be removed in the future. Use snark_wrapper_vk_hash instead.';
6 changes: 2 additions & 4 deletions core/lib/dal/src/models/storage_protocol_version.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ pub struct StorageProtocolVersion {
pub minor: i32,
pub patch: i32,
pub timestamp: i64,
pub recursion_scheduler_level_vk_hash: Vec<u8>,
pub snark_wrapper_vk_hash: Vec<u8>,
pub bootloader_code_hash: Vec<u8>,
pub default_account_code_hash: Vec<u8>,
}
Expand All @@ -29,9 +29,7 @@ pub(crate) fn protocol_version_from_storage(
},
timestamp: storage_version.timestamp as u64,
l1_verifier_config: L1VerifierConfig {
recursion_scheduler_level_vk_hash: H256::from_slice(
&storage_version.recursion_scheduler_level_vk_hash,
),
snark_wrapper_vk_hash: H256::from_slice(&storage_version.snark_wrapper_vk_hash),
},
base_system_contracts_hashes: BaseSystemContractsHashes {
bootloader: H256::from_slice(&storage_version.bootloader_code_hash),
Expand Down
20 changes: 8 additions & 12 deletions core/lib/dal/src/protocol_versions_dal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,16 +71,14 @@ impl ProtocolVersionsDal<'_, '_> {
sqlx::query!(
r#"
INSERT INTO
protocol_patches (minor, patch, recursion_scheduler_level_vk_hash, created_at)
protocol_patches (minor, patch, snark_wrapper_vk_hash, created_at)
VALUES
($1, $2, $3, NOW())
ON CONFLICT DO NOTHING
"#,
version.minor as i32,
version.patch.0 as i32,
l1_verifier_config
.recursion_scheduler_level_vk_hash
.as_bytes(),
l1_verifier_config.snark_wrapper_vk_hash.as_bytes(),
)
.instrument("save_protocol_version#patch")
.with_arg("version", &version)
Expand Down Expand Up @@ -235,7 +233,7 @@ impl ProtocolVersionsDal<'_, '_> {
protocol_versions.bootloader_code_hash,
protocol_versions.default_account_code_hash,
protocol_patches.patch,
protocol_patches.recursion_scheduler_level_vk_hash
protocol_patches.snark_wrapper_vk_hash
FROM
protocol_versions
JOIN protocol_patches ON protocol_patches.minor = protocol_versions.id
Expand Down Expand Up @@ -268,7 +266,7 @@ impl ProtocolVersionsDal<'_, '_> {
let row = sqlx::query!(
r#"
SELECT
recursion_scheduler_level_vk_hash
snark_wrapper_vk_hash
FROM
protocol_patches
WHERE
Expand All @@ -282,16 +280,14 @@ impl ProtocolVersionsDal<'_, '_> {
.await
.unwrap()?;
Some(L1VerifierConfig {
recursion_scheduler_level_vk_hash: H256::from_slice(
&row.recursion_scheduler_level_vk_hash,
),
snark_wrapper_vk_hash: H256::from_slice(&row.snark_wrapper_vk_hash),
})
}

pub async fn get_patch_versions_for_vk(
&mut self,
minor_version: ProtocolVersionId,
recursion_scheduler_level_vk_hash: H256,
snark_wrapper_vk_hash: H256,
) -> DalResult<Vec<VersionPatch>> {
let rows = sqlx::query!(
r#"
Expand All @@ -301,12 +297,12 @@ impl ProtocolVersionsDal<'_, '_> {
protocol_patches
WHERE
minor = $1
AND recursion_scheduler_level_vk_hash = $2
AND snark_wrapper_vk_hash = $2
ORDER BY
patch DESC
"#,
minor_version as i32,
recursion_scheduler_level_vk_hash.as_bytes()
snark_wrapper_vk_hash.as_bytes()
)
.instrument("get_patch_versions_for_vk")
.fetch_all(self.storage)
Expand Down
2 changes: 1 addition & 1 deletion core/lib/env_config/src/genesis.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ impl FromEnv for GenesisConfig {
l1_chain_id: L1ChainId(network_config.network.chain_id().0),
sl_chain_id: Some(network_config.network.chain_id()),
l2_chain_id: network_config.zksync_network_id,
recursion_scheduler_level_vk_hash: contracts_config.snark_wrapper_vk_hash,
snark_wrapper_vk_hash: contracts_config.snark_wrapper_vk_hash,
fee_account: state_keeper
.fee_account_addr
.context("Fee account required for genesis")?,
Expand Down
17 changes: 10 additions & 7 deletions core/lib/protobuf_config/src/genesis.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,13 @@ impl ProtoRepr for proto::Genesis {
0.into(),
)
};
// Check either of fields, use old name as a fallback.
let snark_wrapper_vk_hash = match (&prover.snark_wrapper_vk_hash, &prover.recursion_scheduler_level_vk_hash) {
(Some(x), _) => parse_h256(x).context("snark_wrapper_vk_hash")?,
(_, Some(x)) => parse_h256(x).context("recursion_scheduler_level_vk_hash")?,
_ => anyhow::bail!("Either snark_wrapper_vk_hash or recursion_scheduler_level_vk_hash should be presented"),
};

Ok(Self::Type {
protocol_version: Some(protocol_version),
genesis_root_hash: Some(
Expand Down Expand Up @@ -75,9 +82,7 @@ impl ProtoRepr for proto::Genesis {
l2_chain_id: required(&self.l2_chain_id)
.and_then(|x| L2ChainId::try_from(*x).map_err(|a| anyhow::anyhow!(a)))
.context("l2_chain_id")?,
recursion_scheduler_level_vk_hash: required(&prover.recursion_scheduler_level_vk_hash)
.and_then(|x| parse_h256(x))
.context("recursion_scheduler_level_vk_hash")?,
snark_wrapper_vk_hash,
fee_account: required(&self.fee_account)
.and_then(|x| parse_h160(x))
.context("fee_account")?,
Expand All @@ -104,11 +109,9 @@ impl ProtoRepr for proto::Genesis {
l1_chain_id: Some(this.l1_chain_id.0),
l2_chain_id: Some(this.l2_chain_id.as_u64()),
prover: Some(proto::Prover {
recursion_scheduler_level_vk_hash: Some(format!(
"{:?}",
this.recursion_scheduler_level_vk_hash
)),
recursion_scheduler_level_vk_hash: None, // Deprecated field.
dummy_verifier: Some(this.dummy_verifier),
snark_wrapper_vk_hash: Some(format!("{:?}", this.snark_wrapper_vk_hash)),
}),
l1_batch_commit_data_generator_mode: Some(
proto::L1BatchCommitDataGeneratorMode::new(
Expand Down
Loading
Loading