fix: fixing some minor issues on pre-testnet (#719)

cmd/cmd.go

@@ -401,7 +401,11 @@ func StartNode(workingDir string, passwordFetcher func(*wallet.Wallet) (string,
 	}
 	validatorAddrs := make([]string, conf.Node.NumValidators)
 	for i := 0; i < conf.Node.NumValidators; i++ {
-		validatorAddrs[i] = addrLabels[i].Address
+		valAddr, _ := crypto.AddressFromString(addrLabels[i].Address)
+		if !valAddr.IsValidatorAddress() {
+			return nil, nil, fmt.Errorf("invalid validator address: %s", addrLabels[i].Address)
+		}
+		validatorAddrs[i] = valAddr.String()
 	}
 	valKeys := make([]*bls.ValidatorKey, conf.Node.NumValidators)
 	password, ok := passwordFetcher(walletInstance)
@@ -417,19 +421,26 @@ func StartNode(workingDir string, passwordFetcher func(*wallet.Wallet) (string,
 	}
 
 	// Create reward addresses
-	rewardAddrs := make([]crypto.Address, conf.Node.NumValidators)
+	rewardAddrs := make([]crypto.Address, 0, conf.Node.NumValidators)
 	if len(conf.Node.RewardAddresses) != 0 {
-		for i, addrStr := range conf.Node.RewardAddresses {
-			rewardAddrs[i], _ = crypto.AddressFromString(addrStr)
+		for _, addrStr := range conf.Node.RewardAddresses {
+			addr, _ := crypto.AddressFromString(addrStr)
+			rewardAddrs = append(rewardAddrs, addr)
 		}
 	} else {
-		if len(addrLabels) < 2*conf.Node.NumValidators {
-			return nil, nil, fmt.Errorf("not enough addresses in wallet")
-		}
-		for i := 0; i < conf.Node.NumValidators; i++ {
-			rewardAddrs[i], _ = crypto.AddressFromString(addrLabels[conf.Node.NumValidators+i].Address)
+		for i := conf.Node.NumValidators; i < len(addrLabels); i++ {
+			addr, _ := crypto.AddressFromString(addrLabels[i].Address)
+			if addr.IsAccountAddress() {
+				rewardAddrs = append(rewardAddrs, addr)
+				if len(rewardAddrs) == conf.Node.NumValidators {
+					break
+				}
+			}
 		}
 	}
+	if len(rewardAddrs) != conf.Node.NumValidators {
+		return nil, nil, fmt.Errorf("not enough addresses in wallet")
+	}
 
 	nodeInstance, err := node.NewNode(gen, conf, valKeys, rewardAddrs)
 	if err != nil {

config/config.go

@@ -55,15 +55,20 @@ func (conf *NodeConfig) BasicCheck() error {
 		return errors.Errorf(errors.ErrInvalidConfig, "number of validators must be between 1 and 32")
 	}
 
-	if len(conf.RewardAddresses) != conf.NumValidators {
+	if len(conf.RewardAddresses) > 0 &&
+		len(conf.RewardAddresses) != conf.NumValidators {
 		return errors.Errorf(errors.ErrInvalidConfig, "reward addresses should be %v", conf.NumValidators)
 	}
 
-	for _, addr := range conf.RewardAddresses {
-		_, err := crypto.AddressFromString(addr)
+	for _, addrStr := range conf.RewardAddresses {
+		addr, err := crypto.AddressFromString(addrStr)
 		if err != nil {
 			return errors.Errorf(errors.ErrInvalidConfig, "invalid reward address: %v", err.Error())
 		}
+
+		if !addr.IsAccountAddress() {
+			return errors.Errorf(errors.ErrInvalidConfig, "reward address is not an account address: %s", addrStr)
+		}
 	}
 	return nil
 }
@@ -118,8 +123,8 @@ func SaveTestnetConfig(path string, numValidators int) error {
 	conf.Network.Bootstrap.MaxThreshold = 8
 	conf.Network.EnableRelay = true
 	conf.Network.RelayAddrs = []string{
-		"/ip4/139.162.153.10/udp/4002/quic/p2p/12D3KooWNR79jqHVVNhNVrqnDbxbJJze4VjbEsBjZhz6mkvinHAN",
 		"/ip4/139.162.153.10/tcp/4002/p2p/12D3KooWNR79jqHVVNhNVrqnDbxbJJze4VjbEsBjZhz6mkvinHAN",
+		"/ip4/139.162.153.10/udp/4002/quic/p2p/12D3KooWNR79jqHVVNhNVrqnDbxbJJze4VjbEsBjZhz6mkvinHAN",
 		"/ip6/2a01:7e01::f03c:93ff:fed2:84c5/tcp/4002/p2p/12D3KooWNR79jqHVVNhNVrqnDbxbJJze4VjbEsBjZhz6mkvinHAN",
 		"/ip6/2a01:7e01::f03c:93ff:fed2:84c5/udp/4002/quic/p2p/12D3KooWNR79jqHVVNhNVrqnDbxbJJze4VjbEsBjZhz6mkvinHAN",
 		"/ip4/94.101.184.118/tcp/4002/p2p/12D3KooWCRHn8vjrKNBEQcut8uVCYX5q77RKidPaE6iMK31qEVHb",
@@ -131,7 +136,7 @@ func SaveTestnetConfig(path string, numValidators int) error {
 	conf.GRPC.Gateway.Listen = "[::]:8080"
 	conf.HTTP.Enable = false
 	conf.HTTP.Listen = "[::]:80"
-	conf.Nanomsg.Enable = true
+	conf.Nanomsg.Enable = false
 	conf.Nanomsg.Listen = "tcp://127.0.0.1:40799"
 
 	return util.WriteFile(path, conf.toTOML())
@@ -187,6 +192,9 @@ func LoadFromFile(file string, strict bool) (*Config, error) {
 
 // BasicCheck performs basic checks on the configuration.
 func (conf *Config) BasicCheck() error {
+	if err := conf.Node.BasicCheck(); err != nil {
+		return err
+	}
 	if err := conf.Store.BasicCheck(); err != nil {
 		return err
 	}

config/config_test.go

@@ -111,6 +111,16 @@ func TestNodeConfigBasicCheck(t *testing.T) {
 		assert.Error(t, conf.BasicCheck())
 	})
 
+	t.Run("validator address as reward address", func(t *testing.T) {
+		conf := DefaultNodeConfig()
+		conf.NumValidators = 1
+		conf.RewardAddresses = []string{
+			ts.RandValAddress().String(),
+		}
+
+		assert.Error(t, conf.BasicCheck())
+	})
+
 	t.Run("ok", func(t *testing.T) {
 		conf := DefaultNodeConfig()
 		conf.NumValidators = 2
@@ -121,4 +131,12 @@ func TestNodeConfigBasicCheck(t *testing.T) {
 
 		assert.NoError(t, conf.BasicCheck())
 	})
+
+	t.Run("no reward addresses inside config, Ok", func(t *testing.T) {
+		conf := DefaultNodeConfig()
+		conf.NumValidators = 2
+		conf.RewardAddresses = []string{}
+
+		assert.NoError(t, conf.BasicCheck())
+	})
 }

sync/firewall/firewall.go

@@ -48,7 +48,8 @@ func (f *Firewall) OpenGossipBundle(data []byte, source peer.ID, from peer.ID) *
 
 	bdl, err := f.openBundle(bytes.NewReader(data), source)
 	if err != nil {
-		f.logger.Warn("firewall: unable to open a gossip bundle", "error", err)
+		f.logger.Warn("firewall: unable to open a gossip bundle",
+			"error", err, "bundle", bdl)
 		f.closeConnection(from)
 		return nil
 	}
@@ -62,7 +63,8 @@ func (f *Firewall) OpenGossipBundle(data []byte, source peer.ID, from peer.ID) *
 func (f *Firewall) OpenStreamBundle(r io.Reader, from peer.ID) *bundle.Bundle {
 	bdl, err := f.openBundle(r, from)
 	if err != nil {
-		f.logger.Warn("firewall: unable to open a stream bundle", "error", err)
+		f.logger.Warn("firewall: unable to open a stream bundle",
+			"error", err, "bundle", bdl)
 		f.closeConnection(from)
 		return nil
 	}
@@ -91,7 +93,7 @@ func (f *Firewall) openBundle(r io.Reader, source peer.ID) (*bundle.Bundle, erro
 
 	if err := f.checkBundle(bdl, source); err != nil {
 		f.peerSet.IncreaseInvalidBundlesCounter(source)
-		return nil, err
+		return bdl, err
 	}
 
 	return bdl, nil

sync/firewall/firewall_test.go

@@ -112,6 +112,16 @@ func TestGossipMessage(t *testing.T) {
 		assert.True(t, td.network.IsClosed(td.badPeerID))
 	})
 
+	t.Run("Message is nil => should close the connection", func(t *testing.T) {
+		td := setup(t)
+
+		assert.False(t, td.network.IsClosed(td.badPeerID))
+		assert.False(t, td.network.IsClosed(td.unknownPeerID))
+		assert.Nil(t, td.firewall.OpenGossipBundle(nil, td.badPeerID, td.unknownPeerID))
+		assert.True(t, td.network.IsClosed(td.badPeerID))
+		assert.True(t, td.network.IsClosed(td.unknownPeerID))
+	})
+
 	t.Run("Message source: bad, from: unknown => should close the connection", func(t *testing.T) {
 		td := setup(t)
 
@@ -151,6 +161,14 @@ func TestGossipMessage(t *testing.T) {
 }
 
 func TestStreamMessage(t *testing.T) {
+	t.Run("Message is nil => should close the connection", func(t *testing.T) {
+		td := setup(t)
+
+		assert.False(t, td.network.IsClosed(td.badPeerID))
+		assert.Nil(t, td.firewall.OpenStreamBundle(bytes.NewReader(nil), td.badPeerID))
+		assert.True(t, td.network.IsClosed(td.badPeerID))
+	})
+
 	t.Run("Message source: bad => should close the connection", func(t *testing.T) {
 		td := setup(t)
 

sync/handler_blocks_request.go

@@ -95,8 +95,9 @@ func (handler *blocksRequestHandler) PrepareBundle(m message.Message) *bundle.Bu
 
 func (handler *blocksRequestHandler) respond(msg *message.BlocksResponseMessage, to peer.ID) error {
 	if msg.ResponseCode == message.ResponseCodeRejected {
-		handler.logger.Warn("rejecting block request message", "message", msg,
+		handler.logger.Error("rejecting block request message", "message", msg,
 			"to", to.ShortString(), "reason", msg.Reason)
+		handler.network.CloseConnection(to)
 	} else {
 		handler.logger.Info("responding block request message", "message", msg,
 			"to", to.ShortString())

types/block/block_test.go

@@ -45,23 +45,24 @@ func TestBasicCheck(t *testing.T) {
 
 		err := b.BasicCheck()
 		assert.ErrorIs(t, err, block.BasicCheckError{
-			Reason: "invalid certificate: certificate basic check failed: height is not positive: 0",
+			Reason: "invalid certificate: height is not positive: 0",
 		})
 	})
 
-	//TODO fix me later
-	//t.Run("Invalid transaction", func(t *testing.T) {
-	//	b0 := ts.GenerateTestBlock()
-	//	trxs0 := b0.Transactions()
-	//	invalidValKey := ts.RandValKey()
-	//	invalidValKey.Sign(trxs0[0].SignBytes())
-	//	b := block.NewBlock(b0.Header(), b0.PrevCertificate(), trxs0)
-	//
-	//	err := b.BasicCheck()
-	//	assert.ErrorIs(t, err, block.BasicCheckError{
-	//		Reason: "invalid transaction: transaction basic check failed: invalid address: invalid address",
-	//	})
-	//})
+	t.Run("Invalid transaction", func(t *testing.T) {
+		b0 := ts.GenerateTestBlock()
+		trxs0 := b0.Transactions()
+		invalidValKey := ts.RandValKey()
+		invSig := invalidValKey.Sign(trxs0[0].SignBytes())
+		trxs0[0].SetSignature(invSig)
+
+		b := block.NewBlock(b0.Header(), b0.PrevCertificate(), trxs0)
+
+		err := b.BasicCheck()
+		assert.ErrorIs(t, err, block.BasicCheckError{
+			Reason: "invalid transaction: invalid signature",
+		})
+	})
 
 	t.Run("Invalid state root hash", func(t *testing.T) {
 		d := ts.DecodingHex(

types/block/errors.go

@@ -6,5 +6,5 @@ type BasicCheckError struct {
 }
 
 func (e BasicCheckError) Error() string {
-	return "block basic check failed: " + e.Reason
+	return e.Reason
 }

types/certificate/errors.go

@@ -13,7 +13,7 @@ type BasicCheckError struct {
 }
 
 func (e BasicCheckError) Error() string {
-	return "certificate basic check failed: " + e.Reason
+	return e.Reason
 }
 
 // UnexpectedHeightError is returned when the height of the certificate

types/tx/errors.go

@@ -8,7 +8,7 @@ type BasicCheckError struct {
 }
 
 func (e BasicCheckError) Error() string {
-	return "transaction basic check failed: " + e.Reason
+	return e.Reason
 }
 
 // InvalidPayloadTypeError is returned when the payload type is not valid.

types/tx/payload/bond.go

@@ -31,13 +31,13 @@ func (p *BondPayload) Value() int64 {
 func (p *BondPayload) BasicCheck() error {
 	if !p.From.IsAccountAddress() {
 		return BasicCheckError{
-			Reason: "sender is not an account address",
+			Reason: "sender is not an account address: " + p.From.ShortString(),
 		}
 	}
 
 	if !p.To.IsValidatorAddress() {
 		return BasicCheckError{
-			Reason: "receiver is not a validator address",
+			Reason: "receiver is not a validator address: " + p.To.ShortString(),
 		}
 	}
 

types/tx/payload/bond_test.go

@@ -93,7 +93,7 @@ func TestBondDecoding(t *testing.T) {
 			value:   0x200000,
 			readErr: nil,
 			basicErr: BasicCheckError{
-				Reason: "sender is not an account address",
+				Reason: "sender is not an account address: pc1pqgpsgpgx",
 			},
 		},
 		{
@@ -110,7 +110,7 @@ func TestBondDecoding(t *testing.T) {
 			value:   0x200000,
 			readErr: nil,
 			basicErr: BasicCheckError{
-				Reason: "receiver is not a validator address",
+				Reason: "receiver is not a validator address: pc1zzgf3g9gk",
 			},
 		},
 		{

types/tx/payload/errors.go

@@ -11,5 +11,5 @@ type BasicCheckError struct {
 }
 
 func (e BasicCheckError) Error() string {
-	return "payload basic check failed: " + e.Reason
+	return e.Reason
 }

types/tx/payload/sortition.go

@@ -29,7 +29,7 @@ func (p *SortitionPayload) Value() int64 {
 func (p *SortitionPayload) BasicCheck() error {
 	if !p.Validator.IsValidatorAddress() {
 		return BasicCheckError{
-			Reason: "address is not a validator address",
+			Reason: "address is not a validator address: " + p.Validator.ShortString(),
 		}
 	}
 

types/tx/payload/sortition_test.go

@@ -84,7 +84,7 @@ func TestSortitionDecoding(t *testing.T) {
 			value:   0,
 			readErr: nil,
 			basicErr: BasicCheckError{
-				Reason: "address is not a validator address",
+				Reason: "address is not a validator address: pc1zqgpsgpgx",
 			},
 		},
 	}

types/tx/payload/transfer.go

@@ -29,12 +29,12 @@ func (p *TransferPayload) Value() int64 {
 func (p *TransferPayload) BasicCheck() error {
 	if !p.From.IsAccountAddress() {
 		return BasicCheckError{
-			Reason: "sender is not an account address",
+			Reason: "sender is not an account address: " + p.From.ShortString(),
 		}
 	}
 	if !p.To.IsAccountAddress() {
 		return BasicCheckError{
-			Reason: "receiver is not an account address",
+			Reason: "receiver is not an account address: " + p.To.ShortString(),
 		}
 	}
 	return nil

types/tx/payload/transfer_test.go

@@ -73,7 +73,7 @@ func TestTransferDecoding(t *testing.T) {
 			value:   0x200000,
 			readErr: nil,
 			basicErr: BasicCheckError{
-				Reason: "sender is not an account address",
+				Reason: "sender is not an account address: pc1pqgpsgpgx",
 			},
 		},
 		{
@@ -89,7 +89,7 @@ func TestTransferDecoding(t *testing.T) {
 			value:   0x200000,
 			readErr: nil,
 			basicErr: BasicCheckError{
-				Reason: "receiver is not an account address",
+				Reason: "receiver is not an account address: pc1pzgf3g9gk",
 			},
 		},
 		{

types/tx/payload/withdraw.go

@@ -30,12 +30,12 @@ func (p *WithdrawPayload) Value() int64 {
 func (p *WithdrawPayload) BasicCheck() error {
 	if !p.From.IsValidatorAddress() {
 		return BasicCheckError{
-			Reason: "sender is not a validator address",
+			Reason: "sender is not a validator address: " + p.From.ShortString(),
 		}
 	}
 	if !p.To.IsAccountAddress() {
 		return BasicCheckError{
-			Reason: "receiver is not an account address",
+			Reason: "receiver is not an account address: " + p.To.ShortString(),
 		}
 	}
 

types/tx/tx.go

@@ -189,7 +189,7 @@ func (tx *Tx) checkSignature() error {
 		bs := tx.SignBytes()
 		if err := tx.PublicKey().Verify(bs, tx.Signature()); err != nil {
 			return BasicCheckError{
-				Reason: fmt.Sprintf("invalid signature: %s", tx.Signature().String()),
+				Reason: "invalid signature",
 			}
 		}
 	}