Skip to content

Commit f958ba0

Browse files
[Backport] Retry upon coordination proposal failure (#3807)
This pull request backports #3804 to the `releases/mainnet/v2.0.1` branch.
2 parents 45acddd + 552f9c1 commit f958ba0

File tree

2 files changed

+141
-2
lines changed

2 files changed

+141
-2
lines changed

pkg/tbtc/coordination.go

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ import (
77
"fmt"
88
"math/rand"
99
"sort"
10+
"strings"
11+
"time"
1012

1113
"github.com/keep-network/keep-core/pkg/internal/pb"
1214
"go.uber.org/zap"
@@ -578,13 +580,15 @@ func (ce *coordinationExecutor) executeLeaderRoutine(
578580
) (CoordinationProposal, error) {
579581
walletPublicKeyHash := ce.walletPublicKeyHash()
580582

581-
proposal, err := ce.proposalGenerator.Generate(
583+
proposal, err := ce.generateProposal(
582584
&CoordinationProposalRequest{
583585
WalletPublicKeyHash: walletPublicKeyHash,
584586
WalletOperators: ce.coordinatedWallet.signingGroupOperators,
585587
ExecutingOperator: ce.operatorAddress,
586588
ActionsChecklist: actionsChecklist,
587589
},
590+
2, // 2 attempts at most
591+
1*time.Minute, // 1 minute between attempts
588592
)
589593
if err != nil {
590594
return nil, fmt.Errorf("failed to generate proposal: [%v]", err)
@@ -615,6 +619,40 @@ func (ce *coordinationExecutor) executeLeaderRoutine(
615619
return proposal, nil
616620
}
617621

622+
// generateProposal generates a proposal for the given coordination request.
623+
// The generator retries the proposal generation if it fails. The number of
624+
// attempts is limited to attemptLimit. The generator waits for retryDelay
625+
// between attempts.
626+
func (ce *coordinationExecutor) generateProposal(
627+
request *CoordinationProposalRequest,
628+
attemptLimit uint,
629+
retryDelay time.Duration,
630+
) (CoordinationProposal, error) {
631+
var attemptErrs []string
632+
633+
for attempt := uint(1); attempt <= attemptLimit; attempt++ {
634+
if attempt > 1 {
635+
time.Sleep(retryDelay)
636+
}
637+
638+
proposal, err := ce.proposalGenerator.Generate(request)
639+
if err != nil {
640+
attemptErrs = append(
641+
attemptErrs,
642+
fmt.Sprintf("attempt [%v] error: [%v]", attempt, err),
643+
)
644+
continue
645+
}
646+
647+
return proposal, nil
648+
}
649+
650+
return nil, fmt.Errorf(
651+
"all attempts failed: [%v]",
652+
strings.Join(attemptErrs, "; "),
653+
)
654+
}
655+
618656
// executeFollowerRoutine executes the follower's routine for the given coordination
619657
// window. The routine listens for the coordination message from the leader and
620658
// validates it. If the leader's proposal is valid, it returns the received

pkg/tbtc/coordination_test.go

Lines changed: 102 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -306,6 +306,7 @@ func TestCoordinationExecutor_Coordinate(t *testing.T) {
306306
func(
307307
walletPublicKeyHash [20]byte,
308308
actionsChecklist []WalletActionType,
309+
_ uint,
309310
) (CoordinationProposal, error) {
310311
for _, action := range actionsChecklist {
311312
if walletPublicKeyHash == publicKeyHash && action == ActionRedemption {
@@ -692,6 +693,7 @@ func TestCoordinationExecutor_ExecuteLeaderRoutine(t *testing.T) {
692693
func(
693694
walletPublicKeyHash [20]byte,
694695
actionsChecklist []WalletActionType,
696+
_ uint,
695697
) (
696698
CoordinationProposal,
697699
error,
@@ -791,6 +793,97 @@ func TestCoordinationExecutor_ExecuteLeaderRoutine(t *testing.T) {
791793
}
792794
}
793795

796+
func TestCoordinationExecutor_GenerateProposal(t *testing.T) {
797+
var tests = map[string]struct {
798+
proposalGenerator CoordinationProposalGenerator
799+
expectedProposal CoordinationProposal
800+
expectedError error
801+
}{
802+
"first attempt success": {
803+
proposalGenerator: newMockCoordinationProposalGenerator(
804+
func(
805+
_ [20]byte,
806+
_ []WalletActionType,
807+
_ uint,
808+
) (CoordinationProposal, error) {
809+
return &NoopProposal{}, nil
810+
},
811+
),
812+
expectedProposal: &NoopProposal{},
813+
expectedError: nil,
814+
},
815+
"last attempt success": {
816+
proposalGenerator: newMockCoordinationProposalGenerator(
817+
func(
818+
_ [20]byte,
819+
_ []WalletActionType,
820+
call uint,
821+
) (CoordinationProposal, error) {
822+
if call == 1 {
823+
return nil, fmt.Errorf("unexpected error")
824+
} else if call == 2 {
825+
return &NoopProposal{}, nil
826+
} else {
827+
panic("unexpected call")
828+
}
829+
},
830+
),
831+
expectedProposal: &NoopProposal{},
832+
expectedError: nil,
833+
},
834+
"all attempts failed": {
835+
proposalGenerator: newMockCoordinationProposalGenerator(
836+
func(
837+
_ [20]byte,
838+
_ []WalletActionType,
839+
call uint,
840+
) (CoordinationProposal, error) {
841+
return nil, fmt.Errorf("unexpected error %v", call)
842+
},
843+
),
844+
expectedProposal: nil,
845+
expectedError: fmt.Errorf(
846+
"all attempts failed: [attempt [1] error: [unexpected error 1]; attempt [2] error: [unexpected error 2]]",
847+
),
848+
},
849+
}
850+
851+
for testName, test := range tests {
852+
t.Run(testName, func(t *testing.T) {
853+
executor := &coordinationExecutor{
854+
// Set only relevant fields.
855+
proposalGenerator: test.proposalGenerator,
856+
}
857+
858+
proposal, err := executor.generateProposal(
859+
&CoordinationProposalRequest{}, // request fields not relevant
860+
2,
861+
1*time.Second,
862+
)
863+
864+
if !reflect.DeepEqual(test.expectedError, err) {
865+
t.Errorf(
866+
"unexpected error\n"+
867+
"expected: %v\n"+
868+
"actual: %v\n",
869+
test.expectedError,
870+
err,
871+
)
872+
}
873+
874+
if !reflect.DeepEqual(test.expectedProposal, proposal) {
875+
t.Errorf(
876+
"unexpected proposal\n"+
877+
"expected: %v\n"+
878+
"actual: %v\n",
879+
test.expectedProposal,
880+
proposal,
881+
)
882+
}
883+
})
884+
}
885+
}
886+
794887
func TestCoordinationExecutor_ExecuteFollowerRoutine(t *testing.T) {
795888
// Uncompressed public key corresponding to the 20-byte public key hash:
796889
// aa768412ceed10bd423c025542ca90071f9fb62d.
@@ -1163,16 +1256,19 @@ func TestCoordinationExecutor_ExecuteFollowerRoutine_WithIdleLeader(t *testing.T
11631256
}
11641257

11651258
type mockCoordinationProposalGenerator struct {
1259+
calls uint
11661260
delegate func(
11671261
walletPublicKeyHash [20]byte,
11681262
actionsChecklist []WalletActionType,
1263+
call uint,
11691264
) (CoordinationProposal, error)
11701265
}
11711266

11721267
func newMockCoordinationProposalGenerator(
11731268
delegate func(
11741269
walletPublicKeyHash [20]byte,
11751270
actionsChecklist []WalletActionType,
1271+
call uint,
11761272
) (CoordinationProposal, error),
11771273
) *mockCoordinationProposalGenerator {
11781274
return &mockCoordinationProposalGenerator{
@@ -1183,5 +1279,10 @@ func newMockCoordinationProposalGenerator(
11831279
func (mcpg *mockCoordinationProposalGenerator) Generate(
11841280
request *CoordinationProposalRequest,
11851281
) (CoordinationProposal, error) {
1186-
return mcpg.delegate(request.WalletPublicKeyHash, request.ActionsChecklist)
1282+
mcpg.calls++
1283+
return mcpg.delegate(
1284+
request.WalletPublicKeyHash,
1285+
request.ActionsChecklist,
1286+
mcpg.calls,
1287+
)
11871288
}

0 commit comments

Comments
 (0)