From aecdd74dc8c6967c4aceba0980adf1d1fb4a5107 Mon Sep 17 00:00:00 2001 From: Wany Date: Tue, 29 Mar 2022 15:44:48 +0800 Subject: [PATCH 1/4] fix GetTransactionReceiptsByBlock bug fix fields transactionHash --- internal/ethapi/api.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/internal/ethapi/api.go b/internal/ethapi/api.go index 0d3aeb35c6..f5d6016efe 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -670,6 +670,7 @@ func (s *PublicBlockChainAPI) GetTransactionReceiptsByBlock(ctx context.Context, "contractAddress": nil, "logs": receipt.Logs, "logsBloom": receipt.Bloom, + "type": hexutil.Uint(tx.Type()), } // Assign receipt status or post state. @@ -681,7 +682,7 @@ func (s *PublicBlockChainAPI) GetTransactionReceiptsByBlock(ctx context.Context, if receipt.Logs == nil { fields["logs"] = [][]*types.Log{} } - if borReceipt != nil { + if borReceipt != nil && idx == len(receipts)-1 { fields["transactionHash"] = txHash } // If the ContractAddress is 20 0x0 bytes, assume it is not a contract creation From b18370be6a86af68d9a031939eaa3690fc27bfbf Mon Sep 17 00:00:00 2001 From: Shivam Sharma Date: Tue, 2 Aug 2022 00:57:42 +0530 Subject: [PATCH 2/4] fix : TestGetTransactionReceiptsByBlock --- eth/backend.go | 4 + tests/bor/bor_api_test.go | 310 ++++++++++++++++++++--------------- tests/bor/bor_filter_test.go | 2 +- 3 files changed, 180 insertions(+), 136 deletions(-) diff --git a/eth/backend.go b/eth/backend.go index 8f2ae1ea2c..c159df7725 100644 --- a/eth/backend.go +++ b/eth/backend.go @@ -382,6 +382,10 @@ func (s *Ethereum) ResetWithGenesisBlock(gb *types.Block) { s.blockchain.ResetWithGenesisBlock(gb) } +func (s *Ethereum) PublicBlockChainAPI() *ethapi.PublicBlockChainAPI { + return s.handler.ethAPI +} + func (s *Ethereum) Etherbase() (eb common.Address, err error) { s.lock.RLock() etherbase := s.etherbase diff --git a/tests/bor/bor_api_test.go b/tests/bor/bor_api_test.go index fc81480902..f9d7ef4d5f 100644 --- a/tests/bor/bor_api_test.go +++ b/tests/bor/bor_api_test.go @@ -1,168 +1,208 @@ -//go:build integration - package bor import ( "context" + "fmt" "math/big" "testing" - "github.com/golang/mock/gomock" - "github.com/stretchr/testify/require" - "github.com/ethereum/go-ethereum/common" - "github.com/ethereum/go-ethereum/consensus/bor" - "github.com/ethereum/go-ethereum/consensus/bor/clerk" - "github.com/ethereum/go-ethereum/consensus/bor/heimdall/checkpoint" - "github.com/ethereum/go-ethereum/consensus/bor/valset" + "github.com/ethereum/go-ethereum/consensus/ethash" + "github.com/ethereum/go-ethereum/core" "github.com/ethereum/go-ethereum/core/rawdb" "github.com/ethereum/go-ethereum/core/types" - "github.com/ethereum/go-ethereum/internal/ethapi" + "github.com/ethereum/go-ethereum/crypto" + "github.com/ethereum/go-ethereum/eth" + "github.com/ethereum/go-ethereum/eth/ethconfig" + "github.com/ethereum/go-ethereum/node" + "github.com/ethereum/go-ethereum/params" "github.com/ethereum/go-ethereum/rpc" - "github.com/ethereum/go-ethereum/tests/bor/mocks" + "github.com/stretchr/testify/assert" ) -func TestGetTransactionReceiptsByBlock(t *testing.T) { - init := buildEthereumInstance(t, rawdb.NewMemoryDatabase()) - chain := init.ethereum.BlockChain() - engine := init.ethereum.Engine() +func duplicateInArray(arr []common.Hash) bool { + visited := make(map[common.Hash]bool, 0) + for i := 0; i < len(arr); i++ { + if visited[arr[i]] == true { + return true + } else { + visited[arr[i]] = true + } + } + return false +} - ctrl := gomock.NewController(t) - defer ctrl.Finish() +func areDifferentHashes(receipts []map[string]interface{}) bool { + addresses := []common.Hash{} + for i := 0; i < len(receipts); i++ { - _bor := engine.(*bor.Bor) - defer _bor.Close() - - // Mock /bor/span/1 - res, _ := loadSpanFromFile(t) - - h := mocks.NewMockIHeimdallClient(ctrl) - - h.EXPECT().Span(gomock.Any(), uint64(1)).Return(&res.Result, nil).MinTimes(1) - h.EXPECT().Close().MinTimes(1) - h.EXPECT().FetchCheckpoint(gomock.Any(), int64(-1)).Return(&checkpoint.Checkpoint{ - Proposer: res.Result.SelectedProducers[0].Address, - StartBlock: big.NewInt(0), - EndBlock: big.NewInt(int64(spanSize)), - }, nil).AnyTimes() - - // Mock State Sync events - // at # sprintSize, events are fetched for [fromID, (block-sprint).Time) - fromID := uint64(1) - to := int64(chain.GetHeaderByNumber(0).Time) - sample := getSampleEventRecord(t) - - // First query will be from [id=1, (block-sprint).Time] - eventRecords := []*clerk.EventRecordWithTime{ - buildStateEvent(sample, 1, 1), - buildStateEvent(sample, 2, 2), - buildStateEvent(sample, 3, 3), + addresses = append(addresses, receipts[i]["transactionHash"].(common.Hash)) + if duplicateInArray(addresses) { + return false + } } - h.EXPECT().StateSyncEvents(gomock.Any(), fromID, to).Return(eventRecords, nil).MinTimes(1) - _bor.SetHeimdallClient(h) + return true +} - // Insert blocks for 0th sprint - db := init.ethereum.ChainDb() - block := init.genesis.ToBlock(db) - - signer := types.LatestSigner(init.genesis.Config) - toAddress := common.HexToAddress("0x000000000000000000000000000000000000aaaa") - - currentValidators := []*valset.Validator{valset.NewValidator(addr, 10)} - txHashes := map[int]common.Hash{} // blockNumber -> txHash +func TestGetTransactionReceiptsByBlock(t *testing.T) { + t.Parallel() var ( - err error - nonce uint64 - tx *types.Transaction - txs []*types.Transaction + key1, _ = crypto.HexToECDSA("b71c71a67e1177ad4e901695e1b4b9ee17ae16c6668d313eac2f96dbcda3f291") + addr = crypto.PubkeyToAddress(key1.PublicKey) + stack, _ = node.New(&node.DefaultConfig) + backend, _ = eth.New(stack, ðconfig.Defaults) + db = backend.ChainDb() + hash1 = common.BytesToHash([]byte("topic1")) + hash2 = common.BytesToHash([]byte("topic2")) + hash3 = common.BytesToHash([]byte("topic3")) + hash4 = common.BytesToHash([]byte("topic4")) + hash5 = common.BytesToHash([]byte("topic5")) ) - for i := uint64(1); i <= sprintSize; i++ { - if IsSpanEnd(i) { - currentValidators = []*valset.Validator{valset.NewValidator(addr, 10)} - } + genesis := core.GenesisBlockForTesting(db, addr, big.NewInt(1000000)) + sprint := params.TestChainConfig.Bor.Sprint + + chain, receipts := core.GenerateChain(params.TestChainConfig, genesis, ethash.NewFaker(), db, 6, func(i int, gen *core.BlockGen) { + switch i { + + case 1: // 1 normal transaction on block 2 + receipt := types.NewReceipt(nil, false, 0) + receipt.Logs = []*types.Log{ + { + Address: addr, + Topics: []common.Hash{hash1}, + }, + } + gen.AddUncheckedReceipt(receipt) + gen.AddUncheckedTx(types.NewTransaction(24, common.HexToAddress("0x24"), big.NewInt(24), 24, gen.BaseFee(), nil)) + + case 2: // 2 normal transactions on block 3 + receipt := types.NewReceipt(nil, false, 0) + receipt.Logs = []*types.Log{ + { + Address: addr, + Topics: []common.Hash{hash2}, + }, + } + gen.AddUncheckedReceipt(receipt) + gen.AddUncheckedTx(types.NewTransaction(992, common.HexToAddress("0x992"), big.NewInt(992), 992, gen.BaseFee(), nil)) + + receipt2 := types.NewReceipt(nil, false, 0) + receipt2.Logs = []*types.Log{ + { + Address: addr, + Topics: []common.Hash{hash3}, + }, + } + gen.AddUncheckedReceipt(receipt2) + gen.AddUncheckedTx(types.NewTransaction(993, common.HexToAddress("0x993"), big.NewInt(993), 993, gen.BaseFee(), nil)) + + case 3: // 1 normal transaction, 1 state-sync transaction on block 4 + receipt := types.NewReceipt(nil, false, 0) + receipt.Logs = []*types.Log{ + { + Address: addr, + Topics: []common.Hash{hash4}, + }, + } + gen.AddUncheckedReceipt(receipt) + gen.AddUncheckedTx(types.NewTransaction(1000, common.HexToAddress("0x0"), big.NewInt(1000), 1000, gen.BaseFee(), nil)) + + // state-sync transaction + receipt2 := types.NewReceipt(nil, false, 0) + receipt2.Logs = []*types.Log{ + { + Address: addr, + Topics: []common.Hash{hash5}, + }, + } + gen.AddUncheckedReceipt(receipt2) + // not adding unchecked tx as it will be added as a state-sync tx later + + } + }) + + for i, block := range chain { + // write the block to database + rawdb.WriteBlock(db, block) + rawdb.WriteCanonicalHash(db, block.Hash(), block.NumberU64()) + rawdb.WriteHeadBlockHash(db, block.Hash()) + + blockBatch := db.NewBatch() + + if i%int(sprint-1) != 0 { + + // if it is not sprint start write all the transactions as normal transactions. + rawdb.WriteReceipts(db, block.Hash(), block.NumberU64(), receipts[i]) + + } else { + + // check for blocks with receipts. Since in state-sync block, we have 1 normal txn and 1 state-sync txn. + if len(receipts[i]) > 0 { + // We write receipts for the normal transaction. + rawdb.WriteReceipts(db, block.Hash(), block.NumberU64(), receipts[i][:1]) + + // write the state-sync receipts to database => receipts[i][1:] => receipts[i][1] + // State sync logs don't have tx index, tx hash and other necessary fields, DeriveFieldsForBorLogs will fill those fields for websocket subscriptions + // DeriveFieldsForBorLogs argurments: + // 1. State-sync logs + // 2. Block Hash + // 3. Block Number + // 4. Transactions in the block(except state-sync) i.e. 1 in our case + // 5. AllLogs -(minus) StateSyncLogs ; since we only have state-sync tx, it will be 1 + types.DeriveFieldsForBorLogs(receipts[i][1].Logs, block.Hash(), block.NumberU64(), uint(1), uint(1)) + + rawdb.WriteBorReceipt(blockBatch, block.Hash(), block.NumberU64(), &types.ReceiptForStorage{ + Status: types.ReceiptStatusSuccessful, // make receipt status successful + Logs: receipts[i][1].Logs, + }) + + rawdb.WriteBorTxLookupEntry(blockBatch, block.Hash(), block.NumberU64()) - if i%3 == 0 { - txdata := &types.LegacyTx{ - Nonce: nonce, - To: &toAddress, - Gas: 30000, - GasPrice: newGwei(5), } - nonce++ - - tx = types.NewTx(txdata) - tx, err = types.SignTx(tx, signer, key) - require.Nil(t, err, "an incorrect transaction or signer") - - txs = []*types.Transaction{tx} - } else { - txs = nil } - block = buildNextBlock(t, _bor, chain, block, nil, init.genesis.Config.Bor, txs, currentValidators) - insertNewBlock(t, chain, block) + if err := blockBatch.Write(); err != nil { - if len(txs) != 0 { - txHashes[int(block.Number().Uint64())] = tx.Hash() + fmt.Println("Failed to write block into disk", "err", err) } + + } + publicBlockchainAPI := backend.PublicBlockChainAPI() + + // check 1 : zero transactions + receiptsOut, err := publicBlockchainAPI.GetTransactionReceiptsByBlock(context.Background(), rpc.BlockNumberOrHashWithNumber(1)) + if err != nil { + fmt.Println(err) + } + assert.Equal(t, 0, len(receiptsOut)) + + // check 2 : one transactions ( normal ) + receiptsOut, err = publicBlockchainAPI.GetTransactionReceiptsByBlock(context.Background(), rpc.BlockNumberOrHashWithNumber(2)) + if err != nil { + fmt.Println(err) + } + assert.Equal(t, 1, len(receiptsOut)) + assert.True(t, areDifferentHashes(receiptsOut)) + + // check 3 : two transactions ( both normal ) + receiptsOut, err = publicBlockchainAPI.GetTransactionReceiptsByBlock(context.Background(), rpc.BlockNumberOrHashWithNumber(3)) + if err != nil { + fmt.Println(err) + } + assert.Equal(t, 2, len(receiptsOut)) + assert.True(t, areDifferentHashes(receiptsOut)) + + // check 4 : two transactions ( one normal + one state-sync) + receiptsOut, err = publicBlockchainAPI.GetTransactionReceiptsByBlock(context.Background(), rpc.BlockNumberOrHashWithNumber(4)) + if err != nil { + fmt.Println(err) } - // state 6 was not written - // - fromID = uint64(4) - to = int64(chain.GetHeaderByNumber(sprintSize).Time) + assert.Equal(t, 2, len(receiptsOut)) + assert.True(t, areDifferentHashes(receiptsOut)) - eventRecords = []*clerk.EventRecordWithTime{ - buildStateEvent(sample, 4, 4), - buildStateEvent(sample, 5, 5), - } - h.EXPECT().StateSyncEvents(gomock.Any(), fromID, to).Return(eventRecords, nil).MinTimes(1) - - for i := sprintSize + 1; i <= spanSize; i++ { - block = buildNextBlock(t, _bor, chain, block, nil, init.genesis.Config.Bor, nil, currentValidators) - insertNewBlock(t, chain, block) - } - - ethAPI := ethapi.NewPublicBlockChainAPI(init.ethereum.APIBackend) - txPoolAPI := ethapi.NewPublicTransactionPoolAPI(init.ethereum.APIBackend, nil) - - for n := 0; n < int(spanSize)+1; n++ { - rpcNumber := rpc.BlockNumberOrHashWithNumber(rpc.BlockNumber(n)) - - txs, err := ethAPI.GetTransactionReceiptsByBlock(context.Background(), rpcNumber) - require.Nil(t, err) - - tx := txPoolAPI.GetTransactionByBlockNumberAndIndex(context.Background(), rpc.BlockNumber(n), 0) - - blockMap, err := ethAPI.GetBlockByNumber(context.Background(), rpc.BlockNumber(n), true) - require.Nil(t, err) - - expectedTxHash, ok := txHashes[n] - // FIXME: add `IsSprintStart(uint64(n)) || IsSpanStart(uint64(n))` after adding a full state receiver contract - if ok { - require.Len(t, txs, 1) - - require.NotNil(t, tx, "not nil receipt expected") - - require.Equal(t, expectedTxHash, tx.Hash, "got different from expected receipt") - - blockTxs, ok := blockMap["transactions"].([]interface{}) - require.Len(t, blockTxs, 1) - - blockTx, ok := blockTxs[0].(*ethapi.RPCTransaction) - require.True(t, ok) - require.Equal(t, expectedTxHash, blockTx.Hash) - } else { - require.Len(t, txs, 0) - - require.Nil(t, tx, "nil receipt expected") - - blockTxs, _ := blockMap["transactions"].([]interface{}) - require.Len(t, blockTxs, 0) - } - } } diff --git a/tests/bor/bor_filter_test.go b/tests/bor/bor_filter_test.go index c37b4ab001..25cfa078a1 100644 --- a/tests/bor/bor_filter_test.go +++ b/tests/bor/bor_filter_test.go @@ -97,7 +97,7 @@ func TestBorFilters(t *testing.T) { // since all the transactions are state-sync, we will not include them as normal receipts rawdb.WriteReceipts(db, block.Hash(), block.NumberU64(), []*types.Receipt{}) - // check for blocks with receipts. Since the only receipt is state-sync, we can chack the length of receipts + // check for blocks with receipts. Since the only receipt is state-sync, we can check the length of receipts if len(receipts[i]) > 0 { // write the state-sync receipts to database // State sync logs don't have tx index, tx hash and other necessary fields, DeriveFieldsForBorLogs will fill those fields for websocket subscriptions From 423734c0d42c1f52887e92e3c92db3465aa20009 Mon Sep 17 00:00:00 2001 From: Shivam Sharma Date: Tue, 2 Aug 2022 01:19:42 +0530 Subject: [PATCH 3/4] fix : deferred close() + lint --- internal/ethapi/api.go | 2 ++ tests/bor/bor_api_test.go | 19 ++++++++++++------- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/internal/ethapi/api.go b/internal/ethapi/api.go index f5d6016efe..91d88ec815 100644 --- a/internal/ethapi/api.go +++ b/internal/ethapi/api.go @@ -682,9 +682,11 @@ func (s *PublicBlockChainAPI) GetTransactionReceiptsByBlock(ctx context.Context, if receipt.Logs == nil { fields["logs"] = [][]*types.Log{} } + if borReceipt != nil && idx == len(receipts)-1 { fields["transactionHash"] = txHash } + // If the ContractAddress is 20 0x0 bytes, assume it is not a contract creation if receipt.ContractAddress != (common.Address{}) { fields["contractAddress"] = receipt.ContractAddress diff --git a/tests/bor/bor_api_test.go b/tests/bor/bor_api_test.go index f9d7ef4d5f..c105787e7e 100644 --- a/tests/bor/bor_api_test.go +++ b/tests/bor/bor_api_test.go @@ -17,6 +17,7 @@ import ( "github.com/ethereum/go-ethereum/node" "github.com/ethereum/go-ethereum/params" "github.com/ethereum/go-ethereum/rpc" + "github.com/stretchr/testify/assert" ) @@ -29,13 +30,13 @@ func duplicateInArray(arr []common.Hash) bool { visited[arr[i]] = true } } + return false } func areDifferentHashes(receipts []map[string]interface{}) bool { addresses := []common.Hash{} for i := 0; i < len(receipts); i++ { - addresses = append(addresses, receipts[i]["transactionHash"].(common.Hash)) if duplicateInArray(addresses) { return false @@ -61,6 +62,12 @@ func TestGetTransactionReceiptsByBlock(t *testing.T) { hash5 = common.BytesToHash([]byte("topic5")) ) + defer func() { + if err := stack.Close(); err != nil { + t.Error(err) + } + }() + genesis := core.GenesisBlockForTesting(db, addr, big.NewInt(1000000)) sprint := params.TestChainConfig.Bor.Sprint @@ -133,12 +140,9 @@ func TestGetTransactionReceiptsByBlock(t *testing.T) { blockBatch := db.NewBatch() if i%int(sprint-1) != 0 { - // if it is not sprint start write all the transactions as normal transactions. rawdb.WriteReceipts(db, block.Hash(), block.NumberU64(), receipts[i]) - } else { - // check for blocks with receipts. Since in state-sync block, we have 1 normal txn and 1 state-sync txn. if len(receipts[i]) > 0 { // We write receipts for the normal transaction. @@ -166,11 +170,10 @@ func TestGetTransactionReceiptsByBlock(t *testing.T) { } if err := blockBatch.Write(); err != nil { - fmt.Println("Failed to write block into disk", "err", err) } - } + publicBlockchainAPI := backend.PublicBlockChainAPI() // check 1 : zero transactions @@ -178,6 +181,7 @@ func TestGetTransactionReceiptsByBlock(t *testing.T) { if err != nil { fmt.Println(err) } + assert.Equal(t, 0, len(receiptsOut)) // check 2 : one transactions ( normal ) @@ -185,6 +189,7 @@ func TestGetTransactionReceiptsByBlock(t *testing.T) { if err != nil { fmt.Println(err) } + assert.Equal(t, 1, len(receiptsOut)) assert.True(t, areDifferentHashes(receiptsOut)) @@ -193,6 +198,7 @@ func TestGetTransactionReceiptsByBlock(t *testing.T) { if err != nil { fmt.Println(err) } + assert.Equal(t, 2, len(receiptsOut)) assert.True(t, areDifferentHashes(receiptsOut)) @@ -204,5 +210,4 @@ func TestGetTransactionReceiptsByBlock(t *testing.T) { assert.Equal(t, 2, len(receiptsOut)) assert.True(t, areDifferentHashes(receiptsOut)) - } From a69109ce5e4d7649cc46858636aceded1fc82d7c Mon Sep 17 00:00:00 2001 From: Shivam Sharma Date: Tue, 2 Aug 2022 10:34:25 +0530 Subject: [PATCH 4/4] fix : removed debug logs --- tests/bor/bor_api_test.go | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/tests/bor/bor_api_test.go b/tests/bor/bor_api_test.go index c105787e7e..b0e627bd54 100644 --- a/tests/bor/bor_api_test.go +++ b/tests/bor/bor_api_test.go @@ -2,7 +2,6 @@ package bor import ( "context" - "fmt" "math/big" "testing" @@ -170,7 +169,7 @@ func TestGetTransactionReceiptsByBlock(t *testing.T) { } if err := blockBatch.Write(); err != nil { - fmt.Println("Failed to write block into disk", "err", err) + t.Error("Failed to write block into disk", "err", err) } } @@ -179,7 +178,7 @@ func TestGetTransactionReceiptsByBlock(t *testing.T) { // check 1 : zero transactions receiptsOut, err := publicBlockchainAPI.GetTransactionReceiptsByBlock(context.Background(), rpc.BlockNumberOrHashWithNumber(1)) if err != nil { - fmt.Println(err) + t.Error(err) } assert.Equal(t, 0, len(receiptsOut)) @@ -187,7 +186,7 @@ func TestGetTransactionReceiptsByBlock(t *testing.T) { // check 2 : one transactions ( normal ) receiptsOut, err = publicBlockchainAPI.GetTransactionReceiptsByBlock(context.Background(), rpc.BlockNumberOrHashWithNumber(2)) if err != nil { - fmt.Println(err) + t.Error(err) } assert.Equal(t, 1, len(receiptsOut)) @@ -196,7 +195,7 @@ func TestGetTransactionReceiptsByBlock(t *testing.T) { // check 3 : two transactions ( both normal ) receiptsOut, err = publicBlockchainAPI.GetTransactionReceiptsByBlock(context.Background(), rpc.BlockNumberOrHashWithNumber(3)) if err != nil { - fmt.Println(err) + t.Error(err) } assert.Equal(t, 2, len(receiptsOut)) @@ -205,7 +204,7 @@ func TestGetTransactionReceiptsByBlock(t *testing.T) { // check 4 : two transactions ( one normal + one state-sync) receiptsOut, err = publicBlockchainAPI.GetTransactionReceiptsByBlock(context.Background(), rpc.BlockNumberOrHashWithNumber(4)) if err != nil { - fmt.Println(err) + t.Error(err) } assert.Equal(t, 2, len(receiptsOut))