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

refactor: cctx validate inbound #2340

Merged
merged 51 commits into from
Jun 21, 2024
Merged
Show file tree
Hide file tree
Changes from 47 commits
Commits
Show all changes
51 commits
Select commit Hold shift + click to select a range
4abefdb
move code to validate outbound methods
skosito Jun 4, 2024
28aa31e
refactor
skosito Jun 4, 2024
e6609e5
renaming and moving code around
skosito Jun 5, 2024
2e826a7
fixes
skosito Jun 5, 2024
61a3594
Merge branch 'develop' into cctx-validate-outbound
skosito Jun 5, 2024
6cc517e
refactoring
skosito Jun 5, 2024
e2d3c8b
changelog
skosito Jun 5, 2024
5f91691
make generate
skosito Jun 5, 2024
6cb69cd
fix initiate outbound tests
skosito Jun 5, 2024
47b7b88
fix validate outbound tests
skosito Jun 5, 2024
08dfea1
fix tests
skosito Jun 5, 2024
0bba576
fix e2e test
skosito Jun 5, 2024
20c775e
fix unit tests
skosito Jun 5, 2024
1f153e8
make generate
skosito Jun 5, 2024
455bce5
Merge branch 'develop' into cctx-validate-outbound
skosito Jun 6, 2024
0b5f4be
cleanup
skosito Jun 6, 2024
9394425
try to split initiate and validate outbound for zevm
skosito Jun 6, 2024
4288868
Merge branch 'develop' into cctx-validate-outbound
skosito Jun 6, 2024
992c55b
make generate
skosito Jun 6, 2024
bd125a8
cleanup
skosito Jun 6, 2024
8333985
cleanup
skosito Jun 6, 2024
b497cbc
e2e test fix
skosito Jun 6, 2024
d437a39
PR comment
skosito Jun 7, 2024
f856cd4
Merge branch 'develop' into cctx-validate-outbound
skosito Jun 7, 2024
c683384
fix typo after merge
skosito Jun 7, 2024
a11d1b2
Merge branch 'develop' into cctx-validate-outbound
skosito Jun 10, 2024
16f858f
renaming
skosito Jun 10, 2024
59c6d5f
move validate inbound for observers logic to separate method
skosito Jun 10, 2024
36b38f8
Move cctx gateway outside of crosschain keeper wip
skosito Jun 12, 2024
5583307
cleanup
skosito Jun 12, 2024
ec7465e
changelog
skosito Jun 12, 2024
303c112
make generate
skosito Jun 12, 2024
f675251
lint fixes
skosito Jun 12, 2024
0ef2093
fix evm hooks tests
skosito Jun 12, 2024
a87f4cd
fix init outbound tests
skosito Jun 12, 2024
c89b079
Merge branch 'develop' into cctx-validate-inbound
skosito Jun 12, 2024
1dbfdb9
cleanup
skosito Jun 12, 2024
e7cedab
Merge branch 'develop' into cctx-validate-outbound
skosito Jun 12, 2024
be56c37
Merge branch 'cctx-validate-outbound' into cctx-validate-inbound
skosito Jun 12, 2024
4e468f7
cleanup
skosito Jun 12, 2024
dd56896
PR comments
skosito Jun 13, 2024
0a7234e
Merge branch 'cctx-validate-outbound' into cctx-validate-inbound
skosito Jun 13, 2024
b823a45
fix changelog
skosito Jun 17, 2024
0859a62
Merge branch 'cctx-validate-outbound' into cctx-validate-inbound
skosito Jun 17, 2024
665b2ff
PR comments pt1
skosito Jun 17, 2024
5c49a9c
PR comments pt2
skosito Jun 18, 2024
2c2d7e5
add todo
skosito Jun 18, 2024
86c1f5c
Merge branch 'develop' into cctx-validate-inbound
skosito Jun 19, 2024
ae38f3b
PR comments
skosito Jun 19, 2024
7f3bf86
Merge branch 'develop' into cctx-validate-inbound
skosito Jun 20, 2024
e835137
Merge branch 'develop' into cctx-validate-inbound
skosito Jun 20, 2024
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
9 changes: 0 additions & 9 deletions app/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,6 @@ import (

"github.com/zeta-chain/zetacore/app/ante"
"github.com/zeta-chain/zetacore/docs/openapi"
"github.com/zeta-chain/zetacore/pkg/chains"
zetamempool "github.com/zeta-chain/zetacore/pkg/mempool"
srvflags "github.com/zeta-chain/zetacore/server/flags"
authoritymodule "github.com/zeta-chain/zetacore/x/authority"
Expand Down Expand Up @@ -598,14 +597,6 @@ func New(
app.LightclientKeeper,
)

// initializing map of cctx gateways so crosschain module can decide which one to use
// based on chain info of destination chain
cctxGateways := map[chains.CCTXGateway]crosschainkeeper.CCTXGateway{
chains.CCTXGateway_observers: crosschainkeeper.NewCCTXGatewayObservers(app.CrosschainKeeper),
chains.CCTXGateway_zevm: crosschainkeeper.NewCCTXGatewayZEVM(app.CrosschainKeeper),
}
app.CrosschainKeeper.SetCCTXGateways(cctxGateways)
skosito marked this conversation as resolved.
Show resolved Hide resolved

// initialize ibccrosschain keeper and set it to the crosschain keeper
// there is a circular dependency between the two keepers, crosschain keeper must be initialized first

Expand Down
2 changes: 2 additions & 0 deletions changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@
* [2269](https://github.com/zeta-chain/node/pull/2269) - refactor MsgUpdateCrosschainFlags into MsgEnableCCTX, MsgDisableCCTX and MsgUpdateGasPriceIncreaseFlags
* [2306](https://github.com/zeta-chain/node/pull/2306) - refactor zetaclient outbound transaction signing logic
* [2296](https://github.com/zeta-chain/node/pull/2296) - move `testdata` package to `testutil` to organize test-related utilities
* [2317](https://github.com/zeta-chain/node/pull/2317) - add ValidateOutbound method for cctx orchestrator
* [2340](https://github.com/zeta-chain/node/pull/2340) - add ValidateInbound method for cctx orchestrator

### Tests

Expand Down
8 changes: 0 additions & 8 deletions testutil/keeper/crosschain.go
Original file line number Diff line number Diff line change
Expand Up @@ -174,16 +174,8 @@ func CrosschainKeeperWithMocks(
lightclientKeeper,
)

cctxGateways := map[chains.CCTXGateway]keeper.CCTXGateway{
chains.CCTXGateway_observers: keeper.NewCCTXGatewayObservers(*k),
chains.CCTXGateway_zevm: keeper.NewCCTXGatewayZEVM(*k),
}

k.SetCCTXGateways(cctxGateways)

// initialize ibccrosschain keeper and set it to the crosschain keeper
// there is a circular dependency between the two keepers, crosschain keeper must be initialized first

var ibcCrosschainKeeperTmp types.IBCCrosschainKeeper = initIBCCrosschainKeeper(
cdc,
db,
Expand Down
55 changes: 37 additions & 18 deletions x/crosschain/keeper/cctx_gateway_observers.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
package keeper

import (
"fmt"

sdk "github.com/cosmos/cosmos-sdk/types"

"github.com/zeta-chain/zetacore/pkg/chains"
"github.com/zeta-chain/zetacore/x/crosschain/types"
)

Expand Down Expand Up @@ -32,29 +35,45 @@ InitiateOutbound updates the store so observers can use the PendingCCTX query:
*/
func (c CCTXGatewayObservers) InitiateOutbound(
ctx sdk.Context,
cctx *types.CrossChainTx,
) (newCCTXStatus types.CctxStatus) {
config InitiateOutboundConfig,
skosito marked this conversation as resolved.
Show resolved Hide resolved
lumtis marked this conversation as resolved.
Show resolved Hide resolved
) (newCCTXStatus types.CctxStatus, err error) {
tmpCtx, commit := ctx.CacheContext()
outboundReceiverChainID := cctx.GetCurrentOutboundParam().ReceiverChainId
err := func() error {
err := c.crosschainKeeper.PayGasAndUpdateCctx(
tmpCtx,
outboundReceiverChainID,
cctx,
cctx.InboundParams.Amount,
false,
)
if err != nil {
return err
outboundReceiverChainID := config.CCTX.GetCurrentOutboundParam().ReceiverChainId
// TODO (https://github.com/zeta-chain/node/issues/1010): workaround for this bug
noEthereumTxEvent := false
if chains.IsZetaChain(config.CCTX.InboundParams.SenderChainId) {
noEthereumTxEvent = true
}
lumtis marked this conversation as resolved.
Show resolved Hide resolved

err = func() error {
// If ShouldPayGas flag is set during ValidateInbound PayGasAndUpdateCctx should be called
// which will set GasPrice and Amount. Otherwise, use median gas price and InboundParams amount.
if config.ShouldPayGas {
err := c.crosschainKeeper.PayGasAndUpdateCctx(
tmpCtx,
outboundReceiverChainID,
config.CCTX,
config.CCTX.InboundParams.Amount,
noEthereumTxEvent,
)
if err != nil {
return err
}
} else {
gasPrice, found := c.crosschainKeeper.GetMedianGasPriceInUint(ctx, config.CCTX.GetCurrentOutboundParam().ReceiverChainId)
if !found {
return fmt.Errorf("gasprice not found for %d", config.CCTX.GetCurrentOutboundParam().ReceiverChainId)
}
config.CCTX.GetCurrentOutboundParam().GasPrice = gasPrice.String()
config.CCTX.GetCurrentOutboundParam().Amount = config.CCTX.InboundParams.Amount
}
return c.crosschainKeeper.UpdateNonce(tmpCtx, outboundReceiverChainID, cctx)
return c.crosschainKeeper.SetObserverOutboundInfo(tmpCtx, outboundReceiverChainID, config.CCTX)
}()
if err != nil {
// do not commit anything here as the CCTX should be aborted
cctx.SetAbort(err.Error())
return types.CctxStatus_Aborted
config.CCTX.SetAbort(err.Error())
return types.CctxStatus_Aborted, err
}
commit()
cctx.SetPendingOutbound("")
return types.CctxStatus_PendingOutbound
return types.CctxStatus_PendingOutbound, nil
}
87 changes: 14 additions & 73 deletions x/crosschain/keeper/cctx_gateway_zevm.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
package keeper

import (
"fmt"

sdk "github.com/cosmos/cosmos-sdk/types"

"github.com/zeta-chain/zetacore/x/crosschain/types"
Expand All @@ -20,81 +18,24 @@ func NewCCTXGatewayZEVM(crosschainKeeper Keeper) CCTXGatewayZEVM {
}
}

/*
InitiateOutbound handles evm deposit and call ValidateOutbound.
TODO (https://github.com/zeta-chain/node/issues/2278): move remaining of this comment to ValidateOutbound once it's added.

- If the deposit is successful, the CCTX status is changed to OutboundMined.

- If the deposit returns an internal error i.e if HandleEVMDeposit() returns an error, but isContractReverted is false, the CCTX status is changed to Aborted.

- If the deposit is reverted, the function tries to create a revert cctx with status PendingRevert.

- If the creation of revert tx also fails it changes the status to Aborted.

Note : Aborted CCTXs are not refunded in this function. The refund is done using a separate refunding mechanism.
We do not return an error from this function , as all changes need to be persisted to the state.
Instead we use a temporary context to make changes and then commit the context on for the happy path ,i.e cctx is set to OutboundMined.
New CCTX status after preprocessing is returned.
*/
func (c CCTXGatewayZEVM) InitiateOutbound(ctx sdk.Context, cctx *types.CrossChainTx) (newCCTXStatus types.CctxStatus) {
// InitiateOutbound handles evm deposit and immediately validates pending outbound
func (c CCTXGatewayZEVM) InitiateOutbound(
ctx sdk.Context,
config InitiateOutboundConfig,
) (newCCTXStatus types.CctxStatus, err error) {
tmpCtx, commit := ctx.CacheContext()
isContractReverted, err := c.crosschainKeeper.HandleEVMDeposit(tmpCtx, cctx)
isContractReverted, err := c.crosschainKeeper.HandleEVMDeposit(tmpCtx, config.CCTX)

// TODO (https://github.com/zeta-chain/node/issues/2278): further processing will be in validateOutbound(...), for now keeping it here
if err != nil && !isContractReverted {
// exceptional case; internal error; should abort CCTX
cctx.SetAbort(err.Error())
return types.CctxStatus_Aborted
} else if err != nil && isContractReverted {
// contract call reverted; should refund via a revert tx
revertMessage := err.Error()
senderChain := c.crosschainKeeper.zetaObserverKeeper.GetSupportedChainFromChainID(ctx, cctx.InboundParams.SenderChainId)
if senderChain == nil {
cctx.SetAbort(fmt.Sprintf("invalid sender chain id %d", cctx.InboundParams.SenderChainId))
return types.CctxStatus_Aborted
}
gasLimit, err := c.crosschainKeeper.GetRevertGasLimit(ctx, *cctx)
if err != nil {
cctx.SetAbort(fmt.Sprintf("revert gas limit error: %s", err.Error()))
return types.CctxStatus_Aborted
}
if gasLimit == 0 {
// use same gas limit of outbound as a fallback -- should not be required
gasLimit = cctx.GetCurrentOutboundParam().GasLimit
}
config.CCTX.SetAbort(err.Error())
return types.CctxStatus_Aborted, err
}

err = cctx.AddRevertOutbound(gasLimit)
if err != nil {
cctx.SetAbort(fmt.Sprintf("revert outbound error: %s", err.Error()))
return types.CctxStatus_Aborted
}
// we create a new cached context, and we don't commit the previous one with EVM deposit
tmpCtxRevert, commitRevert := ctx.CacheContext()
err = func() error {
err := c.crosschainKeeper.PayGasAndUpdateCctx(
tmpCtxRevert,
senderChain.ChainId,
cctx,
cctx.InboundParams.Amount,
false,
)
if err != nil {
return err
}
// Update nonce using senderchain id as this is a revert tx and would go back to the original sender
return c.crosschainKeeper.UpdateNonce(tmpCtxRevert, senderChain.ChainId, cctx)
}()
if err != nil {
cctx.SetAbort(fmt.Sprintf("deposit revert message: %s err : %s", revertMessage, err.Error()))
return types.CctxStatus_Aborted
}
commitRevert()
cctx.SetPendingRevert(revertMessage)
return types.CctxStatus_PendingRevert
newCCTXStatus = c.crosschainKeeper.ValidateOutboundZEVM(ctx, config.CCTX, err, isContractReverted)
if newCCTXStatus == types.CctxStatus_OutboundMined {
commit()
}
// successful HandleEVMDeposit;
commit()
cctx.SetOutboundMined("Remote omnichain contract call completed")
return types.CctxStatus_OutboundMined

return newCCTXStatus, nil
}
29 changes: 29 additions & 0 deletions x/crosschain/keeper/cctx_gateways.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
package keeper

import (
sdk "github.com/cosmos/cosmos-sdk/types"

"github.com/zeta-chain/zetacore/pkg/chains"
"github.com/zeta-chain/zetacore/x/crosschain/types"
)

// CCTXGateway is interface implemented by every gateway. It is one of interfaces used for communication
// between CCTX gateways and crosschain module, and it is called by crosschain module.
type CCTXGateway interface {
// Initiate a new outbound, this tells the CCTXGateway to carry out the action to execute the outbound.
// It is the only entry point to initiate an outbound and it returns new CCTX status after it is completed.
InitiateOutbound(ctx sdk.Context, config InitiateOutboundConfig) (newCCTXStatus types.CctxStatus, err error)
}

var cctxGateways map[chains.CCTXGateway]CCTXGateway

// ResolveCCTXGateway respolves cctx gateway implementation based on provided cctx gateway
func ResolveCCTXGateway(c chains.CCTXGateway, keeper Keeper) (CCTXGateway, bool) {
cctxGateways = map[chains.CCTXGateway]CCTXGateway{
chains.CCTXGateway_observers: NewCCTXGatewayObservers(keeper),
chains.CCTXGateway_zevm: NewCCTXGatewayZEVM(keeper),
}

cctxGateway, ok := cctxGateways[c]
return cctxGateway, ok
}
50 changes: 50 additions & 0 deletions x/crosschain/keeper/cctx_orchestrator_validate_inbound.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
package keeper

import (
sdk "github.com/cosmos/cosmos-sdk/types"

"github.com/zeta-chain/zetacore/x/crosschain/types"
observertypes "github.com/zeta-chain/zetacore/x/observer/types"
)

// ValidateInbound is the only entry-point to create new CCTX (eg. when observers voting is done or new inbound event is detected).
// It creates new CCTX object and calls InitiateOutbound method.
func (k Keeper) ValidateInbound(
skosito marked this conversation as resolved.
Show resolved Hide resolved
ctx sdk.Context,
msg *types.MsgVoteInbound,
shouldPayGas bool,
) (*types.CrossChainTx, error) {
tss, tssFound := k.zetaObserverKeeper.GetTSS(ctx)
if !tssFound {
return nil, types.ErrCannotFindTSSKeys
}

// Do not process if inbound is disabled
if !k.zetaObserverKeeper.IsInboundEnabled(ctx) {
return nil, observertypes.ErrInboundDisabled
}

// create a new CCTX from the inbound message. The status of the new CCTX is set to PendingInbound.
cctx, err := types.NewCCTX(ctx, *msg, tss.TssPubkey)
if err != nil {
return nil, err

Check warning on line 30 in x/crosschain/keeper/cctx_orchestrator_validate_inbound.go

View check run for this annotation

Codecov / codecov/patch

x/crosschain/keeper/cctx_orchestrator_validate_inbound.go#L30

Added line #L30 was not covered by tests
}
skosito marked this conversation as resolved.
Show resolved Hide resolved

// Initiate outbound, the process function manages the state commit and cctx status change.
// If the process fails, the changes to the evm state are rolled back.
_, err = k.InitiateOutbound(ctx, InitiateOutboundConfig{
CCTX: &cctx,
ShouldPayGas: shouldPayGas,
})
if err != nil {
return nil, err
}

inCctxIndex, ok := ctx.Value("inCctxIndex").(string)
skosito marked this conversation as resolved.
Show resolved Hide resolved
if ok {
cctx.InboundParams.ObservedHash = inCctxIndex

Check warning on line 45 in x/crosschain/keeper/cctx_orchestrator_validate_inbound.go

View check run for this annotation

Codecov / codecov/patch

x/crosschain/keeper/cctx_orchestrator_validate_inbound.go#L45

Added line #L45 was not covered by tests
}
k.SetCctxAndNonceToCctxAndInboundHashToCctx(ctx, cctx)

return &cctx, nil
}
Loading
Loading