miniquorum: dedup signatures, invalidate duped signatures

crates/by-chain/icepick-cosmos/src/coin_denoms.rs

@@ -52,6 +52,7 @@ impl Bech32Config {
     }
 
     fn with_similar_prefix(prefix: &'static str) -> Self {
+        #[allow(clippy::useless_format)]
         Self {
             account_address_prefix: format!("{prefix}"),
             account_address_public_prefix: format!("{prefix}pub"),
@@ -194,8 +195,26 @@ fn seda_chains() -> Vec<Blockchain> {
             .fee_currencies(&[CurrencyWithGas::builder()
             .currency(aseda.clone())
             .gas_price_step(aseda_gas.clone()).build()])
-            .gas_price_step(aseda_gas)
+            .gas_price_step(aseda_gas.clone())
-            .stake_currency(aseda)
+            .stake_currency(aseda.clone())
+            .build(),
+    );
+
+    chains.push(
+        Blockchain::builder()
+            .chain_id("seda-1")
+            .chain_name("seda")
+            .rpc_url("https://rpc.seda.xyz")
+            .rest_url("https://lcd.seda.xyz")
+            .explorer_url_format("https://explorer.seda.xyz/txs/%s")
+            .bip44_config(Bip44Config::builder().coin_type(118).build())
+            .bech32_config(Bech32Config::with_similar_prefix("seda"))
+            .currencies(&[aseda.clone()])
+            .fee_currencies(&[CurrencyWithGas::builder()
+            .currency(aseda.clone())
+            .gas_price_step(aseda_gas.clone()).build()])
+            .gas_price_step(aseda_gas.clone())
+            .stake_currency(aseda.clone())
             .build(),
     );
 
@@ -217,6 +236,18 @@ fn kyve_chains() -> Vec<Blockchain> {
         .high(0.03)
         .build();
 
+    let ukyve = Currency::builder()
+        .coin_denom("KYVE")
+        .coin_minimal_denom("ukyve")
+        .coin_decimals(6)
+        .coin_gecko_id("unknown")
+        .build();
+    let ukyve_gas = GasPriceStep::builder()
+        .low(0.01)
+        .average(0.025)
+        .high(0.03)
+        .build();
+
     chains.push(
         Blockchain::builder()
             .chain_id("korellia-2")
@@ -236,6 +267,44 @@ fn kyve_chains() -> Vec<Blockchain> {
             .build(),
     );
 
+    chains.push(
+        Blockchain::builder()
+            .chain_id("kaon-1")
+            .chain_name("kaon")
+            .rpc_url("https://rpc.kaon.kyve.network")
+            .rest_url("https://api.kaon.kyve.network")
+            .explorer_url_format("https://explorer.kyve.network/kaon/tx/%s")
+            .bip44_config(Bip44Config::builder().coin_type(118).build())
+            .bech32_config(Bech32Config::with_similar_prefix("kyve"))
+            .currencies(&[tkyve.clone()])
+            .fee_currencies(&[CurrencyWithGas::builder()
+                .currency(tkyve.clone())
+                .gas_price_step(tkyve_gas.clone())
+                .build()])
+            .gas_price_step(tkyve_gas.clone())
+            .stake_currency(tkyve.clone())
+            .build(),
+    );
+
+    chains.push(
+        Blockchain::builder()
+            .chain_id("kyve-1")
+            .chain_name("kyve")
+            .rpc_url("https://rpc.kyve.network")
+            .rest_url("https://api.kyve.network")
+            .explorer_url_format("https://explorer.kyve.network/kyve/tx/%s")
+            .bip44_config(Bip44Config::builder().coin_type(118).build())
+            .bech32_config(Bech32Config::with_similar_prefix("kyve"))
+            .currencies(&[ukyve.clone()])
+            .fee_currencies(&[CurrencyWithGas::builder()
+                .currency(ukyve.clone())
+                .gas_price_step(ukyve_gas.clone())
+                .build()])
+            .gas_price_step(ukyve_gas.clone())
+            .stake_currency(ukyve.clone())
+            .build(),
+    );
+
     chains
 }
 

crates/by-chain/icepick-solana/src/lib.rs

@@ -238,9 +238,11 @@ pub struct Inspect {
 #[derive(Serialize, Deserialize, Debug)]
 pub struct Sign {
     blockhash: String,
-    transaction: solana_sdk::transaction::Transaction,
+    instructions: Vec<solana_sdk::instruction::Instruction>,
     #[serde(default)]
     signing_keys: Vec<[u8; Keypair::SECRET_KEY_LENGTH]>,
+    #[serde(default)]
+    payer_address: Option<String>,
 }
 
 #[derive(Serialize, Deserialize, Debug)]
@@ -987,9 +989,9 @@ impl Module for Solana {
                 derivation_accounts,
                 mut instructions,
             }) => {
-                use solana_sdk::{hash::Hash, message::Message, transaction::Transaction};
+                use solana_sdk::hash::Hash;
 
-                let (hash, transaction) = match hashable {
+                let hash = match hashable {
                     // We already have the account from GetNonceAccountData,
                     // which also gives us the authority and the nonce itself.
                     Hashable::Nonce {
@@ -1005,21 +1007,14 @@ impl Module for Solana {
                             system_instruction::advance_nonce_account(&account_pk, &authority_pk);
 
                         instructions.insert(0, increment_nonce);
-                        let message = Message::new(&instructions, None);
+                        hash
-                        let transaction = Transaction::new_unsigned(message);
-                        (hash, transaction)
-                    }
-                    Hashable::Blockhash { blockhash } => {
-                        let blockhash = Hash::from_str(&blockhash).unwrap();
-                        let message = Message::new(&instructions, None);
-                        let transaction = Transaction::new_unsigned(message);
-                        (blockhash, transaction)
                     }
+                    Hashable::Blockhash { blockhash } => Hash::from_str(&blockhash).unwrap(),
                 };
                 Ok(serde_json::json!({
                     "blob": {
                         "hash": hash,
-                        "transaction": transaction,
+                        "instructions": instructions,
                     },
                     "derivation_accounts": derivation_accounts,
                 }))
@@ -1035,9 +1030,12 @@ impl Module for Solana {
             }
             Operation::Sign(Sign {
                 blockhash,
-                mut transaction,
+                instructions,
                 signing_keys,
+                payer_address,
             }) => {
+                use solana_sdk::{message::Message, transaction::Transaction};
+
                 let keys = request
                     .derived_keys
                     .unwrap_or_default()
@@ -1046,10 +1044,21 @@ impl Module for Solana {
                     .map(|k| Self::keypair_from_bytes(*k))
                     .collect::<Vec<_>>();
 
+                let payer_pk = payer_address
+                    .as_deref()
+                    .map(Pubkey::from_str)
+                    .transpose()
+                    .unwrap();
+
+                let message =
+                    Message::new(&instructions, Some(&payer_pk.unwrap_or(keys[0].pubkey())));
+                let mut transaction = Transaction::new_unsigned(message);
+
                 let hash = solana_sdk::hash::Hash::from_str(&blockhash).unwrap();
                 transaction
                     .try_sign(&keys, hash)
                     .expect("not enough keys provided");
+
                 Ok(serde_json::json!({
                     "blob": {
                         "transaction": transaction,
@@ -1065,7 +1074,15 @@ impl Module for Solana {
 
                 transaction.verify().expect("invalid signatures");
                 let client = solana_rpc_client::rpc_client::RpcClient::new(cluster_url);
-                let _simulated_response = client.simulate_transaction(&transaction).unwrap();
+                let simulated_response = client.simulate_transaction(&transaction).unwrap();
+                if let Some(err) = simulated_response.value.err {
+                    return Ok(serde_json::json!({
+                        "blob": {
+                            "status": "simulate_transaction",
+                            "error": err.to_string(),
+                        }
+                    }))
+                }
                 let response = client.send_and_confirm_transaction(&transaction);
                 let cluster_suffix = {
                     if cluster == Cluster::MainnetBeta {

crates/icepick/workflows/sol/generate-nonce-account.yaml

@@ -46,12 +46,12 @@ step:
     derivation_accounts: "derivation_accounts"
     blockhash: "blockhash"
   outputs:
-    transaction: "unsigned_transaction"
+    instructions: "nonced_instructions"
 - type: "sol-sign"
   inputs:
     blockhash: "blockhash"
     signing_keys: "private_keys"
-    transaction: "unsigned_transaction"
+    instructions: "nonced_instructions"
   outputs:
     transaction: "signed_transaction"
 - type: "sol-broadcast"

crates/icepick/workflows/sol/transfer-token.yaml

@@ -46,10 +46,10 @@ step:
     nonce_authority: nonce_authority
     nonce_data: nonce_data
   outputs:
-    transaction: unsigned_transaction
+    instructions: nonced_instructions
 - type: sol-sign
   inputs:
-    transaction: unsigned_transaction
+    instructions: nonced_instructions
     blockhash: nonce_data
   outputs:
     transaction: transaction

crates/icepick/workflows/sol/transfer.yaml

@@ -35,11 +35,11 @@ step:
     nonce_authority: "nonce_authority"
     nonce_data: "nonce_data"
   outputs:
-    transaction: "unsigned_transaction"
+    instructions: "nonced_instructions"
 - type: "sol-sign"
   inputs:
     blockhash: "nonce_data"
-    transaction: "unsigned_transaction"
+    instructions: "nonced_instructions"
   outputs:
     transaction: "signed_transaction"
 - type: "internal-save-file"

crates/miniquorum/src/lib.rs

@@ -61,6 +61,26 @@ pub enum BaseError {
     /// The JSON object is not a valid value.
     #[error("the JSON object is not a valid value")]
     InvalidJSONValue,
+
+    /// No signing key was found on smartcard.
+    #[error("no signing key was found on smartcard")]
+    NoSigningKey,
+
+    /// A signature exists for the current smartcard.
+    #[error("a signature exists for the key on the current smartcard: {0}")]
+    ConflictingSignature(openpgp::Fingerprint),
+
+    /// A bad packet type was encountered.
+    #[error("a bad OpenPGP packet was encountered: {0}")]
+    BadOpenPGPPacket(openpgp::packet::Tag),
+
+    /// A signature could not have been added; a smartcard might not have been pluggedi n.
+    #[error("a signature could not be added")]
+    NoSignatureAdded,
+
+    /// The signature matched a key that was already used to verify another signature.
+    #[error("signature {1} matched key {0} previously used to sign signature {2}")]
+    DuplicateSignature(openpgp::Fingerprint, usize, usize),
 }
 
 impl BaseError {
@@ -151,7 +171,11 @@ impl PayloadVerification {
 
     /// Set a threshold for required signatures.
     pub fn with_threshold(self, threshold: u8) -> Self {
-        Self { one_each: false, threshold, ..self }
+        Self {
+            one_each: false,
+            threshold,
+            ..self
+        }
     }
 
     /// Require a single valid signature; other signatures may be invalid.
@@ -259,6 +283,12 @@ impl Payload {
     ///
     /// The method may error if a signature could not be created.
     pub fn add_signature(&mut self) -> Result<(), Box<dyn std::error::Error>> {
+        let signatures = self
+            .signatures
+            .iter()
+            .map(|signature_text| Packet::from_bytes(signature_text.as_bytes()).map_err(Into::into))
+            .collect::<Result<Vec<_>, Box<dyn std::error::Error>>>()?;
+
         let unhashed = unhashed(serde_json::to_value(&self)?)?;
         let builder =
             SignatureBuilder::new(SignatureType::Binary).set_hash_algo(HashAlgorithm::SHA512);
@@ -268,10 +298,26 @@ impl Payload {
             ..Default::default()
         };
 
+        let mut has_signed_any = false;
         for backend in card_backend_pcsc::PcscBackend::cards(None)? {
             let mut card = Card::<Open>::new(backend?)?;
             let mut transaction = card.transaction()?;
 
+            let key_fps = transaction.fingerprints()?;
+            let signing_key_fp = key_fps.signature().ok_or(BaseError::NoSigningKey)?;
+
+            for packet in &signatures {
+                let Packet::Signature(signature) = packet else {
+                    return Err(BaseError::BadOpenPGPPacket(packet.tag()).into());
+                };
+
+                for issuer_fp in signature.issuer_fingerprints() {
+                    if issuer_fp.as_bytes() == signing_key_fp.as_bytes() {
+                        return Err(BaseError::ConflictingSignature(issuer_fp.clone()).into());
+                    }
+                }
+            }
+
             let cardholder_name = format_name(transaction.cardholder_name()?);
             let card_id = transaction.application_identifier()?.ident();
             let mut pin = None;
@@ -329,9 +375,14 @@ impl Payload {
             writer.finalize()?;
 
             self.signatures.push(String::from_utf8(armored_signature)?);
+            has_signed_any = true;
         }
 
-        Ok(())
+        if has_signed_any {
+            Ok(())
+        } else {
+            Err(BaseError::NoSignatureAdded.into())
+        }
     }
 
     /// Verify the keychain and certificates using either a Key ID or an OpenPGP card.
@@ -370,14 +421,32 @@ impl Payload {
             threshold = certs.len() as u8;
         }
 
-        for signature in &self.signatures {
+        let mut seen = std::collections::HashMap::new();
+
+        for (index, signature) in self.signatures.iter().enumerate() {
+            dbg!(&index);
             let packet = Packet::from_bytes(signature.as_bytes())?;
             let Packet::Signature(signature) = packet else {
                 panic!("bad packet found: {}", packet.tag());
             };
             let mut signature_matched = false;
-            for issuer in signature.get_issuers() {
+            // NOTE: It is allowable, by the specification, to have a packet that doesn't include
+            // an issuer fingerprint, but instead just a key ID. However, filtering by both key ID
+            // and by fingerprint triggers the "duplicate signature" mechanism. For that reason, we
+            // are only going to filter over fingerprints.
+            //
+            // Any program that makes these signatures should be using fingerprints.
+            for issuer in signature.issuer_fingerprints() {
+                let mut currently_seen = std::collections::HashMap::new();
                 for cert in &certs {
+                    if let Some(seen_index) = seen.get(&cert.fingerprint()) {
+                        return Err(BaseError::DuplicateSignature(
+                            cert.fingerprint(),
+                            index,
+                            *seen_index,
+                        )
+                        .into());
+                    }
                     match cert
                         .with_policy(&policy, None)?
                         .keys()
@@ -390,6 +459,9 @@ impl Payload {
                         Some(Ok(())) => {
                             // key found, signature matched
                             signature_matched = true;
+
+                            // mark the cert as seen, so it isn't reusable
+                            currently_seen.insert(cert.fingerprint(), index);
                         }
                         Some(Err(e)) => {
                             if error_on_invalid {
@@ -401,6 +473,7 @@ impl Payload {
                         }
                     }
                 }
+                seen.extend(currently_seen);
             }
 
             if signature_matched {