fix(p2pk): allow listing and spending p2pk utxos created with uncompressed pubkeys

mm2src/coins/utxo.rs

@@ -1827,8 +1827,17 @@ pub fn output_script(address: &Address) -> Result<Script, keys::Error> {
     }
 }
 
-/// Builds transaction output script for a legacy P2PK address
-pub fn output_script_p2pk(pubkey: &Public) -> Script { Builder::build_p2pk(pubkey) }
+/// Builds transaction output script for a legacy P2PK address.
+///
+/// This actually builds two scripts, the first is for the P2PK constructed using a compressed 33-byte pubkey and the second for the
+/// P2PK constructed using a 65-byte uncompressed pubkey. Both of these scripts are accepted by the blockchain and can contain funds.
+pub fn output_scripts_p2pk(pubkey: &Public) -> Result<[Script; 2], keys::Error> {
+    let secp_pubkey = pubkey.to_secp256k1_pubkey()?;
+    Ok([
+        Builder::build_p2pk(&Public::Compressed(secp_pubkey.serialize().into())),
+        Builder::build_p2pk(&Public::Normal(secp_pubkey.serialize_uncompressed().into())),
+    ])
+}
 
 pub fn address_by_conf_and_pubkey_str(
     coin: &str,

mm2src/coins/utxo/rpc_clients/electrum_rpc/client.rs

@@ -10,7 +10,7 @@ use super::rpc_responses::*;
 
 use crate::utxo::rpc_clients::ConcurrentRequestMap;
 use crate::utxo::utxo_block_header_storage::BlockHeaderStorage;
-use crate::utxo::{output_script, output_script_p2pk, GetBlockHeaderError, GetConfirmedTxError, GetTxHeightError,
+use crate::utxo::{output_script, output_scripts_p2pk, GetBlockHeaderError, GetConfirmedTxError, GetTxHeightError,
                   ScripthashNotification};
 use crate::RpcTransportEventHandler;
 use crate::SharableRpcTransportEventHandler;
@@ -816,8 +816,9 @@ impl UtxoRpcClientOps for ElectrumClient {
             // We don't want to show P2PK outputs along with segwit ones (P2WPKH).
             // Allow listing the P2PK outputs only if the address is not segwit (i.e. show P2PK outputs along with P2PKH).
             if !address.addr_format().is_segwit() {
-                let p2pk_output_script = output_script_p2pk(pubkey);
-                output_scripts.push(p2pk_output_script);
+                let p2pk_scripts = try_f!(output_scripts_p2pk(pubkey)
+                    .map_err(|e| UtxoRpcError::Internal(format!("Couldn't get p2pk output scripts: {}", e))));
+                output_scripts.extend(p2pk_scripts);
             }
         }
 
@@ -950,8 +951,15 @@ impl UtxoRpcClientOps for ElectrumClient {
         if let Some(pubkey) = address.pubkey() {
             // Show the balance in P2PK outputs only for the non-segwit legacy addresses (P2PKH).
             if !address.addr_format().is_segwit() {
-                let p2pk_output_script = output_script_p2pk(pubkey);
-                hashes.push(hex::encode(electrum_script_hash(&p2pk_output_script)));
+                let p2pk_scripts = try_f!(output_scripts_p2pk(pubkey).map_err(|err| {
+                    JsonRpcError::new(
+                        UtxoJsonRpcClientInfo::client_info(self),
+                        rpc_req!(self, "blockchain.scripthash.get_balance").into(),
+                        JsonRpcErrorType::Internal(err.to_string()),
+                    )
+                }));
+                let p2pk_hashes = p2pk_scripts.iter().map(|s| hex::encode(electrum_script_hash(s)));
+                hashes.extend(p2pk_hashes);
             }
         }
 

mm2src/coins/utxo/utxo_common.rs

@@ -3762,20 +3762,20 @@ pub async fn request_tx_history<T>(coin: &T, metrics: MetricsArc) -> RequestTxHi
 where
     T: UtxoCommonOps + MmCoin + MarketCoinOps,
 {
-    let my_address = match coin.my_address() {
-        Ok(addr) => addr,
-        Err(e) => {
-            return RequestTxHistoryResult::CriticalError(ERRL!(
-                "Error on getting self address: {}. Stop tx history",
-                e
-            ))
-        },
-    };
-
     let tx_ids = match &coin.as_ref().rpc_client {
         UtxoRpcClientEnum::Native(client) => {
             let mut from = 0;
             let mut all_transactions = vec![];
+            let my_address = match coin.my_address() {
+                Ok(addr) => addr,
+                Err(e) => {
+                    return RequestTxHistoryResult::CriticalError(ERRL!(
+                        "Error on getting self address: {}. Stop tx history",
+                        e
+                    ))
+                },
+            };
+
             loop {
                 mm_counter!(metrics, "tx.history.request.count", 1,
                     "coin" => coin.as_ref().conf.ticker.clone(), "client" => "native", "method" => "listtransactions");
@@ -3818,16 +3818,27 @@ where
                 Ok(my_address) => my_address,
                 Err(e) => return RequestTxHistoryResult::CriticalError(e.to_string()),
             };
-            let script = match output_script(&my_address) {
-                Ok(script) => script,
+            let mut output_scripts = match output_script(&my_address) {
+                Ok(script) => vec![script],
                 Err(err) => return RequestTxHistoryResult::CriticalError(err.to_string()),
             };
-            let script_hash = electrum_script_hash(&script);
-
+            // We support P2PK addresses for Iguana (i.e. non-HD Wallet) mode.
+            if coin.as_ref().derivation_method.hd_wallet().is_none() && !my_address.addr_format().is_segwit() {
+                if let Some(pubkey) = my_address.pubkey() {
+                    let p2pk_scripts = match output_scripts_p2pk(pubkey) {
+                        Ok(scripts) => scripts,
+                        Err(e) => return RequestTxHistoryResult::CriticalError(e.to_string()),
+                    };
+                    output_scripts.extend(p2pk_scripts);
+                }
+            }
             mm_counter!(metrics, "tx.history.request.count", 1,
                 "coin" => coin.as_ref().conf.ticker.clone(), "client" => "electrum", "method" => "blockchain.scripthash.get_history");
 
-            let electrum_history = match client.scripthash_get_history(&hex::encode(script_hash)).compat().await {
+            let script_hashes = output_scripts
+                .iter()
+                .map(|script| hex::encode(electrum_script_hash(script)));
+            let electrum_history = match client.scripthash_get_history_batch(script_hashes).compat().await {
                 Ok(value) => value,
                 Err(e) => match &e.error {
                     JsonRpcErrorType::InvalidRequest(e)
@@ -3855,15 +3866,11 @@ where
             mm_counter!(metrics, "tx.history.response.total_length", electrum_history.len() as u64,
                 "coin" => coin.as_ref().conf.ticker.clone(), "client" => "electrum", "method" => "blockchain.scripthash.get_history");
 
-            // electrum returns the most recent transactions in the end but we need to
-            // process them first so rev is required
             electrum_history
                 .into_iter()
-                .rev()
-                .map(|item| {
-                    let height = if item.height < 0 { 0 } else { item.height as u64 };
-                    (item.tx_hash, height)
-                })
+                .flatten()
+                .map(|item| (item.tx_hash, item.height.max(0) as u64))
+                .sorted_by(|(_, h1), (_, h2)| h2.cmp(h1))
                 .collect()
         },
     };

mm2src/coins/utxo/utxo_tests.rs

@@ -3593,24 +3593,41 @@ fn test_withdraw_p2pk_balance() {
     UtxoStandardCoin::get_unspent_ordered_list.mock_safe(|coin, _| {
         let fut = async move {
             let cache = coin.as_ref().recently_spent_outpoints.lock().await;
-            let unspents = vec![UnspentInfo {
-                outpoint: OutPoint {
-                    hash: 1.into(),
-                    index: 0,
+            let [compressed_p2pk_script, uncompressed_p2pk_script] = output_scripts_p2pk(
+                &coin
+                    .as_ref()
+                    .derivation_method
+                    .unwrap_single_addr()
+                    .await
+                    .pubkey()
+                    .unwrap(),
+            )
+            .unwrap();
+            let unspents = vec![
+                UnspentInfo {
+                    outpoint: OutPoint {
+                        hash: 1.into(),
+                        index: 0,
+                    },
+                    // 1 BTC to spend. Next utxo has another 1 BTC.
+                    value: 100000000
+                    + 1000 // To cover the fees.
+                    + 5000, // To get back as change.
+                    height: Default::default(),
+                    // Use a p2pk output (using a 33-byte pubkey) script for this UTXO
+                    script: compressed_p2pk_script,
                 },
-                value: 1000000000,
-                height: Default::default(),
-                // Use a p2pk output script for this UTXO
-                script: output_script_p2pk(
-                    &coin
-                        .as_ref()
-                        .derivation_method
-                        .unwrap_single_addr()
-                        .await
-                        .pubkey()
-                        .unwrap(),
-                ),
-            }];
+                UnspentInfo {
+                    outpoint: OutPoint {
+                        hash: 1.into(),
+                        index: 0,
+                    },
+                    value: 100000000,
+                    height: Default::default(),
+                    // Use a p2pk output (using a 65-byte pubkey) script for this UTXO
+                    script: uncompressed_p2pk_script,
+                },
+            ];
             Ok((unspents, cache))
         };
         MockResult::Return(fut.boxed())
@@ -3620,7 +3637,7 @@ fn test_withdraw_p2pk_balance() {
     let my_p2pkh_address = block_on(coin.as_ref().derivation_method.unwrap_single_addr());
 
     let withdraw_req = WithdrawRequest {
-        amount: 1.into(),
+        amount: 2.into(),
         to: my_p2pkh_address.to_string(),
         coin: TEST_COIN_NAME.into(),
         ..Default::default()
@@ -3634,7 +3651,54 @@ fn test_withdraw_p2pk_balance() {
     assert_eq!(output_script, expected_script);
 
     // And it should have this value (p2pk balance - amount sent - fees).
-    assert_eq!(transaction.outputs[1].value, 899999000);
+    assert_eq!(transaction.outputs[1].value, 5000);
+}
+
+#[test]
+#[cfg(not(target_arch = "wasm32"))]
+fn test_list_unspent_lists_p2pk_utxos() {
+    use mm2_test_helpers::{electrums::tbtc_electrums, for_tests::tbtc_conf};
+
+    let conf = tbtc_conf();
+    let req = json!({
+         "method": "electrum",
+         "servers": tbtc_electrums(),
+    });
+
+    let ctx = MmCtxBuilder::new().into_mm_arc();
+    let params = UtxoActivationParams::from_legacy_req(&req).unwrap();
+
+    // This is the private key for two p2pk utxos (one created using comressed pubkey and the other using uncompressed pubkey).
+    // PK of the P2PK balance: 03f8f8fa2062590ba9a0a7a86f937de22f540c015864aad35a2a9f6766de906265 (owned 0.00076 tBTC - https://mempool.space/testnet/address/03f8f8fa2062590ba9a0a7a86f937de22f540c015864aad35a2a9f6766de906265)
+    // Uncompressed form: 04f8f8fa2062590ba9a0a7a86f937de22f540c015864aad35a2a9f6766de9062655d6a466270be272069ec95b7b83724a6ec24238395980ea7efd69fd9155d2a6d (owns 0.00003 tBTC - https://mempool.space/testnet/address/04f8f8fa2062590ba9a0a7a86f937de22f540c015864aad35a2a9f6766de9062655d6a466270be272069ec95b7b83724a6ec24238395980ea7efd69fd9155d2a6d)
+    let priv_key = Private::from_str("18GYrxNbvwoBi5Xwe4o92tDVoSf49ybs78BiQYDPKUkFp7qVn2wZ")
+        .unwrap()
+        .secret;
+    let coin = block_on(utxo_standard_coin_with_priv_key(&ctx, "tBTC", &conf, &params, priv_key)).unwrap();
+
+    let my_address = block_on(coin.as_ref().derivation_method.single_addr_or_err()).unwrap();
+    let (unspents, _recently_spent) = block_on_f01(coin.get_unspent_ordered_list(&my_address).compat()).unwrap();
+    let expected_unspents = vec![
+        UnspentInfo {
+            outpoint: OutPoint {
+                hash: H256::from_reversed_str("8085b7a738f711a00a44ba291265453f52b3af7b71586a3e5166612d2438d355"),
+                index: 0,
+            },
+            value: 3000,
+            height: Some(4296371),
+            script: "4104f8f8fa2062590ba9a0a7a86f937de22f540c015864aad35a2a9f6766de9062655d6a466270be272069ec95b7b83724a6ec24238395980ea7efd69fd9155d2a6dac".into(),
+        },
+        UnspentInfo {
+            outpoint: OutPoint {
+                hash: H256::from_reversed_str("0e440b23e8c12e8ba32b1ff0cdd7d0fa6a58f8b594f7813f073eee4834f15b4e"),
+                index: 0,
+            },
+            value: 76000,
+            height: Some(2575577),
+            script: "2103f8f8fa2062590ba9a0a7a86f937de22f540c015864aad35a2a9f6766de906265ac".into(),
+        },
+    ];
+    assert_eq!(unspents, expected_unspents);
 }
 
 /// `UtxoStandardCoin` has to check UTXO maturity if `check_utxo_maturity` is `true`.

mm2src/coins/utxo_signer/src/lib.rs

@@ -60,6 +60,8 @@ pub enum UtxoSignTxError {
         script
     )]
     UnspendableUTXO { script: Script },
+    #[display(fmt = "Couldn't get secp256k1 pubkey from keypair: {}", error)]
+    BadPublicKey { error: String },
     #[display(fmt = "Transport error: {}", _0)]
     Transport(String),
     #[display(fmt = "Internal error: {}", _0)]
@@ -87,8 +89,9 @@ impl From<UtxoSignWithKeyPairError> for UtxoSignTxError {
             // that are expected to be checked by [`sign_common::UtxoSignTxParamsBuilder::build`] already.
             // So if this error happens, it's our internal error.
             UtxoSignWithKeyPairError::InputIndexOutOfBound { .. } => UtxoSignTxError::Internal(error),
-            UtxoSignWithKeyPairError::UnspendableUTXO { script } => UtxoSignTxError::UnspendableUTXO { script },
             UtxoSignWithKeyPairError::ErrorSigning(sign) => UtxoSignTxError::ErrorSigning(sign),
+            UtxoSignWithKeyPairError::UnspendableUTXO { script } => UtxoSignTxError::UnspendableUTXO { script },
+            UtxoSignWithKeyPairError::BadPublicKey { error } => UtxoSignTxError::BadPublicKey { error },
         }
     }
 }

mm2src/coins/utxo_signer/src/with_key_pair.rs

@@ -35,6 +35,8 @@ pub enum UtxoSignWithKeyPairError {
         script
     )]
     UnspendableUTXO { script: Script },
+    #[display(fmt = "Couldn't get secp256k1 pubkey from keypair: {}", error)]
+    BadPublicKey { error: String },
     #[display(fmt = "Error signing using a private key")]
     ErrorSigning(keys::Error),
 }
@@ -77,20 +79,31 @@ pub fn p2pk_spend(
     fork_id: u32,
 ) -> UtxoSignWithKeyPairResult<TransactionInput> {
     let unsigned_input = get_input(signer, input_index)?;
-
-    let script = Builder::build_p2pk(key_pair.public());
-    if script != unsigned_input.prev_script {
+    // P2PK UTXOs can have either compressed or uncompressed public keys in the scriptPubkey.
+    // We need to check that one of them matches the prev_script of the input being spent.
+    let pubkey = key_pair
+        .public()
+        .to_secp256k1_pubkey()
+        .map_err(|e| UtxoSignWithKeyPairError::BadPublicKey { error: e.to_string() })?;
+    // Build the scriptPubKey for both compressed and uncompressed public keys.
+    let possible_script_pubkeys = [
+        Builder::build_p2pk(&keys::Public::Compressed(pubkey.serialize().into())),
+        Builder::build_p2pk(&keys::Public::Normal(pubkey.serialize_uncompressed().into())),
+    ];
+    // Check that one of the scriptPubkeys matches the prev_script of the input being spent.
+    if !possible_script_pubkeys.contains(&unsigned_input.prev_script) {
         return MmError::err(UtxoSignWithKeyPairError::MismatchScript {
             script_type: "P2PK".to_owned(),
-            script,
+            // Safe indexing since the array has exactly 2 elements.
+            script: possible_script_pubkeys[0].clone(),
             prev_script: unsigned_input.prev_script.clone(),
         });
     }
 
     let signature = calc_and_sign_sighash(
         signer,
         input_index,
-        &script,
+        &unsigned_input.prev_script,
         key_pair,
         signature_version,
         SIGHASH_ALL,

mm2src/mm2_bitcoin/keys/src/public.rs

@@ -37,7 +37,18 @@ impl Public {
         }
     }
 
-    pub fn address_hash(&self) -> H160 { dhash160(self) }
+    pub fn address_hash(&self) -> H160 {
+        match self {
+            Public::Compressed(public) => dhash160(public.as_slice()),
+            // If the public key isn't compressed, we wanna compress it then get the hash.
+            // No body uses the uncompressed form to get an address hash.
+            Public::Normal(public) => match PublicKey::from_slice(public.as_slice()) {
+                Ok(public) => dhash160(&public.serialize()),
+                // This should never happen, as then the public key would be invalid. If so, return a dummy value.
+                Err(_) => H160::default(),
+            },
+        }
+    }
 
     pub fn verify(&self, message: &Message, signature: &Signature) -> Result<bool, Error> {
         let public = match self {
@@ -82,7 +93,12 @@ impl Public {
     }
 
     #[inline(always)]
-    pub fn to_secp256k1_pubkey(&self) -> Result<PublicKey, SecpError> { PublicKey::from_slice(self.deref()) }
+    pub fn to_secp256k1_pubkey(&self) -> Result<PublicKey, SecpError> {
+        match self {
+            Public::Compressed(public) => PublicKey::from_slice(&**public),
+            Public::Normal(public) => PublicKey::from_slice(&**public),
+        }
+    }
 }
 
 impl Deref for Public {

mm2src/mm2_main/tests/mm2_tests/mm2_tests_inner.rs

@@ -249,7 +249,8 @@ fn test_my_balance() {
 // TODO: Add a p2pk spending test in the docker tests when electrum nodes are available (also try to invoke the utxo cache by spending in rapid succession).
 #[test]
 fn test_p2pk_my_balance() {
-    // PK of the P2PK balance: 03f8f8fa2062590ba9a0a7a86f937de22f540c015864aad35a2a9f6766de906265
+    // PK of the P2PK balance: 03f8f8fa2062590ba9a0a7a86f937de22f540c015864aad35a2a9f6766de906265 (owned 0.00076 tBTC - https://mempool.space/testnet/address/03f8f8fa2062590ba9a0a7a86f937de22f540c015864aad35a2a9f6766de906265)
+    // Uncompressed form: 04f8f8fa2062590ba9a0a7a86f937de22f540c015864aad35a2a9f6766de9062655d6a466270be272069ec95b7b83724a6ec24238395980ea7efd69fd9155d2a6d (owns 0.00003 tBTC - https://mempool.space/testnet/address/04f8f8fa2062590ba9a0a7a86f937de22f540c015864aad35a2a9f6766de9062655d6a466270be272069ec95b7b83724a6ec24238395980ea7efd69fd9155d2a6d)
     let seed = "salmon angle cushion sauce accuse earth volume until zone youth emerge favorite";
     let coins = json!([tbtc_conf()]);
     let conf = Mm2TestConf::seednode(seed, &coins);
@@ -261,9 +262,11 @@ fn test_p2pk_my_balance() {
     block_on(enable_electrum(&mm, "tBTC", false, TBTC_ELECTRUMS));
     let my_balance = block_on(my_balance(&mm, "tBTC"));
 
-    assert_eq!(my_balance.balance, "0.00076".parse().unwrap());
+    // The total balance is the sum of the balances in the p2pkh, compressed-pubkey p2pk, uncompressed-pubkey p2pk addresses.
+    // In this case, we only have the compressed-pubkey p2pk (0.00076 tBTC) and uncompressed-pubkey p2pk (0.00003 tBTC) addresses.
+    assert_eq!(my_balance.balance, "0.00079".parse().unwrap());
     assert_eq!(my_balance.unspendable_balance, BigDecimal::from(0));
-    // Even though the address is a P2PK, it's formatted as P2PKH like most explorers do.
+    // Even though the addresses containing the coins are P2PK, it's formatted as P2PKH like most explorers do.
     assert_eq!(my_balance.address, "mgrM9w49Q7vqtroLKGekLTqCVFye5u6G3v");
 }