From 0dd4b72ac5369cac711fab0aba1b37bfe496c975 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: Mon, 19 Aug 2024 13:57:44 +0200 Subject: [PATCH 1/5] feat: add error log to txstatus (#3788) ## Overview - [x] Bump celestia-core verison including refactored TxStatus work - [x] Extend the rpc response to return the error log - [x] Update TxClient with extended TxStatus struct --------- Co-authored-by: Callum Waters (cherry picked from commit 02b604de49ecd9873a83d2011641bf30723e1a8f) # Conflicts: # app/test/big_blob_test.go # app/test/prepare_proposal_context_test.go # app/test/square_size_test.go # app/test/std_sdk_test.go # pkg/user/tx_client.go # pkg/user/tx_client_test.go # x/blobstream/integration_test.go # x/signal/legacy_test.go --- app/grpc/tx/server.go | 1 + app/grpc/tx/tx.pb.go | 99 +++++++++++++++++------ app/test/big_blob_test.go | 4 + app/test/prepare_proposal_context_test.go | 6 ++ app/test/square_size_test.go | 7 ++ app/test/std_sdk_test.go | 6 ++ go.mod | 2 +- go.sum | 4 +- pkg/user/tx_client.go | 58 +++++++++++++ pkg/user/tx_client_test.go | 24 ++++++ proto/celestia/core/v1/tx/tx.proto | 4 +- test/interchain/go.mod | 4 +- test/interchain/go.sum | 8 +- x/blobstream/integration_test.go | 6 ++ x/signal/legacy_test.go | 5 ++ 15 files changed, 205 insertions(+), 33 deletions(-) diff --git a/app/grpc/tx/server.go b/app/grpc/tx/server.go index 93b3c9da36..417b7c088b 100644 --- a/app/grpc/tx/server.go +++ b/app/grpc/tx/server.go @@ -76,6 +76,7 @@ func (s *txServer) TxStatus(ctx context.Context, req *TxStatusRequest) (*TxStatu Height: resTx.Height, Index: resTx.Index, ExecutionCode: resTx.ExecutionCode, + Error: resTx.Error, Status: resTx.Status, }, nil } diff --git a/app/grpc/tx/tx.pb.go b/app/grpc/tx/tx.pb.go index c962ec2dd5..de589600d2 100644 --- a/app/grpc/tx/tx.pb.go +++ b/app/grpc/tx/tx.pb.go @@ -82,8 +82,10 @@ type TxStatusResponse struct { // and returns whether it was successful or errored. A non zero // execution code indicated an error. ExecutionCode uint32 `protobuf:"varint,3,opt,name=execution_code,json=executionCode,proto3" json:"execution_code,omitempty"` + // error log for failed transactions. + Error string `protobuf:"bytes,4,opt,name=error,proto3" json:"error,omitempty"` // status is the status of the transaction. - Status string `protobuf:"bytes,4,opt,name=status,proto3" json:"status,omitempty"` + Status string `protobuf:"bytes,5,opt,name=status,proto3" json:"status,omitempty"` } func (m *TxStatusResponse) Reset() { *m = TxStatusResponse{} } @@ -140,6 +142,13 @@ func (m *TxStatusResponse) GetExecutionCode() uint32 { return 0 } +func (m *TxStatusResponse) GetError() string { + if m != nil { + return m.Error + } + return "" +} + func (m *TxStatusResponse) GetStatus() string { if m != nil { return m.Status @@ -155,28 +164,29 @@ func init() { func init() { proto.RegisterFile("celestia/core/v1/tx/tx.proto", fileDescriptor_7d8b070565b0dcb6) } var fileDescriptor_7d8b070565b0dcb6 = []byte{ - // 322 bytes of a gzipped FileDescriptorProto - 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0x84, 0x51, 0xcd, 0x4a, 0x03, 0x31, - 0x18, 0x6c, 0xfa, 0x87, 0x06, 0xaa, 0x92, 0x8a, 0x2c, 0xa5, 0x84, 0x52, 0x5a, 0xe9, 0xc5, 0x84, - 0xea, 0x1b, 0xe8, 0xa9, 0xd7, 0xb5, 0x27, 0x2f, 0x65, 0xbb, 0x1b, 0xb6, 0x81, 0xba, 0x89, 0x9b, - 0x6f, 0x4b, 0x40, 0x8a, 0xa0, 0x2f, 0x20, 0xf8, 0x52, 0x1e, 0x0b, 0x5e, 0x3c, 0x4a, 0xeb, 0x83, - 0x48, 0xd3, 0x76, 0x05, 0x29, 0x78, 0x08, 0x64, 0x32, 0x93, 0x6f, 0x32, 0x13, 0xdc, 0x0c, 0xc5, - 0x54, 0x18, 0x90, 0x01, 0x0f, 0x55, 0x2a, 0xf8, 0xac, 0xcf, 0xc1, 0x72, 0xb0, 0x4c, 0xa7, 0x0a, - 0x14, 0xa9, 0xef, 0x58, 0xb6, 0x66, 0xd9, 0xac, 0xcf, 0xc0, 0x36, 0x9a, 0xb1, 0x52, 0xf1, 0x54, - 0xf0, 0x40, 0x4b, 0x1e, 0x24, 0x89, 0x82, 0x00, 0xa4, 0x4a, 0xcc, 0xe6, 0x4a, 0xfb, 0x1c, 0x1f, - 0x0f, 0xed, 0x2d, 0x04, 0x90, 0x19, 0x5f, 0x3c, 0x64, 0xc2, 0x00, 0xa9, 0xe3, 0x0a, 0xd8, 0x91, - 0x8c, 0x3c, 0xd4, 0x42, 0xbd, 0x43, 0xbf, 0x0c, 0x76, 0x10, 0xb5, 0x9f, 0xf0, 0xc9, 0xaf, 0xce, - 0x68, 0x95, 0x18, 0x41, 0xce, 0x70, 0x75, 0x22, 0x64, 0x3c, 0x01, 0xa7, 0x2c, 0xf9, 0x5b, 0x44, - 0x4e, 0x71, 0x45, 0x26, 0x91, 0xb0, 0x5e, 0xb1, 0x85, 0x7a, 0x35, 0x7f, 0x03, 0x48, 0x17, 0x1f, - 0x09, 0x2b, 0xc2, 0x6c, 0xed, 0x3e, 0x0a, 0x55, 0x24, 0xbc, 0x92, 0xa3, 0x6b, 0xf9, 0xe9, 0x8d, - 0x8a, 0xdc, 0x50, 0xe3, 0x6c, 0xbc, 0xb2, 0xb3, 0xdf, 0xa2, 0xcb, 0x17, 0x84, 0x8b, 0x43, 0x4b, - 0xe6, 0xf8, 0x60, 0xf7, 0x0e, 0xd2, 0x61, 0x7b, 0xf2, 0xb2, 0x3f, 0x71, 0x1a, 0xdd, 0x7f, 0x54, - 0x9b, 0x30, 0xed, 0xce, 0xf3, 0xc7, 0xf7, 0x5b, 0x91, 0x92, 0x26, 0xdf, 0x57, 0xf1, 0xa3, 0x6b, - 0x64, 0x7e, 0x3d, 0x78, 0x5f, 0x52, 0xb4, 0x58, 0x52, 0xf4, 0xb5, 0xa4, 0xe8, 0x75, 0x45, 0x0b, - 0x8b, 0x15, 0x2d, 0x7c, 0xae, 0x68, 0xe1, 0x8e, 0xc7, 0x12, 0x26, 0xd9, 0x98, 0x85, 0xea, 0x3e, - 0x9f, 0xa0, 0xd2, 0x38, 0xdf, 0x5f, 0x04, 0x5a, 0xf3, 0xf5, 0x8a, 0x53, 0x1d, 0x72, 0xb0, 0xe3, - 0xaa, 0xfb, 0x80, 0xab, 0x9f, 0x00, 0x00, 0x00, 0xff, 0xff, 0xdb, 0x2a, 0x51, 0xff, 0xd3, 0x01, - 0x00, 0x00, + // 337 bytes of a gzipped FileDescriptorProto + 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0x84, 0x51, 0xcd, 0x4a, 0x33, 0x31, + 0x14, 0x6d, 0xfa, 0xc7, 0xf7, 0x05, 0xfa, 0x7d, 0x92, 0x8a, 0x0c, 0xa5, 0x0c, 0xa5, 0xb4, 0xd2, + 0x8d, 0x13, 0xaa, 0x6f, 0xa0, 0xab, 0x6e, 0xc7, 0xae, 0xdc, 0x94, 0xe9, 0xcc, 0x65, 0x1a, 0xa8, + 0x93, 0x31, 0xb9, 0x53, 0x02, 0xd2, 0x8d, 0xbe, 0x80, 0x20, 0xbe, 0x93, 0xcb, 0x82, 0x1b, 0x97, + 0xd2, 0xfa, 0x20, 0x32, 0x99, 0xb6, 0x82, 0x14, 0x5c, 0x04, 0x72, 0xee, 0x39, 0x37, 0x27, 0xf7, + 0x5c, 0xda, 0x0e, 0x61, 0x0e, 0x1a, 0x45, 0xc0, 0x43, 0xa9, 0x80, 0x2f, 0x86, 0x1c, 0x0d, 0x47, + 0xe3, 0xa5, 0x4a, 0xa2, 0x64, 0xcd, 0x1d, 0xeb, 0xe5, 0xac, 0xb7, 0x18, 0x7a, 0x68, 0x5a, 0xed, + 0x58, 0xca, 0x78, 0x0e, 0x3c, 0x48, 0x05, 0x0f, 0x92, 0x44, 0x62, 0x80, 0x42, 0x26, 0xba, 0x68, + 0xe9, 0x9e, 0xd2, 0xff, 0x63, 0x73, 0x8d, 0x01, 0x66, 0xda, 0x87, 0xbb, 0x0c, 0x34, 0xb2, 0x26, + 0xad, 0xa1, 0x99, 0x88, 0xc8, 0x21, 0x1d, 0x32, 0xf8, 0xeb, 0x57, 0xd1, 0x8c, 0xa2, 0xee, 0x0b, + 0xa1, 0x47, 0xdf, 0x42, 0x9d, 0xca, 0x44, 0x03, 0x3b, 0xa1, 0xf5, 0x19, 0x88, 0x78, 0x86, 0x56, + 0x5a, 0xf1, 0xb7, 0x88, 0x1d, 0xd3, 0x9a, 0x48, 0x22, 0x30, 0x4e, 0xb9, 0x43, 0x06, 0x0d, 0xbf, + 0x00, 0xac, 0x4f, 0xff, 0x81, 0x81, 0x30, 0xcb, 0xed, 0x27, 0xa1, 0x8c, 0xc0, 0xa9, 0x58, 0xba, + 0xb1, 0xaf, 0x5e, 0xc9, 0x08, 0xf2, 0x66, 0x50, 0x4a, 0x2a, 0xa7, 0x6a, 0xed, 0x0b, 0x90, 0x5b, + 0x69, 0x6b, 0xee, 0xd4, 0x6c, 0x79, 0x8b, 0xce, 0x1f, 0x09, 0x2d, 0x8f, 0x0d, 0x5b, 0xd2, 0x3f, + 0xbb, 0xdf, 0xb1, 0x9e, 0x77, 0x20, 0x06, 0xef, 0xc7, 0x94, 0xad, 0xfe, 0x2f, 0xaa, 0x62, 0xc4, + 0x6e, 0xef, 0xe1, 0xed, 0xf3, 0xb9, 0xec, 0xb2, 0x36, 0x3f, 0x94, 0xfc, 0xbd, 0x0d, 0x6a, 0x79, + 0x39, 0x7a, 0x5d, 0xbb, 0x64, 0xb5, 0x76, 0xc9, 0xc7, 0xda, 0x25, 0x4f, 0x1b, 0xb7, 0xb4, 0xda, + 0xb8, 0xa5, 0xf7, 0x8d, 0x5b, 0xba, 0xe1, 0xb1, 0xc0, 0x59, 0x36, 0xf5, 0x42, 0x79, 0xbb, 0x7f, + 0x41, 0xaa, 0x78, 0x7f, 0x3f, 0x0b, 0xd2, 0x94, 0xe7, 0x27, 0x56, 0x69, 0xc8, 0xd1, 0x4c, 0xeb, + 0x76, 0x2f, 0x17, 0x5f, 0x01, 0x00, 0x00, 0xff, 0xff, 0x5a, 0x2f, 0x1b, 0xd1, 0xea, 0x01, 0x00, + 0x00, } // Reference imports to suppress errors if they are not otherwise used. @@ -324,6 +334,13 @@ func (m *TxStatusResponse) MarshalToSizedBuffer(dAtA []byte) (int, error) { copy(dAtA[i:], m.Status) i = encodeVarintTx(dAtA, i, uint64(len(m.Status))) i-- + dAtA[i] = 0x2a + } + if len(m.Error) > 0 { + i -= len(m.Error) + copy(dAtA[i:], m.Error) + i = encodeVarintTx(dAtA, i, uint64(len(m.Error))) + i-- dAtA[i] = 0x22 } if m.ExecutionCode != 0 { @@ -383,6 +400,10 @@ func (m *TxStatusResponse) Size() (n int) { if m.ExecutionCode != 0 { n += 1 + sovTx(uint64(m.ExecutionCode)) } + l = len(m.Error) + if l > 0 { + n += 1 + l + sovTx(uint64(l)) + } l = len(m.Status) if l > 0 { n += 1 + l + sovTx(uint64(l)) @@ -565,6 +586,38 @@ func (m *TxStatusResponse) Unmarshal(dAtA []byte) error { } } case 4: + if wireType != 2 { + return fmt.Errorf("proto: wrong wireType = %d for field Error", wireType) + } + var stringLen uint64 + for shift := uint(0); ; shift += 7 { + if shift >= 64 { + return ErrIntOverflowTx + } + if iNdEx >= l { + return io.ErrUnexpectedEOF + } + b := dAtA[iNdEx] + iNdEx++ + stringLen |= uint64(b&0x7F) << shift + if b < 0x80 { + break + } + } + intStringLen := int(stringLen) + if intStringLen < 0 { + return ErrInvalidLengthTx + } + postIndex := iNdEx + intStringLen + if postIndex < 0 { + return ErrInvalidLengthTx + } + if postIndex > l { + return io.ErrUnexpectedEOF + } + m.Error = string(dAtA[iNdEx:postIndex]) + iNdEx = postIndex + case 5: if wireType != 2 { return fmt.Errorf("proto: wrong wireType = %d for field Status", wireType) } diff --git a/app/test/big_blob_test.go b/app/test/big_blob_test.go index e3cd56e84c..6dd7d58990 100644 --- a/app/test/big_blob_test.go +++ b/app/test/big_blob_test.go @@ -83,7 +83,11 @@ func (s *BigBlobSuite) TestErrBlobsTooLarge() { res, err := txClient.SubmitPayForBlob(subCtx, []*blob.Blob{tc.blob}, user.SetGasLimitAndGasPrice(1e9, appconsts.DefaultMinGasPrice)) require.Error(t, err) require.NotNil(t, res) +<<<<<<< HEAD require.Equal(t, tc.want, res.Code, res.Logs) +======= + require.Equal(t, tc.want, res.Code, err.Error()) +>>>>>>> 02b604de (feat: add error log to txstatus (#3788)) }) } } diff --git a/app/test/prepare_proposal_context_test.go b/app/test/prepare_proposal_context_test.go index cb181a2a31..f238c51877 100644 --- a/app/test/prepare_proposal_context_test.go +++ b/app/test/prepare_proposal_context_test.go @@ -80,6 +80,12 @@ func TestTimeInPrepareProposalContext(t *testing.T) { msgs, _ := tt.msgFunc() res, err := txClient.SubmitTx(cctx.GoContext(), msgs, user.SetGasLimit(1000000), user.SetFee(2000)) require.NoError(t, err) +<<<<<<< HEAD +======= + serviceClient := sdktx.NewServiceClient(cctx.GRPCClient) + getTxResp, err := serviceClient.GetTx(cctx.GoContext(), &sdktx.GetTxRequest{Hash: res.TxHash}) + require.NoError(t, err) +>>>>>>> 02b604de (feat: add error log to txstatus (#3788)) require.NotNil(t, res) assert.Equal(t, abci.CodeTypeOK, res.Code, res.RawLog) }) diff --git a/app/test/square_size_test.go b/app/test/square_size_test.go index 28748a3bf7..f2ddba6742 100644 --- a/app/test/square_size_test.go +++ b/app/test/square_size_test.go @@ -173,7 +173,14 @@ func (s *SquareSizeIntegrationTest) setBlockSizeParams(t *testing.T, squareSize, res, err := txClient.SubmitTx(s.cctx.GoContext(), []sdk.Msg{msg}, blobfactory.DefaultTxOpts()...) require.NoError(t, err) +<<<<<<< HEAD require.Equal(t, res.Code, abci.CodeTypeOK, res.RawLog) +======= + serviceClient := sdktx.NewServiceClient(s.cctx.GRPCClient) + getTxResp, err := serviceClient.GetTx(s.cctx.GoContext(), &sdktx.GetTxRequest{Hash: res.TxHash}) + require.NoError(t, err) + require.Equal(t, res.Code, abci.CodeTypeOK, getTxResp.TxResponse.RawLog) +>>>>>>> 02b604de (feat: add error log to txstatus (#3788)) require.NoError(t, s.cctx.WaitForNextBlock()) diff --git a/app/test/std_sdk_test.go b/app/test/std_sdk_test.go index 3e1f3e4492..98149f4db1 100644 --- a/app/test/std_sdk_test.go +++ b/app/test/std_sdk_test.go @@ -325,6 +325,12 @@ func (s *StandardSDKIntegrationTestSuite) TestStandardSDK() { } else { require.NoError(t, err) } +<<<<<<< HEAD +======= + serviceClient := sdktx.NewServiceClient(s.cctx.GRPCClient) + getTxResp, err := serviceClient.GetTx(s.cctx.GoContext(), &sdktx.GetTxRequest{Hash: res.TxHash}) + require.NoError(t, err) +>>>>>>> 02b604de (feat: add error log to txstatus (#3788)) require.NotNil(t, res) assert.Equal(t, tt.expectedCode, res.Code, res.RawLog) }) diff --git a/go.mod b/go.mod index 1ad8066b03..3d70adff83 100644 --- a/go.mod +++ b/go.mod @@ -255,5 +255,5 @@ replace ( github.com/cosmos/ledger-cosmos-go => github.com/cosmos/ledger-cosmos-go v0.12.4 github.com/gogo/protobuf => github.com/regen-network/protobuf v1.3.3-alpha.regen.1 github.com/syndtr/goleveldb => github.com/syndtr/goleveldb v1.0.1-0.20210819022825-2ae1ddf74ef7 - github.com/tendermint/tendermint => github.com/celestiaorg/celestia-core v1.39.0-tm-v0.34.29 + github.com/tendermint/tendermint => github.com/celestiaorg/celestia-core v1.40.0-tm-v0.34.29 ) diff --git a/go.sum b/go.sum index 5dd9b2ae03..85842213a0 100644 --- a/go.sum +++ b/go.sum @@ -320,8 +320,8 @@ github.com/celestiaorg/bittwister v0.0.0-20231213180407-65cdbaf5b8c7 h1:nxplQi8w github.com/celestiaorg/bittwister v0.0.0-20231213180407-65cdbaf5b8c7/go.mod h1:1EF5MfOxVf0WC51Gb7pJ6bcZxnXKNAf9pqWtjgPBAYc= github.com/celestiaorg/blobstream-contracts/v3 v3.1.0 h1:h1Y4V3EMQ2mFmNtWt2sIhZIuyASInj1a9ExI8xOsTOw= github.com/celestiaorg/blobstream-contracts/v3 v3.1.0/go.mod h1:x4DKyfKOSv1ZJM9NwV+Pw01kH2CD7N5zTFclXIVJ6GQ= -github.com/celestiaorg/celestia-core v1.39.0-tm-v0.34.29 h1:9Co/2peu4+9S6KMVNPFS0NTI/RYIRirNpM4N7dmi9ak= -github.com/celestiaorg/celestia-core v1.39.0-tm-v0.34.29/go.mod h1:5jJ5magtH7gQOwSYfS/m5fliIS7irKunLV7kLNaD8o0= +github.com/celestiaorg/celestia-core v1.40.0-tm-v0.34.29 h1:J79TAjizxwIvm7/k+WI3PPH1aFj4AjOSjajoq5UzAwI= +github.com/celestiaorg/celestia-core v1.40.0-tm-v0.34.29/go.mod h1:5jJ5magtH7gQOwSYfS/m5fliIS7irKunLV7kLNaD8o0= github.com/celestiaorg/cosmos-sdk v1.24.1-sdk-v0.46.16 h1:SeQ7Y/CyOcUMKo7mQiexaj/pZ/xIgyuZFIwYZwpSkWE= github.com/celestiaorg/cosmos-sdk v1.24.1-sdk-v0.46.16/go.mod h1:Bpl1LSWiDpQumgOhhMTZBMopqa0j7fRasIhvTZB44P0= github.com/celestiaorg/go-square v1.1.0 h1:K4tBL5PCJwDtpBfyDxxZ3N962aC9VYb5/bw3LjagEtY= diff --git a/pkg/user/tx_client.go b/pkg/user/tx_client.go index 7897f0e578..24e359a6b3 100644 --- a/pkg/user/tx_client.go +++ b/pkg/user/tx_client.go @@ -23,12 +23,23 @@ import ( abci "github.com/tendermint/tendermint/abci/types" "google.golang.org/grpc" +<<<<<<< HEAD "github.com/celestiaorg/celestia-app/v2/app" "github.com/celestiaorg/celestia-app/v2/app/encoding" apperrors "github.com/celestiaorg/celestia-app/v2/app/errors" "github.com/celestiaorg/celestia-app/v2/pkg/appconsts" "github.com/celestiaorg/celestia-app/v2/x/blob/types" "github.com/celestiaorg/celestia-app/v2/x/minfee" +======= + "github.com/celestiaorg/celestia-app/v3/app" + "github.com/celestiaorg/celestia-app/v3/app/encoding" + apperrors "github.com/celestiaorg/celestia-app/v3/app/errors" + "github.com/celestiaorg/celestia-app/v3/app/grpc/tx" + "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" +>>>>>>> 02b604de (feat: add error log to txstatus (#3788)) ) const ( @@ -202,7 +213,11 @@ func SetupTxClient( func (client *TxClient) SubmitPayForBlob(ctx context.Context, blobs []*blob.Blob, opts ...TxOption) (*sdktypes.TxResponse, error) { resp, err := client.BroadcastPayForBlob(ctx, blobs, opts...) if err != nil { +<<<<<<< HEAD return resp, err +======= + return parseTxResponse(resp, fmt.Errorf("failed to broadcast pay for blob: %v", err)) +>>>>>>> 02b604de (feat: add error log to txstatus (#3788)) } return client.ConfirmTx(ctx, resp.TxHash) @@ -211,7 +226,11 @@ func (client *TxClient) SubmitPayForBlob(ctx context.Context, blobs []*blob.Blob func (client *TxClient) SubmitPayForBlobWithAccount(ctx context.Context, account string, blobs []*blob.Blob, opts ...TxOption) (*sdktypes.TxResponse, error) { resp, err := client.BroadcastPayForBlobWithAccount(ctx, account, blobs, opts...) if err != nil { +<<<<<<< HEAD return resp, err +======= + return parseTxResponse(resp, fmt.Errorf("failed to broadcast pay for blob with account: %v", err)) +>>>>>>> 02b604de (feat: add error log to txstatus (#3788)) } return client.ConfirmTx(ctx, resp.TxHash) @@ -255,7 +274,11 @@ func (client *TxClient) BroadcastPayForBlobWithAccount(ctx context.Context, acco func (client *TxClient) SubmitTx(ctx context.Context, msgs []sdktypes.Msg, opts ...TxOption) (*sdktypes.TxResponse, error) { resp, err := client.BroadcastTx(ctx, msgs, opts...) if err != nil { +<<<<<<< HEAD return resp, err +======= + return parseTxResponse(resp, fmt.Errorf("failed to broadcast tx: %v", err)) +>>>>>>> 02b604de (feat: add error log to txstatus (#3788)) } return client.ConfirmTx(ctx, resp.TxHash) @@ -430,11 +453,39 @@ func (client *TxClient) ConfirmTx(ctx context.Context, txHash string) (*sdktypes return &sdktypes.TxResponse{}, err } +<<<<<<< HEAD // Wait for the next round. select { case <-ctx.Done(): return &sdktypes.TxResponse{}, ctx.Err() case <-pollTicker.C: +======= + if err == nil && resp != nil { + switch resp.Status { + case core.TxStatusPending: + // Continue polling if the transaction is still pending + select { + case <-ctx.Done(): + return &TxResponse{}, ctx.Err() + case <-pollTicker.C: + continue + } + case core.TxStatusCommitted: + txResponse := &TxResponse{ + Height: resp.Height, + 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) + } + return txResponse, nil + case core.TxStatusEvicted: + return &TxResponse{TxHash: txHash}, fmt.Errorf("tx: %s was evicted from the mempool", txHash) + default: + return &TxResponse{}, fmt.Errorf("unknown tx: %s", txHash) + } +>>>>>>> 02b604de (feat: add error log to txstatus (#3788)) } } } @@ -541,6 +592,13 @@ 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 ebdb6e1c66..f382be5fb3 100644 --- a/pkg/user/tx_client_test.go +++ b/pkg/user/tx_client_test.go @@ -12,12 +12,22 @@ import ( abci "github.com/tendermint/tendermint/abci/types" "github.com/tendermint/tendermint/libs/rand" +<<<<<<< HEAD "github.com/celestiaorg/celestia-app/v2/app" "github.com/celestiaorg/celestia-app/v2/app/encoding" "github.com/celestiaorg/celestia-app/v2/pkg/appconsts" "github.com/celestiaorg/celestia-app/v2/pkg/user" "github.com/celestiaorg/celestia-app/v2/test/util/blobfactory" "github.com/celestiaorg/celestia-app/v2/test/util/testnode" +======= + "github.com/celestiaorg/celestia-app/v3/app" + "github.com/celestiaorg/celestia-app/v3/app/encoding" + "github.com/celestiaorg/celestia-app/v3/pkg/appconsts" + "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" +>>>>>>> 02b604de (feat: add error log to txstatus (#3788)) ) func TestTxClientTestSuite(t *testing.T) { @@ -45,6 +55,10 @@ func (suite *TxClientTestSuite) SetupSuite() { suite.Require().NoError(err) suite.txClient, err = user.SetupTxClient(suite.ctx.GoContext(), suite.ctx.Keyring, suite.ctx.GRPCClient, suite.encCfg, user.WithGasMultiplier(1.2)) suite.Require().NoError(err) +<<<<<<< HEAD +======= + suite.serviceClient = sdktx.NewServiceClient(suite.ctx.GRPCClient) +>>>>>>> 02b604de (feat: add error log to txstatus (#3788)) } func (suite *TxClientTestSuite) TestSubmitPayForBlob() { @@ -157,6 +171,16 @@ func (suite *TxClientTestSuite) TestConfirmTx() { require.Error(t, err) }) + t.Run("should return error log when execution fails", func(t *testing.T) { + innerMsg := bank.NewMsgSend(testnode.RandomAddress().(sdk.AccAddress), testnode.RandomAddress().(sdk.AccAddress), sdk.NewCoins(sdk.NewInt64Coin(app.BondDenom, 10))) + msg := authz.NewMsgExec(suite.txClient.DefaultAddress(), []sdk.Msg{innerMsg}) + resp, err := suite.txClient.BroadcastTx(suite.ctx.GoContext(), []sdk.Msg{&msg}, fee, gas) + require.NoError(t, err) + _, err = suite.txClient.ConfirmTx(suite.ctx.GoContext(), resp.TxHash) + require.Error(t, err) + require.Contains(t, err.Error(), "authorization not found") + }) + t.Run("should success when tx is found immediately", func(t *testing.T) { addr := suite.txClient.DefaultAddress() msg := bank.NewMsgSend(addr, testnode.RandomAddress().(sdk.AccAddress), sdk.NewCoins(sdk.NewInt64Coin(app.BondDenom, 10))) diff --git a/proto/celestia/core/v1/tx/tx.proto b/proto/celestia/core/v1/tx/tx.proto index f4a8301606..0da698ad84 100644 --- a/proto/celestia/core/v1/tx/tx.proto +++ b/proto/celestia/core/v1/tx/tx.proto @@ -33,6 +33,8 @@ message TxStatusResponse { // and returns whether it was successful or errored. A non zero // execution code indicated an error. uint32 execution_code = 3; + // error log for failed transactions. + string error = 4; // status is the status of the transaction. - string status = 4; + string status = 5; } \ No newline at end of file diff --git a/test/interchain/go.mod b/test/interchain/go.mod index 52ef37781b..b511091c87 100644 --- a/test/interchain/go.mod +++ b/test/interchain/go.mod @@ -33,7 +33,7 @@ require ( github.com/bgentry/go-netrc v0.0.0-20140422174119-9fd32a8b3d3d // indirect github.com/bgentry/speakeasy v0.1.0 // indirect github.com/btcsuite/btcd/btcec/v2 v2.3.2 // indirect - github.com/celestiaorg/nmt v0.21.0 // indirect + github.com/celestiaorg/nmt v0.22.0 // indirect github.com/centrifuge/go-substrate-rpc-client/v4 v4.0.10 // indirect github.com/cespare/xxhash v1.1.0 // indirect github.com/cespare/xxhash/v2 v2.2.0 // indirect @@ -229,5 +229,5 @@ replace ( github.com/docker/docker => github.com/docker/docker v24.0.1+incompatible github.com/gogo/protobuf => github.com/regen-network/protobuf v1.3.3-alpha.regen.1 github.com/syndtr/goleveldb => github.com/syndtr/goleveldb v1.0.1-0.20210819022825-2ae1ddf74ef7 - github.com/tendermint/tendermint => github.com/celestiaorg/celestia-core v1.38.0-tm-v0.34.29 + github.com/tendermint/tendermint => github.com/celestiaorg/celestia-core v1.40.0-tm-v0.34.29 ) diff --git a/test/interchain/go.sum b/test/interchain/go.sum index 3f306fe454..c24352cb35 100644 --- a/test/interchain/go.sum +++ b/test/interchain/go.sum @@ -249,12 +249,12 @@ github.com/btcsuite/btcd/chaincfg/chainhash v1.0.1 h1:q0rUy8C/TYNBQS1+CGKw68tLOF github.com/btcsuite/btcd/chaincfg/chainhash v1.0.1/go.mod h1:7SFka0XMvUgj3hfZtydOrQY2mwhPclbT2snogU7SQQc= github.com/btcsuite/btcutil v1.0.3-0.20201208143702-a53e38424cce h1:YtWJF7RHm2pYCvA5t0RPmAaLUhREsKuKd+SLhxFbFeQ= github.com/btcsuite/btcutil v1.0.3-0.20201208143702-a53e38424cce/go.mod h1:0DVlHczLPewLcPGEIeUEzfOJhqGPQ0mJJRDBtD307+o= -github.com/celestiaorg/celestia-core v1.38.0-tm-v0.34.29 h1:HwbA4OegRvXX0aNchBA7Cmu+oIxnH7xRcOhISuDP0ak= -github.com/celestiaorg/celestia-core v1.38.0-tm-v0.34.29/go.mod h1:MyElURdWAOJkOp84WZnfEUJ+OLvTwOOHG2lbK9E8XRI= +github.com/celestiaorg/celestia-core v1.40.0-tm-v0.34.29 h1:J79TAjizxwIvm7/k+WI3PPH1aFj4AjOSjajoq5UzAwI= +github.com/celestiaorg/celestia-core v1.40.0-tm-v0.34.29/go.mod h1:5jJ5magtH7gQOwSYfS/m5fliIS7irKunLV7kLNaD8o0= github.com/celestiaorg/cosmos-sdk v1.24.0-sdk-v0.46.16 h1:AlBZS4WykzrwfcNbKD+yQQM1RTMz7lYDC1NS7ClAidM= github.com/celestiaorg/cosmos-sdk v1.24.0-sdk-v0.46.16/go.mod h1:Bpl1LSWiDpQumgOhhMTZBMopqa0j7fRasIhvTZB44P0= -github.com/celestiaorg/nmt v0.21.0 h1:81MBqxNn3orByoiCtdNVjwi5WsLgMkzHwP02ZMhTBHM= -github.com/celestiaorg/nmt v0.21.0/go.mod h1:ia/EpCk0enD5yO5frcxoNoFToz2Ghtk2i+blmCRjIY8= +github.com/celestiaorg/nmt v0.22.0 h1:AGtfmBiVgreR1KkIV5R7XFNeMp/H4IUDLlBbLjZZ3zk= +github.com/celestiaorg/nmt v0.22.0/go.mod h1:ia/EpCk0enD5yO5frcxoNoFToz2Ghtk2i+blmCRjIY8= github.com/cenkalti/backoff v2.2.1+incompatible h1:tNowT99t7UNflLxfYYSlKYsBpXdEet03Pg2g16Swow4= github.com/cenkalti/backoff/v4 v4.1.3 h1:cFAlzYUlVYDysBEH2T5hyJZMh3+5+WCBvSnK6Q8UtC4= github.com/cenkalti/backoff/v4 v4.1.3/go.mod h1:scbssz8iZGpm3xbr14ovlUdkxfGXNInqkPWOWmG2CLw= diff --git a/x/blobstream/integration_test.go b/x/blobstream/integration_test.go index c30d1ed439..d66a2b7bb2 100644 --- a/x/blobstream/integration_test.go +++ b/x/blobstream/integration_test.go @@ -89,6 +89,12 @@ func (s *BlobstreamIntegrationSuite) TestBlobstream() { } else { require.Error(t, err) } +<<<<<<< HEAD +======= + serviceClient := sdktx.NewServiceClient(s.cctx.GRPCClient) + getTxResp, err := serviceClient.GetTx(s.cctx.GoContext(), &sdktx.GetTxRequest{Hash: res.TxHash}) + require.NoError(t, err) +>>>>>>> 02b604de (feat: add error log to txstatus (#3788)) require.NotNil(t, res) require.Equal(t, tt.expectedTxCode, res.Code, res.RawLog) }) diff --git a/x/signal/legacy_test.go b/x/signal/legacy_test.go index 84c70a554c..e139d1510d 100644 --- a/x/signal/legacy_test.go +++ b/x/signal/legacy_test.go @@ -184,8 +184,13 @@ func (s *LegacyUpgradeTestSuite) TestIBCUpgradeFailure() { defer cancel() res, err := signer.SubmitTx(subCtx, []sdk.Msg{msg}, blobfactory.DefaultTxOpts()...) require.Error(t, err) +<<<<<<< HEAD require.EqualValues(t, 9, res.Code, res.RawLog) // we're only submitting the tx, so we expect everything to work assert.Contains(t, res.RawLog, "ibc upgrade proposal not supported") +======= + require.EqualValues(t, 9, res.Code) // we're only submitting the tx, so we expect everything to work + assert.Contains(t, err.Error(), "ibc upgrade proposal not supported") +>>>>>>> 02b604de (feat: add error log to txstatus (#3788)) } func getAddress(account string, kr keyring.Keyring) sdk.AccAddress { From 66ef700017f125ccd94f3a114ebb636e54864a38 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: Fri, 16 Aug 2024 14:15:37 +0200 Subject: [PATCH 2/5] chore: cherry pick refactor: signer to use txstatus --- app/test/big_blob_test.go | 4 -- app/test/prepare_proposal_context_test.go | 8 +-- app/test/square_size_test.go | 5 +- app/test/std_sdk_test.go | 6 +- pkg/user/tx_client.go | 70 +++++++---------------- pkg/user/tx_client_test.go | 68 ++++++++++++---------- test/txsim/account.go | 2 +- x/blobstream/integration_test.go | 6 +- x/signal/legacy_test.go | 13 ++--- 9 files changed, 73 insertions(+), 109 deletions(-) diff --git a/app/test/big_blob_test.go b/app/test/big_blob_test.go index 6dd7d58990..1adc1f62fe 100644 --- a/app/test/big_blob_test.go +++ b/app/test/big_blob_test.go @@ -83,11 +83,7 @@ func (s *BigBlobSuite) TestErrBlobsTooLarge() { res, err := txClient.SubmitPayForBlob(subCtx, []*blob.Blob{tc.blob}, user.SetGasLimitAndGasPrice(1e9, appconsts.DefaultMinGasPrice)) require.Error(t, err) require.NotNil(t, res) -<<<<<<< HEAD - require.Equal(t, tc.want, res.Code, res.Logs) -======= require.Equal(t, tc.want, res.Code, err.Error()) ->>>>>>> 02b604de (feat: add error log to txstatus (#3788)) }) } } diff --git a/app/test/prepare_proposal_context_test.go b/app/test/prepare_proposal_context_test.go index f238c51877..9a5769f7e9 100644 --- a/app/test/prepare_proposal_context_test.go +++ b/app/test/prepare_proposal_context_test.go @@ -12,6 +12,7 @@ import ( "github.com/cosmos/cosmos-sdk/crypto/hd" "github.com/cosmos/cosmos-sdk/crypto/keyring" sdk "github.com/cosmos/cosmos-sdk/types" + sdktx "github.com/cosmos/cosmos-sdk/types/tx" vestingtypes "github.com/cosmos/cosmos-sdk/x/auth/vesting/types" banktypes "github.com/cosmos/cosmos-sdk/x/bank/types" "github.com/stretchr/testify/assert" @@ -20,7 +21,7 @@ import ( ) // TestTimeInPrepareProposalContext checks for an edge case where the block time -// needs to be included in the sdk.Context that is being used in the +// needs to be included in theMA sdk.Context that is being used in the // antehandlers. If a time is not included in the context, then the second // transaction in this test will always be filtered out, result in vesting // accounts never being able to spend funds. @@ -80,14 +81,11 @@ func TestTimeInPrepareProposalContext(t *testing.T) { msgs, _ := tt.msgFunc() res, err := txClient.SubmitTx(cctx.GoContext(), msgs, user.SetGasLimit(1000000), user.SetFee(2000)) require.NoError(t, err) -<<<<<<< HEAD -======= serviceClient := sdktx.NewServiceClient(cctx.GRPCClient) getTxResp, err := serviceClient.GetTx(cctx.GoContext(), &sdktx.GetTxRequest{Hash: res.TxHash}) require.NoError(t, err) ->>>>>>> 02b604de (feat: add error log to txstatus (#3788)) require.NotNil(t, res) - assert.Equal(t, abci.CodeTypeOK, res.Code, res.RawLog) + assert.Equal(t, abci.CodeTypeOK, res.Code, getTxResp.TxResponse.RawLog) }) } } diff --git a/app/test/square_size_test.go b/app/test/square_size_test.go index f2ddba6742..ec928f580a 100644 --- a/app/test/square_size_test.go +++ b/app/test/square_size_test.go @@ -17,6 +17,7 @@ import ( "github.com/celestiaorg/celestia-app/v2/test/util/testnode" blobtypes "github.com/celestiaorg/celestia-app/v2/x/blob/types" sdk "github.com/cosmos/cosmos-sdk/types" + sdktx "github.com/cosmos/cosmos-sdk/types/tx" v1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1" oldgov "github.com/cosmos/cosmos-sdk/x/gov/types/v1beta1" "github.com/cosmos/cosmos-sdk/x/params/types/proposal" @@ -173,14 +174,10 @@ func (s *SquareSizeIntegrationTest) setBlockSizeParams(t *testing.T, squareSize, res, err := txClient.SubmitTx(s.cctx.GoContext(), []sdk.Msg{msg}, blobfactory.DefaultTxOpts()...) require.NoError(t, err) -<<<<<<< HEAD - require.Equal(t, res.Code, abci.CodeTypeOK, res.RawLog) -======= serviceClient := sdktx.NewServiceClient(s.cctx.GRPCClient) getTxResp, err := serviceClient.GetTx(s.cctx.GoContext(), &sdktx.GetTxRequest{Hash: res.TxHash}) require.NoError(t, err) require.Equal(t, res.Code, abci.CodeTypeOK, getTxResp.TxResponse.RawLog) ->>>>>>> 02b604de (feat: add error log to txstatus (#3788)) require.NoError(t, s.cctx.WaitForNextBlock()) diff --git a/app/test/std_sdk_test.go b/app/test/std_sdk_test.go index 98149f4db1..75d9bceb16 100644 --- a/app/test/std_sdk_test.go +++ b/app/test/std_sdk_test.go @@ -21,6 +21,7 @@ import ( "github.com/cosmos/cosmos-sdk/crypto/keyring" "github.com/cosmos/cosmos-sdk/testutil/mock" sdk "github.com/cosmos/cosmos-sdk/types" + sdktx "github.com/cosmos/cosmos-sdk/types/tx" vestingtypes "github.com/cosmos/cosmos-sdk/x/auth/vesting/types" banktypes "github.com/cosmos/cosmos-sdk/x/bank/types" disttypes "github.com/cosmos/cosmos-sdk/x/distribution/types" @@ -325,14 +326,11 @@ func (s *StandardSDKIntegrationTestSuite) TestStandardSDK() { } else { require.NoError(t, err) } -<<<<<<< HEAD -======= serviceClient := sdktx.NewServiceClient(s.cctx.GRPCClient) getTxResp, err := serviceClient.GetTx(s.cctx.GoContext(), &sdktx.GetTxRequest{Hash: res.TxHash}) require.NoError(t, err) ->>>>>>> 02b604de (feat: add error log to txstatus (#3788)) require.NotNil(t, res) - assert.Equal(t, tt.expectedCode, res.Code, res.RawLog) + 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 24e359a6b3..56baedc6ef 100644 --- a/pkg/user/tx_client.go +++ b/pkg/user/tx_client.go @@ -23,23 +23,14 @@ import ( abci "github.com/tendermint/tendermint/abci/types" "google.golang.org/grpc" -<<<<<<< HEAD "github.com/celestiaorg/celestia-app/v2/app" "github.com/celestiaorg/celestia-app/v2/app/encoding" apperrors "github.com/celestiaorg/celestia-app/v2/app/errors" + "github.com/celestiaorg/celestia-app/v2/app/grpc/tx" "github.com/celestiaorg/celestia-app/v2/pkg/appconsts" "github.com/celestiaorg/celestia-app/v2/x/blob/types" "github.com/celestiaorg/celestia-app/v2/x/minfee" -======= - "github.com/celestiaorg/celestia-app/v3/app" - "github.com/celestiaorg/celestia-app/v3/app/encoding" - apperrors "github.com/celestiaorg/celestia-app/v3/app/errors" - "github.com/celestiaorg/celestia-app/v3/app/grpc/tx" - "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" ->>>>>>> 02b604de (feat: add error log to txstatus (#3788)) ) const ( @@ -49,6 +40,14 @@ const ( type Option func(client *TxClient) +// TxResponse is a response from the chain after +// a transaction has been submitted. +type TxResponse struct { + Height int64 + TxHash string + Code uint32 +} + // WithGasMultiplier is a functional option allows to configure the gas multiplier. func WithGasMultiplier(multiplier float64) Option { return func(c *TxClient) { @@ -210,27 +209,21 @@ func SetupTxClient( // SubmitPayForBlob forms a transaction from the provided blobs, signs it, and submits it to the chain. // TxOptions may be provided to set the fee and gas limit. -func (client *TxClient) SubmitPayForBlob(ctx context.Context, blobs []*blob.Blob, opts ...TxOption) (*sdktypes.TxResponse, error) { +func (client *TxClient) SubmitPayForBlob(ctx context.Context, blobs []*blob.Blob, opts ...TxOption) (*TxResponse, error) { resp, err := client.BroadcastPayForBlob(ctx, blobs, opts...) if err != nil { -<<<<<<< HEAD - return resp, err -======= return parseTxResponse(resp, fmt.Errorf("failed to broadcast pay for blob: %v", err)) ->>>>>>> 02b604de (feat: add error log to txstatus (#3788)) } return client.ConfirmTx(ctx, resp.TxHash) } -func (client *TxClient) SubmitPayForBlobWithAccount(ctx context.Context, account string, blobs []*blob.Blob, opts ...TxOption) (*sdktypes.TxResponse, error) { +// SubmitPayForBlobWithAccount forms a transaction from the provided blobs, signs it with the provided account, and submits it to the chain. +// TxOptions may be provided to set the fee and gas limit. +func (client *TxClient) SubmitPayForBlobWithAccount(ctx context.Context, account string, blobs []*blob.Blob, opts ...TxOption) (*TxResponse, error) { resp, err := client.BroadcastPayForBlobWithAccount(ctx, account, blobs, opts...) if err != nil { -<<<<<<< HEAD - return resp, err -======= return parseTxResponse(resp, fmt.Errorf("failed to broadcast pay for blob with account: %v", err)) ->>>>>>> 02b604de (feat: add error log to txstatus (#3788)) } return client.ConfirmTx(ctx, resp.TxHash) @@ -271,14 +264,10 @@ func (client *TxClient) BroadcastPayForBlobWithAccount(ctx context.Context, acco // SubmitTx forms a transaction from the provided messages, signs it, and submits it to the chain. TxOptions // may be provided to set the fee and gas limit. -func (client *TxClient) SubmitTx(ctx context.Context, msgs []sdktypes.Msg, opts ...TxOption) (*sdktypes.TxResponse, error) { +func (client *TxClient) SubmitTx(ctx context.Context, msgs []sdktypes.Msg, opts ...TxOption) (*TxResponse, error) { resp, err := client.BroadcastTx(ctx, msgs, opts...) if err != nil { -<<<<<<< HEAD - return resp, err -======= return parseTxResponse(resp, fmt.Errorf("failed to broadcast tx: %v", err)) ->>>>>>> 02b604de (feat: add error log to txstatus (#3788)) } return client.ConfirmTx(ctx, resp.TxHash) @@ -432,35 +421,19 @@ func (client *TxClient) retryBroadcastingTx(ctx context.Context, txBytes []byte) // ConfirmTx periodically pings the provided node for the commitment of a transaction by its // hash. It will continually loop until the context is cancelled, the tx is found or an error // is encountered. -func (client *TxClient) ConfirmTx(ctx context.Context, txHash string) (*sdktypes.TxResponse, error) { - txClient := sdktx.NewServiceClient(client.grpc) +func (client *TxClient) ConfirmTx(ctx context.Context, txHash string) (*TxResponse, error) { + txClient := tx.NewTxClient(client.grpc) pollTicker := time.NewTicker(client.pollTime) defer pollTicker.Stop() for { - resp, err := txClient.GetTx(ctx, &sdktx.GetTxRequest{Hash: txHash}) - if err == nil { - if resp.TxResponse.Code != 0 { - return resp.TxResponse, fmt.Errorf("tx was included but failed with code %d: %s", resp.TxResponse.Code, resp.TxResponse.RawLog) - } - return resp.TxResponse, nil - } - // FIXME: this is a relatively brittle of working out whether to retry or not. The tx might be not found for other - // reasons. It may have been removed from the mempool at a later point. We should build an endpoint that gives the - // signer more information on the status of their transaction and then update the logic here - if !strings.Contains(err.Error(), "not found") { - return &sdktypes.TxResponse{}, err + resp, err := txClient.TxStatus(ctx, &tx.TxStatusRequest{TxId: txHash}) + if err != nil { + return &TxResponse{}, err } -<<<<<<< HEAD - // Wait for the next round. - select { - case <-ctx.Done(): - return &sdktypes.TxResponse{}, ctx.Err() - case <-pollTicker.C: -======= - if err == nil && resp != nil { + if resp != nil { switch resp.Status { case core.TxStatusPending: // Continue polling if the transaction is still pending @@ -476,7 +449,7 @@ func (client *TxClient) ConfirmTx(ctx context.Context, txHash string) (*sdktypes TxHash: txHash, Code: resp.ExecutionCode, } - if resp.ExecutionCode != 0 { + if resp.ExecutionCode != abci.CodeTypeOK { return txResponse, fmt.Errorf("tx was committed but failed with code %d: %s", resp.ExecutionCode, resp.Error) } return txResponse, nil @@ -485,7 +458,6 @@ func (client *TxClient) ConfirmTx(ctx context.Context, txHash string) (*sdktypes default: return &TxResponse{}, fmt.Errorf("unknown tx: %s", txHash) } ->>>>>>> 02b604de (feat: add error log to txstatus (#3788)) } } } diff --git a/pkg/user/tx_client_test.go b/pkg/user/tx_client_test.go index f382be5fb3..fb8069b9fd 100644 --- a/pkg/user/tx_client_test.go +++ b/pkg/user/tx_client_test.go @@ -6,28 +6,20 @@ import ( "time" sdk "github.com/cosmos/cosmos-sdk/types" + sdktx "github.com/cosmos/cosmos-sdk/types/tx" bank "github.com/cosmos/cosmos-sdk/x/bank/types" "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" abci "github.com/tendermint/tendermint/abci/types" "github.com/tendermint/tendermint/libs/rand" -<<<<<<< HEAD "github.com/celestiaorg/celestia-app/v2/app" "github.com/celestiaorg/celestia-app/v2/app/encoding" "github.com/celestiaorg/celestia-app/v2/pkg/appconsts" "github.com/celestiaorg/celestia-app/v2/pkg/user" "github.com/celestiaorg/celestia-app/v2/test/util/blobfactory" "github.com/celestiaorg/celestia-app/v2/test/util/testnode" -======= - "github.com/celestiaorg/celestia-app/v3/app" - "github.com/celestiaorg/celestia-app/v3/app/encoding" - "github.com/celestiaorg/celestia-app/v3/pkg/appconsts" - "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" ->>>>>>> 02b604de (feat: add error log to txstatus (#3788)) ) func TestTxClientTestSuite(t *testing.T) { @@ -40,9 +32,10 @@ func TestTxClientTestSuite(t *testing.T) { type TxClientTestSuite struct { suite.Suite - ctx testnode.Context - encCfg encoding.Config - txClient *user.TxClient + ctx testnode.Context + encCfg encoding.Config + txClient *user.TxClient + serviceClient sdktx.ServiceClient } func (suite *TxClientTestSuite) SetupSuite() { @@ -55,10 +48,7 @@ func (suite *TxClientTestSuite) SetupSuite() { suite.Require().NoError(err) suite.txClient, err = user.SetupTxClient(suite.ctx.GoContext(), suite.ctx.Keyring, suite.ctx.GRPCClient, suite.encCfg, user.WithGasMultiplier(1.2)) suite.Require().NoError(err) -<<<<<<< HEAD -======= suite.serviceClient = sdktx.NewServiceClient(suite.ctx.GRPCClient) ->>>>>>> 02b604de (feat: add error log to txstatus (#3788)) } func (suite *TxClientTestSuite) TestSubmitPayForBlob() { @@ -71,8 +61,10 @@ func (suite *TxClientTestSuite) TestSubmitPayForBlob() { t.Run("submit blob without provided fee and gas limit", func(t *testing.T) { resp, err := suite.txClient.SubmitPayForBlob(subCtx, blobs) require.NoError(t, err) + getTxResp, err := suite.serviceClient.GetTx(subCtx, &sdktx.GetTxRequest{Hash: resp.TxHash}) + require.NoError(t, err) require.EqualValues(t, 0, resp.Code) - require.Greater(t, resp.GasWanted, int64(0)) + require.Greater(t, getTxResp.TxResponse.GasWanted, int64(0)) }) t.Run("submit blob with provided fee and gas limit", func(t *testing.T) { @@ -80,15 +72,19 @@ func (suite *TxClientTestSuite) TestSubmitPayForBlob() { gas := user.SetGasLimit(1e6) resp, err := suite.txClient.SubmitPayForBlob(subCtx, blobs, fee, gas) require.NoError(t, err) + getTxResp, err := suite.serviceClient.GetTx(subCtx, &sdktx.GetTxRequest{Hash: resp.TxHash}) + require.NoError(t, err) require.EqualValues(t, 0, resp.Code) - require.EqualValues(t, resp.GasWanted, 1e6) + require.EqualValues(t, getTxResp.TxResponse.GasWanted, 1e6) }) t.Run("submit blob with different account", func(t *testing.T) { resp, err := suite.txClient.SubmitPayForBlobWithAccount(subCtx, "c", blobs, user.SetFee(1e6), user.SetGasLimit(1e6)) require.NoError(t, err) + getTxResp, err := suite.serviceClient.GetTx(subCtx, &sdktx.GetTxRequest{Hash: resp.TxHash}) + require.NoError(t, err) require.EqualValues(t, 0, resp.Code) - require.EqualValues(t, resp.GasWanted, 1e6) + require.EqualValues(t, getTxResp.TxResponse.GasWanted, 1e6) }) t.Run("try submit a blob with an account that doesn't exist", func(t *testing.T) { @@ -110,14 +106,18 @@ func (suite *TxClientTestSuite) TestSubmitTx() { resp, err := suite.txClient.SubmitTx(suite.ctx.GoContext(), []sdk.Msg{msg}) require.NoError(t, err) require.Equal(t, abci.CodeTypeOK, resp.Code) - require.Greater(t, resp.GasWanted, int64(0)) + getTxResp, err := suite.serviceClient.GetTx(suite.ctx.GoContext(), &sdktx.GetTxRequest{Hash: resp.TxHash}) + require.NoError(t, err) + require.Greater(t, getTxResp.TxResponse.GasWanted, int64(0)) }) t.Run("submit tx with provided gas limit", func(t *testing.T) { resp, err := suite.txClient.SubmitTx(suite.ctx.GoContext(), []sdk.Msg{msg}, gasLimitOption) require.NoError(t, err) require.Equal(t, abci.CodeTypeOK, resp.Code) - require.EqualValues(t, gasLimit, resp.GasWanted) + getTxResp, err := suite.serviceClient.GetTx(suite.ctx.GoContext(), &sdktx.GetTxRequest{Hash: resp.TxHash}) + require.NoError(t, err) + require.EqualValues(t, int64(gasLimit), getTxResp.TxResponse.GasWanted) }) t.Run("submit tx with provided fee", func(t *testing.T) { @@ -130,7 +130,9 @@ func (suite *TxClientTestSuite) TestSubmitTx() { resp, err := suite.txClient.SubmitTx(suite.ctx.GoContext(), []sdk.Msg{msg}, feeOption, gasLimitOption) require.NoError(t, err) require.Equal(t, abci.CodeTypeOK, resp.Code) - require.EqualValues(t, gasLimit, resp.GasWanted) + getTxResp, err := suite.serviceClient.GetTx(suite.ctx.GoContext(), &sdktx.GetTxRequest{Hash: resp.TxHash}) + require.NoError(t, err) + require.EqualValues(t, int64(gasLimit), getTxResp.TxResponse.GasWanted) }) t.Run("submit tx with a different account", func(t *testing.T) { @@ -159,7 +161,12 @@ func (suite *TxClientTestSuite) TestConfirmTx() { t.Run("deadline exceeded when the context times out", func(t *testing.T) { ctx, cancel := context.WithTimeout(suite.ctx.GoContext(), time.Second) defer cancel() - _, err := suite.txClient.ConfirmTx(ctx, "E32BD15CAF57AF15D17B0D63CF4E63A9835DD1CEBB059C335C79586BC3013728") + + msg := bank.NewMsgSend(suite.txClient.DefaultAddress(), testnode.RandomAddress().(sdk.AccAddress), sdk.NewCoins(sdk.NewInt64Coin(app.BondDenom, 10))) + resp, err := suite.txClient.BroadcastTx(ctx, []sdk.Msg{msg}) + require.NoError(t, err) + + _, err = suite.txClient.ConfirmTx(ctx, resp.TxHash) require.Error(t, err) require.Contains(t, err.Error(), context.DeadlineExceeded.Error()) }) @@ -167,8 +174,8 @@ func (suite *TxClientTestSuite) TestConfirmTx() { t.Run("should error when tx is not found", func(t *testing.T) { ctx, cancel := context.WithTimeout(suite.ctx.GoContext(), 5*time.Second) defer cancel() - _, err := suite.txClient.ConfirmTx(ctx, "not found tx") - require.Error(t, err) + _, err := suite.txClient.ConfirmTx(ctx, "E32BD15CAF57AF15D17B0D63CF4E63A9835DD1CEBB059C335C79586BC3013728") + require.Contains(t, err.Error(), "unknown tx: E32BD15CAF57AF15D17B0D63CF4E63A9835DD1CEBB059C335C79586BC3013728") }) t.Run("should return error log when execution fails", func(t *testing.T) { @@ -189,9 +196,9 @@ func (suite *TxClientTestSuite) TestConfirmTx() { require.NotNil(t, resp) ctx, cancel := context.WithTimeout(suite.ctx.GoContext(), 30*time.Second) defer cancel() - resp, err = suite.txClient.ConfirmTx(ctx, resp.TxHash) + confirmTxResp, err := suite.txClient.ConfirmTx(ctx, resp.TxHash) require.NoError(t, err) - require.Equal(t, abci.CodeTypeOK, resp.Code) + require.Equal(t, abci.CodeTypeOK, confirmTxResp.Code) }) t.Run("should error when tx is found with a non-zero error code", func(t *testing.T) { @@ -202,9 +209,9 @@ 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) - resp, err = suite.txClient.ConfirmTx(suite.ctx.GoContext(), resp.TxHash) + confirmTxResp, err := suite.txClient.ConfirmTx(suite.ctx.GoContext(), resp.TxHash) require.Error(t, err) - require.NotEqual(t, abci.CodeTypeOK, resp.Code) + require.NotEqual(t, abci.CodeTypeOK, confirmTxResp.Code) }) } @@ -245,8 +252,11 @@ func (suite *TxClientTestSuite) TestGasConsumption() { amountDeducted := balanceBefore - balanceAfter - utiaToSend require.Equal(t, int64(fee), amountDeducted) + res, err := suite.serviceClient.GetTx(suite.ctx.GoContext(), &sdktx.GetTxRequest{Hash: resp.TxHash}) + require.NoError(t, err) + // verify that the amount deducted does not depend on the actual gas used. - gasUsedBasedDeduction := resp.GasUsed * gasPrice + gasUsedBasedDeduction := res.TxResponse.GasUsed * gasPrice require.NotEqual(t, gasUsedBasedDeduction, amountDeducted) // The gas used based deduction should be less than the fee because the fee is 1 TIA. require.Less(t, gasUsedBasedDeduction, int64(fee)) diff --git a/test/txsim/account.go b/test/txsim/account.go index ea1a45f251..760195ae11 100644 --- a/test/txsim/account.go +++ b/test/txsim/account.go @@ -240,7 +240,7 @@ func (am *AccountManager) Submit(ctx context.Context, op Operation) error { } var ( - res *types.TxResponse + res *user.TxResponse err error ) if len(op.Blobs) > 0 { diff --git a/x/blobstream/integration_test.go b/x/blobstream/integration_test.go index d66a2b7bb2..9660ca2dba 100644 --- a/x/blobstream/integration_test.go +++ b/x/blobstream/integration_test.go @@ -11,6 +11,7 @@ import ( "github.com/celestiaorg/celestia-app/v2/test/util/testnode" blobstreamtypes "github.com/celestiaorg/celestia-app/v2/x/blobstream/types" sdk "github.com/cosmos/cosmos-sdk/types" + sdktx "github.com/cosmos/cosmos-sdk/types/tx" staking "github.com/cosmos/cosmos-sdk/x/staking/types" gethcommon "github.com/ethereum/go-ethereum/common" "github.com/stretchr/testify/require" @@ -89,14 +90,11 @@ func (s *BlobstreamIntegrationSuite) TestBlobstream() { } else { require.Error(t, err) } -<<<<<<< HEAD -======= serviceClient := sdktx.NewServiceClient(s.cctx.GRPCClient) getTxResp, err := serviceClient.GetTx(s.cctx.GoContext(), &sdktx.GetTxRequest{Hash: res.TxHash}) require.NoError(t, err) ->>>>>>> 02b604de (feat: add error log to txstatus (#3788)) require.NotNil(t, res) - require.Equal(t, tt.expectedTxCode, res.Code, res.RawLog) + 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 e139d1510d..74ba28de83 100644 --- a/x/signal/legacy_test.go +++ b/x/signal/legacy_test.go @@ -127,7 +127,7 @@ func (s *LegacyUpgradeTestSuite) TestLegacyGovUpgradeFailure() { res, err := signer.SubmitTx(subCtx, []sdk.Msg{msg}, blobfactory.DefaultTxOpts()...) require.Error(t, err) // As the type is not registered, the message will fail with unable to resolve type URL - require.EqualValues(t, 2, res.Code, res.RawLog) + require.EqualValues(t, 2, res.Code) } // TestNewGovUpgradeFailure verifies that a transaction with a @@ -156,7 +156,7 @@ func (s *LegacyUpgradeTestSuite) TestNewGovUpgradeFailure() { res, err := signer.SubmitTx(subCtx, []sdk.Msg{msg}, blobfactory.DefaultTxOpts()...) require.Error(t, err) // As the type is not registered, the message will fail with unable to resolve type URL - require.EqualValues(t, 2, res.Code, res.RawLog) + require.EqualValues(t, 2, res.Code) } func (s *LegacyUpgradeTestSuite) TestIBCUpgradeFailure() { @@ -178,19 +178,14 @@ func (s *LegacyUpgradeTestSuite) TestIBCUpgradeFailure() { 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()...) + res, err := txClient.SubmitTx(subCtx, []sdk.Msg{msg}, blobfactory.DefaultTxOpts()...) require.Error(t, err) -<<<<<<< HEAD - require.EqualValues(t, 9, res.Code, res.RawLog) // we're only submitting the tx, so we expect everything to work - assert.Contains(t, res.RawLog, "ibc upgrade proposal not supported") -======= require.EqualValues(t, 9, res.Code) // we're only submitting the tx, so we expect everything to work assert.Contains(t, err.Error(), "ibc upgrade proposal not supported") ->>>>>>> 02b604de (feat: add error log to txstatus (#3788)) } func getAddress(account string, kr keyring.Keyring) sdk.AccAddress { From 90979a1a3e3c8632a76287c948c5def5d08b5a70 Mon Sep 17 00:00:00 2001 From: Nina Barbakadze Date: Tue, 20 Aug 2024 12:13:18 +0200 Subject: [PATCH 3/5] refactor: if we return error return empty types --- app/test/prepare_proposal_context_test.go | 2 +- pkg/user/tx_client.go | 18 ++++++------------ 2 files changed, 7 insertions(+), 13 deletions(-) diff --git a/app/test/prepare_proposal_context_test.go b/app/test/prepare_proposal_context_test.go index 9a5769f7e9..1c8df70878 100644 --- a/app/test/prepare_proposal_context_test.go +++ b/app/test/prepare_proposal_context_test.go @@ -21,7 +21,7 @@ import ( ) // TestTimeInPrepareProposalContext checks for an edge case where the block time -// needs to be included in theMA sdk.Context that is being used in the +// needs to be included in the sdk.Context that is being used in the // antehandlers. If a time is not included in the context, then the second // transaction in this test will always be filtered out, result in vesting // accounts never being able to spend funds. diff --git a/pkg/user/tx_client.go b/pkg/user/tx_client.go index 56baedc6ef..48a79eca89 100644 --- a/pkg/user/tx_client.go +++ b/pkg/user/tx_client.go @@ -43,6 +43,7 @@ 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 @@ -212,7 +213,7 @@ func SetupTxClient( func (client *TxClient) SubmitPayForBlob(ctx context.Context, blobs []*blob.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 &TxResponse{}, fmt.Errorf("failed to broadcast pay for blob: %v", err) } return client.ConfirmTx(ctx, resp.TxHash) @@ -223,7 +224,7 @@ func (client *TxClient) SubmitPayForBlob(ctx context.Context, blobs []*blob.Blob func (client *TxClient) SubmitPayForBlobWithAccount(ctx context.Context, account string, blobs []*blob.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 &TxResponse{}, fmt.Errorf("failed to broadcast pay for blob with account: %v", err) } return client.ConfirmTx(ctx, resp.TxHash) @@ -267,7 +268,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 &TxResponse{}, fmt.Errorf("failed to broadcast tx: %v", err) } return client.ConfirmTx(ctx, resp.TxHash) @@ -450,11 +451,11 @@ func (client *TxClient) ConfirmTx(ctx context.Context, txHash string) (*TxRespon Code: resp.ExecutionCode, } if resp.ExecutionCode != abci.CodeTypeOK { - return txResponse, fmt.Errorf("tx was committed but failed with code %d: %s", resp.ExecutionCode, resp.Error) + return &TxResponse{}, fmt.Errorf("tx was committed but failed with code %d: %s", resp.ExecutionCode, resp.Error) } return txResponse, nil case core.TxStatusEvicted: - return &TxResponse{TxHash: txHash}, fmt.Errorf("tx: %s was evicted from the mempool", txHash) + return &TxResponse{}, fmt.Errorf("tx: %s was evicted from the mempool", txHash) default: return &TxResponse{}, fmt.Errorf("unknown tx: %s", txHash) } @@ -564,13 +565,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 From 8f19bff50ada638196733bd24599206d41b89900 Mon Sep 17 00:00:00 2001 From: Nina Barbakadze Date: Tue, 20 Aug 2024 20:26:54 +0200 Subject: [PATCH 4/5] refactor: improve error handling --- app/test/big_blob_test.go | 5 ++- app/test/std_sdk_test.go | 16 +++++--- pkg/user/tx_client.go | 64 +++++++++++++++++++++++++++----- pkg/user/tx_client_test.go | 8 ++-- x/blobstream/integration_test.go | 16 +++++--- x/signal/legacy_test.go | 20 ++++++---- 6 files changed, 98 insertions(+), 31 deletions(-) diff --git a/app/test/big_blob_test.go b/app/test/big_blob_test.go index 1adc1f62fe..283c09617b 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, []*blob.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 75d9bceb16..848c824669 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 48a79eca89..12e9be7443 100644 --- a/pkg/user/tx_client.go +++ b/pkg/user/tx_client.go @@ -49,6 +49,40 @@ type TxResponse struct { Code uint32 } +// BroadcastTxError is an error that occurs when broadcasting a transaction. +type BroadcastTxError struct { + TxHash string + Code uint32 + ErrorLog string + Err error +} + +func (e *BroadcastTxError) Error() string { + return fmt.Sprintf("%v", e.Err) +} + +// EvictionError is an error that occurs when a transaction is evicted from the mempool. +type EvictionError struct { + TxHash string + Err error +} + +func (e *EvictionError) Error() string { + return fmt.Sprintf("%v", e.Err) +} + +// ExecutionError is an error that occurs when a transaction gets executed. +type ExecutionError struct { + TxHash string + Code uint32 + ErrorLog string + Err error +} + +func (e *ExecutionError) Error() string { + return fmt.Sprintf("%v", e.Err) +} + // WithGasMultiplier is a functional option allows to configure the gas multiplier. func WithGasMultiplier(multiplier float64) Option { return func(c *TxClient) { @@ -213,7 +247,7 @@ func SetupTxClient( func (client *TxClient) SubmitPayForBlob(ctx context.Context, blobs []*blob.Blob, opts ...TxOption) (*TxResponse, error) { resp, err := client.BroadcastPayForBlob(ctx, blobs, opts...) if err != nil { - return &TxResponse{}, fmt.Errorf("failed to broadcast pay for blob: %v", err) + return nil, err } return client.ConfirmTx(ctx, resp.TxHash) @@ -224,7 +258,7 @@ func (client *TxClient) SubmitPayForBlob(ctx context.Context, blobs []*blob.Blob func (client *TxClient) SubmitPayForBlobWithAccount(ctx context.Context, account string, blobs []*blob.Blob, opts ...TxOption) (*TxResponse, error) { resp, err := client.BroadcastPayForBlobWithAccount(ctx, account, blobs, opts...) if err != nil { - return &TxResponse{}, fmt.Errorf("failed to broadcast pay for blob with account: %v", err) + return nil, err } return client.ConfirmTx(ctx, resp.TxHash) @@ -268,7 +302,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 &TxResponse{}, fmt.Errorf("failed to broadcast tx: %v", err) + return nil, err } return client.ConfirmTx(ctx, resp.TxHash) @@ -354,7 +388,13 @@ 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, + Err: fmt.Errorf("tx failed with code %d: %s", resp.TxResponse.Code, resp.TxResponse.RawLog), + ErrorLog: resp.TxResponse.RawLog, + } + return resp.TxResponse, broadcastTxErr } // after the transaction has been submitted, we can increment the @@ -431,7 +471,7 @@ 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 resp != nil { @@ -440,7 +480,7 @@ func (client *TxClient) ConfirmTx(ctx context.Context, txHash string) (*TxRespon // Continue polling if the transaction is still pending select { case <-ctx.Done(): - return &TxResponse{}, ctx.Err() + return nil, ctx.Err() case <-pollTicker.C: continue } @@ -451,13 +491,19 @@ func (client *TxClient) ConfirmTx(ctx context.Context, txHash string) (*TxRespon Code: resp.ExecutionCode, } if resp.ExecutionCode != abci.CodeTypeOK { - return &TxResponse{}, fmt.Errorf("tx was committed but failed with code %d: %s", resp.ExecutionCode, resp.Error) + executionErr := &ExecutionError{ + TxHash: txHash, + Code: resp.ExecutionCode, + ErrorLog: resp.Error, + Err: fmt.Errorf("execution failed with code %d", resp.ExecutionCode), + } + return nil, executionErr } return txResponse, nil case core.TxStatusEvicted: - return &TxResponse{}, fmt.Errorf("tx: %s was evicted from the mempool", txHash) + return nil, &EvictionError{TxHash: txHash, Err: 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) } } } diff --git a/pkg/user/tx_client_test.go b/pkg/user/tx_client_test.go index fb8069b9fd..e60873bb5e 100644 --- a/pkg/user/tx_client_test.go +++ b/pkg/user/tx_client_test.go @@ -185,7 +185,8 @@ func (suite *TxClientTestSuite) TestConfirmTx() { require.NoError(t, err) _, err = suite.txClient.ConfirmTx(suite.ctx.GoContext(), resp.TxHash) require.Error(t, err) - require.Contains(t, err.Error(), "authorization not found") + errorLog := err.(*user.ExecutionError).ErrorLog + require.Contains(t, errorLog, "authorization not found") }) t.Run("should success when tx is found immediately", func(t *testing.T) { @@ -209,9 +210,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 9660ca2dba..93f358c9a8 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 74ba28de83..e024b23a91 100644 --- a/x/signal/legacy_test.go +++ b/x/signal/legacy_test.go @@ -8,6 +8,7 @@ import ( "github.com/celestiaorg/celestia-app/v2/app" "github.com/celestiaorg/celestia-app/v2/app/encoding" + "github.com/celestiaorg/celestia-app/v2/pkg/user" testutil "github.com/celestiaorg/celestia-app/v2/test/util" "github.com/celestiaorg/celestia-app/v2/test/util/blobfactory" "github.com/celestiaorg/celestia-app/v2/test/util/genesis" @@ -124,10 +125,11 @@ func (s *LegacyUpgradeTestSuite) TestLegacyGovUpgradeFailure() { 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()...) + _, err = signer.SubmitTx(subCtx, []sdk.Msg{msg}, blobfactory.DefaultTxOpts()...) require.Error(t, err) // As the type is not registered, the message will fail with unable to resolve type URL - require.EqualValues(t, 2, res.Code) + code := err.(*user.BroadcastTxError).Code + require.EqualValues(t, 2, code) } // TestNewGovUpgradeFailure verifies that a transaction with a @@ -153,10 +155,12 @@ func (s *LegacyUpgradeTestSuite) TestNewGovUpgradeFailure() { 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()...) + _, err = signer.SubmitTx(subCtx, []sdk.Msg{msg}, blobfactory.DefaultTxOpts()...) require.Error(t, err) // As the type is not registered, the message will fail with unable to resolve type URL - require.EqualValues(t, 2, res.Code) + // unpakc err + code := err.(*user.BroadcastTxError).Code + require.EqualValues(t, 2, code) } func (s *LegacyUpgradeTestSuite) TestIBCUpgradeFailure() { @@ -182,10 +186,12 @@ 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 - assert.Contains(t, err.Error(), "ibc upgrade proposal not supported") + code := err.(*user.ExecutionError).Code + errLog := err.(*user.ExecutionError).ErrorLog + require.EqualValues(t, 9, code) // we're only submitting the tx, so we expect everything to work + assert.Contains(t, errLog, "ibc upgrade proposal not supported") } func getAddress(account string, kr keyring.Keyring) sdk.AccAddress { From 0650800df0999f953c654f584dc63f1002b7c015 Mon Sep 17 00:00:00 2001 From: Nina Barbakadze Date: Tue, 20 Aug 2024 21:52:11 +0200 Subject: [PATCH 5/5] refactor: address nits --- pkg/user/tx_client.go | 28 +++++++++++----------------- x/signal/legacy_test.go | 11 ++++++----- 2 files changed, 17 insertions(+), 22 deletions(-) diff --git a/pkg/user/tx_client.go b/pkg/user/tx_client.go index 12e9be7443..8b107e9de0 100644 --- a/pkg/user/tx_client.go +++ b/pkg/user/tx_client.go @@ -51,32 +51,26 @@ type TxResponse struct { // BroadcastTxError is an error that occurs when broadcasting a transaction. type BroadcastTxError struct { - TxHash string - Code uint32 + TxHash string + Code uint32 + // ErrorLog is the error output of the app's logger ErrorLog string - Err error + // Err is the error that occurred during broadcasting + Err error } func (e *BroadcastTxError) Error() string { return fmt.Sprintf("%v", e.Err) } -// EvictionError is an error that occurs when a transaction is evicted from the mempool. -type EvictionError struct { - TxHash string - Err error -} - -func (e *EvictionError) Error() string { - return fmt.Sprintf("%v", e.Err) -} - // ExecutionError is an error that occurs when a transaction gets executed. type ExecutionError struct { - TxHash string - Code uint32 + TxHash string + Code uint32 + // ErrorLog is the error output of the app's logger ErrorLog string - Err error + // Err is the error that occurred during execution + Err error } func (e *ExecutionError) Error() string { @@ -501,7 +495,7 @@ func (client *TxClient) ConfirmTx(ctx context.Context, txHash string) (*TxRespon } return txResponse, nil case core.TxStatusEvicted: - return nil, &EvictionError{TxHash: txHash, Err: fmt.Errorf("tx was evicted from the mempool")} + return nil, fmt.Errorf("tx was evicted from the mempool") default: return nil, fmt.Errorf("unknown tx: %s", txHash) } diff --git a/x/signal/legacy_test.go b/x/signal/legacy_test.go index e024b23a91..d3776bdfdc 100644 --- a/x/signal/legacy_test.go +++ b/x/signal/legacy_test.go @@ -126,10 +126,11 @@ func (s *LegacyUpgradeTestSuite) TestLegacyGovUpgradeFailure() { subCtx, cancel := context.WithTimeout(s.cctx.GoContext(), time.Minute) defer cancel() _, err = signer.SubmitTx(subCtx, []sdk.Msg{msg}, blobfactory.DefaultTxOpts()...) - require.Error(t, err) // As the type is not registered, the message will fail with unable to resolve type URL + require.Error(t, err) code := err.(*user.BroadcastTxError).Code - require.EqualValues(t, 2, code) + errLog := err.(*user.BroadcastTxError).ErrorLog + require.EqualValues(t, 2, code, errLog) } // TestNewGovUpgradeFailure verifies that a transaction with a @@ -156,11 +157,11 @@ func (s *LegacyUpgradeTestSuite) TestNewGovUpgradeFailure() { subCtx, cancel := context.WithTimeout(s.cctx.GoContext(), time.Minute) defer cancel() _, err = signer.SubmitTx(subCtx, []sdk.Msg{msg}, blobfactory.DefaultTxOpts()...) - require.Error(t, err) // As the type is not registered, the message will fail with unable to resolve type URL - // unpakc err + require.Error(t, err) code := err.(*user.BroadcastTxError).Code - require.EqualValues(t, 2, code) + errLog := err.(*user.BroadcastTxError).ErrorLog + require.EqualValues(t, 2, code, errLog) } func (s *LegacyUpgradeTestSuite) TestIBCUpgradeFailure() {