From 26339c75dc00819e12b5e7677ba1b1fde8f264e4 Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Thu, 10 Sep 2020 14:43:24 +0300 Subject: [PATCH] vm/core: drop old key caching system Obsoleted by f5f58a7e91774fad2ce2c65d0f288478d4bb3a4b. --- pkg/core/blockchain.go | 41 +++++++------------------------------ pkg/core/blockchain_test.go | 16 +++++++-------- pkg/vm/vm.go | 39 ++++++++--------------------------- pkg/vm/vm_test.go | 20 ------------------ 4 files changed, 23 insertions(+), 93 deletions(-) diff --git a/pkg/core/blockchain.go b/pkg/core/blockchain.go index 6c577c0de..cb2cb67fc 100644 --- a/pkg/core/blockchain.go +++ b/pkg/core/blockchain.go @@ -115,11 +115,6 @@ type Blockchain struct { memPool *mempool.Pool - // This lock protects concurrent access to keyCache. - keyCacheLock sync.RWMutex - // cache for block verification keys. - keyCache map[util.Uint160]map[string]*keys.PublicKey - sbCommittee keys.PublicKeys log *zap.Logger @@ -169,7 +164,6 @@ func NewBlockchain(s storage.Store, cfg config.ProtocolConfiguration, log *zap.L stopCh: make(chan struct{}), runToExitCh: make(chan struct{}), memPool: mempool.New(cfg.MemPoolSize), - keyCache: make(map[util.Uint160]map[string]*keys.PublicKey), sbCommittee: committee, log: log, events: make(chan bcEvent), @@ -771,8 +765,8 @@ func (bc *Blockchain) processNEP5Transfer(cache *dao.Cached, h util.Uint256, b * if nativeContract != nil { id = nativeContract.Metadata().ContractID } else { - assetContract := bc.GetContractState(sc) - if assetContract == nil { + assetContract, err := cache.GetContractState(sc) + if err != nil { return } id = assetContract.ID @@ -1482,7 +1476,7 @@ var ( ) // initVerificationVM initializes VM for witness check. -func initVerificationVM(ic *interop.Context, hash util.Uint160, witness *transaction.Witness, keyCache map[string]*keys.PublicKey) error { +func initVerificationVM(ic *interop.Context, hash util.Uint160, witness *transaction.Witness) error { var offset int var initMD *manifest.Method verification := witness.VerificationScript @@ -1511,9 +1505,6 @@ func initVerificationVM(ic *interop.Context, hash util.Uint160, witness *transac v.Call(v.Context(), initMD.Offset) } v.LoadScript(witness.InvocationScript) - if keyCache != nil { - v.SetPublicKeys(keyCache) - } return nil } @@ -1521,11 +1512,11 @@ func initVerificationVM(ic *interop.Context, hash util.Uint160, witness *transac func (bc *Blockchain) VerifyWitness(h util.Uint160, c crypto.Verifiable, w *transaction.Witness, gas int64) error { ic := bc.newInteropContext(trigger.Verification, bc.dao, nil, nil) ic.Container = c - return bc.verifyHashAgainstScript(h, w, ic, true, gas) + return bc.verifyHashAgainstScript(h, w, ic, gas) } // verifyHashAgainstScript verifies given hash against the given witness. -func (bc *Blockchain) verifyHashAgainstScript(hash util.Uint160, witness *transaction.Witness, interopCtx *interop.Context, useKeys bool, gas int64) error { +func (bc *Blockchain) verifyHashAgainstScript(hash util.Uint160, witness *transaction.Witness, interopCtx *interop.Context, gas int64) error { gasPolicy := bc.contracts.Policy.GetMaxVerificationGas(interopCtx.DAO) if gas > gasPolicy { gas = gasPolicy @@ -1534,15 +1525,7 @@ func (bc *Blockchain) verifyHashAgainstScript(hash util.Uint160, witness *transa vm := interopCtx.SpawnVM() vm.SetPriceGetter(getPrice) vm.GasLimit = gas - var keyCache map[string]*keys.PublicKey - if useKeys { - bc.keyCacheLock.RLock() - if bc.keyCache[hash] != nil { - keyCache = bc.keyCache[hash] - } - bc.keyCacheLock.RUnlock() - } - if err := initVerificationVM(interopCtx, hash, witness, keyCache); err != nil { + if err := initVerificationVM(interopCtx, hash, witness); err != nil { return err } err := vm.Run() @@ -1561,16 +1544,6 @@ func (bc *Blockchain) verifyHashAgainstScript(hash util.Uint160, witness *transa if vm.Estack().Len() != 0 { return fmt.Errorf("%w: expected exactly one returned value", ErrVerificationFailed) } - if useKeys { - bc.keyCacheLock.RLock() - _, ok := bc.keyCache[hash] - bc.keyCacheLock.RUnlock() - if !ok { - bc.keyCacheLock.Lock() - bc.keyCache[hash] = vm.GetPublicKeys() - bc.keyCacheLock.Unlock() - } - } } else { return fmt.Errorf("%w: no result returned from the script", ErrVerificationFailed) } @@ -1589,7 +1562,7 @@ func (bc *Blockchain) verifyTxWitnesses(t *transaction.Transaction, block *block } interopCtx := bc.newInteropContext(trigger.Verification, bc.dao, block, t) for i := range t.Signers { - err := bc.verifyHashAgainstScript(t.Signers[i].Account, &t.Scripts[i], interopCtx, false, t.NetworkFee) + err := bc.verifyHashAgainstScript(t.Signers[i].Account, &t.Scripts[i], interopCtx, t.NetworkFee) if err != nil { return fmt.Errorf("witness #%d: %w", i, err) } diff --git a/pkg/core/blockchain_test.go b/pkg/core/blockchain_test.go index bf56275a6..bbf3c6351 100644 --- a/pkg/core/blockchain_test.go +++ b/pkg/core/blockchain_test.go @@ -394,22 +394,22 @@ func TestVerifyHashAgainstScript(t *testing.T) { newH := cs.ScriptHash() newH[0] = ^newH[0] w := &transaction.Witness{InvocationScript: []byte{byte(opcode.PUSH4)}} - err := bc.verifyHashAgainstScript(newH, w, ic, false, gas) + err := bc.verifyHashAgainstScript(newH, w, ic, gas) require.True(t, errors.Is(err, ErrUnknownVerificationContract)) }) t.Run("Invalid", func(t *testing.T) { w := &transaction.Witness{InvocationScript: []byte{byte(opcode.PUSH4)}} - err := bc.verifyHashAgainstScript(csInvalid.ScriptHash(), w, ic, false, gas) + err := bc.verifyHashAgainstScript(csInvalid.ScriptHash(), w, ic, gas) require.True(t, errors.Is(err, ErrInvalidVerificationContract)) }) t.Run("ValidSignature", func(t *testing.T) { w := &transaction.Witness{InvocationScript: []byte{byte(opcode.PUSH4)}} - err := bc.verifyHashAgainstScript(cs.ScriptHash(), w, ic, false, gas) + err := bc.verifyHashAgainstScript(cs.ScriptHash(), w, ic, gas) require.NoError(t, err) }) t.Run("InvalidSignature", func(t *testing.T) { w := &transaction.Witness{InvocationScript: []byte{byte(opcode.PUSH3)}} - err := bc.verifyHashAgainstScript(cs.ScriptHash(), w, ic, false, gas) + err := bc.verifyHashAgainstScript(cs.ScriptHash(), w, ic, gas) require.True(t, errors.Is(err, ErrVerificationFailed)) }) }) @@ -419,7 +419,7 @@ func TestVerifyHashAgainstScript(t *testing.T) { InvocationScript: []byte{byte(opcode.NOP)}, VerificationScript: verif, } - err := bc.verifyHashAgainstScript(hash.Hash160(verif), w, ic, false, 1) + err := bc.verifyHashAgainstScript(hash.Hash160(verif), w, ic, 1) require.True(t, errors.Is(err, ErrVerificationFailed)) }) t.Run("NoResult", func(t *testing.T) { @@ -428,7 +428,7 @@ func TestVerifyHashAgainstScript(t *testing.T) { InvocationScript: []byte{byte(opcode.PUSH1)}, VerificationScript: verif, } - err := bc.verifyHashAgainstScript(hash.Hash160(verif), w, ic, false, gas) + err := bc.verifyHashAgainstScript(hash.Hash160(verif), w, ic, gas) require.True(t, errors.Is(err, ErrVerificationFailed)) }) t.Run("BadResult", func(t *testing.T) { @@ -439,7 +439,7 @@ func TestVerifyHashAgainstScript(t *testing.T) { InvocationScript: []byte{byte(opcode.NOP)}, VerificationScript: verif, } - err := bc.verifyHashAgainstScript(hash.Hash160(verif), w, ic, false, gas) + err := bc.verifyHashAgainstScript(hash.Hash160(verif), w, ic, gas) require.True(t, errors.Is(err, ErrVerificationFailed)) }) t.Run("TooManyResults", func(t *testing.T) { @@ -448,7 +448,7 @@ func TestVerifyHashAgainstScript(t *testing.T) { InvocationScript: []byte{byte(opcode.PUSH1), byte(opcode.PUSH1)}, VerificationScript: verif, } - err := bc.verifyHashAgainstScript(hash.Hash160(verif), w, ic, false, gas) + err := bc.verifyHashAgainstScript(hash.Hash160(verif), w, ic, gas) require.True(t, errors.Is(err, ErrVerificationFailed)) }) } diff --git a/pkg/vm/vm.go b/pkg/vm/vm.go index 0a426fb6a..17d6521e6 100644 --- a/pkg/vm/vm.go +++ b/pkg/vm/vm.go @@ -80,9 +80,6 @@ type VM struct { SyscallHandler func(v *VM, id uint32) error trigger trigger.Type - - // Public keys cache. - keys map[string]*keys.PublicKey } // New returns a new VM object ready to load AVM bytecode scripts. @@ -96,7 +93,6 @@ func NewWithTrigger(t trigger.Type) *VM { state: NoneState, istack: NewStack("invocation"), refs: newRefCounter(), - keys: make(map[string]*keys.PublicKey), trigger: t, SyscallHandler: defaultSyscallHandler, @@ -140,17 +136,6 @@ func (v *VM) Istack() *Stack { return v.istack } -// SetPublicKeys sets internal key cache to the specified value (note -// that it doesn't copy them). -func (v *VM) SetPublicKeys(keys map[string]*keys.PublicKey) { - v.keys = keys -} - -// GetPublicKeys returns internal key cache (note that it doesn't copy it). -func (v *VM) GetPublicKeys() map[string]*keys.PublicKey { - return v.keys -} - // LoadArgs loads in the arguments used in the Mian entry point. func (v *VM) LoadArgs(method []byte, args []stackitem.Item) { if len(args) > 0 { @@ -1586,8 +1571,8 @@ func CheckMultisigPar(v *VM, curve elliptic.Curve, h []byte, pkeys [][]byte, sig go worker(tasks, results) } - tasks <- task{pub: v.bytesToPublicKey(pkeys[k1], curve), signum: s1} - tasks <- task{pub: v.bytesToPublicKey(pkeys[k2], curve), signum: s2} + tasks <- task{pub: bytesToPublicKey(pkeys[k1], curve), signum: s1} + tasks <- task{pub: bytesToPublicKey(pkeys[k2], curve), signum: s2} sigok := true taskCount := 2 @@ -1631,7 +1616,7 @@ loop: nextKey = k2 } taskCount++ - tasks <- task{pub: v.bytesToPublicKey(pkeys[nextKey], curve), signum: nextSig} + tasks <- task{pub: bytesToPublicKey(pkeys[nextKey], curve), signum: nextSig} } close(tasks) @@ -1641,7 +1626,7 @@ loop: func checkMultisig1(v *VM, curve elliptic.Curve, h []byte, pkeys [][]byte, sig []byte) bool { for i := range pkeys { - pkey := v.bytesToPublicKey(pkeys[i], curve) + pkey := bytesToPublicKey(pkeys[i], curve) if pkey.Verify(sig, h) { return true } @@ -1696,18 +1681,10 @@ func (v *VM) checkInvocationStackSize() { // bytesToPublicKey is a helper deserializing keys using cache and panicing on // error. -func (v *VM) bytesToPublicKey(b []byte, curve elliptic.Curve) *keys.PublicKey { - var pkey *keys.PublicKey - s := string(b) - if v.keys[s] != nil { - pkey = v.keys[s] - } else { - var err error - pkey, err = keys.NewPublicKeyFromBytes(b, curve) - if err != nil { - panic(err.Error()) - } - v.keys[s] = pkey +func bytesToPublicKey(b []byte, curve elliptic.Curve) *keys.PublicKey { + pkey, err := keys.NewPublicKeyFromBytes(b, curve) + if err != nil { + panic(err.Error()) } return pkey } diff --git a/pkg/vm/vm_test.go b/pkg/vm/vm_test.go index 198d2183e..c32424ec2 100644 --- a/pkg/vm/vm_test.go +++ b/pkg/vm/vm_test.go @@ -2,7 +2,6 @@ package vm import ( "bytes" - "crypto/elliptic" "encoding/binary" "encoding/hex" "errors" @@ -103,25 +102,6 @@ func TestAddGas(t *testing.T) { require.False(t, v.AddGas(5)) } -func TestBytesToPublicKey(t *testing.T) { - v := newTestVM() - cache := v.GetPublicKeys() - assert.Equal(t, 0, len(cache)) - keyHex := "03b209fd4f53a7170ea4444e0cb0a6bb6a53c2bd016926989cf85f9b0fba17a70c" - keyBytes, _ := hex.DecodeString(keyHex) - key := v.bytesToPublicKey(keyBytes, elliptic.P256()) - assert.NotNil(t, key) - key2 := v.bytesToPublicKey(keyBytes, elliptic.P256()) - assert.Equal(t, key, key2) - - cache = v.GetPublicKeys() - assert.Equal(t, 1, len(cache)) - assert.NotNil(t, cache[string(keyBytes)]) - - keyBytes[0] = 0xff - require.Panics(t, func() { v.bytesToPublicKey(keyBytes, elliptic.P256()) }) -} - func TestPushBytes1to75(t *testing.T) { buf := io.NewBufBinWriter() for i := 1; i <= 75; i++ {