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

Replace explorer prefix search with exact match for block and transaction hash #722

Merged
merged 3 commits into from
Oct 30, 2024
Merged
Show file tree
Hide file tree
Changes from 2 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
2 changes: 1 addition & 1 deletion api/explorer.toml
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ Returns

[route.get_search_result]
PATH = ["search/:query"]
":query" = "Literal"
":query" = "TaggedBase64"
DOC = """
Retrieve search results for blocks or transactions that can be identified in some what by the given string ":query".
At the moment the only field this matches against is the hash of the Block or Transaction.
Expand Down
3 changes: 2 additions & 1 deletion src/data_source/extension.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ use async_trait::async_trait;
use hotshot_types::traits::node_implementation::NodeType;
use jf_merkle_tree::prelude::MerkleProof;
use std::ops::RangeBounds;
use tagged_base64::TaggedBase64;

/// Wrapper to add extensibility to an existing data source.
///
Expand Down Expand Up @@ -403,7 +404,7 @@ where

async fn get_search_results(
&self,
query: String,
query: TaggedBase64,
) -> Result<
explorer::query_data::SearchResult<Types>,
explorer::query_data::GetSearchResultsError,
Expand Down
3 changes: 2 additions & 1 deletion src/data_source/fetching.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ use std::{
ops::{Bound, Range, RangeBounds},
time::Duration,
};
use tagged_base64::TaggedBase64;
use tracing::Instrument;

mod block;
Expand Down Expand Up @@ -1202,7 +1203,7 @@ where

async fn get_search_results(
&self,
query: String,
query: TaggedBase64,
) -> Result<
explorer::query_data::SearchResult<Types>,
explorer::query_data::GetSearchResultsError,
Expand Down
3 changes: 2 additions & 1 deletion src/data_source/fetching/notify_storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ use futures::future::Future;
use hotshot_types::traits::node_implementation::NodeType;
use jf_merkle_tree::{prelude::MerkleProof, MerkleTreeScheme};
use std::ops::RangeBounds;
use tagged_base64::TaggedBase64;

#[derive(Debug)]
pub(super) struct NotifyStorage<Types, S>
Expand Down Expand Up @@ -513,7 +514,7 @@ where

async fn get_search_results(
&mut self,
query: String,
query: TaggedBase64,
) -> Result<
explorer::query_data::SearchResult<Types>,
explorer::query_data::GetSearchResultsError,
Expand Down
3 changes: 2 additions & 1 deletion src/data_source/storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ use async_trait::async_trait;
use hotshot_types::traits::node_implementation::NodeType;
use jf_merkle_tree::prelude::MerkleProof;
use std::ops::RangeBounds;
use tagged_base64::TaggedBase64;

pub mod fs;
mod ledger_log;
Expand Down Expand Up @@ -233,7 +234,7 @@ where
/// query string.
async fn get_search_results(
&mut self,
query: String,
query: TaggedBase64,
) -> Result<SearchResult<Types>, GetSearchResultsError>;
}

Expand Down
127 changes: 73 additions & 54 deletions src/data_source/storage/sql/queries/explorer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,23 +20,27 @@ use crate::{
availability::{BlockQueryData, QueryableHeader, QueryablePayload, TransactionIndex},
data_source::storage::ExplorerStorage,
explorer::{
self, errors::NotFound, query_data::TransactionDetailResponse, traits::ExplorerHeader,
self,
errors::{self, NotFound},
query_data::TransactionDetailResponse,
traits::ExplorerHeader,
BalanceAmount, BlockDetail, BlockIdentifier, BlockRange, BlockSummary, ExplorerHistograms,
ExplorerSummary, GenesisOverview, GetBlockDetailError, GetBlockSummariesError,
GetBlockSummariesRequest, GetExplorerSummaryError, GetSearchResultsError,
GetTransactionDetailError, GetTransactionSummariesError, GetTransactionSummariesRequest,
MonetaryValue, SearchResult, TransactionIdentifier, TransactionRange, TransactionSummary,
TransactionSummaryFilter,
},
Header, Payload, QueryError, QueryResult,
Header, Payload, QueryError, QueryResult, Transaction as HotshotTransaction,
};
use async_trait::async_trait;
use committable::Committable;
use committable::{Commitment, Committable};
use futures::stream::{self, StreamExt, TryStreamExt};
use hotshot_types::traits::node_implementation::NodeType;
use itertools::Itertools;
use sqlx::{types::Json, FromRow, Row};
use std::num::NonZeroUsize;
use tagged_base64::{Tagged, TaggedBase64};

impl From<sqlx::Error> for GetExplorerSummaryError {
fn from(err: sqlx::Error) -> Self {
Expand Down Expand Up @@ -509,61 +513,76 @@ where

async fn get_search_results(
&mut self,
search_query: String,
search_query: TaggedBase64,
) -> Result<SearchResult<Types>, GetSearchResultsError> {
let block_query = format!(
"SELECT {BLOCK_COLUMNS}
FROM header AS h
JOIN payload AS p ON h.height = p.height
WHERE h.hash LIKE $1 || '%'
ORDER BY h.height DESC
LIMIT 5"
);
let block_query_rows = query(block_query.as_str())
.bind(&search_query)
.fetch(self.as_mut());
let block_query_result: Vec<BlockSummary<Types>> = block_query_rows
.map(|row| BlockSummary::from_row(&row?))
.try_collect()
.await?;
let search_tag = search_query.tag();
let header_tag = Commitment::<Header<Types>>::tag();
let tx_tag = Commitment::<HotshotTransaction<Types>>::tag();

let transactions_query = format!(
"SELECT {BLOCK_COLUMNS}
FROM header AS h
JOIN payload AS p ON h.height = p.height
JOIN transaction AS t ON h.height = t.block_height
WHERE t.hash LIKE $1 || '%'
ORDER BY h.height DESC
LIMIT 5"
);
let transactions_query_rows = query(transactions_query.as_str())
.bind(&search_query)
.fetch(self.as_mut());
let transactions_query_result: Vec<TransactionSummary<Types>> = transactions_query_rows
.map(|row| -> Result<Vec<TransactionSummary<Types>>, QueryError>{
let block = BlockQueryData::<Types>::from_row(&row?)?;
let transactions = block
.enumerate()
.enumerate()
.filter(|(_, (_, txn))| txn.commit().to_string().starts_with(&search_query))
.map(|(offset, (_, txn))| {
Ok(TransactionSummary::try_from((
&block, offset, txn,
))?)
})
.try_collect::<TransactionSummary<Types>, Vec<TransactionSummary<Types>>, QueryError>()?;
if search_tag != header_tag && search_tag != tx_tag {
return Err(GetSearchResultsError::InvalidQuery(errors::BadQuery {}));
}

Ok(transactions)
let search_query_string = search_query.to_string();
if search_tag == header_tag {
let block_query = format!(
"SELECT {BLOCK_COLUMNS}
FROM header AS h
JOIN payload AS p ON h.height = p.height
WHERE h.hash = $1
ORDER BY h.height DESC
LIMIT 5"
);
let block_query_rows = query(block_query.as_str())
.bind(&search_query_string)
.fetch(self.as_mut());
Copy link
Member

Choose a reason for hiding this comment

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

We can do LIMIT 1 and fetch_one here, querying for blocks by equality on hash is only ever going to return one block

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah

Copy link
Contributor Author

@imabdulbasit imabdulbasit Oct 29, 2024

Choose a reason for hiding this comment

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

I am considering making the blocks field an Option<BlockSummary> since there can only be one block now. However, I don't want to break the frontend, and I also think that SearchResult should be an enum

Copy link
Member

Choose a reason for hiding this comment

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

Making this return an enum would indeed be a backwards incompatible change. I can possibly support it with a backwards compatible decoder that can support either representation, but I think that would need to be in place in the Block Explorer client-side code before that sort of change is introduced on the server side.

let block_query_result: Vec<BlockSummary<Types>> = block_query_rows
.map(|row| BlockSummary::from_row(&row?))
.try_collect()
.await?;

Ok(SearchResult {
blocks: block_query_result,
transactions: Vec::new(),
})
.try_collect::<Vec<Vec<TransactionSummary<Types>>>>()
.await?
.into_iter()
.flatten()
.collect();
} else {
let transactions_query = format!(
"SELECT {BLOCK_COLUMNS}
FROM header AS h
JOIN payload AS p ON h.height = p.height
JOIN transaction AS t ON h.height = t.block_height
WHERE t.hash = $1
ORDER BY h.height DESC
LIMIT 5"
);
let transactions_query_rows = query(transactions_query.as_str())
.bind(&search_query_string)
.fetch(self.as_mut());
let transactions_query_result: Vec<TransactionSummary<Types>> = transactions_query_rows
.map(|row| -> Result<Vec<TransactionSummary<Types>>, QueryError>{
let block = BlockQueryData::<Types>::from_row(&row?)?;
let transactions = block
.enumerate()
.enumerate()
.filter(|(_, (_, txn))| txn.commit().to_string().starts_with(&search_query_string))
.map(|(offset, (_, txn))| {
Ok(TransactionSummary::try_from((
&block, offset, txn,
))?)
})
.try_collect::<TransactionSummary<Types>, Vec<TransactionSummary<Types>>, QueryError>()?;
Ok(transactions)
})
.try_collect::<Vec<Vec<TransactionSummary<Types>>>>()
.await?
.into_iter()
.flatten()
.collect();

Ok(SearchResult {
blocks: block_query_result,
transactions: transactions_query_result,
})
Ok(SearchResult {
blocks: Vec::new(),
transactions: transactions_query_result,
})
}
}
}
7 changes: 3 additions & 4 deletions src/explorer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -373,16 +373,15 @@ where
.get("get_search_result", move |req, state| {
async move {
let query = req
.string_param("query")
.tagged_base64_param("query")
.map_err(|err| {
tracing::error!("query param error: {}", err);
GetSearchResultsError::InvalidQuery(errors::BadQuery {})
})
.map_err(Error::GetSearchResults)?
.to_string();
.map_err(Error::GetSearchResults)?;

state
.get_search_results(query)
.get_search_results(query.clone())
.await
.map(SearchResultResponse::from)
.map_err(Error::GetSearchResults)
Expand Down
3 changes: 2 additions & 1 deletion src/explorer/data_source.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ use crate::{
};
use async_trait::async_trait;
use hotshot_types::traits::node_implementation::NodeType;
use tagged_base64::TaggedBase64;

/// An interface for querying Data and Statistics from the HotShot Blockchain.
///
Expand Down Expand Up @@ -87,6 +88,6 @@ where
/// query string.
async fn get_search_results(
&self,
query: String,
query: TaggedBase64,
) -> Result<SearchResult<Types>, GetSearchResultsError>;
}