Skip to content

Commit

Permalink
FAB-11583 Fix erroneous anchor peer output
Browse files Browse the repository at this point in the history
This CR cleans up the channel creation tx code considerably by removing
an unused parameter that triggered a more complex code path.  It also
updates the configtxlator.Update code to use proto comparisons instead
of reflection and adds a test case for the erroneous anchor peer update
which is now fixed.

Change-Id: Iecbc03327b1919b92bcf23a13f584da9e29323b4
Signed-off-by: Jason Yellick <jyellick@us.ibm.com>
  • Loading branch information
Jason Yellick committed Sep 11, 2018
1 parent 0ceb6d7 commit fb12372
Show file tree
Hide file tree
Showing 11 changed files with 45 additions and 112 deletions.
62 changes: 13 additions & 49 deletions common/tools/configtxgen/encoder/encoder.go
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,7 @@ func NewConsortiumGroup(conf *genesisconfig.Consortium) (*cb.ConfigGroup, error)

// NewChannelCreateConfigUpdate generates a ConfigUpdate which can be sent to the orderer to create a new channel. Optionally, the channel group of the
// ordering system channel may be passed in, and the resulting ConfigUpdate will extract the appropriate versions from this file.
func NewChannelCreateConfigUpdate(channelID string, orderingSystemChannelGroup *cb.ConfigGroup, conf *genesisconfig.Profile) (*cb.ConfigUpdate, error) {
func NewChannelCreateConfigUpdate(channelID string, conf *genesisconfig.Profile) (*cb.ConfigUpdate, error) {
if conf.Application == nil {
return nil, errors.New("cannot define a new channel with no Application section")
}
Expand All @@ -383,61 +383,25 @@ func NewChannelCreateConfigUpdate(channelID string, orderingSystemChannelGroup *
return nil, errors.New("cannot define a new channel with no Consortium value")
}

// Otherwise, parse only the application section, and encapsulate it inside a channel group
// Parse only the application section, and encapsulate it inside a channel group
ag, err := NewApplicationGroup(conf.Application)
if err != nil {
return nil, errors.Wrapf(err, "could not turn channel application profile into application group")
}

var template, newChannelGroup *cb.ConfigGroup

if orderingSystemChannelGroup != nil {
// In the case that a ordering system channel definition was provided, use it to compute the update
if orderingSystemChannelGroup.Groups == nil {
return nil, errors.New("missing all channel groups")
}

consortiums, ok := orderingSystemChannelGroup.Groups[channelconfig.ConsortiumsGroupKey]
if !ok {
return nil, errors.New("bad consortiums group")
}

consortium, ok := consortiums.Groups[conf.Consortium]
if !ok {
return nil, errors.Errorf("bad consortium: %s", conf.Consortium)
}

template = proto.Clone(orderingSystemChannelGroup).(*cb.ConfigGroup)
template.Groups[channelconfig.ApplicationGroupKey] = proto.Clone(consortium).(*cb.ConfigGroup)
// This is a bit of a hack. If the channel config specifies all consortium members, then it does not look
// like a modification. The below adds a fake org with an illegal name which cannot actually exist, which
// will always appear to be deleted, triggering the correct update computation.
template.Groups[channelconfig.ApplicationGroupKey].Groups["*IllegalKey*!"] = &cb.ConfigGroup{}
delete(template.Groups, channelconfig.ConsortiumsGroupKey)

newChannelGroup = proto.Clone(orderingSystemChannelGroup).(*cb.ConfigGroup)
delete(newChannelGroup.Groups, channelconfig.ConsortiumsGroupKey)
newChannelGroup.Groups[channelconfig.ApplicationGroupKey].Values = ag.Values
newChannelGroup.Groups[channelconfig.ApplicationGroupKey].Policies = ag.Policies

for orgName, org := range template.Groups[channelconfig.ApplicationGroupKey].Groups {
if _, ok := ag.Groups[orgName]; ok {
newChannelGroup.Groups[channelconfig.ApplicationGroupKey].Groups[orgName] = org
}
}
} else {
newChannelGroup = &cb.ConfigGroup{
Groups: map[string]*cb.ConfigGroup{
channelconfig.ApplicationGroupKey: ag,
},
}

// Otherwise assume the orgs have not been modified
template = proto.Clone(newChannelGroup).(*cb.ConfigGroup)
template.Groups[channelconfig.ApplicationGroupKey].Values = nil
template.Groups[channelconfig.ApplicationGroupKey].Policies = nil
newChannelGroup = &cb.ConfigGroup{
Groups: map[string]*cb.ConfigGroup{
channelconfig.ApplicationGroupKey: ag,
},
}

// Assume the orgs have not been modified
template = proto.Clone(newChannelGroup).(*cb.ConfigGroup)
template.Groups[channelconfig.ApplicationGroupKey].Values = nil
template.Groups[channelconfig.ApplicationGroupKey].Policies = nil

updt, err := update.Compute(&cb.Config{ChannelGroup: template}, &cb.Config{ChannelGroup: newChannelGroup})
if err != nil {
return nil, errors.Wrapf(err, "could not compute update")
Expand All @@ -457,8 +421,8 @@ func NewChannelCreateConfigUpdate(channelID string, orderingSystemChannelGroup *
}

// MakeChannelCreationTransaction is a handy utility function for creating transactions for channel creation
func MakeChannelCreationTransaction(channelID string, signer crypto.LocalSigner, orderingSystemChannelConfigGroup *cb.ConfigGroup, conf *genesisconfig.Profile) (*cb.Envelope, error) {
newChannelConfigUpdate, err := NewChannelCreateConfigUpdate(channelID, orderingSystemChannelConfigGroup, conf)
func MakeChannelCreationTransaction(channelID string, signer crypto.LocalSigner, conf *genesisconfig.Profile) (*cb.Envelope, error) {
newChannelConfigUpdate, err := NewChannelCreateConfigUpdate(channelID, conf)
if err != nil {
return nil, errors.Wrap(err, "config update generation failure")
}
Expand Down
64 changes: 16 additions & 48 deletions common/tools/configtxgen/encoder/encoder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,79 +73,47 @@ func TestConfigParsing(t *testing.T) {
}

func TestGoodChannelCreateConfigUpdate(t *testing.T) {
config := configtxgentest.Load(genesisconfig.SampleDevModeSoloProfile)
systemChannel, err := NewChannelGroup(config)
assert.NoError(t, err)
assert.NotNil(t, systemChannel)

createConfig := configtxgentest.Load(genesisconfig.SampleSingleMSPChannelProfile)

//ACLs does not marshal deterministically. Set it to nil is ok as its not
//updated anyway
createConfig.Application.ACLs = nil

configUpdate, err := NewChannelCreateConfigUpdate("channel.id", nil, createConfig)
configUpdate, err := NewChannelCreateConfigUpdate("channel.id", createConfig)
assert.NoError(t, err)
assert.NotNil(t, configUpdate)
}

defaultConfigUpdate, err := NewChannelCreateConfigUpdate("channel.id", systemChannel, createConfig)
func TestGoodChannelCreateNoAnchorPeers(t *testing.T) {
createConfig := configtxgentest.Load(genesisconfig.SampleSingleMSPChannelProfile)
createConfig.Application.Organizations[0].AnchorPeers = nil

configUpdate, err := NewChannelCreateConfigUpdate("channel.id", createConfig)
assert.NoError(t, err)
assert.NotNil(t, defaultConfigUpdate)
assert.NotNil(t, configUpdate)

assert.True(t, proto.Equal(configUpdate, defaultConfigUpdate), "the config used has had no updates, so should equal default")
// Anchor peers should not be set
assert.True(t, proto.Equal(
configUpdate.WriteSet.Groups["Application"].Groups["SampleOrg"],
&cb.ConfigGroup{},
))
}

func TestChannelCreateWithResources(t *testing.T) {
t.Run("AtV1.0", func(t *testing.T) {
createConfig := configtxgentest.Load(genesisconfig.SampleSingleMSPChannelProfile)
createConfig.Application.Capabilities = nil

configUpdate, err := NewChannelCreateConfigUpdate("channel.id", nil, createConfig)
configUpdate, err := NewChannelCreateConfigUpdate("channel.id", createConfig)
assert.NoError(t, err)
assert.NotNil(t, configUpdate)
assert.Nil(t, configUpdate.IsolatedData)
})
}

func TestNegativeChannelCreateConfigUpdate(t *testing.T) {
config := configtxgentest.Load(genesisconfig.SampleDevModeSoloProfile)
channelConfig := configtxgentest.Load(genesisconfig.SampleSingleMSPChannelProfile)
group, err := NewChannelGroup(config)
assert.NoError(t, err)
assert.NotNil(t, group)

t.Run("NoGroups", func(t *testing.T) {
channelGroup := proto.Clone(group).(*cb.ConfigGroup)
channelGroup.Groups = nil
_, err := NewChannelCreateConfigUpdate("channel.id", &cb.ConfigGroup{}, channelConfig)
assert.Error(t, err)
assert.Regexp(t, "missing all channel groups", err.Error())
})

t.Run("NoConsortiumsGroup", func(t *testing.T) {
channelGroup := proto.Clone(group).(*cb.ConfigGroup)
delete(channelGroup.Groups, channelconfig.ConsortiumsGroupKey)
_, err := NewChannelCreateConfigUpdate("channel.id", channelGroup, channelConfig)
assert.Error(t, err)
assert.Regexp(t, "bad consortiums group", err.Error())
})

t.Run("NoConsortiums", func(t *testing.T) {
channelGroup := proto.Clone(group).(*cb.ConfigGroup)
delete(channelGroup.Groups[channelconfig.ConsortiumsGroupKey].Groups, genesisconfig.SampleConsortiumName)
_, err := NewChannelCreateConfigUpdate("channel.id", channelGroup, channelConfig)
assert.Error(t, err)
assert.Regexp(t, "bad consortium:", err.Error())
})
}

func TestMakeChannelCreationTransactionWithSigner(t *testing.T) {
channelID := "foo"

msptesttools.LoadDevMsp()
signer := localmsp.NewSigner()

cct, err := MakeChannelCreationTransaction(channelID, signer, nil, configtxgentest.Load(genesisconfig.SampleSingleMSPChannelProfile))
cct, err := MakeChannelCreationTransaction(channelID, signer, configtxgentest.Load(genesisconfig.SampleSingleMSPChannelProfile))
assert.NoError(t, err, "Making chain creation tx")

assert.NotEmpty(t, cct.Signature, "Should have signature")
Expand All @@ -165,7 +133,7 @@ func TestMakeChannelCreationTransactionWithSigner(t *testing.T) {

func TestMakeChannelCreationTransactionNoSigner(t *testing.T) {
channelID := "foo"
cct, err := MakeChannelCreationTransaction(channelID, nil, nil, configtxgentest.Load(genesisconfig.SampleSingleMSPChannelProfile))
cct, err := MakeChannelCreationTransaction(channelID, nil, configtxgentest.Load(genesisconfig.SampleSingleMSPChannelProfile))
assert.NoError(t, err, "Making chain creation tx")

assert.Empty(t, cct.Signature, "Should have empty signature")
Expand Down
6 changes: 3 additions & 3 deletions common/tools/configtxgen/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ func doOutputBlock(config *genesisconfig.Profile, channelID string, outputBlock
func doOutputChannelCreateTx(conf *genesisconfig.Profile, channelID string, outputChannelCreateTx string) error {
logger.Info("Generating new channel configtx")

configtx, err := encoder.MakeChannelCreationTransaction(channelID, nil, nil, conf)
configtx, err := encoder.MakeChannelCreationTransaction(channelID, nil, conf)
if err != nil {
return err
}
Expand Down Expand Up @@ -222,9 +222,9 @@ func main() {

flag.Parse()

if channelID == "" {
if channelID == "" && (outputBlock != "" || outputChannelCreateTx != "" || outputAnchorPeersUpdate != "") {
channelID = genesisconfig.TestChainID
logger.Warningf("Omitting the channel ID for configtxgen is deprecated. Explicitly passing the channel ID will be required in the future, defaulting to '%s'.", channelID)
logger.Warningf("Omitting the channel ID for configtxgen for output operations is deprecated. Explicitly passing the channel ID will be required in the future, defaulting to '%s'.", channelID)
}

// show version
Expand Down
7 changes: 4 additions & 3 deletions common/tools/configtxlator/update/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,10 @@ limitations under the License.
package update

import (
"bytes"
"fmt"
"reflect"

"github.com/golang/protobuf/proto"
cb "github.com/hyperledger/fabric/protos/common"
)

Expand All @@ -38,7 +39,7 @@ func computePoliciesMapUpdate(original, updated map[string]*cb.ConfigPolicy) (re
continue
}

if originalPolicy.ModPolicy == updatedPolicy.ModPolicy && reflect.DeepEqual(originalPolicy.Policy, updatedPolicy.Policy) {
if originalPolicy.ModPolicy == updatedPolicy.ModPolicy && proto.Equal(originalPolicy.Policy, updatedPolicy.Policy) {
sameSet[policyName] = &cb.ConfigPolicy{
Version: originalPolicy.Version,
}
Expand Down Expand Up @@ -83,7 +84,7 @@ func computeValuesMapUpdate(original, updated map[string]*cb.ConfigValue) (readS
continue
}

if originalValue.ModPolicy == updatedValue.ModPolicy && reflect.DeepEqual(originalValue.Value, updatedValue.Value) {
if originalValue.ModPolicy == updatedValue.ModPolicy && bytes.Equal(originalValue.Value, updatedValue.Value) {
sameSet[valueName] = &cb.ConfigValue{
Version: originalValue.Version,
}
Expand Down
2 changes: 1 addition & 1 deletion core/common/validation/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ import (
func TestValidateConfigTx(t *testing.T) {
chainID := util.GetTestChainID()
profile := configtxgentest.Load(genesisconfig.SampleSingleMSPChannelProfile)
chCrtEnv, err := encoder.MakeChannelCreationTransaction(genesisconfig.SampleConsortiumName, nil, nil, profile)
chCrtEnv, err := encoder.MakeChannelCreationTransaction(genesisconfig.SampleConsortiumName, nil, profile)
if err != nil {
t.Fatalf("MakeChannelCreationTransaction failed, err %s", err)
return
Expand Down
2 changes: 1 addition & 1 deletion orderer/common/msgprocessor/systemchannel_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -674,7 +674,7 @@ func TestNewChannelConfig(t *testing.T) {

// Successful
t.Run("Success", func(t *testing.T) {
createTx, err := encoder.MakeChannelCreationTransaction("foo", nil, nil, configtxgentest.Load(genesisconfig.SampleSingleMSPChannelProfile))
createTx, err := encoder.MakeChannelCreationTransaction("foo", nil, configtxgentest.Load(genesisconfig.SampleSingleMSPChannelProfile))
assert.Nil(t, err)
res, err := templator.NewChannelConfig(createTx)
assert.Nil(t, err)
Expand Down
6 changes: 3 additions & 3 deletions orderer/common/msgprocessor/systemchannelfilter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ func TestGoodProposal(t *testing.T) {

mcc := newMockChainCreator()

configUpdate, err := encoder.MakeChannelCreationTransaction(newChainID, nil, nil, configtxgentest.Load(genesisconfig.SampleSingleMSPChannelProfile))
configUpdate, err := encoder.MakeChannelCreationTransaction(newChainID, nil, configtxgentest.Load(genesisconfig.SampleSingleMSPChannelProfile))
assert.Nil(t, err, "Error constructing configtx")
ingressTx := makeConfigTxFromConfigUpdateTx(configUpdate)

Expand All @@ -149,7 +149,7 @@ func TestProposalRejectedByConfig(t *testing.T) {
mcc := newMockChainCreator()
mcc.NewChannelConfigErr = fmt.Errorf("desired err text")

configUpdate, err := encoder.MakeChannelCreationTransaction(newChainID, nil, nil, configtxgentest.Load(genesisconfig.SampleSingleMSPChannelProfile))
configUpdate, err := encoder.MakeChannelCreationTransaction(newChainID, nil, configtxgentest.Load(genesisconfig.SampleSingleMSPChannelProfile))
assert.Nil(t, err, "Error constructing configtx")
ingressTx := makeConfigTxFromConfigUpdateTx(configUpdate)

Expand All @@ -169,7 +169,7 @@ func TestNumChainsExceeded(t *testing.T) {
mcc.ms.msc.MaxChannelsCountVal = 1
mcc.newChains = make([]*cb.Envelope, 2)

configUpdate, err := encoder.MakeChannelCreationTransaction(newChainID, nil, nil, configtxgentest.Load(genesisconfig.SampleSingleMSPChannelProfile))
configUpdate, err := encoder.MakeChannelCreationTransaction(newChainID, nil, configtxgentest.Load(genesisconfig.SampleSingleMSPChannelProfile))
assert.Nil(t, err, "Error constructing configtx")
ingressTx := makeConfigTxFromConfigUpdateTx(configUpdate)

Expand Down
2 changes: 1 addition & 1 deletion orderer/common/multichannel/registrar_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ func TestNewChain(t *testing.T) {
manager := NewRegistrar(lf, consenters, mockCrypto())
orglessChannelConf := configtxgentest.Load(genesisconfig.SampleSingleMSPChannelProfile)
orglessChannelConf.Application.Organizations = nil
envConfigUpdate, err := encoder.MakeChannelCreationTransaction(newChainID, mockCrypto(), nil, orglessChannelConf)
envConfigUpdate, err := encoder.MakeChannelCreationTransaction(newChainID, mockCrypto(), orglessChannelConf)
assert.NoError(t, err, "Constructing chain creation tx")

res, err := manager.NewChannelConfig(envConfigUpdate)
Expand Down
2 changes: 1 addition & 1 deletion orderer/common/performance/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ func CreateChannel(server *BenchmarkServer, channelProfile *genesisconfig.Profil
defer client.Close()

channelID := RandomID(10)
createChannelTx, err := encoder.MakeChannelCreationTransaction(channelID, localmsp.NewSigner(), nil, channelProfile)
createChannelTx, err := encoder.MakeChannelCreationTransaction(channelID, localmsp.NewSigner(), channelProfile)
if err != nil {
logger.Panicf("Failed to create channel creation transaction: %s", err)
}
Expand Down
2 changes: 1 addition & 1 deletion orderer/sample_clients/broadcast_config/newchain.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (
)

func newChainRequest(consensusType, creationPolicy, newChannelID string) *cb.Envelope {
env, err := encoder.MakeChannelCreationTransaction(newChannelID, localmsp.NewSigner(), nil, genesisconfig.Load(genesisconfig.SampleSingleMSPChannelProfile))
env, err := encoder.MakeChannelCreationTransaction(newChannelID, localmsp.NewSigner(), genesisconfig.Load(genesisconfig.SampleSingleMSPChannelProfile))
if err != nil {
panic(err)
}
Expand Down
2 changes: 1 addition & 1 deletion peer/channel/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ func createCmd(cf *ChannelCmdFactory) *cobra.Command {
}

func createChannelFromDefaults(cf *ChannelCmdFactory) (*cb.Envelope, error) {
chCrtEnv, err := encoder.MakeChannelCreationTransaction(channelID, localsigner.NewSigner(), nil, genesisconfig.Load(genesisconfig.SampleSingleMSPChannelProfile))
chCrtEnv, err := encoder.MakeChannelCreationTransaction(channelID, localsigner.NewSigner(), genesisconfig.Load(genesisconfig.SampleSingleMSPChannelProfile))
if err != nil {
return nil, err
}
Expand Down

0 comments on commit fb12372

Please sign in to comment.