Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add cli options for server configuration #487

Merged
merged 6 commits into from
Dec 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion crates/cli/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,14 @@ pub struct Cli {
/// Disable auto and interval mining, and mine on demand instead.
#[arg(long, visible_alias = "no-mine", conflicts_with = "block_time")]
pub no_mining: bool,

/// The cors `allow_origin` header
#[arg(long, default_value = "*", help_heading = "Server options")]
pub allow_origin: String,

/// Disable CORS.
#[arg(long, default_missing_value = "true", num_args(0..=1), conflicts_with = "allow_origin", help_heading = "Server options")]
pub no_cors: Option<bool>,
}

#[derive(Debug, Subcommand, Clone)]
Expand Down Expand Up @@ -381,7 +389,9 @@ impl Cli {
None
})
.with_block_time(self.block_time)
.with_no_mining(self.no_mining);
.with_no_mining(self.no_mining)
.with_allow_origin(self.allow_origin)
.with_no_cors(self.no_cors);

if self.emulate_evm && self.dev_system_contracts != Some(SystemContractsOptions::Local) {
return Err(eyre::eyre!(
Expand Down
13 changes: 12 additions & 1 deletion crates/cli/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ use futures::{
FutureExt,
};
use jsonrpc_core::MetaIoHandler;
use jsonrpc_http_server::DomainsValidation;
use logging_middleware::LoggingMiddleware;
use std::fs::File;
use std::{env, net::SocketAddr, str::FromStr};
Expand All @@ -50,6 +51,8 @@ async fn build_json_http<
log_level_filter: LevelFilter,
node: InMemoryNode<S>,
enable_health_api: bool,
cors_allow_origin: String,
disable_cors: bool,
) -> tokio::task::JoinHandle<()> {
let (sender, recv) = oneshot::channel::<()>();

Expand All @@ -76,9 +79,15 @@ async fn build_json_http<
.build()
.unwrap();

let allow_origin = if disable_cors {
"null"
} else {
&cors_allow_origin
};
let mut builder = jsonrpc_http_server::ServerBuilder::new(io_handler)
.threads(1)
.event_loop_executor(runtime.handle().clone());
.event_loop_executor(runtime.handle().clone())
.cors(DomainsValidation::AllowOnly(vec![allow_origin.into()]));

if enable_health_api {
builder = builder.health_api(("/health", "web3_clientVersion"));
Expand Down Expand Up @@ -324,6 +333,8 @@ async fn main() -> anyhow::Result<()> {
log_level_filter,
node.clone(),
config.health_check_endpoint,
config.allow_origin.clone(),
config.no_cors,
)
}))
.await;
Expand Down
24 changes: 24 additions & 0 deletions crates/config/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,10 @@ pub struct TestNodeConfig {
pub max_transactions: usize,
/// Disable automatic sealing mode and use `BlockSealer::Noop` instead
pub no_mining: bool,
/// The cors `allow_origin` header
pub allow_origin: String,
/// Disable CORS if true
pub no_cors: bool,
}

impl Default for TestNodeConfig {
Expand Down Expand Up @@ -171,6 +175,10 @@ impl Default for TestNodeConfig {
no_mining: false,

max_transactions: 1000,

// Server configuration
allow_origin: "*".to_string(),
no_cors: false,
}
}
}
Expand Down Expand Up @@ -869,4 +877,20 @@ impl TestNodeConfig {
self.no_mining = no_mining;
self
}

// Set allow_origin CORS header
#[must_use]
pub fn with_allow_origin(mut self, allow_origin: String) -> Self {
self.allow_origin = allow_origin;
self
}

// Enable or disable CORS
#[must_use]
pub fn with_no_cors(mut self, no_cors: Option<bool>) -> Self {
if let Some(no_cors) = no_cors {
self.no_cors = no_cors;
}
self
}
}
2 changes: 1 addition & 1 deletion e2e-tests-rust/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,4 @@ mod provider;
mod utils;

pub use ext::{ReceiptExt, ZksyncWalletProviderExt};
pub use provider::{init_testing_provider, AnvilZKsyncApi, TestingProvider, DEFAULT_TX_VALUE};
pub use provider::{init_testing_provider, init_testing_provider_with_http_headers, AnvilZKsyncApi, TestingProvider, DEFAULT_TX_VALUE};
2 changes: 1 addition & 1 deletion e2e-tests-rust/src/provider/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@ mod anvil_zksync;
mod testing;

pub use anvil_zksync::AnvilZKsyncApi;
pub use testing::{init_testing_provider, TestingProvider, DEFAULT_TX_VALUE};
pub use testing::{init_testing_provider, init_testing_provider_with_http_headers, TestingProvider, DEFAULT_TX_VALUE};
100 changes: 84 additions & 16 deletions e2e-tests-rust/src/provider/testing.rs
Original file line number Diff line number Diff line change
@@ -1,21 +1,32 @@
use crate::utils::LockedPort;
use crate::utils::{LockedPort,get_node_binary_path};
use crate::ReceiptExt;
use alloy::network::primitives::{BlockTransactionsKind, HeaderResponse as _};
use alloy::network::{Network, ReceiptResponse as _, TransactionBuilder};
use alloy::primitives::{Address, U256};
use alloy::signers::local::LocalSigner;
use alloy::providers::{
PendingTransaction, PendingTransactionBuilder, PendingTransactionError, Provider, RootProvider,
SendableTx, WalletProvider,
};
use alloy::rpc::types::{Block, TransactionRequest};
use alloy::transports::http::{reqwest, Http};
use alloy::rpc::{
types::{Block, TransactionRequest},
client::RpcClient,
};
use alloy::transports::http::{
reqwest,
reqwest::{
header::HeaderMap,
Client,
},
Http
};
use alloy::transports::{RpcError, Transport, TransportErrorKind, TransportResult};
use alloy_zksync::network::header_response::HeaderResponse;
use alloy_zksync::network::receipt_response::ReceiptResponse;
use alloy_zksync::network::transaction_response::TransactionResponse;
use alloy_zksync::network::Zksync;
use alloy_zksync::node_bindings::EraTestNode;
use alloy_zksync::provider::{zksync_provider, ProviderBuilderExt};
use alloy_zksync::node_bindings::{EraTestNode,EraTestNodeError::NoKeysAvailable};
use alloy_zksync::provider::{zksync_provider, ProviderBuilderExt, layers::era_test_node::EraTestNodeLayer};
use alloy_zksync::wallet::ZksyncWallet;
use anyhow::Context as _;
use itertools::Itertools;
Expand Down Expand Up @@ -71,10 +82,7 @@ pub async fn init_testing_provider(
.with_recommended_fillers()
.on_era_test_node_with_wallet_and_config(|node| {
f(node
.path(
std::env::var("ANVIL_ZKSYNC_BINARY_PATH")
.unwrap_or("../target/release/anvil-zksync".to_string()),
)
.path(get_node_binary_path())
.port(locked_port.port))
});

Expand All @@ -93,6 +101,66 @@ pub async fn init_testing_provider(
})
}

// Init testing provider which sends specified HTTP headers e.g. for authentication
// Outside of `TestingProvider` to avoid specifying `P`
pub async fn init_testing_provider_with_http_headers(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we solve it by using Box<dyn FullZksyncProvider<T>> instead of template parameters?
Transport can also be erased by using Box<dyn Transport + Clone>.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Possibly, but we already have the default init_testing_provider function which I copied. Can be refactored though.

headers: HeaderMap,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, isn't it possible to express the logic of adding headers as a fn add_headers(EraTestNode) -> EraTestNode, so that this method is not needed at all? Then you will be able to use init_testing_provider(add_headers) or smth.

Note that function is composable, so we can wrap any number of layers here, e.g. init_testing_provider(|node| f1(f2(node)))

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, because it's not the setting on the node, it's the setting on provider.

Copy link
Contributor Author

@Romsters Romsters Dec 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could extend the provider code of on_era_test_node_with_wallet_and_config fn or add another one on the alloy-zksync side, but didn't want to do that as not sure if it is useful for any other cases.

f: impl FnOnce(EraTestNode) -> EraTestNode,
) -> anyhow::Result<
TestingProvider<impl FullZksyncProvider<Http<reqwest::Client>>, Http<reqwest::Client>>,
> {
use alloy::signers::Signer;

let locked_port = LockedPort::acquire_unused().await?;
let node_layer = EraTestNodeLayer::from(
f(
EraTestNode::new()
.path(get_node_binary_path())
.port(locked_port.port)
)
);

let client_with_headers = Client::builder().default_headers(headers).build()?;
let rpc_url = node_layer.endpoint_url();
let http = Http::with_client(client_with_headers, rpc_url);
let rpc_client = RpcClient::new(http, true);

let default_keys = node_layer.instance().keys().to_vec();
let (default_key, remaining_keys) = default_keys
.split_first()
.ok_or(NoKeysAvailable)?;

let default_signer = LocalSigner::from(default_key.clone())
.with_chain_id(Some(node_layer.instance().chain_id()));
let mut wallet = ZksyncWallet::from(default_signer);

for key in remaining_keys {
let signer = LocalSigner::from(key.clone());
wallet.register_signer(signer)
}

let provider = zksync_provider()
.with_recommended_fillers()
.wallet(wallet)
.layer(node_layer)
.on_client(rpc_client);

// Grab default rich accounts right after init. Note that subsequent calls to this method
// might return different value as wallet's signers are dynamic and can be changed by the user.
let rich_accounts = provider.signer_addresses().collect::<Vec<_>>();
// Wait for anvil-zksync to get up and be able to respond
// Ignore error response (should not fail here if provider is used with intentionally wrong origin for testing purposes)
let _ = provider.get_chain_id().await;
// Explicitly unlock the port to showcase why we waited above
drop(locked_port);

Ok(TestingProvider {
inner: provider,
rich_accounts,
_pd: Default::default(),
})
}

impl<P, T> TestingProvider<P, T>
where
P: FullZksyncProvider<T>,
Expand Down Expand Up @@ -383,13 +451,7 @@ where
self
}

/// Builder-pattern method for setting the chain id.
pub fn with_chain_id(mut self, id: u64) -> Self {
self.inner = self.inner.with_chain_id(id);
self
}

/// Builder-pattern method for setting the recipient.
/// Builder-pattern method for setting the receiver.
pub fn with_to(mut self, to: Address) -> Self {
self.inner = self.inner.with_to(to);
self
Expand All @@ -401,6 +463,12 @@ where
self
}

/// Builder-pattern method for setting the chain id.
pub fn with_chain_id(mut self, id: u64) -> Self {
self.inner = self.inner.with_chain_id(id);
self
}

/// Submits transaction to the node.
///
/// This does not wait for the transaction to be confirmed, but returns a [`PendingTransactionFinalizable`]
Expand Down
5 changes: 5 additions & 0 deletions e2e-tests-rust/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,3 +71,8 @@ impl Drop for LockedPort {
.unwrap();
}
}

pub fn get_node_binary_path() -> String {
std::env::var("ANVIL_ZKSYNC_BINARY_PATH")
.unwrap_or("../target/release/anvil-zksync".to_string())
}
37 changes: 36 additions & 1 deletion e2e-tests-rust/tests/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,13 @@ use alloy::network::ReceiptResponse;
use alloy::providers::ext::AnvilApi;
use alloy::providers::Provider;
use anvil_zksync_e2e_tests::{
init_testing_provider, AnvilZKsyncApi, ReceiptExt, ZksyncWalletProviderExt, DEFAULT_TX_VALUE,
init_testing_provider, init_testing_provider_with_http_headers, AnvilZKsyncApi, ReceiptExt, ZksyncWalletProviderExt, DEFAULT_TX_VALUE,
};
use alloy::{
primitives::U256,
signers::local::PrivateKeySigner,
};
use alloy::transports::http::reqwest::header::{HeaderMap, HeaderValue, ORIGIN};
use std::convert::identity;
use std::time::Duration;

Expand Down Expand Up @@ -363,3 +364,37 @@ async fn set_chain_id() -> anyhow::Result<()> {

Ok(())
}

#[tokio::test]
async fn cli_no_cors() -> anyhow::Result<()> {
let mut headers = HeaderMap::new();
headers.insert(ORIGIN, HeaderValue::from_static("http://some.origin"));

// Verify all origins are allowed by default
let provider = init_testing_provider_with_http_headers(headers.clone(), identity).await?;
provider.get_chain_id().await?;

// Verify no origins are allowed with --no-cors
let provider_with_no_cors = init_testing_provider_with_http_headers(headers.clone(), |node| node.arg("--no-cors=true")).await?;
let error_resp = provider_with_no_cors.get_chain_id().await.unwrap_err();
assert_eq!(error_resp.to_string().contains("Origin of the request is not whitelisted"), true);

Ok(())
}

#[tokio::test]
async fn cli_allow_origin() -> anyhow::Result<()> {
let mut headers = HeaderMap::new();
headers.insert(ORIGIN, HeaderValue::from_static("http://some.origin"));

// Verify allowed origin can make requests
let provider_with_allowed_origin = init_testing_provider_with_http_headers(headers.clone(), |node| node.arg("--allow-origin=http://some.origin")).await?;
provider_with_allowed_origin.get_chain_id().await?;

// Verify different origin is not allowed
let provider_with_not_allowed_origin = init_testing_provider_with_http_headers(headers.clone(), |node| node.arg("--allow-origin=http://other.origin")).await?;
let error_resp = provider_with_not_allowed_origin.get_chain_id().await.unwrap_err();
assert_eq!(error_resp.to_string().contains("Origin of the request is not whitelisted"), true);

Ok(())
}
Loading