From 102c926ef3779ffab1e37cd454e3eab4e43ff98d Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Sun, 22 Sep 2019 20:06:52 +0300 Subject: [PATCH 1/4] core: don't print useless persist messages with nil errors Errors should be printed only when there are errors. --- pkg/core/blockchain.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pkg/core/blockchain.go b/pkg/core/blockchain.go index 4fc630860..93991ff15 100644 --- a/pkg/core/blockchain.go +++ b/pkg/core/blockchain.go @@ -169,7 +169,9 @@ func (bc *Blockchain) Run(ctx context.Context) { case <-persistTimer.C: go func() { err := bc.Persist(ctx) - log.Warnf("failed to persist blockchain: %s", err) + if err != nil { + log.Warnf("failed to persist blockchain: %s", err) + } }() persistTimer.Reset(persistInterval) } From 4b4bac675b930e9d12efc9ac7d069910a6d41582 Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Sun, 22 Sep 2019 20:08:15 +0300 Subject: [PATCH 2/4] core: don't print persisting errors twice As they're already printed in the calling goroutine. And they're alse not always useful when printed by Persist(). --- pkg/core/blockchain.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/core/blockchain.go b/pkg/core/blockchain.go index 93991ff15..068401b03 100644 --- a/pkg/core/blockchain.go +++ b/pkg/core/blockchain.go @@ -414,7 +414,6 @@ func (bc *Blockchain) Persist(ctx context.Context) (err error) { hash := headerList.Get(int(bc.BlockHeight() + 1)) if block, ok := bc.blockCache.GetBlock(hash); ok { if err = bc.persistBlock(block); err != nil { - log.Warnf("failed to persist blocks: %s", err) return } bc.blockCache.Delete(hash) From 5a0f08f2c0ae33dc1f11e564a4895c2cde117223 Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Sun, 22 Sep 2019 20:09:55 +0300 Subject: [PATCH 3/4] network: fix SIGSEGV on unknown message acceptance For example, at the moment our node can't handle `consensus` message, so when it received it before the patch it just crashed because of uninitialized `p`. --- pkg/network/message.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pkg/network/message.go b/pkg/network/message.go index 1f702fc9f..187c7c9d9 100644 --- a/pkg/network/message.go +++ b/pkg/network/message.go @@ -3,6 +3,7 @@ package network import ( "encoding/binary" "errors" + "fmt" "github.com/CityOfZion/neo-go/config" "github.com/CityOfZion/neo-go/pkg/core" @@ -193,6 +194,8 @@ func (m *Message) decodePayload(br *io.BinReader) error { p = &transaction.Transaction{} case CMDMerkleBlock: p = &payload.MerkleBlock{} + default: + return fmt.Errorf("can't decode command %s", cmdByteArrayToString(m.Command)) } p.DecodeBinary(r) if r.Err != nil { From ac5d2f94d3413143a8d0a46742de8ce9eeeb1b2c Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Sun, 22 Sep 2019 20:11:34 +0300 Subject: [PATCH 4/4] storage: fix BoltDB batched Put() It must copy both the value and the key because they can be reused for other purposes between Put() and PutBatch(). This actually happens with values in headers processing, leading to wrong data being written into the DB. Extend the batch test to check for that. --- pkg/core/storage/boltdb_store.go | 6 +++++- pkg/core/storage/boltdb_store_test.go | 10 ++++++++-- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/pkg/core/storage/boltdb_store.go b/pkg/core/storage/boltdb_store.go index e63ec95e0..ffa1d4ecb 100644 --- a/pkg/core/storage/boltdb_store.go +++ b/pkg/core/storage/boltdb_store.go @@ -36,7 +36,11 @@ func (b *BoltDBBatch) Len() int { // Put implements the Batch interface. func (b *BoltDBBatch) Put(k, v []byte) { - b.mem[&k] = v + vcopy := make([]byte, len(v)) + copy(vcopy, v) + kcopy := make([]byte, len(k)) + copy(kcopy, k) + b.mem[&kcopy] = vcopy } // NewBoltDBStore returns a new ready to use BoltDB storage with created bucket. diff --git a/pkg/core/storage/boltdb_store_test.go b/pkg/core/storage/boltdb_store_test.go index 51128b749..ac23d136a 100644 --- a/pkg/core/storage/boltdb_store_test.go +++ b/pkg/core/storage/boltdb_store_test.go @@ -26,11 +26,17 @@ func TestBoltDBBatch_Len(t *testing.T) { func TestBoltDBBatch_PutBatchAndGet(t *testing.T) { key := []byte("foo") + keycopy := make([]byte, len(key)) + copy(keycopy, key) value := []byte("bar") - batch := &BoltDBBatch{mem: map[*[]byte][]byte{&key: value}} - + valuecopy := make([]byte, len(value)) + copy(valuecopy, value) boltDBStore := openStore(t) + batch := boltDBStore.Batch() + batch.Put(keycopy, valuecopy) + copy(valuecopy, key) + copy(keycopy, value) errPut := boltDBStore.PutBatch(batch) assert.Nil(t, errPut, "Error while PutBatch")