tidy up

Signed-off-by: Chengxuan Xing <chengxuan.xing@kaleido.io>

Author: Chengxuan

internal/ethereum/blocklistener_blockquery.go

@@ -56,12 +56,6 @@ func (bl *blockListener) addToBlockCache(blockInfo *blockInfoJSONRPC) {
 func (bl *blockListener) getBlockInfoContainsTxHash(ctx context.Context, txHash string) (*ffcapi.MinimalBlockInfo, error) {
 
 	// Query the chain to find the transaction block
-	// Note: should consider have an in-memory map of transaction hash to block for faster lookup
-	// The extra memory usage of the map should be outweighed by the speed improvement of lookup
-	// But I saw we have a ffcapi.MinimalBlockInfo struct that intentionally removes the tx hashes
-	// so need to figure out the reason first
-
-	// TODO: add a cache if map cannot be used
 	res, reason, receiptErr := bl.c.TransactionReceipt(ctx, &ffcapi.TransactionReceiptRequest{
 		TransactionHash: txHash,
 	})

internal/ethereum/confirmation_reconciler.go

@@ -14,6 +14,11 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+// The confirmation reconciler manages transaction confirmation queues by:
+// - Copying blocks from the canonical chain and the existing confirmation queue
+// - Detecting blockchain forks and rebuilding confirmation queues when necessary
+// - Filling gaps in confirmation queues by fetching missing blocks
+// - Determining when transactions have reached the target confirmation count
 package ethereum
 
 import (
@@ -26,15 +31,17 @@ import (
 	"github.com/hyperledger/firefly-transaction-manager/pkg/ffcapi"
 )
 
+// reconcileConfirmationsForTransaction reconciles the confirmation queue for a transaction
 func (bl *blockListener) reconcileConfirmationsForTransaction(ctx context.Context, txHash string, existingConfirmations []*ffcapi.MinimalBlockInfo, targetConfirmationCount uint64) (*ffcapi.ConfirmationMapUpdateResult, error) {
-	// Initialize the output context
+	// Initialize the result with existing confirmations
 	reconcileResult := &ffcapi.ConfirmationMapUpdateResult{
 		Confirmations:           existingConfirmations,
 		NewFork:                 false,
 		Confirmed:               false,
 		TargetConfirmationCount: targetConfirmationCount,
 	}
 
+	// Fetch the block containing the transaction
 	txBlockInfo, err := bl.getBlockInfoContainsTxHash(ctx, txHash)
 	if err != nil {
 		log.L(ctx).Errorf("Failed to fetch block info using tx hash %s: %v", txHash, err)
@@ -49,16 +56,17 @@ func (bl *blockListener) reconcileConfirmationsForTransaction(ctx context.Contex
 	return bl.compareAndUpdateConfirmationQueue(ctx, reconcileResult, txBlockInfo, targetConfirmationCount)
 }
 
+// compareAndUpdateConfirmationQueue orchestrates the confirmation reconciliation process.
+// It builds new confirmations from the canonical chain and fills gaps in the confirmation queue.
 func (bl *blockListener) compareAndUpdateConfirmationQueue(ctx context.Context, reconcileResult *ffcapi.ConfirmationMapUpdateResult, txBlockInfo *ffcapi.MinimalBlockInfo, targetConfirmationCount uint64) (*ffcapi.ConfirmationMapUpdateResult, error) {
 	var err error
-	// Compare the and build the tail part of the confirmation queue using the canonical chain
+	// Build new confirmations from the canonical chain and get existing confirmations
 	newConfirmationsWithoutTxBlock, existingConfirmations, returnResult := bl.buildConfirmationQueueUsingCanonicalChain(ctx, reconcileResult, txBlockInfo, targetConfirmationCount)
 	if returnResult {
 		return reconcileResult, nil
 	}
 
-	// Validate and process existing confirmations
-	// and fill in the gap in the confirmation queue
+	// Validate existing confirmations and fill gaps in the confirmation queue
 	var confirmations []*ffcapi.MinimalBlockInfo
 	var newFork bool
 	confirmations, newFork, err = bl.checkAndFillInGap(ctx, newConfirmationsWithoutTxBlock, existingConfirmations, txBlockInfo, targetConfirmationCount)
@@ -70,47 +78,52 @@ func (bl *blockListener) compareAndUpdateConfirmationQueue(ctx context.Context,
 	return reconcileResult, err
 }
 
-// NOTE: this function only build up the confirmation queue uses the in-memory canonical chain
-// it does not build up the canonical chain
-// compareAndUpdateConfirmationQueueUsingCanonicalChain compares the existing confirmation queue with the in-memory linked list
-// this function obtains the read lock on the canonical chain, so it should not make any long-running queries
-
+// buildConfirmationQueueUsingCanonicalChain builds the confirmation queue using the in-memory canonical chain.
+// It does not modify the canonical chain itself, only reads from it.
+// This function holds a read lock on the canonical chain, so it should not make long-running queries.
 func (bl *blockListener) buildConfirmationQueueUsingCanonicalChain(ctx context.Context, reconcileResult *ffcapi.ConfirmationMapUpdateResult, txBlockInfo *ffcapi.MinimalBlockInfo, targetConfirmationCount uint64) (newConfirmationsWithoutTxBlock []*ffcapi.MinimalBlockInfo, existingConfirmations []*ffcapi.MinimalBlockInfo, returnResult bool) {
 	bl.mux.RLock()
 	defer bl.mux.RUnlock()
 	txBlockNumber := txBlockInfo.BlockNumber.Uint64()
 	targetBlockNumber := txBlockInfo.BlockNumber.Uint64() + targetConfirmationCount
 
+	// Check if the canonical chain has caught up to the transaction block
 	chainTail := bl.canonicalChain.Back().Value.(*ffcapi.MinimalBlockInfo)
 	if chainTail == nil || chainTail.BlockNumber.Uint64() < txBlockNumber {
 		log.L(ctx).Debugf("Canonical chain is waiting for the transaction block %d to be indexed", txBlockNumber)
 		return nil, nil, true
 	}
 
-	// Initialize confirmation map and get existing queue
+	// Initialize confirmation map and get existing confirmations
 	existingConfirmations = bl.initializeConfirmationMap(reconcileResult, txBlockInfo)
 
-	// if the target confirmation count is 0, we should just return the transaction block
+	// Special case: if targetConfirmationCount is 0, transaction is immediately confirmed
 	if targetConfirmationCount == 0 {
 		reconcileResult.Confirmed = true
 		reconcileResult.Confirmations = []*ffcapi.MinimalBlockInfo{txBlockInfo}
 		return nil, existingConfirmations, true
 	}
 
-	// build the tail part of the queue from the canonical chain
+	// Build new confirmations from blocks after the transaction block
 
 	newConfirmationsWithoutTxBlock = []*ffcapi.MinimalBlockInfo{}
 	currentBlock := bl.canonicalChain.Front()
 	for currentBlock != nil {
 		currentBlockInfo := currentBlock.Value.(*ffcapi.MinimalBlockInfo)
+
+		// If we've reached the target confirmation count, mark as confirmed
 		if currentBlockInfo.BlockNumber.Uint64() > targetBlockNumber {
 			reconcileResult.Confirmed = true
 			break
 		}
+
+		// Skip blocks at or before the transaction block
 		if currentBlockInfo.BlockNumber.Uint64() <= txBlockNumber {
 			currentBlock = currentBlock.Next()
 			continue
 		}
+
+		// Add blocks after the transaction block to confirmations
 		newConfirmationsWithoutTxBlock = append(newConfirmationsWithoutTxBlock, &ffcapi.MinimalBlockInfo{
 			BlockHash:   currentBlockInfo.BlockHash,
 			BlockNumber: fftypes.FFuint64(currentBlockInfo.BlockNumber.Uint64()),
@@ -121,18 +134,22 @@ func (bl *blockListener) buildConfirmationQueueUsingCanonicalChain(ctx context.C
 	return newConfirmationsWithoutTxBlock, existingConfirmations, false
 }
 
+// initializeConfirmationMap initializes the confirmation map with the transaction block
+// and validates existing confirmations against the current transaction block.
+// Returns existing confirmations if valid, or nil if a fork is detected.
 func (bl *blockListener) initializeConfirmationMap(reconcileResult *ffcapi.ConfirmationMapUpdateResult, txBlockInfo *ffcapi.MinimalBlockInfo) []*ffcapi.MinimalBlockInfo {
+	// If no existing confirmations, initialize with the transaction block
 	if len(reconcileResult.Confirmations) == 0 {
 		reconcileResult.Confirmations = []*ffcapi.MinimalBlockInfo{txBlockInfo}
 		return nil
 	}
 
+	// Validate existing confirmations against the current transaction block
 	existingQueue := reconcileResult.Confirmations
 	if len(existingQueue) > 0 {
 		existingTxBlock := existingQueue[0]
 		if !existingTxBlock.Equal(txBlockInfo) {
-			// the tx block in the existing queue does not match the new tx block we queried from the chain
-			// rebuild a new confirmation queue with the new tx block
+			// Transaction block mismatch indicates a fork - rebuild confirmation queue
 			reconcileResult.NewFork = true
 			reconcileResult.Confirmations = []*ffcapi.MinimalBlockInfo{txBlockInfo}
 			return nil
@@ -142,9 +159,13 @@ func (bl *blockListener) initializeConfirmationMap(reconcileResult *ffcapi.Confi
 	return existingQueue
 }
 
+// checkAndFillInGap validates existing confirmations, detects forks, and fills gaps
+// in the confirmation queue using existing confirmations or fetching missing blocks from the blockchain.
+// It ensures the confirmation chain is valid and connected to the transaction block.
 func (bl *blockListener) checkAndFillInGap(ctx context.Context, newConfirmationsWithoutTxBlock []*ffcapi.MinimalBlockInfo, existingConfirmations []*ffcapi.MinimalBlockInfo, txBlockInfo *ffcapi.MinimalBlockInfo, targetConfirmationCount uint64) ([]*ffcapi.MinimalBlockInfo, bool, error) {
 	var hasNewFork bool
-	// check whether there are forks in the newConfirmations
+
+	// Detect forks by comparing new confirmations with existing ones
 	for _, confirmation := range newConfirmationsWithoutTxBlock {
 		for _, existingConfirmation := range existingConfirmations {
 			if confirmation.BlockNumber.Uint64() == existingConfirmation.BlockNumber.Uint64() && !confirmation.Equal(existingConfirmation) {
@@ -157,59 +178,75 @@ func (bl *blockListener) checkAndFillInGap(ctx context.Context, newConfirmations
 		}
 	}
 
+	// Determine the range of blocks to validate and fill gaps
 	blockNumberToReach := txBlockInfo.BlockNumber.Uint64() + targetConfirmationCount
 	var lastValidatedBlock *ffcapi.MinimalBlockInfo
 	if len(newConfirmationsWithoutTxBlock) > 0 {
+		// Start from the block before the first new confirmation
 		blockNumberToReach = newConfirmationsWithoutTxBlock[0].BlockNumber.Uint64() - 1
 		lastValidatedBlock = newConfirmationsWithoutTxBlock[0]
 	}
 
+	// Fill gaps by validating blocks from target down to transaction block
 	for i := blockNumberToReach; i > txBlockInfo.BlockNumber.Uint64(); i-- {
-		// first use the block info from the confirmation queue if matches are found
 		fetchedFromExistingQueue := false
-		if lastValidatedBlock != nil {
 
+		// First, try to use existing confirmations if they match
+		if lastValidatedBlock != nil {
 			for _, confirmation := range existingConfirmations {
 				if confirmation.BlockNumber.Uint64() == i {
 					if confirmation.IsParentOf(lastValidatedBlock) {
+						// Valid existing confirmation - prepend to queue
 						newConfirmationsWithoutTxBlock = append([]*ffcapi.MinimalBlockInfo{confirmation}, newConfirmationsWithoutTxBlock...)
 						lastValidatedBlock = confirmation
 						fetchedFromExistingQueue = true
 						break
 					}
+					// Block number matches but parent relationship is invalid - fork detected
 					hasNewFork = true
 				}
 			}
 		}
+
 		if fetchedFromExistingQueue {
 			continue
 		}
-		// if no match is found, fetch the block info from the chain
+
+		// Fetch block from blockchain if not found in existing confirmations
 		freshBlockInfo, _, err := bl.getBlockInfoByNumber(ctx, i, false, "", "")
 		if err != nil {
 			return nil, hasNewFork, err
 		}
+		if freshBlockInfo == nil {
+			return nil, hasNewFork, i18n.NewError(ctx, msgs.MsgBlockNotAvailable)
+		}
+
 		fetchedBlock := &ffcapi.MinimalBlockInfo{
 			BlockNumber: fftypes.FFuint64(freshBlockInfo.Number.BigInt().Uint64()),
 			BlockHash:   freshBlockInfo.Hash.String(),
 			ParentHash:  freshBlockInfo.ParentHash.String(),
 		}
+
+		// Validate parent-child relationship
 		if lastValidatedBlock != nil && !fetchedBlock.IsParentOf(lastValidatedBlock) {
-			// the fetched block is not the parent of the last validated block
-			// chain is not in a stable stable to build the confirmation queue
 			return nil, hasNewFork, i18n.NewError(ctx, msgs.MsgFailedToBuildConfirmationQueue)
 		}
+
+		// Prepend fetched block to confirmation queue
 		newConfirmationsWithoutTxBlock = append([]*ffcapi.MinimalBlockInfo{fetchedBlock}, newConfirmationsWithoutTxBlock...)
 		lastValidatedBlock = fetchedBlock
 	}
 
-	// we've rebuilt the confirmations queue, now check the front of the queue still connect to the tx block
-	if !txBlockInfo.IsParentOf(newConfirmationsWithoutTxBlock[0]) {
+	// Final validation: ensure the confirmation chain connects to the transaction block
+	if len(newConfirmationsWithoutTxBlock) > 0 && !txBlockInfo.IsParentOf(newConfirmationsWithoutTxBlock[0]) {
 		return nil, hasNewFork, i18n.NewError(ctx, msgs.MsgFailedToBuildConfirmationQueue)
 	}
+
 	return append([]*ffcapi.MinimalBlockInfo{txBlockInfo}, newConfirmationsWithoutTxBlock...), hasNewFork, nil
 }
 
+// ReconcileConfirmationsForTransaction is the public API for reconciling transaction confirmations.
+// It delegates to the blockListener's internal reconciliation logic.
 func (c *ethConnector) ReconcileConfirmationsForTransaction(ctx context.Context, txHash string, existingConfirmations []*ffcapi.MinimalBlockInfo, targetConfirmationCount uint64) (*ffcapi.ConfirmationMapUpdateResult, error) {
 	return c.blockListener.reconcileConfirmationsForTransaction(ctx, txHash, existingConfirmations, targetConfirmationCount)
 }