Support for reconcile confirmation against the in-memory canonical chain #174

Chengxuan · 2025-09-10T16:01:49Z

Requires hyperledger/firefly-transaction-manager#148

This PR introduced a new method, ReconcileConfirmationsForTransaction, which can be used to directly calculate the confirmation map of a given transaction.

The diagram below shows the core logic of the function:

Details of the confirmation Map struct can be found in this PR: hyperledger/firefly-transaction-manager#148

Reconciliation logic for an existing confirmations that get passed in

If the transaction is in a block hash that hasn't been recorded, that new block hash will be added to the map and a new confirmation queue will be built and stored under that block hash (map key)
If the transaction is in an existing block hash that's been tracked
- If any of the existing confirmation block numbers are connectable to any of the block numbers in the current in-memory canonical chain
  - if all of them are chainable (all on the same fork or partially on the same fork), the confirmations on the same fork will be carried over into the new confirmation map, the newFork flag will be set to true in the response accordingly
- otherwise they are discarded

Confirmation queue for different scenarios:

In all cases, the first item in the confirmation queue is the tx block (the block that contains the tx).
In the following scenario, a tx confirmation queue will have a full set of MinimalBlockInfo as the confirmation history
- all confirmation blocks are in the current in-memory canonical chain
- existing confirmation queue (provided by the consumer) shares a fork with the current in-memory canonical chain, all confirmation blocks, and:
  - all confirmation blocks are in the existing queue
  - or all confirmation blocks are in the existing queue + the in-memory canonical chain
- In the following scenario, a tx confirmation queue will have partial confirmation blocks:
  - no existing confirmations provided, some confirmation blocks are ahead of the first item in the in-memory canonical chain.
  - existing confirmation queue shares a fork with current in-memory canonical chain, but it only has partial confirmation blocks in the existing confirmation queue.
- In the scenario of the target confirmation block number of a transaction is ahead of the in-memory canonical chain or the combined existing confirmation + in-memory canonical chain blocks, the confirmation queue will only have 2 items: the tx block and the head block of the in-memory canonical chain.

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

internal/ethereum/confirmation_reconciler.go

internal/ethereum/blocklistener_blockquery.go

Co-authored-by: alexey semenyuk <alexsemenyuk88@gmail.com> Signed-off-by: Chengxuan Xing <chengxuan.xing@kaleido.io>

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

EnriqueL8

Thanks, looks really good - a few questions and suggestions but thanks for all the comments in the flow

EnriqueL8 · 2025-09-15T11:15:37Z

internal/ethereum/blocklistener_blockquery.go

+	// 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


Are you going to do this figuring out as part of the PR? I did wonder why the TX hash -> block was not a thing

EnriqueL8 · 2025-09-15T11:17:18Z

internal/ethereum/confirmation_reconciler.go

+	}
+
+	// Compare the existing confirmation queue with the in-memory linked list
+	bl.compareAndUpdateConfirmationQueue(ctx, occ, txBlockInfo, targetConfirmationCount)


EnriqueL8 · 2025-09-15T11:22:49Z

internal/ethereum/confirmation_reconciler.go

+
+	chainHead := bl.canonicalChain.Front().Value.(*ffcapi.MinimalBlockInfo)
+	chainTail := bl.canonicalChain.Back().Value.(*ffcapi.MinimalBlockInfo)
+	if chainHead == nil || chainTail == nil || chainTail.BlockNumber.Uint64() < txBlockNumber {


So we check that the canonical chain's tail (oldest block) is smaller than the TX block number - that could happen and still be correct to carry on no? or is the tail the newest block?

or did you mean to check the chainHead?

EnriqueL8 · 2025-09-15T11:23:53Z

internal/ethereum/confirmation_reconciler.go

+	}
+}
+
+func (bl *blockListener) initializeConfirmationMap(occ *ffcapi.ConfirmationMapUpdateResult, txBlockInfo *ffcapi.MinimalBlockInfo) []*ffcapi.MinimalBlockInfo {


what is this occ naming format? maybe confMapResult to keep with txBlockInfo

ah it's output context, but it's not a like a OutputContext object that is why the confusion

EnriqueL8 · 2025-09-15T11:25:58Z

internal/ethereum/confirmation_reconciler.go

+	return existingQueue
+}
+
+func (bl *blockListener) processExistingConfirmations(ctx context.Context, occ *ffcapi.ConfirmationMapUpdateResult, txBlockInfo *ffcapi.MinimalBlockInfo, existingQueue []*ffcapi.MinimalBlockInfo, chainHead *ffcapi.MinimalBlockInfo, targetConfirmationCount uint64) ([]*ffcapi.MinimalBlockInfo, *list.Element) {


just a nit so up to you but maybe to reduce number of params as you have done in the past having a context object with all the things we need

EnriqueL8 · 2025-09-15T11:40:31Z

internal/ethereum/confirmation_reconciler.go

+			}
+
+			// If we've trimmed off all the existing confirmations, we need to add the canonical chain head
+			// to tell use the head block we used to confirmed the transaction


Suggested change

// to tell use the head block we used to confirmed the transaction

// to tell us the head block we used to confirm the transaction

EnriqueL8 · 2025-09-15T11:40:48Z

internal/ethereum/confirmation_reconciler.go

+	confirmationBlockNumber := txBlockNumber + targetConfirmationCount
+	if lastBlockInNewQueue.BlockNumber.Uint64() >= confirmationBlockNumber {
+		chainHead := bl.canonicalChain.Front().Value.(*ffcapi.MinimalBlockInfo)
+		// we've got a confirmable so whether the rest of the chain has forked is not longer relevant


Suggested change

// we've got a confirmable so whether the rest of the chain has forked is not longer relevant

// we've got a confirmation so whether the rest of the chain has forked is no longer relevant

EnriqueL8 · 2025-09-15T11:41:03Z

internal/ethereum/confirmation_reconciler.go

+	)
+}
+
+func (bl *blockListener) validateExistingConfirmations(ctx context.Context, occ *ffcapi.ConfirmationMapUpdateResult, newQueue []*ffcapi.MinimalBlockInfo, existingConfirmations []*ffcapi.MinimalBlockInfo, currentBlock *list.Element, chainHead *ffcapi.MinimalBlockInfo, txBlockInfo *ffcapi.MinimalBlockInfo, targetConfirmationCount uint64) ([]*ffcapi.MinimalBlockInfo, *list.Element) {


This function was quite difficult to follow FYI

EnriqueL8 · 2025-09-15T11:44:13Z

internal/ethereum/confirmation_reconciler.go

+	existingQueue := bl.initializeConfirmationMap(occ, txBlockInfo)
+
+	// Validate and process existing confirmations
+	newQueue, currentBlock := bl.processExistingConfirmations(ctx, occ, txBlockInfo, existingQueue, chainHead, targetConfirmationCount)


I think this name of currentBlock is quite confusing... I get the name cause it can be the front of the chain, the block at which the confirmation reached the limit...

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

Chengxuan added 4 commits September 10, 2025 01:04

adding function to calculate comfirmation map state

4e82ee6

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

refactor confirmation map reconcile logic

2a64479

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

fixign test and refactor

8651d54

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

adding more tests and fixing bugs

4ccb06d

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

Chengxuan mentioned this pull request Sep 11, 2025

adding a new function for reconciling confirmations map in the connector interface hyperledger/firefly-transaction-manager#148

Open

Chengxuan marked this pull request as ready for review September 11, 2025 11:42

Chengxuan requested a review from a team as a code owner September 11, 2025 11:42

alex-semenyuk reviewed Sep 11, 2025

View reviewed changes

internal/ethereum/confirmation_reconciler.go Outdated Show resolved Hide resolved

alex-semenyuk reviewed Sep 11, 2025

View reviewed changes

internal/ethereum/blocklistener_blockquery.go Show resolved Hide resolved

alex-semenyuk reviewed Sep 11, 2025

View reviewed changes

internal/ethereum/blocklistener_blockquery.go Outdated Show resolved Hide resolved

Chengxuan and others added 4 commits September 11, 2025 19:21

Apply suggestions from code review

ea0f58d

Co-authored-by: alexey semenyuk <alexsemenyuk88@gmail.com> Signed-off-by: Chengxuan Xing <chengxuan.xing@kaleido.io>

fixing get_blockInfo not found logic.

0a08612

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

fixing formating

578db35

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

don't treat brand new confirmation as a new fork

5b9f1c8

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

EnriqueL8 reviewed Sep 15, 2025

View reviewed changes

fixing 0 confirmations behaviour

b16e6a8

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Support for reconcile confirmation against the in-memory canonical chain #174

Support for reconcile confirmation against the in-memory canonical chain #174

Uh oh!

Chengxuan commented Sep 10, 2025 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

EnriqueL8 left a comment

Uh oh!

EnriqueL8 Sep 15, 2025

Uh oh!

EnriqueL8 Sep 15, 2025

Uh oh!

EnriqueL8 Sep 15, 2025

Uh oh!

EnriqueL8 Sep 15, 2025

Uh oh!

EnriqueL8 Sep 15, 2025

Uh oh!

EnriqueL8 Sep 15, 2025

Uh oh!

EnriqueL8 Sep 15, 2025

Uh oh!

EnriqueL8 Sep 15, 2025

Uh oh!

EnriqueL8 Sep 15, 2025

Uh oh!

EnriqueL8 Sep 15, 2025

Uh oh!

EnriqueL8 Sep 15, 2025

Uh oh!

Uh oh!

	// to tell use the head block we used to confirmed the transaction
	// to tell us the head block we used to confirm the transaction

	// we've got a confirmable so whether the rest of the chain has forked is not longer relevant
	// we've got a confirmation so whether the rest of the chain has forked is no longer relevant

Support for reconcile confirmation against the in-memory canonical chain #174

Are you sure you want to change the base?

Support for reconcile confirmation against the in-memory canonical chain #174

Uh oh!

Conversation

Chengxuan commented Sep 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reconciliation logic for an existing confirmations that get passed in

Confirmation queue for different scenarios:

Uh oh!

Uh oh!

Uh oh!

Uh oh!

EnriqueL8 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Chengxuan commented Sep 10, 2025 •

edited

Loading