Skip to content

Commit

Permalink
fix(wallet)_: address PR feedback
Browse files Browse the repository at this point in the history
fixes #21071
  • Loading branch information
friofry committed Sep 16, 2024
1 parent 99fff41 commit 24fe8af
Show file tree
Hide file tree
Showing 13 changed files with 308 additions and 95 deletions.
9 changes: 0 additions & 9 deletions node/status_node_services.go
Original file line number Diff line number Diff line change
Expand Up @@ -584,14 +584,6 @@ func (b *StatusNode) SetWalletCommunityInfoProvider(provider thirdparty.Communit
}
}

type PeerOnlineChecker struct {
StatusNode *StatusNode
}

func (f *PeerOnlineChecker) Online() bool {
return f.StatusNode.PeerCount() > 0
}

func (b *StatusNode) walletService(accountsDB *accounts.Database, appDB *sql.DB, accountsFeed *event.Feed, settingsFeed *event.Feed, walletFeed *event.Feed, statusProxyStageName string) *wallet.Service {
if b.walletSrvc == nil {
b.walletSrvc = wallet.NewService(
Expand All @@ -602,7 +594,6 @@ func (b *StatusNode) walletService(accountsDB *accounts.Database, appDB *sql.DB,
walletFeed,
b.httpServer,
statusProxyStageName,
&PeerOnlineChecker{StatusNode: b},
)
}
return b.walletSrvc
Expand Down
27 changes: 15 additions & 12 deletions rpc/chain/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"github.com/ethereum/go-ethereum/log"
"github.com/ethereum/go-ethereum/rpc"
"github.com/status-im/status-go/circuitbreaker"
"github.com/status-im/status-go/services/network_utils"
"github.com/status-im/status-go/services/rpcstats"
"github.com/status-im/status-go/services/wallet/connection"
)
Expand Down Expand Up @@ -225,17 +226,11 @@ func isVMError(err error) bool {
return false
}

func isRPSLimitError(err error) bool {
return strings.Contains(err.Error(), "backoff_seconds") ||
strings.Contains(err.Error(), "has exceeded its throughput limit") ||
strings.Contains(err.Error(), "request rate exceeded")
}

func (c *ClientWithFallback) SetIsConnected(value bool) {
func (c *ClientWithFallback) setIsConnected(value bool, skipNotification bool) {
c.LastCheckedAt = time.Now().Unix()
if !value {
if c.isConnected.Load() {
if c.WalletNotifier != nil {
if c.WalletNotifier != nil && !skipNotification {
c.WalletNotifier(c.ChainID, "down")
}
c.isConnected.Store(false)
Expand All @@ -244,13 +239,17 @@ func (c *ClientWithFallback) SetIsConnected(value bool) {
} else {
if !c.isConnected.Load() {
c.isConnected.Store(true)
if c.WalletNotifier != nil {
if c.WalletNotifier != nil && !skipNotification {
c.WalletNotifier(c.ChainID, "up")
}
}
}
}

func (c *ClientWithFallback) SetIsConnected(value bool) {
c.setIsConnected(value, false)
}

func (c *ClientWithFallback) IsConnected() bool {
return c.isConnected.Load()
}
Expand Down Expand Up @@ -279,7 +278,7 @@ func (c *ClientWithFallback) makeCall(ctx context.Context, ethClients []*EthClie

res, err := f(provider)
if err != nil {
if isRPSLimitError(err) {
if network_utils.IsRPSLimitError(err) {
provider.limiter.ReduceLimit()

err = provider.limiter.WaitForRequestsAvailability(1)
Expand Down Expand Up @@ -977,15 +976,19 @@ func (c *ClientWithFallback) SetWalletNotifier(notifier func(chainId uint64, mes

func (c *ClientWithFallback) toggleConnectionState(err error) {
connected := true
skipNotification := false
if err != nil {
if !isNotFoundError(err) && !isVMError(err) && !errors.Is(err, ErrRequestsOverLimit) && !errors.Is(err, context.Canceled) {
if network_utils.IsConnectionError(err) {
connected = false
skipNotification = true
} else if !isNotFoundError(err) && !isVMError(err) && !errors.Is(err, ErrRequestsOverLimit) && !errors.Is(err, context.Canceled) {
log.Warn("Error not in chain call", "error", err, "chain", c.ChainID)
connected = false
} else {
log.Warn("Error in chain call", "error", err)
}
}
c.SetIsConnected(connected)
c.setIsConnected(connected, skipNotification)
}

func (c *ClientWithFallback) Tag() string {
Expand Down
92 changes: 92 additions & 0 deletions services/network_utils/network_errors.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
package network_utils

import (
"context"
"crypto/tls"
"errors"
"net"
"net/http"
"strings"
)

// IsConnectionError checks if the error is related to network issues.
func IsConnectionError(err error) bool {
if err == nil {
return false
}

// Check for net.Error (timeout or other network errors)
var netErr net.Error
if errors.As(err, &netErr) {
if netErr.Timeout() {
return true
}
}

// Check for DNS errors
var dnsErr *net.DNSError
if errors.As(err, &dnsErr) {
return true
}

// Check for network operation errors (e.g., connection refused)
var opErr *net.OpError
if errors.As(err, &opErr) {
return true
}

// Check for TLS errors
var tlsRecordErr *tls.RecordHeaderError
if errors.As(err, &tlsRecordErr) {
return true
}

// FIXME: Check for TLS ECH Rejection Error (tls.ECHRejectionError is added in go 1.23)

// Check for TLS Certificate Verification Error
var certVerifyErr *tls.CertificateVerificationError
if errors.As(err, &certVerifyErr) {
return true
}

// Check for TLS Alert Error
var alertErr tls.AlertError
if errors.As(err, &alertErr) {
return true
}

// Check if context deadline exceeded
if errors.Is(err, context.DeadlineExceeded) {
return true
}

// Check for specific HTTP server closed error
if errors.Is(err, http.ErrServerClosed) {
return true
}

// Common connection refused or timeout errors
errMsg := strings.ToLower(err.Error())
if strings.Contains(errMsg, "i/o timeout") ||
strings.Contains(errMsg, "connection refused") ||
strings.Contains(errMsg, "network is unreachable") ||
strings.Contains(errMsg, "no such host") ||
strings.Contains(errMsg, "tls handshake timeout") {
return true
}

return false
}

func IsRPSLimitError(err error) bool {
if err == nil {
return false
}
errMsg := strings.ToLower(err.Error())
if strings.Contains(errMsg, "backoff_seconds") ||
strings.Contains(errMsg, "has exceeded its throughput limit") ||
strings.Contains(errMsg, "request rate exceeded") {
return true
}
return false
}
157 changes: 157 additions & 0 deletions services/network_utils/network_errors_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,157 @@
package network_utils

import (
"context"
"crypto/tls"
"errors"
"net"
"net/http"
"testing"
)

// TestIsConnectionError tests the isConnectionError function.
func TestIsConnectionError(t *testing.T) {
tests := []struct {
name string
err error
wantResult bool
}{
{
name: "nil error",
err: nil,
wantResult: false,
},
{
name: "net.Error timeout",
err: &net.DNSError{IsTimeout: true},
wantResult: true,
},
{
name: "DNS error",
err: &net.DNSError{},
wantResult: true,
},
{
name: "net.OpError",
err: &net.OpError{},
wantResult: true,
},
{
name: "tls.RecordHeaderError",
err: &tls.RecordHeaderError{},
wantResult: true,
},
{
name: "tls.CertificateVerificationError",
err: &tls.CertificateVerificationError{},
wantResult: true,
},
{
name: "tls.AlertError",
err: tls.AlertError(0),
wantResult: true,
},
{
name: "context.DeadlineExceeded",
err: context.DeadlineExceeded,
wantResult: true,
},
{
name: "http.ErrServerClosed",
err: http.ErrServerClosed,
wantResult: true,
},
{
name: "i/o timeout error message",
err: errors.New("i/o timeout"),
wantResult: true,
},
{
name: "connection refused error message",
err: errors.New("connection refused"),
wantResult: true,
},
{
name: "network is unreachable error message",
err: errors.New("network is unreachable"),
wantResult: true,
},
{
name: "no such host error message",
err: errors.New("no such host"),
wantResult: true,
},
{
name: "tls handshake timeout error message",
err: errors.New("tls handshake timeout"),
wantResult: true,
},
{
name: "rps limit error 1",
err: errors.New("backoff_seconds"),
wantResult: false,
},
{
name: "rps limit error 2",
err: errors.New("has exceeded its throughput limit"),
wantResult: false,
},
{
name: "rps limit error 3",
err: errors.New("request rate exceeded"),
wantResult: false,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := IsConnectionError(tt.err)
if got != tt.wantResult {
t.Errorf("isConnectionError(%v) = %v; want %v", tt.err, got, tt.wantResult)
}
})
}
}

func TestIsRPSLimitError(t *testing.T) {
tests := []struct {
name string
err error
wantResult bool
}{
{
name: "Error contains 'backoff_seconds'",
err: errors.New("Error: backoff_seconds: 30"),
wantResult: true,
},
{
name: "Error contains 'has exceeded its throughput limit'",
err: errors.New("Your application has exceeded its throughput limit."),
wantResult: true,
},
{
name: "Error contains 'request rate exceeded'",
err: errors.New("Request rate exceeded. Please try again later."),
wantResult: true,
},
{
name: "Error does not contain any matching phrases",
err: errors.New("Some other error occurred."),
wantResult: false,
},
{
name: "Error is nil",
err: nil,
wantResult: false,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := IsRPSLimitError(tt.err)
if got != tt.wantResult {
t.Errorf("IsRPSLimitError(%v) = %v; want %v", tt.err, got, tt.wantResult)
}
})
}
}
2 changes: 1 addition & 1 deletion services/wallet/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ func TestAPI_GetAddressDetails(t *testing.T) {
chainClient.SetWalletNotifier(func(chainID uint64, message string) {})
c.SetWalletNotifier(func(chainID uint64, message string) {})

service := NewService(db, accountsDb, appDB, c, accountFeed, nil, nil, nil, &params.NodeConfig{}, nil, nil, nil, nil, nil, "", nil)
service := NewService(db, accountsDb, appDB, c, accountFeed, nil, nil, nil, &params.NodeConfig{}, nil, nil, nil, nil, nil, "")

api := &API{
s: service,
Expand Down
13 changes: 0 additions & 13 deletions services/wallet/common/mock/online_checker_mock.go

This file was deleted.

5 changes: 0 additions & 5 deletions services/wallet/common/online_checker_interface.go

This file was deleted.

2 changes: 1 addition & 1 deletion services/wallet/keycard_pairings_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ func TestKeycardPairingsFile(t *testing.T) {

accountFeed := &event.Feed{}

service := NewService(db, accountsDb, appDB, &rpc.Client{NetworkManager: network.NewManager(db)}, accountFeed, nil, nil, nil, &params.NodeConfig{}, nil, nil, nil, nil, nil, "", nil)
service := NewService(db, accountsDb, appDB, &rpc.Client{NetworkManager: network.NewManager(db)}, accountFeed, nil, nil, nil, &params.NodeConfig{}, nil, nil, nil, nil, nil, "")

data, err := service.KeycardPairings().GetPairingsJSONFileContent()
require.NoError(t, err)
Expand Down
Loading

0 comments on commit 24fe8af

Please sign in to comment.