Skip to content

Commit

Permalink
backwrds compatible version (except batches)
Browse files Browse the repository at this point in the history
  • Loading branch information
dvush committed Jan 22, 2021
1 parent 9bb75eb commit eb72c45
Show file tree
Hide file tree
Showing 16 changed files with 143 additions and 62 deletions.
2 changes: 1 addition & 1 deletion core/bin/zksync_api/src/signature_checker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
4 changes: 2 additions & 2 deletions core/lib/state/benches/criterion/ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
2 changes: 1 addition & 1 deletion core/lib/types/src/gas_counter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 10 additions & 1 deletion core/lib/types/src/operations/change_pubkey_op.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,16 @@ impl ChangePubKeyOp {
}

pub fn get_eth_witness(&self) -> Vec<u8> {
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(&eth_signature.serialize_packed());
bytes
} else {
Vec::new()
}
}

pub fn from_public_data(bytes: &[u8]) -> Result<Self, anyhow::Error> {
Expand Down
4 changes: 2 additions & 2 deletions core/lib/types/src/tests/hardcoded.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()),
Expand Down
125 changes: 97 additions & 28 deletions core/lib/types/src/tx/change_pubkey.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<PackedEthSignature>,
/// 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<ChangePubKeyEthAuthData>,
/// 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<u32>,
pub valid_until: Option<u32>,
#[serde(skip)]
cached_signer: VerifiedSignatureCache,
}

impl ChangePubKey {
Expand All @@ -152,14 +156,16 @@ impl ChangePubKey {
eth_signature: Option<PackedEthSignature>,
) -> 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,
Expand All @@ -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),
Expand Down Expand Up @@ -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
}

Expand All @@ -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());
Expand All @@ -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<Vec<u8>, 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
}
}

Expand All @@ -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()
}
}
}
2 changes: 1 addition & 1 deletion core/lib/types/src/tx/primitives/batch_sign_data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand Down
11 changes: 6 additions & 5 deletions core/lib/types/src/tx/primitives/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
11 changes: 6 additions & 5 deletions core/lib/types/src/tx/transfer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
11 changes: 6 additions & 5 deletions core/lib/types/src/tx/withdraw.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
2 changes: 1 addition & 1 deletion core/lib/types/src/tx/zksync_tx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
6 changes: 3 additions & 3 deletions core/tests/test_account/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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!(
Expand Down
8 changes: 4 additions & 4 deletions etc/env/dev.env.example
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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

Expand Down
Binary file not shown.
2 changes: 1 addition & 1 deletion sdk/zksync-rs/src/signer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ impl<S: EthereumSigner> Signer<S> {
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(),
Expand Down
4 changes: 2 additions & 2 deletions sdk/zksync-rs/tests/unit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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!(&eth_signature[..], expected_eth_signature.as_slice());
Expand Down

0 comments on commit eb72c45

Please sign in to comment.