From b51c530672bdb679fbe720e2192ca153c9d10acd Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Mon, 7 Oct 2024 20:37:39 +0300 Subject: [PATCH] refactor(eth): Brush up `eth_signer` crate (#3014) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## What ❔ Brushes up the `eth_signer` crate so that its API is easier to use (e.g., it doesn't require an async runtime). ## Why ❔ Improved DevEx. ## 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_supervisor fmt` and `zk_supervisor lint`. --- Cargo.lock | 5 +- core/lib/eth_client/src/types.rs | 12 +--- core/lib/eth_signer/Cargo.toml | 9 ++- core/lib/eth_signer/src/error.rs | 1 - core/lib/eth_signer/src/lib.rs | 3 +- core/lib/eth_signer/src/pk_signer.rs | 65 +++++++++++-------- core/lib/eth_signer/src/raw_ethereum_tx.rs | 6 +- core/lib/multivm/Cargo.toml | 1 - .../versions/vm_fast/tests/require_eip712.rs | 9 ++- .../vm_latest/tests/require_eip712.rs | 9 ++- core/tests/test_account/src/lib.rs | 6 +- prover/Cargo.lock | 3 +- 12 files changed, 66 insertions(+), 63 deletions(-) delete mode 100644 core/lib/eth_signer/src/error.rs diff --git a/Cargo.lock b/Cargo.lock index 0873faae904c..47d39f437c9c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -10151,8 +10151,8 @@ dependencies = [ "async-trait", "rlp", "thiserror", - "tokio", - "zksync_types", + "zksync_basic_types", + "zksync_crypto_primitives", ] [[package]] @@ -10473,7 +10473,6 @@ dependencies = [ "once_cell", "pretty_assertions", "thiserror", - "tokio", "tracing", "vise", "zk_evm 0.131.0-rc.2", diff --git a/core/lib/eth_client/src/types.rs b/core/lib/eth_client/src/types.rs index 59fb1cdeddcc..dd332351afb0 100644 --- a/core/lib/eth_client/src/types.rs +++ b/core/lib/eth_client/src/types.rs @@ -320,7 +320,7 @@ pub struct FailureInfo { #[cfg(test)] mod tests { - use zksync_eth_signer::{EthereumSigner, PrivateKeySigner, TransactionParameters}; + use zksync_eth_signer::{PrivateKeySigner, TransactionParameters}; use zksync_types::{ eth_sender::{EthTxBlobSidecarV1, SidecarBlobV1}, web3, K256PrivateKey, EIP_4844_TX_TYPE, H256, U256, U64, @@ -384,10 +384,7 @@ mod tests { .as_ref(), )]), }; - let raw_tx = signer - .sign_transaction(raw_transaction.clone()) - .await - .unwrap(); + let raw_tx = signer.sign_transaction(raw_transaction.clone()); let hash = web3::keccak256(&raw_tx).into(); // Transaction generated with https://github.com/inphi/blob-utils with @@ -493,10 +490,7 @@ mod tests { blob_versioned_hashes: Some(vec![versioned_hash_1, versioned_hash_2]), }; - let raw_tx = signer - .sign_transaction(raw_transaction.clone()) - .await - .unwrap(); + let raw_tx = signer.sign_transaction(raw_transaction); let hash = web3::keccak256(&raw_tx).into(); // Transaction generated with https://github.com/inphi/blob-utils with diff --git a/core/lib/eth_signer/Cargo.toml b/core/lib/eth_signer/Cargo.toml index f760134e09bb..92bb47824f3c 100644 --- a/core/lib/eth_signer/Cargo.toml +++ b/core/lib/eth_signer/Cargo.toml @@ -11,10 +11,9 @@ keywords.workspace = true categories.workspace = true [dependencies] -zksync_types.workspace = true +zksync_basic_types.workspace = true +zksync_crypto_primitives.workspace = true + +async-trait.workspace = true rlp.workspace = true thiserror.workspace = true -async-trait.workspace = true - -[dev-dependencies] -tokio = { workspace = true, features = ["full"] } diff --git a/core/lib/eth_signer/src/error.rs b/core/lib/eth_signer/src/error.rs deleted file mode 100644 index 8b137891791f..000000000000 --- a/core/lib/eth_signer/src/error.rs +++ /dev/null @@ -1 +0,0 @@ - diff --git a/core/lib/eth_signer/src/lib.rs b/core/lib/eth_signer/src/lib.rs index 3a92d47b062a..8b6025eb15da 100644 --- a/core/lib/eth_signer/src/lib.rs +++ b/core/lib/eth_signer/src/lib.rs @@ -1,5 +1,6 @@ use async_trait::async_trait; -use zksync_types::{Address, EIP712TypedStructure, Eip712Domain, PackedEthSignature}; +use zksync_basic_types::Address; +use zksync_crypto_primitives::{EIP712TypedStructure, Eip712Domain, PackedEthSignature}; pub use crate::{pk_signer::PrivateKeySigner, raw_ethereum_tx::TransactionParameters}; diff --git a/core/lib/eth_signer/src/pk_signer.rs b/core/lib/eth_signer/src/pk_signer.rs index 47b0e1109911..0f55425a0d58 100644 --- a/core/lib/eth_signer/src/pk_signer.rs +++ b/core/lib/eth_signer/src/pk_signer.rs @@ -1,5 +1,7 @@ -use zksync_types::{ - Address, EIP712TypedStructure, Eip712Domain, K256PrivateKey, PackedEthSignature, +use async_trait::async_trait; +use zksync_basic_types::Address; +use zksync_crypto_primitives::{ + EIP712TypedStructure, Eip712Domain, K256PrivateKey, PackedEthSignature, }; use crate::{ @@ -12,22 +14,20 @@ pub struct PrivateKeySigner { private_key: K256PrivateKey, } +// We define inherent methods duplicating `EthereumSigner` ones because they are sync and (other than `sign_typed_data`) infallible. impl PrivateKeySigner { pub fn new(private_key: K256PrivateKey) -> Self { Self { private_key } } -} -#[async_trait::async_trait] -impl EthereumSigner for PrivateKeySigner { - /// Get Ethereum address that matches the private key. - async fn get_address(&self) -> Result { - Ok(self.private_key.address()) + /// Gets an Ethereum address that matches this private key. + pub fn address(&self) -> Address { + self.private_key.address() } /// Signs typed struct using Ethereum private key by EIP-712 signature standard. /// Result of this function is the equivalent of RPC calling `eth_signTypedData`. - async fn sign_typed_data( + pub fn sign_typed_data( &self, domain: &Eip712Domain, typed_struct: &S, @@ -39,16 +39,11 @@ impl EthereumSigner for PrivateKeySigner { } /// Signs and returns the RLP-encoded transaction. - async fn sign_transaction( - &self, - raw_tx: TransactionParameters, - ) -> Result, SignerError> { + pub fn sign_transaction(&self, raw_tx: TransactionParameters) -> Vec { // According to the code in web3 // We should use `max_fee_per_gas` as `gas_price` if we use EIP1559 let gas_price = raw_tx.max_fee_per_gas; - let max_priority_fee_per_gas = raw_tx.max_priority_fee_per_gas; - let tx = Transaction { to: raw_tx.to, nonce: raw_tx.nonce, @@ -62,21 +57,42 @@ impl EthereumSigner for PrivateKeySigner { max_fee_per_blob_gas: raw_tx.max_fee_per_blob_gas, blob_versioned_hashes: raw_tx.blob_versioned_hashes, }; - let signed = tx.sign(&self.private_key, raw_tx.chain_id); - Ok(signed.raw_transaction.0) + signed.raw_transaction.0 + } +} + +#[async_trait] +impl EthereumSigner for PrivateKeySigner { + async fn get_address(&self) -> Result { + Ok(self.address()) + } + + async fn sign_typed_data( + &self, + domain: &Eip712Domain, + typed_struct: &S, + ) -> Result { + self.sign_typed_data(domain, typed_struct) + } + + async fn sign_transaction( + &self, + raw_tx: TransactionParameters, + ) -> Result, SignerError> { + Ok(self.sign_transaction(raw_tx)) } } #[cfg(test)] mod test { - use zksync_types::{K256PrivateKey, H160, H256, U256, U64}; + use zksync_basic_types::{H160, H256, U256, U64}; + use zksync_crypto_primitives::K256PrivateKey; - use super::PrivateKeySigner; - use crate::{raw_ethereum_tx::TransactionParameters, EthereumSigner}; + use super::*; - #[tokio::test] - async fn test_generating_signed_raw_transaction() { + #[test] + fn test_generating_signed_raw_transaction() { let private_key = K256PrivateKey::from_bytes(H256::from([5; 32])).unwrap(); let signer = PrivateKeySigner::new(private_key); let raw_transaction = TransactionParameters { @@ -94,10 +110,7 @@ mod test { blob_versioned_hashes: None, max_fee_per_blob_gas: None, }; - let raw_tx = signer - .sign_transaction(raw_transaction.clone()) - .await - .unwrap(); + let raw_tx = signer.sign_transaction(raw_transaction); assert_ne!(raw_tx.len(), 1); // pre-calculated signature with right algorithm implementation let precalculated_raw_tx: Vec = vec![ diff --git a/core/lib/eth_signer/src/raw_ethereum_tx.rs b/core/lib/eth_signer/src/raw_ethereum_tx.rs index 9479b5bd9d79..bea64305b47c 100644 --- a/core/lib/eth_signer/src/raw_ethereum_tx.rs +++ b/core/lib/eth_signer/src/raw_ethereum_tx.rs @@ -10,11 +10,11 @@ //! Link to @Deniallugo's PR to web3: https://github.com/tomusdrw/rust-web3/pull/630 use rlp::RlpStream; -use zksync_types::{ - ethabi::Address, +use zksync_basic_types::{ web3::{keccak256, AccessList, Signature, SignedTransaction}, - K256PrivateKey, H256, U256, U64, + Address, H256, U256, U64, }; +use zksync_crypto_primitives::K256PrivateKey; const LEGACY_TX_ID: u64 = 0; const ACCESSLISTS_TX_ID: u64 = 1; diff --git a/core/lib/multivm/Cargo.toml b/core/lib/multivm/Cargo.toml index 2c2cd4f044b9..7d604157d1a3 100644 --- a/core/lib/multivm/Cargo.toml +++ b/core/lib/multivm/Cargo.toml @@ -41,7 +41,6 @@ vise.workspace = true [dev-dependencies] assert_matches.workspace = true pretty_assertions.workspace = true -tokio = { workspace = true, features = ["time"] } zksync_test_account.workspace = true ethabi.workspace = true zksync_eth_signer.workspace = true diff --git a/core/lib/multivm/src/versions/vm_fast/tests/require_eip712.rs b/core/lib/multivm/src/versions/vm_fast/tests/require_eip712.rs index e119cea01143..88fe2dab5c95 100644 --- a/core/lib/multivm/src/versions/vm_fast/tests/require_eip712.rs +++ b/core/lib/multivm/src/versions/vm_fast/tests/require_eip712.rs @@ -1,5 +1,5 @@ use ethabi::Token; -use zksync_eth_signer::{EthereumSigner, TransactionParameters}; +use zksync_eth_signer::TransactionParameters; use zksync_system_constants::L2_BASE_TOKEN_ADDRESS; use zksync_types::{ fee::Fee, l2::L2Tx, transaction_request::TransactionRequest, @@ -38,8 +38,8 @@ impl VmTester<()> { /// This test deploys 'buggy' account abstraction code, and then tries accessing it both with legacy /// and EIP712 transactions. /// Currently we support both, but in the future, we should allow only EIP712 transactions to access the AA accounts. -#[tokio::test] -async fn test_require_eip712() { +#[test] +fn test_require_eip712() { // Use 3 accounts: // - `private_address` - EOA account, where we have the key // - `account_address` - AA account, where the contract is deployed @@ -104,7 +104,7 @@ async fn test_require_eip712() { blob_versioned_hashes: None, }; - let aa_tx = private_account.sign_legacy_tx(aa_raw_tx).await; + let aa_tx = private_account.sign_legacy_tx(aa_raw_tx); let (tx_request, hash) = TransactionRequest::from_bytes(&aa_tx, L2ChainId::from(270)).unwrap(); let mut l2_tx: L2Tx = L2Tx::from_request(tx_request, 10000).unwrap(); @@ -151,7 +151,6 @@ async fn test_require_eip712() { let signature = private_account .get_pk_signer() .sign_typed_data(&domain, &transaction_request) - .await .unwrap(); let encoded_tx = transaction_request.get_signed_bytes(&signature).unwrap(); diff --git a/core/lib/multivm/src/versions/vm_latest/tests/require_eip712.rs b/core/lib/multivm/src/versions/vm_latest/tests/require_eip712.rs index cdd71354c8de..a6dc7118005a 100644 --- a/core/lib/multivm/src/versions/vm_latest/tests/require_eip712.rs +++ b/core/lib/multivm/src/versions/vm_latest/tests/require_eip712.rs @@ -1,5 +1,5 @@ use ethabi::Token; -use zksync_eth_signer::{EthereumSigner, TransactionParameters}; +use zksync_eth_signer::TransactionParameters; use zksync_system_constants::L2_BASE_TOKEN_ADDRESS; use zksync_types::{ fee::Fee, l2::L2Tx, transaction_request::TransactionRequest, @@ -29,11 +29,11 @@ impl VmTester { } // TODO refactor this test it use too much internal details of the VM -#[tokio::test] +#[test] /// This test deploys 'buggy' account abstraction code, and then tries accessing it both with legacy /// and EIP712 transactions. /// Currently we support both, but in the future, we should allow only EIP712 transactions to access the AA accounts. -async fn test_require_eip712() { +fn test_require_eip712() { // Use 3 accounts: // - `private_address` - EOA account, where we have the key // - `account_address` - AA account, where the contract is deployed @@ -95,7 +95,7 @@ async fn test_require_eip712() { blob_versioned_hashes: None, }; - let aa_tx = private_account.sign_legacy_tx(aa_raw_tx).await; + let aa_tx = private_account.sign_legacy_tx(aa_raw_tx); let (tx_request, hash) = TransactionRequest::from_bytes(&aa_tx, L2ChainId::from(270)).unwrap(); let mut l2_tx: L2Tx = L2Tx::from_request(tx_request, 10000).unwrap(); @@ -142,7 +142,6 @@ async fn test_require_eip712() { let signature = private_account .get_pk_signer() .sign_typed_data(&domain, &transaction_request) - .await .unwrap(); let encoded_tx = transaction_request.get_signed_bytes(&signature).unwrap(); diff --git a/core/tests/test_account/src/lib.rs b/core/tests/test_account/src/lib.rs index d0c97abab729..999ea6eb6e09 100644 --- a/core/tests/test_account/src/lib.rs +++ b/core/tests/test_account/src/lib.rs @@ -2,7 +2,7 @@ use ethabi::Token; use zksync_contracts::{ deployer_contract, load_contract, test_contracts::LoadnextContractExecutionParams, }; -use zksync_eth_signer::{EthereumSigner, PrivateKeySigner, TransactionParameters}; +use zksync_eth_signer::{PrivateKeySigner, TransactionParameters}; use zksync_system_constants::{ CONTRACT_DEPLOYER_ADDRESS, DEFAULT_L2_TX_GAS_PER_PUBDATA_BYTE, REQUIRED_L1_TO_L2_GAS_PER_PUBDATA_BYTE, @@ -255,8 +255,8 @@ impl Account { PrivateKeySigner::new(self.private_key.clone()) } - pub async fn sign_legacy_tx(&self, tx: TransactionParameters) -> Vec { + pub fn sign_legacy_tx(&self, tx: TransactionParameters) -> Vec { let pk_signer = self.get_pk_signer(); - pk_signer.sign_transaction(tx).await.unwrap() + pk_signer.sign_transaction(tx) } } diff --git a/prover/Cargo.lock b/prover/Cargo.lock index 4ea83108a42e..bcca59763a81 100644 --- a/prover/Cargo.lock +++ b/prover/Cargo.lock @@ -7629,7 +7629,8 @@ dependencies = [ "async-trait", "rlp", "thiserror", - "zksync_types", + "zksync_basic_types", + "zksync_crypto_primitives", ] [[package]]