Repository: trustwallet/blockatlas
Pull request #883 (2020-02-17 21:41:09)

fix Zilliqa block request and data race
# Headline

- Fix block parser for Zilliqa
- Fix data race
- Ignore 'TxBlock has no transactions' error to avoid retries

## Issue

- The object to parse block transactions was wrong
- Data race
- If the block doesn't have a transaction, the retry system tries to fetch the block again

closes https://github.com/trustwallet/blockatlas/issues/863
closes https://github.com/trustwallet/blockatlas/issues/885


Patch:
diff --git a/platform/nimiq/transaction.go b/platform/nimiq/transaction.go
index dc220aa..3bfba60 100644
--- a/platform/nimiq/transaction.go
+++ b/platform/nimiq/transaction.go
@@ -3,6 +3,7 @@ package nimiq
 import (
 	"github.com/trustwallet/blockatlas/coin"
 	"github.com/trustwallet/blockatlas/pkg/blockatlas"
+	"time"
 )
 
 func (p *Platform) GetTxsByAddress(address string) (blockatlas.TxPage, error) {
@@ -15,7 +16,11 @@ func (p *Platform) GetTxsByAddress(address string) (blockatlas.TxPage, error) {
 
 // NormalizeTx converts a Nimiq transaction into the generic model
 func NormalizeTx(srcTx *Tx) blockatlas.Tx {
-	date, _ := srcTx.Timestamp.Int64()
+	date, err := srcTx.Timestamp.Int64()
+	// Pending transaction doesn't have a timestamp, we gonna use the current time
+	if err != nil || len(srcTx.BlockHash) == 0 {
+		date = time.Now().Unix()
+	}
 	return blockatlas.Tx{
 		ID:    srcTx.Hash,
 		Coin:  coin.NIM,
diff --git a/platform/zilliqa/block.go b/platform/zilliqa/block.go
index dbfb6c2..4180c65 100644
--- a/platform/zilliqa/block.go
+++ b/platform/zilliqa/block.go
@@ -30,10 +30,10 @@ func (p *Platform) GetBlockByNumber(num int64) (*blockatlas.Block, error) {
 		tx := Normalize(&srcTx)
 		normalized = append(normalized, tx)
 	}
+
 	block := blockatlas.Block{
 		Number: num,
 		Txs:    normalized,
 	}
-
 	return &block, nil
 }
diff --git a/platform/zilliqa/model.go b/platform/zilliqa/model.go
index f4ab3a1..77f2357 100644
--- a/platform/zilliqa/model.go
+++ b/platform/zilliqa/model.go
@@ -2,10 +2,23 @@ package zilliqa
 
 import (
 	"encoding/hex"
+	"github.com/trustwallet/blockatlas/pkg/blockatlas"
 	"math/big"
 	"strconv"
 )
 
+type BlockTxs [][]string
+
+func (b BlockTxs) txs() []string {
+	txs := make([]string, 0)
+	for _, ids := range b {
+		for _, id := range ids {
+			txs = append(txs, id)
+		}
+	}
+	return txs
+}
+
 type Tx struct {
 	Hash           string      `json:"hash"`
 	BlockHeight    uint64      `json:"blockHeight"`
@@ -75,3 +88,10 @@ func (t *TxRPC) toTx() Tx {
 	}
 	return tx
 }
+
+type BlockTxRpc struct {
+	JsonRpc string               `json:"jsonrpc"`
+	Error   *blockatlas.RpcError `json:"error,omitempty"`
+	Result  BlockTxs             `json:"result,omitempty"`
+	Id      string               `json:"id,omitempty"`
+}
diff --git a/platform/zilliqa/rpc.go b/platform/zilliqa/rpc.go
index d64a468..966dc96 100644
--- a/platform/zilliqa/rpc.go
+++ b/platform/zilliqa/rpc.go
@@ -2,6 +2,7 @@ package zilliqa
 
 import (
 	"github.com/trustwallet/blockatlas/pkg/blockatlas"
+	"github.com/trustwallet/blockatlas/pkg/errors"
 	"strconv"
 	"sync"
 )
@@ -15,37 +16,58 @@ func (c *RpcClient) GetBlockchainInfo() (info *ChainInfo, err error) {
 	return
 }
 
-func (c *RpcClient) GetTx(hash string) (tx Tx, err error) {
+func (c *RpcClient) GetTx(hash string) (tx TxRPC, err error) {
 	err = c.RpcCall(&tx, "GetTransaction", []string{hash})
 	return
 }
 
-func (c *RpcClient) GetTxInBlock(number int64) (txs []Tx, err error) {
-	strNumber := strconv.FormatInt(number, 10)
-	var results [][]string
-	err = c.RpcCall(&results, "GetTransactionsForTxBlock", []string{strNumber})
+func (c *RpcClient) GetBlockByNumber(number int64) ([]string, error) {
+	strNumber := strconv.Itoa(int(number))
+	req := &blockatlas.RpcRequest{
+		JsonRpc: blockatlas.JsonRpcVersion,
+		Method:  "GetTransactionsForTxBlock",
+		Params:  []string{strNumber},
+		Id:      "GetTransactionsForTxBlock_" + strNumber,
+	}
+	var resp *BlockTxRpc
+	err := c.Post(&resp, "", req)
+	if err != nil {
+		return nil, err
+	}
+	if resp.Error != nil && resp.Error.Code != -1 {
+		return nil, errors.E("RPC Call error", errors.Params{
+			"method":        "GetTransactionsForTxBlock",
+			"error_code":    resp.Error.Code,
+			"error_message": resp.Error.Message})
+	}
+	return resp.Result.txs(), nil
+}
+
+func (c *RpcClient) GetTxInBlock(number int64) ([]Tx, error) {
+	txs := make([]Tx, 0)
+	hashes, err := c.GetBlockByNumber(number)
+	if err != nil {
+		return txs, err
+	}
 
 	var wg sync.WaitGroup
-	out := make(chan Tx)
-	for _, ids := range results {
-		for _, id := range ids {
-			wg.Add(1)
-			go func(id string) {
-				defer wg.Done()
-				tx, errTx := c.GetTx(id)
-				if errTx != nil {
-					return
-				}
-				out <- tx
-			}(id)
-		}
+	out := make(chan Tx, len(hashes))
+	wg.Add(len(hashes))
+	for _, id := range hashes {
+		go func(hash string, txChan chan Tx, wg *sync.WaitGroup) {
+			defer wg.Done()
+			tx, errTx := c.GetTx(hash)
+			if errTx != nil {
+				return
+			}
+			txChan <- tx.toTx()
+		}(id, out, &wg)
 	}
-	go func() {
-		for tx := range out {
-			txs = append(txs, tx)
-		}
-	}()
 	wg.Wait()
 	close(out)
-	return
+
+	for tx := range out {
+		txs = append(txs, tx)
+	}
+	return txs, nil
 }


Test patch:
diff --git a/platform/nimiq/transaction_test.go b/platform/nimiq/transaction_test.go
index 8699887..edbc3a4 100644
--- a/platform/nimiq/transaction_test.go
+++ b/platform/nimiq/transaction_test.go
@@ -1,14 +1,16 @@
 package nimiq
 
 import (
-	"bytes"
 	"encoding/json"
+	"github.com/stretchr/testify/assert"
 	"github.com/trustwallet/blockatlas/coin"
 	"github.com/trustwallet/blockatlas/pkg/blockatlas"
 	"testing"
+	"time"
 )
 
-const basicSrc = `
+const (
+	basicSrc = `
 {
 	"hash": "8b219949f4c1dfe9e7a9cdc5dbbc507e40dc16f44a1a5182ed6125c9a6891a50",
 	"blockHash": "ab36a0909c6ed5761a984ef261d9c3456b7c1aea6a52d531c5bf2518526a32e6",
@@ -26,44 +28,71 @@ const basicSrc = `
 	"flags": 0
 }
 `
-
-var basicDst = blockatlas.Tx{
-	ID:    "8b219949f4c1dfe9e7a9cdc5dbbc507e40dc16f44a1a5182ed6125c9a6891a50",
-	Coin:  coin.NIM,
-	From:  "NQ69 9A4A MB83 HXDQ 4J46 BH5R 4JFF QMA9 C3GN",
-	To:    "NQ15 MLJN 23YB 8FBM 61TN 7LYG 2212 LVBG 4V19",
-	Fee:   "138",
-	Date:  1538924505,
-	Block: 252575,
-	Meta: blockatlas.Transfer{
-		Value:    "10000000000000",
-		Symbol:   "NIM",
-		Decimals: 5,
-	},
+	pendingSrc = `
+{
+  "data": null,
+  "fee": 300,
+  "flags": 0,
+  "from": "d48182276127b149a9710e78c436fb4bc1c4dc0b",
+  "fromAddress": "NQ74 SJ0Q 49T1 4XQL KABH 1RUC 8DPT 9F0U 9P0B",
+  "hash": "79719d16f3f347cc98c35cd7a9af708cdce97de578b5135c5ae4393fd7920d61",
+  "to": "0a17218ffdc42385f45329ba4089919236dd2743",
+  "toAddress": "NQ97 18BJ 33YV QGHQ BV2K 56V4 12CH J8TD S9S3",
+  "value": 100000
 }
+`
+)
 
-func TestNormalizeTx(t *testing.T) {
-	var srcTx Tx
-	err := json.Unmarshal([]byte(basicSrc), &srcTx)
-	if err != nil {
-		t.Error(err)
-		return
+var (
+	basicDst = blockatlas.Tx{
+		ID:    "8b219949f4c1dfe9e7a9cdc5dbbc507e40dc16f44a1a5182ed6125c9a6891a50",
+		Coin:  coin.NIM,
+		From:  "NQ69 9A4A MB83 HXDQ 4J46 BH5R 4JFF QMA9 C3GN",
+		To:    "NQ15 MLJN 23YB 8FBM 61TN 7LYG 2212 LVBG 4V19",
+		Fee:   "138",
+		Date:  1538924505,
+		Block: 252575,
+		Meta: blockatlas.Transfer{
+			Value:    "10000000000000",
+			Symbol:   "NIM",
+			Decimals: 5,
+		},
 	}
-
-	tx := NormalizeTx(&srcTx)
-	resJSON, err := json.Marshal(&tx)
-	if err != nil {
-		t.Fatal(err)
+	pendingDst = blockatlas.Tx{
+		ID:    "79719d16f3f347cc98c35cd7a9af708cdce97de578b5135c5ae4393fd7920d61",
+		Coin:  coin.NIM,
+		From:  "NQ74 SJ0Q 49T1 4XQL KABH 1RUC 8DPT 9F0U 9P0B",
+		To:    "NQ97 18BJ 33YV QGHQ BV2K 56V4 12CH J8TD S9S3",
+		Fee:   "300",
+		Date:  time.Now().Unix(),
+		Block: 0,
+		Meta: blockatlas.Transfer{
+			Value:    "100000",
+			Symbol:   "NIM",
+			Decimals: 5,
+		},
 	}
+)
 
-	dstJSON, err := json.Marshal(&basicDst)
-	if err != nil {
-		t.Fatal(err)
+func TestNormalizeTx1(t *testing.T) {
+	tests := []struct {
+		name  string
+		srcTx string
+		want  blockatlas.Tx
+	}{
+		{"test transaction", basicSrc, basicDst},
+		{"test pending transaction", pendingSrc, pendingDst},
 	}
-
-	if !bytes.Equal(resJSON, dstJSON) {
-		println(string(resJSON))
-		println(string(dstJSON))
-		t.Error("basic: tx don't equal")
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			var srcTx Tx
+			err := json.Unmarshal([]byte(tt.srcTx), &srcTx)
+			if err != nil {
+				t.Error(err)
+				return
+			}
+			got := NormalizeTx(&srcTx)
+			assert.Equal(t, tt.want, got)
+		})
 	}
 }
diff --git a/platform/zilliqa/model_test.go b/platform/zilliqa/model_test.go
index 1178197..737894c 100644
--- a/platform/zilliqa/model_test.go
+++ b/platform/zilliqa/model_test.go
@@ -70,3 +70,25 @@ func TestTx_NonceValue(t *testing.T) {
 		})
 	}
 }
+
+func TestBlockTxs_txs(t *testing.T) {
+	tests := []struct {
+		name string
+		b    BlockTxs
+		want []string
+	}{
+		{"test 1 tx", BlockTxs{{"tx1"}, {}}, []string{"tx1"}},
+		{"test 2 txs  1", BlockTxs{{"tx1"}, {"tx2"}}, []string{"tx1", "tx2"}},
+		{"test 2 txs 2", BlockTxs{{"tx1", "tx2"}}, []string{"tx1", "tx2"}},
+		{"test 3 txs 1", BlockTxs{{"tx1", "tx2"}, {"tx3"}}, []string{"tx1", "tx2", "tx3"}},
+		{"test 3 txs 2", BlockTxs{{"tx1"}, {"tx2"}, {"tx3"}}, []string{"tx1", "tx2", "tx3"}},
+		{"test 4 txs", BlockTxs{{"tx1", "tx2"}, {"tx3"}, {"tx4"}}, []string{"tx1", "tx2", "tx3", "tx4"}},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := tt.b.txs(); !reflect.DeepEqual(got, tt.want) {
+				t.Errorf("txs() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}