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: allow unset blocklist clients #589

Merged
merged 7 commits into from
Oct 1, 2024
Merged
Show file tree
Hide file tree
Changes from 6 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: 4 additions & 8 deletions signer/src/blocklist_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,19 +41,15 @@ impl BlocklistChecker for BlocklistClient {

impl BlocklistClient {
/// Construct a new [`BlocklistClient`]
pub fn new(ctx: &impl Context) -> Self {
let base_url = format!(
"http://{}:{}",
ctx.config().blocklist_client.host,
ctx.config().blocklist_client.port
);
pub fn new(ctx: &impl Context) -> Option<Self> {
let config = ctx.config().blocklist_client.as_ref()?;

let config = Configuration {
base_path: base_url.clone(),
base_path: config.endpoint.to_string(),
..Default::default()
};

BlocklistClient { config }
Some(BlocklistClient { config })
}

#[cfg(test)]
Expand Down
12 changes: 9 additions & 3 deletions signer/src/config/default.toml
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,15 @@
# !! ==============================================================================
# !! Blocklist Client Configuration
# !! ==============================================================================
[blocklist_client]
host = "127.0.0.1"
port = 8080
# You may specify a blocklist client url. If one is not specified, then
# deposit or withdrawal requests are always accepted.
#
# Format: "http(s)://<host>:<port>"
# Default: <none>
# Required: false
# Environment: SIGNER_BLOCKLIST_CLIENT__ENDPOINT
# [blocklist_client]
# endpoint = "http://127.0.0.1:8080"

# !! ==============================================================================
# !! Emily API Configuration
Expand Down
64 changes: 30 additions & 34 deletions signer/src/config/mod.rs
Original file line number Diff line number Diff line change
@@ -1,17 +1,21 @@
//! Configuration management for the signer
use config::{Config, ConfigError, Environment, File};
use config::Config;
use config::ConfigError;
use config::Environment;
use config::File;
use libp2p::Multiaddr;
use serde::Deserialize;
use stacks_common::types::chainstate::StacksAddress;
use std::path::Path;
use url::Url;

use crate::config::error::SignerConfigError;
use crate::config::serialization::p2p_multiaddr_deserializer_vec;
use crate::config::serialization::parse_stacks_address;
use crate::config::serialization::private_key_deserializer;
use crate::config::serialization::url_deserializer_single;
use crate::config::serialization::url_deserializer_vec;
use crate::keys::PrivateKey;
use error::SignerConfigError;
use serialization::{
p2p_multiaddr_deserializer_vec, parse_stacks_address, private_key_deserializer,
url_deserializer_single, url_deserializer_vec,
};

mod error;
mod serialization;
Expand Down Expand Up @@ -76,7 +80,7 @@ impl NetworkKind {
#[derive(Deserialize, Clone, Debug)]
pub struct Settings {
/// Blocklist client specific config
pub blocklist_client: BlocklistClientConfig,
pub blocklist_client: Option<BlocklistClientConfig>,
/// Electrum notifier specific config
pub block_notifier: BlockNotifierConfig,
/// Signer-specific configuration
Expand Down Expand Up @@ -137,27 +141,9 @@ impl Validatable for P2PNetworkConfig {
/// Blocklist client specific config
#[derive(Deserialize, Clone, Debug)]
pub struct BlocklistClientConfig {
/// Host of the blocklist client
pub host: String,
/// Port of the blocklist client
pub port: u16,
}

impl Validatable for BlocklistClientConfig {
fn validate(&self, _: &Settings) -> Result<(), ConfigError> {
if self.host.is_empty() {
return Err(ConfigError::Message(
"[blocklist_client] Host cannot be empty".to_string(),
));
}
if !(1..=65535).contains(&self.port) {
return Err(ConfigError::Message(
"[blocklist_client] Port must be between 1 and 65535".to_string(),
));
}

Ok(())
}
/// the url for the blocklist client
#[serde(deserialize_with = "url_deserializer_single")]
pub endpoint: Url,
}

/// Emily API configuration.
Expand Down Expand Up @@ -328,7 +314,6 @@ impl Settings {

/// Perform validation on the configuration.
fn validate(&self) -> Result<(), ConfigError> {
self.blocklist_client.validate(self)?;
self.block_notifier.validate(self)?;
self.signer.validate(self)?;

Expand Down Expand Up @@ -367,9 +352,10 @@ impl Validatable for StacksConfig {

#[cfg(test)]
mod tests {
use std::{net::SocketAddr, str::FromStr};
use std::net::SocketAddr;
use std::str::FromStr;

use serialization::try_parse_p2p_multiaddr;
use crate::config::serialization::try_parse_p2p_multiaddr;

use crate::error::Error;
use crate::testing::clear_env;
Expand All @@ -396,8 +382,7 @@ mod tests {
clear_env();

let settings = Settings::new_from_default_config().unwrap();
assert_eq!(settings.blocklist_client.host, "127.0.0.1");
assert_eq!(settings.blocklist_client.port, 8080);
assert!(settings.blocklist_client.is_none());

assert_eq!(settings.block_notifier.server, "tcp://localhost:60401");
assert_eq!(settings.block_notifier.retry_interval, 10);
Expand Down Expand Up @@ -577,6 +562,18 @@ mod tests {
assert_eq!(settings.stacks.endpoints[0].port(), Some(9101));
}

#[test]
fn blocklist_client_endpoint() {
clear_env();

let endpoint = "http://127.0.0.1:12345";
std::env::set_var("SIGNER_BLOCKLIST_CLIENT__ENDPOINT", endpoint);
let settings = Settings::new_from_default_config().unwrap();

let actual_endpoint = settings.blocklist_client.unwrap().endpoint;
assert_eq!(actual_endpoint, url::Url::parse(endpoint).unwrap());
}

#[test]
fn invalid_private_key_length_returns_correct_error() {
clear_env();
Expand Down Expand Up @@ -630,7 +627,6 @@ mod tests {
let hex_err = hex::decode("zz").unwrap_err();

let settings = Settings::new_from_default_config();
assert!(settings.is_err());
assert!(matches!(
settings.unwrap_err(),
ConfigError::Message(msg) if msg == Error::DecodeHexBytes(hex_err).to_string()
Expand Down
1 change: 1 addition & 0 deletions signer/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ async fn main() -> Result<(), Box<dyn std::error::Error>> {
// Our global termination signal watcher. This does not run using `run_checked`
// as it sends its own shutdown signal.
run_shutdown_signal_watcher(context.clone()),
run_checked(run_block_observer, &context),
// The rest of our services which run concurrently, and must all be
// running for the signer to be operational.
djordon marked this conversation as resolved.
Show resolved Hide resolved
run_checked(run_stacks_event_observer, &context),
Expand Down
5 changes: 2 additions & 3 deletions signer/src/testing/transaction_signer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ struct EventLoopHarness<S, Rng> {
impl<S, Rng> EventLoopHarness<S, Rng>
where
S: storage::DbRead + storage::DbWrite + Clone + Send + Sync + 'static,
Rng: rand::RngCore + rand::CryptoRng + Send + 'static,
Rng: rand::RngCore + rand::CryptoRng + Send + Sync + 'static,
{
fn create(
network: network::in_memory::MpmcBroadcaster,
Expand All @@ -46,7 +46,6 @@ where
threshold: u32,
rng: Rng,
) -> Self {
let blocklist_checker = ();
let (block_observer_notification_tx, block_observer_notifications) =
tokio::sync::watch::channel(());

Expand All @@ -56,7 +55,7 @@ where
event_loop: transaction_signer::TxSignerEventLoop {
storage: storage.clone(),
network,
blocklist_checker,
blocklist_checker: Some(()),
block_observer_notifications,
signer_private_key,
context_window,
Expand Down
23 changes: 12 additions & 11 deletions signer/src/transaction_signer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ pub struct TxSignerEventLoop<Network, Storage, BlocklistChecker, Rng> {
/// Database connection.
pub storage: Storage,
/// Blocklist checker.
pub blocklist_checker: BlocklistChecker,
pub blocklist_checker: Option<BlocklistChecker>,
/// Notification receiver from the block observer.
pub block_observer_notifications: tokio::sync::watch::Receiver<()>,
/// Private key of the signer for network communication.
Expand Down Expand Up @@ -599,12 +599,7 @@ where
.collect::<Result<Vec<bitcoin::Address>, _>>()?;

let is_accepted = futures::stream::iter(&addresses)
.any(|address| async {
self.blocklist_checker
.can_accept(&address.to_string())
.await
.unwrap_or(false)
})
.any(|address| async { self.check_blocklist(&address.to_string()).await })
.await;

let msg = message::SignerDepositDecision {
Expand Down Expand Up @@ -637,10 +632,8 @@ where
// TODO: Do we want to do this on the sender address of the
// recipient address?
let is_accepted = self
.blocklist_checker
.can_accept(&withdraw_request.sender_address.to_string())
.await
.unwrap_or(false);
.check_blocklist(&withdraw_request.sender_address.to_string())
.await;

let signer_decision = model::WithdrawalSigner {
request_id: withdraw_request.request_id,
Expand All @@ -657,6 +650,14 @@ where
Ok(())
}

async fn check_blocklist(&self, address: &str) -> bool {
djordon marked this conversation as resolved.
Show resolved Hide resolved
let Some(client) = self.blocklist_checker.as_ref() else {
return true;
};

client.can_accept(address).await.unwrap_or(false)
}

#[tracing::instrument(skip(self))]
async fn persist_received_deposit_decision(
&mut self,
Expand Down
2 changes: 2 additions & 0 deletions signer/tests/integration/bitcoin_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use signer::bitcoin::BitcoinInteract;
use signer::util::ApiFallbackClient;
use url::Url;

#[cfg_attr(not(feature = "integration-tests"), ignore)]
#[tokio::test]
async fn test_get_block_not_found() {
let url: Url = "http://devnet:devnet@localhost:18443".parse().unwrap();
Expand All @@ -26,6 +27,7 @@ async fn test_get_block_not_found() {
// TODO: Figure out how to let this (and similar tests) run against the wallet
// generated by `initialize_blockchain()`. See comment in the test below.
//#[ignore = "This test needs to be run against a 'fresh' bitcoin core instance"]
#[cfg_attr(not(feature = "integration-tests"), ignore)]
#[tokio::test]
async fn test_get_block_works() {
let (_, faucet) = regtest::initialize_blockchain();
Expand Down
Loading