Merge pull request #1125 from nspcc-dev/fix/consensus

Copy validators when returning to the outside

pkg/consensus/consensus.go

@@ -256,12 +256,12 @@ func (s *service) getKeyPair(pubs []crypto.PublicKey) (int, crypto.PrivateKey, c
 // OnPayload handles Payload receive.
 func (s *service) OnPayload(cp *Payload) {
 	log := s.log.With(zap.Stringer("hash", cp.Hash()))
-	if !s.validatePayload(cp) {
+	if s.cache.Has(cp.Hash()) {
-		log.Debug("can't validate payload")
-		return
-	} else if s.cache.Has(cp.Hash()) {
 		log.Debug("payload is already in cache")
 		return
+	} else if !s.validatePayload(cp) {
+		log.Debug("can't validate payload")
+		return
 	}
 
 	s.Config.Broadcast(cp)

pkg/core/blockchain.go

@@ -1243,9 +1243,7 @@ func (bc *Blockchain) PoolTx(t *transaction.Transaction) error {
 
 //GetStandByValidators returns validators from the configuration.
 func (bc *Blockchain) GetStandByValidators() keys.PublicKeys {
-	res := make(keys.PublicKeys, len(bc.sbValidators))
+	return bc.sbValidators.Copy()
-	copy(res, bc.sbValidators)
-	return res
 }
 
 // GetValidators returns next block validators.

pkg/core/native/native_gas.go

@@ -91,7 +91,7 @@ func (g *GAS) OnPersist(ic *interop.Context) error {
 		absAmount := big.NewInt(int64(tx.SystemFee + tx.NetworkFee))
 		g.burn(ic, tx.Sender, absAmount)
 	}
-	validators, err := g.NEO.GetNextBlockValidatorsInternal(ic.Chain, ic.DAO)
+	validators, err := g.NEO.getNextBlockValidatorsInternal(ic.Chain, ic.DAO)
 	if err != nil {
 		return fmt.Errorf("cannot get block validators: %v", err)
 	}

pkg/core/native/native_neo.go

@@ -77,6 +77,7 @@ func NewNEO() *NEO {
 	nep5.ContractID = neoContractID
 
 	n.nep5TokenNative = *nep5
+	n.validators.Store(keys.PublicKeys(nil))
 
 	onp := n.Methods["onPersist"]
 	onp.Func = getOnPersistWrapper(n.onPersist)
@@ -141,7 +142,7 @@ func (n *NEO) Initialize(ic *interop.Context) error {
 
 // OnPersist implements Contract interface.
 func (n *NEO) OnPersist(ic *interop.Context) error {
-	pubs, err := n.GetValidatorsInternal(ic.Chain, ic.DAO)
+	pubs, err := n.getValidatorsInternal(ic.Chain, ic.DAO)
 	if err != nil {
 		return err
 	}
@@ -333,7 +334,7 @@ func (n *NEO) ModifyAccountVotes(acc *state.NEOBalanceState, d dao.DAO, value *b
 			return err
 		}
 	}
-	n.validators.Store(nil)
+	n.validators.Store(keys.PublicKeys(nil))
 	return nil
 }
 
@@ -384,10 +385,10 @@ func (n *NEO) getRegisteredValidatorsCall(ic *interop.Context, _ []stackitem.Ite
 	return stackitem.NewArray(arr)
 }
 
-// GetValidatorsInternal returns a list of current validators.
+// getValidatorsInternal returns a list of current validators.
-func (n *NEO) GetValidatorsInternal(bc blockchainer.Blockchainer, d dao.DAO) (keys.PublicKeys, error) {
+func (n *NEO) getValidatorsInternal(bc blockchainer.Blockchainer, d dao.DAO) (keys.PublicKeys, error) {
-	if vals := n.validators.Load(); vals != nil {
+	if vals := n.validators.Load().(keys.PublicKeys); vals != nil {
-		return vals.(keys.PublicKeys), nil
+		return vals, nil
 	}
 	standByValidators := bc.GetStandByValidators()
 	si := d.GetStorageItem(n.ContractID, validatorsCountKey)
@@ -441,7 +442,7 @@ func (n *NEO) GetValidatorsInternal(bc blockchainer.Blockchainer, d dao.DAO) (ke
 }
 
 func (n *NEO) getValidators(ic *interop.Context, _ []stackitem.Item) stackitem.Item {
-	result, err := n.GetValidatorsInternal(ic.Chain, ic.DAO)
+	result, err := n.getValidatorsInternal(ic.Chain, ic.DAO)
 	if err != nil {
 		panic(err)
 	}
@@ -449,7 +450,7 @@ func (n *NEO) getValidators(ic *interop.Context, _ []stackitem.Item) stackitem.I
 }
 
 func (n *NEO) getNextBlockValidators(ic *interop.Context, _ []stackitem.Item) stackitem.Item {
-	result, err := n.GetNextBlockValidatorsInternal(ic.Chain, ic.DAO)
+	result, err := n.getNextBlockValidatorsInternal(ic.Chain, ic.DAO)
 	if err != nil {
 		panic(err)
 	}
@@ -458,9 +459,18 @@ func (n *NEO) getNextBlockValidators(ic *interop.Context, _ []stackitem.Item) st
 
 // GetNextBlockValidatorsInternal returns next block validators.
 func (n *NEO) GetNextBlockValidatorsInternal(bc blockchainer.Blockchainer, d dao.DAO) (keys.PublicKeys, error) {
+	pubs, err := n.getNextBlockValidatorsInternal(bc, d)
+	if err != nil {
+		return nil, err
+	}
+	return pubs.Copy(), nil
+}
+
+// getNextBlockValidatorsInternal returns next block validators.
+func (n *NEO) getNextBlockValidatorsInternal(bc blockchainer.Blockchainer, d dao.DAO) (keys.PublicKeys, error) {
 	si := d.GetStorageItem(n.ContractID, nextValidatorsKey)
 	if si == nil {
-		return n.GetValidatorsInternal(bc, d)
+		return n.getValidatorsInternal(bc, d)
 	}
 	pubs := keys.PublicKeys{}
 	err := pubs.DecodeBytes(si.Value)

pkg/crypto/keys/publickey.go

@@ -55,6 +55,13 @@ func (keys PublicKeys) Contains(pKey *PublicKey) bool {
 	return false
 }
 
+// Copy returns copy of keys.
+func (keys PublicKeys) Copy() PublicKeys {
+	res := make(PublicKeys, len(keys))
+	copy(res, keys)
+	return res
+}
+
 // Unique returns set of public keys.
 func (keys PublicKeys) Unique() PublicKeys {
 	unique := PublicKeys{}

pkg/crypto/keys/publickey_test.go

@@ -37,6 +37,23 @@ func TestEncodeDecodePublicKey(t *testing.T) {
 	}
 }
 
+func TestPublicKeys_Copy(t *testing.T) {
+	pubs := make(PublicKeys, 5)
+	for i := range pubs {
+		priv, err := NewPrivateKey()
+		require.NoError(t, err)
+		pubs[i] = priv.PublicKey()
+	}
+
+	cp := pubs.Copy()
+	priv, err := NewPrivateKey()
+	require.NoError(t, err)
+	cp[0] = priv.PublicKey()
+
+	require.NotEqual(t, pubs[0], cp[0])
+	require.Equal(t, pubs[1:], cp[1:])
+}
+
 func TestNewPublicKeyFromBytes(t *testing.T) {
 	priv, err := NewPrivateKey()
 	require.NoError(t, err)

pkg/smartcontract/context/context_test.go

@@ -70,8 +70,7 @@ func TestParameterContext_AddSignatureMultisig(t *testing.T) {
 	tx := getContractTx()
 	c := NewParameterContext("Neo.Core.ContractTransaction", tx)
 	privs, pubs := getPrivateKeys(t, 4)
-	pubsCopy := make(keys.PublicKeys, len(pubs))
+	pubsCopy := keys.PublicKeys(pubs).Copy()
-	copy(pubsCopy, pubs)
 	script, err := smartcontract.CreateMultiSigRedeemScript(3, pubsCopy)
 	require.NoError(t, err)