chore: add tests for bridge module (#381)

* chore: add tests for bridge module

* refactor sample methods

.github/workflows/e2e-test.yml → .github/workflows/tests.yml

@@ -1,4 +1,4 @@
-name: End to End Test
+name: Tests / Code Coverage
 
 on:
   push:
@@ -9,7 +9,7 @@ on:
   pull_request:
 
 jobs:
-  end-to-end-test:
+  tests:
     strategy:
       matrix:
         go-version: [1.20.x]
@@ -44,22 +44,22 @@ jobs:
           ${{ runner.os }}-go-
     - name: Setup GitHub Token
       run: git config --global url.https://[email protected]/.insteadOf https://github.com/
-    - name: run e2e test
+    - name: run tests
       run: |
-        make e2e_test
+        make test
     - name: make coverage report
       id: coverage-report
       if: github.event_name == 'pull_request'
       continue-on-error: true
       run: |
-        echo '## E2E Test Coverage Report' >> coverage-report.txt
+        echo '## Test Coverage Report' >> coverage-report.txt
         echo 'commit-id: ${{ github.event.pull_request.head.sha }}' >> coverage-report.txt
         echo 'generated by https://github.com/vladopajic/go-test-coverage' >> coverage-report.txt
         echo >> coverage-report.txt
         echo '<details><summary>Additional details and impacted files</summary>' >> coverage-report.txt
         echo >> coverage-report.txt
         echo '```' >> coverage-report.txt
-        make check-e2e-coverage >> coverage-report.txt
+        make check-coverage >> coverage-report.txt
         echo '```' >> coverage-report.txt
         echo >> coverage-report.txt
         echo '</details>' >> coverage-report.txt

.testcoverage.yml

@@ -34,9 +34,15 @@ exclude:
     - \.pb\.gw\.go$    # excludes all protobuf generated files
     - .*_mocks.go$    # excludes all protobuf generated files
     - testutil/.*
-    - e2e/.* 
+    - e2e/.*
+    - types/.*
+    - sdk/.*
     - internal/sequence/.*
-
+    - x/types/.*
+    - .*/simulation/.*
+    - .*/module.go
+    - .*/module_simulation.go
+
 # NOTES:
 # - symbol `/` in all path regexps will be replaced by
-#   current OS file path separator to properly work on Windows
+#   current OS file path separator to properly work on Windows

Makefile

@@ -48,20 +48,23 @@ docker-image:
 	go mod vendor # temporary, should be removed after open source
 	docker build . -t ${IMAGE_NAME}
 
-test:
+unit_test:
 	go test -failfast $$(go list ./... | grep -v e2e | grep -v sdk)
 
 e2e_init_localchain: build
 	bash ./deployment/localup/localup.sh init 1 7
 	bash ./deployment/localup/localup.sh generate 1 7
 
 e2e_test: e2e_init_localchain
-	go test -p 1 -failfast -v ./e2e/... -coverpkg=./... -covermode=atomic -coverprofile=./coverage.out -timeout 99999s
+	go test -p 1 -failfast -v ./e2e/... -timeout 99999s
+
+test: e2e_init_localchain
+	go test -p 1 -failfast -v $$(go list ./... | grep -v sdk) -coverpkg=./... -covermode=atomic -coverprofile=./coverage.out -timeout 99999s
 
 install-go-test-coverage:
 	@go install github.com/vladopajic/go-test-coverage/v2@latest
 
-check-e2e-coverage: install-go-test-coverage
+check-coverage: install-go-test-coverage
 	@go-test-coverage --config=./.testcoverage.yml || true
 
 lint:

e2e/tests/bridge_test.go

@@ -8,9 +8,12 @@ import (
 	"time"
 
 	sdkmath "cosmossdk.io/math"
+	sdkClient "github.com/cosmos/cosmos-sdk/client"
 	"github.com/cosmos/cosmos-sdk/types"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/cosmos/cosmos-sdk/types/tx"
+	"github.com/cosmos/cosmos-sdk/types/tx/signing"
+	authtx "github.com/cosmos/cosmos-sdk/x/auth/tx"
 	authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
 	banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
 	crosschaintypes "github.com/cosmos/cosmos-sdk/x/crosschain/types"
@@ -23,6 +26,7 @@ import (
 	"github.com/bnb-chain/greenfield/e2e/core"
 	gnfdtypes "github.com/bnb-chain/greenfield/sdk/types"
 	types2 "github.com/bnb-chain/greenfield/sdk/types"
+	"github.com/bnb-chain/greenfield/x/bridge/client/cli"
 	bridgetypes "github.com/bnb-chain/greenfield/x/bridge/types"
 )
 
@@ -36,6 +40,44 @@ func (s *BridgeTestSuite) SetupSuite() {
 
 func (s *BridgeTestSuite) SetupTest() {}
 
+func (s *BridgeTestSuite) TestCliQuery() {
+	ctx := context.Background()
+	cliCtx := &sdkClient.Context{Client: s.TmClient.TmClient, Codec: s.Client.GetCodec()}
+	ctx = context.WithValue(ctx, sdkClient.ClientContextKey, cliCtx)
+	queryCmd := cli.GetQueryCmd()
+
+	// query params
+	queryCmd.SetArgs([]string{"params"})
+	s.Require().NoError(queryCmd.ExecuteContext(ctx))
+}
+
+func (s *BridgeTestSuite) TestCliTx() {
+	ctx := context.Background()
+	txCfg := authtx.NewTxConfig(s.Client.GetCodec(), []signing.SignMode{signing.SignMode_SIGN_MODE_EIP_712})
+	cliCtx := &sdkClient.Context{
+		FromAddress:       s.Validator.GetAddr(),
+		Client:            s.TmClient.TmClient,
+		InterfaceRegistry: s.Client.GetCodec().InterfaceRegistry(),
+		Codec:             s.Client.GetCodec(),
+		From:              s.Validator.String(),
+		AccountRetriever:  authtypes.AccountRetriever{},
+		ChainID:           s.Config.ChainId,
+		TxConfig:          txCfg,
+		SkipConfirm:       true,
+		Simulate:          true,
+	}
+	ctx = context.WithValue(ctx, sdkClient.ClientContextKey, cliCtx)
+	txCmd := cli.GetTxCmd()
+
+	// wrong to address
+	txCmd.SetArgs([]string{"transfer-out", "test", "1000000000000000000BNB"})
+	s.Require().Contains(txCmd.ExecuteContext(ctx).Error(), "invalid address hex length")
+
+	// tx transfer-out
+	txCmd.SetArgs([]string{"transfer-out", sdk.AccAddress("test").String(), "1000000000000000000BNB"})
+	s.Require().NoError(txCmd.ExecuteContext(ctx))
+}
+
 func (s *BridgeTestSuite) TestTransferOut() {
 	users := s.GenAndChargeAccounts(2, 1000000)
 

testutil/sample/sample.go

@@ -5,20 +5,28 @@ import (
 	"encoding/hex"
 
 	"github.com/cometbft/cometbft/crypto/tmhash"
-	"github.com/cosmos/cosmos-sdk/crypto/keys/ed25519"
 	"github.com/cosmos/cosmos-sdk/crypto/keys/eth/ethsecp256k1"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/prysmaticlabs/prysm/crypto/bls"
 )
 
-// AccAddress returns a sample account address
-func AccAddress() string {
-	pk := ed25519.GenPrivKey().PubKey()
-	addr := pk.Address()
-	return sdk.AccAddress(addr).String()
+func RandAccAddress() sdk.AccAddress {
+	pk, err := ethsecp256k1.GenPrivKey()
+	if err != nil {
+		panic(err)
+	}
+	return sdk.AccAddress(pk.PubKey().Address())
+}
+
+func RandAccAddressHex() string {
+	pk, err := ethsecp256k1.GenPrivKey()
+	if err != nil {
+		panic(err)
+	}
+	return sdk.AccAddress(pk.PubKey().Address()).String()
 }
 
-func RandSignBytes() (addr sdk.AccAddress, signBytes []byte, sig []byte) {
+func RandSignBytes() (addr sdk.AccAddress, signBytes, sig []byte) {
 	signBytes = RandStr(256)
 	privKey, _ := ethsecp256k1.GenPrivKey()
 
@@ -28,12 +36,6 @@ func RandSignBytes() (addr sdk.AccAddress, signBytes []byte, sig []byte) {
 	return addr, signBytes, sig
 }
 
-func RandAccAddress() sdk.AccAddress {
-	pk := ed25519.GenPrivKey().PubKey()
-	addr := pk.Address()
-	return sdk.AccAddress(addr)
-}
-
 func Checksum() []byte {
 	return sdk.Keccak256(RandStr(256))
 }

types/grn_test.go

@@ -17,7 +17,7 @@ func TestGRNBasic(t *testing.T) {
 	var grn types3.GRN
 	testBucketName := storageutils.GenRandomBucketName()
 	testObjectName := storageutils.GenRandomObjectName()
-	testAcc := sample.AccAddress()
+	testAcc := sample.RandAccAddressHex()
 	testGroupName := storageutils.GenRandomGroupName()
 
 	err := grn.ParseFromString("grn:b::"+testBucketName, false)

x/bridge/client/cli/query.go

@@ -10,7 +10,7 @@ import (
 )
 
 // GetQueryCmd returns the cli query commands for this module
-func GetQueryCmd(queryRoute string) *cobra.Command {
+func GetQueryCmd() *cobra.Command {
 	// Group bridge queries under a subcommand
 	cmd := &cobra.Command{
 		Use:                        types.ModuleName,

x/bridge/keeper/cross_app.go

@@ -2,11 +2,9 @@ package keeper
 
 import (
 	"encoding/hex"
-	"math/big"
 
 	"cosmossdk.io/errors"
 	sdk "github.com/cosmos/cosmos-sdk/types"
-	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 	crosschaintypes "github.com/cosmos/cosmos-sdk/x/crosschain/types"
 
 	"github.com/bnb-chain/greenfield/x/bridge/types"
@@ -38,17 +36,6 @@ func NewTransferOutApp(keeper Keeper) *TransferOutApp {
 	}
 }
 
-func (app *TransferOutApp) CheckPackage(refundPackage *types.TransferOutRefundPackage) error {
-	if refundPackage.RefundAddr.Empty() {
-		return errors.Wrapf(sdkerrors.ErrInvalidAddress, "refund address is empty")
-	}
-
-	if refundPackage.RefundAmount.Cmp(big.NewInt(0)) < 0 {
-		return errors.Wrapf(types.ErrInvalidAmount, "amount to refund should not be negative")
-	}
-	return nil
-}
-
 func (app *TransferOutApp) ExecuteAckPackage(ctx sdk.Context, appCtx *sdk.CrossChainAppContext, payload []byte) sdk.ExecuteResult {
 	if len(payload) == 0 {
 		return sdk.ExecuteResult{}
@@ -58,22 +45,14 @@ func (app *TransferOutApp) ExecuteAckPackage(ctx sdk.Context, appCtx *sdk.CrossC
 
 	refundPackage, err := types.DeserializeTransferOutRefundPackage(payload)
 	if err != nil {
-		app.bridgeKeeper.Logger(ctx).Error("unmarshal transfer out refund claim error", "err", err.Error(), "claim", hex.EncodeToString(payload))
-		return sdk.ExecuteResult{
-			Err: err,
-		}
-	}
-
-	err = app.CheckPackage(refundPackage)
-	if err != nil {
-		app.bridgeKeeper.Logger(ctx).Error("check transfer out refund package error", "err", err.Error(), "claim", hex.EncodeToString(payload))
+		app.bridgeKeeper.Logger(ctx).Error("decode transfer out refund claim error", "err", err.Error(), "claim", hex.EncodeToString(payload))
 		return sdk.ExecuteResult{
 			Err: err,
 		}
 	}
 
 	denom := app.bridgeKeeper.stakingKeeper.BondDenom(ctx) // only support native token so far
-	err = app.bridgeKeeper.bankKeeper.SendCoinsFromModuleToAccount(ctx, crosschaintypes.ModuleName, refundPackage.RefundAddr,
+	err = app.bridgeKeeper.bankKeeper.SendCoinsFromModuleToAccount(ctx, crosschaintypes.ModuleName, refundPackage.RefundAddress,
 		sdk.Coins{
 			sdk.Coin{
 				Denom:  denom,
@@ -89,7 +68,7 @@ func (app *TransferOutApp) ExecuteAckPackage(ctx sdk.Context, appCtx *sdk.CrossC
 	}
 
 	err = ctx.EventManager().EmitTypedEvent(&types.EventCrossTransferOutRefund{
-		RefundAddress: refundPackage.RefundAddr.String(),
+		RefundAddress: refundPackage.RefundAddress.String(),
 		Amount: &sdk.Coin{
 			Denom:  denom,
 			Amount: sdk.NewIntFromBigInt(refundPackage.RefundAmount),
@@ -116,12 +95,6 @@ func (app *TransferOutApp) ExecuteFailAckPackage(ctx sdk.Context, appCtx *sdk.Cr
 		}
 	}
 
-	if transferOutPackage.Amount.Cmp(big.NewInt(0)) < 0 {
-		return sdk.ExecuteResult{
-			Err: errors.Wrapf(types.ErrInvalidAmount, "amount to refund should not be negative"),
-		}
-	}
-
 	denom := app.bridgeKeeper.stakingKeeper.BondDenom(ctx) // only support native token so far
 	err = app.bridgeKeeper.bankKeeper.SendCoinsFromModuleToAccount(ctx, crosschaintypes.ModuleName, transferOutPackage.RefundAddress,
 		sdk.Coins{
@@ -173,22 +146,6 @@ func NewTransferInApp(bridgeKeeper Keeper) *TransferInApp {
 	}
 }
 
-func (app *TransferInApp) CheckTransferInSynPackage(transferInPackage *types.TransferInSynPackage) error {
-	if transferInPackage.Amount.Cmp(big.NewInt(0)) < 0 {
-		return errors.Wrapf(types.ErrInvalidAmount, "amount should not be negative")
-	}
-
-	if transferInPackage.ReceiverAddress.Empty() {
-		return errors.Wrapf(sdkerrors.ErrInvalidAddress, "receiver address should not be empty")
-	}
-
-	if transferInPackage.RefundAddress.Empty() {
-		return errors.Wrapf(types.ErrInvalidAddress, "refund address should not be empty")
-	}
-
-	return nil
-}
-
 func (app *TransferInApp) ExecuteAckPackage(ctx sdk.Context, header *sdk.CrossChainAppContext, payload []byte) sdk.ExecuteResult {
 	app.bridgeKeeper.Logger(ctx).Error("received transfer in ack package", "payload", hex.EncodeToString(payload))
 	return sdk.ExecuteResult{}
@@ -202,14 +159,8 @@ func (app *TransferInApp) ExecuteFailAckPackage(ctx sdk.Context, header *sdk.Cro
 func (app *TransferInApp) ExecuteSynPackage(ctx sdk.Context, appCtx *sdk.CrossChainAppContext, payload []byte) sdk.ExecuteResult {
 	transferInPackage, err := types.DeserializeTransferInSynPackage(payload)
 	if err != nil {
-		app.bridgeKeeper.Logger(ctx).Error("unmarshal transfer in claim error", "err", err.Error(), "claim", string(payload))
-		panic("unmarshal transfer in claim error")
-	}
-
-	err = app.CheckTransferInSynPackage(transferInPackage)
-	if err != nil {
-		app.bridgeKeeper.Logger(ctx).Error("check transfer in package error", "err", err.Error(), "claim", string(payload))
-		panic(err)
+		app.bridgeKeeper.Logger(ctx).Error("decode transfer in claim error", "err", err.Error(), "claim", string(payload))
+		panic("decode transfer in claim error")
 	}
 
 	denom := app.bridgeKeeper.stakingKeeper.BondDenom(ctx)
@@ -218,14 +169,14 @@ func (app *TransferInApp) ExecuteSynPackage(ctx sdk.Context, appCtx *sdk.CrossCh
 	err = app.bridgeKeeper.bankKeeper.SendCoinsFromModuleToAccount(ctx, crosschaintypes.ModuleName, transferInPackage.ReceiverAddress, sdk.Coins{amount})
 	if err != nil {
 		app.bridgeKeeper.Logger(ctx).Error("send coins error", "err", err.Error())
-		refundPackage, err := app.bridgeKeeper.GetRefundTransferInPayload(transferInPackage, uint32(types.REFUND_REASON_INSUFFICIENT_BALANCE))
-		if err != nil {
-			app.bridgeKeeper.Logger(ctx).Error("get refund transfer in payload error", "err", err.Error())
-			panic(err)
+		refundPackage, refundErr := app.bridgeKeeper.GetRefundTransferInPayload(transferInPackage, uint32(types.REFUND_REASON_INSUFFICIENT_BALANCE))
+		if refundErr != nil {
+			app.bridgeKeeper.Logger(ctx).Error("get refund transfer in payload error", "err", refundErr.Error())
+			panic(refundErr)
 		}
 		return sdk.ExecuteResult{
 			Payload: refundPackage,
-			Err:     errors.Wrapf(types.ErrInvalidLength, "balance of cross chain module is insufficient"),
+			Err:     errors.Wrapf(types.ErrInvalidPackage, "send coins error: %s", err.Error()),
 		}
 	}