Skip to content

Commit

Permalink
fix(consensus)!: defer proposing txs with conflicting unversioned inp…
Browse files Browse the repository at this point in the history
…uts (#1125)

Description
---
fix(consensus)!: defer proposing txs with conflicting unversioned inputs
refactor: locks take into account if the requested input was versioned
or not

Motivation and Context
---
A transaction may provide an unversioned input. In the multi-shard case,
if this input is locked by another transaction, the first transaction
should not be proposed until that lock is cleared.

How Has This Been Tested?
---
Manually, submitting transactions with conflicting unversioned inputs.

What process can a PR reviewer use to test or verify this change?
---
Submit transactions with transaction_generator and
transaction_submitter. The default generated transactions use
unversioned inputs.

Breaking Changes
---

- [ ] None
- [x] Requires data directory to be deleted
- [ ] Other - Please specify
  • Loading branch information
sdbondi authored Sep 3, 2024
1 parent f91bcd9 commit b3f1507
Show file tree
Hide file tree
Showing 98 changed files with 906 additions and 593 deletions.
4 changes: 3 additions & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ use tari_crypto::{
ristretto::RistrettoPublicKey,
tari_utilities::{hex::Hex, ByteArray},
};
use tari_dan_common_types::{optional::Optional, NodeAddressable};
use tari_dan_common_types::{optional::Optional, NodeAddressable, VersionedSubstateId};
use tari_dan_storage::{
consensus_models::{BurntUtxo, SubstateRecord},
global::{GlobalDb, MetadataKey},
Expand All @@ -59,7 +59,6 @@ use tari_epoch_manager::{base_layer::EpochManagerHandle, EpochManagerError, Epoc
use tari_shutdown::ShutdownSignal;
use tari_state_store_sqlite::SqliteStateStore;
use tari_template_lib::models::{EncryptedData, TemplateAddress, UnclaimedConfidentialOutputAddress};
use tari_transaction::VersionedSubstateId;
use tokio::{task, task::JoinHandle, time};

use crate::{
Expand Down
32 changes: 18 additions & 14 deletions applications/tari_dan_app_utilities/src/transaction_executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,22 +8,23 @@ use log::*;
use tari_common::configuration::Network;
use tari_common_types::types::PublicKey;
use tari_crypto::tari_utilities::ByteArray;
use tari_dan_common_types::services::template_provider::TemplateProvider;
use tari_dan_common_types::{
services::template_provider::TemplateProvider,
SubstateLockType,
SubstateRequirement,
VersionedSubstateId,
};
use tari_dan_engine::{
fees::{FeeModule, FeeTable},
runtime::{AuthParams, RuntimeModule},
state_store::{memory::MemoryStateStore, StateStoreError},
template::LoadedTemplate,
transaction::{TransactionError, TransactionProcessor},
};
use tari_dan_storage::consensus_models::{SubstateLockType, VersionedSubstateIdLockIntent};
use tari_engine_types::{
commit_result::ExecuteResult,
substate::{Substate, SubstateId},
virtual_substate::VirtualSubstates,
};
use tari_dan_storage::consensus_models::VersionedSubstateIdLockIntent;
use tari_engine_types::{commit_result::ExecuteResult, substate::Substate, virtual_substate::VirtualSubstates};
use tari_template_lib::{crypto::RistrettoPublicKeyBytes, prelude::NonFungibleAddress};
use tari_transaction::{Transaction, VersionedSubstateId};
use tari_transaction::Transaction;

const _LOG_TARGET: &str = "tari::dan::transaction_executor";

Expand All @@ -45,20 +46,23 @@ pub struct ExecutionOutput {
}

impl ExecutionOutput {
pub fn resolve_inputs(&self, inputs: &IndexMap<SubstateId, Substate>) -> Vec<VersionedSubstateIdLockIntent> {
pub fn resolve_inputs(
&self,
inputs: &IndexMap<SubstateRequirement, Substate>,
) -> Vec<VersionedSubstateIdLockIntent> {
if let Some(diff) = self.result.finalize.accept() {
inputs
.iter()
.map(|(substate_id, substate)| {
let lock_flag = if diff.down_iter().any(|(id, _)| id == substate_id) {
.map(|(substate_req, substate)| {
let lock_flag = if diff.down_iter().any(|(id, _)| id == substate_req.substate_id()) {
// Update all inputs that were DOWNed to be write locked
SubstateLockType::Write
} else {
// Any input not downed, gets a read lock
SubstateLockType::Read
};
VersionedSubstateIdLockIntent::new(
VersionedSubstateId::new(substate_id.clone(), substate.version()),
VersionedSubstateId::new(substate_req.substate_id().clone(), substate.version()),
lock_flag,
)
})
Expand All @@ -68,9 +72,9 @@ impl ExecutionOutput {
// shards involved but do not lock them. We dont actually lock anything for rejected transactions anyway.
inputs
.iter()
.map(|(substate_id, substate)| {
.map(|(substate_req, substate)| {
VersionedSubstateIdLockIntent::new(
VersionedSubstateId::new(substate_id.clone(), substate.version()),
VersionedSubstateId::new(substate_req.substate_id().clone(), substate.version()),
SubstateLockType::Read,
)
})
Expand Down
4 changes: 2 additions & 2 deletions applications/tari_dan_wallet_cli/src/command/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ use anyhow::anyhow;
use clap::{Args, Subcommand};
use tari_bor::decode_exact;
use tari_common_types::types::PublicKey;
use tari_dan_common_types::{Epoch, SubstateAddress};
use tari_dan_common_types::{Epoch, SubstateAddress, SubstateRequirement};
use tari_dan_engine::abi::Type;
use tari_dan_wallet_sdk::apis::confidential_transfer::ConfidentialTransferInputSelection;
use tari_engine_types::{
Expand All @@ -53,7 +53,7 @@ use tari_template_lib::{
models::{Amount, BucketId, NonFungibleAddress, NonFungibleId},
prelude::ResourceAddress,
};
use tari_transaction::{SubstateRequirement, TransactionId};
use tari_transaction::TransactionId;
use tari_transaction_manifest::{parse_manifest, ManifestValue};
use tari_utilities::{hex::to_hex, ByteArray};
use tari_wallet_daemon_client::{
Expand Down
4 changes: 2 additions & 2 deletions applications/tari_dan_wallet_daemon/src/handlers/accounts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use tari_crypto::{
ristretto::{RistrettoComSig, RistrettoPublicKey},
tari_utilities::ByteArray,
};
use tari_dan_common_types::optional::Optional;
use tari_dan_common_types::{optional::Optional, SubstateRequirement};
use tari_dan_wallet_crypto::ConfidentialProofStatement;
use tari_dan_wallet_sdk::{
apis::{confidential_transfer::TransferParams, jwt::JrpcPermission, key_manager, substate::ValidatorScanResult},
Expand All @@ -36,7 +36,7 @@ use tari_template_lib::{
models::{Amount, UnclaimedConfidentialOutputAddress},
prelude::CONFIDENTIAL_TARI_RESOURCE_ADDRESS,
};
use tari_transaction::{SubstateRequirement, Transaction};
use tari_transaction::Transaction;
use tari_wallet_daemon_client::{
types::{
AccountGetDefaultRequest,
Expand Down
3 changes: 2 additions & 1 deletion applications/tari_dan_wallet_daemon/src/handlers/nfts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use anyhow::anyhow;
use log::info;
use tari_common_types::types::PublicKey;
use tari_crypto::{keys::PublicKey as PK, ristretto::RistrettoSecretKey, tari_utilities::ByteArray};
use tari_dan_common_types::SubstateRequirement;
use tari_dan_wallet_sdk::{
apis::{jwt::JrpcPermission, key_manager},
models::Account,
Expand All @@ -18,7 +19,7 @@ use tari_template_lib::{
crypto::RistrettoPublicKeyBytes,
prelude::{Amount, ComponentAddress, Metadata, NonFungibleAddress, NonFungibleId, ResourceAddress},
};
use tari_transaction::{SubstateRequirement, Transaction, TransactionId};
use tari_transaction::{Transaction, TransactionId};
use tari_wallet_daemon_client::types::{
GetAccountNftRequest,
GetAccountNftResponse,
Expand Down
4 changes: 2 additions & 2 deletions applications/tari_dan_wallet_daemon/src/indexer_jrpc_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use std::sync::{Arc, Mutex};

use axum::async_trait;
use reqwest::{IntoUrl, Url};
use tari_dan_common_types::{optional::IsNotFoundError, substate_type::SubstateType};
use tari_dan_common_types::{optional::IsNotFoundError, substate_type::SubstateType, SubstateRequirement};
use tari_dan_wallet_sdk::network::{
SubstateListItem,
SubstateListResult,
Expand All @@ -28,7 +28,7 @@ use tari_indexer_client::{
},
};
use tari_template_lib::models::TemplateAddress;
use tari_transaction::{SubstateRequirement, Transaction, TransactionId};
use tari_transaction::{Transaction, TransactionId};
use url::ParseError;

#[derive(Debug, Clone)]
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
// Copyright 2024 The Tari Project
// SPDX-License-Identifier: BSD-3-Clause

use tari_dan_common_types::SubstateRequirement;
use tari_dan_wallet_sdk::models::NewAccountInfo;
use tari_engine_types::commit_result::ExecuteResult;
use tari_transaction::{SubstateRequirement, Transaction, TransactionId};
use tari_transaction::{Transaction, TransactionId};
use tokio::sync::{mpsc, oneshot};

use super::TransactionServiceError;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
use std::{sync::Arc, time::Duration};

use log::*;
use tari_dan_common_types::optional::IsNotFoundError;
use tari_dan_common_types::{optional::IsNotFoundError, SubstateRequirement};
use tari_dan_wallet_sdk::{
models::{NewAccountInfo, TransactionStatus},
network::WalletNetworkInterface,
Expand All @@ -13,7 +13,7 @@ use tari_dan_wallet_sdk::{
};
use tari_engine_types::commit_result::ExecuteResult;
use tari_shutdown::ShutdownSignal;
use tari_transaction::{SubstateRequirement, Transaction, TransactionId};
use tari_transaction::{Transaction, TransactionId};
use tokio::{
sync::{mpsc, watch, Semaphore},
time,
Expand Down
4 changes: 2 additions & 2 deletions applications/tari_indexer/src/dry_run/processor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ use tari_dan_app_utilities::{
template_manager::implementation::TemplateManager,
transaction_executor::{TariDanTransactionProcessor, TransactionExecutor as _},
};
use tari_dan_common_types::{Epoch, PeerAddress, SubstateAddress};
use tari_dan_common_types::{Epoch, PeerAddress, SubstateAddress, SubstateRequirement};
use tari_dan_engine::{fees::FeeTable, state_store::new_memory_store};
use tari_engine_types::{
commit_result::ExecuteResult,
Expand All @@ -42,7 +42,7 @@ use tari_indexer_lib::{
substate_scanner::SubstateScanner,
transaction_autofiller::TransactionAutofiller,
};
use tari_transaction::{SubstateRequirement, Transaction};
use tari_transaction::Transaction;
use tari_validator_node_rpc::client::{
SubstateResult,
TariValidatorNodeRpcClientFactory,
Expand Down
4 changes: 3 additions & 1 deletion applications/tari_indexer/src/transaction_manager/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ use tari_dan_common_types::{
optional::{IsNotFoundError, Optional},
NodeAddressable,
SubstateAddress,
SubstateRequirement,
ToSubstateAddress,
};
use tari_engine_types::substate::SubstateId;
use tari_epoch_manager::EpochManagerReader;
Expand All @@ -37,7 +39,7 @@ use tari_indexer_lib::{
substate_scanner::SubstateScanner,
transaction_autofiller::TransactionAutofiller,
};
use tari_transaction::{SubstateRequirement, Transaction, TransactionId};
use tari_transaction::{Transaction, TransactionId};
use tari_validator_node_rpc::client::{
SubstateResult,
TransactionResultStatus,
Expand Down
13 changes: 11 additions & 2 deletions applications/tari_validator_node/src/bootstrap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,16 @@ use tari_dan_app_utilities::{
template_manager::{implementation::TemplateManager, interface::TemplateManagerHandle},
transaction_executor::TariDanTransactionProcessor,
};
use tari_dan_common_types::{shard::Shard, Epoch, NodeAddressable, NodeHeight, NumPreshards, PeerAddress, ShardGroup};
use tari_dan_common_types::{
shard::Shard,
Epoch,
NodeAddressable,
NodeHeight,
NumPreshards,
PeerAddress,
ShardGroup,
VersionedSubstateId,
};
use tari_dan_engine::fees::FeeTable;
use tari_dan_p2p::TariMessagingSpec;
use tari_dan_storage::{
Expand Down Expand Up @@ -86,7 +95,7 @@ use tari_template_lib::{
prelude::{ComponentAccessRules, OwnerRule, ResourceType},
resource::TOKEN_SYMBOL,
};
use tari_transaction::{Transaction, VersionedSubstateId};
use tari_transaction::Transaction;
use tari_validator_node_rpc::client::TariValidatorNodeRpcClientFactory;
use tokio::{sync::mpsc, task::JoinHandle};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,11 @@ use indexmap::IndexMap;
use log::info;
use tari_consensus::traits::{BlockTransactionExecutor, BlockTransactionExecutorError};
use tari_dan_app_utilities::transaction_executor::TransactionExecutor;
use tari_dan_common_types::Epoch;
use tari_dan_common_types::{Epoch, SubstateRequirement};
use tari_dan_engine::state_store::{memory::MemoryStateStore, new_memory_store, AtomicDb, StateWriter};
use tari_dan_storage::{consensus_models::ExecutedTransaction, StateStore};
use tari_engine_types::{
substate::{Substate, SubstateId},
substate::Substate,
virtual_substate::{VirtualSubstate, VirtualSubstateId, VirtualSubstates},
};
use tari_transaction::Transaction;
Expand All @@ -38,7 +38,7 @@ where TExecutor: TransactionExecutor

fn add_substates_to_memory_db(
&self,
inputs: &IndexMap<SubstateId, Substate>,
inputs: &IndexMap<SubstateRequirement, Substate>,
out: &MemoryStateStore,
) -> Result<(), BlockTransactionExecutorError> {
// TODO: pass the impl SubstateStore directly into the engine
Expand All @@ -47,7 +47,7 @@ where TExecutor: TransactionExecutor
.map_err(|e| BlockTransactionExecutorError::StateStoreError(e.to_string()))?;
for (id, substate) in inputs {
access
.set_state(id, substate)
.set_state(id.substate_id(), substate)
.map_err(|e| BlockTransactionExecutorError::StateStoreError(e.to_string()))?;
}
access
Expand Down Expand Up @@ -81,7 +81,7 @@ where
&self,
transaction: Transaction,
current_epoch: Epoch,
resolved_inputs: &IndexMap<SubstateId, Substate>,
resolved_inputs: &IndexMap<SubstateRequirement, Substate>,
) -> Result<ExecutedTransaction, BlockTransactionExecutorError> {
let id = *transaction.id();

Expand Down
7 changes: 6 additions & 1 deletion applications/tari_validator_node/src/p2p/rpc/service_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -309,10 +309,15 @@ impl ValidatorNodeRpcService for ValidatorNodeRpcServiceImpl {
}

async fn get_high_qc(&self, _request: Request<GetHighQcRequest>) -> Result<Response<GetHighQcResponse>, RpcStatus> {
let current_epoch = self
.epoch_manager
.current_epoch()
.await
.map_err(RpcStatus::log_internal_error(LOG_TARGET))?;
let high_qc = self
.shard_state_store
.with_read_tx(|tx| {
HighQc::get(tx)
HighQc::get(tx, current_epoch)
.optional()?
.map(|hqc| hqc.get_quorum_certificate(tx))
.transpose()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,14 @@
use std::{collections::HashSet, fmt::Display, iter};

use log::*;
use tari_dan_common_types::{optional::Optional, NumPreshards, PeerAddress, ShardGroup, SubstateAddress};
use tari_dan_common_types::{
optional::Optional,
NumPreshards,
PeerAddress,
ShardGroup,
SubstateAddress,
ToSubstateAddress,
};
use tari_dan_p2p::{DanMessage, NewTransactionMessage};
use tari_dan_storage::{consensus_models::TransactionRecord, StateStore};
use tari_engine_types::commit_result::RejectReason;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,12 @@ use std::collections::HashSet;

use async_trait::async_trait;
use indexmap::IndexMap;
use tari_dan_common_types::Epoch;
use tari_dan_common_types::{Epoch, SubstateRequirement};
use tari_engine_types::{
substate::{Substate, SubstateId},
virtual_substate::VirtualSubstates,
};
use tari_transaction::{SubstateRequirement, Transaction};
use tari_transaction::Transaction;

pub struct ResolvedSubstates {
pub local: IndexMap<SubstateId, Substate>,
Expand Down
Loading

0 comments on commit b3f1507

Please sign in to comment.