chore_: needed router changes (#5620)

* fix_: setting best route only when we have it

* chore_: when returning errors, give precedence to custom (network) errors

* chore_: skip flaky tests (#5606)

* chore_: skip TestPeerCount

* chore_: skip flaky wallet tests

* chore_: skip flaky TestMemberReceivesPendingRequestToJoinAfterAfterGettingTokenMasterRole

* test_: flaky router tests fixed

Closes https://github.com/status-im/status-go/issues/5601

---------

Co-authored-by: Igor Sirotin <sirotin@status.im>
This commit is contained in:
saledjenic 2024-07-29 19:03:13 +02:00 committed by GitHub
parent 07641bf38f
commit 82b63ccf29
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 143 additions and 22 deletions

View File

@ -10,6 +10,7 @@ import (
gethcommon "github.com/ethereum/go-ethereum/common"
hexutil "github.com/ethereum/go-ethereum/common/hexutil"
gethbridge "github.com/status-im/status-go/eth-node/bridge/geth"
"github.com/status-im/status-go/eth-node/crypto"
"github.com/status-im/status-go/eth-node/types"
@ -819,6 +820,8 @@ func (s *MessengerCommunitiesSharedMemberAddressSuite) TestTokenMasterReceivesAc
}
func (s *MessengerCommunitiesSharedMemberAddressSuite) TestMemberReceivesPendingRequestToJoinAfterAfterGettingTokenMasterRole() {
s.T().Skip("flaky test")
community, _ := createOnRequestCommunity(&s.Suite, s.owner)
advertiseCommunityTo(&s.Suite, community, s.owner, s.alice)

View File

@ -29,6 +29,6 @@ var (
ErrLockedAmountExcludesAllSupported = &errors.ErrorResponse{Code: errors.ErrorCode("WR-021"), Details: "all supported chains are excluded, routing impossible"}
ErrTokenNotFound = &errors.ErrorResponse{Code: errors.ErrorCode("WR-022"), Details: "token not found"}
ErrNoBestRouteFound = &errors.ErrorResponse{Code: errors.ErrorCode("WR-023"), Details: "no best route found"}
ErrCannotCheckReceiverBalance = &errors.ErrorResponse{Code: errors.ErrorCode("WR-024"), Details: "cannot check receiver balance"}
ErrCannotCheckBalance = &errors.ErrorResponse{Code: errors.ErrorCode("WR-024"), Details: "cannot check balance"}
ErrCannotCheckLockedAmounts = &errors.ErrorResponse{Code: errors.ErrorCode("WR-025"), Details: "cannot check locked amounts"}
)

View File

@ -95,3 +95,30 @@ func createErrorResponse(processorName string, err error) error {
customErrResp.Details = genericErrResp.Details
return customErrResp
}
func IsCustomError(err error) bool {
if err == nil {
return false
}
errResp, ok := err.(*errors.ErrorResponse)
if !ok {
return false
}
switch errResp {
case ErrTransferCustomError,
ErrERC721TransferCustomError,
ErrERC1155TransferCustomError,
ErrBridgeHopCustomError,
ErrBridgeCellerCustomError,
ErrSwapParaswapCustomError,
ErrENSRegisterCustomError,
ErrENSReleaseCustomError,
ErrENSPublicKeyCustomError,
ErrStickersBuyCustomError:
return true
default:
return false
}
}

View File

@ -68,3 +68,38 @@ func TestNonGenericErrorResponse(t *testing.T) {
require.Equal(t, errResp.Code, castPPErrResp.Code)
require.Equal(t, errResp.Details, castPPErrResp.Details)
}
func TestCustomErrors(t *testing.T) {
tests := []struct {
name string
err error
expected bool
}{
{
name: "no error - nil",
err: nil,
expected: false,
},
{
name: "not error response error",
err: errors.New("unknown error"),
expected: false,
},
{
name: "not custom error",
err: ErrFromChainNotSupported,
expected: false,
},
{
name: "custom error",
err: ErrTransferCustomError,
expected: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
require.Equal(t, tt.expected, IsCustomError(tt.err))
})
}
}

View File

@ -175,7 +175,6 @@ func newSuggestedRoutesV2(
suggestedRoutes := &SuggestedRoutesV2{
Uuid: uuid,
Candidates: candidates,
Best: candidates,
TokenPrice: tokenPrice,
NativeChainTokenPrice: nativeChainTokenPrice,
}
@ -190,6 +189,14 @@ func newSuggestedRoutesV2(
allRoutes := node.buildAllRoutesV2()
allRoutes = filterRoutesV2(allRoutes, amountIn, fromLockedAmount)
if len(allRoutes) > 0 {
sort.Slice(allRoutes, func(i, j int) bool {
iRoute := getRoutePriority(allRoutes[i])
jRoute := getRoutePriority(allRoutes[j])
return iRoute <= jRoute
})
}
return suggestedRoutes, allRoutes
}
@ -507,7 +514,16 @@ func (r *Router) SuggestedRoutesV2(ctx context.Context, input *RouteInputParams)
// No best route found, but no error given.
if len(processorErrors) > 0 {
// Return one of the path processor errors if present.
err = errors.CreateErrorResponseFromError(processorErrors[0].Error)
// Give precedence to the custom error message.
for _, processorError := range processorErrors {
if processorError.Error != nil && pathprocessor.IsCustomError(processorError.Error) {
err = processorError.Error
break
}
}
if err == nil {
err = errors.CreateErrorResponseFromError(processorErrors[0].Error)
}
} else {
err = ErrNoBestRouteFound
}
@ -995,6 +1011,12 @@ func (r *Router) resolveCandidates(ctx context.Context, input *RouteInputParams,
})
}
sort.Slice(candidates, func(i, j int) bool {
iChain := getChainPriority(candidates[i].FromChain.ChainID)
jChain := getChainPriority(candidates[j].FromChain.ChainID)
return iChain <= jChain
})
group.Wait()
return candidates, processorErrors, nil
}
@ -1004,7 +1026,7 @@ func (r *Router) checkBalancesForTheBestRoute(ctx context.Context, bestRoute []*
return new(big.Int).Set(v.(*big.Int))
}).(map[string]*big.Int)
if balanceMapCopy == nil {
return ErrCannotCheckReceiverBalance
return ErrCannotCheckBalance
}
// check the best route for the required balances
@ -1071,6 +1093,27 @@ func removeBestRouteFromAllRouters(allRoutes [][]*PathV2, best []*PathV2) [][]*P
return nil
}
func getChainPriority(chainID uint64) int {
switch chainID {
case walletCommon.EthereumMainnet, walletCommon.EthereumSepolia:
return 1
case walletCommon.OptimismMainnet, walletCommon.OptimismSepolia:
return 2
case walletCommon.ArbitrumMainnet, walletCommon.ArbitrumSepolia:
return 3
default:
return 0
}
}
func getRoutePriority(route []*PathV2) int {
priority := 0
for _, path := range route {
priority += getChainPriority(path.FromChain.ChainID)
}
return priority
}
func (r *Router) resolveRoutes(ctx context.Context, input *RouteInputParams, candidates []*PathV2, balanceMap map[string]*big.Int) (suggestedRoutes *SuggestedRoutesV2, err error) {
var prices map[string]float64
if input.testsMode {
@ -1088,6 +1131,16 @@ func (r *Router) resolveRoutes(ctx context.Context, input *RouteInputParams, can
var allRoutes [][]*PathV2
suggestedRoutes, allRoutes = newSuggestedRoutesV2(input.Uuid, input.AmountIn.ToInt(), candidates, input.FromLockedAmount, tokenPrice, nativeTokenPrice)
defer func() {
if suggestedRoutes.Best != nil && len(suggestedRoutes.Best) > 0 {
sort.Slice(suggestedRoutes.Best, func(i, j int) bool {
iChain := getChainPriority(suggestedRoutes.Best[i].FromChain.ChainID)
jChain := getChainPriority(suggestedRoutes.Best[j].FromChain.ChainID)
return iChain <= jChain
})
}
}()
for len(allRoutes) > 0 {
best := findBestV2(allRoutes, tokenPrice, nativeTokenPrice)
@ -1098,18 +1151,16 @@ func (r *Router) resolveRoutes(ctx context.Context, input *RouteInputParams, can
if (input.SendType == Transfer ||
input.SendType == Bridge) &&
len(allRoutes) > 1 {
allRoutes = removeBestRouteFromAllRouters(allRoutes, best)
continue
} else {
suggestedRoutes.Best = best
return suggestedRoutes, errors.CreateErrorResponseFromError(err)
}
}
if len(best) > 0 {
sort.Slice(best, func(i, j int) bool {
return best[i].AmountInLocked
})
// At this point we have to do the final check and update the amountIn (subtracting fees) if complete balance is going to be sent for native token (ETH)
for _, path := range best {
if path.subtractFees && path.FromToken.IsNative() {

View File

@ -230,20 +230,23 @@ func TestNoBalanceForTheBestRouteRouterV2(t *testing.T) {
// Test async endpoints
for _, tt := range tests {
router.SuggestedRoutesV2Async(tt.input)
t.Run(tt.name, func(t *testing.T) {
select {
case asyncRoutes := <-suggestedRoutesCh:
assert.Equal(t, tt.input.Uuid, asyncRoutes.Uuid)
assert.Equal(t, tt.expectedError, asyncRoutes.ErrorResponse)
assertPathsEqual(t, tt.expectedCandidates, asyncRoutes.Candidates)
if tt.expectedError == nil {
assertPathsEqual(t, tt.expectedBest, asyncRoutes.Best)
router.SuggestedRoutesV2Async(tt.input)
select {
case asyncRoutes := <-suggestedRoutesCh:
assert.Equal(t, tt.input.Uuid, asyncRoutes.Uuid)
assert.Equal(t, tt.expectedError, asyncRoutes.ErrorResponse)
assertPathsEqual(t, tt.expectedCandidates, asyncRoutes.Candidates)
if tt.expectedError == nil {
assertPathsEqual(t, tt.expectedBest, asyncRoutes.Best)
}
break
case <-time.After(10 * time.Second):
t.FailNow()
}
break
case <-time.After(10 * time.Second):
t.FailNow()
}
})
}
}

View File

@ -2728,7 +2728,7 @@ func getNoBalanceTestParamsList() []noBalanceTestParams {
},
expectedError: &errors.ErrorResponse{
Code: ErrNotEnoughTokenBalance.Code,
Details: fmt.Sprintf(ErrNotEnoughTokenBalance.Details, pathprocessor.UsdcSymbol, walletCommon.EthereumMainnet),
Details: fmt.Sprintf(ErrNotEnoughTokenBalance.Details, pathprocessor.UsdcSymbol, walletCommon.ArbitrumMainnet),
},
expectedCandidates: []*PathV2{
{
@ -2788,7 +2788,7 @@ func getNoBalanceTestParamsList() []noBalanceTestParams {
},
expectedError: &errors.ErrorResponse{
Code: ErrNotEnoughNativeBalance.Code,
Details: fmt.Sprintf(ErrNotEnoughNativeBalance.Details, pathprocessor.EthSymbol, walletCommon.EthereumMainnet),
Details: fmt.Sprintf(ErrNotEnoughNativeBalance.Details, pathprocessor.EthSymbol, walletCommon.ArbitrumMainnet),
},
expectedCandidates: []*PathV2{
{

View File

@ -352,6 +352,8 @@ func setDefaultConfig(config *wakuv2.Config, lightMode bool) {
var testStoreENRBootstrap = "enrtree://AI4W5N5IFEUIHF5LESUAOSMV6TKWF2MB6GU2YK7PU4TYUGUNOCEPW@store.staging.shards.nodes.status.im"
func TestPeerCount(t *testing.T) {
t.Skip("flaky test")
expectedCondition := func(received []TelemetryRequest) (shouldSucceed bool, shouldFail bool) {
found := slices.ContainsFunc(received, func(req TelemetryRequest) bool {
t.Log(req)