diff --git a/common/config/api.go b/common/config/api.go index 36823317d50..c5a7e9e44fb 100644 --- a/common/config/api.go +++ b/common/config/api.go @@ -21,7 +21,6 @@ package config import ( "time" - cb "github.com/hyperledger/fabric/protos/common" ab "github.com/hyperledger/fabric/protos/orderer" pb "github.com/hyperledger/fabric/protos/peer" ) @@ -86,10 +85,7 @@ type Orderer interface { type ValueProposer interface { // BeginValueProposals called when a config proposal is begun - BeginValueProposals(tx interface{}, groups []string) ([]ValueProposer, error) - - // ProposeValue called when config is added to a proposal - ProposeValue(tx interface{}, key string, configValue *cb.ConfigValue) error + BeginValueProposals(tx interface{}, groups []string) (ValueDeserializer, []ValueProposer, error) // RollbackProposals called when a config proposal is abandoned RollbackProposals(tx interface{}) diff --git a/common/config/proposer.go b/common/config/proposer.go index 2bed04c3fe8..49d2b289a1c 100644 --- a/common/config/proposer.go +++ b/common/config/proposer.go @@ -20,24 +20,27 @@ import ( "fmt" "sync" - cb "github.com/hyperledger/fabric/protos/common" - "github.com/golang/protobuf/proto" logging "github.com/op/go-logging" ) var logger = logging.MustGetLogger("common/config") -// ValuesDeserializer provides a mechanism to retrieve proto messages to deserialize config values into -type ValuesDeserializer interface { - // ProtoMsg behaves like a map lookup for key - ProtoMsg(key string) (proto.Message, bool) +// ValueDeserializer provides a mechanism to retrieve proto messages to deserialize config values into +type ValueDeserializer interface { + // Deserialize takes a Value key as a string, and a marshaled Value value as bytes + // and returns the deserialized version of that value. Note, this function operates + // with side effects intended. Using a ValueDeserializer to deserialize a message will + // generally set the value in the Values interface that the ValueDeserializer derived from + // Therefore, the proto.Message may be safely discarded, but may be retained for + // inspection and or debugging purposes. + Deserialize(key string, value []byte) (proto.Message, error) } // Values defines a mechanism to supply messages to unamrshal from config // and a mechanism to validate the results type Values interface { - ValuesDeserializer + ValueDeserializer // Validate should ensure that the values set into the proto messages are correct // and that the new group values are allowed. It also includes a tx ID in case cross @@ -75,7 +78,7 @@ func NewProposer(vh Handler) *Proposer { } // BeginValueProposals called when a config proposal is begun -func (p *Proposer) BeginValueProposals(tx interface{}, groups []string) ([]ValueProposer, error) { +func (p *Proposer) BeginValueProposals(tx interface{}, groups []string) (ValueDeserializer, []ValueProposer, error) { p.pendingLock.Lock() defer p.pendingLock.Unlock() if _, ok := p.pending[tx]; ok { @@ -104,7 +107,7 @@ func (p *Proposer) BeginValueProposals(tx interface{}, groups []string) ([]Value group, err = p.vh.NewGroup(groupName) if err != nil { pending = nil - return nil, fmt.Errorf("Error creating group %s: %s", groupName, err) + return nil, nil, fmt.Errorf("Error creating group %s: %s", groupName, err) } } @@ -114,28 +117,7 @@ func (p *Proposer) BeginValueProposals(tx interface{}, groups []string) ([]Value p.pending[tx] = pending - return result, nil -} - -// ProposeValue called when config is added to a proposal -func (p *Proposer) ProposeValue(tx interface{}, key string, configValue *cb.ConfigValue) error { - p.pendingLock.RLock() - pending, ok := p.pending[tx] - p.pendingLock.RUnlock() - if !ok { - logger.Panicf("Serious Programming Error: attempted to propose value for tx which had not been begun") - } - - msg, ok := pending.allocated.ProtoMsg(key) - if !ok { - return fmt.Errorf("Unknown value key %s for %T", key, p.vh) - } - - if err := proto.Unmarshal(configValue.Value, msg); err != nil { - return fmt.Errorf("Error unmarshaling key to proto message: %s", err) - } - - return nil + return pending.allocated, result, nil } // Validate ensures that the new config values is a valid change diff --git a/common/config/proposer_test.go b/common/config/proposer_test.go index 1d6fe6173a0..273a7bb9c3f 100644 --- a/common/config/proposer_test.go +++ b/common/config/proposer_test.go @@ -37,9 +37,17 @@ type mockValues struct { ValidateReturn error } -func (v *mockValues) ProtoMsg(key string) (proto.Message, bool) { +func (v *mockValues) Deserialize(key string, value []byte) (proto.Message, error) { msg, ok := v.ProtoMsgMap[key] - return msg, ok + if !ok { + return nil, fmt.Errorf("Missing message key: %s", key) + } + err := proto.Unmarshal(value, msg) + if err != nil { + return nil, err + } + + return msg, nil } func (v *mockValues) Validate(interface{}, map[string]ValueProposer) error { @@ -110,17 +118,19 @@ func TestGoodKeys(t *testing.T) { mh.AllocateReturn.ProtoMsgMap["Payload"] = &cb.Payload{} p := NewProposer(mh) - _, err := p.BeginValueProposals(t, nil) + vd, _, err := p.BeginValueProposals(t, nil) assert.NoError(t, err) env := &cb.Envelope{Payload: []byte("SOME DATA")} pay := &cb.Payload{Data: []byte("SOME OTHER DATA")} - assert.NoError(t, p.ProposeValue(t, "Envelope", &cb.ConfigValue{Value: utils.MarshalOrPanic(env)})) - assert.NoError(t, p.ProposeValue(t, "Payload", &cb.ConfigValue{Value: utils.MarshalOrPanic(pay)})) + msg, err := vd.Deserialize("Envelope", utils.MarshalOrPanic(env)) + assert.NoError(t, err) + assert.Equal(t, msg, env) - assert.Equal(t, mh.AllocateReturn.ProtoMsgMap["Envelope"], env) - assert.Equal(t, mh.AllocateReturn.ProtoMsgMap["Payload"], pay) + msg, err = vd.Deserialize("Payload", utils.MarshalOrPanic(pay)) + assert.NoError(t, err) + assert.Equal(t, msg, pay) } func TestBadMarshaling(t *testing.T) { @@ -128,10 +138,11 @@ func TestBadMarshaling(t *testing.T) { mh.AllocateReturn.ProtoMsgMap["Envelope"] = &cb.Envelope{} p := NewProposer(mh) - _, err := p.BeginValueProposals(t, nil) + vd, _, err := p.BeginValueProposals(t, nil) assert.NoError(t, err) - assert.Error(t, p.ProposeValue(t, "Envelope", &cb.ConfigValue{Value: []byte("GARBAGE")}), "Should have errored unmarshaling") + _, err = vd.Deserialize("Envelope", []byte("GARBAGE")) + assert.Error(t, err, "Should have errored unmarshaling") } func TestBadMissingMessage(t *testing.T) { @@ -139,10 +150,11 @@ func TestBadMissingMessage(t *testing.T) { mh.AllocateReturn.ProtoMsgMap["Payload"] = &cb.Payload{} p := NewProposer(mh) - _, err := p.BeginValueProposals(t, nil) + vd, _, err := p.BeginValueProposals(t, nil) assert.NoError(t, err) - assert.Error(t, p.ProposeValue(t, "Envelope", &cb.ConfigValue{Value: utils.MarshalOrPanic(&cb.Envelope{})}), "Should have errored on unexpected message") + _, err = vd.Deserialize("Envelope", utils.MarshalOrPanic(&cb.Envelope{})) + assert.Error(t, err, "Should have errored on unexpected message") } func TestGroups(t *testing.T) { @@ -151,18 +163,18 @@ func TestGroups(t *testing.T) { mh.NewGroupMap["bar"] = nil p := NewProposer(mh) - _, err := p.BeginValueProposals(t, []string{"foo", "bar"}) + _, _, err := p.BeginValueProposals(t, []string{"foo", "bar"}) assert.NoError(t, err, "Both groups were present") p.CommitProposals(t) mh.NewGroupMap = make(map[string]ValueProposer) - _, err = p.BeginValueProposals(t, []string{"foo", "bar"}) + _, _, err = p.BeginValueProposals(t, []string{"foo", "bar"}) assert.NoError(t, err, "Should not have tried to recreate the groups") p.CommitProposals(t) - _, err = p.BeginValueProposals(t, []string{"foo", "other"}) + _, _, err = p.BeginValueProposals(t, []string{"foo", "other"}) assert.Error(t, err, "Should not have errored when trying to create 'other'") - _, err = p.BeginValueProposals(t, []string{"foo"}) + _, _, err = p.BeginValueProposals(t, []string{"foo"}) assert.NoError(t, err, "Should be able to begin again without rolling back because of error") } diff --git a/common/config/root.go b/common/config/root.go index be25e8ca9af..f2d24ac9fa8 100644 --- a/common/config/root.go +++ b/common/config/root.go @@ -20,7 +20,8 @@ import ( "fmt" "github.com/hyperledger/fabric/common/config/msp" - cb "github.com/hyperledger/fabric/protos/common" + + "github.com/golang/protobuf/proto" ) // Root acts as the object which anchors the rest of the config @@ -39,16 +40,22 @@ func NewRoot(mspConfigHandler *msp.MSPConfigHandler) *Root { } } +type failDeserializer struct{} + +func (fd failDeserializer) Deserialize(key string, value []byte) (proto.Message, error) { + return nil, fmt.Errorf("Programming error, this should never be invoked") +} + // BeginValueProposals is used to start a new config proposal -func (r *Root) BeginValueProposals(tx interface{}, groups []string) ([]ValueProposer, error) { +func (r *Root) BeginValueProposals(tx interface{}, groups []string) (ValueDeserializer, []ValueProposer, error) { if len(groups) != 1 { - return nil, fmt.Errorf("Root config only supports having one base group") + return nil, nil, fmt.Errorf("Root config only supports having one base group") } if groups[0] != ChannelGroupKey { - return nil, fmt.Errorf("Root group must have channel") + return nil, nil, fmt.Errorf("Root group must have channel") } r.mspConfigHandler.BeginConfig(tx) - return []ValueProposer{r.channel}, nil + return failDeserializer{}, []ValueProposer{r.channel}, nil } // RollbackConfig is used to abandon a new config proposal @@ -66,11 +73,6 @@ func (r *Root) CommitProposals(tx interface{}) { r.mspConfigHandler.CommitProposals(tx) } -// ProposeValue should not be invoked on this object -func (r *Root) ProposeValue(tx interface{}, key string, value *cb.ConfigValue) error { - return fmt.Errorf("Programming error, this should never be invoked") -} - // Channel returns the associated Channel level config func (r *Root) Channel() *ChannelGroup { return r.channel diff --git a/common/config/root_test.go b/common/config/root_test.go index 8ab716127e8..40be82971d0 100644 --- a/common/config/root_test.go +++ b/common/config/root_test.go @@ -32,16 +32,19 @@ func init() { func TestBeginBadRoot(t *testing.T) { r := NewRoot(&msp.MSPConfigHandler{}) - _, err := r.BeginValueProposals(t, []string{ChannelGroupKey, ChannelGroupKey}) + _, _, err := r.BeginValueProposals(t, []string{ChannelGroupKey, ChannelGroupKey}) assert.Error(t, err, "Only one root element allowed") - _, err = r.BeginValueProposals(t, []string{"foo"}) + _, _, err = r.BeginValueProposals(t, []string{"foo"}) assert.Error(t, err, "Non %s group not allowed", ChannelGroupKey) } func TestProposeValue(t *testing.T) { - r := NewRoot(&msp.MSPConfigHandler{}) + r := NewRoot(msp.NewMSPConfigHandler()) + + vd, _, err := r.BeginValueProposals(t, []string{ChannelGroupKey}) + assert.NoError(t, err) - err := r.ProposeValue(t, "foo", nil) + _, err = vd.Deserialize("foo", nil) assert.Error(t, err, "ProposeValue should return error") } diff --git a/common/config/standardvalues.go b/common/config/standardvalues.go index 21b0c8fc6c7..6840eb1bc24 100644 --- a/common/config/standardvalues.go +++ b/common/config/standardvalues.go @@ -47,9 +47,21 @@ func NewStandardValues(protosStructs ...interface{}) (*standardValues, error) { return sv, nil } -func (sv *standardValues) ProtoMsg(key string) (proto.Message, bool) { +// Deserialize looks up the backing Values proto of the given name, unmarshals the given bytes +// to populate the backing message structure, and retuns a referenced to the retained deserialized +// message (or an error, either because the key did not exist, or there was an an error unmarshaling +func (sv *standardValues) Deserialize(key string, value []byte) (proto.Message, error) { msg, ok := sv.lookup[key] - return msg, ok + if !ok { + return nil, fmt.Errorf("Not found") + } + + err := proto.Unmarshal(value, msg) + if err != nil { + return nil, err + } + + return msg, nil } func (sv *standardValues) initializeProtosStruct(objValue reflect.Value) error { diff --git a/common/config/standardvalues_test.go b/common/config/standardvalues_test.go index 809b8327c1b..32eb987479c 100644 --- a/common/config/standardvalues_test.go +++ b/common/config/standardvalues_test.go @@ -20,6 +20,7 @@ import ( "testing" cb "github.com/hyperledger/fabric/protos/common" + "github.com/hyperledger/fabric/protos/utils" "github.com/stretchr/testify/assert" ) @@ -53,12 +54,12 @@ func TestSingle(t *testing.T) { assert.NotNil(t, fooVal.Msg1, "Should have initialized Msg1") assert.NotNil(t, fooVal.Msg2, "Should have initialized Msg2") - msg1, ok := sv.ProtoMsg("Msg1") - assert.True(t, ok, "Should have found map entry") + msg1, err := sv.Deserialize("Msg1", utils.MarshalOrPanic(&cb.Envelope{})) + assert.NoError(t, err, "Should have found map entry") assert.Equal(t, msg1, fooVal.Msg1, "Should be same entry") - msg2, ok := sv.ProtoMsg("Msg2") - assert.True(t, ok, "Should have found map entry") + msg2, err := sv.Deserialize("Msg2", utils.MarshalOrPanic(&cb.Payload{})) + assert.NoError(t, err, "Should have found map entry") assert.Equal(t, msg2, fooVal.Msg2, "Should be same entry") } @@ -71,20 +72,20 @@ func TestPair(t *testing.T) { assert.NotNil(t, fooVal.Msg2, "Should have initialized Msg2") assert.NotNil(t, barVal.Msg3, "Should have initialized Msg3") - msg1, ok := sv.ProtoMsg("Msg1") - assert.True(t, ok, "Should have found map entry") + msg1, err := sv.Deserialize("Msg1", utils.MarshalOrPanic(&cb.Envelope{})) + assert.NoError(t, err, "Should have found map entry") assert.Equal(t, msg1, fooVal.Msg1, "Should be same entry") - msg2, ok := sv.ProtoMsg("Msg2") - assert.True(t, ok, "Should have found map entry") + msg2, err := sv.Deserialize("Msg2", utils.MarshalOrPanic(&cb.Payload{})) + assert.NoError(t, err, "Should have found map entry") assert.Equal(t, msg2, fooVal.Msg2, "Should be same entry") - msg3, ok := sv.ProtoMsg("Msg3") - assert.True(t, ok, "Should have found map entry") + msg3, err := sv.Deserialize("Msg3", utils.MarshalOrPanic(&cb.Header{})) + assert.NoError(t, err, "Should have found map entry") assert.Equal(t, msg3, barVal.Msg3, "Should be same entry") } -func TestNestingConflict(t *testing.T) { +func TestPairConflict(t *testing.T) { _, err := NewStandardValues(&foo{}, &conflict{}) assert.Error(t, err, "Conflicting keys provided") } diff --git a/common/configtx/config.go b/common/configtx/config.go index 568e972421b..fc067038818 100644 --- a/common/configtx/config.go +++ b/common/configtx/config.go @@ -23,8 +23,22 @@ import ( "github.com/hyperledger/fabric/common/configtx/api" "github.com/hyperledger/fabric/common/policies" cb "github.com/hyperledger/fabric/protos/common" + + "github.com/golang/protobuf/proto" ) +type configGroupWrapper struct { + *cb.ConfigGroup + deserializedValues map[string]proto.Message +} + +func newConfigGroupWrapper(group *cb.ConfigGroup) *configGroupWrapper { + return &configGroupWrapper{ + ConfigGroup: group, + deserializedValues: make(map[string]proto.Message), + } +} + type configResult struct { tx interface{} handler api.Transactional @@ -62,7 +76,7 @@ func (cr *configResult) rollback() { // it will in turn recursively call itself until all groups have been exhausted // at each call, it returns the handler that was passed in, plus any handlers returned // by recursive calls into proposeGroup -func (cm *configManager) proposeGroup(tx interface{}, name string, group *cb.ConfigGroup, handler config.ValueProposer, policyHandler policies.Proposer) (*configResult, error) { +func (cm *configManager) proposeGroup(tx interface{}, name string, group *configGroupWrapper, handler config.ValueProposer, policyHandler policies.Proposer) (*configResult, error) { subGroups := make([]string, len(group.Groups)) i := 0 for subGroup := range group.Groups { @@ -71,7 +85,7 @@ func (cm *configManager) proposeGroup(tx interface{}, name string, group *cb.Con } logger.Debugf("Beginning new config for channel %s and group %s", cm.current.channelID, name) - subHandlers, err := handler.BeginValueProposals(tx, subGroups) + valueDeserializer, subHandlers, err := handler.BeginValueProposals(tx, subGroups) if err != nil { return nil, err } @@ -93,7 +107,7 @@ func (cm *configManager) proposeGroup(tx interface{}, name string, group *cb.Con } for i, subGroup := range subGroups { - subResult, err := cm.proposeGroup(tx, name+"/"+subGroup, group.Groups[subGroup], subHandlers[i], subPolicyHandlers[i]) + subResult, err := cm.proposeGroup(tx, name+"/"+subGroup, newConfigGroupWrapper(group.Groups[subGroup]), subHandlers[i], subPolicyHandlers[i]) if err != nil { result.rollback() return nil, err @@ -102,10 +116,12 @@ func (cm *configManager) proposeGroup(tx interface{}, name string, group *cb.Con } for key, value := range group.Values { - if err := handler.ProposeValue(tx, key, value); err != nil { + msg, err := valueDeserializer.Deserialize(key, value.Value) + if err != nil { result.rollback() return nil, err } + group.deserializedValues[key] = msg } for key, policy := range group.Policies { @@ -127,7 +143,7 @@ func (cm *configManager) proposeGroup(tx interface{}, name string, group *cb.Con func (cm *configManager) processConfig(channelGroup *cb.ConfigGroup) (*configResult, error) { helperGroup := cb.NewConfigGroup() helperGroup.Groups[RootGroupKey] = channelGroup - groupResult, err := cm.proposeGroup(channelGroup, "", helperGroup, cm.initializer.ValueProposer(), cm.initializer.PolicyProposer()) + groupResult, err := cm.proposeGroup(channelGroup, "", newConfigGroupWrapper(helperGroup), cm.initializer.ValueProposer(), cm.initializer.PolicyProposer()) if err != nil { return nil, err } diff --git a/common/configtx/manager_test.go b/common/configtx/manager_test.go index a17628e4a88..6b558c1d869 100644 --- a/common/configtx/manager_test.go +++ b/common/configtx/manager_test.go @@ -391,7 +391,7 @@ func TestInvalidProposal(t *testing.T) { t.Fatalf("Error constructing config manager: %s", err) } - initializer.ValueProposerVal = &mockconfigtx.ValueProposer{ErrorForProposeConfig: fmt.Errorf("err")} + initializer.ValueProposerVal = &mockconfigtx.ValueProposer{DeserializeError: fmt.Errorf("err")} newConfig := makeConfigUpdateEnvelope(defaultChain, makeConfigSet(), makeConfigSet(makeConfigPair("foo", "foo", 1, []byte("foo")))) diff --git a/common/mocks/configtx/configtx.go b/common/mocks/configtx/configtx.go index 2f13fc32b31..ec8b82699ef 100644 --- a/common/mocks/configtx/configtx.go +++ b/common/mocks/configtx/configtx.go @@ -22,6 +22,8 @@ import ( "github.com/hyperledger/fabric/common/policies" "github.com/hyperledger/fabric/msp" cb "github.com/hyperledger/fabric/protos/common" + + "github.com/golang/protobuf/proto" ) type Resources struct { @@ -129,22 +131,21 @@ type ValueProposer struct { LastKey string LastValue *cb.ConfigValue ErrorForProposeConfig error -} - -// ProposeConfig sets LastKey to key, and LastValue to configValue, returning ErrorForProposedConfig -func (vp *ValueProposer) ProposeValue(tx interface{}, key string, configValue *cb.ConfigValue) error { - vp.LastKey = key - vp.LastValue = configValue - return vp.ErrorForProposeConfig + DeserializeReturn proto.Message + DeserializeError error } // BeginConfig returns slices populated by self -func (vp *ValueProposer) BeginValueProposals(tx interface{}, groups []string) ([]config.ValueProposer, error) { +func (vp *ValueProposer) BeginValueProposals(tx interface{}, groups []string) (config.ValueDeserializer, []config.ValueProposer, error) { handlers := make([]config.ValueProposer, len(groups)) for i := range handlers { handlers[i] = vp } - return handlers, nil + return vp, handlers, nil +} + +func (vp *ValueProposer) Deserialize(key string, value []byte) (proto.Message, error) { + return vp.DeserializeReturn, vp.DeserializeError } // Manager is a mock implementation of configtxapi.Manager