fix(consensus)!: deferred execution of txs with unversioned inputs (#…

…1023) Description --- - Mark transactions as deferred and execute in consensus - Set resolved inputs for each transaction that includes which lock is required - Remove input refs from transactions - Determine read locks from transaction execution result Motivation and Context --- Deferred execution previously had a default COMMIT decision which led to bugs when running rejected transactions (they would still get a COMMIT decision, but the validator does not have an accepted result to commit). This PR partially implements late execution for local-only transactions. Additional work is required to support this and it has been disabled i.e. will ABORT the transaction for transactions without versions. There was a bug where inputs without versions were provided but filled inputs for those substates were provided, the transaction would still detect that one or more inputs do not have a version. Input refs are unnecessary because the validator will always know which substates were written at lock time. TODOs: - missing transactions needs to differentiate between transactions pending execution and transactions that are deferred. - correct pledging procedure for unversioned inputs - blocks should generate a diff which is committed on a 3-chain - current committed state + diffs from uncommitted blocks can be used to determine inputs for subsequent transactions How Has This Been Tested? --- Existing tests pass, a new consensus unit test was written which is ignored because it fails, requires additional work to succeed, manually (previously working transactions still work). What process can a PR reviewer use to test or verify this change? --- Submit a transaction with a missing version Breaking Changes --- - [ ] None - [x] Requires data directory to be deleted - [ ] Other - Please specify
tari-project · Apr 30, 2024 · 81d884a · 81d884a
1 parent a77eda6
commit 81d884a
Show file tree

Hide file tree

Showing 89 changed files with 1,546 additions and 1,208 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -179,7 +179,7 @@ httpmock = "0.6.8"
 humantime = "2.1.0"
 humantime-serde = "1.1.1"
 include_dir = "0.7.2"
-indexmap = "2.1.0"
+indexmap = "2.2.6"
 indoc = "1.0.6"
 itertools = "0.11.0"
 lazy_static = "1.4.0"

diff --git a/applications/tari_dan_app_utilities/Cargo.toml b/applications/tari_dan_app_utilities/Cargo.toml
@@ -26,7 +26,7 @@ tari_transaction = { workspace = true }
 tari_validator_node_client = { workspace = true }
 tari_bor = { workspace = true, default-features = true }
 tari_indexer_lib = { workspace = true }
-tari_networking = { workspace = true}
+tari_networking = { workspace = true }
 tari_validator_node_rpc = { workspace = true }
 
 anyhow = { workspace = true }
@@ -49,11 +49,11 @@ serde = { workspace = true, features = ["default", "derive"] }
 serde_json = { workspace = true }
 thiserror = { workspace = true }
 tokio = { workspace = true, features = [
-  "default",
-  "macros",
-  "time",
-  "sync",
-  "rt-multi-thread",
-]}
+    "default",
+    "macros",
+    "time",
+    "sync",
+    "rt-multi-thread",
+] }
 tokio-stream = { workspace = true, features = ["sync"] }
 config = { workspace = true }
diff --git a/applications/tari_dan_app_utilities/src/transaction_executor.rs b/applications/tari_dan_app_utilities/src/transaction_executor.rs
@@ -76,7 +76,7 @@ where TTemplateProvider: TemplateProvider<Template = LoadedTemplate>
 
         let processor = TransactionProcessor::new(
             self.template_provider.clone(),
-            state_store,
+            state_store.clone(),
             auth_params,
             virtual_substates,
             modules,

diff --git a/applications/tari_dan_wallet_daemon/src/handlers/accounts.rs b/applications/tari_dan_wallet_daemon/src/handlers/accounts.rs
@@ -106,11 +106,6 @@ pub async fn handle_create(
         .locate_dependent_substates(&[default_account.address.clone()])
         .await?;
 
-    // We aren't mutating the resources
-    let (input_refs, inputs) = inputs
-        .into_iter()
-        .partition::<Vec<_>, _>(|s| s.substate_id.is_resource());
-
     let signing_key_index = req.key_id.unwrap_or(default_account.key_index);
     let signing_key = key_manager_api.derive_key(key_manager::TRANSACTION_BRANCH, signing_key_index)?;
 
@@ -129,11 +124,6 @@ pub async fn handle_create(
     let transaction = Transaction::builder()
         .fee_transaction_pay_from_component(default_account.address.as_component_address().unwrap(), max_fee)
         .create_account(owner_pk.clone())
-        .with_input_refs(
-            input_refs
-                .iter()
-                .map(|s| SubstateRequirement::new(s.substate_id.clone(), Some(s.version))),
-        )
         .with_inputs(
             inputs
                 .iter()
@@ -954,7 +944,7 @@ pub async fn handle_transfer(
     let transaction = Transaction::builder()
         .with_fee_instructions(fee_instructions)
         .with_instructions(instructions)
-        .with_input_refs(vec![resource_substate_address])
+        .with_inputs(vec![resource_substate_address])
         .sign(&account_secret_key.key)
         .build();
 

diff --git a/applications/tari_dan_wallet_web_ui/src/routes/AssetVault/Components/SendMoney.tsx b/applications/tari_dan_wallet_web_ui/src/routes/AssetVault/Components/SendMoney.tsx
@@ -79,12 +79,12 @@ export function SendMoneyDialog(props: SendMoneyDialogProps) {
     outputToConfidential: false,
     inputSelection: "PreferRevealed",
     amount: "",
+    fee: "",
     badge: null,
   };
   const isConfidential = props.resource_type === "Confidential";
   const [useBadge, setUseBadge] = useState(false);
   const [disabled, setDisabled] = useState(false);
-  const [estimatedFee, setEstimatedFee] = useState(0);
   const [transferFormState, setTransferFormState] = useState(INITIAL_VALUES);
   const [validity, setValidity] = useState<object>({
     publicKey: false,
@@ -108,7 +108,8 @@ export function SendMoneyDialog(props: SendMoneyDialogProps) {
     // HACK: default to XTR2 because the resource is only set when open==true, and we cannot conditionally call hooks i.e. when props.resource_address is set
     props.resource_address || XTR2,
     transferFormState.publicKey,
-    estimatedFee,
+    parseInt(transferFormState.fee),
+    // estimatedFee,
     props.resource_type === "Confidential",
     !transferFormState.outputToConfidential,
     transferFormState.inputSelection as ConfidentialTransferInputSelection,
@@ -140,29 +141,26 @@ export function SendMoneyDialog(props: SendMoneyDialogProps) {
         [e.target.name]: e.target.validity.valid,
       });
     }
-    setEstimatedFee(0);
   }
 
   function setSelectFormValue(e: SelectChangeEvent<unknown>) {
     setTransferFormState({
       ...transferFormState,
       [e.target.name]: e.target.value,
     });
-    setEstimatedFee(0);
   }
 
   function setCheckboxFormValue(e: React.ChangeEvent<HTMLInputElement>) {
     setTransferFormState({
       ...transferFormState,
       [e.target.name]: e.target.checked,
     });
-    setEstimatedFee(0);
   }
 
   const onTransfer = async () => {
     if (accountName) {
       setDisabled(true);
-      if (estimatedFee) {
+      if (!isNaN(parseInt(transferFormState.fee))) {
         sendIt?.()
           .then(() => {
             setTransferFormState(INITIAL_VALUES);
@@ -187,7 +185,7 @@ export function SendMoneyDialog(props: SendMoneyDialogProps) {
               });
               return;
             }
-            setEstimatedFee(result.fee);
+            setTransferFormState({ ...transferFormState, fee: result.fee.toString() });
           })
           .catch((e) => {
             setPopup({ title: "Fee estimate failed", error: true, message: e.message });
@@ -303,10 +301,11 @@ export function SendMoneyDialog(props: SendMoneyDialogProps) {
           <TextField
             name="fee"
             label="Fee"
-            value={estimatedFee || "Press fee estimate to calculate"}
-            style={{ flexGrow: 1 }}
+            value={transferFormState.fee}
+            placeholder="Enter fee or press Estimate Fee to calculate"
+            onChange={setFormValue}
             disabled={disabled}
-            InputProps={{ readOnly: true }}
+            style={{ flexGrow: 1 }}
           />
           <Box
             className="flex-container"
@@ -318,7 +317,7 @@ export function SendMoneyDialog(props: SendMoneyDialogProps) {
               Cancel
             </Button>
             <Button variant="contained" type="submit" disabled={disabled || !allValid}>
-              {estimatedFee ? "Send" : "Estimate fee"}
+              {isNaN(parseInt(transferFormState.fee)) ? "Estimate fee" : "Send"}
             </Button>
           </Box>
         </Form>

diff --git a/applications/tari_indexer/src/dry_run/error.rs b/applications/tari_indexer/src/dry_run/error.rs
@@ -22,6 +22,7 @@
 
 use tari_dan_app_utilities::transaction_executor::TransactionProcessorError;
 use tari_dan_common_types::{Epoch, SubstateAddress};
+use tari_dan_engine::state_store::StateStoreError;
 use tari_engine_types::substate::SubstateId;
 use tari_epoch_manager::EpochManagerError;
 use tari_indexer_lib::{error::IndexerError, transaction_autofiller::TransactionAutofillerError};
@@ -53,4 +54,6 @@ pub enum DryRunTransactionProcessorError {
     },
     #[error("Indexer error : {0}")]
     IndexerError(#[from] IndexerError),
+    #[error("StateStore error: {0}")]
+    StateStoreError(#[from] StateStoreError),
 }
diff --git a/applications/tari_indexer/src/dry_run/processor.rs b/applications/tari_indexer/src/dry_run/processor.rs
@@ -115,8 +115,8 @@ where TSubstateCache: SubstateCache + 'static
 
         let virtual_substates = self.get_virtual_substates(&transaction, epoch).await?;
 
-        let mut state_store = new_state_store();
-        state_store.extend(found_substates);
+        let state_store = new_state_store();
+        state_store.set_many(found_substates)?;
 
         // execute the payload in the WASM engine and return the result
         let result = task::block_in_place(|| payload_processor.execute(transaction, state_store, virtual_substates))?;
@@ -155,13 +155,19 @@ where TSubstateCache: SubstateCache + 'static
     ) -> Result<HashMap<SubstateId, Substate>, DryRunTransactionProcessorError> {
         let mut substates = HashMap::new();
 
-        for address in transaction.inputs().iter().chain(transaction.input_refs()) {
+        // Fetch explicit inputs that may not have been resolved by the autofiller
+        for requirement in transaction.inputs() {
+            let Some(address) = requirement.to_substate_address() else {
+                // No version, we cant fetch it
+                continue;
+            };
             // If the input has been filled, we've already fetched the substate
-            if transaction.filled_inputs().contains(address) {
+            // Note: this works because VersionedSubstateId hashes the same as SubstateId internally.
+            if transaction.filled_inputs().contains(&requirement.substate_id) {
                 continue;
             }
 
-            let (id, substate) = self.fetch_substate(address.to_substate_address(), epoch).await?;
+            let (id, substate) = self.fetch_substate(address, epoch).await?;
             substates.insert(id, substate);
         }
 

diff --git a/applications/tari_indexer/src/json_rpc/handlers.rs b/applications/tari_indexer/src/json_rpc/handlers.rs
@@ -545,9 +545,31 @@ impl JsonRpcHandlers {
             }));
         }
 
+        // If there are no requested substates, we skip auto-filling altogether
+        let transaction = if request.required_substates.is_empty() {
+            request.transaction
+        } else {
+            // automatically scan the inputs and add all related involved objects
+            // note that this operation does not alter the transaction hash
+            self.transaction_manager
+                .autofill_transaction(request.transaction, request.required_substates)
+                .await
+                .map_err(|e| match e {
+                    TransactionManagerError::AllValidatorsFailed { .. } => JsonRpcResponse::error(
+                        answer_id,
+                        JsonRpcError::new(
+                            JsonRpcErrorReason::ApplicationError(400),
+                            format!("All validators failed: {}", e),
+                            json::Value::Null,
+                        ),
+                    ),
+                    e => Self::internal_error(answer_id, e),
+                })?
+        };
+
         let transaction_id = self
             .transaction_manager
-            .submit_transaction(request.transaction, request.required_substates)
+            .submit_transaction(transaction)
             .await
             .map_err(|e| match e {
                 TransactionManagerError::AllValidatorsFailed { .. } => JsonRpcResponse::error(

diff --git a/applications/tari_indexer/src/transaction_manager/mod.rs b/applications/tari_indexer/src/transaction_manager/mod.rs
@@ -76,41 +76,42 @@ where
         }
     }
 
-    pub async fn submit_transaction(
-        &self,
-        transaction: Transaction,
-        required_substates: Vec<SubstateRequirement>,
-    ) -> Result<TransactionId, TransactionManagerError> {
+    pub async fn submit_transaction(&self, transaction: Transaction) -> Result<TransactionId, TransactionManagerError> {
         let tx_hash = *transaction.id();
 
         info!(
             target: LOG_TARGET,
             "Submitting transaction with hash {} to the validator node", tx_hash
         );
-        // automatically scan the inputs and add all related involved objects
-        // note that this operation does not alter the transaction hash
-        let (autofilled_transaction, _) = self
-            .transaction_autofiller
-            .autofill_transaction(transaction, required_substates)
-            .await?;
-
         let transaction_substate_address = SubstateAddress::for_transaction_receipt(tx_hash.into_array().into());
 
-        if autofilled_transaction.involved_shards_iter().count() == 0 {
+        if transaction.involved_shards_iter().count() == 0 {
             self.try_with_committee(iter::once(transaction_substate_address), |mut client| {
-                let transaction = autofilled_transaction.clone();
+                let transaction = transaction.clone();
                 async move { client.submit_transaction(transaction).await }
             })
             .await
         } else {
-            self.try_with_committee(autofilled_transaction.involved_shards_iter(), |mut client| {
-                let transaction = autofilled_transaction.clone();
+            self.try_with_committee(transaction.involved_shards_iter(), |mut client| {
+                let transaction = transaction.clone();
                 async move { client.submit_transaction(transaction).await }
             })
             .await
         }
     }
 
+    pub async fn autofill_transaction(
+        &self,
+        transaction: Transaction,
+        required_substates: Vec<SubstateRequirement>,
+    ) -> Result<Transaction, TransactionManagerError> {
+        let (transaction, _) = self
+            .transaction_autofiller
+            .autofill_transaction(transaction, required_substates)
+            .await?;
+        Ok(transaction)
+    }
+
     pub async fn get_transaction_result(
         &self,
         transaction_id: TransactionId,

diff --git a/applications/tari_validator_node/src/bootstrap.rs b/applications/tari_validator_node/src/bootstrap.rs
@@ -93,7 +93,6 @@ use crate::{
                 FeeTransactionValidator,
                 HasInputs,
                 HasInvolvedShards,
-                InputRefsValidator,
                 MempoolError,
                 MempoolHandle,
                 OutputsDontExistLocally,
@@ -536,7 +535,5 @@ fn create_mempool_before_execute_validator(
 fn create_mempool_after_execute_validator<TAddr: NodeAddressable>(
     store: SqliteStateStore<TAddr>,
 ) -> impl Validator<ExecutedTransaction, Error = MempoolError> {
-    HasInvolvedShards::new()
-        .and_then(InputRefsValidator::new())
-        .and_then(OutputsDontExistLocally::new(store))
+    HasInvolvedShards::new().and_then(OutputsDontExistLocally::new(store))
 }