Fix fetch txs by height on legacy REST endpoint (#7730)

* Add test

* Fix potential flakiness in sequence

* Remove test with ?height

* Fix test

* Fix tests

Co-authored-by: Aleksandr Bezobchuk <alexanderbez@users.noreply.github.com>
Co-authored-by: Zaki Manian <zaki@manian.org>
Co-authored-by: Aaron Craelius <aaron@regen.network>
Co-authored-by: Aaron Craelius <aaronc@users.noreply.github.com>
This commit is contained in:
Amaury Martiny 2020-11-02 12:52:52 +01:00 committed by GitHub
parent cdc73ac605
commit 194d2aa196
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 87 additions and 41 deletions

View File

@ -6,12 +6,12 @@ import (
"io/ioutil"
"net/http"
"github.com/cosmos/cosmos-sdk/x/auth/signing"
"github.com/cosmos/cosmos-sdk/client"
clienttx "github.com/cosmos/cosmos-sdk/client/tx"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
"github.com/cosmos/cosmos-sdk/types/rest"
"github.com/cosmos/cosmos-sdk/x/auth/legacy/legacytx"
"github.com/cosmos/cosmos-sdk/x/auth/signing"
)
type (
@ -47,8 +47,9 @@ func DecodeTxRequestHandlerFn(clientCtx client.Context) http.HandlerFunc {
return
}
stdTx, ok := convertToStdTx(w, clientCtx, txBytes)
if !ok {
stdTx, err := convertToStdTx(w, clientCtx, txBytes)
if err != nil {
// Error is already returned by convertToStdTx.
return
}
@ -61,22 +62,22 @@ func DecodeTxRequestHandlerFn(clientCtx client.Context) http.HandlerFunc {
// convertToStdTx converts tx proto binary bytes retrieved from Tendermint into
// a StdTx. Returns the StdTx, as well as a flag denoting if the function
// successfully converted or not.
func convertToStdTx(w http.ResponseWriter, clientCtx client.Context, txBytes []byte) (legacytx.StdTx, bool) {
func convertToStdTx(w http.ResponseWriter, clientCtx client.Context, txBytes []byte) (legacytx.StdTx, error) {
txI, err := clientCtx.TxConfig.TxDecoder()(txBytes)
if rest.CheckBadRequestError(w, err) {
return legacytx.StdTx{}, false
return legacytx.StdTx{}, err
}
tx, ok := txI.(signing.Tx)
if !ok {
rest.WriteErrorResponse(w, http.StatusBadRequest, fmt.Sprintf("%+v is not backwards compatible with %T", tx, legacytx.StdTx{}))
return legacytx.StdTx{}, false
return legacytx.StdTx{}, sdkerrors.Wrapf(sdkerrors.ErrInvalidType, "expected %T, got %T", (signing.Tx)(nil), txI)
}
stdTx, err := clienttx.ConvertTxToStdTx(clientCtx.LegacyAmino, tx)
if rest.CheckBadRequestError(w, err) {
return legacytx.StdTx{}, false
return legacytx.StdTx{}, err
}
return stdTx, true
return stdTx, nil
}

View File

@ -9,6 +9,7 @@ import (
"github.com/gorilla/mux"
"github.com/cosmos/cosmos-sdk/client"
codectypes "github.com/cosmos/cosmos-sdk/codec/types"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/types/rest"
authclient "github.com/cosmos/cosmos-sdk/x/auth/client"
@ -102,6 +103,10 @@ func QueryTxsRequestHandlerFn(clientCtx client.Context) http.HandlerFunc {
return
}
for _, txRes := range searchResult.Txs {
packStdTxResponse(w, clientCtx, txRes)
}
rest.PostProcessResponseBare(w, clientCtx, searchResult)
}
}
@ -128,11 +133,9 @@ func QueryTxRequestHandlerFn(clientCtx client.Context) http.HandlerFunc {
return
}
// We just unmarshalled from Tendermint, we take the proto Tx's raw
// bytes, and convert them into a StdTx to be displayed.
txBytes := output.Tx.Value
stdTx, ok := convertToStdTx(w, clientCtx, txBytes)
if !ok {
err = packStdTxResponse(w, clientCtx, output)
if err != nil {
// Error is already returned by packStdTxResponse.
return
}
@ -140,7 +143,7 @@ func QueryTxRequestHandlerFn(clientCtx client.Context) http.HandlerFunc {
rest.WriteErrorResponse(w, http.StatusNotFound, fmt.Sprintf("no transaction found with hash %s", hashHexStr))
}
rest.PostProcessResponseBare(w, clientCtx, stdTx)
rest.PostProcessResponseBare(w, clientCtx, output)
}
}
@ -161,3 +164,21 @@ func queryParamsHandler(clientCtx client.Context) http.HandlerFunc {
rest.PostProcessResponse(w, clientCtx, res)
}
}
// packStdTxResponse takes a sdk.TxResponse, converts the Tx into a StdTx, and
// packs the StdTx again into the sdk.TxResponse Any. Amino then takes care of
// seamlessly JSON-outputting the Any.
func packStdTxResponse(w http.ResponseWriter, clientCtx client.Context, txRes *sdk.TxResponse) error {
// We just unmarshalled from Tendermint, we take the proto Tx's raw
// bytes, and convert them into a StdTx to be displayed.
txBytes := txRes.Tx.Value
stdTx, err := convertToStdTx(w, clientCtx, txBytes)
if err != nil {
return err
}
// Pack the amino stdTx into the TxResponse's Any.
txRes.Tx = codectypes.UnsafePackAny(stdTx)
return nil
}

View File

@ -2,7 +2,6 @@ package rest_test
import (
"fmt"
"strings"
"testing"
"github.com/stretchr/testify/suite"
@ -29,6 +28,9 @@ type IntegrationTestSuite struct {
cfg network.Config
network *network.Network
stdTx legacytx.StdTx
stdTxRes sdk.TxResponse
}
func (s *IntegrationTestSuite) SetupSuite() {
@ -46,6 +48,17 @@ func (s *IntegrationTestSuite) SetupSuite() {
_, err = s.network.WaitForHeight(1)
s.Require().NoError(err)
// Broadcast a StdTx used for tests.
s.stdTx = s.createTestStdTx(s.network.Validators[0], 0, 1)
res, err := s.broadcastReq(s.stdTx, "block")
s.Require().NoError(err)
// NOTE: this uses amino explicitly, don't migrate it!
s.Require().NoError(s.cfg.LegacyAmino.UnmarshalJSON(res, &s.stdTxRes))
s.Require().Equal(uint32(0), s.stdTxRes.Code)
s.Require().NoError(s.network.WaitForNextBlock())
}
func (s *IntegrationTestSuite) TearDownSuite() {
@ -114,39 +127,48 @@ func (s *IntegrationTestSuite) TestBroadcastTxRequest() {
func (s *IntegrationTestSuite) TestQueryTxByHash() {
val0 := s.network.Validators[0]
// Create and broadcast a tx.
stdTx := s.createTestStdTx(val0, 1) // Validator's sequence starts at 1.
res, err := s.broadcastReq(stdTx, "block")
s.Require().NoError(err)
var txRes sdk.TxResponse
// NOTE: this uses amino explicitly, don't migrate it!
s.Require().NoError(s.cfg.LegacyAmino.UnmarshalJSON(res, &txRes))
// We broadcasted a StdTx in SetupSuite.
// we just check for a non-empty TxHash here, the actual hash will depend on the underlying tx configuration
s.Require().NotEmpty(txRes.TxHash)
s.Require().NotEmpty(s.stdTxRes.TxHash)
s.network.WaitForNextBlock()
// We now fetch the tx by has on the `/tx/{hash}` route.
txJSON, err := rest.GetRequest(fmt.Sprintf("%s/txs/%s", val0.APIAddress, txRes.TxHash))
// We now fetch the tx by hash on the `/tx/{hash}` route.
txJSON, err := rest.GetRequest(fmt.Sprintf("%s/txs/%s", val0.APIAddress, s.stdTxRes.TxHash))
s.Require().NoError(err)
// txJSON should contain the whole tx, we just make sure that our custom
// memo is there.
s.Require().True(strings.Contains(string(txJSON), stdTx.Memo))
s.Require().Contains(string(txJSON), s.stdTx.Memo)
}
func (s *IntegrationTestSuite) TestQueryTxByHeight() {
val0 := s.network.Validators[0]
// We broadcasted a StdTx in SetupSuite.
// we just check for a non-empty height here, as we'll need to for querying.
s.Require().NotEmpty(s.stdTxRes.Height)
// We now fetch the tx on `/txs` route, filtering by `tx.height`
txJSON, err := rest.GetRequest(fmt.Sprintf("%s/txs?limit=10&page=1&tx.height=%d", val0.APIAddress, s.stdTxRes.Height))
s.Require().NoError(err)
// txJSON should contain the whole tx, we just make sure that our custom
// memo is there.
s.Require().Contains(string(txJSON), s.stdTx.Memo)
}
func (s *IntegrationTestSuite) TestQueryTxByHashWithServiceMessage() {
val := s.network.Validators[0]
account, err := val.ClientCtx.Keyring.Key("newAccount")
s.Require().NoError(err)
sendTokens := sdk.NewInt64Coin(s.cfg.BondDenom, 10)
// Right after this line, we're sending a tx. Might need to wait a block
// to refresh sequences.
s.Require().NoError(s.network.WaitForNextBlock())
out, err := bankcli.ServiceMsgSendExec(
val.ClientCtx,
val.Address,
account.GetAddress(),
val.Address,
sdk.NewCoins(sendTokens),
fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation),
fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastBlock),
@ -157,18 +179,20 @@ func (s *IntegrationTestSuite) TestQueryTxByHashWithServiceMessage() {
s.Require().NoError(err)
var txRes sdk.TxResponse
s.Require().NoError(val.ClientCtx.JSONMarshaler.UnmarshalJSON(out.Bytes(), &txRes))
s.Require().Equal(uint32(0), txRes.Code)
s.Require().NoError(s.network.WaitForNextBlock())
txJSON, err := rest.GetRequest(fmt.Sprintf("%s/txs/%s", val.APIAddress, txRes.TxHash))
s.Require().NoError(err)
var result legacytx.StdTx
s.Require().NoError(val.ClientCtx.LegacyAmino.UnmarshalJSON(txJSON, &result))
s.Require().NotNil(result)
msgs := result.GetMsgs()
var txResAmino sdk.TxResponse
s.Require().NoError(val.ClientCtx.LegacyAmino.UnmarshalJSON(txJSON, &txResAmino))
stdTx, ok := txResAmino.Tx.GetCachedValue().(legacytx.StdTx)
s.Require().True(ok)
msgs := stdTx.GetMsgs()
s.Require().Equal(len(msgs), 1)
_, ok := msgs[0].(*types.MsgSend)
_, ok = msgs[0].(*types.MsgSend)
s.Require().True(ok)
}
@ -197,9 +221,8 @@ func (s *IntegrationTestSuite) TestMultipleSyncBroadcastTxRequests() {
}
for _, tc := range testCases {
s.Run(fmt.Sprintf("Case %s", tc.desc), func() {
// broadcast test with sync mode, as we want to run CheckTx to verify account sequence is correct
stdTx := s.createTestStdTx(s.network.Validators[0], tc.sequence)
stdTx := s.createTestStdTx(s.network.Validators[1], 1, tc.sequence)
res, err := s.broadcastReq(stdTx, "sync")
s.Require().NoError(err)
@ -224,7 +247,7 @@ func (s *IntegrationTestSuite) TestMultipleSyncBroadcastTxRequests() {
}
}
func (s *IntegrationTestSuite) createTestStdTx(val *network.Validator, sequence uint64) legacytx.StdTx {
func (s *IntegrationTestSuite) createTestStdTx(val *network.Validator, accNum, sequence uint64) legacytx.StdTx {
txConfig := legacytx.StdTxConfig{Cdc: s.cfg.LegacyAmino}
msg := &types.MsgSend{
@ -248,6 +271,7 @@ func (s *IntegrationTestSuite) createTestStdTx(val *network.Validator, sequence
WithKeybase(val.ClientCtx.Keyring).
WithTxConfig(txConfig).
WithSignMode(signing.SignMode_SIGN_MODE_LEGACY_AMINO_JSON).
WithAccountNumber(accNum).
WithSequence(sequence)
// sign Tx (offline mode so we can manually set sequence number)