From 2f3ce715c4bc441bc0dfd241a691498a9de0d505 Mon Sep 17 00:00:00 2001 From: coderofstuff <114628839+coderofstuff@users.noreply.github.com> Date: Tue, 19 Mar 2024 20:49:52 -0600 Subject: [PATCH 01/14] Remote duplicate GetInfo on wrpc route --- rpc/wrpc/server/src/router.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/rpc/wrpc/server/src/router.rs b/rpc/wrpc/server/src/router.rs index 4d0e20625..513f04414 100644 --- a/rpc/wrpc/server/src/router.rs +++ b/rpc/wrpc/server/src/router.rs @@ -53,7 +53,6 @@ impl Router { GetFeeEstimateExperimental, GetHeaders, GetInfo, - GetInfo, GetMempoolEntries, GetMempoolEntriesByAddresses, GetMempoolEntry, From f7e2576f688066929b2414a64b271c9b5efdfb64 Mon Sep 17 00:00:00 2001 From: coderofstuff <114628839+coderofstuff@users.noreply.github.com> Date: Sat, 16 Mar 2024 09:01:24 -0600 Subject: [PATCH 02/14] Add GetUtxoReturnAddress RPC scaffolding --- cli/src/modules/rpc.rs | 15 +++++ components/consensusmanager/src/session.rs | 6 +- consensus/core/src/api/mod.rs | 6 +- consensus/src/consensus/mod.rs | 65 ++++++++++++++++++- rpc/core/src/api/ops.rs | 2 + rpc/core/src/api/rpc.rs | 12 ++++ rpc/core/src/model/message.rs | 63 ++++++++++++++++++ rpc/grpc/client/src/lib.rs | 1 + rpc/grpc/core/proto/messages.proto | 2 + rpc/grpc/core/proto/rpc.proto | 12 +++- rpc/grpc/core/src/convert/kaspad.rs | 2 + rpc/grpc/core/src/convert/message.rs | 28 +++++++- rpc/grpc/core/src/ops.rs | 1 + .../server/src/request_handler/factory.rs | 1 + rpc/grpc/server/src/tests/rpc_core_mock.rs | 8 +++ rpc/service/src/service.rs | 11 ++++ rpc/wrpc/client/src/client.rs | 1 + rpc/wrpc/server/src/router.rs | 2 + testing/integration/src/rpc_tests.rs | 17 +++++ wallet/core/src/tests/rpc_core_mock.rs | 8 +++ 20 files changed, 258 insertions(+), 5 deletions(-) diff --git a/cli/src/modules/rpc.rs b/cli/src/modules/rpc.rs index f32523c4a..5505504eb 100644 --- a/cli/src/modules/rpc.rs +++ b/cli/src/modules/rpc.rs @@ -258,6 +258,21 @@ impl Rpc { let result = rpc.get_current_block_color_call(None, GetCurrentBlockColorRequest { hash }).await?; self.println(&ctx, result); } + RpcApiOps::GetUtxoReturnAddress => { + if argv.is_empty() || argv.len() != 2 { + return Err(Error::custom("Please specify a txid and a accepting_block_daa_score")); + } + + let txid = argv.remove(0); + let txid = RpcHash::from_hex(txid.as_str())?; + + let accepting_block_daa_score = argv.remove(0).parse::()?; + + let result = + rpc.get_utxo_return_address_call(None, GetUtxoReturnAddressRequest { txid, accepting_block_daa_score }).await?; + + self.println(&ctx, result); + } _ => { tprintln!(ctx, "rpc method exists but is not supported by the cli: '{op_str}'\r\n"); return Ok(()); diff --git a/components/consensusmanager/src/session.rs b/components/consensusmanager/src/session.rs index 81d589148..44b767a92 100644 --- a/components/consensusmanager/src/session.rs +++ b/components/consensusmanager/src/session.rs @@ -12,7 +12,7 @@ use kaspa_consensus_core::{ header::Header, pruning::{PruningPointProof, PruningPointTrustedData, PruningPointsList}, trusted::{ExternalGhostdagData, TrustedBlock}, - tx::{MutableTransaction, Transaction, TransactionOutpoint, UtxoEntry}, + tx::{MutableTransaction, ScriptPublicKey, Transaction, TransactionOutpoint, UtxoEntry}, BlockHashSet, BlueWorkType, ChainPath, Hash, }; use kaspa_utils::sync::rwlock::*; @@ -308,6 +308,10 @@ impl ConsensusSessionOwned { self.clone().spawn_blocking(|c| c.get_chain_block_samples()).await } + pub async fn async_get_utxo_return_address(&self, txid: Hash, accepting_block_daa_score: u64) -> Option { + self.clone().spawn_blocking(move |c| c.get_utxo_return_address(txid, accepting_block_daa_score)).await + } + /// Returns the antipast of block `hash` from the POV of `context`, i.e. `antipast(hash) ∩ past(context)`. /// Since this might be an expensive operation for deep blocks, we allow the caller to specify a limit /// `max_traversal_allowed` on the maximum amount of blocks to traverse for obtaining the answer diff --git a/consensus/core/src/api/mod.rs b/consensus/core/src/api/mod.rs index 4833c7659..f9e804875 100644 --- a/consensus/core/src/api/mod.rs +++ b/consensus/core/src/api/mod.rs @@ -19,7 +19,7 @@ use crate::{ header::Header, pruning::{PruningPointProof, PruningPointTrustedData, PruningPointsList}, trusted::{ExternalGhostdagData, TrustedBlock}, - tx::{MutableTransaction, Transaction, TransactionOutpoint, UtxoEntry}, + tx::{MutableTransaction, ScriptPublicKey, Transaction, TransactionOutpoint, UtxoEntry}, BlockHashSet, BlueWorkType, ChainPath, }; use kaspa_hashes::Hash; @@ -165,6 +165,10 @@ pub trait ConsensusApi: Send + Sync { unimplemented!() } + fn get_utxo_return_address(&self, txid: Hash, daa_score: u64) -> Option { + unimplemented!() + } + fn get_virtual_parents(&self) -> BlockHashSet { unimplemented!() } diff --git a/consensus/src/consensus/mod.rs b/consensus/src/consensus/mod.rs index 8474a6864..1d9c5e67b 100644 --- a/consensus/src/consensus/mod.rs +++ b/consensus/src/consensus/mod.rs @@ -66,7 +66,7 @@ use kaspa_consensus_core::{ network::NetworkType, pruning::{PruningPointProof, PruningPointTrustedData, PruningPointsList}, trusted::{ExternalGhostdagData, TrustedBlock}, - tx::{MutableTransaction, Transaction, TransactionOutpoint, UtxoEntry}, + tx::{MutableTransaction, ScriptPublicKey, Transaction, TransactionOutpoint, UtxoEntry}, BlockHashSet, BlueWorkType, ChainPath, HashMapCustomHasher, }; use kaspa_consensus_notify::root::ConsensusNotificationRoot; @@ -684,6 +684,69 @@ impl ConsensusApi for Consensus { sample_headers } + fn get_utxo_return_address(&self, txid: Hash, daa_score: u64) -> Option { + // We need consistency between the past pruning points, selected chain and header store reads + let _guard = self.pruning_lock.blocking_read(); + + let sc_read = self.selected_chain_store.read(); + + let low = self.pruning_point_store.read().get().unwrap().pruning_point; + let high = sc_read.get_tip().unwrap().1; + + let mut low_index = sc_read.get_by_hash(low).unwrap(); + let mut high_index = sc_read.get_by_hash(high).unwrap(); + + // let mut locator = Vec::with_capacity((high_index - low_index).ceiling_log_base_2() as usize); + // let mut current_index = high_index; + let mut matching_chain_block_hash: Option = None; + while low_index < high_index { + let mid = low_index + (high_index - low_index) / 2; + + if let Ok(hash) = sc_read.get_by_index(mid) { + if let Ok(compact_header) = self.headers_store.get_compact_header_data(hash) { + if compact_header.daa_score == daa_score { + // We found the chain block we need + matching_chain_block_hash = Some(hash); + break; + } else if compact_header.daa_score < daa_score { + low_index = mid + 1; + } else { + high_index = mid - 1; + } + } else { + println!("Did not find a compact header with hash {}", hash); + break; + } + } else { + println!("Did not find a hash at index {}", mid); + break; + } + } + + if matching_chain_block_hash.is_none() { + println!("No matching chain block hash found"); + return None; + } + + if let Ok(acceptance_data) = self.acceptance_data_store.get(matching_chain_block_hash.unwrap()) { + let containing_acceptance = acceptance_data + .iter() + .find(|&mbad| mbad.accepted_transactions.iter().find(|&tx| tx.transaction_id == txid).is_some()) + .cloned(); + + if let Some(containing_acceptance) = containing_acceptance { + // I found the merged block containing the TXID + // let txs = self.block_transactions_store.get(containing_acceptance.block_hash).unwrap().iter().find(|tx| => ) + } else { + return None; + } + + return None; + } else { + return None; + }; + } + fn get_virtual_parents(&self) -> BlockHashSet { self.lkg_virtual_state.load().parents.iter().copied().collect() } diff --git a/rpc/core/src/api/ops.rs b/rpc/core/src/api/ops.rs index 822798a1d..ffa3e7127 100644 --- a/rpc/core/src/api/ops.rs +++ b/rpc/core/src/api/ops.rs @@ -132,6 +132,8 @@ pub enum RpcApiOps { GetFeeEstimateExperimental = 148, /// Block color determination by iterating DAG. GetCurrentBlockColor = 149, + /// Get UTXO Return Addresses + GetUtxoReturnAddress = 150, } impl RpcApiOps { diff --git a/rpc/core/src/api/rpc.rs b/rpc/core/src/api/rpc.rs index 85713e547..7866c44f1 100644 --- a/rpc/core/src/api/rpc.rs +++ b/rpc/core/src/api/rpc.rs @@ -436,6 +436,18 @@ pub trait RpcApi: Sync + Send + AnySync { request: GetDaaScoreTimestampEstimateRequest, ) -> RpcResult; + async fn get_utxo_return_address(&self, txid: RpcHash, accepting_block_daa_score: u64) -> RpcResult> { + Ok(self + .get_utxo_return_address_call(None, GetUtxoReturnAddressRequest { txid, accepting_block_daa_score }) + .await? + .return_address) + } + async fn get_utxo_return_address_call( + &self, + _connection: Option<&DynRpcConnection>, + request: GetUtxoReturnAddressRequest, + ) -> RpcResult; + // ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ // Fee estimation API diff --git a/rpc/core/src/model/message.rs b/rpc/core/src/model/message.rs index ba8d6abf7..ab327bdeb 100644 --- a/rpc/core/src/model/message.rs +++ b/rpc/core/src/model/message.rs @@ -2666,6 +2666,69 @@ impl Deserializer for GetCurrentBlockColorResponse { } } +#[derive(Clone, Debug, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct GetUtxoReturnAddressRequest { + pub txid: RpcHash, + pub accepting_block_daa_score: u64, +} + +impl GetUtxoReturnAddressRequest { + pub fn new(txid: RpcHash, accepting_block_daa_score: u64) -> Self { + Self { txid, accepting_block_daa_score } + } +} + +impl Serializer for GetUtxoReturnAddressRequest { + fn serialize(&self, writer: &mut W) -> std::io::Result<()> { + store!(u16, &1, writer)?; + store!(RpcHash, &self.txid, writer)?; + store!(u64, &self.accepting_block_daa_score, writer)?; + + Ok(()) + } +} + +impl Deserializer for GetUtxoReturnAddressRequest { + fn deserialize(reader: &mut R) -> std::io::Result { + let _version = load!(u16, reader)?; + let txid = load!(RpcHash, reader)?; + let accepting_block_daa_score = load!(u64, reader)?; + + Ok(Self { txid, accepting_block_daa_score }) + } +} + +#[derive(Clone, Debug, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct GetUtxoReturnAddressResponse { + pub return_address: Option, +} + +impl GetUtxoReturnAddressResponse { + pub fn new(return_address: Option) -> Self { + Self { return_address } + } +} + +impl Serializer for GetUtxoReturnAddressResponse { + fn serialize(&self, writer: &mut W) -> std::io::Result<()> { + store!(u16, &1, writer)?; + store!(Option, &self.return_address, writer)?; + + Ok(()) + } +} + +impl Deserializer for GetUtxoReturnAddressResponse { + fn deserialize(reader: &mut R) -> std::io::Result { + let _version = load!(u16, reader)?; + let return_address = load!(Option, reader)?; + + Ok(Self { return_address }) + } +} + // ---------------------------------------------------------------------------- // Subscriptions & notifications // ---------------------------------------------------------------------------- diff --git a/rpc/grpc/client/src/lib.rs b/rpc/grpc/client/src/lib.rs index 74db82c4e..330f80e0f 100644 --- a/rpc/grpc/client/src/lib.rs +++ b/rpc/grpc/client/src/lib.rs @@ -277,6 +277,7 @@ impl RpcApi for GrpcClient { route!(get_fee_estimate_call, GetFeeEstimate); route!(get_fee_estimate_experimental_call, GetFeeEstimateExperimental); route!(get_current_block_color_call, GetCurrentBlockColor); + route!(get_utxo_return_address_call, GetUtxoReturnAddress); // ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ // Notification API diff --git a/rpc/grpc/core/proto/messages.proto b/rpc/grpc/core/proto/messages.proto index 2d6310d9e..ccb2798b6 100644 --- a/rpc/grpc/core/proto/messages.proto +++ b/rpc/grpc/core/proto/messages.proto @@ -65,6 +65,7 @@ message KaspadRequest { GetFeeEstimateRequestMessage getFeeEstimateRequest = 1106; GetFeeEstimateExperimentalRequestMessage getFeeEstimateExperimentalRequest = 1108; GetCurrentBlockColorRequestMessage getCurrentBlockColorRequest = 1110; + GetUtxoReturnAddressRequestMessage GetUtxoReturnAddressRequest = 1112; } } @@ -130,6 +131,7 @@ message KaspadResponse { GetFeeEstimateResponseMessage getFeeEstimateResponse = 1107; GetFeeEstimateExperimentalResponseMessage getFeeEstimateExperimentalResponse = 1109; GetCurrentBlockColorResponseMessage getCurrentBlockColorResponse = 1111; + GetUtxoReturnAddressResponseMessage GetUtxoReturnAddressResponse = 1113; } } diff --git a/rpc/grpc/core/proto/rpc.proto b/rpc/grpc/core/proto/rpc.proto index 7a38f8852..dc7108742 100644 --- a/rpc/grpc/core/proto/rpc.proto +++ b/rpc/grpc/core/proto/rpc.proto @@ -900,7 +900,7 @@ message GetDaaScoreTimestampEstimateRequestMessage { repeated uint64 daa_scores = 1; } -message GetDaaScoreTimestampEstimateResponseMessage{ +message GetDaaScoreTimestampEstimateResponseMessage { repeated uint64 timestamps = 1; RPCError error = 1000; } @@ -970,3 +970,13 @@ message GetCurrentBlockColorResponseMessage { RPCError error = 1000; } + +message GetUtxoReturnAddressRequestMessage { + string txid = 1; + uint64 accepting_block_daa_score = 2; +} + +message GetUtxoReturnAddressResponseMessage { + string return_address = 1; + RPCError error = 1000; +} diff --git a/rpc/grpc/core/src/convert/kaspad.rs b/rpc/grpc/core/src/convert/kaspad.rs index c3411545c..7243fd401 100644 --- a/rpc/grpc/core/src/convert/kaspad.rs +++ b/rpc/grpc/core/src/convert/kaspad.rs @@ -63,6 +63,7 @@ pub mod kaspad_request_convert { impl_into_kaspad_request!(GetFeeEstimate); impl_into_kaspad_request!(GetFeeEstimateExperimental); impl_into_kaspad_request!(GetCurrentBlockColor); + impl_into_kaspad_request!(GetUtxoReturnAddress); impl_into_kaspad_request!(NotifyBlockAdded); impl_into_kaspad_request!(NotifyNewBlockTemplate); @@ -200,6 +201,7 @@ pub mod kaspad_response_convert { impl_into_kaspad_response!(GetFeeEstimate); impl_into_kaspad_response!(GetFeeEstimateExperimental); impl_into_kaspad_response!(GetCurrentBlockColor); + impl_into_kaspad_response!(GetUtxoReturnAddress); impl_into_kaspad_notify_response!(NotifyBlockAdded); impl_into_kaspad_notify_response!(NotifyNewBlockTemplate); diff --git a/rpc/grpc/core/src/convert/message.rs b/rpc/grpc/core/src/convert/message.rs index c0e75cf03..723311704 100644 --- a/rpc/grpc/core/src/convert/message.rs +++ b/rpc/grpc/core/src/convert/message.rs @@ -19,7 +19,7 @@ //! The SubmitBlockResponse is a notable exception to this general rule. use crate::protowire::{self, submit_block_response_message::RejectReason}; -use kaspa_consensus_core::network::NetworkId; +use kaspa_consensus_core::{network::NetworkId, Hash}; use kaspa_core::debug; use kaspa_notify::subscription::Command; use kaspa_rpc_core::{ @@ -430,6 +430,21 @@ from!(item: RpcResult<&kaspa_rpc_core::GetCurrentBlockColorResponse>, protowire: Self { blue: item.blue, error: None } }); +from!(item: &kaspa_rpc_core::GetUtxoReturnAddressRequest, protowire::GetUtxoReturnAddressRequestMessage, { + Self { + txid: item.txid.to_string(), + accepting_block_daa_score: item.accepting_block_daa_score + } +}); +from!(item: RpcResult<&kaspa_rpc_core::GetUtxoReturnAddressResponse>, protowire::GetUtxoReturnAddressResponseMessage, { + if let Some(return_address) = &item.return_address { + Self { return_address: return_address.into(), error: None } + } else { + Self { return_address: String::from(""), error: None } + } + +}); + from!(&kaspa_rpc_core::PingRequest, protowire::PingRequestMessage); from!(RpcResult<&kaspa_rpc_core::PingResponse>, protowire::PingResponseMessage); @@ -916,6 +931,17 @@ try_from!(item: &protowire::GetCurrentBlockColorResponseMessage, RpcResult, { + // Self { return_address: Some(Address::from(item.return_address)) } + // TODO: import Address + Self { return_address: None } +}); try_from!(&protowire::PingRequestMessage, kaspa_rpc_core::PingRequest); try_from!(&protowire::PingResponseMessage, RpcResult); diff --git a/rpc/grpc/core/src/ops.rs b/rpc/grpc/core/src/ops.rs index f3bc12c82..223774c74 100644 --- a/rpc/grpc/core/src/ops.rs +++ b/rpc/grpc/core/src/ops.rs @@ -87,6 +87,7 @@ pub enum KaspadPayloadOps { GetFeeEstimate, GetFeeEstimateExperimental, GetCurrentBlockColor, + GetUtxoReturnAddress, // Subscription commands for starting/stopping notifications NotifyBlockAdded, diff --git a/rpc/grpc/server/src/request_handler/factory.rs b/rpc/grpc/server/src/request_handler/factory.rs index b6a5b4476..9fec86e47 100644 --- a/rpc/grpc/server/src/request_handler/factory.rs +++ b/rpc/grpc/server/src/request_handler/factory.rs @@ -81,6 +81,7 @@ impl Factory { GetFeeEstimate, GetFeeEstimateExperimental, GetCurrentBlockColor, + GetUtxoReturnAddress, NotifyBlockAdded, NotifyNewBlockTemplate, NotifyFinalityConflict, diff --git a/rpc/grpc/server/src/tests/rpc_core_mock.rs b/rpc/grpc/server/src/tests/rpc_core_mock.rs index dd6de46d2..ada801850 100644 --- a/rpc/grpc/server/src/tests/rpc_core_mock.rs +++ b/rpc/grpc/server/src/tests/rpc_core_mock.rs @@ -362,6 +362,14 @@ impl RpcApi for RpcCoreMock { Err(RpcError::NotImplemented) } + async fn get_utxo_return_address_call( + &self, + _connection: Option<&DynRpcConnection>, + _request: GetUtxoReturnAddressRequest, + ) -> RpcResult { + Err(RpcError::NotImplemented) + } + // ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ // Notification API diff --git a/rpc/service/src/service.rs b/rpc/service/src/service.rs index 2c22fd6bb..86d2cb087 100644 --- a/rpc/service/src/service.rs +++ b/rpc/service/src/service.rs @@ -781,6 +781,17 @@ NOTE: This error usually indicates an RPC conversion error between the node and } } + async fn get_utxo_return_address_call( + &self, + _connection: Option<&DynRpcConnection>, + request: GetUtxoReturnAddressRequest, + ) -> RpcResult { + // let session = self.consensus_manager.consensus().session().await; + println!("{} {}", request.txid, request.accepting_block_daa_score); + // let mut maybe_spk = session.async_get_utxo_return_address(request.txid, request.accepting_block_daa_score).await; + Ok(GetUtxoReturnAddressResponse { return_address: None }) + } + async fn ping_call(&self, _connection: Option<&DynRpcConnection>, _: PingRequest) -> RpcResult { Ok(PingResponse {}) } diff --git a/rpc/wrpc/client/src/client.rs b/rpc/wrpc/client/src/client.rs index 2bd66b7cc..eed197970 100644 --- a/rpc/wrpc/client/src/client.rs +++ b/rpc/wrpc/client/src/client.rs @@ -635,6 +635,7 @@ impl RpcApi for KaspaRpcClient { GetSubnetwork, GetSyncStatus, GetSystemInfo, + GetUtxoReturnAddress, GetUtxosByAddresses, GetVirtualChainFromBlock, ResolveFinalityConflict, diff --git a/rpc/wrpc/server/src/router.rs b/rpc/wrpc/server/src/router.rs index 513f04414..b4c74a337 100644 --- a/rpc/wrpc/server/src/router.rs +++ b/rpc/wrpc/server/src/router.rs @@ -47,6 +47,8 @@ impl Router { GetCurrentBlockColor, GetCoinSupply, GetConnectedPeerInfo, + GetDaaScoreTimestampEstimate, + GetUtxoReturnAddress, GetCurrentNetwork, GetDaaScoreTimestampEstimate, GetFeeEstimate, diff --git a/testing/integration/src/rpc_tests.rs b/testing/integration/src/rpc_tests.rs index 3c4df601b..3ca619423 100644 --- a/testing/integration/src/rpc_tests.rs +++ b/testing/integration/src/rpc_tests.rs @@ -656,6 +656,23 @@ async fn sanity_test() { }) } + KaspadPayloadOps::GetUtxoReturnAddress => { + let rpc_client = client.clone(); + tst!(op, { + let results = rpc_client.get_utxo_return_address(RpcHash::from_bytes([0; 32]), 1000).await; + + assert!(results.is_err_and(|err| { + match err { + kaspa_rpc_core::RpcError::General(msg) => { + info!("Expected error message: {}", msg); + true + } + _ => false, + } + })); + }) + } + KaspadPayloadOps::NotifyBlockAdded => { let rpc_client = client.clone(); let id = listener_id; diff --git a/wallet/core/src/tests/rpc_core_mock.rs b/wallet/core/src/tests/rpc_core_mock.rs index 4d10cdd9b..529fffffe 100644 --- a/wallet/core/src/tests/rpc_core_mock.rs +++ b/wallet/core/src/tests/rpc_core_mock.rs @@ -379,6 +379,14 @@ impl RpcApi for RpcCoreMock { Err(RpcError::NotImplemented) } + async fn get_utxo_return_address_call( + &self, + _connection: Option<&DynRpcConnection>, + _request: GetUtxoReturnAddressRequest, + ) -> RpcResult { + Err(RpcError::NotImplemented) + } + // ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ // Notification API From 94dd8139acf9d8eb14180e2d8c503c788c098de8 Mon Sep 17 00:00:00 2001 From: coderofstuff <114628839+coderofstuff@users.noreply.github.com> Date: Tue, 19 Mar 2024 23:29:54 -0600 Subject: [PATCH 03/14] Apply end-to-end logic for utxo return address --- components/consensusmanager/src/session.rs | 8 +- consensus/core/src/api/mod.rs | 2 +- consensus/src/consensus/mod.rs | 108 +++++++++++++-------- rpc/service/src/service.rs | 31 +++++- 4 files changed, 104 insertions(+), 45 deletions(-) diff --git a/components/consensusmanager/src/session.rs b/components/consensusmanager/src/session.rs index 44b767a92..b965f7375 100644 --- a/components/consensusmanager/src/session.rs +++ b/components/consensusmanager/src/session.rs @@ -308,8 +308,12 @@ impl ConsensusSessionOwned { self.clone().spawn_blocking(|c| c.get_chain_block_samples()).await } - pub async fn async_get_utxo_return_address(&self, txid: Hash, accepting_block_daa_score: u64) -> Option { - self.clone().spawn_blocking(move |c| c.get_utxo_return_address(txid, accepting_block_daa_score)).await + pub async fn async_get_utxo_return_script_public_key( + &self, + txid: Hash, + accepting_block_daa_score: u64, + ) -> Option { + self.clone().spawn_blocking(move |c| c.get_utxo_return_script_public_key(txid, accepting_block_daa_score)).await } /// Returns the antipast of block `hash` from the POV of `context`, i.e. `antipast(hash) ∩ past(context)`. diff --git a/consensus/core/src/api/mod.rs b/consensus/core/src/api/mod.rs index f9e804875..e91ffa46d 100644 --- a/consensus/core/src/api/mod.rs +++ b/consensus/core/src/api/mod.rs @@ -165,7 +165,7 @@ pub trait ConsensusApi: Send + Sync { unimplemented!() } - fn get_utxo_return_address(&self, txid: Hash, daa_score: u64) -> Option { + fn get_utxo_return_script_public_key(&self, txid: Hash, daa_score: u64) -> Option { unimplemented!() } diff --git a/consensus/src/consensus/mod.rs b/consensus/src/consensus/mod.rs index 1d9c5e67b..3f9d66dd4 100644 --- a/consensus/src/consensus/mod.rs +++ b/consensus/src/consensus/mod.rs @@ -77,6 +77,7 @@ use crossbeam_channel::{ use itertools::Itertools; use kaspa_consensusmanager::{SessionLock, SessionReadGuard}; +use kaspa_core::{trace, warn}; use kaspa_database::prelude::StoreResultExtensions; use kaspa_hashes::Hash; use kaspa_muhash::MuHash; @@ -102,6 +103,9 @@ use crate::model::stores::selected_chain::SelectedChainStoreReader; use std::cmp; +use crate::model::stores::utxo_diffs::UtxoDiffsStoreReader; +use kaspa_consensus_core::utxo::utxo_diff::ImmutableUtxoDiff; + pub struct Consensus { // DB db: Arc, @@ -684,67 +688,95 @@ impl ConsensusApi for Consensus { sample_headers } - fn get_utxo_return_address(&self, txid: Hash, daa_score: u64) -> Option { + fn get_utxo_return_script_public_key(&self, txid: Hash, target_daa_score: u64) -> Option { // We need consistency between the past pruning points, selected chain and header store reads let _guard = self.pruning_lock.blocking_read(); let sc_read = self.selected_chain_store.read(); - let low = self.pruning_point_store.read().get().unwrap().pruning_point; - let high = sc_read.get_tip().unwrap().1; + let pp_hash = self.pruning_point_store.read().get().unwrap().pruning_point; + let pp_index = sc_read.get_by_hash(pp_hash).unwrap(); + let (tip_index, tip_hash) = sc_read.get_tip().unwrap(); + let tip_daa_score = self.headers_store.get_compact_header_data(tip_hash).unwrap().daa_score; - let mut low_index = sc_read.get_by_hash(low).unwrap(); - let mut high_index = sc_read.get_by_hash(high).unwrap(); + let mut low_index = tip_index.saturating_sub(tip_daa_score.saturating_sub(target_daa_score)).max(pp_index); + let mut high_index = tip_index; - // let mut locator = Vec::with_capacity((high_index - low_index).ceiling_log_base_2() as usize); - // let mut current_index = high_index; let mut matching_chain_block_hash: Option = None; - while low_index < high_index { + while low_index <= high_index { let mid = low_index + (high_index - low_index) / 2; if let Ok(hash) = sc_read.get_by_index(mid) { if let Ok(compact_header) = self.headers_store.get_compact_header_data(hash) { - if compact_header.daa_score == daa_score { - // We found the chain block we need - matching_chain_block_hash = Some(hash); - break; - } else if compact_header.daa_score < daa_score { - low_index = mid + 1; - } else { - high_index = mid - 1; + match compact_header.daa_score.cmp(&target_daa_score) { + cmp::Ordering::Equal => { + // We found the chain block we need + matching_chain_block_hash = Some(hash); + break; + } + cmp::Ordering::Greater => { + high_index = mid - 1; + } + cmp::Ordering::Less => { + low_index = mid + 1; + } } } else { - println!("Did not find a compact header with hash {}", hash); - break; + trace!("Did not find a compact header with hash {}", hash); + return None; } } else { - println!("Did not find a hash at index {}", mid); - break; + trace!("Did not find a hash at index {}", mid); + return None; } } - if matching_chain_block_hash.is_none() { - println!("No matching chain block hash found"); - return None; - } + let matching_chain_block_hash = matching_chain_block_hash?; + + if let Ok(acceptance_data) = self.acceptance_data_store.get(matching_chain_block_hash) { + let maybe_index_and_containing_acceptance = + acceptance_data.iter().find_map(|mbad| { + let tx_arr_index = mbad.accepted_transactions.iter().enumerate().find_map(|(index, tx)| { + if tx.transaction_id == txid { + Some(index) + } else { + None + } + }); + + tx_arr_index.map(|index| (index, mbad.clone())) + }); + + if let Some((index, containing_acceptance)) = maybe_index_and_containing_acceptance { + // Found Merged block containing the TXID + let tx = &self.block_transactions_store.get(containing_acceptance.block_hash).unwrap()[index]; + + if tx.id() != txid { + // Should never happen, but do a sanity check. This would mean something went wrong with storing block transactions + // Sanity check is necessary to guarantee that this function will never give back a wrong address (err on the side of None) + warn!( + "Expected {} to match {} when checking block_transaction_store using array index of transaction", + tx.id(), + txid + ); + return None; + } - if let Ok(acceptance_data) = self.acceptance_data_store.get(matching_chain_block_hash.unwrap()) { - let containing_acceptance = acceptance_data - .iter() - .find(|&mbad| mbad.accepted_transactions.iter().find(|&tx| tx.transaction_id == txid).is_some()) - .cloned(); + if tx.inputs.is_empty() { + // A transaction may have no inputs (like a coinbase transaction) + return None; + } - if let Some(containing_acceptance) = containing_acceptance { - // I found the merged block containing the TXID - // let txs = self.block_transactions_store.get(containing_acceptance.block_hash).unwrap().iter().find(|tx| => ) - } else { - return None; - } + let first_input_prev_outpoint = &tx.inputs[0].previous_outpoint; + // Expected to never fail, since + let utxo_diff = self.utxo_diffs_store.get(matching_chain_block_hash).unwrap(); + let removed_diffs = utxo_diff.removed(); - return None; - } else { - return None; + return Some(removed_diffs.get(first_input_prev_outpoint)?.script_public_key.clone()); + } }; + + None } fn get_virtual_parents(&self) -> BlockHashSet { diff --git a/rpc/service/src/service.rs b/rpc/service/src/service.rs index 86d2cb087..e754995c0 100644 --- a/rpc/service/src/service.rs +++ b/rpc/service/src/service.rs @@ -63,6 +63,7 @@ use kaspa_rpc_core::{ notify::connection::ChannelConnection, Notification, RpcError, RpcResult, }; +use kaspa_txscript::opcodes::codes; use kaspa_txscript::{extract_script_pub_key_address, pay_to_address_script}; use kaspa_utils::expiring_cache::ExpiringCache; use kaspa_utils::sysinfo::SystemInfo; @@ -786,10 +787,32 @@ NOTE: This error usually indicates an RPC conversion error between the node and _connection: Option<&DynRpcConnection>, request: GetUtxoReturnAddressRequest, ) -> RpcResult { - // let session = self.consensus_manager.consensus().session().await; - println!("{} {}", request.txid, request.accepting_block_daa_score); - // let mut maybe_spk = session.async_get_utxo_return_address(request.txid, request.accepting_block_daa_score).await; - Ok(GetUtxoReturnAddressResponse { return_address: None }) + let session = self.consensus_manager.consensus().session().await; + + let mut return_address = None; + + // Convert a SPK to an Address + if let Some(spk) = session.async_get_utxo_return_script_public_key(request.txid, request.accepting_block_daa_score).await { + let script = spk.script(); + + // Standard Address scripts are only either 34 or 35 in length: + if script.len() == 34 && script[0] == codes::OpData32 && script[33] == codes::OpCheckSig { + // This is a standard Schnorr Address + return_address = Some(RpcAddress::new(self.config.prefix(), kaspa_addresses::Version::PubKey, &script[1..33])); + } else if script.len() == 35 { + // Could be ECDSA address OR P2SH + if script[0] == codes::OpData33 && script[34] == codes::OpCheckSigECDSA { + // This is an standard ECDSA Address + return_address = + Some(RpcAddress::new(self.config.prefix(), kaspa_addresses::Version::PubKeyECDSA, &script[1..34])); + } else if script[0] == codes::OpBlake2b && script[1] == codes::OpData32 && script[34] == codes::OpEqual { + // This is a standard P2SH Address + return_address = Some(RpcAddress::new(self.config.prefix(), kaspa_addresses::Version::ScriptHash, &script[2..34])); + } + } + }; + + Ok(GetUtxoReturnAddressResponse { return_address }) } async fn ping_call(&self, _connection: Option<&DynRpcConnection>, _: PingRequest) -> RpcResult { From 3deaf0de7e919d2c7cc82be048d61ce099a65970 Mon Sep 17 00:00:00 2001 From: coderofstuff <114628839+coderofstuff@users.noreply.github.com> Date: Fri, 22 Mar 2024 21:09:00 -0600 Subject: [PATCH 04/14] Refactor to reduce nestedness; Add comments Co-authored-by: Maxim <59533214+biryukovmaxim@users.noreply.github.com> --- consensus/src/consensus/mod.rs | 128 ++++++++++++++++----------------- 1 file changed, 62 insertions(+), 66 deletions(-) diff --git a/consensus/src/consensus/mod.rs b/consensus/src/consensus/mod.rs index 3f9d66dd4..804b5086e 100644 --- a/consensus/src/consensus/mod.rs +++ b/consensus/src/consensus/mod.rs @@ -702,81 +702,77 @@ impl ConsensusApi for Consensus { let mut low_index = tip_index.saturating_sub(tip_daa_score.saturating_sub(target_daa_score)).max(pp_index); let mut high_index = tip_index; - let mut matching_chain_block_hash: Option = None; - while low_index <= high_index { + let matching_chain_block_hash = loop { + // Binary search for the chain block that matches the target_daa_score + // 0. Get the mid point index let mid = low_index + (high_index - low_index) / 2; - if let Ok(hash) = sc_read.get_by_index(mid) { - if let Ok(compact_header) = self.headers_store.get_compact_header_data(hash) { - match compact_header.daa_score.cmp(&target_daa_score) { - cmp::Ordering::Equal => { - // We found the chain block we need - matching_chain_block_hash = Some(hash); - break; - } - cmp::Ordering::Greater => { - high_index = mid - 1; - } - cmp::Ordering::Less => { - low_index = mid + 1; - } - } - } else { + // 1. Get the chain block hash at that index. Error if we don't find a hash at an index + let hash = sc_read + .get_by_index(mid) + .map_err(|err| { + trace!("Did not find a hash at index {}", mid); + err + }) + .ok()?; + + // 2. Get the compact header so we have access to the daa_score. Error if we + let compact_header = self + .headers_store + .get_compact_header_data(hash) + .map_err(|err| { trace!("Did not find a compact header with hash {}", hash); - return None; + err + }) + .ok()?; + + // 3. Compare block daa score to our target + match compact_header.daa_score.cmp(&target_daa_score) { + cmp::Ordering::Equal => { + // We found the chain block we need + break Some(hash); } - } else { - trace!("Did not find a hash at index {}", mid); - return None; - } - } - - let matching_chain_block_hash = matching_chain_block_hash?; - - if let Ok(acceptance_data) = self.acceptance_data_store.get(matching_chain_block_hash) { - let maybe_index_and_containing_acceptance = - acceptance_data.iter().find_map(|mbad| { - let tx_arr_index = mbad.accepted_transactions.iter().enumerate().find_map(|(index, tx)| { - if tx.transaction_id == txid { - Some(index) - } else { - None - } - }); - - tx_arr_index.map(|index| (index, mbad.clone())) - }); - - if let Some((index, containing_acceptance)) = maybe_index_and_containing_acceptance { - // Found Merged block containing the TXID - let tx = &self.block_transactions_store.get(containing_acceptance.block_hash).unwrap()[index]; - - if tx.id() != txid { - // Should never happen, but do a sanity check. This would mean something went wrong with storing block transactions - // Sanity check is necessary to guarantee that this function will never give back a wrong address (err on the side of None) - warn!( - "Expected {} to match {} when checking block_transaction_store using array index of transaction", - tx.id(), - txid - ); - return None; + cmp::Ordering::Greater => { + high_index = mid - 1; } - - if tx.inputs.is_empty() { - // A transaction may have no inputs (like a coinbase transaction) - return None; + cmp::Ordering::Less => { + low_index = mid + 1; } + } - let first_input_prev_outpoint = &tx.inputs[0].previous_outpoint; - // Expected to never fail, since - let utxo_diff = self.utxo_diffs_store.get(matching_chain_block_hash).unwrap(); - let removed_diffs = utxo_diff.removed(); - - return Some(removed_diffs.get(first_input_prev_outpoint)?.script_public_key.clone()); + if low_index > high_index { + break None; } - }; + }?; + + let acceptance_data = self.acceptance_data_store.get(matching_chain_block_hash).ok()?; + let (index, containing_acceptance) = acceptance_data.iter().find_map(|mbad| { + let tx_arr_index = + mbad.accepted_transactions.iter().enumerate().find_map(|(index, tx)| (tx.transaction_id == txid).then_some(index)); + tx_arr_index.map(|index| (index, mbad.clone())) + })?; + + // Found Merged block containing the TXID + let tx = &self.block_transactions_store.get(containing_acceptance.block_hash).unwrap()[index]; + + if tx.id() != txid { + // Should never happen, but do a sanity check. This would mean something went wrong with storing block transactions + // Sanity check is necessary to guarantee that this function will never give back a wrong address (err on the side of None) + warn!("Expected {} to match {} when checking block_transaction_store using array index of transaction", tx.id(), txid); + return None; + } - None + if tx.inputs.is_empty() { + // A transaction may have no inputs (like a coinbase transaction) + return None; + } + + let first_input_prev_outpoint = &tx.inputs[0].previous_outpoint; + // Expected to never fail, since we found the acceptance data and therefore there must be matching diff + let utxo_diff = self.utxo_diffs_store.get(matching_chain_block_hash).unwrap(); + let removed_diffs = utxo_diff.removed(); + + Some(removed_diffs.get(first_input_prev_outpoint)?.script_public_key.clone()) } fn get_virtual_parents(&self) -> BlockHashSet { From 1695480e60de5db728fa03c0d50bec7f2304ec31 Mon Sep 17 00:00:00 2001 From: coderofstuff <114628839+coderofstuff@users.noreply.github.com> Date: Sat, 23 Mar 2024 12:20:23 -0600 Subject: [PATCH 05/14] Re-use existing script-to-address logic --- rpc/service/src/service.rs | 27 ++++++--------------------- 1 file changed, 6 insertions(+), 21 deletions(-) diff --git a/rpc/service/src/service.rs b/rpc/service/src/service.rs index e754995c0..9780951d6 100644 --- a/rpc/service/src/service.rs +++ b/rpc/service/src/service.rs @@ -63,7 +63,6 @@ use kaspa_rpc_core::{ notify::connection::ChannelConnection, Notification, RpcError, RpcResult, }; -use kaspa_txscript::opcodes::codes; use kaspa_txscript::{extract_script_pub_key_address, pay_to_address_script}; use kaspa_utils::expiring_cache::ExpiringCache; use kaspa_utils::sysinfo::SystemInfo; @@ -789,27 +788,13 @@ NOTE: This error usually indicates an RPC conversion error between the node and ) -> RpcResult { let session = self.consensus_manager.consensus().session().await; - let mut return_address = None; - // Convert a SPK to an Address - if let Some(spk) = session.async_get_utxo_return_script_public_key(request.txid, request.accepting_block_daa_score).await { - let script = spk.script(); - - // Standard Address scripts are only either 34 or 35 in length: - if script.len() == 34 && script[0] == codes::OpData32 && script[33] == codes::OpCheckSig { - // This is a standard Schnorr Address - return_address = Some(RpcAddress::new(self.config.prefix(), kaspa_addresses::Version::PubKey, &script[1..33])); - } else if script.len() == 35 { - // Could be ECDSA address OR P2SH - if script[0] == codes::OpData33 && script[34] == codes::OpCheckSigECDSA { - // This is an standard ECDSA Address - return_address = - Some(RpcAddress::new(self.config.prefix(), kaspa_addresses::Version::PubKeyECDSA, &script[1..34])); - } else if script[0] == codes::OpBlake2b && script[1] == codes::OpData32 && script[34] == codes::OpEqual { - // This is a standard P2SH Address - return_address = Some(RpcAddress::new(self.config.prefix(), kaspa_addresses::Version::ScriptHash, &script[2..34])); - } - } + let return_address = if let Some(spk) = + session.async_get_utxo_return_script_public_key(request.txid, request.accepting_block_daa_score).await + { + extract_script_pub_key_address(&spk, self.config.prefix()).ok() + } else { + None }; Ok(GetUtxoReturnAddressResponse { return_address }) From 6be25c402a63d1cec1769d92d5b58947a14fe256 Mon Sep 17 00:00:00 2001 From: coderofstuff <114628839+coderofstuff@users.noreply.github.com> Date: Sun, 24 Mar 2024 23:47:22 -0600 Subject: [PATCH 06/14] Refactor utxo return address result --- components/consensusmanager/src/session.rs | 12 ++-- consensus/core/src/api/mod.rs | 35 +++++++++- consensus/src/consensus/mod.rs | 81 ++++++++++++++-------- rpc/core/src/error.rs | 5 +- rpc/service/src/service.rs | 13 ++-- 5 files changed, 100 insertions(+), 46 deletions(-) diff --git a/components/consensusmanager/src/session.rs b/components/consensusmanager/src/session.rs index b965f7375..8139bd087 100644 --- a/components/consensusmanager/src/session.rs +++ b/components/consensusmanager/src/session.rs @@ -4,7 +4,7 @@ use kaspa_consensus_core::{ acceptance_data::AcceptanceData, - api::{BlockCount, BlockValidationFutures, ConsensusApi, ConsensusStats, DynConsensus}, + api::{BlockCount, BlockValidationFutures, ConsensusApi, ConsensusStats, DynConsensus, ReturnAddress}, block::Block, blockstatus::BlockStatus, daa_score_timestamp::DaaScoreTimestamp, @@ -12,7 +12,7 @@ use kaspa_consensus_core::{ header::Header, pruning::{PruningPointProof, PruningPointTrustedData, PruningPointsList}, trusted::{ExternalGhostdagData, TrustedBlock}, - tx::{MutableTransaction, ScriptPublicKey, Transaction, TransactionOutpoint, UtxoEntry}, + tx::{MutableTransaction, Transaction, TransactionOutpoint, UtxoEntry}, BlockHashSet, BlueWorkType, ChainPath, Hash, }; use kaspa_utils::sync::rwlock::*; @@ -308,12 +308,8 @@ impl ConsensusSessionOwned { self.clone().spawn_blocking(|c| c.get_chain_block_samples()).await } - pub async fn async_get_utxo_return_script_public_key( - &self, - txid: Hash, - accepting_block_daa_score: u64, - ) -> Option { - self.clone().spawn_blocking(move |c| c.get_utxo_return_script_public_key(txid, accepting_block_daa_score)).await + pub async fn async_get_utxo_return_script_public_key(&self, txid: Hash, accepting_block_daa_score: u64) -> ReturnAddress { + self.clone().spawn_blocking(move |c| c.get_utxo_return_address(txid, accepting_block_daa_score)).await } /// Returns the antipast of block `hash` from the POV of `context`, i.e. `antipast(hash) ∩ past(context)`. diff --git a/consensus/core/src/api/mod.rs b/consensus/core/src/api/mod.rs index e91ffa46d..b395120b3 100644 --- a/consensus/core/src/api/mod.rs +++ b/consensus/core/src/api/mod.rs @@ -1,6 +1,10 @@ use futures_util::future::BoxFuture; +use kaspa_addresses::Address; use kaspa_muhash::MuHash; -use std::sync::Arc; +use std::{ + fmt::{Display, Formatter}, + sync::Arc, +}; use crate::{ acceptance_data::AcceptanceData, @@ -19,7 +23,7 @@ use crate::{ header::Header, pruning::{PruningPointProof, PruningPointTrustedData, PruningPointsList}, trusted::{ExternalGhostdagData, TrustedBlock}, - tx::{MutableTransaction, ScriptPublicKey, Transaction, TransactionOutpoint, UtxoEntry}, + tx::{MutableTransaction, Transaction, TransactionOutpoint, UtxoEntry}, BlockHashSet, BlueWorkType, ChainPath, }; use kaspa_hashes::Hash; @@ -43,6 +47,31 @@ pub struct BlockValidationFutures { pub virtual_state_task: BlockValidationFuture, } +#[derive(Debug, Clone)] +pub enum ReturnAddress { + Found(Address), + AlreadyPruned, + TxFromCoinbase, + NoTxAtScore, + NonStandard, + NotFound(String), +} + +impl Display for ReturnAddress { + #[inline] + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + let s = match self { + ReturnAddress::AlreadyPruned => "Transaction is already pruned".to_string(), + ReturnAddress::NoTxAtScore => "Transaction not found at given accepting daa score".to_string(), + ReturnAddress::NonStandard => "Transaction was found but not standard".to_string(), + ReturnAddress::TxFromCoinbase => "Transaction return address is coinbase".to_string(), + ReturnAddress::NotFound(reason) => format!("Transaction return address not found: {}", reason), + ReturnAddress::Found(address) => address.to_string(), + }; + f.write_str(&s) + } +} + /// Abstracts the consensus external API #[allow(unused_variables)] pub trait ConsensusApi: Send + Sync { @@ -165,7 +194,7 @@ pub trait ConsensusApi: Send + Sync { unimplemented!() } - fn get_utxo_return_script_public_key(&self, txid: Hash, daa_score: u64) -> Option { + fn get_utxo_return_address(&self, txid: Hash, daa_score: u64) -> ReturnAddress { unimplemented!() } diff --git a/consensus/src/consensus/mod.rs b/consensus/src/consensus/mod.rs index 804b5086e..35ff173b2 100644 --- a/consensus/src/consensus/mod.rs +++ b/consensus/src/consensus/mod.rs @@ -46,7 +46,7 @@ use kaspa_consensus_core::{ api::{ args::{TransactionValidationArgs, TransactionValidationBatchArgs}, stats::BlockCount, - BlockValidationFutures, ConsensusApi, ConsensusStats, + BlockValidationFutures, ConsensusApi, ConsensusStats, ReturnAddress, }, block::{Block, BlockTemplate, TemplateBuildMode, TemplateTransactionSelector, VirtualStateApproxId}, blockhash::BlockHashExtensions, @@ -66,7 +66,7 @@ use kaspa_consensus_core::{ network::NetworkType, pruning::{PruningPointProof, PruningPointTrustedData, PruningPointsList}, trusted::{ExternalGhostdagData, TrustedBlock}, - tx::{MutableTransaction, ScriptPublicKey, Transaction, TransactionOutpoint, UtxoEntry}, + tx::{MutableTransaction, Transaction, TransactionOutpoint, UtxoEntry}, BlockHashSet, BlueWorkType, ChainPath, HashMapCustomHasher, }; use kaspa_consensus_notify::root::ConsensusNotificationRoot; @@ -81,7 +81,7 @@ use kaspa_core::{trace, warn}; use kaspa_database::prelude::StoreResultExtensions; use kaspa_hashes::Hash; use kaspa_muhash::MuHash; -use kaspa_txscript::caches::TxScriptCacheCounters; +use kaspa_txscript::{caches::TxScriptCacheCounters, extract_script_pub_key_address}; use std::{ cmp::Reverse, @@ -688,13 +688,20 @@ impl ConsensusApi for Consensus { sample_headers } - fn get_utxo_return_script_public_key(&self, txid: Hash, target_daa_score: u64) -> Option { + fn get_utxo_return_address(&self, txid: Hash, target_daa_score: u64) -> ReturnAddress { // We need consistency between the past pruning points, selected chain and header store reads let _guard = self.pruning_lock.blocking_read(); let sc_read = self.selected_chain_store.read(); let pp_hash = self.pruning_point_store.read().get().unwrap().pruning_point; + + // Pruning Point hash is always expected to be in get_compact_header_data so unwrap should never fail + if target_daa_score < self.headers_store.get_compact_header_data(pp_hash).unwrap().daa_score { + // Early exit if target daa score is lower than that of pruning point's daa score: + return ReturnAddress::AlreadyPruned; + } + let pp_index = sc_read.get_by_hash(pp_hash).unwrap(); let (tip_index, tip_hash) = sc_read.get_tip().unwrap(); let tip_daa_score = self.headers_store.get_compact_header_data(tip_hash).unwrap().daa_score; @@ -708,29 +715,28 @@ impl ConsensusApi for Consensus { let mid = low_index + (high_index - low_index) / 2; // 1. Get the chain block hash at that index. Error if we don't find a hash at an index - let hash = sc_read - .get_by_index(mid) - .map_err(|err| { + let hash = match sc_read.get_by_index(mid) { + Ok(hash) => hash, + Err(_) => { trace!("Did not find a hash at index {}", mid); - err - }) - .ok()?; + return ReturnAddress::NotFound(format!("Did not find a hash at index {}", mid)); + } + }; // 2. Get the compact header so we have access to the daa_score. Error if we - let compact_header = self - .headers_store - .get_compact_header_data(hash) - .map_err(|err| { + let compact_header = match self.headers_store.get_compact_header_data(hash) { + Ok(compact_header) => compact_header, + Err(_) => { trace!("Did not find a compact header with hash {}", hash); - err - }) - .ok()?; + return ReturnAddress::NotFound(format!("Did not find a compact header with hash {}", hash)); + } + }; // 3. Compare block daa score to our target match compact_header.daa_score.cmp(&target_daa_score) { cmp::Ordering::Equal => { // We found the chain block we need - break Some(hash); + break hash; } cmp::Ordering::Greater => { high_index = mid - 1; @@ -741,30 +747,44 @@ impl ConsensusApi for Consensus { } if low_index > high_index { - break None; + return ReturnAddress::NoTxAtScore; } - }?; + }; - let acceptance_data = self.acceptance_data_store.get(matching_chain_block_hash).ok()?; - let (index, containing_acceptance) = acceptance_data.iter().find_map(|mbad| { + let acceptance_data = match self.acceptance_data_store.get(matching_chain_block_hash) { + Ok(acceptance_data) => acceptance_data, + Err(_) => { + return ReturnAddress::NotFound("Did not find acceptance data".to_string()); + } + }; + let (index, containing_acceptance) = match acceptance_data.iter().find_map(|mbad| { let tx_arr_index = mbad.accepted_transactions.iter().enumerate().find_map(|(index, tx)| (tx.transaction_id == txid).then_some(index)); tx_arr_index.map(|index| (index, mbad.clone())) - })?; + }) { + Some((index, containing_acceptance)) => (index, containing_acceptance), + None => { + return ReturnAddress::NotFound("Did not find containing_acceptance".to_string()); + } + }; // Found Merged block containing the TXID let tx = &self.block_transactions_store.get(containing_acceptance.block_hash).unwrap()[index]; if tx.id() != txid { // Should never happen, but do a sanity check. This would mean something went wrong with storing block transactions - // Sanity check is necessary to guarantee that this function will never give back a wrong address (err on the side of None) + // Sanity check is necessary to guarantee that this function will never give back a wrong address (err on the side of NotFound) warn!("Expected {} to match {} when checking block_transaction_store using array index of transaction", tx.id(), txid); - return None; + return ReturnAddress::NotFound(format!( + "Expected {} to match {} when checking block_transaction_store using array index of transaction", + tx.id(), + txid + )); } if tx.inputs.is_empty() { // A transaction may have no inputs (like a coinbase transaction) - return None; + return ReturnAddress::TxFromCoinbase; } let first_input_prev_outpoint = &tx.inputs[0].previous_outpoint; @@ -772,7 +792,14 @@ impl ConsensusApi for Consensus { let utxo_diff = self.utxo_diffs_store.get(matching_chain_block_hash).unwrap(); let removed_diffs = utxo_diff.removed(); - Some(removed_diffs.get(first_input_prev_outpoint)?.script_public_key.clone()) + if let Ok(address) = extract_script_pub_key_address( + &removed_diffs.get(first_input_prev_outpoint).unwrap().script_public_key, + self.config.prefix(), + ) { + ReturnAddress::Found(address) + } else { + ReturnAddress::NonStandard + } } fn get_virtual_parents(&self) -> BlockHashSet { diff --git a/rpc/core/src/error.rs b/rpc/core/src/error.rs index 235ea639e..5e94dafee 100644 --- a/rpc/core/src/error.rs +++ b/rpc/core/src/error.rs @@ -1,4 +1,4 @@ -use kaspa_consensus_core::{subnets::SubnetworkConversionError, tx::TransactionId}; +use kaspa_consensus_core::{api::ReturnAddress, subnets::SubnetworkConversionError, tx::TransactionId}; use kaspa_utils::networking::IpAddress; use std::{net::AddrParseError, num::TryFromIntError}; use thiserror::Error; @@ -130,6 +130,9 @@ pub enum RpcError { #[error(transparent)] ConsensusClient(#[from] kaspa_consensus_client::error::Error), + + #[error("utxo return address could not be found -> {0}")] + UtxoReturnAddressNotFound(ReturnAddress), } impl From for RpcError { diff --git a/rpc/service/src/service.rs b/rpc/service/src/service.rs index 9780951d6..f538d0663 100644 --- a/rpc/service/src/service.rs +++ b/rpc/service/src/service.rs @@ -6,6 +6,7 @@ use crate::converter::{consensus::ConsensusConverter, index::IndexConverter, pro use crate::service::NetworkType::{Mainnet, Testnet}; use async_trait::async_trait; use kaspa_consensus_core::api::counters::ProcessingCounters; +use kaspa_consensus_core::api::ReturnAddress; use kaspa_consensus_core::errors::block::RuleError; use kaspa_consensus_core::{ block::Block, @@ -789,13 +790,11 @@ NOTE: This error usually indicates an RPC conversion error between the node and let session = self.consensus_manager.consensus().session().await; // Convert a SPK to an Address - let return_address = if let Some(spk) = - session.async_get_utxo_return_script_public_key(request.txid, request.accepting_block_daa_score).await - { - extract_script_pub_key_address(&spk, self.config.prefix()).ok() - } else { - None - }; + let return_address = + match session.async_get_utxo_return_script_public_key(request.txid, request.accepting_block_daa_score).await { + ReturnAddress::Found(address) => Some(address), + other => return Err(RpcError::UtxoReturnAddressNotFound(other)), + }; Ok(GetUtxoReturnAddressResponse { return_address }) } From c502278a88b1b70b4835ecb4821aaa619a7e19a7 Mon Sep 17 00:00:00 2001 From: coderofstuff <114628839+coderofstuff@users.noreply.github.com> Date: Fri, 6 Sep 2024 18:10:14 -0600 Subject: [PATCH 07/14] Use source as low index instead of pp --- consensus/src/consensus/mod.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/consensus/src/consensus/mod.rs b/consensus/src/consensus/mod.rs index 35ff173b2..0ee2a444a 100644 --- a/consensus/src/consensus/mod.rs +++ b/consensus/src/consensus/mod.rs @@ -694,19 +694,19 @@ impl ConsensusApi for Consensus { let sc_read = self.selected_chain_store.read(); - let pp_hash = self.pruning_point_store.read().get().unwrap().pruning_point; + let source_hash = self.get_source(); // Pruning Point hash is always expected to be in get_compact_header_data so unwrap should never fail - if target_daa_score < self.headers_store.get_compact_header_data(pp_hash).unwrap().daa_score { + if target_daa_score < self.headers_store.get_compact_header_data(source_hash).unwrap().daa_score { // Early exit if target daa score is lower than that of pruning point's daa score: return ReturnAddress::AlreadyPruned; } - let pp_index = sc_read.get_by_hash(pp_hash).unwrap(); + let source_index = sc_read.get_by_hash(source_hash).unwrap(); let (tip_index, tip_hash) = sc_read.get_tip().unwrap(); let tip_daa_score = self.headers_store.get_compact_header_data(tip_hash).unwrap().daa_score; - let mut low_index = tip_index.saturating_sub(tip_daa_score.saturating_sub(target_daa_score)).max(pp_index); + let mut low_index = tip_index.saturating_sub(tip_daa_score.saturating_sub(target_daa_score)).max(source_index); let mut high_index = tip_index; let matching_chain_block_hash = loop { From 9b99059fd4e39c9f270b881446c115e7fada8f71 Mon Sep 17 00:00:00 2001 From: coderofstuff <114628839+coderofstuff@users.noreply.github.com> Date: Fri, 6 Sep 2024 19:33:15 -0600 Subject: [PATCH 08/14] Use correct index of tx in block --- consensus/src/consensus/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/consensus/src/consensus/mod.rs b/consensus/src/consensus/mod.rs index 0ee2a444a..e1134c72a 100644 --- a/consensus/src/consensus/mod.rs +++ b/consensus/src/consensus/mod.rs @@ -759,7 +759,7 @@ impl ConsensusApi for Consensus { }; let (index, containing_acceptance) = match acceptance_data.iter().find_map(|mbad| { let tx_arr_index = - mbad.accepted_transactions.iter().enumerate().find_map(|(index, tx)| (tx.transaction_id == txid).then_some(index)); + mbad.accepted_transactions.iter().find_map(|tx| (tx.transaction_id == txid).then_some(tx.index_within_block as usize)); tx_arr_index.map(|index| (index, mbad.clone())) }) { Some((index, containing_acceptance)) => (index, containing_acceptance), From 656b1630cb7424ddfa8cdb512ae6ef2f19ffd9cd Mon Sep 17 00:00:00 2001 From: coderofstuff <114628839+coderofstuff@users.noreply.github.com> Date: Thu, 3 Oct 2024 23:17:10 -0600 Subject: [PATCH 09/14] Handle rare case of create-and-immediately-spent UTXO --- consensus/src/consensus/mod.rs | 30 ++++++++++++++++++++++++++---- 1 file changed, 26 insertions(+), 4 deletions(-) diff --git a/consensus/src/consensus/mod.rs b/consensus/src/consensus/mod.rs index 608c137da..651832651 100644 --- a/consensus/src/consensus/mod.rs +++ b/consensus/src/consensus/mod.rs @@ -804,10 +804,32 @@ impl ConsensusApi for Consensus { let utxo_diff = self.utxo_diffs_store.get(matching_chain_block_hash).unwrap(); let removed_diffs = utxo_diff.removed(); - if let Ok(address) = extract_script_pub_key_address( - &removed_diffs.get(first_input_prev_outpoint).unwrap().script_public_key, - self.config.prefix(), - ) { + let spk = if let Some(utxo_entry) = removed_diffs.get(first_input_prev_outpoint) { + utxo_entry.script_public_key.clone() + } else { + // This handles this rare scenario: + // - UTXO0 is spent by TX1 and creates UTXO1 + // - UTXO1 is spent by TX2 and creates UTXO2 + // - A chain block happens to accept both of these + // In this case, removed_diff wouldn't contain the outpoint of the created-and-immediately-spent UTXO + // so we use the transaction (which also has acceptance data in this block) and look at its outputs + let other_txid = first_input_prev_outpoint.transaction_id; + let (other_index, other_containing_acceptance) = acceptance_data + .iter() + .find_map(|ombad| { + let otx_arr_index = ombad + .accepted_transactions + .iter() + .find_map(|otx| (otx.transaction_id == other_txid).then_some(otx.index_within_block as usize)); + otx_arr_index.map(|index| (index, ombad.clone())) + }) + .expect("The other transaction's acceptance data must also be in the same block in this case"); + let other_tx = &self.block_transactions_store.get(other_containing_acceptance.block_hash).unwrap()[other_index]; + + other_tx.outputs[first_input_prev_outpoint.index as usize].script_public_key.clone() + }; + + if let Ok(address) = extract_script_pub_key_address(&spk, self.config.prefix()) { ReturnAddress::Found(address) } else { ReturnAddress::NonStandard From b6b6ec16c932f9abb734e5e8cfa015698213c9b1 Mon Sep 17 00:00:00 2001 From: coderofstuff <114628839+coderofstuff@users.noreply.github.com> Date: Fri, 11 Oct 2024 21:42:40 -0600 Subject: [PATCH 10/14] Move utxo_return_address code to virtual processor --- consensus/src/consensus/mod.rs | 140 +---------------- .../pipeline/virtual_processor/processor.rs | 148 +++++++++++++++++- 2 files changed, 145 insertions(+), 143 deletions(-) diff --git a/consensus/src/consensus/mod.rs b/consensus/src/consensus/mod.rs index 651832651..fc44a6458 100644 --- a/consensus/src/consensus/mod.rs +++ b/consensus/src/consensus/mod.rs @@ -77,11 +77,10 @@ use crossbeam_channel::{ use itertools::Itertools; use kaspa_consensusmanager::{SessionLock, SessionReadGuard}; -use kaspa_core::{trace, warn}; use kaspa_database::prelude::StoreResultExtensions; use kaspa_hashes::Hash; use kaspa_muhash::MuHash; -use kaspa_txscript::{caches::TxScriptCacheCounters, extract_script_pub_key_address}; +use kaspa_txscript::caches::TxScriptCacheCounters; use std::{ cmp::Reverse, @@ -103,9 +102,6 @@ use crate::model::stores::selected_chain::SelectedChainStoreReader; use std::cmp; -use crate::model::stores::utxo_diffs::UtxoDiffsStoreReader; -use kaspa_consensus_core::utxo::utxo_diff::ImmutableUtxoDiff; - pub struct Consensus { // DB db: Arc, @@ -701,139 +697,7 @@ impl ConsensusApi for Consensus { } fn get_utxo_return_address(&self, txid: Hash, target_daa_score: u64) -> ReturnAddress { - // We need consistency between the past pruning points, selected chain and header store reads - let _guard = self.pruning_lock.blocking_read(); - - let sc_read = self.selected_chain_store.read(); - - let source_hash = self.get_source(); - - // Pruning Point hash is always expected to be in get_compact_header_data so unwrap should never fail - if target_daa_score < self.headers_store.get_compact_header_data(source_hash).unwrap().daa_score { - // Early exit if target daa score is lower than that of pruning point's daa score: - return ReturnAddress::AlreadyPruned; - } - - let source_index = sc_read.get_by_hash(source_hash).unwrap(); - let (tip_index, tip_hash) = sc_read.get_tip().unwrap(); - let tip_daa_score = self.headers_store.get_compact_header_data(tip_hash).unwrap().daa_score; - - let mut low_index = tip_index.saturating_sub(tip_daa_score.saturating_sub(target_daa_score)).max(source_index); - let mut high_index = tip_index; - - let matching_chain_block_hash = loop { - // Binary search for the chain block that matches the target_daa_score - // 0. Get the mid point index - let mid = low_index + (high_index - low_index) / 2; - - // 1. Get the chain block hash at that index. Error if we don't find a hash at an index - let hash = match sc_read.get_by_index(mid) { - Ok(hash) => hash, - Err(_) => { - trace!("Did not find a hash at index {}", mid); - return ReturnAddress::NotFound(format!("Did not find a hash at index {}", mid)); - } - }; - - // 2. Get the compact header so we have access to the daa_score. Error if we - let compact_header = match self.headers_store.get_compact_header_data(hash) { - Ok(compact_header) => compact_header, - Err(_) => { - trace!("Did not find a compact header with hash {}", hash); - return ReturnAddress::NotFound(format!("Did not find a compact header with hash {}", hash)); - } - }; - - // 3. Compare block daa score to our target - match compact_header.daa_score.cmp(&target_daa_score) { - cmp::Ordering::Equal => { - // We found the chain block we need - break hash; - } - cmp::Ordering::Greater => { - high_index = mid - 1; - } - cmp::Ordering::Less => { - low_index = mid + 1; - } - } - - if low_index > high_index { - return ReturnAddress::NoTxAtScore; - } - }; - - let acceptance_data = match self.acceptance_data_store.get(matching_chain_block_hash) { - Ok(acceptance_data) => acceptance_data, - Err(_) => { - return ReturnAddress::NotFound("Did not find acceptance data".to_string()); - } - }; - let (index, containing_acceptance) = match acceptance_data.iter().find_map(|mbad| { - let tx_arr_index = - mbad.accepted_transactions.iter().find_map(|tx| (tx.transaction_id == txid).then_some(tx.index_within_block as usize)); - tx_arr_index.map(|index| (index, mbad.clone())) - }) { - Some((index, containing_acceptance)) => (index, containing_acceptance), - None => { - return ReturnAddress::NotFound("Did not find containing_acceptance".to_string()); - } - }; - - // Found Merged block containing the TXID - let tx = &self.block_transactions_store.get(containing_acceptance.block_hash).unwrap()[index]; - - if tx.id() != txid { - // Should never happen, but do a sanity check. This would mean something went wrong with storing block transactions - // Sanity check is necessary to guarantee that this function will never give back a wrong address (err on the side of NotFound) - warn!("Expected {} to match {} when checking block_transaction_store using array index of transaction", tx.id(), txid); - return ReturnAddress::NotFound(format!( - "Expected {} to match {} when checking block_transaction_store using array index of transaction", - tx.id(), - txid - )); - } - - if tx.inputs.is_empty() { - // A transaction may have no inputs (like a coinbase transaction) - return ReturnAddress::TxFromCoinbase; - } - - let first_input_prev_outpoint = &tx.inputs[0].previous_outpoint; - // Expected to never fail, since we found the acceptance data and therefore there must be matching diff - let utxo_diff = self.utxo_diffs_store.get(matching_chain_block_hash).unwrap(); - let removed_diffs = utxo_diff.removed(); - - let spk = if let Some(utxo_entry) = removed_diffs.get(first_input_prev_outpoint) { - utxo_entry.script_public_key.clone() - } else { - // This handles this rare scenario: - // - UTXO0 is spent by TX1 and creates UTXO1 - // - UTXO1 is spent by TX2 and creates UTXO2 - // - A chain block happens to accept both of these - // In this case, removed_diff wouldn't contain the outpoint of the created-and-immediately-spent UTXO - // so we use the transaction (which also has acceptance data in this block) and look at its outputs - let other_txid = first_input_prev_outpoint.transaction_id; - let (other_index, other_containing_acceptance) = acceptance_data - .iter() - .find_map(|ombad| { - let otx_arr_index = ombad - .accepted_transactions - .iter() - .find_map(|otx| (otx.transaction_id == other_txid).then_some(otx.index_within_block as usize)); - otx_arr_index.map(|index| (index, ombad.clone())) - }) - .expect("The other transaction's acceptance data must also be in the same block in this case"); - let other_tx = &self.block_transactions_store.get(other_containing_acceptance.block_hash).unwrap()[other_index]; - - other_tx.outputs[first_input_prev_outpoint.index as usize].script_public_key.clone() - }; - - if let Ok(address) = extract_script_pub_key_address(&spk, self.config.prefix()) { - ReturnAddress::Found(address) - } else { - ReturnAddress::NonStandard - } + self.virtual_processor.get_utxo_return_address(txid, target_daa_score, self.get_source(), &self.config) } fn get_virtual_parents(&self) -> BlockHashSet { diff --git a/consensus/src/pipeline/virtual_processor/processor.rs b/consensus/src/pipeline/virtual_processor/processor.rs index 88fee97bf..078dd7eab 100644 --- a/consensus/src/pipeline/virtual_processor/processor.rs +++ b/consensus/src/pipeline/virtual_processor/processor.rs @@ -25,7 +25,7 @@ use crate::{ pruning_utxoset::PruningUtxosetStores, reachability::DbReachabilityStore, relations::{DbRelationsStore, RelationsStoreReader}, - selected_chain::{DbSelectedChainStore, SelectedChainStore}, + selected_chain::{DbSelectedChainStore, SelectedChainStore, SelectedChainStoreReader}, statuses::{DbStatusesStore, StatusesStore, StatusesStoreBatchExtensions, StatusesStoreReader}, tips::{DbTipsStore, TipsStoreReader}, utxo_diffs::{DbUtxoDiffsStore, UtxoDiffsStoreReader}, @@ -48,17 +48,20 @@ use crate::{ }; use kaspa_consensus_core::{ acceptance_data::AcceptanceData, - api::args::{TransactionValidationArgs, TransactionValidationBatchArgs}, + api::{ + args::{TransactionValidationArgs, TransactionValidationBatchArgs}, + ReturnAddress, + }, block::{BlockTemplate, MutableBlock, TemplateBuildMode, TemplateTransactionSelector}, blockstatus::BlockStatus::{StatusDisqualifiedFromChain, StatusUTXOValid}, coinbase::MinerData, - config::genesis::GenesisBlock, + config::{genesis::GenesisBlock, Config}, header::Header, merkle::calc_hash_merkle_root, pruning::PruningPointsList, tx::{MutableTransaction, Transaction}, utxo::{ - utxo_diff::UtxoDiff, + utxo_diff::{ImmutableUtxoDiff, UtxoDiff}, utxo_view::{UtxoView, UtxoViewComposition}, }, BlockHashSet, ChainPath, @@ -76,6 +79,7 @@ use kaspa_database::prelude::{StoreError, StoreResultEmptyTuple, StoreResultExte use kaspa_hashes::Hash; use kaspa_muhash::MuHash; use kaspa_notify::{events::EventType, notifier::Notify}; +use kaspa_txscript::extract_script_pub_key_address; use super::errors::{PruningImportError, PruningImportResult}; use crossbeam_channel::{Receiver as CrossbeamReceiver, Sender as CrossbeamSender}; @@ -90,7 +94,7 @@ use rayon::{ }; use rocksdb::WriteBatch; use std::{ - cmp::min, + cmp::{self, min}, collections::{BinaryHeap, HashMap, VecDeque}, ops::Deref, sync::{atomic::Ordering, Arc}, @@ -1132,6 +1136,140 @@ impl VirtualStateProcessor { Ok(()) } + pub fn get_utxo_return_address(&self, txid: Hash, target_daa_score: u64, source_hash: Hash, config: &Config) -> ReturnAddress { + // We need consistency between the past pruning points, selected chain and header store reads + let _guard = self.pruning_lock.blocking_read(); + + let sc_read = self.selected_chain_store.read(); + + // Pruning Point hash is always expected to be in get_compact_header_data so unwrap should never fail + if target_daa_score < self.headers_store.get_compact_header_data(source_hash).unwrap().daa_score { + // Early exit if target daa score is lower than that of pruning point's daa score: + return ReturnAddress::AlreadyPruned; + } + + let source_index = sc_read.get_by_hash(source_hash).unwrap(); + let (tip_index, tip_hash) = sc_read.get_tip().unwrap(); + let tip_daa_score = self.headers_store.get_compact_header_data(tip_hash).unwrap().daa_score; + + let mut low_index = tip_index.saturating_sub(tip_daa_score.saturating_sub(target_daa_score)).max(source_index); + let mut high_index = tip_index; + + let matching_chain_block_hash = loop { + // Binary search for the chain block that matches the target_daa_score + // 0. Get the mid point index + let mid = low_index + (high_index - low_index) / 2; + + // 1. Get the chain block hash at that index. Error if we don't find a hash at an index + let hash = match sc_read.get_by_index(mid) { + Ok(hash) => hash, + Err(_) => { + trace!("Did not find a hash at index {}", mid); + return ReturnAddress::NotFound(format!("Did not find a hash at index {}", mid)); + } + }; + + // 2. Get the compact header so we have access to the daa_score. Error if we + let compact_header = match self.headers_store.get_compact_header_data(hash) { + Ok(compact_header) => compact_header, + Err(_) => { + trace!("Did not find a compact header with hash {}", hash); + return ReturnAddress::NotFound(format!("Did not find a compact header with hash {}", hash)); + } + }; + + // 3. Compare block daa score to our target + match compact_header.daa_score.cmp(&target_daa_score) { + cmp::Ordering::Equal => { + // We found the chain block we need + break hash; + } + cmp::Ordering::Greater => { + high_index = mid - 1; + } + cmp::Ordering::Less => { + low_index = mid + 1; + } + } + + if low_index > high_index { + return ReturnAddress::NoTxAtScore; + } + }; + + let acceptance_data = match self.acceptance_data_store.get(matching_chain_block_hash) { + Ok(acceptance_data) => acceptance_data, + Err(_) => { + return ReturnAddress::NotFound("Did not find acceptance data".to_string()); + } + }; + let (index, containing_acceptance) = match acceptance_data.iter().find_map(|mbad| { + let tx_arr_index = + mbad.accepted_transactions.iter().find_map(|tx| (tx.transaction_id == txid).then_some(tx.index_within_block as usize)); + tx_arr_index.map(|index| (index, mbad.clone())) + }) { + Some((index, containing_acceptance)) => (index, containing_acceptance), + None => { + return ReturnAddress::NotFound("Did not find containing_acceptance".to_string()); + } + }; + + // Found Merged block containing the TXID + let tx = &self.block_transactions_store.get(containing_acceptance.block_hash).unwrap()[index]; + + if tx.id() != txid { + // Should never happen, but do a sanity check. This would mean something went wrong with storing block transactions + // Sanity check is necessary to guarantee that this function will never give back a wrong address (err on the side of NotFound) + warn!("Expected {} to match {} when checking block_transaction_store using array index of transaction", tx.id(), txid); + return ReturnAddress::NotFound(format!( + "Expected {} to match {} when checking block_transaction_store using array index of transaction", + tx.id(), + txid + )); + } + + if tx.inputs.is_empty() { + // A transaction may have no inputs (like a coinbase transaction) + return ReturnAddress::TxFromCoinbase; + } + + let first_input_prev_outpoint = &tx.inputs[0].previous_outpoint; + // Expected to never fail, since we found the acceptance data and therefore there must be matching diff + let utxo_diff = self.utxo_diffs_store.get(matching_chain_block_hash).unwrap(); + let removed_diffs = utxo_diff.removed(); + + let spk = if let Some(utxo_entry) = removed_diffs.get(first_input_prev_outpoint) { + utxo_entry.script_public_key.clone() + } else { + // This handles this rare scenario: + // - UTXO0 is spent by TX1 and creates UTXO1 + // - UTXO1 is spent by TX2 and creates UTXO2 + // - A chain block happens to accept both of these + // In this case, removed_diff wouldn't contain the outpoint of the created-and-immediately-spent UTXO + // so we use the transaction (which also has acceptance data in this block) and look at its outputs + let other_txid = first_input_prev_outpoint.transaction_id; + let (other_index, other_containing_acceptance) = acceptance_data + .iter() + .find_map(|ombad| { + let otx_arr_index = ombad + .accepted_transactions + .iter() + .find_map(|otx| (otx.transaction_id == other_txid).then_some(otx.index_within_block as usize)); + otx_arr_index.map(|index| (index, ombad.clone())) + }) + .expect("The other transaction's acceptance data must also be in the same block in this case"); + let other_tx = &self.block_transactions_store.get(other_containing_acceptance.block_hash).unwrap()[other_index]; + + other_tx.outputs[first_input_prev_outpoint.index as usize].script_public_key.clone() + }; + + if let Ok(address) = extract_script_pub_key_address(&spk, config.prefix()) { + ReturnAddress::Found(address) + } else { + ReturnAddress::NonStandard + } + } + pub fn are_pruning_points_violating_finality(&self, pp_list: PruningPointsList) -> bool { // Ideally we would want to check if the last known pruning point has the finality point // in its chain, but in some cases it's impossible: let `lkp` be the last known pruning From 95c8405b34dd57f8b62161d7b7acc393c7494800 Mon Sep 17 00:00:00 2001 From: coderofstuff <114628839+coderofstuff@users.noreply.github.com> Date: Sat, 12 Oct 2024 01:08:50 -0600 Subject: [PATCH 11/14] Refactor ReturnAddress location --- components/consensusmanager/src/session.rs | 3 +- consensus/core/src/api/mod.rs | 32 ++----------------- consensus/core/src/lib.rs | 1 + consensus/core/src/return_address.rs | 28 ++++++++++++++++ consensus/src/consensus/mod.rs | 3 +- .../pipeline/virtual_processor/processor.rs | 6 ++-- rpc/core/src/error.rs | 2 +- rpc/service/src/service.rs | 2 +- 8 files changed, 39 insertions(+), 38 deletions(-) create mode 100644 consensus/core/src/return_address.rs diff --git a/components/consensusmanager/src/session.rs b/components/consensusmanager/src/session.rs index 95df3ab20..d144abd70 100644 --- a/components/consensusmanager/src/session.rs +++ b/components/consensusmanager/src/session.rs @@ -4,13 +4,14 @@ use kaspa_consensus_core::{ acceptance_data::AcceptanceData, - api::{BlockCount, BlockValidationFutures, ConsensusApi, ConsensusStats, DynConsensus, ReturnAddress}, + api::{BlockCount, BlockValidationFutures, ConsensusApi, ConsensusStats, DynConsensus}, block::Block, blockstatus::BlockStatus, daa_score_timestamp::DaaScoreTimestamp, errors::consensus::ConsensusResult, header::Header, pruning::{PruningPointProof, PruningPointTrustedData, PruningPointsList}, + return_address::ReturnAddress, trusted::{ExternalGhostdagData, TrustedBlock}, tx::{MutableTransaction, Transaction, TransactionOutpoint, UtxoEntry}, BlockHashSet, BlueWorkType, ChainPath, Hash, diff --git a/consensus/core/src/api/mod.rs b/consensus/core/src/api/mod.rs index 95495fb6a..011ce0fab 100644 --- a/consensus/core/src/api/mod.rs +++ b/consensus/core/src/api/mod.rs @@ -1,10 +1,6 @@ use futures_util::future::BoxFuture; -use kaspa_addresses::Address; use kaspa_muhash::MuHash; -use std::{ - fmt::{Display, Formatter}, - sync::Arc, -}; +use std::sync::Arc; use crate::{ acceptance_data::AcceptanceData, @@ -22,6 +18,7 @@ use crate::{ }, header::Header, pruning::{PruningPointProof, PruningPointTrustedData, PruningPointsList}, + return_address::ReturnAddress, trusted::{ExternalGhostdagData, TrustedBlock}, tx::{MutableTransaction, Transaction, TransactionOutpoint, UtxoEntry}, BlockHashSet, BlueWorkType, ChainPath, @@ -47,31 +44,6 @@ pub struct BlockValidationFutures { pub virtual_state_task: BlockValidationFuture, } -#[derive(Debug, Clone)] -pub enum ReturnAddress { - Found(Address), - AlreadyPruned, - TxFromCoinbase, - NoTxAtScore, - NonStandard, - NotFound(String), -} - -impl Display for ReturnAddress { - #[inline] - fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { - let s = match self { - ReturnAddress::AlreadyPruned => "Transaction is already pruned".to_string(), - ReturnAddress::NoTxAtScore => "Transaction not found at given accepting daa score".to_string(), - ReturnAddress::NonStandard => "Transaction was found but not standard".to_string(), - ReturnAddress::TxFromCoinbase => "Transaction return address is coinbase".to_string(), - ReturnAddress::NotFound(reason) => format!("Transaction return address not found: {}", reason), - ReturnAddress::Found(address) => address.to_string(), - }; - f.write_str(&s) - } -} - /// Abstracts the consensus external API #[allow(unused_variables)] pub trait ConsensusApi: Send + Sync { diff --git a/consensus/core/src/lib.rs b/consensus/core/src/lib.rs index 188b2403b..0e61157ed 100644 --- a/consensus/core/src/lib.rs +++ b/consensus/core/src/lib.rs @@ -30,6 +30,7 @@ pub mod merkle; pub mod muhash; pub mod network; pub mod pruning; +pub mod return_address; pub mod sign; pub mod subnets; pub mod trusted; diff --git a/consensus/core/src/return_address.rs b/consensus/core/src/return_address.rs new file mode 100644 index 000000000..6472f6ac8 --- /dev/null +++ b/consensus/core/src/return_address.rs @@ -0,0 +1,28 @@ +use std::fmt::{Display, Formatter}; + +use kaspa_addresses::Address; + +#[derive(Debug, Clone)] +pub enum ReturnAddress { + Found(Address), + AlreadyPruned, + TxFromCoinbase, + NoTxAtScore, + NonStandard, + NotFound(String), +} + +impl Display for ReturnAddress { + #[inline] + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + let s = match self { + ReturnAddress::AlreadyPruned => "Transaction is already pruned".to_string(), + ReturnAddress::NoTxAtScore => "Transaction not found at given accepting daa score".to_string(), + ReturnAddress::NonStandard => "Transaction was found but not standard".to_string(), + ReturnAddress::TxFromCoinbase => "Transaction return address is coinbase".to_string(), + ReturnAddress::NotFound(reason) => format!("Transaction return address not found: {}", reason), + ReturnAddress::Found(address) => address.to_string(), + }; + f.write_str(&s) + } +} diff --git a/consensus/src/consensus/mod.rs b/consensus/src/consensus/mod.rs index fc44a6458..aaa6b8df3 100644 --- a/consensus/src/consensus/mod.rs +++ b/consensus/src/consensus/mod.rs @@ -46,7 +46,7 @@ use kaspa_consensus_core::{ api::{ args::{TransactionValidationArgs, TransactionValidationBatchArgs}, stats::BlockCount, - BlockValidationFutures, ConsensusApi, ConsensusStats, ReturnAddress, + BlockValidationFutures, ConsensusApi, ConsensusStats, }, block::{Block, BlockTemplate, TemplateBuildMode, TemplateTransactionSelector, VirtualStateApproxId}, blockhash::BlockHashExtensions, @@ -65,6 +65,7 @@ use kaspa_consensus_core::{ muhash::MuHashExtensions, network::NetworkType, pruning::{PruningPointProof, PruningPointTrustedData, PruningPointsList}, + return_address::ReturnAddress, trusted::{ExternalGhostdagData, TrustedBlock}, tx::{MutableTransaction, Transaction, TransactionOutpoint, UtxoEntry}, BlockHashSet, BlueWorkType, ChainPath, HashMapCustomHasher, diff --git a/consensus/src/pipeline/virtual_processor/processor.rs b/consensus/src/pipeline/virtual_processor/processor.rs index 078dd7eab..66ebb58a4 100644 --- a/consensus/src/pipeline/virtual_processor/processor.rs +++ b/consensus/src/pipeline/virtual_processor/processor.rs @@ -48,10 +48,7 @@ use crate::{ }; use kaspa_consensus_core::{ acceptance_data::AcceptanceData, - api::{ - args::{TransactionValidationArgs, TransactionValidationBatchArgs}, - ReturnAddress, - }, + api::args::{TransactionValidationArgs, TransactionValidationBatchArgs}, block::{BlockTemplate, MutableBlock, TemplateBuildMode, TemplateTransactionSelector}, blockstatus::BlockStatus::{StatusDisqualifiedFromChain, StatusUTXOValid}, coinbase::MinerData, @@ -59,6 +56,7 @@ use kaspa_consensus_core::{ header::Header, merkle::calc_hash_merkle_root, pruning::PruningPointsList, + return_address::ReturnAddress, tx::{MutableTransaction, Transaction}, utxo::{ utxo_diff::{ImmutableUtxoDiff, UtxoDiff}, diff --git a/rpc/core/src/error.rs b/rpc/core/src/error.rs index 73177fa51..38c8ab3cf 100644 --- a/rpc/core/src/error.rs +++ b/rpc/core/src/error.rs @@ -2,7 +2,7 @@ //! [`RpcError`] enum used by RPC primitives. //! -use kaspa_consensus_core::{api::ReturnAddress, subnets::SubnetworkConversionError, tx::TransactionId}; +use kaspa_consensus_core::{return_address::ReturnAddress, subnets::SubnetworkConversionError, tx::TransactionId}; use kaspa_utils::networking::IpAddress; use std::{net::AddrParseError, num::TryFromIntError}; use thiserror::Error; diff --git a/rpc/service/src/service.rs b/rpc/service/src/service.rs index 4d07f2bd4..3abb488ec 100644 --- a/rpc/service/src/service.rs +++ b/rpc/service/src/service.rs @@ -6,8 +6,8 @@ use crate::converter::{consensus::ConsensusConverter, index::IndexConverter, pro use crate::service::NetworkType::{Mainnet, Testnet}; use async_trait::async_trait; use kaspa_consensus_core::api::counters::ProcessingCounters; -use kaspa_consensus_core::api::ReturnAddress; use kaspa_consensus_core::errors::block::RuleError; +use kaspa_consensus_core::return_address::ReturnAddress; use kaspa_consensus_core::{ block::Block, coinbase::MinerData, From c63fc02117a84973b0ddf0cad05905f9d759dd26 Mon Sep 17 00:00:00 2001 From: coderofstuff <114628839+coderofstuff@users.noreply.github.com> Date: Sun, 13 Oct 2024 23:27:21 -0600 Subject: [PATCH 12/14] Refactoring and safety - Refactor to multiple functions - Avoid unwrap as much as possible --- .../pipeline/virtual_processor/processor.rs | 229 ++++++++++++------ 1 file changed, 150 insertions(+), 79 deletions(-) diff --git a/consensus/src/pipeline/virtual_processor/processor.rs b/consensus/src/pipeline/virtual_processor/processor.rs index 66ebb58a4..3ea094367 100644 --- a/consensus/src/pipeline/virtual_processor/processor.rs +++ b/consensus/src/pipeline/virtual_processor/processor.rs @@ -47,7 +47,7 @@ use crate::{ }, }; use kaspa_consensus_core::{ - acceptance_data::AcceptanceData, + acceptance_data::{AcceptanceData, MergesetBlockAcceptanceData}, api::args::{TransactionValidationArgs, TransactionValidationBatchArgs}, block::{BlockTemplate, MutableBlock, TemplateBuildMode, TemplateTransactionSelector}, blockstatus::BlockStatus::{StatusDisqualifiedFromChain, StatusUTXOValid}, @@ -71,7 +71,7 @@ use kaspa_consensus_notify::{ }, root::ConsensusNotificationRoot, }; -use kaspa_consensusmanager::SessionLock; +use kaspa_consensusmanager::{SessionLock, SessionReadGuard}; use kaspa_core::{debug, info, time::unix_now, trace, warn}; use kaspa_database::prelude::{StoreError, StoreResultEmptyTuple, StoreResultExtensions}; use kaspa_hashes::Hash; @@ -1135,86 +1135,55 @@ impl VirtualStateProcessor { } pub fn get_utxo_return_address(&self, txid: Hash, target_daa_score: u64, source_hash: Hash, config: &Config) -> ReturnAddress { - // We need consistency between the past pruning points, selected chain and header store reads - let _guard = self.pruning_lock.blocking_read(); + // We need consistency between the utxo_diffs_store, block_transactions_store, selected chain and header store reads + let guard = self.pruning_lock.blocking_read(); - let sc_read = self.selected_chain_store.read(); - - // Pruning Point hash is always expected to be in get_compact_header_data so unwrap should never fail - if target_daa_score < self.headers_store.get_compact_header_data(source_hash).unwrap().daa_score { + let source_daa_score = if let Ok(compact_header) = self.headers_store.get_compact_header_data(source_hash) { + compact_header.daa_score + } else { + trace!("Did not find compact header for source hash {}", source_hash); + return ReturnAddress::NotFound(format!("Did not find compact header for source hash {}", source_hash)); + }; + if target_daa_score < source_daa_score { // Early exit if target daa score is lower than that of pruning point's daa score: return ReturnAddress::AlreadyPruned; } - let source_index = sc_read.get_by_hash(source_hash).unwrap(); - let (tip_index, tip_hash) = sc_read.get_tip().unwrap(); - let tip_daa_score = self.headers_store.get_compact_header_data(tip_hash).unwrap().daa_score; - - let mut low_index = tip_index.saturating_sub(tip_daa_score.saturating_sub(target_daa_score)).max(source_index); - let mut high_index = tip_index; - - let matching_chain_block_hash = loop { - // Binary search for the chain block that matches the target_daa_score - // 0. Get the mid point index - let mid = low_index + (high_index - low_index) / 2; - - // 1. Get the chain block hash at that index. Error if we don't find a hash at an index - let hash = match sc_read.get_by_index(mid) { + let (matching_chain_block_hash, acceptance_data) = + match self.find_accepting_chain_block_hash_at_daa_score(target_daa_score, source_hash, guard) { Ok(hash) => hash, - Err(_) => { - trace!("Did not find a hash at index {}", mid); - return ReturnAddress::NotFound(format!("Did not find a hash at index {}", mid)); + Err(return_address) => { + return return_address; } }; - // 2. Get the compact header so we have access to the daa_score. Error if we - let compact_header = match self.headers_store.get_compact_header_data(hash) { - Ok(compact_header) => compact_header, - Err(_) => { - trace!("Did not find a compact header with hash {}", hash); - return ReturnAddress::NotFound(format!("Did not find a compact header with hash {}", hash)); + let (index, containing_acceptance) = + match self.find_tx_acceptance_data_and_index_from_block_acceptance(txid, acceptance_data.clone()) { + Some((index, containing_acceptance)) => (index, containing_acceptance), + None => { + return ReturnAddress::NotFound("Did not find containing_acceptance".to_string()); } }; - // 3. Compare block daa score to our target - match compact_header.daa_score.cmp(&target_daa_score) { - cmp::Ordering::Equal => { - // We found the chain block we need - break hash; - } - cmp::Ordering::Greater => { - high_index = mid - 1; - } - cmp::Ordering::Less => { - low_index = mid + 1; - } - } - - if low_index > high_index { - return ReturnAddress::NoTxAtScore; - } - }; - - let acceptance_data = match self.acceptance_data_store.get(matching_chain_block_hash) { - Ok(acceptance_data) => acceptance_data, - Err(_) => { - return ReturnAddress::NotFound("Did not find acceptance data".to_string()); - } - }; - let (index, containing_acceptance) = match acceptance_data.iter().find_map(|mbad| { - let tx_arr_index = - mbad.accepted_transactions.iter().find_map(|tx| (tx.transaction_id == txid).then_some(tx.index_within_block as usize)); - tx_arr_index.map(|index| (index, mbad.clone())) - }) { - Some((index, containing_acceptance)) => (index, containing_acceptance), - None => { - return ReturnAddress::NotFound("Did not find containing_acceptance".to_string()); + // Found Merged block containing the TXID + let tx = if let Ok(block_txs) = self.block_transactions_store.get(containing_acceptance.block_hash) { + if let Some(tx) = block_txs.get(index) { + tx.clone() + } else { + warn!("Did not find tx {} at index {} in chain block tx store of {}", txid, index, containing_acceptance.block_hash); + return ReturnAddress::NotFound(format!( + "Did not find tx {} at index {} in chain block tx store of {}", + txid, index, containing_acceptance.block_hash + )); } + } else { + warn!("Did not find tx {} at index {} in chain block tx store of {}", txid, index, containing_acceptance.block_hash); + return ReturnAddress::NotFound(format!( + "Did not find tx {} at index {} in chain block tx store of {}", + txid, index, containing_acceptance.block_hash + )); }; - // Found Merged block containing the TXID - let tx = &self.block_transactions_store.get(containing_acceptance.block_hash).unwrap()[index]; - if tx.id() != txid { // Should never happen, but do a sanity check. This would mean something went wrong with storing block transactions // Sanity check is necessary to guarantee that this function will never give back a wrong address (err on the side of NotFound) @@ -1233,7 +1202,11 @@ impl VirtualStateProcessor { let first_input_prev_outpoint = &tx.inputs[0].previous_outpoint; // Expected to never fail, since we found the acceptance data and therefore there must be matching diff - let utxo_diff = self.utxo_diffs_store.get(matching_chain_block_hash).unwrap(); + let utxo_diff = if let Ok(res) = self.utxo_diffs_store.get(matching_chain_block_hash) { + res + } else { + return ReturnAddress::NotFound(format!("Did not find a utxo diff for chain block {}", matching_chain_block_hash)); + }; let removed_diffs = utxo_diff.removed(); let spk = if let Some(utxo_entry) = removed_diffs.get(first_input_prev_outpoint) { @@ -1246,17 +1219,37 @@ impl VirtualStateProcessor { // In this case, removed_diff wouldn't contain the outpoint of the created-and-immediately-spent UTXO // so we use the transaction (which also has acceptance data in this block) and look at its outputs let other_txid = first_input_prev_outpoint.transaction_id; - let (other_index, other_containing_acceptance) = acceptance_data - .iter() - .find_map(|ombad| { - let otx_arr_index = ombad - .accepted_transactions - .iter() - .find_map(|otx| (otx.transaction_id == other_txid).then_some(otx.index_within_block as usize)); - otx_arr_index.map(|index| (index, ombad.clone())) - }) - .expect("The other transaction's acceptance data must also be in the same block in this case"); - let other_tx = &self.block_transactions_store.get(other_containing_acceptance.block_hash).unwrap()[other_index]; + let (other_index, other_containing_acceptance) = + if let Some(res) = self.find_tx_acceptance_data_and_index_from_block_acceptance(other_txid, acceptance_data) { + res + } else { + return ReturnAddress::NotFound( + "The other transaction's acceptance data must also be in the same block in this case".to_string(), + ); + }; + let other_tx = if let Ok(block_txs) = self.block_transactions_store.get(other_containing_acceptance.block_hash) { + if let Some(tx) = block_txs.get(other_index) { + tx.clone() + } else { + warn!( + "Did not find tx {} at index {} in chain block tx store of {}", + other_txid, other_index, other_containing_acceptance.block_hash + ); + return ReturnAddress::NotFound(format!( + "Did not find tx {} at index {} in chain block tx store of {}", + other_txid, other_index, other_containing_acceptance.block_hash + )); + } + } else { + warn!( + "Did not find tx {} at index {} in chain block tx store of {}", + other_txid, other_index, other_containing_acceptance.block_hash + ); + return ReturnAddress::NotFound(format!( + "Did not find tx {} at index {} in chain block tx store of {}", + other_txid, other_index, other_containing_acceptance.block_hash + )); + }; other_tx.outputs[first_input_prev_outpoint.index as usize].script_public_key.clone() }; @@ -1268,6 +1261,84 @@ impl VirtualStateProcessor { } } + fn find_accepting_chain_block_hash_at_daa_score( + &self, + target_daa_score: u64, + source_hash: Hash, + _guard: SessionReadGuard, + ) -> Result<(Hash, Arc>), ReturnAddress> { + let sc_read = self.selected_chain_store.read(); + + let source_index = sc_read + .get_by_hash(source_hash) + .map_err(|_| ReturnAddress::NotFound(format!("Did not find index for hash {}", source_hash)))?; + let (tip_index, tip_hash) = sc_read.get_tip().map_err(|_| ReturnAddress::NotFound("Did not find tip data".to_string()))?; + let tip_daa_score = self + .headers_store + .get_compact_header_data(tip_hash) + .map(|tip| tip.daa_score) + .map_err(|_| ReturnAddress::NotFound(format!("Did not find compact header data for tip hash {}", tip_hash)))?; + + let mut low_index = tip_index.saturating_sub(tip_daa_score.saturating_sub(target_daa_score)).max(source_index); + let mut high_index = tip_index; + + let matching_chain_block_hash = loop { + // Binary search for the chain block that matches the target_daa_score + // 0. Get the mid point index + let mid = low_index + (high_index - low_index) / 2; + + // 1. Get the chain block hash at that index. Error if we don't find a hash at an index + let hash = sc_read.get_by_index(mid).map_err(|_| { + trace!("Did not find a hash at index {}", mid); + ReturnAddress::NotFound(format!("Did not find a hash at index {}", mid)) + })?; + + // 2. Get the compact header so we have access to the daa_score. Error if we + let compact_header = self.headers_store.get_compact_header_data(hash).map_err(|_| { + trace!("Did not find a compact header with hash {}", hash); + ReturnAddress::NotFound(format!("Did not find a compact header with hash {}", hash)) + })?; + + // 3. Compare block daa score to our target + match compact_header.daa_score.cmp(&target_daa_score) { + cmp::Ordering::Equal => { + // We found the chain block we need + break hash; + } + cmp::Ordering::Greater => { + high_index = mid - 1; + } + cmp::Ordering::Less => { + low_index = mid + 1; + } + } + + if low_index > high_index { + return Err(ReturnAddress::NoTxAtScore); + } + }; + + let acceptance_data = self.acceptance_data_store.get(matching_chain_block_hash).map_err(|_| { + ReturnAddress::NotFound(format!("Did not find acceptance data for chain block {}", matching_chain_block_hash)) + })?; + + Ok((matching_chain_block_hash, acceptance_data)) + } + + fn find_tx_acceptance_data_and_index_from_block_acceptance( + &self, + tx_id: Hash, + block_acceptance_data: Arc>, + ) -> Option<(usize, MergesetBlockAcceptanceData)> { + block_acceptance_data.iter().find_map(|mbad| { + let tx_arr_index = mbad + .accepted_transactions + .iter() + .find_map(|tx| (tx.transaction_id == tx_id).then_some(tx.index_within_block as usize)); + tx_arr_index.map(|index| (index, mbad.clone())) + }) + } + pub fn are_pruning_points_violating_finality(&self, pp_list: PruningPointsList) -> bool { // Ideally we would want to check if the last known pruning point has the finality point // in its chain, but in some cases it's impossible: let `lkp` be the last known pruning From 8e07ad7124a561bbffd83ce36988a4516cf49964 Mon Sep 17 00:00:00 2001 From: coderofstuff <114628839+coderofstuff@users.noreply.github.com> Date: Mon, 14 Oct 2024 00:55:10 -0600 Subject: [PATCH 13/14] Change ReturnAddress to be an Error; Refactoring --- Cargo.lock | 3 + components/consensusmanager/Cargo.toml | 1 + components/consensusmanager/src/session.rs | 9 +- consensus/Cargo.toml | 1 + consensus/core/src/api/mod.rs | 5 +- consensus/core/src/return_address.rs | 29 +--- consensus/src/consensus/mod.rs | 5 +- .../pipeline/virtual_processor/processor.rs | 156 ++++++++---------- rpc/core/src/api/rpc.rs | 2 +- rpc/core/src/error.rs | 4 +- rpc/core/src/model/message.rs | 8 +- rpc/grpc/core/Cargo.toml | 1 + rpc/grpc/core/src/convert/message.rs | 12 +- rpc/service/src/service.rs | 12 +- 14 files changed, 109 insertions(+), 139 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8e9829fea..c4e559ee1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2505,6 +2505,7 @@ dependencies = [ "futures-util", "indexmap 2.4.0", "itertools 0.13.0", + "kaspa-addresses", "kaspa-consensus-core", "kaspa-consensus-notify", "kaspa-consensusmanager", @@ -2651,6 +2652,7 @@ dependencies = [ "futures", "futures-util", "itertools 0.13.0", + "kaspa-addresses", "kaspa-consensus-core", "kaspa-consensus-notify", "kaspa-core", @@ -2765,6 +2767,7 @@ dependencies = [ "faster-hex", "futures", "h2 0.4.6", + "kaspa-addresses", "kaspa-consensus-core", "kaspa-core", "kaspa-notify", diff --git a/components/consensusmanager/Cargo.toml b/components/consensusmanager/Cargo.toml index 16f690087..c9f364557 100644 --- a/components/consensusmanager/Cargo.toml +++ b/components/consensusmanager/Cargo.toml @@ -14,6 +14,7 @@ duration-string.workspace = true futures-util.workspace = true futures.workspace = true itertools.workspace = true +kaspa-addresses.workspace=true kaspa-consensus-core.workspace = true kaspa-consensus-notify.workspace = true kaspa-core.workspace = true diff --git a/components/consensusmanager/src/session.rs b/components/consensusmanager/src/session.rs index d144abd70..e5a7e1b2b 100644 --- a/components/consensusmanager/src/session.rs +++ b/components/consensusmanager/src/session.rs @@ -2,6 +2,7 @@ //! //! We use newtypes in order to simplify changing the underlying lock in the future +use kaspa_addresses::Address; use kaspa_consensus_core::{ acceptance_data::AcceptanceData, api::{BlockCount, BlockValidationFutures, ConsensusApi, ConsensusStats, DynConsensus}, @@ -11,7 +12,7 @@ use kaspa_consensus_core::{ errors::consensus::ConsensusResult, header::Header, pruning::{PruningPointProof, PruningPointTrustedData, PruningPointsList}, - return_address::ReturnAddress, + return_address::ReturnAddressError, trusted::{ExternalGhostdagData, TrustedBlock}, tx::{MutableTransaction, Transaction, TransactionOutpoint, UtxoEntry}, BlockHashSet, BlueWorkType, ChainPath, Hash, @@ -314,7 +315,11 @@ impl ConsensusSessionOwned { self.clone().spawn_blocking(|c| c.get_chain_block_samples()).await } - pub async fn async_get_utxo_return_script_public_key(&self, txid: Hash, accepting_block_daa_score: u64) -> ReturnAddress { + pub async fn async_get_utxo_return_script_public_key( + &self, + txid: Hash, + accepting_block_daa_score: u64, + ) -> Result { self.clone().spawn_blocking(move |c| c.get_utxo_return_address(txid, accepting_block_daa_score)).await } diff --git a/consensus/Cargo.toml b/consensus/Cargo.toml index 3f4a1b456..20b21af18 100644 --- a/consensus/Cargo.toml +++ b/consensus/Cargo.toml @@ -18,6 +18,7 @@ faster-hex.workspace = true futures-util.workspace = true indexmap.workspace = true itertools.workspace = true +kaspa-addresses.workspace = true kaspa-consensus-core.workspace = true kaspa-consensus-notify.workspace = true kaspa-consensusmanager.workspace = true diff --git a/consensus/core/src/api/mod.rs b/consensus/core/src/api/mod.rs index 011ce0fab..b843962f0 100644 --- a/consensus/core/src/api/mod.rs +++ b/consensus/core/src/api/mod.rs @@ -1,4 +1,5 @@ use futures_util::future::BoxFuture; +use kaspa_addresses::Address; use kaspa_muhash::MuHash; use std::sync::Arc; @@ -18,7 +19,7 @@ use crate::{ }, header::Header, pruning::{PruningPointProof, PruningPointTrustedData, PruningPointsList}, - return_address::ReturnAddress, + return_address::ReturnAddressError, trusted::{ExternalGhostdagData, TrustedBlock}, tx::{MutableTransaction, Transaction, TransactionOutpoint, UtxoEntry}, BlockHashSet, BlueWorkType, ChainPath, @@ -171,7 +172,7 @@ pub trait ConsensusApi: Send + Sync { unimplemented!() } - fn get_utxo_return_address(&self, txid: Hash, daa_score: u64) -> ReturnAddress { + fn get_utxo_return_address(&self, txid: Hash, daa_score: u64) -> Result { unimplemented!() } diff --git a/consensus/core/src/return_address.rs b/consensus/core/src/return_address.rs index 6472f6ac8..760f3fd9d 100644 --- a/consensus/core/src/return_address.rs +++ b/consensus/core/src/return_address.rs @@ -1,28 +1,15 @@ -use std::fmt::{Display, Formatter}; +use thiserror::Error; -use kaspa_addresses::Address; - -#[derive(Debug, Clone)] -pub enum ReturnAddress { - Found(Address), +#[derive(Error, Debug, Clone)] +pub enum ReturnAddressError { + #[error("Transaction is already pruned")] AlreadyPruned, + #[error("Transaction return address is coinbase")] TxFromCoinbase, + #[error("Transaction not found at given accepting daa score")] NoTxAtScore, + #[error("Transaction was found but not standard")] NonStandard, + #[error("Transaction return address not found: {0}")] NotFound(String), } - -impl Display for ReturnAddress { - #[inline] - fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { - let s = match self { - ReturnAddress::AlreadyPruned => "Transaction is already pruned".to_string(), - ReturnAddress::NoTxAtScore => "Transaction not found at given accepting daa score".to_string(), - ReturnAddress::NonStandard => "Transaction was found but not standard".to_string(), - ReturnAddress::TxFromCoinbase => "Transaction return address is coinbase".to_string(), - ReturnAddress::NotFound(reason) => format!("Transaction return address not found: {}", reason), - ReturnAddress::Found(address) => address.to_string(), - }; - f.write_str(&s) - } -} diff --git a/consensus/src/consensus/mod.rs b/consensus/src/consensus/mod.rs index aaa6b8df3..69b7b6d94 100644 --- a/consensus/src/consensus/mod.rs +++ b/consensus/src/consensus/mod.rs @@ -41,6 +41,7 @@ use crate::{ window::{WindowManager, WindowType}, }, }; +use kaspa_addresses::Address; use kaspa_consensus_core::{ acceptance_data::AcceptanceData, api::{ @@ -65,7 +66,7 @@ use kaspa_consensus_core::{ muhash::MuHashExtensions, network::NetworkType, pruning::{PruningPointProof, PruningPointTrustedData, PruningPointsList}, - return_address::ReturnAddress, + return_address::ReturnAddressError, trusted::{ExternalGhostdagData, TrustedBlock}, tx::{MutableTransaction, Transaction, TransactionOutpoint, UtxoEntry}, BlockHashSet, BlueWorkType, ChainPath, HashMapCustomHasher, @@ -697,7 +698,7 @@ impl ConsensusApi for Consensus { sample_headers } - fn get_utxo_return_address(&self, txid: Hash, target_daa_score: u64) -> ReturnAddress { + fn get_utxo_return_address(&self, txid: Hash, target_daa_score: u64) -> Result { self.virtual_processor.get_utxo_return_address(txid, target_daa_score, self.get_source(), &self.config) } diff --git a/consensus/src/pipeline/virtual_processor/processor.rs b/consensus/src/pipeline/virtual_processor/processor.rs index 3ea094367..9ea1827ba 100644 --- a/consensus/src/pipeline/virtual_processor/processor.rs +++ b/consensus/src/pipeline/virtual_processor/processor.rs @@ -46,6 +46,7 @@ use crate::{ window::WindowManager, }, }; +use kaspa_addresses::Address; use kaspa_consensus_core::{ acceptance_data::{AcceptanceData, MergesetBlockAcceptanceData}, api::args::{TransactionValidationArgs, TransactionValidationBatchArgs}, @@ -56,7 +57,7 @@ use kaspa_consensus_core::{ header::Header, merkle::calc_hash_merkle_root, pruning::PruningPointsList, - return_address::ReturnAddress, + return_address::ReturnAddressError, tx::{MutableTransaction, Transaction}, utxo::{ utxo_diff::{ImmutableUtxoDiff, UtxoDiff}, @@ -1134,79 +1135,69 @@ impl VirtualStateProcessor { Ok(()) } - pub fn get_utxo_return_address(&self, txid: Hash, target_daa_score: u64, source_hash: Hash, config: &Config) -> ReturnAddress { + pub fn get_utxo_return_address( + &self, + txid: Hash, + target_daa_score: u64, + source_hash: Hash, + config: &Config, + ) -> Result { // We need consistency between the utxo_diffs_store, block_transactions_store, selected chain and header store reads let guard = self.pruning_lock.blocking_read(); - let source_daa_score = if let Ok(compact_header) = self.headers_store.get_compact_header_data(source_hash) { - compact_header.daa_score - } else { - trace!("Did not find compact header for source hash {}", source_hash); - return ReturnAddress::NotFound(format!("Did not find compact header for source hash {}", source_hash)); - }; + let source_daa_score = self + .headers_store + .get_compact_header_data(source_hash) + .map(|compact_header| compact_header.daa_score) + .map_err(|_| ReturnAddressError::NotFound(format!("Did not find compact header for source hash {}", source_hash)))?; + if target_daa_score < source_daa_score { // Early exit if target daa score is lower than that of pruning point's daa score: - return ReturnAddress::AlreadyPruned; + return Err(ReturnAddressError::AlreadyPruned); } let (matching_chain_block_hash, acceptance_data) = - match self.find_accepting_chain_block_hash_at_daa_score(target_daa_score, source_hash, guard) { - Ok(hash) => hash, - Err(return_address) => { - return return_address; - } - }; + self.find_accepting_chain_block_hash_at_daa_score(target_daa_score, source_hash, guard)?; - let (index, containing_acceptance) = - match self.find_tx_acceptance_data_and_index_from_block_acceptance(txid, acceptance_data.clone()) { - Some((index, containing_acceptance)) => (index, containing_acceptance), - None => { - return ReturnAddress::NotFound("Did not find containing_acceptance".to_string()); - } - }; + let (index, containing_acceptance) = self + .find_tx_acceptance_data_and_index_from_block_acceptance(txid, acceptance_data.clone()) + .ok_or(ReturnAddressError::NotFound(format!("Did not find containing_acceptance for tx {}", txid)))?; // Found Merged block containing the TXID - let tx = if let Ok(block_txs) = self.block_transactions_store.get(containing_acceptance.block_hash) { - if let Some(tx) = block_txs.get(index) { - tx.clone() - } else { - warn!("Did not find tx {} at index {} in chain block tx store of {}", txid, index, containing_acceptance.block_hash); - return ReturnAddress::NotFound(format!( - "Did not find tx {} at index {} in chain block tx store of {}", - txid, index, containing_acceptance.block_hash - )); - } - } else { - warn!("Did not find tx {} at index {} in chain block tx store of {}", txid, index, containing_acceptance.block_hash); - return ReturnAddress::NotFound(format!( - "Did not find tx {} at index {} in chain block tx store of {}", - txid, index, containing_acceptance.block_hash - )); - }; + let tx = self + .block_transactions_store + .get(containing_acceptance.block_hash) + .map_err(|_| ReturnAddressError::NotFound(format!("Did not block {} at block tx store", containing_acceptance.block_hash))) + .and_then(|block_txs| { + block_txs.get(index).cloned().ok_or_else(|| { + ReturnAddressError::NotFound(format!( + "Did not find index {} in transactions of block {}", + index, containing_acceptance.block_hash + )) + }) + })?; if tx.id() != txid { // Should never happen, but do a sanity check. This would mean something went wrong with storing block transactions // Sanity check is necessary to guarantee that this function will never give back a wrong address (err on the side of NotFound) warn!("Expected {} to match {} when checking block_transaction_store using array index of transaction", tx.id(), txid); - return ReturnAddress::NotFound(format!( + return Err(ReturnAddressError::NotFound(format!( "Expected {} to match {} when checking block_transaction_store using array index of transaction", tx.id(), txid - )); + ))); } if tx.inputs.is_empty() { // A transaction may have no inputs (like a coinbase transaction) - return ReturnAddress::TxFromCoinbase; + return Err(ReturnAddressError::TxFromCoinbase); } let first_input_prev_outpoint = &tx.inputs[0].previous_outpoint; // Expected to never fail, since we found the acceptance data and therefore there must be matching diff - let utxo_diff = if let Ok(res) = self.utxo_diffs_store.get(matching_chain_block_hash) { - res - } else { - return ReturnAddress::NotFound(format!("Did not find a utxo diff for chain block {}", matching_chain_block_hash)); - }; + let utxo_diff = self.utxo_diffs_store.get(matching_chain_block_hash).map_err(|_| { + ReturnAddressError::NotFound(format!("Did not find a utxo diff for chain block {}", matching_chain_block_hash)) + })?; let removed_diffs = utxo_diff.removed(); let spk = if let Some(utxo_entry) = removed_diffs.get(first_input_prev_outpoint) { @@ -1219,45 +1210,33 @@ impl VirtualStateProcessor { // In this case, removed_diff wouldn't contain the outpoint of the created-and-immediately-spent UTXO // so we use the transaction (which also has acceptance data in this block) and look at its outputs let other_txid = first_input_prev_outpoint.transaction_id; - let (other_index, other_containing_acceptance) = - if let Some(res) = self.find_tx_acceptance_data_and_index_from_block_acceptance(other_txid, acceptance_data) { - res - } else { - return ReturnAddress::NotFound( - "The other transaction's acceptance data must also be in the same block in this case".to_string(), - ); - }; - let other_tx = if let Ok(block_txs) = self.block_transactions_store.get(other_containing_acceptance.block_hash) { - if let Some(tx) = block_txs.get(other_index) { - tx.clone() - } else { - warn!( - "Did not find tx {} at index {} in chain block tx store of {}", - other_txid, other_index, other_containing_acceptance.block_hash - ); - return ReturnAddress::NotFound(format!( - "Did not find tx {} at index {} in chain block tx store of {}", - other_txid, other_index, other_containing_acceptance.block_hash - )); - } - } else { - warn!( - "Did not find tx {} at index {} in chain block tx store of {}", - other_txid, other_index, other_containing_acceptance.block_hash - ); - return ReturnAddress::NotFound(format!( - "Did not find tx {} at index {} in chain block tx store of {}", - other_txid, other_index, other_containing_acceptance.block_hash - )); - }; + let (other_index, other_containing_acceptance) = self + .find_tx_acceptance_data_and_index_from_block_acceptance(other_txid, acceptance_data) + .ok_or(ReturnAddressError::NotFound( + "The other transaction's acceptance data must also be in the same block in this case".to_string(), + ))?; + let other_tx = self + .block_transactions_store + .get(other_containing_acceptance.block_hash) + .map_err(|_| { + ReturnAddressError::NotFound(format!("Did not block {} at block tx store", other_containing_acceptance.block_hash)) + }) + .and_then(|block_txs| { + block_txs.get(other_index).cloned().ok_or_else(|| { + ReturnAddressError::NotFound(format!( + "Did not find index {} in transactions of block {}", + other_index, other_containing_acceptance.block_hash + )) + }) + })?; other_tx.outputs[first_input_prev_outpoint.index as usize].script_public_key.clone() }; if let Ok(address) = extract_script_pub_key_address(&spk, config.prefix()) { - ReturnAddress::Found(address) + Ok(address) } else { - ReturnAddress::NonStandard + Err(ReturnAddressError::NonStandard) } } @@ -1266,18 +1245,19 @@ impl VirtualStateProcessor { target_daa_score: u64, source_hash: Hash, _guard: SessionReadGuard, - ) -> Result<(Hash, Arc>), ReturnAddress> { + ) -> Result<(Hash, Arc>), ReturnAddressError> { let sc_read = self.selected_chain_store.read(); let source_index = sc_read .get_by_hash(source_hash) - .map_err(|_| ReturnAddress::NotFound(format!("Did not find index for hash {}", source_hash)))?; - let (tip_index, tip_hash) = sc_read.get_tip().map_err(|_| ReturnAddress::NotFound("Did not find tip data".to_string()))?; + .map_err(|_| ReturnAddressError::NotFound(format!("Did not find index for hash {}", source_hash)))?; + let (tip_index, tip_hash) = + sc_read.get_tip().map_err(|_| ReturnAddressError::NotFound("Did not find tip data".to_string()))?; let tip_daa_score = self .headers_store .get_compact_header_data(tip_hash) .map(|tip| tip.daa_score) - .map_err(|_| ReturnAddress::NotFound(format!("Did not find compact header data for tip hash {}", tip_hash)))?; + .map_err(|_| ReturnAddressError::NotFound(format!("Did not find compact header data for tip hash {}", tip_hash)))?; let mut low_index = tip_index.saturating_sub(tip_daa_score.saturating_sub(target_daa_score)).max(source_index); let mut high_index = tip_index; @@ -1290,13 +1270,13 @@ impl VirtualStateProcessor { // 1. Get the chain block hash at that index. Error if we don't find a hash at an index let hash = sc_read.get_by_index(mid).map_err(|_| { trace!("Did not find a hash at index {}", mid); - ReturnAddress::NotFound(format!("Did not find a hash at index {}", mid)) + ReturnAddressError::NotFound(format!("Did not find a hash at index {}", mid)) })?; // 2. Get the compact header so we have access to the daa_score. Error if we let compact_header = self.headers_store.get_compact_header_data(hash).map_err(|_| { trace!("Did not find a compact header with hash {}", hash); - ReturnAddress::NotFound(format!("Did not find a compact header with hash {}", hash)) + ReturnAddressError::NotFound(format!("Did not find a compact header with hash {}", hash)) })?; // 3. Compare block daa score to our target @@ -1314,12 +1294,12 @@ impl VirtualStateProcessor { } if low_index > high_index { - return Err(ReturnAddress::NoTxAtScore); + return Err(ReturnAddressError::NoTxAtScore); } }; let acceptance_data = self.acceptance_data_store.get(matching_chain_block_hash).map_err(|_| { - ReturnAddress::NotFound(format!("Did not find acceptance data for chain block {}", matching_chain_block_hash)) + ReturnAddressError::NotFound(format!("Did not find acceptance data for chain block {}", matching_chain_block_hash)) })?; Ok((matching_chain_block_hash, acceptance_data)) diff --git a/rpc/core/src/api/rpc.rs b/rpc/core/src/api/rpc.rs index 34be67da9..d9011c167 100644 --- a/rpc/core/src/api/rpc.rs +++ b/rpc/core/src/api/rpc.rs @@ -438,7 +438,7 @@ pub trait RpcApi: Sync + Send + AnySync { request: GetDaaScoreTimestampEstimateRequest, ) -> RpcResult; - async fn get_utxo_return_address(&self, txid: RpcHash, accepting_block_daa_score: u64) -> RpcResult> { + async fn get_utxo_return_address(&self, txid: RpcHash, accepting_block_daa_score: u64) -> RpcResult { Ok(self .get_utxo_return_address_call(None, GetUtxoReturnAddressRequest { txid, accepting_block_daa_score }) .await? diff --git a/rpc/core/src/error.rs b/rpc/core/src/error.rs index 38c8ab3cf..1f82c40d2 100644 --- a/rpc/core/src/error.rs +++ b/rpc/core/src/error.rs @@ -2,7 +2,7 @@ //! [`RpcError`] enum used by RPC primitives. //! -use kaspa_consensus_core::{return_address::ReturnAddress, subnets::SubnetworkConversionError, tx::TransactionId}; +use kaspa_consensus_core::{return_address::ReturnAddressError, subnets::SubnetworkConversionError, tx::TransactionId}; use kaspa_utils::networking::IpAddress; use std::{net::AddrParseError, num::TryFromIntError}; use thiserror::Error; @@ -136,7 +136,7 @@ pub enum RpcError { ConsensusClient(#[from] kaspa_consensus_client::error::Error), #[error("utxo return address could not be found -> {0}")] - UtxoReturnAddressNotFound(ReturnAddress), + UtxoReturnAddressNotFound(ReturnAddressError), } impl From for RpcError { diff --git a/rpc/core/src/model/message.rs b/rpc/core/src/model/message.rs index ab327bdeb..cb663c394 100644 --- a/rpc/core/src/model/message.rs +++ b/rpc/core/src/model/message.rs @@ -2702,11 +2702,11 @@ impl Deserializer for GetUtxoReturnAddressRequest { #[derive(Clone, Debug, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] pub struct GetUtxoReturnAddressResponse { - pub return_address: Option, + pub return_address: RpcAddress, } impl GetUtxoReturnAddressResponse { - pub fn new(return_address: Option) -> Self { + pub fn new(return_address: RpcAddress) -> Self { Self { return_address } } } @@ -2714,7 +2714,7 @@ impl GetUtxoReturnAddressResponse { impl Serializer for GetUtxoReturnAddressResponse { fn serialize(&self, writer: &mut W) -> std::io::Result<()> { store!(u16, &1, writer)?; - store!(Option, &self.return_address, writer)?; + store!(RpcAddress, &self.return_address, writer)?; Ok(()) } @@ -2723,7 +2723,7 @@ impl Serializer for GetUtxoReturnAddressResponse { impl Deserializer for GetUtxoReturnAddressResponse { fn deserialize(reader: &mut R) -> std::io::Result { let _version = load!(u16, reader)?; - let return_address = load!(Option, reader)?; + let return_address = load!(RpcAddress, reader)?; Ok(Self { return_address }) } diff --git a/rpc/grpc/core/Cargo.toml b/rpc/grpc/core/Cargo.toml index 2edc10b60..61734df9e 100644 --- a/rpc/grpc/core/Cargo.toml +++ b/rpc/grpc/core/Cargo.toml @@ -10,6 +10,7 @@ license.workspace = true repository.workspace = true [dependencies] +kaspa-addresses.workspace = true kaspa-consensus-core.workspace = true kaspa-core.workspace = true kaspa-notify.workspace = true diff --git a/rpc/grpc/core/src/convert/message.rs b/rpc/grpc/core/src/convert/message.rs index d6ce30f13..c92e824ed 100644 --- a/rpc/grpc/core/src/convert/message.rs +++ b/rpc/grpc/core/src/convert/message.rs @@ -19,6 +19,7 @@ //! The SubmitBlockResponse is a notable exception to this general rule. use crate::protowire::{self, submit_block_response_message::RejectReason}; +use kaspa_addresses::Address; use kaspa_consensus_core::{network::NetworkId, Hash}; use kaspa_core::debug; use kaspa_notify::subscription::Command; @@ -437,12 +438,7 @@ from!(item: &kaspa_rpc_core::GetUtxoReturnAddressRequest, protowire::GetUtxoRetu } }); from!(item: RpcResult<&kaspa_rpc_core::GetUtxoReturnAddressResponse>, protowire::GetUtxoReturnAddressResponseMessage, { - if let Some(return_address) = &item.return_address { - Self { return_address: return_address.into(), error: None } - } else { - Self { return_address: String::from(""), error: None } - } - + Self { return_address: item.return_address.address_to_string(), error: None } }); from!(&kaspa_rpc_core::PingRequest, protowire::PingRequestMessage); @@ -938,9 +934,7 @@ try_from!(item: &protowire::GetUtxoReturnAddressRequestMessage, kaspa_rpc_core:: } }); try_from!(item: &protowire::GetUtxoReturnAddressResponseMessage, RpcResult, { - // Self { return_address: Some(Address::from(item.return_address)) } - // TODO: import Address - Self { return_address: None } + Self { return_address: Address::try_from(item.return_address.clone())? } }); try_from!(&protowire::PingRequestMessage, kaspa_rpc_core::PingRequest); diff --git a/rpc/service/src/service.rs b/rpc/service/src/service.rs index 3abb488ec..94b4b8f9c 100644 --- a/rpc/service/src/service.rs +++ b/rpc/service/src/service.rs @@ -7,7 +7,6 @@ use crate::service::NetworkType::{Mainnet, Testnet}; use async_trait::async_trait; use kaspa_consensus_core::api::counters::ProcessingCounters; use kaspa_consensus_core::errors::block::RuleError; -use kaspa_consensus_core::return_address::ReturnAddress; use kaspa_consensus_core::{ block::Block, coinbase::MinerData, @@ -803,13 +802,10 @@ NOTE: This error usually indicates an RPC conversion error between the node and let session = self.consensus_manager.consensus().session().await; // Convert a SPK to an Address - let return_address = - match session.async_get_utxo_return_script_public_key(request.txid, request.accepting_block_daa_score).await { - ReturnAddress::Found(address) => Some(address), - other => return Err(RpcError::UtxoReturnAddressNotFound(other)), - }; - - Ok(GetUtxoReturnAddressResponse { return_address }) + match session.async_get_utxo_return_script_public_key(request.txid, request.accepting_block_daa_score).await { + Ok(return_address) => return Ok(GetUtxoReturnAddressResponse { return_address }), + Err(error) => return Err(RpcError::UtxoReturnAddressNotFound(error)), + }; } async fn ping_call(&self, _connection: Option<&DynRpcConnection>, _: PingRequest) -> RpcResult { From 77e9715eca655100b7c9c62fc6cee4828ead4e72 Mon Sep 17 00:00:00 2001 From: coderofstuff <114628839+coderofstuff@users.noreply.github.com> Date: Tue, 15 Oct 2024 17:25:34 -0600 Subject: [PATCH 14/14] Add integration test for utxo return address --- .../src/daemon_integration_tests.rs | 20 ++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/testing/integration/src/daemon_integration_tests.rs b/testing/integration/src/daemon_integration_tests.rs index 460cf049c..25471cab1 100644 --- a/testing/integration/src/daemon_integration_tests.rs +++ b/testing/integration/src/daemon_integration_tests.rs @@ -274,7 +274,8 @@ async fn daemon_utxos_propagation_test() { clients.iter().for_each(|x| x.utxos_changed_listener().unwrap().drain()); clients.iter().for_each(|x| x.virtual_daa_score_changed_listener().unwrap().drain()); - // Spend some coins + // Spend some coins - sending funds from miner address to user address + // The transaction here is later used to verify utxo return address RPC const NUMBER_INPUTS: u64 = 2; const NUMBER_OUTPUTS: u64 = 2; const TX_AMOUNT: u64 = SIMNET_PARAMS.pre_deflationary_phase_base_subsidy * (NUMBER_INPUTS * 5 - 1) / 5; @@ -324,6 +325,23 @@ async fn daemon_utxos_propagation_test() { assert_eq!(user_balance, TX_AMOUNT); } + // UTXO Return Address Test + // Mine another block to accept the transactions from the previous block + // The tx above is sending from miner address to user address + mine_block(blank_address.clone(), &rpc_client1, &clients).await; + let new_utxos = rpc_client1.get_utxos_by_addresses(vec![user_address]).await.unwrap(); + let new_utxo = new_utxos + .iter() + .find(|utxo| utxo.outpoint.transaction_id == transaction.id()) + .expect("Did not find a utxo for the tx we just created but expected to"); + + let utxo_return_address = rpc_client1 + .get_utxo_return_address(new_utxo.outpoint.transaction_id, new_utxo.utxo_entry.block_daa_score) + .await + .expect("We just created the tx and utxo here"); + + assert_eq!(miner_address, utxo_return_address); + // Terminate multi-listener clients for x in clients.iter() { x.disconnect().await.unwrap();