Skip to content

Commit

Permalink
fix: refactor: use temporary context for cleaner error handling (#845)
Browse files Browse the repository at this point in the history
* fix+refactor: use temporary context in error handling

* smoketest: add zeta supply infra

* make linter happy

* fix unit tests

* refactor: rename and add some comments

---------

Co-authored-by: brewmaster012 <>
  • Loading branch information
brewmaster012 authored Jul 31, 2023
1 parent 94f9048 commit 13d3938
Show file tree
Hide file tree
Showing 9 changed files with 99 additions and 124 deletions.
23 changes: 17 additions & 6 deletions contrib/localnet/orchestrator/smoketest/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import (
"context"
"fmt"
"github.com/btcsuite/btcutil"
authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
"github.com/spf13/cobra"
"github.com/zeta-chain/zetacore/contrib/localnet/orchestrator/smoketest/contracts/zevmswap"
"github.com/zeta-chain/zetacore/zetaclient/config"
Expand Down Expand Up @@ -52,11 +54,15 @@ var (
)

type SmokeTest struct {
zevmClient *ethclient.Client
goerliClient *ethclient.Client
cctxClient types.QueryClient
btcRPCClient *rpcclient.Client
fungibleClient fungibletypes.QueryClient
zevmClient *ethclient.Client
goerliClient *ethclient.Client
btcRPCClient *rpcclient.Client

cctxClient types.QueryClient
fungibleClient fungibletypes.QueryClient
authClient authtypes.QueryClient
bankClient banktypes.QueryClient

wg sync.WaitGroup
ZetaEth *zetaeth.ZetaEth
ZetaEthAddr ethcommon.Address
Expand Down Expand Up @@ -104,6 +110,7 @@ func init() {

func NewSmokeTest(goerliClient *ethclient.Client, zevmClient *ethclient.Client,
cctxClient types.QueryClient, fungibleClient fungibletypes.QueryClient,
authClient authtypes.QueryClient, bankClient banktypes.QueryClient,
goerliAuth *bind.TransactOpts, zevmAuth *bind.TransactOpts,
btcRPCClient *rpcclient.Client) *SmokeTest {
// query system contract address
Expand Down Expand Up @@ -140,6 +147,8 @@ func NewSmokeTest(goerliClient *ethclient.Client, zevmClient *ethclient.Client,
goerliClient: goerliClient,
cctxClient: cctxClient,
fungibleClient: fungibleClient,
authClient: authClient,
bankClient: bankClient,
wg: sync.WaitGroup{},
goerliAuth: goerliAuth,
zevmAuth: zevmAuth,
Expand Down Expand Up @@ -204,6 +213,8 @@ func LocalSmokeTest(_ *cobra.Command, _ []string) {

cctxClient := types.NewQueryClient(grpcConn)
fungibleClient := fungibletypes.NewQueryClient(grpcConn)
authClient := authtypes.NewQueryClient(grpcConn)
bankClient := banktypes.NewQueryClient(grpcConn)

// Wait for Genesis and keygen to be completed. ~ height 30
time.Sleep(20 * time.Second)
Expand Down Expand Up @@ -240,7 +251,7 @@ func LocalSmokeTest(_ *cobra.Command, _ []string) {
panic(err)
}

smokeTest := NewSmokeTest(goerliClient, zevmClient, cctxClient, fungibleClient, goerliAuth, zevmAuth, btcRPCClient)
smokeTest := NewSmokeTest(goerliClient, zevmClient, cctxClient, fungibleClient, authClient, bankClient, goerliAuth, zevmAuth, btcRPCClient)

// The following deployment must happen here and in this order, please do not change
// ==================== Deploying contracts ====================
Expand Down
18 changes: 18 additions & 0 deletions contrib/localnet/orchestrator/smoketest/test_message_passing.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package main
import (
"context"
"fmt"
banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
"math/big"
"time"

Expand Down Expand Up @@ -171,6 +172,15 @@ func (sm *SmokeTest) TestMessagePassingRevertSuccess() {
if err != nil {
panic(err)
}

res2, err := sm.bankClient.SupplyOf(context.Background(), &banktypes.QuerySupplyOfRequest{
Denom: "azeta",
})
if err != nil {
panic(err)
}
fmt.Printf("$$$ Before: SUPPLY OF AZETA: %d\n", res2.Amount.Amount)

tx, err = testDApp.SendHelloWorld(auth, sm.TestDAppAddr, big.NewInt(1337), amount, true)
if err != nil {
panic(err)
Expand Down Expand Up @@ -198,4 +208,12 @@ func (sm *SmokeTest) TestMessagePassingRevertSuccess() {
fmt.Printf(" Message: %x\n", event.Message)
}
}
res3, err := sm.bankClient.SupplyOf(context.Background(), &banktypes.QuerySupplyOfRequest{
Denom: "azeta",
})
if err != nil {
panic(err)
}
fmt.Printf("$$$ After: SUPPLY OF AZETA: %d\n", res3.Amount.Amount.BigInt())
fmt.Printf("$$$ Diff: SUPPLY OF AZETA: %d\n", res3.Amount.Amount.Sub(res2.Amount.Amount).BigInt())
}
6 changes: 5 additions & 1 deletion contrib/localnet/orchestrator/smoketest/test_stress.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ package main
import (
"context"
"fmt"
authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
"github.com/ethereum/go-ethereum/accounts/abi/bind"
ethcommon "github.com/ethereum/go-ethereum/common"
"github.com/ethereum/go-ethereum/crypto"
Expand Down Expand Up @@ -100,6 +102,8 @@ func StressTest(_ *cobra.Command, _ []string) {

cctxClient := types.NewQueryClient(grpcConn)
fungibleClient := fungibletypes.NewQueryClient(grpcConn)
bankClient := banktypes.NewQueryClient(grpcConn)
authClient := authtypes.NewQueryClient(grpcConn)

// Wait for Genesis and keygen to be completed. ~ height 30
time.Sleep(20 * time.Second)
Expand Down Expand Up @@ -136,7 +140,7 @@ func StressTest(_ *cobra.Command, _ []string) {
panic(err)
}

smokeTest := NewSmokeTest(goerliClient, zevmClient, cctxClient, fungibleClient, goerliAuth, zevmAuth, nil)
smokeTest := NewSmokeTest(goerliClient, zevmClient, cctxClient, fungibleClient, authClient, bankClient, goerliAuth, zevmAuth, nil)

// If stress test is running on local docker environment
if stressTestArgs.local {
Expand Down
23 changes: 13 additions & 10 deletions x/crosschain/keeper/keeper_cross_chain_tx_vote_inbound_tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,18 +96,18 @@ func (k msgServer) VoteOnObservedInboundTx(goCtx context.Context, msg *types.Msg
tmpCtx, commit := ctx.CacheContext()
isContractReverted, err := k.HandleEVMDeposit(tmpCtx, &cctx, *msg, observationChain)
if err != nil && !isContractReverted { // exceptional case; internal error; should abort CCTX
cctx.CctxStatus.ChangeStatus(&ctx, types.CctxStatus_Aborted, err.Error(), cctx.LogIdentifierForCCTX())
cctx.CctxStatus.ChangeStatus(types.CctxStatus_Aborted, err.Error())
return &types.MsgVoteOnObservedInboundTxResponse{}, nil
} else if err != nil && isContractReverted { // contract call reverted; should refund
revertMessage := err.Error()
chain := k.zetaObserverKeeper.GetParams(ctx).GetChainFromChainID(cctx.InboundTxParams.SenderChainId)
if chain == nil {
cctx.CctxStatus.ChangeStatus(&ctx, types.CctxStatus_Aborted, "invalid sender chain", cctx.LogIdentifierForCCTX())
cctx.CctxStatus.ChangeStatus(types.CctxStatus_Aborted, "invalid sender chain")
return &types.MsgVoteOnObservedInboundTxResponse{}, nil
}
medianGasPrice, isFound := k.GetMedianGasPriceInUint(ctx, chain.ChainId)
if !isFound {
cctx.CctxStatus.ChangeStatus(&ctx, types.CctxStatus_Aborted, "cannot find gas price", cctx.LogIdentifierForCCTX())
cctx.CctxStatus.ChangeStatus(types.CctxStatus_Aborted, "cannot find gas price")
return &types.MsgVoteOnObservedInboundTxResponse{}, nil
}
// create new OutboundTxParams for the revert
Expand All @@ -120,35 +120,38 @@ func (k msgServer) VoteOnObservedInboundTx(goCtx context.Context, msg *types.Msg
OutboundTxGasPrice: medianGasPrice.MulUint64(2).String(),
})
if err = k.UpdateNonce(ctx, chain.ChainId, &cctx); err != nil {
cctx.CctxStatus.ChangeStatus(&ctx, types.CctxStatus_Aborted, err.Error(), cctx.LogIdentifierForCCTX())
cctx.CctxStatus.ChangeStatus(types.CctxStatus_Aborted, err.Error())
return &types.MsgVoteOnObservedInboundTxResponse{}, nil
}
// do not commit() here;
cctx.CctxStatus.ChangeStatus(&ctx, types.CctxStatus_PendingRevert, revertMessage, cctx.LogIdentifierForCCTX())
cctx.CctxStatus.ChangeStatus(types.CctxStatus_PendingRevert, revertMessage)
return &types.MsgVoteOnObservedInboundTxResponse{}, nil
} else { // successful HandleEVMDeposit;
commit()
cctx.CctxStatus.ChangeStatus(&ctx, types.CctxStatus_OutboundMined, "Remote omnichain contract call completed", cctx.LogIdentifierForCCTX())
cctx.CctxStatus.ChangeStatus(types.CctxStatus_OutboundMined, "Remote omnichain contract call completed")
return &types.MsgVoteOnObservedInboundTxResponse{}, nil
}
} else { // Cross Chain SWAP
tmpCtx, commit := ctx.CacheContext()
err = func() error {
cctx.InboundTxParams.InboundTxFinalizedZetaHeight = uint64(ctx.BlockHeader().Height)
err := k.UpdatePrices(ctx, receiverChain.ChainId, &cctx)
err := k.PayGasInZetaAndUpdateCctx(tmpCtx, receiverChain.ChainId, &cctx)
if err != nil {
return err
}
err = k.UpdateNonce(ctx, receiverChain.ChainId, &cctx)
err = k.UpdateNonce(tmpCtx, receiverChain.ChainId, &cctx)
if err != nil {
return err
}
return nil
}()
if err != nil {
cctx.CctxStatus.ChangeStatus(&ctx, types.CctxStatus_Aborted, err.Error(), cctx.LogIdentifierForCCTX())
// do not commit anything here as the CCTX should be aborted
cctx.CctxStatus.ChangeStatus(types.CctxStatus_Aborted, err.Error())
return &types.MsgVoteOnObservedInboundTxResponse{}, nil
}
cctx.CctxStatus.ChangeStatus(&ctx, types.CctxStatus_PendingOutbound, "", cctx.LogIdentifierForCCTX())
commit()
cctx.CctxStatus.ChangeStatus(types.CctxStatus_PendingOutbound, "")
return &types.MsgVoteOnObservedInboundTxResponse{}, nil
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ import (
// // NativeTokenSymbol: "",
// // MedianIndex: 0,
// //})
// err := keeper.UpdatePrices(ctx, cctx[0].OutboundTxParams.ReceiverChain, &cctx[0])
// err := keeper.PayGasInZetaAndUpdateCctx(ctx, cctx[0].OutboundTxParams.ReceiverChain, &cctx[0])
// assert.NoError(t, err)
// fmt.Println(cctx[0].String())
//}
Expand Down Expand Up @@ -94,11 +94,11 @@ func TestStatus_StatusTransition(t *testing.T) {
IsErr: false,
},
}
_, ctx := setupKeeper(t)
_, _ = setupKeeper(t)
for _, test := range tt {
test := test
t.Run(test.Name, func(t *testing.T) {
test.Status.ChangeStatus(&ctx, test.NonErrStatus, test.Msg, "")
test.Status.ChangeStatus(test.NonErrStatus, test.Msg)
if test.IsErr {
assert.Equal(t, test.ErrStatus, test.Status.Status)
} else {
Expand Down
45 changes: 12 additions & 33 deletions x/crosschain/keeper/keeper_cross_chain_tx_vote_outbound_tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,9 @@ import (
"context"
"fmt"

"cosmossdk.io/math"
sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
"github.com/rs/zerolog/log"
"github.com/zeta-chain/zetacore/common"
"github.com/zeta-chain/zetacore/x/crosschain/types"
observerKeeper "github.com/zeta-chain/zetacore/x/observer/keeper"
observerTypes "github.com/zeta-chain/zetacore/x/observer/types"
Expand Down Expand Up @@ -105,32 +103,20 @@ func (k msgServer) VoteOnObservedOutboundTx(goCtx context.Context, msg *types.Ms
// FinalizeOutbound sets final status for a successful vote
// FinalizeOutbound updates CCTX Prices and Nonce for a revert

err = func() error { //err = FinalizeOutbound(k, ctx, &cctx, msg, ballot.BallotStatus) // remove this line
tmpCtx, commit := ctx.CacheContext()
err = func() error { //err = FinalizeOutbound(k, ctx, &cctx, msg, ballot.BallotStatus)
cctx.GetCurrentOutTxParam().OutboundTxObservedExternalHeight = msg.ObservedOutTxBlockHeight
zetaBurnt := cctx.InboundTxParams.Amount
zetaMinted := cctx.GetCurrentOutTxParam().Amount
oldStatus := cctx.CctxStatus.Status
switch ballot.BallotStatus {
case observerTypes.BallotStatus_BallotFinalized_SuccessObservation:
switch oldStatus {
case types.CctxStatus_PendingRevert:
cctx.CctxStatus.ChangeStatus(&ctx, types.CctxStatus_Reverted, "", cctx.LogIdentifierForCCTX())
cctx.CctxStatus.ChangeStatus(types.CctxStatus_Reverted, "")
case types.CctxStatus_PendingOutbound:
cctx.CctxStatus.ChangeStatus(&ctx, types.CctxStatus_OutboundMined, "", cctx.LogIdentifierForCCTX())
cctx.CctxStatus.ChangeStatus(types.CctxStatus_OutboundMined, "")
}

newStatus := cctx.CctxStatus.Status.String()
if zetaBurnt.LT(zetaMinted) {
// TODO :Handle Error ?
}
balanceAmount := zetaBurnt.Sub(zetaMinted)
if cctx.GetCurrentOutTxParam().CoinType == common.CoinType_Zeta { // TODO : Handle Fee for other coins
err := HandleFeeBalances(k, ctx, balanceAmount)
if err != nil {
return err
}
}
EmitOutboundSuccess(ctx, msg, oldStatus.String(), newStatus, cctx)
EmitOutboundSuccess(tmpCtx, msg, oldStatus.String(), newStatus, cctx)
case observerTypes.BallotStatus_BallotFinalized_FailureObservation:
switch oldStatus {
case types.CctxStatus_PendingOutbound:
Expand All @@ -142,45 +128,38 @@ func (k msgServer) VoteOnObservedOutboundTx(goCtx context.Context, msg *types.Ms
CoinType: cctx.InboundTxParams.CoinType,
OutboundTxGasLimit: cctx.OutboundTxParams[0].OutboundTxGasLimit, // NOTE(pwu): revert gas limit = initial outbound gas limit set by user;
})
err := k.UpdatePrices(ctx, cctx.InboundTxParams.SenderChainId, &cctx)
err := k.PayGasInZetaAndUpdateCctx(tmpCtx, cctx.InboundTxParams.SenderChainId, &cctx)
if err != nil {
return err
}
err = k.UpdateNonce(ctx, cctx.InboundTxParams.SenderChainId, &cctx)
err = k.UpdateNonce(tmpCtx, cctx.InboundTxParams.SenderChainId, &cctx)
if err != nil {
return err
}
cctx.CctxStatus.ChangeStatus(&ctx, types.CctxStatus_PendingRevert, "Outbound failed, start revert", cctx.LogIdentifierForCCTX())
cctx.CctxStatus.ChangeStatus(types.CctxStatus_PendingRevert, "Outbound failed, start revert")
case types.CctxStatus_PendingRevert:
cctx.CctxStatus.ChangeStatus(&ctx, types.CctxStatus_Aborted, "Outbound failed: revert failed; abort TX", cctx.LogIdentifierForCCTX())
cctx.CctxStatus.ChangeStatus(types.CctxStatus_Aborted, "Outbound failed: revert failed; abort TX")
}
newStatus := cctx.CctxStatus.Status.String()
EmitOutboundFailure(ctx, msg, oldStatus.String(), newStatus, cctx)
}
return nil
}()
if err != nil {
cctx.CctxStatus.ChangeStatus(&ctx, types.CctxStatus_Aborted, err.Error(), cctx.LogIdentifierForCCTX())
// do not commit tmpCtx
cctx.CctxStatus.ChangeStatus(types.CctxStatus_Aborted, err.Error())
ctx.Logger().Error(err.Error())
k.RemoveOutTxTracker(ctx, msg.OutTxChain, msg.OutTxTssNonce)
k.RemoveFromPendingNonces(ctx, tss.TssPubkey, msg.OutTxChain, int64(msg.OutTxTssNonce))
k.RemoveOutTxTracker(ctx, msg.OutTxChain, msg.OutTxTssNonce)
k.SetCctxAndNonceToCctxAndInTxHashToCctx(ctx, cctx)
return &types.MsgVoteOnObservedOutboundTxResponse{}, nil
}
commit()
// Set the ballot index to the finalized ballot
cctx.GetCurrentOutTxParam().OutboundTxBallotIndex = ballotIndex
k.RemoveFromPendingNonces(ctx, tss.TssPubkey, msg.OutTxChain, int64(msg.OutTxTssNonce))
k.RemoveOutTxTracker(ctx, msg.OutTxChain, msg.OutTxTssNonce)
k.SetCctxAndNonceToCctxAndInTxHashToCctx(ctx, cctx)
return &types.MsgVoteOnObservedOutboundTxResponse{}, nil
}

func HandleFeeBalances(k msgServer, ctx sdk.Context, balanceAmount math.Uint) error {
err := k.bankKeeper.MintCoins(ctx, types.ModuleName, sdk.NewCoins(sdk.NewCoin(common.ZETADenom, sdk.NewIntFromBigInt(balanceAmount.BigInt()))))
if err != nil {
log.Error().Msgf("ReceiveConfirmation: failed to mint coins: %s", err.Error())
return sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, fmt.Sprintf("failed to mint coins: %s", err.Error()))
}
return nil
}
Loading

0 comments on commit 13d3938

Please sign in to comment.