feat(ckbtc): add check_transaction_str to Bitcoin Checker (#3212)

XC-259: Add a new public method `check_transaction_str` to Bitcoin
Checker that takes transaction id as a text argument.

Author: ninegua

rs/bitcoin/checker/btc_checker_canister.did

@@ -9,6 +9,8 @@ type CheckAddressResponse = variant { Passed; Failed };
 
 type CheckTransactionArgs = record { txid : blob };
 
+type CheckTransactionStrArgs = record { txid : text };
+
 // The result of a check_transaction call.
 type CheckTransactionResponse = variant {
     // When check finishes and all input addresses passed.
@@ -85,7 +87,11 @@ service : (CheckArg) -> {
     // of the return result.
     check_transaction: (CheckTransactionArgs) -> (CheckTransactionResponse);
 
+    // Same as check_transaction, but taking the transaction id argument as a string.
+    check_transaction_str: (CheckTransactionStrArgs) -> (CheckTransactionResponse);
+
     // Return `Passed` if the given Bitcoin address passes the Bitcoin checker, or `Failed` otherwise.
     // May throw error (trap) if the given address is malformed or not a mainnet address.
     check_address: (CheckAddressArgs) -> (CheckAddressResponse) query;
+
 }

rs/bitcoin/checker/src/main.rs

@@ -3,7 +3,8 @@ use ic_btc_checker::{
     blocklist_contains, get_tx_cycle_cost, BtcNetwork, CheckAddressArgs, CheckAddressResponse,
     CheckArg, CheckMode, CheckTransactionArgs, CheckTransactionIrrecoverableError,
     CheckTransactionResponse, CheckTransactionRetriable, CheckTransactionStatus,
-    CHECK_TRANSACTION_CYCLES_REQUIRED, CHECK_TRANSACTION_CYCLES_SERVICE_FEE,
+    CheckTransactionStrArgs, CHECK_TRANSACTION_CYCLES_REQUIRED,
+    CHECK_TRANSACTION_CYCLES_SERVICE_FEE,
 };
 use ic_btc_interface::Txid;
 use ic_canister_log::{export as export_logs, log};
@@ -42,10 +43,10 @@ pub fn is_response_too_large(code: &RejectionCode, message: &str) -> bool {
         && (message.contains("size limit") || message.contains("length limit"))
 }
 
-#[ic_cdk::query]
 /// Return `Passed` if the given bitcion address passed the check, or
 /// `Failed` otherwise.
 /// May throw error (trap) if the given address is malformed or not a mainnet address.
+#[ic_cdk::query]
 fn check_address(args: CheckAddressArgs) -> CheckAddressResponse {
     let config = get_config();
     let btc_network = config.btc_network();
@@ -69,7 +70,6 @@ fn check_address(args: CheckAddressArgs) -> CheckAddressResponse {
     }
 }
 
-#[ic_cdk::update]
 /// Return `Passed` if all input addresses of the transaction of the given
 /// transaction id passed the check, or `Failed` if any of them did not.
 ///
@@ -87,14 +87,29 @@ fn check_address(args: CheckAddressArgs) -> CheckAddressResponse {
 /// If a permanent error occurred in the process, e.g, when a transaction data
 /// fails to decode or its transaction id does not match, then `Error` is returned
 /// together with a text description.
+#[ic_cdk::update]
 async fn check_transaction(args: CheckTransactionArgs) -> CheckTransactionResponse {
+    check_transaction_with(|| Txid::try_from(args.txid.as_ref()).map_err(|err| err.to_string()))
+        .await
+}
+
+#[ic_cdk::update]
+async fn check_transaction_str(args: CheckTransactionStrArgs) -> CheckTransactionResponse {
+    use std::str::FromStr;
+    check_transaction_with(|| Txid::from_str(args.txid.as_ref()).map_err(|err| err.to_string()))
+        .await
+}
+
+async fn check_transaction_with<F: FnOnce() -> Result<Txid, String>>(
+    get_txid: F,
+) -> CheckTransactionResponse {
     if ic_cdk::api::call::msg_cycles_accept128(CHECK_TRANSACTION_CYCLES_SERVICE_FEE)
         < CHECK_TRANSACTION_CYCLES_SERVICE_FEE
     {
         return CheckTransactionStatus::NotEnoughCycles.into();
     }
 
-    match Txid::try_from(args.txid.as_ref()) {
+    match get_txid() {
         Ok(txid) => {
             STATS.with(|s| s.borrow_mut().check_transaction_count += 1);
             if ic_cdk::api::call::msg_cycles_available128()
@@ -107,9 +122,7 @@ async fn check_transaction(args: CheckTransactionArgs) -> CheckTransactionRespon
                 check_transaction_inputs(txid).await
             }
         }
-        Err(err) => {
-            CheckTransactionIrrecoverableError::InvalidTransactionId(err.to_string()).into()
-        }
+        Err(err) => CheckTransactionIrrecoverableError::InvalidTransactionId(err).into(),
     }
 }
 

rs/bitcoin/checker/src/types.rs

@@ -19,6 +19,11 @@ pub struct CheckTransactionArgs {
     pub txid: Vec<u8>,
 }
 
+#[derive(CandidType, Debug, Deserialize, Serialize)]
+pub struct CheckTransactionStrArgs {
+    pub txid: String,
+}
+
 #[derive(CandidType, Debug, Clone, Deserialize, Serialize)]
 pub enum CheckTransactionResponse {
     /// When check finishes and all input addresses passed.

rs/bitcoin/checker/tests/tests.rs

@@ -3,8 +3,8 @@ use ic_base_types::PrincipalId;
 use ic_btc_checker::{
     blocklist, get_tx_cycle_cost, BtcNetwork, CheckAddressArgs, CheckAddressResponse, CheckArg,
     CheckMode, CheckTransactionArgs, CheckTransactionIrrecoverableError, CheckTransactionResponse,
-    CheckTransactionRetriable, CheckTransactionStatus, InitArg, UpgradeArg,
-    CHECK_TRANSACTION_CYCLES_REQUIRED, CHECK_TRANSACTION_CYCLES_SERVICE_FEE,
+    CheckTransactionRetriable, CheckTransactionStatus, CheckTransactionStrArgs, InitArg,
+    UpgradeArg, CHECK_TRANSACTION_CYCLES_REQUIRED, CHECK_TRANSACTION_CYCLES_SERVICE_FEE,
     INITIAL_MAX_RESPONSE_BYTES,
 };
 use ic_btc_interface::Txid;
@@ -29,11 +29,11 @@ const TEST_SUBNET_NODES: u16 = 34;
 // by a small margin. Namely, the universal_canister itself would consume
 // some cycle for decoding args and sending the call.
 //
-// The number 42_000_000 is obtained empirically by running tests with pocket-ic
+// The number 43_000_000 is obtained empirically by running tests with pocket-ic
 // and checking the actual consumptions. It is both big enough to allow tests to
 // succeed, and small enough not to interfere with the expected cycle cost we
 // are testing for.
-const UNIVERSAL_CANISTER_CYCLE_MARGIN: u128 = 42_000_000;
+const UNIVERSAL_CANISTER_CYCLE_MARGIN: u128 = 43_000_000;
 
 struct Setup {
     // Owner of canisters created for the setup.
@@ -274,20 +274,21 @@ fn test_check_transaction_passed() {
     let txid =
         Txid::from_str("c80763842edc9a697a2114517cf0c138c5403a761ef63cfad1fa6993fa3475ed").unwrap();
     let env = &setup.env;
+    let check_transaction_args = Encode!(&CheckTransactionArgs {
+        txid: txid.as_ref().to_vec()
+    })
+    .unwrap();
+    let check_transaction_str_args = Encode!(&CheckTransactionStrArgs {
+        txid: txid.to_string()
+    })
+    .unwrap();
 
     // Normal operation requires making http outcalls.
     // We'll run this again after testing other CheckMode.
-    let test_normal_operation = || {
+    let test_normal_operation = |method, arg| {
         let cycles_before = setup.env.cycle_balance(setup.caller);
         let call_id = setup
-            .submit_btc_checker_call(
-                "check_transaction",
-                Encode!(&CheckTransactionArgs {
-                    txid: txid.as_ref().to_vec()
-                })
-                .unwrap(),
-                CHECK_TRANSACTION_CYCLES_REQUIRED,
-            )
+            .submit_btc_checker_call(method, arg, CHECK_TRANSACTION_CYCLES_REQUIRED)
             .expect("submit_call failed to return call id");
         // The response body used for testing below is generated from the output of
         //
@@ -375,7 +376,7 @@ fn test_check_transaction_passed() {
     };
 
     // With default installation
-    test_normal_operation();
+    test_normal_operation("check_transaction", check_transaction_args.clone());
 
     // Test CheckMode::RejectAll
     env.tick();
@@ -470,7 +471,7 @@ fn test_check_transaction_passed() {
     )
     .unwrap();
 
-    test_normal_operation();
+    test_normal_operation("check_transaction_str", check_transaction_str_args);
 }
 
 #[test]
@@ -669,6 +670,37 @@ fn test_check_transaction_error() {
     let actual_cost = cycles_before - cycles_after;
     assert!(actual_cost > expected_cost);
     assert!(actual_cost - expected_cost < UNIVERSAL_CANISTER_CYCLE_MARGIN);
+
+    // Test for malformatted txid in string form
+    let cycles_before = setup.env.cycle_balance(setup.caller);
+    let too_short_txid =
+        "a80763842edc9a697a2114517cf0c138c5403a761ef63cfad1fa6993fa3475".to_string();
+    let call_id = setup
+        .submit_btc_checker_call(
+            "check_transaction_str",
+            Encode!(&CheckTransactionStrArgs {
+                txid: too_short_txid
+            })
+            .unwrap(),
+            CHECK_TRANSACTION_CYCLES_REQUIRED,
+        )
+        .expect("submit_call failed to return call id");
+    let result = setup
+        .env
+        .await_call(call_id)
+        .expect("the fetch request didn't finish");
+    assert!(matches!(
+        decode::<CheckTransactionResponse>(&result),
+        CheckTransactionResponse::Unknown(CheckTransactionStatus::Error(
+            CheckTransactionIrrecoverableError::InvalidTransactionId(_)
+        ))
+    ));
+
+    let cycles_after = setup.env.cycle_balance(setup.caller);
+    let expected_cost = CHECK_TRANSACTION_CYCLES_SERVICE_FEE;
+    let actual_cost = cycles_before - cycles_after;
+    assert!(actual_cost > expected_cost);
+    assert!(actual_cost - expected_cost < UNIVERSAL_CANISTER_CYCLE_MARGIN);
 }
 
 fn tick_until_next_request(env: &PocketIc) -> Vec<CanisterHttpRequest> {