Skip to content

Commit

Permalink
refactor(eth): Brush up eth_signer crate (#3014)
Browse files Browse the repository at this point in the history
## 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`.
  • Loading branch information
slowli authored Oct 7, 2024
1 parent f0bfd2a commit b51c530
Show file tree
Hide file tree
Showing 12 changed files with 66 additions and 63 deletions.
5 changes: 2 additions & 3 deletions Cargo.lock

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

12 changes: 3 additions & 9 deletions core/lib/eth_client/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
9 changes: 4 additions & 5 deletions core/lib/eth_signer/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"] }
1 change: 0 additions & 1 deletion core/lib/eth_signer/src/error.rs

This file was deleted.

3 changes: 2 additions & 1 deletion core/lib/eth_signer/src/lib.rs
Original file line number Diff line number Diff line change
@@ -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};

Expand Down
65 changes: 39 additions & 26 deletions core/lib/eth_signer/src/pk_signer.rs
Original file line number Diff line number Diff line change
@@ -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::{
Expand All @@ -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<Address, SignerError> {
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<S: EIP712TypedStructure + Sync>(
pub fn sign_typed_data<S: EIP712TypedStructure + Sync>(
&self,
domain: &Eip712Domain,
typed_struct: &S,
Expand All @@ -39,16 +39,11 @@ impl EthereumSigner for PrivateKeySigner {
}

/// Signs and returns the RLP-encoded transaction.
async fn sign_transaction(
&self,
raw_tx: TransactionParameters,
) -> Result<Vec<u8>, SignerError> {
pub fn sign_transaction(&self, raw_tx: TransactionParameters) -> Vec<u8> {
// According to the code in web3 <https://docs.rs/web3/latest/src/web3/api/accounts.rs.html#86>
// 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,
Expand All @@ -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<Address, SignerError> {
Ok(self.address())
}

async fn sign_typed_data<S: EIP712TypedStructure + Sync>(
&self,
domain: &Eip712Domain,
typed_struct: &S,
) -> Result<PackedEthSignature, SignerError> {
self.sign_typed_data(domain, typed_struct)
}

async fn sign_transaction(
&self,
raw_tx: TransactionParameters,
) -> Result<Vec<u8>, 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 {
Expand All @@ -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<u8> = vec![
Expand Down
6 changes: 3 additions & 3 deletions core/lib/eth_signer/src/raw_ethereum_tx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
1 change: 0 additions & 1 deletion core/lib/multivm/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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
9 changes: 4 additions & 5 deletions core/lib/multivm/src/versions/vm_fast/tests/require_eip712.rs
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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();

Expand Down
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -29,11 +29,11 @@ impl VmTester<HistoryDisabled> {
}

// 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
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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();

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 @@ -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,
Expand Down Expand Up @@ -255,8 +255,8 @@ impl Account {
PrivateKeySigner::new(self.private_key.clone())
}

pub async fn sign_legacy_tx(&self, tx: TransactionParameters) -> Vec<u8> {
pub fn sign_legacy_tx(&self, tx: TransactionParameters) -> Vec<u8> {
let pk_signer = self.get_pk_signer();
pk_signer.sign_transaction(tx).await.unwrap()
pk_signer.sign_transaction(tx)
}
}
3 changes: 2 additions & 1 deletion prover/Cargo.lock

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

0 comments on commit b51c530

Please sign in to comment.