Skip to content

Commit

Permalink
chore(ICP-Rosetta): FI-1479: fix search transactions endpoint (#1585)
Browse files Browse the repository at this point in the history
This MR proposes the following changes:

1. Delete the existing implementation of search transactions
2. Add an implementation of search transactions that matches the
standard and the icrc1 rosetta implementation
3. Add tests for all search options

---------

Co-authored-by: Mathias Björkqvist <mathias.bjorkqvist@dfinity.org>
  • Loading branch information
NikolasHai and mbjorkqvist authored Sep 24, 2024
1 parent e76a069 commit abdaefa
Show file tree
Hide file tree
Showing 9 changed files with 468 additions and 206 deletions.
2 changes: 2 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions rs/rosetta-api/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,9 @@ DEPENDENCIES = [
"@crate_index//:rand",
"@crate_index//:reqwest",
"@crate_index//:rolling-file",
"@crate_index//:rusqlite",
"@crate_index//:serde",
"@crate_index//:serde_bytes",
"@crate_index//:serde_cbor",
"@crate_index//:serde_json",
"@crate_index//:strum",
Expand Down
2 changes: 2 additions & 0 deletions rs/rosetta-api/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,9 @@ rand = { workspace = true }
reqwest = { workspace = true }
rolling-file = { workspace = true }
rosetta-core = { path = "rosetta_core" }
rusqlite = { version = "~0.28.0", features = ["bundled"] }
serde = { workspace = true }
serde_bytes = { workspace = true }
serde_cbor = { workspace = true }
serde_json = { workspace = true }
strum = { workspace = true }
Expand Down
28 changes: 27 additions & 1 deletion rs/rosetta-api/ledger_canister_blocks_synchronizer/src/blocks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,21 @@ mod database_access {
};
use ic_ledger_hash_of::HashOf;
use icp_ledger::{AccountIdentifier, Block, Operation};
use rusqlite::{named_params, params, Connection, Statement};
use rusqlite::{named_params, params, Connection, Params, Statement};

pub fn get_blocks_by_custom_query<P>(
connection: &Connection,
sql_query: String,
params: P,
) -> Result<Vec<HashedBlock>, BlockStoreError>
where
P: Params,
{
let mut stmt = connection
.prepare_cached(&sql_query)
.map_err(|e| BlockStoreError::Other(e.to_string()))?;
read_hashed_blocks(&mut stmt, params)
}

pub fn push_hashed_block(
con: &mut Connection,
Expand Down Expand Up @@ -1567,6 +1581,18 @@ impl Blocks {
})?;
database_access::contains_block(&mut connection, block_idx)
}

pub fn get_blocks_by_custom_query<P>(
&self,
sql_query: String,
params: P,
) -> Result<Vec<HashedBlock>, BlockStoreError>
where
P: rusqlite::Params,
{
let open_connection = self.connection.lock().unwrap();
database_access::get_blocks_by_custom_query(&open_connection, sql_query, params)
}
}

fn sql_bytes_to_block(cell: Vec<u8>) -> Result<Block, rusqlite::Error> {
Expand Down
10 changes: 5 additions & 5 deletions rs/rosetta-api/rosetta_core/src/identifiers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ impl SubNetworkIdentifier {
}

/// The block_identifier uniquely identifies a block in a particular network.
#[derive(Clone, Eq, PartialEq, Debug, Deserialize, Serialize)]
#[derive(Clone, Eq, PartialEq, Debug, Deserialize, Serialize, Hash)]
pub struct BlockIdentifier {
/// This is also known as the block height.
pub index: u64,
Expand Down Expand Up @@ -137,7 +137,7 @@ impl From<BlockIdentifier> for PartialBlockIdentifier {
///
/// The transaction_identifier uniquely identifies a transaction in a particular
/// network and block or in the mempool.
#[derive(Clone, Eq, PartialEq, Debug, Deserialize, Serialize)]
#[derive(Clone, Eq, PartialEq, Debug, Deserialize, Serialize, Hash)]
pub struct TransactionIdentifier {
/// Any transactions that are attributable only to a block (ex: a block event) should use the hash of the block as the identifier. This should be normalized according to the case specified in the transaction_hash_case in network options.
pub hash: String,
Expand Down Expand Up @@ -198,7 +198,7 @@ impl OperationIdentifier {
/// The account_identifier uniquely identifies an account within a network. All
/// fields in the account_identifier are utilized to determine this uniqueness
/// (including the metadata field, if populated).
#[derive(Clone, Eq, PartialEq, Debug, Deserialize, Serialize)]
#[derive(Clone, Eq, PartialEq, Debug, Deserialize, Serialize, Hash)]
pub struct AccountIdentifier {
/// The address may be a cryptographic public key (or some encoding of it)
/// or a provided username.
Expand Down Expand Up @@ -278,7 +278,7 @@ impl TryFrom<AccountIdentifier> for icp_ledger::AccountIdentifier {
/// An account may have state specific to a contract address (ERC-20 token)
/// and/or a stake (delegated balance). The sub_account_identifier should
/// specify which state (if applicable) an account instantiation refers to.
#[derive(Clone, Eq, PartialEq, Debug, Deserialize, Serialize)]
#[derive(Clone, Eq, PartialEq, Debug, Deserialize, Serialize, Hash)]
pub struct SubAccountIdentifier {
/// The SubAccount address may be a cryptographic value or some other
/// identifier (ex: bonded) that uniquely specifies a SubAccount.
Expand All @@ -293,7 +293,7 @@ pub struct SubAccountIdentifier {
}

/// CoinIdentifier uniquely identifies a Coin.
#[derive(Clone, Eq, PartialEq, Debug, Deserialize, Serialize)]
#[derive(Clone, Eq, PartialEq, Debug, Deserialize, Serialize, Hash)]
pub struct CoinIdentifier {
/// Identifier should be populated with a globally unique identifier of a
/// Coin. In Bitcoin, this identifier would be transaction_hash:index.
Expand Down
14 changes: 7 additions & 7 deletions rs/rosetta-api/rosetta_core/src/objects.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ pub type ObjectMap = serde_json::map::Map<String, Object>;

/// Currency is composed of a canonical Symbol and Decimals. This Decimals value is used to convert an Amount.
/// Value from atomic units (Satoshis) to standard units (Bitcoins).
#[derive(Clone, Eq, PartialEq, Debug, Default, Deserialize, Serialize)]
#[derive(Clone, Eq, PartialEq, Debug, Default, Deserialize, Serialize, Hash)]
pub struct Currency {
/// Canonical symbol associated with a currency.
pub symbol: String,
Expand Down Expand Up @@ -90,7 +90,7 @@ impl Block {
/// Operations contain all balance-changing information within a transaction.
/// They are always one-sided (only affect 1 AccountIdentifier) and can succeed
/// or fail independently from a Transaction.
#[derive(Clone, Eq, PartialEq, Debug, Deserialize, Serialize)]
#[derive(Clone, Eq, PartialEq, Debug, Deserialize, Serialize, Hash)]
pub struct Operation {
/// The operation_identifier uniquely identifies an operation within a transaction.
pub operation_identifier: OperationIdentifier,
Expand Down Expand Up @@ -161,7 +161,7 @@ impl Operation {
/// UTXOs allows for supporting both account-based transfers and UTXO-based
/// transfers on the same blockchain (when a transfer is account-based, don't
/// populate this model).
#[derive(Clone, Eq, PartialEq, Debug, Deserialize, Serialize)]
#[derive(Clone, Eq, PartialEq, Debug, Deserialize, Serialize, Hash)]
pub struct CoinChange {
/// CoinIdentifier uniquely identifies a Coin.
pub coin_identifier: CoinIdentifier,
Expand All @@ -187,7 +187,7 @@ impl CoinChange {
/// as `#[repr(C)]` which helps with FFI.
#[allow(non_camel_case_types)]
#[repr(C)]
#[derive(Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Debug, Deserialize, Serialize)]
#[derive(Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Debug, Deserialize, Serialize, Hash)]
pub enum CoinAction {
/// CoinAction indicating a Coin was created.
#[serde(rename = "coin_created")]
Expand Down Expand Up @@ -264,7 +264,7 @@ impl ::std::str::FromStr for Direction {

/// Transactions contain an array of Operations that are attributable to the
/// same TransactionIdentifier.
#[derive(Clone, Eq, PartialEq, Debug, Deserialize, Serialize)]
#[derive(Clone, Eq, PartialEq, Debug, Deserialize, Serialize, Hash)]
pub struct Transaction {
/// The transaction_identifier uniquely identifies a transaction in a particular network and block or in the mempool.
pub transaction_identifier: TransactionIdentifier,
Expand All @@ -291,7 +291,7 @@ impl Transaction {

// Amount is some Value of a Currency. It is considered invalid to specify a
/// Value without a Currency.
#[derive(Clone, Eq, PartialEq, Debug, Deserialize, Serialize)]
#[derive(Clone, Eq, PartialEq, Debug, Deserialize, Serialize, Hash)]
pub struct Amount {
/// Value of the transaction in atomic units represented as an
/// arbitrary-sized signed integer. For example, 1 BTC would be represented
Expand Down Expand Up @@ -692,7 +692,7 @@ pub struct Signature {
}

/// BlockTransaction contains a populated Transaction and the BlockIdentifier that contains it.
#[derive(Clone, Eq, PartialEq, Debug, Deserialize, Serialize)]
#[derive(Clone, Eq, PartialEq, Debug, Deserialize, Serialize, Hash)]
pub struct BlockTransaction {
/// The block_identifier uniquely identifies a block in a particular network.
pub block_identifier: BlockIdentifier,
Expand Down
4 changes: 2 additions & 2 deletions rs/rosetta-api/runner/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ pub async fn start_rosetta(
cmd.arg("--offline");
}

let _proc = KillOnDrop(cmd.spawn().unwrap_or_else(|e| {
let proc = KillOnDrop(cmd.spawn().unwrap_or_else(|e| {
panic!(
"Failed to execute ic-rosetta-api (path = {}, exists? = {}): {}",
rosetta_bin.display(),
Expand Down Expand Up @@ -178,7 +178,7 @@ pub async fn start_rosetta(
}

RosettaContext {
proc: _proc,
proc,
state_directory,
port,
}
Expand Down
Loading

0 comments on commit abdaefa

Please sign in to comment.