Skip to content

load unmined tx defensive checks - bug fix #183

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Nov 26, 2025
Merged

load unmined tx defensive checks - bug fix #183

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
716a279
load unmined tx defensive checks - bug fix - don't shadow i variable.…
freemans13 Nov 18, 2025
8107cf5
new setting blockassembly_filterTransactionsWithInvalidParents
freemans13 Nov 25, 2025
43087c0
Merge branch 'main' of https://github.com/bsv-blockchain/teranode int…
freemans13 Nov 25, 2025
d812896
linting
freemans13 Nov 25, 2025
6f66bb9
rename ValidateParentChainOnRestart -> OnRestartValidateParentChain
freemans13 Nov 26, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 43 additions & 25 deletions services/blockassembly/BlockAssembler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1791,18 +1791,18 @@ func (b *BlockAssembler) getNextNbits(nextBlockTime int64) (*model.NBit, error)
return nbit, nil
}

// filterTransactionsWithValidParents filters unmined transactions to ensure their parent transactions
// are either on the best chain or also unmined (to be processed together).
// validateParentChain validates that unmined transactions have their parent transactions
// either on the best chain or also unmined (to be processed together).
//
// Parameters:
// - ctx: Context for cancellation
// - unminedTxs: List of unmined transactions to filter
// - unminedTxs: List of unmined transactions to validate
// - bestBlockHeaderIDsMap: Map of block IDs on the best chain
//
// Returns:
// - []*utxo.UnminedTransaction: Filtered list of transactions with valid parents
// - []*utxo.UnminedTransaction: List of transactions (filtered if OnRestartRemoveInvalidParentChainTxs is enabled)
// - error: Context cancellation error if cancelled, nil otherwise
func (b *BlockAssembler) filterTransactionsWithValidParents(
func (b *BlockAssembler) validateParentChain(
ctx context.Context,
unminedTxs []*utxo.UnminedTransaction,
bestBlockHeaderIDsMap map[uint32]bool,
Expand All @@ -1812,7 +1812,7 @@ func (b *BlockAssembler) filterTransactionsWithValidParents(
return unminedTxs, nil
}

b.logger.Infof("[BlockAssembler][filterTransactionsWithValidParents] Starting parent chain validation for %d unmined transactions", len(unminedTxs))
b.logger.Infof("[BlockAssembler][validateParentChain] Starting parent chain validation for %d unmined transactions", len(unminedTxs))

// OPTIMIZATION: Two-pass approach to minimize memory usage
// Pass 1: Collect only the parent hashes that are actually referenced
Expand All @@ -1824,7 +1824,7 @@ func (b *BlockAssembler) filterTransactionsWithValidParents(
referencedParents[parentHash] = true
}
}
b.logger.Debugf("[BlockAssembler][filterTransactionsWithValidParents] Found %d unique parent references out of %d transactions",
b.logger.Debugf("[BlockAssembler][validateParentChain] Found %d unique parent references out of %d transactions",
len(referencedParents), len(unminedTxs))

// Pass 2: Build index ONLY for transactions that are referenced as parents
Expand All @@ -1845,7 +1845,7 @@ func (b *BlockAssembler) filterTransactionsWithValidParents(
// Check for context cancellation at start of each batch
select {
case <-ctx.Done():
b.logger.Infof("[BlockAssembler][filterTransactionsWithValidParents] Parent validation cancelled during batch processing at index %d", i)
b.logger.Infof("[BlockAssembler][validateParentChain] Parent validation cancelled during batch processing at index %d", i)
return nil, ctx.Err()
default:
}
Expand Down Expand Up @@ -1877,10 +1877,10 @@ func (b *BlockAssembler) filterTransactionsWithValidParents(

// Create UnresolvedMetaData slice for batch operation
unresolvedParents := make([]*utxo.UnresolvedMetaData, 0, len(parentTxIDs))
for i, parentTxID := range parentTxIDs {
for parentIdx, parentTxID := range parentTxIDs {
unresolvedParents = append(unresolvedParents, &utxo.UnresolvedMetaData{
Hash: parentTxID,
Idx: i,
Idx: parentIdx,
})
}

Expand All @@ -1890,14 +1890,14 @@ func (b *BlockAssembler) filterTransactionsWithValidParents(
fields.BlockIDs, fields.UnminedSince, fields.Locked)
if err != nil {
// Log the batch error but continue - individual errors are in UnresolvedMetaData
b.logger.Warnf("[BlockAssembler][filterTransactionsWithValidParents] BatchDecorate error (will check individual results): %v", err)
b.logger.Warnf("[BlockAssembler][validateParentChain] BatchDecorate error (will check individual results): %v", err)
}

// Process results - check each parent's fetch result
for _, unresolved := range unresolvedParents {
if unresolved.Err != nil {
// Parent doesn't exist or error retrieving it
b.logger.Debugf("[BlockAssembler][filterTransactionsWithValidParents] Failed to get parent tx %s metadata: %v",
b.logger.Errorf("[BlockAssembler][validateParentChain] Failed to get parent tx %s metadata: %v",
unresolved.Hash.String(), unresolved.Err)
continue
}
Expand All @@ -1921,11 +1921,15 @@ func (b *BlockAssembler) filterTransactionsWithValidParents(
}
}
if onBestChain {
// Transaction is already on the best chain - skip it
// Transaction is already on the best chain
// (though it shouldn't be in unmined list - this is a data inconsistency)
b.logger.Warnf("[BlockAssembler][filterTransactionsWithValidParents] Transaction %s is already on best chain but marked as unmined - skipping it", tx.Hash.String())
skippedCount++
continue
b.logger.Warnf("[BlockAssembler][validateParentChain] Transaction %s is already on best chain but marked as unmined", tx.Hash.String())
if b.settings.BlockAssembly.OnRestartRemoveInvalidParentChainTxs {
// Filtering enabled - skip this transaction
skippedCount++
continue
}
// Filtering disabled - keep transaction despite being on best chain
}
// Transaction has BlockIDs but not on best chain - it's on an orphaned chain
// Continue to validate its parents to decide if it can be re-included
Expand All @@ -1946,6 +1950,7 @@ func (b *BlockAssembler) filterTransactionsWithValidParents(
// This means BatchDecorate couldn't find it - it doesn't exist
allParentsValid = false
invalidReason = fmt.Sprintf("parent tx %s not found in UTXO store", parentTxID.String())
b.logger.Warnf("[BlockAssembler][validateParentChain] Transaction %s has invalid parent: %s", tx.Hash.String(), invalidReason)
break
}

Expand All @@ -1962,6 +1967,7 @@ func (b *BlockAssembler) filterTransactionsWithValidParents(
allParentsValid = false
invalidReason = fmt.Sprintf("parent tx %s is on wrong chain (blocks: %v)",
parentTxID.String(), parentMeta.BlockIDs)
b.logger.Warnf("[BlockAssembler][validateParentChain] Transaction %s has invalid parent: %s", tx.Hash.String(), invalidReason)
break
}
} else {
Expand All @@ -1974,6 +1980,7 @@ func (b *BlockAssembler) filterTransactionsWithValidParents(
// Unmined but not in our list - this is a problem
allParentsValid = false
invalidReason = fmt.Sprintf("parent tx %s is unmined but not in processing list", parentTxID.String())
b.logger.Warnf("[BlockAssembler][validateParentChain] Transaction %s has invalid parent: %s", tx.Hash.String(), invalidReason)
break
}
}
Expand All @@ -1992,7 +1999,7 @@ func (b *BlockAssembler) filterTransactionsWithValidParents(
if !parentExists {
// Parent not in index map - this means it's not in the unmined list
// This shouldn't happen as we just checked it was referenced
b.logger.Errorf("[BlockAssembler][filterTransactionsWithValidParents] Parent tx %s not found in index map", parentTxID.String())
b.logger.Errorf("[BlockAssembler][validateParentChain] Parent tx %s not found in index map", parentTxID.String())
hasInvalidOrdering = true
break
}
Expand All @@ -2003,7 +2010,7 @@ func (b *BlockAssembler) filterTransactionsWithValidParents(
hasInvalidOrdering = true
invalidReason = fmt.Sprintf("parent tx %s (index %d) comes after child tx %s (index %d)",
parentTxID.String(), parentIdx, tx.Hash.String(), currentIdx)
b.logger.Debugf("[BlockAssembler][filterTransactionsWithValidParents] Skipping tx %s: %s", tx.Hash.String(), invalidReason)
b.logger.Warnf("[BlockAssembler][validateParentChain] Skipping tx %s: %s", tx.Hash.String(), invalidReason)
break
}
}
Expand All @@ -2020,18 +2027,29 @@ func (b *BlockAssembler) filterTransactionsWithValidParents(
if allParentsValid {
validTxs = append(validTxs, tx)
} else {
skippedCount++
b.logger.Debugf("[BlockAssembler][filterTransactionsWithValidParents] Skipping tx %s: %s", tx.Hash.String(), invalidReason)
// Transaction has invalid parent chain - use setting to decide whether to exclude
if b.settings.BlockAssembly.OnRestartRemoveInvalidParentChainTxs {
// Filtering enabled - skip this transaction
skippedCount++
} else {
// Filtering disabled (default) - keep transaction despite invalid parents
validTxs = append(validTxs, tx)
}
}
}
}

filteringStatus := "disabled"
if b.settings.BlockAssembly.OnRestartRemoveInvalidParentChainTxs {
filteringStatus = "enabled"
}

if skippedCount > 0 {
b.logger.Warnf("[BlockAssembler][filterTransactionsWithValidParents] Skipped %d transactions due to invalid/missing parent chains", skippedCount)
b.logger.Warnf("[BlockAssembler][validateParentChain] Skipped %d transactions due to invalid/missing parent chains (filtering: %s)", skippedCount, filteringStatus)
}

b.logger.Infof("[BlockAssembler][filterTransactionsWithValidParents] Parent chain validation complete: %d valid, %d skipped",
len(validTxs), skippedCount)
b.logger.Infof("[BlockAssembler][validateParentChain] Parent chain validation complete: %d valid, %d skipped (filtering: %s)",
len(validTxs), skippedCount, filteringStatus)

return validTxs, nil
}
Expand Down Expand Up @@ -2207,9 +2225,9 @@ func (b *BlockAssembler) loadUnminedTransactions(ctx context.Context, fullScan b
})

// Apply parent chain validation if enabled
if b.settings.BlockAssembly.ValidateParentChainOnRestart {
if b.settings.BlockAssembly.OnRestartValidateParentChain {
var err error
unminedTransactions, err = b.filterTransactionsWithValidParents(ctx, unminedTransactions, bestBlockHeaderIDsMap)
unminedTransactions, err = b.validateParentChain(ctx, unminedTransactions, bestBlockHeaderIDsMap)
if err != nil {
// Context was cancelled during parent validation
return err
Expand Down
6 changes: 3 additions & 3 deletions services/blockassembly/BlockAssembler_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2219,7 +2219,7 @@ func TestBlockAssembly_LoadUnminedTransactions_ReseedsMinedTx_WhenUnminedSinceNo
require.NotNil(t, items)

// Disable parent validation for this test as it tests edge cases with UTXO store states
items.blockAssembler.settings.BlockAssembly.ValidateParentChainOnRestart = false
items.blockAssembler.settings.BlockAssembly.OnRestartValidateParentChain = false

// Create a test tx and insert into UTXO store as unmined initially (unmined_since set)
tx := newTx(42)
Expand Down Expand Up @@ -2265,7 +2265,7 @@ func TestBlockAssembly_LoadUnminedTransactions_ReorgCornerCase_MisUnsetMinedStat
require.NotNil(t, items)

// Disable parent validation for this test as it tests edge cases with UTXO store states
items.blockAssembler.settings.BlockAssembly.ValidateParentChainOnRestart = false
items.blockAssembler.settings.BlockAssembly.OnRestartValidateParentChain = false

// Prepare a mined tx on the main chain
tx := newTx(43)
Expand Down Expand Up @@ -2314,7 +2314,7 @@ func TestBlockAssembly_LoadUnminedTransactions_SkipsTransactionsOnCurrentChain(t
require.NotNil(t, items)

// Disable parent validation for this test as it tests transaction filtering logic independently
items.blockAssembler.settings.BlockAssembly.ValidateParentChainOnRestart = false
items.blockAssembler.settings.BlockAssembly.OnRestartValidateParentChain = false

// Create two test transactions
tx1 := newTx(100)
Expand Down
Loading