Skip to content

Commit

Permalink
Merge branch 'maciej-maxblocks-index' into 'master'
Browse files Browse the repository at this point in the history
feat(ICP-Index): limit endpoints returning blocks to 50 blocks/request for ingress replicated queries

 

See merge request dfinity-lab/public/ic!19370
  • Loading branch information
maciejdfinity committed May 29, 2024
2 parents 522341a + d66a6bd commit b9a0f18
Show file tree
Hide file tree
Showing 5 changed files with 221 additions and 11 deletions.
3 changes: 3 additions & 0 deletions Cargo.lock

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

5 changes: 4 additions & 1 deletion rs/rosetta-api/icp_ledger/index/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@ DEPENDENCIES = [
"//rs/rosetta-api/icrc1/index-ng",
"//rs/rosetta-api/ledger_core",
"//rs/rust_canisters/canister_log",
"//rs/rust_canisters/dfn_core",
"//rs/rust_canisters/http_types",
"//rs/types/base_types",
"@crate_index//:candid",
"@crate_index//:ciborium",
"@crate_index//:ic-cdk",
Expand All @@ -26,9 +28,10 @@ DEPENDENCIES = [

DEV_DEPENDENCIES = [
"//rs/rosetta-api/ledger_canister_core",
"//rs/rust_canisters/dfn_protobuf",
"//rs/rust_canisters/on_wire",
"//rs/state_machine_tests",
"//rs/test_utilities/load_wasm",
"//rs/types/base_types",
"@crate_index//:candid_parser",
]

Expand Down
5 changes: 4 additions & 1 deletion rs/rosetta-api/icp_ledger/index/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ path = "src/main.rs"
[dependencies]
candid = { workspace = true }
ciborium = { workspace = true }
dfn_core = { path = "../../../rust_canisters/dfn_core" }
ic-base-types = { path = "../../../types/base_types" }
ic-canister-log = { path = "../../../rust_canisters/canister_log" }
ic-canisters-http-types = { path = "../../../rust_canisters/http_types" }
ic-cdk = { workspace = true }
Expand All @@ -32,7 +34,8 @@ serde_json = { workspace = true }

[dev-dependencies]
candid_parser = { workspace = true }
ic-base-types = { path = "../../../types/base_types" }
dfn_protobuf = { path = "../../../rust_canisters/dfn_protobuf" }
on_wire = { path = "../../../rust_canisters/on_wire" }
ic-ledger-canister-core = { path = "../../ledger_canister_core" }
ic-state-machine-tests = { path = "../../../state_machine_tests" }
ic-test-utilities-load-wasm = { path = "../../../test_utilities/load_wasm" }
5 changes: 3 additions & 2 deletions rs/rosetta-api/icp_ledger/index/src/main.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use candid::{candid_method, Principal};
use dfn_core::api::caller;
use ic_canister_log::{export as export_logs, log};
use ic_canisters_http_types::{HttpRequest, HttpResponse, HttpResponseBuilder};
use ic_cdk_macros::{init, post_upgrade, query};
Expand Down Expand Up @@ -490,7 +491,7 @@ fn get_block_range_from_stable_memory(
start: u64,
length: u64,
) -> Result<Vec<EncodedBlock>, String> {
let length = length.min(DEFAULT_MAX_BLOCKS_PER_RESPONSE as u64);
let length = length.min(icp_ledger::max_blocks_per_request(&caller()) as u64);
with_blocks(|blocks| {
let limit = blocks.len().min(start.saturating_add(length));
let mut res = vec![];
Expand Down Expand Up @@ -588,7 +589,7 @@ fn get_account_identifier_transactions(
) -> GetAccountIdentifierTransactionsResult {
let length = arg
.max_results
.min(DEFAULT_MAX_BLOCKS_PER_RESPONSE as u64)
.min(icp_ledger::max_blocks_per_request(&caller()) as u64)
.min(usize::MAX as u64) as usize;
// TODO: deal with the user setting start to u64::MAX
let start = arg.start.map_or(u64::MAX, |n| n);
Expand Down
214 changes: 207 additions & 7 deletions rs/rosetta-api/icp_ledger/index/tests/tests.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use candid::{Decode, Encode, Nat};
use candid::{Decode, Encode, Nat, Principal};
use ic_base_types::{CanisterId, PrincipalId};
use ic_icp_index::{
GetAccountIdentifierTransactionsArgs, GetAccountIdentifierTransactionsResponse,
Expand All @@ -12,7 +12,7 @@ use ic_ledger_core::Tokens;
use ic_state_machine_tests::StateMachine;
use icp_ledger::{
AccountIdentifier, GetBlocksArgs, QueryBlocksResponse, QueryEncodedBlocksResponse, Transaction,
MAX_BLOCKS_PER_REQUEST,
MAX_BLOCKS_PER_INGRESS_REPLICATED_QUERY_REQUEST, MAX_BLOCKS_PER_REQUEST,
};
use icp_ledger::{FeatureFlags, LedgerCanisterInitPayload, Memo, Operation};
use icrc_ledger_types::icrc1::account::Account;
Expand All @@ -21,6 +21,7 @@ use icrc_ledger_types::icrc2::approve::{ApproveArgs, ApproveError};
use icrc_ledger_types::icrc2::transfer_from::{TransferFromArgs, TransferFromError};
use icrc_ledger_types::icrc3::blocks::GetBlocksRequest;
use num_traits::cast::ToPrimitive;
use on_wire::FromWire;
use serde_bytes::ByteBuf;
use std::collections::HashMap;
use std::convert::TryFrom;
Expand Down Expand Up @@ -184,6 +185,17 @@ fn status(env: &StateMachine, index_id: CanisterId) -> Status {
Decode!(&res, Status).expect("Failed to decode status response")
}

fn icp_ledger_tip(env: &StateMachine, ledger_id: CanisterId) -> u64 {
let res = env
.query(ledger_id, "tip_of_chain_pb", vec![])
.expect("Failed to send tip_of_chain_pb request")
.bytes();
let tip: icp_ledger::TipOfChainRes = dfn_protobuf::ProtoBuf::from_bytes(res)
.map(|c| c.0)
.expect("failed to decode tip_of_chain_pb result");
tip.tip_index
}

fn icp_get_blocks(env: &StateMachine, ledger_id: CanisterId) -> Vec<icp_ledger::Block> {
let req = GetBlocksArgs {
start: 0u64,
Expand Down Expand Up @@ -273,15 +285,34 @@ fn icp_query_blocks(env: &StateMachine, ledger_id: CanisterId) -> Vec<icp_ledger
}

fn index_get_blocks(env: &StateMachine, index_id: CanisterId) -> Vec<icp_ledger::Block> {
let query = |req: Vec<u8>| {
env.query(index_id, "get_blocks", req)
.expect("Failed to send get_blocks request")
.bytes()
};
call_index_get_blocks(&query)
}

fn index_get_blocks_update(
env: &StateMachine,
index_id: CanisterId,
caller: Principal,
) -> Vec<icp_ledger::Block> {
let update = |req: Vec<u8>| {
env.execute_ingress_as(PrincipalId(caller), index_id, "get_blocks", req)
.expect("Failed to send get_blocks request")
.bytes()
};
call_index_get_blocks(&update)
}

fn call_index_get_blocks(query_or_update: &dyn Fn(Vec<u8>) -> Vec<u8>) -> Vec<icp_ledger::Block> {
let req = GetBlocksRequest {
start: 0u8.into(),
length: u64::MAX.into(),
};
let req = Encode!(&req).expect("Failed to encode GetBlocksRequest");
let res = env
.query(index_id, "get_blocks", req)
.expect("Failed to send get_blocks request")
.bytes();
let res = query_or_update(req);
Decode!(&res, ic_icp_index::GetBlocksResponse)
.expect("Failed to decode ic_icp_index::GetBlocksResponse")
.blocks
Expand All @@ -291,6 +322,106 @@ fn index_get_blocks(env: &StateMachine, index_id: CanisterId) -> Vec<icp_ledger:
.unwrap()
}

fn get_account_id_transactions_len(
env: &StateMachine,
index_id: CanisterId,
account: &Account,
) -> usize {
let query = |req: Vec<u8>| {
env.query(index_id, "get_account_identifier_transactions", req)
.expect("Failed to send get_account_identifier_transactions request")
.bytes()
};
call_get_account_id_transactions(&query, account)
}

fn get_account_id_transactions_update_len(
env: &StateMachine,
index_id: CanisterId,
caller: Principal,
account: &Account,
) -> usize {
let update = |req: Vec<u8>| {
env.execute_ingress_as(
PrincipalId(caller),
index_id,
"get_account_identifier_transactions",
req,
)
.expect("Failed to send get_account_identifier_transactions request")
.bytes()
};
call_get_account_id_transactions(&update, account)
}

fn call_get_account_id_transactions(
query_or_update: &dyn Fn(Vec<u8>) -> Vec<u8>,
account: &Account,
) -> usize {
let req = GetAccountIdentifierTransactionsArgs {
start: None,
max_results: u64::MAX,
account_identifier: (*account).into(),
};
let req = Encode!(&req).expect("Failed to encode GetAccountIdentifierTransactionsArgs");
let res = query_or_update(req);
Decode!(&res, ic_icp_index::GetAccountIdentifierTransactionsResult)
.expect("Failed to decode ic_icp_index::GetAccountIdentifierTransactionsResult")
.unwrap()
.transactions
.len()
}

fn get_account_transactions_len(
env: &StateMachine,
index_id: CanisterId,
account: &Account,
) -> usize {
let query = |req: Vec<u8>| {
env.query(index_id, "get_account_transactions", req)
.expect("Failed to send get_account_transactions request")
.bytes()
};
call_get_account_transactions(&query, account)
}

fn get_account_transactions_update_len(
env: &StateMachine,
index_id: CanisterId,
caller: Principal,
account: &Account,
) -> usize {
let update = |req: Vec<u8>| {
env.execute_ingress_as(
PrincipalId(caller),
index_id,
"get_account_transactions",
req,
)
.expect("Failed to send get_account_transactions request")
.bytes()
};
call_get_account_transactions(&update, account)
}

fn call_get_account_transactions(
query_or_update: &dyn Fn(Vec<u8>) -> Vec<u8>,
account: &Account,
) -> usize {
let req = GetAccountTransactionsArgs {
start: None,
max_results: u64::MAX.into(),
account: *account,
};
let req = Encode!(&req).expect("Failed to encode GetAccountTransactionsArgs");
let res = query_or_update(req);
Decode!(&res, ic_icp_index::GetAccountTransactionsResult)
.expect("Failed to decode ic_icp_index::GetAccountTransactionsResult")
.unwrap()
.transactions
.len()
}

fn transfer(
env: &StateMachine,
ledger_id: CanisterId,
Expand Down Expand Up @@ -435,7 +566,7 @@ fn wait_until_sync_is_completed(env: &StateMachine, index_id: CanisterId, ledger
env.advance_time(SYNC_STEP_SECONDS);
env.tick();
num_blocks_synced = status(env, index_id).num_blocks_synced;
chain_length = icp_get_blocks(env, ledger_id).len() as u64;
chain_length = icp_ledger_tip(env, ledger_id) + 1;
if num_blocks_synced == chain_length {
return;
}
Expand Down Expand Up @@ -1559,3 +1690,72 @@ fn test_post_upgrade_start_timer() {
transfer(env, ledger_id, account(1, 0), account(2, 0), 2_000_000);
wait_until_sync_is_completed(env, index_id, ledger_id);
}

#[test]
fn check_block_endpoint_limits() {
// check that the index canister can incrementally get the blocks from the ledger.

let mut initial_balances = HashMap::new();
initial_balances.insert(
AccountIdentifier::from(account(1, 0)),
Tokens::from_e8s(1_000_000_000_000),
);
let env = &StateMachine::new();
let ledger_id = install_ledger(env, initial_balances, default_archive_options());
let index_id = install_index(env, ledger_id);

for _ in 0..MAX_BLOCKS_PER_REQUEST {
transfer(env, ledger_id, account(1, 0), account(2, 0), 1);
}
wait_until_sync_is_completed(env, index_id, ledger_id);

let user_principal =
Principal::from_text("luwgt-ouvkc-k5rx5-xcqkq-jx5hm-r2rj2-ymqjc-pjvhb-kij4p-n4vms-gqe")
.unwrap();
let canister_principal = Principal::from_text("2chl6-4hpzw-vqaaa-aaaaa-c").unwrap();

// get_blocks
let blocks = index_get_blocks(env, index_id);
assert_eq!(blocks.len(), MAX_BLOCKS_PER_REQUEST);

let blocks = index_get_blocks_update(env, index_id, canister_principal);
assert_eq!(blocks.len(), MAX_BLOCKS_PER_REQUEST);

let blocks = index_get_blocks_update(env, index_id, user_principal);
assert_eq!(
blocks.len(),
MAX_BLOCKS_PER_INGRESS_REPLICATED_QUERY_REQUEST
);

// get_account_identifier_transactions
assert_eq!(
get_account_id_transactions_len(env, index_id, &account(2, 0)),
MAX_BLOCKS_PER_REQUEST
);

assert_eq!(
get_account_id_transactions_update_len(env, index_id, canister_principal, &account(2, 0)),
MAX_BLOCKS_PER_REQUEST
);

assert_eq!(
get_account_id_transactions_update_len(env, index_id, user_principal, &account(2, 0)),
MAX_BLOCKS_PER_INGRESS_REPLICATED_QUERY_REQUEST
);

// get_account_transactions
assert_eq!(
get_account_transactions_len(env, index_id, &account(2, 0)),
MAX_BLOCKS_PER_REQUEST
);

assert_eq!(
get_account_transactions_update_len(env, index_id, canister_principal, &account(2, 0)),
MAX_BLOCKS_PER_REQUEST
);

assert_eq!(
get_account_transactions_update_len(env, index_id, user_principal, &account(2, 0)),
MAX_BLOCKS_PER_INGRESS_REPLICATED_QUERY_REQUEST
);
}

0 comments on commit b9a0f18

Please sign in to comment.