From 1a2f46b3088e701e537a5df5bb587d2978880cb7 Mon Sep 17 00:00:00 2001 From: Edvard Date: Wed, 20 Feb 2019 12:01:19 +0100 Subject: [PATCH] Fix small issues from review --- integration_test/geth_blockchain_test.go | 11 ++++++++--- .../postgres/repositories/block_repository.go | 6 ++---- .../postgres/repositories/logs_repository.go | 13 ++++++++++--- .../postgres/repositories/logs_repository_test.go | 15 +++++++++++---- pkg/geth/blockchain_test.go | 3 ++- 5 files changed, 33 insertions(+), 15 deletions(-) diff --git a/integration_test/geth_blockchain_test.go b/integration_test/geth_blockchain_test.go index 2cd85cb3..bec8d39b 100644 --- a/integration_test/geth_blockchain_test.go +++ b/integration_test/geth_blockchain_test.go @@ -48,9 +48,13 @@ var _ = Describe("Reading from the Geth blockchain", func() { It("reads two blocks", func(done Done) { blocks := fakes.NewMockBlockRepository() - lastBlock, _ := blockChain.LastBlock() + lastBlock, err := blockChain.LastBlock() + Expect(err).NotTo(HaveOccurred()) + queriedBlocks := []int64{lastBlock.Int64() - 5, lastBlock.Int64() - 6} - history.RetrieveAndUpdateBlocks(blockChain, blocks, queriedBlocks) + _, err = history.RetrieveAndUpdateBlocks(blockChain, blocks, queriedBlocks) + Expect(err).NotTo(HaveOccurred()) + blocks.AssertCreateOrUpdateBlocksCallCountAndBlockNumbersEquals(2, []int64{lastBlock.Int64() - 5, lastBlock.Int64() - 6}) close(done) }, 30) @@ -60,8 +64,9 @@ var _ = Describe("Reading from the Geth blockchain", func() { Expect(err).ToNot(HaveOccurred()) firstBlock, err := blockChain.GetBlockByNumber(int64(1)) Expect(err).ToNot(HaveOccurred()) - lastBlockNumber, _ := blockChain.LastBlock() + lastBlockNumber, err := blockChain.LastBlock() + Expect(err).NotTo(HaveOccurred()) Expect(genesisBlock.Number).To(Equal(int64(0))) Expect(firstBlock.Number).To(Equal(int64(1))) Expect(lastBlockNumber.Int64()).To(BeNumerically(">", 0)) diff --git a/pkg/datastore/postgres/repositories/block_repository.go b/pkg/datastore/postgres/repositories/block_repository.go index ff52fe5a..4a5d19ff 100644 --- a/pkg/datastore/postgres/repositories/block_repository.go +++ b/pkg/datastore/postgres/repositories/block_repository.go @@ -49,10 +49,8 @@ func (blockRepository BlockRepository) SetBlocksStatus(chainHead int64) error { UPDATE blocks SET is_final = TRUE WHERE is_final = FALSE AND number < $1`, cutoff) - if err != nil { - return err - } - return nil + + return err } func (blockRepository BlockRepository) CreateOrUpdateBlock(block core.Block) (int64, error) { diff --git a/pkg/datastore/postgres/repositories/logs_repository.go b/pkg/datastore/postgres/repositories/logs_repository.go index 9bdaca84..704ac350 100644 --- a/pkg/datastore/postgres/repositories/logs_repository.go +++ b/pkg/datastore/postgres/repositories/logs_repository.go @@ -40,13 +40,19 @@ func (logRepository LogRepository) CreateLogs(lgs []core.Log, receiptId int64) e tlog.BlockNumber, tlog.Address, tlog.TxHash, tlog.Index, tlog.Topics[0], tlog.Topics[1], tlog.Topics[2], tlog.Topics[3], tlog.Data, receiptId, ) if err != nil { - tx.Rollback() + err = tx.Rollback() + if err != nil { + logrus.Error("CreateLogs: could not perform rollback: ", err) + } return postgres.ErrDBInsertFailed } } err := tx.Commit() if err != nil { - tx.Rollback() + err = tx.Rollback() + if err != nil { + logrus.Error("CreateLogs: could not perform rollback: ", err) + } return postgres.ErrDBInsertFailed } return nil @@ -83,7 +89,8 @@ func (logRepository LogRepository) loadLogs(logsRows *sql.Rows) ([]core.Log, err var topics core.Topics err := logsRows.Scan(&blockNumber, &address, &txHash, &index, &topics[0], &topics[1], &topics[2], &topics[3], &data) if err != nil { - logrus.Warn("loadLogs: Error scanning a row in logRows") + logrus.Error("loadLogs: Error scanning a row in logRows: ", err) + return []core.Log{}, err } lg := core.Log{ BlockNumber: blockNumber, diff --git a/pkg/datastore/postgres/repositories/logs_repository_test.go b/pkg/datastore/postgres/repositories/logs_repository_test.go index 1402d672..e6923b85 100644 --- a/pkg/datastore/postgres/repositories/logs_repository_test.go +++ b/pkg/datastore/postgres/repositories/logs_repository_test.go @@ -56,7 +56,7 @@ var _ = Describe("Logs Repository", func() { Expect(err).NotTo(HaveOccurred()) receiptId, err := receiptRepository.CreateReceipt(blockId, core.Receipt{}) Expect(err).NotTo(HaveOccurred()) - logsRepository.CreateLogs([]core.Log{{ + err = logsRepository.CreateLogs([]core.Log{{ BlockNumber: blockNumber, Index: 0, Address: "x123", @@ -64,6 +64,7 @@ var _ = Describe("Logs Repository", func() { Topics: core.Topics{0: "x777", 1: "x888", 2: "x999"}, Data: "xabc", }}, receiptId) + Expect(err).NotTo(HaveOccurred()) log, err := logsRepository.GetLogs("x123", blockNumber) @@ -91,7 +92,8 @@ var _ = Describe("Logs Repository", func() { Expect(err).NotTo(HaveOccurred()) receiptId, err := receiptRepository.CreateReceipt(blockId, core.Receipt{}) Expect(err).NotTo(HaveOccurred()) - _ = logsRepository.CreateLogs([]core.Log{{ + + err = logsRepository.CreateLogs([]core.Log{{ BlockNumber: blockNumber, Index: 0, Address: "x123", @@ -99,7 +101,9 @@ var _ = Describe("Logs Repository", func() { Topics: core.Topics{0: "x777", 1: "x888", 2: "x999"}, Data: "xabc", }}, receiptId) - _ = logsRepository.CreateLogs([]core.Log{{ + Expect(err).NotTo(HaveOccurred()) + + err = logsRepository.CreateLogs([]core.Log{{ BlockNumber: blockNumber, Index: 1, Address: "x123", @@ -107,7 +111,9 @@ var _ = Describe("Logs Repository", func() { Topics: core.Topics{0: "x111", 1: "x222", 2: "x333"}, Data: "xdef", }}, receiptId) - _ = logsRepository.CreateLogs([]core.Log{{ + Expect(err).NotTo(HaveOccurred()) + + err = logsRepository.CreateLogs([]core.Log{{ BlockNumber: 2, Index: 0, Address: "x123", @@ -115,6 +121,7 @@ var _ = Describe("Logs Repository", func() { Topics: core.Topics{0: "x777", 1: "x888", 2: "x999"}, Data: "xabc", }}, receiptId) + Expect(err).NotTo(HaveOccurred()) log, err := logsRepository.GetLogs("x123", blockNumber) Expect(err).NotTo(HaveOccurred()) diff --git a/pkg/geth/blockchain_test.go b/pkg/geth/blockchain_test.go index 370563e2..5d011f72 100644 --- a/pkg/geth/blockchain_test.go +++ b/pkg/geth/blockchain_test.go @@ -221,7 +221,8 @@ var _ = Describe("Geth blockchain", func() { blockNumber := int64(100) mockClient.SetHeaderByNumberReturnHeader(&types.Header{Number: big.NewInt(blockNumber)}) - result, _ := blockChain.LastBlock() + result, err := blockChain.LastBlock() + Expect(err).NotTo(HaveOccurred()) mockClient.AssertHeaderByNumberCalledWith(context.Background(), nil) Expect(result).To(Equal(big.NewInt(blockNumber)))