Merge pull request #240 from anandrgitnirman/version

Issue 224: Ensure Claim does not fail even when group ID changes
singnet · Apr 3, 2019 · 812e2a1 · 812e2a1
2 parents 6005fdf + c1e1ce8
commit 812e2a1
Show file tree

Hide file tree

Showing 4 changed files with 49 additions and 18 deletions.
diff --git a/escrow/escrow.go b/escrow/escrow.go
@@ -45,8 +45,18 @@ func (h *lockingPaymentChannelService) PaymentChannel(key *PaymentChannelKey) (c
 		return
 	}
 
+
 	blockchainChannel, blockchainOk, err := h.blockchainReader.GetChannelStateFromBlockchain(key)
+
 	if !storageOk {
+		//Group ID check is only done for the first time , when the channel is added to storage from the block chain ,
+		//if the channel is already present in the storage the group ID check is skipped.
+	    if blockchainChannel != nil {
+			blockChainGroupID,err := h.blockchainReader.replicaGroupID()
+		    if err = h.verifyGroupId(blockChainGroupID,blockchainChannel.GroupID) ;err != nil {
+				return nil, false, err
+			}
+		}
 		return blockchainChannel, blockchainOk, err
 	}
 	if err != nil || !blockchainOk {
@@ -56,6 +66,15 @@ func (h *lockingPaymentChannelService) PaymentChannel(key *PaymentChannelKey) (c
 	return MergeStorageAndBlockchainChannelState(storageChannel, blockchainChannel), true, nil
 }
 
+//Check if the channel belongs to the same group Id
+func (h *lockingPaymentChannelService) verifyGroupId(configGroupID [32]byte ,blockChainGroupID  [32]byte ) error {
+	if blockChainGroupID != configGroupID {
+		log.WithField("configGroupId", configGroupID).Warn("Channel received belongs to another group of replicas")
+		return fmt.Errorf("Channel received belongs to another group of replicas, current group: %v, channel group: %v", configGroupID, blockChainGroupID)
+	}
+	return nil
+}
+
 func (h *lockingPaymentChannelService) ListChannels() (channels []*PaymentChannelData, err error) {
 	return h.storage.GetAll()
 }

diff --git a/escrow/escrow_test.go b/escrow/escrow_test.go
@@ -2,6 +2,7 @@ package escrow
 
 import (
 	"crypto/ecdsa"
+	"errors"
 	"fmt"
 	"math/big"
 	"testing"
@@ -280,3 +281,31 @@ func (suite *PaymentChannelServiceSuite) TestStartClaim() {
 	assert.Equal(suite.T(), suite.payment(), claim.Payment())
 	assert.Equal(suite.T(), []*Payment{suite.payment()}, claims)
 }
+
+func (suite *PaymentChannelServiceSuite) TestVerifyGroupId() {
+
+
+	service := suite.service
+	service.(*lockingPaymentChannelService).blockchainReader.replicaGroupID =
+		func() ([32]byte, error) {
+			return [32]byte{125}, nil
+		}
+	//GroupId check will be applied only first time when channel is added to storage from the blockchain.
+	//Group ID is different
+	channel, ok, err := service.PaymentChannel(&PaymentChannelKey{ID: big.NewInt(13)})
+	assert.Equal(suite.T(), errors.New("Channel received belongs to another group of replicas, current group: [125 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0], channel group: [123 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0]"), err)
+	assert.False(suite.T(), ok)
+	assert.Nil(suite.T(), channel)
+	assert.NotNil(suite.T(),err)
+	//GroupId check will be applied only first time when channel is added to storage from the blockchain.
+	//Group ID is the same ( no error should happen)
+	//also re setting the value here again to make sure the original state is retained
+	service.(*lockingPaymentChannelService).blockchainReader.replicaGroupID =
+		func() ([32]byte, error) {
+			return [32]byte{123}, nil
+		}
+	channel, ok, err = suite.service.PaymentChannel(&PaymentChannelKey{ID: big.NewInt(13)})
+	assert.True(suite.T(), ok)
+	assert.NotNil(suite.T(), channel)
+	assert.Nil(suite.T(),err)
+}
diff --git a/escrow/payment_channel_storage.go b/escrow/payment_channel_storage.go
@@ -121,16 +121,9 @@ func (reader *BlockchainChannelReader) GetChannelStateFromBlockchain(key *Paymen
 		return
 	}
 
-	configGroupID, err := reader.replicaGroupID()
-	if err != nil {
-		return nil, false, err
-	}
+
 	recipientPaymentAddress := reader.recipientPaymentAddress()
 
-	if ch.GroupId != configGroupID {
-		log.WithField("configGroupId", configGroupID).Warn("Channel received belongs to another group of replicas")
-		return nil, false, fmt.Errorf("Channel received belongs to another group of replicas, current group: %v, channel group: %v", configGroupID, ch.GroupId)
-	}
 
 	if recipientPaymentAddress != ch.Recipient {
 		log.WithField("recipientPaymentAddress", recipientPaymentAddress).

diff --git a/escrow/payment_channel_storage_test.go b/escrow/payment_channel_storage_test.go
@@ -153,17 +153,7 @@ func (suite *BlockchainChannelReaderSuite) TestGetChannelState() {
 	assert.Equal(suite.T(), suite.channel(), channel)
 }
 
-func (suite *BlockchainChannelReaderSuite) TestGetChannelStateIncorrectGroupId() {
-	reader := suite.reader
-	reader.replicaGroupID = func() ([32]byte, error) { return [32]byte{32}, nil }
-	reader.recipientPaymentAddress = func() common.Address { return suite.recipientAddress }
-
-	channel, ok, err := reader.GetChannelStateFromBlockchain(suite.channelKey())
 
-	assert.Equal(suite.T(), errors.New("Channel received belongs to another group of replicas, current group: [32 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0], channel group: [123 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0]"), err)
-	assert.False(suite.T(), ok)
-	assert.Nil(suite.T(), channel)
-}
 
 func (suite *BlockchainChannelReaderSuite) TestGetChannelStateIncorrectRecipeintAddress() {
 	reader := suite.reader