Skip to content

Commit

Permalink
[FAB-3493] Fix LAST_CONFIG on new channels
Browse files Browse the repository at this point in the history
The fix submitted in [1] works in that it delivers what we were looking
for (a pointer to the last config block) but is based on an incorrect
assumption: that the sequence number of the genesis block of a
non-system channel should be zero.

I assumed that should be the case, what with it being the genesis block
and all, but this breaks the abstraction that the config update
framework introduces. This changeset introduces the right fix. Thanks to
Jason Yellick for pointing this out.

[1] https://gerrit.hyperledger.org/r/#/c/8825/

Change-Id: I61ef2825c4cfe55495a4d81cff759fd6c340c191
Signed-off-by: Kostas Christidis <kostas@christidis.io>
  • Loading branch information
kchristidis committed May 9, 2017
1 parent 587387f commit db236d6
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 3 deletions.
2 changes: 1 addition & 1 deletion orderer/common/broadcast/broadcast.go
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ func (bh *handlerImpl) Handle(srv ab.AtomicBroadcast_BroadcastServer) error {
}

if logger.IsEnabledFor(logging.DEBUG) {
logger.Debugf("Broadcast has successfully enqueued message of type %d for chain %s", chdr.Type, chdr.ChannelId)
logger.Debugf("Broadcast has successfully enqueued message of type %s for chain %s", cb.HeaderType_name[chdr.Type], chdr.ChannelId)
}

err = srv.Send(&ab.BroadcastResponse{Status: cb.Status_SUCCESS})
Expand Down
3 changes: 3 additions & 0 deletions orderer/multichain/chainsupport.go
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,7 @@ func (cs *chainSupport) addLastConfigSignature(block *cb.Block) {
}

lastConfigValue := utils.MarshalOrPanic(&cb.LastConfig{Index: cs.lastConfig})
logger.Debugf("[channel: %s] About to write block, setting its LAST_CONFIG to %d", cs.ChainID(), cs.lastConfig)

lastConfigSignature.Signature = utils.SignOrPanic(cs.signer, util.ConcatenateBytes(lastConfigValue, lastConfigSignature.SignatureHeader, block.Header.Bytes()))

Expand All @@ -257,6 +258,8 @@ func (cs *chainSupport) WriteBlock(block *cb.Block, committers []filter.Committe
if err != nil {
logger.Panicf("[channel: %s] Could not append block: %s", cs.ChainID(), err)
}
logger.Debugf("[channel: %s] Wrote block %d", cs.ChainID(), block.GetHeader().Number)

return block
}

Expand Down
5 changes: 5 additions & 0 deletions orderer/multichain/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,11 @@ func (ml *multiLedger) newChain(configtx *cb.Envelope) {

logger.Infof("Created and starting new chain %s", chainID)

cs.lastConfigSeq = cs.Sequence()
// The sequence number on the genesis block of the system channel will be 0.
// The sequence number on the genesis block of every non-system channel will be 1.
logger.Debugf("[channel: %s] Last config set to %d", chainID, cs.lastConfigSeq)

newChains[string(chainID)] = cs
cs.start()

Expand Down
21 changes: 19 additions & 2 deletions orderer/multichain/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"testing"
"time"

"github.com/golang/protobuf/proto"
"github.com/hyperledger/fabric/common/configtx"
genesisconfig "github.com/hyperledger/fabric/common/configtx/tool/localconfig"
"github.com/hyperledger/fabric/common/configtx/tool/provisional"
Expand Down Expand Up @@ -235,15 +236,17 @@ func TestSignatureFilter(t *testing.T) {

// This test brings up the entire system, with the mock consenter, including the broadcasters etc. and creates a new chain
func TestNewChain(t *testing.T) {
expectedLastConfigBlockNumber := uint64(0)
expectedLastConfigSeq := uint64(1)
newChainID := "TestNewChain"

lf, rl := NewRAMLedgerAndFactory(10)

consenters := make(map[string]Consenter)
consenters[conf.Orderer.OrdererType] = &mockConsenter{}

manager := NewManagerImpl(lf, consenters, mockCrypto())

newChainID := "TestNewChain"

envConfigUpdate, err := configtx.MakeChainCreationTransaction(newChainID, genesisconfig.SampleConsortiumName, mockSigningIdentity)
assert.NoError(t, err, "Constructing chain creation tx")

Expand All @@ -252,6 +255,7 @@ func TestNewChain(t *testing.T) {

configEnv, err := cm.ProposeConfigUpdate(envConfigUpdate)
assert.NoError(t, err, "Proposing initial update")
assert.Equal(t, expectedLastConfigSeq, configEnv.GetConfig().Sequence, "Sequence of config envelope for new channel should always be set to %d", expectedLastConfigSeq)

ingressTx, err := utils.CreateSignedEnvelope(cb.HeaderType_CONFIG, newChainID, mockCrypto(), configEnv, msgVersion, epoch)
assert.NoError(t, err, "Creating ingresstx")
Expand All @@ -260,6 +264,7 @@ func TestNewChain(t *testing.T) {

chainSupport, ok := manager.GetChain(manager.SystemChannelID())
assert.True(t, ok, "Could not find system channel")

chainSupport.Enqueue(wrapped)

it, _ := rl.Iterator(&ab.SeekPosition{Type: &ab.SeekPosition_Specified{Specified: &ab.SeekSpecified{Number: 1}}})
Expand Down Expand Up @@ -300,6 +305,7 @@ func TestNewChain(t *testing.T) {
if status != cb.Status_SUCCESS {
t.Fatalf("Could not retrieve new chain genesis block")
}
testLastConfigBlockNumber(t, block, expectedLastConfigBlockNumber)
if len(block.Data.Data) != 1 {
t.Fatalf("Should have had only one message in the new genesis block")
}
Expand All @@ -315,6 +321,7 @@ func TestNewChain(t *testing.T) {
if status != cb.Status_SUCCESS {
t.Fatalf("Could not retrieve block on new chain")
}
testLastConfigBlockNumber(t, block, expectedLastConfigBlockNumber)
for i := 0; i < int(conf.Orderer.BatchSize.MaxMessageCount); i++ {
if !reflect.DeepEqual(utils.ExtractEnvelopeOrPanic(block, i), messages[i]) {
t.Errorf("Block contents wrong at index %d in new chain", i)
Expand All @@ -324,3 +331,13 @@ func TestNewChain(t *testing.T) {
t.Fatalf("Block 1 not produced after timeout on new chain")
}
}

func testLastConfigBlockNumber(t *testing.T, block *cb.Block, expectedBlockNumber uint64) {
metadataItem := &cb.Metadata{}
err := proto.Unmarshal(block.Metadata.Metadata[cb.BlockMetadataIndex_LAST_CONFIG], metadataItem)
assert.NoError(t, err, "Block should carry LAST_CONFIG metadata item")
lastConfig := &cb.LastConfig{}
err = proto.Unmarshal(metadataItem.Value, lastConfig)
assert.NoError(t, err, "LAST_CONFIG metadata item should carry last config value")
assert.Equal(t, expectedBlockNumber, lastConfig.Index, "LAST_CONFIG value should point to last config block")
}

0 comments on commit db236d6

Please sign in to comment.