Skip to content

Commit

Permalink
Move app data fetching logic to app_data::Registry (#2297)
Browse files Browse the repository at this point in the history
# Description
We are currently seeing a lot of `Unknown appData pre-image` logs,
because we do not attempt to resolve the app data hash when orders are
being quoted. This makes it impossible to see if we are ready to
disallow orders with unknown app data and what app data overrides we
should potentially add.

# Changes
<!-- List of detailed changes (how the change is accomplished) -->

- [x] Move app_data fetching logic into the registry where it can be
shared between the orderbook (placements) and quoter.

## How to test
unit tests
  • Loading branch information
fleupold authored Jan 17, 2024
1 parent 9db37c2 commit d122b53
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 43 deletions.
40 changes: 38 additions & 2 deletions crates/orderbook/src/app_data.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
use {
crate::database::{app_data::InsertError, Postgres},
crate::{
database::{app_data::InsertError, Postgres},
ipfs_app_data::IpfsAppData,
},
anyhow::{Context, Result},
model::app_data::AppDataHash,
shared::app_data,
};
Expand All @@ -8,14 +12,20 @@ use {
pub struct Registry {
validator: app_data::Validator,
database: Postgres,
ipfs: Option<IpfsAppData>,
}

impl Registry {
/// Creates a new instance of an app-data registry.
pub fn new(validator: app_data::Validator, database: Postgres) -> Self {
pub fn new(
validator: app_data::Validator,
database: Postgres,
ipfs: Option<IpfsAppData>,
) -> Self {
Self {
validator,
database,
ipfs,
}
}

Expand Down Expand Up @@ -57,6 +67,32 @@ impl Registry {
Err(InsertError::Other(err)) => Err(RegisterError::Other(err)),
}
}

/// Finds full app data for an order that only has the contract app data
/// hash.
///
/// The full app data can be located in the database or on IPFS.
pub async fn find(&self, contract_app_data: &AppDataHash) -> Result<Option<String>> {
// we reserve the 0 app data to indicate empty app data.
if contract_app_data.is_zero() {
return Ok(Some(app_data::EMPTY.to_string()));
}

if let Some(app_data) = self
.database
.get_full_app_data(contract_app_data)
.await
.context("from database")?
{
tracing::debug!(?contract_app_data, "full app data in database");
return Ok(Some(app_data));
}

let Some(ipfs) = &self.ipfs else {
return Ok(None);
};
ipfs.fetch(contract_app_data).await.context("from ipfs")
}
}

#[derive(Debug)]
Expand Down
47 changes: 11 additions & 36 deletions crates/orderbook/src/orderbook.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
use {
crate::{
app_data,
database::orders::{InsertionError, OrderStoring},
dto,
ipfs_app_data::IpfsAppData,
},
anyhow::{Context, Result},
chrono::Utc,
Expand All @@ -24,7 +24,6 @@ use {
},
primitive_types::H160,
shared::{
app_data,
metrics::LivenessChecking,
order_validation::{OrderValidating, ValidationError},
},
Expand Down Expand Up @@ -176,7 +175,7 @@ pub struct Orderbook {
settlement_contract: H160,
database: crate::database::Postgres,
order_validator: Arc<dyn OrderValidating>,
ipfs: Option<IpfsAppData>,
app_data: Arc<app_data::Registry>,
}

impl Orderbook {
Expand All @@ -186,53 +185,24 @@ impl Orderbook {
settlement_contract: H160,
database: crate::database::Postgres,
order_validator: Arc<dyn OrderValidating>,
ipfs: Option<IpfsAppData>,
app_data: Arc<app_data::Registry>,
) -> Self {
Metrics::initialize();
Self {
domain_separator,
settlement_contract,
database,
order_validator,
ipfs,
app_data,
}
}

/// Finds full app data for an order that only has the contract app data
/// hash.
///
/// The full app data can be located in the database or on IPFS.
pub async fn find_full_app_data(
&self,
contract_app_data: &AppDataHash,
) -> Result<Option<String>> {
// we reserve the 0 app data to indicate empty app data.
if contract_app_data.is_zero() {
return Ok(Some(app_data::EMPTY.to_string()));
}

if let Some(app_data) = self
.database
.get_full_app_data(contract_app_data)
.await
.context("from database")?
{
tracing::debug!(?contract_app_data, "full app data in database");
return Ok(Some(app_data));
}

let Some(ipfs) = &self.ipfs else {
return Ok(None);
};
ipfs.fetch(contract_app_data).await.context("from ipfs")
}

pub async fn add_order(
&self,
payload: OrderCreation,
) -> Result<(OrderUid, Option<QuoteId>), AddOrderError> {
let full_app_data_override = match payload.app_data {
OrderCreationAppData::Hash { hash } => self.find_full_app_data(&hash).await?,
OrderCreationAppData::Hash { hash } => self.app_data.find(&hash).await?,
_ => None,
};

Expand Down Expand Up @@ -497,12 +467,17 @@ mod tests {
let database = crate::database::Postgres::new("postgresql://").unwrap();
database::clear_DANGER(&database.pool).await.unwrap();
database.insert_order(&old_order, None).await.unwrap();
let app_data = Arc::new(app_data::Registry::new(
shared::app_data::Validator::new(8192),
database.clone(),
None,
));
let orderbook = Orderbook {
database,
order_validator: Arc::new(order_validator),
domain_separator: Default::default(),
settlement_contract: H160([0xba; 20]),
ipfs: None,
app_data,
};

// App data does not encode cancellation.
Expand Down
11 changes: 6 additions & 5 deletions crates/orderbook/src/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -495,12 +495,17 @@ pub async fn run(args: Arguments) {
)
})
.map(IpfsAppData::new);
let app_data = Arc::new(app_data::Registry::new(
app_data_validator,
postgres.clone(),
ipfs,
));
let orderbook = Arc::new(Orderbook::new(
domain_separator,
settlement_contract.address(),
postgres.clone(),
order_validator.clone(),
ipfs,
app_data.clone(),
));

if let Some(uniswap_v3) = uniswap_v3_pool_fetcher {
Expand All @@ -511,10 +516,6 @@ pub async fn run(args: Arguments) {
check_database_connection(orderbook.as_ref()).await;
let quotes =
Arc::new(QuoteHandler::new(order_validator, optimal_quoter).with_fast_quoter(fast_quoter));
let app_data = Arc::new(app_data::Registry::new(
app_data_validator,
postgres.clone(),
));

let (shutdown_sender, shutdown_receiver) = tokio::sync::oneshot::channel();
let serve_api = serve_api(
Expand Down

0 comments on commit d122b53

Please sign in to comment.