Skip to content

Commit

Permalink
some minimum code refactor
Browse files Browse the repository at this point in the history
  • Loading branch information
ws4charlie committed Apr 19, 2024
1 parent aab441e commit 00ab5ac
Show file tree
Hide file tree
Showing 8 changed files with 78 additions and 62 deletions.
20 changes: 20 additions & 0 deletions testutil/keeper/mocks/crosschain/observer.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion x/crosschain/keeper/cctx_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ func (k Keeper) GetRevertGasLimit(ctx sdk.Context, cctx types.CrossChainTx) (uin
return 0, nil
}

func IsPending(cctx types.CrossChainTx) bool {
func IsPending(cctx *types.CrossChainTx) bool {
// pending inbound is not considered a "pending" state because it has not reached consensus yet
return cctx.CctxStatus.Status == types.CctxStatus_PendingOutbound || cctx.CctxStatus.Status == types.CctxStatus_PendingRevert
}
Expand Down
2 changes: 1 addition & 1 deletion x/crosschain/keeper/cctx_utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ func Test_IsPending(t *testing.T) {
}
for _, tc := range tt {
t.Run(fmt.Sprintf("status %s", tc.status), func(t *testing.T) {
require.Equal(t, tc.expected, crosschainkeeper.IsPending(types.CrossChainTx{CctxStatus: &types.Status{Status: tc.status}}))
require.Equal(t, tc.expected, crosschainkeeper.IsPending(&types.CrossChainTx{CctxStatus: &types.Status{Status: tc.status}}))
})
}
}
Expand Down
98 changes: 40 additions & 58 deletions x/crosschain/keeper/grpc_query_cctx.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,16 +82,12 @@ func (k Keeper) CctxByNonce(c context.Context, req *types.QueryGetCctxByNonceReq
return nil, status.Error(codes.Internal, "tss not found")
}
// #nosec G701 always in range
res, found := k.GetObserverKeeper().GetNonceToCctx(ctx, tss.TssPubkey, req.ChainID, int64(req.Nonce))
if !found {
return nil, status.Error(codes.Internal, fmt.Sprintf("nonceToCctx not found: nonce %d, chainid %d", req.Nonce, req.ChainID))
}
val, found := k.GetCrossChainTx(ctx, res.CctxIndex)
if !found {
return nil, status.Error(codes.Internal, fmt.Sprintf("cctx not found: index %s", res.CctxIndex))
cctx, err := getCctxByChainIDAndNonce(k, ctx, tss.TssPubkey, req.ChainID, int64(req.Nonce))
if err != nil {
return nil, err
}

return &types.QueryGetCctxResponse{CrossChainTx: &val}, nil
return &types.QueryGetCctxResponse{CrossChainTx: cctx}, nil
}

// CctxListPending returns a list of pending cctxs and the total number of pending cctxs
Expand Down Expand Up @@ -138,20 +134,16 @@ func (k Keeper) CctxListPending(c context.Context, req *types.QueryListCctxPendi
startNonce = 0
}
for i := startNonce; i < pendingNonces.NonceLow; i++ {
nonceToCctx, found := k.GetObserverKeeper().GetNonceToCctx(ctx, tss.TssPubkey, req.ChainId, i)
if !found {
return nil, status.Error(codes.Internal, fmt.Sprintf("nonceToCctx not found: nonce %d, chainid %d", i, req.ChainId))
}
cctx, found := k.GetCrossChainTx(ctx, nonceToCctx.CctxIndex)
if !found {
return nil, status.Error(codes.Internal, fmt.Sprintf("cctx not found: index %s", nonceToCctx.CctxIndex))
cctx, err := getCctxByChainIDAndNonce(k, ctx, tss.TssPubkey, req.ChainId, i)
if err != nil {
return nil, err

Check warning on line 139 in x/crosschain/keeper/grpc_query_cctx.go

View check run for this annotation

Codecov / codecov/patch

x/crosschain/keeper/grpc_query_cctx.go#L139

Added line #L139 was not covered by tests
}

// only take a `limit` number of pending cctxs as result but still count the total pending cctxs
if IsPending(cctx) {
totalPending++
if !maxCCTXsReached() {
cctxs = append(cctxs, &cctx)
cctxs = append(cctxs, cctx)
}
}
}
Expand All @@ -162,15 +154,11 @@ func (k Keeper) CctxListPending(c context.Context, req *types.QueryListCctxPendi

// now query the pending nonces that we know are pending
for i := pendingNonces.NonceLow; i < pendingNonces.NonceHigh && !maxCCTXsReached(); i++ {
nonceToCctx, found := k.GetObserverKeeper().GetNonceToCctx(ctx, tss.TssPubkey, req.ChainId, i)
if !found {
return nil, status.Error(codes.Internal, "nonceToCctx not found")
cctx, err := getCctxByChainIDAndNonce(k, ctx, tss.TssPubkey, req.ChainId, i)
if err != nil {
return nil, err

Check warning on line 159 in x/crosschain/keeper/grpc_query_cctx.go

View check run for this annotation

Codecov / codecov/patch

x/crosschain/keeper/grpc_query_cctx.go#L159

Added line #L159 was not covered by tests
}
cctx, found := k.GetCrossChainTx(ctx, nonceToCctx.CctxIndex)
if !found {
return nil, status.Error(codes.Internal, "cctxIndex not found")
}
cctxs = append(cctxs, &cctx)
cctxs = append(cctxs, cctx)
}

return &types.QueryListCctxPendingResponse{
Expand Down Expand Up @@ -243,27 +231,20 @@ func (k Keeper) CctxListPendingWithinRateLimit(c context.Context, req *types.Que
return uint32(len(cctxs)) >= limit

Check warning on line 231 in x/crosschain/keeper/grpc_query_cctx.go

View check run for this annotation

Codecov / codecov/patch

x/crosschain/keeper/grpc_query_cctx.go#L231

Added line #L231 was not covered by tests
}

// query pending nonces for each supported chain
// query pending nonces for each foreign chain
// Note: The pending nonces could change during the RPC call, so query them beforehand
chains := k.zetaObserverKeeper.GetSupportedChains(ctx)
chains := k.zetaObserverKeeper.GetSupportedForeignChains(ctx)
for _, chain := range chains {
if chain.IsZetaChain() {
continue
}
pendingNonces, found := k.GetObserverKeeper().GetPendingNonces(ctx, tss.TssPubkey, chain.ChainId)
if !found {
return nil, status.Error(codes.Internal, "pending nonces not found")
}
pendingNoncesMap[chain.ChainId] = &pendingNonces

Check warning on line 242 in x/crosschain/keeper/grpc_query_cctx.go

View check run for this annotation

Codecov / codecov/patch

x/crosschain/keeper/grpc_query_cctx.go#L242

Added line #L242 was not covered by tests
}

// query backwards for potential missed pending cctxs for each supported chain
// query backwards for potential missed pending cctxs for each foreign chain
LoopBackwards:
for _, chain := range chains {

Check warning on line 247 in x/crosschain/keeper/grpc_query_cctx.go

View check run for this annotation

Codecov / codecov/patch

x/crosschain/keeper/grpc_query_cctx.go#L246-L247

Added lines #L246 - L247 were not covered by tests
if chain.IsZetaChain() {
continue
}

// we should at least query 1000 prior to find any pending cctx that we might have missed
// this logic is needed because a confirmation of higher nonce will automatically update the p.NonceLow
// therefore might mask some lower nonce cctx that is still pending.
Expand All @@ -276,13 +257,9 @@ LoopBackwards:

// query cctx by nonce backwards to the left boundary of the rate limit sliding window
for nonce := startNonce; nonce >= 0; nonce-- {
nonceToCctx, found := k.GetObserverKeeper().GetNonceToCctx(ctx, tss.TssPubkey, chain.ChainId, nonce)
if !found {
return nil, status.Error(codes.Internal, fmt.Sprintf("nonceToCctx not found: chainid %d, nonce %d", chain.ChainId, nonce))
}
cctx, found := k.GetCrossChainTx(ctx, nonceToCctx.CctxIndex)
if !found {
return nil, status.Error(codes.Internal, fmt.Sprintf("cctx not found: index %s", nonceToCctx.CctxIndex))
cctx, err := getCctxByChainIDAndNonce(k, ctx, tss.TssPubkey, chain.ChainId, nonce)
if err != nil {
return nil, err

Check warning on line 262 in x/crosschain/keeper/grpc_query_cctx.go

View check run for this annotation

Codecov / codecov/patch

x/crosschain/keeper/grpc_query_cctx.go#L259-L262

Added lines #L259 - L262 were not covered by tests
}

// We should at least go backwards by 1000 nonces to pick up missed pending cctxs
Expand All @@ -299,7 +276,7 @@ LoopBackwards:
break

Check warning on line 276 in x/crosschain/keeper/grpc_query_cctx.go

View check run for this annotation

Codecov / codecov/patch

x/crosschain/keeper/grpc_query_cctx.go#L275-L276

Added lines #L275 - L276 were not covered by tests
}
// criteria #3: if rate limiter is enabled, we should finish the RPC call if the rate limit is exceeded
if rateLimitExceeded(chain.ChainId, &cctx, gasCoinRates, erc20CoinRates, erc20Coins, &totalCctxValueInZeta, rateLimitInZeta) {
if rateLimitExceeded(chain.ChainId, cctx, gasCoinRates, erc20CoinRates, erc20Coins, &totalCctxValueInZeta, rateLimitInZeta) {
limitExceeded = true
break LoopBackwards

Check warning on line 281 in x/crosschain/keeper/grpc_query_cctx.go

View check run for this annotation

Codecov / codecov/patch

x/crosschain/keeper/grpc_query_cctx.go#L279-L281

Added lines #L279 - L281 were not covered by tests
}
Expand All @@ -309,7 +286,7 @@ LoopBackwards:
if IsPending(cctx) {
totalPending++
if !maxCCTXsReached() {
cctxs = append(cctxs, &cctx)
cctxs = append(cctxs, cctx)

Check warning on line 289 in x/crosschain/keeper/grpc_query_cctx.go

View check run for this annotation

Codecov / codecov/patch

x/crosschain/keeper/grpc_query_cctx.go#L286-L289

Added lines #L286 - L289 were not covered by tests
}
}
}
Expand All @@ -321,35 +298,27 @@ LoopBackwards:
totalPending += uint64(pendingNonces.NonceHigh - pendingNonces.NonceLow)

Check warning on line 298 in x/crosschain/keeper/grpc_query_cctx.go

View check run for this annotation

Codecov / codecov/patch

x/crosschain/keeper/grpc_query_cctx.go#L298

Added line #L298 was not covered by tests
}

// query forwards for pending cctxs for each supported chain
// query forwards for pending cctxs for each foreign chain
LoopForwards:
for _, chain := range chains {

Check warning on line 303 in x/crosschain/keeper/grpc_query_cctx.go

View check run for this annotation

Codecov / codecov/patch

x/crosschain/keeper/grpc_query_cctx.go#L302-L303

Added lines #L302 - L303 were not covered by tests
if chain.IsZetaChain() {
continue
}

// query the pending cctxs in range [NonceLow, NonceHigh)
pendingNonces := pendingNoncesMap[chain.ChainId]
for i := pendingNonces.NonceLow; i < pendingNonces.NonceHigh; i++ {
nonceToCctx, found := k.GetObserverKeeper().GetNonceToCctx(ctx, tss.TssPubkey, chain.ChainId, i)
if !found {
return nil, status.Error(codes.Internal, "nonceToCctx not found")
}
cctx, found := k.GetCrossChainTx(ctx, nonceToCctx.CctxIndex)
if !found {
return nil, status.Error(codes.Internal, "cctxIndex not found")
for nonce := pendingNonces.NonceLow; nonce < pendingNonces.NonceHigh; nonce++ {
cctx, err := getCctxByChainIDAndNonce(k, ctx, tss.TssPubkey, chain.ChainId, nonce)
if err != nil {
return nil, err

Check warning on line 309 in x/crosschain/keeper/grpc_query_cctx.go

View check run for this annotation

Codecov / codecov/patch

x/crosschain/keeper/grpc_query_cctx.go#L305-L309

Added lines #L305 - L309 were not covered by tests
}

// only take a `limit` number of pending cctxs as result
if maxCCTXsReached() {
break LoopForwards

Check warning on line 314 in x/crosschain/keeper/grpc_query_cctx.go

View check run for this annotation

Codecov / codecov/patch

x/crosschain/keeper/grpc_query_cctx.go#L313-L314

Added lines #L313 - L314 were not covered by tests
}
// criteria #3: if rate limiter is enabled, we should finish the RPC call if the rate limit is exceeded
if applyLimit && rateLimitExceeded(chain.ChainId, &cctx, gasCoinRates, erc20CoinRates, erc20Coins, &totalCctxValueInZeta, rateLimitInZeta) {
if applyLimit && rateLimitExceeded(chain.ChainId, cctx, gasCoinRates, erc20CoinRates, erc20Coins, &totalCctxValueInZeta, rateLimitInZeta) {
limitExceeded = true
break LoopForwards

Check warning on line 319 in x/crosschain/keeper/grpc_query_cctx.go

View check run for this annotation

Codecov / codecov/patch

x/crosschain/keeper/grpc_query_cctx.go#L317-L319

Added lines #L317 - L319 were not covered by tests
}
cctxs = append(cctxs, &cctx)
cctxs = append(cctxs, cctx)

Check warning on line 321 in x/crosschain/keeper/grpc_query_cctx.go

View check run for this annotation

Codecov / codecov/patch

x/crosschain/keeper/grpc_query_cctx.go#L321

Added line #L321 was not covered by tests
}
}

Expand All @@ -360,6 +329,19 @@ LoopForwards:
}, nil

Check warning on line 329 in x/crosschain/keeper/grpc_query_cctx.go

View check run for this annotation

Codecov / codecov/patch

x/crosschain/keeper/grpc_query_cctx.go#L325-L329

Added lines #L325 - L329 were not covered by tests
}

// getCctxByChainIDAndNonce returns the cctx by chainID and nonce
func getCctxByChainIDAndNonce(k Keeper, ctx sdk.Context, tssPubkey string, chainID int64, nonce int64) (*types.CrossChainTx, error) {
nonceToCctx, found := k.GetObserverKeeper().GetNonceToCctx(ctx, tssPubkey, chainID, nonce)
if !found {
return nil, status.Error(codes.Internal, fmt.Sprintf("nonceToCctx not found: chainid %d, nonce %d", chainID, nonce))
}
cctx, found := k.GetCrossChainTx(ctx, nonceToCctx.CctxIndex)
if !found {
return nil, status.Error(codes.Internal, fmt.Sprintf("cctx not found: index %s", nonceToCctx.CctxIndex))
}
return &cctx, nil
}

// convertCctxValue converts the value of the cctx in ZETA using given conversion rates
func convertCctxValue(
chainID int64,
Expand Down
2 changes: 1 addition & 1 deletion x/crosschain/keeper/msg_server_add_to_outtx_tracker.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ func (k msgServer) AddToOutTxTracker(goCtx context.Context, msg *types.MsgAddToO
}

// tracker submission is only allowed when the cctx is pending
if !IsPending(*cctx.CrossChainTx) {
if !IsPending(cctx.CrossChainTx) {
// garbage tracker (for any reason) is harmful to outTx observation and should be removed if it exists
// it if does not exist, RemoveOutTxTracker is a no-op
k.RemoveOutTxTracker(ctx, msg.ChainId, msg.Nonce)
Expand Down
1 change: 1 addition & 0 deletions x/crosschain/types/expected_keepers.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ type ObserverKeeper interface {
) (bool, bool, observertypes.Ballot, string, error)
GetSupportedChainFromChainID(ctx sdk.Context, chainID int64) *chains.Chain
GetSupportedChains(ctx sdk.Context) []*chains.Chain
GetSupportedForeignChains(ctx sdk.Context) []*chains.Chain
}

type FungibleKeeper interface {
Expand Down
13 changes: 13 additions & 0 deletions x/observer/keeper/chain_params.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,3 +71,16 @@ func (k Keeper) GetSupportedChains(ctx sdk.Context) []*chains.Chain {
}
return c
}

// GetSupportedForeignChains returns the list of supported foreign chains
func (k Keeper) GetSupportedForeignChains(ctx sdk.Context) []*chains.Chain {
allChains := k.GetSupportedChains(ctx)

Check warning on line 77 in x/observer/keeper/chain_params.go

View check run for this annotation

Codecov / codecov/patch

x/observer/keeper/chain_params.go#L76-L77

Added lines #L76 - L77 were not covered by tests

foreignChains := make([]*chains.Chain, 0)
for _, chain := range allChains {
if !chain.IsZetaChain() {
foreignChains = append(foreignChains, chain)

Check warning on line 82 in x/observer/keeper/chain_params.go

View check run for this annotation

Codecov / codecov/patch

x/observer/keeper/chain_params.go#L79-L82

Added lines #L79 - L82 were not covered by tests
}
}
return foreignChains

Check warning on line 85 in x/observer/keeper/chain_params.go

View check run for this annotation

Codecov / codecov/patch

x/observer/keeper/chain_params.go#L85

Added line #L85 was not covered by tests
}
2 changes: 1 addition & 1 deletion zetaclient/evm/evm_signer.go
Original file line number Diff line number Diff line change
Expand Up @@ -560,7 +560,7 @@ func (signer *Signer) reportToOutTxTracker(zetaBridge interfaces.ZetaCoreBridger
cctx, err := zetaBridge.GetCctxByNonce(chainID, nonce)
if err != nil {
logger.Err(err).Msgf("reportToOutTxTracker: error getting cctx for chain %d nonce %d outTxHash %s", chainID, nonce, outTxHash)
} else if !crosschainkeeper.IsPending(*cctx) {
} else if !crosschainkeeper.IsPending(cctx) {

Check warning on line 563 in zetaclient/evm/evm_signer.go

View check run for this annotation

Codecov / codecov/patch

zetaclient/evm/evm_signer.go#L563

Added line #L563 was not covered by tests
logger.Info().Msgf("reportToOutTxTracker: cctx already finalized for chain %d nonce %d outTxHash %s", chainID, nonce, outTxHash)
break
}
Expand Down

0 comments on commit 00ab5ac

Please sign in to comment.