improve code consistency

Signed-off-by: Daisuke Kanda <daisuke.kanda@datachain.jp>

Author: Daisuke Kanda

cmd/tx.go

@@ -457,7 +457,7 @@ func relayMsgsCmd(ctx *config.Context) *cobra.Command {
 				return err
 			}
 
-			sp, err = st.ProcessTimeoutPackets(cmd.Context(), c[src], c[dst], sh, sp)
+			err = st.ProcessTimeoutPackets(cmd.Context(), c[src], c[dst], sh, sp) // update sp
 			if err != nil {
 				return err
 			}

core/naive-strategy.go

@@ -201,7 +201,7 @@ func (st *NaiveStrategy) UnrelayedPackets(ctx context.Context, src, dst *Provabl
 	}, nil
 }
 
-func (st *NaiveStrategy) ProcessTimeoutPackets(ctx context.Context, src, dst *ProvableChain, sh SyncHeaders, rp *RelayPackets) (*RelayPackets, error) {
+func (st *NaiveStrategy) ProcessTimeoutPackets(ctx context.Context, src, dst *ProvableChain, sh SyncHeaders, rp *RelayPackets) error {
 	logger := GetChannelPairLogger(src, dst)
 	var (
 		srcPackets   PacketInfoList
@@ -219,44 +219,44 @@ func (st *NaiveStrategy) ProcessTimeoutPackets(ctx context.Context, src, dst *Pr
 	if 0 < len(rp.Src) {
 		if h, err := dst.LatestHeight(ctx); err != nil {
 			logger.Error("fail to get dst.LatestHeight", err)
-			return nil, err
+			return err
 		} else {
 			dstLatestHeight = h
 		}
 
 		if t, err := dst.Timestamp(ctx, dstLatestHeight); err != nil {
 			logger.Error("fail to get dst.Timestamp of latestHeight", err)
-			return nil, err
+			return err
 		} else {
 			dstLatestTimestamp = uint64(t.UnixNano())
 		}
 
 		dstLatestFinalizedHeight = sh.GetLatestFinalizedHeader(dst.ChainID()).GetHeight()
 		if t, err := dst.Timestamp(ctx, dstLatestFinalizedHeight); err != nil {
 			logger.Error("fail to get dst.Timestamp of latestFinalizedHeight", err)
-			return nil, err
+			return err
 		} else {
 			dstLatestFinalizedTimestamp = uint64(t.UnixNano())
 		}
 	}
 	if 0 < len(rp.Dst) {
 		if h, err := src.LatestHeight(ctx); err != nil {
 			logger.Error("fail to get src.LatestHeight", err)
-			return nil, err
+			return err
 		} else {
 			srcLatestHeight = h
 		}
 		if t, err := src.Timestamp(ctx, srcLatestHeight); err != nil {
 			logger.Error("fail to get src.Timestamp", err)
-			return nil, err
+			return err
 		} else {
 			srcLatestTimestamp = uint64(t.UnixNano())
 		}
 
 		srcLatestFinalizedHeight = sh.GetLatestFinalizedHeader(src.ChainID()).GetHeight()
 		if t, err := src.Timestamp(ctx, srcLatestFinalizedHeight); err != nil {
 			logger.Error("fail to get src.Timestamp", err)
-			return nil, err
+			return err
 		} else {
 			srcLatestFinalizedTimestamp = uint64(t.UnixNano())
 		}
@@ -267,16 +267,20 @@ func (st *NaiveStrategy) ProcessTimeoutPackets(ctx context.Context, src, dst *Pr
 			(p.TimeoutTimestamp != 0 && p.TimeoutTimestamp <= timestamp)
 	}
 
+	var srcTimeoutPacket, dstTimeoutPacket *PacketInfo
 	for i, p := range rp.Src {
 		if isTimeout(p, dstLatestFinalizedHeight, dstLatestFinalizedTimestamp) {
 			p.TimedOut = true
 			if src.Path().GetOrder() == chantypes.ORDERED {
+				//  For ordered channel, a timeout notification will cause the channel to be closed.
+				//  Packets proceeding the timeout packet is relayed first
+				// so that they can be proceeded before the channel is closed.
 				if i == 0 {
-					dstPackets = append(dstPackets, p)
+					srcTimeoutPacket = p
 				}
 				break
 			} else {
-				dstPackets = append(dstPackets, p)
+				srcTimeoutPacket = p
 			}
 		} else if isTimeout(p, dstLatestHeight, dstLatestTimestamp) {
 			break
@@ -290,11 +294,11 @@ func (st *NaiveStrategy) ProcessTimeoutPackets(ctx context.Context, src, dst *Pr
 			p.TimedOut = true
 			if dst.Path().GetOrder() == chantypes.ORDERED {
 				if i == 0 {
-					srcPackets = append(srcPackets, p)
+					dstTimeoutPacket = p
 				}
 				break
 			} else {
-				srcPackets = append(srcPackets, p)
+				dstTimeoutPacket = p
 			}
 		} else if (isTimeout(p, srcLatestHeight, srcLatestTimestamp)) {
 			break
@@ -303,10 +307,15 @@ func (st *NaiveStrategy) ProcessTimeoutPackets(ctx context.Context, src, dst *Pr
 			dstPackets = append(dstPackets, p)
 		}
 	}
-	return &RelayPackets{
-		Src: srcPackets,
-		Dst: dstPackets,
-	}, nil
+	if srcTimeoutPacket != nil {
+		dstPackets = append(dstPackets, srcTimeoutPacket)
+	}
+	if dstTimeoutPacket != nil {
+		srcPackets = append(dstPackets, dstTimeoutPacket)
+	}
+	rp.Src = srcPackets
+	rp.Dst = dstPackets
+	return nil
 }
 
 func (st *NaiveStrategy) RelayPackets(ctx context.Context, src, dst *ProvableChain, rp *RelayPackets, sh SyncHeaders, doExecuteRelaySrc, doExecuteRelayDst bool) (*RelayMsgs, error) {

core/service.go

@@ -119,7 +119,7 @@ func (srv *RelayService) Serve(ctx context.Context) error {
 		return err
 	}
 
-	pseqs, err = srv.st.ProcessTimeoutPackets(ctx, srv.src, srv.dst, srv.sh, pseqs)
+	err = srv.st.ProcessTimeoutPackets(ctx, srv.src, srv.dst, srv.sh, pseqs) // update pseqs
 	if err != nil {
 		logger.Error("failed to process timeout packets", err)
 		return err

core/service_test.go

@@ -44,10 +44,10 @@ func (s *NaiveStrategyWrap) UnrelayedPackets(ctx context.Context, src, dst *core
 	return ret, err
 }
 
-func (s *NaiveStrategyWrap) ProcessTimeoutPackets(ctx context.Context, src, dst *core.ProvableChain, sh core.SyncHeaders, rp *core.RelayPackets) (*core.RelayPackets, error) {
-	ret, err := s.Inner.ProcessTimeoutPackets(ctx, src, dst, sh, rp)
-	s.ProcessTimeoutPacketsOut = ret
-	return ret, err
+func (s *NaiveStrategyWrap) ProcessTimeoutPackets(ctx context.Context, src, dst *core.ProvableChain, sh core.SyncHeaders, rp *core.RelayPackets) error {
+	err := s.Inner.ProcessTimeoutPackets(ctx, src, dst, sh, rp)
+	s.ProcessTimeoutPacketsOut = rp
+	return err
 }
 
 func (s *NaiveStrategyWrap) RelayPackets(ctx context.Context, src, dst *core.ProvableChain, rp *core.RelayPackets, sh core.SyncHeaders, doExecuteRelaySrc, doExecuteRelayDst bool) (*core.RelayMsgs, error) {

core/strategies.go

@@ -25,7 +25,7 @@ type StrategyI interface {
 	UnrelayedAcknowledgements(ctx context.Context, src, dst *ProvableChain, sh SyncHeaders, includeRelayedButUnfinalized bool) (*RelayPackets, error)
 
 	// ProcessTimeoutPackets processes timeout packets in given RelayPackets and returns sorted RelayPackets. Note that input Packet object may be modified.
-	ProcessTimeoutPackets(ctx context.Context, src, dst *ProvableChain, sh SyncHeaders, rp *RelayPackets) (*RelayPackets, error)
+	ProcessTimeoutPackets(ctx context.Context, src, dst *ProvableChain, sh SyncHeaders, rp *RelayPackets) error
 
 	// RelayAcknowledgements executes AcknowledgePacket to the packets contained in `rp` on both chains (`src` and `dst`).
 	RelayAcknowledgements(ctx context.Context, src, dst *ProvableChain, rp *RelayPackets, sh SyncHeaders, doExecuteAckSrc, doExecuteAckDst bool) (*RelayMsgs, error)