Skip to content

Commit

Permalink
Logging and error cleanup
Browse files Browse the repository at this point in the history 
Improved logging
Adjusting errors for go vet
Adding some error checks
  • Loading branch information
@ineiti
ineiti committed Jan 16, 2019
1 parent 3af2fef commit 95d6bc0
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 25 deletions.
2 changes: 1 addition & 1 deletion byzcoin/proto.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ type DownloadState struct {
// ByzCoinID of the state to download
ByzCoinID skipchain.SkipBlockID
// Nonce is 0 for a new download, else it must be
// equal to the nonce returned in DDownloadStateResponse.
// equal to the nonce returned in DownloadStateResponse.
// In case Nonce is non-zero, but doesn't correspond
// to the current session, an error is returned,
// as only one download-session can be active at
Expand Down
36 changes: 23 additions & 13 deletions byzcoin/service.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -353,6 +353,9 @@ func (s *Service) GetProof(req *GetProof) (resp *GetProofResponse, err error) {
if req.Version != CurrentVersion {
return nil, errors.New("version mismatch")
}

log.Lvlf2("Returning proof for %x from chain '%x'", req.Key, req.ID)

sb := s.db().GetByID(req.ID)
if sb == nil {
err = errors.New("cannot find skipblock while getting proof")
Expand All @@ -373,6 +376,8 @@ func (s *Service) GetProof(req *GetProof) (resp *GetProofResponse, err error) {
return
}

_, v := proof.InclusionProof.KeyValue()
log.Lvlf3("value is %x", v)
resp = &GetProofResponse{
Version: CurrentVersion,
Proof: *proof,
Expand Down Expand Up @@ -1292,7 +1297,7 @@ func (s *Service) startPolling(scID skipchain.SkipBlockID) chan bool {
" This function should never be called on a skipchain that does not exist.")
}

log.Lvlf2("%s: Starting new block %d for chain %x", s.ServerIdentity(), latest.Index+1, scID)
log.Lvlf3("%s: Starting new block %d for chain %x", s.ServerIdentity(), latest.Index+1, scID)
tree := bcConfig.Roster.GenerateNaryTree(len(bcConfig.Roster.List))

proto, err := s.CreateProtocol(collectTxProtocol, tree)
Expand Down Expand Up @@ -1557,7 +1562,7 @@ func (s *Service) createStateChanges(sst *stagingStateTrie, scID skipchain.SkipB
var err error
merkleRoot, txOut, states, err = s.stateChangeCache.get(scID, txIn.Hash())
if err == nil {
log.Lvl3(s.ServerIdentity(), "loaded state changes from cache")
log.Lvlf3("%s: loaded state changes %x from cache", s.ServerIdentity(), scID)
return
}
log.Lvl3(s.ServerIdentity(), "state changes from cache: MISS")
Expand Down Expand Up @@ -1585,7 +1590,12 @@ clientTransactions:
for _, instr := range tx.ClientTransaction.Instructions {
scs, cout, err := s.executeInstruction(sstTempC, cin, instr, h)
if err != nil {
log.Errorf("%s Call to contract returned error: %s", s.ServerIdentity(), err)
_, _, cid, _, err2 := sstTempC.GetValues(instr.InstanceID.Slice())
if err2 != nil {
err = fmt.Errorf("%s - while getting value: %s", err, err2)
}
log.Errorf("%s Contract %s got Instruction %s and returned error: %s", s.ServerIdentity(),
cid, instr, err)
tx.Accepted = false
txOut = append(txOut, tx)
continue clientTransactions
Expand Down Expand Up @@ -1691,7 +1701,7 @@ clientTransactions:
func (s *Service) executeInstruction(st ReadOnlyStateTrie, cin []Coin, instr Instruction, ctxHash []byte) (scs StateChanges, cout []Coin, err error) {
defer func() {
if re := recover(); re != nil {
err = errors.New(re.(string))
err = fmt.Errorf("%s", re)
}
}()

Expand All @@ -1711,7 +1721,7 @@ func (s *Service) executeInstruction(st ReadOnlyStateTrie, cin []Coin, instr Ins
// If the leader does not have a verifier for this contract, it drops the
// transaction.
if !exists {
err = fmt.Errorf("Leader is dropping instruction of unknown contract \"%s\" on instance \"%x\"", contractID, instr.InstanceID.Slice())
err = fmt.Errorf("leader is dropping instruction of unknown contract \"%s\" on instance \"%x\"", contractID, instr.InstanceID.Slice())
return
}
// Now we call the contract function with the data of the key.
Expand All @@ -1722,7 +1732,7 @@ func (s *Service) executeInstruction(st ReadOnlyStateTrie, cin []Coin, instr Ins
return nil, nil, err
}
if c == nil {
return nil, nil, errors.New("conrtact factory returned nil contract instance")
return nil, nil, errors.New("contract factory returned nil contract instance")
}

err = c.VerifyInstruction(st, instr, ctxHash)
Expand Down Expand Up @@ -1811,7 +1821,6 @@ func (s *Service) getTxs(leader *network.ServerIdentity, roster *onet.Roster, sc
// failure).
ourLatest, err := s.db().GetLatestByID(scID)
if err != nil {
log.Warn(s.ServerIdentity(), "we do not know about the skipchain ID")
return []ClientTransaction{}
}
latestSB := s.db().GetByID(latestID)
Expand Down Expand Up @@ -1893,9 +1902,10 @@ func (s *Service) monitorLeaderFailure() {
gen := []byte(key)
latest, err := s.db().GetLatestByID(gen)
if err != nil {
panic("heartbeat monitors are started after " +
"the creation of the genesis block, " +
"so the block should always exist")
log.Error("heartbeat monitors are started after "+
"the creation of the genesis block, "+
"so the block should always exist: ", err)
s.heartbeats.stop(key)
}
req := viewchange.InitReq{
SignerID: s.ServerIdentity().ID,
Expand Down Expand Up @@ -1928,7 +1938,7 @@ func (s *Service) startAllChains() error {
s.closedMutex.Lock()
defer s.closedMutex.Unlock()
if !s.closed {
return errors.New("Can only call startAllChains if the service has been closed before")
return errors.New("can only call startAllChains if the service has been closed before")
}
s.SetPropagationTimeout(120 * time.Second)
msg, err := s.Load(storageID)
Expand All @@ -1939,7 +1949,7 @@ func (s *Service) startAllChains() error {
var ok bool
s.storage, ok = msg.(*bcStorage)
if !ok {
return errors.New("Data of wrong type")
return errors.New("data of wrong type")
}
}
s.stateTries = make(map[string]*stateTrie)
Expand Down Expand Up @@ -2095,7 +2105,7 @@ func (s *Service) trySyncAll() {
req := &skipchain.GetSingleBlockByIndex{Genesis: sb.SkipChainID(), Index: index}
lksb, err := s.skService().GetSingleBlockByIndex(req)
if lksb != nil {
log.Lvlf2("Start creating state changes for skipchain %x", sb.SkipChainID())
log.Lvlf2("Starting to create state changes for skipchain %x", sb.SkipChainID())
_, err = s.buildStateChanges(lksb.SkipBlock.Hash, sst, []Coin{})
if err != nil {
log.Error(s.ServerIdentity(), err)
Expand Down
29 changes: 18 additions & 11 deletions skipchain/struct.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,7 @@ func (sbf *SkipBlockFix) CalculateHash() SkipBlockID {
hash := sha256.New()
for _, i := range []int{sbf.Index, sbf.Height, sbf.MaximumHeight,
sbf.BaseHeight} {
binary.Write(hash, binary.LittleEndian, i)
_ = binary.Write(hash, binary.LittleEndian, i)
}
for _, bl := range sbf.BackLinkIDs {
hash.Write(bl)
Expand All @@ -330,7 +330,10 @@ func (sbf *SkipBlockFix) CalculateHash() SkipBlockID {
hash.Write(sbf.Data)
if sbf.Roster != nil {
for _, pub := range sbf.Roster.Publics() {
pub.MarshalTo(hash)
_, err := pub.MarshalTo(hash)
if err != nil {
panic("couldn't marshall point to hash: " + err.Error())
}
}
}
buf := hash.Sum(nil)
Expand Down Expand Up @@ -646,7 +649,7 @@ func NewSkipBlockDB(db *bolt.DB, bn []byte) *SkipBlockDB {
// GetStatus is a function that returns the status report of the db.
func (db *SkipBlockDB) GetStatus() *onet.Status {
out := make(map[string]string)
db.DB.View(func(tx *bolt.Tx) error {
err := db.DB.View(func(tx *bolt.Tx) error {
b := tx.Bucket([]byte(db.bucketName))
s := b.Stats()
out["Blocks"] = strconv.Itoa(s.KeyN)
Expand All @@ -655,6 +658,10 @@ func (db *SkipBlockDB) GetStatus() *onet.Status {
out["Bytes"] = strconv.Itoa(total)
return nil
})
if err != nil {
log.Error(err)
return nil
}
return &onet.Status{Field: out}
}

Expand Down Expand Up @@ -806,7 +813,7 @@ func (db *SkipBlockDB) latestUpdate(sb *SkipBlock) {
// Length returns the actual length using mutexes
func (db *SkipBlockDB) Length() int {
var i int
db.View(func(tx *bolt.Tx) error {
_ = db.View(func(tx *bolt.Tx) error {
b := tx.Bucket([]byte(db.bucketName))
i = b.Stats().KeyN
return nil
Expand All @@ -830,16 +837,16 @@ func (db *SkipBlockDB) GetResponsible(sb *SkipBlock) (*SkipBlock, error) {
}
ret := db.GetByID(sb.ParentBlockID)
if ret == nil {
return nil, errors.New("No Roster and no parent")
return nil, errors.New("no Roster and no parent")
}
return ret, nil
}
if len(sb.BackLinkIDs) == 0 {
return nil, errors.New("Invalid block: no backlink")
return nil, errors.New("invalid block: no backlink")
}
prev := db.GetByID(sb.BackLinkIDs[0])
if prev == nil {
return nil, errors.New("Didn't find responsible")
return nil, errors.New("didn't find responsible")
}
return prev, nil
}
Expand All @@ -859,7 +866,7 @@ func (db *SkipBlockDB) VerifyLinks(sb *SkipBlock) error {
if !sb.ParentBlockID.IsNull() {
parent := db.GetByID(sb.ParentBlockID)
if parent == nil {
return errors.New("Didn't find parent")
return errors.New("didn't find parent")
}
if err := parent.VerifyForwardSignatures(); err != nil {
return err
Expand Down Expand Up @@ -888,7 +895,7 @@ func (db *SkipBlockDB) VerifyLinks(sb *SkipBlock) error {
log.Lvl3("Didn't find back-link, but have a good forward-link")
return nil
}
return errors.New("Didn't find height-0 skipblock in db")
return errors.New("didn't find height-0 skipblock in db")
}
if err := sbBack.VerifyForwardSignatures(); err != nil {
return err
Expand Down Expand Up @@ -955,7 +962,7 @@ func (db *SkipBlockDB) GetFuzzy(id string) (*SkipBlock, error) {
}

var sb *SkipBlock
db.View(func(tx *bolt.Tx) error {
err = db.View(func(tx *bolt.Tx) error {
c := tx.Bucket([]byte(db.bucketName)).Cursor()
for k, v := c.First(); k != nil; k, v = c.Next() {
if bytes.HasPrefix(k, match) {
Expand All @@ -979,7 +986,7 @@ func (db *SkipBlockDB) GetFuzzy(id string) (*SkipBlock, error) {
}
return nil
})
return sb, nil
return sb, err
}

// GetProof returns the shortest chain from the genesis to the latest block
Expand Down

0 comments on commit 95d6bc0

Please sign in to comment.