Skip to content

Commit

Permalink
Merge #1605
Browse files Browse the repository at this point in the history
1605: Require ETH signature for swap orders r=Deniallugo a=ly0va



Co-authored-by: Lyova Potyomkin <[email protected]>
  • Loading branch information
bors-matterlabs-dev[bot] and ly0va authored May 24, 2021
2 parents b6043ed + 8884177 commit b791a26
Show file tree
Hide file tree
Showing 21 changed files with 2,223 additions and 1,590 deletions.
3 changes: 1 addition & 2 deletions contracts/scripts/upgrade-testnet.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,7 @@ async function main() {
});
parser.addArgument('--initArgs', {
required: false,
help:
'Upgrade function parameters comma-separated, RLP serialized in hex (Governance,Verifier,ZkSync): 0xaa..aa,0xbb..bb,0xcc..c or zero by default.',
help: 'Upgrade function parameters comma-separated, RLP serialized in hex (Governance,Verifier,ZkSync): 0xaa..aa,0xbb..bb,0xcc..c or zero by default.',
defaultValue: '0x,0x,0x'
});
parser.addArgument('--cancelPreviousUpgrade', {
Expand Down
46 changes: 33 additions & 13 deletions core/bin/zksync_api/src/api_server/rest/v1/transactions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,10 @@ pub use zksync_api_client::rest::v1::{
use zksync_storage::{
chain::operations_ext::records::TxReceiptResponse, QueryResult, StorageProcessor,
};
use zksync_types::{tx::TxHash, BatchFee, BlockNumber, Fee, SignedZkSyncTx};
use zksync_types::{
tx::{TxEthSignatureVariant, TxHash},
BatchFee, BlockNumber, Fee, SignedZkSyncTx,
};
// Local uses
use super::{Error as ApiError, JsonResult, Pagination, PaginationQuery};
use crate::api_server::rpc_server::types::TxWithSignature;
Expand Down Expand Up @@ -257,7 +260,7 @@ async fn submit_tx_batch(
.into_iter()
.map(|tx| TxWithSignature {
tx,
signature: None,
signature: TxEthSignatureVariant::Single(None),
})
.collect();

Expand Down Expand Up @@ -643,12 +646,17 @@ mod tests {
// Submit correct transaction.
let tx = TestServerConfig::gen_zk_txs(1_00).txs[0].0.clone();
let expected_tx_hash = tx.hash();
assert_eq!(client.submit_tx(tx, None, None).await?, expected_tx_hash);
assert_eq!(
client
.submit_tx(tx, TxEthSignatureVariant::Single(None), None)
.await?,
expected_tx_hash
);

// Submit transaction without fee.
let tx = TestServerConfig::gen_zk_txs(0).txs[0].0.clone();
assert!(client
.submit_tx(tx, None, None)
.submit_tx(tx, TxEthSignatureVariant::Single(None), None)
.await
.unwrap_err()
.to_string()
Expand Down Expand Up @@ -718,7 +726,7 @@ mod tests {
assert!(client
.submit_tx(
transfer_bad_token.clone(),
eth_sig.map(TxEthSignature::EthereumSignature),
TxEthSignatureVariant::Single(eth_sig.map(TxEthSignature::EthereumSignature)),
None
)
.await
Expand Down Expand Up @@ -832,7 +840,7 @@ mod tests {
client
.submit_tx(
transfer1.clone(),
eth_sig1.map(TxEthSignature::EthereumSignature),
TxEthSignatureVariant::Single(eth_sig1.map(TxEthSignature::EthereumSignature)),
None,
)
.await
Expand All @@ -857,7 +865,7 @@ mod tests {
client
.submit_tx(
transfer2.clone(),
eth_sig2.map(TxEthSignature::EthereumSignature),
TxEthSignatureVariant::Single(eth_sig2.map(TxEthSignature::EthereumSignature)),
None,
)
.await
Expand Down Expand Up @@ -895,7 +903,9 @@ mod tests {
client
.submit_tx(
ZkSyncTx::Transfer(Box::new(tx.clone())),
eth_sig.clone().map(TxEthSignature::EthereumSignature),
TxEthSignatureVariant::Single(
eth_sig.clone().map(TxEthSignature::EthereumSignature),
),
Some(true),
)
.await
Expand All @@ -904,15 +914,19 @@ mod tests {
client
.submit_tx(
ZkSyncTx::Transfer(Box::new(tx.clone())),
eth_sig.clone().map(TxEthSignature::EthereumSignature),
TxEthSignatureVariant::Single(
eth_sig.clone().map(TxEthSignature::EthereumSignature),
),
Some(false),
)
.await?;
// Submit without fast-processing flag.
client
.submit_tx(
ZkSyncTx::Transfer(Box::new(tx)),
eth_sig.clone().map(TxEthSignature::EthereumSignature),
TxEthSignatureVariant::Single(
eth_sig.clone().map(TxEthSignature::EthereumSignature),
),
None,
)
.await?;
Expand All @@ -931,23 +945,29 @@ mod tests {
client
.submit_tx(
ZkSyncTx::Withdraw(Box::new(tx.clone())),
eth_sig.clone().map(TxEthSignature::EthereumSignature),
TxEthSignatureVariant::Single(
eth_sig.clone().map(TxEthSignature::EthereumSignature),
),
Some(true),
)
.await?;
// Submit with the disabled fast-processing.
client
.submit_tx(
ZkSyncTx::Withdraw(Box::new(tx.clone())),
eth_sig.clone().map(TxEthSignature::EthereumSignature),
TxEthSignatureVariant::Single(
eth_sig.clone().map(TxEthSignature::EthereumSignature),
),
Some(false),
)
.await?;
// Submit without fast-processing flag.
client
.submit_tx(
ZkSyncTx::Withdraw(Box::new(tx)),
eth_sig.clone().map(TxEthSignature::EthereumSignature),
TxEthSignatureVariant::Single(
eth_sig.clone().map(TxEthSignature::EthereumSignature),
),
None,
)
.await?;
Expand Down
4 changes: 2 additions & 2 deletions core/bin/zksync_api/src/api_server/rpc_server/rpc_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use jsonrpc_core::{Error, Result};
// Workspace uses
use zksync_api_client::rest::v1::accounts::NFT;
use zksync_types::{
tx::{EthBatchSignatures, TxEthSignature, TxHash},
tx::{EthBatchSignatures, TxEthSignatureVariant, TxHash},
Address, BatchFee, Fee, Token, TokenId, TokenLike, TxFeeTypes, ZkSyncTx,
};

Expand Down Expand Up @@ -96,7 +96,7 @@ impl RpcApp {
pub async fn _impl_tx_submit(
self,
tx: Box<ZkSyncTx>,
signature: Box<Option<TxEthSignature>>,
signature: Box<TxEthSignatureVariant>,
fast_processing: Option<bool>,
) -> Result<TxHash> {
let start = Instant::now();
Expand Down
6 changes: 3 additions & 3 deletions core/bin/zksync_api/src/api_server/rpc_server/rpc_trait.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use jsonrpc_derive::rpc;
use zksync_api_client::rest::v1::accounts::NFT;
use zksync_crypto::params::ZKSYNC_VERSION;
use zksync_types::{
tx::{EthBatchSignatures, TxEthSignature, TxHash},
tx::{EthBatchSignatures, TxEthSignatureVariant, TxHash},
Address, BatchFee, Fee, Token, TokenId, TokenLike, TxFeeTypes, ZkSyncTx,
};

Expand All @@ -33,7 +33,7 @@ pub trait Rpc {
fn tx_submit(
&self,
tx: Box<ZkSyncTx>,
signature: Box<Option<TxEthSignature>>,
signature: Box<TxEthSignatureVariant>,
fast_processing: Option<bool>,
) -> FutureResp<TxHash>;

Expand Down Expand Up @@ -115,7 +115,7 @@ impl Rpc for RpcApp {
fn tx_submit(
&self,
tx: Box<ZkSyncTx>,
signature: Box<Option<TxEthSignature>>,
signature: Box<TxEthSignatureVariant>,
fast_processing: Option<bool>,
) -> FutureResp<TxHash> {
let handle = self.runtime_handle.clone();
Expand Down
5 changes: 3 additions & 2 deletions core/bin/zksync_api/src/api_server/rpc_server/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use serde::{Deserialize, Serialize};
// Workspace uses
use zksync_storage::StorageProcessor;
use zksync_types::{
tx::TxEthSignature, Account, AccountId, Address, Nonce, PriorityOp, PubKeyHash, TokenId,
tx::TxEthSignatureVariant, Account, AccountId, Address, Nonce, PriorityOp, PubKeyHash, TokenId,
ZkSyncPriorityOp, ZkSyncTx,
};
use zksync_utils::{BigUintSerdeAsRadix10Str, BigUintSerdeWrapper};
Expand All @@ -25,7 +25,8 @@ use crate::{
#[serde(rename_all = "camelCase")]
pub struct TxWithSignature {
pub tx: ZkSyncTx,
pub signature: Option<TxEthSignature>,
#[serde(default)]
pub signature: TxEthSignatureVariant,
}

#[derive(Debug, Clone, Serialize, Deserialize, Default)]
Expand Down
105 changes: 89 additions & 16 deletions core/bin/zksync_api/src/api_server/tx_sender.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ use zksync_config::ZkSyncConfig;
use zksync_storage::{chain::account::records::EthAccountType, ConnectionPool};
use zksync_types::{
tx::{
EthBatchSignData, EthBatchSignatures, EthSignData, SignedZkSyncTx, TxEthSignature, TxHash,
EthBatchSignData, EthBatchSignatures, EthSignData, Order, SignedZkSyncTx, TxEthSignature,
TxEthSignatureVariant, TxHash,
},
AccountId, Address, BatchFee, Fee, Token, TokenId, TokenLike, TxFeeTypes, ZkSyncTx, H160,
};
Expand All @@ -37,7 +38,9 @@ use crate::{
api_server::rpc_server::types::TxWithSignature,
core_api_client::CoreApiClient,
fee_ticker::{ResponseBatchFee, ResponseFee, TickerRequest, TokenPriceRequestType},
signature_checker::{BatchRequest, RequestData, TxRequest, VerifiedTx, VerifySignatureRequest},
signature_checker::{
BatchRequest, OrderRequest, RequestData, TxRequest, VerifiedTx, VerifySignatureRequest,
},
tx_error::TxAddError,
utils::{block_details_cache::BlockDetailsCache, token_db_cache::TokenDBCache},
};
Expand Down Expand Up @@ -237,37 +240,87 @@ impl TxSender {
/// the corresponding address.
async fn get_tx_sender(&self, tx: &ZkSyncTx) -> Result<Address, anyhow::Error> {
match tx {
ZkSyncTx::ForcedExit(tx) => self
.pool
.access_storage()
.await?
.chain()
.account_schema()
.account_address_by_id(tx.initiator_account_id)
.await?
.ok_or_else(|| anyhow::anyhow!("Forced Exit account is not found in db")),
ZkSyncTx::ForcedExit(tx) => self.get_address_by_id(tx.initiator_account_id).await,
_ => Ok(tx.account()),
}
}

async fn get_address_by_id(&self, id: AccountId) -> Result<Address, anyhow::Error> {
self.pool
.access_storage()
.await?
.chain()
.account_schema()
.account_address_by_id(id)
.await?
.ok_or_else(|| anyhow::anyhow!("Order signer account id not found in db"))
}

async fn get_tx_sender_type(&self, tx: &ZkSyncTx) -> Result<EthAccountType, SubmitError> {
self.get_sender_type(tx.account_id().or(Err(SubmitError::AccountCloseDisabled))?)
.await
}

async fn get_sender_type(&self, id: AccountId) -> Result<EthAccountType, SubmitError> {
Ok(self
.pool
.access_storage()
.await
.map_err(|_| SubmitError::TxAdd(TxAddError::DbError))?
.chain()
.account_schema()
.account_type_by_id(tx.account_id().or(Err(SubmitError::AccountCloseDisabled))?)
.account_type_by_id(id)
.await
.map_err(|_| SubmitError::TxAdd(TxAddError::DbError))?
.unwrap_or(EthAccountType::Owned))
}

async fn verify_order_eth_signature(
&self,
order: &Order,
signature: Option<TxEthSignature>,
) -> Result<(), SubmitError> {
let signer_type = self.get_sender_type(order.account_id).await?;
if matches!(signer_type, EthAccountType::CREATE2) {
return if signature.is_some() {
Err(SubmitError::IncorrectTx(
"Eth signature from CREATE2 account not expected".to_string(),
))
} else {
Ok(())
};
}
let signature = signature.ok_or(SubmitError::TxAdd(TxAddError::MissingEthSignature))?;
let signer = self
.get_address_by_id(order.account_id)
.await
.or(Err(SubmitError::TxAdd(TxAddError::DbError)))?;

let token_sell = self.token_info_from_id(order.token_sell).await?;
let token_buy = self.token_info_from_id(order.token_buy).await?;
let message = order
.get_ethereum_sign_message(&token_sell.symbol, &token_buy.symbol, token_sell.decimals)
.into_bytes();
let eth_sign_data = EthSignData { signature, message };
let (sender, receiever) = oneshot::channel();

let request = VerifySignatureRequest {
data: RequestData::Order(OrderRequest {
order: Box::new(order.clone()),
sign_data: eth_sign_data,
sender: signer,
}),
response: sender,
};

send_verify_request_and_recv(request, self.sign_verify_requests.clone(), receiever).await?;
Ok(())
}

pub async fn submit_tx(
&self,
mut tx: ZkSyncTx,
signature: Option<TxEthSignature>,
signature: TxEthSignatureVariant,
fast_processing: Option<bool>,
) -> Result<TxHash, SubmitError> {
if tx.is_close() {
Expand Down Expand Up @@ -374,13 +427,21 @@ impl TxSender {
tx_sender,
token.clone(),
self.get_tx_sender_type(&tx).await?,
signature.clone(),
signature.tx_signature().clone(),
msg_to_sign,
sign_verify_channel,
)
.await?
.unwrap_tx();

if let ZkSyncTx::Swap(tx) = &tx {
let signatures = signature.orders_signatures();
self.verify_order_eth_signature(&tx.orders.0, signatures.0.clone())
.await?;
self.verify_order_eth_signature(&tx.orders.1, signatures.1.clone())
.await?;
}

let tx_hash = verified_tx.tx.hash();
// Send verified transactions to the mempool.
self.core_api_client
Expand Down Expand Up @@ -560,6 +621,16 @@ impl TxSender {
}
}

for tx in txs.iter() {
if let ZkSyncTx::Swap(swap) = &tx.tx {
let signatures = tx.signature.orders_signatures();
self.verify_order_eth_signature(&swap.orders.0, signatures.0.clone())
.await?;
self.verify_order_eth_signature(&swap.orders.1, signatures.1.clone())
.await?;
}
}

let mut verified_txs = Vec::with_capacity(txs.len());
let mut verified_signatures = Vec::new();

Expand Down Expand Up @@ -955,18 +1026,20 @@ async fn verify_txs_batch_signature(
let eth_sign_data = if let Some(message) = message {
match sender_type {
EthAccountType::CREATE2 => {
if tx.signature.is_some() {
if tx.signature.exists() {
return Err(SubmitError::IncorrectTx(
"Eth signature from CREATE2 account not expected".to_string(),
));
}
None
}
EthAccountType::Owned => {
if batch_sign_data.is_none() && tx.signature.is_none() {
if batch_sign_data.is_none() && !tx.signature.exists() {
return Err(SubmitError::TxAdd(TxAddError::MissingEthSignature));
}
tx.signature
.tx_signature()
.clone()
.map(|signature| EthSignData { signature, message })
}
}
Expand Down
Loading

0 comments on commit b791a26

Please sign in to comment.