From b9ff23d4484cdf3673b953a0e01800a79a620e46 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?nina=20/=20=E1=83=9C=E1=83=98=E1=83=9C=E1=83=90?= Date: Wed, 28 Aug 2024 17:41:07 +0200 Subject: [PATCH] chore: tx status forward port (#3816) ## Overview Forward-porting some work that was only addressed on v2.x branch --------- Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> --- app/test/big_blob_test.go | 5 ++- app/test/std_sdk_test.go | 16 +++++--- pkg/user/tx_client.go | 66 +++++++++++++++++++++++--------- pkg/user/tx_client_test.go | 7 ++-- x/blobstream/integration_test.go | 16 +++++--- x/signal/legacy_test.go | 23 ++++++----- 6 files changed, 89 insertions(+), 44 deletions(-) diff --git a/app/test/big_blob_test.go b/app/test/big_blob_test.go index ecb257031f..cede1ae65d 100644 --- a/app/test/big_blob_test.go +++ b/app/test/big_blob_test.go @@ -82,8 +82,9 @@ func (s *BigBlobSuite) TestErrBlobsTooLarge() { defer cancel() res, err := txClient.SubmitPayForBlob(subCtx, []*share.Blob{tc.blob}, user.SetGasLimitAndGasPrice(1e9, appconsts.DefaultMinGasPrice)) require.Error(t, err) - require.NotNil(t, res) - require.Equal(t, tc.want, res.Code, err.Error()) + require.Nil(t, res) + code := err.(*user.BroadcastTxError).Code + require.Equal(t, tc.want, code, err.Error()) }) } } diff --git a/app/test/std_sdk_test.go b/app/test/std_sdk_test.go index 8d3e02aa3d..75eb932b3f 100644 --- a/app/test/std_sdk_test.go +++ b/app/test/std_sdk_test.go @@ -317,20 +317,26 @@ func (s *StandardSDKIntegrationTestSuite) TestStandardSDK() { // sign and submit the transactions for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + serviceClient := sdktx.NewServiceClient(s.cctx.GRPCClient) msgs, signer := tt.msgFunc() txClient, err := user.SetupTxClient(s.cctx.GoContext(), s.cctx.Keyring, s.cctx.GRPCClient, s.ecfg, user.WithDefaultAccount(signer)) require.NoError(t, err) res, err := txClient.SubmitTx(s.cctx.GoContext(), msgs, blobfactory.DefaultTxOpts()...) if tt.expectedCode != abci.CodeTypeOK { require.Error(t, err) + require.Nil(t, res) + txHash := err.(*user.ExecutionError).TxHash + code := err.(*user.ExecutionError).Code + getTxResp, err := serviceClient.GetTx(s.cctx.GoContext(), &sdktx.GetTxRequest{Hash: txHash}) + require.NoError(t, err) + assert.Equal(t, tt.expectedCode, code, getTxResp.TxResponse.RawLog) } else { require.NoError(t, err) + require.NotNil(t, res) + getTxResp, err := serviceClient.GetTx(s.cctx.GoContext(), &sdktx.GetTxRequest{Hash: res.TxHash}) + require.NoError(t, err) + assert.Equal(t, tt.expectedCode, res.Code, getTxResp.TxResponse.RawLog) } - serviceClient := sdktx.NewServiceClient(s.cctx.GRPCClient) - getTxResp, err := serviceClient.GetTx(s.cctx.GoContext(), &sdktx.GetTxRequest{Hash: res.TxHash}) - require.NoError(t, err) - require.NotNil(t, res) - assert.Equal(t, tt.expectedCode, res.Code, getTxResp.TxResponse.RawLog) }) } } diff --git a/pkg/user/tx_client.go b/pkg/user/tx_client.go index b2c5b750a3..a403a91991 100644 --- a/pkg/user/tx_client.go +++ b/pkg/user/tx_client.go @@ -22,6 +22,7 @@ import ( sdktx "github.com/cosmos/cosmos-sdk/types/tx" paramtypes "github.com/cosmos/cosmos-sdk/x/params/types/proposal" abci "github.com/tendermint/tendermint/abci/types" + "github.com/tendermint/tendermint/rpc/core" "google.golang.org/grpc" "github.com/celestiaorg/celestia-app/v3/app" @@ -31,7 +32,6 @@ import ( "github.com/celestiaorg/celestia-app/v3/pkg/appconsts" "github.com/celestiaorg/celestia-app/v3/x/blob/types" "github.com/celestiaorg/celestia-app/v3/x/minfee" - "github.com/tendermint/tendermint/rpc/core" ) const ( @@ -44,11 +44,36 @@ type Option func(client *TxClient) // TxResponse is a response from the chain after // a transaction has been submitted. type TxResponse struct { + // Height is the block height at which the transaction was included on-chain. Height int64 TxHash string Code uint32 } +// BroadcastTxError is an error that occurs when broadcasting a transaction. +type BroadcastTxError struct { + TxHash string + Code uint32 + // ErrorLog is the error output of the app's logger + ErrorLog string +} + +func (e *BroadcastTxError) Error() string { + return fmt.Sprintf("broadcast tx error: %s", e.ErrorLog) +} + +// ExecutionError is an error that occurs when a transaction gets executed. +type ExecutionError struct { + TxHash string + Code uint32 + // ErrorLog is the error output of the app's logger + ErrorLog string +} + +func (e *ExecutionError) Error() string { + return fmt.Sprintf("tx execution failed with code %d: %s", e.Code, e.ErrorLog) +} + // WithGasMultiplier is a functional option allows to configure the gas multiplier. func WithGasMultiplier(multiplier float64) Option { return func(c *TxClient) { @@ -213,7 +238,7 @@ func SetupTxClient( func (client *TxClient) SubmitPayForBlob(ctx context.Context, blobs []*share.Blob, opts ...TxOption) (*TxResponse, error) { resp, err := client.BroadcastPayForBlob(ctx, blobs, opts...) if err != nil { - return parseTxResponse(resp, fmt.Errorf("failed to broadcast pay for blob: %v", err)) + return nil, err } return client.ConfirmTx(ctx, resp.TxHash) @@ -224,7 +249,7 @@ func (client *TxClient) SubmitPayForBlob(ctx context.Context, blobs []*share.Blo func (client *TxClient) SubmitPayForBlobWithAccount(ctx context.Context, account string, blobs []*share.Blob, opts ...TxOption) (*TxResponse, error) { resp, err := client.BroadcastPayForBlobWithAccount(ctx, account, blobs, opts...) if err != nil { - return parseTxResponse(resp, fmt.Errorf("failed to broadcast pay for blob with account: %v", err)) + return nil, err } return client.ConfirmTx(ctx, resp.TxHash) @@ -268,7 +293,7 @@ func (client *TxClient) BroadcastPayForBlobWithAccount(ctx context.Context, acco func (client *TxClient) SubmitTx(ctx context.Context, msgs []sdktypes.Msg, opts ...TxOption) (*TxResponse, error) { resp, err := client.BroadcastTx(ctx, msgs, opts...) if err != nil { - return parseTxResponse(resp, fmt.Errorf("failed to broadcast tx: %v", err)) + return nil, err } return client.ConfirmTx(ctx, resp.TxHash) @@ -354,7 +379,12 @@ func (client *TxClient) broadcastTx(ctx context.Context, txBytes []byte, signer } return client.retryBroadcastingTx(ctx, txBytes) } - return resp.TxResponse, fmt.Errorf("tx failed with code %d: %s", resp.TxResponse.Code, resp.TxResponse.RawLog) + broadcastTxErr := &BroadcastTxError{ + TxHash: resp.TxResponse.TxHash, + Code: resp.TxResponse.Code, + ErrorLog: resp.TxResponse.RawLog, + } + return resp.TxResponse, broadcastTxErr } // after the transaction has been submitted, we can increment the @@ -435,16 +465,16 @@ func (client *TxClient) ConfirmTx(ctx context.Context, txHash string) (*TxRespon for { resp, err := txClient.TxStatus(ctx, &tx.TxStatusRequest{TxId: txHash}) if err != nil { - return &TxResponse{}, err + return nil, err } - if err == nil && resp != nil { + if resp != nil { switch resp.Status { case core.TxStatusPending: // Continue polling if the transaction is still pending select { case <-ctx.Done(): - return &TxResponse{}, ctx.Err() + return nil, ctx.Err() case <-pollTicker.C: continue } @@ -454,14 +484,19 @@ func (client *TxClient) ConfirmTx(ctx context.Context, txHash string) (*TxRespon TxHash: txHash, Code: resp.ExecutionCode, } - if resp.ExecutionCode != 0 { - return txResponse, fmt.Errorf("tx was committed but failed with code %d: %s", resp.ExecutionCode, resp.Error) + if resp.ExecutionCode != abci.CodeTypeOK { + executionErr := &ExecutionError{ + TxHash: txHash, + Code: resp.ExecutionCode, + ErrorLog: resp.Error, + } + return nil, executionErr } return txResponse, nil case core.TxStatusEvicted: - return &TxResponse{TxHash: txHash}, fmt.Errorf("tx: %s was evicted from the mempool", txHash) + return nil, fmt.Errorf("tx was evicted from the mempool") default: - return &TxResponse{}, fmt.Errorf("unknown tx: %s", txHash) + return nil, fmt.Errorf("unknown tx: %s", txHash) } } } @@ -569,13 +604,6 @@ func (client *TxClient) getAccountNameFromMsgs(msgs []sdktypes.Msg) (string, err return record.Name, nil } -func parseTxResponse(resp *sdktypes.TxResponse, err error) (*TxResponse, error) { - if resp != nil { - return &TxResponse{Code: resp.Code, TxHash: resp.TxHash}, err - } - return &TxResponse{}, err -} - // Signer exposes the tx clients underlying signer func (client *TxClient) Signer() *Signer { return client.signer diff --git a/pkg/user/tx_client_test.go b/pkg/user/tx_client_test.go index c416ba2d8e..1588c3a060 100644 --- a/pkg/user/tx_client_test.go +++ b/pkg/user/tx_client_test.go @@ -7,6 +7,7 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" sdktx "github.com/cosmos/cosmos-sdk/types/tx" + "github.com/cosmos/cosmos-sdk/x/authz" bank "github.com/cosmos/cosmos-sdk/x/bank/types" "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" @@ -19,7 +20,6 @@ import ( "github.com/celestiaorg/celestia-app/v3/pkg/user" "github.com/celestiaorg/celestia-app/v3/test/util/blobfactory" "github.com/celestiaorg/celestia-app/v3/test/util/testnode" - "github.com/cosmos/cosmos-sdk/x/authz" ) func TestTxClientTestSuite(t *testing.T) { @@ -209,9 +209,10 @@ func (suite *TxClientTestSuite) TestConfirmTx() { resp, err := suite.txClient.BroadcastTx(suite.ctx.GoContext(), []sdk.Msg{msg}, fee, gas) require.NoError(t, err) require.NotNil(t, resp) - confirmTxResp, err := suite.txClient.ConfirmTx(suite.ctx.GoContext(), resp.TxHash) + _, err = suite.txClient.ConfirmTx(suite.ctx.GoContext(), resp.TxHash) require.Error(t, err) - require.NotEqual(t, abci.CodeTypeOK, confirmTxResp.Code) + code := err.(*user.ExecutionError).Code + require.NotEqual(t, abci.CodeTypeOK, code) }) } diff --git a/x/blobstream/integration_test.go b/x/blobstream/integration_test.go index c4a7b6d9a6..a8f19bf4b6 100644 --- a/x/blobstream/integration_test.go +++ b/x/blobstream/integration_test.go @@ -81,20 +81,26 @@ func (s *BlobstreamIntegrationSuite) TestBlobstream() { // sign and submit the transactions for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + serviceClient := sdktx.NewServiceClient(s.cctx.GRPCClient) msgs, _ := tt.msgFunc() txClient, err := user.SetupTxClient(s.cctx.GoContext(), s.cctx.Keyring, s.cctx.GRPCClient, s.ecfg) require.NoError(t, err) res, err := txClient.SubmitTx(s.cctx.GoContext(), msgs, blobfactory.DefaultTxOpts()...) if tt.expectedTxCode == abci.CodeTypeOK { require.NoError(t, err) + require.NotNil(t, res) + getTxResp, err := serviceClient.GetTx(s.cctx.GoContext(), &sdktx.GetTxRequest{Hash: res.TxHash}) + require.NoError(t, err) + require.Equal(t, tt.expectedTxCode, res.Code, getTxResp.TxResponse.RawLog) } else { require.Error(t, err) + require.Nil(t, res) + txHash := err.(*user.ExecutionError).TxHash + code := err.(*user.ExecutionError).Code + getTxResp, err := serviceClient.GetTx(s.cctx.GoContext(), &sdktx.GetTxRequest{Hash: txHash}) + require.NoError(t, err) + require.Equal(t, tt.expectedTxCode, code, getTxResp.TxResponse.RawLog) } - serviceClient := sdktx.NewServiceClient(s.cctx.GRPCClient) - getTxResp, err := serviceClient.GetTx(s.cctx.GoContext(), &sdktx.GetTxRequest{Hash: res.TxHash}) - require.NoError(t, err) - require.NotNil(t, res) - require.Equal(t, tt.expectedTxCode, res.Code, getTxResp.TxResponse.RawLog) }) } } diff --git a/x/signal/legacy_test.go b/x/signal/legacy_test.go index aa53038cec..21f2695aa0 100644 --- a/x/signal/legacy_test.go +++ b/x/signal/legacy_test.go @@ -8,6 +8,7 @@ import ( "github.com/celestiaorg/celestia-app/v3/app" "github.com/celestiaorg/celestia-app/v3/app/encoding" + "github.com/celestiaorg/celestia-app/v3/pkg/user" testutil "github.com/celestiaorg/celestia-app/v3/test/util" "github.com/celestiaorg/celestia-app/v3/test/util/blobfactory" "github.com/celestiaorg/celestia-app/v3/test/util/genesis" @@ -120,14 +121,14 @@ func (s *LegacyUpgradeTestSuite) TestLegacyGovUpgradeFailure() { require.NoError(t, err) // submit the transaction and wait a block for it to be included - signer, err := testnode.NewTxClientFromContext(s.cctx) + txClient, err := testnode.NewTxClientFromContext(s.cctx) require.NoError(t, err) subCtx, cancel := context.WithTimeout(s.cctx.GoContext(), time.Minute) defer cancel() - res, err := signer.SubmitTx(subCtx, []sdk.Msg{msg}, blobfactory.DefaultTxOpts()...) - require.Error(t, err) + _, err = txClient.SubmitTx(subCtx, []sdk.Msg{msg}, blobfactory.DefaultTxOpts()...) + code := err.(*user.BroadcastTxError).Code // As the type is not registered, the message will fail with unable to resolve type URL - require.EqualValues(t, 2, res.Code) + require.EqualValues(t, 2, code, err.Error()) } // TestNewGovUpgradeFailure verifies that a transaction with a @@ -149,14 +150,15 @@ func (s *LegacyUpgradeTestSuite) TestNewGovUpgradeFailure() { require.NoError(t, err) // submit the transaction and wait a block for it to be included - signer, err := testnode.NewTxClientFromContext(s.cctx) + txClient, err := testnode.NewTxClientFromContext(s.cctx) require.NoError(t, err) subCtx, cancel := context.WithTimeout(s.cctx.GoContext(), time.Minute) defer cancel() - res, err := signer.SubmitTx(subCtx, []sdk.Msg{msg}, blobfactory.DefaultTxOpts()...) - require.Error(t, err) + _, err = txClient.SubmitTx(subCtx, []sdk.Msg{msg}, blobfactory.DefaultTxOpts()...) // As the type is not registered, the message will fail with unable to resolve type URL - require.EqualValues(t, 2, res.Code) + require.Error(t, err) + code := err.(*user.BroadcastTxError).Code + require.EqualValues(t, 2, code, err.Error()) } func (s *LegacyUpgradeTestSuite) TestIBCUpgradeFailure() { @@ -182,9 +184,10 @@ func (s *LegacyUpgradeTestSuite) TestIBCUpgradeFailure() { require.NoError(t, err) subCtx, cancel := context.WithTimeout(s.cctx.GoContext(), time.Minute) defer cancel() - res, err := txClient.SubmitTx(subCtx, []sdk.Msg{msg}, blobfactory.DefaultTxOpts()...) + _, err = txClient.SubmitTx(subCtx, []sdk.Msg{msg}, blobfactory.DefaultTxOpts()...) require.Error(t, err) - require.EqualValues(t, 9, res.Code) // we're only submitting the tx, so we expect everything to work + code := err.(*user.ExecutionError).Code + require.EqualValues(t, 9, code) // we're only submitting the tx, so we expect everything to work assert.Contains(t, err.Error(), "ibc upgrade proposal not supported") }