diff --git a/core/bin/zksync_api/src/signature_checker.rs b/core/bin/zksync_api/src/signature_checker.rs index 0a07e04c1c..cc31a8dd42 100644 --- a/core/bin/zksync_api/src/signature_checker.rs +++ b/core/bin/zksync_api/src/signature_checker.rs @@ -115,7 +115,7 @@ async fn verify_eth_signature_single_tx( let start = Instant::now(); // Check if the tx is a `ChangePubKey` operation without an Ethereum signature. if let ZkSyncTx::ChangePubKey(change_pk) = &tx.tx { - if change_pk.eth_auth_data.is_onchain() { + if change_pk.is_onchain() { // Check that user is allowed to perform this operation. let is_authorized = eth_checker .is_new_pubkey_hash_authorized( diff --git a/core/lib/state/benches/criterion/ops.rs b/core/lib/state/benches/criterion/ops.rs index 3995b2a1e3..9e6a9a6f9e 100644 --- a/core/lib/state/benches/criterion/ops.rs +++ b/core/lib/state/benches/criterion/ops.rs @@ -249,10 +249,10 @@ fn apply_change_pubkey_op(b: &mut Bencher<'_>) { .expect("Failed to construct ChangePubKey signed message."); let eth_signature = PackedEthSignature::sign(eth_private_key, &sign_bytes).expect("Signing failed"); - ChangePubKeyEthAuthData::ECDSA(ChangePubKeyECDSAData { + Some(ChangePubKeyEthAuthData::ECDSA(ChangePubKeyECDSAData { eth_signature, batch_hash: H256::zero(), - }) + })) }; let change_pubkey_tx = ZkSyncTx::ChangePubKey(Box::new(change_pubkey)); diff --git a/core/lib/types/src/gas_counter.rs b/core/lib/types/src/gas_counter.rs index 99da27178f..109d270f7a 100644 --- a/core/lib/types/src/gas_counter.rs +++ b/core/lib/types/src/gas_counter.rs @@ -43,7 +43,7 @@ impl CommitCost { ZkSyncOp::Deposit(_) => Self::DEPOSIT_COST, ZkSyncOp::ChangePubKeyOffchain(change_pubkey) => { // TODO: determine correct cost of this tx - if change_pubkey.tx.eth_auth_data.is_ecdsa() { + if change_pubkey.tx.is_ecdsa() { Self::CHANGE_PUBKEY_COST_OFFCHAIN } else { Self::CHANGE_PUBKEY_COST_ONCHAIN diff --git a/core/lib/types/src/operations/change_pubkey_op.rs b/core/lib/types/src/operations/change_pubkey_op.rs index d1896de009..899d656f2e 100644 --- a/core/lib/types/src/operations/change_pubkey_op.rs +++ b/core/lib/types/src/operations/change_pubkey_op.rs @@ -36,7 +36,16 @@ impl ChangePubKeyOp { } pub fn get_eth_witness(&self) -> Vec { - self.tx.eth_auth_data.get_eth_witness() + if let Some(eth_auth_data) = &self.tx.eth_auth_data { + eth_auth_data.get_eth_witness() + } else if let Some(eth_signature) = &self.tx.eth_signature { + let mut bytes = Vec::new(); + bytes.push(0x02); + bytes.extend_from_slice(ð_signature.serialize_packed()); + bytes + } else { + Vec::new() + } } pub fn from_public_data(bytes: &[u8]) -> Result { diff --git a/core/lib/types/src/tests/hardcoded.rs b/core/lib/types/src/tests/hardcoded.rs index 673768ee66..4cd61055c2 100644 --- a/core/lib/types/src/tests/hardcoded.rs +++ b/core/lib/types/src/tests/hardcoded.rs @@ -238,12 +238,12 @@ pub mod operations_test { ChangePubKeyOp::from_public_data(&hex::decode(CHANGE_PUBKEY_PUBLIC_DATA).unwrap()) .unwrap(); - change_pubkey.tx.eth_auth_data = ChangePubKeyEthAuthData::ECDSA(ChangePubKeyECDSAData { + change_pubkey.tx.eth_auth_data = Some(ChangePubKeyEthAuthData::ECDSA(ChangePubKeyECDSAData { eth_signature: PackedEthSignature::deserialize_packed( &hex::decode("2a0a81e257a2f5d6ed4f07b81dbda09f107bd026dbda09f107bd026f5d6ed4f02a0a81e257a2f5d6ed4f07b81dbda09f107bd026dbda09f107bd026f5d6ed4f0d4").unwrap(), ).expect("Hex signature deserialization"), batch_hash: H256::from([0xCEu8; 32]) - }); + })); assert_eq!( hex::encode(change_pubkey.get_eth_witness()), diff --git a/core/lib/types/src/tx/change_pubkey.rs b/core/lib/types/src/tx/change_pubkey.rs index 0dda77759b..4e056d77c1 100644 --- a/core/lib/types/src/tx/change_pubkey.rs +++ b/core/lib/types/src/tx/change_pubkey.rs @@ -120,14 +120,18 @@ pub struct ChangePubKey { /// fields can't be changed by an attacker. #[serde(default)] pub signature: TxSignature, + /// Transaction Ethereum signature. It may be `None` if `ChangePubKey` operation is authorized + /// onchain, otherwise the message must be signed by the Ethereum private key corresponding + /// to the account address. + pub eth_signature: Option, /// Data needed to check if Ethereum address authorized ChangePubKey operation - pub eth_auth_data: ChangePubKeyEthAuthData, - #[serde(skip)] - cached_signer: VerifiedSignatureCache, + pub eth_auth_data: Option, /// Unix epoch format of the time when the transaction is valid /// This fields must be Option<...> because of backward compatibility with first version of ZkSync pub valid_from: Option, pub valid_until: Option, + #[serde(skip)] + cached_signer: VerifiedSignatureCache, } impl ChangePubKey { @@ -152,14 +156,16 @@ impl ChangePubKey { eth_signature: Option, ) -> Self { // TODO: support CREATE2 - let eth_auth_data = eth_signature - .map(|eth_signature| { - ChangePubKeyEthAuthData::ECDSA(ChangePubKeyECDSAData { - eth_signature, - batch_hash: H256::zero(), + let eth_auth_data = Some( + eth_signature + .map(|eth_signature| { + ChangePubKeyEthAuthData::ECDSA(ChangePubKeyECDSAData { + eth_signature, + batch_hash: H256::zero(), + }) }) - }) - .unwrap_or(ChangePubKeyEthAuthData::Onchain); + .unwrap_or(ChangePubKeyEthAuthData::Onchain), + ); let mut tx = Self { account_id, @@ -169,6 +175,7 @@ impl ChangePubKey { fee, nonce, signature: signature.clone().unwrap_or_default(), + eth_signature: None, eth_auth_data, cached_signer: VerifiedSignatureCache::NotCached, valid_from: Some(valid_from), @@ -235,11 +242,12 @@ impl ChangePubKey { out.extend_from_slice(&self.fee_token.to_be_bytes()); out.extend_from_slice(&pack_fee_amount(&self.fee)); out.extend_from_slice(&self.nonce.to_be_bytes()); - - // We use 64 bytes for timestamps in the signed message - out.extend_from_slice(&u64::from(self.valid_from.unwrap_or(0)).to_be_bytes()); - out.extend_from_slice(&u64::from(self.valid_until.unwrap_or(u32::MAX)).to_be_bytes()); - + if let Some(valid_from) = &self.valid_from { + out.extend_from_slice(&u64::from(*valid_from).to_be_bytes()); + } + if let Some(valid_until) = &self.valid_until { + out.extend_from_slice(&u64::from(*valid_until).to_be_bytes()); + } out } @@ -259,7 +267,7 @@ impl ChangePubKey { eth_signed_msg.extend_from_slice(&self.nonce.to_be_bytes()); eth_signed_msg.extend_from_slice(&self.account_id.to_be_bytes()); // In case this transaction is not part of a batch, we simply append zeros. - if let ChangePubKeyEthAuthData::ECDSA(ChangePubKeyECDSAData { batch_hash, .. }) = + if let Some(ChangePubKeyEthAuthData::ECDSA(ChangePubKeyECDSAData { batch_hash, .. })) = self.eth_auth_data { eth_signed_msg.extend_from_slice(batch_hash.as_bytes()); @@ -275,20 +283,65 @@ impl ChangePubKey { Ok(eth_signed_msg) } + /// Provides an old message to be signed with the Ethereum private key. + pub fn get_old_eth_signed_data(&self) -> Result, anyhow::Error> { + // Fee data is not included into ETH signature input, since it would require + // to either have more chunks in pubdata (if fee amount is unpacked), unpack + // fee on contract (if fee amount is packed), or display non human-readable + // amount in message (if fee amount is packed and is not unpacked on contract). + // Either of these options is either non user-friendly or increase cost of + // operation. Instead, fee data is signed via zkSync signature, which is essentially + // free. This signature will be verified in the circuit. + + const CHANGE_PUBKEY_SIGNATURE_LEN: usize = 152; + let mut eth_signed_msg = Vec::with_capacity(CHANGE_PUBKEY_SIGNATURE_LEN); + eth_signed_msg.extend_from_slice(b"Register zkSync pubkey:\n\n"); + eth_signed_msg.extend_from_slice( + format!( + "{pubkey}\n\ + nonce: 0x{nonce}\n\ + account id: 0x{account_id}\ + \n\n", + pubkey = hex::encode(&self.new_pk_hash.data).to_ascii_lowercase(), + nonce = hex::encode(&self.nonce.to_be_bytes()).to_ascii_lowercase(), + account_id = hex::encode(&self.account_id.to_be_bytes()).to_ascii_lowercase() + ) + .as_bytes(), + ); + eth_signed_msg.extend_from_slice(b"Only sign this message for a trusted client!"); + ensure!( + eth_signed_msg.len() == CHANGE_PUBKEY_SIGNATURE_LEN, + "Change pubkey signed message len is too big: {}, expected: {}", + eth_signed_msg.len(), + CHANGE_PUBKEY_SIGNATURE_LEN + ); + Ok(eth_signed_msg) + } + pub fn is_eth_auth_data_valid(&self) -> bool { - match &self.eth_auth_data { - ChangePubKeyEthAuthData::Onchain => true, // Should query Ethereum to check it - ChangePubKeyEthAuthData::ECDSA(ChangePubKeyECDSAData { eth_signature, .. }) => { - let recovered_address = self - .get_eth_signed_data() - .ok() - .and_then(|msg| eth_signature.signature_recover_signer(&msg).ok()); - recovered_address == Some(self.account) - } - ChangePubKeyEthAuthData::CREATE2(create2_data) => { - let create2_address = create2_data.get_address(&self.new_pk_hash); - create2_address == self.account + if let Some(eth_auth_data) = &self.eth_auth_data { + match eth_auth_data { + ChangePubKeyEthAuthData::Onchain => true, // Should query Ethereum to check it + ChangePubKeyEthAuthData::ECDSA(ChangePubKeyECDSAData { eth_signature, .. }) => { + let recovered_address = self + .get_eth_signed_data() + .ok() + .and_then(|msg| eth_signature.signature_recover_signer(&msg).ok()); + recovered_address == Some(self.account) + } + ChangePubKeyEthAuthData::CREATE2(create2_data) => { + let create2_address = create2_data.get_address(&self.new_pk_hash); + create2_address == self.account + } } + } else if let Some(old_eth_signature) = &self.eth_signature { + let recovered_address = self + .get_old_eth_signed_data() + .ok() + .and_then(|msg| old_eth_signature.signature_recover_signer(&msg).ok()); + recovered_address == Some(self.account) + } else { + true } } @@ -307,4 +360,20 @@ impl ChangePubKey { && is_fee_amount_packable(&self.fee) && self.valid_from.unwrap_or(0) <= self.valid_until.unwrap_or(u32::MAX) } + + pub fn is_ecdsa(&self) -> bool { + if let Some(auth_data) = &self.eth_auth_data { + auth_data.is_ecdsa() + } else { + self.eth_signature.is_some() + } + } + + pub fn is_onchain(&self) -> bool { + if let Some(auth_data) = &self.eth_auth_data { + auth_data.is_onchain() + } else { + self.eth_signature.is_none() + } + } } diff --git a/core/lib/types/src/tx/primitives/batch_sign_data.rs b/core/lib/types/src/tx/primitives/batch_sign_data.rs index 5e154788c5..2673a4a8f7 100644 --- a/core/lib/types/src/tx/primitives/batch_sign_data.rs +++ b/core/lib/types/src/tx/primitives/batch_sign_data.rs @@ -37,7 +37,7 @@ impl BatchSignData { ); // We only prefix the batch message in case `change_pub_key` has Ethereum signature. let change_pub_key_message = change_pub_key - .filter(|tx| tx.eth_auth_data.is_ecdsa()) + .filter(|tx| tx.is_ecdsa()) .map(|tx| tx.get_eth_signed_data()) .transpose()?; // The hash is already present in `change_pub_key_message`. diff --git a/core/lib/types/src/tx/primitives/tests.rs b/core/lib/types/src/tx/primitives/tests.rs index 4f2d1efe15..b57746ba47 100644 --- a/core/lib/types/src/tx/primitives/tests.rs +++ b/core/lib/types/src/tx/primitives/tests.rs @@ -103,11 +103,12 @@ fn test_batch_message() -> Result<()> { ZkSyncTx::ChangePubKey(tx) => tx, _ => panic!("ChangePubKey is supposed to be the last element in Vec of test transactions"), }; - change_pub_key.as_mut().eth_auth_data = ChangePubKeyEthAuthData::ECDSA(ChangePubKeyECDSAData { - batch_hash: H256::from_slice(batch_hash.as_slice()), - eth_signature: PackedEthSignature::deserialize_packed(&vec![0u8; 65]) - .expect("Creation of fake eth signature fail"), - }); + change_pub_key.as_mut().eth_auth_data = + Some(ChangePubKeyEthAuthData::ECDSA(ChangePubKeyECDSAData { + batch_hash: H256::from_slice(batch_hash.as_slice()), + eth_signature: PackedEthSignature::deserialize_packed(&vec![0u8; 65]) + .expect("Creation of fake eth signature fail"), + })); let change_pub_key_message = change_pub_key.as_ref().get_eth_signed_data()?; assert!(change_pub_key_message.ends_with(batch_hash.as_slice())); // Shouldn't fail. diff --git a/core/lib/types/src/tx/transfer.rs b/core/lib/types/src/tx/transfer.rs index 88ec873cf4..12e65af27b 100644 --- a/core/lib/types/src/tx/transfer.rs +++ b/core/lib/types/src/tx/transfer.rs @@ -132,11 +132,12 @@ impl Transfer { out.extend_from_slice(&pack_token_amount(&self.amount)); out.extend_from_slice(&pack_fee_amount(&self.fee)); out.extend_from_slice(&self.nonce.to_be_bytes()); - - // We use 64 bytes for timestamps in the signed message - out.extend_from_slice(&u64::from(self.valid_from.unwrap_or(0)).to_be_bytes()); - out.extend_from_slice(&u64::from(self.valid_until.unwrap_or(u32::MAX)).to_be_bytes()); - + if let Some(valid_from) = &self.valid_from { + out.extend_from_slice(&u64::from(*valid_from).to_be_bytes()); + } + if let Some(valid_until) = &self.valid_from { + out.extend_from_slice(&u64::from(*valid_until).to_be_bytes()); + } out } diff --git a/core/lib/types/src/tx/withdraw.rs b/core/lib/types/src/tx/withdraw.rs index 8b119f39bf..b43c0b413d 100644 --- a/core/lib/types/src/tx/withdraw.rs +++ b/core/lib/types/src/tx/withdraw.rs @@ -137,11 +137,12 @@ impl Withdraw { out.extend_from_slice(&self.amount.to_u128().unwrap().to_be_bytes()); out.extend_from_slice(&pack_fee_amount(&self.fee)); out.extend_from_slice(&self.nonce.to_be_bytes()); - - // We use 64 bytes for timestamps in the signed message - out.extend_from_slice(&u64::from(self.valid_from.unwrap_or(0)).to_be_bytes()); - out.extend_from_slice(&u64::from(self.valid_until.unwrap_or(u32::MAX)).to_be_bytes()); - + if let Some(valid_from) = &self.valid_from { + out.extend_from_slice(&u64::from(*valid_from).to_be_bytes()); + } + if let Some(valid_until) = &self.valid_from { + out.extend_from_slice(&u64::from(*valid_until).to_be_bytes()); + } out } diff --git a/core/lib/types/src/tx/zksync_tx.rs b/core/lib/types/src/tx/zksync_tx.rs index 9b34b4a300..b9f605f10c 100644 --- a/core/lib/types/src/tx/zksync_tx.rs +++ b/core/lib/types/src/tx/zksync_tx.rs @@ -212,7 +212,7 @@ impl ZkSyncTx { )), ZkSyncTx::ChangePubKey(change_pubkey) => Some(( TxFeeTypes::ChangePubKey { - onchain_pubkey_auth: !change_pubkey.eth_auth_data.is_ecdsa(), + onchain_pubkey_auth: !change_pubkey.is_ecdsa(), }, TokenLike::Id(change_pubkey.fee_token), change_pubkey.fee.clone(), diff --git a/core/tests/test_account/src/lib.rs b/core/tests/test_account/src/lib.rs index b18aaa47c9..a32857f052 100644 --- a/core/tests/test_account/src/lib.rs +++ b/core/tests/test_account/src/lib.rs @@ -278,17 +278,17 @@ impl ZkSyncAccount { ) .expect("Can't sign ChangePubKey operation"); change_pubkey.eth_auth_data = if auth_onchain { - ChangePubKeyEthAuthData::Onchain + Some(ChangePubKeyEthAuthData::Onchain) } else { let sign_bytes = change_pubkey .get_eth_signed_data() .expect("Failed to construct change pubkey signed message."); let eth_signature = PackedEthSignature::sign(&self.eth_private_key, &sign_bytes) .expect("Signature should succeed"); - ChangePubKeyEthAuthData::ECDSA(ChangePubKeyECDSAData { + Some(ChangePubKeyEthAuthData::ECDSA(ChangePubKeyECDSAData { eth_signature, batch_hash: H256::zero(), - }) + })) }; assert!( diff --git a/etc/env/dev.env.example b/etc/env/dev.env.example index 97db80c1ed..95f4259db3 100755 --- a/etc/env/dev.env.example +++ b/etc/env/dev.env.example @@ -109,8 +109,8 @@ KEY_DIR=keys/contracts-4 SUPPORTED_BLOCK_CHUNKS_SIZES=10,32,72,156,322,654 SUPPORTED_BLOCK_CHUNKS_SIZES_SETUP_POWERS=21,22,23,24,25,26 -SUPPORTED_AGGREGATED_PROOF_SIZES=1,4,8,18,36 -SUPPORTED_AGGREGATED_PROOF_SIZES_SETUP_POWERS=22,23,24,25,26 +SUPPORTED_AGGREGATED_PROOF_SIZES=1,4,8,18 +SUPPORTED_AGGREGATED_PROOF_SIZES_SETUP_POWERS=22,23,24,25 # Since withdraw is an expensive operation, we have to limit amount of # withdrawals in one block to not exceed the gas limit in prover. @@ -119,8 +119,8 @@ SUPPORTED_AGGREGATED_PROOF_SIZES_SETUP_POWERS=22,23,24,25,26 # the remaining withdrawals will go to the next block. MAX_NUMBER_OF_WITHDRAWALS_PER_BLOCK=10 -BLOCK_CHUNK_SIZES=6,30 -AGGREGATED_PROOF_SIZES=1,5 +BLOCK_CHUNK_SIZES=10,32 +AGGREGATED_PROOF_SIZES=1,4 ACCOUNT_TREE_DEPTH=32 BALANCE_TREE_DEPTH=11 diff --git a/keys/packed/verify-keys-contracts-4-account-32_-balance-11.tar.gz b/keys/packed/verify-keys-contracts-4-account-32_-balance-11.tar.gz index 65e5e25313..bff504b4f9 100644 Binary files a/keys/packed/verify-keys-contracts-4-account-32_-balance-11.tar.gz and b/keys/packed/verify-keys-contracts-4-account-32_-balance-11.tar.gz differ diff --git a/sdk/zksync-rs/src/signer.rs b/sdk/zksync-rs/src/signer.rs index 521e192a2d..901bd5db84 100644 --- a/sdk/zksync-rs/src/signer.rs +++ b/sdk/zksync-rs/src/signer.rs @@ -126,7 +126,7 @@ impl Signer { batch_hash: H256::zero(), }) }; - change_pubkey.eth_auth_data = eth_auth_data; + change_pubkey.eth_auth_data = Some(eth_auth_data); assert!( change_pubkey.is_eth_auth_data_valid(), diff --git a/sdk/zksync-rs/tests/unit.rs b/sdk/zksync-rs/tests/unit.rs index ca289416ab..7d5cff9e34 100644 --- a/sdk/zksync-rs/tests/unit.rs +++ b/sdk/zksync-rs/tests/unit.rs @@ -333,10 +333,10 @@ mod signatures_with_vectors { if let Some(expected_eth_signature) = outputs.eth_signature { let eth_signature = match &change_pub_key.eth_auth_data { - ChangePubKeyEthAuthData::ECDSA(ChangePubKeyECDSAData { + Some(ChangePubKeyEthAuthData::ECDSA(ChangePubKeyECDSAData { eth_signature, .. - }) => eth_signature.serialize_packed(), + })) => eth_signature.serialize_packed(), _ => panic!("No ChangePubKey ethereum siganture"), }; assert_eq!(ð_signature[..], expected_eth_signature.as_slice());