Skip to content

Commit

Permalink
Reject re-defining identical uncommitted cc def
Browse files Browse the repository at this point in the history
When user attempts to update an uncommitted chaincode definition
with identical content as before, endorser should fail the invocation.
Otherwise, it results in an empty wset, which would be validated
against LifecycleEndorsement policy by default, instead of the policy
of org's own implicit collection.

FAB-17371

Signed-off-by: Jay Guo <guojiannan1101@gmail.com>
  • Loading branch information
guoger authored and ale-linux committed Jan 12, 2020
1 parent ac9f694 commit 58af762
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 2 deletions.
26 changes: 25 additions & 1 deletion core/chaincode/lifecycle/lifecycle.go
Original file line number Diff line number Diff line change
Expand Up @@ -407,6 +407,30 @@ func (ef *ExternalFunctions) ApproveChaincodeDefinitionForOrg(chname, ccname str
}

privateName := fmt.Sprintf("%s#%d", ccname, requestedSequence)

// if requested sequence is not committed, and attempt is made to update its content,
// we need to check whether new definition actually contains updated content, to avoid
// empty write set.
if requestedSequence == currentSequence+1 {
uncommittedMetadata, ok, err := ef.Resources.Serializer.DeserializeMetadata(NamespacesName, privateName, orgState)
if err != nil {
return errors.WithMessage(err, "could not fetch uncommitted definition")
}

if ok {
logger.Debugf("Attempting to redefine uncommitted definition at sequence %d", requestedSequence)

uncommittedParameters := &ChaincodeParameters{}
if err := ef.Resources.Serializer.Deserialize(NamespacesName, privateName, uncommittedMetadata, uncommittedParameters, orgState); err != nil {
return errors.WithMessagef(err, "could not deserialize namespace %s as chaincode", privateName)
}

if err := uncommittedParameters.Equal(cd.Parameters()); err == nil {
return errors.Errorf("attempted to redefine uncommitted sequence (%d) for namespace %s with unchanged content", requestedSequence, ccname)
}
}
}

if err := ef.Resources.Serializer.Serialize(NamespacesName, privateName, cd.Parameters(), orgState); err != nil {
return errors.WithMessage(err, "could not serialize chaincode parameters to state")
}
Expand All @@ -423,7 +447,7 @@ func (ef *ExternalFunctions) ApproveChaincodeDefinitionForOrg(chname, ccname str
return errors.WithMessage(err, "could not serialize chaincode package info to state")
}

logger.Infof("successfully approved definition %s, name '%s' on channel '%s'", cd, ccname, chname)
logger.Infof("Successfully approved definition %s, name '%s' on channel '%s'", cd, ccname, chname)

return nil
}
Expand Down
24 changes: 23 additions & 1 deletion core/chaincode/lifecycle/lifecycle_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -662,7 +662,7 @@ var _ = Describe("ExternalFunctions", func() {
})
})

Context("when the sequence number already has a definition", func() {
Context("when the sequence number already has a committed definition", func() {
BeforeEach(func() {
err := resources.Serializer.Serialize("namespaces", "cc-name", &lifecycle.ChaincodeDefinition{
Sequence: 5,
Expand Down Expand Up @@ -743,6 +743,28 @@ var _ = Describe("ExternalFunctions", func() {
})
})

Context("when the sequence number already has an uncommitted definition", func() {
BeforeEach(func() {
err := ef.ApproveChaincodeDefinitionForOrg("my-channel", "cc-name", testDefinition, "hash", fakePublicState, fakeOrgState)
Expect(err).NotTo(HaveOccurred())
})

Context("when uncommitted definition differs from update", func() {
It("succeeds", func() {
testDefinition.ValidationInfo.ValidationParameter = []byte("more awesome policy")
err := ef.ApproveChaincodeDefinitionForOrg("my-channel", "cc-name", testDefinition, "hash", fakePublicState, fakeOrgState)
Expect(err).NotTo(HaveOccurred())
})
})

Context("when uncommitted definition is identical to update", func() {
It("returns error", func() {
err := ef.ApproveChaincodeDefinitionForOrg("my-channel", "cc-name", testDefinition, "hash", fakePublicState, fakeOrgState)
Expect(err).To(MatchError("attempted to redefine uncommitted sequence (5) for namespace cc-name with unchanged content"))
})
})
})

Context("when the definition is for an expired sequence number", func() {
BeforeEach(func() {
testDefinition.Sequence = 3
Expand Down

0 comments on commit 58af762

Please sign in to comment.