review fixes

statediff/service.go

@@ -138,6 +138,7 @@ func (sds *Service) Loop(chainEventCh chan core.ChainEvent) {
 			payload, err := sds.processStateDiff(currentBlock, parentBlock)
 			if err != nil {
 				log.Error(fmt.Sprintf("Error building statediff for block %d; error: ", currentBlock.Number()) + err.Error())
+				continue
 			}
 			sds.send(*payload)
 		case err := <-errCh:

statediff/service_test.go

@@ -93,7 +93,7 @@ func testErrorInChainEventLoop(t *testing.T) {
 	blockMapping := make(map[common.Hash]*types.Block)
 	blockMapping[parentBlock1.Hash()] = parentBlock1
 	blockMapping[parentBlock2.Hash()] = parentBlock2
-	blockChain.SetParentBlocksToReturn(blockMapping)
+	blockChain.SetBlocksForHashes(blockMapping)
 	blockChain.SetChainEvents([]core.ChainEvent{event1, event2, event3})
 	blockChain.SetReceiptsForHash(testBlock1.Hash(), testReceipts1)
 	blockChain.SetReceiptsForHash(testBlock2.Hash(), testReceipts2)
@@ -149,9 +149,9 @@ func testErrorInChainEventLoop(t *testing.T) {
 	}
 	//look up the parent block from its hash
 	expectedHashes := []common.Hash{testBlock1.ParentHash(), testBlock2.ParentHash()}
-	if !reflect.DeepEqual(blockChain.ParentHashesLookedUp, expectedHashes) {
+	if !reflect.DeepEqual(blockChain.HashesLookedUp, expectedHashes) {
 		t.Error("Test failure:", t.Name())
-		t.Logf("Actual parent hash does not equal expected.\nactual:%+v\nexpected: %+v", blockChain.ParentHashesLookedUp, expectedHashes)
+		t.Logf("Actual parent hash does not equal expected.\nactual:%+v\nexpected: %+v", blockChain.HashesLookedUp, expectedHashes)
 	}
 }
 
@@ -170,7 +170,7 @@ func testErrorInBlockLoop(t *testing.T) {
 	service.Subscribe(rpc.NewID(), payloadChan, quitChan)
 	blockMapping := make(map[common.Hash]*types.Block)
 	blockMapping[parentBlock1.Hash()] = parentBlock1
-	blockChain.SetParentBlocksToReturn(blockMapping)
+	blockChain.SetBlocksForHashes(blockMapping)
 	blockChain.SetChainEvents([]core.ChainEvent{event1, event2})
 	// Need to have listeners on the channels or the subscription will be closed and the processing halted
 	go func() {
@@ -230,7 +230,7 @@ func testErrorInStateDiffAt(t *testing.T) {
 	blockChain := mocks.BlockChain{}
 	blockMapping := make(map[common.Hash]*types.Block)
 	blockMapping[parentBlock1.Hash()] = parentBlock1
-	blockChain.SetParentBlocksToReturn(blockMapping)
+	blockChain.SetBlocksForHashes(blockMapping)
 	blockChain.SetBlockForNumber(testBlock1, testBlock1.NumberU64())
 	blockChain.SetReceiptsForHash(testBlock1.Hash(), testReceipts1)
 	service := statediff.Service{

statediff/testhelpers/mocks/api.go

@@ -36,6 +36,7 @@ import (
 type MockStateDiffService struct {
 	sync.Mutex
 	Builder         statediff.Builder
+	BlockChain      *BlockChain
 	ReturnProtocol  []p2p.Protocol
 	ReturnAPIs      []rpc.API
 	BlockChan       chan *types.Block
@@ -77,10 +78,12 @@ func (sds *MockStateDiffService) Loop(chan core.ChainEvent) {
 					"current block number", currentBlock.Number())
 				continue
 			}
-			if err := sds.process(currentBlock, parentBlock); err != nil {
+			payload, err := sds.processStateDiff(currentBlock, parentBlock)
-				println(err.Error())
+			if err != nil {
 				log.Error("Error building statediff", "block number", currentBlock.Number(), "error", err)
+				continue
 			}
+			sds.send(*payload)
 		case <-sds.QuitChan:
 			log.Debug("Quitting the statediff block channel")
 			sds.close()
@@ -89,16 +92,16 @@ func (sds *MockStateDiffService) Loop(chan core.ChainEvent) {
 	}
 }
 
-// process method builds the state diff payload from the current and parent block and streams it to listening subscriptions
+// processStateDiff method builds the state diff payload from the current and parent block and streams it to listening subscriptions
-func (sds *MockStateDiffService) process(currentBlock, parentBlock *types.Block) error {
+func (sds *MockStateDiffService) processStateDiff(currentBlock, parentBlock *types.Block) (*statediff.Payload, error) {
 	stateDiff, err := sds.Builder.BuildStateDiff(parentBlock.Root(), currentBlock.Root(), currentBlock.Number(), currentBlock.Hash())
 	if err != nil {
-		return err
+		return nil, err
 	}
 
 	stateDiffRlp, err := rlp.EncodeToBytes(stateDiff)
 	if err != nil {
-		return err
+		return nil, err
 	}
 	payload := statediff.Payload{
 		StateDiffRlp: stateDiffRlp,
@@ -106,14 +109,11 @@ func (sds *MockStateDiffService) process(currentBlock, parentBlock *types.Block)
 	if sds.streamBlock {
 		rlpBuff := new(bytes.Buffer)
 		if err = currentBlock.EncodeRLP(rlpBuff); err != nil {
-			return err
+			return nil, err
 		}
 		payload.BlockRlp = rlpBuff.Bytes()
 	}
-
+	return &payload, nil
-	// If we have any websocket subscription listening in, send the data to them
-	sds.send(payload)
-	return nil
 }
 
 // Subscribe mock method
@@ -188,5 +188,8 @@ func (sds *MockStateDiffService) Stop() error {
 
 // StateDiffAt mock method
 func (sds *MockStateDiffService) StateDiffAt(blockNumber uint64) (*statediff.Payload, error) {
-	panic("implement me")
+	currentBlock := sds.BlockChain.GetBlockByNumber(blockNumber)
+	parentBlock := sds.BlockChain.GetBlockByHash(currentBlock.ParentHash())
+	log.Info(fmt.Sprintf("sending state diff at %d", blockNumber))
+	return sds.processStateDiff(currentBlock, parentBlock)
 }

statediff/testhelpers/mocks/api_test.go

@@ -55,6 +55,10 @@ var bankAccount1, _ = rlp.EncodeToBytes(state.Account{
 })
 
 func TestAPI(t *testing.T) {
+	testSubscriptionAPI(t)
+	testHTTPAPI(t)
+}
+func testSubscriptionAPI(t *testing.T) {
 	_, blockMap, chain := testhelpers.MakeChain(3, testhelpers.Genesis)
 	defer chain.Stop()
 	block0Hash := common.HexToHash("0xd1721cfd0b29c36fd7a68f25c128e86413fb666a6e1d68e89b875bd299262661")
@@ -140,3 +144,75 @@ func TestAPI(t *testing.T) {
 		t.Errorf("channel quit before delivering payload")
 	}
 }
+
+func testHTTPAPI(t *testing.T) {
+	_, blockMap, chain := testhelpers.MakeChain(3, testhelpers.Genesis)
+	defer chain.Stop()
+	block0Hash := common.HexToHash("0xd1721cfd0b29c36fd7a68f25c128e86413fb666a6e1d68e89b875bd299262661")
+	block1Hash := common.HexToHash("0xbbe88de60ba33a3f18c0caa37d827bfb70252e19e40a07cd34041696c35ecb1a")
+	block0 = blockMap[block0Hash]
+	block1 = blockMap[block1Hash]
+	config := statediff.Config{
+		PathsAndProofs:    true,
+		IntermediateNodes: false,
+	}
+	mockBlockChain := &BlockChain{}
+	mockBlockChain.SetBlocksForHashes(blockMap)
+	mockBlockChain.SetBlockForNumber(block1, block1.Number().Uint64())
+	mockService := MockStateDiffService{
+		Mutex:       sync.Mutex{},
+		Builder:     statediff.NewBuilder(testhelpers.Testdb, chain, config),
+		BlockChain:  mockBlockChain,
+		streamBlock: true,
+	}
+	payload, err := mockService.StateDiffAt(block1.Number().Uint64())
+	expectedBlockRlp, _ := rlp.EncodeToBytes(block1)
+	if !bytes.Equal(payload.BlockRlp, expectedBlockRlp) {
+		t.Errorf("payload does not have expected block\r\actual block rlp: %v\r\nexpected block rlp: %v", payload.BlockRlp, expectedBlockRlp)
+	}
+	expectedStateDiff := statediff.StateDiff{
+		BlockNumber: block1.Number(),
+		BlockHash:   block1.Hash(),
+		CreatedAccounts: []statediff.AccountDiff{
+			{
+				Leaf:  true,
+				Key:   burnLeafKey.Bytes(),
+				Value: burnAccount1,
+				Proof: [][]byte{{248, 113, 160, 87, 118, 82, 182, 37, 183, 123, 219, 91, 247, 123, 196, 63, 49, 37, 202, 215, 70, 77, 103, 157, 21, 117, 86, 82, 119, 211, 97, 27, 128, 83, 231, 128, 128, 128, 128, 160, 254, 136, 159, 16, 229, 219, 143, 44, 43, 243, 85, 146, 129, 82, 161, 127, 110, 59, 185, 154, 146, 65, 172, 109, 132, 199, 126, 98, 100, 80, 156, 121, 128, 128, 128, 128, 128, 128, 128, 128, 160, 17, 219, 12, 218, 52, 168, 150, 218, 190, 182, 131, 155, 176, 106, 56, 244, 149, 20, 207, 164, 134, 67, 89, 132, 235, 1, 59, 125, 249, 238, 133, 197, 128, 128},
+					{248, 113, 160, 51, 128, 199, 183, 174, 129, 165, 142, 185, 141, 156, 120, 222, 74, 31, 215, 253, 149, 53, 252, 149, 62, 210, 190, 96, 45, 170, 164, 23, 103, 49, 42, 184, 78, 248, 76, 128, 136, 27, 193, 109, 103, 78, 200, 0, 0, 160, 86, 232, 31, 23, 27, 204, 85, 166, 255, 131, 69, 230, 146, 192, 248, 110, 91, 72, 224, 27, 153, 108, 173, 192, 1, 98, 47, 181, 227, 99, 180, 33, 160, 197, 210, 70, 1, 134, 247, 35, 60, 146, 126, 125, 178, 220, 199, 3, 192, 229, 0, 182, 83, 202, 130, 39, 59, 123, 250, 216, 4, 93, 133, 164, 112}},
+				Path:    []byte{5, 3, 8, 0, 12, 7, 11, 7, 10, 14, 8, 1, 10, 5, 8, 14, 11, 9, 8, 13, 9, 12, 7, 8, 13, 14, 4, 10, 1, 15, 13, 7, 15, 13, 9, 5, 3, 5, 15, 12, 9, 5, 3, 14, 13, 2, 11, 14, 6, 0, 2, 13, 10, 10, 10, 4, 1, 7, 6, 7, 3, 1, 2, 10, 16},
+				Storage: []statediff.StorageDiff{},
+			},
+			{
+				Leaf:  true,
+				Key:   testhelpers.Account1LeafKey.Bytes(),
+				Value: account1,
+				Proof: [][]byte{{248, 113, 160, 87, 118, 82, 182, 37, 183, 123, 219, 91, 247, 123, 196, 63, 49, 37, 202, 215, 70, 77, 103, 157, 21, 117, 86, 82, 119, 211, 97, 27, 128, 83, 231, 128, 128, 128, 128, 160, 254, 136, 159, 16, 229, 219, 143, 44, 43, 243, 85, 146, 129, 82, 161, 127, 110, 59, 185, 154, 146, 65, 172, 109, 132, 199, 126, 98, 100, 80, 156, 121, 128, 128, 128, 128, 128, 128, 128, 128, 160, 17, 219, 12, 218, 52, 168, 150, 218, 190, 182, 131, 155, 176, 106, 56, 244, 149, 20, 207, 164, 134, 67, 89, 132, 235, 1, 59, 125, 249, 238, 133, 197, 128, 128},
+					{248, 107, 160, 57, 38, 219, 105, 170, 206, 213, 24, 233, 185, 240, 244, 52, 164, 115, 231, 23, 65, 9, 201, 67, 84, 139, 184, 242, 59, 228, 28, 167, 109, 154, 210, 184, 72, 248, 70, 128, 130, 39, 16, 160, 86, 232, 31, 23, 27, 204, 85, 166, 255, 131, 69, 230, 146, 192, 248, 110, 91, 72, 224, 27, 153, 108, 173, 192, 1, 98, 47, 181, 227, 99, 180, 33, 160, 197, 210, 70, 1, 134, 247, 35, 60, 146, 126, 125, 178, 220, 199, 3, 192, 229, 0, 182, 83, 202, 130, 39, 59, 123, 250, 216, 4, 93, 133, 164, 112}},
+				Path:    []byte{14, 9, 2, 6, 13, 11, 6, 9, 10, 10, 12, 14, 13, 5, 1, 8, 14, 9, 11, 9, 15, 0, 15, 4, 3, 4, 10, 4, 7, 3, 14, 7, 1, 7, 4, 1, 0, 9, 12, 9, 4, 3, 5, 4, 8, 11, 11, 8, 15, 2, 3, 11, 14, 4, 1, 12, 10, 7, 6, 13, 9, 10, 13, 2, 16},
+				Storage: []statediff.StorageDiff{},
+			},
+		},
+		DeletedAccounts: emptyAccountDiffEventualMap,
+		UpdatedAccounts: []statediff.AccountDiff{
+			{
+				Leaf:  true,
+				Key:   testhelpers.BankLeafKey.Bytes(),
+				Value: bankAccount1,
+				Proof: [][]byte{{248, 113, 160, 87, 118, 82, 182, 37, 183, 123, 219, 91, 247, 123, 196, 63, 49, 37, 202, 215, 70, 77, 103, 157, 21, 117, 86, 82, 119, 211, 97, 27, 128, 83, 231, 128, 128, 128, 128, 160, 254, 136, 159, 16, 229, 219, 143, 44, 43, 243, 85, 146, 129, 82, 161, 127, 110, 59, 185, 154, 146, 65, 172, 109, 132, 199, 126, 98, 100, 80, 156, 121, 128, 128, 128, 128, 128, 128, 128, 128, 160, 17, 219, 12, 218, 52, 168, 150, 218, 190, 182, 131, 155, 176, 106, 56, 244, 149, 20, 207, 164, 134, 67, 89, 132, 235, 1, 59, 125, 249, 238, 133, 197, 128, 128},
+					{248, 109, 160, 48, 191, 73, 244, 64, 161, 205, 5, 39, 228, 208, 110, 39, 101, 101, 76, 15, 86, 69, 34, 87, 81, 109, 121, 58, 155, 141, 96, 77, 207, 223, 42, 184, 74, 248, 72, 1, 132, 5, 245, 185, 240, 160, 86, 232, 31, 23, 27, 204, 85, 166, 255, 131, 69, 230, 146, 192, 248, 110, 91, 72, 224, 27, 153, 108, 173, 192, 1, 98, 47, 181, 227, 99, 180, 33, 160, 197, 210, 70, 1, 134, 247, 35, 60, 146, 126, 125, 178, 220, 199, 3, 192, 229, 0, 182, 83, 202, 130, 39, 59, 123, 250, 216, 4, 93, 133, 164, 112}},
+				Path:    []byte{0, 0, 11, 15, 4, 9, 15, 4, 4, 0, 10, 1, 12, 13, 0, 5, 2, 7, 14, 4, 13, 0, 6, 14, 2, 7, 6, 5, 6, 5, 4, 12, 0, 15, 5, 6, 4, 5, 2, 2, 5, 7, 5, 1, 6, 13, 7, 9, 3, 10, 9, 11, 8, 13, 6, 0, 4, 13, 12, 15, 13, 15, 2, 10, 16},
+				Storage: []statediff.StorageDiff{},
+			},
+		},
+	}
+	expectedStateDiffBytes, err := rlp.EncodeToBytes(expectedStateDiff)
+	if err != nil {
+		t.Error(err)
+	}
+	sort.Slice(payload.StateDiffRlp, func(i, j int) bool { return payload.StateDiffRlp[i] < payload.StateDiffRlp[j] })
+	sort.Slice(expectedStateDiffBytes, func(i, j int) bool { return expectedStateDiffBytes[i] < expectedStateDiffBytes[j] })
+	if !bytes.Equal(payload.StateDiffRlp, expectedStateDiffBytes) {
+		t.Errorf("payload does not have expected state diff\r\actual state diff rlp: %v\r\nexpected state diff rlp: %v", payload.StateDiffRlp, expectedStateDiffBytes)
+	}
+}

statediff/testhelpers/mocks/blockchain.go

@@ -29,9 +29,9 @@ import (
 
 // BlockChain is a mock blockchain for testing
 type BlockChain struct {
-	ParentHashesLookedUp         []common.Hash
+	HashesLookedUp         []common.Hash
-	parentBlocksToReturn         map[common.Hash]*types.Block
+	blocksToReturnByHash   map[common.Hash]*types.Block
-	parentBlocksToReturnByNumber map[uint64]*types.Block
+	blocksToReturnByNumber map[uint64]*types.Block
 	callCount              int
 	ChainEvents            []core.ChainEvent
 	Receipts               map[common.Hash]types.Receipts
@@ -40,24 +40,24 @@ type BlockChain struct {
 // AddToStateDiffProcessedCollection mock method
 func (blockChain *BlockChain) AddToStateDiffProcessedCollection(hash common.Hash) {}
 
-// SetParentBlocksToReturn mock method
+// SetBlocksForHashes mock method
-func (blockChain *BlockChain) SetParentBlocksToReturn(blocks map[common.Hash]*types.Block) {
+func (blockChain *BlockChain) SetBlocksForHashes(blocks map[common.Hash]*types.Block) {
-	if blockChain.parentBlocksToReturn == nil {
+	if blockChain.blocksToReturnByHash == nil {
-		blockChain.parentBlocksToReturn = make(map[common.Hash]*types.Block)
+		blockChain.blocksToReturnByHash = make(map[common.Hash]*types.Block)
 	}
-	blockChain.parentBlocksToReturn = blocks
+	blockChain.blocksToReturnByHash = blocks
 }
 
 // GetBlockByHash mock method
 func (blockChain *BlockChain) GetBlockByHash(hash common.Hash) *types.Block {
-	blockChain.ParentHashesLookedUp = append(blockChain.ParentHashesLookedUp, hash)
+	blockChain.HashesLookedUp = append(blockChain.HashesLookedUp, hash)
 
-	var parentBlock *types.Block
+	var block *types.Block
-	if len(blockChain.parentBlocksToReturn) > 0 {
+	if len(blockChain.blocksToReturnByHash) > 0 {
-		parentBlock = blockChain.parentBlocksToReturn[hash]
+		block = blockChain.blocksToReturnByHash[hash]
 	}
 
-	return parentBlock
+	return block
 }
 
 // SetChainEvents mock method
@@ -67,7 +67,7 @@ func (blockChain *BlockChain) SetChainEvents(chainEvents []core.ChainEvent) {
 
 // SubscribeChainEvent mock method
 func (blockChain *BlockChain) SubscribeChainEvent(ch chan<- core.ChainEvent) event.Subscription {
-	subErr := errors.New("Subscription Error")
+	subErr := errors.New("subscription error")
 
 	var eventCounter int
 	subscription := event.NewSubscription(func(quit <-chan struct{}) error {
@@ -104,13 +104,13 @@ func (blockChain *BlockChain) GetReceiptsByHash(hash common.Hash) types.Receipts
 
 // SetBlockForNumber mock method
 func (blockChain *BlockChain) SetBlockForNumber(block *types.Block, number uint64) {
-	if blockChain.parentBlocksToReturnByNumber == nil {
+	if blockChain.blocksToReturnByNumber == nil {
-		blockChain.parentBlocksToReturnByNumber = make(map[uint64]*types.Block)
+		blockChain.blocksToReturnByNumber = make(map[uint64]*types.Block)
 	}
-	blockChain.parentBlocksToReturnByNumber[number] = block
+	blockChain.blocksToReturnByNumber[number] = block
 }
 
 // GetBlockByNumber mock method
 func (blockChain *BlockChain) GetBlockByNumber(number uint64) *types.Block {
-	return blockChain.parentBlocksToReturnByNumber[number]
+	return blockChain.blocksToReturnByNumber[number]
 }