Skip to content

Commit e3e140d

Browse files
committed
[FAB-6744]: Verify hash of pvt rwset
Currently at the block commit time, coordinator retrieves from the transient store required private read write sets, while doesn't validate whenever hash of extracted private rwset matches the hash value published on rwset of the transaction in the block. As a consequence peer fails to commit thus leading to panic. This commit adds a test to cover this use case and provides a fix, which takes care to filter out private rwsets with unexpcted hash values. Change-Id: I8c603327bcbb2a1ddc2bd819b85bc927df0c036e Signed-off-by: Artem Barger <bartem@il.ibm.com>
1 parent 43d3e63 commit e3e140d

File tree

2 files changed

+102
-8
lines changed

2 files changed

+102
-8
lines changed

gossip/privdata/coordinator.go

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -620,14 +620,6 @@ func (c *coordinator) listMissingPrivateData(block *common.Block, ownedRWsets ma
620620
if err != nil {
621621
return nil, errors.WithStack(err)
622622
}
623-
// In the end, iterate over the ownedRWsets, and if the key doesn't exist in
624-
// the privateRWsetsInBlock - delete it from the ownedRWsets
625-
for k := range ownedRWsets {
626-
if _, exists := privateRWsetsInBlock[k]; !exists {
627-
logger.Warning("Removed", k.namespace, k.collection, "hash", k.hash, "from the data passed to the ledger")
628-
delete(ownedRWsets, k)
629-
}
630-
}
631623

632624
privateInfo := &privateDataInfo{
633625
sources: sources,
@@ -640,6 +632,15 @@ func (c *coordinator) listMissingPrivateData(block *common.Block, ownedRWsets ma
640632
// Put into ownedRWsets RW sets that are missing and found in the transient store
641633
c.fetchMissingFromTransientStore(privateInfo.missingKeysByTxIDs, ownedRWsets)
642634

635+
// In the end, iterate over the ownedRWsets, and if the key doesn't exist in
636+
// the privateRWsetsInBlock - delete it from the ownedRWsets
637+
for k := range ownedRWsets {
638+
if _, exists := privateRWsetsInBlock[k]; !exists {
639+
logger.Warning("Removed", k.namespace, k.collection, "hash", k.hash, "from the data passed to the ledger")
640+
delete(ownedRWsets, k)
641+
}
642+
}
643+
643644
privateInfo.missingKeys = privateInfo.missingKeysByTxIDs.flatten()
644645
// Remove all keys we already own
645646
privateInfo.missingKeys.exclude(func(key rwSetKey) bool {

gossip/privdata/coordinator_test.go

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -683,6 +683,99 @@ func TestCoordinatorStoreInvalidBlock(t *testing.T) {
683683
assert.Contains(t, err.Error(), "Block data is empty")
684684
}
685685

686+
func TestCoordinatorToFilterOutPvtRWSetsWithWrongHash(t *testing.T) {
687+
/*
688+
Test case, where peer receives new block for commit
689+
it has ns1:c1 in transient store, while it has wrong
690+
hash, hence it will fetch ns1:c1 from other peers
691+
*/
692+
peerSelfSignedData := common.SignedData{
693+
Identity: []byte{0, 1, 2},
694+
Signature: []byte{3, 4, 5},
695+
Data: []byte{6, 7, 8},
696+
}
697+
698+
expectedPvtData := map[uint64]*ledger.TxPvtData{
699+
0: {SeqInBlock: 0, WriteSet: &rwset.TxPvtReadWriteSet{
700+
DataModel: rwset.TxReadWriteSet_KV,
701+
NsPvtRwset: []*rwset.NsPvtReadWriteSet{
702+
{
703+
Namespace: "ns1",
704+
CollectionPvtRwset: []*rwset.CollectionPvtReadWriteSet{
705+
{
706+
CollectionName: "c1",
707+
Rwset: []byte("rws-original"),
708+
},
709+
},
710+
},
711+
},
712+
}},
713+
}
714+
715+
cs := createcollectionStore(peerSelfSignedData).thatAcceptsAll()
716+
committer := &committerMock{}
717+
store := &mockTransientStore{t: t}
718+
719+
fetcher := &fetcherMock{t: t}
720+
721+
var commitHappened bool
722+
723+
committer.On("CommitWithPvtData", mock.Anything).Run(func(args mock.Arguments) {
724+
var privateDataPassed2Ledger privateData = args.Get(0).(*ledger.BlockAndPvtData).BlockPvtData
725+
assert.True(t, privateDataPassed2Ledger.Equal(expectedPvtData))
726+
commitHappened = true
727+
}).Return(nil)
728+
729+
hash := util2.ComputeSHA256([]byte("rws-original"))
730+
bf := &blockFactory{
731+
channelID: "test",
732+
}
733+
734+
block := bf.AddTxnWithEndorsement("tx1", "ns1", hash, "org1", "c1").create()
735+
store.On("GetTxPvtRWSetByTxid", "tx1", mock.Anything).Return((&mockRWSetScanner{}).withRWSet("ns1", "c1"), nil)
736+
737+
coordinator := NewCoordinator(Support{
738+
CollectionStore: cs,
739+
Committer: committer,
740+
Fetcher: fetcher,
741+
TransientStore: store,
742+
Validator: &validatorMock{},
743+
}, peerSelfSignedData)
744+
745+
fetcher.On("fetch", mock.Anything).expectingDigests([]*proto.PvtDataDigest{
746+
{
747+
TxId: "tx1", Namespace: "ns1", Collection: "c1", BlockSeq: 1,
748+
},
749+
}).expectingEndorsers("org1").Return([]*proto.PvtDataElement{
750+
{
751+
Digest: &proto.PvtDataDigest{
752+
BlockSeq: 1,
753+
Collection: "c1",
754+
Namespace: "ns1",
755+
TxId: "tx1",
756+
},
757+
Payload: [][]byte{[]byte("rws-original")},
758+
},
759+
}, nil)
760+
store.On("Persist", mock.Anything, uint64(1), mock.Anything).
761+
expectRWSet("ns1", "c1", []byte("rws-original")).Return(nil)
762+
763+
purgedTxns := make(map[string]struct{})
764+
store.On("PurgeByTxids", mock.Anything).Run(func(args mock.Arguments) {
765+
for _, txn := range args.Get(0).([]string) {
766+
purgedTxns[txn] = struct{}{}
767+
}
768+
}).Return(nil)
769+
770+
coordinator.StoreBlock(block, nil)
771+
// Assert blocks was eventually committed
772+
assert.True(t, commitHappened)
773+
774+
// Assert transaction has been purged
775+
_, exists := purgedTxns["tx1"]
776+
assert.True(t, exists)
777+
}
778+
686779
func TestCoordinatorStoreBlock(t *testing.T) {
687780
peerSelfSignedData := common.SignedData{
688781
Identity: []byte{0, 1, 2},

0 commit comments

Comments
 (0)