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.

Sign up for GitHub

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

Jump to bottom

feat!: remove support for bitcoin query API #3788

Merged
merged 6 commits into from
Jun 12, 2024
Merged

feat!: remove support for bitcoin query API #3788

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
eda434a
rm the e2e test cases
lwshang Jun 11, 2024
8c3ca9d
remove code in call.rs
lwshang Jun 11, 2024
731ad8f
changelog
lwshang Jun 11, 2024
7430716
Update src/dfx/src/commands/canister/call.rs
lwshang Jun 12, 2024
7cc986e
update agent
lwshang Jun 12, 2024
eb66970
Merge branch 'master' into lwshang/rm_bitcoin_query
lwshang Jun 12, 2024
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
4 changes: 4 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

# UNRELEASED

### feat!: remove support for bitcoin query API

`dfx call --query aaaaa-aa bitcoin_get_balance_query/bitcoin_get_utxos_query` will result in an error.

### fix: simplified log message when using the default shared network configuration

Now displays `Using the default configuration for the local shared network.`
Expand Down
23 changes: 0 additions & 23 deletions e2e/tests-dfx/bitcoin.bash
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,27 +196,4 @@ set_local_network_bitcoin_enabled() {
# The error message indicates that the argument is in correct format, only the inner transaction is malformed.
assert_command_fail dfx canister call --wallet "$WALLET_ID" aaaaa-aa --candid bitcoin.did bitcoin_send_transaction '(record { transaction = vec {0:nat8}; network = variant { regtest } })'
assert_contains "send_transaction failed: MalformedTransaction"

# the query Bitcoin API can be called directly

# bitcoin_get_balance_query
assert_command dfx canister call --query aaaaa-aa --candid bitcoin.did bitcoin_get_balance_query '(
record {
network = variant { regtest };
address = "bcrt1qu58aj62urda83c00eylc6w34yl2s6e5rkzqet7";
min_confirmations = opt (1 : nat32);
}
)'
# shellcheck disable=SC2154
assert_eq "(0 : nat64)" "$stdout"

# bitcoin_get_balance_query
assert_command dfx canister call --query aaaaa-aa --candid bitcoin.did bitcoin_get_utxos_query '(
record {
network = variant { regtest };
filter = opt variant { min_confirmations = 1 : nat32 };
address = "bcrt1qu58aj62urda83c00eylc6w34yl2s6e5rkzqet7";
}
)'
assert_contains "tip_height = 0 : nat32;"
}
49 changes: 0 additions & 49 deletions e2e/tests-dfx/call.bash
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,55 +237,6 @@ teardown() {
assert_match '("Hello, you!")'
}

@test "call management canister - bitcoin query API on the IC mainnet" {
WARNING="call to the management canister cannot be benefit from the \"Replica Signed Queries\" feature.
The response might not be trustworthy.
If you want to get reliable result, you can make an update call to the secure alternative:"
# bitcoin_get_balance_query
## bitcoin mainnet
assert_command dfx canister call --network ic --query aaaaa-aa bitcoin_get_balance_query '(
record {
network = variant { mainnet };
address = "bcrt1qu58aj62urda83c00eylc6w34yl2s6e5rkzqet7";
}
)'
# shellcheck disable=SC2154
assert_contains "bitcoin_get_balance_query $WARNING bitcoin_get_balance" "$stderr"

# TODO: re-enable when testnet back to normal, tracking at https://dfinity.atlassian.net/browse/SDKTG-323

# ## bitcoin testnet
# assert_command dfx canister call --network ic --query aaaaa-aa bitcoin_get_balance_query '(
# record {
# network = variant { testnet };
# address = "bcrt1qu58aj62urda83c00eylc6w34yl2s6e5rkzqet7";
# }
# )'
# # shellcheck disable=SC2154
# assert_contains "bitcoin_get_balance_query $WARNING bitcoin_get_balance" "$stderr"

# bitcoin_get_utxos_query
## bitcoin mainnet
assert_command dfx canister call --network ic --query aaaaa-aa bitcoin_get_utxos_query '(
record {
network = variant { mainnet };
address = "bcrt1qu58aj62urda83c00eylc6w34yl2s6e5rkzqet7";
}
)'
# shellcheck disable=SC2154
assert_contains "bitcoin_get_utxos_query $WARNING bitcoin_get_utxos" "$stderr"

# ## bitcoin testnet
# assert_command dfx canister call --network ic --query aaaaa-aa bitcoin_get_utxos_query '(
# record {
# network = variant { testnet };
# address = "bcrt1qu58aj62urda83c00eylc6w34yl2s6e5rkzqet7";
# }
# )'
# # shellcheck disable=SC2154
# assert_contains "bitcoin_get_utxos_query $WARNING bitcoin_get_utxos" "$stderr"
}

@test "inter-canister calls" {
dfx_new_rust inter
install_asset inter
Expand Down
52 changes: 8 additions & 44 deletions src/dfx/src/commands/canister/call.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,9 +194,13 @@ pub fn get_effective_canister_id(
Decode!(arg_value, In).context("Argument is not valid for InstallChunkedCode")?;
Ok(in_args.target_canister)
}
MgmtMethod::BitcoinGetUtxosQuery | MgmtMethod::BitcoinGetBalanceQuery => {
Ok(CanisterId::management_canister())
}
// TODO: SDKTG-347
// BitcoinGetUtxosQuery and BitcoinGetBalanceQuery are to be removed.
// We should remove this match arm when they are removed from `MgmtMethod` in ic-utils.
_ => Err(anyhow!(
lwshang marked this conversation as resolved.
Show resolved Hide resolved
"Attempted to call an unsupported management canister method: {}",
method_name
)),
}
}

Expand Down Expand Up @@ -250,7 +254,6 @@ pub async fn exec(
opts.always_assist,
)?;

let mut disable_verify_query_signatures = false;
let effective_canister_id = if canister_id == CanisterId::management_canister() {
let management_method = MgmtMethod::from_str(method_name).map_err(|_| {
anyhow!(
Expand All @@ -259,39 +262,6 @@ pub async fn exec(
)
})?;

if matches!(
management_method,
MgmtMethod::BitcoinGetBalanceQuery | MgmtMethod::BitcoinGetUtxosQuery
) {
match call_sender {
CallSender::SelectedId => {
// Query calls to those two bitcoin query methods can not make a following read_state call.
// We have to use a non-verify_query_signatures agent to make the call.
// Rust's lifetime rule forces us to create the special env/agent outside this block.
// agent = non_verify_query_signatures_agent;
disable_verify_query_signatures = true;
let secure_alt = match management_method {
MgmtMethod::BitcoinGetBalanceQuery => "bitcoin_get_balance",
MgmtMethod::BitcoinGetUtxosQuery => "bitcoin_get_utxos",
_ => unreachable!(),
};
warn!(env.get_logger(), "{method_name} call to the management canister cannot be benefit from the \"Replica Signed Queries\" feature.
The response might not be trustworthy.
If you want to get reliable result, you can make an update call to the secure alternative: {secure_alt}");
}
CallSender::Wallet(_) => {
return Err(DiagnosedError::new(
format!(
"{} can only be called directly without a wallet proxy.",
method_name
),
"Please remove the `--wallet <wallet id>` option.".to_string(),
))
.context("Method only callable without wallet proxy.");
}
}
}

if matches!(call_sender, CallSender::SelectedId)
&& matches!(
management_method,
Expand Down Expand Up @@ -364,13 +334,7 @@ To figure out the id of your wallet, run 'dfx identity get-wallet (--network ic)
.query(&canister_id, method_name)
.with_effective_canister_id(effective_canister_id)
.with_arg(arg_value);
match disable_verify_query_signatures {
true => query_builder
.call_without_verification()
.await
.context("Failed query call.")?,
false => query_builder.call().await.context("Failed query call.")?,
}
query_builder.call().await.context("Failed query call.")?
}
CallSender::Wallet(wallet_id) => {
let wallet = build_wallet_canister(*wallet_id, agent).await?;
Expand Down
Loading