From f67b8ce6077588ebba8f1ed66a2287225c461b5b Mon Sep 17 00:00:00 2001 From: Evgenii Stratonikov Date: Mon, 29 Jun 2020 10:21:44 +0300 Subject: [PATCH 1/4] consensus: check if payload is present in cache before validation Don't check signature correctness twice. --- pkg/consensus/consensus.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/consensus/consensus.go b/pkg/consensus/consensus.go index 0cff84edc..bb3a61bf9 100644 --- a/pkg/consensus/consensus.go +++ b/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) { - log.Debug("can't validate payload") - return - } else if s.cache.Has(cp.Hash()) { + 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) From bbe02ac584375aa227882b0c091be7308e2f1cff Mon Sep 17 00:00:00 2001 From: Evgenii Stratonikov Date: Mon, 29 Jun 2020 10:36:44 +0300 Subject: [PATCH 2/4] native: store typed `nil` in validators cache Fix `error encountered at instruction 0 (SYSCALL): sync/atomic: store of nil value into Value"`. --- pkg/core/native/native_neo.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/pkg/core/native/native_neo.go b/pkg/core/native/native_neo.go index f42fa4286..677c47266 100644 --- a/pkg/core/native/native_neo.go +++ b/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) @@ -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 } @@ -386,8 +387,8 @@ func (n *NEO) getRegisteredValidatorsCall(ic *interop.Context, _ []stackitem.Ite // GetValidatorsInternal returns a list of current validators. func (n *NEO) GetValidatorsInternal(bc blockchainer.Blockchainer, d dao.DAO) (keys.PublicKeys, error) { - if vals := n.validators.Load(); vals != nil { - return vals.(keys.PublicKeys), nil + if vals := n.validators.Load().(keys.PublicKeys); vals != nil { + return vals, nil } standByValidators := bc.GetStandByValidators() si := d.GetStorageItem(n.ContractID, validatorsCountKey) From 8c18142e8a2bf5fb2428c768be93792784ba3929 Mon Sep 17 00:00:00 2001 From: Evgenii Stratonikov Date: Mon, 29 Jun 2020 10:44:31 +0300 Subject: [PATCH 3/4] keys: implement `PublicKeys.Copy()` Implement convenient wrapper over explicit allocation and copying. --- pkg/core/blockchain.go | 4 +--- pkg/crypto/keys/publickey.go | 7 +++++++ pkg/crypto/keys/publickey_test.go | 17 +++++++++++++++++ pkg/smartcontract/context/context_test.go | 3 +-- 4 files changed, 26 insertions(+), 5 deletions(-) diff --git a/pkg/core/blockchain.go b/pkg/core/blockchain.go index 56123da5e..bb29bd349 100644 --- a/pkg/core/blockchain.go +++ b/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)) - copy(res, bc.sbValidators) - return res + return bc.sbValidators.Copy() } // GetValidators returns next block validators. diff --git a/pkg/crypto/keys/publickey.go b/pkg/crypto/keys/publickey.go index d03bdb22b..1d1c0103a 100644 --- a/pkg/crypto/keys/publickey.go +++ b/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{} diff --git a/pkg/crypto/keys/publickey_test.go b/pkg/crypto/keys/publickey_test.go index 3ec900784..a2aa2060b 100644 --- a/pkg/crypto/keys/publickey_test.go +++ b/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) diff --git a/pkg/smartcontract/context/context_test.go b/pkg/smartcontract/context/context_test.go index 30271ac6d..1971ea88a 100644 --- a/pkg/smartcontract/context/context_test.go +++ b/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)) - copy(pubsCopy, pubs) + pubsCopy := keys.PublicKeys(pubs).Copy() script, err := smartcontract.CreateMultiSigRedeemScript(3, pubsCopy) require.NoError(t, err) From f007cca80b13e2d3bb8977688b59ead415e9cd1a Mon Sep 17 00:00:00 2001 From: Evgenii Stratonikov Date: Mon, 29 Jun 2020 10:48:35 +0300 Subject: [PATCH 4/4] native: hide native contract methods Copy public keys when returning them to the outside and hide unused `GetValidators` method. --- pkg/core/native/native_gas.go | 2 +- pkg/core/native/native_neo.go | 21 +++++++++++++++------ 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/pkg/core/native/native_gas.go b/pkg/core/native/native_gas.go index 08683863a..dcc17d27f 100644 --- a/pkg/core/native/native_gas.go +++ b/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) } diff --git a/pkg/core/native/native_neo.go b/pkg/core/native/native_neo.go index 677c47266..73388bbdd 100644 --- a/pkg/core/native/native_neo.go +++ b/pkg/core/native/native_neo.go @@ -142,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 } @@ -385,8 +385,8 @@ func (n *NEO) getRegisteredValidatorsCall(ic *interop.Context, _ []stackitem.Ite return stackitem.NewArray(arr) } -// GetValidatorsInternal returns a list of current validators. -func (n *NEO) GetValidatorsInternal(bc blockchainer.Blockchainer, d dao.DAO) (keys.PublicKeys, error) { +// getValidatorsInternal returns a list of current validators. +func (n *NEO) getValidatorsInternal(bc blockchainer.Blockchainer, d dao.DAO) (keys.PublicKeys, error) { if vals := n.validators.Load().(keys.PublicKeys); vals != nil { return vals, nil } @@ -442,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) } @@ -450,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) } @@ -459,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)