Skip to content

Commit

Permalink
Implement more review suggestions
Browse files Browse the repository at this point in the history
  • Loading branch information
StanislavBreadless committed Dec 6, 2021
1 parent afbe557 commit 47ba69f
Show file tree
Hide file tree
Showing 5 changed files with 71 additions and 73 deletions.
4 changes: 2 additions & 2 deletions core/bin/zksync_api/src/api_server/rest/v02/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ async fn submit_tx(
) -> ApiResult<TxHashSerializeWrapper> {
let tx_hash = data
.tx_sender
.submit_tx(body.tx, body.signature, false)
.submit_tx(body.tx, body.signature, None)
.await
.map_err(Error::from);

Expand All @@ -165,7 +165,7 @@ async fn submit_batch(
) -> ApiResult<SubmitBatchResponse> {
let response = data
.tx_sender
.submit_txs_batch(body.txs, body.signature, false)
.submit_txs_batch(body.txs, body.signature, None)
.await
.map_err(Error::from);
response.into()
Expand Down
6 changes: 1 addition & 5 deletions core/bin/zksync_api/src/api_server/rpc_server/mod.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// Built-in uses
use std::{collections::HashSet, time::Instant};
use std::time::Instant;

// External uses
use futures::{
Expand Down Expand Up @@ -53,8 +53,6 @@ pub struct RpcApp {
pub confirmations_for_eth_event: u64,

tx_sender: TxSender,

pub subsidized_ips: HashSet<String>,
}

impl RpcApp {
Expand Down Expand Up @@ -89,8 +87,6 @@ impl RpcApp {
confirmations_for_eth_event,

tx_sender,

subsidized_ips: config.subsidized_ips.clone().into_iter().collect(),
}
}

Expand Down
50 changes: 10 additions & 40 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 @@ -3,7 +3,6 @@ use std::time::Instant;
// External uses
use bigdecimal::BigDecimal;
use jsonrpc_core::{Error, Result};
use num::{rational::Ratio, BigUint};
// Workspace uses
use zksync_api_types::{
v02::{
Expand All @@ -29,14 +28,6 @@ use crate::{
use super::{types::*, RpcApp};

impl RpcApp {
fn should_subsidie_ip(&self, meta: Option<RequestMetadata>) -> bool {
if let Some(meta) = meta {
self.subsidized_ips.contains(&meta.ip)
} else {
false
}
}

pub async fn _impl_account_info(self, address: Address) -> Result<AccountInfoResp> {
let start = Instant::now();

Expand Down Expand Up @@ -137,11 +128,10 @@ impl RpcApp {
meta: Option<RequestMetadata>,
) -> Result<TxHash> {
let start = Instant::now();
let should_subsidie_cpk = self.should_subsidie_ip(meta);

let result = self
.tx_sender
.submit_tx_with_separate_fp(*tx, *signature, fast_processing, should_subsidie_cpk)
.submit_tx_with_separate_fp(*tx, *signature, fast_processing, meta)
.await
.map_err(Error::from);
metrics::histogram!("api.rpc.tx_submit", start.elapsed());
Expand All @@ -156,11 +146,9 @@ impl RpcApp {
) -> Result<Vec<TxHash>> {
let start = Instant::now();

let should_subsidize_ip = self.should_subsidie_ip(meta);

let result: Result<Vec<TxHash>> = self
.tx_sender
.submit_txs_batch(txs, eth_signatures, should_subsidize_ip)
.submit_txs_batch(txs, eth_signatures, meta)
.await
.map_err(Error::from)
.map(|response| {
Expand Down Expand Up @@ -260,16 +248,17 @@ impl RpcApp {
let result =
Self::ticker_request(ticker.clone(), tx_type.into(), address, token.clone()).await?;

let should_subsidie_cpk = self
.should_subsidie_cpk(
let should_subsidize_cpk = self
.tx_sender
.should_subsidize_cpk(
&result.normal_fee.total_fee,
&result.subsidized_fee.total_fee,
&result.subsidy_size_usd,
meta,
)
.await?;

let fee = if should_subsidie_cpk {
let fee = if should_subsidize_cpk {
result.subsidized_fee
} else {
result.normal_fee
Expand All @@ -279,26 +268,6 @@ impl RpcApp {
Ok(fee)
}

async fn should_subsidie_cpk(
&self,
normal_fee: &BigUint,
subsidized_fee: &BigUint,
subsidy_size_usd: &Ratio<BigUint>,
meta: Option<RequestMetadata>,
) -> Result<bool> {
let should_subsidize_ip = self.should_subsidie_ip(meta);

let result = should_subsidize_ip
&& subsidized_fee < normal_fee
&& self
.tx_sender
.can_subsidize(subsidy_size_usd.clone())
.await
.map_err(SubmitError::Internal)?;

Ok(result)
}

pub async fn _impl_get_txs_batch_fee_in_wei(
self,
tx_types: Vec<ApiTxFeeTypes>,
Expand Down Expand Up @@ -330,16 +299,17 @@ impl RpcApp {

let result = Self::ticker_batch_fee_request(ticker, transactions, token.clone()).await?;

let should_subsidie_cpk = self
.should_subsidie_cpk(
let should_subsidize_cpk = self
.tx_sender
.should_subsidize_cpk(
&result.normal_fee.total_fee,
&result.subsidized_fee.total_fee,
&result.subsidy_size_usd,
meta,
)
.await?;

let fee = if should_subsidie_cpk {
let fee = if should_subsidize_cpk {
result.subsidized_fee
} else {
result.normal_fee
Expand Down
82 changes: 57 additions & 25 deletions core/bin/zksync_api/src/api_server/tx_sender.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ use crate::{
};
use zksync_config::configs::api::CommonApiConfig;

use super::rpc_server::types::RequestMetadata;

const VALIDNESS_INTERVAL_MINUTES: i64 = 40;

#[derive(Clone)]
Expand All @@ -75,6 +77,7 @@ pub struct TxSender {

pub current_subsidy_type: String,
pub max_subsidy_usd: Ratio<BigUint>,
pub subsidized_ips: HashSet<String>,
}

#[derive(Debug, Error)]
Expand Down Expand Up @@ -181,6 +184,7 @@ impl TxSender {
max_number_of_authors_per_batch,
current_subsidy_type: config.subsidy_name.clone(),
max_subsidy_usd: config.max_subsidy_usd(),
subsidized_ips: config.subsidized_ips.clone().into_iter().collect(),
}
}

Expand Down Expand Up @@ -365,7 +369,7 @@ impl TxSender {
mut tx: ZkSyncTx,
signature: TxEthSignatureVariant,
fast_processing: Option<bool>,
should_subsidie_cpk: bool,
meta: Option<RequestMetadata>,
) -> Result<TxHash, SubmitError> {
let fast_processing = fast_processing.unwrap_or(false);
if fast_processing && !tx.is_withdraw() {
Expand All @@ -383,7 +387,7 @@ impl TxSender {
withdraw.fast = fast_processing;
}

self.submit_tx(tx, signature, should_subsidie_cpk).await
self.submit_tx(tx, signature, meta).await
}

pub async fn can_subsidize(
Expand All @@ -408,6 +412,29 @@ impl TxSender {
Ok(result)
}

pub async fn should_subsidize_cpk(
&self,
normal_fee: &BigUint,
subsidized_fee: &BigUint,
subsidy_size_usd: &Ratio<BigUint>,
meta: Option<RequestMetadata>,
) -> Result<bool, SubmitError> {
let should_subsidize_ip = if let Some(meta) = meta {
self.subsidized_ips.contains(&meta.ip)
} else {
false
};

let result = should_subsidize_ip
&& subsidized_fee < normal_fee
&& self
.can_subsidize(subsidy_size_usd.clone())
.await
.map_err(SubmitError::Internal)?;

Ok(result)
}

pub async fn store_subsidy_data(
&self,
hash: TxHash,
Expand Down Expand Up @@ -455,7 +482,7 @@ impl TxSender {
&self,
tx: ZkSyncTx,
signature: TxEthSignatureVariant,
should_subsidie_cpk: bool,
meta: Option<RequestMetadata>,
) -> Result<TxHash, SubmitError> {
if tx.is_close() {
return Err(SubmitError::AccountCloseDisabled);
Expand Down Expand Up @@ -502,13 +529,14 @@ impl TxSender {
Self::ticker_request(ticker_request_sender, tx_type, address, token.clone())
.await?;

let required_fee_data = if should_subsidie_cpk
&& required_fee_data.subsidized_fee.total_fee
< required_fee_data.normal_fee.total_fee
&& self
.can_subsidize(required_fee_data.subsidy_size_usd.clone())
.await
.map_err(SubmitError::Internal)?
let required_fee_data = if self
.should_subsidize_cpk(
&required_fee_data.normal_fee.total_fee,
&required_fee_data.subsidized_fee.total_fee,
&required_fee_data.subsidy_size_usd,
meta,
)
.await?
{
fee_data_for_subsidy = Some(required_fee_data.clone());
required_fee_data.subsidized_fee
Expand Down Expand Up @@ -592,7 +620,7 @@ impl TxSender {
&self,
txs: Vec<TxWithSignature>,
eth_signatures: Option<EthBatchSignatures>,
should_subsidie_cpk: bool,
meta: Option<RequestMetadata>,
) -> Result<SubmitBatchResponse, SubmitError> {
// Bring the received signatures into a vector for simplified work.
let eth_signatures = EthBatchSignatures::api_arg_to_vec(eth_signatures);
Expand Down Expand Up @@ -685,12 +713,14 @@ impl TxSender {
)
.await?;

let required_fee = if should_subsidie_cpk
&& self
.can_subsidize(batch_token_fee.subsidy_size_usd.clone())
.await
.map_err(SubmitError::Internal)?
&& batch_token_fee.subsidized_fee.total_fee < batch_token_fee.normal_fee.total_fee
let required_fee = if self
.should_subsidize_cpk(
&batch_token_fee.normal_fee.total_fee,
&batch_token_fee.subsidized_fee.total_fee,
&batch_token_fee.subsidy_size_usd,
meta,
)
.await?
{
fee_data_for_subsidy = Some(batch_token_fee.clone());
batch_token_fee.subsidized_fee.total_fee
Expand Down Expand Up @@ -720,12 +750,14 @@ impl TxSender {
)
.await?;

let required_fee = if should_subsidie_cpk
&& required_eth_fee.subsidized_fee.total_fee < required_eth_fee.normal_fee.total_fee
&& self
.can_subsidize(required_eth_fee.subsidy_size_usd.clone())
.await
.map_err(SubmitError::Internal)?
let required_fee = if self
.should_subsidize_cpk(
&required_eth_fee.normal_fee.total_fee,
&required_eth_fee.subsidized_fee.total_fee,
&required_eth_fee.subsidy_size_usd,
meta,
)
.await?
{
fee_data_for_subsidy = Some(required_eth_fee.clone());
required_eth_fee.subsidized_fee.total_fee
Expand Down Expand Up @@ -845,9 +877,9 @@ impl TxSender {
token_fees_ids[0]
} else {
// When there are more than token to pay the fee with,
// We get the price of the batch in ETH and then convert it to USD.
// we get the price of the batch in ETH and then convert it to USD.
// Since the `subsidies` table contains the token_id field and the only fee which is fetched from the fee_ticker is
// in ETH, then we can consider ETH as the token_id of the subsidy. Even though formally this is not the case.
// in ETH, then we can consider ETH as the token_id of the subsidy. Even though formally this may not be the case.
TokenId(0)
};

Expand Down
2 changes: 1 addition & 1 deletion core/lib/storage/src/misc/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ impl<'a, 'c> MiscSchema<'a, 'c> {
.total
.unwrap_or_else(|| BigDecimal::from(0));

metrics::histogram!("sql.token.load_tokens_asc", start.elapsed());
metrics::histogram!("sql.token.get_total_used_subsidy_for_type", start.elapsed());
Ok(sum)
}
}

0 comments on commit 47ba69f

Please sign in to comment.