From e0dd7a06a8c40deca6a414b5b37645106d9c10d5 Mon Sep 17 00:00:00 2001 From: goshawk Date: Thu, 28 Jul 2022 18:13:17 +0300 Subject: [PATCH 1/3] Verify certificate as precondition for re-broadcast --- pkg/core/chain/chain.go | 44 +++++++++-------------------------------- 1 file changed, 9 insertions(+), 35 deletions(-) diff --git a/pkg/core/chain/chain.go b/pkg/core/chain/chain.go index 2c0abebae..a1dc4f1b7 100644 --- a/pkg/core/chain/chain.go +++ b/pkg/core/chain/chain.go @@ -388,8 +388,12 @@ func (c *Chain) ProcessSyncTimerExpired(strPeerAddr string) error { func (c *Chain) acceptSuccessiveBlock(blk block.Block, kadcastHeight byte) error { log.WithField("height", blk.Header.Height).Trace("accepting succeeding block") - // TODO: Verify Certificate - if err := c.propagateBlock(blk, kadcastHeight); err != nil { + if err := c.isValidBlock(blk, *c.tip, *c.p, log, true); err != nil { + log.WithError(err).Error("invalid block") + return err + } + + if err := c.kadcastBlock(blk, kadcastHeight); err != nil { log.WithError(err).Error("block propagation failed") return err } @@ -731,40 +735,10 @@ func (c *Chain) ExecuteStateTransition(ctx context.Context, txs []transactions.C return c.proxy.Executor().ExecuteStateTransition(c.ctx, txs, blockGasLimit, blockHeight, generator) } -// propagateBlock send inventory message to all peers in gossip network or rebroadcast block in kadcast network. -func (c *Chain) propagateBlock(b block.Block, kadcastHeight byte) error { - // Disable gossiping messages if kadcast mode - if config.Get().Kadcast.Enabled { - log.WithField("blk_height", b.Header.Height). - WithField("kadcast_h", kadcastHeight).Trace("propagate block") - return c.kadcastBlock(b, kadcastHeight) - } - - log.WithField("blk_height", b.Header.Height).Trace("propagate block") - - msg := &message.Inv{} - - msg.AddItem(message.InvTypeBlock, b.Header.Hash) - - buf := new(bytes.Buffer) - if err := msg.Encode(buf); err != nil { - // TODO: shall this really panic ? - log.Panic(err) - } - - if err := topics.Prepend(buf, topics.Inv); err != nil { - // TODO: shall this really panic ? - log.Panic(err) - } - - m := message.New(topics.Inv, *buf) - errList := c.eventBus.Publish(topics.Gossip, m) - - diagnostics.LogPublishErrors("chain/chain.go, topics.Gossip, topics.Inv", errList) - return nil -} - func (c *Chain) kadcastBlock(blk block.Block, kadcastHeight byte) error { + log.WithField("blk_height", blk.Header.Height). + WithField("kadcast_h", kadcastHeight).Trace("propagate block") + buf := new(bytes.Buffer) if err := message.MarshalBlock(buf, &blk); err != nil { return err From 26c90910414f5c05e0298ad6ace9ace7ed382fae Mon Sep 17 00:00:00 2001 From: goshawk Date: Mon, 1 Aug 2022 10:26:25 +0300 Subject: [PATCH 2/3] Rename isValidBlock to isValidHeader --- pkg/core/chain/chain.go | 12 ++++++------ pkg/core/chain/fallback.go | 4 ++-- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/pkg/core/chain/chain.go b/pkg/core/chain/chain.go index a1dc4f1b7..0f523b888 100644 --- a/pkg/core/chain/chain.go +++ b/pkg/core/chain/chain.go @@ -360,7 +360,7 @@ func (c *Chain) TryNextConsecutiveBlockIsValid(blk block.Block) error { l := log.WithFields(fields) - return c.isValidBlock(blk, *c.tip, *c.p, l, true) + return c.isValidHeader(blk, *c.tip, *c.p, l, true) } // ProcessSyncTimerExpired called by outsync timer when a peer does not provide GetData response. @@ -388,7 +388,7 @@ func (c *Chain) ProcessSyncTimerExpired(strPeerAddr string) error { func (c *Chain) acceptSuccessiveBlock(blk block.Block, kadcastHeight byte) error { log.WithField("height", blk.Header.Height).Trace("accepting succeeding block") - if err := c.isValidBlock(blk, *c.tip, *c.p, log, true); err != nil { + if err := c.isValidHeader(blk, *c.tip, *c.p, log, true); err != nil { log.WithError(err).Error("invalid block") return err } @@ -536,12 +536,12 @@ func (c *Chain) sanityCheckStateHash() error { return nil } -func (c *Chain) isValidBlock(newBlock, prevBlock block.Block, provisioners user.Provisioners, l *logrus.Entry, withSanityCheck bool) error { - l.Debug("verifying block") +func (c *Chain) isValidHeader(newBlock, prevBlock block.Block, provisioners user.Provisioners, l *logrus.Entry, withSanityCheck bool) error { + l.Debug("verifying block header") // Check that stateless and stateful checks pass if withSanityCheck { if err := c.verifier.SanityCheckBlock(prevBlock, newBlock); err != nil { - l.WithError(err).Error("block verification failed") + l.WithError(err).Error("block header verification failed") return err } } @@ -579,7 +579,7 @@ func (c *Chain) acceptBlock(blk block.Block, withSanityCheck bool) error { var err error // 1. Ensure block fields and certificate are valid - if err = c.isValidBlock(blk, *c.tip, *c.p, l, withSanityCheck); err != nil { + if err = c.isValidHeader(blk, *c.tip, *c.p, l, withSanityCheck); err != nil { l.WithError(err).Error("invalid block error") return err } diff --git a/pkg/core/chain/fallback.go b/pkg/core/chain/fallback.go index 4168f8453..2832e5cc8 100644 --- a/pkg/core/chain/fallback.go +++ b/pkg/core/chain/fallback.go @@ -35,7 +35,7 @@ func (c *Chain) allowFallback(b block.Block, l *logrus.Entry) error { // Ensure block fields and certificate are valid against previous block and // current provisioners set. - if err = c.isValidBlock(b, prevBlk, *c.p, l, true); err != nil { + if err = c.isValidHeader(b, prevBlk, *c.p, l, true); err != nil { return err } @@ -231,7 +231,7 @@ func (c *Chain) isBlockFromFork(b block.Block) (bool, error) { // A weak assumption is made here that provisioners state has not changed // since recvBlk.Header.Height - err = c.isValidBlock(b, *pb, *c.p, log, true) + err = c.isValidHeader(b, *pb, *c.p, log, true) if err != nil { return false, err } From a9dfd459f0bfb92927b46aa6e90dad72af283a33 Mon Sep 17 00:00:00 2001 From: goshawk Date: Mon, 1 Aug 2022 10:44:51 +0300 Subject: [PATCH 3/3] Remove the obsolete TestAcceptFromPeer unit test --- pkg/core/chain/chain_test.go | 40 ------------------------------------ 1 file changed, 40 deletions(-) diff --git a/pkg/core/chain/chain_test.go b/pkg/core/chain/chain_test.go index a4d715345..5a627b927 100644 --- a/pkg/core/chain/chain_test.go +++ b/pkg/core/chain/chain_test.go @@ -30,7 +30,6 @@ import ( "github.com/dusk-network/dusk-blockchain/pkg/p2p/wire/topics" "github.com/dusk-network/dusk-blockchain/pkg/util/nativeutils/eventbus" "github.com/dusk-network/dusk-blockchain/pkg/util/nativeutils/rpcbus" - "github.com/sirupsen/logrus" assert "github.com/stretchr/testify/require" ) @@ -92,45 +91,6 @@ func mutateFirstChan(propagatedHeight uint64, eb eventbus.Publisher, acceptedBlo return blkMsg2.Payload().(block.Block) } -// This test ensures the correct behavior from the Chain, when -// accepting a block from a peer. -func TestAcceptFromPeer(t *testing.T) { - logrus.SetLevel(logrus.InfoLevel) - - assert := assert.New(t) - - startingHeight := uint64(1) - eb, c := setupChainTest(t, startingHeight) - - streamer := eventbus.NewGossipStreamer() - l := eventbus.NewStreamListener(streamer) - eb.Subscribe(topics.Gossip, l) - - blk := mockAcceptableBlock(*c.tip) - - c.TryNextConsecutiveBlockInSync(*blk, 0) - - // the order of received stuff cannot be guaranteed. So we just search for - // getRoundResult topic. If it hasn't been received the test fails. - // One message should be the block gossip. The other, the round result - for i := 0; i < 2; i++ { - m, err := streamer.Read() - assert.NoError(err) - - if streamer.SeenTopics()[i] == topics.Inv { - // Read hash of the advertised block - var decoder message.Inv - - decoder.Decode(bytes.NewBuffer(m)) - assert.Equal(decoder.InvList[0].Type, message.InvTypeBlock) - assert.True(bytes.Equal(decoder.InvList[0].Hash, blk.Header.Hash)) - return - } - } - - assert.Fail("expected a round result to be received, but it is not in the ringbuffer") -} - // This test ensures the correct behavior when accepting a block // directly from the consensus. func TestAcceptBlock(t *testing.T) {