From 2141d04257bf351bc8810df567f6462701f763f0 Mon Sep 17 00:00:00 2001 From: Martin Raszyk Date: Fri, 16 Aug 2024 07:56:08 +0200 Subject: [PATCH 1/8] feat(PocketIC): VerifiedApplication subnets --- packages/pocket-ic/src/common/rest.rs | 16 ++++++++++++++++ packages/pocket-ic/src/lib.rs | 6 ++++++ rs/pocket_ic_server/src/pocket_ic.rs | 20 ++++++++++++++++++-- rs/pocket_ic_server/src/state_api/routes.rs | 3 ++- 4 files changed, 42 insertions(+), 3 deletions(-) diff --git a/packages/pocket-ic/src/common/rest.rs b/packages/pocket-ic/src/common/rest.rs index ffae11dbe67..714c4f9c41d 100644 --- a/packages/pocket-ic/src/common/rest.rs +++ b/packages/pocket-ic/src/common/rest.rs @@ -369,6 +369,7 @@ pub enum SubnetKind { NNS, SNS, System, + VerifiedApplication, } /// This represents which named subnets the user wants to create, and how @@ -382,12 +383,14 @@ pub struct SubnetConfigSet { pub bitcoin: bool, pub system: usize, pub application: usize, + pub verified_application: usize, } impl SubnetConfigSet { pub fn validate(&self) -> Result<(), String> { if self.system > 0 || self.application > 0 + || self.verified_application > 0 || self.nns || self.sns || self.ii @@ -410,6 +413,7 @@ impl From for ExtendedSubnetConfigSet { bitcoin, system, application, + verified_application, }: SubnetConfigSet, ) -> Self { ExtendedSubnetConfigSet { @@ -440,6 +444,7 @@ impl From for ExtendedSubnetConfigSet { }, system: vec![SubnetSpec::default(); system], application: vec![SubnetSpec::default(); application], + verified_application: vec![SubnetSpec::default(); verified_application], } } } @@ -460,6 +465,7 @@ pub struct ExtendedSubnetConfigSet { pub bitcoin: Option, pub system: Vec, pub application: Vec, + pub verified_application: Vec, } /// Specifies various configurations for a subnet. @@ -617,6 +623,7 @@ impl ExtendedSubnetConfigSet { pub fn validate(&self) -> Result<(), String> { if !self.system.is_empty() || !self.application.is_empty() + || !self.verified_application.is_empty() || self.nns.is_some() || self.sns.is_some() || self.ii.is_some() @@ -646,6 +653,11 @@ impl ExtendedSubnetConfigSet { .into_iter() .map(|conf| conf.with_dts_flag(dts_flag)) .collect(); + self.verified_application = self + .verified_application + .into_iter() + .map(|conf| conf.with_dts_flag(dts_flag)) + .collect(); self } } @@ -677,6 +689,10 @@ impl Topology { self.find_subnets(SubnetKind::Application, None) } + pub fn get_verified_app_subnets(&self) -> Vec { + self.find_subnets(SubnetKind::VerifiedApplication, None) + } + pub fn get_benchmarking_app_subnets(&self) -> Vec { self.find_subnets( SubnetKind::Application, diff --git a/packages/pocket-ic/src/lib.rs b/packages/pocket-ic/src/lib.rs index d3c8f662a28..d488e52c24b 100644 --- a/packages/pocket-ic/src/lib.rs +++ b/packages/pocket-ic/src/lib.rs @@ -249,6 +249,12 @@ impl PocketIcBuilder { self } + /// Add an empty verified application subnet + pub fn with_verified_application_subnet(mut self) -> Self { + self.config.verified_application.push(SubnetSpec::default()); + self + } + pub fn with_benchmarking_application_subnet(mut self) -> Self { self.config .application diff --git a/rs/pocket_ic_server/src/pocket_ic.rs b/rs/pocket_ic_server/src/pocket_ic.rs index 0b4dafd6cc6..928236f3d95 100644 --- a/rs/pocket_ic_server/src/pocket_ic.rs +++ b/rs/pocket_ic_server/src/pocket_ic.rs @@ -358,7 +358,16 @@ impl PocketIc { spec.get_dts_flag(), ) }); - sys.chain(app) + let verified_app = subnet_configs.verified_application.iter().map(|spec| { + ( + SubnetKind::VerifiedApplication, + spec.get_state_path(), + spec.get_subnet_id(), + spec.get_instruction_config(), + spec.get_dts_flag(), + ) + }); + sys.chain(app).chain(verified_app) }; let mut subnet_config_info: Vec = vec![]; @@ -566,6 +575,11 @@ impl PocketIc { if let Some(subnet) = random_app_subnet { return subnet; } + let random_verified_app_subnet = + self.get_random_subnet_of_type(rest::SubnetKind::VerifiedApplication); + if let Some(subnet) = random_verified_app_subnet { + return subnet; + } let random_system_subnet = self.get_random_subnet_of_type(rest::SubnetKind::System); if let Some(subnet) = random_system_subnet { return subnet; @@ -672,6 +686,7 @@ fn conv_type(inp: rest::SubnetKind) -> SubnetType { match inp { Application | Fiduciary | SNS => SubnetType::Application, Bitcoin | II | NNS | System => SubnetType::System, + VerifiedApplication => SubnetType::VerifiedApplication, } } @@ -679,6 +694,7 @@ fn subnet_size(subnet: SubnetKind) -> u64 { use rest::SubnetKind::*; match subnet { Application => 13, + VerifiedApplication => 13, Fiduciary => 28, SNS => 34, Bitcoin => 13, @@ -698,7 +714,7 @@ fn from_range(range: &CanisterIdRange) -> rest::CanisterIdRange { fn subnet_kind_canister_range(subnet_kind: SubnetKind) -> Option> { use rest::SubnetKind::*; match subnet_kind { - Application | System => None, + Application | VerifiedApplication | System => None, NNS => Some(vec![ gen_range("rwlgt-iiaaa-aaaaa-aaaaa-cai", "renrk-eyaaa-aaaaa-aaada-cai"), gen_range("qoctq-giaaa-aaaaa-aaaea-cai", "n5n4y-3aaaa-aaaaa-p777q-cai"), diff --git a/rs/pocket_ic_server/src/state_api/routes.rs b/rs/pocket_ic_server/src/state_api/routes.rs index 88582b7c890..fc7348976dd 100644 --- a/rs/pocket_ic_server/src/state_api/routes.rs +++ b/rs/pocket_ic_server/src/state_api/routes.rs @@ -1032,7 +1032,8 @@ fn contains_unimplemented(config: ExtendedSubnetConfigSet) -> bool { .into_iter() .flatten() .chain(config.system) - .chain(config.application), + .chain(config.application) + .chain(config.verified_application), |spec: pocket_ic::common::rest::SubnetSpec| { spec.get_subnet_id().is_some() || !spec.is_supported() }, From 4beafe519117dad6a404d613dc9ce0b23dc4dded Mon Sep 17 00:00:00 2001 From: Martin Raszyk Date: Fri, 16 Aug 2024 08:00:03 +0200 Subject: [PATCH 2/8] changelog --- packages/pocket-ic/CHANGELOG.md | 4 ++++ rs/pocket_ic_server/CHANGELOG.md | 1 + 2 files changed, 5 insertions(+) diff --git a/packages/pocket-ic/CHANGELOG.md b/packages/pocket-ic/CHANGELOG.md index 1b0d713dff7..8fc8997e811 100644 --- a/packages/pocket-ic/CHANGELOG.md +++ b/packages/pocket-ic/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased +### Added +- Support for *verified application* subnets: the library function `PocketIcBuilder::with_verified_application_subnet` adds a verified application subnet to the PocketIC instance; + the library function `PocketIc::get_verified_app_subnets` lists all verified application subnets of the PocketIC instance; + ## 4.0.0 - 2024-07-22 diff --git a/rs/pocket_ic_server/CHANGELOG.md b/rs/pocket_ic_server/CHANGELOG.md index 83fc6f3fced..8d4952a65e9 100644 --- a/rs/pocket_ic_server/CHANGELOG.md +++ b/rs/pocket_ic_server/CHANGELOG.md @@ -16,6 +16,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - New argument `ip_addr` of the endpoint `/http_gateway` to specify the IP address at which the HTTP gateway should listen (defaults to `127.0.0.1`). - New GET endpoint `/http_gateway` listing all HTTP gateways and their details. - Support for query statistics in the management canister. +- Support for *verified application* subnets. ### Changed - The argument `listen_at` of the endpoint `/http_gateway` has been renamed to `port`. From 708871cc826a0281cdd8ea1470dbce7f7bc4d361 Mon Sep 17 00:00:00 2001 From: mraszyk <31483726+mraszyk@users.noreply.github.com> Date: Fri, 16 Aug 2024 11:50:51 +0200 Subject: [PATCH 3/8] Update packages/pocket-ic/CHANGELOG.md --- packages/pocket-ic/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/pocket-ic/CHANGELOG.md b/packages/pocket-ic/CHANGELOG.md index 8fc8997e811..e75acc70e4c 100644 --- a/packages/pocket-ic/CHANGELOG.md +++ b/packages/pocket-ic/CHANGELOG.md @@ -8,7 +8,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased ### Added -- Support for *verified application* subnets: the library function `PocketIcBuilder::with_verified_application_subnet` adds a verified application subnet to the PocketIC instance; +- Support for *verified application* subnets: the library function `PocketIcBuilder::with_verified_application_subnet` adds a verified application subnet to the PocketIC instance. the library function `PocketIc::get_verified_app_subnets` lists all verified application subnets of the PocketIC instance; From b443abb95bfd7adbf8edeb729aa1e92b476efffe Mon Sep 17 00:00:00 2001 From: mraszyk <31483726+mraszyk@users.noreply.github.com> Date: Fri, 16 Aug 2024 11:51:58 +0200 Subject: [PATCH 4/8] Update packages/pocket-ic/CHANGELOG.md --- packages/pocket-ic/CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/pocket-ic/CHANGELOG.md b/packages/pocket-ic/CHANGELOG.md index e75acc70e4c..6b6cd21979e 100644 --- a/packages/pocket-ic/CHANGELOG.md +++ b/packages/pocket-ic/CHANGELOG.md @@ -8,8 +8,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased ### Added -- Support for *verified application* subnets: the library function `PocketIcBuilder::with_verified_application_subnet` adds a verified application subnet to the PocketIC instance. - the library function `PocketIc::get_verified_app_subnets` lists all verified application subnets of the PocketIC instance; +- Support for *verified application* subnets: the library function `PocketIcBuilder::with_verified_application_subnet` adds a verified application subnet to the PocketIC instance; + the library function `PocketIc::get_verified_app_subnets` lists all verified application subnets of the PocketIC instance. From 0b5c0c6f0cbf953052d2db1bd72f8defc3816986 Mon Sep 17 00:00:00 2001 From: NikolasHai <113891786+NikolasHai@users.noreply.github.com> Date: Fri, 16 Aug 2024 13:58:38 +0200 Subject: [PATCH 5/8] chore(ICP-Rosetta): FI-1419: icp rosetta database table consolidation (#872) This MR proposes the following changes: 1. Delete the transaction table 2. Consolidate both block and transaction table content into a single table --------- Co-authored-by: maciejdfinity <122265298+maciejdfinity@users.noreply.github.com> --- rs/rosetta-api/CHANGELOG.md | 2 +- .../src/blocks.rs | 379 +++++++----------- .../tests/store_tests.rs | 8 +- rs/rosetta-api/scripts/diff_rosetta_data.sh | 6 +- 4 files changed, 143 insertions(+), 252 deletions(-) diff --git a/rs/rosetta-api/CHANGELOG.md b/rs/rosetta-api/CHANGELOG.md index 555aa67d533..f28f70cddcd 100644 --- a/rs/rosetta-api/CHANGELOG.md +++ b/rs/rosetta-api/CHANGELOG.md @@ -13,8 +13,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Handle Errors that may occur while deserializing objects using serde_json ### Added - /call endpoint with the method 'query_block_range' to fetch multiple blocks at once - ### Changed +- [BREAKING CHANGE]: consolidate block and transaction tables into a single table ## [2.0.0] - 2024-01-18 ### Fixes diff --git a/rs/rosetta-api/ledger_canister_blocks_synchronizer/src/blocks.rs b/rs/rosetta-api/ledger_canister_blocks_synchronizer/src/blocks.rs index 88a6d9ddd3c..eb1f619b3e3 100644 --- a/rs/rosetta-api/ledger_canister_blocks_synchronizer/src/blocks.rs +++ b/rs/rosetta-api/ledger_canister_blocks_synchronizer/src/blocks.rs @@ -1,4 +1,3 @@ -use self::database_access::INSERT_INTO_TRANSACTIONS_STATEMENT; use crate::rosetta_block::RosettaBlock; use crate::{iso8601_to_timestamp, timestamp_to_iso8601}; use ic_ledger_canister_core::ledger::LedgerTransaction; @@ -28,176 +27,105 @@ mod database_access { }; use ic_ledger_hash_of::HashOf; use icp_ledger::{AccountIdentifier, Block, Operation}; - use rusqlite::{named_params, params, types::Null, Connection, Statement}; + use rusqlite::{named_params, params, Connection, Statement}; pub fn push_hashed_block( con: &mut Connection, hb: &HashedBlock, ) -> Result<(), BlockStoreError> { - let mut stmt = con - .prepare("INSERT INTO blocks (hash, block, parent_hash, idx, verified, timestamp) VALUES (?1, ?2, ?3, ?4, FALSE, ?5)") - .map_err(|e| BlockStoreError::Other(e.to_string()))?; - push_hashed_block_execution(hb, &mut stmt) - } - - pub fn push_hashed_block_execution( - hb: &HashedBlock, - stmt: &mut Statement, - ) -> Result<(), BlockStoreError> { - let hash = hb.hash.into_bytes().to_vec(); - let parent_hash = hb.parent_hash.map(|ph| ph.into_bytes().to_vec()); - stmt.execute(params![ - hash, - hb.block.clone().into_vec(), - parent_hash, - hb.index, - timestamp_to_iso8601(hb.timestamp), - ]) - .map_err(|e| BlockStoreError::Other(e.to_string()))?; - Ok(()) - } - - pub const INSERT_INTO_TRANSACTIONS_STATEMENT: &str = "INSERT INTO transactions (block_idx, tx_hash, operation_type, from_account, to_account, amount, fee, created_at_time, memo, icrc1_memo, spender_account, allowance, expected_allowance, expires_at) VALUES (:index, :tx_hash, :op, :from, :to, :tokens, :fee, :created_at_time, :memo, :icrc1_memo, :spender, :allowance, :expected_allowance, :expires_at)"; - - pub fn push_transaction( - connection: &mut Connection, - tx: &icp_ledger::Transaction, - index: &u64, - ) -> Result<(), BlockStoreError> { - let mut stmt = connection - .prepare(INSERT_INTO_TRANSACTIONS_STATEMENT) - .map_err(|e| BlockStoreError::Other(e.to_string()))?; - push_transaction_execution(tx, &mut stmt, index) + let mut stmt = con.prepare("INSERT INTO blocks (block_hash, encoded_block, parent_hash, block_idx, verified, timestamp, tx_hash, operation_type, from_account, to_account, spender_account, amount, allowance, expected_allowance, fee, created_at_time, expires_at, memo, icrc1_memo) VALUES ( + :block_hash, :encoded_block, :parent_hash, :block_idx, FALSE, :timestamp, :tx_hash, :operation_type, :from_account, :to_account, :spender_account, :amount, :allowance, :expected_allowance, :fee, :created_at_time, :expires_at, :memo, :icrc1_memo + )").map_err(|e| BlockStoreError::Other(e.to_string()))?; + execute_insert_block_statement(&mut stmt, hb) } - pub fn push_transaction_execution( - tx: &icp_ledger::Transaction, + pub fn execute_insert_block_statement( stmt: &mut Statement, - index: &u64, + hb: &HashedBlock, ) -> Result<(), BlockStoreError> { - let tx_hash = tx.hash().into_bytes().to_vec(); - let created_at_time = tx.created_at_time.map(timestamp_to_iso8601); - let memo = tx.memo.0.to_string(); - let icrc1_memo = tx.icrc1_memo.as_ref().map(|memo| memo.to_vec()); + let tx = Block::decode(hb.block.clone()) + .map_err(|e| BlockStoreError::Other(e.to_string()))? + .transaction; let operation_type = tx.operation.clone(); + let mut from_account = None; + let mut to_account = None; + let mut spender_account = None; + let mut amount: Option = None; + let mut allowance = None; + let mut expected_allowance = None; + let mut expires_at = None; + let mut fee = None; + match operation_type { - Operation::Burn { from, amount, .. } => { - let op_string: &str = operation_type.into(); - let from_account = from.to_hex(); - let tokens = amount.get_e8s(); - let to_account = Null; - let fees = Null; - stmt.execute(named_params! { - ":index": index, - ":tx_hash": tx_hash, - ":op": op_string, - ":from": from_account, - ":to": to_account, - ":tokens": tokens, - ":fee": fees, - ":created_at_time": created_at_time, - ":memo": memo, - ":icrc1_memo": icrc1_memo, - }) - .map_err(|e| BlockStoreError::Other(e.to_string()))?; + Operation::Burn { + from, amount: a, .. + } => { + from_account = Some(from.to_hex()); + amount = Some(a.get_e8s().to_string()); } - Operation::Mint { to, amount } => { - let op_string: &str = operation_type.into(); - let from_account = Null; - let tokens = amount.get_e8s(); - let to_account = to.to_hex(); - let fees = Null; - stmt.execute(named_params! { - ":index": index, - ":tx_hash": tx_hash, - ":op": op_string, - ":from": from_account, - ":to": to_account, - ":tokens": tokens, - ":fee": fees, - ":created_at_time": created_at_time, - ":memo": memo, - ":icrc1_memo": icrc1_memo, - }) - .map_err(|e| BlockStoreError::Other(e.to_string()))?; + Operation::Mint { to, amount: a } => { + amount = Some(a.get_e8s().to_string()); + to_account = Some(to.to_hex()); } Operation::Approve { from, spender, - allowance, - expected_allowance, - expires_at, - fee, + allowance: al, + expected_allowance: eal, + expires_at: ea, + fee: f, } => { - let op_string: &str = operation_type.into(); - let from_account = from.to_hex(); - let allowance = allowance.get_e8s().to_string(); - let expected_allowance = expected_allowance.map(|a| a.get_e8s().to_string()); - let spender_account = spender.to_hex(); - let expires_at = expires_at.map(timestamp_to_iso8601); - let fees = fee.get_e8s(); - stmt.execute(named_params! { - ":index": index, - ":tx_hash": tx_hash, - ":op": op_string, - ":from": from_account, - ":spender": spender_account, - ":allowance": allowance, - ":expected_allowance": expected_allowance, - ":expires_at": expires_at, - ":fee": fees, - ":created_at_time": created_at_time, - ":memo": memo, - ":icrc1_memo": icrc1_memo, - }) - .map_err(|e| BlockStoreError::Other(e.to_string()))?; + from_account = Some(from.to_hex()); + allowance = Some(al.get_e8s().to_string()); + expected_allowance = eal.map(|eal| eal.get_e8s().to_string()); + spender_account = Some(spender.to_hex()); + expires_at = ea.map(timestamp_to_iso8601); + fee = Some(f.get_e8s().to_string()); } Operation::Transfer { from, to, - amount, - fee, - .. + amount: a, + fee: f, + spender, } => { - let op_string: &str = operation_type.into(); - let from_account = from.to_hex(); - let tokens = amount.get_e8s(); - let to_account = to.to_hex(); - let fees = fee.get_e8s(); - stmt.execute(named_params! { - ":index": index, - ":tx_hash": tx_hash, - ":op": op_string, - ":from": from_account, - ":to": to_account, - ":tokens": tokens, - ":fee": fees, - ":created_at_time": created_at_time, - ":memo": memo, - ":icrc1_memo": icrc1_memo, - }) - .map_err(|e| BlockStoreError::Other(e.to_string()))?; + from_account = Some(from.to_hex()); + amount = Some(a.get_e8s().to_string()); + to_account = Some(to.to_hex()); + fee = Some(f.get_e8s().to_string()); + spender_account = spender.map(|sp| sp.to_hex()); } - } + }; + let params = named_params! { + ":block_hash": hb.hash.into_bytes().to_vec(), + ":encoded_block": hb.block.clone().into_vec(), + ":parent_hash": hb.parent_hash.map(|ph| ph.into_bytes().to_vec()), + ":block_idx": hb.index, + ":timestamp": timestamp_to_iso8601(hb.timestamp), + ":tx_hash": tx.hash().into_bytes().to_vec(), + ":operation_type": >::into(operation_type), + ":from_account": from_account, + ":to_account": to_account, + ":spender_account": spender_account, + ":amount": amount, + ":allowance": allowance, + ":expected_allowance": expected_allowance, + ":fee": fee, + ":created_at_time": tx.created_at_time.map(timestamp_to_iso8601), + ":expires_at": expires_at, + ":memo": tx.memo.0.to_string(), + ":icrc1_memo": tx.icrc1_memo.as_ref().map(|memo| memo.to_vec()), + }; + stmt.execute(params) + .map_err(|e| BlockStoreError::Other(e.to_string()))?; Ok(()) } + pub fn get_all_block_indices_from_blocks_table( connection: &mut Connection, ) -> Result, BlockStoreError> { let mut stmt = connection - .prepare("SELECT idx from blocks") - .map_err(|e| BlockStoreError::Other(e.to_string()))?; - let indices = stmt - .query_map(params![], |row| row.get(0)) - .map_err(|e| BlockStoreError::Other(e.to_string()))?; - let block_indices: Vec = indices.map(|x| x.unwrap()).collect(); - Ok(block_indices) - } - pub fn get_all_block_indices_from_transactions_table( - connection: &mut Connection, - ) -> Result, BlockStoreError> { - let mut stmt = connection - .prepare("SELECT block_idx FROM transactions") + .prepare("SELECT block_idx from blocks") .map_err(|e| BlockStoreError::Other(e.to_string()))?; let indices = stmt .query_map(params![], |row| row.get(0)) @@ -224,7 +152,7 @@ mod database_access { block_idx: &u64, ) -> Result { let mut stmt = connection - .prepare("SELECT Null FROM blocks WHERE idx = ?") + .prepare("SELECT Null FROM blocks WHERE block_idx = ?") .map_err(|e| BlockStoreError::Other(e.to_string()))?; let mut rows = stmt .query(params![block_idx]) @@ -238,7 +166,7 @@ mod database_access { connection: &mut Connection, block_idx: &u64, ) -> Result { - let command = "SELECT block from blocks where idx = ?"; + let command = "SELECT encoded_block from blocks where block_idx = ?"; let mut stmt = connection .prepare(command) .map_err(|e| BlockStoreError::Other(e.to_string())) @@ -262,13 +190,13 @@ mod database_access { ) -> Result { let mut statement = con .prepare_cached( - r#"SELECT hash, block, parent_hash, idx, timestamp + r#"SELECT block_hash, encoded_block, parent_hash, block_idx, timestamp FROM blocks - WHERE idx = :idx"#, + WHERE block_idx = :block_idx"#, ) .map_err(|e| format!("Unable to prepare statement: {e:?}"))?; let mut blocks = statement - .query_map(named_params! { ":idx": block_idx }, |row| { + .query_map(named_params! { ":block_idx": block_idx }, |row| { HashedBlock::try_from(row) }) .map_err(|e| format!("Unable to query hashed block {block_idx}: {e:?}"))?; @@ -295,7 +223,7 @@ mod database_access { connection: &mut Connection, block_idx: &u64, ) -> Result>, BlockStoreError> { - let command = "SELECT tx_hash from transactions where block_idx = ?"; + let command = "SELECT tx_hash from blocks where block_idx = ?"; let mut stmt = connection .prepare(command) .map_err(|e| BlockStoreError::Other(e.to_string())) @@ -320,7 +248,7 @@ mod database_access { hash: &HashOf, ) -> Result, BlockStoreError> { let mut stmt = connection - .prepare("SELECT block_idx from transactions where tx_hash = ?") + .prepare("SELECT block_idx from blocks where tx_hash = ?") .map_err(|e| BlockStoreError::Other(e.to_string())) .unwrap(); let mut rows = stmt @@ -342,7 +270,7 @@ mod database_access { hash: &HashOf, ) -> Result { let mut stmt = connection - .prepare("SELECT idx from blocks where hash = ?") + .prepare("SELECT block_idx from blocks where block_hash = ?") .map_err(|e| BlockStoreError::Other(e.to_string())) .unwrap(); let block_idx = stmt @@ -361,8 +289,8 @@ mod database_access { verified: Option, ) -> Result { let mut stmt = con.prepare_cached(&match verified { - Some(verified) => format!("SELECT hash, block, parent_hash, idx, timestamp from blocks WHERE verified = {} ORDER BY idx ASC Limit 2",verified), - None => "SELECT hash, block, parent_hash, idx, timestamp from blocks ORDER BY idx ASC Limit 2".to_string() + Some(verified) => format!("SELECT block_hash, encoded_block, parent_hash, block_idx, timestamp from blocks WHERE verified = {} ORDER BY block_idx ASC Limit 2",verified), + None => "SELECT block_hash, encoded_block, parent_hash, block_idx, timestamp from blocks ORDER BY block_idx ASC Limit 2".to_string() }).map_err(|e| BlockStoreError::Other(e.to_string()))?; let mut blocks = read_hashed_blocks(&mut stmt, params![])?.into_iter(); match blocks.next() { @@ -386,8 +314,8 @@ mod database_access { verified: Option, ) -> Result { let mut stmt = con.prepare_cached(&match verified { - Some(verified) => format!("SELECT hash, block, parent_hash, idx, timestamp from blocks WHERE verified = {} ORDER BY idx DESC Limit 1",verified), - None => "SELECT hash, block, parent_hash, idx, timestamp from blocks ORDER BY idx DESC Limit 1".to_string() + Some(verified) => format!("SELECT block_hash, encoded_block, parent_hash, block_idx, timestamp from blocks WHERE verified = {} ORDER BY block_idx DESC Limit 1",verified), + None => "SELECT block_hash, encoded_block, parent_hash, block_idx, timestamp from blocks ORDER BY block_idx DESC Limit 1".to_string() }).map_err(|e| BlockStoreError::Other(e.to_string()))?; let mut blocks = read_hashed_blocks(&mut stmt, params![])?.into_iter(); match blocks.next() { @@ -656,7 +584,7 @@ mod database_access { } pub fn is_verified(con: &mut Connection, block_idx: &u64) -> Result { - let command = "SELECT null from blocks WHERE verified=TRUE AND idx=?"; + let command = "SELECT null from blocks WHERE verified=TRUE AND block_idx=?"; let mut stmt = con .prepare(command) .map_err(|e| BlockStoreError::Other(e.to_string())) @@ -802,39 +730,30 @@ impl Blocks { tx.execute( r#" CREATE TABLE IF NOT EXISTS blocks ( - hash BLOB NOT NULL, - block BLOB NOT NULL, + block_hash BLOB NOT NULL, + encoded_block BLOB NOT NULL, parent_hash BLOB, - idx INTEGER NOT NULL PRIMARY KEY, + block_idx INTEGER NOT NULL PRIMARY KEY, verified BOOLEAN, - timestamp TEXT - ) - "#, - [], - )?; - tx.execute( - r#" - CREATE TABLE IF NOT EXISTS transactions ( - block_idx INTEGER NOT NULL, + timestamp TEXT, tx_hash BLOB NOT NULL, operation_type VARCHAR NOT NULL, from_account VARCHAR(64), to_account VARCHAR(64), spender_account VARCHAR(64), - amount INTEGER, + amount TEXT, allowance TEXT, expected_allowance TEXT, - fee INTEGER, + fee TEXT, created_at_time TEXT, expires_at TEXT, memo TEXT, - icrc1_memo BLOB, - PRIMARY KEY(block_idx), - FOREIGN KEY(block_idx) REFERENCES blocks(idx) + icrc1_memo BLOB ) "#, [], )?; + tx.execute( r#" CREATE TABLE IF NOT EXISTS account_balances ( @@ -865,7 +784,7 @@ impl Blocks { r#" CREATE TABLE IF NOT EXISTS rosetta_blocks_transactions ( rosetta_block_idx INTEGER NOT NULL REFERENCES rosetta_blocks(rosetta_block_idx), - block_idx INTEGER NOT NULL REFERENCES blocks(idx), + block_idx INTEGER NOT NULL REFERENCES blocks(block_idx), PRIMARY KEY(rosetta_block_idx, block_idx) ) "#, @@ -873,6 +792,30 @@ impl Blocks { )?; } + tx.execute( + r#" + CREATE INDEX IF NOT EXISTS block_idx_account_balances + ON account_balances(block_idx) + "#, + [], + )?; + + tx.execute( + r#" + CREATE INDEX IF NOT EXISTS tx_hash_index + ON blocks(tx_hash) + "#, + [], + )?; + + tx.execute( + r#" + CREATE INDEX IF NOT EXISTS block_hash_index + ON blocks(block_hash) + "#, + [], + )?; + tx.commit() } @@ -918,7 +861,7 @@ impl Blocks { let first_rosetta_block_index = match first_rosetta_block_index { Some(first_rosetta_block_index) => first_rosetta_block_index, None => connection - .query_row("SELECT max(idx) + 1 FROM blocks", [], |row| { + .query_row("SELECT max(block_idx) + 1 FROM blocks", [], |row| { row.get::<_, Option>(0) })? .unwrap_or(0), @@ -938,16 +881,10 @@ impl Blocks { .execute_batch("BEGIN TRANSACTION;") .map_err(|e| BlockStoreError::Other(format!("{}", e)))?; - connection - .execute( - "DELETE FROM transactions WHERE block_idx > 0 AND block_idx < ?", - params![hb.index], - ) - .map_err(|e| BlockStoreError::Other(e.to_string()))?; database_access::prune_account_balances(&mut connection, &hb.index)?; connection .execute( - "DELETE FROM blocks WHERE idx > 0 AND idx < ?", + "DELETE FROM blocks WHERE block_idx > 0 AND block_idx < ?", params![hb.index], ) .map_err(|e| BlockStoreError::Other(e.to_string()))?; @@ -1035,8 +972,6 @@ impl Blocks { let mut connection = self.connection.lock().unwrap(); let mut block_indices = database_access::get_all_block_indices_from_blocks_table(&mut connection)?; - let mut transaction_block_indices = - database_access::get_all_block_indices_from_transactions_table(&mut connection)?; let mut account_balances_block_indices = database_access::get_all_block_indices_from_account_balances_table(&mut connection)?; let vec_sorted_diff = |blocks_indices: &mut [u64], @@ -1068,40 +1003,15 @@ impl Blocks { } Ok(result) }; - let mut all_indices: Vec = block_indices - .iter() - .cloned() - .chain(transaction_block_indices.iter().cloned()) - .collect(); - all_indices.sort_by(|a, b| a.partial_cmp(b).unwrap()); - all_indices.dedup(); + block_indices.sort_by(|a, b| a.partial_cmp(b).unwrap()); block_indices.dedup(); - transaction_block_indices.sort_by(|a, b| a.partial_cmp(b).unwrap()); - transaction_block_indices.dedup(); + account_balances_block_indices.sort_by(|a, b| a.partial_cmp(b).unwrap()); account_balances_block_indices.dedup(); - if !all_indices.is_empty() { - let diff = vec_sorted_diff(all_indices.as_mut_slice(), block_indices.as_mut_slice())?; - assert!( - diff.is_empty(), - "Transaction Table has more unique block indizes than Blocks Table" - ); - let difference_transaction_indices: Vec = vec_sorted_diff( - all_indices.as_mut_slice(), - transaction_block_indices.as_mut_slice(), - )?; - for missing_index in difference_transaction_indices { - let missing_block = - database_access::get_hashed_block(&mut connection, &missing_index)?; - database_access::push_transaction( - &mut connection, - &Block::decode(missing_block.block).unwrap().transaction, - &missing_index, - )?; - } + if !block_indices.is_empty() { let difference_account_balances_indices: Vec = vec_sorted_diff( - all_indices.as_mut_slice(), + block_indices.as_mut_slice(), account_balances_block_indices.as_mut_slice(), )?; for missing_index in difference_account_balances_indices { @@ -1134,13 +1044,11 @@ impl Blocks { pub fn get_first_hashed_block(&self) -> Result { let mut connection = self.connection.lock().unwrap(); - database_access::get_first_hashed_block(&mut connection, None) } pub fn get_latest_hashed_block(&self) -> Result { let mut connection = self.connection.lock().unwrap(); - database_access::get_latest_hashed_block(&mut connection, None) } @@ -1148,6 +1056,7 @@ impl Blocks { let mut connection = self.connection.lock().unwrap(); database_access::get_latest_hashed_block(&mut connection, Some(true)) } + pub fn get_account_balance( &self, account: &AccountIdentifier, @@ -1173,7 +1082,7 @@ impl Blocks { if range.end > range.start && database_access::contains_block(&mut connection, &range.start).unwrap_or(false) { - let mut stmt = connection.prepare_cached(r#"SELECT hash, block, parent_hash, idx, timestamp FROM blocks WHERE idx >= :start AND idx < :end"#).map_err(|e| BlockStoreError::Other(e.to_string()))?; + let mut stmt = connection.prepare_cached(r#"SELECT block_hash, encoded_block, parent_hash, block_idx, timestamp FROM blocks WHERE block_idx >= :start AND block_idx < :end"#).map_err(|e| BlockStoreError::Other(e.to_string()))?; database_access::read_hashed_blocks( &mut stmt, named_params! { @@ -1194,11 +1103,6 @@ impl Blocks { con.execute_batch("BEGIN TRANSACTION;") .map_err(|e| BlockStoreError::Other(format!("{}", e)))?; database_access::push_hashed_block(&mut con, hb)?; - database_access::push_transaction( - &mut con, - &Block::decode(hb.block.clone()).unwrap().transaction, - &hb.index, - )?; database_access::update_balance_book(&mut con, hb)?; con.execute_batch("COMMIT TRANSACTION;") .map_err(|e| BlockStoreError::Other(format!("{}", e)))?; @@ -1216,11 +1120,9 @@ impl Blocks { connection .execute_batch("BEGIN TRANSACTION;") .map_err(|e| BlockStoreError::Other(format!("{}", e)))?; - let mut stmt_hb = connection.prepare("INSERT INTO blocks (hash, block, parent_hash, idx, verified, timestamp) VALUES (?1, ?2, ?3, ?4, FALSE, ?5)") - .map_err(|e| BlockStoreError::Other(e.to_string()))?; - let mut stmt_tx = connection - .prepare(INSERT_INTO_TRANSACTIONS_STATEMENT) - .map_err(|e| BlockStoreError::Other(e.to_string()))?; + let mut stmt_hb = connection.prepare("INSERT INTO blocks (block_hash, encoded_block, parent_hash, block_idx, verified, timestamp, tx_hash, operation_type, from_account, to_account, spender_account, amount, allowance, expected_allowance, fee, created_at_time, expires_at, memo, icrc1_memo) VALUES ( + :block_hash, :encoded_block, :parent_hash, :block_idx, FALSE, :timestamp, :tx_hash, :operation_type, :from_account, :to_account, :spender_account, :amount, :allowance, :expected_allowance, :fee, :created_at_time, :expires_at, :memo, :icrc1_memo + )").map_err(|e| BlockStoreError::Other(e.to_string()))?; let mut stmt_select = connection .prepare("SELECT block_idx,account,tokens FROM account_balances WHERE account=?1 AND block_idx<=?2 ORDER BY block_idx DESC LIMIT 1") .map_err(|e| BlockStoreError::Other(e.to_string()))?; @@ -1229,7 +1131,7 @@ impl Blocks { .map_err(|e| BlockStoreError::Other(e.to_string()))?; for hb in &batch { - match database_access::push_hashed_block_execution(hb, &mut stmt_hb) { + match database_access::execute_insert_block_statement(&mut stmt_hb, hb) { Ok(_) => (), Err(e) => { connection @@ -1238,19 +1140,6 @@ impl Blocks { return Err(e); } }; - match database_access::push_transaction_execution( - &Block::decode(hb.block.clone()).unwrap().transaction, - &mut stmt_tx, - &hb.index, - ) { - Ok(_) => (), - Err(e) => { - connection - .execute_batch("ROLLBACK TRANSACTION;") - .map_err(|e| BlockStoreError::Other(format!("{}", e)))?; - return Err(e); - } - } match database_access::update_balance_book_execution( hb, &mut stmt_select, @@ -1316,7 +1205,7 @@ impl Blocks { *block_height }; let mut stmt = connection - .prepare("UPDATE blocks SET verified = TRUE WHERE idx >= ?1 AND idx <= ?2") + .prepare("UPDATE blocks SET verified = TRUE WHERE block_idx >= ?1 AND block_idx <= ?2") .map_err(|e| BlockStoreError::Other(e.to_string()))?; stmt.execute(params![verified.index, height]) .map_err(|e| BlockStoreError::Other(e.to_string()))?; @@ -1329,7 +1218,7 @@ impl Blocks { *block_height }; let mut stmt = connection - .prepare("UPDATE blocks SET verified = TRUE WHERE idx <= ?") + .prepare("UPDATE blocks SET verified = TRUE WHERE block_idx <= ?") .map_err(|e| BlockStoreError::Other(e.to_string()))?; stmt.execute(params![height]) .map_err(|e| BlockStoreError::Other(e.to_string()))?; @@ -1435,8 +1324,8 @@ impl Blocks { let mut select_hashed_blocks_stmt = sql_tx .prepare_cached( - r#"SELECT hash, block, parent_hash, idx, timestamp - FROM blocks WHERE idx >= :start AND idx <= :end"#, + r#"SELECT block_hash, encoded_block, parent_hash, block_idx, timestamp + FROM blocks WHERE block_idx >= :start AND block_idx <= :end"#, ) .unwrap(); @@ -1649,9 +1538,9 @@ impl Blocks { .map_err(|e| format!("Unable to aquire the connection mutex: {e:?}"))?; let mut statement = connection .prepare_cached( - r#"SELECT blocks.idx, block + r#"SELECT blocks.block_idx, encoded_block FROM rosetta_blocks_transactions JOIN blocks - ON rosetta_blocks_transactions.block_idx = blocks.idx + ON rosetta_blocks_transactions.block_idx = blocks.block_idx WHERE rosetta_blocks_transactions.rosetta_block_idx=:idx"#, ) .map_err(|e| format!("Unable to select block: {e:?}"))?; diff --git a/rs/rosetta-api/ledger_canister_blocks_synchronizer/tests/store_tests.rs b/rs/rosetta-api/ledger_canister_blocks_synchronizer/tests/store_tests.rs index cd2f8b69ab4..8f079de9e71 100644 --- a/rs/rosetta-api/ledger_canister_blocks_synchronizer/tests/store_tests.rs +++ b/rs/rosetta-api/ledger_canister_blocks_synchronizer/tests/store_tests.rs @@ -95,7 +95,8 @@ async fn store_coherence_test() { for hb in &scribe.blockchain { let hash = hb.hash.into_bytes().to_vec(); let parent_hash = hb.parent_hash.map(|ph| ph.into_bytes().to_vec()); - let command = "INSERT INTO blocks (hash, block, parent_hash, idx, verified, timestamp) VALUES (?1, ?2, ?3, ?4, FALSE, ?5)"; + let transaction = Block::decode(hb.block.clone()).unwrap().transaction; + let command = "INSERT INTO blocks (block_hash, encoded_block, parent_hash, block_idx, verified, timestamp,tx_hash,operation_type) VALUES (?1, ?2, ?3, ?4, FALSE, ?5,?6,?7)"; con.execute( command, params![ @@ -103,7 +104,9 @@ async fn store_coherence_test() { hb.block.clone().into_vec(), parent_hash, hb.index, - timestamp_to_iso8601(hb.timestamp) + timestamp_to_iso8601(hb.timestamp), + transaction.hash().into_bytes().to_vec(), + >::into(transaction.operation.clone()) ], ) .unwrap(); @@ -111,7 +114,6 @@ async fn store_coherence_test() { drop(con); for hb in &scribe.blockchain { assert_eq!(store.get_hashed_block(&hb.index).unwrap(), *hb); - assert_eq!(store.get_transaction_hash(&hb.index).unwrap(), None); assert!(store.get_all_accounts().unwrap().is_empty()); } let store = sqlite_on_disk_store(location); diff --git a/rs/rosetta-api/scripts/diff_rosetta_data.sh b/rs/rosetta-api/scripts/diff_rosetta_data.sh index 18af0fbf680..33f40041451 100755 --- a/rs/rosetta-api/scripts/diff_rosetta_data.sh +++ b/rs/rosetta-api/scripts/diff_rosetta_data.sh @@ -29,13 +29,13 @@ if [ ! -f "$ROSETTA_DB_NEW" ]; then exit 3 fi -last_block_to_check=$(sqlite3 "$ROSETTA_DB_OLD" 'select max(idx) from blocks') +last_block_to_check=$(sqlite3 "$ROSETTA_DB_OLD" 'select max(block_idx) from blocks') -QUERY="select hex(hash), hex(block), hex(parent_hash), idx, verified from blocks" +QUERY="select hex(block_hash), hex(encoded_block), hex(parent_hash), block_idx, verified from blocks" diff \ <(sqlite3 "$ROSETTA_DB_OLD" "$QUERY") \ - <(sqlite3 "$ROSETTA_DB_NEW" "$QUERY where idx <= $last_block_to_check") + <(sqlite3 "$ROSETTA_DB_NEW" "$QUERY where block_idx <= $last_block_to_check") RES=$? From 212aaa60b637f0ae1550a6a9e38f53a1b71537a2 Mon Sep 17 00:00:00 2001 From: NikolasHai <113891786+NikolasHai@users.noreply.github.com> Date: Fri, 16 Aug 2024 14:51:26 +0200 Subject: [PATCH 6/8] chore(ICP-Ledger): FI-1426: remove maximum number of accounts (#972) This MR proposes the following changes: 1. Remove the maximum number of accounts from the upgrade arguments This change is done, to close a possible vulnerability where the maximum number of accounts is set to 0 or a very small number. --- rs/rosetta-api/icp_ledger/ledger.did | 1 - rs/rosetta-api/icp_ledger/ledger/src/lib.rs | 3 --- rs/rosetta-api/icp_ledger/ledger/tests/tests.rs | 2 -- rs/rosetta-api/icp_ledger/src/lib.rs | 11 ----------- rs/rosetta-api/icp_ledger/tests/golden_nns_state.rs | 10 +++++----- rs/rosetta-api/icp_ledger/tests/tests.rs | 6 +----- rs/rosetta-api/icrc1/index-ng/tests/tests.rs | 1 - rs/rosetta-api/icrc1/ledger/ledger.did | 1 - rs/rosetta-api/icrc1/ledger/sm-tests/src/lib.rs | 1 - rs/rosetta-api/icrc1/ledger/src/lib.rs | 5 ----- 10 files changed, 6 insertions(+), 35 deletions(-) diff --git a/rs/rosetta-api/icp_ledger/ledger.did b/rs/rosetta-api/icp_ledger/ledger.did index 62725e3e49d..e35a627d6e5 100644 --- a/rs/rosetta-api/icp_ledger/ledger.did +++ b/rs/rosetta-api/icp_ledger/ledger.did @@ -350,7 +350,6 @@ type Value = variant { }; type UpgradeArgs = record { - maximum_number_of_accounts : opt nat64; icrc1_minting_account : opt Account; feature_flags : opt FeatureFlags; }; diff --git a/rs/rosetta-api/icp_ledger/ledger/src/lib.rs b/rs/rosetta-api/icp_ledger/ledger/src/lib.rs index 1539d818c5c..a4715c3d299 100644 --- a/rs/rosetta-api/icp_ledger/ledger/src/lib.rs +++ b/rs/rosetta-api/icp_ledger/ledger/src/lib.rs @@ -432,9 +432,6 @@ impl Ledger { } pub fn upgrade(&mut self, args: UpgradeArgs) { - if let Some(maximum_number_of_accounts) = args.maximum_number_of_accounts { - self.maximum_number_of_accounts = maximum_number_of_accounts; - } if let Some(icrc1_minting_account) = args.icrc1_minting_account { if Some(AccountIdentifier::from(icrc1_minting_account)) != self.minting_account_id { trap_with( diff --git a/rs/rosetta-api/icp_ledger/ledger/tests/tests.rs b/rs/rosetta-api/icp_ledger/ledger/tests/tests.rs index 85ed982cc27..02985342367 100644 --- a/rs/rosetta-api/icp_ledger/ledger/tests/tests.rs +++ b/rs/rosetta-api/icp_ledger/ledger/tests/tests.rs @@ -1119,7 +1119,6 @@ fn test_feature_flags() { canister_id, ledger_wasm.clone(), Encode!(&LedgerCanisterPayload::Upgrade(Some(UpgradeArgs { - maximum_number_of_accounts: None, icrc1_minting_account: None, feature_flags: Some(FeatureFlags { icrc2: false }), }))) @@ -1140,7 +1139,6 @@ fn test_feature_flags() { canister_id, ledger_wasm, Encode!(&LedgerCanisterPayload::Upgrade(Some(UpgradeArgs { - maximum_number_of_accounts: None, icrc1_minting_account: None, feature_flags: Some(FeatureFlags { icrc2: true }), }))) diff --git a/rs/rosetta-api/icp_ledger/src/lib.rs b/rs/rosetta-api/icp_ledger/src/lib.rs index e8b957a0d4c..fc3addb2e35 100644 --- a/rs/rosetta-api/icp_ledger/src/lib.rs +++ b/rs/rosetta-api/icp_ledger/src/lib.rs @@ -453,9 +453,6 @@ pub struct LedgerCanisterUpgradePayload(pub LedgerCanisterPayload); #[derive(Serialize, Deserialize, CandidType, Clone, Debug, PartialEq, Eq)] pub struct UpgradeArgs { - #[serde(default, skip_serializing_if = "Option::is_none")] - pub maximum_number_of_accounts: Option, - #[serde(default, skip_serializing_if = "Option::is_none")] pub icrc1_minting_account: Option, @@ -643,7 +640,6 @@ impl LedgerCanisterInitPayloadBuilder { } pub struct LedgerCanisterUpgradePayloadBuilder { - maximum_number_of_accounts: Option, icrc1_minting_account: Option, feature_flags: Option, } @@ -651,17 +647,11 @@ pub struct LedgerCanisterUpgradePayloadBuilder { impl LedgerCanisterUpgradePayloadBuilder { fn new() -> Self { Self { - maximum_number_of_accounts: None, icrc1_minting_account: None, feature_flags: None, } } - pub fn maximum_number_of_accounts(mut self, maximum_number_of_accounts: usize) -> Self { - self.maximum_number_of_accounts = Some(maximum_number_of_accounts); - self - } - pub fn icrc1_minting_account(mut self, minting_account: Account) -> Self { self.icrc1_minting_account = Some(minting_account); self @@ -670,7 +660,6 @@ impl LedgerCanisterUpgradePayloadBuilder { pub fn build(self) -> Result { Ok(LedgerCanisterUpgradePayload( LedgerCanisterPayload::Upgrade(Some(UpgradeArgs { - maximum_number_of_accounts: self.maximum_number_of_accounts, icrc1_minting_account: self.icrc1_minting_account, feature_flags: self.feature_flags, })), diff --git a/rs/rosetta-api/icp_ledger/tests/golden_nns_state.rs b/rs/rosetta-api/icp_ledger/tests/golden_nns_state.rs index 9b98f1b9ef4..05b3dfd5bc9 100644 --- a/rs/rosetta-api/icp_ledger/tests/golden_nns_state.rs +++ b/rs/rosetta-api/icp_ledger/tests/golden_nns_state.rs @@ -76,11 +76,11 @@ fn upgrade_index(state_machine: &StateMachine, wasm_bytes: Vec) { } fn upgrade_ledger(state_machine: &StateMachine, wasm_bytes: Vec) { - let ledger_upgrade_args = LedgerCanisterPayload::Upgrade(Some(UpgradeArgs { - maximum_number_of_accounts: None, - icrc1_minting_account: None, - feature_flags: Some(FeatureFlags { icrc2: true }), - })); + let ledger_upgrade_args: LedgerCanisterPayload = + LedgerCanisterPayload::Upgrade(Some(UpgradeArgs { + icrc1_minting_account: None, + feature_flags: Some(FeatureFlags { icrc2: true }), + })); state_machine .upgrade_canister( diff --git a/rs/rosetta-api/icp_ledger/tests/tests.rs b/rs/rosetta-api/icp_ledger/tests/tests.rs index 38f8eb00d08..4020e645c61 100644 --- a/rs/rosetta-api/icp_ledger/tests/tests.rs +++ b/rs/rosetta-api/icp_ledger/tests/tests.rs @@ -314,10 +314,7 @@ fn upgrade_test() { ledger .upgrade_to_self_binary( CandidOne(Some( - LedgerCanisterUpgradePayload::builder() - .maximum_number_of_accounts(28_000_000) - .build() - .unwrap(), + LedgerCanisterUpgradePayload::builder().build().unwrap(), )) .into_bytes() .unwrap(), @@ -336,7 +333,6 @@ fn upgrade_test() { .upgrade_to_self_binary( CandidOne(Some( LedgerCanisterUpgradePayload::builder() - .maximum_number_of_accounts(28_000_000) .icrc1_minting_account(minting_account_principal.into()) .build() .unwrap(), diff --git a/rs/rosetta-api/icrc1/index-ng/tests/tests.rs b/rs/rosetta-api/icrc1/index-ng/tests/tests.rs index 07f224446b0..5b9b4e4243a 100644 --- a/rs/rosetta-api/icrc1/index-ng/tests/tests.rs +++ b/rs/rosetta-api/icrc1/index-ng/tests/tests.rs @@ -54,7 +54,6 @@ fn upgrade_ledger( change_fee_collector, max_memo_length: None, feature_flags: None, - maximum_number_of_accounts: None, accounts_overflow_trim_quantity: None, change_archive_options: None, })); diff --git a/rs/rosetta-api/icrc1/ledger/ledger.did b/rs/rosetta-api/icrc1/ledger/ledger.did index 53f49e45d10..ac3a7bff087 100644 --- a/rs/rosetta-api/icrc1/ledger/ledger.did +++ b/rs/rosetta-api/icrc1/ledger/ledger.did @@ -144,7 +144,6 @@ type UpgradeArgs = record { change_fee_collector : opt ChangeFeeCollector; max_memo_length : opt nat16; feature_flags : opt FeatureFlags; - maximum_number_of_accounts: opt nat64; accounts_overflow_trim_quantity: opt nat64; change_archive_options : opt ChangeArchiveOptions; }; diff --git a/rs/rosetta-api/icrc1/ledger/sm-tests/src/lib.rs b/rs/rosetta-api/icrc1/ledger/sm-tests/src/lib.rs index aadaa816f1d..8c922131f42 100644 --- a/rs/rosetta-api/icrc1/ledger/sm-tests/src/lib.rs +++ b/rs/rosetta-api/icrc1/ledger/sm-tests/src/lib.rs @@ -109,7 +109,6 @@ pub struct UpgradeArgs { pub transfer_fee: Option, pub change_fee_collector: Option, pub feature_flags: Option, - pub maximum_number_of_accounts: Option, pub accounts_overflow_trim_quantity: Option, pub change_archive_options: Option, } diff --git a/rs/rosetta-api/icrc1/ledger/src/lib.rs b/rs/rosetta-api/icrc1/ledger/src/lib.rs index ab1ce2202c7..12598297e28 100644 --- a/rs/rosetta-api/icrc1/ledger/src/lib.rs +++ b/rs/rosetta-api/icrc1/ledger/src/lib.rs @@ -314,8 +314,6 @@ pub struct UpgradeArgs { #[serde(default, skip_serializing_if = "Option::is_none")] pub feature_flags: Option, #[serde(default, skip_serializing_if = "Option::is_none")] - pub maximum_number_of_accounts: Option, - #[serde(default, skip_serializing_if = "Option::is_none")] pub accounts_overflow_trim_quantity: Option, #[serde(default, skip_serializing_if = "Option::is_none")] pub change_archive_options: Option, @@ -649,9 +647,6 @@ impl Ledger { } self.feature_flags = feature_flags; } - if let Some(maximum_number_of_accounts) = args.maximum_number_of_accounts { - self.maximum_number_of_accounts = maximum_number_of_accounts.try_into().unwrap(); - } if let Some(accounts_overflow_trim_quantity) = args.accounts_overflow_trim_quantity { self.accounts_overflow_trim_quantity = accounts_overflow_trim_quantity.try_into().unwrap(); From 08211fdbbfec5031f21506869a41b28c087fbc80 Mon Sep 17 00:00:00 2001 From: mraszyk <31483726+mraszyk@users.noreply.github.com> Date: Fri, 16 Aug 2024 15:22:50 +0200 Subject: [PATCH 7/8] feat(PocketIC): artificial delay in auto progress mode of PocketIC (#970) This PR adds support for an artificial delay in auto progress mode of PocketIC specifying the minimum delay between consecutive rounds in auto progress mode. --- packages/pocket-ic/src/common/rest.rs | 5 +++++ packages/pocket-ic/src/nonblocking.rs | 15 +++++++++------ rs/pocket_ic_server/CHANGELOG.md | 2 ++ rs/pocket_ic_server/src/state_api/routes.rs | 18 ++++++++++++++---- rs/pocket_ic_server/src/state_api/state.rs | 16 ++++++++++++++-- .../rosetta-integration-tests/tests/tests.rs | 4 +--- 6 files changed, 45 insertions(+), 15 deletions(-) diff --git a/packages/pocket-ic/src/common/rest.rs b/packages/pocket-ic/src/common/rest.rs index 714c4f9c41d..bc73f074817 100644 --- a/packages/pocket-ic/src/common/rest.rs +++ b/packages/pocket-ic/src/common/rest.rs @@ -14,6 +14,11 @@ use std::path::PathBuf; pub type InstanceId = usize; +#[derive(Debug, Clone, Eq, Hash, PartialEq, Serialize, Deserialize, JsonSchema)] +pub struct AutoProgressConfig { + pub artificial_delay_ms: Option, +} + #[derive(Debug, Clone, Eq, Hash, PartialEq, Serialize, Deserialize, JsonSchema)] pub enum HttpGatewayBackend { Replica(String), diff --git a/packages/pocket-ic/src/nonblocking.rs b/packages/pocket-ic/src/nonblocking.rs index 1531c70e0c7..86d12b9615e 100644 --- a/packages/pocket-ic/src/nonblocking.rs +++ b/packages/pocket-ic/src/nonblocking.rs @@ -1,9 +1,9 @@ use crate::common::rest::{ - ApiResponse, BlobCompression, BlobId, CanisterHttpRequest, CreateHttpGatewayResponse, - CreateInstanceResponse, ExtendedSubnetConfigSet, HttpGatewayBackend, HttpGatewayConfig, - HttpGatewayInfo, HttpsConfig, InstanceConfig, InstanceId, MockCanisterHttpResponse, - RawAddCycles, RawCanisterCall, RawCanisterHttpRequest, RawCanisterId, RawCanisterResult, - RawCycles, RawEffectivePrincipal, RawMessageId, RawMockCanisterHttpResponse, + ApiResponse, AutoProgressConfig, BlobCompression, BlobId, CanisterHttpRequest, + CreateHttpGatewayResponse, CreateInstanceResponse, ExtendedSubnetConfigSet, HttpGatewayBackend, + HttpGatewayConfig, HttpGatewayInfo, HttpsConfig, InstanceConfig, InstanceId, + MockCanisterHttpResponse, RawAddCycles, RawCanisterCall, RawCanisterHttpRequest, RawCanisterId, + RawCanisterResult, RawCycles, RawEffectivePrincipal, RawMessageId, RawMockCanisterHttpResponse, RawSetStableMemory, RawStableMemory, RawSubmitIngressResult, RawSubnetId, RawTime, RawVerifyCanisterSigArg, RawWasmResult, SubnetId, Topology, }; @@ -290,7 +290,10 @@ impl PocketIc { let now = std::time::SystemTime::now(); self.set_time(now).await; let endpoint = "auto_progress"; - self.post::<(), _>(endpoint, "").await; + let auto_progress_config = AutoProgressConfig { + artificial_delay_ms: None, + }; + self.post::<(), _>(endpoint, auto_progress_config).await; self.instance_url() } diff --git a/rs/pocket_ic_server/CHANGELOG.md b/rs/pocket_ic_server/CHANGELOG.md index 8d4952a65e9..29986b14917 100644 --- a/rs/pocket_ic_server/CHANGELOG.md +++ b/rs/pocket_ic_server/CHANGELOG.md @@ -16,10 +16,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - New argument `ip_addr` of the endpoint `/http_gateway` to specify the IP address at which the HTTP gateway should listen (defaults to `127.0.0.1`). - New GET endpoint `/http_gateway` listing all HTTP gateways and their details. - Support for query statistics in the management canister. +- The argument of the endpoint `/instances//auto_progress` becomes a struct with an optional field `artificial_delay_ms` specifying the minimum delay between consecutive rounds in auto progress mode. - Support for *verified application* subnets. ### Changed - The argument `listen_at` of the endpoint `/http_gateway` has been renamed to `port`. +- The endpoint `/instances//auto_progress` returns an error if the corresponding PocketIC instance is already in auto progress mode. diff --git a/rs/pocket_ic_server/src/state_api/routes.rs b/rs/pocket_ic_server/src/state_api/routes.rs index fc7348976dd..9c44aaaffd5 100644 --- a/rs/pocket_ic_server/src/state_api/routes.rs +++ b/rs/pocket_ic_server/src/state_api/routes.rs @@ -34,8 +34,8 @@ use hyper::header; use ic_http_endpoints_public::cors_layer; use ic_types::CanisterId; use pocket_ic::common::rest::{ - self, ApiResponse, ExtendedSubnetConfigSet, HttpGatewayConfig, HttpGatewayDetails, - InstanceConfig, MockCanisterHttpResponse, RawAddCycles, RawCanisterCall, + self, ApiResponse, AutoProgressConfig, ExtendedSubnetConfigSet, HttpGatewayConfig, + HttpGatewayDetails, InstanceConfig, MockCanisterHttpResponse, RawAddCycles, RawCanisterCall, RawCanisterHttpRequest, RawCanisterId, RawCanisterResult, RawCycles, RawMessageId, RawMockCanisterHttpResponse, RawSetStableMemory, RawStableMemory, RawSubmitIngressResult, RawSubnetId, RawTime, RawWasmResult, Topology, @@ -1152,9 +1152,19 @@ pub async fn stop_http_gateway( pub async fn auto_progress( State(AppState { api_state, .. }): State, Path(id): Path, + extract::Json(auto_progress_config): extract::Json, ) -> (StatusCode, Json>) { - api_state.auto_progress(id).await; - (StatusCode::OK, Json(ApiResponse::Success(()))) + if let Err(e) = api_state + .auto_progress(id, auto_progress_config.artificial_delay_ms) + .await + { + ( + StatusCode::BAD_REQUEST, + Json(ApiResponse::Error { message: e }), + ) + } else { + (StatusCode::OK, Json(ApiResponse::Success(()))) + } } pub async fn stop_progress( diff --git a/rs/pocket_ic_server/src/state_api/state.rs b/rs/pocket_ic_server/src/state_api/state.rs index 837ca572b78..3bca3c8fcdb 100644 --- a/rs/pocket_ic_server/src/state_api/state.rs +++ b/rs/pocket_ic_server/src/state_api/state.rs @@ -1124,7 +1124,12 @@ impl ApiState { Some(()) } - pub async fn auto_progress(&self, instance_id: InstanceId) { + pub async fn auto_progress( + &self, + instance_id: InstanceId, + artificial_delay_ms: Option, + ) -> Result<(), String> { + let artificial_delay = Duration::from_millis(artificial_delay_ms.unwrap_or_default()); let progress_threads = self.progress_threads.read().await; let mut progress_thread = progress_threads[instance_id].lock().await; let instances = self.instances.clone(); @@ -1161,13 +1166,20 @@ impl ApiState { return; } let duration = start.elapsed(); - sleep(std::cmp::max(duration, MIN_OPERATION_DELAY)).await; + sleep(std::cmp::max( + duration, + std::cmp::max(artificial_delay, MIN_OPERATION_DELAY), + )) + .await; if received_stop_signal(&mut rx) { return; } } }); *progress_thread = Some(ProgressThread { handle, sender: tx }); + Ok(()) + } else { + Err("Auto progress mode has already been enabled.".to_string()) } } diff --git a/rs/rosetta-api/icp_ledger/rosetta-integration-tests/tests/tests.rs b/rs/rosetta-api/icp_ledger/rosetta-integration-tests/tests/tests.rs index ab05ba5d1f9..b0a68ad7154 100644 --- a/rs/rosetta-api/icp_ledger/rosetta-integration-tests/tests/tests.rs +++ b/rs/rosetta-api/icp_ledger/rosetta-integration-tests/tests/tests.rs @@ -862,7 +862,6 @@ async fn test_rosetta_blocks_dont_contain_transactions_duplicates() { #[tokio::test] async fn test_query_block_range() { let env = TestEnv::setup(false, true).await.unwrap(); - env.pocket_ic.auto_progress().await; let minter = test_identity() .sender() @@ -1224,11 +1223,10 @@ async fn test_network_status_single_genesis_transaction() { let mut env = TestEnv::setup(false, true).await.unwrap(); let t1 = env.pocket_ic.get_time().await; // We need to advance the time to make sure only a single transaction gets into the genesis block - env.pocket_ic.auto_progress().await; tokio::time::sleep(Duration::from_secs(1)).await; - env.pocket_ic.stop_progress().await; let t2 = env.pocket_ic.get_time().await; assert!(t1 < t2); + env.pocket_ic.stop_progress().await; // We want two transactions with unique tx hashes env.icrc1_transfers(vec![ TransferArg { From 496affe0885974776400883bb5840d0909fe74f2 Mon Sep 17 00:00:00 2001 From: Martin Raszyk Date: Fri, 16 Aug 2024 15:40:45 +0200 Subject: [PATCH 8/8] changelog --- packages/pocket-ic/CHANGELOG.md | 2 +- rs/pocket_ic_server/CHANGELOG.md | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/pocket-ic/CHANGELOG.md b/packages/pocket-ic/CHANGELOG.md index 6b6cd21979e..1e808c28870 100644 --- a/packages/pocket-ic/CHANGELOG.md +++ b/packages/pocket-ic/CHANGELOG.md @@ -8,7 +8,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased ### Added -- Support for *verified application* subnets: the library function `PocketIcBuilder::with_verified_application_subnet` adds a verified application subnet to the PocketIC instance; +- Support for verified application subnets: the library function `PocketIcBuilder::with_verified_application_subnet` adds a verified application subnet to the PocketIC instance; the library function `PocketIc::get_verified_app_subnets` lists all verified application subnets of the PocketIC instance. diff --git a/rs/pocket_ic_server/CHANGELOG.md b/rs/pocket_ic_server/CHANGELOG.md index 29986b14917..7219537fd70 100644 --- a/rs/pocket_ic_server/CHANGELOG.md +++ b/rs/pocket_ic_server/CHANGELOG.md @@ -17,7 +17,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - New GET endpoint `/http_gateway` listing all HTTP gateways and their details. - Support for query statistics in the management canister. - The argument of the endpoint `/instances//auto_progress` becomes a struct with an optional field `artificial_delay_ms` specifying the minimum delay between consecutive rounds in auto progress mode. -- Support for *verified application* subnets. +- Support for verified application subnets: the record types `SubnetConfigSet` and `ExtendedSubnetConfigSet` contain a new field `verified_application` specifying verified application subnets; + the enumeration type `SubnetKind` has a new variant `VerifiedApplication`. ### Changed - The argument `listen_at` of the endpoint `/http_gateway` has been renamed to `port`.