diff --git a/blockchain/skipchain/block.go b/blockchain/skipchain/block.go index b398764e6..257390661 100644 --- a/blockchain/skipchain/block.go +++ b/blockchain/skipchain/block.go @@ -253,7 +253,7 @@ func (f blockFactory) FromVerifiable(src proto.Message) (blockchain.Block, error return nil, xerrors.Errorf("couldn't get the chain factory: %v", err) } - // Integrity of the chain is verifier during decoding. + // Integrity of the chain is verified during decoding. chain, err := chainFactory.FromProto(in.GetChain()) if err != nil { return nil, xerrors.Errorf("couldn't decode the chain: %v", err) diff --git a/consensus/cosipbft/forward_link.go b/consensus/cosipbft/forward_link.go index b8b62b48f..fd881bd16 100644 --- a/consensus/cosipbft/forward_link.go +++ b/consensus/cosipbft/forward_link.go @@ -87,15 +87,15 @@ func (fl forwardLink) Pack(enc encoding.ProtoMarshaler) (proto.Message, error) { } func (fl forwardLink) packChangeSet(enc encoding.ProtoMarshaler) (*ChangeSet, error) { + add, err := fl.packPlayers(fl.changeset.Add, enc) + if err != nil { + return nil, xerrors.Errorf("couldn't pack players: %v", err) + } + pb := &ChangeSet{ Leader: fl.changeset.Leader, Remove: fl.changeset.Remove, - } - - var err error - pb.Add, err = fl.packPlayers(fl.changeset.Add, enc) - if err != nil { - return nil, xerrors.Errorf("couldn't pack players: %v", err) + Add: add, } return pb, nil @@ -156,6 +156,8 @@ func (fl forwardLink) computeHash(h hash.Hash, enc encoding.ProtoMarshaler) ([]b } } + // This buffer will store all the encoded integers where each one is using 4 + // bytes: leader index + removal indices. buffer := make([]byte, 4+(4*len(fl.changeset.Remove))) binary.LittleEndian.PutUint32(buffer, fl.changeset.Leader) @@ -246,17 +248,17 @@ func (f *unsecureChainFactory) decodeForwardLink(pb proto.Message) (forwardLink, return fl, xerrors.Errorf("unknown message type: %T", msg) } - fl = forwardLink{ - from: src.GetFrom(), - to: src.GetTo(), - } - - var err error - fl.changeset, err = f.decodeChangeSet(src.GetChangeSet()) + changeset, err := f.decodeChangeSet(src.GetChangeSet()) if err != nil { return fl, xerrors.Errorf("couldn't decode changeset: %v", err) } + fl = forwardLink{ + from: src.GetFrom(), + to: src.GetTo(), + changeset: changeset, + } + if src.GetPrepare() != nil { sig, err := f.signatureFactory.FromProto(src.GetPrepare()) if err != nil { @@ -286,15 +288,15 @@ func (f *unsecureChainFactory) decodeForwardLink(pb proto.Message) (forwardLink, } func (f *unsecureChainFactory) decodeChangeSet(in *ChangeSet) (viewchange.ChangeSet, error) { + add, err := f.decodeAdd(in.GetAdd()) + if err != nil { + return viewchange.ChangeSet{}, xerrors.Errorf("couldn't decode add: %v", err) + } + changeset := viewchange.ChangeSet{ Leader: in.GetLeader(), Remove: in.GetRemove(), - } - - var err error - changeset.Add, err = f.decodeAdd(in.GetAdd()) - if err != nil { - return changeset, xerrors.Errorf("couldn't decode add: %v", err) + Add: add, } return changeset, nil @@ -373,6 +375,8 @@ func newChainFactory(cosi cosi.CollectiveSigning, m mino.Mino, func (f *chainFactory) FromProto(pb proto.Message) (consensus.Chain, error) { chain, err := f.unsecureChainFactory.FromProto(pb) if err != nil { + // No wrapping to avoid redundancy in the error message. It will point + // anyway to a meaningful line. return nil, err } diff --git a/consensus/cosipbft/mod.go b/consensus/cosipbft/mod.go index 060450feb..d55743707 100644 --- a/consensus/cosipbft/mod.go +++ b/consensus/cosipbft/mod.go @@ -130,9 +130,9 @@ func (a pbftActor) Propose(p consensus.Proposal) error { leader, ok := a.viewchange.Wait(p, authority) if !ok { fabric.Logger.Trace().Msg("proposal skipped by view change") - // Not authorized to propose a block as the leader is moving - // forward so we drop the proposal. The upper layer is responsible to - // try again until the leader includes the data. + // Not authorized to propose a block as the leader is moving forward so + // we drop the proposal. The upper layer is responsible for trying again + // until the leader includes the data. return nil } @@ -186,7 +186,9 @@ func (a pbftActor) Propose(p consensus.Proposal) error { return nil } -func (a pbftActor) newPrepareRequest(prop consensus.Proposal, cs viewchange.ChangeSet) (Prepare, error) { +func (a pbftActor) newPrepareRequest(prop consensus.Proposal, + cs viewchange.ChangeSet) (Prepare, error) { + req := Prepare{proposal: prop} forwardLink := forwardLink{ @@ -195,12 +197,13 @@ func (a pbftActor) newPrepareRequest(prop consensus.Proposal, cs viewchange.Chan changeset: cs, } - var err error - req.digest, err = forwardLink.computeHash(a.hashFactory.New(), a.encoder) + digest, err := forwardLink.computeHash(a.hashFactory.New(), a.encoder) if err != nil { return req, xerrors.Errorf("couldn't compute hash: %v", err) } + req.digest = digest + return req, nil } diff --git a/consensus/viewchange/constant/mod.go b/consensus/viewchange/constant/mod.go index 57d6368ea..292f49360 100644 --- a/consensus/viewchange/constant/mod.go +++ b/consensus/viewchange/constant/mod.go @@ -21,8 +21,8 @@ func NewViewChange(addr mino.Address) ViewChange { } } -// Wait implements viewchange.ViewChange. It true if the first player of the -// authority is the current participant. +// Wait implements viewchange.ViewChange. It returns true if the first player of +// the authority is the current participant. func (vc ViewChange) Wait(p consensus.Proposal, a crypto.CollectiveAuthority) (uint32, bool) { leader := a.AddressIterator().GetNext() diff --git a/consensus/viewchange/mod.go b/consensus/viewchange/mod.go index 95ac5e9d0..159e634cc 100644 --- a/consensus/viewchange/mod.go +++ b/consensus/viewchange/mod.go @@ -7,7 +7,7 @@ import ( ) // ViewChange provides primitives to verify if a participant is allowed to -// propose a block as the leader. Some consensus needs a single node to propose +// propose a block as the leader. Some consensus need a single node to propose // and the others as backups when it is failing. The index returned announces // who is allowed to be the leader. type ViewChange interface { @@ -45,11 +45,12 @@ type EvolvableAuthority interface { // Governance is an interface to get information about the collective authority // of a proposal. type Governance interface { - // GetAuthority must return the authority that governs the block. It will be - // used to sign the forward link to the next proposal. + // GetAuthority must return the authority that governs the proposal at the + // given index. It will be used to sign the forward link to the next + // proposal. GetAuthority(index uint64) (EvolvableAuthority, error) // GetChangeSet must return the changes to the authority that will be - // applied for the next proposal. + // applied for the proposal following the given index. GetChangeSet(index uint64) ChangeSet } diff --git a/cosi/mod.go b/cosi/mod.go index b3e88176a..ecaca080e 100644 --- a/cosi/mod.go +++ b/cosi/mod.go @@ -43,8 +43,8 @@ type CollectiveSigning interface { // GetSignatureFactory returns the signature factory. GetSignatureFactory() crypto.SignatureFactory - // GetVerifier returns a verifier that can verify the signature created from - // a collective signing. + // GetVerifierFactory returns a factory that can create a verifier to check + // the validity of a signature. GetVerifierFactory() crypto.VerifierFactory // Listen starts the collective signing so that it will answer to requests. diff --git a/docs/introduction.md b/docs/introduction.md index cf0caceae..0edf1156e 100644 --- a/docs/introduction.md +++ b/docs/introduction.md @@ -29,6 +29,9 @@ definitions of modular pieces that build a blockchain. algorithm that can be used to verify the integrity of some data. One example is the inventory page integrity to prove which instances are stored. +- **governance** - Governance is a black box that provides to act on the + participants of a consensus. + - **instance** - An instance is the smallest unit of storage in a ledger. It is identified by a unique key and stores a generic piece of data. diff --git a/ledger/byzcoin/mod_test.go b/ledger/byzcoin/mod_test.go index 7e2a56ead..6818698e8 100644 --- a/ledger/byzcoin/mod_test.go +++ b/ledger/byzcoin/mod_test.go @@ -53,7 +53,7 @@ func TestLedger_Basic(t *testing.T) { for _, actor := range actors { err := <-actor.HasStarted() - require.Nil(t, err) + require.NoError(t, err) } ctx, cancel := context.WithCancel(context.Background()) diff --git a/ledger/byzcoin/roster.go b/ledger/byzcoin/roster.go index 6f62f23d1..ce778d29c 100644 --- a/ledger/byzcoin/roster.go +++ b/ledger/byzcoin/roster.go @@ -17,15 +17,13 @@ type iterator struct { } func (i *iterator) HasNext() bool { - return i.index+1 < i.roster.Len() + return i.index < i.roster.Len() } func (i *iterator) GetNext() int { - if i.HasNext() { - i.index++ - return i.index - } - return -1 + res := i.index + i.index++ + return res } // addressIterator is an iterator for a list of addresses. @@ -37,9 +35,8 @@ type addressIterator struct { // GetNext implements mino.AddressIterator. It returns the next address. func (i *addressIterator) GetNext() mino.Address { - index := i.iterator.GetNext() - if index >= 0 { - return i.roster.addrs[index] + if i.iterator.HasNext() { + return i.roster.addrs[i.iterator.GetNext()] } return nil } @@ -53,9 +50,8 @@ type publicKeyIterator struct { // GetNext implements crypto.PublicKeyIterator. It returns the next public key. func (i *publicKeyIterator) GetNext() crypto.PublicKey { - index := i.iterator.GetNext() - if index >= 0 { - return i.roster.pubkeys[index] + if i.iterator.HasNext() { + return i.roster.pubkeys[i.iterator.GetNext()] } return nil } @@ -116,13 +112,13 @@ func (r roster) GetPublicKey(target mino.Address) (crypto.PublicKey, int) { // AddressIterator implements mino.Players. It returns an iterator of the // addresses of the roster in a deterministic order. func (r roster) AddressIterator() mino.AddressIterator { - return &addressIterator{iterator: &iterator{roster: &r, index: -1}} + return &addressIterator{iterator: &iterator{roster: &r}} } // PublicKeyIterator implements crypto.CollectiveAuthority. It returns an // iterator of the public keys of the roster in a deterministic order. func (r roster) PublicKeyIterator() crypto.PublicKeyIterator { - return &publicKeyIterator{iterator: &iterator{roster: &r, index: -1}} + return &publicKeyIterator{iterator: &iterator{roster: &r}} } // Pack implements encoding.Packable. It returns the protobuf message for the @@ -170,11 +166,9 @@ func (f rosterFactory) New(authority crypto.CollectiveAuthority) roster { addrIter := authority.AddressIterator() pubkeyIter := authority.PublicKeyIterator() - index := 0 - for addrIter.HasNext() && pubkeyIter.HasNext() { - roster.addrs[index] = addrIter.GetNext() - roster.pubkeys[index] = pubkeyIter.GetNext() - index++ + for i := 0; addrIter.HasNext() && pubkeyIter.HasNext(); i++ { + roster.addrs[i] = addrIter.GetNext() + roster.pubkeys[i] = pubkeyIter.GetNext() } return roster @@ -194,19 +188,23 @@ func (f rosterFactory) FromProto(in proto.Message) (viewchange.EvolvableAuthorit len(pb.Addresses), len(pb.PublicKeys)) } - roster := roster{ - addrs: make([]mino.Address, len(pb.Addresses)), - pubkeys: make([]crypto.PublicKey, len(pb.PublicKeys)), - } + addrs := make([]mino.Address, len(pb.Addresses)) + pubkeys := make([]crypto.PublicKey, len(pb.PublicKeys)) - var err error for i, addrpb := range pb.GetAddresses() { - roster.addrs[i] = f.addressFactory.FromText(addrpb) + addrs[i] = f.addressFactory.FromText(addrpb) - roster.pubkeys[i], err = f.pubkeyFactory.FromProto(pb.GetPublicKeys()[i]) + pubkey, err := f.pubkeyFactory.FromProto(pb.GetPublicKeys()[i]) if err != nil { return nil, xerrors.Errorf("couldn't decode public key: %v", err) } + + pubkeys[i] = pubkey + } + + roster := roster{ + addrs: addrs, + pubkeys: pubkeys, } return roster, nil diff --git a/ledger/byzcoin/roster_test.go b/ledger/byzcoin/roster_test.go index 0b4f683f7..3794f8d6c 100644 --- a/ledger/byzcoin/roster_test.go +++ b/ledger/byzcoin/roster_test.go @@ -12,18 +12,17 @@ import ( func TestIterator_HasNext(t *testing.T) { iter := &iterator{ roster: &roster{addrs: make([]mino.Address, 3)}, - index: -1, } require.True(t, iter.HasNext()) - iter.index = 0 - require.True(t, iter.HasNext()) - iter.index = 1 require.True(t, iter.HasNext()) iter.index = 2 + require.True(t, iter.HasNext()) + + iter.index = 3 require.False(t, iter.HasNext()) iter.index = 10 @@ -33,7 +32,6 @@ func TestIterator_HasNext(t *testing.T) { func TestIterator_GetNext(t *testing.T) { iter := &iterator{ roster: &roster{addrs: make([]mino.Address, 3)}, - index: -1, } for i := 0; i < 3; i++ { @@ -41,7 +39,7 @@ func TestIterator_GetNext(t *testing.T) { require.NotNil(t, c) } - require.Equal(t, -1, iter.GetNext()) + require.Equal(t, 3, iter.GetNext()) } func TestAddressIterator_GetNext(t *testing.T) { @@ -49,7 +47,6 @@ func TestAddressIterator_GetNext(t *testing.T) { iter := &addressIterator{ iterator: &iterator{ roster: &roster, - index: -1, }, } @@ -66,7 +63,6 @@ func TestPublicKeyIterator_GetNext(t *testing.T) { iter := &publicKeyIterator{ iterator: &iterator{ roster: &roster, - index: -1, }, }