fix(op-node/op-batcher/op-proposer): the fallback client should alway…

…s try recover (#118) Co-authored-by: Welkin <[email protected]>
bnb-chain · Mar 7, 2024 · 2fbabc7 · 2fbabc7
1 parent 5c5bf63
commit 2fbabc7
Show file tree

Hide file tree

Showing 4 changed files with 46 additions and 35 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -15,15 +15,15 @@ jobs:
         uses: actions/checkout@v3
 
       - name: Set up Go
-        uses: actions/setup-go@v2
+        uses: actions/setup-go@v5
         with:
-          go-version-file: go.mod
+          go-version: '1.20.13'
 
       - name: golangci-lint
-        uses: golangci/golangci-lint-action@v3
+        uses: golangci/golangci-lint-action@v4
         with:
           working-directory: op-node
-          version: latest
+          version: v1.55.2
           args: -E goimports,sqlclosecheck,bodyclose,asciicheck,misspell,errorlint --timeout 5m -e "errors.As" -e "errors.Is"
 
   op-batcher-lint:
@@ -34,15 +34,15 @@ jobs:
         uses: actions/checkout@v3
 
       - name: Set up Go
-        uses: actions/setup-go@v2
+        uses: actions/setup-go@v5
         with:
-          go-version-file: go.mod
+          go-version: '1.20.13'
 
       - name: golangci-lint
-        uses: golangci/golangci-lint-action@v3
+        uses: golangci/golangci-lint-action@v4
         with:
           working-directory: op-batcher
-          version: latest
+          version: v1.55.2
           args: -E goimports,sqlclosecheck,bodyclose,asciicheck,misspell,errorlint --timeout 5m -e "errors.As" -e "errors.Is"
 
   op-proposer-lint:
@@ -53,15 +53,15 @@ jobs:
         uses: actions/checkout@v3
 
       - name: Set up Go
-        uses: actions/setup-go@v2
+        uses: actions/setup-go@v5
         with:
-          go-version-file: go.mod
+          go-version: '1.20.13'
 
       - name: golangci-lint
-        uses: golangci/golangci-lint-action@v3
+        uses: golangci/golangci-lint-action@v4
         with:
           working-directory: op-proposer
-          version: latest
+          version: v1.55.2
           args: -E goimports,sqlclosecheck,bodyclose,asciicheck,misspell,errorlint --timeout 5m -e "errors.As" -e "errors.Is"
 
   op-node-test:
@@ -73,9 +73,9 @@ jobs:
         uses: actions/checkout@v3
 
       - name: Set up Go
-        uses: actions/setup-go@v2
+        uses: actions/setup-go@v5
         with:
-          go-version-file: go.mod
+          go-version: '1.20.13'
 
       - name: Install gotestsum
         uses: autero1/[email protected]
@@ -102,9 +102,9 @@ jobs:
         uses: actions/checkout@v3
 
       - name: Set up Go
-        uses: actions/setup-go@v2
+        uses: actions/setup-go@v5
         with:
-          go-version-file: go.mod
+          go-version: '1.20.13'
 
       - name: Install gotestsum
         uses: autero1/[email protected]
@@ -131,9 +131,9 @@ jobs:
         uses: actions/checkout@v3
 
       - name: Set up Go
-        uses: actions/setup-go@v2
+        uses: actions/setup-go@v5
         with:
-          go-version-file: go.mod
+          go-version: '1.20.13'
 
       - name: Install gotestsum
         uses: autero1/[email protected]
@@ -160,9 +160,9 @@ jobs:
         uses: actions/checkout@v3
 
       - name: Set up Go
-        uses: actions/setup-go@v2
+        uses: actions/setup-go@v5
         with:
-          go-version-file: go.mod
+          go-version: '1.20.13'
 
       - name: Install gotestsum
         uses: autero1/[email protected]
@@ -191,9 +191,9 @@ jobs:
         uses: actions/checkout@v3
 
       - name: Set up Go
-        uses: actions/setup-go@v2
+        uses: actions/setup-go@v5
         with:
-          go-version-file: go.mod
+          go-version: '1.20.13'
 
       - name: Install gotestsum
         uses: autero1/[email protected]

diff --git a/op-e2e/actions/fallback_client_test.go b/op-e2e/actions/fallback_client_test.go
@@ -1,6 +1,10 @@
 package actions
 
 import (
+	"math/big"
+	"testing"
+	"time"
+
 	"github.com/ethereum-optimism/optimism/op-e2e/e2eutils"
 	"github.com/ethereum-optimism/optimism/op-node/client"
 	"github.com/ethereum-optimism/optimism/op-node/eth"
@@ -12,9 +16,6 @@ import (
 	"github.com/ethereum/go-ethereum/log"
 	"github.com/ethereum/go-ethereum/params"
 	"github.com/stretchr/testify/require"
-	"math/big"
-	"testing"
-	"time"
 )
 
 func setupFallbackClientTest(t Testing, sd *e2eutils.SetupData, log log.Logger, l1Url string) (*L1Miner, *L1Replica, *L1Replica, *L2Engine, *L2Sequencer, *sources.FallbackClient) {

diff --git a/op-node/sources/fallback_client.go b/op-node/sources/fallback_client.go
@@ -157,6 +157,13 @@ func (l *FallbackClient) switchCurrentRpc() {
 }
 
 func (l *FallbackClient) switchCurrentRpcLogic() error {
+	//Use defer to ensure that recoverIfFirstRpcHealth will always be executed regardless of the circumstances.
+	defer func() {
+		if !l.isInFallbackState {
+			l.isInFallbackState = true
+			l.recoverIfFirstRpcHealth()
+		}
+	}()
 	url := l.urlList[l.currentIndex]
 	newRpc, err := l.rpcInitFunc(url)
 	if err != nil {
@@ -179,10 +186,6 @@ func (l *FallbackClient) switchCurrentRpcLogic() error {
 		}
 	}
 	l.log.Info("switched current rpc to new url", "url", url)
-	if !l.isInFallbackState {
-		l.isInFallbackState = true
-		l.recoverIfFirstRpcHealth()
-	}
 	return nil
 }
 
@@ -221,7 +224,9 @@ func (l *FallbackClient) recoverIfFirstRpcHealth() {
 		}
 		lastRpc := *l.currentRpc.Load()
 		l.currentRpc.Store(&l.firstRpc)
-		lastRpc.Close()
+		if lastRpc != l.firstRpc {
+			lastRpc.Close()
+		}
 		l.lastMinuteFail.Store(0)
 		l.currentIndex = 0
 		l.isInFallbackState = false

diff --git a/op-service/client/fallback_client.go b/op-service/client/fallback_client.go
@@ -240,6 +240,13 @@ func (l *FallbackClient) switchCurrentClient() {
 	if l.lastMinuteFail.Load() <= l.fallbackThreshold {
 		return
 	}
+	//Use defer to ensure that recoverIfFirstRpcHealth will always be executed regardless of the circumstances.
+	defer func() {
+		if !l.isInFallbackState {
+			l.isInFallbackState = true
+			l.recoverIfFirstRpcHealth()
+		}
+	}()
 	l.currentIndex++
 	if l.currentIndex >= len(l.urlList) {
 		l.log.Error("the fallback client has tried all urls")
@@ -259,10 +266,6 @@ func (l *FallbackClient) switchCurrentClient() {
 	}
 	l.lastMinuteFail.Store(0)
 	l.log.Info("switched current rpc to new url", "url", url)
-	if !l.isInFallbackState {
-		l.isInFallbackState = true
-		l.recoverIfFirstRpcHealth()
-	}
 }
 
 func (l *FallbackClient) recoverIfFirstRpcHealth() {
@@ -287,7 +290,9 @@ func (l *FallbackClient) recoverIfFirstRpcHealth() {
 		}
 		lastClient := *l.currentClient.Load()
 		l.currentClient.Store(&l.firstClient)
-		lastClient.Close()
+		if lastClient != l.firstClient {
+			lastClient.Close()
+		}
 		l.lastMinuteFail.Store(0)
 		l.currentIndex = 0
 		l.isInFallbackState = false