Skip to content

Commit e0ed0c0

Browse files
committed
address review comment, tolerate invalid input
Signed-off-by: Chengxuan Xing <[email protected]>
1 parent 4dd1b1c commit e0ed0c0

File tree

4 files changed

+141
-180
lines changed

4 files changed

+141
-180
lines changed

go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ require (
77
github.com/hashicorp/golang-lru v1.0.2
88
github.com/hyperledger/firefly-common v1.5.6-0.20250630201730-e234335c0381
99
github.com/hyperledger/firefly-signer v1.1.21
10-
github.com/hyperledger/firefly-transaction-manager v1.4.1-0.20251017153149-d6f944cab6b6
10+
github.com/hyperledger/firefly-transaction-manager v1.4.1-0.20251024174118-8e8e3629f7fa
1111
github.com/sirupsen/logrus v1.9.3
1212
github.com/spf13/cobra v1.8.0
1313
github.com/stretchr/testify v1.9.0

go.sum

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -104,10 +104,8 @@ github.com/hyperledger/firefly-common v1.5.6-0.20250630201730-e234335c0381 h1:4m
104104
github.com/hyperledger/firefly-common v1.5.6-0.20250630201730-e234335c0381/go.mod h1:1Xawm5PUhxT7k+CL/Kr3i1LE3cTTzoQwZMLimvlW8rs=
105105
github.com/hyperledger/firefly-signer v1.1.21 h1:r7cTOw6e/6AtiXLf84wZy6Z7zppzlc191HokW2hv4N4=
106106
github.com/hyperledger/firefly-signer v1.1.21/go.mod h1:axrlSQeKrd124UdHF5L3MkTjb5DeTcbJxJNCZ3JmcWM=
107-
github.com/hyperledger/firefly-transaction-manager v1.4.1-0.20251003113149-46c9271ca66e h1:jUumirQuZR8cQCBq9BNmpLM4rKo9ahQPdFzGvf/RYIs=
108-
github.com/hyperledger/firefly-transaction-manager v1.4.1-0.20251003113149-46c9271ca66e/go.mod h1:KHGvK/QqD2jpRZ04lQoX/k1T2o644NCkRlr3FbvKqnA=
109-
github.com/hyperledger/firefly-transaction-manager v1.4.1-0.20251017153149-d6f944cab6b6 h1:FGTXGnOoAaFkdA3n3SMYDXmHePrpX425P5YyljnaSyU=
110-
github.com/hyperledger/firefly-transaction-manager v1.4.1-0.20251017153149-d6f944cab6b6/go.mod h1:KHGvK/QqD2jpRZ04lQoX/k1T2o644NCkRlr3FbvKqnA=
107+
github.com/hyperledger/firefly-transaction-manager v1.4.1-0.20251024174118-8e8e3629f7fa h1:OTgB+KnpMS47RhIXs4G7FwSDKozbHpNNgPrbwdngaqk=
108+
github.com/hyperledger/firefly-transaction-manager v1.4.1-0.20251024174118-8e8e3629f7fa/go.mod h1:KHGvK/QqD2jpRZ04lQoX/k1T2o644NCkRlr3FbvKqnA=
111109
github.com/imdario/mergo v0.3.11/go.mod h1:jmQim1M+e3UYxmgPu/WyfjB3N3VflVyUjjjwH0dnCYA=
112110
github.com/imdario/mergo v0.3.16 h1:wwQJbIsHYGMUyLSPrEq1CT16AhnhNJQ51+4fdHUnCl4=
113111
github.com/imdario/mergo v0.3.16/go.mod h1:WBLT9ZmE3lPoWsEzCh9LPo3TiwVN+ZKEjmz+hD27ysY=

internal/ethereum/confirmation_reconciler.go

Lines changed: 38 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -29,12 +29,6 @@ import (
2929
// ReconcileConfirmationsForTransaction is the public API for reconciling transaction confirmations.
3030
// It delegates to the blockListener's internal reconciliation logic.
3131
func (c *ethConnector) ReconcileConfirmationsForTransaction(ctx context.Context, txHash string, existingConfirmations []*ffcapi.MinimalBlockInfo, targetConfirmationCount uint64) (*ffcapi.ConfirmationUpdateResult, error) {
32-
// Before we start, make sure that the existing confirmations queue is valid and consistent with itself
33-
err := validateExistingConfirmations(ctx, existingConfirmations)
34-
if err != nil {
35-
return nil, err
36-
}
37-
3832
// Now we can start the reconciliation process
3933
return c.blockListener.reconcileConfirmationsForTransaction(ctx, txHash, existingConfirmations, targetConfirmationCount)
4034
}
@@ -60,14 +54,16 @@ func (bl *blockListener) buildConfirmationList(ctx context.Context, existingConf
6054
// Primary objective of this algorithm is to build a contiguous, linked list of `MinimalBlockInfo` structs, starting from the transaction block and ending as far as our current knowledge of the in-memory partial canonical chain allows.
6155
// Secondary objective is to report whether any fork was detected (and corrected) during this analysis
6256

63-
// before we get into the main algorithm, handle a couple of special cases to reduce readability of the main algorithm
64-
reconcileResult, err := bl.handleSpecialCases(ctx, existingConfirmations, txBlockInfo, targetConfirmationCount)
65-
if reconcileResult != nil || err != nil {
66-
return reconcileResult, err
57+
// handle confirmation count of 0 as a special case to reduce complexity of the main algorithm
58+
if targetConfirmationCount == 0 {
59+
reconcileResult, err := bl.handleZeroTargetConfirmationCount(ctx, txBlockInfo)
60+
if reconcileResult != nil || err != nil {
61+
return reconcileResult, err
62+
}
6763
}
6864

6965
// Initialize the result with the target confirmation count
70-
reconcileResult = &ffcapi.ConfirmationUpdateResult{
66+
reconcileResult := &ffcapi.ConfirmationUpdateResult{
7167
TargetConfirmationCount: targetConfirmationCount,
7268
}
7369

@@ -79,6 +75,15 @@ func (bl *blockListener) buildConfirmationList(ctx context.Context, existingConf
7975
// - The `lateList`. This is the most recent set of blocks that we are interesting in and we believe are accurate for the current state of the chain
8076

8177
earlyList := createEarlyList(existingConfirmations, txBlockInfo, reconcileResult)
78+
79+
// if early list is sufficient to meet the target confirmation count, we handle this as a special case as well
80+
if len(earlyList) > 0 && earlyList[len(earlyList)-1].BlockNumber.Uint64()+1 >= txBlockInfo.BlockNumber.Uint64()+targetConfirmationCount {
81+
reconcileResult := bl.handleTargetCountMetWithEarlyList(earlyList, txBlockInfo, targetConfirmationCount)
82+
if reconcileResult != nil {
83+
return reconcileResult, nil
84+
}
85+
}
86+
8287
lateList, err := createLateList(ctx, txBlockInfo, targetConfirmationCount, bl)
8388
if err != nil {
8489
return nil, err
@@ -121,7 +126,6 @@ func (bl *blockListener) buildConfirmationList(ctx context.Context, existingConf
121126
}
122127

123128
reconcileResult.Confirmed = uint64(len(reconcileResult.Confirmations)) > targetConfirmationCount // do this maths here as a utility so that the consumer doesn't have to do it
124-
125129
return reconcileResult, nil
126130
}
127131

@@ -219,11 +223,24 @@ func (s *splice) toSingleLinkedList() []*ffcapi.MinimalBlockInfo {
219223
// createEarlyList will return a list of blocks that starts with the latest transaction block and followed by any blocks in the existing confirmations list that are still valid
220224
// any blocks that are not contiguous will be discarded
221225
func createEarlyList(existingConfirmations []*ffcapi.MinimalBlockInfo, txBlockInfo *ffcapi.MinimalBlockInfo, reconcileResult *ffcapi.ConfirmationUpdateResult) (earlyList []*ffcapi.MinimalBlockInfo) {
222-
if len(existingConfirmations) > 0 && !existingConfirmations[0].Equal(txBlockInfo) {
223-
// otherwise we discard the existing confirmations list
224-
reconcileResult.NewFork = true
225-
} else {
226-
earlyList = existingConfirmations
226+
if len(existingConfirmations) > 0 {
227+
if !existingConfirmations[0].Equal(txBlockInfo) {
228+
// we discard the existing confirmations list if the transaction block doesn't match
229+
reconcileResult.NewFork = true
230+
} else {
231+
// validate and trim the confirmations list to only include linked blocks
232+
233+
earlyList = []*ffcapi.MinimalBlockInfo{txBlockInfo}
234+
for i := 1; i < len(existingConfirmations); i++ {
235+
if !earlyList[i-1].IsParentOf(existingConfirmations[i]) {
236+
// set rebuilt flag to true to indicate the existing confirmations list is not contiguous
237+
reconcileResult.Rebuilt = true
238+
break
239+
}
240+
earlyList = append(earlyList, existingConfirmations[i])
241+
}
242+
}
243+
227244
}
228245

229246
if len(earlyList) == 0 {
@@ -302,20 +319,6 @@ func (bl *blockListener) buildConfirmationQueueUsingInMemoryPartialChain(ctx con
302319
return newConfirmationsWithoutTxBlock, nil
303320
}
304321

305-
func validateExistingConfirmations(ctx context.Context, existingConfirmations []*ffcapi.MinimalBlockInfo) error {
306-
var previousBlock *ffcapi.MinimalBlockInfo
307-
for _, existingConfirmation := range existingConfirmations {
308-
if previousBlock != nil {
309-
if !previousBlock.IsParentOf(existingConfirmation) {
310-
return i18n.NewError(ctx, msgs.MsgFailedToBuildExistingConfirmationInvalid)
311-
}
312-
}
313-
previousBlock = existingConfirmation
314-
315-
}
316-
return nil
317-
}
318-
319322
func (bl *blockListener) handleZeroTargetConfirmationCount(ctx context.Context, txBlockInfo *ffcapi.MinimalBlockInfo) (*ffcapi.ConfirmationUpdateResult, error) {
320323
bl.mux.RLock()
321324
defer bl.mux.RUnlock()
@@ -332,7 +335,7 @@ func (bl *blockListener) handleZeroTargetConfirmationCount(ctx context.Context,
332335
return nil, i18n.NewError(ctx, msgs.MsgInMemoryPartialChainNotCaughtUp, txBlockInfo.BlockNumber.Uint64(), txBlockInfo.BlockHash)
333336
}
334337

335-
func (bl *blockListener) handleTargetCountLessThanExistingConfirmationLength(_ context.Context, existingConfirmations []*ffcapi.MinimalBlockInfo, txBlockInfo *ffcapi.MinimalBlockInfo, targetConfirmationCount uint64) (*ffcapi.ConfirmationUpdateResult, error) {
338+
func (bl *blockListener) handleTargetCountMetWithEarlyList(existingConfirmations []*ffcapi.MinimalBlockInfo, txBlockInfo *ffcapi.MinimalBlockInfo, targetConfirmationCount uint64) *ffcapi.ConfirmationUpdateResult {
336339
bl.mux.RLock()
337340
defer bl.mux.RUnlock()
338341
nextInMemoryBlock := bl.canonicalChain.Front()
@@ -353,7 +356,7 @@ func (bl *blockListener) handleTargetCountLessThanExistingConfirmationLength(_ c
353356
return &ffcapi.ConfirmationUpdateResult{
354357
Confirmed: true,
355358
Confirmations: existingConfirmations[:targetConfirmationCount+1],
356-
}, nil
359+
}
357360
}
358361
// only the existing confirmations are not enough, need to fetch more blocks from the in memory partial chain
359362
newList := existingConfirmations
@@ -369,19 +372,7 @@ func (bl *blockListener) handleTargetCountLessThanExistingConfirmationLength(_ c
369372
return &ffcapi.ConfirmationUpdateResult{
370373
Confirmed: uint64(len(newList)) > targetConfirmationCount,
371374
Confirmations: newList,
372-
}, nil
373-
}
374-
return nil, nil
375-
}
376-
377-
// handleSpecialCases contains the logic that cannot be included in the main algorithm because they reduce the readability of the code
378-
func (bl *blockListener) handleSpecialCases(ctx context.Context, existingConfirmations []*ffcapi.MinimalBlockInfo, txBlockInfo *ffcapi.MinimalBlockInfo, targetConfirmationCount uint64) (*ffcapi.ConfirmationUpdateResult, error) {
379-
if targetConfirmationCount == 0 {
380-
return bl.handleZeroTargetConfirmationCount(ctx, txBlockInfo)
381-
}
382-
383-
if len(existingConfirmations) > 0 && existingConfirmations[len(existingConfirmations)-1].BlockNumber.Uint64()+1 >= txBlockInfo.BlockNumber.Uint64()+targetConfirmationCount {
384-
return bl.handleTargetCountLessThanExistingConfirmationLength(ctx, existingConfirmations, txBlockInfo, targetConfirmationCount)
375+
}
385376
}
386-
return nil, nil
377+
return nil
387378
}

0 commit comments

Comments
 (0)