[FAB-2335] Add PreCommit transaction hook

https://jira.hyperledger.org/browse/FAB-2335

Today, config transactions go through three phases:

BeginValueProposals()
ProposeValue(...)
...
ProposeValue(...)
CommitProposals()

This works well for verifying individual components of configuration,
but fails when configuration must be validated in its totality.  This is
exhibited for instance in the instantiation of the MSP manager where the
check is performed at every value proposal, rather than only once as is
necessary.  So, the new path becomes:

BeginValueProposals()
ProposeValue(...)
...
ProposeValue(...)
PreCommit()
CommitProposals()

Where PreCommit may return an error and stop the commit process.

This concept also works nicely with some other pending config work
intended to remove a significant amount of duplicated code.

Change-Id: Ibf7450e6b6e355690af2cb14278f0fce4efdf0d7
Signed-off-by: Jason Yellick <[email protected]>

Author: Jason Yellick

common/configtx/api/api.go

@@ -72,6 +72,9 @@ type Transactional interface {
 	// RollbackConfig called when a config proposal is abandoned
 	RollbackProposals()
 
+	// PreCommit verifies that the transaction can be committed successfully
+	PreCommit() error
+
 	// CommitConfig called when a config proposal is committed
 	CommitProposals()
 }

common/configtx/config.go

@@ -31,6 +31,16 @@ type configResult struct {
 	subResults    []*configResult
 }
 
+func (cr *configResult) preCommit() error {
+	for _, subResult := range cr.subResults {
+		err := subResult.preCommit()
+		if err != nil {
+			return err
+		}
+	}
+	return cr.handler.PreCommit()
+}
+
 func (cr *configResult) commit() {
 	for _, subResult := range cr.subResults {
 		subResult.commit()
@@ -103,6 +113,12 @@ func (cm *configManager) proposeGroup(name string, group *cb.ConfigGroup, handle
 		}
 	}
 
+	err = result.preCommit()
+	if err != nil {
+		result.rollback()
+		return nil, err
+	}
+
 	return result, nil
 }
 

common/configtx/initializer.go

@@ -117,6 +117,12 @@ func (i *valueProposerRoot) RollbackProposals() {
 	i.mspConfigHandler.RollbackProposals()
 }
 
+// PreCommit is used to verify total configuration before commit
+func (i *valueProposerRoot) PreCommit() error {
+	logger.Debugf("Calling pre commit for MSP manager")
+	return i.mspConfigHandler.PreCommit()
+}
+
 // CommitConfig is used to commit a new config proposal
 func (i *valueProposerRoot) CommitProposals() {
 	logger.Debugf("Calling commit for MSP manager")
@@ -139,6 +145,11 @@ func (i *policyProposerRoot) ProposePolicy(key string, policy *cb.ConfigPolicy)
 	return fmt.Errorf("Programming error, this should never be invoked")
 }
 
+// PreCommit is a no-op and returns nil
+func (i *policyProposerRoot) PreCommit() error {
+	return nil
+}
+
 // RollbackConfig is used to abandon a new config proposal
 func (i *policyProposerRoot) RollbackProposals() {}
 

common/configvalues/api.go

@@ -84,6 +84,11 @@ type ValueProposer interface {
 	// RollbackProposals called when a config proposal is abandoned
 	RollbackProposals()
 
+	// PreCommit is invoked before committing the config to catch
+	// any errors which cannot be caught on a per proposal basis
+	// TODO, rename other methods to remove Value/Proposal references
+	PreCommit() error
+
 	// CommitProposals called when a config proposal is committed
 	CommitProposals()
 }

common/configvalues/channel/application/organization.go

@@ -111,3 +111,6 @@ func (oc *ApplicationOrgConfig) ProposeValue(key string, configValue *cb.ConfigV
 
 	return nil
 }
+
+// PreCommit returns nil
+func (c *ApplicationOrgConfig) PreCommit() error { return nil }

common/configvalues/channel/application/sharedconfig.go

@@ -121,3 +121,6 @@ func (di *SharedConfigImpl) ProposeValue(key string, configValue *cb.ConfigValue
 func (di *SharedConfigImpl) Organizations() map[string]api.ApplicationOrg {
 	return di.config.orgs
 }
+
+// PreCommit returns nil
+func (di *SharedConfigImpl) PreCommit() error { return nil }

common/configvalues/channel/common/organization/organization.go

@@ -143,3 +143,6 @@ func (oc *OrgConfig) ProposeValue(key string, configValue *cb.ConfigValue) error
 	}
 	return nil
 }
+
+// PreCommit returns nil
+func (oc *OrgConfig) PreCommit() error { return nil }

common/configvalues/channel/config.go

@@ -158,6 +158,9 @@ func (c *Config) CommitProposals() {
 	c.pending = nil
 }
 
+// PreCommit returns nil
+func (c *Config) PreCommit() error { return nil }
+
 // ProposeValue is used to add new config to the config proposal
 func (c *Config) ProposeValue(key string, configValue *cb.ConfigValue) error {
 	switch key {

common/configvalues/channel/orderer/sharedconfig.go

@@ -315,3 +315,6 @@ func brokerEntrySeemsValid(broker string) bool {
 	matched := re.FindString(host)
 	return len(matched) == len(host)
 }
+
+// PreCommit returns nil
+func (pm *ManagerImpl) PreCommit() error { return nil }

common/configvalues/msp/config.go

@@ -93,11 +93,21 @@ func (bh *MSPConfigHandler) ProposeMSP(mspConfig *mspprotos.MSPConfig) (msp.MSP,
 	existingPendingMSPConfig, ok := bh.pendingConfig.idMap[mspID]
 	if ok && !reflect.DeepEqual(existingPendingMSPConfig.mspConfig, mspConfig) {
 		return nil, fmt.Errorf("Attempted to define two different versions of MSP: %s", mspID)
-	} else {
-		bh.pendingConfig.idMap[mspID] = &pendingMSPConfig{
-			mspConfig: mspConfig,
-			msp:       mspInst,
-		}
+	}
+
+	bh.pendingConfig.idMap[mspID] = &pendingMSPConfig{
+		mspConfig: mspConfig,
+		msp:       mspInst,
+	}
+
+	return mspInst, nil
+}
+
+// PreCommit instantiates the MSP manager
+func (bh *MSPConfigHandler) PreCommit() error {
+	if len(bh.pendingConfig.idMap) == 0 {
+		// Cannot instantiate an MSP manager with no MSPs
+		return nil
 	}
 
 	mspList := make([]msp.MSP, len(bh.pendingConfig.idMap))
@@ -107,14 +117,7 @@ func (bh *MSPConfigHandler) ProposeMSP(mspConfig *mspprotos.MSPConfig) (msp.MSP,
 		i++
 	}
 
-	// the only way to make sure that I have a
-	// workable config is to toss the proposed
-	// manager, create a new one, call setup on
-	// it and return whatever error setup gives me
 	bh.pendingConfig.proposedMgr = msp.NewMSPManager()
-	err = bh.pendingConfig.proposedMgr.Setup(mspList)
-	if err != nil {
-		return nil, err
-	}
-	return mspInst, nil
+	err := bh.pendingConfig.proposedMgr.Setup(mspList)
+	return err
 }

common/configvalues/msp/config_test.go

@@ -35,6 +35,7 @@ func TestMSPConfigManager(t *testing.T) {
 	mspCH.BeginConfig()
 	_, err = mspCH.ProposeMSP(conf)
 	assert.NoError(t, err)
+	mspCH.PreCommit()
 	mspCH.CommitProposals()
 
 	msps, err := mspCH.GetMSPs()

common/mocks/configtx/configtx.go

@@ -70,6 +70,9 @@ func (r *Resources) MSPManager() msp.MSPManager {
 // Transactional implements the configtxapi.Transactional
 type Transactional struct{}
 
+// PreCommit returns nil
+func (t *Transactional) PreCommit() error { return nil }
+
 // CommitConfig does nothing
 func (t *Transactional) CommitProposals() {}
 

common/policies/policy.go

@@ -54,13 +54,20 @@ type Manager interface {
 
 // Proposer is the interface used by the configtx manager for policy management
 type Proposer interface {
+	// BeginPolicyProposals starts a policy update transaction
 	BeginPolicyProposals(groups []string) ([]Proposer, error)
 
+	// ProposePolicy createss a pending policy update from a ConfigPolicy
 	ProposePolicy(name string, policy *cb.ConfigPolicy) error
 
+	// RollbackProposals discards the pending policy updates
 	RollbackProposals()
 
+	// CommitProposals commits the pending policy updates
 	CommitProposals()
+
+	// PreCommit tests if a commit will apply
+	PreCommit() error
 }
 
 // Provider provides the backing implementation of a policy
@@ -176,6 +183,11 @@ func (pm *ManagerImpl) RollbackProposals() {
 	pm.pendingConfig = nil
 }
 
+// PreCommit is currently a no-op for the policy manager and always returns nil
+func (pm *ManagerImpl) PreCommit() error {
+	return nil
+}
+
 // CommitProposals is used to commit a new config proposal
 func (pm *ManagerImpl) CommitProposals() {
 	if pm.pendingConfig == nil {