Skip to content

Commit

Permalink
fix(api): query all possible ibc seq numbers instead of hardcoding th…
Browse files Browse the repository at this point in the history
…e JSON path (#52)

* fix(api): query all possible ibc seq numbers instead of hardcoding the JSON path

See comment on line 137.

* chore(tx): refactor ibc sequence number function to be more testable

Added unit tests around the gjson query, so that we can detect and easily fix when/if Tendermint/Cosmos SDK change anything in the IBC send structure.
  • Loading branch information
gsora authored Feb 16, 2022
1 parent 5381431 commit ee77570
Show file tree
Hide file tree
Showing 2 changed files with 158 additions and 8 deletions.
60 changes: 52 additions & 8 deletions api/tx/ibc.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,26 @@ import (
// can be updated to more readable format after typed events are introduced in sdk
const (
// packetSequencePath may need updates if ibc-go/transfer events change
packetSequencePath = "tx_response.logs.0.events.2.attributes.3.value"
packetSequencePath = `tx_response.logs.#.events.#(type=="send_packet").attributes.#(key=="packet_sequence").value`
// txPath may need updates if tendermint response changes
txPath = "result.txs.0.hash"
timeout = 10 * time.Second
)

// getIBCSeqFromTx returns a list of sequence numbers gotten from data,
// from the sent IBC packet event.
// If no IBC sequence numbers are found, the resulting slice is empty.
func getIBCSeqFromTx(data []byte) []string {
raw := gjson.GetBytes(data, packetSequencePath).Array()

ret := make([]string, 0, len(raw))
for _, r := range raw {
ret = append(ret, r.String())
}

return ret
}

// GetDestTx returns tx hash on destination chain.
// @Summary Gets tx hash on destination chain.
// @Tags Tx
Expand Down Expand Up @@ -134,19 +148,47 @@ func GetDestTx(c *gin.Context) {
return
}

r := gjson.GetBytes(sdkRes, packetSequencePath)
url := fmt.Sprintf("http://%s:26657/tx_search?query=\"recv_packet.packet_sequence=%s\"", destChainInfo.ChainName, r.String())
// This query always returns an array of sequence numbers.
// Emeris-generated IBC transfers are always sent out alone, meaning that
// there are no more than 1 IBC transfer per tx.
// This code is ready to be adapted to support multiple IBC transfer/transaction, but
// for now we just get the first seq number found and roll with it.
r := getIBCSeqFromTx(sdkRes)
if len(r) == 0 {
e := deps.NewError(
"chains",
fmt.Errorf("provided transaction is not ibc transfer"),
http.StatusBadRequest,
)

d.WriteError(c, e,
"provided transaction is not ibc transfer",
"id",
e.ID,
"txHash",
txHash,
"src srcChainInfo name",
srcChain,
"error",
err,
)

return
}

seqNum := r[0]
url := fmt.Sprintf("http://%s:26657/tx_search?query=\"recv_packet.packet_sequence=%s\"", destChainInfo.ChainName, seqNum)

httpClient := &http.Client{
Timeout: timeout,
}

// we're validating inputs and hence gosec-G107 can be ignored
resp, err := httpClient.Get(url) // nolint: gosec
if err != nil {
if err != nil || resp.StatusCode != http.StatusOK {
e := deps.NewError(
"chains",
fmt.Errorf("cannot retrieve tx with packet sequence %s on %s", r.String(), destChain),
fmt.Errorf("cannot retrieve tx with packet sequence %s on %s", seqNum, destChain),
http.StatusBadRequest,
)

Expand All @@ -160,6 +202,8 @@ func GetDestTx(c *gin.Context) {
destChain,
"error",
err,
"status_code",
resp.Status,
)

return
Expand All @@ -170,7 +214,7 @@ func GetDestTx(c *gin.Context) {
if err != nil {
e := deps.NewError(
"chains",
fmt.Errorf("cannot retrieve tx with packet sequence %s on %s", r.String(), destChain),
fmt.Errorf("cannot retrieve tx with packet sequence %s on %s", seqNum, destChain),
http.StatusBadRequest,
)

Expand All @@ -189,9 +233,9 @@ func GetDestTx(c *gin.Context) {
return
}

r = gjson.GetBytes(bz, txPath)
otherSideTxHash := gjson.GetBytes(bz, txPath)
c.JSON(http.StatusOK, DestTxResponse{
DestChain: destChain,
TxHash: r.String(),
TxHash: otherSideTxHash.String(),
})
}
106 changes: 106 additions & 0 deletions api/tx/ibc_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
package tx

import (
"testing"

"github.com/stretchr/testify/require"
)

func Test_getIBCSeqFromTx(t *testing.T) {
// This test assumes tx_response.logs.events is always present,
// because Tendermint events are formatted this way.
tests := []struct {
name string
payload string
want []string
}{
{
"ibc send payload with a single transfer",
`{
"tx_response":{
"logs":[
{
"events":[
{
"type":"send_packet",
"attributes":[
{
"key":"packet_sequence",
"value":"42"
}
]
}
]
}
]
}
}`,
[]string{"42"},
},
{
"ibc send payload with multiple transfer",
`{
"tx_response":{
"logs":[
{
"events":[
{
"type":"send_packet",
"attributes":[
{
"key":"packet_sequence",
"value":"42"
}
]
}
]
},
{
"events":[
{
"type":"send_packet",
"attributes":[
{
"key":"packet_sequence",
"value":"44"
}
]
}
]
}
]
}
}`,
[]string{"42", "44"},
},
{
"transaction with no IBC-related tx's",
`{
"tx_response":{
"logs":[
{
"events":[
{
"type":"fake_event",
"attributes":[
{
"key":"packet_sequence",
"value":"42"
}
]
}
]
}
]
}
}`,
[]string{},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
res := getIBCSeqFromTx([]byte(tt.payload))
require.ElementsMatch(t, res, tt.want)
})
}
}

0 comments on commit ee77570

Please sign in to comment.