From 81dfd12665982d996698ece9a00502b1a0fc1938 Mon Sep 17 00:00:00 2001 From: Rob Mulholand Date: Wed, 27 Mar 2019 12:26:04 -0500 Subject: [PATCH] Address PR comments --- libraries/shared/transactions/syncer.go | 8 +-- libraries/shared/transactions/syncer_test.go | 33 ++++------ pkg/datastore/errors.go | 8 +-- .../repositories/header_repository.go | 15 +++-- .../repositories/header_repository_test.go | 65 +++++++++---------- pkg/datastore/repository.go | 2 +- pkg/fakes/mock_header_repository.go | 10 +-- .../converters/rpc/transaction_converter.go | 2 +- 8 files changed, 69 insertions(+), 74 deletions(-) diff --git a/libraries/shared/transactions/syncer.go b/libraries/shared/transactions/syncer.go index 5f3085c7..8e043dd7 100644 --- a/libraries/shared/transactions/syncer.go +++ b/libraries/shared/transactions/syncer.go @@ -32,11 +32,9 @@ func (syncer TransactionsSyncer) SyncTransactions(headerID int64, logs []types.L if transactionErr != nil { return transactionErr } - for _, transaction := range transactions { - writeErr := syncer.Repository.CreateTransaction(headerID, transaction) - if writeErr != nil { - return writeErr - } + writeErr := syncer.Repository.CreateTransactions(headerID, transactions) + if writeErr != nil { + return writeErr } return nil } diff --git a/libraries/shared/transactions/syncer_test.go b/libraries/shared/transactions/syncer_test.go index 954d2a0b..53df7817 100644 --- a/libraries/shared/transactions/syncer_test.go +++ b/libraries/shared/transactions/syncer_test.go @@ -11,11 +11,19 @@ import ( ) var _ = Describe("Transaction syncer", func() { - It("fetches transactions for logs", func() { - db := test_config.NewTestDB(test_config.NewTestNode()) - blockChain := fakes.NewMockBlockChain() - syncer := transactions.NewTransactionsSyncer(db, blockChain) + var ( + blockChain *fakes.MockBlockChain + syncer transactions.TransactionsSyncer + ) + BeforeEach(func() { + db := test_config.NewTestDB(test_config.NewTestNode()) + test_config.CleanTestDB(db) + blockChain = fakes.NewMockBlockChain() + syncer = transactions.NewTransactionsSyncer(db, blockChain) + }) + + It("fetches transactions for logs", func() { err := syncer.SyncTransactions(0, []types.Log{}) Expect(err).NotTo(HaveOccurred()) @@ -23,10 +31,6 @@ var _ = Describe("Transaction syncer", func() { }) It("only fetches transactions with unique hashes", func() { - db := test_config.NewTestDB(test_config.NewTestNode()) - blockChain := fakes.NewMockBlockChain() - syncer := transactions.NewTransactionsSyncer(db, blockChain) - err := syncer.SyncTransactions(0, []types.Log{{ TxHash: fakes.FakeHash, }, { @@ -38,10 +42,7 @@ var _ = Describe("Transaction syncer", func() { }) It("returns error if fetching transactions fails", func() { - db := test_config.NewTestDB(test_config.NewTestNode()) - blockChain := fakes.NewMockBlockChain() blockChain.GetTransactionsError = fakes.FakeError - syncer := transactions.NewTransactionsSyncer(db, blockChain) err := syncer.SyncTransactions(0, []types.Log{}) @@ -50,26 +51,20 @@ var _ = Describe("Transaction syncer", func() { }) It("passes transactions to repository for persistence", func() { - db := test_config.NewTestDB(test_config.NewTestNode()) - blockChain := fakes.NewMockBlockChain() blockChain.Transactions = []core.TransactionModel{{}} - syncer := transactions.NewTransactionsSyncer(db, blockChain) mockHeaderRepository := fakes.NewMockHeaderRepository() syncer.Repository = mockHeaderRepository err := syncer.SyncTransactions(0, []types.Log{}) Expect(err).NotTo(HaveOccurred()) - Expect(mockHeaderRepository.CreateTransactionCalled).To(BeTrue()) + Expect(mockHeaderRepository.CreateTransactionsCalled).To(BeTrue()) }) It("returns error if persisting transactions fails", func() { - db := test_config.NewTestDB(test_config.NewTestNode()) - blockChain := fakes.NewMockBlockChain() blockChain.Transactions = []core.TransactionModel{{}} - syncer := transactions.NewTransactionsSyncer(db, blockChain) mockHeaderRepository := fakes.NewMockHeaderRepository() - mockHeaderRepository.CreateTransactionError = fakes.FakeError + mockHeaderRepository.CreateTransactionsError = fakes.FakeError syncer.Repository = mockHeaderRepository err := syncer.SyncTransactions(0, []types.Log{}) diff --git a/pkg/datastore/errors.go b/pkg/datastore/errors.go index ec734427..97da92e5 100644 --- a/pkg/datastore/errors.go +++ b/pkg/datastore/errors.go @@ -2,18 +2,18 @@ package datastore import "fmt" -var ErrBlockDoesNotExist = func(blockNumber int64) error { +func ErrBlockDoesNotExist(blockNumber int64) error { return fmt.Errorf("Block number %d does not exist", blockNumber) } -var ErrContractDoesNotExist = func(contractHash string) error { +func ErrContractDoesNotExist(contractHash string) error { return fmt.Errorf("Contract %v does not exist", contractHash) } -var ErrFilterDoesNotExist = func(name string) error { +func ErrFilterDoesNotExist(name string) error { return fmt.Errorf("filter %s does not exist", name) } -var ErrReceiptDoesNotExist = func(txHash string) error { +func ErrReceiptDoesNotExist(txHash string) error { return fmt.Errorf("Receipt for tx: %v does not exist", txHash) } diff --git a/pkg/datastore/postgres/repositories/header_repository.go b/pkg/datastore/postgres/repositories/header_repository.go index f2235365..6ee990f8 100644 --- a/pkg/datastore/postgres/repositories/header_repository.go +++ b/pkg/datastore/postgres/repositories/header_repository.go @@ -49,14 +49,19 @@ func (repository HeaderRepository) CreateOrUpdateHeader(header core.Header) (int return 0, ErrValidHeaderExists } -func (repository HeaderRepository) CreateTransaction(headerID int64, transaction core.TransactionModel) error { - _, err := repository.database.Exec(`INSERT INTO public.light_sync_transactions +func (repository HeaderRepository) CreateTransactions(headerID int64, transactions []core.TransactionModel) error { + for _, transaction := range transactions { + _, err := repository.database.Exec(`INSERT INTO public.light_sync_transactions (header_id, hash, gaslimit, gasprice, input_data, nonce, raw, tx_from, tx_index, tx_to, "value") VALUES ($1, $2, $3::NUMERIC, $4::NUMERIC, $5, $6::NUMERIC, $7, $8, $9::NUMERIC, $10, $11::NUMERIC) ON CONFLICT DO NOTHING`, headerID, transaction.Hash, transaction.GasLimit, transaction.GasPrice, - transaction.Data, transaction.Nonce, transaction.Raw, transaction.From, transaction.TxIndex, transaction.To, - transaction.Value) - return err + transaction.Data, transaction.Nonce, transaction.Raw, transaction.From, transaction.TxIndex, transaction.To, + transaction.Value) + if err != nil { + return err + } + } + return nil } func (repository HeaderRepository) GetHeader(blockNumber int64) (core.Header, error) { diff --git a/pkg/datastore/postgres/repositories/header_repository_test.go b/pkg/datastore/postgres/repositories/header_repository_test.go index ef491306..a22f6131 100644 --- a/pkg/datastore/postgres/repositories/header_repository_test.go +++ b/pkg/datastore/postgres/repositories/header_repository_test.go @@ -181,14 +181,21 @@ var _ = Describe("Block header repository", func() { }) Describe("creating a transaction", func() { - It("adds a transaction", func() { - headerID, err := repo.CreateOrUpdateHeader(header) + var ( + headerID int64 + transactions []core.TransactionModel + ) + + BeforeEach(func() { + var err error + headerID, err = repo.CreateOrUpdateHeader(header) Expect(err).NotTo(HaveOccurred()) fromAddress := common.HexToAddress("0x1234") toAddress := common.HexToAddress("0x5678") txHash := common.HexToHash("0x9876") + txHashTwo := common.HexToHash("0x5432") txIndex := big.NewInt(123) - transaction := core.TransactionModel{ + transactions = []core.TransactionModel{{ Data: []byte{}, From: fromAddress.Hex(), GasLimit: 0, @@ -199,44 +206,34 @@ var _ = Describe("Block header repository", func() { To: toAddress.Hex(), TxIndex: txIndex.Int64(), Value: "0", - } - - insertErr := repo.CreateTransaction(headerID, transaction) + }, { + Data: []byte{}, + From: fromAddress.Hex(), + GasLimit: 1, + GasPrice: 1, + Hash: txHashTwo.Hex(), + Nonce: 1, + Raw: []byte{}, + To: toAddress.Hex(), + TxIndex: 1, + Value: "1", + }} + insertErr := repo.CreateTransactions(headerID, transactions) Expect(insertErr).NotTo(HaveOccurred()) - var dbTransaction core.TransactionModel - err = db.Get(&dbTransaction, + }) + + It("adds transactions", func() { + var dbTransactions []core.TransactionModel + err = db.Select(&dbTransactions, `SELECT hash, gaslimit, gasprice, input_data, nonce, raw, tx_from, tx_index, tx_to, "value" FROM public.light_sync_transactions WHERE header_id = $1`, headerID) Expect(err).NotTo(HaveOccurred()) - Expect(dbTransaction).To(Equal(transaction)) + Expect(dbTransactions).To(ConsistOf(transactions)) }) It("silently ignores duplicate inserts", func() { - headerID, err := repo.CreateOrUpdateHeader(header) - Expect(err).NotTo(HaveOccurred()) - fromAddress := common.HexToAddress("0x1234") - toAddress := common.HexToAddress("0x5678") - txHash := common.HexToHash("0x9876") - txIndex := big.NewInt(123) - transaction := core.TransactionModel{ - Data: []byte{}, - From: fromAddress.Hex(), - GasLimit: 0, - GasPrice: 0, - Hash: txHash.Hex(), - Nonce: 0, - Raw: []byte{}, - Receipt: core.Receipt{}, - To: toAddress.Hex(), - TxIndex: txIndex.Int64(), - Value: "0", - } - - insertErr := repo.CreateTransaction(headerID, transaction) - Expect(insertErr).NotTo(HaveOccurred()) - - insertTwoErr := repo.CreateTransaction(headerID, transaction) + insertTwoErr := repo.CreateTransactions(headerID, transactions) Expect(insertTwoErr).NotTo(HaveOccurred()) var dbTransactions []core.TransactionModel @@ -244,7 +241,7 @@ var _ = Describe("Block header repository", func() { `SELECT hash, gaslimit, gasprice, input_data, nonce, raw, tx_from, tx_index, tx_to, "value" FROM public.light_sync_transactions WHERE header_id = $1`, headerID) Expect(err).NotTo(HaveOccurred()) - Expect(len(dbTransactions)).To(Equal(1)) + Expect(len(dbTransactions)).To(Equal(2)) }) }) diff --git a/pkg/datastore/repository.go b/pkg/datastore/repository.go index 0b5af3dd..db0c220b 100644 --- a/pkg/datastore/repository.go +++ b/pkg/datastore/repository.go @@ -41,7 +41,7 @@ type FilterRepository interface { type HeaderRepository interface { CreateOrUpdateHeader(header core.Header) (int64, error) - CreateTransaction(headerID int64, transaction core.TransactionModel) error + CreateTransactions(headerID int64, transactions []core.TransactionModel) error GetHeader(blockNumber int64) (core.Header, error) MissingBlockNumbers(startingBlockNumber, endingBlockNumber int64, nodeID string) ([]int64, error) } diff --git a/pkg/fakes/mock_header_repository.go b/pkg/fakes/mock_header_repository.go index c89d8f16..8f268146 100644 --- a/pkg/fakes/mock_header_repository.go +++ b/pkg/fakes/mock_header_repository.go @@ -27,8 +27,8 @@ type MockHeaderRepository struct { createOrUpdateHeaderErr error createOrUpdateHeaderPassedBlockNumbers []int64 createOrUpdateHeaderReturnID int64 - CreateTransactionCalled bool - CreateTransactionError error + CreateTransactionsCalled bool + CreateTransactionsError error getHeaderError error getHeaderReturnBlockHash string missingBlockNumbers []int64 @@ -58,9 +58,9 @@ func (repository *MockHeaderRepository) CreateOrUpdateHeader(header core.Header) return repository.createOrUpdateHeaderReturnID, repository.createOrUpdateHeaderErr } -func (repository *MockHeaderRepository) CreateTransaction(headerID int64, transaction core.TransactionModel) error { - repository.CreateTransactionCalled = true - return repository.CreateTransactionError +func (repository *MockHeaderRepository) CreateTransactions(headerID int64, transactions []core.TransactionModel) error { + repository.CreateTransactionsCalled = true + return repository.CreateTransactionsError } func (repository *MockHeaderRepository) GetHeader(blockNumber int64) (core.Header, error) { diff --git a/pkg/geth/converters/rpc/transaction_converter.go b/pkg/geth/converters/rpc/transaction_converter.go index 067a1e99..1c06ac87 100644 --- a/pkg/geth/converters/rpc/transaction_converter.go +++ b/pkg/geth/converters/rpc/transaction_converter.go @@ -25,8 +25,8 @@ import ( "strings" "github.com/ethereum/go-ethereum/common" - "github.com/ethereum/go-ethereum/rlp" "github.com/ethereum/go-ethereum/core/types" + "github.com/ethereum/go-ethereum/rlp" "golang.org/x/sync/errgroup" "github.com/vulcanize/vulcanizedb/pkg/core"