diff --git a/consensus/client/src/input.rs b/consensus/client/src/input.rs index b0d35f2e22..5bab1c872c 100644 --- a/consensus/client/src/input.rs +++ b/consensus/client/src/input.rs @@ -7,6 +7,7 @@ use crate::TransactionOutpoint; use crate::UtxoEntryReference; use crate::imports::*; +use crate::invalid_input_mass_variant; use crate::result::Result; use kaspa_utils::hex::*; @@ -260,18 +261,26 @@ impl TransactionInput { } } -impl From> for cctx::TransactionInput { - fn from(value: TransactionInputWithVersion<'_>) -> Self { +impl TryFrom> for cctx::TransactionInput { + type Error = Error; + + fn try_from(value: TransactionInputWithVersion<'_>) -> Result { let inner = value.tx_input.inner(); - cctx::TransactionInput { + Ok(cctx::TransactionInput { previous_outpoint: inner.previous_outpoint.clone().into(), signature_script: inner.signature_script.clone().unwrap_or_default(), // TODO - discuss: should this unwrap_or_default or return an error? sequence: inner.sequence, mass: if cctx::TxInputMass::version_expects_compute_budget_field(value.version) { + if inner.sig_op_count != 0 { + return Err(invalid_input_mass_variant("sig_op_count", value.version)); + } cctx::TxInputMass::ComputeBudget(inner.compute_budget.into()) } else { + if inner.compute_budget != 0 { + return Err(invalid_input_mass_variant("compute_budget", value.version)); + } cctx::TxInputMass::SigopCount(inner.sig_op_count.into()) }, - } + }) } } diff --git a/consensus/client/src/lib.rs b/consensus/client/src/lib.rs index 39a6c4b13f..301f874dbc 100644 --- a/consensus/client/src/lib.rs +++ b/consensus/client/src/lib.rs @@ -22,6 +22,11 @@ pub mod result; mod serializable; mod transaction; mod utxo; + +fn invalid_input_mass_variant(field: &str, version: u16) -> error::Error { + error::Error::Custom(format!("TransactionInput.{field} is inconsistent with transaction version {version}")) +} + pub use covenant::*; pub use input::*; pub use outpoint::*; diff --git a/consensus/client/src/serializable/mod.rs b/consensus/client/src/serializable/mod.rs index 885898bde8..6fefc178fd 100644 --- a/consensus/client/src/serializable/mod.rs +++ b/consensus/client/src/serializable/mod.rs @@ -22,13 +22,8 @@ pub mod numeric; pub mod string; -use crate::error::Error; use wasm_bindgen::prelude::*; -fn invalid_input_mass_variant(field: &str, version: u16) -> Error { - Error::Custom(format!("TransactionInput.{field} is inconsistent with transaction version {version}")) -} - #[wasm_bindgen(typescript_custom_section)] const TS_TYPES: &'static str = r#" diff --git a/consensus/client/src/serializable/numeric.rs b/consensus/client/src/serializable/numeric.rs index daeeae67fb..2693988687 100644 --- a/consensus/client/src/serializable/numeric.rs +++ b/consensus/client/src/serializable/numeric.rs @@ -6,9 +6,9 @@ //! but may be deserialized in other JSON-capable environments that support large integers) //! -use super::invalid_input_mass_variant; use crate::error::Error; use crate::imports::*; +use crate::invalid_input_mass_variant; use crate::result::Result; use crate::{ CovenantBinding, Transaction, TransactionInput, TransactionInputInner, TransactionOutpoint, TransactionOutpointInner, diff --git a/consensus/client/src/serializable/string.rs b/consensus/client/src/serializable/string.rs index c90221e513..be308ca1ee 100644 --- a/consensus/client/src/serializable/string.rs +++ b/consensus/client/src/serializable/string.rs @@ -3,9 +3,9 @@ //! where all large integer values (`u64`) are serialized to and from JSON as strings. //! -use super::invalid_input_mass_variant; use crate::error::Error; use crate::imports::*; +use crate::invalid_input_mass_variant; use crate::result::Result; use crate::{ CovenantBinding, Transaction, TransactionInput, TransactionInputInner, TransactionOutpoint, TransactionOutpointInner, diff --git a/consensus/client/src/transaction.rs b/consensus/client/src/transaction.rs index ff63c26522..38b70d14cc 100644 --- a/consensus/client/src/transaction.rs +++ b/consensus/client/src/transaction.rs @@ -150,7 +150,7 @@ impl Transaction { /// Recompute and finalize the tx id based on updated tx fields pub fn finalize(&self) -> Result { - let tx: cctx::Transaction = self.into(); + let tx: cctx::Transaction = self.try_into()?; self.inner().id = tx.id(); Ok(self.inner().id) } @@ -352,19 +352,29 @@ impl From for Transaction { } } -impl From<&Transaction> for cctx::Transaction { - fn from(tx: &Transaction) -> Self { +impl TryFrom<&Transaction> for cctx::Transaction { + type Error = Error; + + fn try_from(tx: &Transaction) -> Result { let inner = tx.inner(); let inputs: Vec = inner .inputs .clone() .into_iter() - .map(|input| input.as_ref().with_version(inner.version).into()) - .collect::>(); + .map(|input| input.as_ref().with_version(inner.version).try_into()) + .collect::>>()?; let outputs: Vec = inner.outputs.clone().into_iter().map(|output| output.as_ref().into()).collect::>(); - cctx::Transaction::new(inner.version, inputs, outputs, inner.lock_time, inner.subnetwork_id, inner.gas, inner.payload.clone()) - .with_mass(inner.mass) + Ok(cctx::Transaction::new( + inner.version, + inputs, + outputs, + inner.lock_time, + inner.subnetwork_id, + inner.gas, + inner.payload.clone(), + ) + .with_mass(inner.mass)) } } @@ -409,7 +419,7 @@ impl Transaction { .clone() .into_iter() .map(|input| { - inputs.push(input.as_ref().with_version(inner.version).into()); + inputs.push(input.as_ref().with_version(inner.version).try_into()?); Ok(input.get_utxo().ok_or(Error::MissingUtxoEntry)?.entry().as_ref().into()) }) .collect::>>()?; @@ -445,17 +455,20 @@ impl Transaction { inner.outputs.iter().map(|output| output.into()).collect::>() } - pub fn inputs(&self) -> Vec { + pub fn inputs(&self) -> Result> { let inner = self.inner(); - inner.inputs.iter().map(|input| input.with_version(inner.version).into()).collect::>() + inner.inputs.iter().map(|input| input.with_version(inner.version).try_into()).collect::>>() } - pub fn inputs_outputs(&self) -> (Vec, Vec) { + pub fn inputs_outputs(&self) -> Result<(Vec, Vec)> { let inner = self.inner(); - let inputs = - inner.inputs.iter().map(|input| input.with_version(inner.version).into()).collect::>(); + let inputs = inner + .inputs + .iter() + .map(|input| input.with_version(inner.version).try_into()) + .collect::>>()?; let outputs = inner.outputs.iter().map(Into::into).collect::>(); - (inputs, outputs) + Ok((inputs, outputs)) } pub fn set_signature_script(&self, input_index: usize, signature_script: Vec) -> Result<()> { @@ -475,7 +488,7 @@ impl Transaction { } pub fn populate_genesis_covenants(&self, groups: &[GenesisCovenantGroup]) -> Result<()> { - let mut tx: cctx::Transaction = self.into(); + let mut tx: cctx::Transaction = self.try_into()?; tx.populate_genesis_covenants(groups)?; self.inner().outputs = tx.outputs.iter().map(TransactionOutput::from).collect::>(); Ok(()) @@ -558,6 +571,28 @@ mod tests { .expect("transaction construction should succeed") } + fn construct_tx_with_input_mass(version: u16, sig_op_count: u8, compute_budget: u16) -> Transaction { + let fixed_txid = TransactionId::from_slice(&[0u8; 32]); + let spk = construct_spk(); + let expects_compute_budget = cctx::TxInputMass::version_expects_compute_budget_field(version); + let input = TransactionInput::new( + TransactionOutpoint::new(fixed_txid, 0), + None, + 0, + if expects_compute_budget { 0 } else { sig_op_count }, + if expects_compute_budget { compute_budget } else { 0 }, + None, + ); + let output = TransactionOutput::ctor(100, &spk, None); + + let tx = Transaction::new(None, version, vec![input], vec![output], 0, SubnetworkId::from_bytes([0u8; 20]), 0, vec![], 0) + .expect("transaction construction should succeed"); + let input = tx.inner().inputs[0].clone(); + input.inner().sig_op_count = sig_op_count; + input.inner().compute_budget = compute_budget; + tx + } + // Helper - construct GenesisCovenantGroup[] JS array fn construct_groups_array(groups: &[(u16, &[u32])]) -> js_sys::Array { let arr = js_sys::Array::new(); @@ -608,4 +643,15 @@ mod tests { assert_eq!(output.get_script_public_key(), spk); } } + + #[wasm_bindgen_test] + fn test_try_into_cctx_transaction_rejects_version_incompatible_input_mass_fields() { + let v0_tx = construct_tx_with_input_mass(0, 1, 7); + let v0_err = cctx::Transaction::try_from(&v0_tx).expect_err("v0 conversion should reject non-zero compute budget"); + assert!(v0_err.to_string().contains("compute_budget")); + + let v1_tx = construct_tx_with_input_mass(1, 1, 7); + let v1_err = cctx::Transaction::try_from(&v1_tx).expect_err("v1 conversion should reject non-zero sig op count"); + assert!(v1_err.to_string().contains("sig_op_count")); + } } diff --git a/wallet/core/src/tx/mass.rs b/wallet/core/src/tx/mass.rs index a67eed0db6..9d8fbeaea6 100644 --- a/wallet/core/src/tx/mass.rs +++ b/wallet/core/src/tx/mass.rs @@ -300,7 +300,7 @@ impl MassCalculator { /// Calculates the overall mass of this transaction, combining both compute and storage masses. pub fn calc_overall_mass_for_unsigned_client_transaction(&self, tx: &kcc::Transaction, minimum_signatures: u16) -> Result { - let cctx = Transaction::from(tx); + let cctx = Transaction::try_from(tx)?; let storage_mass = self.calc_storage_mass_for_transaction(tx)?.ok_or(Error::MassCalculationError)?; let compute_mass = self.calc_compute_mass_for_unsigned_consensus_transaction(&cctx, minimum_signatures); Ok(self.combine_mass(compute_mass, storage_mass)) diff --git a/wallet/pskt/src/convert.rs b/wallet/pskt/src/convert.rs index 062f7c67de..79a2eef4d7 100644 --- a/wallet/pskt/src/convert.rs +++ b/wallet/pskt/src/convert.rs @@ -13,16 +13,19 @@ use kaspa_consensus_core::tx as cctx; impl TryFrom for Inner { type Error = Error; - fn try_from(_transaction: Transaction) -> Result { - Inner::try_from(cctx::Transaction::from(&_transaction)) + fn try_from(transaction: Transaction) -> Result { + Inner::try_from(cctx::Transaction::try_from(&transaction)?) } } impl TryFrom for Input { type Error = Error; fn try_from(input: TransactionInput) -> std::result::Result { - let TransactionInputInner { previous_outpoint, signature_script: _, sequence: _, sig_op_count, compute_budget: _, utxo } = + let TransactionInputInner { previous_outpoint, signature_script: _, sequence: _, sig_op_count, compute_budget, utxo } = &*input.inner(); + if *compute_budget != 0 { + return Err(Error::UnsupportedInputComputeBudget); + } let input = InputBuilder::default() .utxo_entry(utxo.as_ref().ok_or(Error::MissingUtxoEntry)?.into()) @@ -68,6 +71,9 @@ impl TryFrom<(cctx::Transaction, Vec<(&cctx::TransactionInput, &cctx::UtxoEntry) fn try_from( (transaction, populated_inputs): (cctx::Transaction, Vec<(&cctx::TransactionInput, &cctx::UtxoEntry)>), ) -> Result { + if cctx::TxInputMass::version_expects_compute_budget_field(transaction.version) { + return Err(Error::UnsupportedInputComputeBudget); + } let inputs: Result, Self::Error> = populated_inputs .into_iter() .map(|(input, utxo)| { @@ -96,6 +102,9 @@ impl TryFrom<(cctx::Transaction, Vec<(&cctx::TransactionInput, &cctx::UtxoEntry) impl TryFrom for Inner { type Error = Error; fn try_from(transaction: cctx::Transaction) -> Result { + if cctx::TxInputMass::version_expects_compute_budget_field(transaction.version) { + return Err(Error::UnsupportedInputComputeBudget); + } let inputs = transaction .inputs .iter() @@ -115,3 +124,44 @@ impl TryFrom for Inner { Ok(Inner { global: Global::default(), inputs, outputs }) } } + +#[cfg(test)] +mod tests { + use super::*; + use kaspa_consensus_client::{TransactionInput, TransactionOutpoint}; + use kaspa_consensus_core::{ + mass::ComputeBudget, + subnets::SUBNETWORK_ID_NATIVE, + tx::{ + ScriptPublicKey, Transaction as CoreTransaction, TransactionId, TransactionInput as CoreTransactionInput, + TransactionOutput, + }, + }; + + #[test] + fn transaction_input_with_compute_budget_is_rejected() { + let input = TransactionInput::new(TransactionOutpoint::new(TransactionId::default(), 0), None, 0, 0, 1, None); + let err = Input::try_from(input).unwrap_err(); + assert!(matches!(err, Error::UnsupportedInputComputeBudget)); + } + + #[test] + fn v1_transaction_conversion_to_pskt_is_rejected() { + let tx = CoreTransaction::new( + 1, + vec![CoreTransactionInput { + previous_outpoint: Default::default(), + signature_script: vec![], + sequence: 0, + mass: cctx::TxInputMass::ComputeBudget(ComputeBudget(1)), + }], + vec![TransactionOutput::new(0, ScriptPublicKey::new(0, vec![].into()))], + 0, + SUBNETWORK_ID_NATIVE, + 0, + vec![], + ); + let err = Inner::try_from(tx).unwrap_err(); + assert!(matches!(err, Error::UnsupportedInputComputeBudget)); + } +} diff --git a/wallet/pskt/src/error.rs b/wallet/pskt/src/error.rs index 930e0a7060..3ee27f4fa4 100644 --- a/wallet/pskt/src/error.rs +++ b/wallet/pskt/src/error.rs @@ -9,6 +9,8 @@ pub enum Error { #[error("{0}")] Custom(String), #[error(transparent)] + ConsensusClient(#[from] kaspa_consensus_client::error::Error), + #[error(transparent)] ConstructorError(#[from] ConstructorError), #[error("OutputNotModifiable")] OutOfBounds, @@ -46,6 +48,8 @@ pub enum Error { PayloadRequiresVersion1(crate::pskt::Version), #[error("Outputs not allowed to contain covenant due to pskt or tx versions mismatch")] Covenant, + #[error("PSKT does not yet support transaction input compute_budget")] + UnsupportedInputComputeBudget, } #[derive(thiserror::Error, Debug)] pub enum ConstructorError {