Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(crosschain): only use EVM supported chains for gas stability pool and prevent errors to stop iterating other chains #1687

Merged
merged 10 commits into from
Feb 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,12 @@

### Fixes

* [1687](https://github.com/zeta-chain/node/pull/1687) - only use EVM supported chains for gas stability pool

## Version: v12.2.4

### Fixes

* [1638](https://github.com/zeta-chain/node/issues/1638) - additional check to make sure external chain height always increases
* [1672](https://github.com/zeta-chain/node/pull/1672) - paying 50% more than base gas price to buffer EIP1559 gas price increase
* [1642](https://github.com/zeta-chain/node/pull/1642) - Change WhitelistERC20 authorization from group1 to group2
Expand Down
6 changes: 6 additions & 0 deletions proto/crosschain/events.proto
Original file line number Diff line number Diff line change
Expand Up @@ -58,3 +58,9 @@ message EventOutboundSuccess {
string new_status = 4;
string value_received = 5;
}

message EventCCTXGasPriceIncreased {
string cctx_index = 1;
string gas_price_increase = 2;
string additional_fees = 3;
}
34 changes: 34 additions & 0 deletions typescript/crosschain/events_pb.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -291,3 +291,37 @@ export declare class EventOutboundSuccess extends Message<EventOutboundSuccess>
static equals(a: EventOutboundSuccess | PlainMessage<EventOutboundSuccess> | undefined, b: EventOutboundSuccess | PlainMessage<EventOutboundSuccess> | undefined): boolean;
}

/**
* @generated from message zetachain.zetacore.crosschain.EventCCTXGasPriceIncreased
*/
export declare class EventCCTXGasPriceIncreased extends Message<EventCCTXGasPriceIncreased> {
/**
* @generated from field: string cctx_index = 1;
*/
cctxIndex: string;

/**
* @generated from field: string gas_price_increase = 2;
*/
gasPriceIncrease: string;

/**
* @generated from field: string additional_fees = 3;
*/
additionalFees: string;

constructor(data?: PartialMessage<EventCCTXGasPriceIncreased>);

static readonly runtime: typeof proto3;
static readonly typeName = "zetachain.zetacore.crosschain.EventCCTXGasPriceIncreased";
static readonly fields: FieldList;

static fromBinary(bytes: Uint8Array, options?: Partial<BinaryReadOptions>): EventCCTXGasPriceIncreased;

static fromJson(jsonValue: JsonValue, options?: Partial<JsonReadOptions>): EventCCTXGasPriceIncreased;

static fromJsonString(jsonString: string, options?: Partial<JsonReadOptions>): EventCCTXGasPriceIncreased;

static equals(a: EventCCTXGasPriceIncreased | PlainMessage<EventCCTXGasPriceIncreased> | undefined, b: EventCCTXGasPriceIncreased | PlainMessage<EventCCTXGasPriceIncreased> | undefined): boolean;
}

84 changes: 63 additions & 21 deletions x/crosschain/keeper/abci.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,11 @@ import (
"fmt"
"time"

"github.com/zeta-chain/zetacore/common"

cosmoserrors "cosmossdk.io/errors"
"cosmossdk.io/math"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/zeta-chain/zetacore/common"
"github.com/zeta-chain/zetacore/x/crosschain/types"
observertypes "github.com/zeta-chain/zetacore/x/observer/types"
)
Expand All @@ -17,8 +18,21 @@ const (
RemainingFeesToStabilityPoolPercent = 95
)

// CheckAndUpdateCctxGasPriceFunc is a function type for checking and updating the gas price of a cctx
type CheckAndUpdateCctxGasPriceFunc func(
ctx sdk.Context,
k Keeper,
cctx types.CrossChainTx,
flags observertypes.GasPriceIncreaseFlags,
) (math.Uint, math.Uint, error)

// IterateAndUpdateCctxGasPrice iterates through all cctx and updates the gas price if pending for too long
func (k Keeper) IterateAndUpdateCctxGasPrice(ctx sdk.Context) error {
// The function returns the number of cctxs updated and the gas price increase flags used
func (k Keeper) IterateAndUpdateCctxGasPrice(
ctx sdk.Context,
chains []*common.Chain,
updateFunc CheckAndUpdateCctxGasPriceFunc,
) (int, observertypes.GasPriceIncreaseFlags) {
// fetch the gas price increase flags or use default
gasPriceIncreaseFlags := observertypes.DefaultGasPriceIncreaseFlags
crosschainFlags, found := k.zetaObserverKeeper.GetCrosschainFlags(ctx)
Expand All @@ -28,38 +42,66 @@ func (k Keeper) IterateAndUpdateCctxGasPrice(ctx sdk.Context) error {

// skip if haven't reached epoch end
if ctx.BlockHeight()%gasPriceIncreaseFlags.EpochLength != 0 {
return nil
return 0, gasPriceIncreaseFlags
}

// iterate all chains' pending cctx
chains := common.DefaultChainsList()
cctxCount := 0

IterateChains:
for _, chain := range chains {
res, err := k.CctxListPending(sdk.UnwrapSDKContext(ctx), &types.QueryListCctxPendingRequest{
ChainId: chain.ChainId,
Limit: gasPriceIncreaseFlags.MaxPendingCctxs,
})
if err != nil {
return err
}
// support only external evm chains
if common.IsEVMChain(chain.ChainId) && !common.IsZetaChain(chain.ChainId) {
res, err := k.CctxListPending(sdk.UnwrapSDKContext(ctx), &types.QueryListCctxPendingRequest{
ChainId: chain.ChainId,
Limit: gasPriceIncreaseFlags.MaxPendingCctxs,
})
if err != nil {
ctx.Logger().Info("GasStabilityPool: fetching pending cctx failed",
"chainID", chain.ChainId,
"err", err.Error(),
)
continue IterateChains
}

// iterate through all pending cctx
for _, pendingCctx := range res.CrossChainTx {
if pendingCctx != nil {
_, _, err := k.CheckAndUpdateCctxGasPrice(ctx, *pendingCctx, gasPriceIncreaseFlags)
if err != nil {
return err
// iterate through all pending cctx
for _, pendingCctx := range res.CrossChainTx {
if pendingCctx != nil {
gasPriceIncrease, additionalFees, err := updateFunc(ctx, k, *pendingCctx, gasPriceIncreaseFlags)
if err != nil {
ctx.Logger().Info("GasStabilityPool: updating gas price for pending cctx failed",
"cctxIndex", pendingCctx.Index,
"err", err.Error(),
)
continue IterateChains
kingpinXD marked this conversation as resolved.
Show resolved Hide resolved
}
if !gasPriceIncrease.IsNil() && !gasPriceIncrease.IsZero() {
// Emit typed event for gas price increase
if err := ctx.EventManager().EmitTypedEvent(
&types.EventCCTXGasPriceIncreased{
CctxIndex: pendingCctx.Index,
GasPriceIncrease: gasPriceIncrease.String(),
AdditionalFees: additionalFees.String(),
}); err != nil {
ctx.Logger().Error(
"GasStabilityPool: failed to emit EventCCTXGasPriceIncreased",
"err", err.Error(),
)
}
cctxCount++
}
}
}
}
}

return nil
return cctxCount, gasPriceIncreaseFlags
}

// CheckAndUpdateCctxGasPrice checks if the retry interval is reached and updates the gas price if so
// The function returns the gas price increase and the additional fees paid from the gas stability pool
func (k Keeper) CheckAndUpdateCctxGasPrice(
func CheckAndUpdateCctxGasPrice(
ctx sdk.Context,
k Keeper,
cctx types.CrossChainTx,
flags observertypes.GasPriceIncreaseFlags,
) (math.Uint, math.Uint, error) {
Expand Down Expand Up @@ -108,7 +150,7 @@ func (k Keeper) CheckAndUpdateCctxGasPrice(
if err := k.fungibleKeeper.WithdrawFromGasStabilityPool(ctx, chainID, additionalFees.BigInt()); err != nil {
return math.ZeroUint(), math.ZeroUint(), cosmoserrors.Wrap(
types.ErrNotEnoughFunds,
fmt.Sprintf("cannot withdraw %s from gas stability pool", additionalFees.String()),
fmt.Sprintf("cannot withdraw %s from gas stability pool, error: %s", additionalFees.String(), err.Error()),
)
}

Expand Down
96 changes: 94 additions & 2 deletions x/crosschain/keeper/abci_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,105 @@ import (
"time"

"cosmossdk.io/math"

sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/stretchr/testify/require"
"github.com/zeta-chain/zetacore/common"
testkeeper "github.com/zeta-chain/zetacore/testutil/keeper"
"github.com/zeta-chain/zetacore/testutil/sample"
"github.com/zeta-chain/zetacore/x/crosschain/keeper"
"github.com/zeta-chain/zetacore/x/crosschain/types"
observertypes "github.com/zeta-chain/zetacore/x/observer/types"
)

func TestKeeper_CheckAndUpdateCctxGasPrice(t *testing.T) {
func TestKeeper_IterateAndUpdateCctxGasPrice(t *testing.T) {
k, ctx, _, zk := testkeeper.CrosschainKeeper(t)

// updateFuncMap tracks the calls done with cctx index
updateFuncMap := make(map[string]struct{})

// failMap gives the cctx index that should fail
failMap := make(map[string]struct{})

// updateFunc mocks the update function and keep track of the calls done with cctx index
updateFunc := func(
ctx sdk.Context,
k keeper.Keeper,
cctx types.CrossChainTx,
flags observertypes.GasPriceIncreaseFlags,
) (math.Uint, math.Uint, error) {
if _, ok := failMap[cctx.Index]; ok {
return math.NewUint(0), math.NewUint(0), errors.New("failed")
}

updateFuncMap[cctx.Index] = struct{}{}
return math.NewUint(10), math.NewUint(10), nil
}

// add some evm and non-evm chains
supportedChains := []*common.Chain{
{ChainId: common.EthChain().ChainId},
{ChainId: common.BtcMainnetChain().ChainId},
{ChainId: common.BscMainnetChain().ChainId},
{ChainId: common.ZetaChainMainnet().ChainId},
}

// set pending cctx
tss := sample.Tss()
zk.ObserverKeeper.SetTSS(ctx, tss)
createCctxWithNonceRange(t, ctx, *k, 10, 15, common.EthChain().ChainId, tss, zk)
createCctxWithNonceRange(t, ctx, *k, 20, 25, common.BtcMainnetChain().ChainId, tss, zk)
createCctxWithNonceRange(t, ctx, *k, 30, 35, common.BscMainnetChain().ChainId, tss, zk)
createCctxWithNonceRange(t, ctx, *k, 40, 45, common.ZetaChainMainnet().ChainId, tss, zk)

// set a cctx where the update function should fail to test that the next cctx are not updated but the next chains are
failMap["1-12"] = struct{}{}

// test that the default crosschain flags are used when not set and the epoch length is not reached
ctx = ctx.WithBlockHeight(observertypes.DefaultCrosschainFlags().GasPriceIncreaseFlags.EpochLength + 1)

cctxCount, flags := k.IterateAndUpdateCctxGasPrice(ctx, supportedChains, updateFunc)
require.Equal(t, 0, cctxCount)
require.Equal(t, *observertypes.DefaultCrosschainFlags().GasPriceIncreaseFlags, flags)

// test that custom crosschain flags are used when set and the epoch length is reached
customFlags := observertypes.GasPriceIncreaseFlags{
EpochLength: 100,
RetryInterval: time.Minute * 10,
GasPriceIncreasePercent: 100,
GasPriceIncreaseMax: 200,
MaxPendingCctxs: 10,
}
crosschainFlags := sample.CrosschainFlags()
crosschainFlags.GasPriceIncreaseFlags = &customFlags
zk.ObserverKeeper.SetCrosschainFlags(ctx, *crosschainFlags)

cctxCount, flags = k.IterateAndUpdateCctxGasPrice(ctx, supportedChains, updateFunc)
require.Equal(t, 0, cctxCount)
require.Equal(t, customFlags, flags)

// test that cctx are iterated and updated when the epoch length is reached

ctx = ctx.WithBlockHeight(observertypes.DefaultCrosschainFlags().GasPriceIncreaseFlags.EpochLength * 2)
cctxCount, flags = k.IterateAndUpdateCctxGasPrice(ctx, supportedChains, updateFunc)

// 2 eth + 5 bsc = 7
require.Equal(t, 7, cctxCount)
require.Equal(t, customFlags, flags)

// check that the update function was called with the cctx index
require.Equal(t, 7, len(updateFuncMap))
require.Contains(t, updateFuncMap, "1-10")
require.Contains(t, updateFuncMap, "1-11")

require.Contains(t, updateFuncMap, "56-30")
require.Contains(t, updateFuncMap, "56-31")
require.Contains(t, updateFuncMap, "56-32")
require.Contains(t, updateFuncMap, "56-33")
require.Contains(t, updateFuncMap, "56-34")
}

func TestCheckAndUpdateCctxGasPrice(t *testing.T) {
sampleTimestamp := time.Now()
retryIntervalReached := sampleTimestamp.Add(observertypes.DefaultGasPriceIncreaseFlags.RetryInterval + time.Second)
retryIntervalNotReached := sampleTimestamp.Add(observertypes.DefaultGasPriceIncreaseFlags.RetryInterval - time.Second)
Expand Down Expand Up @@ -282,7 +374,7 @@ func TestKeeper_CheckAndUpdateCctxGasPrice(t *testing.T) {
}

// check and update gas price
gasPriceIncrease, feesPaid, err := k.CheckAndUpdateCctxGasPrice(ctx, tc.cctx, tc.flags)
gasPriceIncrease, feesPaid, err := keeper.CheckAndUpdateCctxGasPrice(ctx, *k, tc.cctx, tc.flags)

if tc.isError {
require.Error(t, err)
Expand Down
20 changes: 10 additions & 10 deletions x/crosschain/keeper/grpc_query_cctx_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,14 @@ func createCctxWithNonceRange(
t *testing.T,
ctx sdk.Context,
k keeper.Keeper,
low int,
high int,
lowPending int,
highPending int,
chainID int64,
tss observertypes.TSS,
zk keepertest.ZetaKeepers,
) (cctxs []*types.CrossChainTx) {
for i := 0; i < low; i++ {
cctx := sample.CrossChainTx(t, fmt.Sprintf("%d", i))
for i := 0; i < lowPending; i++ {
cctx := sample.CrossChainTx(t, fmt.Sprintf("%d-%d", chainID, i))
cctx.CctxStatus.Status = types.CctxStatus_OutboundMined
cctx.InboundTxParams.SenderChainId = chainID
k.SetCrossChainTx(ctx, *cctx)
Expand All @@ -40,8 +40,8 @@ func createCctxWithNonceRange(
Tss: tss.TssPubkey,
})
}
for i := low; i < high; i++ {
cctx := sample.CrossChainTx(t, fmt.Sprintf("%d", i))
for i := lowPending; i < highPending; i++ {
cctx := sample.CrossChainTx(t, fmt.Sprintf("%d-%d", chainID, i))
cctx.CctxStatus.Status = types.CctxStatus_PendingOutbound
cctx.InboundTxParams.SenderChainId = chainID
k.SetCrossChainTx(ctx, *cctx)
Expand All @@ -55,8 +55,8 @@ func createCctxWithNonceRange(
}
zk.ObserverKeeper.SetPendingNonces(ctx, observertypes.PendingNonces{
ChainId: chainID,
NonceLow: int64(low),
NonceHigh: int64(high),
NonceLow: int64(lowPending),
NonceHigh: int64(highPending),
Tss: tss.TssPubkey,
})

Expand Down Expand Up @@ -135,12 +135,12 @@ func TestKeeper_CctxListPending(t *testing.T) {
cctxs := createCctxWithNonceRange(t, ctx, *k, 1000, 2000, chainID, tss, zk)

// set some cctxs as pending below nonce
cctx1, found := k.GetCrossChainTx(ctx, "940")
cctx1, found := k.GetCrossChainTx(ctx, "1337-940")
require.True(t, found)
cctx1.CctxStatus.Status = types.CctxStatus_PendingOutbound
k.SetCrossChainTx(ctx, cctx1)

cctx2, found := k.GetCrossChainTx(ctx, "955")
cctx2, found := k.GetCrossChainTx(ctx, "1337-955")
require.True(t, found)
cctx2.CctxStatus.Status = types.CctxStatus_PendingOutbound
k.SetCrossChainTx(ctx, cctx2)
Expand Down
10 changes: 6 additions & 4 deletions x/crosschain/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -188,10 +188,12 @@ func (AppModule) ConsensusVersion() uint64 { return 4 }

// BeginBlock executes all ABCI BeginBlock logic respective to the crosschain module.
func (am AppModule) BeginBlock(ctx sdk.Context, _ abci.RequestBeginBlock) {
err := am.keeper.IterateAndUpdateCctxGasPrice(ctx)
if err != nil {
ctx.Logger().Info("Error iterating and updating pending cctx gas price", "err", err.Error())
}
// get all supported chains
supportedChains := am.keeper.GetObserverKeeper().GetSupportedChains(ctx)

// iterate and update gas price for cctx that are pending for too long
// error is logged in the function
am.keeper.IterateAndUpdateCctxGasPrice(ctx, supportedChains, keeper.CheckAndUpdateCctxGasPrice)
}

// EndBlock executes all ABCI EndBlock logic respective to the crosschain module. It
Expand Down
Loading
Loading