https://github.com/singnet/snet-daemon/issues/224

Group ID check should only apply at the time of storing channel state in etcd for the first time

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()
 }

escrow/escrow_test.go

@@ -2,6 +2,7 @@ package escrow
 
 import (
 	"crypto/ecdsa"
+	"errors"
 	"fmt"
 	"math/big"
 	"testing"
@@ -280,3 +281,41 @@ 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 := NewPaymentChannelService(
+		suite.storage,
+		suite.paymentStorage,
+		&BlockchainChannelReader{
+			replicaGroupID: func() ([32]byte, error) {
+				return [32]byte{125}, nil
+			},
+			readChannelFromBlockchain: func(channelID *big.Int) (*blockchain.MultiPartyEscrowChannel, bool, error) {
+				return suite.mpeChannel(), true, nil
+			},
+			recipientPaymentAddress: func() common.Address {
+				return suite.recipientAddress
+			},
+		},
+		NewEtcdLocker(suite.memoryStorage),
+		&ChannelPaymentValidator{
+			currentBlock:               func() (*big.Int, error) { return big.NewInt(99), nil },
+			paymentExpirationThreshold: func() *big.Int { return big.NewInt(0) },
+		},
+	)
+	//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)
+	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)
+}

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).

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